-
Notifications
You must be signed in to change notification settings - Fork 0
sprint-12 add database schema docs #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| final Set<Integer> seen = new HashSet<>(); | ||
| final LinkedHashSet<Genre> normalized = new LinkedHashSet<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
попробуй обозначить тип переменной Set
- final Set normalized = new LinkedHashSet<>();
не должно ничего поменятся, т.к. конкретный объект подставляется в правой стороне.
Такой подход дает возможность переменной, методам не зависеть от конкретной реализации. И при каком либо измененении, рефакторингу подвергнется только одна строка в одном методе одного класса.
| final String sql = "SELECT f.id, f.name, f.description, f.release_date, f.duration, f.mpa_id, " | ||
| + "m.name AS mpa_name FROM films f JOIN mpa_ratings m ON f.mpa_id = m.id ORDER BY f.id"; | ||
| final List<Film> films = jdbcTemplate.query(sql, FILM_MAPPER); | ||
| films.forEach(this::enrichFilm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут у нас беда. К примеру фильмов 1000.
Мы ожидаем, что вытащим данные из базы в 1-3 запроса.
- но в текущей реализации стреляет ошибка N+1, один общий запрос + на каждый фильм по два запроса жанры и лайки, итого 1 + 2*1000 = 2001 запрос в базу на ровном месте.
- если 10ть пользователей кликнет получение фильмов, то получим 20010 запросов в течении короткого времени.
| if (userId == null) { | ||
| continue; | ||
| } | ||
| jdbcTemplate.update(sql, film.getId(), userId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тоже запросы в цикле. попробуй батчАпдейт
| + "JOIN mpa_ratings m ON f.mpa_id = m.id " | ||
| + "LEFT JOIN film_likes fl ON f.id = fl.film_id " | ||
| + "GROUP BY f.id, f.name, f.description, f.release_date, f.duration, f.mpa_id, m.name " | ||
| + "ORDER BY COUNT(fl.user_id) DESC, f.id ASC LIMIT ?"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- fl.user_id боюсь, что может быть не оптимальным запросом подсчет каждому фильму кол-ва лайков и затем применение лимита. Как будто всю таблицу перетрясти может. Можно конечно проверить анализом запроса к базе.
Есть идея. к примеру добавить фильму поле Integer - рейтинг, когда лайк добавили , обновить поле , убавили - обновить. Но при выводе не потребуется считать . - еще вопрос, применили соединение с таблицей film_likes и получил все нужные данные, но двумя строками ниже в цикле вызываем loadLikes и снова N+1
| .withTableName("users") | ||
| .usingGeneratedKeyColumns("id"); | ||
| final LocalDate birthday = user.getBirthday(); | ||
| final HashMap<String, Object> values = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
применяй более широкий тип переменной, тут к примеру будет оптимальным:
final Map<String, Object> values = new HashMap<>();
| if (friendId == null || friendId.equals(user.getId())) { | ||
| continue; | ||
| } | ||
| jdbcTemplate.update(sql, user.getId(), friendId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
попробуй batchUpdate
b40b6cc to
f7787da
Compare
|
работу принимаю. Рекомендую два инструмента:
|
|
Обновление норм жанров в FilmService, которое позволяет использовать заданный интерфейс вместо конкретной реализации, сохраняя гибкость метода для последующих изменений. Переработан FilmDbStorage для загрузки жанров и лайков, стандартизации карт вставок и пакетного сохранения жанров/лайков, чтобы удалить шаблоны доступа N+1 и сократить количество запросов. Изменен UserDbStorage для использования карт, типизированных интерфейсов и обновлений пакетного сохранения отношений между странами для более эффективной записи. Оптимизировал загрузку дружбы для пользователей findAll, чтобы получилось N+1 запроса, и повторно использовал помощника для однопользовательского поиска. Уточнил название количества нормализованных жанров, сохраняющих использование интерфейса Set, чтобы лучше отображать рекомендации по брендам и названиям. Переписал SQL для выбора популярных фильмов, добавив подзапрос с предварительной агрегацией лайков и использованием COALESCE, что увеличивает нагрузку от повторных подсчетов по таблице лайков. Дедуплицирует идентификаторы фильмов перед воспроизведением загрузкой жанров и стремится к минимальному количеству пакетных запросов для любого расширенного результирующего набора. Дедулицировал идентификаторы пользователей перед извлечением данных о дружбе и согласовал обработку пустых результатов с Collections.emptyMap(), чтобы избежать избыточного расширения заполнения. Переиспользовал общий помощник richUsers внутри getById, чтобы при запросе одного пользователя его друзья тоже сразу подгружались, и для этого не нужен отдельный «загрузчик» друзей. Удал лишний метод, который загружал друзей для одного пользователя, потому что теперь всё происходит через общий путь обогащения данных (тот самый richUsers). Переименовал нормализованную коллекцию жанров в FilmService, чтобы было ясно, что это именно Set (множество), а не просто список, и таким образом жанры в ней не дублируются — всё в соответствии с примечаниями из код-ревью. Повторно использовался специальный конструктор-заполнитель в UserDbStorage, так что при включении в другом SQL используется тот же помощник при формировании параметров, который используется в других местах, что подтверждает согласованность и удобочитаемость. Повторно использовал модель нормализации фильма при обогащении результатов запроса и переключил подготовку JDBC vararg на toArray(Object[]::new) для жанра загрузки и аналогичных файлов. Применил то же типобезопасное преобразование массива при расширении данных пользователей о дружбе. Теперь фильмы, в которых нет жанров, получают новый пустой LinkedHashSet, Добавил импорт JDBC ResultSet и SQLException и переработал загрузку так, чтобы использовать отдельный ResultSetExtractor. За счёт этого: все друзья подгружаются одним пакетным запросом к БД; при этом повторно используется уже существующая логика, которая выполняет Map с результатом (то есть код не дублируется). Обобщил (унифицировал) помощники обогащения фильмов: Обновил утилиты обогащения для пользователей, чтобы они вернули Сделал загрузку лайков детерминированной – то теперь есть лайки приходят всегда в одном и том же порядке. Мы добавили этот запрос сортировки в таблицу Film_likes перед темой, чтобы собирать результаты. Сохранил упорядоченность коллекций лайков, перейдя на LinkedHashSet при извлечении данных из ResultSet. Нормализация жанров переносится в сеттер сущности. Теперь фильм сам по себе, чтобы жанры были в нужном виде. В обогащении фильмов В расширении пользователей Упрощенное обогащение фильмов по умолчанию, основанное на Collections.emptySet(), что позволяет избежать происхождения основных определений, сохраняя при этом количество лайков и жанров без изменений. Применил тот же вывод о пустом наборе для обогащения пользователей, чтобы друзья постоянно использовали общие вспомогательные действия. Заменил пакетные запросы MERGE в хранилище фильмов на обычный INSERT, теперь SQL более «переносимый» между разными БД (без особенностей конкретного диалекта); при этом сохраняется защита от дублей при загрузке жанров и лайков (защита дедупликации). Сделал ту же замену INSERT для пакетного обновления дружбы пользователей, Упростил вспомогательные методы для обогащения фильмов и пользователей в JDBC — То же самое сделал в пайплайне выбор популярных фильмов в памяти — Заранее задал размер вспомогательных наборов в FilmService.normalizeGenres на основе размера входной коллекции жанров, чтобы уменьшить количество перехэширований при нормализации. Для кэшей обогащения фильмов То же самое для пользователей дружбы Вынес основную часть SQL-запроса для выбора фильмов в одну константу и Смысл: теперь везде, где выбираются, используются один и тот же фильмы SELECT f.id, f.name, ..., Заранее задал размер вспомогательных наборов (наборов), которые использовались при пакетной загрузке жанров и лайков фильма, чтобы уменьшить количество перехешированных во время сохранения в базе. Уменьшение количества повторяющихся поисковиков по картам при просмотре фильмов за счет повторного использования выбранных жанровых и аналогичных пакетов перед применением их к каждому объекту. Защищенный поиск друзей пользователя, поэтому идентификаторы друзей включаются только тогда, когда пакетный поиск предоставляет данные. Переиспользовал локальные ссылки на коллекции жанров и лайков при включении пакетной загрузки в FilmDbStorage и заранее задает размер вспомогательных контейнеров, чтобы избежать лишних обращений к объекту фильма во время JDBC-обновлений. Упростил обогащение фильмов, теперь все основано на Film#setGenres, а сама модель обрабатывает нулевые или пустые наборы жанров без исключений. Теперь при обогащении фильмов, если в фильме нет жанров или лайков, Добавил проверку на отсутствие лайков перед пакетной вставкой в фильм_лайки: Заменил существующую модификацию ограничений в FilmService неизменяемой локальной переменной перед делегированием в хранилище, сохранив обработку по умолчанию неизменной. Нормализовал предел параметров внутри FilmDbStorage.findMostPopular с помощью выделенной локальной переменной, чтобы метод больше не изменял свои аргументы, по-прежнему постоянно используя централизованную логику по умолчанию. Обновлены параметры карты вставки JDBC в FilmDbStorage, чтобы предварительно использовать LinkedHashMap определенного размера, исключить определенный порядок столбцов и удалить неиспользуемые импортируемые хэш-карты. Применил тот же подход к детерминированному упорядочению параметров вставки в UserDbStorage и заменил избыточный импорт хэш-карты. Повторно использовал константу DEFAULT_POPULAR_LIMIT через интерфейс FilmStorage, чтобы выровнять сервис и хранилище JDBC для резервного показа популярности. Нормализована обработка пустых жанров с помощью Collections.emptySet() перед передачей разработчику фильма, что обеспечивает использование типизированного интерфейса во всем сервисе. |
Как проверить
сначала кэш надо удалить
mvn -q clean testmvn testПримеры запросов (PowerShell,
Invoke-RestMethod)0. Инициализация сессии
1. Справочники
2. Пользователи
3. Дружба (add/remove + проверка)
4. Фильмы и лайки
5. Дополнительно (CRUD-примеры)
Что делать, если вернулся 4xx/5xx
[*]нужно использовать уникальныеemail/login(для пользователей) и, при необходимости, уникальное имя фильма.Примеры выше уже генерируют уникальные значения через
Get-Random.db/(*.mv.db,*.trace.db), затем запусти снова.