Add recommendations by Kirill-Kazantcev · Pull Request #17 · ksupyl/java-filmorate

Kirill-Kazantcev · 2026-10-08T22:17:47Z

Что сделано

Добавлен эндпоинт GET /users/{id}/recommendations для рекомендации фильмов
на основе пользователей с похожими вкусами.

Алгоритм:

Находим пользователя с максимальным пересечением по лайкам.
Возвращаем его фильмы, которых нет у нашего пользователя.

Детали

Рекомендации не хранятся в БД - вычисляются на лету. Схема не менялась.
Детерминированный выбор похожего пользователя: при равном пересечении
берётся меньший id (ORDER BY common DESC, fl2.user_id).
В проекте остаются только три SQL-константы для рекомендаций,
вся логика в UserDbStorage, ничего не «протекает» в сервис или контроллер.

ksupyl

Кирилл, спасибо! 🙌 Алгоритм понятный, тесты Практикума для ветки проходят все, а README с примерами запросов - огонь.

Две вещи нужно поправить до мержа:

Пока не вливай: после режиссёров в develop у тебя падают тесты UserDbStorageTest. Git конфликта не покажет, а зелёная галочка CI старая - проверяла ветку до режиссёров. Подробности в комментарии к @Import.
Фильмы в рекомендациях приходят без жанров и режиссёров — общий набор тестов Практикума на финальном PR это ловит.

И на подходе лента Даши.
После неё при подтягивании develop будут конфликты в README, UserController и UserService - оставляем всё от обоих, только в UserController проверь, что у метода getFeed осталась закрывающая скобка.

И ещё одно невидимое место: FeedDbStorageTest поднимает UserDbStorage, а тот у тебя теперь требует FilmRowMapper - без него падают тесты ленты.
Если запрос фильмов переедет в FilmDbStorage (см. комментарий про жанры), этой проблемы просто не будет.

Остальное - мелочи и один мой уточняющий вопрос.

Когда поправишь - подтяни develop, дождись зелёного CI, и после второго апрува вливай через Squash and merge с заголовком
feat: add film recommendations based on similar users (#17)

ksupyl · 2026-10-09T16:12:38Z

 @AutoConfigureTestDatabase
-@Import({UserDbStorage.class, UserRowMapper.class})
+@Import({UserDbStorage.class, UserRowMapper.class, FilmRowMapper.class,
+        FilmDbStorage.class, GenreRowMapper.class})


После того как в develop влились режиссёры, FilmDbStorage требует ещё DirectorRowMapper, а здесь его нет.
Если подтянуть новой develop, то все 14 тестов этого класса падают на подъёме контекста. Git конфликта не покажет, а зелёная галочка CI сейчас старая: она проверяла ветку до режиссёров.

Нужно подтянуть develop и добавить DirectorRowMapper в @Import.

Вернул @import.
Запрос рекомендаций в FilmDbStorage.
FilmRowMapper, DirectorRowMapper теперь не нужен.

ksupyl · 2026-10-09T16:14:15Z

+        }
+
+        Long similarUserId = similarUserIds.getFirst();
+        return jdbc.query(FIND_RECOMMENDATIONS_QUERY, filmMapper, similarUserId, userId);


Фильмы в рекомендациях приходят без жанров и режиссёров: тут только FilmRowMapper, а жанры и режиссёры в проекте подгружает FilmDbStorage.
Общий набор тестов Практикума на финальном PR develop -> main это проверяет: в "User id=1 Get film recommendations from user id=2" ждут у фильма 2 жанра, а приходит 0.

Фильмы у нас собирает FilmDbStorage - если запрос фильмов для рекомендаций переедет туда, жанры и режиссёры подтянутся тем же путём, что везде.
А UserDbStorage заодно перестанет зависеть от фильмов: сейчас любому тесту, который поднимает UserDbStorage, приходится тащить и FilmRowMapper.

Перенёс запрос фильмов для рекомендаций в FilmDbStorage
Добавил тест shouldReturnRecommendationsWithGenres

ksupyl · 2026-10-09T16:15:50Z

+                    + "WHERE fl1.user_id = ? AND fl2.user_id <> ? "
+                    + "GROUP BY fl2.user_id "
+                    + "ORDER BY common DESC, fl2.user_id "
+                    + "LIMIT 1";


Тут скорее вопрос, а не блокировочный момент: если у двух пользователей пересечение одинаковое, берём только одного - с меньшим id, а фильмы второго теряются.
В ТЗ "найти пользователей с максимальным количеством пересечений", во множественном числе.
Тесты Практикума это не проверяют, так что можно оставить, мне любопытно стало, осознанно ли так было сделано)

Исправил на множественное число,

ksupyl · 2026-10-09T16:16:29Z

+        User boris = createUser("boris");
+        Film first = createFilm("Первый");
+        Film second = createFilm("Второй");
+        userStorage.addFriend(anna.getId(), boris.getId());


