From ec3de5bbb0d9b2deee86b96868e71e4b0c800c80 Mon Sep 17 00:00:00 2001 From: GeWuYou <95328647+GeWuYou@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:44:27 +0800 Subject: [PATCH] =?UTF-8?q?fix(game):=20=E4=BF=AE=E5=A4=8D=20PR=20?= =?UTF-8?q?=E8=AF=84=E5=AE=A1=E9=81=97=E7=95=99=E7=9A=84=E8=BF=81=E7=A7=BB?= =?UTF-8?q?=E4=B8=8E=E6=96=87=E6=A1=A3=E9=97=AE=E9=A2=98?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 修复 SaveRepository 迁移链并发读取，改为单次快照执行 - 补充 VersionedMigrationRunner 与 SettingsModel 的 XML 文档契约 - 更新 PersistenceTests、接入文档与 ai-plan 跟踪记录 --- .../Data/PersistenceTests.cs | 92 ++++++++++++++++++- GFramework.Game/Data/SaveRepository.cs | 21 ++--- .../Internal/VersionedMigrationRunner.cs | 7 ++ GFramework.Game/Setting/SettingsModel.cs | 20 ++++ .../data-repository-persistence-tracking.md | 8 ++ .../data-repository-persistence-trace.md | 32 +++++++ docs/zh-CN/game/index.md | 5 +- 7 files changed, 172 insertions(+), 13 deletions(-) diff --git a/GFramework.Game.Tests/Data/PersistenceTests.cs b/GFramework.Game.Tests/Data/PersistenceTests.cs index 0c92bbe4..8813076b 100644 --- a/GFramework.Game.Tests/Data/PersistenceTests.cs +++ b/GFramework.Game.Tests/Data/PersistenceTests.cs @@ -1,4 +1,5 @@ using System.IO; +using System.Threading; using GFramework.Core.Abstractions.Events; using GFramework.Core.Abstractions.Rule; using GFramework.Core.Abstractions.Storage; @@ -218,7 +219,68 @@ public class PersistenceTests .RegisterMigration(new TestSaveMigrationV1ToV2ReturningV3()); var exception = Assert.ThrowsAsync(async () => await repository.LoadAsync(1)); - Assert.That(exception!.Message, Does.Contain("declared target version 2")); + var persisted = await storage.ReadAsync("saves/slot_1/save"); + + Assert.Multiple(() => + { + Assert.That(exception!.Message, Does.Contain("declared target version 2")); + Assert.That(persisted.Version, Is.EqualTo(1)); + Assert.That(persisted.Name, Is.EqualTo("legacy")); + Assert.That(persisted.Level, Is.EqualTo(3)); + Assert.That(persisted.Experience, Is.EqualTo(0)); + }); + } + + /// + /// 验证加载流程会在开始迁移前固定迁移表快照，避免并发注册让同一次加载看到变化中的链路。 + /// + /// 表示异步测试完成的任务。 + /// 当快照中缺少后续迁移链时抛出。 + [Test] + public async Task SaveRepository_LoadAsync_Should_Use_Migration_Snapshot_When_Registrations_Change_Concurrently() + { + var root = CreateTempRoot(); + using var storage = new FileStorage(root, new JsonSerializer()); + var config = new SaveConfiguration + { + SaveRoot = "saves", + SaveSlotPrefix = "slot_", + SaveFileName = "save" + }; + + var writer = new SaveRepository(storage, config); + await writer.SaveAsync(1, new TestVersionedSaveData + { + Name = "legacy", + Level = 3, + Experience = 0, + Version = 1 + }); + + using var migrationStarted = new ManualResetEventSlim(false); + using var continueMigration = new ManualResetEventSlim(false); + + var repository = new SaveRepository(storage, config) + .RegisterMigration(new BlockingSaveMigrationV1ToV2(migrationStarted, continueMigration)); + + var loadTask = repository.LoadAsync(1); + + Assert.That(migrationStarted.Wait(TimeSpan.FromSeconds(5)), Is.True, "First migration step did not start in time."); + + repository.RegisterMigration(new TestSaveMigrationV2ToV3()); + continueMigration.Set(); + + var exception = Assert.ThrowsAsync(async () => await loadTask); + var persisted = await storage.ReadAsync("saves/slot_1/save"); + + Assert.Multiple(() => + { + Assert.That(exception!.Message, Does.Contain("from version 2")); + Assert.That(persisted.Version, Is.EqualTo(1)); + Assert.That(persisted.Name, Is.EqualTo("legacy")); + Assert.That(persisted.Level, Is.EqualTo(3)); + Assert.That(persisted.Experience, Is.EqualTo(0)); + }); } /// @@ -702,6 +764,34 @@ public class PersistenceTests } } + private sealed class BlockingSaveMigrationV1ToV2( + ManualResetEventSlim migrationStarted, + ManualResetEventSlim continueMigration) : ISaveMigration + { + public int FromVersion => 1; + + public int ToVersion => 2; + + public TestVersionedSaveData Migrate(TestVersionedSaveData oldData) + { + migrationStarted.Set(); + + if (!continueMigration.Wait(TimeSpan.FromSeconds(5))) + { + throw new InvalidOperationException("Timed out while waiting to continue the save migration test."); + } + + return new TestVersionedSaveData + { + Name = $"{oldData.Name}-v2", + Level = oldData.Level, + Experience = oldData.Level * 100, + Version = 2, + LastModified = oldData.LastModified + }; + } + } + private sealed class TestSaveMigrationV2ToV3 : ISaveMigration { public int FromVersion => 2; diff --git a/GFramework.Game/Data/SaveRepository.cs b/GFramework.Game/Data/SaveRepository.cs index 4da134fc..f237468d 100644 --- a/GFramework.Game/Data/SaveRepository.cs +++ b/GFramework.Game/Data/SaveRepository.cs @@ -62,8 +62,8 @@ public class SaveRepository : AbstractContextUtility, ISaveRepository /// /// 迁移器的目标版本不大于源版本。 /// - /// 迁移注册表是可变共享状态。注册与加载可以并发发生，因此所有访问都通过 - /// 串行化，避免读写竞争和“部分可见”的迁移链。 + /// 迁移注册表是可变共享状态。注册路径通过 串行化； + /// 加载路径会在同一把锁下复制一次快照，保证单次加载始终使用同一个迁移链视图。 /// public ISaveRepository RegisterMigration(ISaveMigration migration) { @@ -228,20 +228,19 @@ public class SaveRepository : AbstractContextUtility, ISaveRepository EnsureVersionedSaveType(); + Dictionary> migrationsSnapshot; + lock (_migrationsLock) + { + migrationsSnapshot = new Dictionary>(_migrations); + } + // 迁移链按“当前版本 -> 下一个已注册迁移器”推进；任何缺口都表示运行时无法安全解释旧存档。 - // 读取迁移表时使用同一把锁，保证并发注册不会让加载线程看到不一致的链路状态。 + // 这里先对迁移表拍快照，避免并发注册让同一次加载在不同步骤看到不同版本的链路。 var migrated = VersionedMigrationRunner.MigrateToTargetVersion( data, targetVersion, static saveData => ((IVersionedData)saveData).Version, - fromVersion => - { - lock (_migrationsLock) - { - _migrations.TryGetValue(fromVersion, out var migration); - return migration; - } - }, + fromVersion => migrationsSnapshot.TryGetValue(fromVersion, out var migration) ? migration : null, static migration => migration.ToVersion, static (migration, currentData) => migration.Migrate(currentData), $"{typeof(TSaveData).Name} in slot {slot}", diff --git a/GFramework.Game/Internal/VersionedMigrationRunner.cs b/GFramework.Game/Internal/VersionedMigrationRunner.cs index d81865cd..9c628cc2 100644 --- a/GFramework.Game/Internal/VersionedMigrationRunner.cs +++ b/GFramework.Game/Internal/VersionedMigrationRunner.cs @@ -61,6 +61,13 @@ internal static class VersionedMigrationRunner /// 迁移主体名称，用于异常消息。 /// 迁移类别名称，用于异常消息。 /// 迁移到目标版本后的数据；如果已经是最新版本，则返回原对象。 + /// + /// 、、、 + /// 或 为 时抛出。 + /// + /// + /// 或 为空白时抛出。 + /// /// /// 数据版本高于当前运行时、迁移链缺失、迁移器返回 、 /// 迁移结果版本与声明不一致、版本未前进或超出目标版本时抛出。 diff --git a/GFramework.Game/Setting/SettingsModel.cs b/GFramework.Game/Setting/SettingsModel.cs index e06bf5f5..7d4efd39 100644 --- a/GFramework.Game/Setting/SettingsModel.cs +++ b/GFramework.Game/Setting/SettingsModel.cs @@ -115,6 +115,15 @@ public class SettingsModel(IDataLocationProvider? locationProvider, /// /// 返回当前 ISettingsModel 实例，支持链式调用。 /// + /// + /// 为 时抛出。 + /// + /// + /// 迁移声明的目标版本不大于源版本时抛出。 + /// + /// + /// 同一设置类型与源版本已经注册过迁移器时抛出。 + /// public ISettingsModel RegisterMigration(ISettingsMigration migration) { ArgumentNullException.ThrowIfNull(migration); @@ -292,6 +301,17 @@ public class SettingsModel(IDataLocationProvider? locationProvider, _repository.RegisterDataType(location, type); } + /// + /// 将已加载的设置数据迁移到当前运行时实例声明的目标版本。 + /// + /// 从仓库读取的设置数据。 + /// 当前内存中的设置实例，其 Version 值代表目标版本。 + /// 迁移后的设置数据；如果无需迁移则返回原对象。 + /// + /// 该方法按设置类型缓存迁移表，并始终以 的版本作为目标运行时版本， + /// 避免把旧文件中的版本号误当成当前版本。具体的缺链、版本一致性与前进性校验都委托给 + /// 统一处理。 + /// private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData) { var type = data.GetType(); diff --git a/ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md b/ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md index 175e6a88..ad7c246e 100644 --- a/ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md +++ b/ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md @@ -14,6 +14,7 @@ `ai-plan/public/data-repository-persistence/` - 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成，并收入主题内 `archive/` - 已完成 `SettingsModel` / `SaveRepository` 共享迁移执行器收敛与契约补强 + - 已完成 PR #260 的 review follow-up：迁移链快照一致性、XML docs 补齐与文档安全示例修正 - 下一轮需要继续评估 codec / persistence pipeline 边界 ## 当前状态摘要 @@ -32,6 +33,10 @@ - `GFramework.Game.Internal.VersionedMigrationRunner` 已统一前向迁移注册校验、缺链失败、声明版本一致性与非递增防护 - `SettingsModel` 现在以当前内存设置实例的 `Version` 作为目标运行时版本；若迁移失败则保留当前实例并记录错误日志 - `SaveRepository` 继续在 `LoadAsync(slot)` 期间迁移并回写，但其核心链式校验已与设置迁移共用同一实现 +- PR #260 最新 review 仍要求补齐 `VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约，并确保 + `SaveRepository` 单次加载不会在并发注册期间读取到变化中的迁移链 +- `docs/zh-CN/game/index.md` 当前仍承担最低接入示例，因此其中的 `JsonSerializer` 配置必须避免鼓励对 + 用户可篡改存档启用不受限的多态反序列化 ## 当前风险 @@ -53,6 +58,9 @@ - `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~JsonSerializerTests"` 已通过（9/9） - 已完成 `VersionedMigrationRunner` 抽取，并让 `SettingsModel` / `SaveRepository` 共用链式迁移校验 - `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests"` 已通过（20/20） +- 已完成 PR #260 follow-up，并新增定向回归测试锁定迁移快照与失败不污染持久化数据的约束 +- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests" -m:1 -nodeReuse:false` + 已通过（21/21） - 本次定向验证过程中出现的 analyzer warning 来自仓库既有代码，不属于本轮新增问题 ## 下一步 diff --git a/ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md b/ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md index b68337ef..c595bf42 100644 --- a/ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md +++ b/ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md @@ -75,3 +75,35 @@ 1. 进入 codec / persistence pipeline 边界评估 2. 重点查看压缩、加密、元数据、备份是否仍然跨越 `Serializer` / `Storage` / `Repository` 多层分散 + +### 阶段：PR #260 review follow-up（RP-001） + +- 复核当前 PR review 后确认两条未解决 inline 线程仍成立： + - `SaveRepository.MigrateIfNeededAsync` 在每一步迁移时都现查 `_migrations`，会让并发 `RegisterMigration` + 把同一次加载暴露给变化中的迁移链 + - `VersionedMigrationRunner.MigrateToTargetVersion` 的 XML docs 仍缺少参数校验异常契约 +- 同步接受两条 outside-diff / nitpick 中仍然成立且低成本的 follow-up： + - `SettingsModel.RegisterMigration` 与 `MigrateIfNeeded` 需要补齐 XML 文档，和当前迁移约束保持一致 + - `PersistenceTests` 需要锁定“迁移失败后不会污染已持久化存档”的行为 +- 额外复核 `docs/zh-CN/game/index.md` 后确认：最低接入示例仍把 `TypeNameHandling.Auto` 用在用户可编辑的存档场景， + 这与当前仓库安全约束不一致，因此一并改为默认安全配置并补充白名单说明 +- 本轮实现计划： + - `SaveRepository` 在加载前复制迁移表快照，再把 resolver 切换到快照读取 + - 新增并发回归测试，证明加载过程不会在迁移途中读到后续注册的链路 + - 补齐 `VersionedMigrationRunner` / `SettingsModel` XML docs + - 更新 `docs/zh-CN/game/index.md` 示例与 active tracking +- 实际落地结果： + - `SaveRepository` 已切换为在加载前复制 `_migrations` 快照，并在同一次迁移链执行中只读取快照 + - `VersionedMigrationRunner`、`SettingsModel.RegisterMigration` 与 `SettingsModel.MigrateIfNeeded` 已补齐缺失 XML docs + - `PersistenceTests` 已新增“迁移失败不污染持久化数据”断言，以及并发注册下固定迁移快照的回归测试 + - `docs/zh-CN/game/index.md` 的 `JsonSerializer` 接入示例已改为 `TypeNameHandling.None`，并补充白名单 binder 说明 + +### 验证 + +1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests" -m:1 -nodeReuse:false` 已通过（21/21） +2. 本次验证未再出现本轮新增的 XML doc warning；输出中的 analyzer warning 仍为仓库既有项 + +### 下一步 + +1. 回到 codec / persistence pipeline 边界评估 +2. 继续判断压缩、加密、元数据与备份策略是否需要新的 dedicated pipeline abstraction diff --git a/docs/zh-CN/game/index.md b/docs/zh-CN/game/index.md index ef71e1e4..37d8949a 100644 --- a/docs/zh-CN/game/index.md +++ b/docs/zh-CN/game/index.md @@ -690,7 +690,7 @@ public class GameDataSerializer Formatting = Formatting.Indented, NullValueHandling = NullValueHandling.Ignore, DefaultValueHandling = DefaultValueHandling.Populate, - TypeNameHandling = TypeNameHandling.Auto + TypeNameHandling = TypeNameHandling.None }); // 自定义转换器 @@ -729,6 +729,9 @@ public class GameDataSerializer } ``` +对于玩家可直接编辑的存档文件，默认应保持 `TypeNameHandling.None`。只有确实需要多态反序列化时，才应配合 +白名单 `SerializationBinder` 显式限制允许的类型集合。 + ### 自定义 JSON 转换器 ```csharp