diff --git a/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs b/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs index eeec3349..76706417 100644 --- a/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs +++ b/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs @@ -363,6 +363,7 @@ public sealed class DirectoryMessagePackBackingStore : CacheBackingStore currentManifest.FormatVersion, holdsIds.Values.Select(holds => (bool?)holds).ToArray(), copiesAny: currentIndex.Values.Any(record => !written.Contains(record.Key)), + copiedHoldIds: false, // only C# writes a directory store, and it marks by content directoryStore: true, holdsVariants: false); WriteManifest(targetCatalog, catalogBytes, currentIndex.Values diff --git a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md index e1d5d0c0..009fa696 100644 --- a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md +++ b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md @@ -423,8 +423,17 @@ elements retargets nothing. the record's next write, or `CultCache.MintElementIds()`. - A store that holds an id declares the `v3` header (above), so a reader older than ids refuses it instead of skipping the id slot and rewriting the elements without ids. - TypeScript, Python and Rust read `v3` and keep a `v3` header they found when they - rewrite a store; they do not decide when a document carries ids. + A C# write also marks the store when it carries forward a record it read holding an + id, as long as the file still holds that record at the version it read. TypeScript, + Python and Rust read `v3` and keep a `v3` header they found when they rewrite a + store; they carry ids and do not decide when a document carries them. +- An object list's elements are written by their `[Key]` layout, because the id is a + keyed member of the element. A list whose element type is not keyed + (`[MessagePackObject]` or `[Union]`) is a value list and is replaced whole. For an + object list, C# refuses at registration a concrete element type that is not keyed and + `[MessagePackFormatter]` on any type or member on the way to the list or on the id + member, and refuses at the first encode or decode of the document a formatter + resolver that claims one of those types. ## Canonical Semantic Schema Hash diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index 7f164e4c..4fca998b 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -3545,10 +3545,12 @@ namespace GameCult.Caching // The header a write leaves, for either store kind: HeaderFor decides. A record's bytes do not say whether it holds an // element id (an id is an ordinary keyed string), so only the store header carries that fact. A write that copies any // record forward keeps a marked header; a write that encodes every record the store will hold decides by content. An - // opaque payload (null) never marks an unmarked store, and keeps a marked one marked. - protected static string HeaderForWrite(string? durableHeader, IReadOnlyCollection written, bool copiesAny, bool directoryStore, bool holdsVariants) => + // opaque payload (null) never marks an unmarked store, and keeps a marked one marked. A copied record this store read + // holding an id, at the version the file still holds, marks the store too: another runtime may have written ids under + // an unmarked header. Nothing a load records can unmark a store. + protected static string HeaderForWrite(string? durableHeader, IReadOnlyCollection written, bool copiesAny, bool copiedHoldIds, bool directoryStore, bool holdsVariants) => HeaderFor( - holdsIds: written.Any(value => value == true), + holdsIds: copiedHoldIds || written.Any(value => value == true), durableHeader, wholeStore: !copiesAny && written.All(value => value != null), directoryStore, @@ -3944,6 +3946,10 @@ namespace GameCult.Caching // A write into a store holding a variant lands only while the durable versions still equal these. private readonly Dictionary _lastRead = new(StringComparer.Ordinal); + // The version of each plain record a pull decoded holding an element id. Only a positive fact about those bytes: a + // write marks the header for a copied record still at that version, and nothing here ever unmarks a store. + private readonly Dictionary _readHoldingIds = new(StringComparer.Ordinal); + /// /// A store file from its parts, each already encoded: a catalog entry or record this store carries forward is the bytes its /// file held, one it writes is or of the object. @@ -4032,6 +4038,7 @@ namespace GameCult.Caching if (durable == null) { _lastRead.Clear(); + _readHoldingIds.Clear(); SetLastSchemaMigrationReports(Array.Empty()); SetForeignRecords(Array.Empty()); return; @@ -4042,6 +4049,7 @@ namespace GameCult.Caching var persisted = new Dictionary(StringComparer.Ordinal); var foreign = new List(); var versions = new Dictionary(StringComparer.Ordinal); + var holdingIds = new Dictionary(StringComparer.Ordinal); foreach (var record in snapshot.Records) { var version = versions[record.Key] = durable.Version(record.Key); @@ -4054,6 +4062,8 @@ namespace GameCult.Caching var stored = ToStoredDocument(record, version, snapshot.SchemaCatalog, DeserializePayload, out var report); reports.Add(report); persisted[stored.Key.Value] = stored; + if (stored.Variant == null && CultElementIds.Holds(stored.Document)) + holdingIds[stored.Key.Value] = version; } var loaded = persisted.Values @@ -4071,6 +4081,9 @@ namespace GameCult.Caching _lastRead.Clear(); foreach (var (key, version) in versions) _lastRead[key] = version; + _readHoldingIds.Clear(); + foreach (var (key, version) in holdingIds) + _readHoldingIds[key] = version; SetLastSchemaMigrationReports(reports); SetForeignRecords(foreign); } @@ -4250,6 +4263,8 @@ namespace GameCult.Caching durable?.Snapshot.FormatVersion, upserts.Select(staged => staged.HoldsIds).ToArray(), copiesAny: copied.Length > 0, + copiedHoldIds: copied.Any(record => _readHoldingIds.TryGetValue(record.Key, out var read) && + SameStoredIdentity(read, durable!.Version(record.Key))), directoryStore: false, holdsVariants: records.Any(record => record.Variant != null)); diff --git a/tests/GameCult.Caching.Tests/PythonWrittenStoreTests.cs b/tests/GameCult.Caching.Tests/PythonWrittenStoreTests.cs index cc76c760..e20fe926 100644 --- a/tests/GameCult.Caching.Tests/PythonWrittenStoreTests.cs +++ b/tests/GameCult.Caching.Tests/PythonWrittenStoreTests.cs @@ -67,6 +67,71 @@ namespace GameCult.Caching.Tests } } + private static string HeaderOf(string path) => + CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).FormatVersion; + + private static async Task<(CultCache Cache, SingleFileMessagePackBackingStore Store)> PulledAsync(string path) + { + var cache = new CultCache(); + var store = new SingleFileMessagePackBackingStore(path); + cache.AddBackingStore(store); + await cache.PullAllBackingStoresAsync(); + return (cache, store); + } + + // cultcache-py writes element ids under an unmarked v1 header. A C# write that carries such a record forward marks the + // store, because a reader from before ids would otherwise rewrite the elements without them. + [Test] + public async Task ACSharpWriteThatCopiesAPythonRecordHoldingIdsMarksTheStore() + { + var scratch = Directory.CreateTempSubdirectory().FullName; + try + { + var path = Store("leaf-record.cc", scratch); + Assert.That(HeaderOf(path), Is.EqualTo(CultPersistedStoreSnapshot.FormatV1), "cultcache-py leaves the header unmarked"); + var (cache, store) = await PulledAsync(path); + await cache.AddAsync(new NumericLeafVectorTests.LeafRecord { Id = "other" }); + store.PushAll(); + Assert.That(HeaderOf(path), Is.EqualTo(CultPersistedStoreSnapshot.FormatV3)); + + var (reloaded, _) = await PulledAsync(path); + var py = reloaded.AllEntries.OfType().Single(record => record.Id == "py"); + Assert.That(py.Items.Select(item => item.Id), Is.EqualTo(new[] { "000000000021", "000000000022" })); + } + finally + { + Directory.Delete(scratch, true); + } + } + + // What a load saw is a fact about those bytes at that version. Another writer replaced the record since, so the copy this + // write makes is not the record the load saw, and the header is not judged by it. + [Test] + public async Task ACopiedRecordAnotherWriterChangedIsNotJudgedByTheOldRead() + { + var scratch = Directory.CreateTempSubdirectory().FullName; + try + { + var path = Store("leaf-record.cc", scratch); + var (reader, readerStore) = await PulledAsync(path); + + var (writer, writerStore) = await PulledAsync(path); + var py = writer.AllEntries.OfType().Single(record => record.Id == "py"); + py.Items.Clear(); + await writer.AddAsync(py); + writerStore.PushAll(); + Assert.That(HeaderOf(path), Is.EqualTo(CultPersistedStoreSnapshot.FormatV1), "the replacement holds no id"); + + await reader.AddAsync(new NumericLeafVectorTests.LeafRecord { Id = "other" }); + readerStore.PushAll(); + Assert.That(HeaderOf(path), Is.EqualTo(CultPersistedStoreSnapshot.FormatV1)); + } + finally + { + Directory.Delete(scratch, true); + } + } + [TestCase(3)] [TestCase(5)] [TestCase(7)]