mirror of
https://github.com/GeWuYou/GFramework.git
synced 2026-05-07 00:39:00 +08:00
fix(game): 修复 PR 评审遗留的迁移与文档问题
- 修复 SaveRepository 迁移链并发读取,改为单次快照执行 - 补充 VersionedMigrationRunner 与 SettingsModel 的 XML 文档契约 - 更新 PersistenceTests、接入文档与 ai-plan 跟踪记录
This commit is contained in:
parent
88de1235ae
commit
ec3de5bbb0
@ -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<InvalidOperationException>(async () => await repository.LoadAsync(1));
|
||||
Assert.That(exception!.Message, Does.Contain("declared target version 2"));
|
||||
var persisted = await storage.ReadAsync<TestVersionedSaveData>("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));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 验证加载流程会在开始迁移前固定迁移表快照,避免并发注册让同一次加载看到变化中的链路。
|
||||
/// </summary>
|
||||
/// <returns>表示异步测试完成的任务。</returns>
|
||||
/// <exception cref="InvalidOperationException">当快照中缺少后续迁移链时抛出。</exception>
|
||||
[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<TestVersionedSaveData>(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<TestVersionedSaveData>(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<InvalidOperationException>(async () => await loadTask);
|
||||
var persisted = await storage.ReadAsync<TestVersionedSaveData>("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));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@ -702,6 +764,34 @@ public class PersistenceTests
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class BlockingSaveMigrationV1ToV2(
|
||||
ManualResetEventSlim migrationStarted,
|
||||
ManualResetEventSlim continueMigration) : ISaveMigration<TestVersionedSaveData>
|
||||
{
|
||||
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<TestVersionedSaveData>
|
||||
{
|
||||
public int FromVersion => 2;
|
||||
|
||||
@ -62,8 +62,8 @@ public class SaveRepository<TSaveData> : AbstractContextUtility, ISaveRepository
|
||||
/// </exception>
|
||||
/// <exception cref="ArgumentException">迁移器的目标版本不大于源版本。</exception>
|
||||
/// <remarks>
|
||||
/// 迁移注册表是可变共享状态。注册与加载可以并发发生,因此所有访问都通过 <see cref="_migrationsLock" />
|
||||
/// 串行化,避免读写竞争和“部分可见”的迁移链。
|
||||
/// 迁移注册表是可变共享状态。注册路径通过 <see cref="_migrationsLock" /> 串行化;
|
||||
/// 加载路径会在同一把锁下复制一次快照,保证单次加载始终使用同一个迁移链视图。
|
||||
/// </remarks>
|
||||
public ISaveRepository<TSaveData> RegisterMigration(ISaveMigration<TSaveData> migration)
|
||||
{
|
||||
@ -228,20 +228,19 @@ public class SaveRepository<TSaveData> : AbstractContextUtility, ISaveRepository
|
||||
|
||||
EnsureVersionedSaveType();
|
||||
|
||||
Dictionary<int, ISaveMigration<TSaveData>> migrationsSnapshot;
|
||||
lock (_migrationsLock)
|
||||
{
|
||||
migrationsSnapshot = new Dictionary<int, ISaveMigration<TSaveData>>(_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}",
|
||||
|
||||
@ -61,6 +61,13 @@ internal static class VersionedMigrationRunner
|
||||
/// <param name="subjectName">迁移主体名称,用于异常消息。</param>
|
||||
/// <param name="migrationKind">迁移类别名称,用于异常消息。</param>
|
||||
/// <returns>迁移到目标版本后的数据;如果已经是最新版本,则返回原对象。</returns>
|
||||
/// <exception cref="ArgumentNullException">
|
||||
/// <paramref name="data" />、<paramref name="getVersion" />、<paramref name="resolveMigration" />、
|
||||
/// <paramref name="getToVersion" /> 或 <paramref name="applyMigration" /> 为 <see langword="null" /> 时抛出。
|
||||
/// </exception>
|
||||
/// <exception cref="ArgumentException">
|
||||
/// <paramref name="subjectName" /> 或 <paramref name="migrationKind" /> 为空白时抛出。
|
||||
/// </exception>
|
||||
/// <exception cref="InvalidOperationException">
|
||||
/// 数据版本高于当前运行时、迁移链缺失、迁移器返回 <see langword="null" />、
|
||||
/// 迁移结果版本与声明不一致、版本未前进或超出目标版本时抛出。
|
||||
|
||||
@ -115,6 +115,15 @@ public class SettingsModel<TRepository>(IDataLocationProvider? locationProvider,
|
||||
/// <returns>
|
||||
/// 返回当前 ISettingsModel 实例,支持链式调用。
|
||||
/// </returns>
|
||||
/// <exception cref="ArgumentNullException">
|
||||
/// <paramref name="migration" /> 为 <see langword="null" /> 时抛出。
|
||||
/// </exception>
|
||||
/// <exception cref="ArgumentException">
|
||||
/// 迁移声明的目标版本不大于源版本时抛出。
|
||||
/// </exception>
|
||||
/// <exception cref="InvalidOperationException">
|
||||
/// 同一设置类型与源版本已经注册过迁移器时抛出。
|
||||
/// </exception>
|
||||
public ISettingsModel RegisterMigration(ISettingsMigration migration)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(migration);
|
||||
@ -292,6 +301,17 @@ public class SettingsModel<TRepository>(IDataLocationProvider? locationProvider,
|
||||
_repository.RegisterDataType(location, type);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 将已加载的设置数据迁移到当前运行时实例声明的目标版本。
|
||||
/// </summary>
|
||||
/// <param name="data">从仓库读取的设置数据。</param>
|
||||
/// <param name="latestData">当前内存中的设置实例,其 <c>Version</c> 值代表目标版本。</param>
|
||||
/// <returns>迁移后的设置数据;如果无需迁移则返回原对象。</returns>
|
||||
/// <remarks>
|
||||
/// 该方法按设置类型缓存迁移表,并始终以 <paramref name="latestData" /> 的版本作为目标运行时版本,
|
||||
/// 避免把旧文件中的版本号误当成当前版本。具体的缺链、版本一致性与前进性校验都委托给
|
||||
/// <see cref="VersionedMigrationRunner" /> 统一处理。
|
||||
/// </remarks>
|
||||
private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData)
|
||||
{
|
||||
var type = data.GetType();
|
||||
|
||||
@ -14,6 +14,7 @@
|
||||
`ai-plan/public/data-repository-persistence/`
|
||||
- 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成,并收入主题内 `archive/`
|
||||
- 已完成 `SettingsModel` / `SaveRepository<T>` 共享迁移执行器收敛与契约补强
|
||||
- 已完成 PR #260 的 review follow-up:迁移链快照一致性、XML docs 补齐与文档安全示例修正
|
||||
- 下一轮需要继续评估 codec / persistence pipeline 边界
|
||||
|
||||
## 当前状态摘要
|
||||
@ -32,6 +33,10 @@
|
||||
- `GFramework.Game.Internal.VersionedMigrationRunner` 已统一前向迁移注册校验、缺链失败、声明版本一致性与非递增防护
|
||||
- `SettingsModel` 现在以当前内存设置实例的 `Version` 作为目标运行时版本;若迁移失败则保留当前实例并记录错误日志
|
||||
- `SaveRepository<T>` 继续在 `LoadAsync(slot)` 期间迁移并回写,但其核心链式校验已与设置迁移共用同一实现
|
||||
- PR #260 最新 review 仍要求补齐 `VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约,并确保
|
||||
`SaveRepository<T>` 单次加载不会在并发注册期间读取到变化中的迁移链
|
||||
- `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<T>` 共用链式迁移校验
|
||||
- `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 来自仓库既有代码,不属于本轮新增问题
|
||||
|
||||
## 下一步
|
||||
|
||||
@ -75,3 +75,35 @@
|
||||
|
||||
1. 进入 codec / persistence pipeline 边界评估
|
||||
2. 重点查看压缩、加密、元数据、备份是否仍然跨越 `Serializer` / `Storage` / `Repository` 多层分散
|
||||
|
||||
### 阶段:PR #260 review follow-up(RP-001)
|
||||
|
||||
- 复核当前 PR review 后确认两条未解决 inline 线程仍成立:
|
||||
- `SaveRepository<T>.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<T>` 在加载前复制迁移表快照,再把 resolver 切换到快照读取
|
||||
- 新增并发回归测试,证明加载过程不会在迁移途中读到后续注册的链路
|
||||
- 补齐 `VersionedMigrationRunner` / `SettingsModel` XML docs
|
||||
- 更新 `docs/zh-CN/game/index.md` 示例与 active tracking
|
||||
- 实际落地结果:
|
||||
- `SaveRepository<T>` 已切换为在加载前复制 `_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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user