From 31ca8cc963b62f4f02ad4d81066cfa8c949c854b Mon Sep 17 00:00:00 2001 From: gewuyou <95328647+GeWuYou@users.noreply.github.com> Date: Mon, 20 Apr 2026 09:36:11 +0800 Subject: [PATCH 1/5] =?UTF-8?q?docs(game):=20=E6=BE=84=E6=B8=85=20JsonSeri?= =?UTF-8?q?alizer=20=E9=85=8D=E7=BD=AE=E4=B8=8E=E5=B9=B6=E5=8F=91=E5=A5=91?= =?UTF-8?q?=E7=BA=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 补充 JsonSerializer 对 settings 与 converters 生命周期的 XML 注释 - 更新序列化文档与 README，修正线程安全和组合根配置说明 - 新增 JsonSerializer 配置实例暴露契约测试，并回写 data-repository-persistence 跟踪 --- .../Serializer/JsonSerializerTests.cs | 15 ++++- GFramework.Game/README.md | 4 +- GFramework.Game/Serializer/JsonSerializer.cs | 21 +++++- .../data-repository-persistence-tracking.md | 16 +++-- .../data-repository-persistence-trace.md | 17 +++++ docs/zh-CN/game/index.md | 2 + docs/zh-CN/game/serialization.md | 66 +++++++++++++++++-- 7 files changed, 124 insertions(+), 17 deletions(-) diff --git a/GFramework.Game.Tests/Serializer/JsonSerializerTests.cs b/GFramework.Game.Tests/Serializer/JsonSerializerTests.cs index b6eb1a49..a9f7e962 100644 --- a/GFramework.Game.Tests/Serializer/JsonSerializerTests.cs +++ b/GFramework.Game.Tests/Serializer/JsonSerializerTests.cs @@ -49,6 +49,19 @@ public sealed class JsonSerializerTests }); } + [Test] + public void Settings_And_Converters_Should_Expose_Live_Configuration_Instance() + { + var settings = new JsonSerializerSettings(); + var serializer = new GameJsonSerializer(settings); + + Assert.Multiple(() => + { + Assert.That(serializer.Settings, Is.SameAs(settings)); + Assert.That(serializer.Converters, Is.SameAs(settings.Converters)); + }); + } + [Test] public void Converters_Should_Be_Used_For_Serialization_And_Deserialization() { @@ -174,4 +187,4 @@ public sealed class JsonSerializerTests }; } } -} \ No newline at end of file +} diff --git a/GFramework.Game/README.md b/GFramework.Game/README.md index 9bc7f8ff..2b0cf825 100644 --- a/GFramework.Game/README.md +++ b/GFramework.Game/README.md @@ -180,12 +180,14 @@ using GFramework.Core.Abstractions.Storage; using GFramework.Game.Serializer; using GFramework.Game.Storage; -ISerializer serializer = new JsonSerializer(); +var serializer = new JsonSerializer(); IStorage storage = new FileStorage("GameData", serializer); await storage.WriteAsync("player/profile", new { Name = "Alice", Level = 3 }); ``` +这里的 `JsonSerializer` 建议在组合根只创建并配置一次；如果需要自定义 `JsonSerializerSettings` 或 converters，请在把它注册给 `IStorage`、`DataRepository` 或架构 utility 之前完成。 + 如果你需要逻辑隔离，再包一层 `ScopedStorage`： ```csharp diff --git a/GFramework.Game/Serializer/JsonSerializer.cs b/GFramework.Game/Serializer/JsonSerializer.cs index 3e498840..53c45268 100644 --- a/GFramework.Game/Serializer/JsonSerializer.cs +++ b/GFramework.Game/Serializer/JsonSerializer.cs @@ -4,8 +4,12 @@ using Newtonsoft.Json; namespace GFramework.Game.Serializer; /// -/// JSON序列化器实现类，用于将对象序列化为JSON字符串或将JSON字符串反序列化为对象 +/// 基于 Newtonsoft.Json 的运行时 JSON 序列化器。 /// +/// +/// 该类型会直接持有并复用外部提供的 实例及其转换器集合，而不会在构造时复制配置。 +/// 请在组合根或启动阶段完成全部配置，并在注册给其他组件后将这些配置视为只读；否则在并发调用期间同时修改设置或转换器集合可能导致不可预测行为。 +/// public sealed class JsonSerializer : IRuntimeTypeSerializer { @@ -14,7 +18,10 @@ public sealed class JsonSerializer /// /// 初始化 JSON 序列化器。 /// - /// 可选的 Newtonsoft.Json 配置；不提供时使用默认配置。 + /// + /// 可选的 Newtonsoft.Json 配置实例；不提供时使用默认配置。 + /// 传入的实例会被当前序列化器直接复用，后续对该实例的修改会影响所有后续序列化与反序列化调用。 + /// public JsonSerializer(JsonSerializerSettings? settings = null) { _settings = settings ?? new JsonSerializerSettings(); @@ -23,11 +30,19 @@ public sealed class JsonSerializer /// /// 获取当前序列化器使用的 Newtonsoft.Json 配置实例。 /// + /// + /// 返回的是当前序列化器持有的活动配置实例，适合在启动阶段补充 contract resolver、格式化策略或 converter。 + /// 一旦该序列化器被共享给其他组件，应避免再修改返回值，以免破坏调用方对并发读行为的假设。 + /// public JsonSerializerSettings Settings => _settings; /// /// 获取当前序列化器使用的自定义转换器集合。 /// + /// + /// 该集合与 的 引用相同。 + /// 请在注册序列化器前完成 converter 配置，并避免在序列化器已经发布后继续增删转换器。 + /// public IList Converters => _settings.Converters; /// @@ -115,4 +130,4 @@ public sealed class JsonSerializer return result; } -} \ No newline at end of file +} 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 b0fb361d..a35c7333 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 @@ -13,7 +13,8 @@ - 已将根目录 legacy `local-plan/settings-persistence-serialization-tracking.md` 迁入 `ai-plan/public/data-repository-persistence/` - 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成，并收入主题内 `archive/` - - 下一轮需要继续评估 `JsonSerializer` 配置说明、迁移模型统一抽象与 codec / persistence pipeline 边界 + - 当前正在补齐 `JsonSerializer` 的配置生命周期、只读约束与线程安全说明 + - 下一轮需要继续评估迁移模型统一抽象与 codec / persistence pipeline 边界 ## 当前状态摘要 @@ -26,11 +27,11 @@ - 原 `local-plan` 只有一份混合 tracking 文件，没有独立的 `todos/` 与 `traces/` - 详细历史已拆分迁入主题内 `archive/`，active tracking / trace 只保留当前恢复点、风险与下一步 - 历史已验证结果包括 `GFramework.Game.Tests` 的定向与全量通过，以及 `docs/zh-CN/game/*` 的同步更新 +- `GFramework.Game.Serializer.JsonSerializer` 当前直接暴露活动中的 `JsonSerializerSettings` 与 converters 集合，配置不会被复制 +- `docs/zh-CN/game/serialization.md` 现有“序列化器本身线程安全”表述与源码契约不一致，需要在本轮修正 ## 当前风险 -- 只读配置 / 线程安全说明缺口：`JsonSerializer` 新增 settings 与 converter 扩展后，若不补充约束说明，后续容易被误用 - - 缓解措施：下一轮先核对源码与文档，必要时补 XML docs 或采用文档 - 迁移模型分叉风险：`SettingsModel`、`DataRepository` 与 `SaveRepository` 的版本演进机制仍可能继续分叉 - 缓解措施：在新增更多 persistence feature 前，先评估能否抽出统一的 migration abstraction - Active 入口回膨胀风险：若后续把实现细节继续堆回 active 文档，会重新退化成旧 `local-plan` @@ -45,9 +46,12 @@ - 旧混合 `local-plan` 已拆分迁入主题内 archive - active 跟踪文件已按 `ai-plan` 治理规则精简为当前恢复入口 +- 已补充 `JsonSerializer` XML docs、文档示例与最小契约测试 +- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~JsonSerializerTests"` 已通过（9/9） +- 本次定向验证过程中出现的 analyzer warning 来自仓库既有代码，不属于本轮新增问题 ## 下一步 -1. 先评估 `JsonSerializer` 的只读配置、线程安全与实例级 converter 使用说明是否需要补足 -2. 再评估设置 / 通用仓库 / 存档仓库的迁移模型是否要统一抽象 -3. 最后评估压缩 / 加密 / 元数据策略是否应落入更明确的 codec / persistence pipeline +1. 再评估设置 / 通用仓库 / 存档仓库的迁移模型是否要统一抽象 +2. 最后评估压缩 / 加密 / 元数据策略是否应落入更明确的 codec / persistence pipeline +3. 若进入下一轮实现，先确定是否需要新的 dedicated recovery point 以避免 RP-001 active 入口继续膨胀 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 1f26c3ff..e18a51a2 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 @@ -28,3 +28,20 @@ 1. 后续继续该主题时，只从 `ai-plan/public/data-repository-persistence/` 进入，不再恢复 `local-plan/` 2. 若 active 入口再次积累多轮已完成且已验证阶段，继续按同一模式迁入该主题自己的 `archive/` + +## 2026-04-20 + +### 阶段：JsonSerializer 配置契约补充（RP-001） + +- 复核 `GFramework.Game/Serializer/JsonSerializer.cs` 后确认：当前实现直接复用传入的 `JsonSerializerSettings`，并通过 `Settings` / `Converters` 暴露活动配置对象 +- 复核 `docs/zh-CN/game/serialization.md` 后确认：现有 FAQ 把 `JsonSerializer` 写成“本身线程安全”，与当前可变配置契约不一致 +- 决定本轮只补齐契约说明而不改变运行时行为： + - 在源码 XML docs 中说明 settings / converters 的生命周期与并发约束 + - 在定向单测中固定“序列化器暴露活动配置实例”的当前契约 + - 在 `docs/zh-CN/game/serialization.md`、`docs/zh-CN/game/index.md` 与 `GFramework.Game/README.md` 中同步修正接入建议 + +### 下一步 + +1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~JsonSerializerTests"` 已通过（9/9） +2. 验证过程中出现的 analyzer warning 为仓库既有 warning，未在本轮扩大 +3. 下一步回到 migration abstraction 与 codec / persistence pipeline 的后续评估 diff --git a/docs/zh-CN/game/index.md b/docs/zh-CN/game/index.md index 7beee74b..ef71e1e4 100644 --- a/docs/zh-CN/game/index.md +++ b/docs/zh-CN/game/index.md @@ -683,6 +683,8 @@ public class GameDataSerializer public GameDataSerializer() { + // 在构造阶段完成全部 JsonSerializerSettings / Converter 配置， + // 后续把 _serializer 视为共享只读实例。 _serializer = new JsonSerializer(new JsonSerializerSettings { Formatting = Formatting.Indented, diff --git a/docs/zh-CN/game/serialization.md b/docs/zh-CN/game/serialization.md index b1a827bb..fdc24ba1 100644 --- a/docs/zh-CN/game/serialization.md +++ b/docs/zh-CN/game/serialization.md @@ -55,7 +55,8 @@ public interface IRuntimeTypeSerializer : ISerializer ### JSON 序列化器 -`JsonSerializer` 是基于 Newtonsoft.Json 的实现： +`JsonSerializer` 是基于 Newtonsoft.Json 的实现。它会直接复用构造时提供的 +`JsonSerializerSettings` 与 `Converters` 集合，因此推荐在组合根统一完成配置，再把同一个实例注册到架构或仓库中复用： ```csharp public sealed class JsonSerializer : IRuntimeTypeSerializer @@ -81,8 +82,10 @@ public class GameArchitecture : Architecture { protected override void Init() { - // 注册 JSON 序列化器 + // 在启动阶段一次性完成配置，后续将该实例视为只读 var jsonSerializer = new JsonSerializer(); + jsonSerializer.Converters.Add(new PlayerDataJsonConverter()); + RegisterUtility(jsonSerializer); RegisterUtility(jsonSerializer); } @@ -166,6 +169,40 @@ public void SerializeRuntimeType() } ``` +### 配置生命周期约束 + +`JsonSerializer` 不会复制 `JsonSerializerSettings`。这意味着： + +- 传给构造函数的 settings 会被原样保留 +- `serializer.Settings` 与 `serializer.Converters` 返回的都是活动配置对象 +- 一旦序列化器实例已经注册给其他模块或开始被并发调用，就不应继续修改这些配置 + +推荐模式： + +```csharp +var settings = new JsonSerializerSettings +{ + Formatting = Formatting.Indented, + NullValueHandling = NullValueHandling.Ignore +}; + +settings.Converters.Add(new Vector2JsonConverter()); + +var serializer = new JsonSerializer(settings); + +architecture.RegisterUtility(serializer); +architecture.RegisterUtility(serializer); +``` + +不推荐模式： + +```csharp +var serializer = architecture.GetUtility(); + +// 已经共享给运行时后再修改配置，容易让并发调用得到不稳定行为 +((JsonSerializer)serializer).Converters.Add(new LateBoundConverter()); +``` + ## 高级用法 ### 与存储系统集成 @@ -463,10 +500,11 @@ public PlayerDataV2 LoadWithMigration(string json) ## 最佳实践 -1. **使用接口而非具体类型**：依赖 `ISerializer` 接口 +1. **优先依赖接口，在组合根统一实例化**：业务代码依赖 `ISerializer`，具体 `JsonSerializer` 在启动阶段配置一次即可 ```csharp ✓ var serializer = this.GetUtility(); - ✗ var serializer = new JsonSerializer(); // 避免直接实例化 + ✓ var serializer = new JsonSerializer(settings); // 仅在组合根/启动阶段配置 + ✗ var serializer = new JsonSerializer(); // 避免在业务逻辑中临时创建 ``` 2. **为数据类提供默认值**：确保反序列化的健壮性 @@ -731,10 +769,26 @@ public async Task LoadEncrypted(string key) ### 问题：序列化器是线程安全的吗？ **解答**： -`JsonSerializer` 本身是线程安全的，但建议通过架构的 Utility 系统访问： +`JsonSerializer` 的并发读行为只在“配置不再变化”这个前提下才安全。当前实现会暴露活动中的 +`JsonSerializerSettings` 与 `Converters` 集合，因此： + +- 可以在启动阶段创建并配置一个共享实例 +- 可以在配置冻结后把该实例注册为 Utility 或注入到仓库 +- 不应在序列化器已经被多个调用方使用时继续修改 settings、contract resolver 或 converters + +推荐按下面的方式在启动阶段完成配置，然后只做读操作： ```csharp -// 线程安全的访问方式 +// 启动阶段完成全部配置 +var serializer = new JsonSerializer(new JsonSerializerSettings +{ + NullValueHandling = NullValueHandling.Ignore +}); +serializer.Converters.Add(new GameDataJsonConverter()); + +architecture.RegisterUtility(serializer); + +// 运行阶段只复用，不再修改配置 public async Task ParallelSave() { var tasks = Enumerable.Range(0, 10).Select(async i => From 88de1235ae0a1a8de625b169f3d447d4f7402d06 Mon Sep 17 00:00:00 2001 From: gewuyou <95328647+GeWuYou@users.noreply.github.com> Date: Mon, 20 Apr 2026 09:52:37 +0800 Subject: [PATCH 2/5] =?UTF-8?q?refactor(game):=20=E6=94=B6=E6=95=9B?= =?UTF-8?q?=E7=89=88=E6=9C=AC=E8=BF=81=E7=A7=BB=E9=93=BE=E6=89=A7=E8=A1=8C?= =?UTF-8?q?=E5=99=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 新增 internal 迁移执行器，统一 settings 与 save 的链式版本校验 - 修复 SettingsModel 重复注册、缺链回填与目标版本判定的迁移约束 - 补充 Persistence 与 SettingsModel 定向测试，并更新迁移文档和 ai-plan 跟踪 --- .../Data/PersistenceTests.cs | 52 +++++++ .../Setting/SettingsModelTests.cs | 91 ++++++++++- GFramework.Game/Data/SaveRepository.cs | 76 +++------- .../Internal/VersionedMigrationRunner.cs | 142 ++++++++++++++++++ GFramework.Game/Setting/SettingsModel.cs | 64 ++++++-- .../data-repository-persistence-tracking.md | 19 ++- .../data-repository-persistence-trace.md | 30 ++++ docs/zh-CN/game/data.md | 2 + docs/zh-CN/game/setting.md | 8 + 9 files changed, 410 insertions(+), 74 deletions(-) create mode 100644 GFramework.Game/Internal/VersionedMigrationRunner.cs diff --git a/GFramework.Game.Tests/Data/PersistenceTests.cs b/GFramework.Game.Tests/Data/PersistenceTests.cs index 31192a26..0c92bbe4 100644 --- a/GFramework.Game.Tests/Data/PersistenceTests.cs +++ b/GFramework.Game.Tests/Data/PersistenceTests.cs @@ -188,6 +188,39 @@ public class PersistenceTests Assert.That(exception!.Message, Does.Contain("from version 2")); } + /// + /// 验证迁移器声明的目标版本必须与返回数据上的实际版本一致，避免错误迁移结果被静默接受。 + /// + /// 表示异步测试完成的任务。 + /// 当迁移器返回的版本与声明目标版本不一致时抛出。 + [Test] + public async Task SaveRepository_LoadAsync_Should_Throw_When_Migration_Result_Version_Does_Not_Match_Declaration() + { + 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 + }); + + var repository = new SaveRepository(storage, config) + .RegisterMigration(new TestSaveMigrationV1ToV2ReturningV3()); + + var exception = Assert.ThrowsAsync(async () => await repository.LoadAsync(1)); + Assert.That(exception!.Message, Does.Contain("declared target version 2")); + } + /// /// 验证统一设置仓库能够保存、重新加载并批量读取已注册的设置数据。 /// @@ -707,6 +740,25 @@ public class PersistenceTests } } + private sealed class TestSaveMigrationV1ToV2ReturningV3 : ISaveMigration + { + public int FromVersion => 1; + + public int ToVersion => 2; + + public TestVersionedSaveData Migrate(TestVersionedSaveData oldData) + { + return new TestVersionedSaveData + { + Name = oldData.Name, + Level = oldData.Level, + Experience = oldData.Experience, + Version = 3, + LastModified = oldData.LastModified + }; + } + } + private sealed class TestNonVersionedMigration : ISaveMigration { public int FromVersion => 1; diff --git a/GFramework.Game.Tests/Setting/SettingsModelTests.cs b/GFramework.Game.Tests/Setting/SettingsModelTests.cs index 984c517c..6adc5f1d 100644 --- a/GFramework.Game.Tests/Setting/SettingsModelTests.cs +++ b/GFramework.Game.Tests/Setting/SettingsModelTests.cs @@ -47,6 +47,7 @@ public sealed class SettingsModelTests await model.InitializeAsync(); Assert.That(model.GetData().Version, Is.EqualTo(1)); + model.GetData().Version = 2; model.RegisterMigration(new TestSettingsMigration()); repository.Stored["TestSettingsData"] = new TestSettingsData @@ -65,6 +66,49 @@ public sealed class SettingsModelTests }); } + [Test] + public void RegisterMigration_Should_Reject_Duplicate_FromVersion_For_Same_SettingsType() + { + var locationProvider = new TestDataLocationProvider(); + var repository = new FakeSettingsDataRepository(); + var model = new SettingsModel(locationProvider, repository); + + model.RegisterMigration(new TestSettingsMigration()); + + var exception = Assert.Throws(() => model.RegisterMigration(new TestSettingsMigration())); + + Assert.That(exception!.Message, Does.Contain("Duplicate settings migration registration")); + } + + [Test] + public async Task InitializeAsync_Should_Keep_Current_Instance_When_Migration_Chain_Is_Incomplete() + { + var locationProvider = new TestDataLocationProvider(); + var repository = new FakeSettingsDataRepository(); + var model = new SettingsModel(locationProvider, repository); + ((IContextAware)model).SetContext(new Mock(MockBehavior.Loose).Object); + + _ = model.GetData(); + ((IInitializable)model).Initialize(); + + repository.Stored["TestLatestSettingsData"] = new TestLatestSettingsData + { + Version = 1, + Value = "legacy" + }; + + model.RegisterMigration(new TestLatestSettingsMigrationV1ToV2()); + + await model.InitializeAsync(); + + var current = model.GetData(); + Assert.Multiple(() => + { + Assert.That(current.Version, Is.EqualTo(3)); + Assert.That(current.Value, Is.EqualTo("default-v3")); + }); + } + private sealed class TestSettingsData : ISettingsData { public string Value { get; set; } = "default"; @@ -110,6 +154,51 @@ public sealed class SettingsModelTests } } + private sealed class TestLatestSettingsData : ISettingsData + { + public string Value { get; set; } = "default-v3"; + + public int Version { get; set; } = 3; + + public DateTime LastModified { get; } = DateTime.UtcNow; + + public void Reset() + { + Value = "default-v3"; + Version = 3; + } + + public void LoadFrom(ISettingsData source) + { + if (source is not TestLatestSettingsData data) + { + return; + } + + Value = data.Value; + Version = data.Version; + } + } + + private sealed class TestLatestSettingsMigrationV1ToV2 : ISettingsMigration + { + public Type SettingsType => typeof(TestLatestSettingsData); + + public int FromVersion => 1; + + public int ToVersion => 2; + + public ISettingsSection Migrate(ISettingsSection oldData) + { + var data = (TestLatestSettingsData)oldData; + return new TestLatestSettingsData + { + Version = 2, + Value = $"{data.Value}-migrated" + }; + } + } + private sealed class FakeSettingsDataRepository : ISettingsDataRepository { public Dictionary RegisteredTypes { get; } = new(StringComparer.Ordinal); @@ -178,4 +267,4 @@ public sealed class SettingsModelTests public IReadOnlyDictionary? Metadata => null; } -} \ No newline at end of file +} diff --git a/GFramework.Game/Data/SaveRepository.cs b/GFramework.Game/Data/SaveRepository.cs index e0d440d4..4da134fc 100644 --- a/GFramework.Game/Data/SaveRepository.cs +++ b/GFramework.Game/Data/SaveRepository.cs @@ -19,6 +19,7 @@ using System.Threading.Tasks; using GFramework.Core.Abstractions.Storage; using GFramework.Core.Utility; using GFramework.Game.Abstractions.Data; +using GFramework.Game.Internal; using GFramework.Game.Storage; namespace GFramework.Game.Data; @@ -69,12 +70,12 @@ public class SaveRepository : AbstractContextUtility, ISaveRepository ArgumentNullException.ThrowIfNull(migration); EnsureVersionedSaveType(); - if (migration.ToVersion <= migration.FromVersion) - { - throw new ArgumentException( - $"Migration for {typeof(TSaveData).Name} must advance the version number.", - nameof(migration)); - } + VersionedMigrationRunner.ValidateForwardOnlyRegistration( + typeof(TSaveData).Name, + "Save migration", + migration.FromVersion, + migration.ToVersion, + nameof(migration)); lock (_migrationsLock) { @@ -227,57 +228,24 @@ public class SaveRepository : AbstractContextUtility, ISaveRepository EnsureVersionedSaveType(); - var migrated = data; - // 迁移链按“当前版本 -> 下一个已注册迁移器”推进；任何缺口都表示运行时无法安全解释旧存档。 // 读取迁移表时使用同一把锁，保证并发注册不会让加载线程看到不一致的链路状态。 - while (currentVersion < targetVersion) - { - ISaveMigration? migration; - lock (_migrationsLock) + var migrated = VersionedMigrationRunner.MigrateToTargetVersion( + data, + targetVersion, + static saveData => ((IVersionedData)saveData).Version, + fromVersion => { - _migrations.TryGetValue(currentVersion, out migration); - } - - if (migration is null) - { - throw new InvalidOperationException( - $"No save migration is registered for {typeof(TSaveData).Name} from version {currentVersion}."); - } - - migrated = migration.Migrate(migrated) ?? - throw new InvalidOperationException( - $"Save migration for {typeof(TSaveData).Name} from version {currentVersion} returned null."); - - if (migrated is not IVersionedData migratedVersionedData) - { - throw new InvalidOperationException( - $"Save migration for {typeof(TSaveData).Name} must return data implementing {nameof(IVersionedData)}."); - } - - // 显式校验迁移器声明与实际结果，避免版本号不前进导致死循环或把旧数据错误回写为“已升级”。 - if (migration.ToVersion != migratedVersionedData.Version) - { - throw new InvalidOperationException( - $"Save migration for {typeof(TSaveData).Name} declared target version {migration.ToVersion} " + - $"but returned version {migratedVersionedData.Version}."); - } - - if (migratedVersionedData.Version <= currentVersion) - { - throw new InvalidOperationException( - $"Save migration for {typeof(TSaveData).Name} must advance beyond version {currentVersion}."); - } - - if (migratedVersionedData.Version > targetVersion) - { - throw new InvalidOperationException( - $"Save migration for {typeof(TSaveData).Name} produced version {migratedVersionedData.Version}, " + - $"which exceeds the current runtime version {targetVersion}."); - } - - currentVersion = migratedVersionedData.Version; - } + lock (_migrationsLock) + { + _migrations.TryGetValue(fromVersion, out var migration); + return migration; + } + }, + static migration => migration.ToVersion, + static (migration, currentData) => migration.Migrate(currentData), + $"{typeof(TSaveData).Name} in slot {slot}", + "save migration"); await storage.WriteAsync(_config.SaveFileName, migrated); return migrated; diff --git a/GFramework.Game/Internal/VersionedMigrationRunner.cs b/GFramework.Game/Internal/VersionedMigrationRunner.cs new file mode 100644 index 00000000..d81865cd --- /dev/null +++ b/GFramework.Game/Internal/VersionedMigrationRunner.cs @@ -0,0 +1,142 @@ +// Copyright (c) 2026 GeWuYou +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace GFramework.Game.Internal; + +/// +/// 提供版本化数据迁移链的共享执行逻辑。 +/// +/// +/// 该运行器只负责“按版本号推进”的公共约束，包括： +/// 前向注册校验、缺失链路失败、声明目标版本与实际结果版本一致性，以及避免版本回退或死循环。 +/// 它不关心具体存储、日志、回写或异常吞吐策略；这些由调用方负责。 +/// +internal static class VersionedMigrationRunner +{ + /// + /// 校验迁移注册是否表示一次有效的前向升级。 + /// + /// 迁移所作用的主体名称，例如设置类型或存档类型。 + /// 用于异常消息的迁移类别名称。 + /// 源版本。 + /// 目标版本。 + /// 异常中要使用的参数名。 + /// 目标版本不大于源版本时抛出。 + internal static void ValidateForwardOnlyRegistration( + string subjectName, + string migrationKind, + int fromVersion, + int toVersion, + string paramName) + { + if (toVersion <= fromVersion) + { + throw new ArgumentException( + $"{migrationKind} for {subjectName} must advance the version number.", + paramName); + } + } + + /// + /// 按目标运行时版本执行连续迁移。 + /// + /// 迁移数据类型。 + /// 迁移描述类型。 + /// 原始加载的数据。 + /// 当前运行时支持的目标版本。 + /// 从数据对象提取版本号的委托。 + /// 根据当前版本查找下一步迁移器的委托。 + /// 从迁移器提取声明目标版本的委托。 + /// 执行单步迁移的委托。 + /// 迁移主体名称，用于异常消息。 + /// 迁移类别名称，用于异常消息。 + /// 迁移到目标版本后的数据；如果已经是最新版本，则返回原对象。 + /// + /// 数据版本高于当前运行时、迁移链缺失、迁移器返回 、 + /// 迁移结果版本与声明不一致、版本未前进或超出目标版本时抛出。 + /// + internal static TData MigrateToTargetVersion( + TData data, + int targetVersion, + Func getVersion, + Func resolveMigration, + Func getToVersion, + Func applyMigration, + string subjectName, + string migrationKind) + where TData : class + { + ArgumentNullException.ThrowIfNull(data); + ArgumentNullException.ThrowIfNull(getVersion); + ArgumentNullException.ThrowIfNull(resolveMigration); + ArgumentNullException.ThrowIfNull(getToVersion); + ArgumentNullException.ThrowIfNull(applyMigration); + ArgumentException.ThrowIfNullOrWhiteSpace(subjectName); + ArgumentException.ThrowIfNullOrWhiteSpace(migrationKind); + + var currentVersion = getVersion(data); + if (currentVersion > targetVersion) + { + throw new InvalidOperationException( + $"{subjectName} is version {currentVersion}, which is newer than the current runtime version {targetVersion}."); + } + + if (currentVersion == targetVersion) + { + return data; + } + + var current = data; + + while (currentVersion < targetVersion) + { + var migration = resolveMigration(currentVersion); + if (migration is null) + { + throw new InvalidOperationException( + $"No {migrationKind} is registered for {subjectName} from version {currentVersion}."); + } + + current = applyMigration(migration, current) + ?? throw new InvalidOperationException( + $"{migrationKind} for {subjectName} from version {currentVersion} returned null."); + + var migratedVersion = getVersion(current); + var declaredTargetVersion = getToVersion(migration); + + if (declaredTargetVersion != migratedVersion) + { + throw new InvalidOperationException( + $"{migrationKind} for {subjectName} declared target version {declaredTargetVersion} " + + $"but returned version {migratedVersion}."); + } + + if (migratedVersion <= currentVersion) + { + throw new InvalidOperationException( + $"{migrationKind} for {subjectName} must advance beyond version {currentVersion}."); + } + + if (migratedVersion > targetVersion) + { + throw new InvalidOperationException( + $"{migrationKind} for {subjectName} produced version {migratedVersion}, " + + $"which exceeds the current runtime version {targetVersion}."); + } + + currentVersion = migratedVersion; + } + + return current; + } +} diff --git a/GFramework.Game/Setting/SettingsModel.cs b/GFramework.Game/Setting/SettingsModel.cs index 3a187351..e06bf5f5 100644 --- a/GFramework.Game/Setting/SettingsModel.cs +++ b/GFramework.Game/Setting/SettingsModel.cs @@ -4,6 +4,7 @@ using GFramework.Core.Logging; using GFramework.Core.Model; using GFramework.Game.Abstractions.Data; using GFramework.Game.Abstractions.Setting; +using GFramework.Game.Internal; using GFramework.Game.Setting.Events; namespace GFramework.Game.Setting; @@ -116,7 +117,21 @@ public class SettingsModel(IDataLocationProvider? locationProvider, /// public ISettingsModel RegisterMigration(ISettingsMigration migration) { - _migrations[(migration.SettingsType, migration.FromVersion)] = migration; + ArgumentNullException.ThrowIfNull(migration); + + VersionedMigrationRunner.ValidateForwardOnlyRegistration( + migration.SettingsType.Name, + "Settings migration", + migration.FromVersion, + migration.ToVersion, + nameof(migration)); + + if (!_migrations.TryAdd((migration.SettingsType, migration.FromVersion), migration)) + { + throw new InvalidOperationException( + $"Duplicate settings migration registration for {migration.SettingsType.Name} from version {migration.FromVersion}."); + } + _migrationCache.TryRemove(migration.SettingsType, out _); return this; } @@ -156,7 +171,7 @@ public class SettingsModel(IDataLocationProvider? locationProvider, if (raw is not ISettingsData loaded) continue; - var migrated = MigrateIfNeeded(loaded); + var migrated = MigrateIfNeeded(loaded, data); // 回填（不替换实例） data.LoadFrom(migrated); @@ -277,14 +292,9 @@ public class SettingsModel(IDataLocationProvider? locationProvider, _repository.RegisterDataType(location, type); } - private ISettingsData MigrateIfNeeded(ISettingsData data) + private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData) { - if (data is not IVersionedData versioned) - return data; - var type = data.GetType(); - var current = data; - if (!_migrationCache.TryGetValue(type, out var versionMap)) { versionMap = _migrations @@ -294,12 +304,42 @@ public class SettingsModel(IDataLocationProvider? locationProvider, _migrationCache[type] = versionMap; } - while (versionMap.TryGetValue(versioned.Version, out var migration)) + return VersionedMigrationRunner.MigrateToTargetVersion( + data, + latestData.Version, + static settings => settings.Version, + fromVersion => versionMap.TryGetValue(fromVersion, out var migration) ? migration : null, + static migration => migration.ToVersion, + static (migration, current) => ApplySettingsMigration(migration, current), + $"{type.Name} settings", + "settings migration"); + } + + /// + /// 执行单步设置迁移，并验证迁移结果仍然属于已注册的设置类型。 + /// + /// 要执行的迁移器。 + /// 当前版本的数据。 + /// 迁移后的设置数据。 + /// + /// 迁移结果不实现 ，或返回了与声明设置类型不兼容的数据时抛出。 + /// + private static ISettingsData ApplySettingsMigration(ISettingsMigration migration, ISettingsData currentData) + { + var fromVersion = currentData.Version; + var migrated = migration.Migrate(currentData); + if (migrated is not ISettingsData migratedData) { - current = (ISettingsData)migration.Migrate(current); - versioned = current; + throw new InvalidOperationException( + $"Settings migration for {migration.SettingsType.Name} from version {fromVersion} must return {nameof(ISettingsData)}."); } - return current; + if (!migration.SettingsType.IsInstanceOfType(migratedData)) + { + throw new InvalidOperationException( + $"Settings migration for {migration.SettingsType.Name} from version {fromVersion} returned incompatible data type {migratedData.GetType().Name}."); + } + + return migratedData; } } 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 a35c7333..175e6a88 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 @@ -13,14 +13,15 @@ - 已将根目录 legacy `local-plan/settings-persistence-serialization-tracking.md` 迁入 `ai-plan/public/data-repository-persistence/` - 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成，并收入主题内 `archive/` - - 当前正在补齐 `JsonSerializer` 的配置生命周期、只读约束与线程安全说明 - - 下一轮需要继续评估迁移模型统一抽象与 codec / persistence pipeline 边界 + - 已完成 `SettingsModel` / `SaveRepository` 共享迁移执行器收敛与契约补强 + - 下一轮需要继续评估 codec / persistence pipeline 边界 ## 当前状态摘要 - 高优先级实现、测试与文档对齐已在本主题历史阶段完成，当前 active 入口主要保留后续 design/backlog 恢复点 - 当前分支 `feat/data-repository-persistence` 已在 `ai-plan/public/README.md` 建立 topic 映射 - 旧单文件不再同时承担 todo 与 trace 角色，后续恢复统一从本 topic 的 active tracking / trace 进入 +- `SettingsModel` 与 `SaveRepository` 的版本迁移链现在共用同一个 internal runner；继续沿这条线扩展时应优先复用而不是再复制链式迁移逻辑 ## 当前活跃事实 @@ -28,12 +29,14 @@ - 详细历史已拆分迁入主题内 `archive/`，active tracking / trace 只保留当前恢复点、风险与下一步 - 历史已验证结果包括 `GFramework.Game.Tests` 的定向与全量通过，以及 `docs/zh-CN/game/*` 的同步更新 - `GFramework.Game.Serializer.JsonSerializer` 当前直接暴露活动中的 `JsonSerializerSettings` 与 converters 集合，配置不会被复制 -- `docs/zh-CN/game/serialization.md` 现有“序列化器本身线程安全”表述与源码契约不一致，需要在本轮修正 +- `GFramework.Game.Internal.VersionedMigrationRunner` 已统一前向迁移注册校验、缺链失败、声明版本一致性与非递增防护 +- `SettingsModel` 现在以当前内存设置实例的 `Version` 作为目标运行时版本；若迁移失败则保留当前实例并记录错误日志 +- `SaveRepository` 继续在 `LoadAsync(slot)` 期间迁移并回写，但其核心链式校验已与设置迁移共用同一实现 ## 当前风险 -- 迁移模型分叉风险：`SettingsModel`、`DataRepository` 与 `SaveRepository` 的版本演进机制仍可能继续分叉 - - 缓解措施：在新增更多 persistence feature 前，先评估能否抽出统一的 migration abstraction +- codec / persistence pipeline 边界风险：压缩、加密、元数据与备份策略还散落在仓库与存储语义之间 + - 缓解措施：下一轮先梳理现有 `Serializer` / `Storage` / `Repository` 的责任边界，再决定是否需要新的 pipeline abstraction - Active 入口回膨胀风险：若后续把实现细节继续堆回 active 文档，会重新退化成旧 `local-plan` - 缓解措施：后续阶段完成并验证后，继续迁入本 topic 的 `archive/` @@ -48,10 +51,12 @@ - active 跟踪文件已按 `ai-plan` 治理规则精简为当前恢复入口 - 已补充 `JsonSerializer` XML docs、文档示例与最小契约测试 - `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） - 本次定向验证过程中出现的 analyzer warning 来自仓库既有代码，不属于本轮新增问题 ## 下一步 -1. 再评估设置 / 通用仓库 / 存档仓库的迁移模型是否要统一抽象 -2. 最后评估压缩 / 加密 / 元数据策略是否应落入更明确的 codec / persistence pipeline +1. 评估压缩 / 加密 / 元数据策略是否应落入更明确的 codec / persistence pipeline +2. 梳理 `Serializer`、`Storage`、`DataRepositoryOptions` 与统一文件仓库之间的扩展点重叠 3. 若进入下一轮实现，先确定是否需要新的 dedicated recovery point 以避免 RP-001 active 入口继续膨胀 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 e18a51a2..b68337ef 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 @@ -45,3 +45,33 @@ 1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~JsonSerializerTests"` 已通过（9/9） 2. 验证过程中出现的 analyzer warning 为仓库既有 warning，未在本轮扩大 3. 下一步回到 migration abstraction 与 codec / persistence pipeline 的后续评估 + +### 阶段：迁移执行器统一收敛（RP-001） + +- 对 `SettingsModel`、`DataRepository`、`UnifiedSettingsDataRepository`、`SaveRepository` 的实现进行并排核对后确认： + - `DataRepository` 与 `UnifiedSettingsDataRepository` 不直接承担按版本号推进的迁移链 + - 实际重复点只在 `SettingsModel` 与 `SaveRepository` 的“版本迁移链执行与校验”逻辑 +- 决定不新增 public migration abstraction，而是抽出 internal `VersionedMigrationRunner` + - 统一前向注册校验 + - 统一缺链失败 + - 统一声明目标版本与实际结果版本一致性校验 + - 统一非递增 / 超目标版本防护 +- `SettingsModel` 本轮额外补强： + - 拒绝同一设置类型同一 `FromVersion` 的重复注册 + - 以当前内存设置实例的 `Version` 作为目标运行时版本 + - 迁移失败时保持当前实例不被旧数据覆盖，并继续记录错误日志 +- `SaveRepository` 改为复用同一个 internal runner，但保留“加载成功后自动回写升级结果”的现有仓库语义 +- 同步更新 `docs/zh-CN/game/setting.md` 与 `docs/zh-CN/game/data.md`，补迁移链约束说明 +- 新增 / 更新测试： + - `SettingsModelTests`：重复注册拒绝、不完整链路保持当前实例、缓存失效场景 + - `PersistenceTests`：迁移结果版本与声明版本不一致时显式失败 + +### 验证 + +1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests"` 已通过（20/20） +2. 过程中出现的 analyzer warning 来自仓库既有项，未在本轮扩大 + +### 下一步 + +1. 进入 codec / persistence pipeline 边界评估 +2. 重点查看压缩、加密、元数据、备份是否仍然跨越 `Serializer` / `Storage` / `Repository` 多层分散 diff --git a/docs/zh-CN/game/data.md b/docs/zh-CN/game/data.md index df65b706..b2e3fb71 100644 --- a/docs/zh-CN/game/data.md +++ b/docs/zh-CN/game/data.md @@ -299,7 +299,9 @@ public partial class AutoSaveController : IController - `TSaveData` 需要实现 `IVersionedData` - 仓库以 `new TSaveData().Version` 作为当前运行时目标版本 - 每个迁移器负责一个 `FromVersion -> ToVersion` 跳转 +- 同一个 `FromVersion` 只能注册一个迁移器，且 `ToVersion` 必须严格大于 `FromVersion` - 加载时仓库会按链路连续执行迁移，并在成功后自动回写升级后的存档 +- 迁移器返回数据上的 `Version` 必须与声明的 `ToVersion` 一致 - 如果缺少中间迁移器，或者读到了比当前运行时更高的版本，`LoadAsync` 会抛出异常，避免静默加载错误数据 ```csharp diff --git a/docs/zh-CN/game/setting.md b/docs/zh-CN/game/setting.md index 619146db..67f998e8 100644 --- a/docs/zh-CN/game/setting.md +++ b/docs/zh-CN/game/setting.md @@ -181,6 +181,14 @@ public interface ISettingsMigration 当 `InitializeAsync()` 读取到旧版本设置时，会按已注册迁移链逐步升级，再通过 `LoadFrom` 回填到当前实例。 +迁移规则如下： + +- 同一个设置类型的同一个 `FromVersion` 只能注册一个迁移器 +- `ToVersion` 必须严格大于 `FromVersion` +- `InitializeAsync()` 会以当前运行时代码里该设置实例的 `Version` 作为目标版本 +- 如果迁移链缺口、迁移结果类型不兼容、迁移结果版本与声明不一致，或者读取到比当前运行时更高的版本，当前设置节不会覆盖内存中的最新实例，并会记录错误日志 +- 与 `SaveRepository` 不同，设置初始化阶段会跳过失败的设置节并继续处理其他设置节，而不是把异常继续向外抛出 + ## 依赖项 要让设置系统完整工作，通常需要准备： 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 3/5] =?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 From a0cc418e0534fba72cb9226d638efbd3e0e42fbc Mon Sep 17 00:00:00 2001 From: GeWuYou <95328647+GeWuYou@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:46:08 +0800 Subject: [PATCH 4/5] =?UTF-8?q?docs(ai-plan):=20=E4=BF=AE=E6=AD=A3=20PR=20?= =?UTF-8?q?=E8=AF=84=E5=AE=A1=E5=90=8E=E7=9A=84=E6=81=A2=E5=A4=8D=E6=96=87?= =?UTF-8?q?=E6=A1=A3=E7=8A=B6=E6=80=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 更新 tracking 文档中的 PR #260 review follow-up 状态描述 - 修复 trace 文档重复的三级标题，避免 Markdown 锚点冲突 --- .../todos/data-repository-persistence-tracking.md | 4 ++-- .../traces/data-repository-persistence-trace.md | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) 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 ad7c246e..84858d2c 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 @@ -33,8 +33,8 @@ - `GFramework.Game.Internal.VersionedMigrationRunner` 已统一前向迁移注册校验、缺链失败、声明版本一致性与非递增防护 - `SettingsModel` 现在以当前内存设置实例的 `Version` 作为目标运行时版本；若迁移失败则保留当前实例并记录错误日志 - `SaveRepository` 继续在 `LoadAsync(slot)` 期间迁移并回写，但其核心链式校验已与设置迁移共用同一实现 -- PR #260 最新 review 仍要求补齐 `VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约，并确保 - `SaveRepository` 单次加载不会在并发注册期间读取到变化中的迁移链 +- PR #260 review follow-up 已完成：`VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约已补齐， + `SaveRepository` 单次加载已切换为迁移表快照，避免并发注册期间读取变化中的迁移链 - `docs/zh-CN/game/index.md` 当前仍承担最低接入示例，因此其中的 `JsonSerializer` 配置必须避免鼓励对 用户可篡改存档启用不受限的多态反序列化 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 c595bf42..bb2dec07 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 @@ -24,7 +24,7 @@ - 历史 trace 归档： - `ai-plan/public/data-repository-persistence/archive/traces/data-repository-persistence-history-pre-rp001.md` -### 下一步 +### 下一步：JsonSerializer 配置契约补充 1. 后续继续该主题时，只从 `ai-plan/public/data-repository-persistence/` 进入，不再恢复 `local-plan/` 2. 若 active 入口再次积累多轮已完成且已验证阶段，继续按同一模式迁入该主题自己的 `archive/` @@ -40,7 +40,7 @@ - 在定向单测中固定“序列化器暴露活动配置实例”的当前契约 - 在 `docs/zh-CN/game/serialization.md`、`docs/zh-CN/game/index.md` 与 `GFramework.Game/README.md` 中同步修正接入建议 -### 下一步 +### 下一步：JsonSerializer 配置契约补充 1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~JsonSerializerTests"` 已通过（9/9） 2. 验证过程中出现的 analyzer warning 为仓库既有 warning，未在本轮扩大 @@ -66,12 +66,12 @@ - `SettingsModelTests`：重复注册拒绝、不完整链路保持当前实例、缓存失效场景 - `PersistenceTests`：迁移结果版本与声明版本不一致时显式失败 -### 验证 +### 验证：迁移执行器统一收敛 1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests"` 已通过（20/20） 2. 过程中出现的 analyzer warning 来自仓库既有项，未在本轮扩大 -### 下一步 +### 下一步：迁移执行器统一收敛 1. 进入 codec / persistence pipeline 边界评估 2. 重点查看压缩、加密、元数据、备份是否仍然跨越 `Serializer` / `Storage` / `Repository` 多层分散 @@ -98,12 +98,12 @@ - `PersistenceTests` 已新增“迁移失败不污染持久化数据”断言，以及并发注册下固定迁移快照的回归测试 - `docs/zh-CN/game/index.md` 的 `JsonSerializer` 接入示例已改为 `TypeNameHandling.None`，并补充白名单 binder 说明 -### 验证 +### 验证：PR #260 review follow-up 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 仍为仓库既有项 -### 下一步 +### 下一步：PR #260 review follow-up 1. 回到 codec / persistence pipeline 边界评估 2. 继续判断压缩、加密、元数据与备份策略是否需要新的 dedicated pipeline abstraction From 5353d5bd45bbcfc55139740d5491421b791a83b2 Mon Sep 17 00:00:00 2001 From: GeWuYou <95328647+GeWuYou@users.noreply.github.com> Date: Mon, 20 Apr 2026 13:02:49 +0800 Subject: [PATCH 5/5] =?UTF-8?q?fix(game):=20=E4=BF=AE=E5=A4=8D=E8=AE=BE?= =?UTF-8?q?=E7=BD=AE=E8=BF=81=E7=A7=BB=E7=BC=93=E5=AD=98=E5=B9=B6=E5=8F=91?= =?UTF-8?q?=E4=B8=80=E8=87=B4=E6=80=A7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 修复 SettingsModel 迁移注册与缓存重建的并发竞争 - 新增 SettingsModel 并发回归测试并更新 ai-plan 跟踪 --- .../Setting/SettingsModelTests.cs | 81 +++++++++++++++++++ GFramework.Game/Setting/SettingsModel.cs | 39 ++++++--- .../data-repository-persistence-tracking.md | 7 +- .../data-repository-persistence-trace.md | 21 +++++ 4 files changed, 137 insertions(+), 11 deletions(-) diff --git a/GFramework.Game.Tests/Setting/SettingsModelTests.cs b/GFramework.Game.Tests/Setting/SettingsModelTests.cs index 6adc5f1d..8f7fa047 100644 --- a/GFramework.Game.Tests/Setting/SettingsModelTests.cs +++ b/GFramework.Game.Tests/Setting/SettingsModelTests.cs @@ -1,3 +1,5 @@ +using System.Reflection; +using System.Threading; using GFramework.Core.Abstractions.Architectures; using GFramework.Core.Abstractions.Lifecycle; using GFramework.Core.Abstractions.Rule; @@ -109,6 +111,66 @@ public sealed class SettingsModelTests }); } + [Test] + public async Task RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache() + { + var locationProvider = new TestDataLocationProvider(); + var repository = new FakeSettingsDataRepository(); + var model = new SettingsModel(locationProvider, repository); + ((IContextAware)model).SetContext(new Mock(MockBehavior.Loose).Object); + + _ = model.GetData(); + ((IInitializable)model).Initialize(); + + model.RegisterMigration(new TestLatestSettingsMigrationV1ToV2()); + + repository.Stored["TestLatestSettingsData"] = new TestLatestSettingsData + { + Version = 1, + Value = "legacy" + }; + + var lockField = typeof(SettingsModel) + .GetField("_migrationMapLock", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(lockField, Is.Not.Null); + + var migrationMapLock = lockField!.GetValue(model); + Assert.That(migrationMapLock, Is.Not.Null); + + Task initializeTask; + Task registerTask; + lock (migrationMapLock!) + { + initializeTask = Task.Run(() => model.InitializeAsync()); + registerTask = Task.Run(() => model.RegisterMigration(new TestLatestSettingsMigrationV2ToV3())); + + Thread.Sleep(50); + + Assert.Multiple(() => + { + Assert.That(initializeTask.IsCompleted, Is.False); + Assert.That(registerTask.IsCompleted, Is.False); + }); + } + + await Task.WhenAll(initializeTask, registerTask); + + repository.Stored["TestLatestSettingsData"] = new TestLatestSettingsData + { + Version = 1, + Value = "legacy" + }; + + await model.InitializeAsync(); + + var current = model.GetData(); + Assert.Multiple(() => + { + Assert.That(current.Version, Is.EqualTo(3)); + Assert.That(current.Value, Is.EqualTo("legacy-migrated-v3")); + }); + } + private sealed class TestSettingsData : ISettingsData { public string Value { get; set; } = "default"; @@ -199,6 +261,25 @@ public sealed class SettingsModelTests } } + private sealed class TestLatestSettingsMigrationV2ToV3 : ISettingsMigration + { + public Type SettingsType => typeof(TestLatestSettingsData); + + public int FromVersion => 2; + + public int ToVersion => 3; + + public ISettingsSection Migrate(ISettingsSection oldData) + { + var data = (TestLatestSettingsData)oldData; + return new TestLatestSettingsData + { + Version = 3, + Value = $"{data.Value}-v3" + }; + } + } + private sealed class FakeSettingsDataRepository : ISettingsDataRepository { public Dictionary RegisteredTypes { get; } = new(StringComparer.Ordinal); diff --git a/GFramework.Game/Setting/SettingsModel.cs b/GFramework.Game/Setting/SettingsModel.cs index 7d4efd39..4e735b7d 100644 --- a/GFramework.Game/Setting/SettingsModel.cs +++ b/GFramework.Game/Setting/SettingsModel.cs @@ -29,6 +29,7 @@ public class SettingsModel(IDataLocationProvider? locationProvider, private readonly ConcurrentDictionary _data = new(); private readonly ConcurrentDictionary> _migrationCache = new(); + private readonly object _migrationMapLock = new(); private readonly ConcurrentDictionary<(Type type, int from), ISettingsMigration> _migrations = new(); private volatile bool _initialized; @@ -124,6 +125,10 @@ public class SettingsModel(IDataLocationProvider? locationProvider, /// /// 同一设置类型与源版本已经注册过迁移器时抛出。 /// + /// + /// 迁移注册表与按类型缓存的版本映射需要保持一致；因此注册与 cache miss 时的缓存重建 + /// 统一通过 串行化，避免并发加载把旧快照重新写回缓存。 + /// public ISettingsModel RegisterMigration(ISettingsMigration migration) { ArgumentNullException.ThrowIfNull(migration); @@ -135,13 +140,17 @@ public class SettingsModel(IDataLocationProvider? locationProvider, migration.ToVersion, nameof(migration)); - if (!_migrations.TryAdd((migration.SettingsType, migration.FromVersion), migration)) + lock (_migrationMapLock) { - throw new InvalidOperationException( - $"Duplicate settings migration registration for {migration.SettingsType.Name} from version {migration.FromVersion}."); + if (!_migrations.TryAdd((migration.SettingsType, migration.FromVersion), migration)) + { + throw new InvalidOperationException( + $"Duplicate settings migration registration for {migration.SettingsType.Name} from version {migration.FromVersion}."); + } + + _migrationCache.TryRemove(migration.SettingsType, out _); } - _migrationCache.TryRemove(migration.SettingsType, out _); return this; } @@ -310,18 +319,28 @@ public class SettingsModel(IDataLocationProvider? locationProvider, /// /// 该方法按设置类型缓存迁移表，并始终以 的版本作为目标运行时版本， /// 避免把旧文件中的版本号误当成当前版本。具体的缺链、版本一致性与前进性校验都委托给 - /// 统一处理。 + /// 统一处理。缓存重建与迁移注册共用 + /// ，确保运行中的初始化不会把过期迁移快照写回缓存。 /// private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData) { var type = data.GetType(); - if (!_migrationCache.TryGetValue(type, out var versionMap)) + Dictionary versionMap; + lock (_migrationMapLock) { - versionMap = _migrations - .Where(kv => kv.Key.type == type) - .ToDictionary(kv => kv.Key.from, kv => kv.Value); + if (!_migrationCache.TryGetValue(type, out var cachedVersionMap)) + { + // cache miss 与 RegisterMigration 共用同一把锁，避免注册新迁移后又被旧快照覆盖回缓存。 + versionMap = _migrations + .Where(kv => kv.Key.type == type) + .ToDictionary(kv => kv.Key.from, kv => kv.Value); - _migrationCache[type] = versionMap; + _migrationCache[type] = versionMap; + } + else + { + versionMap = cachedVersionMap; + } } return VersionedMigrationRunner.MigrateToTargetVersion( 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 84858d2c..d82bde4c 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,7 +14,7 @@ `ai-plan/public/data-repository-persistence/` - 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成，并收入主题内 `archive/` - 已完成 `SettingsModel` / `SaveRepository` 共享迁移执行器收敛与契约补强 - - 已完成 PR #260 的 review follow-up：迁移链快照一致性、XML docs 补齐与文档安全示例修正 + - 已完成 PR #260 的追加 review follow-up：`SettingsModel` 迁移缓存并发一致性 - 下一轮需要继续评估 codec / persistence pipeline 边界 ## 当前状态摘要 @@ -35,6 +35,8 @@ - `SaveRepository` 继续在 `LoadAsync(slot)` 期间迁移并回写，但其核心链式校验已与设置迁移共用同一实现 - PR #260 review follow-up 已完成：`VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约已补齐， `SaveRepository` 单次加载已切换为迁移表快照，避免并发注册期间读取变化中的迁移链 +- `SettingsModel` 现已通过 `_migrationMapLock` 串行化迁移注册与 cache miss 时的按类型缓存重建， + 避免并发注册把旧快照重新写回 `_migrationCache` - `docs/zh-CN/game/index.md` 当前仍承担最低接入示例，因此其中的 `JsonSerializer` 配置必须避免鼓励对 用户可篡改存档启用不受限的多态反序列化 @@ -61,6 +63,9 @@ - 已完成 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） +- 已新增 `SettingsModelTests` 并发回归测试，锁定迁移注册与 cache miss 重建不会留下 stale cache +- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests" -m:1 -nodeReuse:false` + 已通过（5/5） - 本次定向验证过程中出现的 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 bb2dec07..e251c429 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 @@ -107,3 +107,24 @@ 1. 回到 codec / persistence pipeline 边界评估 2. 继续判断压缩、加密、元数据与备份策略是否需要新的 dedicated pipeline abstraction + +### 阶段：SettingsModel 迁移缓存并发修复（RP-001） + +- 重新使用 `$gframework-pr-review` 复核 PR #260 后确认：此前遗漏了一条仍然 open 的 `SettingsModel.cs` major comment， + 问题点不是迁移执行本身，而是 `_migrations` 与 `_migrationCache` 在并发注册和 cache miss 重建交错时，可能把旧快照写回缓存 +- 确认该 comment 来自 `2026-04-20T04:23:09Z` 的 CodeRabbit review run；当前修复策略采用同一把私有锁串行化： + - `RegisterMigration` 中的 `_migrations.TryAdd(...)` 与 `_migrationCache.TryRemove(...)` + - `MigrateIfNeeded` 在 cache miss 时按类型重建 `versionMap` 并写回 `_migrationCache` +- 同步补充源码注释与 XML remarks，明确运行时注册与缓存重建共享同一并发语义 +- 计划新增 `SettingsModelTests` 回归测试，验证 cache rebuild 与运行时注册在同一把锁前排队后，后续初始化能看到新增迁移而不会留下 stale cache + +### 验证：SettingsModel 迁移缓存并发修复 + +1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests" -m:1 -nodeReuse:false` 已通过（5/5） +2. 本轮验证中未再出现 `SettingsModel` 新增的 nullable warning；输出中的 analyzer warning 仍为仓库既有项加上新的 `MA0158` + 建议，后者来自本轮新增对象锁 + +### 下一步：SettingsModel 迁移缓存并发修复 + +1. 若继续收口 analyzer 反馈，可评估是否将 `_migrationMapLock` 升级为 `System.Threading.Lock`，同时保留可验证的并发回归测试策略 +2. 否则恢复到 codec / persistence pipeline 边界评估