Removing a foreign record is judged against the store as it is, not the last load

A removal of a key the cache does not hold writes nothing, so it never reached
the apply; it was checked against the foreign list of the last load, and a
foreign record that arrived since was reported removed. Each store now reads
its durable state for such a removal (RefuseRemovalOfUndeclared) and refuses
through the apply's own RefuseUndeclaredOnDisk, which reloads and lists the
record. The load-time check (RefuseRemovalOfForeign) is deleted: the durable
read refuses everything it refused, and it could not save that read. A store
kind that cannot read its durable state keeps the view check as its default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Meta Vens 2026-10-03 15:13:32 +02:00
commit 43a63b0792
3 changed files with 57 additions and 16 deletions

View file

@ -185,7 +185,7 @@ public sealed class DirectoryMessagePackBackingStore : CacheBackingStore
manifest.Records.ToDictionary(record => record.Key, record => VersionOf(record.Payload), StringComparer.Ordinal), Entries.Values))
return CultCommitOutcome.Mismatch;
RefuseUndeclaredOnDisk(manifest, request.Upserts.Concat(request.Deletes).Select(entry => entry.Key.Value));
RefuseUndeclaredOnDisk(manifest, request.Upserts.Concat(request.Deletes).Select(entry => entry.Key.Value).Concat(StagedKeys));
var previousEntries = Entries.ToDictionary(pair => pair.Key, pair => pair.Value, StringComparer.Ordinal);
var previousDirtyKeys = _dirtyKeys.Keys.ToArray();
var previousDeletedKeys = _deletedKeys.Keys.ToArray();
@ -236,11 +236,18 @@ public sealed class DirectoryMessagePackBackingStore : CacheBackingStore
Directory.CreateDirectory(_manifestFile.DirectoryName!);
using var commitLease = AcquireCommitLease(wait: true, create: true);
var (manifest, catalogBytes) = ReadManifest();
RefuseUndeclaredOnDisk(manifest, Array.Empty<string>());
RefuseUndeclaredOnDisk(manifest, StagedKeys);
WriteGeneration(manifest, catalogBytes);
});
}
private IEnumerable<string> StagedKeys => _deletedKeys.Keys.Concat(_dirtyKeys.Keys);
// The manifest as it is now, read without the commit lease: nothing is written after the read, the manifest is only ever
// replaced whole, and a read-only store must not create the lease.
protected internal override void RefuseRemovalOfUndeclared(IReadOnlyCollection<string> keys) =>
Held(() => RefuseUndeclaredOnDisk(ReadManifest().Snapshot, keys));
// The manifest's other records stay as they are, foreign and read-only ones included; a write that would replace or remove a
// record the manifest holds under an id no registered type declares is refused before any page is written, by the one rule every
// store shares (CacheBackingStore.RefuseUndeclaredOnDisk). The refused records reload as a pull loads them.
@ -249,7 +256,7 @@ public sealed class DirectoryMessagePackBackingStore : CacheBackingStore
CultPersistedRecord? HeldAt(string key) =>
currentManifest.Records.FirstOrDefault(record => string.Equals(record.Key, key, StringComparison.Ordinal));
RefuseUndeclaredOnDisk(
written.Concat(_deletedKeys.Keys).Concat(_dirtyKeys.Keys),
written,
key => HeldAt(key) is { } held ? Undeclared(held, currentManifest.SchemaCatalog, Registry.Declares) : null,
key => HeldAt(key) is { } held && Foreign(held, currentManifest.SchemaCatalog) == null
? LoadPage(held, currentManifest.SchemaCatalog, out _, out _)

View file

@ -2358,7 +2358,7 @@ namespace GameCult.Caching
{
if (!_entries.TryGetValue(key.Value, out var existing))
{
RefuseRemovalOfForeign(key.Value);
RefuseRemovalOfUndeclared(new[] { key.Value });
return false;
}
Admit(Array.Empty<CultStoredDocument>(), new[] { existing }, null, (home, _) =>
@ -2631,8 +2631,10 @@ namespace GameCult.Caching
: op.Variant != null ? StampVariant(op.Key, DescriptorOfBase(op.Key, op.Variant.BaseKey, batch), op.Variant)
: StampFlatten(op.Key))
.ToArray();
foreach (var op in batch.Operations.Values.Where(op => op.IsRemoval && !_entries.ContainsKey(op.Key.Value)))
RefuseRemovalOfForeign(op.Key.Value);
RefuseRemovalOfUndeclared(batch.Operations.Values
.Where(op => op.IsRemoval && !_entries.ContainsKey(op.Key.Value))
.Select(op => op.Key.Value)
.ToArray());
var deletes = batch.Operations.Values
.Where(op => op.IsRemoval && _entries.ContainsKey(op.Key.Value))
.Select(op => _entries[op.Key.Value])
@ -2656,12 +2658,15 @@ namespace GameCult.Caching
});
}
// A removal of a key the cache does not hold removes nothing, so no store sees it; but a store that carries a foreign record
// at the key refuses it, as it refuses every write of that record, rather than let the caller believe it was removed.
private void RefuseRemovalOfForeign(string key)
// A removal of a key the cache does not hold removes nothing, so no write carries it to a store; but a store that holds a
// record there under an id no registered type declares refuses it, as it refuses every write of that record, rather than let
// the caller believe it was removed.
private void RefuseRemovalOfUndeclared(IReadOnlyCollection<string> keys)
{
if (keys.Count == 0)
return;
foreach (var (store, _) in _stores)
store.RefuseUndeclared(key);
store.RefuseRemovalOfUndeclared(keys);
}
// Every add and remove passes here: single writes, committed batches, and loads (source set).
@ -3649,6 +3654,16 @@ namespace GameCult.Caching
throw Overwrites(null, new[] { (key, held.StoredSchemaId, held.Descriptor.SchemaName) });
}
// The apply's refusal for removals of keys the cache does not hold. Such a removal writes nothing, so no write carries it to
// where the store is read; this reads it instead. A key whose durable record is under an id no registered type declares is
// refused and reloaded as the apply refuses and reloads it, including a record another writer put there after the load.
// A store kind that cannot read its durable state answers from its view (RefuseUndeclared).
protected internal virtual void RefuseRemovalOfUndeclared(IReadOnlyCollection<string> keys)
{
foreach (var key in keys)
RefuseUndeclared(key);
}
// A stored record under an id the writer does not declare, named by the catalog entry that publishes the id. Header only: no
// payload is decoded. Null for a record under a declared id.
protected static (string Key, string SchemaId, string SchemaName)? Undeclared(
@ -4165,6 +4180,11 @@ namespace GameCult.Caching
inner));
}
// The file as it is now, read as a pull reads it, without the lock: nothing is written after the read, and the file is only
// ever replaced whole.
protected internal override void RefuseRemovalOfUndeclared(IReadOnlyCollection<string> keys) =>
Held(() => RefuseUndeclaredOnDisk(ReadSnapshot(), keys, Registry.Declares));
// A cache declares through its registry. A cacheless CultMesh write declares the ids of the catalog entry it writes: its own id
// and the ids it lists compatible.
private void RefuseUndeclaredOnDisk(Durable? durable, IReadOnlyCollection<CultStagedRecord> upserts, IReadOnlyCollection<string> removals) =>

