Доработал Filmorate добавив ветку add-database с функциональностью сохранения состояния данных после перезапуска. #4

ViktorSalk · 2025-02-17T16:35:12Z

Добавил в проект зависимости com.h2database.h2
Сконфигурировал базу данных для рабочего режима с помощью файла настроек application.properties.
Сформировал базу данных и собрал SQL запросы в schema.sql и данные в data.sql
Создал DAO для получения данных по фильмам и юзерам из базы.Реализовав имплементацию UserDbStorage и FilmDbStorage с помощью аннотаций @qualifier.
Для самопроверки создал DataPersistenceTest с уникальными данными, сохраняемыми после перезапуска.
Реализовал CRUD-операции с использованием JdbcTemplate.
Доработал бизнес логику.
Обновил старые тесты под доработанную логику.
Добавил тесты на интеграционное тестирование через @AutoConfigureTestDatabase.

@qualifier

…хранения состояния данных после перезапуска. 1) Добавил в проект зависимости com.h2database.h2 2) Сконфигурировал базу данных для рабочего режима с помощью файла настроек application.properties. 3) Сформировал базу данных и собрал SQL запросы в schema.sql и данные в data.sql 4) Создал DAO для получения данных по фильмам и юзерам из базы.Реализовав имплементацию UserDbStorage и FilmDbStorage с помощью аннотаций @qualifier. 5) Для самопроверки создал DataPersistenceTest с уникальными данными, сохраняемыми после перезапуска. 6) Реализовал CRUD-операции с использованием JdbcTemplate. 7) Доработал бизнес логику. 8) Обновил старые тесты под доработанную логику. 9) Добавил тесты на интеграционное тестирование через @AutoConfigureTestDatabase.

EugeneLenkevich

Привет, Виктор! Спасибо за PR.

Отличная работа! :)
В первую очередь попробуй избавиться от логики в контроллерах (это архитектурно не очень правильно) и от запросов в циклах (потенциальная проблема с производительностью, N+1 запрос) по возможности, отметил в комментариях ниже, посмотри, пожалуйста

EugeneLenkevich · 2025-02-17T16:56:13Z

src/main/java/ru/yandex/practicum/filmorate/controller/FilmController.java

    }
-}
+
+    private void validateMpaAndGenres(Film film) {


По своей сути контроллер только обрабатывает (маршрутизирует сервисам) запросы, а тут же в контроллере выполняется некоторая бизнес-логика, ее лучше перенести в сервис, иначе логика размазывается по нескольким классам :)
Допустим сервис был вызван из другого сервиса, тогда у нас логика будет другой - без валидации, вряд ли это правильное поведение, лучше когда все работает единообразно

Спасибо) Перенес логику проверки validateMpaAndGenres из FilmController в FilmService приведя к большему единообразию.

EugeneLenkevich · 2025-02-17T16:58:38Z

src/main/java/ru/yandex/practicum/filmorate/dao/FilmDbStorage.java

+        if (film.getGenres() != null && !film.getGenres().isEmpty()) {
+            String sql = "INSERT INTO film_genres (film_id, genre_id) VALUES (?, ?)";
+            for (GenreDto genre : film.getGenres()) {
+                jdbcTemplate.update(sql, film.getId(), genre.getId());


Запрос в БД - дорогая операция, их лучше минимизировать, а тут же в цикле идут запросы для каждого элемента. Это не особо эффективно особенно если у нас бтудет много записей в БД.
Можно слепить все инсерты в одну строку и выполнить ее за раз, ак будет гораздо эффективней с точки зрения производительности

В FilmDbStorage оптимизировал updateFilmGenres для использования пакетных операций вместо отдельных запросов в цикле.

EugeneLenkevich · 2025-02-17T17:00:26Z

src/main/java/ru/yandex/practicum/filmorate/dao/FilmDbStorage.java

+
+    private void loadGenresForFilms(List<Film> films) {
+        for (Film film : films) {
+            loadGenresForFilm(film);


Тут бы тоже от запосов в циуле избавится
Запрос будет что то вроде ... WHERE fg.film_id in (ids)

В FilmDbStorage loadGenresForFilms теперь используется один запрос с предложением IN вместо нескольких запросов.

EugeneLenkevich · 2025-02-17T17:02:18Z

src/main/java/ru/yandex/practicum/filmorate/service/UserService.java

-                .filter(Objects::nonNull)
-                .collect(Collectors.toList());
+    public void addFriend(Long userId, Long friendId) {
+        userStorage.get(userId);


userStorage.get(userId);

А что тут происходит не совсем понятно, предположу что проверка на существование записи, может тогда завести метод exist который возвращает boolean, код должен стать наглядней и проще :)

Спасибо ), так и есть: здесь была проверка на существование записи; в UserService внедрил использование метода exists() для более четкой и эффективной проверки существования пользователя.

Тут такая история. На уровне сервиса мы оперируем терминами бизнес-логики, переменные - это наши существительные, а методы - глаголы. В таком виде код читается как простой текст-описание какой то бизнесовой задачи. Такой код потом гораздо проще поддерживать. Потому тут лучше задуматься о семантике :)

…и более удобен в обслуживании. 1) Перенес логику проверки validateMpaAndGenres из FilmController в FilmService приведя к большему единообразию. 2) В FilmDbStorage оптимизировал updateFilmGenres для использования пакетных операций вместо отдельных запросов в цикле. 3) В FilmDbStorage loadGenresForFilms теперь используется один запрос с предложением IN вместо нескольких запросов. 4) В UserService внедрил использование метода exists() для более четкой и эффективной проверки существования пользователя.

EugeneLenkevich

Теперь все отлично, спасибо

PS дальше у нас уже будет Spring Data, запросы станет писать и раскладывать результаты по объектам гораздо проще :)

EugeneLenkevich · 2025-02-18T18:19:58Z

src/main/java/ru/yandex/practicum/filmorate/dao/UserDbStorage.java

+
+    @Override
+    public boolean exists(Long userId) {
+        String sql = "SELECT COUNT(*) FROM users WHERE user_id = ?";


А есть еще отдельный exists-запрос

https://stackoverflow.com/questions/7471625/fastest-check-if-row-exists-in-postgresql

EugeneLenkevich suggested changes Feb 17, 2025

View reviewed changes

EugeneLenkevich approved these changes Feb 18, 2025

View reviewed changes

ViktorSalk merged commit b9357c5 into main Feb 22, 2025
2 checks passed

Доработал Filmorate добавив ветку add-database с функциональностью сохранения состояния данных после перезапуска. #4

Доработал Filmorate добавив ветку add-database с функциональностью сохранения состояния данных после перезапуска. #4

Uh oh!

Conversation

ViktorSalk commented Feb 17, 2025

Uh oh!

EugeneLenkevich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EugeneLenkevich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants