diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index 1875b0b1..7c8b2d1a 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -3669,7 +3669,7 @@ namespace GameCult.Caching // An entry under the type's own id that lists the record's id says the record is this schema's: it is re-encoded. var publisher = Publisher(record.SchemaId, snapshot.SchemaCatalog); if (!Owns(held.Descriptor, publisher.SchemaId)) - _unowned[record.Key] = (held, record, publisher); + _unowned[record.Key] = (held, record, publisher, SerializePayload(held.Document)); } } @@ -3677,8 +3677,9 @@ namespace GameCult.Caching // earlier declaration of the schema, resolved through the catalog's name or its pointer to a local id), each with the // catalog entry that published it, keyed while the store holds the entry it loaded. A whole-view write lays such a record // back exactly as it was stored, id and catalog entry included; only a write of the record itself stores it under its - // type's own id. - private readonly Dictionary _unowned = + // type's own id. Payload is what the loaded document serialized to when it was loaded, so a change made to the document in + // place is told from a record nobody wrote. + private readonly Dictionary _unowned = new(StringComparer.Ordinal); private static bool Owns(CultDocumentDescriptor descriptor, string schemaId) => @@ -3728,7 +3729,7 @@ namespace GameCult.Caching view.Records, Entries.Values.Select(entry => entry.Descriptor.ToCatalogEntry()).ToArray(), view.Arrived, - Entries.Values.Any(entry => entry.HoldsIds), + Entries.Values.Any(entry => entry.HoldsIds && !view.LaidBack.Contains(entry.Key.Value)), disk?.FormatVersion, wholeStore: foreign.Count == 0); WroteWholeView(view.Records, view.LaidBack); @@ -3758,11 +3759,7 @@ namespace GameCult.Caching // and staged single writes land with it. A conditional commit (store clean) lands the batch onto the file as it is. var ontoDisk = request.HasConditions && !IsDirty; var foreign = ForeignOnDisk(disk); - foreach (var entry in request.Upserts.Concat(request.Deletes)) - { - if (foreign.TryGetValue(entry.Key.Value, out var carried)) - throw Overwrites(carried.Foreign); - } + RefuseForeign(request.Upserts.Concat(request.Deletes), foreign); var view = ontoDisk ? null : WholeView(Entries.Values, disk, foreign); var records = (view?.Records ?? disk.Records).ToDictionary(record => record.Key, StringComparer.Ordinal); @@ -3781,7 +3778,7 @@ namespace GameCult.Caching // replaces or removes no longer counts. var replaced = request.Upserts.Concat(request.Deletes).Select(entry => entry.Key.Value).ToHashSet(StringComparer.Ordinal); var holdsIds = request.Upserts.Any(entry => entry.HoldsIds) || - (!ontoDisk && Entries.Values.Any(entry => entry.HoldsIds && !replaced.Contains(entry.Key.Value))); + (!ontoDisk && Entries.Values.Any(entry => entry.HoldsIds && !replaced.Contains(entry.Key.Value) && !view!.LaidBack.Contains(entry.Key.Value))); var registered = Entries.Values.Select(entry => entry.Descriptor.ToCatalogEntry()) .Concat(request.Upserts.Select(entry => entry.Descriptor.ToCatalogEntry())) .ToArray(); @@ -3808,29 +3805,47 @@ namespace GameCult.Caching { public CultPersistedRecord[] Records = Array.Empty(); public CultSchemaCatalogEntry[] Arrived = Array.Empty(); + // Keys written as the file holds or held them, not as this cache holds them. public HashSet LaidBack = new(StringComparer.Ordinal); } + // A write over a record the file holds as foreign is refused, and what this store held for the refused keys goes with the + // refusal: its copy is not what the file holds, and kept it would refuse every later write to other records too. + private void RefuseForeign( + IEnumerable written, + IReadOnlyDictionary foreign) + { + var refused = written.Where(entry => foreign.ContainsKey(entry.Key.Value)).ToArray(); + if (refused.Length == 0) + return; + foreach (var entry in refused) + Entries.TryRemove(entry.Key.Value, out _); + throw Overwrites(foreign[refused[0].Key.Value].Foreign); + } + private WholeViewWrite WholeView( IEnumerable view, CultPersistedStoreSnapshot? disk, IReadOnlyDictionary foreign) { var entries = view.ToArray(); - foreach (var entry in entries) - { - if (foreign.TryGetValue(entry.Key.Value, out var carried)) - throw Overwrites(carried.Foreign); - } + RefuseForeign(entries, foreign); var write = new WholeViewWrite(); var laidBackEntries = new List(); var onDisk = (disk?.Records ?? Array.Empty()).ToDictionary(record => record.Key, StringComparer.Ordinal); write.Records = entries.Select(entry => { - if (_unowned.TryGetValue(entry.Key.Value, out var loaded) && ReferenceEquals(loaded.Loaded, entry)) + if (_unowned.TryGetValue(entry.Key.Value, out var loaded) && ReferenceEquals(loaded.Loaded, entry) && + SerializePayload(entry.Document).AsSpan().SequenceEqual(loaded.Payload)) { + // Laid back only while the file still holds what was loaded: after that the file's record is its owner's newer + // write, which stays as it is, and a record the file no longer holds is not written back. write.LaidBack.Add(entry.Key.Value); + if (!onDisk.TryGetValue(entry.Key.Value, out var current)) + return null; + if (current.SchemaId != loaded.Record.SchemaId || current.StoredAt != loaded.Record.StoredAt) + return current; laidBackEntries.Add(loaded.Entry); return loaded.Record; } @@ -3842,7 +3857,7 @@ namespace GameCult.Caching if (!unchanged) record.StoredAt = CultCache.MintStoredAt(record.StoredAt); return record; - }).Concat(foreign.Values.Select(carried => carried.Record)).ToArray(); + }).OfType().Concat(foreign.Values.Select(carried => carried.Record)).ToArray(); // An entry that published a laid-back record is written as it was loaded, whatever the file's entry for that id says now. var laidBackIds = laidBackEntries.Select(entry => entry.SchemaId).ToHashSet(StringComparer.Ordinal); @@ -3879,11 +3894,12 @@ namespace GameCult.Caching .All(same => same)); // The file now holds every record written, under the id and at the storedAt the write gave it: the store's entries and the - // cache's say so. A laid-back record is on disk as it was loaded, so the cache learns nothing new about it: in particular the - // ids it minted at load are still in memory only. + // cache's say so. A laid-back or carried record is on disk as the file had it, so the cache learns nothing new about it: in + // particular the ids it minted at load are still in memory only. private void WroteWholeView(IReadOnlyCollection written, HashSet laidBack) { - foreach (var record in written) + var asHeld = written.Where(record => !laidBack.Contains(record.Key)).ToArray(); + foreach (var record in asHeld) { if (!Entries.TryGetValue(record.Key, out var entry)) continue; @@ -3891,7 +3907,7 @@ namespace GameCult.Caching entry.StoredAt = record.StoredAt; } - Cache?.WroteWholeView(written.Where(record => !laidBack.Contains(record.Key))); + Cache?.WroteWholeView(asHeld); } // Only a variant makes a merge more than the committer's own judgement: a variant it lands, one another writer wrote diff --git a/tests/GameCult.Caching.Tests/ElementIdTests.cs b/tests/GameCult.Caching.Tests/ElementIdTests.cs index 8c8f1ed8..dfcc956a 100644 --- a/tests/GameCult.Caching.Tests/ElementIdTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdTests.cs @@ -219,13 +219,8 @@ namespace GameCult.Caching.Tests { var path = PathOf("unowned-pre-id.cc"); WritePreIdStore(path, null, ("old", "o", 1)); - var snapshot = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)); - var ownId = snapshot.Records.Single().SchemaId; - var entry = snapshot.SchemaCatalog.Single(); - entry.SchemaId = "other.runtime.deck"; - entry.CompatibleSchemaIds = new[] { "other.runtime.deck" }; - snapshot.Records.Single().SchemaId = "other.runtime.deck"; - File.WriteAllBytes(path, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); + var ownId = DiskRecord(path, "old").SchemaId; + RelabelAsOtherRuntime(path); var stored = DiskRecord(path, "old"); using var cache = Open(path); @@ -237,6 +232,115 @@ namespace GameCult.Caching.Tests Assert.That(DiskRecord(path, "old").SchemaId, Is.EqualTo(ownId), "a write of the record stores it under the type's own id"); } + // The store's one record, stored as another runtime stores that schema: under an id the type does not own, published by a + // catalog entry of its own. + private static void RelabelAsOtherRuntime(string path) + { + var snapshot = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)); + var entry = snapshot.SchemaCatalog.Single(); + entry.SchemaId = "other.runtime.deck"; + entry.CompatibleSchemaIds = new[] { "other.runtime.deck" }; + snapshot.Records.Single().SchemaId = "other.runtime.deck"; + File.WriteAllBytes(path, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); + } + + private string UnownedStore(string name) + { + var path = PathOf(name); + WritePreIdStore(path, null, ("old", "o", 1)); + RelabelAsOtherRuntime(path); + return path; + } + + // The record's owner wrote it after this cache loaded it: the file holds the owner's newer record, and this cache's write of + // another record leaves it as it is, storedAt included (a record's storedAt never moves backwards). + [Test] + public void AWholeViewWriteDoesNotRevertARecordItsOwnerWroteAfterTheLoad([Values] bool flush) + { + var path = UnownedStore("owner-wrote.cc"); + using var cache = Open(path); + var loadedAt = DiskRecord(path, "old").StoredAt; + using (var owner = Open(path)) + owner.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("owner"), new CultRecordKey("old"))); + var owned = DiskRecord(path, "old"); + + if (flush) + { + cache.UpsertAsync(typeof(IdDeck), Deck("other"), new CultRecordKey("other")).GetAwaiter().GetResult(); + cache.FlushAsync().GetAwaiter().GetResult(); + } + else + cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("other"), new CultRecordKey("other"))); + + var after = DiskRecord(path, "old"); + Assert.That((after.SchemaId, after.StoredAt, after.Payload), Is.EqualTo((owned.SchemaId, owned.StoredAt, owned.Payload))); + Assert.That(string.CompareOrdinal(after.StoredAt, loadedAt), Is.GreaterThan(0)); + Assert.That(MessagePackSerializer.Deserialize(after.Payload).Name, Is.EqualTo("owner")); + } + + // The owner removed it: this cache's stale copy is not written back. + [Test] + public void AWholeViewWriteDoesNotWriteBackARecordItsOwnerRemovedAfterTheLoad() + { + var path = UnownedStore("owner-removed.cc"); + using var cache = Open(path); + var snapshot = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)); + snapshot.Records = Array.Empty(); + File.WriteAllBytes(path, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); + + cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("other"), new CultRecordKey("other"))); + + Assert.That(CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).Records.Select(record => record.Key), Is.EqualTo(new[] { "other" })); + } + + // A record that carried no ids when it was laid back does not mark the store, though the ids it minted at load are in memory. + [Test] + public void ALaidBackRecordWithoutIdsDoesNotMarkTheStore() + { + var path = UnownedStore("laid-back-v1.cc"); + using var cache = Open(path); + + cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("empty", reels: 0), new CultRecordKey("empty"))); + + Assert.That(DiskRecord(path, "old").SchemaId, Is.EqualTo("other.runtime.deck"), "laid back"); + Assert.That(CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).FormatVersion, Is.EqualTo(CultPersistedStoreSnapshot.FormatV1)); + } + + // A change made to a laid-back document in place is a write of the record, as it is for a record the type owns: it is stored + // under the type's own id, not dropped. + [Test] + public void AnInPlaceChangeToALaidBackDocumentIsWritten() + { + var path = UnownedStore("in-place.cc"); + using var cache = Open(path); + cache.Get(new CultRecordKey("old"))!.Name = "changed"; + + cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("other"), new CultRecordKey("other"))); + + var written = DiskRecord(path, "old"); + Assert.That(written.SchemaId, Is.Not.EqualTo("other.runtime.deck")); + Assert.That(MessagePackSerializer.Deserialize(written.Payload).Name, Is.EqualTo("changed")); + } + + // The catalog entry that published a laid-back record is written as it was loaded, whatever the file's entry for the id says now. + [Test] + public void ALaidBackRecordIsPublishedByTheCatalogEntryItWasLoadedWith() + { + var path = UnownedStore("laid-back-entry.cc"); + using var cache = Open(path); + var loadedHash = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).SchemaCatalog.Single().ContentHash; + var snapshot = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)); + snapshot.SchemaCatalog.Single().ContentHash = "changed-by-another-writer"; + File.WriteAllBytes(path, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); + + cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("other"), new CultRecordKey("other"))); + + var published = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).SchemaCatalog + .Where(entry => entry.SchemaId == "other.runtime.deck").ToArray(); + Assert.That(published, Has.Length.EqualTo(1)); + Assert.That(published[0].ContentHash, Is.EqualTo(loadedHash)); + } + // A whole-view write that persists the ids a record minted at load stores other bytes under the same id: a new store of // the record, so it takes a later storedAt, and the cache that wrote it holds that storedAt. A record it stores exactly as // the file holds it keeps its own.