Develop#22
Conversation
* Обновлен метод getTopFilms в цепочке FilmController->FilmService->FilmDbStorage->FilmRepository согласно новой функциональности (фильтрации по genreId и year) * Обновлен метод getTopFilms в цепочке FilmController->FilmService->FilmDbStorage->FilmRepository согласно новой функциональности (фильтрации по genreId и year) * делаю коммит псÐоскольку force push не триггерит пайплайн по всей видимости * Исправил логику работы метода getTopFilms(...) (параметры строки запроса genreId and year являются необязательными) --------- Co-authored-by: nesailormun <Keyko07@yandex.ru>
* add getCommonFilms for user and friend * little change * checkstyle
* add recommendations * Реализован функционал добавления-исправления режиссёра, получения списка режиссёров по id, получение списка всех режиссёров. Реализовано добавление/исправление/получение фильмов с учетом информации о режиссере. * Добавлен функционал возвращения списка фильмов режиссера отсортированных по лайкам или году выпуска * Исправить ошибки Checkstyle * Отладка тестов Postman: переименовать поле director в directors; внести правки в функционал исправления фильма в части информации о режиссере * Переименовать метод deleteFilms в deleteFilmDirectors в классе FilmRepository. Добавить проверку на корректность введённых параметров в контроллере FilmController в методе findByDirector * Add remove endpoint for films and users (#14) * Добавлена функциональность «Популярные фильмы» (#13) * Обновлен метод getTopFilms в цепочке FilmController->FilmService->FilmDbStorage->FilmRepository согласно новой функциональности (фильтрации по genreId и year) * Обновлен метод getTopFilms в цепочке FilmController->FilmService->FilmDbStorage->FilmRepository согласно новой функциональности (фильтрации по genreId и year) * делаю коммит псÐоскольку force push не триггерит пайплайн по всей видимости * Исправил логику работы метода getTopFilms(...) (параметры строки запроса genreId and year являются необязательными) --------- Co-authored-by: nesailormun <Keyko07@yandex.ru> * add getCommonFilms for user and friend (#10) * add getCommonFilms for user and friend * little change * checkstyle * add getCommonFilms for user and friend * Внести исправление в метод getTopFilms класса FilmRepository: join с таблицей likes заменить на left join --------- Co-authored-by: Plastinin-Igor <igor-plastinin@yandex.ru> Co-authored-by: b1mgd <sparcman1990@gmail.com> Co-authored-by: Stanislav Mun <146199590+Nesailormun@users.noreply.github.com> Co-authored-by: nesailormun <Keyko07@yandex.ru>
…аботе с друзьями и лайками. Ожидаю реализации функциональности review.
Функциональность Поиск
# Conflicts: # src/main/resources/schema.sql
Add review endpoints
…нальности "Отзывы"
Исправить ошибки Postman
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый день, Илья!
Вы отлично поработали над проектом. Функционал задач реализован в соответствии с техническим заданием, в общем и целом разобрались с взаимодействием слоев Controller - service - repository (хотя тут ещё есть что поправить :)). Несмотря на то, что это первый групповой проект, результат можно считать очень хорошим.
От меня только несколько уточнений, часть из них на ваше усмотрение.
Старался не писать повторяющихся комментариев, будете править, проверьте, пожалуйста, аналогичные места и в других частях проекта.
| logging.level.org.zalando.logbook=INFO | ||
| logging.level.ru.yandex.practicum.filmorate=INFO | ||
| spring.sql.init.mode=always | ||
|
|
There was a problem hiding this comment.
Просьба добавить описание того, какие задачи выполнила группа, в описании пул-реквеста или в файле readme
| import lombok.Data; | ||
| import ru.yandex.practicum.filmorate.model.Director; | ||
| import ru.yandex.practicum.filmorate.model.Rating; | ||
|
|
There was a problem hiding this comment.
Насколько я понял, этот класс в проекте не используется, его можно спокойно убрать.
| @@ -0,0 +1,7 @@ | |||
| package ru.yandex.practicum.filmorate.exception; | |||
|
|
|||
There was a problem hiding this comment.
public class ConstraintViolationException extends RuntimeException {
Пишу здесь, потому что класс недоступен для комментария.
Сделано вот такое самодельное исключение. Вопрос, есть же точно такое же , почему нельзя было воспользоваться стандартным?
import jakarta.validation.ConstraintViolationException;
| public InternalServerException(String message) { | ||
| super(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
public class MethodArgumentNotValidException extends RuntimeException {
аналогичный вопрос????
| import java.util.List; | ||
|
|
||
| @Repository | ||
| public class FeedRepository { |
There was a problem hiding this comment.
Вопрос-почему этот репозитарий нельзя было наследовать от базового?
public class BaseRepository<T> {
| import java.util.Optional; | ||
|
|
||
| /* | ||
| Не стал оформлять ReviewDbStorage, так как в остальной программе такие классы лишь делигируют вызовы классу-репозиторию. |
There was a problem hiding this comment.
Честно говоря, я бы вообще избавился от слоя Storage. Часть методов можно перенести в соответствующие классы слоя репозитария, а часть вынести в слой сервисов.
Не настаиваю, но, вообще говоря, желательно. Так, как это сделано сейчас, вносит некую запутанность в архитектуру проекта.
| .orElseThrow(() -> new NotFoundException("Review with ID " + reviewId + " not found")); | ||
| } | ||
|
|
||
| @Transactional |
There was a problem hiding this comment.
Transactional
В принципе, для данного проекта можно обойтись без этой аннотации. )
| @RestController | ||
| @RequestMapping("/directors") | ||
| @RequiredArgsConstructor | ||
| @Validated |
There was a problem hiding this comment.
@validated не имеет смысла, так как отсутствуют аннотации валидации(ограничения) для параметров методов)
|
|
||
| @PostMapping | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public DirectorDto addDirector(@RequestBody @Valid Director director) { |
There was a problem hiding this comment.
public DirectorDto addDirector(@RequestBody @Valid Director director) {
Director director
Так, раз уж используется техника работы с ДТО классами, то давайте разбираться. 🙂
Здесь не должно быть объектов БД. Он же пришёл снаружи, значит должен быть dto.
Передаётся в слой сервиса как dto, там происходит обработка, а перед тем как передать в слой репозитария, делается маппинг в объект базы данных.
Впрочем, маппинг можно делать и на входе в сервис. На Ваше усмотрение.
Ну и так далее, по остальным методам и остальным классам. Проверьте пожалуйста.
Общий принцип такой-контроллеры - это dto, сервисы это бизнес логика и маппинг, а репозиторий это обычные классы (сущности) для работы в базе данных.
| log.info("Received DELETE /films/{}/like/{} request", filmId, userId); | ||
| filmService.deleteLikeToFilm(filmId, userId); | ||
| } | ||
|
|
There was a problem hiding this comment.
Validated
public class FilmController {
Аннотация над классом будет работать в том случае, если к примитивным входным параметрам методов добавлены дополнительные ограничительные аннотации, например:
public FilmDto getFilmById(@PathVariable @Positive long filmId) {
* Добавить в файл README.md список выполненных задач * Удалить класс filmorate/dto/UpdateFilmRequest.java Класс FeedRepository унаследовать от BaseRepository В классе GenreRepository убрать поля jdbc и genreMapper, в методе findGenresByFilmId использовать базовый метод findMany В классе ReviewService убрать аннотацию Transactional В классе DirectorController убрать аннотацию validated В классе DirectorController входные параметры исправить с Director на DirectorDto В классе DirectorService входные параметры исправить с Director на DirectorDto В классе убрать аннотацию FilmController Validated * Delete exceptions, refactor jdbc in FilmRepository * checkstyle * Использовать технику работы с DTO в контроллерах * refactor --------- Co-authored-by: Remenchik Ilya <ilyaremenchik228@gmail.com>
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый день!
Замечаний нет.
Групповой проект принят.
Поздравляю с удачным опытом групповой разработки, успехов!
Уверен, что данный опыт пригодится вам в дальнейшем.
Выполнен групповой проект, реализованы функциональности, обозначенные в ТЗ.