Дружба вроде как рекомендациях не участвует - строчку можно убрать, чтобы тест не наводил на мысль, что рекомендации зависят от друзей.

ksupyl · 2026-10-09T16:17:24Z

+- Режиссёры: таблицы `directors` и `film_director`, у фильма — список режиссёров.
+- Рекомендации фильмов: два SQL-запроса — поиск самого похожего пользователя
+  по пересечению лайков и его фильмы, которых нет у пользователя.
+- Все эндпоинты покрыты интеграционными тестами (`@JdbcTest`).


Тут, кажется, "хранилища": @JdbcTest тестирует хранилища, а не эндпоинты.
А так раздел "Этап 4" классный - в финальном проходе README допишем туда остальные фичи группового проекта)

Поправил на хранилища

DariaPrindina

Посмотрела, согласна с замечаниями Ксюши, нового добавить нечего. Особенно поддерживаю перенос запроса в FilmDbStorage: тогда и жанры с режиссёрами подтянутся, и тесты ленты не будут зависеть от FilmRowMapper. После правок посмотрю ещё раз и апрувну

ksupyl

Кирилл, спасибо, что всё разобрал)
Запрос переехал в FilmDbStorage, жанры подтягиваются, ничьи учитываются - крутяк!

Осталось три момента, подробности в комментариях:

при мерже develop случайно удалились мои тесты на удаление в FilmDbStorageTest - их нужно вернуть;
у рекомендованных фильмов нет режиссёров;
README описывает старый алгоритм.

Если это поправить, то сразу апрув без вариантов)

ksupyl · 2026-10-10T11:24:46Z

        assertTrue(filmStorage.findById(film.getId()).isEmpty());
    }

-    // Без ON DELETE CASCADE удаление упало бы на внешних ключах film_genres и film_likes


Кажется, при мерже develop сюда случайно уехали мои тесты на удаление -shouldDeleteFilmWithGenresAndLikes, shouldNotCountLikesOfDeletedUser и shouldThrowWhenDeletingUnknownFilm (и импорт NotFoundException).

Верни их, пожалуйста. С твоими тестами рекомендаций они прекрасно уживаются)

ksupyl · 2026-10-10T11:25:31Z

+
+        List<Film> films = jdbc.query(sql, mapper, params.toArray());
+        // Жанры подтягиваются тем же путём, что и у остальных списочных запросов
+        loadGenresForFilms(films);


Жанры теперь есть)
А режиссёры у рекомендованных фильмов всё ещё пустые: в остальных методах со списками фильмов рядом стоит и loadDirectors(films), а здесь его нет.

ksupyl · 2026-10-10T11:26:02Z

+WHERE fl1.user_id = 1 AND fl2.user_id <> 1
+GROUP BY fl2.user_id
+ORDER BY common DESC, fl2.user_id
+LIMIT 1;


Тут описан старый вариант - с одним самым похожим пользователем.
Сейчас берутся все с максимальным пересечением: поправь, пожалуйста, пример и строчку про рекомендации в "Этап 4".

ksupyl

Кирилл, всё на месте)
Тесты на удаление вернулись, режиссёры у рекомендаций подтягиваются, в README новый алгоритм.

Апрув! Как будет второй апрув - вливай через Squash and merge, заголовок
feat: add film recommendations based on similar users (#17)

feat: add film recommendations based on similar users

9467c77

Kirill-Kazantcev linked an issue Oct 8, 2026 that may be closed by this pull request

Функциональность «Рекомендации». 3 SP #15

Closed

Kirill-Kazantcev changed the base branch from main to develop October 8, 2026 22:21

Trigger CI

171a549

ksupyl requested changes Oct 9, 2026

View reviewed changes

DariaPrindina self-requested a review October 9, 2026 16:30

DariaPrindina reviewed Oct 9, 2026

View reviewed changes

Kirill-Kazantcev added 3 commits October 10, 2026 03:41

Include all users with max overlap in recommendations

e7b9ff1

Merge branch 'develop' into add-recommendations

28e7779

fix: missing brace after merge

185d2ed

ksupyl requested changes Oct 10, 2026

View reviewed changes

fix: after review

6b71202

ksupyl approved these changes Oct 11, 2026

View reviewed changes

DariaPrindina approved these changes Oct 11, 2026

View reviewed changes

Kirill-Kazantcev merged commit 7d5e051 into develop Oct 11, 2026
2 checks passed

ksupyl mentioned this pull request Oct 11, 2026

Групповой проект: восемь новых функций #24

Merged

Conversation

Kirill-Kazantcev commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Что сделано

Детали

Uh oh!

ksupyl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ksupyl Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ksupyl Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DariaPrindina left a comment

Choose a reason for hiding this comment

Uh oh!

ksupyl 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!

ksupyl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kirill-Kazantcev commented Oct 8, 2026 •

edited

Loading

ksupyl left a comment •

edited

Loading

ksupyl Oct 9, 2026 •

edited

Loading

ksupyl Oct 9, 2026 •

edited

Loading