Исправление замечаний

README.md

@@ -1,4 +1,17 @@
 # java-filmorate
 
 Template repository for Filmorate project.
+
+### Список выполненных задач
+
+1. Функциональность «Отзывы». 4 SP
+2. Функциональность «Поиск». 3 SP
+3. Функциональность «Общие фильмы». 1 SP
+4. Функциональность «Рекомендации». 3 SP
+5. Функциональность «Лента событий». 3 SP
+6. Функциональность «Удаление фильмов и пользователей». 2 SP
+7. Функциональность «Фильмы по режиссёрам». 4 SP
+8. Функциональность «Популярные фильмы». 2 SP
+
 DB scheme - /storage-scheme.jpg
+![storage-scheme.jpg](storage-scheme.jpg)

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

@@ -4,10 +4,8 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.HttpStatus;
-import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
 import ru.yandex.practicum.filmorate.dto.DirectorDto;
-import ru.yandex.practicum.filmorate.model.Director;
 import ru.yandex.practicum.filmorate.service.director.DirectorService;
 
 import java.util.Collection;
@@ -16,7 +14,6 @@
 @RestController
 @RequestMapping("/directors")
 @RequiredArgsConstructor
-@Validated
 public class DirectorController {
 
     private final DirectorService directorService;
@@ -35,13 +32,13 @@ public DirectorDto getDirectorById(@PathVariable long directorId) {
 
     @PostMapping
     @ResponseStatus(HttpStatus.CREATED)
-    public DirectorDto addDirector(@RequestBody @Valid Director director) {
+    public DirectorDto addDirector(@RequestBody @Valid DirectorDto director) {
         log.info("Received POST /directors request with body: {}", director);
         return directorService.addDirector(director);
     }
 
     @PutMapping
-    public DirectorDto updateDirector(@RequestBody @Valid Director director) {
+    public DirectorDto updateDirector(@RequestBody @Valid DirectorDto director) {
         log.info("Received PUT /directors request with body: {}", director);
         return directorService.updateDirector(director);
     }

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

@@ -11,6 +11,13 @@
 @RestControllerAdvice
 public class ErrorHandler {
 
+    @ExceptionHandler
+    @ResponseStatus(HttpStatus.BAD_REQUEST)
+    public ErrorResponse handleParameterNotValid(final ParameterNotValidException e) {
+        log.warn("parameter not valid");
+        return new ErrorResponse(e.getMessage());
+    }
+
     @ExceptionHandler
     @ResponseStatus(HttpStatus.NOT_FOUND)
     public ErrorResponse handleNotFound(final NotFoundException e) {
@@ -25,20 +32,6 @@ public ErrorResponse handleEmptyField(final EmptyFieldException e) {
         return new ErrorResponse(e.getMessage());
     }
 
-    @ExceptionHandler
-    @ResponseStatus(HttpStatus.BAD_REQUEST)
-    public ErrorResponse handleMethodArgumentNotValid(final MethodArgumentNotValidException e) {
-        log.warn("method argument not valid");
-        return new ErrorResponse(e.getMessage());
-    }
-
-    @ExceptionHandler
-    @ResponseStatus(HttpStatus.BAD_REQUEST)
-    public ErrorResponse handleConstraintViolation(final ConstraintViolationException e) {
-        log.warn("constraint violation");
-        return new ErrorResponse(e.getMessage());
-    }
-
     @ExceptionHandler(InternalServerException.class)
     @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
     public ErrorResponse handleException(Exception e) {

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

@@ -4,7 +4,7 @@
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
-import ru.yandex.practicum.filmorate.model.Event;
+import ru.yandex.practicum.filmorate.dto.EventDto;
 import ru.yandex.practicum.filmorate.service.feed.FeedService;
 
 import java.util.List;
@@ -20,7 +20,7 @@ public FeedController(FeedService feedService) {
     }
 
     @GetMapping("/{id}/feed")
-    public List<Event> getUserFeed(@PathVariable("id") long userId) {
+    public List<EventDto> getUserFeed(@PathVariable("id") long userId) {
         return feedService.getUsersEventFeed(userId);
     }
 }

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

@@ -5,9 +5,9 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.HttpStatus;
-import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
 import ru.yandex.practicum.filmorate.dto.FilmDto;
+import ru.yandex.practicum.filmorate.mappers.FilmMapper;
 import ru.yandex.practicum.filmorate.model.Film;
 import ru.yandex.practicum.filmorate.service.film.FilmService;
 
@@ -18,7 +18,6 @@
 @RestController
 @RequestMapping("/films")
 @RequiredArgsConstructor
-@Validated
 public class FilmController {
 
     private final FilmService filmService;
@@ -31,14 +30,15 @@ public Collection<FilmDto> getFilms() {
 
     @PostMapping
     @ResponseStatus(HttpStatus.CREATED)
-    public FilmDto addFilm(@Valid @RequestBody Film film) {
+    public FilmDto addFilm(@Valid @RequestBody FilmDto film) {
         log.info("Received POST /films request with body: {}", film);
         return filmService.addFilm(film);
     }
 
     @PutMapping
-    public FilmDto updateFilm(@Valid @RequestBody Film film) {
-        log.info("Received PUT /films request with body: {}", film);
+    public FilmDto updateFilm(@Valid @RequestBody FilmDto filmDto) {
+        log.info("Received PUT /films request with body: {}", filmDto);
+        Film film = FilmMapper.mapToFilmDto(filmDto);
         return filmService.updateFilm(film);
     }
 

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

@@ -24,8 +24,8 @@ public ReviewController(ReviewService reviewService) {
 
     @GetMapping
     public List<ReviewDto> getFilmReviews(@RequestParam(defaultValue = "0") long filmId,
-                                        @RequestParam(defaultValue = "10")
-                                        @Positive(message = "Count must be positive") long count) {
+                                          @RequestParam(defaultValue = "10")
+                                          @Positive(message = "Count must be positive") long count) {
         log.info("Received GET {} reviews for film with ID = {}", count, filmId);
         return reviewService.getFilmReviews(filmId, count);
     }

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

@@ -7,7 +7,6 @@
 import org.springframework.web.bind.annotation.*;
 import ru.yandex.practicum.filmorate.dto.FilmDto;
 import ru.yandex.practicum.filmorate.dto.UserDto;
-import ru.yandex.practicum.filmorate.model.User;
 import ru.yandex.practicum.filmorate.service.feed.FeedService;
 import ru.yandex.practicum.filmorate.service.film.FilmService;
 import ru.yandex.practicum.filmorate.service.user.UserService;
@@ -36,13 +35,13 @@ public Collection<UserDto> getUsers() {
 
     @ResponseStatus(HttpStatus.CREATED)
     @PostMapping
-    public UserDto addUser(@RequestBody @Valid User user) {
+    public UserDto addUser(@RequestBody @Valid UserDto user) {
         log.info("addUser");
         return userService.addUser(user);
     }
 
     @PutMapping
-    public UserDto updateUser(@RequestBody @Valid User user) {
+    public UserDto updateUser(@RequestBody @Valid UserDto user) {
         log.info("updateUser");
         System.out.println(user);
         return userService.updateUser(user);

src/main/java/ru/yandex/practicum/filmorate/dal/BaseRepository.java

@@ -5,7 +5,6 @@
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.support.GeneratedKeyHolder;
-import ru.yandex.practicum.filmorate.exception.ConstraintViolationException;
 
 import java.sql.PreparedStatement;
 import java.sql.Statement;
@@ -38,7 +37,7 @@ protected boolean delete(String query, long id) {
     protected void update(String query, Object... params) {
         int rowsUpdated = jdbc.update(query, params);
         if (rowsUpdated == 0) {
-            throw new ConstraintViolationException("Не удалось обновить данные");
+            throw new IllegalStateException("Не удалось обновить данные");
         }
     }
 
@@ -58,7 +57,7 @@ protected long insert(String query, Object... params) {
         if (id != null) {
             return id;
         } else {
-            throw new ConstraintViolationException("Не удалось сохранить данные");
+            throw new IllegalStateException("Не удалось сохранить данные");
         }
     }
 }

src/main/java/ru/yandex/practicum/filmorate/dal/FeedRepository.java

@@ -1,31 +1,28 @@
 package ru.yandex.practicum.filmorate.dal;
 
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.RowMapper;
 import org.springframework.stereotype.Repository;
-import ru.yandex.practicum.filmorate.dal.mapper.EventRowMapper;
 import ru.yandex.practicum.filmorate.model.Event;
 
 import java.util.List;
 
 @Repository
-public class FeedRepository {
+public class FeedRepository extends BaseRepository<Event> {
 
-    private final JdbcTemplate jdbc;
-    private final EventRowMapper eventRowMapper;
-
-    public FeedRepository(JdbcTemplate jdbc, EventRowMapper eventRowMapper) {
-        this.jdbc = jdbc;
-        this.eventRowMapper = eventRowMapper;
+    public FeedRepository(JdbcTemplate jdbc, RowMapper<Event> mapper) {
+        super(jdbc, mapper);
     }
 
     public List<Event> getUsersEventFeed(long userId) {
         String sqlQuery = "SELECT * FROM event_feed WHERE user_id = ?;";
-        return jdbc.query(sqlQuery, eventRowMapper, userId);
+        return findMany(sqlQuery, userId);
     }
 
     public void saveEvent(Event event) {
-        String sqlQuery = "INSERT INTO event_feed (timestamp, user_id, event_type, operation, entity_id) VALUES (?, ?, ?, ?, ?);";
-        jdbc.update(sqlQuery, event.getTimestamp(), event.getUserId(), event.getEventType().name(),
+        String sqlQuery = "INSERT INTO event_feed (timestamp, user_id, event_type, operation, entity_id) " +
+                          "VALUES (?, ?, ?, ?, ?);";
+        update(sqlQuery, event.getTimestamp(), event.getUserId(), event.getEventType().name(),
                 event.getOperation().name(), event.getEntityId());
     }
 }

src/main/java/ru/yandex/practicum/filmorate/dal/FilmRepository.java

@@ -4,7 +4,6 @@
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.jdbc.core.RowMapper;
-import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 import org.springframework.jdbc.support.GeneratedKeyHolder;
 import org.springframework.jdbc.support.KeyHolder;
@@ -52,7 +51,7 @@ public class FilmRepository extends BaseRepository<Film> {
             "SELECT fg.film_id, g.id as genre_id, g.name as genre_name " +
                     "FROM genre g " +
                     "JOIN film_genre fg ON g.id = fg.genre_id " +
-                    "WHERE fg.film_id IN (:filmIds)";
+                    "WHERE fg.film_id";
     private static final String DELETE_FILM_QUERY = "DELETE FROM Films WHERE id = ?";
 
     private static final String FIND_DIRECTOR_FOR_FILMS_QUERY = """
@@ -61,7 +60,7 @@ public class FilmRepository extends BaseRepository<Film> {
               	   d.NAME as DIRECTOR_NAME
               FROM FILM_DIRECTORS fd
               JOIN DIRECTORS d ON (fd.DIRECTOR_ID = d.DIRECTOR_ID)
-             WHERE fd.film_id in (:filmIds)
+             WHERE fd.film_id
             """;
 
     private static final String FIND_BY_DIRECTOR_SORT_BY_LIKES = """
@@ -96,8 +95,6 @@ ORDER BY (SELECT COUNT(1) FROM LIKES l WHERE l.FILM_ID = f.ID) DESC
               LEFT JOIN DIRECTORS d ON fd.DIRECTOR_ID = d.DIRECTOR_ID
              WHERE
             """;
-
-    private final JdbcTemplate jdbc;
     private final NamedParameterJdbcTemplate namedJdbcTemplate;
     private FilmRowMapper filmMapper;
 
@@ -139,7 +136,6 @@ public FilmRepository(JdbcTemplate jdbc,
                           NamedParameterJdbcTemplate namedJdbcTemplate,
                           FilmRowMapper filmMapper) {
         super(jdbc, filmMapper);
-        this.jdbc = jdbc;
         this.namedJdbcTemplate = namedJdbcTemplate;
         this.filmMapper = filmMapper;
     }
@@ -188,24 +184,28 @@ public Optional<Film> findById(long id) {
     }
 
     private void setGenresForFilms(List<Film> films) {
+        if (films == null || films.isEmpty()) {
+            return;
+        }
         List<Long> filmIds = films.stream()
                 .map(Film::getId)
+                .distinct()
                 .collect(Collectors.toList());
 
         if (filmIds.isEmpty()) {
             return;
         }
 
-        MapSqlParameterSource parameters = new MapSqlParameterSource();
-        parameters.addValue("filmIds", filmIds);
-
+        String placeholders = String.join(",", Collections.nCopies(filmIds.size(), "?"));
+        String finalSql = FIND_GENRES_FOR_FILMS_QUERY + " IN (" + placeholders + ")";
 
-        List<FilmGenreRelation> relations = namedJdbcTemplate.query(
-                FIND_GENRES_FOR_FILMS_QUERY,
-                parameters,
-                filmGenreRelationRowMapper
+        List<FilmGenreRelation> relations = jdbc.query(
+                finalSql,
+                filmGenreRelationRowMapper,
+                filmIds.toArray()
         );
 
+
         Map<Long, List<Genre>> genresByFilmId = relations.stream()
                 .collect(groupingBy(
                         relation -> relation.filmId,
@@ -387,22 +387,26 @@ private void saveDirector(Film film) {
     }
 
     private void setDirectorForFilm(List<Film> films) {
+        if (films == null || films.isEmpty()) {
+            return;
+        }
         List<Long> filmIds = films.stream()
                 .map(Film::getId)
+                .distinct()
                 .collect(Collectors.toList());
 
         if (filmIds.isEmpty()) {
             return;
         }
 
-        MapSqlParameterSource parameters = new MapSqlParameterSource();
-        parameters.addValue("filmIds", filmIds);
+        String placeholders = String.join(",", Collections.nCopies(filmIds.size(), "?"));
+        String finalSql = FIND_DIRECTOR_FOR_FILMS_QUERY + " IN (" + placeholders + ")";
 
 
-        List<FilmDirectorRelation> relations = namedJdbcTemplate.query(
-                FIND_DIRECTOR_FOR_FILMS_QUERY,
-                parameters,
-                filmDirectorRelationRowMapper
+        List<FilmDirectorRelation> relations = jdbc.query(
+                finalSql,
+                filmDirectorRelationRowMapper,
+                filmIds.toArray()
         );
 
         Map<Long, List<Director>> directorsByFilmId = relations.stream()

src/main/java/ru/yandex/practicum/filmorate/dal/GenreRepository.java

@@ -14,17 +14,12 @@ public class GenreRepository extends BaseRepository<Genre> {
     private static final String GET_GENRE_BY_ID = "SELECT id, name FROM genre WHERE id = ?";
     private static final String FIND_GENRES_BY_FILM_ID_QUERY =
             "SELECT g.id, g.name " +
-                    "FROM genre g JOIN film_genre fg ON g.id = fg.genre_id " +
-                    "WHERE fg.film_id = ? " +
-                    "ORDER BY g.id ASC";
-
-    private final JdbcTemplate jdbc;
-    private final GenreRowMapper genreMapper;
+            "FROM genre g JOIN film_genre fg ON g.id = fg.genre_id " +
+            "WHERE fg.film_id = ? " +
+            "ORDER BY g.id ASC";
 
     public GenreRepository(JdbcTemplate jdbc, GenreRowMapper mapper) {
         super(jdbc, mapper);
-        this.jdbc = jdbc;
-        this.genreMapper = mapper;
     }
 
     public List<Genre> findAll() {
@@ -36,7 +31,6 @@ public Optional<Genre> findById(int id) {
     }
 
     public List<Genre> findGenresByFilmId(long filmId) {
-        List<Genre> genres = jdbc.query(FIND_GENRES_BY_FILM_ID_QUERY, genreMapper, filmId);
-        return genres;
+        return findMany(FIND_GENRES_BY_FILM_ID_QUERY, filmId);
     }
 }