Refuse a plain write that shares a variant's indexed value; a flush sheds overrides the type lacks
The index rule is about the commit's result, so it now runs for every commit that changes an indexed value: a plain record landing on a value a variant of the post-commit set holds is refused naming both keys and the index (the held index is never trusted; a plain write had already overwritten it). Plain-vs-plain is untouched. A variant sheds overrides of slots its type lacks when it is flushed, as a plain record sheds the member, and the load's report names the slot when the schema itself never had it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
5d6847e220
commit
354f9743e6
2 changed files with 245 additions and 3 deletions
|
|
@ -2520,7 +2520,11 @@ namespace GameCult.Caching
|
|||
}
|
||||
|
||||
if (toResolve.Count == 0)
|
||||
return new VariantPlan(admitted);
|
||||
{
|
||||
var unchanged = new VariantPlan(admitted);
|
||||
RefusePlainSharingWithVariant(unchanged, incoming, gone);
|
||||
return unchanged;
|
||||
}
|
||||
|
||||
var pending = new HashSet<string>(toResolve, StringComparer.Ordinal);
|
||||
var done = new Dictionary<string, CultStoredDocument>(StringComparer.Ordinal);
|
||||
|
|
@ -2532,6 +2536,7 @@ 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]);
|
||||
RefusePlainSharingWithVariant(plan, incoming, gone);
|
||||
return plan;
|
||||
|
||||
CultStoredDocument ResolveOne(string key)
|
||||
|
|
@ -2672,6 +2677,61 @@ namespace GameCult.Caching
|
|||
}
|
||||
}
|
||||
|
||||
// The index rule is about the commit's result, so it holds for every commit that changes an indexed value, in either
|
||||
// order: a variant resolving onto another record's value is refused in ResolveOne; a plain record landing on a
|
||||
// variant's value is refused here, against the variants of the post-commit set (never the held index, which the plain
|
||||
// write would already have overwritten). Plain-vs-plain sharing is not this rule's business.
|
||||
private void RefusePlainSharingWithVariant(
|
||||
VariantPlan plan,
|
||||
IReadOnlyDictionary<string, CultStoredDocument> incoming,
|
||||
HashSet<string> gone)
|
||||
{
|
||||
Dictionary<(Type Type, string Alias), Dictionary<string, string>>? variantValues = null;
|
||||
foreach (var stored in plan.Admitted)
|
||||
{
|
||||
if (stored.Variant != null || stored.Descriptor.IndexAccessors.Count == 0)
|
||||
continue;
|
||||
foreach (var pair in stored.Descriptor.IndexAccessors)
|
||||
{
|
||||
var value = pair.Value(stored.Document);
|
||||
if (string.IsNullOrWhiteSpace(value))
|
||||
continue;
|
||||
variantValues ??= HeldByVariants(plan, incoming, gone);
|
||||
if (variantValues.TryGetValue((stored.Descriptor.DocumentType, pair.Key), out var values) &&
|
||||
values.TryGetValue(value, out var variantKey) && variantKey != stored.Key.Value)
|
||||
throw new InvalidOperationException(
|
||||
$"Record {stored.Key.Value} and variant {variantKey} both hold index {pair.Key} '{value}'; " +
|
||||
"a variant may not share an indexed value with another record.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The indexed values the variants of the post-commit set hold: those this commit resolved, and those it left alone.
|
||||
private Dictionary<(Type Type, string Alias), Dictionary<string, string>> HeldByVariants(
|
||||
VariantPlan plan,
|
||||
IReadOnlyDictionary<string, CultStoredDocument> incoming,
|
||||
HashSet<string> gone)
|
||||
{
|
||||
var variants = new Dictionary<string, CultStoredDocument>(StringComparer.Ordinal);
|
||||
foreach (var variantKey in _variantKeys.Where(variantKey => !incoming.ContainsKey(variantKey) && !gone.Contains(variantKey)))
|
||||
variants[variantKey] = _entries[variantKey];
|
||||
foreach (var stored in plan.Admitted.Concat(plan.Dependents).Where(stored => stored.Variant != null))
|
||||
variants[stored.Key.Value] = stored;
|
||||
|
||||
var held = new Dictionary<(Type Type, string Alias), Dictionary<string, string>>();
|
||||
foreach (var stored in variants.Values)
|
||||
{
|
||||
foreach (var pair in stored.Descriptor.IndexAccessors)
|
||||
{
|
||||
var value = pair.Value(stored.Document);
|
||||
if (!string.IsNullOrWhiteSpace(value))
|
||||
MapOf(held, (stored.Descriptor.DocumentType, pair.Key))[value] = stored.Key.Value;
|
||||
}
|
||||
}
|
||||
|
||||
return held;
|
||||
}
|
||||
|
||||
private static InvalidOperationException CycleRefusal(IEnumerable<string> keys) =>
|
||||
new($"Variant bases form a cycle: {string.Join(" -> ", keys)}.");
|
||||
|
||||
|
|
@ -2917,10 +2977,41 @@ namespace GameCult.Caching
|
|||
SchemaId = entry.Descriptor.SchemaId,
|
||||
StoredAt = entry.StoredAt,
|
||||
Payload = entry.Variant != null ? Array.Empty<byte>() : serializePayload(entry.Document),
|
||||
Variant = entry.Variant == null ? null : new CultVariantDelta(entry.Variant.BaseKey, entry.Variant.Overrides)
|
||||
Variant = entry.Variant == null ? null : new CultVariantDelta(entry.Variant.BaseKey, OverridesTheTypeHas(entry).ToArray())
|
||||
};
|
||||
}
|
||||
|
||||
// A plain record sheds a member its type dropped when it is flushed; a variant sheds the override of that slot, so
|
||||
// a later type reusing the slot number cannot resurrect it. The load's report names the shed slots.
|
||||
private static IEnumerable<CultVariantOverride> OverridesTheTypeHas(CultStoredDocument entry) =>
|
||||
entry.Variant!.Overrides.Where(entryOverride =>
|
||||
entryOverride.Path.Count != 1 || entry.Descriptor.RichMembers.Any(member => member.Slot == entryOverride.Path[0].Slot));
|
||||
|
||||
// A variant's override of a slot its type lacks is ignored like an extra slot on a plain record. Schema resolution
|
||||
// reports slots the persisted schema had and the type dropped; an override the schema never had is added here.
|
||||
protected static CultSchemaMigrationReport ReportVariantSheds(CultSchemaMigrationReport report, CultStoredDocument stored)
|
||||
{
|
||||
if (stored.Variant == null)
|
||||
return report;
|
||||
var shed = stored.Variant.Overrides
|
||||
.Where(entryOverride => entryOverride.Path.Count == 1 &&
|
||||
stored.Descriptor.RichMembers.All(member => member.Slot != entryOverride.Path[0].Slot))
|
||||
.Select(entryOverride => entryOverride.Path[0].Slot)
|
||||
.Except(report.IgnoredExtraSlots)
|
||||
.Distinct()
|
||||
.OrderBy(slot => slot)
|
||||
.ToArray();
|
||||
if (shed.Length == 0)
|
||||
return report;
|
||||
report.IgnoredExtraSlots = report.IgnoredExtraSlots.Concat(shed).OrderBy(slot => slot).ToArray();
|
||||
report.Warnings = report.Warnings.Concat(shed.Select(slot => new CultSchemaMigrationWarning
|
||||
{
|
||||
Code = "ignored_extra_slot",
|
||||
Message = $"Variant {stored.Key.Value} overrides slot {slot}, which {stored.Descriptor.DocumentType.Name} does not have; the override is ignored and shed on flush."
|
||||
})).ToArray();
|
||||
return report;
|
||||
}
|
||||
|
||||
protected CultStoredDocument ToStoredDocument(
|
||||
CultPersistedRecord record,
|
||||
IReadOnlyCollection<CultSchemaCatalogEntry> catalog,
|
||||
|
|
@ -2993,8 +3084,9 @@ namespace GameCult.Caching
|
|||
var persisted = new Dictionary<string, CultStoredDocument>(StringComparer.Ordinal);
|
||||
foreach (var record in snapshot.Records)
|
||||
{
|
||||
reports.Add(Registry.ResolvePersistedSchemaReport(record.SchemaId, snapshot.SchemaCatalog));
|
||||
var report = Registry.ResolvePersistedSchemaReport(record.SchemaId, snapshot.SchemaCatalog);
|
||||
var stored = ToStoredDocument(record, snapshot.SchemaCatalog, DeserializePayload);
|
||||
reports.Add(ReportVariantSheds(report, stored));
|
||||
persisted[stored.Key.Value] = stored;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -587,6 +587,140 @@ namespace GameCult.Caching.Tests
|
|||
Assert.That(refused.Message, Does.Contain("hand-variant").And.Contain("hand-base").And.Contain("code"));
|
||||
}
|
||||
|
||||
// ---- the index rule holds for the commit's result, in either write order, and the store always reopens ----
|
||||
|
||||
private static VariantGear Other(string code) => new() { Name = "other", Power = 1, Code = code };
|
||||
|
||||
[Test]
|
||||
public async Task APlainWriteThatWouldShareAVariantsIndexedValueIsRefusedNamingBothKeysAndTheStoreReopens()
|
||||
{
|
||||
var path = PathOf("gear.cc");
|
||||
var other = new CultRecordKey("other");
|
||||
using (var cache = Open(path))
|
||||
{
|
||||
SeedBase(cache);
|
||||
SeedBig(cache);
|
||||
|
||||
var refused = Refused(() => cache.Commit(batch => batch.Upsert(typeof(VariantGear), Other("laser big"), other)));
|
||||
Assert.That(refused.Message, Does.Contain(other.Value).And.Contain(BigKey.Value).And.Contain("code").And.Contain("laser big"));
|
||||
|
||||
var single = Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => cache.UpsertAsync(Other("laser big"), new CultRecordHandle<VariantGear>(other)))!;
|
||||
Assert.That(single.Message, Does.Contain(other.Value).And.Contain(BigKey.Value));
|
||||
|
||||
Assert.That(cache.Get(other), Is.Null, "nothing landed");
|
||||
Assert.That(cache.GetByIndex<VariantGear>("code", "laser big")!.Name, Is.EqualTo("laser big"), "the variant is still found by its value");
|
||||
EditBase(cache, gear => gear.Power = 11);
|
||||
Assert.That(cache.Get<VariantGear>(BigKey)!.Power, Is.EqualTo(25), "the variant is still editable through its base");
|
||||
await Task.CompletedTask;
|
||||
}
|
||||
|
||||
using var reopened = Open(path);
|
||||
Assert.That(reopened.GetByIndex<VariantGear>("code", "laser big")!.Name, Is.EqualTo("laser big"));
|
||||
Assert.That(reopened.Get(other), Is.Null);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ABaseEditOrANewPlainRecordFirstThenAVariantOnTheSameValueIsRefusedAndTheStoreReopens()
|
||||
{
|
||||
var path = PathOf("gear.cc");
|
||||
var other = new CultRecordKey("other");
|
||||
using (var cache = Open(path))
|
||||
{
|
||||
SeedBase(cache);
|
||||
cache.Commit(batch => batch.Upsert(typeof(VariantGear), Other("laser big"), other));
|
||||
|
||||
var refused = Refused(() => SeedBig(cache));
|
||||
Assert.That(refused.Message, Does.Contain(BigKey.Value).And.Contain(other.Value).And.Contain("code").And.Contain("laser big"));
|
||||
Assert.That(cache.Get(BigKey), Is.Null, "nothing landed");
|
||||
|
||||
// The same batch, either staging order: the result is what is judged.
|
||||
var sameBatchPlainFirst = Refused(() => cache.Commit(batch =>
|
||||
{
|
||||
batch.Upsert(typeof(VariantGear), Other("taken"), new CultRecordKey("taker"));
|
||||
batch.UpsertVariant(BigKey, BaseKey, new[]
|
||||
{
|
||||
cache.Override<VariantGear>(nameof(VariantGear.Name), "laser big"),
|
||||
cache.Override<VariantGear>(nameof(VariantGear.Code), "taken")
|
||||
});
|
||||
}));
|
||||
Assert.That(sameBatchPlainFirst.Message, Does.Contain(BigKey.Value).And.Contain("taker"));
|
||||
var sameBatchVariantFirst = Refused(() => cache.Commit(batch =>
|
||||
{
|
||||
batch.UpsertVariant(BigKey, BaseKey, new[]
|
||||
{
|
||||
cache.Override<VariantGear>(nameof(VariantGear.Name), "laser big"),
|
||||
cache.Override<VariantGear>(nameof(VariantGear.Code), "taken")
|
||||
});
|
||||
batch.Upsert(typeof(VariantGear), Other("taken"), new CultRecordKey("taker"));
|
||||
}));
|
||||
Assert.That(sameBatchVariantFirst.Message, Does.Contain(BigKey.Value).And.Contain("taker"));
|
||||
|
||||
// Plain-vs-plain sharing is not this rule's business.
|
||||
Assert.DoesNotThrow(() => cache.Commit(batch => batch.Upsert(typeof(VariantGear), Other("l1"), new CultRecordKey("twin"))));
|
||||
}
|
||||
|
||||
using var reopened = Open(path);
|
||||
Assert.That(reopened.Get(BigKey), Is.Null);
|
||||
Assert.That(reopened.Get(other), Is.Not.Null);
|
||||
}
|
||||
|
||||
// ---- a variant sheds overrides its type lacks when it is flushed, as a plain record sheds the member ----
|
||||
|
||||
[Test]
|
||||
public void AFlushShedsAnOverrideOfADroppedSlotSoALaterTypeReusingTheSlotDoesNotResurrectIt()
|
||||
{
|
||||
var path = PathOf("drift.cc");
|
||||
var wide = CultDocumentRegistry.ForTypes(new[] { typeof(DriftWide) });
|
||||
using (var writer = Open(path, registry: wide))
|
||||
{
|
||||
writer.Commit(batch =>
|
||||
{
|
||||
batch.Upsert(typeof(DriftWide), new DriftWide { Name = "drift", Power = 1, Wing = "base wing" }, new CultRecordKey("drift-base"));
|
||||
batch.UpsertVariant(new CultRecordKey("drift-variant"), new CultRecordKey("drift-base"), new[]
|
||||
{
|
||||
writer.Override<DriftWide>(nameof(DriftWide.Name), "drift variant"),
|
||||
writer.Override<DriftWide>(nameof(DriftWide.Wing), "stale wing")
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
using (var narrow = Open(path, registry: CultDocumentRegistry.ForTypes(new[] { typeof(DriftNarrow) })))
|
||||
narrow.BackingStores[0].PushAll();
|
||||
|
||||
Assert.That(OverriddenSlotsOn(path, "drift-variant"), Is.EqualTo(new[] { 0 }), "the dropped slot's override is gone from the flushed record");
|
||||
|
||||
using var reused = Open(path, registry: CultDocumentRegistry.ForTypes(new[] { typeof(DriftReused) }));
|
||||
Assert.That(reused.Get<DriftReused>(new CultRecordKey("drift-base"))!.Colour, Is.Empty);
|
||||
Assert.That(reused.Get<DriftReused>(new CultRecordKey("drift-variant"))!.Colour, Is.Empty, "the variant inherits its base's colour, not the old wing");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void AnOverrideOfASlotTheTypeNeverHadIsReportedOnLoadAndShedOnTheNextFlush()
|
||||
{
|
||||
var path = WriteStore(PlainGear("hand-base"), OverridingRecord("hand-variant", "hand-base",
|
||||
CultVariantOverride.Set(99, new byte[] { 0x01 }),
|
||||
TextOverride(0, "hand variant"),
|
||||
TextOverride(2, "hand-code")));
|
||||
using (var cache = Open(path))
|
||||
{
|
||||
var variantReport = cache.BackingStores[0].LastSchemaMigrationReports.Single(report => report.IgnoredExtraSlots.Length > 0);
|
||||
Assert.That(variantReport.IgnoredExtraSlots, Is.EqualTo(new[] { 99 }));
|
||||
Assert.That(variantReport.Warnings.Select(warning => warning.Code), Has.Some.EqualTo("ignored_extra_slot"));
|
||||
Assert.That(variantReport.Warnings.Select(warning => warning.Message), Has.Some.Contain("hand-variant"));
|
||||
cache.BackingStores[0].PushAll();
|
||||
}
|
||||
|
||||
Assert.That(OverriddenSlotsOn(path, "hand-variant").OrderBy(slot => slot), Is.EqualTo(new[] { 0, 2 }));
|
||||
using var reopened = Open(path);
|
||||
Assert.That(reopened.BackingStores[0].LastSchemaMigrationReports.SelectMany(report => report.IgnoredExtraSlots), Is.Empty);
|
||||
Assert.That(reopened.Get<VariantGear>(new CultRecordKey("hand-variant"))!.Name, Is.EqualTo("hand variant"));
|
||||
}
|
||||
|
||||
private static int[] OverriddenSlotsOn(string path, string key) =>
|
||||
CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).Records
|
||||
.Single(record => record.Key == key).Variant!.Overrides.Select(entry => entry.Path[0].Slot).ToArray();
|
||||
|
||||
// ---- flatten resolves against the same batch ----
|
||||
|
||||
[Test]
|
||||
|
|
@ -950,6 +1084,22 @@ namespace GameCult.Caching.Tests
|
|||
public string Wing = string.Empty;
|
||||
}
|
||||
|
||||
// A later type that reuses the slot Narrow dropped, for another member.
|
||||
[CultDocument("tests.variant_drift", "tests.variant_drift.v3")]
|
||||
[MessagePackObject(AllowPrivate = true)]
|
||||
internal sealed class DriftReused
|
||||
{
|
||||
[Key(0)]
|
||||
[CultName]
|
||||
public string Name = string.Empty;
|
||||
|
||||
[Key(1)]
|
||||
public int Power;
|
||||
|
||||
[Key(2)]
|
||||
public string Colour = string.Empty;
|
||||
}
|
||||
|
||||
[CultDocument("tests.variant_drift", "tests.variant_drift.v2")]
|
||||
[MessagePackObject(AllowPrivate = true)]
|
||||
internal sealed class DriftNarrow
|
||||
|
|
|
|||
Loading…
Reference in a new issue