-
Notifications
You must be signed in to change notification settings - Fork 20
fix(userdata): resolve profile deactivation file conflict #393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5cb962b
cc49116
1065382
7ca7c9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Text.Json; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using GenHub.Core.Constants; | ||
|
|
@@ -10,6 +11,7 @@ | |
| using GenHub.Core.Interfaces.Workspace; | ||
| using GenHub.Core.Models.Enums; | ||
| using GenHub.Core.Models.Manifest; | ||
| using GenHub.Core.Models.UserData; | ||
| using GenHub.Features.UserData.Services; | ||
| using Microsoft.Extensions.Logging; | ||
| using Moq; | ||
|
|
@@ -824,4 +826,230 @@ public async Task InstallUserDataAsync_WhenBackupFails_AbortsInstallationToPreve | |
| Assert.Contains("Failed to create safety backup", result.FirstError, StringComparison.OrdinalIgnoreCase); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verifies that when a profile is deactivated, another profile can install the same user data files without encountering a conflict. | ||
| /// </summary> | ||
| /// <returns>A task representing the asynchronous test.</returns> | ||
| [Fact] | ||
| public async Task InstallUserDataAsync_WhenPriorOwnerProfileIsDeactivated_SucceedsWithoutConflictAsync() | ||
| { | ||
| // Arrange | ||
| var files = new List<ManifestFile> | ||
| { | ||
| new() | ||
| { | ||
| RelativePath = "Maps/Arabia v2/AdrianeMapSettings.ini", | ||
| Hash = "hash-map-settings", | ||
| Size = 500, | ||
| InstallTarget = ContentInstallTarget.UserDataDirectory, | ||
| }, | ||
| }; | ||
|
|
||
| // 1. Profile A installs the map pack | ||
| var installA = await _trackerService.InstallUserDataAsync( | ||
| "mappack-id", | ||
| "profile-a", | ||
| GameType.ZeroHour, | ||
| files, | ||
| "1.0", | ||
| "Map Pack", | ||
| CancellationToken.None); | ||
|
|
||
| Assert.True(installA.Success); | ||
|
|
||
| // 2. Profile A is deactivated | ||
| var deactivateA = await _trackerService.DeactivateProfileUserDataAsync("profile-a", CancellationToken.None); | ||
| Assert.True(deactivateA.Success); | ||
|
|
||
| // 3. Profile B installs the same map pack | ||
| var installB = await _trackerService.InstallUserDataAsync( | ||
| "mappack-id", | ||
| "profile-b", | ||
| GameType.ZeroHour, | ||
| files, | ||
| "1.0", | ||
| "Map Pack", | ||
| CancellationToken.None); | ||
|
|
||
| // Assert: Installation succeeds for profile B and ownership transfers | ||
| Assert.True(installB.Success); | ||
|
undead2146 marked this conversation as resolved.
|
||
|
|
||
| var targetPath = Path.Combine(_zeroHourDataDir, "Maps", "Arabia v2", "AdrianeMapSettings.ini"); | ||
|
undead2146 marked this conversation as resolved.
|
||
| Assert.True(File.Exists(targetPath)); | ||
|
|
||
| var conflictResult = await _trackerService.CheckFileConflictAsync(targetPath, CancellationToken.None); | ||
| Assert.True(conflictResult.Success); | ||
| Assert.Equal("mappack-id_profile-b", conflictResult.Data); | ||
|
|
||
| var indexPath = Path.Combine(_appDataDir, DirectoryNames.UserData, FileTypes.UserDataIndexFileName); | ||
| var indexJson = await File.ReadAllTextAsync(indexPath); | ||
| var index = JsonSerializer.Deserialize<UserDataIndex>(indexJson); | ||
| Assert.NotNull(index); | ||
| Assert.True(index.FileToInstallationMap.TryGetValue(Path.GetFullPath(targetPath), out var ownerKey)); | ||
| Assert.Equal("mappack-id_profile-b", ownerKey); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verifies that cleaning up an uninstalled or old profile does not delete files or prune mappings owned by a newer active profile. | ||
| /// </summary> | ||
| /// <returns>A task representing the asynchronous test.</returns> | ||
| [Fact] | ||
| public async Task CleanupProfileAsync_WhenPriorOwnerProfileCleanedUpAfterTransfer_PreservesNewOwnerFilesAndIndexMappingAsync() | ||
| { | ||
| // Arrange | ||
| var files = new List<ManifestFile> | ||
| { | ||
| new() | ||
| { | ||
| RelativePath = "Maps/TransferCheck/map.ini", | ||
| Hash = "hash-transfer-test", | ||
| Size = 300, | ||
| InstallTarget = ContentInstallTarget.UserDataDirectory, | ||
| }, | ||
| }; | ||
|
|
||
| // 1. Profile A installs the map pack | ||
| var installA = await _trackerService.InstallUserDataAsync( | ||
| "transfer-manifest", | ||
| "profile-a", | ||
| GameType.ZeroHour, | ||
| files, | ||
| "1.0", | ||
| "Transfer Test", | ||
| CancellationToken.None); | ||
| Assert.True(installA.Success); | ||
|
|
||
| // 2. Profile A is deactivated | ||
| var deactivateA = await _trackerService.DeactivateProfileUserDataAsync("profile-a", CancellationToken.None); | ||
| Assert.True(deactivateA.Success); | ||
|
|
||
| // 3. Profile B installs the same map pack | ||
| var installB = await _trackerService.InstallUserDataAsync( | ||
| "transfer-manifest", | ||
| "profile-b", | ||
| GameType.ZeroHour, | ||
| files, | ||
| "1.0", | ||
| "Transfer Test", | ||
| CancellationToken.None); | ||
| Assert.True(installB.Success); | ||
|
|
||
| var targetPath = Path.Combine(_zeroHourDataDir, "Maps", "TransferCheck", "map.ini"); | ||
| Assert.True(File.Exists(targetPath)); | ||
|
|
||
| // 4. Profile A is cleaned up | ||
| var cleanupA = await _trackerService.CleanupProfileAsync("profile-a", CancellationToken.None); | ||
| Assert.True(cleanupA.Success); | ||
|
|
||
| // Assert: Profile B's file and index mapping remain intact | ||
| Assert.True(File.Exists(targetPath)); | ||
|
|
||
| var conflictResult = await _trackerService.CheckFileConflictAsync(targetPath, CancellationToken.None); | ||
| Assert.True(conflictResult.Success); | ||
| Assert.Equal("transfer-manifest_profile-b", conflictResult.Data); | ||
|
|
||
| var indexPath = Path.Combine(_appDataDir, DirectoryNames.UserData, FileTypes.UserDataIndexFileName); | ||
| var indexJson = await File.ReadAllTextAsync(indexPath); | ||
| var index = JsonSerializer.Deserialize<UserDataIndex>(indexJson); | ||
| Assert.NotNull(index); | ||
| Assert.True(index.FileToInstallationMap.TryGetValue(Path.GetFullPath(targetPath), out var ownerKey)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SUGGESTION]: The new transfer test does not assert that the cleaned-up profile's tracking was actually removed. The test verifies the file mapping still points to Add a check such as: Assert.DoesNotContain("transfer-manifest_profile-a", index.InstallationKeys);
Assert.False(index.ProfileInstallations.ContainsKey("profile-a") ||
!index.ProfileInstallations["profile-a"].Contains("transfer-manifest_profile-a"));Reply with |
||
| Assert.Equal("transfer-manifest_profile-b", ownerKey); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verifies that when a file is temporarily missing on disk but its manifest is active, conflict checking still reports conflict. | ||
| /// </summary> | ||
| /// <returns>A task representing the asynchronous test.</returns> | ||
| [Fact] | ||
| public async Task CheckFileConflictAsync_WhenFileMissingOnDiskButManifestActive_ReportsConflictAsync() | ||
| { | ||
| // Arrange | ||
| var files = new List<ManifestFile> | ||
| { | ||
| new() | ||
| { | ||
| RelativePath = "Maps/TempMissing/map.ini", | ||
| Hash = "hash-missing-test", | ||
| Size = 100, | ||
| InstallTarget = ContentInstallTarget.UserDataDirectory, | ||
| }, | ||
| }; | ||
|
|
||
| var installResult = await _trackerService.InstallUserDataAsync( | ||
| "missing-test-manifest", | ||
| "profile-missing-test", | ||
| GameType.ZeroHour, | ||
| files, | ||
| "1.0", | ||
| "Missing Test", | ||
| CancellationToken.None); | ||
|
|
||
| Assert.True(installResult.Success); | ||
|
|
||
| var targetPath = Path.Combine(_zeroHourDataDir, "Maps", "TempMissing", "map.ini"); | ||
| Assert.True(File.Exists(targetPath)); | ||
|
|
||
| // Temporarily delete the file from disk | ||
| File.Delete(targetPath); | ||
| Assert.False(File.Exists(targetPath)); | ||
|
|
||
| // Act | ||
| var conflictResult = await _trackerService.CheckFileConflictAsync(targetPath, CancellationToken.None); | ||
|
|
||
| // Assert: Conflict is still reported because the owning manifest is active | ||
| Assert.True(conflictResult.Success); | ||
| Assert.Equal("missing-test-manifest_profile-missing-test", conflictResult.Data); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verifies that when a manifest is deactivated, CheckFileConflictAsync prunes the stale mapping and reports no conflict. | ||
| /// </summary> | ||
| /// <returns>A task representing the asynchronous test.</returns> | ||
| [Fact] | ||
| public async Task CheckFileConflictAsync_WhenManifestDeactivated_PrunesStaleMappingAndReturnsNoConflictAsync() | ||
| { | ||
| // Arrange | ||
| var files = new List<ManifestFile> | ||
| { | ||
| new() | ||
| { | ||
| RelativePath = "Maps/DeactivatedCheck/map.ini", | ||
| Hash = "hash-deact-test", | ||
| Size = 100, | ||
| InstallTarget = ContentInstallTarget.UserDataDirectory, | ||
| }, | ||
| }; | ||
|
|
||
| var installResult = await _trackerService.InstallUserDataAsync( | ||
| "deact-test-manifest", | ||
| "profile-deact-test", | ||
| GameType.ZeroHour, | ||
| files, | ||
| "1.0", | ||
| "Deact Test", | ||
| CancellationToken.None); | ||
|
|
||
| Assert.True(installResult.Success); | ||
|
|
||
| var targetPath = Path.Combine(_zeroHourDataDir, "Maps", "DeactivatedCheck", "map.ini"); | ||
|
|
||
| // Deactivate the profile | ||
| var deactivateResult = await _trackerService.DeactivateProfileUserDataAsync("profile-deact-test", CancellationToken.None); | ||
| Assert.True(deactivateResult.Success); | ||
|
|
||
| // Act | ||
| var conflictResult = await _trackerService.CheckFileConflictAsync(targetPath, CancellationToken.None); | ||
|
|
||
| // Assert: No conflict reported and stale mapping is pruned | ||
| Assert.True(conflictResult.Success); | ||
| Assert.Null(conflictResult.Data); | ||
|
undead2146 marked this conversation as resolved.
|
||
|
|
||
| // Verify index file persisted on disk no longer maps the path | ||
| var indexPath = Path.Combine(_appDataDir, DirectoryNames.UserData, FileTypes.UserDataIndexFileName); | ||
| var indexJson = await File.ReadAllTextAsync(indexPath); | ||
| var index = JsonSerializer.Deserialize<UserDataIndex>(indexJson); | ||
| Assert.NotNull(index); | ||
| Assert.False(index.FileToInstallationMap.ContainsKey(Path.GetFullPath(targetPath))); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.