diff --git a/readme.md b/readme.md index a5b57b4..6982ba6 100644 --- a/readme.md +++ b/readme.md @@ -107,7 +107,8 @@ method calling another service's method that needs more. | Method | Path | Auth | |---|---|---| | `GET` | `/api/library/games`, `/api/library/games/{appid}` | JWT | -| `PUT` | `/api/library/games/{appid}/backlog` | JWT | +| `PATCH` | `/api/library/games/{appid}/backlog` (only the sent fields; `null` clears one) | JWT | +| `PUT` | `/api/library/games/{appid}/backlog` (replaces all four; removed in item 29) | JWT | | `PUT`, `DELETE` | `/api/library/games/{appid}/corrections/{field}` (`{value}`; `name`, `image`, `speedrun`, `moods`, `endless`, `genres`; `204`, `400 INVALID_CORRECTION`) | JWT, AUDITOR | | `POST` | `/api/library/games/{appid}/merge` (`{otherAppid}`; `200 {appid}` = the key that stays, `400 SAME_GAME` / `STEAM_GAMES`, `404 GAME_NOT_FOUND`) | JWT, AUDITOR | | `PUT` | `/api/library/games/{appid}/status` (status only, keeps rating, notes, priority) | JWT | diff --git a/src/main/java/core_service/configuration/security/SecurityConfig.java b/src/main/java/core_service/configuration/security/SecurityConfig.java index e8c5596..50c5cd1 100644 --- a/src/main/java/core_service/configuration/security/SecurityConfig.java +++ b/src/main/java/core_service/configuration/security/SecurityConfig.java @@ -53,7 +53,7 @@ public class SecurityConfig { CorsConfiguration configuration = new CorsConfiguration(); // Exact origins only: with credentials, a pattern would send the login cookie to every matching site. configuration.setAllowedOrigins(Arrays.asList(allowedOrigins)); - configuration.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "OPTIONS")); + configuration.setAllowedMethods(List.of("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS")); configuration.setAllowedHeaders(List.of( "Content-Type", "Authorization", ClientHeaderFilter.CLIENT_HEADER, "X-Profile-Ticket")); configuration.setAllowCredentials(true); diff --git a/src/main/java/core_service/modules/library/api/LibraryController.java b/src/main/java/core_service/modules/library/api/LibraryController.java index 510c21f..fe39546 100644 --- a/src/main/java/core_service/modules/library/api/LibraryController.java +++ b/src/main/java/core_service/modules/library/api/LibraryController.java @@ -4,6 +4,7 @@ import core_service.shared.api.ErrorResponse; import core_service.modules.library.api.dto.AchievementSummaryDto; import core_service.modules.library.api.dto.ActivityDto; import core_service.modules.library.api.dto.BacklogEntryDto; +import core_service.modules.library.api.dto.BacklogPatch; import core_service.modules.library.api.dto.BrokenImageReport; import core_service.modules.library.api.dto.CorrectionDto; import core_service.modules.library.api.dto.FinishedHintDismissal; @@ -47,6 +48,7 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -231,6 +233,15 @@ public class LibraryController { .body(GAME_NOT_FOUND)); } + @PatchMapping("/games/{appid}/backlog") + public ResponseEntity patchBacklog(@AuthenticationPrincipal UserPrincipal principal, + @PathVariable("appid") Long appid, + @Valid @RequestBody BacklogPatch patch) { + return backlogService.patch(principal.getIdAsLong(), appid, patch) + .>map(ResponseEntity::ok) + .orElseGet(LibraryController::unknownGame); + } + @PutMapping("/games/{appid}/hidden") public ResponseEntity updateHidden(@AuthenticationPrincipal UserPrincipal principal, @PathVariable("appid") Long appid, diff --git a/src/main/java/core_service/modules/library/api/dto/BacklogEntryDto.java b/src/main/java/core_service/modules/library/api/dto/BacklogEntryDto.java index 86a3454..90faed0 100644 --- a/src/main/java/core_service/modules/library/api/dto/BacklogEntryDto.java +++ b/src/main/java/core_service/modules/library/api/dto/BacklogEntryDto.java @@ -9,6 +9,7 @@ import jakarta.validation.constraints.Size; /** * Request and response of the backlog update. The request replaces all four fields; null clears an optional one. + * The PUT goes away in item 29; the Frontend sends a {@link BacklogPatch}. */ public record BacklogEntryDto( @NotNull GameStatus status, diff --git a/src/main/java/core_service/modules/library/api/dto/BacklogPatch.java b/src/main/java/core_service/modules/library/api/dto/BacklogPatch.java new file mode 100644 index 0000000..39417bc --- /dev/null +++ b/src/main/java/core_service/modules/library/api/dto/BacklogPatch.java @@ -0,0 +1,26 @@ +package core_service.modules.library.api.dto; + +import core_service.modules.library.domain.GamePriority; +import core_service.modules.library.domain.GameStatus; +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; +import lombok.Getter; +import lombok.Setter; + +import java.util.Optional; + +/** + * The backlog fields the request sends; a field it leaves out stays as it is. A field is null when it is not in the + * JSON and an empty Optional when it is null there, which clears it. + * A class, not a record: for a record Jackson gives an empty Optional for a missing field too. + */ +@Getter +@Setter +public class BacklogPatch { + private Optional<@NotNull GameStatus> status; + private Optional<@Min(1) @Max(5) Integer> rating; + private Optional<@Size(max = 2000) String> notes; + private Optional priority; +} diff --git a/src/main/java/core_service/modules/library/service/BacklogService.java b/src/main/java/core_service/modules/library/service/BacklogService.java index 0a144b5..905850b 100644 --- a/src/main/java/core_service/modules/library/service/BacklogService.java +++ b/src/main/java/core_service/modules/library/service/BacklogService.java @@ -1,6 +1,7 @@ package core_service.modules.library.service; import core_service.modules.library.api.dto.BacklogEntryDto; +import core_service.modules.library.api.dto.BacklogPatch; import core_service.modules.library.domain.FinishedHintType; import core_service.modules.library.domain.GameSource; import core_service.modules.library.domain.GameStatus; @@ -40,6 +41,28 @@ public class BacklogService { }); } + /** + * Changes only the fields the patch sends. Only a sent status counts as set by the user, so a save of the notes + * alone still lets the sync move the game from BACKLOG to PLAYING. + * + * @return empty when the game is not in the user's active library + */ + @Transactional + public Optional patch(Long userId, Long appid, BacklogPatch patch) { + return userGameRepository.findById(new UserGameId(userId, appid)) + .filter(game -> game.getRemovedAt() == null) + .map(game -> { + if (patch.getStatus() != null) { + game.setStatus(patch.getStatus().orElseThrow()); + game.setStatusSetByUser(true); + } + if (patch.getRating() != null) game.setRating(patch.getRating().orElse(null)); + if (patch.getNotes() != null) game.setNotes(patch.getNotes().filter(notes -> !notes.isBlank()).orElse(null)); + if (patch.getPriority() != null) game.setPriority(patch.getPriority().orElse(null)); + return toDto(game); + }); + } + /** * Changes only the status and keeps rating, notes and priority. * diff --git a/src/test/java/core_service/configuration/security/ClientHeaderFilterTest.java b/src/test/java/core_service/configuration/security/ClientHeaderFilterTest.java index b662aec..537474d 100644 --- a/src/test/java/core_service/configuration/security/ClientHeaderFilterTest.java +++ b/src/test/java/core_service/configuration/security/ClientHeaderFilterTest.java @@ -48,6 +48,13 @@ class ClientHeaderFilterTest extends AbstractIntegrationTest { .andExpect(header().string("Access-Control-Allow-Origin", FRONTEND)) .andExpect(header().string("Access-Control-Allow-Credentials", "true")); + mockMvc.perform(options("/api/library/games/220/backlog") + .header("Origin", FRONTEND) + .header("Access-Control-Request-Method", "PATCH") + .header("Access-Control-Request-Headers", "x-tbp-client,content-type")) + .andExpect(status().isOk()) + .andExpect(header().string("Access-Control-Allow-Origin", FRONTEND)); + mockMvc.perform(options("/api/library/sync") .header("Origin", "https://evil.andreas-dahm.eu") .header("Access-Control-Request-Method", "POST")) diff --git a/src/test/java/core_service/modules/library/api/LibraryControllerBacklogTest.java b/src/test/java/core_service/modules/library/api/LibraryControllerBacklogTest.java index e1540e7..bf246e8 100644 --- a/src/test/java/core_service/modules/library/api/LibraryControllerBacklogTest.java +++ b/src/test/java/core_service/modules/library/api/LibraryControllerBacklogTest.java @@ -17,11 +17,15 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.ResultActions; import java.time.Instant; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -138,6 +142,87 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest { .andExpect(status().isNotFound()); } + private ResultActions patchBacklog(long appid, String body) throws Exception { + return mockMvc.perform(patch("/api/library/games/" + appid + "/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test") + .contentType(MediaType.APPLICATION_JSON).content(body)); + } + + private void storeAllFields() { + UserGame game = stored(); + game.setRating(4); + game.setNotes("old notes"); + game.setPriority(GamePriority.HIGH); + userGameRepository.save(game); + } + + @Test + void patchBacklog_OnlyNotes_KeepsTheOtherFieldsAndTheAutomaticStatus() throws Exception { + storeAllFields(); + + patchBacklog(APPID, "{\"notes\":\"new notes\"}") + .andExpect(status().isOk()) + .andExpect(jsonPath("$.notes").value("new notes")) + .andExpect(jsonPath("$.rating").value(4)); + + UserGame game = stored(); + assertEquals("new notes", game.getNotes()); + assertEquals(4, game.getRating()); + assertEquals(GamePriority.HIGH, game.getPriority()); + assertEquals(GameStatus.BACKLOG, game.getStatus()); + assertFalse(game.isStatusSetByUser()); + } + + @Test + void patchBacklog_Status_IsSetByTheUser() throws Exception { + patchBacklog(APPID, "{\"status\":\"PLAYING\"}").andExpect(status().isOk()); + + assertEquals(GameStatus.PLAYING, stored().getStatus()); + assertTrue(stored().isStatusSetByUser()); + } + + @Test + void patchBacklog_NullClearsAnOptionalFieldAndBlankNotesAreNull() throws Exception { + storeAllFields(); + + patchBacklog(APPID, "{\"rating\":null,\"priority\":null,\"notes\":\" \"}").andExpect(status().isOk()); + + UserGame game = stored(); + assertNull(game.getRating()); + assertNull(game.getPriority()); + assertNull(game.getNotes()); + } + + @ParameterizedTest + @ValueSource(strings = { + "{\"status\":null}", + "{\"status\":\"WISHLIST\"}", + "{\"rating\":0}", + "{\"rating\":6}", + "{\"rating\":4.7}", + "{\"priority\":\"URGENT\"}"}) + void patchBacklog_InvalidBody_Is400AndStoresNothing(String body) throws Exception { + storeAllFields(); + + patchBacklog(APPID, body).andExpect(status().isBadRequest()); + + UserGame game = stored(); + assertEquals(GameStatus.BACKLOG, game.getStatus()); + assertEquals(4, game.getRating()); + assertEquals(GamePriority.HIGH, game.getPriority()); + } + + @Test + void patchBacklog_NotesTooLong_Is400() throws Exception { + patchBacklog(APPID, "{\"notes\":\"" + "x".repeat(2001) + "\"}").andExpect(status().isBadRequest()); + } + + @Test + void patchBacklog_GameNotOwned_Is404() throws Exception { + patchBacklog(999L, "{\"notes\":\"x\"}") + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.code").value("GAME_NOT_FOUND")); + } + @Test void updateStatus_KeepsRatingNotesAndPriority() throws Exception { UserGame game = stored();