diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index a8abd4cb..9d0f51ef 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -628,9 +628,18 @@ namespace GameCult.Caching throw new InvalidOperationException($"Persisted schema '{schemaId}' is not present in the embedded catalog."); } - if (indexes.ByCompatibleSchemaId.TryGetValue(schemaId, out var declared)) + // No type owns the id, so the one type that lists it is the record's type. Two listers and no owner cannot say which. + if (indexes.ByCompatibleSchemaId.TryGetValue(schemaId, out var listers)) { - return BuildCompatibleResolutionResult(persisted, declared); + if (listers.Length > 1) + throw new CultSchemaConflictException( + $"CultCache schema id '{schemaId}' is owned by no registered type and declared compatible by several " + + $"({string.Join(", ", listers.Select(lister => lister.DocumentType.FullName))}), so a record under it resolves to none. " + + "Register the type that owns the id, or declare it on one type.", + schemaId, + listers.Select(lister => lister.SchemaName).ToArray(), + string.Empty); + return BuildCompatibleResolutionResult(persisted, listers[0]); } foreach (var compatibleSchemaId in persisted.CompatibleSchemaIds.Where(candidate => !string.IsNullOrWhiteSpace(candidate))) @@ -699,18 +708,16 @@ namespace GameCult.Caching descriptor.DocumentType); } - // One type carries each schema id, its own or declared compatible, so which type a record resolves to never - // depends on the order types were registered in. - foreach (var carried in descriptor.CompatibleSchemaIds.Prepend(descriptor.SchemaId)) - { - if (indexes.BySchemaId.TryGetValue(carried, out var claimant) || indexes.ByCompatibleSchemaId.TryGetValue(carried, out claimant)) - throw SchemaIdClaimedTwice(carried, claimant, descriptor); - } - + // A schema id hashes the schema's name and version, so the one other type that could own this id is the one refused or + // aliased above. Other types may list the id as compatible beside its owner, as a v2 class does that keeps v1's class + // registered: a record under the id resolves to its owner, and to a lister only when nothing owns it. Neither depends on + // the order types were registered in. indexes.ByType[descriptor.DocumentType] = descriptor; indexes.BySchemaId[descriptor.SchemaId] = descriptor; foreach (var compatibleSchemaId in descriptor.CompatibleSchemaIds) - indexes.ByCompatibleSchemaId[compatibleSchemaId] = descriptor; + indexes.ByCompatibleSchemaId[compatibleSchemaId] = indexes.ByCompatibleSchemaId.TryGetValue(compatibleSchemaId, out var listers) + ? [.. listers, descriptor] + : [descriptor]; indexes.BySchemaName[descriptor.SchemaName] = schemaNameVersions == null ? [descriptor] : [.. schemaNameVersions, descriptor]; @@ -736,7 +743,7 @@ namespace GameCult.Caching new( $"CultCache schema id '{schemaId}' is already carried by CLR type '{existing.DocumentType.FullName}' " + $"(schema '{existing.SchemaName}') and cannot also be claimed by '{claimed.DocumentType.FullName}' " + - $"(schema '{claimed.SchemaName}'). A schema id, owned or declared compatible, belongs to one registered type.", + $"(schema '{claimed.SchemaName}'). A schema id is owned by one registered type; others may only declare it compatible.", schemaId, new[] { existing.SchemaName, claimed.SchemaName }, string.Empty); @@ -756,7 +763,7 @@ namespace GameCult.Caching { ByType = new Dictionary(); BySchemaId = new Dictionary(StringComparer.Ordinal); - ByCompatibleSchemaId = new Dictionary(StringComparer.Ordinal); + ByCompatibleSchemaId = new Dictionary(StringComparer.Ordinal); BySchemaName = new Dictionary(StringComparer.Ordinal); } @@ -764,13 +771,14 @@ namespace GameCult.Caching { ByType = new Dictionary(source.ByType); BySchemaId = new Dictionary(source.BySchemaId, StringComparer.Ordinal); - ByCompatibleSchemaId = new Dictionary(source.ByCompatibleSchemaId, StringComparer.Ordinal); + ByCompatibleSchemaId = new Dictionary(source.ByCompatibleSchemaId, StringComparer.Ordinal); BySchemaName = new Dictionary(source.BySchemaName, StringComparer.Ordinal); } public Dictionary ByType { get; } public Dictionary BySchemaId { get; } - public Dictionary ByCompatibleSchemaId { get; } + // Every type declaring the id compatible, in registration order; resolution never depends on that order. + public Dictionary ByCompatibleSchemaId { get; } public Dictionary BySchemaName { get; } } diff --git a/tests/GameCult.Caching.Tests/CompatibleSchemaIdTests.cs b/tests/GameCult.Caching.Tests/CompatibleSchemaIdTests.cs index 047d3457..35179a4d 100644 --- a/tests/GameCult.Caching.Tests/CompatibleSchemaIdTests.cs +++ b/tests/GameCult.Caching.Tests/CompatibleSchemaIdTests.cs @@ -559,6 +559,27 @@ namespace GameCult.Caching.Tests Assert.That(Derive(held, current, unchangedOnly: false), Is.EqualTo(CultCommitOutcome.Committed)); } + // The migration CompatibleSchemaIds exists for: v2 declares v1's id and v1's class stays registered beside it, in either + // order. A record under v1's id is v1's, its owner; one under v2's id is v2's. + [TestCase(false)] + [TestCase(true)] + public void AVersionKeptBesideTheVersionDeclaringItsIdReadsItsOwnRecords(bool reversed) + { + using (var older = OpenRoll(RollV1)) + older.Commit(batch => batch.Upsert(RollV1, New(RollV1, ("Name", "k")), RollK)); + using (var newer = OpenRoll(RollV2)) + newer.Commit(batch => + { + batch.Expect(RollY, null); + batch.Upsert(RollV2, New(RollV2, ("Name", "y"), ("Extra", "e")), RollY); + }); + Assert.That(Read(RollPath).Records.Select(record => record.SchemaId), Is.EquivalentTo(new[] { RollV1Id, CultDocumentRegistry.ForTypes(new[] { RollV2 }).GetRequired(RollV2).SchemaId })); + + using var both = Open(RollPath, CultDocumentRegistry.ForTypes(reversed ? new[] { RollV2, RollV1 } : new[] { RollV1, RollV2 })); + Assert.That(both.Get(RollK)!.GetType(), Is.EqualTo(RollV1)); + Assert.That(both.Get(RollY)!.GetType(), Is.EqualTo(RollV2)); + } + [Test] public void ADeclarationIsRegisteredWithoutItsOwnIdAndItsEntryListsTheOwnIdFirst() { diff --git a/tests/GameCult.Caching.Tests/CultDocumentRegistryTests.cs b/tests/GameCult.Caching.Tests/CultDocumentRegistryTests.cs index 6126146d..8512e275 100644 --- a/tests/GameCult.Caching.Tests/CultDocumentRegistryTests.cs +++ b/tests/GameCult.Caching.Tests/CultDocumentRegistryTests.cs @@ -122,8 +122,8 @@ public sealed class CultDocumentRegistryTests }); } - // One type carries each schema id, owned or declared compatible, whichever order the types register in: a second claimant - // is refused, typed, naming both, and the registry keeps what it had. + // A second type owning a schema id is refused, typed, naming both, whichever order the two register in, and the registry + // keeps what it had. private static void AssertSecondClaimRefused(Type first, Type second, string schemaId) { var refusal = Assert.Throws(() => CultDocumentRegistry.ForTypes(new[] { first, second }))!; @@ -138,25 +138,57 @@ public sealed class CultDocumentRegistryTests Assert.That(registry.AllDescriptors.Select(descriptor => descriptor.DocumentType), Is.EqualTo(new[] { first })); } + private static CultSchemaCatalogEntry[] CatalogOf(params Type[] types) => + types.Select(type => CultDocumentRegistry.ForTypes(new[] { type }).GetRequired(type).ToCatalogEntry()).ToArray(); + + // Owner beats lister: a type that owns an id and one that lists it register together in either order (the v1 class kept + // beside a v2 class declaring v1's id). A record under the id resolves to its owner; one under the lister's own id to the + // lister; without the owner registered, the id resolves to the lister. [TestCase(false)] [TestCase(true)] - public void TwoTypesDeclaringOneCompatibleIdAreRefused(bool reversed) - { - var a = Emit("declares_a", "tests.registry.declares_a", "v1", compatibleSchemaIds: new[] { "tests.registry.shared" }); - var b = Emit("declares_b", "tests.registry.declares_b", "v1", compatibleSchemaIds: new[] { "tests.registry.shared" }); - - AssertSecondClaimRefused(reversed ? b : a, reversed ? a : b, "tests.registry.shared"); - } - - [TestCase(false)] - [TestCase(true)] - public void ATypeDeclaringAnIdAnotherTypeOwnsIsRefused(bool reversed) + public void AnOwnerAndATypeListingItsIdRegisterTogetherAndTheOwnerWins(bool reversed) { var owner = Emit("owns", "tests.registry.owns", "v1"); var ownerId = CultDocumentRegistry.ForTypes(new[] { owner }).GetRequired(owner).SchemaId; - var lister = Emit("lists", "tests.registry.lists", "v1", compatibleSchemaIds: new[] { ownerId }); + var lister = Emit("lists", "tests.registry.owns", "v2", new[] { new Field("Extra", typeof(string), 0) }, new[] { ownerId }); + var listerId = CultDocumentRegistry.ForTypes(new[] { lister }).GetRequired(lister).SchemaId; + var catalog = CatalogOf(owner, lister); - AssertSecondClaimRefused(reversed ? lister : owner, reversed ? owner : lister, ownerId); + var registry = CultDocumentRegistry.ForTypes(reversed ? new[] { lister, owner } : new[] { owner, lister }); + + Assert.Multiple(() => + { + Assert.That(registry.AllDescriptors.Select(descriptor => descriptor.DocumentType), Is.EquivalentTo(new[] { owner, lister })); + Assert.That(registry.ResolvePersistedSchema(ownerId, catalog).DocumentType, Is.EqualTo(owner)); + Assert.That(registry.ResolvePersistedSchema(listerId, catalog).DocumentType, Is.EqualTo(lister)); + Assert.That(CultDocumentRegistry.ForTypes(new[] { lister }).ResolvePersistedSchema(ownerId, catalog).DocumentType, Is.EqualTo(lister)); + }); + } + + // Two types listing one id register together in either order. With an owner registered, a record under the id is the + // owner's; with none, the id names no single type, and a record under it is refused, typed, naming the listers. + [TestCase(false)] + [TestCase(true)] + public void TwoTypesListingOneIdRegisterAndARecordUnderItNeedsAnOwner(bool reversed) + { + var owner = Emit("shared_owner", "tests.registry.shared", "v1"); + var sharedId = CultDocumentRegistry.ForTypes(new[] { owner }).GetRequired(owner).SchemaId; + var a = Emit("declares_a", "tests.registry.declares_a", "v1", compatibleSchemaIds: new[] { sharedId }); + var b = Emit("declares_b", "tests.registry.declares_b", "v1", compatibleSchemaIds: new[] { sharedId }); + var listers = reversed ? new[] { b, a } : new[] { a, b }; + var catalog = CatalogOf(owner); + + var ambiguous = CultDocumentRegistry.ForTypes(listers); + var refusal = Assert.Throws(() => ambiguous.ResolvePersistedSchema(sharedId, catalog))!; + Assert.Multiple(() => + { + Assert.That(refusal.SchemaId, Is.EqualTo(sharedId)); + Assert.That(refusal.SchemaNames, Is.EquivalentTo(new[] { "tests.registry.declares_a", "tests.registry.declares_b" })); + Assert.That(refusal.Message, Does.Contain(a.FullName).And.Contain(b.FullName)); + }); + + Assert.That(CultDocumentRegistry.ForTypes(listers.Append(owner)).ResolvePersistedSchema(sharedId, catalog).DocumentType, Is.EqualTo(owner)); + Assert.That(CultDocumentRegistry.ForTypes(listers.Prepend(owner)).ResolvePersistedSchema(sharedId, catalog).DocumentType, Is.EqualTo(owner)); } // The same schema declaring different compatible ids is not an alias: whichever registered second would lose its declaration. diff --git a/tests/GameCult.Caching.Tests/EmittedDocumentTypes.cs b/tests/GameCult.Caching.Tests/EmittedDocumentTypes.cs index 4ff40e25..ac6a6dad 100644 --- a/tests/GameCult.Caching.Tests/EmittedDocumentTypes.cs +++ b/tests/GameCult.Caching.Tests/EmittedDocumentTypes.cs @@ -7,9 +7,9 @@ using MessagePack; namespace GameCult.Caching.Tests { - // Document types built at run time, each in its own dynamic assembly. CultDocumentRegistry.Shared discovers every - // [CultDocument] type in the loaded non-dynamic assemblies and refuses two that claim one schema id, so a fixture that - // needs such a pair (a type and its alias, two versions that share an id) emits them here instead of declaring them. + // Document types built at run time, each in its own dynamic assembly, which CultDocumentRegistry.Shared never scans. A + // fixture emits a type here when declaring it would change what Shared holds: a pair Shared refuses (one schema declared + // two ways), an alias whose resolved type would depend on scan order, or a type a test needs to itself. internal static class EmittedDocumentTypes { internal readonly record struct Field(string Name, Type Type, int Key, bool IsName = false);