From ff008ea176dcd6fe441c1c06f99478747c491dd6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Oct 2025 13:43:50 +0000 Subject: [PATCH 01/15] Initial plan From 1b72f240c8f9e608feadc2fdbce0ab32d6b51757 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Oct 2025 13:54:28 +0000 Subject: [PATCH 02/15] Add entities, repositories, DTOs, service and controller for player management Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- pom.xml | 5 + .../api/controller/PlayersController.java | 66 ++++++++ .../api/exception/ConflictException.java | 18 +++ .../api/exception/GlobalExceptionHandler.java | 69 +++++++++ .../api/exception/NotFoundException.java | 18 +++ .../backend/application/PlayersService.java | 16 ++ .../application/PlayersServiceImpl.java | 141 ++++++++++++++++++ .../application/dto/ErrorResponseTO.java | 7 + .../backend/application/dto/PagingTO.java | 7 + .../application/dto/PlayerListResponseTO.java | 9 ++ .../backend/application/dto/PlayerTO.java | 9 ++ .../application/dto/PlayerWithScoreTO.java | 13 ++ .../backend/application/dto/PlayersQuery.java | 9 ++ .../backend/application/dto/PlayersSort.java | 6 + .../application/dto/UpsertPlayerRequest.java | 14 ++ .../backend/domain/entities/GameEntity.java | 112 ++++++++++++++ .../backend/domain/entities/PlayerEntity.java | 60 ++++++++ .../domain/entities/PlayerScoreEntity.java | 89 +++++++++++ .../domain/repositories/GameRepository.java | 35 +++++ .../domain/repositories/PlayerRepository.java | 41 +++++ .../repositories/PlayerScoreRepository.java | 22 +++ 21 files changed, 766 insertions(+) create mode 100644 src/main/java/com/skat/backend/api/controller/PlayersController.java create mode 100644 src/main/java/com/skat/backend/api/exception/ConflictException.java create mode 100644 src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java create mode 100644 src/main/java/com/skat/backend/api/exception/NotFoundException.java create mode 100644 src/main/java/com/skat/backend/application/PlayersService.java create mode 100644 src/main/java/com/skat/backend/application/PlayersServiceImpl.java create mode 100644 src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java create mode 100644 src/main/java/com/skat/backend/application/dto/PagingTO.java create mode 100644 src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java create mode 100644 src/main/java/com/skat/backend/application/dto/PlayerTO.java create mode 100644 src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java create mode 100644 src/main/java/com/skat/backend/application/dto/PlayersQuery.java create mode 100644 src/main/java/com/skat/backend/application/dto/PlayersSort.java create mode 100644 src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java create mode 100644 src/main/java/com/skat/backend/domain/entities/GameEntity.java create mode 100644 src/main/java/com/skat/backend/domain/entities/PlayerEntity.java create mode 100644 src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java create mode 100644 src/main/java/com/skat/backend/domain/repositories/GameRepository.java create mode 100644 src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java create mode 100644 src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java diff --git a/pom.xml b/pom.xml index b72893a..4d89216 100644 --- a/pom.xml +++ b/pom.xml @@ -43,6 +43,11 @@ spring-boot-starter-data-jpa + + org.springframework.boot + spring-boot-starter-validation + + org.postgresql postgresql diff --git a/src/main/java/com/skat/backend/api/controller/PlayersController.java b/src/main/java/com/skat/backend/api/controller/PlayersController.java new file mode 100644 index 0000000..1c119b0 --- /dev/null +++ b/src/main/java/com/skat/backend/api/controller/PlayersController.java @@ -0,0 +1,66 @@ +package com.skat.backend.api.controller; + +import com.skat.backend.application.PlayersService; +import com.skat.backend.application.dto.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import org.springframework.http.ResponseEntity; +import org.springframework.validation.annotation.Validated; +import org.springframework.web.bind.annotation.*; +import org.springframework.web.servlet.support.ServletUriComponentsBuilder; + +import java.net.URI; +import java.util.UUID; + +@RestController +@RequestMapping("/api/players") +@Validated +public class PlayersController { + + private final PlayersService playersService; + + public PlayersController(PlayersService playersService) { + this.playersService = playersService; + } + + @GetMapping + public ResponseEntity listPlayers( + @RequestParam(name = "sort", required = false, defaultValue = "NAME") PlayersSort sort, + @RequestParam(name = "startIndex", required = false, defaultValue = "0") @Min(0) int startIndex, + @RequestParam(name = "pageSize", required = false, defaultValue = "50") @Min(1) @Max(200) int pageSize + ) { + PlayersQuery query = new PlayersQuery(startIndex, pageSize, sort); + PlayerListResponseTO response = playersService.listPlayers(query); + return ResponseEntity.ok(response); + } + + @PostMapping + public ResponseEntity createPlayer(@Valid @RequestBody UpsertPlayerRequest request) { + PlayerTO player = playersService.createPlayer(request); + URI location = ServletUriComponentsBuilder + .fromCurrentRequest() + .path("/{id}") + .buildAndExpand(player.id()) + .toUri(); + return ResponseEntity.created(location).body(player); + } + + @PutMapping("/{id}") + public ResponseEntity updatePlayer( + @PathVariable UUID id, + @Valid @RequestBody UpsertPlayerRequest request + ) { + PlayerTO player = playersService.updatePlayer(id, request); + return ResponseEntity.ok(player); + } + + @DeleteMapping("/{id}") + public ResponseEntity deletePlayer( + @PathVariable UUID id, + @RequestParam(name = "forceDeletion", required = false, defaultValue = "false") boolean forceDeletion + ) { + playersService.deletePlayer(id, forceDeletion); + return ResponseEntity.noContent().build(); + } +} diff --git a/src/main/java/com/skat/backend/api/exception/ConflictException.java b/src/main/java/com/skat/backend/api/exception/ConflictException.java new file mode 100644 index 0000000..e1c7208 --- /dev/null +++ b/src/main/java/com/skat/backend/api/exception/ConflictException.java @@ -0,0 +1,18 @@ +package com.skat.backend.api.exception; + +public class ConflictException extends RuntimeException { + private final String field; + + public ConflictException(String message) { + this(message, null); + } + + public ConflictException(String message, String field) { + super(message); + this.field = field; + } + + public String getField() { + return field; + } +} diff --git a/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java b/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java new file mode 100644 index 0000000..a6873ff --- /dev/null +++ b/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java @@ -0,0 +1,69 @@ +package com.skat.backend.api.exception; + +import com.skat.backend.application.dto.ErrorResponseTO; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.validation.FieldError; +import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; + +@RestControllerAdvice +public class GlobalExceptionHandler { + + @ExceptionHandler(NotFoundException.class) + public ResponseEntity handleNotFoundException(NotFoundException ex) { + ErrorResponseTO error = new ErrorResponseTO( + "not_found", + ex.getMessage(), + ex.getField() + ); + return ResponseEntity.status(HttpStatus.NOT_FOUND).body(error); + } + + @ExceptionHandler(ConflictException.class) + public ResponseEntity handleConflictException(ConflictException ex) { + ErrorResponseTO error = new ErrorResponseTO( + "conflict", + ex.getMessage(), + ex.getField() + ); + return ResponseEntity.status(HttpStatus.CONFLICT).body(error); + } + + @ExceptionHandler(MethodArgumentNotValidException.class) + public ResponseEntity handleValidationException(MethodArgumentNotValidException ex) { + FieldError fieldError = ex.getBindingResult().getFieldError(); + String field = fieldError != null ? fieldError.getField() : null; + String message = fieldError != null ? fieldError.getDefaultMessage() : "Validation failed"; + + ErrorResponseTO error = new ErrorResponseTO( + "bad_request", + message, + field + ); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } + + @ExceptionHandler(MethodArgumentTypeMismatchException.class) + public ResponseEntity handleTypeMismatchException(MethodArgumentTypeMismatchException ex) { + String message = String.format("Invalid value for parameter '%s'", ex.getName()); + ErrorResponseTO error = new ErrorResponseTO( + "bad_request", + message, + ex.getName() + ); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } + + @ExceptionHandler(IllegalArgumentException.class) + public ResponseEntity handleIllegalArgumentException(IllegalArgumentException ex) { + ErrorResponseTO error = new ErrorResponseTO( + "bad_request", + ex.getMessage(), + null + ); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } +} diff --git a/src/main/java/com/skat/backend/api/exception/NotFoundException.java b/src/main/java/com/skat/backend/api/exception/NotFoundException.java new file mode 100644 index 0000000..26648fc --- /dev/null +++ b/src/main/java/com/skat/backend/api/exception/NotFoundException.java @@ -0,0 +1,18 @@ +package com.skat.backend.api.exception; + +public class NotFoundException extends RuntimeException { + private final String field; + + public NotFoundException(String message) { + this(message, null); + } + + public NotFoundException(String message, String field) { + super(message); + this.field = field; + } + + public String getField() { + return field; + } +} diff --git a/src/main/java/com/skat/backend/application/PlayersService.java b/src/main/java/com/skat/backend/application/PlayersService.java new file mode 100644 index 0000000..7f561e7 --- /dev/null +++ b/src/main/java/com/skat/backend/application/PlayersService.java @@ -0,0 +1,16 @@ +package com.skat.backend.application; + +import com.skat.backend.application.dto.*; + +import java.util.UUID; + +public interface PlayersService { + + PlayerListResponseTO listPlayers(PlayersQuery query); + + PlayerTO createPlayer(UpsertPlayerRequest request); + + PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request); + + void deletePlayer(UUID id, boolean forceDeletion); +} diff --git a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java new file mode 100644 index 0000000..99768d7 --- /dev/null +++ b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java @@ -0,0 +1,141 @@ +package com.skat.backend.application; + +import com.skat.backend.api.exception.ConflictException; +import com.skat.backend.api.exception.NotFoundException; +import com.skat.backend.application.dto.*; +import com.skat.backend.domain.entities.PlayerEntity; +import com.skat.backend.domain.repositories.GameRepository; +import com.skat.backend.domain.repositories.PlayerRepository; +import com.skat.backend.domain.repositories.PlayerScoreRepository; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.math.BigInteger; +import java.sql.Timestamp; +import java.time.OffsetDateTime; +import java.time.ZoneId; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +@Service +public class PlayersServiceImpl implements PlayersService { + + private final PlayerRepository playerRepository; + private final GameRepository gameRepository; + private final PlayerScoreRepository playerScoreRepository; + + public PlayersServiceImpl(PlayerRepository playerRepository, + GameRepository gameRepository, + PlayerScoreRepository playerScoreRepository) { + this.playerRepository = playerRepository; + this.gameRepository = gameRepository; + this.playerScoreRepository = playerScoreRepository; + } + + @Override + @Transactional(readOnly = true) + public PlayerListResponseTO listPlayers(PlayersQuery query) { + String sortValue = query.sort().name(); + List results = playerRepository.findPlayersWithLatestScore( + sortValue, + query.startIndex(), + query.pageSize() + ); + + List items = new ArrayList<>(); + for (Object[] row : results) { + UUID id = row[0] instanceof UUID ? (UUID) row[0] : UUID.fromString(row[0].toString()); + String firstName = (String) row[1]; + String lastName = (String) row[2]; + int totalPoints = row[3] instanceof Number ? ((Number) row[3]).intValue() : 0; + int sequenceIndex = row[4] instanceof Number ? ((Number) row[4]).intValue() : 0; + + OffsetDateTime updatedAt; + if (row[5] instanceof Timestamp) { + updatedAt = ((Timestamp) row[5]).toInstant().atZone(ZoneId.systemDefault()).toOffsetDateTime(); + } else if (row[5] instanceof OffsetDateTime) { + updatedAt = (OffsetDateTime) row[5]; + } else { + updatedAt = OffsetDateTime.now(); + } + + items.add(new PlayerWithScoreTO(id, firstName, lastName, totalPoints, sequenceIndex, updatedAt)); + } + + long total = playerRepository.count(); + PagingTO paging = new PagingTO(query.startIndex(), query.pageSize(), total); + + return new PlayerListResponseTO(items, paging, query.sort()); + } + + @Override + @Transactional + public PlayerTO createPlayer(UpsertPlayerRequest request) { + String firstName = request.first_name().trim(); + String lastName = request.last_name().trim(); + + if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName)) { + throw new ConflictException( + "Player with first_name+last_name already exists", + "first_name,last_name" + ); + } + + PlayerEntity player = new PlayerEntity(firstName, lastName); + player = playerRepository.save(player); + + return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); + } + + @Override + @Transactional + public PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request) { + PlayerEntity player = playerRepository.findById(id) + .orElseThrow(() -> new NotFoundException("Player not found", "id")); + + String firstName = request.first_name().trim(); + String lastName = request.last_name().trim(); + + // Check if the new name conflicts with another player + if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(firstName, lastName, id)) { + throw new ConflictException( + "Player with first_name+last_name already exists", + "first_name,last_name" + ); + } + + player.setFirstName(firstName); + player.setLastName(lastName); + player = playerRepository.save(player); + + return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); + } + + @Override + @Transactional + public void deletePlayer(UUID id, boolean forceDeletion) { + PlayerEntity player = playerRepository.findById(id) + .orElseThrow(() -> new NotFoundException("Player not found", "id")); + + if (!forceDeletion) { + boolean hasGames = gameRepository.existsByPlayerId(id); + boolean hasScores = playerScoreRepository.existsByPlayerId(id); + + if (hasGames || hasScores) { + throw new ConflictException("Player is referenced in games or scores"); + } + + playerRepository.delete(player); + } else { + // Force deletion: nullify references first + gameRepository.nullifyPlayer1References(id); + gameRepository.nullifyPlayer2References(id); + gameRepository.nullifyPlayer3References(id); + gameRepository.nullifyMainPlayerReferences(id); + playerScoreRepository.nullifyPlayerReferences(id); + + playerRepository.delete(player); + } + } +} diff --git a/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java b/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java new file mode 100644 index 0000000..341291d --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java @@ -0,0 +1,7 @@ +package com.skat.backend.application.dto; + +public record ErrorResponseTO( + String error, + String message, + String field +) {} diff --git a/src/main/java/com/skat/backend/application/dto/PagingTO.java b/src/main/java/com/skat/backend/application/dto/PagingTO.java new file mode 100644 index 0000000..3bec040 --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/PagingTO.java @@ -0,0 +1,7 @@ +package com.skat.backend.application.dto; + +public record PagingTO( + int startIndex, + int pageSize, + long total +) {} diff --git a/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java b/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java new file mode 100644 index 0000000..0c86d70 --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java @@ -0,0 +1,9 @@ +package com.skat.backend.application.dto; + +import java.util.List; + +public record PlayerListResponseTO( + List items, + PagingTO paging, + PlayersSort sort +) {} diff --git a/src/main/java/com/skat/backend/application/dto/PlayerTO.java b/src/main/java/com/skat/backend/application/dto/PlayerTO.java new file mode 100644 index 0000000..37b3566 --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/PlayerTO.java @@ -0,0 +1,9 @@ +package com.skat.backend.application.dto; + +import java.util.UUID; + +public record PlayerTO( + UUID id, + String first_name, + String last_name +) {} diff --git a/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java b/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java new file mode 100644 index 0000000..d8752f4 --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java @@ -0,0 +1,13 @@ +package com.skat.backend.application.dto; + +import java.time.OffsetDateTime; +import java.util.UUID; + +public record PlayerWithScoreTO( + UUID id, + String first_name, + String last_name, + int current_total_points, + int current_sequence_index, + OffsetDateTime updated_at +) {} diff --git a/src/main/java/com/skat/backend/application/dto/PlayersQuery.java b/src/main/java/com/skat/backend/application/dto/PlayersQuery.java new file mode 100644 index 0000000..d51acf0 --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/PlayersQuery.java @@ -0,0 +1,9 @@ +package com.skat.backend.application.dto; + +import java.util.UUID; + +public record PlayersQuery( + int startIndex, + int pageSize, + PlayersSort sort +) {} diff --git a/src/main/java/com/skat/backend/application/dto/PlayersSort.java b/src/main/java/com/skat/backend/application/dto/PlayersSort.java new file mode 100644 index 0000000..122978a --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/PlayersSort.java @@ -0,0 +1,6 @@ +package com.skat.backend.application.dto; + +public enum PlayersSort { + NAME, + SCORE_DESC +} diff --git a/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java b/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java new file mode 100644 index 0000000..311bf67 --- /dev/null +++ b/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java @@ -0,0 +1,14 @@ +package com.skat.backend.application.dto; + +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; + +public record UpsertPlayerRequest( + @NotBlank(message = "first_name is required") + @Size(max = 50, message = "first_name must not exceed 50 characters") + String first_name, + + @NotBlank(message = "last_name is required") + @Size(max = 50, message = "last_name must not exceed 50 characters") + String last_name +) {} diff --git a/src/main/java/com/skat/backend/domain/entities/GameEntity.java b/src/main/java/com/skat/backend/domain/entities/GameEntity.java new file mode 100644 index 0000000..3dd6d23 --- /dev/null +++ b/src/main/java/com/skat/backend/domain/entities/GameEntity.java @@ -0,0 +1,112 @@ +package com.skat.backend.domain.entities; + +import jakarta.persistence.*; +import java.time.OffsetDateTime; +import java.util.UUID; + +@Entity +@Table( + name = "game", + indexes = { + @Index(name = "game_main_player_IDX", columnList = "main_player_id"), + @Index(name = "game_played_at_IDX", columnList = "played_at") + } +) +public class GameEntity { + + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; + + @ManyToOne + @JoinColumn(name = "player1_id") + private PlayerEntity player1; + + @ManyToOne + @JoinColumn(name = "player2_id") + private PlayerEntity player2; + + @ManyToOne + @JoinColumn(name = "player3_id") + private PlayerEntity player3; + + @ManyToOne + @JoinColumn(name = "main_player_id") + private PlayerEntity mainPlayer; + + @Column(name = "bid_value") + private Integer bidValue; + + @Column(name = "score") + private Integer score; + + @Column(name = "played_at", nullable = false) + private OffsetDateTime playedAt; + + public GameEntity() { + } + + public UUID getId() { + return id; + } + + public void setId(UUID id) { + this.id = id; + } + + public PlayerEntity getPlayer1() { + return player1; + } + + public void setPlayer1(PlayerEntity player1) { + this.player1 = player1; + } + + public PlayerEntity getPlayer2() { + return player2; + } + + public void setPlayer2(PlayerEntity player2) { + this.player2 = player2; + } + + public PlayerEntity getPlayer3() { + return player3; + } + + public void setPlayer3(PlayerEntity player3) { + this.player3 = player3; + } + + public PlayerEntity getMainPlayer() { + return mainPlayer; + } + + public void setMainPlayer(PlayerEntity mainPlayer) { + this.mainPlayer = mainPlayer; + } + + public Integer getBidValue() { + return bidValue; + } + + public void setBidValue(Integer bidValue) { + this.bidValue = bidValue; + } + + public Integer getScore() { + return score; + } + + public void setScore(Integer score) { + this.score = score; + } + + public OffsetDateTime getPlayedAt() { + return playedAt; + } + + public void setPlayedAt(OffsetDateTime playedAt) { + this.playedAt = playedAt; + } +} diff --git a/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java b/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java new file mode 100644 index 0000000..94d4a6b --- /dev/null +++ b/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java @@ -0,0 +1,60 @@ +package com.skat.backend.domain.entities; + +import jakarta.persistence.*; +import java.util.UUID; + +@Entity +@Table( + name = "player", + uniqueConstraints = { + @UniqueConstraint(name = "player_first_last_name_UQ", columnNames = {"first_name", "last_name"}) + }, + indexes = { + @Index(name = "player_first_name_IDX", columnList = "first_name"), + @Index(name = "player_last_name_IDX", columnList = "last_name") + } +) +public class PlayerEntity { + + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; + + @Column(name = "first_name", nullable = false, length = 50) + private String firstName; + + @Column(name = "last_name", nullable = false, length = 50) + private String lastName; + + public PlayerEntity() { + } + + public PlayerEntity(String firstName, String lastName) { + this.firstName = firstName; + this.lastName = lastName; + } + + public UUID getId() { + return id; + } + + public void setId(UUID id) { + this.id = id; + } + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } +} diff --git a/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java b/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java new file mode 100644 index 0000000..88221cf --- /dev/null +++ b/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java @@ -0,0 +1,89 @@ +package com.skat.backend.domain.entities; + +import jakarta.persistence.*; +import java.time.OffsetDateTime; +import java.util.UUID; + +@Entity +@Table( + name = "player_score", + indexes = { + @Index(name = "player_score_player_IDX", columnList = "player_id"), + @Index(name = "player_score_game_IDX", columnList = "game_id"), + @Index(name = "player_score_sequence_IDX", columnList = "sequence_index") + } +) +public class PlayerScoreEntity { + + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; + + @ManyToOne + @JoinColumn(name = "player_id") + private PlayerEntity player; + + @ManyToOne + @JoinColumn(name = "game_id", nullable = false) + private GameEntity game; + + @Column(name = "sequence_index", nullable = false) + private Integer sequenceIndex; + + @Column(name = "total_points") + private Integer totalPoints; + + @Column(name = "created_at", nullable = false) + private OffsetDateTime createdAt; + + public PlayerScoreEntity() { + } + + public UUID getId() { + return id; + } + + public void setId(UUID id) { + this.id = id; + } + + public PlayerEntity getPlayer() { + return player; + } + + public void setPlayer(PlayerEntity player) { + this.player = player; + } + + public GameEntity getGame() { + return game; + } + + public void setGame(GameEntity game) { + this.game = game; + } + + public Integer getSequenceIndex() { + return sequenceIndex; + } + + public void setSequenceIndex(Integer sequenceIndex) { + this.sequenceIndex = sequenceIndex; + } + + public Integer getTotalPoints() { + return totalPoints; + } + + public void setTotalPoints(Integer totalPoints) { + this.totalPoints = totalPoints; + } + + public OffsetDateTime getCreatedAt() { + return createdAt; + } + + public void setCreatedAt(OffsetDateTime createdAt) { + this.createdAt = createdAt; + } +} diff --git a/src/main/java/com/skat/backend/domain/repositories/GameRepository.java b/src/main/java/com/skat/backend/domain/repositories/GameRepository.java new file mode 100644 index 0000000..6d97edd --- /dev/null +++ b/src/main/java/com/skat/backend/domain/repositories/GameRepository.java @@ -0,0 +1,35 @@ +package com.skat.backend.domain.repositories; + +import com.skat.backend.domain.entities.GameEntity; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; + +import java.util.UUID; + +@Repository +public interface GameRepository extends JpaRepository { + + @Query("SELECT CASE WHEN COUNT(g) > 0 THEN true ELSE false END FROM GameEntity g " + + "WHERE g.player1.id = :playerId OR g.player2.id = :playerId " + + "OR g.player3.id = :playerId OR g.mainPlayer.id = :playerId") + boolean existsByPlayerId(@Param("playerId") UUID playerId); + + @Modifying + @Query("UPDATE GameEntity g SET g.player1 = NULL WHERE g.player1.id = :playerId") + void nullifyPlayer1References(@Param("playerId") UUID playerId); + + @Modifying + @Query("UPDATE GameEntity g SET g.player2 = NULL WHERE g.player2.id = :playerId") + void nullifyPlayer2References(@Param("playerId") UUID playerId); + + @Modifying + @Query("UPDATE GameEntity g SET g.player3 = NULL WHERE g.player3.id = :playerId") + void nullifyPlayer3References(@Param("playerId") UUID playerId); + + @Modifying + @Query("UPDATE GameEntity g SET g.mainPlayer = NULL WHERE g.mainPlayer.id = :playerId") + void nullifyMainPlayerReferences(@Param("playerId") UUID playerId); +} diff --git a/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java b/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java new file mode 100644 index 0000000..3e94a2c --- /dev/null +++ b/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java @@ -0,0 +1,41 @@ +package com.skat.backend.domain.repositories; + +import com.skat.backend.domain.entities.PlayerEntity; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; + +import java.util.List; +import java.util.UUID; + +@Repository +public interface PlayerRepository extends JpaRepository { + + boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(String firstName, String lastName); + + boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(String firstName, String lastName, UUID id); + + @Query(value = """ + SELECT p.id, p.first_name, p.last_name, + COALESCE(ps.total_points, 0) as current_total_points, + COALESCE(ps.sequence_index, 0) as current_sequence_index, + COALESCE(ps.created_at, CURRENT_TIMESTAMP) as updated_at + FROM player p + LEFT JOIN LATERAL ( + SELECT player_id, total_points, sequence_index, created_at + FROM player_score + WHERE player_id = p.id + ORDER BY sequence_index DESC + LIMIT 1 + ) ps ON true + ORDER BY + CASE WHEN :sort = 'SCORE_DESC' THEN COALESCE(ps.total_points, 0) END DESC, + CASE WHEN :sort = 'NAME' THEN p.last_name END ASC, + CASE WHEN :sort = 'SCORE_DESC' THEN p.last_name END ASC, + p.first_name ASC + OFFSET :offset ROWS + FETCH FIRST :limit ROWS ONLY + """, nativeQuery = true) + List findPlayersWithLatestScore(@Param("sort") String sort, @Param("offset") int offset, @Param("limit") int limit); +} diff --git a/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java b/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java new file mode 100644 index 0000000..9933773 --- /dev/null +++ b/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java @@ -0,0 +1,22 @@ +package com.skat.backend.domain.repositories; + +import com.skat.backend.domain.entities.PlayerScoreEntity; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; + +import java.util.UUID; + +@Repository +public interface PlayerScoreRepository extends JpaRepository { + + @Query("SELECT CASE WHEN COUNT(ps) > 0 THEN true ELSE false END FROM PlayerScoreEntity ps " + + "WHERE ps.player.id = :playerId") + boolean existsByPlayerId(@Param("playerId") UUID playerId); + + @Modifying + @Query("UPDATE PlayerScoreEntity ps SET ps.player = NULL WHERE ps.player.id = :playerId") + void nullifyPlayerReferences(@Param("playerId") UUID playerId); +} From e8d58ce3aa649647d1e3797a7ed385a05c7dcf18 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Oct 2025 13:58:53 +0000 Subject: [PATCH 03/15] Add integration tests and fix parameter binding issue Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- pom.xml | 1 + .../api/exception/GlobalExceptionHandler.java | 16 + .../api/controller/PlayersControllerIT.java | 485 ++++++++++++++++++ 3 files changed, 502 insertions(+) create mode 100644 src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java diff --git a/pom.xml b/pom.xml index 4d89216..ec73c23 100644 --- a/pom.xml +++ b/pom.xml @@ -118,6 +118,7 @@ 21 21 + true diff --git a/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java b/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java index a6873ff..825a383 100644 --- a/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java +++ b/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java @@ -1,6 +1,8 @@ package com.skat.backend.api.exception; import com.skat.backend.application.dto.ErrorResponseTO; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.ConstraintViolationException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.validation.FieldError; @@ -66,4 +68,18 @@ public ResponseEntity handleIllegalArgumentException(IllegalArg ); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); } + + @ExceptionHandler(ConstraintViolationException.class) + public ResponseEntity handleConstraintViolationException(ConstraintViolationException ex) { + ConstraintViolation violation = ex.getConstraintViolations().iterator().next(); + String propertyPath = violation.getPropertyPath().toString(); + String field = propertyPath.contains(".") ? propertyPath.substring(propertyPath.lastIndexOf('.') + 1) : propertyPath; + + ErrorResponseTO error = new ErrorResponseTO( + "bad_request", + violation.getMessage(), + field + ); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } } diff --git a/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java b/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java new file mode 100644 index 0000000..e827e0f --- /dev/null +++ b/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java @@ -0,0 +1,485 @@ +package com.skat.backend.api.controller; + +import com.skat.backend.domain.entities.GameEntity; +import com.skat.backend.domain.entities.PlayerEntity; +import com.skat.backend.domain.entities.PlayerScoreEntity; +import com.skat.backend.domain.repositories.GameRepository; +import com.skat.backend.domain.repositories.PlayerRepository; +import com.skat.backend.domain.repositories.PlayerScoreRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.test.web.servlet.MockMvc; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import java.time.OffsetDateTime; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.*; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +/** + * Integration test for PlayersController following ADR-001 (Integration Testing Strategy with Testcontainers). + * Uses Testcontainers for PostgreSQL 18 and tests the full Spring Boot application. + */ +@SpringBootTest +@AutoConfigureMockMvc +@Testcontainers +class PlayersControllerIT { + + @Container + static PostgreSQLContainer postgres = new PostgreSQLContainer<>("postgres:18") + .withDatabaseName("skatdb") + .withUsername("test") + .withPassword("test"); + + @DynamicPropertySource + static void configureProperties(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", postgres::getJdbcUrl); + registry.add("spring.datasource.username", postgres::getUsername); + registry.add("spring.datasource.password", postgres::getPassword); + } + + @Autowired + private MockMvc mockMvc; + + @Autowired + private PlayerRepository playerRepository; + + @Autowired + private GameRepository gameRepository; + + @Autowired + private PlayerScoreRepository playerScoreRepository; + + @BeforeEach + void setUp() { + playerScoreRepository.deleteAll(); + gameRepository.deleteAll(); + playerRepository.deleteAll(); + } + + // AC-1: List players — default sorting and paging + @Test + void given_existingPlayers_when_listPlayersWithoutParameters_then_returns200WithDefaultPaging() throws Exception { + // Given + PlayerEntity player1 = new PlayerEntity("Anna", "Schmidt"); + PlayerEntity player2 = new PlayerEntity("Max", "Mueller"); + playerRepository.save(player1); + playerRepository.save(player2); + + // When & Then + mockMvc.perform(get("/api/players")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items", hasSize(2))) + .andExpect(jsonPath("$.paging.startIndex", is(0))) + .andExpect(jsonPath("$.paging.pageSize", is(50))) + .andExpect(jsonPath("$.paging.total", is(2))) + .andExpect(jsonPath("$.sort", is("NAME"))); + } + + // AC-2: List players — score_desc ordering + @Test + void given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returnsOrderedByScoreDesc() throws Exception { + // Given + PlayerEntity player1 = new PlayerEntity("Anna", "Schmidt"); + PlayerEntity player2 = new PlayerEntity("Max", "Mueller"); + PlayerEntity player3 = new PlayerEntity("Lisa", "Bauer"); + player1 = playerRepository.save(player1); + player2 = playerRepository.save(player2); + player3 = playerRepository.save(player3); + + GameEntity game = new GameEntity(); + game.setPlayedAt(OffsetDateTime.now()); + game = gameRepository.save(game); + + // Create scores for players + PlayerScoreEntity score1 = new PlayerScoreEntity(); + score1.setPlayer(player1); + score1.setGame(game); + score1.setSequenceIndex(0); + score1.setTotalPoints(100); + score1.setCreatedAt(OffsetDateTime.now()); + playerScoreRepository.save(score1); + + PlayerScoreEntity score2 = new PlayerScoreEntity(); + score2.setPlayer(player2); + score2.setGame(game); + score2.setSequenceIndex(0); + score2.setTotalPoints(200); + score2.setCreatedAt(OffsetDateTime.now()); + playerScoreRepository.save(score2); + + PlayerScoreEntity score3 = new PlayerScoreEntity(); + score3.setPlayer(player3); + score3.setGame(game); + score3.setSequenceIndex(0); + score3.setTotalPoints(150); + score3.setCreatedAt(OffsetDateTime.now()); + playerScoreRepository.save(score3); + + // When & Then + mockMvc.perform(get("/api/players") + .param("sort", "SCORE_DESC")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items", hasSize(3))) + .andExpect(jsonPath("$.items[0].first_name", is("Max"))) + .andExpect(jsonPath("$.items[0].current_total_points", is(200))) + .andExpect(jsonPath("$.items[1].first_name", is("Lisa"))) + .andExpect(jsonPath("$.items[1].current_total_points", is(150))) + .andExpect(jsonPath("$.items[2].first_name", is("Anna"))) + .andExpect(jsonPath("$.items[2].current_total_points", is(100))) + .andExpect(jsonPath("$.sort", is("SCORE_DESC"))); + } + + // AC-3: List players — parameter validation + @Test + void given_invalidPageSize_when_listPlayers_then_returns400() throws Exception { + // When & Then + mockMvc.perform(get("/api/players") + .param("pageSize", "0")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", is("bad_request"))) + .andExpect(jsonPath("$.field", is("pageSize"))); + } + + @Test + void given_negativeStartIndex_when_listPlayers_then_returns400() throws Exception { + // When & Then + mockMvc.perform(get("/api/players") + .param("startIndex", "-1")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", is("bad_request"))) + .andExpect(jsonPath("$.field", is("startIndex"))); + } + + @Test + void given_pageSizeTooLarge_when_listPlayers_then_returns400() throws Exception { + // When & Then + mockMvc.perform(get("/api/players") + .param("pageSize", "201")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", is("bad_request"))) + .andExpect(jsonPath("$.field", is("pageSize"))); + } + + // AC-4: Create player — unique full name + @Test + void given_uniquePlayerName_when_createPlayer_then_returns201WithLocation() throws Exception { + // Given + String requestBody = """ + { + "first_name": "Anna", + "last_name": "Schmidt" + } + """; + + // When & Then + mockMvc.perform(post("/api/players") + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isCreated()) + .andExpect(header().exists("Location")) + .andExpect(jsonPath("$.id", notNullValue())) + .andExpect(jsonPath("$.first_name", is("Anna"))) + .andExpect(jsonPath("$.last_name", is("Schmidt"))); + + // Verify player was created + assertThat(playerRepository.findAll()).hasSize(1); + } + + // AC-5: Create player — conflict on duplicate full name + @Test + void given_existingPlayerName_when_createPlayerWithSameName_then_returns409() throws Exception { + // Given + PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); + playerRepository.save(existingPlayer); + + String requestBody = """ + { + "first_name": "Anna", + "last_name": "Schmidt" + } + """; + + // When & Then + mockMvc.perform(post("/api/players") + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.error", is("conflict"))) + .andExpect(jsonPath("$.field", is("first_name,last_name"))); + } + + @Test + void given_missingFirstName_when_createPlayer_then_returns400() throws Exception { + // Given + String requestBody = """ + { + "last_name": "Schmidt" + } + """; + + // When & Then + mockMvc.perform(post("/api/players") + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", is("bad_request"))); + } + + @Test + void given_blankFirstName_when_createPlayer_then_returns400() throws Exception { + // Given + String requestBody = """ + { + "first_name": " ", + "last_name": "Schmidt" + } + """; + + // When & Then + mockMvc.perform(post("/api/players") + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", is("bad_request"))); + } + + @Test + void given_nameTooLong_when_createPlayer_then_returns400() throws Exception { + // Given + String longName = "a".repeat(51); + String requestBody = String.format(""" + { + "first_name": "%s", + "last_name": "Schmidt" + } + """, longName); + + // When & Then + mockMvc.perform(post("/api/players") + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", is("bad_request"))); + } + + // AC-6: Update player — id must exist + @Test + void given_nonExistentPlayerId_when_updatePlayer_then_returns404() throws Exception { + // Given + UUID nonExistentId = UUID.randomUUID(); + String requestBody = """ + { + "first_name": "Anna", + "last_name": "Bauer" + } + """; + + // When & Then + mockMvc.perform(put("/api/players/" + nonExistentId) + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error", is("not_found"))) + .andExpect(jsonPath("$.field", is("id"))); + } + + // AC-7: Update player — uniqueness enforced + @Test + void given_twoPlayers_when_updatePlayerToExistingName_then_returns409() throws Exception { + // Given + PlayerEntity player1 = new PlayerEntity("Anna", "Schmidt"); + PlayerEntity player2 = new PlayerEntity("Anna", "Mueller"); + player1 = playerRepository.save(player1); + player2 = playerRepository.save(player2); + + String requestBody = """ + { + "first_name": "Anna", + "last_name": "Schmidt" + } + """; + + // When & Then + mockMvc.perform(put("/api/players/" + player2.getId()) + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.error", is("conflict"))) + .andExpect(jsonPath("$.field", is("first_name,last_name"))); + } + + @Test + void given_existingPlayer_when_updatePlayerToNewName_then_returns200() throws Exception { + // Given + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player = playerRepository.save(player); + + String requestBody = """ + { + "first_name": "Anna", + "last_name": "Mueller" + } + """; + + // When & Then + mockMvc.perform(put("/api/players/" + player.getId()) + .contentType(MediaType.APPLICATION_JSON) + .content(requestBody)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id", is(player.getId().toString()))) + .andExpect(jsonPath("$.first_name", is("Anna"))) + .andExpect(jsonPath("$.last_name", is("Mueller"))); + + // Verify player was updated + PlayerEntity updatedPlayer = playerRepository.findById(player.getId()).orElseThrow(); + assertThat(updatedPlayer.getLastName()).isEqualTo("Mueller"); + } + + // AC-8: Delete player — safe delete without references + @Test + void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_returns204() throws Exception { + // Given + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player = playerRepository.save(player); + + // When & Then + mockMvc.perform(delete("/api/players/" + player.getId()) + .param("forceDeletion", "false")) + .andExpect(status().isNoContent()); + + // Verify player was deleted + assertThat(playerRepository.findById(player.getId())).isEmpty(); + } + + // AC-9: Delete player — conflict when referenced + @Test + void given_playerReferencedInGame_when_deletePlayerWithoutForce_then_returns409() throws Exception { + // Given + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player = playerRepository.save(player); + + GameEntity game = new GameEntity(); + game.setPlayer1(player); + game.setPlayedAt(OffsetDateTime.now()); + gameRepository.save(game); + + // When & Then + mockMvc.perform(delete("/api/players/" + player.getId()) + .param("forceDeletion", "false")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.error", is("conflict"))); + + // Verify player was not deleted + assertThat(playerRepository.findById(player.getId())).isPresent(); + } + + @Test + void given_playerReferencedInScore_when_deletePlayerWithoutForce_then_returns409() throws Exception { + // Given + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player = playerRepository.save(player); + + GameEntity game = new GameEntity(); + game.setPlayedAt(OffsetDateTime.now()); + game = gameRepository.save(game); + + PlayerScoreEntity score = new PlayerScoreEntity(); + score.setPlayer(player); + score.setGame(game); + score.setSequenceIndex(0); + score.setTotalPoints(100); + score.setCreatedAt(OffsetDateTime.now()); + playerScoreRepository.save(score); + + // When & Then + mockMvc.perform(delete("/api/players/" + player.getId()) + .param("forceDeletion", "false")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.error", is("conflict"))); + + // Verify player was not deleted + assertThat(playerRepository.findById(player.getId())).isPresent(); + } + + // AC-10: Delete player — forced deletion nullifies references + @Test + void given_playerReferencedInGameAndScore_when_forceDeletePlayer_then_returns204AndNullifies() throws Exception { + // Given + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player = playerRepository.save(player); + + GameEntity game = new GameEntity(); + game.setPlayer1(player); + game.setPlayer2(player); + game.setMainPlayer(player); + game.setPlayedAt(OffsetDateTime.now()); + game = gameRepository.save(game); + + PlayerScoreEntity score = new PlayerScoreEntity(); + score.setPlayer(player); + score.setGame(game); + score.setSequenceIndex(0); + score.setTotalPoints(100); + score.setCreatedAt(OffsetDateTime.now()); + score = playerScoreRepository.save(score); + + UUID gameId = game.getId(); + UUID scoreId = score.getId(); + + // When & Then + mockMvc.perform(delete("/api/players/" + player.getId()) + .param("forceDeletion", "true")) + .andExpect(status().isNoContent()); + + // Verify player was deleted + assertThat(playerRepository.findById(player.getId())).isEmpty(); + + // Verify references were nullified + GameEntity updatedGame = gameRepository.findById(gameId).orElseThrow(); + assertThat(updatedGame.getPlayer1()).isNull(); + assertThat(updatedGame.getPlayer2()).isNull(); + assertThat(updatedGame.getMainPlayer()).isNull(); + + PlayerScoreEntity updatedScore = playerScoreRepository.findById(scoreId).orElseThrow(); + assertThat(updatedScore.getPlayer()).isNull(); + } + + @Test + void given_nonExistentPlayer_when_deletePlayer_then_returns404() throws Exception { + // Given + UUID nonExistentId = UUID.randomUUID(); + + // When & Then + mockMvc.perform(delete("/api/players/" + nonExistentId) + .param("forceDeletion", "false")) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error", is("not_found"))); + } + + @Test + void given_newPlayerWithoutScores_when_listPlayers_then_returnsPlayerWithZeroScore() throws Exception { + // Given + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + playerRepository.save(player); + + // When & Then + mockMvc.perform(get("/api/players")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items", hasSize(1))) + .andExpect(jsonPath("$.items[0].first_name", is("Anna"))) + .andExpect(jsonPath("$.items[0].current_total_points", is(0))) + .andExpect(jsonPath("$.items[0].current_sequence_index", is(0))); + } +} From 50e4014a4b8190ffcefff217b344723f2993ca2b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Oct 2025 14:00:54 +0000 Subject: [PATCH 04/15] Add unit tests for PlayersService with Mockito Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- .../application/PlayersServiceTest.java | 316 ++++++++++++++++++ 1 file changed, 316 insertions(+) create mode 100644 src/test/java/com/skat/backend/application/PlayersServiceTest.java diff --git a/src/test/java/com/skat/backend/application/PlayersServiceTest.java b/src/test/java/com/skat/backend/application/PlayersServiceTest.java new file mode 100644 index 0000000..205a3d9 --- /dev/null +++ b/src/test/java/com/skat/backend/application/PlayersServiceTest.java @@ -0,0 +1,316 @@ +package com.skat.backend.application; + +import com.skat.backend.api.exception.ConflictException; +import com.skat.backend.api.exception.NotFoundException; +import com.skat.backend.application.dto.*; +import com.skat.backend.domain.entities.PlayerEntity; +import com.skat.backend.domain.repositories.GameRepository; +import com.skat.backend.domain.repositories.PlayerRepository; +import com.skat.backend.domain.repositories.PlayerScoreRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.*; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +/** + * Pure unit test for PlayersServiceImpl following ADR-002 (Unit Testing Strategy with Maven Surefire). + * Uses Mockito for mocking dependencies and AssertJ for assertions. + * No Spring context is loaded, making this a fast unit test. + */ +class PlayersServiceTest { + + @Mock + private PlayerRepository playerRepository; + + @Mock + private GameRepository gameRepository; + + @Mock + private PlayerScoreRepository playerScoreRepository; + + @InjectMocks + private PlayersServiceImpl playersService; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + } + + @Test + void given_validRequest_when_createPlayer_then_playerIsCreatedAndReturned() { + // Given + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Schmidt"); + PlayerEntity savedPlayer = new PlayerEntity("Anna", "Schmidt"); + UUID playerId = UUID.randomUUID(); + savedPlayer.setId(playerId); + + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) + .thenReturn(false); + when(playerRepository.save(any(PlayerEntity.class))).thenReturn(savedPlayer); + + // When + PlayerTO result = playersService.createPlayer(request); + + // Then + assertThat(result).isNotNull(); + assertThat(result.id()).isEqualTo(playerId); + assertThat(result.first_name()).isEqualTo("Anna"); + assertThat(result.last_name()).isEqualTo("Schmidt"); + + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt"); + verify(playerRepository).save(any(PlayerEntity.class)); + } + + @Test + void given_duplicateName_when_createPlayer_then_throwsConflictException() { + // Given + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Schmidt"); + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) + .thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.createPlayer(request)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player with first_name+last_name already exists"); + + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt"); + verify(playerRepository, never()).save(any()); + } + + @Test + void given_nameWithWhitespace_when_createPlayer_then_nameIsTrimmed() { + // Given + UpsertPlayerRequest request = new UpsertPlayerRequest(" Anna ", " Schmidt "); + PlayerEntity savedPlayer = new PlayerEntity("Anna", "Schmidt"); + UUID playerId = UUID.randomUUID(); + savedPlayer.setId(playerId); + + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) + .thenReturn(false); + when(playerRepository.save(any(PlayerEntity.class))).thenReturn(savedPlayer); + + // When + PlayerTO result = playersService.createPlayer(request); + + // Then + ArgumentCaptor playerCaptor = ArgumentCaptor.forClass(PlayerEntity.class); + verify(playerRepository).save(playerCaptor.capture()); + PlayerEntity capturedPlayer = playerCaptor.getValue(); + assertThat(capturedPlayer.getFirstName()).isEqualTo("Anna"); + assertThat(capturedPlayer.getLastName()).isEqualTo("Schmidt"); + } + + @Test + void given_existingPlayer_when_updatePlayer_then_playerIsUpdatedAndReturned() { + // Given + UUID playerId = UUID.randomUUID(); + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); + PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); + existingPlayer.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(existingPlayer)); + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId)) + .thenReturn(false); + when(playerRepository.save(any(PlayerEntity.class))).thenReturn(existingPlayer); + + // When + PlayerTO result = playersService.updatePlayer(playerId, request); + + // Then + assertThat(result).isNotNull(); + assertThat(result.id()).isEqualTo(playerId); + assertThat(result.first_name()).isEqualTo("Anna"); + assertThat(result.last_name()).isEqualTo("Mueller"); + + verify(playerRepository).findById(playerId); + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId); + verify(playerRepository).save(existingPlayer); + assertThat(existingPlayer.getLastName()).isEqualTo("Mueller"); + } + + @Test + void given_nonExistentPlayer_when_updatePlayer_then_throwsNotFoundException() { + // Given + UUID playerId = UUID.randomUUID(); + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); + when(playerRepository.findById(playerId)).thenReturn(Optional.empty()); + + // When & Then + assertThatThrownBy(() -> playersService.updatePlayer(playerId, request)) + .isInstanceOf(NotFoundException.class) + .hasMessageContaining("Player not found"); + + verify(playerRepository).findById(playerId); + verify(playerRepository, never()).save(any()); + } + + @Test + void given_duplicateNameOnUpdate_when_updatePlayer_then_throwsConflictException() { + // Given + UUID playerId = UUID.randomUUID(); + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); + PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); + existingPlayer.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(existingPlayer)); + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId)) + .thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.updatePlayer(playerId, request)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player with first_name+last_name already exists"); + + verify(playerRepository).findById(playerId); + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId); + verify(playerRepository, never()).save(any()); + } + + @Test + void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_playerIsDeleted() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + when(gameRepository.existsByPlayerId(playerId)).thenReturn(false); + when(playerScoreRepository.existsByPlayerId(playerId)).thenReturn(false); + + // When + playersService.deletePlayer(playerId, false); + + // Then + verify(playerRepository).findById(playerId); + verify(gameRepository).existsByPlayerId(playerId); + verify(playerScoreRepository).existsByPlayerId(playerId); + verify(playerRepository).delete(player); + verify(gameRepository, never()).nullifyPlayer1References(any()); + } + + @Test + void given_playerWithGameReferences_when_deletePlayerWithoutForce_then_throwsConflictException() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + when(gameRepository.existsByPlayerId(playerId)).thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player is referenced in games or scores"); + + verify(playerRepository).findById(playerId); + verify(gameRepository).existsByPlayerId(playerId); + verify(playerRepository, never()).delete(any()); + } + + @Test + void given_playerWithScoreReferences_when_deletePlayerWithoutForce_then_throwsConflictException() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + when(gameRepository.existsByPlayerId(playerId)).thenReturn(false); + when(playerScoreRepository.existsByPlayerId(playerId)).thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player is referenced in games or scores"); + + verify(playerRepository).findById(playerId); + verify(gameRepository).existsByPlayerId(playerId); + verify(playerScoreRepository).existsByPlayerId(playerId); + verify(playerRepository, never()).delete(any()); + } + + @Test + void given_playerWithReferences_when_forceDeletePlayer_then_referencesNullifiedAndPlayerDeleted() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + + // When + playersService.deletePlayer(playerId, true); + + // Then + verify(playerRepository).findById(playerId); + verify(gameRepository).nullifyPlayer1References(playerId); + verify(gameRepository).nullifyPlayer2References(playerId); + verify(gameRepository).nullifyPlayer3References(playerId); + verify(gameRepository).nullifyMainPlayerReferences(playerId); + verify(playerScoreRepository).nullifyPlayerReferences(playerId); + verify(playerRepository).delete(player); + } + + @Test + void given_nonExistentPlayer_when_deletePlayer_then_throwsNotFoundException() { + // Given + UUID playerId = UUID.randomUUID(); + when(playerRepository.findById(playerId)).thenReturn(Optional.empty()); + + // When & Then + assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) + .isInstanceOf(NotFoundException.class) + .hasMessageContaining("Player not found"); + + verify(playerRepository).findById(playerId); + verify(playerRepository, never()).delete(any()); + } + + @Test + void given_validQuery_when_listPlayers_then_returnsPlayerListResponse() { + // Given + PlayersQuery query = new PlayersQuery(0, 50, PlayersSort.NAME); + + List mockResults = new ArrayList<>(); + Object[] row1 = new Object[]{ + UUID.randomUUID(), + "Anna", + "Schmidt", + 100, + 5, + java.sql.Timestamp.valueOf("2024-01-01 10:00:00") + }; + mockResults.add(row1); + + when(playerRepository.findPlayersWithLatestScore("NAME", 0, 50)).thenReturn(mockResults); + when(playerRepository.count()).thenReturn(1L); + + // When + PlayerListResponseTO result = playersService.listPlayers(query); + + // Then + assertThat(result).isNotNull(); + assertThat(result.items()).hasSize(1); + assertThat(result.items().get(0).first_name()).isEqualTo("Anna"); + assertThat(result.items().get(0).last_name()).isEqualTo("Schmidt"); + assertThat(result.items().get(0).current_total_points()).isEqualTo(100); + assertThat(result.items().get(0).current_sequence_index()).isEqualTo(5); + assertThat(result.paging().startIndex()).isEqualTo(0); + assertThat(result.paging().pageSize()).isEqualTo(50); + assertThat(result.paging().total()).isEqualTo(1L); + assertThat(result.sort()).isEqualTo(PlayersSort.NAME); + + verify(playerRepository).findPlayersWithLatestScore("NAME", 0, 50); + verify(playerRepository).count(); + } +} From 334d8d2794917c25cc6cef8485b0e58e4db4e26a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Oct 2025 14:05:50 +0000 Subject: [PATCH 05/15] Add implementation summary document for UC-01 Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- UC-01-IMPLEMENTATION-SUMMARY.md | 209 ++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 UC-01-IMPLEMENTATION-SUMMARY.md diff --git a/UC-01-IMPLEMENTATION-SUMMARY.md b/UC-01-IMPLEMENTATION-SUMMARY.md new file mode 100644 index 0000000..42154eb --- /dev/null +++ b/UC-01-IMPLEMENTATION-SUMMARY.md @@ -0,0 +1,209 @@ +# UC-01: Player Management Service - Implementation Summary + +## Overview +This document summarizes the implementation of the Player Management Service backend for the Skat application, as specified in UC-01. + +## Implemented Components + +### 1. Database Entities (Domain Layer) +Located in: `src/main/java/com/skat/backend/domain/entities/` + +- **PlayerEntity**: Represents a player with UUID id, first name, and last name + - Unique constraint on (first_name, last_name) + - Indexes on first_name and last_name + +- **GameEntity**: Represents a Skat game session + - References to 3 players (player1, player2, player3) and main player + - Bid value, score, and played_at timestamp + - All player references nullable (to support player deletion) + +- **PlayerScoreEntity**: Represents cumulative player scores over time + - References to player and game + - Sequence index for ordering + - Total points and created_at timestamp + +### 2. Repositories (Domain Layer) +Located in: `src/main/java/com/skat/backend/domain/repositories/` + +- **PlayerRepository**: + - Uniqueness checks (case-insensitive) + - Native SQL query for listing players with latest scores using PostgreSQL LATERAL joins + +- **GameRepository**: + - Existence checks for player references + - Methods to nullify player references for force deletion + +- **PlayerScoreRepository**: + - Existence checks for player references + - Method to nullify player references for force deletion + +### 3. DTOs and Transfer Objects (Application Layer) +Located in: `src/main/java/com/skat/backend/application/dto/` + +- **PlayersSort**: Enum for sorting options (NAME, SCORE_DESC) +- **PlayerTO**: Transfer object for player data (id, first_name, last_name) +- **PlayerWithScoreTO**: Player with current score snapshot +- **PlayerListResponseTO**: Response for list endpoint with items, paging, and sort +- **PagingTO**: Pagination metadata (startIndex, pageSize, total) +- **UpsertPlayerRequest**: Request DTO for create/update with validation +- **PlayersQuery**: Query parameters for list endpoint +- **ErrorResponseTO**: Standard error response format + +### 4. Service Layer (Application Layer) +Located in: `src/main/java/com/skat/backend/application/` + +- **PlayersService**: Interface defining service operations +- **PlayersServiceImpl**: Implementation with business logic + - List players with latest scores (sorting and pagination) + - Create player with uniqueness validation + - Update player with uniqueness validation + - Delete player with safe/force deletion modes + - Name trimming on create/update + +### 5. Controller Layer (API Layer) +Located in: `src/main/java/com/skat/backend/api/controller/` + +- **PlayersController**: REST endpoints + - GET /api/players - List players with query parameters + - POST /api/players - Create player + - PUT /api/players/{id} - Update player + - DELETE /api/players/{id}?forceDeletion={boolean} - Delete player + +### 6. Exception Handling (API Layer) +Located in: `src/main/java/com/skat/backend/api/exception/` + +- **NotFoundException**: For 404 errors +- **ConflictException**: For 409 errors +- **GlobalExceptionHandler**: @RestControllerAdvice for consistent error responses + - Handles validation errors (MethodArgumentNotValidException) + - Handles constraint violations (ConstraintViolationException) + - Handles type mismatches + - Returns standard ErrorResponseTO format + +## Test Coverage + +### Unit Tests (Maven Surefire) +Located in: `src/test/java/com/skat/backend/application/` + +- **PlayersServiceTest**: 12 tests covering: + - Create player scenarios (success, duplicate, name trimming) + - Update player scenarios (success, not found, duplicate) + - Delete player scenarios (safe, with references, force, not found) + - List players + +### Integration Tests (Maven Failsafe + Testcontainers) +Located in: `src/test/java/com/skat/backend/api/controller/` + +- **PlayersControllerIT**: 19 tests covering: + - All 10 acceptance criteria from UC-01 + - Additional edge cases for validation + - Full application context with PostgreSQL 18 + +### Test Summary +- **Total tests**: 35 (16 unit + 19 integration) +- **Status**: All passing ✅ +- **Coverage**: All 10 acceptance criteria covered + +## Acceptance Criteria Mapping + +| AC | Description | Test(s) | Status | +|----|-------------|---------|--------| +| AC-1 | List players — default sorting and paging | given_existingPlayers_when_listPlayersWithoutParameters_then_returns200WithDefaultPaging | ✅ | +| AC-2 | List players — score_desc ordering | given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returnsOrderedByScoreDesc | ✅ | +| AC-3 | List players — parameter validation | given_invalidPageSize_when_listPlayers_then_returns400 (+ 2 more) | ✅ | +| AC-4 | Create player — unique full name | given_uniquePlayerName_when_createPlayer_then_returns201WithLocation | ✅ | +| AC-5 | Create player — conflict on duplicate | given_existingPlayerName_when_createPlayerWithSameName_then_returns409 | ✅ | +| AC-6 | Update player — id must exist | given_nonExistentPlayerId_when_updatePlayer_then_returns404 | ✅ | +| AC-7 | Update player — uniqueness enforced | given_twoPlayers_when_updatePlayerToExistingName_then_returns409 | ✅ | +| AC-8 | Delete player — safe delete | given_playerWithoutReferences_when_deletePlayerWithoutForce_then_returns204 | ✅ | +| AC-9 | Delete player — conflict when referenced | given_playerReferencedInGame/Score_when_deletePlayerWithoutForce_then_returns409 | ✅ | +| AC-10 | Delete player — forced deletion | given_playerReferencedInGameAndScore_when_forceDeletePlayer_then_returns204AndNullifies | ✅ | + +## Technical Decisions + +1. **Java 21 & Spring Boot 3.5.6**: As specified in requirements +2. **PostgreSQL 18**: Using Testcontainers for integration tests +3. **UUID Primary Keys**: For all entities +4. **OffsetDateTime**: For all timestamp fields +5. **Bean Validation**: For request validation with jakarta.validation +6. **Native SQL Query**: For efficient player listing with latest scores using LATERAL joins +7. **Transactional Service**: All service methods properly transactional +8. **Case-insensitive Uniqueness**: For player names +9. **Name Trimming**: Automatic trimming of whitespace from names +10. **Maven Parameters Flag**: Added to compiler configuration for proper parameter name resolution + +## Dependencies Added + +- `spring-boot-starter-validation`: For Bean Validation support +- Maven compiler plugin configured with `-parameters` flag + +## Security + +- CodeQL analysis performed: **0 vulnerabilities found** ✅ +- No secrets or sensitive data in code +- Proper input validation at all levels +- SQL injection protection via parameterized queries + +## API Endpoints + +All endpoints follow the specifications in: +- `GET_players_spec.md` +- `POST_players_upsert.md` +- `DELETE_player_spec.md` + +### GET /api/players +- Query params: sort (NAME/SCORE_DESC), startIndex (≥0), pageSize (1-200) +- Returns: PlayerListResponseTO with items, paging, and sort + +### POST /api/players +- Body: UpsertPlayerRequest (first_name, last_name) +- Returns: 201 Created with PlayerTO and Location header + +### PUT /api/players/{id} +- Path param: id (UUID) +- Body: UpsertPlayerRequest +- Returns: 200 OK with PlayerTO + +### DELETE /api/players/{id} +- Path param: id (UUID) +- Query param: forceDeletion (boolean, default false) +- Returns: 204 No Content + +## Error Responses + +All errors follow consistent format: +```json +{ + "error": "bad_request|not_found|conflict", + "message": "Human readable message", + "field": "optional field name" +} +``` + +## Build & Test Commands + +```bash +# Build +mvn clean compile + +# Unit tests (fast) +mvn test + +# Integration tests + unit tests (with Testcontainers) +mvn verify + +# Clean and verify +mvn clean verify +``` + +## Success Criteria + +✅ All 10 acceptance criteria pass +✅ Code structure follows layering (Controller → Service → Repository) +✅ DTOs/TOs match endpoint specifications +✅ All tests passing (35 tests) +✅ No security vulnerabilities (CodeQL) +✅ PostgreSQL 18 compatible +✅ Bean Validation implemented +✅ Proper error handling with consistent error responses +✅ Transactional service layer From e71d16545787e3def7e5ad5e87fb6886299b589b Mon Sep 17 00:00:00 2001 From: Joern Wellniak Date: Fri, 24 Oct 2025 08:35:43 +0200 Subject: [PATCH 06/15] ADR --- docs/architecture/decisions/ADR-004-Lombok.md | 49 +++++++++++++++ .../ADR-005-Repository-uses-entity.md | 43 ++++++++++++++ docs/architecture/decisions/ADR-006-var.md | 37 ++++++++++++ .../decisions/ADR-007-autowired.md | 59 +++++++++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 docs/architecture/decisions/ADR-004-Lombok.md create mode 100644 docs/architecture/decisions/ADR-005-Repository-uses-entity.md create mode 100644 docs/architecture/decisions/ADR-006-var.md create mode 100644 docs/architecture/decisions/ADR-007-autowired.md diff --git a/docs/architecture/decisions/ADR-004-Lombok.md b/docs/architecture/decisions/ADR-004-Lombok.md new file mode 100644 index 0000000..42d4321 --- /dev/null +++ b/docs/architecture/decisions/ADR-004-Lombok.md @@ -0,0 +1,49 @@ +# ADR-004: Use Lombok in TO + +Date: 2025-10-23 + +## Status + +Accepted + +## Context + +The TO (Technical Operations) project involves a significant amount of boilerplate code, such as +getters, setters, constructors, and logging. Writing and maintaining this repetitive code increases +development time and the likelihood of errors. A solution is needed to reduce boilerplate while +maintaining code readability and consistency. + +## Decision + +The project will use Lombok to reduce boilerplate code. Lombok annotations will be applied to +generate commonly used methods like getters, setters, constructors, and `toString()` automatically. +This will improve developer productivity and maintain cleaner code. + +## Consequences + +- **Positive**: + - Reduced boilerplate code. + - Improved readability and maintainability. + - Faster development time. + +- **Negative**: + - Adds a dependency to the project. + - Potential learning curve for developers unfamiliar with Lombok. + - IDE support may require additional configuration. + +## Implementation + +1. Add Lombok as a dependency in the `pom.xml`: + ```xml + + org.projectlombok + lombok + 1.18.30 + provided + + + +2. Use Lombok for all TO, VO and entities: + - Use `@Data` for simple data carriers. + - Use `@Builder` for complex object construction. + - Use `@Slf4j` for logging. diff --git a/docs/architecture/decisions/ADR-005-Repository-uses-entity.md b/docs/architecture/decisions/ADR-005-Repository-uses-entity.md new file mode 100644 index 0000000..7d850ff --- /dev/null +++ b/docs/architecture/decisions/ADR-005-Repository-uses-entity.md @@ -0,0 +1,43 @@ +# ADR-005: Repositories Should Prefer Returning Entities + +Date: 2025-10-23 + +## Status + +Accepted + +## Context + +In the current project, repositories are responsible for data access and retrieval. There is +inconsistency in the return types of repository methods, with some returning entities and others +returning DTOs or projections. This inconsistency can lead to confusion, increased complexity, and +duplication of logic across layers. A unified approach is needed to ensure clarity and +maintainability. + +## Decision + +Repository methods should prefer returning entities instead of DTOs or projections. This ensures +that the domain model remains the central representation of data and avoids duplicating business +logic in multiple layers. + +## Consequences + +- **Positive**: + - Simplifies repository implementation. + - Reduces duplication of logic across layers. + - Ensures consistency in data handling. + - Promotes the use of domain-driven design principles. + +- **Negative**: + - May require additional mapping to convert entities to DTOs in the service layer. + - Potential over-fetching of data if entities contain unnecessary relationships. + +## Implementation + +1. Update repository methods to return entities: + - Use `@Entity`-annotated classes as return types for repository methods. + - Avoid returning DTOs or projections directly from repositories. + +2. Perform any necessary mapping from entities to DTOs in the service layer. + +3. Document this decision in the project's coding standards to ensure consistency across the team. diff --git a/docs/architecture/decisions/ADR-006-var.md b/docs/architecture/decisions/ADR-006-var.md new file mode 100644 index 0000000..e053e33 --- /dev/null +++ b/docs/architecture/decisions/ADR-006-var.md @@ -0,0 +1,37 @@ +# ADR-007: Use `var` Instead of Full Class Names + +Date: 2025-10-23 + +## Status + +Accepted + +## Context + +In Java, local variable type inference (`var`) was introduced in Java 10 to reduce verbosity and +improve code readability. Currently, the project uses explicit type declarations for local +variables, which can make the code more verbose and harder to read. Adopting `var` can simplify the +code while maintaining clarity. + +## Decision + +The project will use `var` for local variable declarations where the type is obvious from the +context. This will reduce verbosity and improve code readability. + +## Consequences + +- **Positive**: + - Reduces boilerplate code. + - Improves readability by focusing on variable names and logic. + - Simplifies refactoring, as the type is inferred automatically. + +- **Negative**: + - May reduce clarity in cases where the type is not immediately obvious. + - Requires developers to be familiar with type inference. + +## Implementation + +1. Use `var` for local variables where the type is clear: + ```java + var list = new ArrayList(); + var count = 10; diff --git a/docs/architecture/decisions/ADR-007-autowired.md b/docs/architecture/decisions/ADR-007-autowired.md new file mode 100644 index 0000000..ed0a641 --- /dev/null +++ b/docs/architecture/decisions/ADR-007-autowired.md @@ -0,0 +1,59 @@ +# ADR-007: Use `@Autowired` on Private Members Instead of Constructors + +Date: 2025-10-23 + +## Status + +Accepted + +## Context + +In the current project, dependency injection is primarily performed through constructors. While this approach is explicit and aligns with best practices, it can lead to verbose constructors when there are many dependencies. Using `@Autowired` on private members can simplify the code and improve readability. Additionally, integration tests require a consistent approach to mocking dependencies, and SpringMocks provides a convenient way to handle this. + +## Decision + +The project will use `@Autowired` on private members for dependency injection instead of constructors. Integration tests will use SpringMocks to mock dependencies and ensure consistent testing behavior. + +## Consequences + +- **Positive**: + - Reduces verbosity in constructors. + - Simplifies class structure by removing explicit constructor injection. + - Improves readability and focuses on the business logic. + - SpringMocks simplifies mocking in integration tests. + +- **Negative**: + - May reduce clarity as dependencies are not explicitly listed in constructors. + - Private member injection is less conventional and may require explanation for new developers. + +## Implementation + +1. Use `@Autowired` on private members for dependency injection: + ```java + @Service + public class ExampleService { + + @Autowired + private ExampleRepository exampleRepository; + + public void performAction() { + exampleRepository.save(new ExampleEntity()); + } + } + + @SpringBootTest + public class ExampleServiceTest { + + @Autowired + private ExampleService exampleService; + + @MockBean + private ExampleRepository exampleRepository; + + @Test + public void testPerformAction() { + exampleService.performAction(); + verify(exampleRepository).save(any(ExampleEntity.class)); + } + } +``` From 485c14ad5e79d3bf2e68cadf019ee315a5b17769 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 06:45:49 +0000 Subject: [PATCH 07/15] Refactor according to ADR-004, ADR-005, and ADR-007 - Add Lombok dependency and use @Data, @NoArgsConstructor, @AllArgsConstructor for entities - Change PlayerRepository to return List instead of Object[] - Add findLatestScoresForPlayers method to PlayerScoreRepository - Refactor PlayersServiceImpl to use @Autowired on private members instead of constructor injection - Update listPlayers to fetch entities first, then load scores separately as per ADR-005 - Update PlayersController to use @Autowired on private members - Update unit tests to match new repository methods Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- pom.xml | 7 ++ .../api/controller/PlayersController.java | 8 +- .../application/PlayersServiceImpl.java | 104 +++++++++++------- .../backend/domain/entities/GameEntity.java | 71 +----------- .../backend/domain/entities/PlayerEntity.java | 33 +----- .../domain/entities/PlayerScoreEntity.java | 55 +-------- .../domain/repositories/PlayerRepository.java | 28 +---- .../repositories/PlayerScoreRepository.java | 13 +++ .../application/PlayersServiceTest.java | 31 +++--- 9 files changed, 126 insertions(+), 224 deletions(-) diff --git a/pom.xml b/pom.xml index ec73c23..5488a65 100644 --- a/pom.xml +++ b/pom.xml @@ -48,6 +48,13 @@ spring-boot-starter-validation + + org.projectlombok + lombok + 1.18.30 + provided + + org.postgresql postgresql diff --git a/src/main/java/com/skat/backend/api/controller/PlayersController.java b/src/main/java/com/skat/backend/api/controller/PlayersController.java index 1c119b0..1738516 100644 --- a/src/main/java/com/skat/backend/api/controller/PlayersController.java +++ b/src/main/java/com/skat/backend/api/controller/PlayersController.java @@ -5,6 +5,7 @@ import jakarta.validation.Valid; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.*; @@ -18,11 +19,8 @@ @Validated public class PlayersController { - private final PlayersService playersService; - - public PlayersController(PlayersService playersService) { - this.playersService = playersService; - } + @Autowired + private PlayersService playersService; @GetMapping public ResponseEntity listPlayers( diff --git a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java index 99768d7..f83502a 100644 --- a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java +++ b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java @@ -4,63 +4,89 @@ import com.skat.backend.api.exception.NotFoundException; import com.skat.backend.application.dto.*; import com.skat.backend.domain.entities.PlayerEntity; +import com.skat.backend.domain.entities.PlayerScoreEntity; import com.skat.backend.domain.repositories.GameRepository; import com.skat.backend.domain.repositories.PlayerRepository; import com.skat.backend.domain.repositories.PlayerScoreRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.math.BigInteger; -import java.sql.Timestamp; import java.time.OffsetDateTime; -import java.time.ZoneId; -import java.util.ArrayList; -import java.util.List; -import java.util.UUID; +import java.util.*; +import java.util.stream.Collectors; @Service public class PlayersServiceImpl implements PlayersService { - private final PlayerRepository playerRepository; - private final GameRepository gameRepository; - private final PlayerScoreRepository playerScoreRepository; + @Autowired + private PlayerRepository playerRepository; - public PlayersServiceImpl(PlayerRepository playerRepository, - GameRepository gameRepository, - PlayerScoreRepository playerScoreRepository) { - this.playerRepository = playerRepository; - this.gameRepository = gameRepository; - this.playerScoreRepository = playerScoreRepository; - } + @Autowired + private GameRepository gameRepository; + + @Autowired + private PlayerScoreRepository playerScoreRepository; @Override @Transactional(readOnly = true) public PlayerListResponseTO listPlayers(PlayersQuery query) { - String sortValue = query.sort().name(); - List results = playerRepository.findPlayersWithLatestScore( - sortValue, - query.startIndex(), - query.pageSize() - ); + Pageable pageable = PageRequest.of(query.startIndex() / query.pageSize(), query.pageSize()); - List items = new ArrayList<>(); - for (Object[] row : results) { - UUID id = row[0] instanceof UUID ? (UUID) row[0] : UUID.fromString(row[0].toString()); - String firstName = (String) row[1]; - String lastName = (String) row[2]; - int totalPoints = row[3] instanceof Number ? ((Number) row[3]).intValue() : 0; - int sequenceIndex = row[4] instanceof Number ? ((Number) row[4]).intValue() : 0; - - OffsetDateTime updatedAt; - if (row[5] instanceof Timestamp) { - updatedAt = ((Timestamp) row[5]).toInstant().atZone(ZoneId.systemDefault()).toOffsetDateTime(); - } else if (row[5] instanceof OffsetDateTime) { - updatedAt = (OffsetDateTime) row[5]; - } else { - updatedAt = OffsetDateTime.now(); + // Fetch players based on sort + List players; + if (query.sort() == PlayersSort.NAME) { + players = playerRepository.findAllOrderedByName(pageable); + } else { + players = playerRepository.findAllPlayers(pageable); + } + + // Extract player IDs + List playerIds = players.stream() + .map(PlayerEntity::getId) + .collect(Collectors.toList()); + + // Fetch latest scores for these players + Map latestScores = new HashMap<>(); + if (!playerIds.isEmpty()) { + List scores = playerScoreRepository.findLatestScoresForPlayers(playerIds); + for (PlayerScoreEntity score : scores) { + if (score.getPlayer() != null) { + latestScores.put(score.getPlayer().getId(), score); + } } - - items.add(new PlayerWithScoreTO(id, firstName, lastName, totalPoints, sequenceIndex, updatedAt)); + } + + // Map to DTOs + List items = players.stream() + .map(player -> { + PlayerScoreEntity score = latestScores.get(player.getId()); + int totalPoints = score != null ? score.getTotalPoints() : 0; + int sequenceIndex = score != null ? score.getSequenceIndex() : 0; + OffsetDateTime updatedAt = score != null ? score.getCreatedAt() : OffsetDateTime.now(); + + return new PlayerWithScoreTO( + player.getId(), + player.getFirstName(), + player.getLastName(), + totalPoints, + sequenceIndex, + updatedAt + ); + }) + .collect(Collectors.toList()); + + // Sort by score if needed + if (query.sort() == PlayersSort.SCORE_DESC) { + items.sort((a, b) -> { + int scoreCompare = Integer.compare(b.current_total_points(), a.current_total_points()); + if (scoreCompare != 0) return scoreCompare; + int lastNameCompare = a.last_name().compareTo(b.last_name()); + if (lastNameCompare != 0) return lastNameCompare; + return a.first_name().compareTo(b.first_name()); + }); } long total = playerRepository.count(); diff --git a/src/main/java/com/skat/backend/domain/entities/GameEntity.java b/src/main/java/com/skat/backend/domain/entities/GameEntity.java index 3dd6d23..1683df8 100644 --- a/src/main/java/com/skat/backend/domain/entities/GameEntity.java +++ b/src/main/java/com/skat/backend/domain/entities/GameEntity.java @@ -1,6 +1,8 @@ package com.skat.backend.domain.entities; import jakarta.persistence.*; +import lombok.Data; +import lombok.NoArgsConstructor; import java.time.OffsetDateTime; import java.util.UUID; @@ -12,6 +14,8 @@ @Index(name = "game_played_at_IDX", columnList = "played_at") } ) +@Data +@NoArgsConstructor public class GameEntity { @Id @@ -42,71 +46,4 @@ public class GameEntity { @Column(name = "played_at", nullable = false) private OffsetDateTime playedAt; - - public GameEntity() { - } - - public UUID getId() { - return id; - } - - public void setId(UUID id) { - this.id = id; - } - - public PlayerEntity getPlayer1() { - return player1; - } - - public void setPlayer1(PlayerEntity player1) { - this.player1 = player1; - } - - public PlayerEntity getPlayer2() { - return player2; - } - - public void setPlayer2(PlayerEntity player2) { - this.player2 = player2; - } - - public PlayerEntity getPlayer3() { - return player3; - } - - public void setPlayer3(PlayerEntity player3) { - this.player3 = player3; - } - - public PlayerEntity getMainPlayer() { - return mainPlayer; - } - - public void setMainPlayer(PlayerEntity mainPlayer) { - this.mainPlayer = mainPlayer; - } - - public Integer getBidValue() { - return bidValue; - } - - public void setBidValue(Integer bidValue) { - this.bidValue = bidValue; - } - - public Integer getScore() { - return score; - } - - public void setScore(Integer score) { - this.score = score; - } - - public OffsetDateTime getPlayedAt() { - return playedAt; - } - - public void setPlayedAt(OffsetDateTime playedAt) { - this.playedAt = playedAt; - } } diff --git a/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java b/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java index 94d4a6b..c8a8487 100644 --- a/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java +++ b/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java @@ -1,6 +1,9 @@ package com.skat.backend.domain.entities; import jakarta.persistence.*; +import lombok.Data; +import lombok.NoArgsConstructor; +import lombok.AllArgsConstructor; import java.util.UUID; @Entity @@ -14,6 +17,9 @@ @Index(name = "player_last_name_IDX", columnList = "last_name") } ) +@Data +@NoArgsConstructor +@AllArgsConstructor public class PlayerEntity { @Id @@ -26,35 +32,8 @@ public class PlayerEntity { @Column(name = "last_name", nullable = false, length = 50) private String lastName; - public PlayerEntity() { - } - public PlayerEntity(String firstName, String lastName) { this.firstName = firstName; this.lastName = lastName; } - - public UUID getId() { - return id; - } - - public void setId(UUID id) { - this.id = id; - } - - public String getFirstName() { - return firstName; - } - - public void setFirstName(String firstName) { - this.firstName = firstName; - } - - public String getLastName() { - return lastName; - } - - public void setLastName(String lastName) { - this.lastName = lastName; - } } diff --git a/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java b/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java index 88221cf..e43cbd1 100644 --- a/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java +++ b/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java @@ -1,6 +1,8 @@ package com.skat.backend.domain.entities; import jakarta.persistence.*; +import lombok.Data; +import lombok.NoArgsConstructor; import java.time.OffsetDateTime; import java.util.UUID; @@ -13,6 +15,8 @@ @Index(name = "player_score_sequence_IDX", columnList = "sequence_index") } ) +@Data +@NoArgsConstructor public class PlayerScoreEntity { @Id @@ -35,55 +39,4 @@ public class PlayerScoreEntity { @Column(name = "created_at", nullable = false) private OffsetDateTime createdAt; - - public PlayerScoreEntity() { - } - - public UUID getId() { - return id; - } - - public void setId(UUID id) { - this.id = id; - } - - public PlayerEntity getPlayer() { - return player; - } - - public void setPlayer(PlayerEntity player) { - this.player = player; - } - - public GameEntity getGame() { - return game; - } - - public void setGame(GameEntity game) { - this.game = game; - } - - public Integer getSequenceIndex() { - return sequenceIndex; - } - - public void setSequenceIndex(Integer sequenceIndex) { - this.sequenceIndex = sequenceIndex; - } - - public Integer getTotalPoints() { - return totalPoints; - } - - public void setTotalPoints(Integer totalPoints) { - this.totalPoints = totalPoints; - } - - public OffsetDateTime getCreatedAt() { - return createdAt; - } - - public void setCreatedAt(OffsetDateTime createdAt) { - this.createdAt = createdAt; - } } diff --git a/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java b/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java index 3e94a2c..ff510e7 100644 --- a/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java +++ b/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java @@ -1,6 +1,7 @@ package com.skat.backend.domain.repositories; import com.skat.backend.domain.entities.PlayerEntity; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; @@ -16,26 +17,9 @@ public interface PlayerRepository extends JpaRepository { boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(String firstName, String lastName, UUID id); - @Query(value = """ - SELECT p.id, p.first_name, p.last_name, - COALESCE(ps.total_points, 0) as current_total_points, - COALESCE(ps.sequence_index, 0) as current_sequence_index, - COALESCE(ps.created_at, CURRENT_TIMESTAMP) as updated_at - FROM player p - LEFT JOIN LATERAL ( - SELECT player_id, total_points, sequence_index, created_at - FROM player_score - WHERE player_id = p.id - ORDER BY sequence_index DESC - LIMIT 1 - ) ps ON true - ORDER BY - CASE WHEN :sort = 'SCORE_DESC' THEN COALESCE(ps.total_points, 0) END DESC, - CASE WHEN :sort = 'NAME' THEN p.last_name END ASC, - CASE WHEN :sort = 'SCORE_DESC' THEN p.last_name END ASC, - p.first_name ASC - OFFSET :offset ROWS - FETCH FIRST :limit ROWS ONLY - """, nativeQuery = true) - List findPlayersWithLatestScore(@Param("sort") String sort, @Param("offset") int offset, @Param("limit") int limit); + @Query("SELECT p FROM PlayerEntity p ORDER BY p.lastName ASC, p.firstName ASC") + List findAllOrderedByName(Pageable pageable); + + @Query("SELECT p FROM PlayerEntity p") + List findAllPlayers(Pageable pageable); } diff --git a/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java b/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java index 9933773..3a247b7 100644 --- a/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java +++ b/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java @@ -7,6 +7,7 @@ import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; +import java.util.List; import java.util.UUID; @Repository @@ -19,4 +20,16 @@ public interface PlayerScoreRepository extends JpaRepository findLatestScoresForPlayers(@Param("playerIds") List playerIds); } diff --git a/src/test/java/com/skat/backend/application/PlayersServiceTest.java b/src/test/java/com/skat/backend/application/PlayersServiceTest.java index 205a3d9..ca3d603 100644 --- a/src/test/java/com/skat/backend/application/PlayersServiceTest.java +++ b/src/test/java/com/skat/backend/application/PlayersServiceTest.java @@ -4,6 +4,7 @@ import com.skat.backend.api.exception.NotFoundException; import com.skat.backend.application.dto.*; import com.skat.backend.domain.entities.PlayerEntity; +import com.skat.backend.domain.entities.PlayerScoreEntity; import com.skat.backend.domain.repositories.GameRepository; import com.skat.backend.domain.repositories.PlayerRepository; import com.skat.backend.domain.repositories.PlayerScoreRepository; @@ -14,11 +15,13 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.time.OffsetDateTime; import java.util.*; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.*; /** @@ -281,18 +284,19 @@ void given_validQuery_when_listPlayers_then_returnsPlayerListResponse() { // Given PlayersQuery query = new PlayersQuery(0, 50, PlayersSort.NAME); - List mockResults = new ArrayList<>(); - Object[] row1 = new Object[]{ - UUID.randomUUID(), - "Anna", - "Schmidt", - 100, - 5, - java.sql.Timestamp.valueOf("2024-01-01 10:00:00") - }; - mockResults.add(row1); - - when(playerRepository.findPlayersWithLatestScore("NAME", 0, 50)).thenReturn(mockResults); + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + List players = List.of(player); + + PlayerScoreEntity score = new PlayerScoreEntity(); + score.setPlayer(player); + score.setTotalPoints(100); + score.setSequenceIndex(5); + score.setCreatedAt(OffsetDateTime.now()); + + when(playerRepository.findAllOrderedByName(any())).thenReturn(players); + when(playerScoreRepository.findLatestScoresForPlayers(anyList())).thenReturn(List.of(score)); when(playerRepository.count()).thenReturn(1L); // When @@ -310,7 +314,8 @@ void given_validQuery_when_listPlayers_then_returnsPlayerListResponse() { assertThat(result.paging().total()).isEqualTo(1L); assertThat(result.sort()).isEqualTo(PlayersSort.NAME); - verify(playerRepository).findPlayersWithLatestScore("NAME", 0, 50); + verify(playerRepository).findAllOrderedByName(any()); + verify(playerScoreRepository).findLatestScoresForPlayers(anyList()); verify(playerRepository).count(); } } From 551a809541df56834ced1a822a03d9629c5affa8 Mon Sep 17 00:00:00 2001 From: Joern Wellniak Date: Fri, 24 Oct 2025 09:16:30 +0200 Subject: [PATCH 08/15] ADR --- .github/workflows/build.yml | 6 +- .../ADR-002-Testing-Strategy-Unit.md | 1 + .../decisions/ADR-008-autowired.md | 72 +++++++++++++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 docs/architecture/decisions/ADR-008-autowired.md diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 13865f1..aa68dd0 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,11 +29,7 @@ jobs: - name: Build Backend with Maven run: | - mvn clean package -DskipTests - - - name: Run Backend Tests - run: | - mvn test + mvn clean verify --no-transfer-progress - name: Upload Backend Artifacts uses: actions/upload-artifact@v4 diff --git a/docs/architecture/decisions/ADR-002-Testing-Strategy-Unit.md b/docs/architecture/decisions/ADR-002-Testing-Strategy-Unit.md index 41e6d84..f18e39e 100644 --- a/docs/architecture/decisions/ADR-002-Testing-Strategy-Unit.md +++ b/docs/architecture/decisions/ADR-002-Testing-Strategy-Unit.md @@ -80,6 +80,7 @@ class ScoreServiceTest { - Deterministic business rules, pure functions, simple orchestrations. - Behavior that can be isolated with mocks **without** relying on Spring. - **Choose Integration (Failsafe + @SpringBootTest + Testcontainers)** when testing: +- ** Avooid using - Spring DI, configuration, serialization, validation, repositories, transactions. - HTTP layer behavior, real database interactions, or cross-cutting concerns. diff --git a/docs/architecture/decisions/ADR-008-autowired.md b/docs/architecture/decisions/ADR-008-autowired.md new file mode 100644 index 0000000..a330482 --- /dev/null +++ b/docs/architecture/decisions/ADR-008-autowired.md @@ -0,0 +1,72 @@ +# ADR-008: Use `@SpringBootTest` Instead of `@MockMvc` + +Date: 2025-10-23 + +## Status + +Accepted + +## Context + +In the current project, integration tests are written using both `@SpringBootTest` and `@MockMvc`. +While `@MockMvc` is useful for testing specific layers (e.g., controllers) in isolation, it does not +load the full application context. This can lead to inconsistencies when testing features that rely +on the complete Spring Boot configuration. A unified approach is needed to ensure reliable and +consistent integration testing. + +## Decision + +The project will use `@SpringBootTest` for all integration tests. This ensures that the full +application context is loaded, providing a more comprehensive testing environment. While `@MockMvc` +will no longer be used for integration tests, alternatives such as `TestRestTemplate` or +`WebTestClient` can be considered for specific scenarios. + +## Alternatives + +1. **`@MockMvc`**: + - **Pros**: + - Faster as it does not load the full application context. + - Useful for testing controllers in isolation. + - **Cons**: + - Limited to the web layer. + - Does not test the full application behavior. + +2. **`TestRestTemplate`**: + - **Pros**: + - Provides a way to test REST endpoints with the full application context. + - **Cons**: + - Slower than `@MockMvc`. + +3. **`WebTestClient`**: + - **Pros**: + - Non-blocking and supports reactive applications. + - **Cons**: + - Requires additional setup for non-reactive applications. + +## Consequences + +- **Positive**: + - Ensures consistent testing by loading the full application context. + - Tests the application as a whole, including configuration and dependencies. + - Reduces the need for mocking in integration tests. + +- **Negative**: + - Slower test execution due to the full context initialization. + - May require additional resources for running tests. + +## Implementation + +1. Replace `@MockMvc` with `@SpringBootTest` in all integration tests: + ```java + @SpringBootTest + public class ExampleIntegrationTest { + + @Autowired + private TestRestTemplate restTemplate; + + @Test + public void testEndpoint() { + ResponseEntity response = restTemplate.getForEntity("/example", String.class); + assertEquals(HttpStatus.OK, response.getStatusCode()); + } + } From 4b2a91be1b6aafd9fb03aef25b12a8dc90b289b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 07:26:21 +0000 Subject: [PATCH 09/15] Apply ADR-006 (var) and ADR-008 (TestRestTemplate) - Replace explicit type declarations with var throughout PlayersServiceImpl as per ADR-006 - Replace @MockMvc with TestRestTemplate in PlayersControllerIT as per ADR-008 - Update integration tests to use @SpringBootTest with RANDOM_PORT and TestRestTemplate - Convert all test assertions to use AssertJ instead of jsonPath matchers - All 35 tests still passing (16 unit + 19 integration) Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- .../application/PlayersServiceImpl.java | 46 +- .../api/controller/PlayersControllerIT.java | 490 +++++++++++------- 2 files changed, 316 insertions(+), 220 deletions(-) diff --git a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java index f83502a..a8e9823 100644 --- a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java +++ b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java @@ -33,7 +33,7 @@ public class PlayersServiceImpl implements PlayersService { @Override @Transactional(readOnly = true) public PlayerListResponseTO listPlayers(PlayersQuery query) { - Pageable pageable = PageRequest.of(query.startIndex() / query.pageSize(), query.pageSize()); + var pageable = PageRequest.of(query.startIndex() / query.pageSize(), query.pageSize()); // Fetch players based on sort List players; @@ -44,15 +44,15 @@ public PlayerListResponseTO listPlayers(PlayersQuery query) { } // Extract player IDs - List playerIds = players.stream() + var playerIds = players.stream() .map(PlayerEntity::getId) .collect(Collectors.toList()); // Fetch latest scores for these players - Map latestScores = new HashMap<>(); + var latestScores = new HashMap(); if (!playerIds.isEmpty()) { - List scores = playerScoreRepository.findLatestScoresForPlayers(playerIds); - for (PlayerScoreEntity score : scores) { + var scores = playerScoreRepository.findLatestScoresForPlayers(playerIds); + for (var score : scores) { if (score.getPlayer() != null) { latestScores.put(score.getPlayer().getId(), score); } @@ -60,12 +60,12 @@ public PlayerListResponseTO listPlayers(PlayersQuery query) { } // Map to DTOs - List items = players.stream() + var items = players.stream() .map(player -> { - PlayerScoreEntity score = latestScores.get(player.getId()); - int totalPoints = score != null ? score.getTotalPoints() : 0; - int sequenceIndex = score != null ? score.getSequenceIndex() : 0; - OffsetDateTime updatedAt = score != null ? score.getCreatedAt() : OffsetDateTime.now(); + var score = latestScores.get(player.getId()); + var totalPoints = score != null ? score.getTotalPoints() : 0; + var sequenceIndex = score != null ? score.getSequenceIndex() : 0; + var updatedAt = score != null ? score.getCreatedAt() : OffsetDateTime.now(); return new PlayerWithScoreTO( player.getId(), @@ -81,16 +81,16 @@ public PlayerListResponseTO listPlayers(PlayersQuery query) { // Sort by score if needed if (query.sort() == PlayersSort.SCORE_DESC) { items.sort((a, b) -> { - int scoreCompare = Integer.compare(b.current_total_points(), a.current_total_points()); + var scoreCompare = Integer.compare(b.current_total_points(), a.current_total_points()); if (scoreCompare != 0) return scoreCompare; - int lastNameCompare = a.last_name().compareTo(b.last_name()); + var lastNameCompare = a.last_name().compareTo(b.last_name()); if (lastNameCompare != 0) return lastNameCompare; return a.first_name().compareTo(b.first_name()); }); } - long total = playerRepository.count(); - PagingTO paging = new PagingTO(query.startIndex(), query.pageSize(), total); + var total = playerRepository.count(); + var paging = new PagingTO(query.startIndex(), query.pageSize(), total); return new PlayerListResponseTO(items, paging, query.sort()); } @@ -98,8 +98,8 @@ public PlayerListResponseTO listPlayers(PlayersQuery query) { @Override @Transactional public PlayerTO createPlayer(UpsertPlayerRequest request) { - String firstName = request.first_name().trim(); - String lastName = request.last_name().trim(); + var firstName = request.first_name().trim(); + var lastName = request.last_name().trim(); if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName)) { throw new ConflictException( @@ -108,7 +108,7 @@ public PlayerTO createPlayer(UpsertPlayerRequest request) { ); } - PlayerEntity player = new PlayerEntity(firstName, lastName); + var player = new PlayerEntity(firstName, lastName); player = playerRepository.save(player); return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); @@ -117,11 +117,11 @@ public PlayerTO createPlayer(UpsertPlayerRequest request) { @Override @Transactional public PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request) { - PlayerEntity player = playerRepository.findById(id) + var player = playerRepository.findById(id) .orElseThrow(() -> new NotFoundException("Player not found", "id")); - String firstName = request.first_name().trim(); - String lastName = request.last_name().trim(); + var firstName = request.first_name().trim(); + var lastName = request.last_name().trim(); // Check if the new name conflicts with another player if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(firstName, lastName, id)) { @@ -141,12 +141,12 @@ public PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request) { @Override @Transactional public void deletePlayer(UUID id, boolean forceDeletion) { - PlayerEntity player = playerRepository.findById(id) + var player = playerRepository.findById(id) .orElseThrow(() -> new NotFoundException("Player not found", "id")); if (!forceDeletion) { - boolean hasGames = gameRepository.existsByPlayerId(id); - boolean hasScores = playerScoreRepository.existsByPlayerId(id); + var hasGames = gameRepository.existsByPlayerId(id); + var hasScores = playerScoreRepository.existsByPlayerId(id); if (hasGames || hasScores) { throw new ConflictException("Player is referenced in games or scores"); diff --git a/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java b/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java index e827e0f..80d33f4 100644 --- a/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java +++ b/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java @@ -1,5 +1,8 @@ package com.skat.backend.api.controller; +import com.skat.backend.application.dto.ErrorResponseTO; +import com.skat.backend.application.dto.PlayerListResponseTO; +import com.skat.backend.application.dto.PlayerTO; import com.skat.backend.domain.entities.GameEntity; import com.skat.backend.domain.entities.PlayerEntity; import com.skat.backend.domain.entities.PlayerScoreEntity; @@ -9,12 +12,15 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; -import org.springframework.test.web.servlet.MockMvc; import org.testcontainers.containers.PostgreSQLContainer; import org.testcontainers.junit.jupiter.Container; import org.testcontainers.junit.jupiter.Testcontainers; @@ -23,16 +29,12 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.*; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; /** - * Integration test for PlayersController following ADR-001 (Integration Testing Strategy with Testcontainers). - * Uses Testcontainers for PostgreSQL 18 and tests the full Spring Boot application. + * Integration test for PlayersController following ADR-001 and ADR-008. + * Uses Testcontainers for PostgreSQL 18 and TestRestTemplate for testing the full application context. */ -@SpringBootTest -@AutoConfigureMockMvc +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @Testcontainers class PlayersControllerIT { @@ -50,7 +52,7 @@ static void configureProperties(DynamicPropertyRegistry registry) { } @Autowired - private MockMvc mockMvc; + private TestRestTemplate restTemplate; @Autowired private PlayerRepository playerRepository; @@ -70,40 +72,43 @@ void setUp() { // AC-1: List players — default sorting and paging @Test - void given_existingPlayers_when_listPlayersWithoutParameters_then_returns200WithDefaultPaging() throws Exception { + void given_existingPlayers_when_listPlayersWithoutParameters_then_returns200WithDefaultPaging() { // Given - PlayerEntity player1 = new PlayerEntity("Anna", "Schmidt"); - PlayerEntity player2 = new PlayerEntity("Max", "Mueller"); + var player1 = new PlayerEntity("Anna", "Schmidt"); + var player2 = new PlayerEntity("Max", "Mueller"); playerRepository.save(player1); playerRepository.save(player2); - // When & Then - mockMvc.perform(get("/api/players")) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.items", hasSize(2))) - .andExpect(jsonPath("$.paging.startIndex", is(0))) - .andExpect(jsonPath("$.paging.pageSize", is(50))) - .andExpect(jsonPath("$.paging.total", is(2))) - .andExpect(jsonPath("$.sort", is("NAME"))); + // When + var response = restTemplate.getForEntity("/api/players", PlayerListResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().items()).hasSize(2); + assertThat(response.getBody().paging().startIndex()).isEqualTo(0); + assertThat(response.getBody().paging().pageSize()).isEqualTo(50); + assertThat(response.getBody().paging().total()).isEqualTo(2); + assertThat(response.getBody().sort().name()).isEqualTo("NAME"); } // AC-2: List players — score_desc ordering @Test - void given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returnsOrderedByScoreDesc() throws Exception { + void given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returnsOrderedByScoreDesc() { // Given - PlayerEntity player1 = new PlayerEntity("Anna", "Schmidt"); - PlayerEntity player2 = new PlayerEntity("Max", "Mueller"); - PlayerEntity player3 = new PlayerEntity("Lisa", "Bauer"); + var player1 = new PlayerEntity("Anna", "Schmidt"); + var player2 = new PlayerEntity("Max", "Mueller"); + var player3 = new PlayerEntity("Lisa", "Bauer"); player1 = playerRepository.save(player1); player2 = playerRepository.save(player2); player3 = playerRepository.save(player3); - GameEntity game = new GameEntity(); + var game = new GameEntity(); game.setPlayedAt(OffsetDateTime.now()); game = gameRepository.save(game); // Create scores for players - PlayerScoreEntity score1 = new PlayerScoreEntity(); + var score1 = new PlayerScoreEntity(); score1.setPlayer(player1); score1.setGame(game); score1.setSequenceIndex(0); @@ -111,7 +116,7 @@ void given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returns score1.setCreatedAt(OffsetDateTime.now()); playerScoreRepository.save(score1); - PlayerScoreEntity score2 = new PlayerScoreEntity(); + var score2 = new PlayerScoreEntity(); score2.setPlayer(player2); score2.setGame(game); score2.setSequenceIndex(0); @@ -119,7 +124,7 @@ void given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returns score2.setCreatedAt(OffsetDateTime.now()); playerScoreRepository.save(score2); - PlayerScoreEntity score3 = new PlayerScoreEntity(); + var score3 = new PlayerScoreEntity(); score3.setPlayer(player3); score3.setGame(game); score3.setSequenceIndex(0); @@ -127,71 +132,83 @@ void given_playersWithDifferentScores_when_listPlayersSortedByScore_then_returns score3.setCreatedAt(OffsetDateTime.now()); playerScoreRepository.save(score3); - // When & Then - mockMvc.perform(get("/api/players") - .param("sort", "SCORE_DESC")) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.items", hasSize(3))) - .andExpect(jsonPath("$.items[0].first_name", is("Max"))) - .andExpect(jsonPath("$.items[0].current_total_points", is(200))) - .andExpect(jsonPath("$.items[1].first_name", is("Lisa"))) - .andExpect(jsonPath("$.items[1].current_total_points", is(150))) - .andExpect(jsonPath("$.items[2].first_name", is("Anna"))) - .andExpect(jsonPath("$.items[2].current_total_points", is(100))) - .andExpect(jsonPath("$.sort", is("SCORE_DESC"))); + // When + var response = restTemplate.getForEntity("/api/players?sort=SCORE_DESC", PlayerListResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().items()).hasSize(3); + assertThat(response.getBody().items().get(0).first_name()).isEqualTo("Max"); + assertThat(response.getBody().items().get(0).current_total_points()).isEqualTo(200); + assertThat(response.getBody().items().get(1).first_name()).isEqualTo("Lisa"); + assertThat(response.getBody().items().get(1).current_total_points()).isEqualTo(150); + assertThat(response.getBody().items().get(2).first_name()).isEqualTo("Anna"); + assertThat(response.getBody().items().get(2).current_total_points()).isEqualTo(100); + assertThat(response.getBody().sort().name()).isEqualTo("SCORE_DESC"); } // AC-3: List players — parameter validation @Test - void given_invalidPageSize_when_listPlayers_then_returns400() throws Exception { - // When & Then - mockMvc.perform(get("/api/players") - .param("pageSize", "0")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", is("bad_request"))) - .andExpect(jsonPath("$.field", is("pageSize"))); + void given_invalidPageSize_when_listPlayers_then_returns400() { + // When + var response = restTemplate.getForEntity("/api/players?pageSize=0", ErrorResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("bad_request"); + assertThat(response.getBody().field()).isEqualTo("pageSize"); } @Test - void given_negativeStartIndex_when_listPlayers_then_returns400() throws Exception { - // When & Then - mockMvc.perform(get("/api/players") - .param("startIndex", "-1")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", is("bad_request"))) - .andExpect(jsonPath("$.field", is("startIndex"))); + void given_negativeStartIndex_when_listPlayers_then_returns400() { + // When + var response = restTemplate.getForEntity("/api/players?startIndex=-1", ErrorResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("bad_request"); + assertThat(response.getBody().field()).isEqualTo("startIndex"); } @Test - void given_pageSizeTooLarge_when_listPlayers_then_returns400() throws Exception { - // When & Then - mockMvc.perform(get("/api/players") - .param("pageSize", "201")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", is("bad_request"))) - .andExpect(jsonPath("$.field", is("pageSize"))); + void given_pageSizeTooLarge_when_listPlayers_then_returns400() { + // When + var response = restTemplate.getForEntity("/api/players?pageSize=201", ErrorResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("bad_request"); + assertThat(response.getBody().field()).isEqualTo("pageSize"); } // AC-4: Create player — unique full name @Test - void given_uniquePlayerName_when_createPlayer_then_returns201WithLocation() throws Exception { + void given_uniquePlayerName_when_createPlayer_then_returns201WithLocation() { // Given - String requestBody = """ + var requestBody = """ { "first_name": "Anna", "last_name": "Schmidt" } """; + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); - // When & Then - mockMvc.perform(post("/api/players") - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isCreated()) - .andExpect(header().exists("Location")) - .andExpect(jsonPath("$.id", notNullValue())) - .andExpect(jsonPath("$.first_name", is("Anna"))) - .andExpect(jsonPath("$.last_name", is("Schmidt"))); + // When + var response = restTemplate.postForEntity("/api/players", request, PlayerTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED); + assertThat(response.getHeaders().getLocation()).isNotNull(); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().id()).isNotNull(); + assertThat(response.getBody().first_name()).isEqualTo("Anna"); + assertThat(response.getBody().last_name()).isEqualTo("Schmidt"); // Verify player was created assertThat(playerRepository.findAll()).hasSize(1); @@ -199,165 +216,214 @@ void given_uniquePlayerName_when_createPlayer_then_returns201WithLocation() thro // AC-5: Create player — conflict on duplicate full name @Test - void given_existingPlayerName_when_createPlayerWithSameName_then_returns409() throws Exception { + void given_existingPlayerName_when_createPlayerWithSameName_then_returns409() { // Given - PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); + var existingPlayer = new PlayerEntity("Anna", "Schmidt"); playerRepository.save(existingPlayer); - String requestBody = """ + var requestBody = """ { "first_name": "Anna", "last_name": "Schmidt" } """; - - // When & Then - mockMvc.perform(post("/api/players") - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isConflict()) - .andExpect(jsonPath("$.error", is("conflict"))) - .andExpect(jsonPath("$.field", is("first_name,last_name"))); + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); + + // When + var response = restTemplate.postForEntity("/api/players", request, ErrorResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("conflict"); + assertThat(response.getBody().field()).isEqualTo("first_name,last_name"); } @Test - void given_missingFirstName_when_createPlayer_then_returns400() throws Exception { + void given_missingFirstName_when_createPlayer_then_returns400() { // Given - String requestBody = """ + var requestBody = """ { "last_name": "Schmidt" } """; + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); - // When & Then - mockMvc.perform(post("/api/players") - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", is("bad_request"))); + // When + var response = restTemplate.postForEntity("/api/players", request, ErrorResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("bad_request"); } @Test - void given_blankFirstName_when_createPlayer_then_returns400() throws Exception { + void given_blankFirstName_when_createPlayer_then_returns400() { // Given - String requestBody = """ + var requestBody = """ { "first_name": " ", "last_name": "Schmidt" } """; + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); + + // When + var response = restTemplate.postForEntity("/api/players", request, ErrorResponseTO.class); - // When & Then - mockMvc.perform(post("/api/players") - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", is("bad_request"))); + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("bad_request"); } @Test - void given_nameTooLong_when_createPlayer_then_returns400() throws Exception { + void given_nameTooLong_when_createPlayer_then_returns400() { // Given - String longName = "a".repeat(51); - String requestBody = String.format(""" + var longName = "a".repeat(51); + var requestBody = String.format(""" { "first_name": "%s", "last_name": "Schmidt" } """, longName); + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); - // When & Then - mockMvc.perform(post("/api/players") - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", is("bad_request"))); + // When + var response = restTemplate.postForEntity("/api/players", request, ErrorResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("bad_request"); } // AC-6: Update player — id must exist @Test - void given_nonExistentPlayerId_when_updatePlayer_then_returns404() throws Exception { + void given_nonExistentPlayerId_when_updatePlayer_then_returns404() { // Given - UUID nonExistentId = UUID.randomUUID(); - String requestBody = """ + var nonExistentId = UUID.randomUUID(); + var requestBody = """ { "first_name": "Anna", "last_name": "Bauer" } """; - - // When & Then - mockMvc.perform(put("/api/players/" + nonExistentId) - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isNotFound()) - .andExpect(jsonPath("$.error", is("not_found"))) - .andExpect(jsonPath("$.field", is("id"))); + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); + + // When + var response = restTemplate.exchange( + "/api/players/" + nonExistentId, + HttpMethod.PUT, + request, + ErrorResponseTO.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("not_found"); + assertThat(response.getBody().field()).isEqualTo("id"); } // AC-7: Update player — uniqueness enforced @Test - void given_twoPlayers_when_updatePlayerToExistingName_then_returns409() throws Exception { + void given_twoPlayers_when_updatePlayerToExistingName_then_returns409() { // Given - PlayerEntity player1 = new PlayerEntity("Anna", "Schmidt"); - PlayerEntity player2 = new PlayerEntity("Anna", "Mueller"); + var player1 = new PlayerEntity("Anna", "Schmidt"); + var player2 = new PlayerEntity("Anna", "Mueller"); player1 = playerRepository.save(player1); player2 = playerRepository.save(player2); - String requestBody = """ + var requestBody = """ { "first_name": "Anna", "last_name": "Schmidt" } """; - - // When & Then - mockMvc.perform(put("/api/players/" + player2.getId()) - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isConflict()) - .andExpect(jsonPath("$.error", is("conflict"))) - .andExpect(jsonPath("$.field", is("first_name,last_name"))); + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); + + // When + var response = restTemplate.exchange( + "/api/players/" + player2.getId(), + HttpMethod.PUT, + request, + ErrorResponseTO.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("conflict"); + assertThat(response.getBody().field()).isEqualTo("first_name,last_name"); } @Test - void given_existingPlayer_when_updatePlayerToNewName_then_returns200() throws Exception { + void given_existingPlayer_when_updatePlayerToNewName_then_returns200() { // Given - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + var player = new PlayerEntity("Anna", "Schmidt"); player = playerRepository.save(player); - String requestBody = """ + var requestBody = """ { "first_name": "Anna", "last_name": "Mueller" } """; - - // When & Then - mockMvc.perform(put("/api/players/" + player.getId()) - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.id", is(player.getId().toString()))) - .andExpect(jsonPath("$.first_name", is("Anna"))) - .andExpect(jsonPath("$.last_name", is("Mueller"))); + var headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + var request = new HttpEntity<>(requestBody, headers); + + // When + var response = restTemplate.exchange( + "/api/players/" + player.getId(), + HttpMethod.PUT, + request, + PlayerTO.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().id()).isEqualTo(player.getId()); + assertThat(response.getBody().first_name()).isEqualTo("Anna"); + assertThat(response.getBody().last_name()).isEqualTo("Mueller"); // Verify player was updated - PlayerEntity updatedPlayer = playerRepository.findById(player.getId()).orElseThrow(); + var updatedPlayer = playerRepository.findById(player.getId()).orElseThrow(); assertThat(updatedPlayer.getLastName()).isEqualTo("Mueller"); } // AC-8: Delete player — safe delete without references @Test - void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_returns204() throws Exception { + void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_returns204() { // Given - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + var player = new PlayerEntity("Anna", "Schmidt"); player = playerRepository.save(player); - // When & Then - mockMvc.perform(delete("/api/players/" + player.getId()) - .param("forceDeletion", "false")) - .andExpect(status().isNoContent()); + // When + var response = restTemplate.exchange( + "/api/players/" + player.getId() + "?forceDeletion=false", + HttpMethod.DELETE, + null, + Void.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT); // Verify player was deleted assertThat(playerRepository.findById(player.getId())).isEmpty(); @@ -365,37 +431,44 @@ void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_returns204 // AC-9: Delete player — conflict when referenced @Test - void given_playerReferencedInGame_when_deletePlayerWithoutForce_then_returns409() throws Exception { + void given_playerReferencedInGame_when_deletePlayerWithoutForce_then_returns409() { // Given - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + var player = new PlayerEntity("Anna", "Schmidt"); player = playerRepository.save(player); - GameEntity game = new GameEntity(); + var game = new GameEntity(); game.setPlayer1(player); game.setPlayedAt(OffsetDateTime.now()); gameRepository.save(game); - // When & Then - mockMvc.perform(delete("/api/players/" + player.getId()) - .param("forceDeletion", "false")) - .andExpect(status().isConflict()) - .andExpect(jsonPath("$.error", is("conflict"))); + // When + var response = restTemplate.exchange( + "/api/players/" + player.getId() + "?forceDeletion=false", + HttpMethod.DELETE, + null, + ErrorResponseTO.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("conflict"); // Verify player was not deleted assertThat(playerRepository.findById(player.getId())).isPresent(); } @Test - void given_playerReferencedInScore_when_deletePlayerWithoutForce_then_returns409() throws Exception { + void given_playerReferencedInScore_when_deletePlayerWithoutForce_then_returns409() { // Given - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + var player = new PlayerEntity("Anna", "Schmidt"); player = playerRepository.save(player); - GameEntity game = new GameEntity(); + var game = new GameEntity(); game.setPlayedAt(OffsetDateTime.now()); game = gameRepository.save(game); - PlayerScoreEntity score = new PlayerScoreEntity(); + var score = new PlayerScoreEntity(); score.setPlayer(player); score.setGame(game); score.setSequenceIndex(0); @@ -403,11 +476,18 @@ void given_playerReferencedInScore_when_deletePlayerWithoutForce_then_returns409 score.setCreatedAt(OffsetDateTime.now()); playerScoreRepository.save(score); - // When & Then - mockMvc.perform(delete("/api/players/" + player.getId()) - .param("forceDeletion", "false")) - .andExpect(status().isConflict()) - .andExpect(jsonPath("$.error", is("conflict"))); + // When + var response = restTemplate.exchange( + "/api/players/" + player.getId() + "?forceDeletion=false", + HttpMethod.DELETE, + null, + ErrorResponseTO.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("conflict"); // Verify player was not deleted assertThat(playerRepository.findById(player.getId())).isPresent(); @@ -415,19 +495,19 @@ void given_playerReferencedInScore_when_deletePlayerWithoutForce_then_returns409 // AC-10: Delete player — forced deletion nullifies references @Test - void given_playerReferencedInGameAndScore_when_forceDeletePlayer_then_returns204AndNullifies() throws Exception { + void given_playerReferencedInGameAndScore_when_forceDeletePlayer_then_returns204AndNullifies() { // Given - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + var player = new PlayerEntity("Anna", "Schmidt"); player = playerRepository.save(player); - GameEntity game = new GameEntity(); + var game = new GameEntity(); game.setPlayer1(player); game.setPlayer2(player); game.setMainPlayer(player); game.setPlayedAt(OffsetDateTime.now()); game = gameRepository.save(game); - PlayerScoreEntity score = new PlayerScoreEntity(); + var score = new PlayerScoreEntity(); score.setPlayer(player); score.setGame(game); score.setSequenceIndex(0); @@ -435,51 +515,67 @@ void given_playerReferencedInGameAndScore_when_forceDeletePlayer_then_returns204 score.setCreatedAt(OffsetDateTime.now()); score = playerScoreRepository.save(score); - UUID gameId = game.getId(); - UUID scoreId = score.getId(); + var gameId = game.getId(); + var scoreId = score.getId(); - // When & Then - mockMvc.perform(delete("/api/players/" + player.getId()) - .param("forceDeletion", "true")) - .andExpect(status().isNoContent()); + // When + var response = restTemplate.exchange( + "/api/players/" + player.getId() + "?forceDeletion=true", + HttpMethod.DELETE, + null, + Void.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT); // Verify player was deleted assertThat(playerRepository.findById(player.getId())).isEmpty(); // Verify references were nullified - GameEntity updatedGame = gameRepository.findById(gameId).orElseThrow(); + var updatedGame = gameRepository.findById(gameId).orElseThrow(); assertThat(updatedGame.getPlayer1()).isNull(); assertThat(updatedGame.getPlayer2()).isNull(); assertThat(updatedGame.getMainPlayer()).isNull(); - PlayerScoreEntity updatedScore = playerScoreRepository.findById(scoreId).orElseThrow(); + var updatedScore = playerScoreRepository.findById(scoreId).orElseThrow(); assertThat(updatedScore.getPlayer()).isNull(); } @Test - void given_nonExistentPlayer_when_deletePlayer_then_returns404() throws Exception { + void given_nonExistentPlayer_when_deletePlayer_then_returns404() { // Given - UUID nonExistentId = UUID.randomUUID(); - - // When & Then - mockMvc.perform(delete("/api/players/" + nonExistentId) - .param("forceDeletion", "false")) - .andExpect(status().isNotFound()) - .andExpect(jsonPath("$.error", is("not_found"))); + var nonExistentId = UUID.randomUUID(); + + // When + var response = restTemplate.exchange( + "/api/players/" + nonExistentId + "?forceDeletion=false", + HttpMethod.DELETE, + null, + ErrorResponseTO.class + ); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().error()).isEqualTo("not_found"); } @Test - void given_newPlayerWithoutScores_when_listPlayers_then_returnsPlayerWithZeroScore() throws Exception { + void given_newPlayerWithoutScores_when_listPlayers_then_returnsPlayerWithZeroScore() { // Given - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + var player = new PlayerEntity("Anna", "Schmidt"); playerRepository.save(player); - // When & Then - mockMvc.perform(get("/api/players")) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.items", hasSize(1))) - .andExpect(jsonPath("$.items[0].first_name", is("Anna"))) - .andExpect(jsonPath("$.items[0].current_total_points", is(0))) - .andExpect(jsonPath("$.items[0].current_sequence_index", is(0))); + // When + var response = restTemplate.getForEntity("/api/players", PlayerListResponseTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().items()).hasSize(1); + assertThat(response.getBody().items().get(0).first_name()).isEqualTo("Anna"); + assertThat(response.getBody().items().get(0).current_total_points()).isEqualTo(0); + assertThat(response.getBody().items().get(0).current_sequence_index()).isEqualTo(0); } } From 45051901942c28284044a5ab9268544af6796671 Mon Sep 17 00:00:00 2001 From: Joern Wellniak Date: Fri, 24 Oct 2025 09:30:38 +0200 Subject: [PATCH 10/15] Cleanup --- .../skat/backend/SkatBackendApplication.java | 6 +- .../backend/api/HelloWorldController.java | 16 +- .../api/controller/PlayersController.java | 93 +++--- .../api/exception/ConflictException.java | 22 +- .../api/exception/GlobalExceptionHandler.java | 122 ++++--- .../api/exception/NotFoundException.java | 22 +- .../backend/application/GreetingService.java | 43 ++- .../backend/application/PlayersService.java | 22 +- .../application/PlayersServiceImpl.java | 303 +++++++++--------- .../application/dto/ErrorResponseTO.java | 8 +- .../backend/application/dto/PagingTO.java | 8 +- .../application/dto/PlayerListResponseTO.java | 8 +- .../backend/application/dto/PlayerTO.java | 8 +- .../application/dto/PlayerWithScoreTO.java | 14 +- .../backend/application/dto/PlayersQuery.java | 10 +- .../backend/application/dto/PlayersSort.java | 4 +- .../application/dto/UpsertPlayerRequest.java | 12 +- .../backend/domain/entities/GameEntity.java | 67 ++-- .../backend/domain/entities/PlayerEntity.java | 52 +-- .../domain/entities/PlayerScoreEntity.java | 57 ++-- .../domain/repositories/GameRepository.java | 36 +-- .../domain/repositories/PlayerRepository.java | 18 +- .../repositories/PlayerScoreRepository.java | 39 ++- 23 files changed, 505 insertions(+), 485 deletions(-) diff --git a/src/main/java/com/skat/backend/SkatBackendApplication.java b/src/main/java/com/skat/backend/SkatBackendApplication.java index 6fb114e..b32b7e2 100644 --- a/src/main/java/com/skat/backend/SkatBackendApplication.java +++ b/src/main/java/com/skat/backend/SkatBackendApplication.java @@ -6,7 +6,7 @@ @SpringBootApplication public class SkatBackendApplication { - public static void main(String[] args) { - SpringApplication.run(SkatBackendApplication.class, args); - } + public static void main(String[] args) { + SpringApplication.run(SkatBackendApplication.class, args); + } } diff --git a/src/main/java/com/skat/backend/api/HelloWorldController.java b/src/main/java/com/skat/backend/api/HelloWorldController.java index 784f07c..ee13e7b 100644 --- a/src/main/java/com/skat/backend/api/HelloWorldController.java +++ b/src/main/java/com/skat/backend/api/HelloWorldController.java @@ -11,14 +11,14 @@ @CrossOrigin(origins = "*") public class HelloWorldController { - private final GreetingService greetingService; + private final GreetingService greetingService; - public HelloWorldController(GreetingService greetingService) { - this.greetingService = greetingService; - } + public HelloWorldController(GreetingService greetingService) { + this.greetingService = greetingService; + } - @GetMapping("/hello") - public String hello() { - return greetingService.generateGreeting(); - } + @GetMapping("/hello") + public String hello() { + return greetingService.generateGreeting(); + } } diff --git a/src/main/java/com/skat/backend/api/controller/PlayersController.java b/src/main/java/com/skat/backend/api/controller/PlayersController.java index 1738516..2578ab6 100644 --- a/src/main/java/com/skat/backend/api/controller/PlayersController.java +++ b/src/main/java/com/skat/backend/api/controller/PlayersController.java @@ -1,64 +1,71 @@ package com.skat.backend.api.controller; import com.skat.backend.application.PlayersService; -import com.skat.backend.application.dto.*; +import com.skat.backend.application.dto.PlayerListResponseTO; +import com.skat.backend.application.dto.PlayerTO; +import com.skat.backend.application.dto.PlayersQuery; +import com.skat.backend.application.dto.PlayersSort; +import com.skat.backend.application.dto.UpsertPlayerRequest; import jakarta.validation.Valid; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; +import java.util.UUID; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.DeleteMapping; +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.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; -import java.net.URI; -import java.util.UUID; - @RestController @RequestMapping("/api/players") @Validated public class PlayersController { - @Autowired - private PlayersService playersService; + @Autowired + private PlayersService playersService; - @GetMapping - public ResponseEntity listPlayers( - @RequestParam(name = "sort", required = false, defaultValue = "NAME") PlayersSort sort, - @RequestParam(name = "startIndex", required = false, defaultValue = "0") @Min(0) int startIndex, - @RequestParam(name = "pageSize", required = false, defaultValue = "50") @Min(1) @Max(200) int pageSize - ) { - PlayersQuery query = new PlayersQuery(startIndex, pageSize, sort); - PlayerListResponseTO response = playersService.listPlayers(query); - return ResponseEntity.ok(response); - } + @GetMapping + public ResponseEntity listPlayers( + @RequestParam(name = "sort", required = false, defaultValue = "NAME") PlayersSort sort, + @RequestParam(name = "startIndex", required = false, defaultValue = "0") @Min(0) int startIndex, + @RequestParam(name = "pageSize", required = false, defaultValue = "50") @Min(1) @Max(200) int pageSize) { + var query = new PlayersQuery(startIndex, pageSize, sort); + var response = playersService.listPlayers(query); + return ResponseEntity.ok(response); + } - @PostMapping - public ResponseEntity createPlayer(@Valid @RequestBody UpsertPlayerRequest request) { - PlayerTO player = playersService.createPlayer(request); - URI location = ServletUriComponentsBuilder - .fromCurrentRequest() - .path("/{id}") - .buildAndExpand(player.id()) - .toUri(); - return ResponseEntity.created(location).body(player); - } + @PostMapping + public ResponseEntity createPlayer(@Valid @RequestBody UpsertPlayerRequest request) { + var player = playersService.createPlayer(request); + var location = ServletUriComponentsBuilder + .fromCurrentRequest() + .path("/{id}") + .buildAndExpand(player.id()) + .toUri(); + return ResponseEntity.created(location).body(player); + } - @PutMapping("/{id}") - public ResponseEntity updatePlayer( - @PathVariable UUID id, - @Valid @RequestBody UpsertPlayerRequest request - ) { - PlayerTO player = playersService.updatePlayer(id, request); - return ResponseEntity.ok(player); - } + @PutMapping("/{id}") + public ResponseEntity updatePlayer( + @PathVariable UUID id, + @Valid @RequestBody UpsertPlayerRequest request) { + var player = playersService.updatePlayer(id, request); + return ResponseEntity.ok(player); + } - @DeleteMapping("/{id}") - public ResponseEntity deletePlayer( - @PathVariable UUID id, - @RequestParam(name = "forceDeletion", required = false, defaultValue = "false") boolean forceDeletion - ) { - playersService.deletePlayer(id, forceDeletion); - return ResponseEntity.noContent().build(); - } + @DeleteMapping("/{id}") + public ResponseEntity deletePlayer( + @PathVariable UUID id, + @RequestParam(name = "forceDeletion", required = false, defaultValue = "false") boolean forceDeletion) { + playersService.deletePlayer(id, forceDeletion); + return ResponseEntity.noContent().build(); + } } diff --git a/src/main/java/com/skat/backend/api/exception/ConflictException.java b/src/main/java/com/skat/backend/api/exception/ConflictException.java index e1c7208..85eaf91 100644 --- a/src/main/java/com/skat/backend/api/exception/ConflictException.java +++ b/src/main/java/com/skat/backend/api/exception/ConflictException.java @@ -1,18 +1,18 @@ package com.skat.backend.api.exception; public class ConflictException extends RuntimeException { - private final String field; + private final String field; - public ConflictException(String message) { - this(message, null); - } + public ConflictException(String message) { + this(message, null); + } - public ConflictException(String message, String field) { - super(message); - this.field = field; - } + public ConflictException(String message, String field) { + super(message); + this.field = field; + } - public String getField() { - return field; - } + public String getField() { + return field; + } } diff --git a/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java b/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java index 825a383..c638429 100644 --- a/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java +++ b/src/main/java/com/skat/backend/api/exception/GlobalExceptionHandler.java @@ -5,7 +5,6 @@ import jakarta.validation.ConstraintViolationException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; -import org.springframework.validation.FieldError; import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; @@ -14,72 +13,67 @@ @RestControllerAdvice public class GlobalExceptionHandler { - @ExceptionHandler(NotFoundException.class) - public ResponseEntity handleNotFoundException(NotFoundException ex) { - ErrorResponseTO error = new ErrorResponseTO( - "not_found", - ex.getMessage(), - ex.getField() - ); - return ResponseEntity.status(HttpStatus.NOT_FOUND).body(error); - } + @ExceptionHandler(NotFoundException.class) + public ResponseEntity handleNotFoundException(NotFoundException ex) { + var error = new ErrorResponseTO( + "not_found", + ex.getMessage(), + ex.getField()); + return ResponseEntity.status(HttpStatus.NOT_FOUND).body(error); + } - @ExceptionHandler(ConflictException.class) - public ResponseEntity handleConflictException(ConflictException ex) { - ErrorResponseTO error = new ErrorResponseTO( - "conflict", - ex.getMessage(), - ex.getField() - ); - return ResponseEntity.status(HttpStatus.CONFLICT).body(error); - } + @ExceptionHandler(ConflictException.class) + public ResponseEntity handleConflictException(ConflictException ex) { + var error = new ErrorResponseTO( + "conflict", + ex.getMessage(), + ex.getField()); + return ResponseEntity.status(HttpStatus.CONFLICT).body(error); + } - @ExceptionHandler(MethodArgumentNotValidException.class) - public ResponseEntity handleValidationException(MethodArgumentNotValidException ex) { - FieldError fieldError = ex.getBindingResult().getFieldError(); - String field = fieldError != null ? fieldError.getField() : null; - String message = fieldError != null ? fieldError.getDefaultMessage() : "Validation failed"; - - ErrorResponseTO error = new ErrorResponseTO( - "bad_request", - message, - field - ); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); - } + @ExceptionHandler(MethodArgumentNotValidException.class) + public ResponseEntity handleValidationException(MethodArgumentNotValidException ex) { + var fieldError = ex.getBindingResult().getFieldError(); + var field = fieldError != null ? fieldError.getField() : null; + var message = fieldError != null ? fieldError.getDefaultMessage() : "Validation failed"; - @ExceptionHandler(MethodArgumentTypeMismatchException.class) - public ResponseEntity handleTypeMismatchException(MethodArgumentTypeMismatchException ex) { - String message = String.format("Invalid value for parameter '%s'", ex.getName()); - ErrorResponseTO error = new ErrorResponseTO( - "bad_request", - message, - ex.getName() - ); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); - } + var error = new ErrorResponseTO( + "bad_request", + message, + field); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } - @ExceptionHandler(IllegalArgumentException.class) - public ResponseEntity handleIllegalArgumentException(IllegalArgumentException ex) { - ErrorResponseTO error = new ErrorResponseTO( - "bad_request", - ex.getMessage(), - null - ); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); - } + @ExceptionHandler(MethodArgumentTypeMismatchException.class) + public ResponseEntity handleTypeMismatchException(MethodArgumentTypeMismatchException ex) { + var message = String.format("Invalid value for parameter '%s'", ex.getName()); + var error = new ErrorResponseTO( + "bad_request", + message, + ex.getName()); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } - @ExceptionHandler(ConstraintViolationException.class) - public ResponseEntity handleConstraintViolationException(ConstraintViolationException ex) { - ConstraintViolation violation = ex.getConstraintViolations().iterator().next(); - String propertyPath = violation.getPropertyPath().toString(); - String field = propertyPath.contains(".") ? propertyPath.substring(propertyPath.lastIndexOf('.') + 1) : propertyPath; - - ErrorResponseTO error = new ErrorResponseTO( - "bad_request", - violation.getMessage(), - field - ); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); - } + @ExceptionHandler(IllegalArgumentException.class) + public ResponseEntity handleIllegalArgumentException(IllegalArgumentException ex) { + var error = new ErrorResponseTO( + "bad_request", + ex.getMessage(), + null); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } + + @ExceptionHandler(ConstraintViolationException.class) + public ResponseEntity handleConstraintViolationException(ConstraintViolationException ex) { + ConstraintViolation violation = ex.getConstraintViolations().iterator().next(); + var propertyPath = violation.getPropertyPath().toString(); + var field = propertyPath.contains(".") ? propertyPath.substring(propertyPath.lastIndexOf('.') + 1) + : propertyPath; + + var error = new ErrorResponseTO( + "bad_request", + violation.getMessage(), + field); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error); + } } diff --git a/src/main/java/com/skat/backend/api/exception/NotFoundException.java b/src/main/java/com/skat/backend/api/exception/NotFoundException.java index 26648fc..9fc47a6 100644 --- a/src/main/java/com/skat/backend/api/exception/NotFoundException.java +++ b/src/main/java/com/skat/backend/api/exception/NotFoundException.java @@ -1,18 +1,18 @@ package com.skat.backend.api.exception; public class NotFoundException extends RuntimeException { - private final String field; + private final String field; - public NotFoundException(String message) { - this(message, null); - } + public NotFoundException(String message) { + this(message, null); + } - public NotFoundException(String message, String field) { - super(message); - this.field = field; - } + public NotFoundException(String message, String field) { + super(message); + this.field = field; + } - public String getField() { - return field; - } + public String getField() { + return field; + } } diff --git a/src/main/java/com/skat/backend/application/GreetingService.java b/src/main/java/com/skat/backend/application/GreetingService.java index 7c71e3b..1ba6c86 100644 --- a/src/main/java/com/skat/backend/application/GreetingService.java +++ b/src/main/java/com/skat/backend/application/GreetingService.java @@ -3,31 +3,30 @@ import org.springframework.stereotype.Service; /** - * Simple greeting service for demonstration purposes. - * This is a pure business logic class that can be unit tested. + * Simple greeting service for demonstration purposes. This is a pure business logic class that can be unit tested. */ @Service public class GreetingService { - /** - * Generates a greeting message. - * - * @return the greeting message - */ - public String generateGreeting() { - return "Hallo"; - } + /** + * Generates a greeting message. + * + * @return the greeting message + */ + public String generateGreeting() { + return "Hallo"; + } - /** - * Formats a personalized greeting. - * - * @param name the name to include in the greeting - * @return personalized greeting message - */ - public String generatePersonalizedGreeting(String name) { - if (name == null || name.trim().isEmpty()) { - return generateGreeting(); - } - return "Hallo " + name; - } + /** + * Formats a personalized greeting. + * + * @param name the name to include in the greeting + * @return personalized greeting message + */ + public String generatePersonalizedGreeting(String name) { + if (name == null || name.trim().isEmpty()) { + return generateGreeting(); + } + return "Hallo " + name; + } } diff --git a/src/main/java/com/skat/backend/application/PlayersService.java b/src/main/java/com/skat/backend/application/PlayersService.java index 7f561e7..9f87015 100644 --- a/src/main/java/com/skat/backend/application/PlayersService.java +++ b/src/main/java/com/skat/backend/application/PlayersService.java @@ -1,16 +1,18 @@ package com.skat.backend.application; -import com.skat.backend.application.dto.*; - +import com.skat.backend.application.dto.PlayerListResponseTO; +import com.skat.backend.application.dto.PlayerTO; +import com.skat.backend.application.dto.PlayersQuery; +import com.skat.backend.application.dto.UpsertPlayerRequest; import java.util.UUID; public interface PlayersService { - - PlayerListResponseTO listPlayers(PlayersQuery query); - - PlayerTO createPlayer(UpsertPlayerRequest request); - - PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request); - - void deletePlayer(UUID id, boolean forceDeletion); + + PlayerListResponseTO listPlayers(PlayersQuery query); + + PlayerTO createPlayer(UpsertPlayerRequest request); + + PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request); + + void deletePlayer(UUID id, boolean forceDeletion); } diff --git a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java index a8e9823..badf5e2 100644 --- a/src/main/java/com/skat/backend/application/PlayersServiceImpl.java +++ b/src/main/java/com/skat/backend/application/PlayersServiceImpl.java @@ -2,166 +2,171 @@ import com.skat.backend.api.exception.ConflictException; import com.skat.backend.api.exception.NotFoundException; -import com.skat.backend.application.dto.*; +import com.skat.backend.application.dto.PagingTO; +import com.skat.backend.application.dto.PlayerListResponseTO; +import com.skat.backend.application.dto.PlayerTO; +import com.skat.backend.application.dto.PlayerWithScoreTO; +import com.skat.backend.application.dto.PlayersQuery; +import com.skat.backend.application.dto.PlayersSort; +import com.skat.backend.application.dto.UpsertPlayerRequest; import com.skat.backend.domain.entities.PlayerEntity; import com.skat.backend.domain.entities.PlayerScoreEntity; import com.skat.backend.domain.repositories.GameRepository; import com.skat.backend.domain.repositories.PlayerRepository; import com.skat.backend.domain.repositories.PlayerScoreRepository; +import java.time.OffsetDateTime; +import java.util.HashMap; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.PageRequest; -import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.time.OffsetDateTime; -import java.util.*; -import java.util.stream.Collectors; - @Service public class PlayersServiceImpl implements PlayersService { - @Autowired - private PlayerRepository playerRepository; - - @Autowired - private GameRepository gameRepository; - - @Autowired - private PlayerScoreRepository playerScoreRepository; - - @Override - @Transactional(readOnly = true) - public PlayerListResponseTO listPlayers(PlayersQuery query) { - var pageable = PageRequest.of(query.startIndex() / query.pageSize(), query.pageSize()); - - // Fetch players based on sort - List players; - if (query.sort() == PlayersSort.NAME) { - players = playerRepository.findAllOrderedByName(pageable); - } else { - players = playerRepository.findAllPlayers(pageable); - } - - // Extract player IDs - var playerIds = players.stream() - .map(PlayerEntity::getId) - .collect(Collectors.toList()); - - // Fetch latest scores for these players - var latestScores = new HashMap(); - if (!playerIds.isEmpty()) { - var scores = playerScoreRepository.findLatestScoresForPlayers(playerIds); - for (var score : scores) { - if (score.getPlayer() != null) { - latestScores.put(score.getPlayer().getId(), score); - } - } - } - - // Map to DTOs - var items = players.stream() - .map(player -> { - var score = latestScores.get(player.getId()); - var totalPoints = score != null ? score.getTotalPoints() : 0; - var sequenceIndex = score != null ? score.getSequenceIndex() : 0; - var updatedAt = score != null ? score.getCreatedAt() : OffsetDateTime.now(); - - return new PlayerWithScoreTO( - player.getId(), - player.getFirstName(), - player.getLastName(), - totalPoints, - sequenceIndex, - updatedAt - ); - }) - .collect(Collectors.toList()); - - // Sort by score if needed - if (query.sort() == PlayersSort.SCORE_DESC) { - items.sort((a, b) -> { - var scoreCompare = Integer.compare(b.current_total_points(), a.current_total_points()); - if (scoreCompare != 0) return scoreCompare; - var lastNameCompare = a.last_name().compareTo(b.last_name()); - if (lastNameCompare != 0) return lastNameCompare; - return a.first_name().compareTo(b.first_name()); - }); - } - - var total = playerRepository.count(); - var paging = new PagingTO(query.startIndex(), query.pageSize(), total); - - return new PlayerListResponseTO(items, paging, query.sort()); - } - - @Override - @Transactional - public PlayerTO createPlayer(UpsertPlayerRequest request) { - var firstName = request.first_name().trim(); - var lastName = request.last_name().trim(); - - if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName)) { - throw new ConflictException( - "Player with first_name+last_name already exists", - "first_name,last_name" - ); - } - - var player = new PlayerEntity(firstName, lastName); - player = playerRepository.save(player); - - return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); - } - - @Override - @Transactional - public PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request) { - var player = playerRepository.findById(id) - .orElseThrow(() -> new NotFoundException("Player not found", "id")); - - var firstName = request.first_name().trim(); - var lastName = request.last_name().trim(); - - // Check if the new name conflicts with another player - if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(firstName, lastName, id)) { - throw new ConflictException( - "Player with first_name+last_name already exists", - "first_name,last_name" - ); - } - - player.setFirstName(firstName); - player.setLastName(lastName); - player = playerRepository.save(player); - - return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); - } - - @Override - @Transactional - public void deletePlayer(UUID id, boolean forceDeletion) { - var player = playerRepository.findById(id) - .orElseThrow(() -> new NotFoundException("Player not found", "id")); - - if (!forceDeletion) { - var hasGames = gameRepository.existsByPlayerId(id); - var hasScores = playerScoreRepository.existsByPlayerId(id); - - if (hasGames || hasScores) { - throw new ConflictException("Player is referenced in games or scores"); - } - - playerRepository.delete(player); - } else { - // Force deletion: nullify references first - gameRepository.nullifyPlayer1References(id); - gameRepository.nullifyPlayer2References(id); - gameRepository.nullifyPlayer3References(id); - gameRepository.nullifyMainPlayerReferences(id); - playerScoreRepository.nullifyPlayerReferences(id); - - playerRepository.delete(player); - } - } + @Autowired + private PlayerRepository playerRepository; + + @Autowired + private GameRepository gameRepository; + + @Autowired + private PlayerScoreRepository playerScoreRepository; + + @Override + @Transactional(readOnly = true) + public PlayerListResponseTO listPlayers(PlayersQuery query) { + var pageable = PageRequest.of(query.startIndex() / query.pageSize(), query.pageSize()); + + // Fetch players based on sort + List players; + if (query.sort() == PlayersSort.NAME) { + players = playerRepository.findAllOrderedByName(pageable); + } else { + players = playerRepository.findAllPlayers(pageable); + } + + // Extract player IDs + var playerIds = players.stream() + .map(PlayerEntity::getId) + .collect(Collectors.toList()); + + // Fetch latest scores for these players + var latestScores = new HashMap(); + if (!playerIds.isEmpty()) { + var scores = playerScoreRepository.findLatestScoresForPlayers(playerIds); + for (var score : scores) { + if (score.getPlayer() != null) { + latestScores.put(score.getPlayer().getId(), score); + } + } + } + + // Map to DTOs + var items = players.stream() + .map(player -> { + var score = latestScores.get(player.getId()); + var totalPoints = score != null ? score.getTotalPoints() : 0; + var sequenceIndex = score != null ? score.getSequenceIndex() : 0; + var updatedAt = score != null ? score.getCreatedAt() : OffsetDateTime.now(); + + return new PlayerWithScoreTO( + player.getId(), + player.getFirstName(), + player.getLastName(), + totalPoints, + sequenceIndex, + updatedAt); + }) + .collect(Collectors.toList()); + + // Sort by score if needed + if (query.sort() == PlayersSort.SCORE_DESC) { + items.sort((a, b) -> { + var scoreCompare = Integer.compare(b.current_total_points(), a.current_total_points()); + if (scoreCompare != 0) + return scoreCompare; + var lastNameCompare = a.last_name().compareTo(b.last_name()); + if (lastNameCompare != 0) + return lastNameCompare; + return a.first_name().compareTo(b.first_name()); + }); + } + + var total = playerRepository.count(); + var paging = new PagingTO(query.startIndex(), query.pageSize(), total); + + return new PlayerListResponseTO(items, paging, query.sort()); + } + + @Override + @Transactional + public PlayerTO createPlayer(UpsertPlayerRequest request) { + var firstName = request.first_name().trim(); + var lastName = request.last_name().trim(); + + if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName)) { + throw new ConflictException( + "Player with first_name+last_name already exists", + "first_name,last_name"); + } + + var player = new PlayerEntity(firstName, lastName); + player = playerRepository.save(player); + + return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); + } + + @Override + @Transactional + public PlayerTO updatePlayer(UUID id, UpsertPlayerRequest request) { + var player = playerRepository.findById(id) + .orElseThrow(() -> new NotFoundException("Player not found", "id")); + + var firstName = request.first_name().trim(); + var lastName = request.last_name().trim(); + + // Check if the new name conflicts with another player + if (playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(firstName, lastName, id)) { + throw new ConflictException( + "Player with first_name+last_name already exists", + "first_name,last_name"); + } + + player.setFirstName(firstName); + player.setLastName(lastName); + player = playerRepository.save(player); + + return new PlayerTO(player.getId(), player.getFirstName(), player.getLastName()); + } + + @Override + @Transactional + public void deletePlayer(UUID id, boolean forceDeletion) { + var player = playerRepository.findById(id) + .orElseThrow(() -> new NotFoundException("Player not found", "id")); + + if (!forceDeletion) { + var hasGames = gameRepository.existsByPlayerId(id); + var hasScores = playerScoreRepository.existsByPlayerId(id); + + if (hasGames || hasScores) { + throw new ConflictException("Player is referenced in games or scores"); + } + + playerRepository.delete(player); + } else { + // Force deletion: nullify references first + gameRepository.nullifyPlayer1References(id); + gameRepository.nullifyPlayer2References(id); + gameRepository.nullifyPlayer3References(id); + gameRepository.nullifyMainPlayerReferences(id); + playerScoreRepository.nullifyPlayerReferences(id); + + playerRepository.delete(player); + } + } } diff --git a/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java b/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java index 341291d..62909c3 100644 --- a/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java +++ b/src/main/java/com/skat/backend/application/dto/ErrorResponseTO.java @@ -1,7 +1,7 @@ package com.skat.backend.application.dto; public record ErrorResponseTO( - String error, - String message, - String field -) {} + String error, + String message, + String field) { +} diff --git a/src/main/java/com/skat/backend/application/dto/PagingTO.java b/src/main/java/com/skat/backend/application/dto/PagingTO.java index 3bec040..515dba0 100644 --- a/src/main/java/com/skat/backend/application/dto/PagingTO.java +++ b/src/main/java/com/skat/backend/application/dto/PagingTO.java @@ -1,7 +1,7 @@ package com.skat.backend.application.dto; public record PagingTO( - int startIndex, - int pageSize, - long total -) {} + int startIndex, + int pageSize, + long total) { +} diff --git a/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java b/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java index 0c86d70..a5679c2 100644 --- a/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java +++ b/src/main/java/com/skat/backend/application/dto/PlayerListResponseTO.java @@ -3,7 +3,7 @@ import java.util.List; public record PlayerListResponseTO( - List items, - PagingTO paging, - PlayersSort sort -) {} + List items, + PagingTO paging, + PlayersSort sort) { +} diff --git a/src/main/java/com/skat/backend/application/dto/PlayerTO.java b/src/main/java/com/skat/backend/application/dto/PlayerTO.java index 37b3566..4280b1e 100644 --- a/src/main/java/com/skat/backend/application/dto/PlayerTO.java +++ b/src/main/java/com/skat/backend/application/dto/PlayerTO.java @@ -3,7 +3,7 @@ import java.util.UUID; public record PlayerTO( - UUID id, - String first_name, - String last_name -) {} + UUID id, + String first_name, + String last_name) { +} diff --git a/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java b/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java index d8752f4..e086ac1 100644 --- a/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java +++ b/src/main/java/com/skat/backend/application/dto/PlayerWithScoreTO.java @@ -4,10 +4,10 @@ import java.util.UUID; public record PlayerWithScoreTO( - UUID id, - String first_name, - String last_name, - int current_total_points, - int current_sequence_index, - OffsetDateTime updated_at -) {} + UUID id, + String first_name, + String last_name, + int current_total_points, + int current_sequence_index, + OffsetDateTime updated_at) { +} diff --git a/src/main/java/com/skat/backend/application/dto/PlayersQuery.java b/src/main/java/com/skat/backend/application/dto/PlayersQuery.java index d51acf0..9eaee09 100644 --- a/src/main/java/com/skat/backend/application/dto/PlayersQuery.java +++ b/src/main/java/com/skat/backend/application/dto/PlayersQuery.java @@ -1,9 +1,7 @@ package com.skat.backend.application.dto; -import java.util.UUID; - public record PlayersQuery( - int startIndex, - int pageSize, - PlayersSort sort -) {} + int startIndex, + int pageSize, + PlayersSort sort) { +} diff --git a/src/main/java/com/skat/backend/application/dto/PlayersSort.java b/src/main/java/com/skat/backend/application/dto/PlayersSort.java index 122978a..7ca4393 100644 --- a/src/main/java/com/skat/backend/application/dto/PlayersSort.java +++ b/src/main/java/com/skat/backend/application/dto/PlayersSort.java @@ -1,6 +1,6 @@ package com.skat.backend.application.dto; public enum PlayersSort { - NAME, - SCORE_DESC + NAME, + SCORE_DESC } diff --git a/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java b/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java index 311bf67..83ad66b 100644 --- a/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java +++ b/src/main/java/com/skat/backend/application/dto/UpsertPlayerRequest.java @@ -4,11 +4,9 @@ import jakarta.validation.constraints.Size; public record UpsertPlayerRequest( - @NotBlank(message = "first_name is required") - @Size(max = 50, message = "first_name must not exceed 50 characters") - String first_name, + @NotBlank(message = "first_name is required") + @Size(max = 50, message = "first_name must not exceed 50 characters") String first_name, - @NotBlank(message = "last_name is required") - @Size(max = 50, message = "last_name must not exceed 50 characters") - String last_name -) {} + @NotBlank(message = "last_name is required") + @Size(max = 50, message = "last_name must not exceed 50 characters") String last_name) { +} diff --git a/src/main/java/com/skat/backend/domain/entities/GameEntity.java b/src/main/java/com/skat/backend/domain/entities/GameEntity.java index 1683df8..a73262c 100644 --- a/src/main/java/com/skat/backend/domain/entities/GameEntity.java +++ b/src/main/java/com/skat/backend/domain/entities/GameEntity.java @@ -1,49 +1,56 @@ package com.skat.backend.domain.entities; -import jakarta.persistence.*; -import lombok.Data; -import lombok.NoArgsConstructor; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; import java.time.OffsetDateTime; import java.util.UUID; +import lombok.Data; +import lombok.NoArgsConstructor; @Entity @Table( - name = "game", - indexes = { - @Index(name = "game_main_player_IDX", columnList = "main_player_id"), - @Index(name = "game_played_at_IDX", columnList = "played_at") - } -) + name = "game", + indexes = { + @Index(name = "game_main_player_IDX", columnList = "main_player_id"), + @Index(name = "game_played_at_IDX", columnList = "played_at") + }) @Data @NoArgsConstructor public class GameEntity { - @Id - @GeneratedValue(strategy = GenerationType.UUID) - private UUID id; + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; - @ManyToOne - @JoinColumn(name = "player1_id") - private PlayerEntity player1; + @ManyToOne + @JoinColumn(name = "player1_id") + private PlayerEntity player1; - @ManyToOne - @JoinColumn(name = "player2_id") - private PlayerEntity player2; + @ManyToOne + @JoinColumn(name = "player2_id") + private PlayerEntity player2; - @ManyToOne - @JoinColumn(name = "player3_id") - private PlayerEntity player3; + @ManyToOne + @JoinColumn(name = "player3_id") + private PlayerEntity player3; - @ManyToOne - @JoinColumn(name = "main_player_id") - private PlayerEntity mainPlayer; + @ManyToOne + @JoinColumn(name = "main_player_id") + private PlayerEntity mainPlayer; - @Column(name = "bid_value") - private Integer bidValue; + @Column(name = "bid_value") + private Integer bidValue; - @Column(name = "score") - private Integer score; + @Column(name = "score") + private Integer score; - @Column(name = "played_at", nullable = false) - private OffsetDateTime playedAt; + @Column(name = "played_at", nullable = false) + private OffsetDateTime playedAt; } diff --git a/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java b/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java index c8a8487..0da876a 100644 --- a/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java +++ b/src/main/java/com/skat/backend/domain/entities/PlayerEntity.java @@ -1,39 +1,45 @@ package com.skat.backend.domain.entities; -import jakarta.persistence.*; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.Table; +import jakarta.persistence.UniqueConstraint; +import java.util.UUID; +import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; -import lombok.AllArgsConstructor; -import java.util.UUID; @Entity @Table( - name = "player", - uniqueConstraints = { - @UniqueConstraint(name = "player_first_last_name_UQ", columnNames = {"first_name", "last_name"}) - }, - indexes = { - @Index(name = "player_first_name_IDX", columnList = "first_name"), - @Index(name = "player_last_name_IDX", columnList = "last_name") - } -) + name = "player", + uniqueConstraints = { + @UniqueConstraint(name = "player_first_last_name_UQ", columnNames = { "first_name", "last_name" }) + }, + indexes = { + @Index(name = "player_first_name_IDX", columnList = "first_name"), + @Index(name = "player_last_name_IDX", columnList = "last_name") + }) @Data @NoArgsConstructor @AllArgsConstructor public class PlayerEntity { - @Id - @GeneratedValue(strategy = GenerationType.UUID) - private UUID id; + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; - @Column(name = "first_name", nullable = false, length = 50) - private String firstName; + @Column(name = "first_name", nullable = false, length = 50) + private String firstName; - @Column(name = "last_name", nullable = false, length = 50) - private String lastName; + @Column(name = "last_name", nullable = false, length = 50) + private String lastName; - public PlayerEntity(String firstName, String lastName) { - this.firstName = firstName; - this.lastName = lastName; - } + public PlayerEntity(String firstName, String lastName) { + this.firstName = firstName; + this.lastName = lastName; + } } diff --git a/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java b/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java index e43cbd1..3cc76a0 100644 --- a/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java +++ b/src/main/java/com/skat/backend/domain/entities/PlayerScoreEntity.java @@ -1,42 +1,49 @@ package com.skat.backend.domain.entities; -import jakarta.persistence.*; -import lombok.Data; -import lombok.NoArgsConstructor; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; import java.time.OffsetDateTime; import java.util.UUID; +import lombok.Data; +import lombok.NoArgsConstructor; @Entity @Table( - name = "player_score", - indexes = { - @Index(name = "player_score_player_IDX", columnList = "player_id"), - @Index(name = "player_score_game_IDX", columnList = "game_id"), - @Index(name = "player_score_sequence_IDX", columnList = "sequence_index") - } -) + name = "player_score", + indexes = { + @Index(name = "player_score_player_IDX", columnList = "player_id"), + @Index(name = "player_score_game_IDX", columnList = "game_id"), + @Index(name = "player_score_sequence_IDX", columnList = "sequence_index") + }) @Data @NoArgsConstructor public class PlayerScoreEntity { - @Id - @GeneratedValue(strategy = GenerationType.UUID) - private UUID id; + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; - @ManyToOne - @JoinColumn(name = "player_id") - private PlayerEntity player; + @ManyToOne + @JoinColumn(name = "player_id") + private PlayerEntity player; - @ManyToOne - @JoinColumn(name = "game_id", nullable = false) - private GameEntity game; + @ManyToOne + @JoinColumn(name = "game_id", nullable = false) + private GameEntity game; - @Column(name = "sequence_index", nullable = false) - private Integer sequenceIndex; + @Column(name = "sequence_index", nullable = false) + private Integer sequenceIndex; - @Column(name = "total_points") - private Integer totalPoints; + @Column(name = "total_points") + private Integer totalPoints; - @Column(name = "created_at", nullable = false) - private OffsetDateTime createdAt; + @Column(name = "created_at", nullable = false) + private OffsetDateTime createdAt; } diff --git a/src/main/java/com/skat/backend/domain/repositories/GameRepository.java b/src/main/java/com/skat/backend/domain/repositories/GameRepository.java index 6d97edd..4030475 100644 --- a/src/main/java/com/skat/backend/domain/repositories/GameRepository.java +++ b/src/main/java/com/skat/backend/domain/repositories/GameRepository.java @@ -1,35 +1,35 @@ package com.skat.backend.domain.repositories; import com.skat.backend.domain.entities.GameEntity; +import java.util.UUID; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; -import java.util.UUID; - @Repository public interface GameRepository extends JpaRepository { - @Query("SELECT CASE WHEN COUNT(g) > 0 THEN true ELSE false END FROM GameEntity g " + - "WHERE g.player1.id = :playerId OR g.player2.id = :playerId " + - "OR g.player3.id = :playerId OR g.mainPlayer.id = :playerId") - boolean existsByPlayerId(@Param("playerId") UUID playerId); + @Query(""" + SELECT CASE WHEN COUNT(g) > 0 THEN true ELSE false END FROM GameEntity g \ + WHERE g.player1.id = :playerId OR g.player2.id = :playerId \ + OR g.player3.id = :playerId OR g.mainPlayer.id = :playerId""") + boolean existsByPlayerId(@Param("playerId") UUID playerId); - @Modifying - @Query("UPDATE GameEntity g SET g.player1 = NULL WHERE g.player1.id = :playerId") - void nullifyPlayer1References(@Param("playerId") UUID playerId); + @Modifying + @Query("UPDATE GameEntity g SET g.player1 = NULL WHERE g.player1.id = :playerId") + void nullifyPlayer1References(@Param("playerId") UUID playerId); - @Modifying - @Query("UPDATE GameEntity g SET g.player2 = NULL WHERE g.player2.id = :playerId") - void nullifyPlayer2References(@Param("playerId") UUID playerId); + @Modifying + @Query("UPDATE GameEntity g SET g.player2 = NULL WHERE g.player2.id = :playerId") + void nullifyPlayer2References(@Param("playerId") UUID playerId); - @Modifying - @Query("UPDATE GameEntity g SET g.player3 = NULL WHERE g.player3.id = :playerId") - void nullifyPlayer3References(@Param("playerId") UUID playerId); + @Modifying + @Query("UPDATE GameEntity g SET g.player3 = NULL WHERE g.player3.id = :playerId") + void nullifyPlayer3References(@Param("playerId") UUID playerId); - @Modifying - @Query("UPDATE GameEntity g SET g.mainPlayer = NULL WHERE g.mainPlayer.id = :playerId") - void nullifyMainPlayerReferences(@Param("playerId") UUID playerId); + @Modifying + @Query("UPDATE GameEntity g SET g.mainPlayer = NULL WHERE g.mainPlayer.id = :playerId") + void nullifyMainPlayerReferences(@Param("playerId") UUID playerId); } diff --git a/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java b/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java index ff510e7..674b60b 100644 --- a/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java +++ b/src/main/java/com/skat/backend/domain/repositories/PlayerRepository.java @@ -1,25 +1,23 @@ package com.skat.backend.domain.repositories; import com.skat.backend.domain.entities.PlayerEntity; +import java.util.List; +import java.util.UUID; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; -import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; -import java.util.List; -import java.util.UUID; - @Repository public interface PlayerRepository extends JpaRepository { - boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(String firstName, String lastName); + boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCase(String firstName, String lastName); - boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(String firstName, String lastName, UUID id); + boolean existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot(String firstName, String lastName, UUID id); - @Query("SELECT p FROM PlayerEntity p ORDER BY p.lastName ASC, p.firstName ASC") - List findAllOrderedByName(Pageable pageable); + @Query("SELECT p FROM PlayerEntity p ORDER BY p.lastName ASC, p.firstName ASC") + List findAllOrderedByName(Pageable pageable); - @Query("SELECT p FROM PlayerEntity p") - List findAllPlayers(Pageable pageable); + @Query("SELECT p FROM PlayerEntity p") + List findAllPlayers(Pageable pageable); } diff --git a/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java b/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java index 3a247b7..de81e18 100644 --- a/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java +++ b/src/main/java/com/skat/backend/domain/repositories/PlayerScoreRepository.java @@ -1,35 +1,34 @@ package com.skat.backend.domain.repositories; import com.skat.backend.domain.entities.PlayerScoreEntity; +import java.util.List; +import java.util.UUID; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; -import java.util.List; -import java.util.UUID; - @Repository public interface PlayerScoreRepository extends JpaRepository { - @Query("SELECT CASE WHEN COUNT(ps) > 0 THEN true ELSE false END FROM PlayerScoreEntity ps " + - "WHERE ps.player.id = :playerId") - boolean existsByPlayerId(@Param("playerId") UUID playerId); + @Query("SELECT CASE WHEN COUNT(ps) > 0 THEN true ELSE false END FROM PlayerScoreEntity ps " + + "WHERE ps.player.id = :playerId") + boolean existsByPlayerId(@Param("playerId") UUID playerId); - @Modifying - @Query("UPDATE PlayerScoreEntity ps SET ps.player = NULL WHERE ps.player.id = :playerId") - void nullifyPlayerReferences(@Param("playerId") UUID playerId); + @Modifying + @Query("UPDATE PlayerScoreEntity ps SET ps.player = NULL WHERE ps.player.id = :playerId") + void nullifyPlayerReferences(@Param("playerId") UUID playerId); - @Query(value = """ - SELECT ps.* - FROM player_score ps - INNER JOIN ( - SELECT player_id, MAX(sequence_index) as max_seq - FROM player_score - WHERE player_id IN :playerIds - GROUP BY player_id - ) latest ON ps.player_id = latest.player_id AND ps.sequence_index = latest.max_seq - """, nativeQuery = true) - List findLatestScoresForPlayers(@Param("playerIds") List playerIds); + @Query(value = """ + SELECT ps.* + FROM player_score ps + INNER JOIN ( + SELECT player_id, MAX(sequence_index) as max_seq + FROM player_score + WHERE player_id IN :playerIds + GROUP BY player_id + ) latest ON ps.player_id = latest.player_id AND ps.sequence_index = latest.max_seq + """, nativeQuery = true) + List findLatestScoresForPlayers(@Param("playerIds") List playerIds); } From 66b1e59493c679d91db3067f78197ec1d3066c06 Mon Sep 17 00:00:00 2001 From: Joern Wellniak Date: Fri, 24 Oct 2025 09:36:12 +0200 Subject: [PATCH 11/15] ADR --- .../decisions/ADR-009-coding-rules.md | 54 ++++++++++++++ .../architecture/decisions/ADR-010-openapi.md | 73 +++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 docs/architecture/decisions/ADR-009-coding-rules.md create mode 100644 docs/architecture/decisions/ADR-010-openapi.md diff --git a/docs/architecture/decisions/ADR-009-coding-rules.md b/docs/architecture/decisions/ADR-009-coding-rules.md new file mode 100644 index 0000000..852ee0a --- /dev/null +++ b/docs/architecture/decisions/ADR-009-coding-rules.md @@ -0,0 +1,54 @@ +# ADR-009: Use Google Coding Rules with Exception for Indentation + +Date: 2025-10-23 + +## Status + +Accepted + +## Context + +The project aims to maintain a consistent and widely recognized coding style to improve code +readability and collaboration. Google Coding Rules provide a comprehensive and well-documented +standard for Java development. However, the team prefers using tabs instead of spaces for +indentation to allow developers to customize their viewing preferences in their IDEs. + +## Decision + +The project will adopt Google Coding Rules as the standard coding style, with the exception of using +tabs (`\t`) instead of spaces for indentation. + +## Consequences + +- **Positive**: + - Ensures a consistent and widely recognized coding style. + - Allows developers to adjust tab width in their IDEs for personal preference. + - Simplifies onboarding for developers familiar with Google Coding Rules. + +- **Negative**: + - Requires configuring tools and IDEs to enforce the use of tabs. + - May require additional effort to ensure compliance with the exception. + +## Implementation + +1. Configure the project to use Google Coding Rules: + - Add the `google-java-format` plugin to the build system (e.g., Maven): + ```xml + + com.github.sherter.google-java-format + google-java-format-maven-plugin + 1.15.0 + + ``` + +2. Modify the formatter configuration to use tabs for indentation: + - Use the `--aosp` flag or customize the formatter to replace spaces with tabs. + +3. Update IDE settings to enforce tabs for indentation: + - For IntelliJ IDEA: + - Go to `Preferences > Code Style > Java`. + - Set "Use tab character" for indentation. + +4. Document this decision in the project's coding standards to ensure team-wide adherence. + +5. Add a pre-commit hook or CI check to validate compliance with the coding rules. diff --git a/docs/architecture/decisions/ADR-010-openapi.md b/docs/architecture/decisions/ADR-010-openapi.md new file mode 100644 index 0000000..599e5e6 --- /dev/null +++ b/docs/architecture/decisions/ADR-010-openapi.md @@ -0,0 +1,73 @@ +# ADR-010: Use OpenAPI Swagger Annotations for All REST API Endpoints + +Date: 2025-10-24 + +## Status + +Accepted + +## Context + +To ensure that the REST API is well-documented and easily understandable for developers and external +consumers, it is essential to provide clear and standardized documentation. OpenAPI (Swagger) +annotations allow for the automatic generation of API documentation, which can be visualized using +tools like Swagger UI. Currently, not all endpoints are annotated, and OpenAPI is not fully enabled +in the Spring Boot application. + +## Decision + +All REST API endpoints in the project will be annotated with OpenAPI Swagger annotations. +Additionally, OpenAPI will be enabled in the Spring Boot application to generate and serve the API +documentation. + +## Consequences + +- **Positive**: + - Provides a standardized and comprehensive API documentation. + - Simplifies integration for external consumers by offering a clear contract. + - Reduces manual effort in maintaining API documentation. + +- **Negative**: + - Requires additional effort to annotate all existing endpoints. + - Developers need to be familiar with OpenAPI annotations. + +## Implementation + +1. Add the `springdoc-openapi` dependency to the `pom.xml`: + ```xml + + org.springdoc + springdoc-openapi-starter-webmvc-ui + 2.1.0 + + ``` + +2. Annotate all REST API endpoints with OpenAPI annotations: + ```java + @RestController + @RequestMapping("/example") + @Tag(name = "Example", description = "Example API") + public class ExampleController { + + @Operation(summary = "Get example data", description = "Fetches example data by ID") + @ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Successful operation"), + @ApiResponse(responseCode = "404", description = "Example not found") + }) + @GetMapping("/{id}") + public ResponseEntity getExample(@PathVariable Long id) { + // Implementation here + return ResponseEntity.ok(new ExampleDto()); + } + } + ``` + +3. Enable OpenAPI in the Spring Boot application: + - No additional configuration is required as `springdoc-openapi` automatically enables Swagger + UI at `/swagger-ui.html`. + +4. Document this decision in the project's development guidelines to ensure all new endpoints are + annotated. + +5. Verify the generated documentation by accessing the Swagger UI at + `http://localhost:8080/swagger-ui.html`. From 393fd386c37944295be1b88230c606c47ab2ccae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 07:42:22 +0000 Subject: [PATCH 12/15] Add OpenAPI/Swagger annotations as per ADR-010 - Add springdoc-openapi-starter-webmvc-ui dependency (version 2.1.0) - Annotate PlayersController with @Tag for API grouping - Add @Operation annotations with summaries and descriptions for all endpoints - Add @ApiResponses documenting all response codes (200, 201, 204, 400, 404, 409) - Add @Parameter annotations with descriptions and examples - Swagger UI automatically available at /swagger-ui.html - OpenAPI spec available at /v3/api-docs - All 35 tests still passing Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- pom.xml | 6 +++ .../api/controller/PlayersController.java | 46 ++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 5488a65..901cadf 100644 --- a/pom.xml +++ b/pom.xml @@ -55,6 +55,12 @@ provided + + org.springdoc + springdoc-openapi-starter-webmvc-ui + 2.1.0 + + org.postgresql postgresql diff --git a/src/main/java/com/skat/backend/api/controller/PlayersController.java b/src/main/java/com/skat/backend/api/controller/PlayersController.java index 2578ab6..7f335dc 100644 --- a/src/main/java/com/skat/backend/api/controller/PlayersController.java +++ b/src/main/java/com/skat/backend/api/controller/PlayersController.java @@ -6,6 +6,13 @@ import com.skat.backend.application.dto.PlayersQuery; import com.skat.backend.application.dto.PlayersSort; import com.skat.backend.application.dto.UpsertPlayerRequest; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; import jakarta.validation.Valid; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; @@ -27,15 +34,25 @@ @RestController @RequestMapping("/api/players") @Validated +@Tag(name = "Players", description = "Player management API") public class PlayersController { @Autowired private PlayersService playersService; @GetMapping + @Operation(summary = "List all players", description = "Retrieves a paginated list of players with their current score snapshot, supporting sorting and pagination") + @ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Successfully retrieved list of players", + content = @Content(schema = @Schema(implementation = PlayerListResponseTO.class))), + @ApiResponse(responseCode = "400", description = "Invalid request parameters", content = @Content) + }) public ResponseEntity listPlayers( + @Parameter(description = "Sort order for players (NAME or SCORE_DESC)", example = "NAME") @RequestParam(name = "sort", required = false, defaultValue = "NAME") PlayersSort sort, + @Parameter(description = "Starting index for pagination (0-based)", example = "0") @RequestParam(name = "startIndex", required = false, defaultValue = "0") @Min(0) int startIndex, + @Parameter(description = "Number of items per page (1-200)", example = "50") @RequestParam(name = "pageSize", required = false, defaultValue = "50") @Min(1) @Max(200) int pageSize) { var query = new PlayersQuery(startIndex, pageSize, sort); var response = playersService.listPlayers(query); @@ -43,7 +60,16 @@ public ResponseEntity listPlayers( } @PostMapping - public ResponseEntity createPlayer(@Valid @RequestBody UpsertPlayerRequest request) { + @Operation(summary = "Create a new player", description = "Creates a new player with unique first and last name combination") + @ApiResponses(value = { + @ApiResponse(responseCode = "201", description = "Player successfully created", + content = @Content(schema = @Schema(implementation = PlayerTO.class))), + @ApiResponse(responseCode = "400", description = "Invalid request body", content = @Content), + @ApiResponse(responseCode = "409", description = "Player with the same name already exists", content = @Content) + }) + public ResponseEntity createPlayer( + @Parameter(description = "Player data to create", required = true) + @Valid @RequestBody UpsertPlayerRequest request) { var player = playersService.createPlayer(request); var location = ServletUriComponentsBuilder .fromCurrentRequest() @@ -54,16 +80,34 @@ public ResponseEntity createPlayer(@Valid @RequestBody UpsertPlayerReq } @PutMapping("/{id}") + @Operation(summary = "Update an existing player", description = "Updates player information by ID, enforcing uniqueness constraints") + @ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Player successfully updated", + content = @Content(schema = @Schema(implementation = PlayerTO.class))), + @ApiResponse(responseCode = "400", description = "Invalid request body", content = @Content), + @ApiResponse(responseCode = "404", description = "Player not found", content = @Content), + @ApiResponse(responseCode = "409", description = "Player with the same name already exists", content = @Content) + }) public ResponseEntity updatePlayer( + @Parameter(description = "Player ID", required = true, example = "123e4567-e89b-12d3-a456-426614174000") @PathVariable UUID id, + @Parameter(description = "Updated player data", required = true) @Valid @RequestBody UpsertPlayerRequest request) { var player = playersService.updatePlayer(id, request); return ResponseEntity.ok(player); } @DeleteMapping("/{id}") + @Operation(summary = "Delete a player", description = "Deletes a player by ID. Use forceDeletion=true to nullify references in games and scores before deletion") + @ApiResponses(value = { + @ApiResponse(responseCode = "204", description = "Player successfully deleted"), + @ApiResponse(responseCode = "404", description = "Player not found", content = @Content), + @ApiResponse(responseCode = "409", description = "Player has references and forceDeletion is false", content = @Content) + }) public ResponseEntity deletePlayer( + @Parameter(description = "Player ID", required = true, example = "123e4567-e89b-12d3-a456-426614174000") @PathVariable UUID id, + @Parameter(description = "Force deletion by nullifying references", example = "false") @RequestParam(name = "forceDeletion", required = false, defaultValue = "false") boolean forceDeletion) { playersService.deletePlayer(id, forceDeletion); return ResponseEntity.noContent().build(); From 94f672ef58dfd01ac44e36c68622b0b662180d7d Mon Sep 17 00:00:00 2001 From: Joern Wellniak Date: Fri, 24 Oct 2025 09:55:50 +0200 Subject: [PATCH 13/15] ADR --- ...autowired.md => ADR-008-springboottest.md} | 0 docs/architecture/decisions/ADR-011-flyway.md | 75 +++++++++++++++++++ .../decisions/ADR-012-flyway-in-tests.md | 65 ++++++++++++++++ 3 files changed, 140 insertions(+) rename docs/architecture/decisions/{ADR-008-autowired.md => ADR-008-springboottest.md} (100%) create mode 100644 docs/architecture/decisions/ADR-011-flyway.md create mode 100644 docs/architecture/decisions/ADR-012-flyway-in-tests.md diff --git a/docs/architecture/decisions/ADR-008-autowired.md b/docs/architecture/decisions/ADR-008-springboottest.md similarity index 100% rename from docs/architecture/decisions/ADR-008-autowired.md rename to docs/architecture/decisions/ADR-008-springboottest.md diff --git a/docs/architecture/decisions/ADR-011-flyway.md b/docs/architecture/decisions/ADR-011-flyway.md new file mode 100644 index 0000000..4dffd69 --- /dev/null +++ b/docs/architecture/decisions/ADR-011-flyway.md @@ -0,0 +1,75 @@ + +# ADR-011: Use Flyway for All DDL Operations + +Date: 2025-10-24 + +## Status + +Accepted + +## Context + +Database schema changes (DDL operations) are currently managed manually or through ad-hoc scripts. +This approach can lead to inconsistencies, difficulties in tracking changes, and challenges in +maintaining a reliable migration history. Flyway provides a structured and version-controlled way to +manage database migrations, ensuring consistency and traceability. + +## Decision + +All DDL operations will be managed using Flyway scripts. Each table will have its own dedicated +Flyway script to ensure modularity and clarity. This approach will standardize database migrations +and simplify collaboration among developers. + +## Consequences + +- **Positive**: + - Ensures a consistent and version-controlled approach to database schema changes. + - Simplifies tracking and auditing of database changes. + - Reduces the risk of conflicts and errors during migrations. + - Modular scripts improve clarity and maintainability. + +- **Negative**: + - Requires developers to learn and adopt Flyway conventions. + - Initial setup effort to migrate existing schema changes into Flyway scripts. + +## Implementation + +1. Add Flyway as a dependency in the `pom.xml`: + ```xml + + org.flywaydb + flyway-core + 9.0.0 + + ``` + +2. Configure Flyway in the `application.properties` file: + ```properties + spring.flyway.enabled=true + spring.flyway.locations=classpath:db/migration + spring.flyway.baseline-on-migrate=true + ``` + +3. Create a new Flyway script for each table: + - Scripts should follow the naming convention `V__.sql`. + - Example for a `users` table: + ```sql + -- File: V1__Create_users_table.sql + CREATE TABLE users ( + id BIGINT PRIMARY KEY, + username VARCHAR(255) NOT NULL, + password VARCHAR(255) NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ); + + ``` + +the table and the columns in the table should be described using SQL comments. + +Use varchar without limitation instead of varchar(xxx) + +4. Store all Flyway scripts in the `src/main/resources/db/migration` directory. + +5. Document this decision in the project's development guidelines to ensure all new DDL operations + are added as Flyway scripts. +```` diff --git a/docs/architecture/decisions/ADR-012-flyway-in-tests.md b/docs/architecture/decisions/ADR-012-flyway-in-tests.md new file mode 100644 index 0000000..a3b3625 --- /dev/null +++ b/docs/architecture/decisions/ADR-012-flyway-in-tests.md @@ -0,0 +1,65 @@ +# ADR-012: Use Flyway Scripts for Database Initialization in Integration Tests + +Date: 2025-10-24 + +## Status + +Accepted + +## Context + +Integration tests (IT) require a consistent and reliable database state to ensure accurate and +reproducible results. Currently, database initialization for integration tests is managed manually +or through ad-hoc scripts, which can lead to inconsistencies and errors. Using the same Flyway +scripts for both production and integration tests ensures that the database schema and data are +consistent across environments. + +## Decision + +Flyway scripts will be used to initialize the database for integration tests. This ensures that the +database schema and data are consistent with the production environment, reducing the risk of +discrepancies and improving test reliability. + +## Consequences + +- **Positive**: + - Ensures consistency between production and test environments. + - Reduces duplication of effort in maintaining separate initialization scripts. + - Simplifies debugging by using the same schema and data definitions. + +- **Negative**: + - May increase test setup time due to Flyway migrations. + - Requires integration tests to handle potential migration errors. + +## Implementation + +1. Configure Flyway in the `application-test.properties` file: + ```properties + spring.flyway.enabled=true + spring.flyway.locations=classpath:db/migration + spring.flyway.clean-on-validation-error=true + spring.flyway.baseline-on-migrate=true + ``` + +2. Ensure that the integration test database is cleaned and migrated before each test run: + - Use the `@BeforeEach` or `@BeforeAll` lifecycle methods to trigger Flyway migrations. + - Example: + ```java + @SpringBootTest + public class ExampleIntegrationTest { + + @Autowired + private Flyway flyway; + + @BeforeEach + public void setupDatabase() { + flyway.clean(); + flyway.migrate(); + } + + @Test + public void testExample() { + // Test logic here + } + } + ``` From 5836a424d31da86fd215fe7c3d51edf002b93ee8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 08:08:17 +0000 Subject: [PATCH 14/15] Add Flyway migrations and Testcontainers configuration per ADR-011 and ADR-012 - Add Flyway dependencies (flyway-core and flyway-database-postgresql) - Add spring-boot-testcontainers dependency for @ServiceConnection support - Configure Flyway in application.properties (enabled, baseline-on-migrate) - Change JPA ddl-auto from 'update' to 'none' (Flyway manages schema now) - Create Flyway migration scripts: - V1__Create_player_table.sql with indexes and constraints - V2__Create_game_table.sql with foreign keys and indexes - V3__Create_player_score_table.sql with indexes - Create PostgresTestcontainersConfig for integration tests - Create application-test.properties with Flyway enabled - Update PlayersControllerIT to use @Import(PostgresTestcontainersConfig.class) and @ActiveProfiles("test") - Remove manual container configuration from tests - All 35 tests passing (16 unit + 19 integration) Co-authored-by: jwellniak <87517644+jwellniak@users.noreply.github.com> --- pom.xml | 17 ++++++ src/main/resources/application.properties | 7 ++- .../db/migration/V1__Create_player_table.sql | 29 +++++++++++ .../db/migration/V2__Create_game_table.sql | 52 +++++++++++++++++++ .../V3__Create_player_score_table.sql | 45 ++++++++++++++++ .../api/controller/PlayersControllerIT.java | 29 +++-------- .../config/PostgresTestcontainersConfig.java | 29 +++++++++++ .../resources/application-test.properties | 9 ++++ 8 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 src/main/resources/db/migration/V1__Create_player_table.sql create mode 100644 src/main/resources/db/migration/V2__Create_game_table.sql create mode 100644 src/main/resources/db/migration/V3__Create_player_score_table.sql create mode 100644 src/test/java/com/skat/backend/config/PostgresTestcontainersConfig.java create mode 100644 src/test/resources/application-test.properties diff --git a/pom.xml b/pom.xml index 901cadf..ba78652 100644 --- a/pom.xml +++ b/pom.xml @@ -61,6 +61,17 @@ 2.1.0 + + org.flywaydb + flyway-core + + + + org.flywaydb + flyway-database-postgresql + runtime + + org.postgresql postgresql @@ -88,6 +99,12 @@ test + + org.springframework.boot + spring-boot-testcontainers + test + + org.testcontainers testcontainers diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index e5d3d93..75b0727 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -8,7 +8,12 @@ spring.datasource.password=postgres spring.datasource.driver-class-name=org.postgresql.Driver # JPA Configuration -spring.jpa.hibernate.ddl-auto=update +spring.jpa.hibernate.ddl-auto=none spring.jpa.show-sql=true spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.PostgreSQLDialect spring.jpa.properties.hibernate.format_sql=true + +# Flyway Configuration +spring.flyway.enabled=true +spring.flyway.locations=classpath:db/migration +spring.flyway.baseline-on-migrate=true diff --git a/src/main/resources/db/migration/V1__Create_player_table.sql b/src/main/resources/db/migration/V1__Create_player_table.sql new file mode 100644 index 0000000..f5bd144 --- /dev/null +++ b/src/main/resources/db/migration/V1__Create_player_table.sql @@ -0,0 +1,29 @@ +-- Table: player +-- Description: Stores player information for the Skat game +-- Each player has a unique combination of first_name and last_name + +CREATE TABLE player ( + -- Unique identifier for the player + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + + -- Player's first name (required, max 50 characters) + first_name VARCHAR NOT NULL, + + -- Player's last name (required, max 50 characters) + last_name VARCHAR NOT NULL, + + -- Ensure unique combination of first and last name + CONSTRAINT player_first_last_name_UQ UNIQUE (first_name, last_name) +); + +-- Index on first_name for faster lookups +CREATE INDEX player_first_name_IDX ON player(first_name); + +-- Index on last_name for faster lookups +CREATE INDEX player_last_name_IDX ON player(last_name); + +-- Add comments to columns +COMMENT ON TABLE player IS 'Stores player information for the Skat game'; +COMMENT ON COLUMN player.id IS 'Unique identifier for the player'; +COMMENT ON COLUMN player.first_name IS 'Player''s first name (required, max 50 characters)'; +COMMENT ON COLUMN player.last_name IS 'Player''s last name (required, max 50 characters)'; diff --git a/src/main/resources/db/migration/V2__Create_game_table.sql b/src/main/resources/db/migration/V2__Create_game_table.sql new file mode 100644 index 0000000..4d17562 --- /dev/null +++ b/src/main/resources/db/migration/V2__Create_game_table.sql @@ -0,0 +1,52 @@ +-- Table: game +-- Description: Stores information about Skat games played +-- Each game can involve up to 3 players with one designated as the main player + +CREATE TABLE game ( + -- Unique identifier for the game + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + + -- First player in the game (optional, can be nullified) + player1_id UUID, + + -- Second player in the game (optional, can be nullified) + player2_id UUID, + + -- Third player in the game (optional, can be nullified) + player3_id UUID, + + -- Main player who made the bid (optional, can be nullified) + main_player_id UUID, + + -- Bid value for the game (optional) + bid_value INTEGER, + + -- Score for the game (optional) + score INTEGER, + + -- Timestamp when the game was played (required) + played_at TIMESTAMP WITH TIME ZONE NOT NULL, + + -- Foreign key constraints (nullable to support player deletion) + CONSTRAINT game_player1_FK FOREIGN KEY (player1_id) REFERENCES player(id), + CONSTRAINT game_player2_FK FOREIGN KEY (player2_id) REFERENCES player(id), + CONSTRAINT game_player3_FK FOREIGN KEY (player3_id) REFERENCES player(id), + CONSTRAINT game_main_player_FK FOREIGN KEY (main_player_id) REFERENCES player(id) +); + +-- Index on main_player_id for faster lookups +CREATE INDEX game_main_player_IDX ON game(main_player_id); + +-- Index on played_at for chronological queries +CREATE INDEX game_played_at_IDX ON game(played_at); + +-- Add comments to columns +COMMENT ON TABLE game IS 'Stores information about Skat games played'; +COMMENT ON COLUMN game.id IS 'Unique identifier for the game'; +COMMENT ON COLUMN game.player1_id IS 'First player in the game (optional, can be nullified)'; +COMMENT ON COLUMN game.player2_id IS 'Second player in the game (optional, can be nullified)'; +COMMENT ON COLUMN game.player3_id IS 'Third player in the game (optional, can be nullified)'; +COMMENT ON COLUMN game.main_player_id IS 'Main player who made the bid (optional, can be nullified)'; +COMMENT ON COLUMN game.bid_value IS 'Bid value for the game'; +COMMENT ON COLUMN game.score IS 'Score for the game'; +COMMENT ON COLUMN game.played_at IS 'Timestamp when the game was played'; diff --git a/src/main/resources/db/migration/V3__Create_player_score_table.sql b/src/main/resources/db/migration/V3__Create_player_score_table.sql new file mode 100644 index 0000000..d0b08f2 --- /dev/null +++ b/src/main/resources/db/migration/V3__Create_player_score_table.sql @@ -0,0 +1,45 @@ +-- Table: player_score +-- Description: Stores cumulative scores for players across games +-- Each record represents a player's score at a specific point in the game sequence + +CREATE TABLE player_score ( + -- Unique identifier for the score record + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + + -- Reference to the player (optional, can be nullified) + player_id UUID, + + -- Reference to the game (required) + game_id UUID NOT NULL, + + -- Sequence index indicating the order of this score in the series (required) + sequence_index INTEGER NOT NULL, + + -- Total cumulative points for the player at this point (optional) + total_points INTEGER, + + -- Timestamp when this score was recorded (required) + created_at TIMESTAMP WITH TIME ZONE NOT NULL, + + -- Foreign key constraints + CONSTRAINT player_score_player_FK FOREIGN KEY (player_id) REFERENCES player(id), + CONSTRAINT player_score_game_FK FOREIGN KEY (game_id) REFERENCES game(id) +); + +-- Index on player_id for faster player score lookups +CREATE INDEX player_score_player_IDX ON player_score(player_id); + +-- Index on game_id for faster game score lookups +CREATE INDEX player_score_game_IDX ON player_score(game_id); + +-- Index on sequence_index for ordered queries +CREATE INDEX player_score_sequence_IDX ON player_score(sequence_index); + +-- Add comments to columns +COMMENT ON TABLE player_score IS 'Stores cumulative scores for players across games'; +COMMENT ON COLUMN player_score.id IS 'Unique identifier for the score record'; +COMMENT ON COLUMN player_score.player_id IS 'Reference to the player (optional, can be nullified)'; +COMMENT ON COLUMN player_score.game_id IS 'Reference to the game (required)'; +COMMENT ON COLUMN player_score.sequence_index IS 'Sequence index indicating the order of this score in the series'; +COMMENT ON COLUMN player_score.total_points IS 'Total cumulative points for the player at this point'; +COMMENT ON COLUMN player_score.created_at IS 'Timestamp when this score was recorded'; diff --git a/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java b/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java index 80d33f4..01b9f8f 100644 --- a/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java +++ b/src/test/java/com/skat/backend/api/controller/PlayersControllerIT.java @@ -3,6 +3,7 @@ import com.skat.backend.application.dto.ErrorResponseTO; import com.skat.backend.application.dto.PlayerListResponseTO; import com.skat.backend.application.dto.PlayerTO; +import com.skat.backend.config.PostgresTestcontainersConfig; import com.skat.backend.domain.entities.GameEntity; import com.skat.backend.domain.entities.PlayerEntity; import com.skat.backend.domain.entities.PlayerScoreEntity; @@ -14,16 +15,13 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.context.annotation.Import; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; -import org.springframework.test.context.DynamicPropertyRegistry; -import org.springframework.test.context.DynamicPropertySource; -import org.testcontainers.containers.PostgreSQLContainer; -import org.testcontainers.junit.jupiter.Container; -import org.testcontainers.junit.jupiter.Testcontainers; +import org.springframework.test.context.ActiveProfiles; import java.time.OffsetDateTime; import java.util.UUID; @@ -31,26 +29,15 @@ import static org.assertj.core.api.Assertions.assertThat; /** - * Integration test for PlayersController following ADR-001 and ADR-008. - * Uses Testcontainers for PostgreSQL 18 and TestRestTemplate for testing the full application context. + * Integration test for PlayersController following ADR-001, ADR-008, and ADR-012. + * Uses Testcontainers with PostgreSQL 18, TestRestTemplate for testing the full application context, + * and Flyway for database migrations. */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) -@Testcontainers +@Import(PostgresTestcontainersConfig.class) +@ActiveProfiles("test") class PlayersControllerIT { - @Container - static PostgreSQLContainer postgres = new PostgreSQLContainer<>("postgres:18") - .withDatabaseName("skatdb") - .withUsername("test") - .withPassword("test"); - - @DynamicPropertySource - static void configureProperties(DynamicPropertyRegistry registry) { - registry.add("spring.datasource.url", postgres::getJdbcUrl); - registry.add("spring.datasource.username", postgres::getUsername); - registry.add("spring.datasource.password", postgres::getPassword); - } - @Autowired private TestRestTemplate restTemplate; diff --git a/src/test/java/com/skat/backend/config/PostgresTestcontainersConfig.java b/src/test/java/com/skat/backend/config/PostgresTestcontainersConfig.java new file mode 100644 index 0000000..7ba4199 --- /dev/null +++ b/src/test/java/com/skat/backend/config/PostgresTestcontainersConfig.java @@ -0,0 +1,29 @@ +package com.skat.backend.config; + +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.testcontainers.service.connection.ServiceConnection; +import org.springframework.context.annotation.Bean; +import org.testcontainers.containers.PostgreSQLContainer; + +/** + * Test configuration for PostgreSQL Testcontainers integration. + * Following ADR-012, this configuration provides a PostgreSQL container + * for integration tests with automatic lifecycle management by Spring Boot 3.1+. + */ +@TestConfiguration(proxyBeanMethods = false) +public class PostgresTestcontainersConfig { + + /** + * Provides a PostgreSQL 18 container for integration tests. + * The @ServiceConnection annotation automatically configures the datasource properties. + * Spring Boot 3.1+ handles container start/stop lifecycle automatically. + * + * @return PostgreSQL container instance + */ + @Bean + @ServiceConnection + PostgreSQLContainer postgres() { + return new PostgreSQLContainer<>("postgres:18"); + // No start() here! Spring Boot 3.1+ starts/stops the container automatically. + } +} diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties new file mode 100644 index 0000000..76b16aa --- /dev/null +++ b/src/test/resources/application-test.properties @@ -0,0 +1,9 @@ +# JPA Configuration for Tests +spring.jpa.hibernate.ddl-auto=none +spring.jpa.show-sql=false +spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.PostgreSQLDialect + +# Flyway Configuration for Tests +spring.flyway.enabled=true +spring.flyway.locations=classpath:db/migration +spring.flyway.baseline-on-migrate=true From 767c921719db2f88c2efa1ae17d49f8879627d71 Mon Sep 17 00:00:00 2001 From: Joern Wellniak Date: Fri, 24 Oct 2025 10:46:55 +0200 Subject: [PATCH 15/15] ADR, Swagger --- docker-compose.yml | 34 + .../decisions/ADR-013-pom-versions.md | 59 ++ pom.xml | 19 +- src/main/resources/application-dev.properties | 5 - src/main/resources/application.properties | 8 +- .../application/PlayersServiceTest.java | 596 +++++++++--------- 6 files changed, 411 insertions(+), 310 deletions(-) create mode 100644 docker-compose.yml create mode 100644 docs/architecture/decisions/ADR-013-pom-versions.md diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..8ac8cc6 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,34 @@ +version: '3.8' + +services: + postgres: + image: postgres:18 + container_name: postgres-db + environment: + POSTGRES_USER: test + POSTGRES_PASSWORD: test + POSTGRES_DB: testdb + # Ensure Flyway scripts are executed correctly + command: > + postgres -c config_file=/var/lib/postgresql/18/docker/postgresql.conf + ports: + - "5432:5432" + volumes: + - postgres-data:/var/lib/postgresql + + flyway: + image: flyway/flyway:9.22.0 + container_name: flyway-migrator + depends_on: + - postgres + environment: + FLYWAY_URL: jdbc:postgresql://postgres:5432/testdb + FLYWAY_USER: test + FLYWAY_PASSWORD: test + volumes: + - ./src/main/resources/db/migration:/flyway/sql + command: -connectRetries=10 migrate + +volumes: + postgres-data: + driver: local diff --git a/docs/architecture/decisions/ADR-013-pom-versions.md b/docs/architecture/decisions/ADR-013-pom-versions.md new file mode 100644 index 0000000..4289457 --- /dev/null +++ b/docs/architecture/decisions/ADR-013-pom-versions.md @@ -0,0 +1,59 @@ +# ADR-013: Define All Versions in `pom.xml` as Properties + +Date: 2025-10-24 + +## Status + +Accepted + +## Context + +In the current project, dependency versions are scattered throughout the `pom.xml` file. This makes +it difficult to manage and update versions consistently. By defining all versions as properties in +the `pom.xml`, we can centralize version management, improve maintainability, and reduce the risk of +version conflicts. + +## Decision + +All dependency and plugin versions will be defined as properties in the `pom.xml` file. These +properties will be declared in a dedicated `` section at the top of the file. + +## Consequences + +- **Positive**: + - Centralized version management simplifies updates and ensures consistency. + - Reduces duplication of version numbers across the `pom.xml` file. + - Improves readability and maintainability of the `pom.xml` file. + +- **Negative**: + - Requires developers to reference properties instead of hardcoding versions. + - Initial effort to refactor the existing `pom.xml` file. + +## Implementation + +1. Define all versions in the `` section of the `pom.xml`: + ```xml + + 2.5.6 + 9.22.0 + 17 + + ``` + +2. Reference these properties in the dependencies and plugins: + ```xml + + org.springframework.boot + spring-boot-starter + ${spring.version} + + + + org.flywaydb + flyway-maven-plugin + ${flyway.version} + + ``` + +3. Document this decision in the project's development guidelines to ensure all new versions are + added as properties. diff --git a/pom.xml b/pom.xml index ba78652..a391a65 100644 --- a/pom.xml +++ b/pom.xml @@ -18,6 +18,11 @@ 3.5.6 1.20.4 3.27.3 + 1.18.30 + 2.6.0 + 3.13.0 + 3.2.5 + 3.2.5 @@ -51,14 +56,14 @@ org.projectlombok lombok - 1.18.30 + ${lombok.version} provided org.springdoc springdoc-openapi-starter-webmvc-ui - 2.1.0 + ${springdoc.version} @@ -144,10 +149,10 @@ org.apache.maven.plugins maven-compiler-plugin - 3.13.0 + ${maven.compiler.plugin.version} - 21 - 21 + ${maven.compiler.source} + ${maven.compiler.target} true @@ -156,7 +161,7 @@ org.apache.maven.plugins maven-surefire-plugin - 3.2.5 + ${maven.surefire.plugin.version} false @@ -169,7 +174,7 @@ org.apache.maven.plugins maven-failsafe-plugin - 3.2.5 + ${maven.failsafe.plugin.version} diff --git a/src/main/resources/application-dev.properties b/src/main/resources/application-dev.properties index b29588a..0a7dc8a 100644 --- a/src/main/resources/application-dev.properties +++ b/src/main/resources/application-dev.properties @@ -1,11 +1,6 @@ # Development profile with H2 in-memory database server.port=8080 -# H2 Configuration for development -spring.datasource.url=jdbc:h2:mem:skatdb -spring.datasource.driverClassName=org.h2.Driver -spring.datasource.username=sa -spring.datasource.password= # JPA Configuration spring.jpa.hibernate.ddl-auto=update diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 75b0727..8b14ee2 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -2,9 +2,9 @@ server.port=8080 # PostgreSQL Configuration -spring.datasource.url=jdbc:postgresql://localhost:5432/skatdb -spring.datasource.username=postgres -spring.datasource.password=postgres +spring.datasource.url=jdbc:postgresql://localhost:5432/testdb +spring.datasource.username=test +spring.datasource.password=test spring.datasource.driver-class-name=org.postgresql.Driver # JPA Configuration @@ -17,3 +17,5 @@ spring.jpa.properties.hibernate.format_sql=true spring.flyway.enabled=true spring.flyway.locations=classpath:db/migration spring.flyway.baseline-on-migrate=true + +springdoc.override-with-generic-response=false diff --git a/src/test/java/com/skat/backend/application/PlayersServiceTest.java b/src/test/java/com/skat/backend/application/PlayersServiceTest.java index ca3d603..a1cdf69 100644 --- a/src/test/java/com/skat/backend/application/PlayersServiceTest.java +++ b/src/test/java/com/skat/backend/application/PlayersServiceTest.java @@ -2,12 +2,20 @@ import com.skat.backend.api.exception.ConflictException; import com.skat.backend.api.exception.NotFoundException; -import com.skat.backend.application.dto.*; +import com.skat.backend.application.dto.PlayerListResponseTO; +import com.skat.backend.application.dto.PlayerTO; +import com.skat.backend.application.dto.PlayersQuery; +import com.skat.backend.application.dto.PlayersSort; +import com.skat.backend.application.dto.UpsertPlayerRequest; import com.skat.backend.domain.entities.PlayerEntity; import com.skat.backend.domain.entities.PlayerScoreEntity; import com.skat.backend.domain.repositories.GameRepository; import com.skat.backend.domain.repositories.PlayerRepository; import com.skat.backend.domain.repositories.PlayerScoreRepository; +import java.time.OffsetDateTime; +import java.util.List; +import java.util.Optional; +import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -15,307 +23,305 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.time.OffsetDateTime; -import java.util.*; - import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** - * Pure unit test for PlayersServiceImpl following ADR-002 (Unit Testing Strategy with Maven Surefire). - * Uses Mockito for mocking dependencies and AssertJ for assertions. - * No Spring context is loaded, making this a fast unit test. + * Pure unit test for PlayersServiceImpl following ADR-002 (Unit Testing Strategy with Maven Surefire). Uses Mockito for + * mocking dependencies and AssertJ for assertions. No Spring context is loaded, making this a fast unit test. */ class PlayersServiceTest { - @Mock - private PlayerRepository playerRepository; - - @Mock - private GameRepository gameRepository; - - @Mock - private PlayerScoreRepository playerScoreRepository; - - @InjectMocks - private PlayersServiceImpl playersService; - - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - } - - @Test - void given_validRequest_when_createPlayer_then_playerIsCreatedAndReturned() { - // Given - UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Schmidt"); - PlayerEntity savedPlayer = new PlayerEntity("Anna", "Schmidt"); - UUID playerId = UUID.randomUUID(); - savedPlayer.setId(playerId); - - when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) - .thenReturn(false); - when(playerRepository.save(any(PlayerEntity.class))).thenReturn(savedPlayer); - - // When - PlayerTO result = playersService.createPlayer(request); - - // Then - assertThat(result).isNotNull(); - assertThat(result.id()).isEqualTo(playerId); - assertThat(result.first_name()).isEqualTo("Anna"); - assertThat(result.last_name()).isEqualTo("Schmidt"); - - verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt"); - verify(playerRepository).save(any(PlayerEntity.class)); - } - - @Test - void given_duplicateName_when_createPlayer_then_throwsConflictException() { - // Given - UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Schmidt"); - when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) - .thenReturn(true); - - // When & Then - assertThatThrownBy(() -> playersService.createPlayer(request)) - .isInstanceOf(ConflictException.class) - .hasMessageContaining("Player with first_name+last_name already exists"); - - verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt"); - verify(playerRepository, never()).save(any()); - } - - @Test - void given_nameWithWhitespace_when_createPlayer_then_nameIsTrimmed() { - // Given - UpsertPlayerRequest request = new UpsertPlayerRequest(" Anna ", " Schmidt "); - PlayerEntity savedPlayer = new PlayerEntity("Anna", "Schmidt"); - UUID playerId = UUID.randomUUID(); - savedPlayer.setId(playerId); - - when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) - .thenReturn(false); - when(playerRepository.save(any(PlayerEntity.class))).thenReturn(savedPlayer); - - // When - PlayerTO result = playersService.createPlayer(request); - - // Then - ArgumentCaptor playerCaptor = ArgumentCaptor.forClass(PlayerEntity.class); - verify(playerRepository).save(playerCaptor.capture()); - PlayerEntity capturedPlayer = playerCaptor.getValue(); - assertThat(capturedPlayer.getFirstName()).isEqualTo("Anna"); - assertThat(capturedPlayer.getLastName()).isEqualTo("Schmidt"); - } - - @Test - void given_existingPlayer_when_updatePlayer_then_playerIsUpdatedAndReturned() { - // Given - UUID playerId = UUID.randomUUID(); - UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); - PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); - existingPlayer.setId(playerId); - - when(playerRepository.findById(playerId)).thenReturn(Optional.of(existingPlayer)); - when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId)) - .thenReturn(false); - when(playerRepository.save(any(PlayerEntity.class))).thenReturn(existingPlayer); - - // When - PlayerTO result = playersService.updatePlayer(playerId, request); - - // Then - assertThat(result).isNotNull(); - assertThat(result.id()).isEqualTo(playerId); - assertThat(result.first_name()).isEqualTo("Anna"); - assertThat(result.last_name()).isEqualTo("Mueller"); - - verify(playerRepository).findById(playerId); - verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId); - verify(playerRepository).save(existingPlayer); - assertThat(existingPlayer.getLastName()).isEqualTo("Mueller"); - } - - @Test - void given_nonExistentPlayer_when_updatePlayer_then_throwsNotFoundException() { - // Given - UUID playerId = UUID.randomUUID(); - UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); - when(playerRepository.findById(playerId)).thenReturn(Optional.empty()); - - // When & Then - assertThatThrownBy(() -> playersService.updatePlayer(playerId, request)) - .isInstanceOf(NotFoundException.class) - .hasMessageContaining("Player not found"); - - verify(playerRepository).findById(playerId); - verify(playerRepository, never()).save(any()); - } - - @Test - void given_duplicateNameOnUpdate_when_updatePlayer_then_throwsConflictException() { - // Given - UUID playerId = UUID.randomUUID(); - UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); - PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); - existingPlayer.setId(playerId); - - when(playerRepository.findById(playerId)).thenReturn(Optional.of(existingPlayer)); - when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId)) - .thenReturn(true); - - // When & Then - assertThatThrownBy(() -> playersService.updatePlayer(playerId, request)) - .isInstanceOf(ConflictException.class) - .hasMessageContaining("Player with first_name+last_name already exists"); - - verify(playerRepository).findById(playerId); - verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId); - verify(playerRepository, never()).save(any()); - } - - @Test - void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_playerIsDeleted() { - // Given - UUID playerId = UUID.randomUUID(); - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); - player.setId(playerId); - - when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); - when(gameRepository.existsByPlayerId(playerId)).thenReturn(false); - when(playerScoreRepository.existsByPlayerId(playerId)).thenReturn(false); - - // When - playersService.deletePlayer(playerId, false); - - // Then - verify(playerRepository).findById(playerId); - verify(gameRepository).existsByPlayerId(playerId); - verify(playerScoreRepository).existsByPlayerId(playerId); - verify(playerRepository).delete(player); - verify(gameRepository, never()).nullifyPlayer1References(any()); - } - - @Test - void given_playerWithGameReferences_when_deletePlayerWithoutForce_then_throwsConflictException() { - // Given - UUID playerId = UUID.randomUUID(); - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); - player.setId(playerId); - - when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); - when(gameRepository.existsByPlayerId(playerId)).thenReturn(true); - - // When & Then - assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) - .isInstanceOf(ConflictException.class) - .hasMessageContaining("Player is referenced in games or scores"); - - verify(playerRepository).findById(playerId); - verify(gameRepository).existsByPlayerId(playerId); - verify(playerRepository, never()).delete(any()); - } - - @Test - void given_playerWithScoreReferences_when_deletePlayerWithoutForce_then_throwsConflictException() { - // Given - UUID playerId = UUID.randomUUID(); - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); - player.setId(playerId); - - when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); - when(gameRepository.existsByPlayerId(playerId)).thenReturn(false); - when(playerScoreRepository.existsByPlayerId(playerId)).thenReturn(true); - - // When & Then - assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) - .isInstanceOf(ConflictException.class) - .hasMessageContaining("Player is referenced in games or scores"); - - verify(playerRepository).findById(playerId); - verify(gameRepository).existsByPlayerId(playerId); - verify(playerScoreRepository).existsByPlayerId(playerId); - verify(playerRepository, never()).delete(any()); - } - - @Test - void given_playerWithReferences_when_forceDeletePlayer_then_referencesNullifiedAndPlayerDeleted() { - // Given - UUID playerId = UUID.randomUUID(); - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); - player.setId(playerId); - - when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); - - // When - playersService.deletePlayer(playerId, true); - - // Then - verify(playerRepository).findById(playerId); - verify(gameRepository).nullifyPlayer1References(playerId); - verify(gameRepository).nullifyPlayer2References(playerId); - verify(gameRepository).nullifyPlayer3References(playerId); - verify(gameRepository).nullifyMainPlayerReferences(playerId); - verify(playerScoreRepository).nullifyPlayerReferences(playerId); - verify(playerRepository).delete(player); - } - - @Test - void given_nonExistentPlayer_when_deletePlayer_then_throwsNotFoundException() { - // Given - UUID playerId = UUID.randomUUID(); - when(playerRepository.findById(playerId)).thenReturn(Optional.empty()); - - // When & Then - assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) - .isInstanceOf(NotFoundException.class) - .hasMessageContaining("Player not found"); - - verify(playerRepository).findById(playerId); - verify(playerRepository, never()).delete(any()); - } - - @Test - void given_validQuery_when_listPlayers_then_returnsPlayerListResponse() { - // Given - PlayersQuery query = new PlayersQuery(0, 50, PlayersSort.NAME); - - UUID playerId = UUID.randomUUID(); - PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); - player.setId(playerId); - List players = List.of(player); - - PlayerScoreEntity score = new PlayerScoreEntity(); - score.setPlayer(player); - score.setTotalPoints(100); - score.setSequenceIndex(5); - score.setCreatedAt(OffsetDateTime.now()); - - when(playerRepository.findAllOrderedByName(any())).thenReturn(players); - when(playerScoreRepository.findLatestScoresForPlayers(anyList())).thenReturn(List.of(score)); - when(playerRepository.count()).thenReturn(1L); - - // When - PlayerListResponseTO result = playersService.listPlayers(query); - - // Then - assertThat(result).isNotNull(); - assertThat(result.items()).hasSize(1); - assertThat(result.items().get(0).first_name()).isEqualTo("Anna"); - assertThat(result.items().get(0).last_name()).isEqualTo("Schmidt"); - assertThat(result.items().get(0).current_total_points()).isEqualTo(100); - assertThat(result.items().get(0).current_sequence_index()).isEqualTo(5); - assertThat(result.paging().startIndex()).isEqualTo(0); - assertThat(result.paging().pageSize()).isEqualTo(50); - assertThat(result.paging().total()).isEqualTo(1L); - assertThat(result.sort()).isEqualTo(PlayersSort.NAME); - - verify(playerRepository).findAllOrderedByName(any()); - verify(playerScoreRepository).findLatestScoresForPlayers(anyList()); - verify(playerRepository).count(); - } + @Mock + private PlayerRepository playerRepository; + + @Mock + private GameRepository gameRepository; + + @Mock + private PlayerScoreRepository playerScoreRepository; + + @InjectMocks + private PlayersServiceImpl playersService; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + } + + @Test + void given_validRequest_when_createPlayer_then_playerIsCreatedAndReturned() { + // Given + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Schmidt"); + PlayerEntity savedPlayer = new PlayerEntity("Anna", "Schmidt"); + UUID playerId = UUID.randomUUID(); + savedPlayer.setId(playerId); + + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) + .thenReturn(false); + when(playerRepository.save(any(PlayerEntity.class))).thenReturn(savedPlayer); + + // When + PlayerTO result = playersService.createPlayer(request); + + // Then + assertThat(result).isNotNull(); + assertThat(result.id()).isEqualTo(playerId); + assertThat(result.first_name()).isEqualTo("Anna"); + assertThat(result.last_name()).isEqualTo("Schmidt"); + + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt"); + verify(playerRepository).save(any(PlayerEntity.class)); + } + + @Test + void given_duplicateName_when_createPlayer_then_throwsConflictException() { + // Given + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Schmidt"); + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) + .thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.createPlayer(request)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player with first_name+last_name already exists"); + + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt"); + verify(playerRepository, never()).save(any()); + } + + @Test + void given_nameWithWhitespace_when_createPlayer_then_nameIsTrimmed() { + // Given + UpsertPlayerRequest request = new UpsertPlayerRequest(" Anna ", " Schmidt "); + PlayerEntity savedPlayer = new PlayerEntity("Anna", "Schmidt"); + UUID playerId = UUID.randomUUID(); + savedPlayer.setId(playerId); + + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCase("Anna", "Schmidt")) + .thenReturn(false); + when(playerRepository.save(any(PlayerEntity.class))).thenReturn(savedPlayer); + + // When + playersService.createPlayer(request); + + // Then + ArgumentCaptor playerCaptor = ArgumentCaptor.forClass(PlayerEntity.class); + verify(playerRepository).save(playerCaptor.capture()); + PlayerEntity capturedPlayer = playerCaptor.getValue(); + assertThat(capturedPlayer.getFirstName()).isEqualTo("Anna"); + assertThat(capturedPlayer.getLastName()).isEqualTo("Schmidt"); + } + + @Test + void given_existingPlayer_when_updatePlayer_then_playerIsUpdatedAndReturned() { + // Given + UUID playerId = UUID.randomUUID(); + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); + PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); + existingPlayer.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(existingPlayer)); + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId)) + .thenReturn(false); + when(playerRepository.save(any(PlayerEntity.class))).thenReturn(existingPlayer); + + // When + PlayerTO result = playersService.updatePlayer(playerId, request); + + // Then + assertThat(result).isNotNull(); + assertThat(result.id()).isEqualTo(playerId); + assertThat(result.first_name()).isEqualTo("Anna"); + assertThat(result.last_name()).isEqualTo("Mueller"); + + verify(playerRepository).findById(playerId); + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId); + verify(playerRepository).save(existingPlayer); + assertThat(existingPlayer.getLastName()).isEqualTo("Mueller"); + } + + @Test + void given_nonExistentPlayer_when_updatePlayer_then_throwsNotFoundException() { + // Given + UUID playerId = UUID.randomUUID(); + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); + when(playerRepository.findById(playerId)).thenReturn(Optional.empty()); + + // When & Then + assertThatThrownBy(() -> playersService.updatePlayer(playerId, request)) + .isInstanceOf(NotFoundException.class) + .hasMessageContaining("Player not found"); + + verify(playerRepository).findById(playerId); + verify(playerRepository, never()).save(any()); + } + + @Test + void given_duplicateNameOnUpdate_when_updatePlayer_then_throwsConflictException() { + // Given + UUID playerId = UUID.randomUUID(); + UpsertPlayerRequest request = new UpsertPlayerRequest("Anna", "Mueller"); + PlayerEntity existingPlayer = new PlayerEntity("Anna", "Schmidt"); + existingPlayer.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(existingPlayer)); + when(playerRepository.existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId)) + .thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.updatePlayer(playerId, request)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player with first_name+last_name already exists"); + + verify(playerRepository).findById(playerId); + verify(playerRepository).existsByFirstNameIgnoreCaseAndLastNameIgnoreCaseAndIdNot("Anna", "Mueller", playerId); + verify(playerRepository, never()).save(any()); + } + + @Test + void given_playerWithoutReferences_when_deletePlayerWithoutForce_then_playerIsDeleted() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + when(gameRepository.existsByPlayerId(playerId)).thenReturn(false); + when(playerScoreRepository.existsByPlayerId(playerId)).thenReturn(false); + + // When + playersService.deletePlayer(playerId, false); + + // Then + verify(playerRepository).findById(playerId); + verify(gameRepository).existsByPlayerId(playerId); + verify(playerScoreRepository).existsByPlayerId(playerId); + verify(playerRepository).delete(player); + verify(gameRepository, never()).nullifyPlayer1References(any()); + } + + @Test + void given_playerWithGameReferences_when_deletePlayerWithoutForce_then_throwsConflictException() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + when(gameRepository.existsByPlayerId(playerId)).thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player is referenced in games or scores"); + + verify(playerRepository).findById(playerId); + verify(gameRepository).existsByPlayerId(playerId); + verify(playerRepository, never()).delete(any()); + } + + @Test + void given_playerWithScoreReferences_when_deletePlayerWithoutForce_then_throwsConflictException() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + when(gameRepository.existsByPlayerId(playerId)).thenReturn(false); + when(playerScoreRepository.existsByPlayerId(playerId)).thenReturn(true); + + // When & Then + assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) + .isInstanceOf(ConflictException.class) + .hasMessageContaining("Player is referenced in games or scores"); + + verify(playerRepository).findById(playerId); + verify(gameRepository).existsByPlayerId(playerId); + verify(playerScoreRepository).existsByPlayerId(playerId); + verify(playerRepository, never()).delete(any()); + } + + @Test + void given_playerWithReferences_when_forceDeletePlayer_then_referencesNullifiedAndPlayerDeleted() { + // Given + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + + when(playerRepository.findById(playerId)).thenReturn(Optional.of(player)); + + // When + playersService.deletePlayer(playerId, true); + + // Then + verify(playerRepository).findById(playerId); + verify(gameRepository).nullifyPlayer1References(playerId); + verify(gameRepository).nullifyPlayer2References(playerId); + verify(gameRepository).nullifyPlayer3References(playerId); + verify(gameRepository).nullifyMainPlayerReferences(playerId); + verify(playerScoreRepository).nullifyPlayerReferences(playerId); + verify(playerRepository).delete(player); + } + + @Test + void given_nonExistentPlayer_when_deletePlayer_then_throwsNotFoundException() { + // Given + UUID playerId = UUID.randomUUID(); + when(playerRepository.findById(playerId)).thenReturn(Optional.empty()); + + // When & Then + assertThatThrownBy(() -> playersService.deletePlayer(playerId, false)) + .isInstanceOf(NotFoundException.class) + .hasMessageContaining("Player not found"); + + verify(playerRepository).findById(playerId); + verify(playerRepository, never()).delete(any()); + } + + @Test + void given_validQuery_when_listPlayers_then_returnsPlayerListResponse() { + // Given + PlayersQuery query = new PlayersQuery(0, 50, PlayersSort.NAME); + + UUID playerId = UUID.randomUUID(); + PlayerEntity player = new PlayerEntity("Anna", "Schmidt"); + player.setId(playerId); + List players = List.of(player); + + PlayerScoreEntity score = new PlayerScoreEntity(); + score.setPlayer(player); + score.setTotalPoints(100); + score.setSequenceIndex(5); + score.setCreatedAt(OffsetDateTime.now()); + + when(playerRepository.findAllOrderedByName(any())).thenReturn(players); + when(playerScoreRepository.findLatestScoresForPlayers(anyList())).thenReturn(List.of(score)); + when(playerRepository.count()).thenReturn(1L); + + // When + PlayerListResponseTO result = playersService.listPlayers(query); + + // Then + assertThat(result).isNotNull(); + assertThat(result.items()).hasSize(1); + assertThat(result.items().get(0).first_name()).isEqualTo("Anna"); + assertThat(result.items().get(0).last_name()).isEqualTo("Schmidt"); + assertThat(result.items().get(0).current_total_points()).isEqualTo(100); + assertThat(result.items().get(0).current_sequence_index()).isEqualTo(5); + assertThat(result.paging().startIndex()).isEqualTo(0); + assertThat(result.paging().pageSize()).isEqualTo(50); + assertThat(result.paging().total()).isEqualTo(1L); + assertThat(result.sort()).isEqualTo(PlayersSort.NAME); + + verify(playerRepository).findAllOrderedByName(any()); + verify(playerScoreRepository).findLatestScoresForPlayers(anyList()); + verify(playerRepository).count(); + } }