Conversation
Общий вывод по проектуПроект представляет собой добротную базу для веб-приложения на сервлетах. Видно уверенное владение инструментами тестирования (JUnit 5, Mockito) и понимание жизненного цикла веб-приложения (использование ServletContextListener, init методов). Код структурирован и легко читается. Основные рекомендации: Безопасность: Реализовать хеширование паролей и защитить эндпоинты через фильтры (Filters), а не ручными проверками в каждом сервлете (это мы еще изучим) Архитектура: Внедрить Service-слой между контроллерами (Servlets) и DAO для размещения там бизнес-логики игрового процесса (после hibernate) Валидация: Добавить более строгие проверки входных данных (null, пустые строки) для предотвращения сбоев в работе приложения (валидация тоже еще будет) Итоговая оценка: A |
| private final Map<String, Quest> allQuests = Collections.synchronizedMap(new HashMap<>()); | ||
|
|
||
| public InMemoryUserDao() { | ||
| User admin = new User(); |
There was a problem hiding this comment.
Использование 'ConcurrentHashMap' для хранения пользователей — это отличная практика для обеспечения потокобезопасности в InMemory хранилище.
| public Quest getQuest(String questId) { | ||
| return allQuests.get(questId); | ||
| } | ||
| public boolean add(User u) { |
There was a problem hiding this comment.
Метод 'add' не проверяет входной объект на null перед вызовом 'u.getUsername()', что может привести к NullPointerException.
| String username = req.getParameter("username"); | ||
| String password = req.getParameter("password"); | ||
| log.info("Login attempt for user: " + username); | ||
| if (username == null || password == null) { |
There was a problem hiding this comment.
Пароли хранятся и сравниваются в открытом виде. Рекомендуется использовать хеширование (например, BCrypt) для обеспечения безопасности данных пользователей.
| java.util.Optional<User> u = dao.authenticate(username.trim(), password); | ||
| if (u.isEmpty()) { | ||
| req.setAttribute("error", "Invalid credentials"); | ||
| log.warning("Invalid credentials for username: " + username); |
There was a problem hiding this comment.
Логирование паролей или факта ошибки входа с указанием конкретного имени пользователя может помочь злоумышленникам при подборе. Используйте более обобщенные сообщения.
| } | ||
| log.info("Registering new user: " + username); | ||
| User u = new User(); | ||
| u.setUsername(username.trim()); |
There was a problem hiding this comment.
Вызов 'username.trim()' выполняется без предварительной проверки на null, что вызовет NPE при отсутствии параметра в запросе.
|
|
||
| @Override | ||
| protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { | ||
| Object o = req.getSession().getAttribute("currentUser"); |
There was a problem hiding this comment.
Для управления состоянием квеста (progress) вместо строк рекомендуется использовать Enum. Это исключит опечатки в логике переходов.
| dao = (InMemoryUserDao) getServletContext().getAttribute("userDao"); | ||
| } | ||
|
|
||
| private boolean isAdmin(HttpServletRequest req) { |
There was a problem hiding this comment.
Проверка роли пользователя осуществляется напрямую в методе контроллера. Целесообразно вынести авторизацию в Servlet Filter для централизованного управления доступом.
| private Role role = Role.USER; | ||
| private String progress = "start"; | ||
|
|
||
| public User() { |
There was a problem hiding this comment.
Поле 'id' инициализируется через 'UUID.randomUUID()'. Это хорошее решение для обеспечения уникальности в распределенных системах.
| public User() { | ||
| } | ||
|
|
||
| public String getId() { |
There was a problem hiding this comment.
Рекомендуется сделать поля 'username' и 'password' финальными, если они не предполагают изменения после регистрации, для обеспечения иммутабельности.
| public class AppContextListener implements ServletContextListener { | ||
| private static final Logger log = Logger.getLogger(AppContextListener.class.getName()); | ||
| @Override | ||
| public void contextInitialized(ServletContextEvent sce) { |
There was a problem hiding this comment.
Инициализация DAO в 'contextInitialized' — правильный подход для внедрения зависимостей (Dependency Injection) через ServletContext.
|
|
||
| @BeforeEach | ||
| void setUp() throws ServletException { | ||
| servlet = new AdminServlet(); |
There was a problem hiding this comment.
Тест вручную вызывает 'init(servletConfig)'. При использовании Mockito лучше настраивать моки так, чтобы сервлет получал зависимости через конструктор или сеттеры.
|
|
||
| public class InMemoryUserDaoTest { | ||
| @Test | ||
| public void addAndFindUser() { |
There was a problem hiding this comment.
Тестовый метод объединяет несколько проверок (создание, поиск, удаление). Рекомендуется разделять тесты: один метод — один проверяемый сценарий.
| this.creator = creator; | ||
| } | ||
|
|
||
| // Backwards-compatible constructor: from linear list of step descriptions |
There was a problem hiding this comment.
В конструкторе класса 'Quest' стоит добавить валидацию на пустой список шагов (steps), чтобы избежать некорректного состояния объекта.
| return true; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Использование 'java.util.Optional' для поиска пользователя — отличный пример современного Java-стиля, помогающий избежать проверок на null у вызывающей стороны.
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { | ||
| Object o = req.getSession().getAttribute("currentUser"); | ||
| if (!(o instanceof User u)) { |
There was a problem hiding this comment.
Применение 'instanceof' с паттерн-матчингом (Java 16+) — это современно и чисто. Хорошая работа с синтаксисом.
| private static final Logger log = Logger.getLogger(LogoutServlet.class.getName()); | ||
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { | ||
| req.getSession().invalidate(); |
There was a problem hiding this comment.
Инвалидация сессии при выходе — критически важная деталь для безопасности веб-приложения. Реализовано верно.
| } | ||
| log.info("Login successful for user: " + username); | ||
| req.getSession().setAttribute("currentUser", u.get()); | ||
| resp.sendRedirect(req.getContextPath() + "/game"); |
There was a problem hiding this comment.
Жесткое кодирование путей для редиректа ('/game') затрудняет поддержку. Стоит использовать константы или конфигурационный файл.
| import java.util.*; | ||
|
|
||
| public class InMemoryUserDao { | ||
| private final Map<String, User> users = Collections.synchronizedMap(new LinkedHashMap<>()); |
There was a problem hiding this comment.
Класс 'InMemoryUserDao' реализует интерфейс (предположительно), но стоит явно выделить интерфейс 'UserDao' для соблюдения принципа инверсии зависимостей (DIP).
|
|
||
|
|
||
| @Test | ||
| void doPost_successfulRegistration() throws ServletException, IOException { |
There was a problem hiding this comment.
Тест на регистрацию проверяет сразу и DAO, и состояние сессии, и редирект. Это делает тест хрупким. Желательно проверять только поведение сервлета.
|
|
||
| public void setUsername(String username) { | ||
| this.username = username; | ||
| } |
There was a problem hiding this comment.
Метод 'equals' и 'hashCode' должны быть переопределены для корректной работы объекта в коллекциях, особенно при использовании 'id' на базе UUID.
| req.setAttribute("users", users); | ||
| req.getRequestDispatcher("/admin.jsp").forward(req, resp); | ||
| } | ||
|
|
There was a problem hiding this comment.
При удалении пользователя не выполняется проверка, не пытается ли администратор удалить самого себя. Стоит добавить такую бизнес-проверку.
Text quest engine with persistent quest progress per user
In-memory database for learning purposes
User authentication (login / logout)
Admin role with user management (delete)
Default admin account for testing
Username: admin
Password: admin
Admins and users can create and play quests
Quest progress is preserved between logins
Multi-language support: EN / DE / RU
Event logging via Tomcat 10 Java Logger
Unit tests for core functionality
Can be built with Java JDK 21 using the Maven build tool and run with Tomcat 10.