fix(tests): correct data inconsistencies and improve test configuration (#248)

Data Corrections:
- Fix Di María position abbreviation (LW → RW to match "Right Winger")
- Correct team name typos (Villareal → Villarreal, Nottingham Forrest → Nottingham Forest)
- Apply corrections consistently across PlayerDTOFakes, PlayerFakes, dml.sql, and production database

Configuration Improvements:
- Add explicit Hibernate dialect (spring.jpa.database-platform=org.hibernate.community.dialect.SQLiteDialect) to test application.properties for consistency with production

Documentation:
- Update PlayersService class-level JavaDoc to accurately document @CacheEvict(allEntries=true) cache strategy
- Add explanation of why allEntries=true is needed (retrieveAll() caches full list under single key)

Addresses code review feedback ensuring test data accuracy and environment consistency.

Author: nanotaboada

AGENTS.md

@@ -48,7 +48,7 @@ open target/site/jacoco/index.html
 ./mvnw test -Dtest=PlayersControllerTests
 
 # Run specific test method
-./mvnw test -Dtest=PlayersControllerTests#givenGetAll_whenServiceRetrieveAllReturnsPlayers_thenResponseIsOkAndResultIsPlayers
+./mvnw test -Dtest=PlayersControllerTests#getAll_playersExist_returnsOkWithAllPlayers
 
 # Run tests without rebuilding
 ./mvnw surefire:test
@@ -99,7 +99,7 @@ java -jar target/java.samples.spring.boot-*.jar
 
 ### Database Management
 
-This project uses **H2 in-memory database for tests** and **SQLite for runtime**.
+This project uses **SQLite in-memory database for tests** and **SQLite for runtime**.
 
 **Runtime (SQLite)**:
 
@@ -115,9 +115,9 @@ rm storage/players-sqlite3.db
 # Database location: storage/players-sqlite3.db
 ```
 
-**Tests (H2)**:
+**Tests (SQLite)**:
 
-- In-memory database per test run
+- In-memory database per test run (jdbc:sqlite::memory:)
 - Automatically cleared after each test
 - Configuration in `src/test/resources/application.properties`
 
@@ -281,7 +281,7 @@ rm storage/players-sqlite3.db
 ./mvnw test -X
 
 # Run single test for debugging
-./mvnw test -Dtest=PlayersControllerTests#givenGetAll_whenServiceRetrieveAllReturnsPlayers_thenResponseIsOkAndResultIsPlayers -X
+./mvnw test -Dtest=PlayersControllerTests#getAll_playersExist_returnsOkWithAllPlayers -X
 ```
 
 ### Maven wrapper issues

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

@@ -8,6 +8,7 @@
 import com.fasterxml.jackson.datatype.jsr310.ser.LocalDateSerializer;
 
 import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
 import jakarta.validation.constraints.Past;
 import jakarta.validation.constraints.Positive;
 import lombok.Data;
@@ -25,8 +26,8 @@
  * <ul>
  * <li><b>firstName:</b> Required (not blank)</li>
  * <li><b>lastName:</b> Required (not blank)</li>
- * <li><b>dateOfBirth:</b> Must be a past date</li>
- * <li><b>squadNumber:</b> Must be a positive integer</li>
+ * <li><b>dateOfBirth:</b> Required (not null) and must be a past date</li>
+ * <li><b>squadNumber:</b> Required (not null) and must be a positive integer</li>
  * <li><b>position:</b> Required (not blank)</li>
  * <li><b>team:</b> Required (not blank)</li>
  * </ul>
@@ -38,6 +39,7 @@
  *
  * @see Player
  * @see jakarta.validation.constraints.NotBlank
+ * @see jakarta.validation.constraints.NotNull
  * @see jakarta.validation.constraints.Past
  * @see jakarta.validation.constraints.Positive
  * @since 4.0.2025
@@ -50,10 +52,12 @@ public class PlayerDTO {
     private String middleName;
     @NotBlank
     private String lastName;
+    @NotNull
     @Past
     @JsonDeserialize(using = LocalDateDeserializer.class)
     @JsonSerialize(using = LocalDateSerializer.class)
     private LocalDate dateOfBirth;
+    @NotNull
     @Positive
     private Integer squadNumber;
     @NotBlank

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

@@ -5,9 +5,9 @@
 
 import org.modelmapper.ModelMapper;
 import org.springframework.cache.annotation.CacheEvict;
-import org.springframework.cache.annotation.CachePut;
 import org.springframework.cache.annotation.Cacheable;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
 
 import ar.com.nanotaboada.java.samples.spring.boot.models.Player;
 import ar.com.nanotaboada.java.samples.spring.boot.models.PlayerDTO;
@@ -31,10 +31,16 @@
  * <h3>Cache Strategy:</h3>
  * <ul>
  * <li><b>@Cacheable:</b> Retrieval operations (read-through cache)</li>
- * <li><b>@CachePut:</b> Create operations (update cache)</li>
- * <li><b>@CacheEvict:</b> Update/Delete operations (invalidate cache)</li>
+ * <li><b>@CacheEvict(allEntries=true):</b> Mutating operations (create/update/delete) - invalidates entire cache to maintain
+ * consistency</li>
  * </ul>
  *
+ * <p>
+ * <b>Why invalidate all entries?</b> The {@code retrieveAll()} method caches the full player list under a single key.
+ * Individual cache evictions would leave this list stale. Using {@code allEntries=true} ensures both individual
+ * player caches and the list cache stay synchronized after any data modification.
+ * </p>
+ *
  * @see PlayersRepository
  * @see PlayerDTO
  * @see Player
@@ -67,9 +73,10 @@ public class PlayersService {
      *
      * @param playerDTO the player data to create (must not be null)
      * @return the created player with auto-generated ID, or null if squad number already exists
-     * @see org.springframework.cache.annotation.CachePut
+     * @see org.springframework.cache.annotation.CacheEvict
      */
-    @CachePut(value = "players", key = "#result.id", unless = "#result == null")
+    @Transactional
+    @CacheEvict(value = "players", allEntries = true)
     public PlayerDTO create(PlayerDTO playerDTO) {
         // Check if squad number already exists
         if (playersRepository.findBySquadNumber(playerDTO.getSquadNumber()).isPresent()) {
@@ -175,9 +182,10 @@ public PlayerDTO searchBySquadNumber(Integer squadNumber) {
      *
      * @param playerDTO the player data to update (must include a valid ID)
      * @return true if the player was updated successfully, false if not found
-     * @see org.springframework.cache.annotation.CachePut
+     * @see org.springframework.cache.annotation.CacheEvict
      */
-    @CachePut(value = "players", key = "#playerDTO.id")
+    @Transactional
+    @CacheEvict(value = "players", allEntries = true)
     public boolean update(PlayerDTO playerDTO) {
         if (playerDTO.getId() != null && playersRepository.existsById(playerDTO.getId())) {
             Player player = mapFrom(playerDTO);
@@ -205,7 +213,8 @@ public boolean update(PlayerDTO playerDTO) {
      * @return true if the player was deleted successfully, false if not found
      * @see org.springframework.cache.annotation.CacheEvict
      */
-    @CacheEvict(value = "players", key = "#id")
+    @Transactional
+    @CacheEvict(value = "players", allEntries = true)
     public boolean delete(Long id) {
         if (playersRepository.existsById(id)) {
             playersRepository.deleteById(id);

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/PlayerDTOFakes.java

@@ -140,7 +140,7 @@ public static java.util.List<PlayerDTO> createAll() {
                 createPlayerDTOWithId(5L, "Nicolás", "Alejandro", "Tagliafico", LocalDate.of(1992, 8, 31), 3,
                         "Left-Back", "LB", "Olympique Lyon", "Ligue 1", true),
                 createPlayerDTOWithId(6L, "Ángel", "Fabián", "Di María", LocalDate.of(1988, 2, 14), 11, "Right Winger",
-                        "LW", "SL Benfica", "Liga Portugal", true),
+                        "RW", "SL Benfica", "Liga Portugal", true),
                 createPlayerDTOWithId(7L, "Rodrigo", "Javier", "de Paul", LocalDate.of(1994, 5, 24), 7,
                         "Central Midfield", "CM", "Atlético Madrid", "La Liga", true),
                 createPlayerDTOWithId(8L, "Enzo", "Jeremías", "Fernández", LocalDate.of(2001, 1, 17), 24,
@@ -157,9 +157,9 @@ public static java.util.List<PlayerDTO> createAll() {
                 createPlayerDTOWithId(13L, "Gerónimo", null, "Rulli", LocalDate.of(1992, 5, 20), 12, "Goalkeeper", "GK",
                         "Ajax Amsterdam", "Eredivisie", false),
                 createPlayerDTOWithId(14L, "Juan", "Marcos", "Foyth", LocalDate.of(1998, 1, 12), 2, "Right-Back", "RB",
-                        "Villareal", "La Liga", false),
+                        "Villarreal", "La Liga", false),
                 createPlayerDTOWithId(15L, "Gonzalo", "Ariel", "Montiel", LocalDate.of(1997, 1, 1), 4, "Right-Back",
-                        "RB", "Nottingham Forrest", "Premier League", false),
+                        "RB", "Nottingham Forest", "Premier League", false),
                 createPlayerDTOWithId(16L, "Germán", "Alejo", "Pezzella", LocalDate.of(1991, 6, 27), 6, "Centre-Back",
                         "CB", "Real Betis Balompié", "La Liga", false),
                 createPlayerDTOWithId(17L, "Marcos", "Javier", "Acuña", LocalDate.of(1991, 10, 28), 8, "Left-Back",

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/PlayerFakes.java

@@ -143,7 +143,7 @@ public static java.util.List<Player> createAll() {
                 createPlayerWithId(5L, "Nicolás", "Alejandro", "Tagliafico", LocalDate.of(1992, 8, 31), 3, "Left-Back",
                         "LB", "Olympique Lyon", "Ligue 1", true),
                 createPlayerWithId(6L, "Ángel", "Fabián", "Di María", LocalDate.of(1988, 2, 14), 11, "Right Winger",
-                        "LW", "SL Benfica", "Liga Portugal", true),
+                        "RW", "SL Benfica", "Liga Portugal", true),
                 createPlayerWithId(7L, "Rodrigo", "Javier", "de Paul", LocalDate.of(1994, 5, 24), 7, "Central Midfield",
                         "CM", "Atlético Madrid", "La Liga", true),
                 createPlayerWithId(8L, "Enzo", "Jeremías", "Fernández", LocalDate.of(2001, 1, 17), 24,
@@ -160,9 +160,9 @@ public static java.util.List<Player> createAll() {
                 createPlayerWithId(13L, "Gerónimo", null, "Rulli", LocalDate.of(1992, 5, 20), 12, "Goalkeeper", "GK",
                         "Ajax Amsterdam", "Eredivisie", false),
                 createPlayerWithId(14L, "Juan", "Marcos", "Foyth", LocalDate.of(1998, 1, 12), 2, "Right-Back", "RB",
-                        "Villareal", "La Liga", false),
+                        "Villarreal", "La Liga", false),
                 createPlayerWithId(15L, "Gonzalo", "Ariel", "Montiel", LocalDate.of(1997, 1, 1), 4, "Right-Back", "RB",
-                        "Nottingham Forrest", "Premier League", false),
+                        "Nottingham Forest", "Premier League", false),
                 createPlayerWithId(16L, "Germán", "Alejo", "Pezzella", LocalDate.of(1991, 6, 27), 6, "Centre-Back",
                         "CB", "Real Betis Balompié", "La Liga", false),
                 createPlayerWithId(17L, "Marcos", "Javier", "Acuña", LocalDate.of(1991, 10, 28), 8, "Left-Back", "LB",

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

@@ -325,6 +325,25 @@ void update_playerNotFound_returnsFalse() {
         assertThat(result).isFalse();
     }
 
+    /**
+     * Given a PlayerDTO has null ID
+     * When update() is called
+     * Then false is returned without checking repository or saving
+     */
+    @Test
+    void update_nullId_returnsFalse() {
+        // Arrange
+        PlayerDTO playerDTO = PlayerDTOFakes.createOneValid();
+        playerDTO.setId(null);
+        // Act
+        boolean result = playersService.update(playerDTO);
+        // Assert
+        verify(playersRepositoryMock, never()).existsById(any());
+        verify(playersRepositoryMock, never()).save(any(Player.class));
+        verify(modelMapperMock, never()).map(any(), any());
+        assertThat(result).isFalse();
+    }
+
     /*
      * -----------------------------------------------------------------------------------------------------------------------
      * Delete

src/test/resources/application.properties

@@ -6,6 +6,7 @@
 # SQLite In-Memory Database Configuration (Test Only)
 spring.datasource.url=jdbc:sqlite::memory:
 spring.datasource.driver-class-name=org.sqlite.JDBC
+spring.jpa.database-platform=org.hibernate.community.dialect.SQLiteDialect
 spring.jpa.hibernate.ddl-auto=none
 spring.jpa.hibernate.naming.physical-strategy=org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl
 spring.sql.init.mode=always

src/test/resources/dml.sql

@@ -11,7 +11,7 @@ INSERT INTO players (id, firstName, middleName, lastName, dateOfBirth, squadNumb
 (3, 'Cristian', 'Gabriel', 'Romero', '1998-04-27T00:00:00.000Z', 13, 'Centre-Back', 'CB', 'Tottenham Hotspur', 'Premier League', 1),
 (4, 'Nicolás', 'Hernán Gonzalo', 'Otamendi', '1988-02-12T00:00:00.000Z', 19, 'Centre-Back', 'CB', 'SL Benfica', 'Liga Portugal', 1),
 (5, 'Nicolás', 'Alejandro', 'Tagliafico', '1992-08-31T00:00:00.000Z', 3, 'Left-Back', 'LB', 'Olympique Lyon', 'Ligue 1', 1),
-(6, 'Ángel', 'Fabián', 'Di María', '1988-02-14T00:00:00.000Z', 11, 'Right Winger', 'LW', 'SL Benfica', 'Liga Portugal', 1),
+(6, 'Ángel', 'Fabián', 'Di María', '1988-02-14T00:00:00.000Z', 11, 'Right Winger', 'RW', 'SL Benfica', 'Liga Portugal', 1),
 (7, 'Rodrigo', 'Javier', 'de Paul', '1994-05-24T00:00:00.000Z', 7, 'Central Midfield', 'CM', 'Atlético Madrid', 'La Liga', 1),
 (8, 'Enzo', 'Jeremías', 'Fernández', '2001-01-17T00:00:00.000Z', 24, 'Central Midfield', 'CM', 'Chelsea FC', 'Premier League', 1),
 (9, 'Alexis', NULL, 'Mac Allister', '1998-12-24T00:00:00.000Z', 20, 'Central Midfield', 'CM', 'Liverpool FC', 'Premier League', 1),
@@ -22,8 +22,8 @@ INSERT INTO players (id, firstName, middleName, lastName, dateOfBirth, squadNumb
 INSERT INTO players (id, firstName, middleName, lastName, dateOfBirth, squadNumber, position, abbrPosition, team, league, starting11) VALUES
 (12, 'Franco', 'Daniel', 'Armani', '1986-10-16T00:00:00.000Z', 1, 'Goalkeeper', 'GK', 'River Plate', 'Copa de la Liga', 0),
 (13, 'Gerónimo', NULL, 'Rulli', '1992-05-20T00:00:00.000Z', 12, 'Goalkeeper', 'GK', 'Ajax Amsterdam', 'Eredivisie', 0),
-(14, 'Juan', 'Marcos', 'Foyth', '1998-01-12T00:00:00.000Z', 2, 'Right-Back', 'RB', 'Villareal', 'La Liga', 0),
-(15, 'Gonzalo', 'Ariel', 'Montiel', '1997-01-01T00:00:00.000Z', 4, 'Right-Back', 'RB', 'Nottingham Forrest', 'Premier League', 0),
+(14, 'Juan', 'Marcos', 'Foyth', '1998-01-12T00:00:00.000Z', 2, 'Right-Back', 'RB', 'Villarreal', 'La Liga', 0),
+(15, 'Gonzalo', 'Ariel', 'Montiel', '1997-01-01T00:00:00.000Z', 4, 'Right-Back', 'RB', 'Nottingham Forest', 'Premier League', 0),
 (16, 'Germán', 'Alejo', 'Pezzella', '1991-06-27T00:00:00.000Z', 6, 'Centre-Back', 'CB', 'Real Betis Balompié', 'La Liga', 0),
 (17, 'Marcos', 'Javier', 'Acuña', '1991-10-28T00:00:00.000Z', 8, 'Left-Back', 'LB', 'Sevilla FC', 'La Liga', 0),
 (18, 'Lisandro', NULL, 'Martínez', '1998-01-18T00:00:00.000Z', 25, 'Centre-Back', 'CB', 'Manchester United', 'Premier League', 0),