refactor(library): own IgdbGame record, one owned rule, layer rules

Stored import candidates keep their JSON shape, so READY imports from before still read.
This commit is contained in:
lobo committed 2026-09-30 09:37:16 +02:00
1 parent 1abc03d2c1
commit c1472c7d59
9 files changed
+126 -51

No files matched your search

+6 -1
View File
@@ -194,6 +194,10 @@ Inside a module: `api` (controllers, DTOs), `service` (business logic), `domain`
`speedrun_adapter`.
- No cycles between `modules`, `shared` and `configuration`.
- `api` never uses `repo`, and `repo` never uses `api`.
- `domain` never uses `api`, `service`, `repo` or an adapter: the import stores IGDB hits as the own record
`IgdbGame`, so a change of the adapter's `IgdbApiService.Game` cannot break stored imports.
- `service` never uses `api`, except the adapters' response DTOs and a fixed list of 8 services that still build
their module's response DTOs. The list only gets shorter.
- Every class in `repo` ends with `Repository`.
- `shared` never uses `modules`, and `modules` never use `configuration`.
- A `*Service` class in a module's `service` package has `@Service`. A `*Controller` class in `api` has
@@ -397,7 +401,8 @@ Runs unit, WebMvc, persistence and integration tests plus the ArchUnit rules. Ne
## Roadmap
- Sale alerts for wishlist games.
- Move `steam_adapter` DTOs out of `api`, then turn on strict module independence in ArchUnit.
- Move the adapter DTOs out of `api`, then turn on strict module independence in ArchUnit and drop the adapter
exception of the "`service` never uses `api`" rule.
## License
@@ -1,7 +1,5 @@
package core_service.modules.library.domain;
import core_service.modules.igdb_adapter.service.IgdbApiService;
import java.time.Instant;
import java.util.List;
@@ -46,7 +44,7 @@ public record GameImport(long id, long userId, Source source, Status status, int
* @param exact the first candidate has the line's name (case and punctuation ignored)
* @param candidates best first, at most five
*/
public record Row(Line line, boolean searchFailed, boolean exact, List<IgdbApiService.Game> candidates) {}
public record Row(Line line, boolean searchFailed, boolean exact, List<IgdbGame> candidates) {}
/**
* The user's pick for a line in the review.
@@ -0,0 +1,9 @@
package core_service.modules.library.domain;
import java.util.List;
/**
* An IGDB game as the library needs it, and the stored shape of an import candidate (V22 {@code candidates}): the
* component names are the JSON keys of rows already stored.
*/
public record IgdbGame(long igdbId, String name, Integer year, String imageUrl, List<String> platforms, Long steamAppid) {}
@@ -1,6 +1,6 @@
package core_service.modules.library.repo;
import core_service.modules.igdb_adapter.service.IgdbApiService;
import core_service.modules.library.domain.IgdbGame;
import core_service.modules.library.domain.GameImport;
import core_service.modules.library.domain.GameStatus;
import lombok.RequiredArgsConstructor;
@@ -189,11 +189,11 @@ public class GameImportRepository {
new MapSqlParameterSource("id", importId), (rs, row) -> new GameImport.Row(line(rs),
rs.getBoolean("search_failed"), rs.getBoolean("exact"),
jsonMapper.readValue(rs.getString("candidates"),
jsonMapper.getTypeFactory().constructCollectionType(List.class, IgdbApiService.Game.class))));
jsonMapper.getTypeFactory().constructCollectionType(List.class, IgdbGame.class))));
}
public void saveMatch(long importId, int lineNo, boolean searchFailed, boolean exact,
List<IgdbApiService.Game> candidates) {
List<IgdbGame> candidates) {
jdbc.update("update game_import_row set matched_at = now(), search_failed = :failed, exact = :exact, "
+ "candidates = cast(:candidates as jsonb) where import_id = :id and line_no = :line",
new MapSqlParameterSource("id", importId).addValue("line", lineNo).addValue("failed", searchFailed)
@@ -1,7 +1,7 @@
package core_service.modules.library.service;
import core_service.modules.igdb_adapter.service.IgdbApiService;
import core_service.modules.library.domain.GameImport;
import core_service.modules.library.domain.IgdbGame;
import core_service.modules.library.repo.GameImportRepository;
import jakarta.annotation.PreDestroy;
import lombok.extern.slf4j.Slf4j;
@@ -49,7 +49,6 @@ public class GameImportWorker {
private final GameImportRepository importRepository;
private final ManualGameService manualGameService;
private final IgdbApiService igdbApiService;
private final TimeToBeatService timeToBeatService;
private final MoodService moodService;
private final Duration stallTime;
@@ -59,11 +58,10 @@ public class GameImportWorker {
private final Lane saving;
public GameImportWorker(GameImportRepository importRepository, ManualGameService manualGameService,
IgdbApiService igdbApiService, TimeToBeatService timeToBeatService, MoodService moodService,
TimeToBeatService timeToBeatService, MoodService moodService,
@Value("${app.library.import-stall-minutes:5}") long stallMinutes) {
this.importRepository = importRepository;
this.manualGameService = manualGameService;
this.igdbApiService = igdbApiService;
this.timeToBeatService = timeToBeatService;
this.moodService = moodService;
this.stallTime = Duration.ofMinutes(stallMinutes);
@@ -140,11 +138,11 @@ public class GameImportWorker {
Map<Integer, GameImport.Row> rows = importRepository.findRows(importId).stream()
.collect(Collectors.toMap(row -> row.line().lineNo(), row -> row));
for (GameImport.Choice choice : choices) {
IgdbApiService.Game game = rows.get(choice.lineNo()).candidates().stream()
IgdbGame game = rows.get(choice.lineNo()).candidates().stream()
.filter(candidate -> candidate.igdbId() == choice.igdbId()).findFirst()
// A game from a new search in the review. IGDB knew it minutes ago, so no answer is an IGDB
// failure: the try stops and the line waits for the next one.
.or(() -> igdbApiService.getGame(choice.igdbId()))
.or(() -> manualGameService.findGame(choice.igdbId()))
.orElseThrow(() -> new IllegalStateException("IGDB did not answer for game " + choice.igdbId()));
if (Thread.currentThread().isInterrupted()) {
return;
@@ -170,11 +168,11 @@ public class GameImportWorker {
importRepository.markSaved(importId);
}
private record Match(boolean searchFailed, boolean exact, List<IgdbApiService.Game> candidates) {}
private record Match(boolean searchFailed, boolean exact, List<IgdbGame> candidates) {}
private Match match(GameImport.Line line) {
if (line.igdbId() != null) {
Optional<IgdbApiService.Game> game = igdbApiService.getGame(line.igdbId());
Optional<IgdbGame> game = manualGameService.findGame(line.igdbId());
return new Match(game.isEmpty(), game.isPresent(), game.stream().toList());
}
return manualGameService.findGames(line.title())
@@ -185,9 +183,9 @@ public class GameImportWorker {
}
// Same name first, among them one on the line's platform; else IGDB's order.
static List<IgdbApiService.Game> rank(GameImport.Line line, List<IgdbApiService.Game> found) {
Comparator<IgdbApiService.Game> order = Comparator
.comparing((IgdbApiService.Game game) -> !sameName(game.name(), line.title()))
static List<IgdbGame> rank(GameImport.Line line, List<IgdbGame> found) {
Comparator<IgdbGame> order = Comparator
.comparing((IgdbGame game) -> !sameName(game.name(), line.title()))
.thenComparing(game -> line.platform() == null
|| game.platforms().stream().noneMatch(line.platform()::equalsIgnoreCase));
return found.stream().sorted(order).limit(MAX_CANDIDATES).toList();
@@ -4,6 +4,7 @@ import core_service.modules.igdb_adapter.service.IgdbApiService;
import core_service.modules.library.api.dto.GameSearchDto;
import core_service.modules.library.api.dto.ManualGameUpdate;
import core_service.modules.library.domain.GameKeys;
import core_service.modules.library.domain.IgdbGame;
import core_service.modules.library.domain.GameSource;
import core_service.modules.library.domain.GameStatus;
import core_service.modules.library.domain.SteamGame;
@@ -93,24 +94,22 @@ public class ManualGameService {
// Games already in ToBePlayed first, with their catalog name and image (an auditor's correction included); then
// games added by hand whose name matches (also a duplicate of a game IGDB now knows on Steam, so an auditor can
// merge it); then the rest in IGDB's order. One hit per key.
private List<GameSearchDto.Hit> hits(Long userId, String query, List<IgdbApiService.Game> games) {
private List<GameSearchDto.Hit> hits(Long userId, String query, List<IgdbGame> games) {
Map<Long, Long> targets = mergeRepository.findTargets(games.stream().flatMap(game -> ownKeys(game).stream()).toList());
Map<Long, SteamGame> catalog = steamGameRepository.findAllById(games.stream().map(game -> key(game, targets)).toList())
.stream().collect(Collectors.toMap(SteamGame::getAppid, game -> game));
List<SteamGame> addedByOthers = steamGameRepository.findManualByName(UserGameRepository.containsPattern(query),
PageRequest.of(0, MAX_CATALOG_HITS));
Set<Long> owned = new HashSet<>(userGameRepository.findActiveAppids(userId, Stream.concat(
games.stream().flatMap(game -> possibleKeys(game, targets).stream()),
addedByOthers.stream().map(SteamGame::getAppid)).toList()));
Set<Long> owned = ownedAppids(userId, games, targets, addedByOthers.stream().map(SteamGame::getAppid).toList());
List<GameSearchDto.Hit> inCatalog = new ArrayList<>();
List<GameSearchDto.Hit> rest = new ArrayList<>();
for (IgdbApiService.Game game : games) {
for (IgdbGame game : games) {
long appid = key(game, targets);
SteamGame entry = catalog.get(appid);
GameSearchDto.Hit hit = new GameSearchDto.Hit(appid, game.igdbId(), entry != null ? entry.getName() : game.name(),
game.year(), entry != null && entry.getImageUrl() != null ? entry.getImageUrl() : game.imageUrl(),
fittingPlatforms(game), possibleKeys(game, targets).stream().anyMatch(owned::contains), entry != null);
fittingPlatforms(game), isOwned(game, targets, owned), entry != null);
(entry != null ? inCatalog : rest).add(hit);
}
Stream<GameSearchDto.Hit> others = addedByOthers.stream().map(game -> new GameSearchDto.Hit(game.getAppid(),
@@ -125,7 +124,7 @@ public class ManualGameService {
// Few hits often mean a typo. IGDB's own hits keep their order; the variant hits follow, closest name first.
// The variants go through the same cache, and one that fails adds nothing.
private List<IgdbApiService.Game> withTypoHelp(String query, List<IgdbApiService.Game> found) {
private List<IgdbGame> withTypoHelp(String query, List<IgdbGame> found) {
if (found.size() >= MIN_HITS) {
return found;
}
@@ -137,12 +136,12 @@ public class ManualGameService {
Set<Long> seen = new HashSet<>();
found.forEach(game -> seen.add(game.igdbId()));
List<IgdbApiService.Game> extra = new ArrayList<>();
List<IgdbGame> extra = new ArrayList<>();
for (String variant : variants) {
if (found.size() + extra.size() >= MIN_HITS) {
break;
}
igdbApiService.searchGames(variant).orElse(List.of()).stream()
igdbApiService.searchGames(variant).orElse(List.of()).stream().map(ManualGameService::toIgdbGame)
.filter(game -> seen.add(game.igdbId()))
.forEach(extra::add);
}
@@ -162,7 +161,7 @@ public class ManualGameService {
return new AddResult.SearchUnavailable();
}
// Outside the transaction: no database connection waits for IGDB.
Optional<IgdbApiService.Game> found = igdbApiService.getGame(igdbId);
Optional<IgdbGame> found = findGame(igdbId);
if (found.isEmpty()) {
return new AddResult.UnknownGame();
}
@@ -181,7 +180,7 @@ public class ManualGameService {
* @param importId the import that adds the game, so its undo can remove it; null for a single add. A removed
* row that comes back keeps no import id.
*/
public AddResult addGame(Long userId, IgdbApiService.Game game, String platform, GameStatus status,
public AddResult addGame(Long userId, IgdbGame game, String platform, GameStatus status,
int playtimeMinutes, Long importId) {
AddResult result;
try {
@@ -201,35 +200,34 @@ public class ManualGameService {
/**
* IGDB's hits for a title, with the same typo help as {@link #search}; empty when IGDB failed or is off.
*/
public Optional<List<IgdbApiService.Game>> findGames(String query) {
public Optional<List<IgdbGame>> findGames(String query) {
if (!igdbApiService.isEnabled()) {
return Optional.empty();
}
return igdbApiService.searchGames(query).map(found -> withTypoHelp(query, found));
return igdbApiService.searchGames(query)
.map(found -> withTypoHelp(query, found.stream().map(ManualGameService::toIgdbGame).toList()));
}
/**
* The IGDB ids of the games the user already has, under any of their keys (a merge included).
*/
public Set<Long> ownedIgdbIds(Long userId, Collection<IgdbApiService.Game> games) {
public Set<Long> ownedIgdbIds(Long userId, Collection<IgdbGame> games) {
if (games.isEmpty()) {
return Set.of();
}
Map<Long, Long> targets = mergeRepository.findTargets(games.stream().flatMap(game -> ownKeys(game).stream()).toList());
Set<Long> owned = new HashSet<>(userGameRepository.findActiveAppids(userId,
games.stream().flatMap(game -> possibleKeys(game, targets).stream()).distinct().toList()));
return games.stream().filter(game -> possibleKeys(game, targets).stream().anyMatch(owned::contains))
.map(IgdbApiService.Game::igdbId).collect(Collectors.toSet());
Set<Long> owned = ownedAppids(userId, games, targets, List.of());
return games.stream().filter(game -> isOwned(game, targets, owned))
.map(IgdbGame::igdbId).collect(Collectors.toSet());
}
private AddResult addInTransaction(Long userId, IgdbApiService.Game game, String platform, GameStatus status,
private AddResult addInTransaction(Long userId, IgdbGame game, String platform, GameStatus status,
int playtimeMinutes, Long importId) {
return transaction.execute(tx -> {
Instant now = Instant.now();
Map<Long, Long> targets = mergeRepository.findTargets(ownKeys(game));
long appid = key(game, targets);
List<Long> possibleKeys = possibleKeys(game, targets);
if (!userGameRepository.findActiveAppids(userId, possibleKeys).isEmpty()) {
if (!ownedAppids(userId, List.of(game), targets, List.of()).isEmpty()) {
return new AddResult.AlreadyInLibrary();
}
Optional<UserGame> existing = userGameRepository.findById(new UserGameId(userId, appid));
@@ -287,7 +285,7 @@ public class ManualGameService {
// A Steam row belongs to the sync, which has the better name and art; only a game off Steam takes IGDB's, and
// only its own: a merged game must not rename the game it went into.
private void saveCatalogEntry(long appid, IgdbApiService.Game game, Instant now) {
private void saveCatalogEntry(long appid, IgdbGame game, Instant now) {
Optional<SteamGame> existing = steamGameRepository.findById(appid);
if (existing.isEmpty()) {
String imageUrl = GameKeys.isSteam(appid) ? null : game.imageUrl();
@@ -300,25 +298,47 @@ public class ManualGameService {
}
// A longer name would not fit the column; with no platform left, the user types one.
static List<String> fittingPlatforms(IgdbApiService.Game game) {
static List<String> fittingPlatforms(IgdbGame game) {
return game.platforms().stream().filter(platform -> platform.length() <= ManualGameUpdate.MAX_PLATFORM_LENGTH).toList();
}
/**
* One game by its IGDB id; empty when IGDB does not know it or failed.
*/
public Optional<IgdbGame> findGame(long igdbId) {
return igdbApiService.getGame(igdbId).map(ManualGameService::toIgdbGame);
}
private static IgdbGame toIgdbGame(IgdbApiService.Game game) {
return new IgdbGame(game.igdbId(), game.name(), game.year(), game.imageUrl(), game.platforms(), game.steamAppid());
}
// IGDB can learn a game's Steam appid after the user added it under minus its IGDB id. Each key goes through its
// merge, if an auditor merged it.
private static List<Long> possibleKeys(IgdbApiService.Game game, Map<Long, Long> targets) {
private static List<Long> possibleKeys(IgdbGame game, Map<Long, Long> targets) {
return ownKeys(game).stream().map(own -> targets.getOrDefault(own, own)).distinct().toList();
}
private Set<Long> ownedAppids(Long userId, Collection<IgdbGame> games, Map<Long, Long> targets,
Collection<Long> extraAppids) {
return new HashSet<>(userGameRepository.findActiveAppids(userId, Stream.concat(
games.stream().flatMap(game -> possibleKeys(game, targets).stream()), extraAppids.stream())
.distinct().toList()));
}
private static boolean isOwned(IgdbGame game, Map<Long, Long> targets, Set<Long> ownedAppids) {
return possibleKeys(game, targets).stream().anyMatch(ownedAppids::contains);
}
/**
* @param targets {@link GameMergeRepository#findTargets} of the {@link #ownKeys}
*/
private static long key(IgdbApiService.Game game, Map<Long, Long> targets) {
private static long key(IgdbGame game, Map<Long, Long> targets) {
return possibleKeys(game, targets).getFirst();
}
// The key IGDB gives today first, then minus the IGDB id when the game is on Steam.
private static List<Long> ownKeys(IgdbApiService.Game game) {
private static List<Long> ownKeys(IgdbGame game) {
long igdbKey = GameKeys.ofIgdb(game.igdbId());
return game.steamAppid() != null ? List.of(game.steamAppid(), igdbKey) : List.of(igdbKey);
}
@@ -1,6 +1,7 @@
package core_service.architecture;
import core_service.shared.test.AbstractArchitectureTest;
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.importer.ImportOption;
import com.tngtech.archunit.junit.AnalyzeClasses;
@@ -59,6 +60,33 @@ class ArchitectureTest extends AbstractArchitectureTest {
.should().dependOnClassesThat().resideInAPackage("..api..")
.because("Persistence layer must be independent of the API layer.");
@ArchTest
static final ArchRule domain_should_not_depend_on_other_layers = noClasses()
.that().resideInAPackage("core_service.modules..domain..")
.should().dependOnClassesThat().resideInAnyPackage("..api..", "..service..", "..repo..",
"core_service.modules.steam_adapter..", "core_service.modules.igdb_adapter..",
"core_service.modules.speedrun_adapter..")
.because("Entities and stored shapes must not change when a controller, service or external API changes.");
// The listed services build their module's response DTOs; the list only gets shorter.
@ArchTest
static final ArchRule services_should_not_depend_on_apis = noClasses()
.that().resideInAPackage("core_service.modules..service..")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.BacklogService")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.GameDetailService")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.GameImportService")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.LibraryQueryService")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.ManualGameService")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.SteamProfileService")
.and().doNotHaveFullyQualifiedName("core_service.modules.library.service.UserStatsService")
.and().doNotHaveFullyQualifiedName("core_service.modules.stats.service.PublicStatsService")
.should().dependOnClassesThat(JavaClass.Predicates.resideInAPackage("..api..")
.and(DescribedPredicate.not(JavaClass.Predicates.resideInAnyPackage(
"core_service.modules.steam_adapter.api.dto.response..",
"core_service.modules.igdb_adapter.api.dto.response..",
"core_service.modules.speedrun_adapter.api.dto.response.."))))
.because("Services must not depend on the HTTP layer; the adapters' response DTOs are the external APIs' shapes.");
@ArchTest
static final ArchRule repos_should_be_interfaces_named_repository = classes()
.that().resideInAPackage("..repo..")
@@ -146,6 +146,23 @@ class GameImportControllerTest extends AbstractIntegrationTest {
USER_ID, appid);
}
@Test
void get_ReadyImportStoredBeforeIgdbGame_ReadsItsCandidates() throws Exception {
long id = insertImport(USER_ID, "READY", "total = 1");
// The JSON the import wrote with IgdbApiService.Game, before the own record IgdbGame.
jdbc.update("insert into game_import_row (import_id, line_no, title, matched_at, exact, candidates) "
+ "values (?, 1, 'Zelda', now(), true, cast(? as jsonb))", id,
"[{\"igdbId\":119388,\"name\":\"Zelda: Tears of the Kingdom\",\"year\":2023,"
+ "\"imageUrl\":null,\"platforms\":[\"Switch\"],\"steamAppid\":null}]");
mockMvc.perform(get("/api/library/imports/" + id))
.andExpect(status().isOk())
.andExpect(jsonPath("$.rows[0].candidates[0].igdbId").value(119388))
.andExpect(jsonPath("$.rows[0].candidates[0].name").value("Zelda: Tears of the Kingdom"))
.andExpect(jsonPath("$.rows[0].candidates[0].year").value(2023))
.andExpect(jsonPath("$.rows[0].candidates[0].platforms[0]").value("Switch"));
}
@Test
void create_MatchesInTheBackground_SameNameFirstAndMarksGamesInTheLibrary() throws Exception {
jdbc.update("insert into steam_game (appid, name) values (1245620, 'ELDEN RING') on conflict do nothing");
@@ -1,6 +1,6 @@
package core_service.modules.library.service;
import core_service.modules.igdb_adapter.service.IgdbApiService;
import core_service.modules.library.domain.IgdbGame;
import core_service.modules.library.domain.GameImport;
import core_service.modules.library.repo.GameImportRepository;
import org.junit.jupiter.api.Test;
@@ -20,8 +20,8 @@ import static org.mockito.Mockito.when;
class GameImportWorkerTest {
private static IgdbApiService.Game game(long igdbId, String name, String... platforms) {
return new IgdbApiService.Game(igdbId, name, null, null, List.of(platforms), null);
private static IgdbGame game(long igdbId, String name, String... platforms) {
return new IgdbGame(igdbId, name, null, null, List.of(platforms), null);
}
private static GameImport.Line line(String title, String platform) {
@@ -37,12 +37,12 @@ class GameImportWorkerTest {
@Test
void rank_SameNameFirst_ThenTheLinesPlatform_ThenIgdbsOrder_AtMostFive() {
List<IgdbApiService.Game> found = List.of(game(1, "Doom Eternal", "PC"), game(2, "DOOM", "PC"),
List<IgdbGame> found = List.of(game(1, "Doom Eternal", "PC"), game(2, "DOOM", "PC"),
game(3, "Doom", "Switch"), game(4, "Doom 3"), game(5, "Doom 64"), game(6, "Doom II"));
assertThat(GameImportWorker.rank(line("Doom", "switch"), found)).extracting(IgdbApiService.Game::igdbId)
assertThat(GameImportWorker.rank(line("Doom", "switch"), found)).extracting(IgdbGame::igdbId)
.containsExactly(3L, 2L, 1L, 4L, 5L);
assertThat(GameImportWorker.rank(line("Doom", null), found)).extracting(IgdbApiService.Game::igdbId)
assertThat(GameImportWorker.rank(line("Doom", null), found)).extracting(IgdbGame::igdbId)
.startsWith(2L, 3L);
assertThat(GameImportWorker.rank(line("x", null), IntStream.range(0, 9)
.mapToObj(i -> game(i, "Game " + i)).toList())).hasSize(GameImportWorker.MAX_CANDIDATES);
@@ -56,7 +56,7 @@ class GameImportWorkerTest {
.thenReturn(Optional.of(7L), Optional.empty());
when(repository.findStalled(any())).thenReturn(Map.of());
GameImportWorker worker = new GameImportWorker(repository, mock(ManualGameService.class),
mock(IgdbApiService.class), mock(TimeToBeatService.class), mock(MoodService.class), 5);
mock(TimeToBeatService.class), mock(MoodService.class), 5);
try {
worker.wakeMatching();
verify(repository, timeout(2000)).findNext(GameImportWorker.MATCHING_QUEUE);