View file

@ -1024,14 +1024,28 @@ namespace GameCult.Caching.Tests
}
// A removal of a foreign record, which the cache does not hold, is refused typed: the caller is never told it removed what the
// store still carries.
// store still carries. The store is read as it is at the removal, so a foreign record another writer added after the load
// is refused too, and listed foreign from then on.
[Test]
public void RemovingAForeignRecordIsRefusedNotReportedAsDone([Values] bool directory, [Values] bool batch)
public void RemovingAForeignRecordIsRefusedNotReportedAsDone([Values] bool directory, [Values] bool batch, [Values] bool appearedAfterLoad)
{
var path = Seeded(directory ? "remove-foreign-dir.cc" : "remove-foreign-file.cc", directory);
var before = Fingerprint(path);
var path = PathOf(directory ? "remove-foreign-dir.cc" : "remove-foreign-file.cc");
using (var seed = Open(path, Full, directory))
{
seed.Commit(stage => stage.Upsert(Deck, DeckOf("d"), D));
if (!appearedAfterLoad)
seed.Commit(stage => stage.Upsert(Widget, WidgetOf("w"), W));
}
using (var cache = Open(path, DeckOnly, directory))
{
if (appearedAfterLoad)
{
using var other = Open(path, Full, directory);
other.Commit(stage => stage.Upsert(Widget, WidgetOf("w"), W));
}
var before = Fingerprint(path);
var refusal = Assert.Throws<CultSchemaConflictException>(() =>
{
if (batch)
@ -1041,10 +1055,10 @@ namespace GameCult.Caching.Tests
})!;
Assert.That((refusal.RecordKey, refusal.SchemaId), Is.EqualTo((W.Value, WidgetId)));
Assert.That(cache.IsDirty, Is.False);
Assert.That(StoreOf(cache).ForeignRecords.Select(record => record.Key), Is.EqualTo(new[] { "w" }));
Assert.That(cache.Remove(new CultRecordKey("absent")), Is.False, "a key nothing holds is still no removal");
Assert.That(Fingerprint(path), Is.EqualTo(before));
}
Assert.That(Fingerprint(path), Is.EqualTo(before));
}
}
}