From 7362257f08a9668bd1bdb6fcd7965ef3d0eebb5c Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 29 Sep 2026 16:48:19 +0200 Subject: [PATCH] CultCache: read names and indexes once before minting; a throwing getter refuses the whole change set Resolve now ends in Project, the only place an admission runs a [CultName] or [CultIndex] getter on a landing record. It runs before land and before any Sequence is minted, so a throwing getter refuses the admission (loads included) with nothing written, minted or left in memory. The variant name and index rules and the indexer read the projection; Apply runs no user code. Co-Authored-By: Claude Sonnet 5 --- src/GameCult.Caching/CultCache.cs | 157 +++++++++---- .../CultCacheProjectionTests.cs | 210 ++++++++++++++++++ .../CultCachePublicationIsolationTests.cs | 63 ++++++ 3 files changed, 388 insertions(+), 42 deletions(-) create mode 100644 tests/GameCult.Caching.Tests/CultCacheProjectionTests.cs create mode 100644 tests/GameCult.Networking.Tests/CultCachePublicationIsolationTests.cs diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index b2f06659..8af4360c 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -2465,11 +2465,82 @@ namespace GameCult.Caching // Variants this admission did not write whose base chain it changed: re-resolved, same delta, same storedAt. public List Dependents { get; } = new(); + + // Name and index values of every record that lands (Admitted and Dependents), by key. Filled once by Project. + public Dictionary Projections { get; } = new(StringComparer.Ordinal); + } + + // What a landing record's [CultName] and [CultIndex] getters said. Indexes holds only non-blank values. + private sealed class Projection + { + public Projection(string? name, (string Alias, string Value)[] indexes) + { + Name = name; + Indexes = indexes; + } + + public string? Name { get; } + public (string Alias, string Value)[] Indexes { get; } + } + + // The only place an admission runs a name or index getter on a landing record. It runs before land and before any + // sequence is minted, so a getter that throws refuses the whole admission with nothing written and nothing minted. + private static void Project(VariantPlan plan) + { + foreach (var stored in plan.Admitted.Concat(plan.Dependents)) + { + var descriptor = stored.Descriptor; + string? name = null; + if (descriptor.NameAccessor != null) + name = Read(stored, $"[CultName] member {descriptor.NameMember}", descriptor.NameAccessor) is { Length: > 0 } read ? read : null; + var indexes = new List<(string Alias, string Value)>(); + foreach (var pair in descriptor.IndexAccessors) + { + var member = descriptor.RichMembers.First(candidate => candidate.IndexAlias == pair.Key).Member.Name; + var value = Read(stored, $"index '{pair.Key}' member {member}", pair.Value); + if (!string.IsNullOrWhiteSpace(value)) + indexes.Add((pair.Key, value!)); + } + + plan.Projections[stored.Key.Value] = new Projection(name, indexes.ToArray()); + } + } + + private static string? Read(CultStoredDocument stored, string what, Func getter) + { + try + { + return getter(stored.Document); + } + catch (Exception exception) + { + throw new InvalidOperationException( + $"Record {stored.Key.Value} ({stored.Descriptor.SchemaName}) is refused: its {what} threw: {exception.GetBaseException().Message}", exception); + } + } + + // Every admission is judged here: variants resolved, then every landing record's name and index values read once, + // then the rules that compare them. It refuses naming the keys, and mutates nothing. + private VariantPlan Resolve( + IReadOnlyList admitted, + IReadOnlyList evicted, + CacheBackingStore? source) + { + var plan = ResolveVariants(admitted, evicted, source); + Project(plan); + // The variant rules are judged only when a variant is involved. + if (_variantKeys.Count != 0 || admitted.Any(stored => stored.Variant != null)) + { + RefuseVariantNameSharing(plan, evicted); + RefuseVariantIndexSharing(plan, evicted); + } + + return plan; } // Resolves every admitted variant and every variant whose base chain this admission changes, base first, against // the post-admission set. It refuses a broken variant naming the keys, and mutates nothing. - private VariantPlan Resolve( + private VariantPlan ResolveVariants( IReadOnlyList admitted, IReadOnlyList evicted, CacheBackingStore? source) @@ -2531,11 +2602,7 @@ namespace GameCult.Caching } if (toResolve.Count == 0) - { - var unchanged = new VariantPlan(admitted); - RefuseVariantIndexSharing(unchanged, incoming, gone); - return unchanged; - } + return new VariantPlan(admitted); var pending = new HashSet(toResolve, StringComparer.Ordinal); var done = new Dictionary(StringComparer.Ordinal); @@ -2546,7 +2613,6 @@ namespace GameCult.Caching var plan = new VariantPlan(admitted.Select(stored => done.TryGetValue(stored.Key.Value, out var resolved) ? resolved : stored).ToList()); foreach (var key in toResolve.Where(key => !incoming.ContainsKey(key))) plan.Dependents.Add(done[key]); - RefuseVariantIndexSharing(plan, incoming, gone); return plan; CultStoredDocument ResolveOne(string key) @@ -2610,21 +2676,6 @@ namespace GameCult.Caching var document = _codec.Deserialize(type, type, kept.Count == 0 ? payload : _codec.Overlay(payload, kept)) ?? throw new InvalidOperationException($"Variant {key} resolved to nothing."); - if (isVariant) - { - // R6: the name index maps one name to one key, so a variant that kept its base's name would shadow it. - if (descriptor.NameAccessor?.Invoke(document) is { Length: > 0 } variantName) - { - for (var ancestor = baseStored; ancestor != null; - ancestor = ancestor.Variant == null ? null : done.TryGetValue(ancestor.Variant.BaseKey, out var above) ? above : Lookup(ancestor.Variant.BaseKey)) - { - if (descriptor.NameAccessor(ancestor.Document) == variantName) - throw new InvalidOperationException( - $"Variant {key} and its base {ancestor.Key.Value} are both named '{variantName}'; a variant must override its name."); - } - } - } - stack.RemoveAt(stack.Count - 1); return done[key] = isVariant ? variant.Resolved(document) @@ -2632,20 +2683,49 @@ namespace GameCult.Caching } } + // R6: the name index maps one name to one key, so a variant that kept its base's name would shadow it. A variant's + // name is its projection; an ancestor that lands is read from its projection, one that stays is read as held. + private void RefuseVariantNameSharing(VariantPlan plan, IReadOnlyList evicted) + { + var gone = evicted.Select(stored => stored.Key.Value).ToHashSet(StringComparer.Ordinal); + var landing = plan.Admitted.Concat(plan.Dependents).ToDictionary(stored => stored.Key.Value, StringComparer.Ordinal); + + CultStoredDocument? Ancestor(string key) => + landing.TryGetValue(key, out var landed) ? landed + : gone.Contains(key) ? null + : _entries.TryGetValue(key, out var held) ? held : null; + + string? NameOf(CultStoredDocument stored) => + plan.Projections.TryGetValue(stored.Key.Value, out var projection) + ? projection.Name + : stored.Descriptor.NameAccessor == null ? null : Read(stored, $"[CultName] member {stored.Descriptor.NameMember}", stored.Descriptor.NameAccessor); + + foreach (var variant in landing.Values.Where(stored => stored.Variant != null)) + { + if (plan.Projections[variant.Key.Value].Name is not { } variantName) + continue; + for (var ancestor = Ancestor(variant.Variant!.BaseKey); ancestor != null; + ancestor = ancestor.Variant == null ? null : Ancestor(ancestor.Variant.BaseKey)) + { + if (NameOf(ancestor) == variantName) + throw new InvalidOperationException( + $"Variant {variant.Key.Value} and its base {ancestor.Key.Value} are both named '{variantName}'; a variant must override its name."); + } + } + } + // The one index rule, judged on the commit's result whatever the write order: a variant may not share an indexed // value with another record. It reads the truthful held index (every holder of a value), minus the records this // commit replaces or removes, plus the records it lands. Plain-vs-plain sharing is not this rule's business. // A unique index maps a value to one key at lookup; a variant sharing it would be found in place of its // holder or lose the value, and the store would refuse to reopen. - private void RefuseVariantIndexSharing( - VariantPlan plan, - IReadOnlyDictionary incoming, - HashSet gone) + private void RefuseVariantIndexSharing(VariantPlan plan, IReadOnlyList evicted) { var landing = plan.Admitted.Concat(plan.Dependents).ToList(); if (landing.All(stored => stored.Descriptor.IndexAccessors.Count == 0)) return; - var replaced = new HashSet(incoming.Keys.Concat(gone).Concat(plan.Dependents.Select(stored => stored.Key.Value)), StringComparer.Ordinal); + var replaced = new HashSet( + plan.Admitted.Concat(evicted).Concat(plan.Dependents).Select(stored => stored.Key.Value), StringComparer.Ordinal); var landingByKey = landing.ToDictionary(stored => stored.Key.Value, StringComparer.Ordinal); bool IsVariant(string key) => @@ -2656,12 +2736,9 @@ namespace GameCult.Caching var landed = new Dictionary<((Type Type, string Alias) Index, string Value), List>(); foreach (var stored in landing) { - foreach (var pair in stored.Descriptor.IndexAccessors) + foreach (var (alias, value) in plan.Projections[stored.Key.Value].Indexes) { - var value = pair.Value(stored.Document); - if (string.IsNullOrWhiteSpace(value)) - continue; - var index = (stored.Descriptor.DocumentType, pair.Key); + var index = (stored.Descriptor.DocumentType, alias); landedValues.Add((stored, index, value)); if (!landed.TryGetValue((index, value), out var landedKeys)) landed[(index, value)] = landedKeys = new List(); @@ -2733,7 +2810,7 @@ namespace GameCult.Caching _entries[stored.Key.Value] = stored; _handles.AddOrUpdate(stored.Document, new KeyBox(stored.Key)); - Index(stored); + Index(stored, plan.Projections[stored.Key.Value]); if (stored.Variant == null) _variantKeys.Remove(stored.Key.Value); else @@ -2790,19 +2867,15 @@ namespace GameCult.Caching } } - private void Index(CultStoredDocument stored) + private void Index(CultStoredDocument stored, Projection projection) { var type = stored.Descriptor.DocumentType; if (stored.Descriptor.IsGlobal) _globals[type] = stored.Key.Value; - if (stored.Descriptor.NameAccessor?.Invoke(stored.Document) is { Length: > 0 } name) - MapOf(_names, type)[name] = stored.Key.Value; - foreach (var pair in stored.Descriptor.IndexAccessors) - { - var value = pair.Value(stored.Document); - if (!string.IsNullOrWhiteSpace(value)) - AddHolder(_indexes, (type, pair.Key), value, stored.Key.Value); - } + if (projection.Name != null) + MapOf(_names, type)[projection.Name] = stored.Key.Value; + foreach (var (alias, value) in projection.Indexes) + AddHolder(_indexes, (type, alias), value, stored.Key.Value); } // Documents are mutable, so a name or index value may have changed since it was indexed: drop by key. diff --git a/tests/GameCult.Caching.Tests/CultCacheProjectionTests.cs b/tests/GameCult.Caching.Tests/CultCacheProjectionTests.cs new file mode 100644 index 00000000..a35db5bd --- /dev/null +++ b/tests/GameCult.Caching.Tests/CultCacheProjectionTests.cs @@ -0,0 +1,210 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using GameCult.Caching; +using GameCult.Caching.MessagePack; +using MessagePack; +using NUnit.Framework; +using R3; + +namespace GameCult.Caching.Tests +{ + // Cut 1 of docs/cultcache-publication-cut.md: an admission reads every landing record's name and index values once, + // before land and before any sequence is minted. A getter that throws refuses the whole change set. + [NonParallelizable] + public class CultCacheProjectionTests + { + private static readonly CultDocumentRegistry Registry = CultDocumentRegistry.ForTypes(new[] { typeof(ProjectedRecord) }); + + private string _directory = string.Empty; + + [SetUp] + public void Prepare() + { + _directory = Path.Combine(Path.GetTempPath(), $"cultlib-projection-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_directory); + ProjectedRecord.Armed = false; + ProjectedRecord.Reads = 0; + } + + [TearDown] + public void Cleanup() + { + ProjectedRecord.Armed = false; + if (Directory.Exists(_directory)) + Directory.Delete(_directory, recursive: true); + } + + [CultDocument("tests.projected_record", "tests.projected_record.v1")] + [MessagePackObject(AllowPrivate = true)] + internal sealed class ProjectedRecord + { + public static long Reads; + public static volatile bool Armed; + [IgnoreMember] private string _name = string.Empty; + [IgnoreMember] private string _code = string.Empty; + + [Key(0)] + [CultName] + public string Name + { + get + { + Interlocked.Increment(ref Reads); + if (Armed && _name == "boom") + throw new InvalidOperationException("name getter boom"); + return _name; + } + set => _name = value; + } + + [Key(1)] + [CultIndex("code")] + public string Code + { + get + { + Interlocked.Increment(ref Reads); + if (Armed && _code == "boom") + throw new InvalidOperationException("index getter boom"); + return _code; + } + set => _code = value; + } + } + + private static readonly CultRecordKey OkKey = new("projected:ok"); + private static readonly CultRecordKey BoomKey = new("projected:boom"); + + private static ProjectedRecord Ok() => new() { Name = "ok", Code = "ok-code" }; + + private static ProjectedRecord Boom(bool name) => name ? new ProjectedRecord { Name = "boom", Code = "boom-code" } : new ProjectedRecord { Name = "boom-name", Code = "boom" }; + + private string PathOf(string name) => Path.Combine(_directory, name); + + private static CultCache Open(string path) => CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = Registry }); + + private static void Put(CultCache cache, ProjectedRecord document, CultRecordKey key) => + cache.Commit(batch => batch.Upsert(typeof(ProjectedRecord), document, key)); + + private static long SequenceOf(CultCache cache) => cache.GetWithSequence(OkKey).Sequence; + + private static void StageBoth(CultCacheBatch batch, ProjectedRecord boom) + { + batch.Upsert(typeof(ProjectedRecord), Ok(), OkKey); + batch.Upsert(typeof(ProjectedRecord), boom, BoomKey); + } + + [TestCase(true, "[CultName] member Name")] + [TestCase(false, "index 'code' member Code")] + public void ARefusedGetterMintsNothingAndChangesNothing(bool nameThrows, string member) + { + using var cache = new CultCache(Registry); + var seen = new List(); + using var watch = cache.Watch().Subscribe(change => seen.Add(change.Sequence)); + Put(cache, new ProjectedRecord { Name = "seed", Code = "seed-code" }, new CultRecordKey("projected:seed")); + var before = SequenceOf(cache); + seen.Clear(); + ProjectedRecord.Armed = true; + + var refusal = Assert.Throws(() => cache.Commit(batch => StageBoth(batch, Boom(nameThrows))))!; + + ProjectedRecord.Armed = false; + Assert.Multiple(() => + { + Assert.That(refusal.Message, Does.Contain(BoomKey.Value).And.Contain(member).And.Contain("getter boom")); + Assert.That(SequenceOf(cache), Is.EqualTo(before), "a refused admission mints nothing"); + Assert.That(cache.Get(OkKey), Is.Null, "the good record did not land"); + Assert.That(cache.Get(BoomKey), Is.Null); + Assert.That(cache.GetByName("ok"), Is.Null, "the name index holds nothing of the refused set"); + Assert.That(cache.GetByIndex("code", "ok-code"), Is.Null); + Assert.That(seen, Is.Empty, "nothing was published"); + }); + + Put(cache, Ok(), OkKey); + Assert.That(SequenceOf(cache), Is.EqualTo(before + 1), "the next write is previous + 1"); + Assert.That(seen, Is.EqualTo(new[] { before + 1 })); + } + + [Test] + public void ARefusedSingleWriteMintsNothing() + { + using var cache = new CultCache(Registry); + ProjectedRecord.Armed = true; + + var refusal = Assert.Throws(() => Put(cache, Boom(true), BoomKey))!; + + ProjectedRecord.Armed = false; + Assert.That(refusal.Message, Does.Contain(BoomKey.Value)); + Assert.That(SequenceOf(cache), Is.EqualTo(0)); + Assert.That(cache.Get(BoomKey), Is.Null); + } + + [Test] + public void ARefusedBatchLeavesAStoreByteIdenticalAndReopensWithNeitherKey() + { + var path = PathOf("projected.cc"); + using (var seed = Open(path)) + Put(seed, new ProjectedRecord { Name = "seed", Code = "seed-code" }, new CultRecordKey("projected:seed")); + var before = File.ReadAllBytes(path); + + using (var cache = Open(path)) + { + ProjectedRecord.Armed = true; + Assert.Throws(() => cache.Commit(batch => StageBoth(batch, Boom(true)))); + ProjectedRecord.Armed = false; + Assert.That(File.ReadAllBytes(path), Is.EqualTo(before)); + } + + using var reopened = Open(path); + Assert.Multiple(() => + { + Assert.That(reopened.Get(OkKey), Is.Null); + Assert.That(reopened.Get(BoomKey), Is.Null); + Assert.That(File.ReadAllBytes(path), Is.EqualTo(before)); + }); + } + + [Test] + public void ALoadWhoseGetterThrowsRefusesTheOpenAndLeavesTheFileByteIdentical() + { + var path = PathOf("poisoned.cc"); + using (var seed = Open(path)) + Put(seed, Boom(true), BoomKey); + var before = File.ReadAllBytes(path); + + ProjectedRecord.Armed = true; + var refusal = Assert.Throws(() => Open(path))!; + ProjectedRecord.Armed = false; + + Assert.That(refusal.Message, Does.Contain(BoomKey.Value).And.Contain("[CultName] member Name")); + Assert.That(File.ReadAllBytes(path), Is.EqualTo(before)); + } + + // A counting getter: each landing record is read once for its name and once for its index, and never again by the + // variant rules or the indexer. A variant re-resolved by a base edit lands too, and is read once. + [Test] + public void EachLandedRecordIsReadOncePerGetterIncludingAVariantDependent() + { + using var cache = new CultCache(Registry, CultCacheMessagePack.CreateCodec(Registry)); + var baseKey = new CultRecordKey("projected:base"); + Put(cache, new ProjectedRecord { Name = "base", Code = "base-code" }, baseKey); + cache.Commit(batch => batch.UpsertVariant(new CultRecordKey("projected:variant"), baseKey, new[] + { + cache.Override(nameof(ProjectedRecord.Name), "variant"), + cache.Override(nameof(ProjectedRecord.Code), "variant-code") + })); + + ProjectedRecord.Reads = 0; + Put(cache, new ProjectedRecord { Name = "base", Code = "base-code-2" }, baseKey); + + // Two landing records (the base, and the variant re-resolved against it), two getters each. + Assert.That(Interlocked.Read(ref ProjectedRecord.Reads), Is.EqualTo(4)); + Assert.That(cache.GetByIndex("code", "base-code-2"), Is.Not.Null); + Assert.That(cache.GetByName("variant"), Is.Not.Null); + } + } +} diff --git a/tests/GameCult.Networking.Tests/CultCachePublicationIsolationTests.cs b/tests/GameCult.Networking.Tests/CultCachePublicationIsolationTests.cs new file mode 100644 index 00000000..9ba78efa --- /dev/null +++ b/tests/GameCult.Networking.Tests/CultCachePublicationIsolationTests.cs @@ -0,0 +1,63 @@ +#nullable enable +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Threading.Tasks; +using GameCult.Caching; +using NUnit.Framework; +using R3; +using static GameCult.Networking.Tests.NetworkingTests; + +namespace GameCult.Networking.Tests +{ + [CultDocument("tests.publication_throwing_name", "tests.publication_throwing_name.v1")] + [MessagePack.MessagePackObject] + public sealed class PublicationThrowingName + { + [MessagePack.Key(0)] + public string Text { get; set; } = string.Empty; + + [MessagePack.Key(1)] + [CultName] + public string Name + { + get => Text == "boom" ? throw new InvalidOperationException("name getter boom") : Text; + set { } + } + } + + // Cache publication cuts 1 and 2 seen through CultNetDatabase: a refused or failing change never stalls the + // publication and log of the changes after it. (Soul pass 3 probes P4 and P3.) + [NonParallelizable] + public sealed class CultCachePublicationIsolationTests + { + private static NetworkSchemaNote Note(string text) => new() { Schema = "tests.networking_note.v1", Text = text }; + private static string SchemaId => CultDocumentRegistry.Shared.GetRequired().SchemaId; + + // An admission that a [CultName] getter refuses publishes nothing, leaves nothing in memory, and stalls nothing after it. + [Test] + public async Task ARefusedAdmissionLeavesNothingAndLaterChangesStillPublishAndLog() + { + var cache = new CultCache(); + var database = new CultNetDatabase(cache); + var seen = new ConcurrentQueue(); + using var watch = database.Watch().Subscribe(change => seen.Enqueue(change.Document?.Text)); + var okKey = new CultRecordKey("pub:ok"); + var refusal = Assert.Throws(() => cache.Commit(batch => + { + batch.Upsert(new PublicationThrowingName { Text = "ok" }, new CultRecordHandle(okKey)); + batch.Upsert(new PublicationThrowingName { Text = "boom" }, new CultRecordHandle(new CultRecordKey("pub:boomname"))); + }))!; + + var after = new CultRecordKey("pub:after"); + await database.PutAsync(after, Note("after")); + Assert.Multiple(() => + { + Assert.That(refusal.Message, Does.Contain("pub:boomname").And.Contain("name getter boom")); + Assert.That(cache.Get(okKey), Is.Null, "the good record of a refused batch did not land"); + Assert.That(seen, Is.EqualTo(new[] { "after" })); + Assert.That(database.LastWriteSequence(SchemaId, after), Is.Not.Null, "a later commit was never logged"); + }); + } + } +}