From e485ad1865dc641429c892a00ba54b620bb4cb5c Mon Sep 17 00:00:00 2001 From: Pat Hartl Date: Sat, 27 Jun 2026 18:38:42 -0500 Subject: [PATCH] Fix reconciliation of library games based on user libraries enabled/disabled Ref #423 --- .../Tests/ImportServiceTests.cs | 81 +++++++++++++++++-- .../ImportService.cs | 47 ++++++++--- 2 files changed, 111 insertions(+), 17 deletions(-) diff --git a/LANCommander.Launcher.Services.Tests/Tests/ImportServiceTests.cs b/LANCommander.Launcher.Services.Tests/Tests/ImportServiceTests.cs index 40a22cb2..1ff9637f 100644 --- a/LANCommander.Launcher.Services.Tests/Tests/ImportServiceTests.cs +++ b/LANCommander.Launcher.Services.Tests/Tests/ImportServiceTests.cs @@ -71,6 +71,15 @@ public class ImportServiceTests await context.SaveChangesAsync(); } + private static async Task SeedCachedGamesAsync(DatabaseContext context, params Game[] games) + { + // Games that exist in the local database but are not associated with any library, mirroring + // records left behind after they were dropped from the library on a previous import. + context.Games!.AddRange(games); + + await context.SaveChangesAsync(); + } + private static async Task> GetLibraryGameIdsAsync(DatabaseContext context, Guid userId) { context.ChangeTracker.Clear(); @@ -83,7 +92,7 @@ public class ImportServiceTests } [Fact] - public async Task ReconcileRemovedGames_removes_local_games_missing_from_remote_library() + public async Task ReconcileLibraryMembership_removes_local_games_missing_from_remote_library() { var userId = Guid.NewGuid(); var keep = GameFactory.Make("Half-Life"); @@ -92,14 +101,14 @@ public class ImportServiceTests await using var context = CreateContext(); await SeedLibraryAsync(context, userId, keep, stale); - await CreateSubject(context, userId).ReconcileRemovedGamesAsync([keep.Id]); + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([keep.Id]); var remaining = await GetLibraryGameIdsAsync(context, userId); remaining.ShouldBe([keep.Id]); } [Fact] - public async Task ReconcileRemovedGames_keeps_games_still_present_remotely() + public async Task ReconcileLibraryMembership_keeps_games_still_present_remotely() { var userId = Guid.NewGuid(); var first = GameFactory.Make("Half-Life"); @@ -108,14 +117,14 @@ public class ImportServiceTests await using var context = CreateContext(); await SeedLibraryAsync(context, userId, first, second); - await CreateSubject(context, userId).ReconcileRemovedGamesAsync([first.Id, second.Id]); + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([first.Id, second.Id]); var remaining = await GetLibraryGameIdsAsync(context, userId); remaining.ShouldBe([first.Id, second.Id], ignoreOrder: true); } [Fact] - public async Task ReconcileRemovedGames_skips_when_remote_library_is_empty() + public async Task ReconcileLibraryMembership_skips_when_remote_library_is_empty() { // An empty remote list is ambiguous with a server-side failure, so the local // library must be left untouched rather than wiped. @@ -125,14 +134,14 @@ public class ImportServiceTests await using var context = CreateContext(); await SeedLibraryAsync(context, userId, game); - await CreateSubject(context, userId).ReconcileRemovedGamesAsync([]); + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([]); var remaining = await GetLibraryGameIdsAsync(context, userId); remaining.ShouldBe([game.Id]); } [Fact] - public async Task ReconcileRemovedGames_does_not_touch_other_users_libraries() + public async Task ReconcileLibraryMembership_does_not_touch_other_users_libraries() { var userId = Guid.NewGuid(); var otherUserId = Guid.NewGuid(); @@ -145,9 +154,65 @@ public class ImportServiceTests // Remote library for the current user no longer contains any games it shares with the // other user, but the other user's library must remain intact. - await CreateSubject(context, userId).ReconcileRemovedGamesAsync([ownGame.Id]); + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([ownGame.Id]); var otherRemaining = await GetLibraryGameIdsAsync(context, otherUserId); otherRemaining.ShouldBe([otherGame.Id]); } + + [Fact] + public async Task ReconcileLibraryMembership_adds_cached_games_missing_from_library() + { + // Reproduces toggling "Enable User Libraries" off: the game was dropped from the library on + // a previous import but its record is still cached, so a subsequent full library must + // re-associate it instead of leaving it hidden. + var userId = Guid.NewGuid(); + var inLibrary = GameFactory.Make("Half-Life"); + var cachedOnly = GameFactory.Make("Quake III Arena"); + + await using var context = CreateContext(); + await SeedLibraryAsync(context, userId, inLibrary); + await SeedCachedGamesAsync(context, cachedOnly); + + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([inLibrary.Id, cachedOnly.Id]); + + var remaining = await GetLibraryGameIdsAsync(context, userId); + remaining.ShouldBe([inLibrary.Id, cachedOnly.Id], ignoreOrder: true); + } + + [Fact] + public async Task ReconcileLibraryMembership_adds_and_removes_in_a_single_pass() + { + var userId = Guid.NewGuid(); + var keep = GameFactory.Make("Half-Life"); + var stale = GameFactory.Make("Removed From Depot"); + var cachedOnly = GameFactory.Make("Quake III Arena"); + + await using var context = CreateContext(); + await SeedLibraryAsync(context, userId, keep, stale); + await SeedCachedGamesAsync(context, cachedOnly); + + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([keep.Id, cachedOnly.Id]); + + var remaining = await GetLibraryGameIdsAsync(context, userId); + remaining.ShouldBe([keep.Id, cachedOnly.Id], ignoreOrder: true); + } + + [Fact] + public async Task ReconcileLibraryMembership_ignores_remote_games_not_cached_locally() + { + // A remote game whose record has not been imported yet cannot be added to the library here; + // reconciliation must simply leave it out rather than fail. + var userId = Guid.NewGuid(); + var inLibrary = GameFactory.Make("Half-Life"); + var notCachedRemoteId = Guid.NewGuid(); + + await using var context = CreateContext(); + await SeedLibraryAsync(context, userId, inLibrary); + + await CreateSubject(context, userId).ReconcileLibraryMembershipAsync([inLibrary.Id, notCachedRemoteId]); + + var remaining = await GetLibraryGameIdsAsync(context, userId); + remaining.ShouldBe([inLibrary.Id]); + } } diff --git a/LANCommander.Launcher.Services/ImportService.cs b/LANCommander.Launcher.Services/ImportService.cs index 11f517f0..e5f659dc 100644 --- a/LANCommander.Launcher.Services/ImportService.cs +++ b/LANCommander.Launcher.Services/ImportService.cs @@ -97,14 +97,14 @@ namespace LANCommander.Launcher.Services await importContext.ImportQueueAsync(); await importContext.DownloadPendingMediaAsync(); - // Remove games from the local library that are no longer in the remote library - await ReconcileRemovedGamesAsync(remoteLibrary.Select(g => g.Id).ToList()); + // Make the local library membership match the remote library exactly + await ReconcileLibraryMembershipAsync(remoteLibrary.Select(g => g.Id).ToList()); // Sync play sessions for all library games await SyncPlaySessionsAsync(remoteLibrary.Select(g => g.Id)); } - internal async Task ReconcileRemovedGamesAsync(IReadOnlyCollection remoteGameIds) + internal async Task ReconcileLibraryMembershipAsync(IReadOnlyCollection remoteGameIds) { // The library endpoint returns an empty list both when the library is genuinely // empty and when it fails server-side. Treating an empty result as authoritative @@ -125,19 +125,48 @@ namespace LANCommander.Launcher.Services if (library == null) return; - var staleGames = library.Games - .Where(g => !remoteGameIds.Contains(g.Id)) - .ToList(); + var remoteGameIdSet = remoteGameIds.ToHashSet(); - if (staleGames.Count == 0) - return; + // Remove games that are no longer present in the remote library. + var staleGames = library.Games + .Where(g => !remoteGameIdSet.Contains(g.Id)) + .ToList(); foreach (var staleGame in staleGames) library.Games.Remove(staleGame); + // Add games that belong in the remote library and already exist locally but are + // missing from the local library. The import above skips games whose cached records + // are unchanged, so toggling "Enable User Libraries" off on the server (which makes + // the endpoint return every game) would otherwise leave previously-dropped games + // hidden from the library view. + var localGameIds = library.Games + .Select(g => g.Id) + .ToHashSet(); + + var missingGameIds = remoteGameIdSet + .Where(id => !localGameIds.Contains(id)) + .ToList(); + + var gamesToAdd = missingGameIds.Count == 0 + ? new List() + : await dbContext.Games + .Where(g => missingGameIds.Contains(g.Id)) + .ToListAsync(); + + foreach (var game in gamesToAdd) + library.Games.Add(game); + + if (staleGames.Count == 0 && gamesToAdd.Count == 0) + return; + await dbContext.SaveChangesAsync(); - Logger?.LogInformation("Removed {Count} game(s) from the local library that are no longer present on the server", staleGames.Count); + if (staleGames.Count > 0) + Logger?.LogInformation("Removed {Count} game(s) from the local library that are no longer present on the server", staleGames.Count); + + if (gamesToAdd.Count > 0) + Logger?.LogInformation("Added {Count} game(s) to the local library that were already cached locally", gamesToAdd.Count); } public async Task ImportGameAsync(Guid gameId)