From b7a246e0b40389ba6c21f720fd54ed13a70faaf5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 3 May 2026 12:02:31 +0000 Subject: [PATCH] fix: address copilot review issues - explicit imports, trailing commas, 404 for not-found search, tests for available-films endpoint Agent-Logs-Url: https://github.com/devitq/movienight-backend/sessions/60cd9b1b-e3e1-46a1-bfd9-04a75bd0d569 Co-authored-by: devitq <118541411+devitq@users.noreply.github.com> --- .../movienight/adapters/web/FilmController.kt | 15 +- .../adapters/web/FilmLibraryController.kt | 33 +-- .../application/services/FilmService.kt | 22 +- .../controllers/FilmControllerTest.kt | 137 ++++++------ .../controllers/FilmLibraryControllerTest.kt | 202 ++++++++++++++---- .../controllers/UserControllerTest.kt | 82 +++---- 6 files changed, 317 insertions(+), 174 deletions(-) diff --git a/src/main/kotlin/com/project/movienight/adapters/web/FilmController.kt b/src/main/kotlin/com/project/movienight/adapters/web/FilmController.kt index df375e4..3699e33 100644 --- a/src/main/kotlin/com/project/movienight/adapters/web/FilmController.kt +++ b/src/main/kotlin/com/project/movienight/adapters/web/FilmController.kt @@ -57,10 +57,11 @@ class FilmController( FilmResponse.fromDomain( editFilmUseCase.edit( id = id, - command = EditFilmCommand( - title = request.title, - description = request.description, - ), + command = + EditFilmCommand( + title = request.title, + description = request.description, + ), ), ) @@ -73,12 +74,10 @@ class FilmController( @GetMapping("/{id}") fun getById( @PathVariable id: UUID, - ): FilmResponse = - FilmResponse.fromDomain(getFilmByIdUseCase.getById(id)) + ): FilmResponse = FilmResponse.fromDomain(getFilmByIdUseCase.getById(id)) @GetMapping - fun getAll(): List = - getAllFilmsUseCase.getAll().map { FilmResponse.fromDomain(it) } + fun getAll(): List = getAllFilmsUseCase.getAll().map { FilmResponse.fromDomain(it) } @GetMapping("/search") fun searchByTitle( diff --git a/src/main/kotlin/com/project/movienight/adapters/web/FilmLibraryController.kt b/src/main/kotlin/com/project/movienight/adapters/web/FilmLibraryController.kt index a4a6175..e4e000d 100644 --- a/src/main/kotlin/com/project/movienight/adapters/web/FilmLibraryController.kt +++ b/src/main/kotlin/com/project/movienight/adapters/web/FilmLibraryController.kt @@ -64,9 +64,10 @@ class FilmLibraryController( fun getAllFilmsInLibrary( @PathVariable userId: UUID, ): List { - val library = getFilmLibraryUseCase.getLibrary( - GetFilmLibraryQuery(userId = userId), - ) + val library = + getFilmLibraryUseCase.getLibrary( + GetFilmLibraryQuery(userId = userId), + ) val film = getFilmByIdUseCase.getById(library.filmId) return listOf(FilmResponse.fromDomain(film)) } @@ -102,21 +103,23 @@ class FilmLibraryController( fun getAvailableFilms( @PathVariable userId: UUID, ): List { - val userLibrary = try { - getFilmLibraryUseCase.getLibrary( - GetFilmLibraryQuery(userId = userId), - ) - } catch (e: EntityNotFoundException) { - null - } + val userLibrary = + try { + getFilmLibraryUseCase.getLibrary( + GetFilmLibraryQuery(userId = userId), + ) + } catch (e: EntityNotFoundException) { + null + } val allFilms = getAllFilmsUseCase.getAll() - val availableFilms = if (userLibrary != null) { - allFilms.filter { it.id != userLibrary.filmId } - } else { - allFilms - } + val availableFilms = + if (userLibrary != null) { + allFilms.filter { it.id != userLibrary.filmId } + } else { + allFilms + } return availableFilms.map { FilmResponse.fromDomain(it) } } diff --git a/src/main/kotlin/com/project/movienight/application/services/FilmService.kt b/src/main/kotlin/com/project/movienight/application/services/FilmService.kt index 555655a..f775de3 100644 --- a/src/main/kotlin/com/project/movienight/application/services/FilmService.kt +++ b/src/main/kotlin/com/project/movienight/application/services/FilmService.kt @@ -28,7 +28,6 @@ class FilmService( GetFilmByIdUseCase, GetAllFilmsUseCase, SearchFilmByTitleUseCase { - override fun create(command: CreateFilmCommand): Film { if (filmConfig.isBlocked(command.title)) { throw BlockedValueException(target = "Film", field = "title") @@ -37,15 +36,19 @@ class FilmService( throw BlockedValueException(target = "Film", field = "description") } - val film = Film( - id = idGenerator.generateId(), - title = command.title, - description = command.description, - ) + val film = + Film( + id = idGenerator.generateId(), + title = command.title, + description = command.description, + ) return filmRepository.save(film) } - override fun edit(id: UUID, command: EditFilmCommand): Film { + override fun edit( + id: UUID, + command: EditFilmCommand, + ): Film { if (filmConfig.isBlocked(command.title)) { throw BlockedValueException(target = "Film", field = "title") } @@ -63,9 +66,8 @@ class FilmService( filmRepository.deleteById(id) } - override fun getById(id: UUID): Film { - return filmRepository.findById(id) ?: throw EntityNotFoundException(entity = "Film", id = id.toString()) - } + override fun getById(id: UUID): Film = + filmRepository.findById(id) ?: throw EntityNotFoundException(entity = "Film", id = id.toString()) override fun getAll(): List = filmRepository.findAll() diff --git a/src/test/kotlin/com/project/movienight/controllers/FilmControllerTest.kt b/src/test/kotlin/com/project/movienight/controllers/FilmControllerTest.kt index 0784280..18a952a 100644 --- a/src/test/kotlin/com/project/movienight/controllers/FilmControllerTest.kt +++ b/src/test/kotlin/com/project/movienight/controllers/FilmControllerTest.kt @@ -9,13 +9,16 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock import org.springframework.boot.test.context.SpringBootTest import org.springframework.http.MediaType import org.springframework.test.web.servlet.MockMvc -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.* -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.* +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status @SpringBootTest @AutoConfigureMockMvc class FilmControllerTest { - @Autowired private lateinit var mockMvc: MockMvc @@ -24,17 +27,18 @@ class FilmControllerTest { @Test fun `create film should return 201 CREATED`() { - val request = CreateFilmRequest( - title = "The Matrix", - description = "A computer hacker learns about the true nature of reality", - ) + val request = + CreateFilmRequest( + title = "The Matrix", + description = "A computer hacker learns about the true nature of reality", + ) - mockMvc.perform( - post("/api/films") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(request)) - ) - .andExpect(status().isCreated) + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request)), + ).andExpect(status().isCreated) .andExpect(jsonPath("$.title").value("The Matrix")) .andExpect(jsonPath("$.description").value("A computer hacker learns about the true nature of reality")) .andExpect(jsonPath("$.id").exists()) @@ -42,86 +46,95 @@ class FilmControllerTest { @Test fun `edit film should return updated film`() { - val createRequest = CreateFilmRequest( - title = "Old Title", - description = "Old Description" - ) + val createRequest = + CreateFilmRequest( + title = "Old Title", + description = "Old Description", + ) - val response = mockMvc.perform( - post("/api/films") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(createRequest)) - ).andReturn() + val response = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(createRequest)), + ).andReturn() val filmId = objectMapper.readTree(response.response.contentAsString).get("id").asText() - val editRequest = EditFilmRequest( - title = "New Title", - description = "New Description", - ) + val editRequest = + EditFilmRequest( + title = "New Title", + description = "New Description", + ) - mockMvc.perform( - patch("/api/films/$filmId") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(editRequest)) - ) - .andExpect(status().isOk) + mockMvc + .perform( + patch("/api/films/$filmId") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(editRequest)), + ).andExpect(status().isOk) .andExpect(jsonPath("$.title").value("New Title")) .andExpect(jsonPath("$.description").value("New Description")) } @Test fun `search film by title should return film`() { - val request = CreateFilmRequest( - title = "Inception", - description = "Dream within a dream" - ) + val request = + CreateFilmRequest( + title = "Inception", + description = "Dream within a dream", + ) mockMvc.perform( post("/api/films") .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(request)) + .content(objectMapper.writeValueAsString(request)), ) - mockMvc.perform( - get("/api/films/search") - .param("title", "Inception") - ) - .andExpect(status().isOk) + mockMvc + .perform( + get("/api/films/search") + .param("title", "Inception"), + ).andExpect(status().isOk) .andExpect(jsonPath("$.title").value("Inception")) .andExpect(jsonPath("$.description").value("Dream within a dream")) } @Test - fun `search film by non-existent title should return empty`() { - mockMvc.perform( - get("/api/films/search") - .param("title", "NonExistentFilm12345") - ) - .andExpect(status().isOk) - .andExpect(content().string("")) + fun `search film by non-existent title should return 404`() { + mockMvc + .perform( + get("/api/films/search") + .param("title", "NonExistentFilm12345"), + ).andExpect(status().isNotFound) } @Test fun `delete film should return 204 NO CONTENT`() { - val request = CreateFilmRequest( - title = "Film To Delete", - description = "This film will be deleted" - ) + val request = + CreateFilmRequest( + title = "Film To Delete", + description = "This film will be deleted", + ) - val response = mockMvc.perform( - post("/api/films") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(request)) - ).andReturn() + val response = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request)), + ).andReturn() val filmId = objectMapper.readTree(response.response.contentAsString).get("id").asText() - mockMvc.perform(delete("/api/films/$filmId")) + mockMvc + .perform(delete("/api/films/$filmId")) .andExpect(status().isNoContent()) - mockMvc.perform(get("/api/films/search").param("title", "Film To Delete")) - .andExpect(status().isOk) - .andExpect(content().string("")) + mockMvc + .perform( + get("/api/films/search").param("title", "Film To Delete"), + ).andExpect(status().isNotFound) } } diff --git a/src/test/kotlin/com/project/movienight/controllers/FilmLibraryControllerTest.kt b/src/test/kotlin/com/project/movienight/controllers/FilmLibraryControllerTest.kt index d24e60e..3bbc177 100644 --- a/src/test/kotlin/com/project/movienight/controllers/FilmLibraryControllerTest.kt +++ b/src/test/kotlin/com/project/movienight/controllers/FilmLibraryControllerTest.kt @@ -10,7 +10,9 @@ import org.springframework.boot.test.context.SpringBootTest import org.springframework.http.MediaType import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status import org.springframework.transaction.annotation.Transactional @@ -18,7 +20,6 @@ import org.springframework.transaction.annotation.Transactional @AutoConfigureMockMvc @Transactional class FilmLibraryControllerTest { - @Autowired private lateinit var mockMvc: MockMvc @@ -27,60 +28,177 @@ class FilmLibraryControllerTest { @Test fun `add film to library should work`() { - val userRequest = CreateUserRequest( - name = "Film Adder", - email = "adder@example.com", - ) - val userResponse = mockMvc.perform( - post("/api/users") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(userRequest)), - ).andReturn() + val userRequest = + CreateUserRequest( + name = "Film Adder", + email = "adder@example.com", + ) + val userResponse = + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(userRequest)), + ).andReturn() val userId = objectMapper.readTree(userResponse.response.contentAsString).get("id").asText() - val filmRequest = CreateFilmRequest( - title = "Library Film", - description = "Film description", - ) - val filmResponse = mockMvc.perform( - post("/api/films") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(filmRequest)), - ).andReturn() + val filmRequest = + CreateFilmRequest( + title = "Library Film", + description = "Film description", + ) + val filmResponse = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(filmRequest)), + ).andReturn() val filmId = objectMapper.readTree(filmResponse.response.contentAsString).get("id").asText() - mockMvc.perform( - post("/api/users/$userId/library/films/$filmId"), - ) - .andExpect(status().isCreated()) + mockMvc + .perform( + post("/api/users/$userId/library/films/$filmId"), + ).andExpect(status().isCreated()) } @Test fun `remove film from library should return 204`() { - val userRequest = CreateUserRequest( - name = "Remove Film", - email = "remove@example.com", - ) - val userResponse = mockMvc.perform( - post("/api/users") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(userRequest)), - ).andReturn() + val userRequest = + CreateUserRequest( + name = "Remove Film", + email = "remove@example.com", + ) + val userResponse = + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(userRequest)), + ).andReturn() val userId = objectMapper.readTree(userResponse.response.contentAsString).get("id").asText() - val filmRequest = CreateFilmRequest( - title = "Film To Remove", - description = "Will be removed", - ) - val filmResponse = mockMvc.perform( - post("/api/films") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(filmRequest)), - ).andReturn() + val filmRequest = + CreateFilmRequest( + title = "Film To Remove", + description = "Will be removed", + ) + val filmResponse = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(filmRequest)), + ).andReturn() val filmId = objectMapper.readTree(filmResponse.response.contentAsString).get("id").asText() mockMvc.perform(post("/api/users/$userId/library/films/$filmId")) - mockMvc.perform(delete("/api/users/$userId/library/films/$filmId")) + mockMvc + .perform(delete("/api/users/$userId/library/films/$filmId")) .andExpect(status().isNoContent()) } + + @Test + fun `get available films should exclude film in user's library`() { + val userRequest = + CreateUserRequest( + name = "Available Films User", + email = "availablefilms@example.com", + ) + val userResponse = + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(userRequest)), + ).andReturn() + val userId = objectMapper.readTree(userResponse.response.contentAsString).get("id").asText() + + val film1Request = + CreateFilmRequest( + title = "Film In Library", + description = "This will be in the library", + ) + val film1Response = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(film1Request)), + ).andReturn() + val film1Id = objectMapper.readTree(film1Response.response.contentAsString).get("id").asText() + + val film2Request = + CreateFilmRequest( + title = "Film Not In Library", + description = "This will not be in the library", + ) + val film2Response = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(film2Request)), + ).andReturn() + val film2Id = objectMapper.readTree(film2Response.response.contentAsString).get("id").asText() + + mockMvc.perform(post("/api/users/$userId/library/films/$film1Id")) + + val result = + mockMvc + .perform( + get("/api/users/$userId/library/available-films"), + ).andExpect(status().isOk) + .andReturn() + + val responseBody = result.response.contentAsString + val films = objectMapper.readTree(responseBody) + val returnedIds = (0 until films.size()).map { films[it].get("id").asText() } + assert(!returnedIds.contains(film1Id)) { "Film in library should not appear in available films" } + assert(returnedIds.contains(film2Id)) { "Film not in library should appear in available films" } + } + + @Test + fun `get available films for user without library returns all films`() { + val userRequest = + CreateUserRequest( + name = "No Library User", + email = "nolibrary@example.com", + ) + val userResponse = + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(userRequest)), + ).andReturn() + val userId = objectMapper.readTree(userResponse.response.contentAsString).get("id").asText() + + val filmRequest = + CreateFilmRequest( + title = "Available Film", + description = "Should appear in available films", + ) + val filmResponse = + mockMvc + .perform( + post("/api/films") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(filmRequest)), + ).andReturn() + val filmId = objectMapper.readTree(filmResponse.response.contentAsString).get("id").asText() + + val result = + mockMvc + .perform( + get("/api/users/$userId/library/available-films"), + ).andExpect(status().isOk) + .andExpect(jsonPath("$[*].id").isArray) + .andReturn() + + val responseBody = result.response.contentAsString + val films = objectMapper.readTree(responseBody) + val returnedIds = (0 until films.size()).map { films[it].get("id").asText() } + assert(returnedIds.contains(filmId)) { "Film should appear in available films when user has no library" } + } } diff --git a/src/test/kotlin/com/project/movienight/controllers/UserControllerTest.kt b/src/test/kotlin/com/project/movienight/controllers/UserControllerTest.kt index db2eecc..7a2dca3 100644 --- a/src/test/kotlin/com/project/movienight/controllers/UserControllerTest.kt +++ b/src/test/kotlin/com/project/movienight/controllers/UserControllerTest.kt @@ -20,7 +20,6 @@ import org.springframework.transaction.annotation.Transactional @AutoConfigureMockMvc @Transactional class UserControllerTest { - @Autowired private lateinit var mockMvc: MockMvc @@ -29,17 +28,18 @@ class UserControllerTest { @Test fun `create user should return 201 CREATED`() { - val request = CreateUserRequest( - name = "John Doe", - email = "john@example.com", - ) + val request = + CreateUserRequest( + name = "John Doe", + email = "john@example.com", + ) - mockMvc.perform( - post("/api/users") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(request)), - ) - .andExpect(status().isCreated) + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request)), + ).andExpect(status().isCreated) .andExpect(jsonPath("$.name").value("John Doe")) .andExpect(jsonPath("$.email").value("john@example.com")) .andExpect(jsonPath("$.id").exists()) @@ -47,54 +47,62 @@ class UserControllerTest { @Test fun `edit user should return updated user`() { - val createRequest = CreateUserRequest( - name = "Old Name", - email = "edit@example.com", - ) + val createRequest = + CreateUserRequest( + name = "Old Name", + email = "edit@example.com", + ) - val response = mockMvc.perform( - post("/api/users") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(createRequest)), - ).andReturn() + val response = + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(createRequest)), + ).andReturn() val userId = objectMapper.readTree(response.response.contentAsString).get("id").asText() val editRequest = EditUserRequest(name = "New Name") - mockMvc.perform( - patch("/api/users/$userId") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(editRequest)), - ) - .andExpect(status().isOk) + mockMvc + .perform( + patch("/api/users/$userId") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(editRequest)), + ).andExpect(status().isOk) .andExpect(jsonPath("$.name").value("New Name")) .andExpect(jsonPath("$.email").value("edit@example.com")) } @Test fun `delete user should return 204 NO CONTENT`() { - val request = CreateUserRequest( - name = "User To Delete", - email = "delete@example.com", - ) + val request = + CreateUserRequest( + name = "User To Delete", + email = "delete@example.com", + ) - val response = mockMvc.perform( - post("/api/users") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(request)), - ).andReturn() + val response = + mockMvc + .perform( + post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request)), + ).andReturn() val userId = objectMapper.readTree(response.response.contentAsString).get("id").asText() - mockMvc.perform(delete("/api/users/$userId")) + mockMvc + .perform(delete("/api/users/$userId")) .andExpect(status().isNoContent()) } @Test fun `delete non-existent user should return 404`() { val nonExistentId = "123e4567-e89b-12d3-a456-426614174000" - mockMvc.perform(delete("/api/users/$nonExistentId")) + mockMvc + .perform(delete("/api/users/$nonExistentId")) .andExpect(status().isNotFound()) } }