fix(api): mark PlayerDTO.id read-only, guard null squadNumber (#268)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: nanotaboada

src/main/java/ar/com/nanotaboada/java/samples/spring/boot/models/PlayerDTO.java

@@ -2,13 +2,15 @@
 
 import java.time.LocalDate;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateDeserializer;
 import com.fasterxml.jackson.datatype.jsr310.ser.LocalDateSerializer;
 
 import java.util.UUID;
 
+import io.swagger.v3.oas.annotations.media.Schema;
 import jakarta.validation.constraints.NotBlank;
 import jakarta.validation.constraints.NotNull;
 import jakarta.validation.constraints.Past;
@@ -48,6 +50,8 @@
  */
 @Data
 public class PlayerDTO {
+    @JsonProperty(access = JsonProperty.Access.READ_ONLY)
+    @Schema(accessMode = Schema.AccessMode.READ_ONLY)
     private UUID id;
     @NotBlank
     private String firstName;

src/main/java/ar/com/nanotaboada/java/samples/spring/boot/services/PlayersService.java

@@ -111,6 +111,7 @@ public PlayerDTO create(PlayerDTO playerDTO) {
      *
      * @return a list of all players (empty list if none found)
      */
+    @Transactional(readOnly = true)
     @Cacheable(value = "players")
     public List<PlayerDTO> retrieveAll() {
         return playersRepository.findAll()
@@ -128,6 +129,7 @@ public List<PlayerDTO> retrieveAll() {
      * @param id the UUID primary key (must not be null)
      * @return the player DTO if found, null otherwise
      */
+    @Transactional(readOnly = true)
     @Cacheable(value = "players", key = "#id", unless = "#result == null")
     public PlayerDTO retrieveById(UUID id) {
         return playersRepository.findById(id)
@@ -144,6 +146,7 @@ public PlayerDTO retrieveById(UUID id) {
      * @param squadNumber the squad number to retrieve (jersey number, typically 1-99)
      * @return the player DTO if found, null otherwise
      */
+    @Transactional(readOnly = true)
     @Cacheable(value = "players", key = "'squad-' + #squadNumber", unless = "#result == null")
     public PlayerDTO retrieveBySquadNumber(Integer squadNumber) {
         return playersRepository.findBySquadNumber(squadNumber)
@@ -163,6 +166,7 @@ public PlayerDTO retrieveBySquadNumber(Integer squadNumber) {
      * @param league the league name to search for (must not be null or blank)
      * @return a list of matching players (empty list if none found)
      */
+    @Transactional(readOnly = true)
     public List<PlayerDTO> searchByLeague(String league) {
         return playersRepository.findByLeagueContainingIgnoreCase(league)
                 .stream()
@@ -192,6 +196,11 @@ public List<PlayerDTO> searchByLeague(String league) {
     public boolean update(Integer squadNumber, PlayerDTO playerDTO) {
         log.debug("Updating player with squad number: {}", squadNumber);
 
+        if (squadNumber == null) {
+            log.warn("Cannot update player - squad number is null");
+            return false;
+        }
+
         return playersRepository.findBySquadNumber(squadNumber)
                 .map(existing -> {
                     Player player = mapFrom(playerDTO);

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/controllers/PlayersControllerTests.java

@@ -186,7 +186,8 @@ void givenPlayersExist_whenGetAll_thenReturnsOkWithAllPlayers()
         then(response.getContentType()).contains("application/json");
         verify(playersServiceMock, times(1)).retrieveAll();
         then(response.getStatus()).isEqualTo(HttpStatus.OK.value());
-        then(actual).usingRecursiveComparison().isEqualTo(expected);
+        expected.forEach(player -> then(content).contains(player.getId().toString()));
+        then(actual).usingRecursiveComparison().ignoringFields("id").isEqualTo(expected);
     }
 
     /**
@@ -216,7 +217,8 @@ void givenPlayerExists_whenGetById_thenReturnsOk()
         then(response.getContentType()).contains("application/json");
         verify(playersServiceMock, times(1)).retrieveById(id);
         then(response.getStatus()).isEqualTo(HttpStatus.OK.value());
-        then(actual).usingRecursiveComparison().isEqualTo(expected);
+        then(content).contains(expected.getId().toString());
+        then(actual).usingRecursiveComparison().ignoringFields("id").isEqualTo(expected);
     }
 
     /**
@@ -274,7 +276,8 @@ void givenPlayerExists_whenGetBySquadNumber_thenReturnsOk()
         then(response.getContentType()).contains("application/json");
         verify(playersServiceMock, times(1)).retrieveBySquadNumber(squadNumber);
         then(response.getStatus()).isEqualTo(HttpStatus.OK.value());
-        then(actual).usingRecursiveComparison().isEqualTo(expected);
+        then(content).contains(expected.getId().toString());
+        then(actual).usingRecursiveComparison().ignoringFields("id").isEqualTo(expected);
         then(actual.getSquadNumber()).isEqualTo(squadNumber);
     }
 
@@ -333,7 +336,8 @@ void givenPlayersExist_whenSearchByLeague_thenReturnsOk()
         then(response.getContentType()).contains("application/json");
         verify(playersServiceMock, times(1)).searchByLeague(any());
         then(response.getStatus()).isEqualTo(HttpStatus.OK.value());
-        then(actual).usingRecursiveComparison().isEqualTo(expected);
+        expected.forEach(player -> then(content).contains(player.getId().toString()));
+        then(actual).usingRecursiveComparison().ignoringFields("id").isEqualTo(expected);
     }
 
     /**

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/services/PlayersServiceTests.java

@@ -380,12 +380,10 @@ void givenPlayerDoesNotExist_whenUpdate_thenReturnsFalse() {
     void givenNullSquadNumber_whenUpdate_thenReturnsFalse() {
         // Given
         PlayerDTO dto = PlayerDTOFakes.createOneValid();
-        Mockito
-                .when(playersRepositoryMock.findBySquadNumber(null))
-                .thenReturn(Optional.empty());
         // When
         boolean actual = playersService.update(null, dto);
         // Then
+        verify(playersRepositoryMock, never()).findBySquadNumber(any());
         verify(playersRepositoryMock, never()).save(any(Player.class));
         verify(modelMapperMock, never()).map(any(), any());
         then(actual).isFalse();

src/test/resources/ddl.sql

@@ -5,7 +5,7 @@
 DROP TABLE IF EXISTS players;
 
 CREATE TABLE players (
-    id          VARCHAR(36)  PRIMARY KEY,
+    id          VARCHAR(36)  NOT NULL PRIMARY KEY,
     squadNumber INTEGER      NOT NULL UNIQUE,
     firstName   TEXT         NOT NULL,
     middleName  TEXT,