From 7777ed1416dd7deabdd09d811685dec2476f54a0 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Wed, 29 Jul 2026 14:15:41 -0700 Subject: [PATCH 01/50] Use lazy-loading in `GetSourceCollectionAsync()` Currently, this forces lazy loading of each collection property it touches, as soon as the `getCollection()` is called, even though it may not actually need the data, thus resulting in multiple calls to the database for each topic. Worse, because these are coming from properties, they're synchronous. We'll be replacing this with a new method that assesses the entire data model for its `TopicPayload` requirements and triggers a single `EnsureLoaded()` call (#118). In preparation for that, I'm kicking things off by fixing this preexisting bug, so that the call to the lazy-loading property is, at least, deferred until it's passed the guard condition (`preconditionsMet`) within the `getCollection()` local function. --- OnTopic/Mapping/TopicMappingService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index a9cab4ce..7adece04 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -823,7 +823,7 @@ private async Task> GetSourceCollectionAsync( \-------------------------------------------------------------------------------------------------------------------------*/ listSource = getCollection( CollectionType.Relationship, - source.Relationships.Contains, + key => source.Relationships.Contains(key), () => source.Relationships.GetValues(collectionKey) ); @@ -832,7 +832,7 @@ private async Task> GetSourceCollectionAsync( \-------------------------------------------------------------------------------------------------------------------------*/ listSource = getCollection( CollectionType.NestedTopics, - source.Children.Contains, + key => source.Children.Contains(key), () => source.Children[collectionKey].Children ); @@ -841,7 +841,7 @@ private async Task> GetSourceCollectionAsync( \-------------------------------------------------------------------------------------------------------------------------*/ listSource = getCollection( CollectionType.IncomingRelationship, - source.IncomingRelationships.Contains, + key => source.IncomingRelationships.Contains(key), () => source.IncomingRelationships.GetValues(collectionKey) ); From 6e064a14bf51ec9df4264d5c72dd2941c21c0374 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Wed, 29 Jul 2026 15:33:27 -0700 Subject: [PATCH 02/50] Added unit tests to confirm load order These confirm the previous fix (7777ed14) where the call to `Collection.Contains()` wasn't properly deferred, and thus triggered lazy loading immediately, even if the collection wasn't needed. This confirms that fix works as expected. This includes a new `RelationshipOnlyTopicViewModel` for test purposes. This is part a prior bug fix that was exposed by lazy loading (#111) and will be necessary to resolve as part of the mapping integration (#118). --- OnTopic.Tests/TopicMappingServiceTest.cs | 62 +++++++++++++++++++ .../RelationshipOnlyTopicViewModel.cs | 33 ++++++++++ 2 files changed, 95 insertions(+) create mode 100644 OnTopic.Tests/ViewModels/RelationshipOnlyTopicViewModel.cs diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 4c761d5a..e24c6dbf 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -6,16 +6,20 @@ using System.ComponentModel.DataAnnotations; using System.Globalization; using OnTopic.Data.Caching; +using OnTopic.Lookup; using OnTopic.Mapping; using OnTopic.Mapping.Internal; using OnTopic.Metadata; using OnTopic.Repositories; using OnTopic.TestDoubles; +using OnTopic.TestDoubles.LazyLoading; using OnTopic.TestDoubles.Metadata; using OnTopic.Tests.Entities; using OnTopic.Tests.Fixtures; +using OnTopic.Tests.TestDoubles; using OnTopic.Tests.ViewModels; using OnTopic.Tests.ViewModels.Metadata; +using OnTopic.ViewModels; using Xunit; namespace OnTopic.Tests; @@ -1036,6 +1040,36 @@ public async Task Map_MapAs_ReturnsRelationships() { } + /*============================================================================================================================ + | TEST: MAP: RELATIONSHIP ONLY: DOES NOT FILL CHILDREN + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Maps a , whose only collection is explicitly typed , against a , and confirms that no + /// synchronous fill occurs. Prior to fixing GetSourceCollectionAsync's + /// collection probes, the NestedTopics probe's signature (source.Children.Contains) was evaluated + /// unconditionally when the argument was constructed, silently triggering the lazy loading of the children regardless of + /// which collection type the view model actually requested. + /// + [Fact] + public async Task Map_RelationshipOnly_DoesNotFillChildren() { + + var stub = new StubLazyLoadingTopicRepository(); + var cache = new CachedTopicRepository(stub); + var typeLookupService = new CompositeTypeLookupService(new TopicViewModelLookupService(), new FakeViewModelLookupService()); + var mappingService = new TopicMappingService(cache, typeLookupService); + + var topic = await cache.Load("Root:Web:Web_0"); + + Contract.Assume(topic); + + var target = await mappingService.MapAsync(topic); + + Assert.NotNull(target); + Assert.Equal(0, stub.GetFetchCount(topic.Id, TopicPayload.Children)); + + } + /*============================================================================================================================ | TEST: MAP: TOPIC REFERENCES AS ATTRIBUTE: RETURNS MAPPED MODEL \---------------------------------------------------------------------------------------------------------------------------*/ @@ -1333,6 +1367,34 @@ public async Task Map_FilterByCollectionType_ReturnsFilteredCollection() { } + /*============================================================================================================================ + | TEST: MAP: DESCENDENT: DOES NOT FILL RELATIONSHIPS + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Maps a , whose only collection is , against a + /// , and confirms that no synchronous + /// fill occurs. The inverse of : The relationship probe's method call + /// (source.Relationships.Contains) was likewise evaluated unconditionally. + /// + [Fact] + public async Task Map_Descendent_DoesNotFillRelationships() { + + var stub = new StubLazyLoadingTopicRepository(); + var cache = new CachedTopicRepository(stub); + var typeLookupService = new CompositeTypeLookupService(new TopicViewModelLookupService(), new FakeViewModelLookupService()); + var mappingService = new TopicMappingService(cache, typeLookupService); + + var topic = await cache.Load("Root:Web:Web_0"); + + Contract.Assume(topic); + + var target = await mappingService.MapAsync(topic); + + Assert.NotNull(target); + Assert.Equal(0, stub.GetFetchCount(topic.Id, TopicPayload.Relationships)); + + } + /*============================================================================================================================ | TEST: MAP: GETTER METHODS: MAP METHOD OUTPUT \---------------------------------------------------------------------------------------------------------------------------*/ diff --git a/OnTopic.Tests/ViewModels/RelationshipOnlyTopicViewModel.cs b/OnTopic.Tests/ViewModels/RelationshipOnlyTopicViewModel.cs new file mode 100644 index 00000000..6a8d8682 --- /dev/null +++ b/OnTopic.Tests/ViewModels/RelationshipOnlyTopicViewModel.cs @@ -0,0 +1,33 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: RELATIONSHIP ONLY +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a simple view model with a single, explicitly typed property (). +/// +/// +/// +/// Intended as a stand-in for cases where a very simple view model is required for test purposes, without introducing other +/// mapping scenarios that might introduce errors, even though they've not part of the test. Unlike , whose maps , is explicitly typed as a relationship, so it exercises only the +/// relationship probe in TopicMappingService.GetSourceCollectionAsync. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +public class RelationshipOnlyTopicViewModel: KeyOnlyTopicViewModel { + + [Collection("Related", Type = CollectionType.Relationship)] + public Collection Related { get; } = new(); + +} //Class \ No newline at end of file From d4b4020261d2245aca5d332ecbe860de84b7c7c5 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Wed, 29 Jul 2026 15:54:35 -0700 Subject: [PATCH 03/50] Ensure `EnsureLoaded()` is awaited Within a synchronous method, we can't call `EnsureLoaded()` with an `await` so we need to use `GetAwaiter().GetResult()`; otherwise, it will run in the background, but the `GetValue()` will (likely) fail assuming it needs get data from the database. This was done correctly in the properties calling `EnsureLoaded()`, but missed in `GetValue()` (9e712ffb). --- OnTopic/Attributes/AttributeCollection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OnTopic/Attributes/AttributeCollection.cs b/OnTopic/Attributes/AttributeCollection.cs index e43a30b0..965bd96f 100644 --- a/OnTopic/Attributes/AttributeCollection.cs +++ b/OnTopic/Attributes/AttributeCollection.cs @@ -142,7 +142,7 @@ public bool IsDirty(bool excludeLastModified) bool autoLoad = true ) { if (autoLoad && LoadState is LoadState.NotLoaded && !Contains(key)) { - ((ITopicLazyLoadable)AssociatedTopic).EnsureLoaded(TopicPayload.ExtendedAttributes); + ((ITopicLazyLoadable)AssociatedTopic).EnsureLoaded(TopicPayload.ExtendedAttributes).GetAwaiter().GetResult(); } return base.GetValue(key, defaultValue, inheritFromParent, maxHops, autoLoad); } From 03ed27c762bd21e48f87d7b4233270893c9cddd5 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Thu, 30 Jul 2026 17:32:15 -0700 Subject: [PATCH 04/50] `EnsureLoaded()` for `AsAttributeDictionary()` `AsAttributeDictionary()` is used primarily by the `TopicMappingService` to retrieve all attributes, including derivatives. Unlike a raw enumeration of the collection, it is expected to be an authoritative source of the attributes available and, thus, must ensure that the extended attributes are fully loaded before returning. This patches a gap introduced by the introduction of lazy loading (#111) and contributes to the mapping fixes being implemented as part of #118. In a future update, we'll be identifying what `TopicPayload` the topic needs upfront so the `TopicMappingService` can forecast and do a single `EnsureLoaded()`. Until that lands, this patches an important gap for not only the ``TopicMappingService`, but also any other consumers that rely on this public method. --- OnTopic/Attributes/AttributeCollection.cs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/OnTopic/Attributes/AttributeCollection.cs b/OnTopic/Attributes/AttributeCollection.cs index 965bd96f..c3d0a9f2 100644 --- a/OnTopic/Attributes/AttributeCollection.cs +++ b/OnTopic/Attributes/AttributeCollection.cs @@ -21,12 +21,12 @@ namespace OnTopic.Attributes; /// The class tracks these through its property, which is an instance of /// the class. /// -/// When is , iterating the collection (e.g., via foreach, -/// LINQ operators, or ) returns only the indexed attributes already present and -/// does not fetch the deferred extended attribute blob. Only a keyed lookup autoloads on a miss. Callers that require a -/// complete set of attributes must first await with . Otherwise, a decision that depends on seeing every attribute may act on a partial -/// view without any error being raised. +/// When is , iterating the collection directly (e.g., via +/// foreach or LINQ operators) returns only the indexed attributes already present and does not fetch the deferred +/// extended attribute blob; only a keyed lookup or autoloads. Callers that +/// enumerate the collection directly and require a complete set of attributes must first await with . Otherwise, a decision that +/// depends on seeing every attribute may act on a partial view without any error being raised. /// /// public class AttributeCollection : TrackedRecordCollection { @@ -217,8 +217,9 @@ public void SetValue( /// /// The method will exclude attributes which correspond to properties on /// which contain specialized getter logic, such as and . Like any enumeration over the collection, this reads only the resident attributes; see the remarks for the completeness contract on a topic. + /// "Topic.LastModified"/>. Unlike a direct enumeration of the collection, this autoloads the extended attribute blob for + /// each source (the current collection, and, if is true, each in the chain) that is , so the result is always complete. /// /// /// Determines if attributes from the should be included. Defaults to false. @@ -229,6 +230,10 @@ public AttributeDictionary AsAttributeDictionary(bool inheritFromBase = false) { var attributes = new AttributeDictionary(); var count = 0; while (sourceAttributes is not null && ++count < 5) { + if (sourceAttributes.LoadState is LoadState.NotLoaded) { + var associatedTopic = (ITopicLazyLoadable)sourceAttributes.AssociatedTopic; + associatedTopic.EnsureLoaded(TopicPayload.ExtendedAttributes).GetAwaiter().GetResult(); + } foreach (var attribute in sourceAttributes) { if (count is 1 || !attributes.ContainsKey(attribute.Key)) { attributes.TryAdd(attribute.Key, attribute.Value); From 51d0f4ede415afd1a03eb48a01e6a303d747b9f7 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Thu, 30 Jul 2026 18:11:12 -0700 Subject: [PATCH 05/50] Added unit test for `AsAttributeDictionary()` fix This validates that the `AsAttributeDictionary()`'s conditional call to `EnsureLoaded()` successfully returns a `LoadState.Loaded` list of attributes, instead of only returning the previously available indexed attributes. This tests a fix (03ed27c7) introduced to patch the `TopicMappingService` (#118) in response to lazy loading (#111). --- OnTopic.Tests/TopicMappingServiceTest.cs | 42 ++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index e24c6dbf..86403e82 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -325,6 +325,48 @@ public async Task Map_AttributeDictionary_ReturnsNewModel() { } + /*============================================================================================================================ + | TEST: MAP: ATTRIBUTE DICTIONARY: NOT LOADED: RETURNS EXTENDED ATTRIBUTES + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a and maps a sparse topic whose extended attributes are still to a view model with an constructor. Confirms the extended + /// attribute is present in the mapped result, i.e., that + /// autoloads the blob rather than silently omitting it by enumerating only resident attributes. + /// + [Fact] + public async Task Map_AttributeDictionary_NotLoaded_ReturnsExtendedAttributes() { + + var records = new StubLazyLoadingTopicRepositoryBuilder() + .AddTopic( + 221, + "Sparse", + "Page", + null, + indexedAttributes : new Dictionary { + ["Title"] = "Value", + ["ShortTitle"] = "Short Title", + ["Subtitle"] = "Subtitle", + ["MetaTitle"] = "Meta Title", + ["MetaDescription"] = "Meta Description" + }, + extendedAttributes : new Dictionary { + ["MappedProperty"] = "Mapped Value" + } + ) + .Build(); + + var stub = new StubLazyLoadingTopicRepository(records); + var topic = await stub.Load("Root:Sparse"); + + Contract.Assume(topic); + + var target = await _mappingService.MapAsync(topic); + + Assert.Equal("Mapped Value", target?.MappedProperty); + + } + /*============================================================================================================================ | TEST: MAP: CONSTRUCTOR: RETURNS NEW MODEL \---------------------------------------------------------------------------------------------------------------------------*/ From faf3639fa8320590c5ebdc8936bd855c9dd2ce78 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Thu, 30 Jul 2026 18:12:24 -0700 Subject: [PATCH 06/50] Ensure view model can't be mapped by reflection The entire purpose of the `AttributeDictionaryConstructorTopicViewModel` is, as the name suggests, to test the `AttributeDictionary` constructor shortcut that bypasses the comparatively expensive reflection-based mapping of well-known view models. To help shore up those tests, I've added `[DisableMapping]` to the view model's properties to ensure that they can _only_ be filled via the constructor. While this isn't necessary for the deliberately unmapped `UnmappedProperty`, as again the name suggested, it helps provide consistency. This is used to ensure that the newly introduced unit test (51d0f4ed) as part of #118 is correctly testing the right mapping method. --- .../AttributeDictionaryConstructorTopicViewModel.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/OnTopic.Tests/ViewModels/AttributeDictionaryConstructorTopicViewModel.cs b/OnTopic.Tests/ViewModels/AttributeDictionaryConstructorTopicViewModel.cs index 942493f2..e6d800f8 100644 --- a/OnTopic.Tests/ViewModels/AttributeDictionaryConstructorTopicViewModel.cs +++ b/OnTopic.Tests/ViewModels/AttributeDictionaryConstructorTopicViewModel.cs @@ -13,7 +13,10 @@ namespace OnTopic.Tests.ViewModels; /// Provides a strongly-typed data transfer object for testing a constructor with a . /// /// -/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// and are decorated with +/// so they can only be populated via the constructor, not the reflection-based property +/// mapper's fallback pass; this isolates tests to the constructor-dictionary path they're meant to exercise. This is a sample +/// class intended for test purposes only; it is not designed for use in a production environment. /// public record AttributeDictionaryConstructorTopicViewModel: PageTopicViewModel { @@ -38,7 +41,10 @@ public AttributeDictionaryConstructorTopicViewModel() { } /*============================================================================================================================ | PROPERTIES \---------------------------------------------------------------------------------------------------------------------------*/ + [DisableMapping] public string? MappedProperty { get; init; } + + [DisableMapping] public string? UnmappedProperty { get; init; } From b88d436f48b3a40269609e92ef6328c85ca39be9 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Thu, 30 Jul 2026 18:12:43 -0700 Subject: [PATCH 07/50] Ensure metadata lookup items are loaded on mapping A metadata relationships allows a reference to a lookup list, typically (but not necessarily) in the configuration for e.g., a list of countries or states or lookup information. It is typically bound to the interface of a view model for the purpose of forms. Given this, when the `TopicMappingService` processes the `MetadataKey`, it needs to ensure that the key's `TopicPayload.Children` are properly loaded so they're available to be bound. This patches another gap in the `TopicMappingService` (#118) in response to the lazy-loading implementation (#111). --- OnTopic/Mapping/TopicMappingService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index 7adece04..4c335a5a 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -872,7 +872,7 @@ sourcePropertyValue[0] is Topic \-------------------------------------------------------------------------------------------------------------------------*/ if (listSource.Count == 0 && !String.IsNullOrWhiteSpace(configuration.MetadataKey)) { var metadataKey = $"Root:Configuration:Metadata:{configuration.MetadataKey}:LookupList"; - var metadataParent = await _topicRepository.Load(metadataKey, source).ConfigureAwait(false); + var metadataParent = await _topicRepository.Load(metadataKey, source, TopicPayload.Children).ConfigureAwait(false); if (metadataParent is not null) { listSource = [.. metadataParent.Children]; } From f287c1d181b7066773116dc493e50950fe21ae41 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Thu, 30 Jul 2026 19:00:36 -0700 Subject: [PATCH 08/50] Prevent duplicate mapping of properties This fixes a preexisting bug where, if the same view model ends up being mapped twice with different requirements, the scalar properties were remapped. This doesn't trigger a reload since any extended attributes would have been loaded the first time around, but it does waste CPU cycles by remapping existing properties that aren't relevant to this round. This happens because for collections and references, attributes can limit which collection properties, if any, get mapped, to allow view models to be reused while limiting their potential to keep triggering mapping of associations that aren't needed. So if one reference or collection disables the mapping of associations, but then a subsequent reference or collection maps the _same_ view model on the _same_ topic, but includes an `[Include]` attribute including associations, we don't need to remap the properties that were already mapped in the first round. This is mitigated by adding a `mapAssociationsOnly` argument to `SetCollectionValueAsync()` and `GetSourceCollectionAsync()` and setting them appropriately on these redundant checks. This bug existed before the lazy-loading (#111) and doesn't contribute to any concurrency issues due to the collections being marked `LoadState.Loaded` after the initial call, and thus this isn't strictly required by #118, but it's good to fix while we're here! --- OnTopic/Mapping/TopicMappingService.cs | 27 ++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index 4c335a5a..d6358833 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -423,7 +423,7 @@ private async Task MapAsync( \-------------------------------------------------------------------------------------------------------------------------*/ async Task getList(Type targetType) { - var sourceList = await GetSourceCollectionAsync(source, associations, parameter, attributePrefix).ConfigureAwait(false); + var sourceList = await GetSourceCollectionAsync(source, associations, parameter, attributePrefix, false).ConfigureAwait(false); var targetList = InitializeCollection(targetType); if (targetList is null) { @@ -496,7 +496,7 @@ await MapAsync( else { var value = await GetValue(source, propertyAccessor.Type, associations, propertyAccessor, cache, attributePrefix, mapAssociationsOnly).ConfigureAwait(false); if (value is null && propertyAccessor.IsList) { - await SetCollectionValueAsync(source, target, associations, propertyAccessor, cache, attributePrefix).ConfigureAwait(false); + await SetCollectionValueAsync(source, target, associations, propertyAccessor, cache, attributePrefix, mapAssociationsOnly).ConfigureAwait(false); } else if (value != null && propertyAccessor.CanWrite) { propertyAccessor.SetValue(target, value, true); @@ -549,7 +549,7 @@ await MapAsync( /*-------------------------------------------------------------------------------------------------------------------------- | Handle by type, attribute \-------------------------------------------------------------------------------------------------------------------------*/ - if (TryGetCompatibleProperty(source, targetType, itemMetadata, attributePrefix, out var compatibleValue)) { + if (!mapAssociationsOnly && TryGetCompatibleProperty(source, targetType, itemMetadata, attributePrefix, out var compatibleValue)) { value = compatibleValue; } else if (itemMetadata.IsConvertible) { @@ -729,13 +729,15 @@ await MapAsync( /// The with details about the property's attributes. /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. + /// Determines if properties not associated with associations should be mapped. private async Task SetCollectionValueAsync( Topic source, object target, AssociationTypes associations, MemberAccessor memberAccessor, MappedTopicCache cache, - string? attributePrefix + string? attributePrefix, + bool mapAssociationsOnly ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -756,7 +758,7 @@ private async Task SetCollectionValueAsync( /*-------------------------------------------------------------------------------------------------------------------------- | Establish source collection to store topics to be mapped \-------------------------------------------------------------------------------------------------------------------------*/ - var sourceList = await GetSourceCollectionAsync(source, associations, memberAccessor, attributePrefix).ConfigureAwait(false); + var sourceList = await GetSourceCollectionAsync(source, associations, memberAccessor, attributePrefix, mapAssociationsOnly).ConfigureAwait(false); /*-------------------------------------------------------------------------------------------------------------------------- | Validate that source collection was identified @@ -788,11 +790,13 @@ private async Task SetCollectionValueAsync( /// Determines what associations the mapping should include, if any. /// The with details about the property's attributes. /// The prefix to apply to the attributes. + /// Determines if properties not associated with associations should be mapped. private async Task> GetSourceCollectionAsync( Topic source, AssociationTypes associations, ItemMetadata itemMetadata, - string? attributePrefix + string? attributePrefix, + bool mapAssociationsOnly ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -850,9 +854,11 @@ private async Task> GetSourceCollectionAsync( \-------------------------------------------------------------------------------------------------------------------------*/ //The following allows a target collection to be mapped to an IList source collection. This is valuable for custom, //curated collections defined on e.g. derivatives of Topic, but which don't otherwise map to a specific collection type. - //For example, the ContentTypeDescriptor's AttributeDescriptors collection, which provides a rollup of - //AttributeDescriptors from the current ContentTypeDescriptor, as well as all of its ascendents. - if (listSource.Count == 0) { + //For example, the ContentTypeDescriptor's AttributeDescriptors collection, which provides a rollup of AttributeDescriptors + //from the current ContentTypeDescriptor, as well as all of its ascendants. On an expansion pass, this fallback runs only + //when MappedCollections is among the claimed associations, avoiding a redundant reflective source-property read (and its + //re-enumeration) for passes that don't claim it. + if (listSource.Count == 0 && (!mapAssociationsOnly || associations.HasFlag(AssociationTypes.MappedCollections))) { var sourceProperty = TypeAccessorCache.GetTypeAccessor(source.GetType()).GetMember(configuration.GetCompositeAttributeKey(attributePrefix)); if ( sourceProperty?.GetValue(source) is IList sourcePropertyValue && @@ -870,7 +876,7 @@ sourcePropertyValue[0] is Topic /*-------------------------------------------------------------------------------------------------------------------------- | Handle Metadata relationship \-------------------------------------------------------------------------------------------------------------------------*/ - if (listSource.Count == 0 && !String.IsNullOrWhiteSpace(configuration.MetadataKey)) { + if (!mapAssociationsOnly && listSource.Count == 0 && !String.IsNullOrWhiteSpace(configuration.MetadataKey)) { var metadataKey = $"Root:Configuration:Metadata:{configuration.MetadataKey}:LookupList"; var metadataParent = await _topicRepository.Load(metadataKey, source, TopicPayload.Children).ConfigureAwait(false); if (metadataParent is not null) { @@ -896,6 +902,7 @@ IList getCollection(CollectionType collection, Func contain var targetAssociations = AssociationMap.Mappings[collection]; var preconditionsMet = listSource.Count == 0 && + (!mapAssociationsOnly || targetAssociations is not AssociationTypes.None) && (collectionType is CollectionType.Any || collectionType.Equals(collection)) && (collectionType is CollectionType.Children || collection is not CollectionType.Children) && (targetAssociations is AssociationTypes.None || associations.HasFlag(targetAssociations)) && From ec102ab208d399ab800f90cd377b9414ea6d2f5a Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Sat, 1 Aug 2026 15:45:25 -0700 Subject: [PATCH 09/50] Introduced view models to test expanded maps These two test view models help evaluate the scenario where an already mapped view model in an association is remapped with additional `[include()]` calls, a scenario which previously resulted in the "compatible" properties" being mapped twice, and as fixed in the previous commit (f287c1d1). This will be used in a subsequent unit test to verify the fix. This bug existed before the lazy-loading (#111) and doesn't contribute to any concurrency issues due to the collections being marked `LoadState.Loaded` after the initial call, and thus this isn't strictly required by #118. --- .../TestDoubles/FakeViewModelLookupService.cs | 2 + .../ExpansionParentTopicViewModel.cs | 57 +++++++++++++++ .../ExpansionSharedTopicViewModel.cs | 73 +++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 OnTopic.Tests/ViewModels/ExpansionParentTopicViewModel.cs create mode 100644 OnTopic.Tests/ViewModels/ExpansionSharedTopicViewModel.cs diff --git a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs index fc379c2d..1f24a195 100644 --- a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs +++ b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs @@ -41,6 +41,8 @@ public FakeViewModelLookupService() { Add(typeof(DescendentSpecializedTopicViewModel)); Add(typeof(DescendentTopicViewModel)); Add(typeof(DisableMappingTopicViewModel)); + Add(typeof(ExpansionParentTopicViewModel)); + Add(typeof(ExpansionSharedTopicViewModel)); Add(typeof(FallbackViewModel)); Add(typeof(FilteredTopicViewModel)); Add(typeof(FlattenChildrenTopicViewModel)); diff --git a/OnTopic.Tests/ViewModels/ExpansionParentTopicViewModel.cs b/OnTopic.Tests/ViewModels/ExpansionParentTopicViewModel.cs new file mode 100644 index 00000000..49f15956 --- /dev/null +++ b/OnTopic.Tests/ViewModels/ExpansionParentTopicViewModel.cs @@ -0,0 +1,57 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: EXPANSION PARENT +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a view model whose two collections both map the same source topic to , +/// but request disjoint associations so that mapping it can exercise an association expansion pass. +/// +/// +/// +/// The relationship and the collection are populated from the same source +/// topic and mapped to the same instance, but request disjoint associations +/// ( and ). Neither association maps +/// anything on , which has no association-typed members: The disjoint requests +/// exist only so the cache sees the second encounter as missing an association and runs an expansion pass, rather than +/// returning the cached instance unchanged. What that expansion pass must not do is redo the target's non-association work. +/// +/// +/// The disjointness is all this view model contributes, and only conditionally: If one encounter finds the instance the +/// other already cached, that encounter has a missing association. Whether the encounters actually resolve that way, as an +/// ordered initial pass followed by a cache-hit expansion pass rather than two concurrent initial passes, is a property +/// of the mapping runtime, not of this view model. The tests that depend on the ordered outcome document why it holds for +/// them. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +public class ExpansionParentTopicViewModel { + + /*============================================================================================================================ + | PROPERTY: RELATED + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// A relationship collection that reaches the shared topic while requesting only . + /// + [Collection("Related", Type = CollectionType.Relationship)] + [Include(AssociationTypes.Children)] + public Collection Related { get; } = new(); + + /*============================================================================================================================ + | PROPERTY: CHILDREN + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// A children collection that reaches the shared topic while requesting only . + /// + [Include(AssociationTypes.Relationships)] + public Collection Children { get; } = new(); + +} //Class \ No newline at end of file diff --git a/OnTopic.Tests/ViewModels/ExpansionSharedTopicViewModel.cs b/OnTopic.Tests/ViewModels/ExpansionSharedTopicViewModel.cs new file mode 100644 index 00000000..3c3efbcb --- /dev/null +++ b/OnTopic.Tests/ViewModels/ExpansionSharedTopicViewModel.cs @@ -0,0 +1,73 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: EXPANSION SHARED +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a view model that encounters more than once during a single mapping +/// operation, so that the second encounter triggers an association expansion pass (mapAssociationsOnly), while the +/// first only includes the properties. +/// +/// +/// +/// This has no association-typed members by design. Its content is , an ungated (gate ) nested-topics collection, and , a "compatible" property (i.e., mapped +/// directly from a first-class property on ). Unlike a gated association, which the cache claims once +/// and its flag check then skips on later passes, neither of these is tied to an association, so an expansion pass would +/// redundantly remap both unless the mapper explicitly skips non-association work. records how +/// many times is assigned, so a test can confirm the compatible property is not reassigned again during +/// the expansion pass. +/// +/// +/// This is only reachable in tandem with , whose two collections perform the +/// initial and expansion passes against a single, cached instance of this view model. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +public class ExpansionSharedTopicViewModel { + + /*============================================================================================================================ + | PRIVATE VARIABLES + \---------------------------------------------------------------------------------------------------------------------------*/ + private int _keyMapCount; + + /*============================================================================================================================ + | PROPERTY: KEY + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// A compatible property, mapped one-to-one from the source . Records each assignment via . + /// + public string? Key { + get; + set { + field = value; + _keyMapCount++; + } + } + + /*============================================================================================================================ + | PROPERTY: KEY MAP COUNT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// The number of times has been assigned by the mapping service. + /// + public int KeyMapCount => _keyMapCount; + + /*============================================================================================================================ + | PROPERTY: CATEGORIES + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// An ungated nested-topics collection, mapped from the source topic's nested Categories container. + /// + public Collection Categories { get; } = new(); + +} //Class \ No newline at end of file From 49a604890b127b18eb6e2795400e071bd1d1f045 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Sat, 1 Aug 2026 16:01:22 -0700 Subject: [PATCH 10/50] Added unit tests to confirm properties mapped once This test evaluates the scenario where an already mapped view model in an association is remapped with additional `[include()]` calls, a scenario which previously resulted in the "compatible" properties" being mapped twice, and as fixed in the previous commit (f287c1d1). This relies on the two newly introduced view models (ec102ab2) to ensure the circumstances are met. This bug existed before the lazy-loading (#111) and doesn't contribute to any concurrency issues due to the collections being marked `LoadState.Loaded` after the initial call, and thus this isn't strictly required by #118. --- OnTopic.Tests/TopicMappingServiceTest.cs | 72 ++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 86403e82..3b0874ec 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -1363,6 +1363,78 @@ public async Task Map_CachedTopic_ReturnsProgressiveReference() { } + /*============================================================================================================================ + | TEST: MAP: EXPANSION PASS: DOES NOT DUPLICATE NESTED TOPICS + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Maps an , which encounters the same source topic twice with disjoint + /// associations, confirming that the second (expansion) pass does not re-append the ungated nested-topics collection that + /// the initial pass already populated. + /// + /// + /// Reliability rests on the eager repository mapping the two collections sequentially, so the encounters are strictly + /// ordered: The first builds and fills the cached view model, and the second, requesting a disjoint association, hits the + /// cache and runs an expansion pass (rather than a second, concurrent initial pass). is then filled once by the ungated nested-topics probe on the initial pass + /// and skipped on the expansion pass, so the count is 2 whichever collection reflection maps first. + /// + [Fact] + public async Task Map_ExpansionPass_DoesNotDuplicateNestedTopics() { + + var parent = new Topic("Parent", "ExpansionParent", null, 700); + var shared = new Topic("Shared", "ExpansionShared", parent, 701); + var categories = new Topic("Categories", "List", shared, 702); + _ = new Topic("Category1", "KeyOnly", categories, 703); + _ = new Topic("Category2", "KeyOnly", categories, 704); + + parent.Relationships.SetValue("Related", shared); + + var target = await _mappingService.MapAsync(parent); + var mappedShared = target?.Children.FirstOrDefault(); + + //Assert.Same confirms both collections resolved to the same cached instance, so the second reach was a cache hit and, given + //the disjoint associations, ran an expansion pass + Assert.NotNull(mappedShared); + Assert.Same(mappedShared, target?.Related.FirstOrDefault()); + Assert.Equal(2, mappedShared.Categories.Count); + + } + + /*============================================================================================================================ + | TEST: MAP: EXPANSION PASS: DOES NOT REMAP COMPATIBLE PROPERTY + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Maps an , which encounters the same source topic twice with disjoint + /// associations, and confirms that the second (expansion) pass does not reassign the compatible property that the initial pass already mapped. + /// + /// + /// Reliability rests on the eager repository mapping the two collections sequentially, so the encounters are strictly + /// ordered: The first builds and fills the cached view model, and the second, requesting a disjoint association, hits the + /// cache and runs an expansion pass (rather than a second, concurrent initial pass). The compatible is then assigned once on the initial pass and skipped on the expansion pass, so + /// is 1 whichever collection reflection maps first. + /// + [Fact] + public async Task Map_ExpansionPass_DoesNotRemapCompatibleProperty() { + + var parent = new Topic("Parent", "ExpansionParent", null, 710); + var shared = new Topic("Shared", "ExpansionShared", parent, 711); + + parent.Relationships.SetValue("Related", shared); + + var target = await _mappingService.MapAsync(parent); + var mappedShared = target?.Children.FirstOrDefault(); + + //Assert.Same confirms both collections resolved to the same cached instance, so the second reach was a cache hit and, given + //the disjoint associations, ran an expansion pass rather than passing vacuously + Assert.NotNull(mappedShared); + Assert.Same(mappedShared, target?.Related.FirstOrDefault()); + Assert.Equal("Shared", mappedShared.Key); + Assert.Equal(1, mappedShared.KeyMapCount); + + } + /*============================================================================================================================ | TEST: MAP: CIRCULAR REFERENCE: RETURNS MAPPED PARENT \---------------------------------------------------------------------------------------------------------------------------*/ From 0f512648e25c4308f1a4a62c05e868d6204c4102 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Sat, 1 Aug 2026 17:40:22 -0700 Subject: [PATCH 11/50] Made `AddMissingAssociations()` concurrent Added a lock to the `AddMissingAssociations()` and, importantly, now return what the newly added associations are, so that the caller only need to process those that are new. This ensures that if there are two overlapping calls, they won't end up processing the same associations. This bug existed before the lazy-loading (#111), but addresses a concurrency issue already exposed by the `TopicMappingService` (#118). --- .../Mapping/Internal/MappedTopicCacheEntry.cs | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs b/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs index e37fb3b6..794dcdea 100644 --- a/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs +++ b/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs @@ -15,14 +15,20 @@ namespace OnTopic.Mapping.Internal; /// /// /// In addition to the actual , this also includes a property for -/// tracking what associations were mapped to the . This allows the to be update the cached object with any missing associations, which can be identified using the method. In turn, the cache can then be updated to reflect those new -/// associations by using . This ensures that even if a topic has -/// already been mapped, its scope can be expanded without duplicating effort. +/// tracking what associations were mapped to the . This allows the +/// to expand the cached object with any missing associations. A caller may peek at the missing associations using the +/// method, or record them and receive the newly added subset in a +/// single atomic operation using , so that concurrent passes don't both +/// end up mapping the same associations. This ensures that even if a topic has already been mapped, its scope can be expanded +/// without duplicating effort. /// internal sealed class MappedTopicCacheEntry { + /*============================================================================================================================ + | PRIVATE VARIABLES + \---------------------------------------------------------------------------------------------------------------------------*/ + private readonly object _lock = new(); + /*============================================================================================================================ | PROPERTY: MAPPED TOPIC \---------------------------------------------------------------------------------------------------------------------------*/ @@ -61,15 +67,33 @@ internal sealed class MappedTopicCacheEntry { /// Given a target , identifies any associations not covered by /// and returns them as a new instance. /// + /// + /// This is intended as a quick hint to decide e.g., whether an expansion is needed at all, without any side effects. It + /// does not record the result; a caller that intends to map the missing associations should instead use , so that concurrent passes cannot both map the same associations. + /// internal AssociationTypes GetMissingAssociations(AssociationTypes associations) => Associations ^ (associations | Associations); /*============================================================================================================================ | METHOD: ADD MISSING ASSOCIATIONS \---------------------------------------------------------------------------------------------------------------------------*/ /// - /// Given a target , adds any missing to the property. + /// Given a target , adds any not already covered by and returns + /// the subset that this call just added. /// - internal void AddMissingAssociations(AssociationTypes associations) => Associations = associations | Associations; + /// + /// This is the mutating counterpart to : It adds any associations + /// that aren't already covered, and reports which ones were added back to the caller so the caller knows which associations + /// to process. Because the delta is calculated and saved under a single lock, two concurrent passes over the same cached + /// instance receive disjoint results, ensuring each association is mapped by exactly one caller. A caller that receives + /// has nothing left to map and should return the cached instance. + /// + internal AssociationTypes AddMissingAssociations(AssociationTypes associations) { + lock (_lock) { + var missing = GetMissingAssociations(associations); + Associations = associations | Associations; + return missing; + } + } } //Class \ No newline at end of file From a971aeb84e7da4835468e9d702c3d29c2ac133ca Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Sat, 1 Aug 2026 17:45:11 -0700 Subject: [PATCH 12/50] Applied `AddMissingAssociations()` to caller This utilizes the new return value from the updated `AddMissingAssociations()` method on the `MappedTopicCacheEntry` to ensure that it's only mapping still missing associations, acknowledging that a concurrent call may have already began mapping some that it has previously identified. This bug existed before the lazy-loading (#111), but addresses a concurrency issue already exposed by the `TopicMappingService` (#118). --- OnTopic/Mapping/TopicMappingService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index d6358833..9d64eba5 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -326,15 +326,15 @@ private async Task MapAsync( | Handle cached objects >------------------------------------------------------------------------------------------------------------------------- | If the cache contains an entry, check to make sure it includes all of the requested associations. If it does, return it. - | If it doesn't, determine the missing associations and request to have those mapped. + | Otherwise, add the missing associations to the cache entry and map only the subset this pass added, so that a concurrent + | pass doesn't remap the same associations. \-------------------------------------------------------------------------------------------------------------------------*/ if (cache.TryGetValue(topic.Id, target.GetType(), out var cacheEntry)) { - associations = cacheEntry.GetMissingAssociations(associations); + associations = cacheEntry.AddMissingAssociations(associations); target = cacheEntry.MappedTopic; if (associations is AssociationTypes.None) { return cacheEntry.MappedTopic; } - cacheEntry.AddMissingAssociations(associations); } else if (!topic.IsNew) { cache.Register( From bc73f876ebc600cb65298435afcf70c176dea09f Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Sat, 1 Aug 2026 17:48:03 -0700 Subject: [PATCH 13/50] Update unit tests for `AddMissingAssociations()` This updates the existing unit tests for `AddMissingAssociations()` to account for the concurrency fix (0f512648, a971aeb8) related to #118. --- OnTopic.Tests/TopicMappingServiceTest.cs | 26 +++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 3b0874ec..8a8fd20f 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -761,8 +761,6 @@ public void MappedTopicCacheEntry_GetMissingAssociations_ReturnsDifference() { var difference = cacheEntry.GetMissingAssociations(associations); - cacheEntry.AddMissingAssociations(difference); - Assert.True(difference.HasFlag(AssociationTypes.References)); Assert.False(difference.HasFlag(AssociationTypes.Children)); Assert.False(difference.HasFlag(AssociationTypes.Parents)); @@ -770,24 +768,32 @@ public void MappedTopicCacheEntry_GetMissingAssociations_ReturnsDifference() { } /*============================================================================================================================ - | TEST: MAPPED TOPIC CACHE ENTRY: ADD MISSING ASSOCIATIONS: SETS UNION + | TEST: MAPPED TOPIC CACHE ENTRY: ADD MISSING ASSOCIATIONS: RETURNS NEWLY ADDED \---------------------------------------------------------------------------------------------------------------------------*/ /// - /// Establishes a with a set of , and then confirms that - /// its correctly extends the missing - /// associations. + /// Establishes a and then confirms that two overlapping calls to its return disjoint flags (each reporting only what it + /// newly added) whose union is the missing set, and that the recorded + /// reflect both calls. /// + /// + /// Each association may only be added once. Even though both requests include , + /// only the first call adds it; the second call sees it as already recorded and returns only the remainder. This is what + /// ensures two concurrent passes map disjoint associations rather than both mapping the overlap. + /// [Fact] - public void MappedTopicCacheEntry_AddMissingAssociations_SetsUnion() { + public void MappedTopicCacheEntry_AddMissingAssociations_ReturnsNewlyAdded() { var cacheEntry = new MappedTopicCacheEntry() { Associations = AssociationTypes.Children }; - var associations = AssociationTypes.Children | AssociationTypes.Parents; - cacheEntry.AddMissingAssociations(associations); + var firstResult = cacheEntry.AddMissingAssociations(AssociationTypes.Children | AssociationTypes.Parents); + var secondResult = cacheEntry.AddMissingAssociations(AssociationTypes.Parents | AssociationTypes.References); - Assert.Equal(AssociationTypes.Children | AssociationTypes.Parents, cacheEntry.Associations); + Assert.Equal(AssociationTypes.Parents, firstResult); + Assert.Equal(AssociationTypes.References, secondResult); + Assert.Equal(AssociationTypes.Children | AssociationTypes.Parents | AssociationTypes.References, cacheEntry.Associations); } From b42673a4f1e0d391475f056ad6b48a3e2660cfee Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 16:19:03 -0700 Subject: [PATCH 14/50] Introduce new `MapPath` class The `MapPath` class will track the chain of a `MapAsync()` process (via the `Parent` property) to determine what topic (via the `TopicId` property) and mapped view model type (via the `Type` property) are in the process of being mapped as part of this specific request, this allowing us to determine if there's a circular reference. This allows constructor mapping (aside from `AttributeDictionary`; #99) to properly map e.g., positional constructors on records, a current capability (#35). This allows us to differentiate between true circular loops within a constructor (which can't be supported) and sibling mappings (which are valid), with the latter now introducing potential concurrency issues (#118) because lazy loading (#111) makes construction suspend on an awaited `Load()`, where this previously completed synchronously, running each branch to completion before the next. This will allow multiple branches to construct the same topic and view model at once, an acceptable overlap `MapPath()` will now be able to distinguish from a circular loop. --- OnTopic/Mapping/Internal/MapPath.cs | 71 +++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 OnTopic/Mapping/Internal/MapPath.cs diff --git a/OnTopic/Mapping/Internal/MapPath.cs b/OnTopic/Mapping/Internal/MapPath.cs new file mode 100644 index 00000000..1403fabd --- /dev/null +++ b/OnTopic/Mapping/Internal/MapPath.cs @@ -0,0 +1,71 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ + +namespace OnTopic.Mapping.Internal; + +/*============================================================================================================================== +| CLASS: MAP PATH +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Represents a single frame in the depth-first path of an in-progress mapping operation, tracking the +/// and target currently being constructed, along with a reference to the frame that preceded it. +/// +/// +/// The allows the to distinguish a genuine constructor cycle, in +/// which a topic is mapped to a type that is already being constructed higher up the same call chain, from sibling +/// concurrency, in which two independent branches happen to map the same topic to the same type at the same time. The former +/// is a true circular reference and must throw; the latter is benign, and the joiner should await the in-progress result. +/// +/// The of the topic being mapped at this frame. +/// The target being constructed at this frame. +/// The preceding frame, or null if this frame is the path root. +internal sealed class MapPath(int topicId, Type type, MapPath? parent) { + + /*============================================================================================================================ + | PROPERTY: TOPIC ID + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// The of the topic being mapped at this frame. + /// + internal int TopicId { get; } = topicId; + + /*============================================================================================================================ + | PROPERTY: TYPE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// The target being constructed at this frame. + /// + internal Type Type { get; } = type; + + /*============================================================================================================================ + | PROPERTY: PARENT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// The preceding frame, or null if this frame is the root of the path. + /// + internal MapPath? Parent { get; } = parent; + + /*============================================================================================================================ + | METHOD: CONTAINS + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Determines whether the supplied and pair already appears anywhere on + /// the current path, indicating a constructor cycle. + /// + /// The to search for. + /// The target to search for. + /// Returns true if the pair is already on the path, and otherwise false. + internal bool Contains(int topicId, Type type) { + // Walk up the parent chain, comparing each frame against the requested pair + for (var frame = this; frame is not null; frame = frame.Parent) { + if (frame.TopicId == topicId && frame.Type == type) { + return true; + } + } + return false; + } + +} //Class \ No newline at end of file From 7a5bd2c2afd3a96e8ad89624f78cca0c2702196e Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 18:06:49 -0700 Subject: [PATCH 15/50] Added completion semantics to mapped cache entries Added the `Completion` property (similar to e.g., `IDataflowBlock` and `ChannelReader` in the BCL) to `MappedTopicCacheEntry` so that a concurrent request can `await` the construction of the initial entry instead of duplicating effort and, potentially, causing an error. This includes a `Complete()` method as well as a `Fault()` method. This will be used to ensure that one topic can be concurrently mapped to the same view model without interference (#118). --- .../Mapping/Internal/MappedTopicCacheEntry.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs b/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs index 794dcdea..b5fae15e 100644 --- a/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs +++ b/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs @@ -28,6 +28,7 @@ internal sealed class MappedTopicCacheEntry { | PRIVATE VARIABLES \---------------------------------------------------------------------------------------------------------------------------*/ private readonly object _lock = new(); + private readonly TaskCompletionSource _completionSource = new(TaskCreationOptions.RunContinuationsAsynchronously); /*============================================================================================================================ | PROPERTY: MAPPED TOPIC @@ -60,6 +61,24 @@ internal sealed class MappedTopicCacheEntry { /// internal AssociationTypes Associations { get; set; } = AssociationTypes.None; + /*============================================================================================================================ + | PROPERTY: COMPLETION + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Returns a that completes once the has been constructed and registered, even + /// if not all properties have yet been mapped, thus allowing a second pass to await a first pass that is still initializing + /// the same entry, instead of duplicating its work or, worse yet, failing. + /// + /// + /// The task is settled by once the target has been constructed, or by + /// if construction throws, so a second pass awaiting it never hangs. It is settled during + /// registration, after the constructor runs but before the first pass maps the target's properties, so a second pass may + /// observe an instance whose constructor parameters are set but whose properties are not yet mapped. This early publication + /// is what allows a property-level circular reference to resolve to the cached (if partially populated) instance instead of + /// recursing indefinitely, while still catching constructor-level circular references. + /// + internal Task Completion => _completionSource.Task; + /*============================================================================================================================ | METHOD: GET MISSING ASSOCIATIONS \---------------------------------------------------------------------------------------------------------------------------*/ @@ -96,4 +115,40 @@ internal AssociationTypes AddMissingAssociations(AssociationTypes associations) } } + /*============================================================================================================================ + | METHOD: COMPLETE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Publishes the constructed and its as the entry's and , and settles the task, releasing any second pass + /// awaiting this entry. + /// + /// + /// This is the sole writer of and the initial writer of , so the + /// instance, its associations, and the completion signal are always published together under a single lock. Only the first + /// completion takes effect; a later duplicate registration is ignored, keeping the cached instance stable, while remaining + /// unobservable before the entry is completed. + /// + /// The constructed view model associated with the entry. + /// The associations that the view model was mapped with. + internal void Complete(object viewModel, AssociationTypes associations) { + lock (_lock) { + if (!_completionSource.Task.IsCompleted) { + MappedTopic = viewModel; + Associations = associations; + _completionSource.TrySetResult(); + } + } + } + + /*============================================================================================================================ + | METHOD: FAULT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Faults the task with the supplied so that any pass awaiting the + /// entry observes the failure instead of hanging when construction of the entry throws. + /// + /// The exception that occurred while constructing the entry. + internal void Fault(Exception exception) => _completionSource.TrySetException(exception); + } //Class \ No newline at end of file From 61b1487c78aeee203d9f360851ea8e9b342fd6ef Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 19:00:30 -0700 Subject: [PATCH 16/50] Added `MapPath` to `MapAsync()` chain This adds the new `MapPath` data object (b42673a4) to the private `MapAsync()` overloads as well as their entire call chain (i.e., `GetParameterAsync()`, `GetTopicReferenceAsync()`, `GetValue()`, `SetPropertyAsync()`, `SetCollectionValueAsync()`, `PopulateTargetCollectionAsync()`). Currently, this doesn't _do_ anything, though once implemented, this will allow us to differentiate between true circular loops within a constructor (which can't be supported) and sibling mappings (which are valid), with the latter now introducing potential concurrency issues (#118) since lazy loading (#111) makes construction suspend on an awaited `Load()`, where this previously completed synchronously, running each branch to completion before the next. This will allow multiple branches to construct the same topic and view model at once, an acceptable overlap `MapPath()` will now be able to distinguish from a true circular loop. --- OnTopic/Mapping/TopicMappingService.cs | 66 +++++++++++++++++--------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index 9d64eba5..1e1e4c0e 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -69,12 +69,14 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe /// Determines what associations the mapping should include, if any. /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. + /// The current mapping request's path, used to detect circular references during construction. /// An instance of the dynamically determined View Model with properties appropriately mapped. private async Task MapAsync( Topic? topic, AssociationTypes associations, MappedTopicCache cache, - string? attributePrefix = null + string? attributePrefix = null, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -99,7 +101,7 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe /*-------------------------------------------------------------------------------------------------------------------------- | Perform mapping \-------------------------------------------------------------------------------------------------------------------------*/ - return await MapAsync(topic, viewModelType, associations, cache, attributePrefix).ConfigureAwait(false); + return await MapAsync(topic, viewModelType, associations, cache, attributePrefix, mapPath).ConfigureAwait(false); } @@ -128,13 +130,15 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe /// Determines what associations the mapping should include, if any. /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. + /// The current mapping request's path, used to detect circular references during construction. /// An instance of the dynamically determined View Model with properties appropriately mapped. private async Task MapAsync( Topic? topic, Type type, AssociationTypes associations, MappedTopicCache cache, - string? attributePrefix = null + string? attributePrefix = null, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -251,7 +255,7 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe foreach (var property in typeAccessor.GetMembers(MemberTypes.Property)) { if (!mappedParameters.Contains(property.Name, StringComparer.OrdinalIgnoreCase)) { - propertyQueue.Add(SetPropertyAsync(topic, target, associations, property, cache, attributePrefix, false)); + propertyQueue.Add(SetPropertyAsync(topic, target, associations, property, cache, attributePrefix, false, mapPath)); } } @@ -292,6 +296,7 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe /// Determines what associations the mapping should include, if any. /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. + /// The current mapping request's path, used to detect circular references during construction. /// /// This internal version passes a private cache of mapped objects from this run. This helps prevent problems with /// recursion in case is referred to multiple times (e.g., a Children collection with MapAsync( object target, AssociationTypes associations, MappedTopicCache cache, - string? attributePrefix = null + string? attributePrefix = null, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -351,7 +357,7 @@ private async Task MapAsync( var typeAccessor = TypeAccessorCache.GetTypeAccessor(target.GetType()); foreach (var property in typeAccessor.GetMembers(MemberTypes.Property)) { - taskQueue.Add(SetPropertyAsync(topic, target, associations, property, cache, attributePrefix, cacheEntry is not null)); + taskQueue.Add(SetPropertyAsync(topic, target, associations, property, cache, attributePrefix, cacheEntry is not null, mapPath)); } await Task.WhenAll([.. taskQueue]).ConfigureAwait(false); @@ -374,12 +380,14 @@ private async Task MapAsync( /// Information related to the current parameter. /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. + /// The current mapping request's path, used to detect circular references during construction. private async Task GetParameterAsync( Topic source, AssociationTypes associations, ParameterMetadata parameter, MappedTopicCache cache, - string? attributePrefix = null + string? attributePrefix = null, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -403,14 +411,15 @@ private async Task MapAsync( parameter.Type, associations, cache, - configuration.AttributePrefix + attributePrefix + configuration.AttributePrefix + attributePrefix, + mapPath ).ConfigureAwait(false); } /*-------------------------------------------------------------------------------------------------------------------------- | Determine value \-------------------------------------------------------------------------------------------------------------------------*/ - var value = await GetValue(source, parameter.Type, associations, parameter, cache, attributePrefix, false).ConfigureAwait(false); + var value = await GetValue(source, parameter.Type, associations, parameter, cache, attributePrefix, false, mapPath).ConfigureAwait(false); if (value is null && parameter.IsList) { return await getList(parameter.Type).ConfigureAwait(false); @@ -430,7 +439,7 @@ private async Task MapAsync( return null; } - await PopulateTargetCollectionAsync(sourceList, targetList, parameter, cache).ConfigureAwait(false); + await PopulateTargetCollectionAsync(sourceList, targetList, parameter, cache, mapPath).ConfigureAwait(false); return targetList; @@ -452,6 +461,7 @@ private async Task MapAsync( /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. /// Determines if properties not associated with associations should be mapped. + /// The current mapping request's path, used to detect circular references during construction. private async Task SetPropertyAsync( Topic source, object target, @@ -459,7 +469,8 @@ private async Task SetPropertyAsync( MemberAccessor propertyAccessor, MappedTopicCache cache, string? attributePrefix = null, - bool mapAssociationsOnly = false + bool mapAssociationsOnly = false, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -485,7 +496,8 @@ await MapAsync( targetProperty, associations, cache, - configuration.AttributePrefix + attributePrefix + configuration.AttributePrefix + attributePrefix, + mapPath ).ConfigureAwait(false); } } @@ -494,9 +506,9 @@ await MapAsync( | Determine value \-------------------------------------------------------------------------------------------------------------------------*/ else { - var value = await GetValue(source, propertyAccessor.Type, associations, propertyAccessor, cache, attributePrefix, mapAssociationsOnly).ConfigureAwait(false); + var value = await GetValue(source, propertyAccessor.Type, associations, propertyAccessor, cache, attributePrefix, mapAssociationsOnly, mapPath).ConfigureAwait(false); if (value is null && propertyAccessor.IsList) { - await SetCollectionValueAsync(source, target, associations, propertyAccessor, cache, attributePrefix, mapAssociationsOnly).ConfigureAwait(false); + await SetCollectionValueAsync(source, target, associations, propertyAccessor, cache, attributePrefix, mapAssociationsOnly, mapPath).ConfigureAwait(false); } else if (value != null && propertyAccessor.CanWrite) { propertyAccessor.SetValue(target, value, true); @@ -523,6 +535,7 @@ await MapAsync( /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. /// Determines if properties not associated with associations should be mapped. + /// The current mapping request's path, used to detect circular references during construction. private async Task GetValue( Topic source, Type targetType, @@ -530,7 +543,8 @@ await MapAsync( ItemMetadata itemMetadata, MappedTopicCache cache, string? attributePrefix = "", - bool mapAssociationsOnly = false + bool mapAssociationsOnly = false, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -562,7 +576,7 @@ await MapAsync( } else if (configuration.GetCompositeAttributeKey(attributePrefix) is "Parent") { if (associations.HasFlag(AssociationTypes.Parents) && source.Parent is not null) { - value = await GetTopicReferenceAsync(source.Parent, targetType, itemMetadata, cache).ConfigureAwait(false); + value = await GetTopicReferenceAsync(source.Parent, targetType, itemMetadata, cache, mapPath).ConfigureAwait(false); } } else if (configuration.MapToParent) { @@ -571,7 +585,7 @@ await MapAsync( else if (itemMetadata.Type.IsClass && associations.HasFlag(AssociationTypes.References)) { var topicReference = await getTopicReference().ConfigureAwait(false); if (topicReference is not null) { - value = await GetTopicReferenceAsync(topicReference, targetType, itemMetadata, cache).ConfigureAwait(false); + value = await GetTopicReferenceAsync(topicReference, targetType, itemMetadata, cache, mapPath).ConfigureAwait(false); } } @@ -730,6 +744,7 @@ await MapAsync( /// A cache to keep track of already-mapped object instances. /// The prefix to apply to the attributes. /// Determines if properties not associated with associations should be mapped. + /// The current mapping request's path, used to detect circular references during construction. private async Task SetCollectionValueAsync( Topic source, object target, @@ -737,7 +752,8 @@ private async Task SetCollectionValueAsync( MemberAccessor memberAccessor, MappedTopicCache cache, string? attributePrefix, - bool mapAssociationsOnly + bool mapAssociationsOnly, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -768,7 +784,7 @@ bool mapAssociationsOnly /*-------------------------------------------------------------------------------------------------------------------------- | Map the topics from the source collection, and add them to the target collection \-------------------------------------------------------------------------------------------------------------------------*/ - await PopulateTargetCollectionAsync(sourceList, targetList, memberAccessor, cache).ConfigureAwait(false); + await PopulateTargetCollectionAsync(sourceList, targetList, memberAccessor, cache, mapPath).ConfigureAwait(false); } @@ -922,11 +938,13 @@ IList getCollection(CollectionType collection, Func contain /// The target to add the mapped objects to. /// The with details about the property's attributes. /// A cache to keep track of already-mapped object instances. + /// The current mapping request's path, used to detect circular references during construction. private async Task PopulateTargetCollectionAsync( IList sourceList, IList targetList, ItemMetadata itemMetadata, - MappedTopicCache cache + MappedTopicCache cache, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -981,7 +999,7 @@ configuration.ContentTypeFilter is not null && if (!typeof(Topic).IsAssignableFrom(listType)) { var mappingType = GetValidatedMappingType(configuration.MapAs, listType)?? GetValidatedMappingType(childTopic, listType); if (mappingType is not null) { - taskQueue.Add(MapAsync(childTopic, mappingType, configuration.IncludeAssociations, cache)); + taskQueue.Add(MapAsync(childTopic, mappingType, configuration.IncludeAssociations, cache, mapPath: mapPath)); } } else { @@ -1055,11 +1073,13 @@ void addToList(object dto) { /// The expected for the mapped . /// The with details about the item's attributes. /// A cache to keep track of already-mapped object instances. + /// The current mapping request's path, used to detect circular references during construction. private async Task GetTopicReferenceAsync( Topic source, Type targetType, ItemMetadata itemMetadata, - MappedTopicCache cache + MappedTopicCache cache, + MapPath? mapPath = null ) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -1083,7 +1103,7 @@ MappedTopicCache cache var mappingType = GetValidatedMappingType(configuration.MapAs, targetType)?? GetValidatedMappingType(source, targetType); if (mappingType is not null) { - topicDto = await MapAsync(source, mappingType, configuration.IncludeAssociations, cache).ConfigureAwait(false); + topicDto = await MapAsync(source, mappingType, configuration.IncludeAssociations, cache, mapPath: mapPath).ConfigureAwait(false); } /*-------------------------------------------------------------------------------------------------------------------------- From 5a6b8a9abd0f9704187217fee7c3672f8eff924c Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 19:14:14 -0700 Subject: [PATCH 17/50] =?UTF-8?q?Added=20`TryGetValue(=E2=80=A6,=20include?= =?UTF-8?q?Initializing)`=20param?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new `includeInitializing` parameter optionally allows the caller to specifically request a cached entry that hasn't yet been initialized. Once implemented, this will allow a caller to check the initialization and, if it's in the process of initializing, `await` the new `Completion` property on the `MappedTopicCacheEntry` (7a5bd2c2), thus preventing the need for two calls to map the same topic to the same view model from constructing two distinct view models, and thus satisfying a core requirement for #118. --- OnTopic/Mapping/Internal/MappedTopicCache.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/OnTopic/Mapping/Internal/MappedTopicCache.cs b/OnTopic/Mapping/Internal/MappedTopicCache.cs index 11be1494..f73539ea 100644 --- a/OnTopic/Mapping/Internal/MappedTopicCache.cs +++ b/OnTopic/Mapping/Internal/MappedTopicCache.cs @@ -32,9 +32,23 @@ internal sealed class MappedTopicCache { /// The associated with the cache entry. /// The that the has been mapped to. /// The containing the cached instance and metadata. + /// + /// Determines whether an entry that is still should be returned. Left + /// false by default, so callers continue to see only fully constructed entries; set true only by callers who + /// are prepared to distinguish a constructor cycle from sibling concurrency and to await an in-progress entry's completion. + /// /// Returns true if a cached entry could be found, and otherwise false. - internal bool TryGetValue(int topicId, Type type, [NotNullWhen(true)] out MappedTopicCacheEntry? cacheEntry) { - if (_cache.TryGetValue(GetCacheKey(topicId, type), out var existingCacheEntry) && !existingCacheEntry.IsInitializing) { + internal bool TryGetValue( + int topicId, + Type type, + [NotNullWhen(true)] + out MappedTopicCacheEntry? cacheEntry, + bool includeInitializing = false + ) { + if ( + _cache.TryGetValue(GetCacheKey(topicId, type), out var existingCacheEntry) && + (includeInitializing || !existingCacheEntry.IsInitializing) + ) { cacheEntry = existingCacheEntry; return true; }; From ee9a9a51a1ff9c5e706b3f730d0e8f8729bbf26f Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 19:26:52 -0700 Subject: [PATCH 18/50] Wire-up `Complete()` in `Register()` When calling `MappedTopicCache.Register()`, call the new `MappedTopicCacheEntry.Complete()` method (7a5bd2c2). As part of this, also allow `Complete()` to set the `IsInitializing`, `MappedTopic`, and the initial state for `Associations` using its payload, which allows us to entirely remove the setters for `IsInitializing` and `MappedTopic`. That forces callers to use the `Completion` semantics that are required for concurrency of the `TopicMappingService` (#118). --- OnTopic/Mapping/Internal/MappedTopicCache.cs | 13 +++---------- .../Mapping/Internal/MappedTopicCacheEntry.cs | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/OnTopic/Mapping/Internal/MappedTopicCache.cs b/OnTopic/Mapping/Internal/MappedTopicCache.cs index f73539ea..cccc47c4 100644 --- a/OnTopic/Mapping/Internal/MappedTopicCache.cs +++ b/OnTopic/Mapping/Internal/MappedTopicCache.cs @@ -73,21 +73,14 @@ internal void Register(int topicId, AssociationTypes associations, object viewMo \-------------------------------------------------------------------------------------------------------------------------*/ var type = viewModel.GetType(); var cacheKey = GetCacheKey(topicId, type); - var cacheEntry = new MappedTopicCacheEntry() { - MappedTopic = viewModel, - Associations = associations - }; + var cacheEntry = new MappedTopicCacheEntry(); /*-------------------------------------------------------------------------------------------------------------------------- | Get or add entry \-------------------------------------------------------------------------------------------------------------------------*/ - if (topicId > 0 && !type.Equals(typeof(object))) { + if (topicId > 0 && type != typeof(object)) { cacheEntry = _cache.GetOrAdd(cacheKey, cacheEntry); - if (cacheEntry.IsInitializing) { - cacheEntry.IsInitializing = false; - cacheEntry.MappedTopic = viewModel; - cacheEntry.Associations = associations; - } + cacheEntry.Complete(viewModel, associations); } } diff --git a/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs b/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs index b5fae15e..b9745a4c 100644 --- a/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs +++ b/OnTopic/Mapping/Internal/MappedTopicCacheEntry.cs @@ -36,7 +36,13 @@ internal sealed class MappedTopicCacheEntry { /// /// Provides a reference to the mapped object. /// - internal object MappedTopic { get; set; } = null!; + /// + /// Assigned only by , which also settles the task, + /// so the mapped instance is never published outside the completion cycle. This topic is fully constructed, but may not yet + /// have all of its properties mapped; the only prevents two instances of + /// the same view model from being constructed, but doesn't guarantee that mapping is finished. + /// + internal object MappedTopic { get; private set; } = null!; /*============================================================================================================================ | PROPERTY: IS INITIALIZING @@ -47,11 +53,12 @@ internal sealed class MappedTopicCacheEntry { /// /// The property allows an entry to be pre-cached prior to the object being completed. This /// allows the to detect circular references within the object initialization sequence. - /// This is important because, unlikely property mapping where a cached reference can be returned, a circular reference - /// in constructor mapping is expected to throw an exception. By registering that an object is being initialized, the - /// is able to detect circuluar references during constructor mapping. + /// This is important because, unlike property mapping where a cached reference can be returned, a circular reference in + /// constructor mapping is expected to throw an exception. It is derived from the task rather than + /// stored, so a faulted entry remains initializing, ensuring an awaiting pass observes the fault instead of a null . /// - internal bool IsInitializing { get; set; } + internal bool IsInitializing => !_completionSource.Task.IsCompletedSuccessfully; /*============================================================================================================================ | PROPERTY: ASSOCIATIONS @@ -103,7 +110,7 @@ internal sealed class MappedTopicCacheEntry { /// /// This is the mutating counterpart to : It adds any associations /// that aren't already covered, and reports which ones were added back to the caller so the caller knows which associations - /// to process. Because the delta is calculated and saved under a single lock, two concurrent passes over the same cached + /// to process. Because the delta is calculated and saved under a single lock, two concurrent passes over the same cached /// instance receive disjoint results, ensuring each association is mapped by exactly one caller. A caller that receives /// has nothing left to map and should return the cached instance. /// From b197889ebe865538c8882f1af95c6a801431cf83 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 20:22:52 -0700 Subject: [PATCH 19/50] Wire-up `Fault()` in `MapAsync()` When calling (the final private overload of) `TopicMappingService.MapAsync()`, call the new `MappedTopicCacheEntry.Fault()` method (7a5bd2c2) if there's any issue constructing the new view model. This notifies callers that the `Completion` is finished, yet also that the mapped object isn't initialized (i.e., `!IsInitialized`). This looks like a big change, but it's mostly an indentation change, as it requires wrapping the construction of the view model into a `try` block, then calling `Fetch()` within a new `catch` block. As such, most of the git diff is just the preexisting code being indented inside of the `try` block. This helps ensure that one topic can be concurrently mapped to the same view model without interference (#118). --- OnTopic/Mapping/TopicMappingService.cs | 125 +++++++++++++++---------- 1 file changed, 74 insertions(+), 51 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index 1e1e4c0e..fbd067bd 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -182,70 +182,93 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe \-------------------------------------------------------------------------------------------------------------------------*/ cache.Preregister(topic.Id, type); - /*-------------------------------------------------------------------------------------------------------------------------- - | Handle AttributeDictionary constructor - >------------------------------------------------------------------------------------------------------------------------- - | A model may optionally expose a constructor with a single parameter accepting an AttributeDictionary. In this scenario, - | the TopicMappingService may optionally pass a lightweight AttributeDictionary, allowing the model's constructor to - | populate scalar values, instead of relying on reflection. - \-------------------------------------------------------------------------------------------------------------------------*/ - if (parameters.Count is 1 && parameters[0].Type == typeof(AttributeDictionary)) { - - // This strategy is only performant if there are quite a several scalar properties and they are well-covered by the - // attributes. As a fast heuristic to evaluate this, we expect five or more attributes and three or more compatible - // properties. In practice, this should be benefitial with any more than mapped attributes, but we also expect that most - // topics will have 2-3 excluded or unmapped attributes (e.g., Title, LastModified). With models, we can be a bit more - // intelligent, by excluding any members that are likely compatible with Topic properties, thus exluding e.g., Id, Key, - // WebPath, etc. This doesn't guarantee that the attributes map to the properties, but a more accurate evaluation would - // undermine the performance benefits of this optimization. - if (topic.Attributes.Count >= 5 && properties.Count(p => !p.MaybeCompatible) >= 3) { - var attributes = topic.Attributes.AsAttributeDictionary(true); - arguments[0] = attributes; - attributeArguments = attributes; - } - else { - parameters = new(); - arguments = []; - } } /*-------------------------------------------------------------------------------------------------------------------------- - | Handle other constructors - >------------------------------------------------------------------------------------------------------------------------- - | A model may optionally expose a constructor with multiple parameters, which can be defined via reflection in the same - | way as properties would be. This is especially useful for records using the positional syntax (i.e., where properties - | are defined using the constructor). This also, optionally, provides the model with more control, where needed, over how - | it's constructed. - \-------------------------------------------------------------------------------------------------------------------------*/ - else { + | Construct and register the target + >--------------------------------------------------------------------------------------------------------------------------- + | Only the construction span is guarded, so the entry's completion is always settled: cache.Register() settles it on + | success and the catch faults it on failure, so a pass awaiting this entry observes the failure instead of hanging. + | Property mapping runs afterward, outside the guard, since the entry is already settled by then. + \-------------------------------------------------------------------------------------------------------------------------*/ + object? target; + + try { + + /*------------------------------------------------------------------------------------------------------------------------ + | Handle AttributeDictionary constructor + >------------------------------------------------------------------------------------------------------------------------- + | A model may optionally expose a constructor with a single parameter accepting an AttributeDictionary. In this scenario, + | the TopicMappingService may optionally pass a lightweight AttributeDictionary, allowing the model's constructor to + | populate scalar values, instead of relying on reflection. + \-----------------------------------------------------------------------------------------------------------------------*/ + if (parameters.Count is 1 && parameters[0].Type == typeof(AttributeDictionary)) { + + // This strategy is only performant if there are quite a several scalar properties and they are well-covered by the + // attributes. As a fast heuristic to evaluate this, we expect five or more attributes and three or more compatible + // properties. In practice, this should be benefitial with any more than mapped attributes, but we also expect that most + // topics will have 2-3 excluded or unmapped attributes (e.g., Title, LastModified). With models, we can be a bit more + // intelligent, by excluding any members that are likely compatible with Topic properties, thus exluding e.g., Id, Key, + // WebPath, etc. This doesn't guarantee that the attributes map to the properties, but a more accurate evaluation would + // undermine the performance benefits of this optimization. + if (topic.Attributes.Count >= 5 && properties.Count(p => !p.MaybeCompatible) >= 3) { + var attributes = topic.Attributes.AsAttributeDictionary(true); + arguments[0] = attributes; + attributeArguments = attributes; + } + else { + parameters = new(); + arguments = []; + } - foreach (var parameter in parameters) { - parameterQueue.Add(parameter.ParameterInfo.Position, GetParameterAsync(topic, associations, parameter, cache, attributePrefix)); } - await Task.WhenAll(parameterQueue.Values).ConfigureAwait(false); + /*------------------------------------------------------------------------------------------------------------------------ + | Handle other constructors + >------------------------------------------------------------------------------------------------------------------------- + | A model may optionally expose a constructor with multiple parameters, which can be defined via reflection in the same + | way as properties would be. This is especially useful for records using the positional syntax (i.e., where properties + | are defined using the constructor). This also, optionally, provides the model with more control, where needed, over how + | it's constructed. + \-----------------------------------------------------------------------------------------------------------------------*/ + else { + + foreach (var parameter in parameters) { + parameterQueue.Add(parameter.ParameterInfo.Position, GetParameterAsync(topic, associations, parameter, cache, attributePrefix, mapPath)); + } + + await Task.WhenAll(parameterQueue.Values).ConfigureAwait(false); + + foreach (var parameter in parameterQueue) { + arguments[parameter.Key] = await parameter.Value.ConfigureAwait(false); + } - foreach (var parameter in parameterQueue) { - arguments[parameter.Key] = await parameter.Value.ConfigureAwait(false); } - } + /*------------------------------------------------------------------------------------------------------------------------ + | Initialize object + \-----------------------------------------------------------------------------------------------------------------------*/ + target = Activator.CreateInstance(type, arguments); - /*-------------------------------------------------------------------------------------------------------------------------- - | Initialize object - \-------------------------------------------------------------------------------------------------------------------------*/ - target = Activator.CreateInstance(type, arguments); + Contract.Assume( + target, + $"The target type '{type}' could not be properly constructed, as required to map the topic '{topic.GetUniqueKey()}'." + ); - Contract.Assume( - target, - $"The target type '{type}' could not be properly constructed, as required to map the topic '{topic.GetUniqueKey()}'." - ); + /*------------------------------------------------------------------------------------------------------------------------ + | Cache object + \-----------------------------------------------------------------------------------------------------------------------*/ + cache.Register(topic.Id, associations, target); - /*-------------------------------------------------------------------------------------------------------------------------- - | Cache object - \-------------------------------------------------------------------------------------------------------------------------*/ - cache.Register(topic.Id, associations, target); + } + + // Construction failed before the entry was registered, so fault it; a concurrent pass awaiting this entry's completion then + // observes the failure instead of hanging on a map that will never complete + catch (Exception exception) { + entry.Fault(exception); + throw; + } /*-------------------------------------------------------------------------------------------------------------------------- | Loop through properties, mapping each one From 1334a2bb7a977d47b124c8e8a795d4cbf9fdec2f Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 21:25:53 -0700 Subject: [PATCH 20/50] Introduce `resolveCachedEntry()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new `resolveCachedEntry()` local function to centralize the creation, patching, and lookup of cached entries in response to `MapAsync()` requests. Notably, it enables awaiting for an existing construction of a topic to the same view model for independent siblings via the new `Completion` semantics ((7a5bd2c2) that were previously integrated (ee9a9a51, b197889e) by relying on the new `TryGetValue(…, includeInitializing)` parameter (5a6b8a9a) to lookup a pending constructor. It also calculates the `MapPath` class (61b1487c, b42673a4) to recognize circular constructor loops, and thus moves the exception handling for that out of `Preregister()`, so that it won't hang the thread. This addresses the biggest issue with regards to the concurrency handling in the `TopicMappingService` in response to the lazy-loading concerns (#118), though there remain some smaller pieces, plus of course unit testing of these updates. --- OnTopic/Mapping/Internal/MappedTopicCache.cs | 23 +++--- OnTopic/Mapping/TopicMappingService.cs | 77 ++++++++++++++++---- 2 files changed, 71 insertions(+), 29 deletions(-) diff --git a/OnTopic/Mapping/Internal/MappedTopicCache.cs b/OnTopic/Mapping/Internal/MappedTopicCache.cs index cccc47c4..4481f4d0 100644 --- a/OnTopic/Mapping/Internal/MappedTopicCache.cs +++ b/OnTopic/Mapping/Internal/MappedTopicCache.cs @@ -90,34 +90,31 @@ internal void Register(int topicId, AssociationTypes associations, object viewMo \---------------------------------------------------------------------------------------------------------------------------*/ /// /// Attempts to preregister a for a that is in the process of - /// being mapped to . + /// being mapped to , returning the entry along with whether this call created it. /// + /// + /// The returned IsNew flag is true when this call established the entry, and therefore owns its construction; + /// it is false when a concurrent pass had already preregistered the same and , in which case the returned entry is that concurrent pass's entry. + /// /// The associated with the cache entry. /// The that the is being mapped to. - internal MappedTopicCacheEntry Preregister(int topicId, Type type) { + internal (MappedTopicCacheEntry Entry, bool IsNew) Preregister(int topicId, Type type) { /*-------------------------------------------------------------------------------------------------------------------------- | Construct cache entry \-------------------------------------------------------------------------------------------------------------------------*/ var cacheKey = GetCacheKey(topicId, type); - var cacheEntry = new MappedTopicCacheEntry() { - IsInitializing = true - }; + var cacheEntry = new MappedTopicCacheEntry(); /*-------------------------------------------------------------------------------------------------------------------------- | Get or add entry \-------------------------------------------------------------------------------------------------------------------------*/ if (topicId > 0 && !type.Equals(typeof(object))) { var existingCacheEntry = _cache.GetOrAdd(cacheKey, cacheEntry); - if (existingCacheEntry != cacheEntry) { - throw new TopicMappingException( - $"An attempt has been made to map '{topicId}' to a {type.Name} has resulted in a circular reference during the " + - $"construction of the {type.Name} instance. This is not allowed. Circular must be be mapped as properties, not " + - $"as constructor parameters, so that cached entries can be returned." - ); - } + return (existingCacheEntry, existingCacheEntry == cacheEntry); } - return cacheEntry; + return (cacheEntry, true); } diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index fbd067bd..f14c333f 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -150,16 +150,14 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe /*-------------------------------------------------------------------------------------------------------------------------- | Handle cached objects + >--------------------------------------------------------------------------------------------------------------------------- + | Included entries that are still initializing, so a circular constructor reference (i.e., the same topic and type are + | already under construction higher up the current chain) can be distinguished from concurrent siblings (i.e., two + | independent branches mapping the same topic and type at once). The former throws; the latter awaits for the first one to + | finish construction and then uses the same cached view model. \-------------------------------------------------------------------------------------------------------------------------*/ - var target = (object?)null; - - if (cache.TryGetValue(topic.Id, type, out var cacheEntry)) { - target = cacheEntry.MappedTopic; - if (cacheEntry.GetMissingAssociations(associations) == AssociationTypes.None) { - return target; - } - //Call MapAsync() with target object to map missing attributes - return await MapAsync(topic, target, associations, cache, attributePrefix).ConfigureAwait(false); + if (cache.TryGetValue(topic.Id, type, out var cacheEntry, includeInitializing: true)) { + return await resolveCachedEntry(cacheEntry, mapPath).ConfigureAwait(false); } /*-------------------------------------------------------------------------------------------------------------------------- @@ -175,16 +173,28 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe /*-------------------------------------------------------------------------------------------------------------------------- | Pre-cache entry >------------------------------------------------------------------------------------------------------------------------- - | In property mapping, we deal with circular references by returning a cached reference. That isn't practical with - | circular references in constructor mapping. To help avoid these, we register a pre-cache entry as IsInitializing, but - | without a mapped object; the TopicMappingCache is expected to throw an exception if an attempt to map that topic to that - | type occurs again prior to the constructor mapping being completed. + | In property mapping, we deal with circular references by returning a cached reference. That isn't practical with circular + | references in constructor mapping. To help avoid these, we register a pre-cache entry as IsInitializing, but without a + | mapped object. If a concurrent sibling wins the race to preregister the same topic and view model type, we defer to its + | result rather than constructing a duplicate. If we're constructing the same topic and view model type as we're already in + | the middle of constructing further up the MapPath chain, however, that's a true circular construction reference, handled + | via the local resolveCachedEntry() function. \-------------------------------------------------------------------------------------------------------------------------*/ - cache.Preregister(topic.Id, type); - + var (entry, isNew) = cache.Preregister(topic.Id, type); + if (!isNew) { + return await resolveCachedEntry(entry, mapPath).ConfigureAwait(false); } + /*-------------------------------------------------------------------------------------------------------------------------- + | Establish mapping path chain + >--------------------------------------------------------------------------------------------------------------------------- + | Now that this pass owns constructing a new view model, establish the initial topic and view model type in the MapPath, so + | any nested mapping that arrives back to the same pair while it is still initializing is recognized as a true circular + | constructor reference rather than harmless concurrency among independent siblings. + \-------------------------------------------------------------------------------------------------------------------------*/ + mapPath = new(topic.Id, type, mapPath); + /*-------------------------------------------------------------------------------------------------------------------------- | Construct and register the target >--------------------------------------------------------------------------------------------------------------------------- @@ -289,6 +299,41 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe \-------------------------------------------------------------------------------------------------------------------------*/ return target; + /*-------------------------------------------------------------------------------------------------------------------------- + | Resolve cached entry + >--------------------------------------------------------------------------------------------------------------------------- + | Returns a cached instance, expanding it with any missing associations when needed. If the entry is still initializing, it + | is either a true circular constructor reference on the current path (in which we throw an exception) or it's concurrent + | mapping of two independent siblings of the same topic and view model (in which we simply await its completion, then + | return the shared instance as a normal cache hit). + \-------------------------------------------------------------------------------------------------------------------------*/ + async Task resolveCachedEntry(MappedTopicCacheEntry pendingEntry, MapPath? path) { + + // Distinguish a constructor cycle from sibling concurrency for an entry that is still being constructed + if (pendingEntry.IsInitializing) { + if (path?.Contains(topic.Id, type) == true) { + throw new TopicMappingException( + $"A circular reference was detected while constructing the '{type.Name}' instance for topic '{topic.Id}'. Circular " + + $"references must be mapped as properties, not as constructor parameters, so that a cached instance can be returned." + ); + } + // Not on the current path: a concurrent sibling owns construction, so await its completion before resolving. A mutual + // constructor cycle split across two concurrently mapped branches is the one case this cannot distinguish and would + // deadlock; such cycles are unsupported, and still throw when reached from a single branch via the path check above. + await pendingEntry.Completion.ConfigureAwait(false); + } + + // Return the cached instance as-is when it already covers the requested associations + var cachedTarget = pendingEntry.MappedTopic; + if (pendingEntry.GetMissingAssociations(associations) is AssociationTypes.None) { + return cachedTarget; + } + + // Otherwise expand the cached instance, mapping only the missing associations + return await MapAsync(topic, cachedTarget, associations, cache, attributePrefix, path).ConfigureAwait(false); + + } + } /*============================================================================================================================ @@ -353,7 +398,7 @@ private async Task MapAsync( /*-------------------------------------------------------------------------------------------------------------------------- | Handle cached objects - >------------------------------------------------------------------------------------------------------------------------- + >--------------------------------------------------------------------------------------------------------------------------- | If the cache contains an entry, check to make sure it includes all of the requested associations. If it does, return it. | Otherwise, add the missing associations to the cache entry and map only the subset this pass added, so that a concurrent | pass doesn't remap the same associations. From 28f0b07cafca253557c62d9d731e1d1657432b56 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 23:19:53 -0700 Subject: [PATCH 21/50] Added unit tests for completion semantics These tests evaluate the new completion semantics (7a5bd2c2), including `Complete()` and `Fault()` as well as the `Completion` and `IsInitializing` (ee9a9a51) properties. This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around avoiding concurrent construction of new topic view models via the `MappedTopicCacheEntry`. --- OnTopic.Tests/TopicMappingServiceTest.cs | 75 ++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 8a8fd20f..ac9145ab 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -797,6 +797,81 @@ public void MappedTopicCacheEntry_AddMissingAssociations_ReturnsNewlyAdded() { } + /*============================================================================================================================ + | TEST: MAPPED TOPIC CACHE ENTRY: IS INITIALIZING: REFLECTS COMPLETION STATE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a set of instances and confirms that is derived from the completion state: A fresh entry is initializing, a + /// completed entry is not, and a faulted entry remains initializing so an awaiting pass observes the fault instead of a + /// null instance. + /// + [Fact] + public void MappedTopicCacheEntry_IsInitializing_ReflectsCompletionState() { + + var completed = new MappedTopicCacheEntry(); + var faulted = new MappedTopicCacheEntry(); + + Assert.True(completed.IsInitializing); + Assert.True(faulted.IsInitializing); + + completed.Complete(new EmptyViewModel(), AssociationTypes.None); + faulted.Fault(new InvalidOperationException()); + + Assert.False(completed.IsInitializing); + Assert.True(faulted.IsInitializing); + + // Observe the faulted task so its exception isn't surfaced as unobserved + Assert.NotNull(faulted.Completion.Exception); + + } + + /*============================================================================================================================ + | TEST: MAPPED TOPIC CACHE ENTRY: COMPLETION: SETTLES ON COMPLETE OR FAULT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a pair of instances and confirms that resolves when the entry is completed, and throws the recorded exception when the + /// entry is faulted, so that a second pass awaiting the entry is always released rather than left hanging. + /// + [Fact] + public async Task MappedTopicCacheEntry_Completion_SettlesOnCompleteOrFault() { + + var completed = new MappedTopicCacheEntry(); + var faulted = new MappedTopicCacheEntry(); + + completed.Complete(new EmptyViewModel(), AssociationTypes.None); + faulted.Fault(new InvalidOperationException("Construction failed.")); + + Assert.True(completed.Completion.IsCompletedSuccessfully); + await Assert.ThrowsAsync(async () => await faulted.Completion.ConfigureAwait(false)); + + } + + /*============================================================================================================================ + | TEST: MAPPED TOPIC CACHE ENTRY: COMPLETE: RETAINS FIRST RESULT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a and confirms that only the first call takes effect: A later duplicate registration is + /// ignored, keeping both the and its stable. + /// + [Fact] + public void MappedTopicCacheEntry_Complete_RetainsFirstResult() { + + var entry = new MappedTopicCacheEntry(); + var first = new EmptyViewModel(); + var second = new EmptyViewModel(); + + entry.Complete(first, AssociationTypes.Children); + entry.Complete(second, AssociationTypes.Parents); + + Assert.Same(first, entry.MappedTopic); + Assert.Equal(AssociationTypes.Children, entry.Associations); + + } + /*============================================================================================================================ | TEST: MAP: RELATIONSHIPS: RETURNS MAPPED MODEL \---------------------------------------------------------------------------------------------------------------------------*/ From a64b1f81c7a77cbb322e0587ead7dbd93a642e5a Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Mon, 3 Aug 2026 23:22:09 -0700 Subject: [PATCH 22/50] Added unit test for `MapPath.Contains()` This provides a unit test for the new `MapPath` data container (b42673a4) and, specifically, it's `Contain()` method, testing a similar situation as implemented in `resolveCacheEntry()` (1334a2bb). This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around avoiding concurrent construction of new topic view models via the `MappedTopicCacheEntry`. --- OnTopic.Tests/TopicMappingServiceTest.cs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index ac9145ab..1486cdd1 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -872,6 +872,27 @@ public void MappedTopicCacheEntry_Complete_RetainsFirstResult() { } + /*============================================================================================================================ + | TEST: MAP PATH: CONTAINS: DETECTS PAIRS ON PATH + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a two-frame and confirms that recognizes a + /// topic and view model type pair anywhere on the path, whether at the current frame or an ancestor, while rejecting pairs + /// that are not on the path, including one whose topic identifier matches but whose type does not. + /// + [Fact] + public void MapPath_Contains_DetectsPairsOnPath() { + + var root = new MapPath(1, typeof(EmptyViewModel), null); + var child = new MapPath(2, typeof(KeyOnlyTopicViewModel), root); + + Assert.True(child.Contains(2, typeof(KeyOnlyTopicViewModel))); + Assert.True(child.Contains(1, typeof(EmptyViewModel))); + Assert.False(child.Contains(3, typeof(EmptyViewModel))); + Assert.False(child.Contains(1, typeof(KeyOnlyTopicViewModel))); + + } + /*============================================================================================================================ | TEST: MAP: RELATIONSHIPS: RETURNS MAPPED MODEL \---------------------------------------------------------------------------------------------------------------------------*/ From dbb47987745c668ad54acb33ef0cc779dff06fe4 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 00:38:41 -0700 Subject: [PATCH 23/50] Introduced `CircularConstructorTopicViewModel` This will be used in a new unit test to confirm that the circular constructor reference detection, including the completion semantics (7a5bd2c2, ee9a9a51, b197889e, 5a6b8a9a) and `MapPath` tracking (b42673a4, 61b1487c) as implemented in `resolveCachedEntry()` (1334a2bb). This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around avoiding concurrent construction of new topic view models via the `MappedTopicCacheEntry`. --- .../TestDoubles/FakeViewModelLookupService.cs | 1 + .../CircularConstructorTopicViewModel.cs | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 OnTopic.Tests/ViewModels/CircularConstructorTopicViewModel.cs diff --git a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs index 1f24a195..e7b7a791 100644 --- a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs +++ b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs @@ -35,6 +35,7 @@ public FakeViewModelLookupService() { Add(typeof(AmbiguousRelationTopicViewModel)); Add(typeof(AscendentSpecializedTopicViewModel)); Add(typeof(AscendentTopicViewModel)); + Add(typeof(CircularConstructorTopicViewModel)); Add(typeof(CircularTopicViewModel)); Add(typeof(ConstructedTopicViewModel)); Add(typeof(DefaultValueTopicViewModel)); diff --git a/OnTopic.Tests/ViewModels/CircularConstructorTopicViewModel.cs b/OnTopic.Tests/ViewModels/CircularConstructorTopicViewModel.cs new file mode 100644 index 00000000..ded41685 --- /dev/null +++ b/OnTopic.Tests/ViewModels/CircularConstructorTopicViewModel.cs @@ -0,0 +1,34 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.Mapping; + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: CIRCULAR CONSTRUCTOR TOPIC +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a strongly typed data transfer object, implemented as a positional record, for testing constructor mapping +/// of a topic reference that may form a circular reference. +/// +/// +/// +/// Unlike , which expresses its circular reference through settable properties, this +/// model maps its reference through a positional constructor parameter on a record. This allows the to be exercised for two distinct behaviors: A non-cyclic reference should map successfully, +/// while a true self-reference should be detected as a constructor cycle and throw a , +/// since a partially constructed instance cannot be returned from a constructor. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +/// The key of the mapped topic. +/// An optional reference to another . +public record CircularConstructorTopicViewModel( + string Key, + [Include(AssociationTypes.References)] CircularConstructorTopicViewModel? Self +); \ No newline at end of file From dc88d41d6a29063aef5612414fd66fb31e8decf1 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 00:43:42 -0700 Subject: [PATCH 24/50] Added unit test for positional constructor mapping Added a unit test that confirms that the ability to map constructor parameters (#35) succeeds on a record using a positional constructor. This was always the primary use case for that capability, but it wasn't explicitly tested via any prior unit tests. While this isn't directly related to testing concurrency with the `TopicMappingService` (#118), it does reuse the new `CircularConstructorTopicViewModel` (dbb47987) that was introduced for those tests, and this test is introduced first to ensure that base functionality works before I introduce a real test for evaluating the circular constructor reference. --- OnTopic.Tests/TopicMappingServiceTest.cs | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 1486cdd1..51f8d0dd 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -467,6 +467,35 @@ await _mappingService.MapAsync(topic).ConfigureAwait( } + /*============================================================================================================================ + | TEST: MAP: CONSTRUCTOR (RECORD): RETURNS NEW MODEL + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a and maps a positional record whose constructor accepts a + /// non-cyclic topic reference, confirming that the reference is resolved and the record is constructed as expected. + /// + /// + /// As this is a mapping of a positional record that carries constructor parameters, it also confirms that the + /// primary constructor is correctly selected and its parameters mapped. + /// + [Fact] + public async Task Map_ConstructorRecord_ReturnsNewModel() { + + var topic = new Topic("Parent", "CircularConstructor", null, 1); + var child = new Topic("Child", "CircularConstructor", null, 2); + + topic.References.SetValue("Self", child); + + var target = await _mappingService.MapAsync(topic); + + Assert.NotNull(target); + Assert.Equal("Parent", target.Key); + Assert.NotNull(target.Self); + Assert.Equal("Child", target.Self.Key); + Assert.Null(target.Self.Self); + + } + /*============================================================================================================================ | TEST: MAP: DISABLED PROPERTY: RETURNS NULL \---------------------------------------------------------------------------------------------------------------------------*/ From 4a67babe63ec02e319b9e4aefe99caa3b527ecec Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 00:45:56 -0700 Subject: [PATCH 25/50] Added unit test for circular constructor reference This unit test utilizes the newly introduced `CircularConstructorTopicViewModel` (dbb47987) to create a scenario where a circular constructor reference occurs. This confirms that the circular constructor reference detection, including the completion semantics (7a5bd2c2, ee9a9a51, b197889e, 5a6b8a9a) and `MapPath` tracking (b42673a4, 61b1487c) as implemented in `resolveCachedEntry()` (1334a2bb). This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around avoiding concurrent construction of new topic view models via the `MappedTopicCacheEntry`. --- OnTopic.Tests/TopicMappingServiceTest.cs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 51f8d0dd..ea335b8f 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -496,6 +496,27 @@ public async Task Map_ConstructorRecord_ReturnsNewModel() { } + /*============================================================================================================================ + | TEST: MAP: CONSTRUCTOR (RECORD): THROWS EXCEPTION + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a and maps a positional record whose constructor references the + /// topic being mapped, and confirms that this circular constructor reference is detected and a is thrown. + /// + [Fact] + public async Task Map_ConstructorRecord_ThrowsException() { + + var topic = new Topic("Topic", "CircularConstructor", null, 1); + + topic.References.SetValue("Self", topic); + + await Assert.ThrowsAsync(async () => + await _mappingService.MapAsync(topic).ConfigureAwait(false) + ); + + } + /*============================================================================================================================ | TEST: MAP: DISABLED PROPERTY: RETURNS NULL \---------------------------------------------------------------------------------------------------------------------------*/ From 73e2f3ab891946201995241eb2bc41eb8afb971a Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 01:11:13 -0700 Subject: [PATCH 26/50] Introduced models for concurrent mapping tests The `ConcurrentReferenceTopicViewModel` sets up two independent references to `SharedConcurrentTopicViewModel` view models, which will be mapped to the same topic, thus allowing us to evaluate that the same topic and can be mapped to the same view model within the same `MapAsync()` chain, so long as they're not in the same `MapPath` (b42673a4, 61b1487c), as detected via `resolveCacheEntry()` (1334a2bb). This will also evaluate the closely related completion semantics (7a5bd2c2, ee9a9a51, b197889e, 5a6b8a9a). This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around allowing concurrent construction of new topic view models via the `MappedTopicCacheEntry`, so long as they're not in the same `MapPath`. --- .../TestDoubles/FakeViewModelLookupService.cs | 2 ++ .../ConcurrentReferenceTopicViewModel.cs | 36 +++++++++++++++++++ .../SharedConcurrentTopicViewModel.cs | 36 +++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 OnTopic.Tests/ViewModels/ConcurrentReferenceTopicViewModel.cs create mode 100644 OnTopic.Tests/ViewModels/SharedConcurrentTopicViewModel.cs diff --git a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs index e7b7a791..9ee6b0c7 100644 --- a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs +++ b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs @@ -37,6 +37,7 @@ public FakeViewModelLookupService() { Add(typeof(AscendentTopicViewModel)); Add(typeof(CircularConstructorTopicViewModel)); Add(typeof(CircularTopicViewModel)); + Add(typeof(ConcurrentReferenceTopicViewModel)); Add(typeof(ConstructedTopicViewModel)); Add(typeof(DefaultValueTopicViewModel)); Add(typeof(DescendentSpecializedTopicViewModel)); @@ -61,6 +62,7 @@ public FakeViewModelLookupService() { Add(typeof(RelationWithChildrenTopicViewModel)); Add(typeof(RequiredObjectTopicViewModel)); Add(typeof(RequiredTopicViewModel)); + Add(typeof(SharedConcurrentTopicViewModel)); Add(typeof(TopicReferenceAttributeDescriptorTopicViewModel)); Add(typeof(TopicReferenceTopicViewModel)); diff --git a/OnTopic.Tests/ViewModels/ConcurrentReferenceTopicViewModel.cs b/OnTopic.Tests/ViewModels/ConcurrentReferenceTopicViewModel.cs new file mode 100644 index 00000000..8308cc70 --- /dev/null +++ b/OnTopic.Tests/ViewModels/ConcurrentReferenceTopicViewModel.cs @@ -0,0 +1,36 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.Mapping; + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: CONCURRENT REFERENCE TOPIC +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a strongly typed data transfer object with two topic references, both intended to resolve to the same shared . +/// +/// +/// +/// Both references are mapped as properties, so the resolves them concurrently within a +/// single mapping pass. When both point at the same topic, this drives two branches to map that topic to the same type at +/// once, which is a supported sibling concurrency scenario. The pins the mapped view model +/// type so the scenario does not depend on the shared topic's content type. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +public class ConcurrentReferenceTopicViewModel { + + [MapAs(typeof(SharedConcurrentTopicViewModel))] + public SharedConcurrentTopicViewModel? FirstReference { get; set; } + + [MapAs(typeof(SharedConcurrentTopicViewModel))] + public SharedConcurrentTopicViewModel? SecondReference { get; set; } + +} //Class \ No newline at end of file diff --git a/OnTopic.Tests/ViewModels/SharedConcurrentTopicViewModel.cs b/OnTopic.Tests/ViewModels/SharedConcurrentTopicViewModel.cs new file mode 100644 index 00000000..a51c84ed --- /dev/null +++ b/OnTopic.Tests/ViewModels/SharedConcurrentTopicViewModel.cs @@ -0,0 +1,36 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.Mapping; + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: SHARED CONCURRENT TOPIC +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a strongly typed data transfer object, implemented as a positional record, for testing that two concurrent +/// branches mapping the same topic to the same view model type share a single instance. +/// +/// +/// +/// The collection is mapped through a constructor parameter, so constructing this model requires the +/// source topic's payload to be loaded. When paired with a repository that suspends inside its lazy load, this lets a test +/// hold one branch mid-construction while a second branch reaches the same still-initializing cache entry, evaluating the +/// 's support of sibling concurrency. +/// +/// +/// The actual sibling references are set up in the accompanying . +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +/// The key of the mapped topic. +/// A collection mapped from a constructor parameter, forcing the source payload to be loaded. +public record SharedConcurrentTopicViewModel( + string Key, + [Collection("Related")] Collection? Related +); \ No newline at end of file From 91d0a21636b00d69229e7d7bb9d50145fd2ad870 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 01:12:36 -0700 Subject: [PATCH 27/50] Added `CreateGatedMappingService()` helper This introduces a new `CreateGatedMappingService()` helper method that constructs a new `BlockingStubLazyLoadingTopicRepository` so that the tests can recreate the exact concurrence scenario that the code confirms. This will be used in the testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around allowing concurrent construction of new topic view models via the `MappedTopicCacheEntry`, so long as they're not in the same `MapPath`. --- OnTopic.Tests/TopicMappingServiceTest.cs | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index ea335b8f..53614307 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -39,6 +39,7 @@ public class TopicMappingServiceTest { \---------------------------------------------------------------------------------------------------------------------------*/ readonly ITopicRepository _topicRepository; readonly ITopicMappingService _mappingService; + readonly ITypeLookupService _typeLookupService; /*============================================================================================================================ | CONSTRUCTOR @@ -64,6 +65,7 @@ public TopicMappingServiceTest(TopicInfrastructureFixture f \-------------------------------------------------------------------------------------------------------------------------*/ _topicRepository = fixture.CachedTopicRepository; _mappingService = fixture.MappingService; + _typeLookupService = fixture.TypeLookupService; } @@ -1996,4 +1998,27 @@ public async Task Map_CachedTopic_ReturnsUniqueReferencePerType() { public static TopicViewModel? GetChildTopic(IEnumerable? topicCollection, string key) => topicCollection?.FirstOrDefault((t) => t.Key.StartsWith(key, StringComparison.Ordinal)); + /*============================================================================================================================ + | METHOD: CREATE GATED MAPPING SERVICE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Assembles a fresh over a , + /// returning the service together with that gated repository and the wrapping it. + /// + /// + /// Each call returns a new, isolated set so concurrent-mapping tests can "arm", release, or fault their own gate without + /// interfering with one another. The stateless is reused, so + /// only the gated repository is constructed per test. + /// + /// The gated repository, the cache over it, and the mapping service. + private ( + BlockingStubLazyLoadingTopicRepository Repository, + CachedTopicRepository Cache, + ITopicMappingService MappingService + ) CreateGatedMappingService() { + var inner = new BlockingStubLazyLoadingTopicRepository(); + var cache = new CachedTopicRepository(inner); + return (inner, cache, new TopicMappingService(cache, _typeLookupService)); + } + } //Class \ No newline at end of file From 678bf643c71de56b71693d44ccb06dd8a8bf3f80 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 01:14:50 -0700 Subject: [PATCH 28/50] Added unit test for concurrent model construction This test utilizes the new `ConcurrentReferenceTopicViewModel` and `SharedConcurrentTopicViewModel` view models (73e2f3ab) as well as the `CreateGatedMappingService()` helper (91d0a216) to evaluate that the same topic and can be mapped to the same view model within the same `MapAsync()` chain, so long as they're not in the same `MapPath` (b42673a4, 61b1487c), as detected via `resolveCacheEntry()` (1334a2bb). This also evaluate the closely related completion semantics (7a5bd2c2, ee9a9a51, b197889e, 5a6b8a9a). This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around allowing concurrent construction of new topic view models via the `MappedTopicCacheEntry`, so long as they're not in the same `MapPath`. --- OnTopic.Tests/TopicMappingServiceTest.cs | 49 ++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 53614307..3be2d6db 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -519,6 +519,55 @@ await _mappingService.MapAsync(topic).Configu } + /*============================================================================================================================ + | TEST: MAP: CONCURRENT SIBLINGS: RETURNS SHARED INSTANCE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Confirms that two concurrent branches mapping the same topic to the same type within a single pass share one instance, + /// rather than the second branch mistaking the first's still-initializing entry for a circular constructor reference. + /// + /// + /// Uses , which suspends inside its own EnsureLoaded until + /// released, to hold the branch that wins construction of the shared model mid-constructor, guaranteeing the second branch + /// reaches the still-initializing entry and thus must await its completion. The shared topic is loaded through the + /// repository so it is stamped for lazy loading, and its constructor's collection parameter actually engages the gate; the + /// root is a plain whose in-memory references cannot trip the gate before the shared model is + /// constructed. Asserting the in-process task has not completed proves the first branch actually suspended, so the test + /// cannot pass without exercising the await path. + /// + [Fact] + public async Task Map_ConcurrentSiblings_ReturnsSharedInstance() { + + var (inner, cache, mappingService) = CreateGatedMappingService(); + + var shared = await cache.Load("Web"); + + Contract.Assume(shared); + + var root = new Topic("ConcurrentRoot", "Container", null, 5); + + root.References.SetValue("FirstReference", shared); + root.References.SetValue("SecondReference", shared); + + // "Arm" the gate so the branch that constructs the shared model suspends mid-constructor + inner.ArmEnsureLoadedGate(); + + var mapTask = mappingService.MapAsync(root); + + // Prove the first branch is genuinely suspended, so the second must await its completion + Assert.False(mapTask.IsCompleted); + + inner.ReleaseEnsureLoadedGate(); + + var result = await mapTask; + + Assert.NotNull(result); + Assert.NotNull(result.FirstReference); + Assert.NotNull(result.SecondReference); + Assert.Same(result.FirstReference, result.SecondReference); + + } + /*============================================================================================================================ | TEST: MAP: DISABLED PROPERTY: RETURNS NULL \---------------------------------------------------------------------------------------------------------------------------*/ From 6d590606f3c5026eb2dc2c968221c8d82e5b8a93 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 01:20:39 -0700 Subject: [PATCH 29/50] Added unit test for faulty model construction This test utilizes the new `ConcurrentReferenceTopicViewModel` and `SharedConcurrentTopicViewModel` view models (73e2f3ab) as well as the `CreateGatedMappingService()` helper (91d0a216) to confirm that a `Fault()` that occurs during mapping, similar to what's implemented in `MapAsync()` (b197889e), throws an exception without hanging during concurrent construction of the same view model reference. This also relies on the broader completion semantics (7a5bd2c2, ee9a9a51, 5a6b8a9a) that are related to the `Fault()` concept. This contributes to testing of the `TopicMappingService`'s concurrency infrastructure (#118), and specifically around allowing concurrent construction of new topic view models via the `MappedTopicCacheEntry`, so long as they're not in the same `MapPath` and no exceptions are thrown (as evaluated in this test). --- OnTopic.Tests/TopicMappingServiceTest.cs | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 3be2d6db..ab5ecbbe 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -568,6 +568,46 @@ public async Task Map_ConcurrentSiblings_ReturnsSharedInstance() { } + /*============================================================================================================================ + | TEST: MAP: CONCURRENT SIBLINGS: OBSERVES FAULT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Confirms that when the branch constructing a shared model faults, a concurrent branch awaiting the same entry observes + /// the exception rather than hanging on a mapping that will never complete. + /// + /// + /// Uses the same setup as , but releases the gate with a fault so + /// the constructing branch throws while the second branch is awaiting the entry's completion. That the map throws, rather + /// than deadlocking, is what confirms the faulted entry releases its waiter. + /// + [Fact] + public async Task Map_ConcurrentSiblings_ObservesFault() { + + var (inner, cache, mappingService) = CreateGatedMappingService(); + + var shared = await cache.Load("Web"); + + Contract.Assume(shared); + + var root = new Topic("ConcurrentRoot", "Container", null, 5); + + root.References.SetValue("FirstReference", shared); + root.References.SetValue("SecondReference", shared); + + // "Arm" the gate so the branch that constructs the shared model suspends mid-constructor + inner.ArmEnsureLoadedGate(); + + var mapTask = mappingService.MapAsync(root); + + // Prove the first branch is genuinely suspended, so the second must await its completion + Assert.False(mapTask.IsCompleted); + + inner.FaultEnsureLoadedGate(new InvalidOperationException("Simulated load failure.")); + + await Assert.ThrowsAsync(async () => await mapTask.ConfigureAwait(false)); + + } + /*============================================================================================================================ | TEST: MAP: DISABLED PROPERTY: RETURNS NULL \---------------------------------------------------------------------------------------------------------------------------*/ From 86c6f7e6148e63c18f61fe758d12bc568f0184ee Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 13:25:37 -0700 Subject: [PATCH 30/50] Lock creation of new view model collections When mapping a new view model via `SetCollectionValueAsync()`, lock the initialization of the collection object itself by the `topicId` (if available) and view model type to ensure that two concurrent mapping requests don't accidentally create two competing collections. This addresses one of the final concurrency issues with the `TopicMappingService` (#118). --- OnTopic/Mapping/TopicMappingService.cs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index f14c333f..f468fc13 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -824,13 +824,30 @@ private async Task SetCollectionValueAsync( MapPath? mapPath = null ) { + /*-------------------------------------------------------------------------------------------------------------------------- + | Establish per-entry lock + >--------------------------------------------------------------------------------------------------------------------------- + | Two concurrent passes expanding the same cached target with disjoint associations (see e.g., the cache-hit path in + | MapAsync) can populate the same member's target list from different sources, so its creation and population are + | synchronized on the shared cache entry. A target that isn't cached (i.e., a new topic) is never shared across passes, so + | its own instance serves as a sufficient and uncontended lock. + \-------------------------------------------------------------------------------------------------------------------------*/ + cache.TryGetValue(source.Id, target.GetType(), out var cacheEntry); + var collectionLock = (object?)cacheEntry?? target; + /*-------------------------------------------------------------------------------------------------------------------------- | Ensure target list is created + >--------------------------------------------------------------------------------------------------------------------------- + | Locked so two concurrent passes can't both observe a null list and instantiate competing instances, which would orphan the + | items added to whichever collection is overwritten. The lock is synchronous and never held across an await. \-------------------------------------------------------------------------------------------------------------------------*/ - var targetList = (IList?)memberAccessor.GetValue(target); - if (targetList is null) { - targetList = InitializeCollection(memberAccessor.Type); - memberAccessor.SetValue(target, targetList); + IList? targetList; + lock (collectionLock) { + targetList = (IList?)memberAccessor.GetValue(target); + if (targetList is null) { + targetList = InitializeCollection(memberAccessor.Type); + memberAccessor.SetValue(target, targetList); + } } Contract.Assume( From 9dfc24c3e1f533d159ec1bcf26e96fe95f8f0636 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 13:32:20 -0700 Subject: [PATCH 31/50] Lock the addition of view models to collections Using the same lock used to synchronize the creation of the collection itself (86c6f7e6), lock the addition of items to the collection via `PopulateTargetCollectionAsync()`, so that two concurrent additions don't conflict with one another. This addresses one of the last big items for `TopicMappingService` concurrency (#118). (That said, we'll still have smaller passes to go through after this.) --- OnTopic/Mapping/TopicMappingService.cs | 27 ++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index f468fc13..aff17e3e 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -507,7 +507,7 @@ private async Task MapAsync( return null; } - await PopulateTargetCollectionAsync(sourceList, targetList, parameter, cache, mapPath).ConfigureAwait(false); + await PopulateTargetCollectionAsync(sourceList, targetList, parameter, cache, targetList, mapPath).ConfigureAwait(false); return targetList; @@ -869,7 +869,7 @@ private async Task SetCollectionValueAsync( /*-------------------------------------------------------------------------------------------------------------------------- | Map the topics from the source collection, and add them to the target collection \-------------------------------------------------------------------------------------------------------------------------*/ - await PopulateTargetCollectionAsync(sourceList, targetList, memberAccessor, cache, mapPath).ConfigureAwait(false); + await PopulateTargetCollectionAsync(sourceList, targetList, memberAccessor, cache, collectionLock, mapPath).ConfigureAwait(false); } @@ -1023,12 +1023,18 @@ IList getCollection(CollectionType collection, Func contain /// The target to add the mapped objects to. /// The with details about the property's attributes. /// A cache to keep track of already-mapped object instances. + /// + /// The object to lock on while adding to , so concurrent passes populating a shared target's + /// list from disjoint sources don't modify it simultaneously. Callers pass the shared cache entry for a cached target, or + /// the (unshared) list itself when populating a constructor parameter. + /// /// The current mapping request's path, used to detect circular references during construction. private async Task PopulateTargetCollectionAsync( IList sourceList, IList targetList, ItemMetadata itemMetadata, MappedTopicCache cache, + object collectionLock, MapPath? mapPath = null ) { @@ -1107,14 +1113,19 @@ configuration.ContentTypeFilter is not null && /*-------------------------------------------------------------------------------------------------------------------------- | Function: Add to List + >--------------------------------------------------------------------------------------------------------------------------- + | Locked so a concurrent pass populating the same shared list from a disjoint source can't add at the same time; the lock + | is synchronous and never held across an await, as child mapping happens outside of it via the above task queue. \-------------------------------------------------------------------------------------------------------------------------*/ void addToList(object dto) { - try { - targetList.Add(dto); - } - catch (ArgumentException) { - //Ignore exceptions caused by duplicate keys, in case the IList represents a keyed collection - //We would defensively check for this, except IList doesn't provide a suitable method to do so + lock (collectionLock) { + try { + targetList.Add(dto); + } + catch (ArgumentException) { + //Ignore exceptions caused by duplicate keys, in case the IList represents a keyed collection + //We would defensively check for this, except IList doesn't provide a suitable method to do so + } } } From a101412d72e6cd035ee2185e7cb58d15e9f085b3 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 13:46:19 -0700 Subject: [PATCH 32/50] Allow the lazy concurrency repository fault This complements the existing `ArmEnsureLoadedGate()` and `ReleaseEnsureLoadedGate()` (2875725d, 8b56f3cd) with a new `FaultEnsureLoadedGate()`, which will allow us to simulate an exception as part of the testing of the mapped collection concurrently (86c6f7e6, 9dfc24c3) tests (#118). --- .../BlockingStubLazyLoadingTopicRepository.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/OnTopic.Tests/TestDoubles/BlockingStubLazyLoadingTopicRepository.cs b/OnTopic.Tests/TestDoubles/BlockingStubLazyLoadingTopicRepository.cs index d3f0ef7e..d8aca6ca 100644 --- a/OnTopic.Tests/TestDoubles/BlockingStubLazyLoadingTopicRepository.cs +++ b/OnTopic.Tests/TestDoubles/BlockingStubLazyLoadingTopicRepository.cs @@ -76,6 +76,16 @@ internal sealed class BlockingStubLazyLoadingTopicRepository: StubLazyLoadingTop /// public void ReleaseEnsureLoadedGate() => _ensureLoadedGate?.SetResult(); + /*============================================================================================================================ + | METHOD: FAULT ENSURE LOADED GATE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Faults a suspended call "armed" via with the supplied + /// , so a test can simulate a lazy load that throws while a second pass awaits the same entry. + /// + /// The exception to surface from the suspended call. + public void FaultEnsureLoadedGate(Exception exception) => _ensureLoadedGate?.SetException(exception); + /*============================================================================================================================ | METHOD: LOAD \---------------------------------------------------------------------------------------------------------------------------*/ From 96eae010fa1b05fec66d73cfd8d94d96cb3985b2 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 13:49:05 -0700 Subject: [PATCH 33/50] Introduced models for collection concurrency tests These two view models setup the conditions that will allow us to test the locks on `SetCollectionValueAsync()` (86c6f7e6), which prevents duplicate collections from being created, and `PopulateTargetCollectionAsync()` (9dfc24c3), which prevents two items from being added concurrently. This contributes to the testing of the `TopicMappingService` concurrency (#118). --- .../TestDoubles/FakeViewModelLookupService.cs | 2 + .../ConcurrentExpansionRootTopicViewModel.cs | 36 ++++++++++++++++ ...ConcurrentExpansionSharedTopicViewModel.cs | 41 +++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 OnTopic.Tests/ViewModels/ConcurrentExpansionRootTopicViewModel.cs create mode 100644 OnTopic.Tests/ViewModels/ConcurrentExpansionSharedTopicViewModel.cs diff --git a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs index 9ee6b0c7..04f2ccde 100644 --- a/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs +++ b/OnTopic.Tests/TestDoubles/FakeViewModelLookupService.cs @@ -37,6 +37,8 @@ public FakeViewModelLookupService() { Add(typeof(AscendentTopicViewModel)); Add(typeof(CircularConstructorTopicViewModel)); Add(typeof(CircularTopicViewModel)); + Add(typeof(ConcurrentExpansionRootTopicViewModel)); + Add(typeof(ConcurrentExpansionSharedTopicViewModel)); Add(typeof(ConcurrentReferenceTopicViewModel)); Add(typeof(ConstructedTopicViewModel)); Add(typeof(DefaultValueTopicViewModel)); diff --git a/OnTopic.Tests/ViewModels/ConcurrentExpansionRootTopicViewModel.cs b/OnTopic.Tests/ViewModels/ConcurrentExpansionRootTopicViewModel.cs new file mode 100644 index 00000000..55d1c093 --- /dev/null +++ b/OnTopic.Tests/ViewModels/ConcurrentExpansionRootTopicViewModel.cs @@ -0,0 +1,36 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: CONCURRENT EXPANSION ROOT +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a parent view model that references the same source topic twice with disjoint associations, forcing two +/// concurrent mapping passes over the shared target. +/// +/// +/// +/// Both and resolve to the same source topic, but request +/// disjoint associations via . Because both reference properties are mapped concurrently (via +/// the property-level Task.WhenAll), one pass constructs the shared instance while the other expands it, each +/// populating the target's list from a different source; this +/// is the scenario that the SetCollectionValueAsync() and PopulateTargetCollectionAsync() locks protect against. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +public class ConcurrentExpansionRootTopicViewModel { + + [Include(AssociationTypes.Relationships)] + public ConcurrentExpansionSharedTopicViewModel? RelationshipsView { get; set; } + + [Include(AssociationTypes.IncomingRelationships)] + public ConcurrentExpansionSharedTopicViewModel? IncomingView { get; set; } + +} //Class \ No newline at end of file diff --git a/OnTopic.Tests/ViewModels/ConcurrentExpansionSharedTopicViewModel.cs b/OnTopic.Tests/ViewModels/ConcurrentExpansionSharedTopicViewModel.cs new file mode 100644 index 00000000..b14393d5 --- /dev/null +++ b/OnTopic.Tests/ViewModels/ConcurrentExpansionSharedTopicViewModel.cs @@ -0,0 +1,41 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ + +namespace OnTopic.Tests.ViewModels; + +/*============================================================================================================================== +| VIEW MODEL: CONCURRENT EXPANSION SHARED +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a shared target for two concurrent mapping passes with disjoint associations, exposing a single collection () populated from two different sources. +/// +/// +/// +/// This is the referenced view model in the concurrency stress test: A single source topic is referenced twice from a +/// parent , once with and +/// once with . Both passes populate the same +/// list: One from the source's outgoing relationships, the other from its incoming relationships, thus exercising the locks +/// on SetCollectionValueAsyn() and PopulateTargetCollectionAsync(). The property is left nullable and settable so the +/// mapper both creates the backing list (SetCollectionValueAsyn()) and adds to it (PopulateTargetCollectionAsync()). +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +[SuppressMessage( + "Usage", + "CA2227:Collection properties should be read only", + Justification = "This view model intentionally exposes a settable, nullable collection property so the TopicMappingService's list-creation runs, which the concurrency test relies on to establish a creation race." +)] +public class ConcurrentExpansionSharedTopicViewModel { + + public string? Key { get; set; } + + [Collection("Related")] + public Collection? Related { get; set; } + +} //Class \ No newline at end of file From f6a85a24bc5abe989d884e2236cf89ac1e8af45a Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 14:40:19 -0700 Subject: [PATCH 34/50] Introduced new `RendezvousTopicLazyLoader` The `RendezvousTopicLazyLoader` allows us to test the locks on `SetCollectionValueAsync()` (86c6f7e6), which prevents duplicate collections from being created, and `PopulateTargetCollectionAsync()` (9dfc24c3), which prevents two items from being added concurrently. Unlike all other `ITopicLazyLoader` implementations, this is _purely_ a lazy loader; it is not bundled with an actual `ITopicRepository`, nor does it rely on e.g., `Load()` or any other integration with a persistence store to serve its narrow testing purpose. This contributes to the testing of the `TopicMappingService` concurrency (#118). --- .../TestDoubles/RendezvousTopicLazyLoader.cs | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 OnTopic.Tests/TestDoubles/RendezvousTopicLazyLoader.cs diff --git a/OnTopic.Tests/TestDoubles/RendezvousTopicLazyLoader.cs b/OnTopic.Tests/TestDoubles/RendezvousTopicLazyLoader.cs new file mode 100644 index 00000000..08c6fd1e --- /dev/null +++ b/OnTopic.Tests/TestDoubles/RendezvousTopicLazyLoader.cs @@ -0,0 +1,139 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.Repositories; + +namespace OnTopic.Tests.TestDoubles; + +/*============================================================================================================================== +| CLASS: RENDEZVOUS TOPIC LAZY LOADER +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// An that suspends each call until a fixed number +/// of concurrent passes have arrived, then releases them together, so a stress test can force two concurrent mapping passes +/// to modify a shared collection at the same time without timing hacks. Conceptually, this is an asynchronous, cyclic for a fixed number of mapping passes. +/// +/// +/// +/// Unlike a repository-backed lazy loader, this performs no fetching: The stress test wires the shared topic's associations +/// before mapping, so only needs to a) "rendezvous" the concurrent passes and +/// b) mark as so the base pass's nested-topic search, which +/// reads the autoloading getter, doesn't re-enter this loader. The rendezvous rearms after +/// each release, so a single instance serves every repetition. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +[ExcludeFromCodeCoverage] +internal sealed class RendezvousTopicLazyLoader: ITopicLazyLoader { + + /*============================================================================================================================ + | PRIVATE FIELDS + \---------------------------------------------------------------------------------------------------------------------------*/ + private readonly object _lock = new(); + private readonly int _participantCount; + private readonly TimeSpan _timeout; + private TaskCompletionSource _gate = new(TaskCreationOptions.RunContinuationsAsynchronously); + private int _arrivals; + + /*============================================================================================================================ + | CONSTRUCTOR + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Instantiates a new instance of the that releases once passes have arrived. + /// + /// The number of concurrent passes to await before releasing them together. + /// + /// The number of seconds an arrived pass waits for the others before throwing, guarding against a hang if the expected + /// concurrency never materializes. + /// + public RendezvousTopicLazyLoader(int participantCount = 2, int timeoutSeconds = 10) { + _participantCount = participantCount; + _timeout = TimeSpan.FromSeconds(timeoutSeconds); + } + + /*============================================================================================================================ + | METHOD: ENSURE LOADED + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + async Task ITopicLazyLoader.EnsureLoaded(Topic topic, TopicPayload payload, CancellationToken cancellationToken) { + + // Suspend until the concurrent passes rendezvous, so they resume together and race on the shared collection + await Rendezvous(cancellationToken).ConfigureAwait(false); + + // Mark children Loaded so the base pass's nested-topic probe, which reads the autoloading Topic.Children getter, doesn't + // re-enter this loader. Relationships needs no such treatment: Its targets are preloaded, so its (derived) LoadState is + // already Loaded and so relationship never autoload. + ((ITopicBackingAccessor)topic).Children.LoadState = LoadState.Loaded; + + } + + /*============================================================================================================================ + | METHOD: RENDEZVOUS + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Suspends the caller until passes have arrived, then releases them together and re-arms + /// for the next batch. + /// + /// A token used to cancel the wait. + private Task Rendezvous(CancellationToken cancellationToken) { + + /*-------------------------------------------------------------------------------------------------------------------------- + | Register arrival and, if last, re-arm the gate for the next batch + \-------------------------------------------------------------------------------------------------------------------------*/ + TaskCompletionSource gate; + bool release; + lock (_lock) { + gate = _gate; + release = ++_arrivals >= _participantCount; + if (release) { + _arrivals = 0; + _gate = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + } + + /*-------------------------------------------------------------------------------------------------------------------------- + | The last arrival releases the batch and proceeds without waiting + \-------------------------------------------------------------------------------------------------------------------------*/ + if (release) { + gate.TrySetResult(); + return Task.CompletedTask; + } + + /*-------------------------------------------------------------------------------------------------------------------------- + | Earlier arrivals wait for the last, throwing on timeout so a broken assumption fails loudly instead of hanging + \-------------------------------------------------------------------------------------------------------------------------*/ + return AwaitGate(gate, cancellationToken); + + } + + /*============================================================================================================================ + | METHOD: AWAIT GATE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Waits for to be released, throwing a if the batch never + /// completes. + /// + /// The gate to wait on for the current batch. + /// A token used to cancel the wait. + private async Task AwaitGate(TaskCompletionSource gate, CancellationToken cancellationToken) { + + // Race the gate against a timeout, so an unexpected participant count can't hang the test suite + var completed = await Task.WhenAny(gate.Task, Task.Delay(_timeout, cancellationToken)).ConfigureAwait(false); + + // Surface a failed rendezvous as an exception rather than proceeding with a corrupt result + if (completed != gate.Task) { + throw new TimeoutException( + $"The rendezvous timed out after {_timeout.TotalSeconds:0} seconds waiting for {_participantCount} concurrent " + + $"passes; the expected concurrency did not occur." + ); + } + + } + +} //Class \ No newline at end of file From 3a50d6b9ca5916c2e6c2c33330af28803c18db21 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 15:00:21 -0700 Subject: [PATCH 35/50] Added unit tests for concurrent collection mapping The `RendezvousTopicLazyLoader` allows us to test the locks on `SetCollectionValueAsync()` (86c6f7e6), which prevents duplicate collections from being created, and `PopulateTargetCollectionAsync()` (9dfc24c3), which prevents two items from being added concurrently. This relies on the `RendezvousTopicLazyLoader` (f6a85a24), the `ConcurrentExpansion` topic view models (96eae010), and the new `FaultEnsureLoadedGate()` (a101412d). It also introduces a new `BuildConcurrentExpansionGraph()` helper to construct the graph itself. This contributes to the testing of the mapped collection concurrently (86c6f7e6, 9dfc24c3) tests (#118). --- OnTopic.Tests/TopicMappingServiceTest.cs | 123 ++++++++++++++++++++++- 1 file changed, 121 insertions(+), 2 deletions(-) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index ab5ecbbe..8809978c 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -608,6 +608,57 @@ public async Task Map_ConcurrentSiblings_ObservesFault() { } + /*============================================================================================================================ + | TEST: MAP: CONCURRENT SHARED COLLECTION: POPULATES DETERMINISTICALLY + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Stress test confirming that two concurrent passes expanding the same shared model with disjoint associations, each + /// populating the same collection from a different source, deterministically produce the union of both sources without + /// corrupting the shared list. + /// + /// + /// A parent references one shared topic twice, with disjoint sets ( and ). Whichever pass wins + /// construction populates the shared list from one source; + /// the other expands it from the other, so the result should always be the union of both, regardless of which pass wins. A + /// holds the two passes at the collection warm-up until both arrive, then releases + /// them together, so their list mutations genuinely overlap. Repeated many times to give the race a chance to manifest; + /// without the shared-list mutation guard, the concurrent adds corrupt the list, dropping items or throwing. + /// + [Fact] + public async Task Map_ConcurrentSharedCollection_PopulatesDeterministically() { + + const int relationshipCount = 12; + const int incomingCount = 12; + const int repetitions = 100; + + var loader = new RendezvousTopicLazyLoader(participantCount: 2); + + for (var repetition = 0; repetition < repetitions; repetition++) { + + var root = BuildConcurrentExpansionGraph(loader, relationshipCount, incomingCount, out var shared); + + // Guard against an incomplete setup: Both sources must be loaded before mapping; read them via the backing accessor so + // the precondition check doesn't itself trip the loader's rendezvous and hang + var backing = (ITopicBackingAccessor)shared; + Assert.Equal(relationshipCount, backing.Relationships.GetValues("Related").Count); + Assert.Equal(incomingCount, shared.IncomingRelationships.GetValues("Related").Count); + + var result = await _mappingService.MapAsync(root); + + Assert.NotNull(result); + Assert.NotNull(result.RelationshipsView); + Assert.NotNull(result.IncomingView); + + // Both references resolve to the one shared instance, whose list holds the union of both sources + Assert.Same(result.RelationshipsView, result.IncomingView); + Assert.NotNull(result.RelationshipsView.Related); + Assert.Equal(relationshipCount + incomingCount, result.RelationshipsView.Related.Count); + + } + + } + /*============================================================================================================================ | TEST: MAP: DISABLED PROPERTY: RETURNS NULL \---------------------------------------------------------------------------------------------------------------------------*/ @@ -776,7 +827,7 @@ public async Task Map_AlternateAttributeKey_ReturnsMappedModel() { \---------------------------------------------------------------------------------------------------------------------------*/ /// /// Establishes a and then confirms that it is returned via . + /// "MappedTopicCache.TryGetValue"/>. /// [Fact] public void MappedTopicCache_TryGetValue_ReturnsEntry() { @@ -799,7 +850,7 @@ public void MappedTopicCache_TryGetValue_ReturnsEntry() { \---------------------------------------------------------------------------------------------------------------------------*/ /// /// Establishes a and then confirms that it is not returned via if the doesn't match. + /// .TryGetValue"/> if the doesn't match. /// [Fact] public void MappedTopicCache_TryGetValue_ReturnsNull() { @@ -2110,4 +2161,72 @@ ITopicMappingService MappingService return (inner, cache, new TopicMappingService(cache, _typeLookupService)); } + /*============================================================================================================================ + | METHOD: BUILD CONCURRENT EXPANSION GRAPH + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Builds an in-memory graph for : A parent that + /// references a single topic twice, that topic having + /// outgoing relationships and incoming relationships under the key Related. + /// + /// + /// Everything is preloaded, so performs no fetching; only the topic is + /// stamped for lazy loading, since only its EnsureLoaded needs to "rendezvous" the two passes. Its is left so the mapper's collection warm-up actually calls + /// the loader, which then marks it before the base pass's nested-topic probe reads it. The + /// relationships are already . The shared topic deliberately has no Related child, so + /// the nested-topic probe never displaces the relationship and incoming-relationship sources. + /// + /// The lazy loader to stamp on the shared topic. + /// The number of outgoing relationships to wire under Related. + /// The number of incoming relationships to wire under Related. + /// The shared topic referenced twice by the returned parent. + /// The parent topic to map. + private static Topic BuildConcurrentExpansionGraph( + ITopicLazyLoader loader, + int relationshipCount, + int incomingCount, + out Topic shared + ) { + + /*-------------------------------------------------------------------------------------------------------------------------- + | Establish the parent and the shared target + \-------------------------------------------------------------------------------------------------------------------------*/ + var identity = 1; + var root = new Topic("ConcurrentExpansionRoot", "ConcurrentExpansionRoot", null, identity++); + shared = new Topic("Shared", "ConcurrentExpansionShared", null, identity++); + var backing = (ITopicBackingAccessor)shared; + + /*-------------------------------------------------------------------------------------------------------------------------- + | Wire the shared target's outgoing relationships (the first source for the shared collection) + \-------------------------------------------------------------------------------------------------------------------------*/ + for (var index = 0; index < relationshipCount; index++) { + backing.Relationships.SetValue("Related", new($"Relationship_{index}", "KeyOnly", null, identity++)); + } + + /*-------------------------------------------------------------------------------------------------------------------------- + | Wire the shared target's incoming relationships (the second source), via each origin's reciprocal outgoing relationship + \-------------------------------------------------------------------------------------------------------------------------*/ + for (var index = 0; index < incomingCount; index++) { + var origin = new Topic($"Incoming_{index}", "KeyOnly", null, identity++); + origin.Relationships.SetValue("Related", shared); + } + + /*-------------------------------------------------------------------------------------------------------------------------- + | Stamp the shared target and leave children NotLoaded so the collection warm-up calls the loader exactly once per pass; the + | relationships are already Loaded (no deferred targets), so the relationship probe reads them without autoloading + \-------------------------------------------------------------------------------------------------------------------------*/ + ((ITopicLazyLoadable)shared).Loader = loader; + backing.Children.LoadState = LoadState.NotLoaded; + + /*-------------------------------------------------------------------------------------------------------------------------- + | Reference the shared target twice from the parent, so both references map it concurrently + \-------------------------------------------------------------------------------------------------------------------------*/ + root.References.SetValue("RelationshipsView", shared); + root.References.SetValue("IncomingView", shared); + + return root; + + } + } //Class \ No newline at end of file From c5f191b6062dad15733b3f1e5ae0f14f81e51209 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 15:28:31 -0700 Subject: [PATCH 36/50] Rely on `WhenAll()` over `WhenAny()` This ensures that the order of siblings is honored. This wasn't an issue prior to lazy loading (#111) since tasks were CPU bound, but with the possibility of some siblings being fully loaded while others need to `await` a call to the persistence store, it's very easy for this to occur. This contributes to the concurrency fixes for `TopicMappingService` (#118). --- OnTopic/Mapping/TopicMappingService.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index aff17e3e..a2e76169 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -1101,11 +1101,13 @@ configuration.ContentTypeFilter is not null && /*-------------------------------------------------------------------------------------------------------------------------- | Process mapping tasks + >--------------------------------------------------------------------------------------------------------------------------- + | Awaited as a batch, then added in the order the tasks were queued in, rather than completion order; this keeps sibling + | order deterministic regardless of which child mappings genuinely await \-------------------------------------------------------------------------------------------------------------------------*/ - while (taskQueue.Count > 0) { - var dtoTask = await Task.WhenAny(taskQueue).ConfigureAwait(false); - var dto = await dtoTask.ConfigureAwait(false); - taskQueue.Remove(dtoTask); + var dtos = await Task.WhenAll(taskQueue).ConfigureAwait(false); + + foreach (var dto in dtos) { if (dto is not null) { addToList(dto); } From 3359853e73559b855f8677e8bd153793ed0bc09d Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 15:36:18 -0700 Subject: [PATCH 37/50] Introduced `StaggeredTopicLazyLoader` This provides a new `ITopicLazyLoader` test double which allows a `delay` to be registered as part of its construction, which is executed as part of `EnsureLoaded()`. This allows multiple calls to different instances of the loader to set different delays and, thus, result in out-of-order resolution of siblings, thus providing a test harness for the new `WhenAll()` fix (c5f191b6). This contributes to the testing of the `ITopicMappingService` concurrency (#118). --- .../TestDoubles/StaggeredTopicLazyLoader.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs diff --git a/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs b/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs new file mode 100644 index 00000000..964735e9 --- /dev/null +++ b/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs @@ -0,0 +1,35 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.Repositories; + +namespace OnTopic.Tests.TestDoubles; + +/*============================================================================================================================== +| CLASS: STAGGERED TOPIC LAZY LOADER +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// An that suspends for a fixed before completing, so a test can attach different instances to sibling topics and force their loads to +/// genuinely complete out of source order. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +[ExcludeFromCodeCoverage] +internal sealed class StaggeredTopicLazyLoader(TimeSpan delay): ITopicLazyLoader { + + /*============================================================================================================================ + | METHOD: ENSURE LOADED + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + async Task ITopicLazyLoader.EnsureLoaded(Topic topic, TopicPayload payload, CancellationToken cancellationToken) { + if (delay > TimeSpan.Zero) { + await Task.Delay(delay, cancellationToken).ConfigureAwait(false); + } + ((ITopicBackingAccessor)topic).Children.LoadState = LoadState.Loaded; + } + +} //Class \ No newline at end of file From 1a631ab344a8a685a521ffea286a5cb24555ff3b Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 15:40:56 -0700 Subject: [PATCH 38/50] Added unit test for collection mapping order This utilizes the newly introduced `StaggeredTopicLazyLoader` (3359853e) to validate the `WhenAll()` fix (c5f191b6) for ensuring source order is maintained when mapping collections. This contributes to the concurrency fixes for `TopicMappingService` (#118). --- OnTopic.Tests/TopicMappingServiceTest.cs | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs index 8809978c..b44fc322 100644 --- a/OnTopic.Tests/TopicMappingServiceTest.cs +++ b/OnTopic.Tests/TopicMappingServiceTest.cs @@ -1272,6 +1272,42 @@ public async Task Map_Children_ReturnsMappedModel() { )); } + /*============================================================================================================================ + | TEST: MAP: CHILDREN: STAGGERED COMPLETION: PRESERVES SOURCE ORDER + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a with children whose mapping tasks are forced to complete in reverse of + /// source order, and tests that the mapped collection nonetheless preserves source order. + /// + /// + /// Each child is stamped with its own and left + /// for , so mapping it genuinely awaits a delay before completing. The first child gets + /// the longest delay and the last gets none, so completion order is the reverse of source order; if collection population + /// added results in completion order rather than source order, this would come back reversed. + /// + [Fact] + public async Task Map_Children_StaggeredCompletion_PreservesSourceOrder() { + + var topic = new Topic("Test", "Descendent"); + var childKeys = new[] { "ChildTopic1", "ChildTopic2", "ChildTopic3", "ChildTopic4" }; + + for (var index = 0; index < childKeys.Length; index++) { + var child = new Topic(childKeys[index], "Descendent", topic); + var delay = TimeSpan.FromMilliseconds((childKeys.Length - index) * 25); + ((ITopicLazyLoadable)child).Loader = new StaggeredTopicLazyLoader(delay); + ((ITopicBackingAccessor)child).Children.LoadState = LoadState.NotLoaded; + } + + var target = await _mappingService.MapAsync(topic); + + Assert.NotNull(target); + Assert.Equal(childKeys.Length, target.Children.Count); + for (var index = 0; index < childKeys.Length; index++) { + Assert.Equal(childKeys[index], target.Children[index].Key); + } + + } + /*============================================================================================================================ | TEST: MAP: WITH DISABLED: SKIPS DISABLED \---------------------------------------------------------------------------------------------------------------------------*/ From 487ecf2d1001c94151dfa052e992fa5cfef261ea Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 15:42:20 -0700 Subject: [PATCH 39/50] Prefer direct `WhenAll(IEnumerable)` Previously, the queue was being wrapped in another array unnecessarily. --- OnTopic/Mapping/TopicMappingService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OnTopic/Mapping/TopicMappingService.cs b/OnTopic/Mapping/TopicMappingService.cs index a2e76169..4ae26e71 100644 --- a/OnTopic/Mapping/TopicMappingService.cs +++ b/OnTopic/Mapping/TopicMappingService.cs @@ -292,7 +292,7 @@ public class TopicMappingService(ITopicRepository topicRepository, ITypeLookupSe } } - await Task.WhenAll([.. propertyQueue]).ConfigureAwait(false); + await Task.WhenAll(propertyQueue).ConfigureAwait(false); /*-------------------------------------------------------------------------------------------------------------------------- | Return target @@ -427,7 +427,7 @@ private async Task MapAsync( foreach (var property in typeAccessor.GetMembers(MemberTypes.Property)) { taskQueue.Add(SetPropertyAsync(topic, target, associations, property, cache, attributePrefix, cacheEntry is not null, mapPath)); } - await Task.WhenAll([.. taskQueue]).ConfigureAwait(false); + await Task.WhenAll(taskQueue).ConfigureAwait(false); /*-------------------------------------------------------------------------------------------------------------------------- | Return result From 3a168365073cddc86d5d0a480b05c01ac810303f Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 16:02:49 -0700 Subject: [PATCH 40/50] Cover `Deferred` in `Topic.Relationships.Clear()` This updates the `Clear()` method on the `TopicRelationshipMultiMap()` to not only include the `Deferred` collection, but also to mark the collection as dirty if only deferred items were cleared. Unlike the rest of this branch, this doesn't pertain to concurrency issues created by lazy loading (#111), but it does relate to a bug exposed in the `ReverseTopicMappingService` by the introduction of the `Deferred` associations. --- .../Associations/TopicRelationshipMultiMap.cs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/OnTopic/Associations/TopicRelationshipMultiMap.cs b/OnTopic/Associations/TopicRelationshipMultiMap.cs index 94021454..dfcdafa1 100644 --- a/OnTopic/Associations/TopicRelationshipMultiMap.cs +++ b/OnTopic/Associations/TopicRelationshipMultiMap.cs @@ -67,20 +67,35 @@ internal void Clear() { } /// - /// Removes all objects grouped by a specific . + /// Removes all objects grouped by a specific , as well as any entries registered under that key. /// /// - /// If there are any objects in the specified , then the will be marked as . Delegates to for each entry so the reciprocal relationship is also removed from each target's . + /// If there are any objects or entries registered under the specified , then the will be marked as . Delegates to for each resolved entry so the + /// reciprocal relationship is also removed from each target's . Clearing the entries prevents a subsequent from resolving and + /// resurrecting relationships this call just removed. /// /// The key of the relationship to be cleared. public void Clear(string relationshipKey) { + Contract.Requires(!String.IsNullOrWhiteSpace(relationshipKey), nameof(relationshipKey)); + + var hadLoadedValues = _storage.GetValues(relationshipKey).Count > 0; + var hadDeferredEntries = Deferred.Remove(relationshipKey); + foreach (var topic in _storage.GetValues(relationshipKey).ToArray()) { Remove(relationshipKey, topic); } + + // Remove() already marks the key dirty for each resident topic it removes; if only deferred entries existed, mark it here + // so the clear isn't silently lost + if (!hadLoadedValues && hadDeferredEntries) { + _dirtyKeys.MarkAs(relationshipKey, markDirty: !_parent.IsNew); + } + } /// From e308bc5bfee26692e966b446c3cb605792adf029 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 16:10:02 -0700 Subject: [PATCH 41/50] Added unit test to confirm `Clear()` fix This confirms that the `Topic.Relationships.Clear()` call successfully removes `Deferred` items as well as resolved items, as per the recent fix (3a168365). Unlike the rest of this branch, this doesn't pertain to concurrency issues created by lazy loading (#111), but it does relate to a bug exposed in the `ReverseTopicMappingService` by the introduction of the `Deferred` associations. --- .../TopicRelationshipMultiMapTest.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/OnTopic.Tests/TopicRelationshipMultiMapTest.cs b/OnTopic.Tests/TopicRelationshipMultiMapTest.cs index bc563d74..4b5604e5 100644 --- a/OnTopic.Tests/TopicRelationshipMultiMapTest.cs +++ b/OnTopic.Tests/TopicRelationshipMultiMapTest.cs @@ -491,6 +491,30 @@ public void Clear_NoTopics_IsNotDirty() { } + /*============================================================================================================================ + | TEST: CLEAR: DEFERRED ENTRIES: REMOVES DEFERRED ENTRIES AND IS DIRTY + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Registers a entry with no corresponding target and + /// calls , confirming that the deferred entry is purged and reports true, even though no target topic was removed. + /// + [Fact] + public void Clear_DeferredEntries_RemovesDeferredEntriesAndIsDirty() { + + var topic = new Topic("Test", "Page", null, 1); + var relationships = new TopicRelationshipMultiMap(topic); + + relationships.Deferred.SetValue("Related", 999); + relationships.Deferred.SetValue("Other", 998); + relationships.Clear("Related"); + + Assert.False(relationships.Deferred.Remove("Related")); + Assert.True(relationships.Deferred.Remove("Other")); + Assert.True(relationships.IsDirty()); + + } + /*============================================================================================================================ | TEST: SET VALUE: MARK NOT DIRTY: IS NOT DIRTY \---------------------------------------------------------------------------------------------------------------------------*/ From 7d5cad06ee3ece3574ff1e33dedfaf12cfbebfee Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 16:18:11 -0700 Subject: [PATCH 42/50] Added unit test to test context of `Clear()` fix In the previous unit test (e308bc5b), I validated the base `Clear()` fix, ensuring that `Deferred` items are cleared alongside resolved targets (3a168365). This test complements that by ensuring that fix prevents `EnsureLoaded()` from "resurrecting" the deferred items after a `Clear()`, which is the core bug that the `ReverseTopicMappingService` was running into. Unlike the rest of this branch, this doesn't pertain to concurrency issues created by lazy loading (#111), but it does relate to a bug exposed in the `ReverseTopicMappingService` by the introduction of the `Deferred` associations (#118). --- OnTopic.Tests/ITopicLazyLoadableTest.cs | 40 ++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/OnTopic.Tests/ITopicLazyLoadableTest.cs b/OnTopic.Tests/ITopicLazyLoadableTest.cs index e90a8ee6..664ff5fa 100644 --- a/OnTopic.Tests/ITopicLazyLoadableTest.cs +++ b/OnTopic.Tests/ITopicLazyLoadableTest.cs @@ -3,6 +3,7 @@ | Client Ignia, LLC | Project Topics Library \=============================================================================================================================*/ +using OnTopic.Associations; using OnTopic.Repositories; using OnTopic.Tests.TestDoubles; using Xunit; @@ -19,6 +20,11 @@ namespace OnTopic.Tests; [ExcludeFromCodeCoverage] public class ITopicLazyLoadableTest { + /*============================================================================================================================ + | PROPERTY: CANCELLATION TOKEN + \---------------------------------------------------------------------------------------------------------------------------*/ + private static CancellationToken CancellationToken => TestContext.Current.CancellationToken; + /*============================================================================================================================ | TEST: IS LOADED: NON-RECURSIVE: IGNORES UNLOADED CHILDREN \---------------------------------------------------------------------------------------------------------------------------*/ @@ -161,7 +167,39 @@ public void IsLoaded_NotLoadedChildren_NeverTriggersLoad() { [Fact] public void EnsureLoaded_NullResolver_DoesNotThrow() { var topic = new Topic("Topic", "Page"); - ((ITopicLazyLoadable)topic).EnsureLoaded(TopicPayload.All); + ((ITopicLazyLoadable)topic).EnsureLoaded(TopicPayload.All, CancellationToken); + } + + /*============================================================================================================================ + | TEST: ENSURE LOADED: CLEARED RELATIONSHIP: DOES NOT RESURRECT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Registers a deferred entry, then calls on that key. Confirms that a subsequent for never reaches the ; since already purged the deferred entry, there is nothing left to resolve, and the + /// previously cleared relationship isn't resurrected. + /// + [Fact] + public async Task EnsureLoaded_ClearedRelationship_DoesNotResurrect() { + + var topic = new Topic("Test", "Page", null, 1); + var rawLoadable = (ITopicLazyLoadable)topic; + var rawTopic = (ITopicBackingAccessor)topic; + var loader = new TrackingTopicLazyLoader(); + + // Set up and clear via the backing accessor so this doesn't itself trigger a load once LoadState flips to NotLoaded; the + // loader is stamped afterward, ahead of the explicit EnsureLoaded() call below + rawTopic.Relationships.Deferred.SetValue("Related", 999); + rawTopic.Relationships.Clear("Related"); + rawLoadable.Loader = loader; + + await rawLoadable.EnsureLoaded(TopicPayload.Relationships, CancellationToken); + + Assert.False(loader.WasCalled); + Assert.Empty(rawTopic.Relationships.GetValues("Related")); + } /*============================================================================================================================ From c6e644dc5755f823f332b96e2618ebe3621c2b31 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 17:13:42 -0700 Subject: [PATCH 43/50] Serialize `PopulateTargetCollectionAsync()` Previously, both `MapAsync()`'s calls to `PopulateTargetCollectionAsync()` as well as `PopulateTargetCollectionAsync()` own mapping process were handled via task queues, allowing collections of view models to be asynchronously mapped and inserted into the target collection on `Topic`. The problem with this is that the `TopicMultiMap` that underlies `Topic.Relationships` is fundamentally not thread safe. And because it's composed of a collection of collections, it would be very difficult to make it thread safe, and especially because of the fact that the topic references themselves may be referenced multiple times, including via reciprocal associations (e.g., `IncomingRelationships`) and, thus, we can't just map a topic once and then serialize its insertion into the `TopicMultiMap`. This is an unfortunate compromise. That said, in practice, the `ReverseTopicMappingService` is typically used for processing a binding model from a form into a single topic and, thus, won't incur the costs, and even if it did, it's a rare process, not something that's happening multiple times for pretty much every request on the site, as it is with the `TopicMappingService`. This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111). --- .../Reverse/ReverseTopicMappingService.cs | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs b/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs index 57d4acc1..dbbab61c 100644 --- a/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs +++ b/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs @@ -183,6 +183,13 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { /// /// An instance of provided with attributes appropriately mapped. /// + /// + /// Properties are mapped sequentially, in source order, rather than concurrently; this avoids concurrent mutation of the + /// association collections (, , etc.), which aren't thread + /// safe, and which individual property mappers write to on the shared . As a result, an exception + /// thrown while mapping one property surfaces immediately, without waiting for or aggregating exceptions from subsequent + /// properties, and any properties mapped before the failure remain applied to . + /// private async Task MapAsync(object? source, Topic target, string? attributePrefix) { /*-------------------------------------------------------------------------------------------------------------------------- @@ -201,11 +208,9 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { /*-------------------------------------------------------------------------------------------------------------------------- | Loop through properties, mapping each one \-------------------------------------------------------------------------------------------------------------------------*/ - List taskQueue = []; foreach (var property in typeAccessor.GetMembers(MemberTypes.Property)) { - taskQueue.Add(SetPropertyAsync(source, target, property, attributePrefix)); + await SetPropertyAsync(source, target, property, attributePrefix).ConfigureAwait(false); } - await Task.WhenAll([.. taskQueue]).ConfigureAwait(false); /*-------------------------------------------------------------------------------------------------------------------------- | Return result @@ -535,27 +540,16 @@ private async Task SetReference( /// /// The to pull the binding models from. /// The target to add the mapped objects to. + /// + /// Children are mapped and added sequentially, in order, rather than concurrently; this + /// avoids concurrent mutation on the shared target and guarantees 's + /// resulting order matches the binding model, instead of varying with completion order. + /// private async Task PopulateTargetCollectionAsync( IList sourceList, KeyedTopicCollection targetList ) { - /*-------------------------------------------------------------------------------------------------------------------------- - | Queue up mapping tasks - \-------------------------------------------------------------------------------------------------------------------------*/ - List> taskQueue = []; - - //Map child binding model to target collection on the target - foreach (ITopicBindingModel childBindingModel in sourceList) { - Contract.Assume(childBindingModel.Key); - if (targetList.Contains(childBindingModel.Key)) { - taskQueue.Add(MapAsync(childBindingModel, targetList.GetValue(childBindingModel.Key)!)); - } - else { - taskQueue.Add(MapAsync(childBindingModel)); - } - } - /*-------------------------------------------------------------------------------------------------------------------------- | Remove orphaned topics \-------------------------------------------------------------------------------------------------------------------------*/ @@ -567,15 +561,23 @@ KeyedTopicCollection targetList } /*-------------------------------------------------------------------------------------------------------------------------- - | Process mapping tasks + | Map and add children in source order + >--------------------------------------------------------------------------------------------------------------------------- + | Sequential by design: concurrent MapAsync() calls would mutate non-thread-safe collections on the shared target Topic in + | parallel, and completion-order nondeterminism would make targetList's resulting order unpredictable. \-------------------------------------------------------------------------------------------------------------------------*/ - while (taskQueue.Count > 0) { - var topicTask = await Task.WhenAny(taskQueue).ConfigureAwait(false); - taskQueue.Remove(topicTask); - var topic = await topicTask.ConfigureAwait(false); - if (topic is not null && !targetList.Contains(topic.Key)) { + foreach (ITopicBindingModel childBindingModel in sourceList) { + + Contract.Assume(childBindingModel.Key); + + var topic = targetList.Contains(childBindingModel.Key) + ? await MapAsync(childBindingModel, targetList.GetValue(childBindingModel.Key)!).ConfigureAwait(false) + : await MapAsync(childBindingModel).ConfigureAwait(false); + + if (topic is not null && !targetList.Contains(topic.Key)) { targetList.Add(topic); } + } } From 277edabe4c047541019092743ba10cedef855a83 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 17:26:08 -0700 Subject: [PATCH 44/50] Introduced `StaggeredStubTopicRepository` The `StaggeredStubTopicRepository` performs the same task as `StaggeredTopicLazyLoader` (3359853e), except for `ITopicRepository.Load()` instead of `ITopicLazyLoader.EnsureLoaded()`. That said, because `StaggeredTopicLazyLoader` could have difference instances "stamped" onto each topic, it only required a simple `delay` argument in its constructor. With the `ITopicMappingService`, however, the same instance needs to be used for the entire workflow. As such, instead of a single `delay` argument, it takes the messier approach of accepting a map of topic keys to delays, which it cross-references with every `Load()` request. This provides a test double for aiding in the testing of the `ReverseTopicMappingService`'s `PopulateTargetCollectionAsync())` serialization (c6e644dc). This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111). --- .../StaggeredStubTopicRepository.cs | 68 +++++++++++++++++++ .../TestDoubles/StaggeredTopicLazyLoader.cs | 8 ++- 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 OnTopic.Tests/TestDoubles/StaggeredStubTopicRepository.cs diff --git a/OnTopic.Tests/TestDoubles/StaggeredStubTopicRepository.cs b/OnTopic.Tests/TestDoubles/StaggeredStubTopicRepository.cs new file mode 100644 index 00000000..d87ae28e --- /dev/null +++ b/OnTopic.Tests/TestDoubles/StaggeredStubTopicRepository.cs @@ -0,0 +1,68 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.Repositories; +using OnTopic.TestDoubles; + +namespace OnTopic.Tests.TestDoubles; + +/*============================================================================================================================== +| CLASS: STAGGERED STUB TOPIC REPOSITORY +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// A that delays by a per-key , letting a test invert completion order relative to call order. +/// +/// +/// +/// This is similar to , except that it staggers calls to , not . +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// +[ExcludeFromCodeCoverage] +internal sealed class StaggeredStubTopicRepository: StubTopicRepository { + + /*============================================================================================================================ + | PRIVATE FIELDS + \---------------------------------------------------------------------------------------------------------------------------*/ + private readonly IReadOnlyDictionary _delaysByKey; + + /*============================================================================================================================ + | CONSTRUCTOR + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Initializes a new instance of the with a delay for each unique key that + /// should complete out of call order. + /// + /// A map of unique topic key to the delay that should precede its resolution. + public StaggeredStubTopicRepository(IReadOnlyDictionary delaysByKey) { + _delaysByKey = delaysByKey; + } + + /*============================================================================================================================ + | METHOD: LOAD + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + public override async Task Load( + string uniqueKey, + Topic? referenceTopic = null, + TopicPayload payload = TopicPayload.None, + int depth = 0 + ) { + + // Delay resolution of this key, if configured + if (_delaysByKey.TryGetValue(uniqueKey, out var delay) && delay > TimeSpan.Zero) { + await Task.Delay(delay).ConfigureAwait(false); + } + + // Delegate to the base implementation to perform the actual lookup + return await base.Load(uniqueKey, referenceTopic, payload, depth).ConfigureAwait(false); + + } + +} //Class \ No newline at end of file diff --git a/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs b/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs index 964735e9..12d2e0a2 100644 --- a/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs +++ b/OnTopic.Tests/TestDoubles/StaggeredTopicLazyLoader.cs @@ -16,7 +16,13 @@ namespace OnTopic.Tests.TestDoubles; /// genuinely complete out of source order. /// /// -/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +/// This is similar to , except that it staggers calls to , not . +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// /// [ExcludeFromCodeCoverage] internal sealed class StaggeredTopicLazyLoader(TimeSpan delay): ITopicLazyLoader { From 909121db8387cffea18b2279432ba08e84e62eee Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 17:30:48 -0700 Subject: [PATCH 45/50] Introduced `NestedReferenceAttribute` model The `NestedReferenceAttributeTopicBindingModel` provides a binding model for testing the serialization fix of the `ReverseTopicMappingService`'s `PopulateTargetCollectionAsync()` method (c6e644dc). This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111). --- ...stedReferenceAttributeTopicBindingModel.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 OnTopic.Tests/BindingModels/NestedReferenceAttributeTopicBindingModel.cs diff --git a/OnTopic.Tests/BindingModels/NestedReferenceAttributeTopicBindingModel.cs b/OnTopic.Tests/BindingModels/NestedReferenceAttributeTopicBindingModel.cs new file mode 100644 index 00000000..9ce23ae0 --- /dev/null +++ b/OnTopic.Tests/BindingModels/NestedReferenceAttributeTopicBindingModel.cs @@ -0,0 +1,26 @@ +/*============================================================================================================================== +| Author Ignia, LLC +| Client Ignia, LLC +| Project Topics Library +\=============================================================================================================================*/ +using OnTopic.ViewModels.BindingModels; + +namespace OnTopic.Tests.BindingModels; + +/*============================================================================================================================== +| BINDING MODEL: NESTED REFERENCE ATTRIBUTE TOPIC +\-----------------------------------------------------------------------------------------------------------------------------*/ +/// +/// Provides a minimal implementation of a custom topic binding model with both a scalar value and a reference property, for +/// use as an item within a collection. +/// +/// +/// This is a sample class intended for test purposes only; it is not designed for use in a production environment. +/// +public class NestedReferenceAttributeTopicBindingModel : AttributeDescriptorTopicBindingModel { + + public NestedReferenceAttributeTopicBindingModel(string key) : base(key, "TextAttributeDescriptor") { } + + public AssociatedTopicBindingModel? BaseTopic { get; set; } + +} //Class \ No newline at end of file From 23f4adfe9ab32f64b83e04514757c7fe9162fdd5 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 17:34:25 -0700 Subject: [PATCH 46/50] Added unit test for topic mapping serialization This provides a unit test for serialization fix of the `ReverseTopicMappingService`'s `PopulateTargetCollectionAsync()` method (c6e644dc), using the newly introduced `StaggeredStubTopicRepository` (277edabe) and `NestedReferenceAttributeTopicBindingModel` (909121db). This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111). --- .../ReverseTopicMappingServiceTest.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/OnTopic.Tests/ReverseTopicMappingServiceTest.cs b/OnTopic.Tests/ReverseTopicMappingServiceTest.cs index 6b352628..4d738526 100644 --- a/OnTopic.Tests/ReverseTopicMappingServiceTest.cs +++ b/OnTopic.Tests/ReverseTopicMappingServiceTest.cs @@ -15,6 +15,7 @@ using OnTopic.TestDoubles.Metadata; using OnTopic.Tests.BindingModels; using OnTopic.Tests.Fixtures; +using OnTopic.Tests.TestDoubles; using Xunit; namespace OnTopic.Tests; @@ -325,6 +326,52 @@ public async Task Map_NestedTopics_ReturnsMappedTopic() { } + /*============================================================================================================================ + | TEST: MAP: NESTED TOPICS: STAGGERED COMPLETION: PRESERVES SOURCE ORDER + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Establishes a backed by a whose + /// per-item topic reference lookups resolve out of call order: The first-declared item resolves slowest, the last-declared + /// item resolves instantly. Confirms nested topics still land in the binding model's source order, since maps and adds each child sequentially rather than racing completions. + /// + [Fact] + public async Task Map_NestedTopics_StaggeredCompletion_PreservesSourceOrder() { + + // Declared in call order; delays fall in reverse, so the first-added item resolves last + List<(string UniqueKey, TimeSpan Delay)> attributes = [ + ("Root:Configuration:ContentTypes:Attributes:Key", TimeSpan.FromMilliseconds(120)), + ("Root:Configuration:ContentTypes:Attributes:ContentType", TimeSpan.FromMilliseconds(60)), + ("Root:Configuration:ContentTypes:Attributes:Title", TimeSpan.Zero) + ]; + + var delaysByKey = attributes.ToDictionary(attribute => attribute.UniqueKey, attribute => attribute.Delay); + var topicRepository = new StaggeredStubTopicRepository(delaysByKey); + var mappingService = new ReverseTopicMappingService(topicRepository); + var bindingModel = new ContentTypeDescriptorTopicBindingModel("Test"); + + for (var i = 0; i < attributes.Count; i++) { + bindingModel.Attributes.Add( + new NestedReferenceAttributeTopicBindingModel($"Attribute{i + 1}") { + BaseTopic = new() { + UniqueKey = attributes[i].UniqueKey + } + } + ); + } + + var topic = new ContentTypeDescriptor("Test", "ContentTypeDescriptor"); + var target = (ContentTypeDescriptor?)await mappingService.MapAsync(bindingModel, topic); + var container = target?.Children.GetValue("Attributes"); + + Assert.NotNull(container); + Assert.Equal( + Enumerable.Range(1, attributes.Count).Select(i => $"Attribute{i}"), + container.Children.Select(child => child.Key) + ); + + } + /*============================================================================================================================ | TEST: MAP: TOPIC REFERENCES: RETURNS MAPPED TOPIC \---------------------------------------------------------------------------------------------------------------------------*/ From f0c65ea20b851003c1530bcb1112a80247e463b8 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 17:54:37 -0700 Subject: [PATCH 47/50] Ensure target topic is fully loaded When mapping a binding model to a topic via the `ReverseTopicMappingService`, we need to make sure, at minimum, that the extended attributes are loaded, as otherwise unchanged values will be marked dirty, potentially resulting in e.g., the blob (in the `SqlTopicRepository`) being versioned despite no actual changes. To solve this, I `EnsureLoaded()` on `MapAsync()`. In addition, if any nested topics are present, I `EnsureLoaded()` children on both the topic as well as its nested topic container via `SetNestedTopicsAsync()`. This isn't strictly necessary, since these would be lazy loaded on access, but because property calls can only load synchronously, doing this preemptively allows the call to be asynchronous. This contributes to the concurrency updates to the `ReverseTopicMappingService` (part of #118) in response to the lazy-loading updates (#111). --- .../Reverse/ReverseTopicMappingService.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs b/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs index dbbab61c..6aac76a3 100644 --- a/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs +++ b/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs @@ -197,6 +197,14 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { \-------------------------------------------------------------------------------------------------------------------------*/ if (source is null) return target; + /*-------------------------------------------------------------------------------------------------------------------------- + | Warm extended attributes + >--------------------------------------------------------------------------------------------------------------------------- + | Without this, TrackedRecordCollection.SetValue() potentially runs against an unloaded extended attributes, and thus marks + | attributes as dirty even if they're unchanged, causing needless version rows on save. + \-------------------------------------------------------------------------------------------------------------------------*/ + await ((ITopicLazyLoadable)target).EnsureLoaded(TopicPayload.ExtendedAttributes).ConfigureAwait(false); + /*-------------------------------------------------------------------------------------------------------------------------- | Validate model \-------------------------------------------------------------------------------------------------------------------------*/ @@ -449,6 +457,13 @@ private async Task SetNestedTopicsAsync( \-------------------------------------------------------------------------------------------------------------------------*/ var sourceList = (IList?)memberAccessor.GetValue(source) ?? new List(); + /*-------------------------------------------------------------------------------------------------------------------------- + | Warm target's children + >--------------------------------------------------------------------------------------------------------------------------- + | Replaces the Children getter's synchronous autoload with an explicit, asynchronous warm-up prior to the below probe + \-------------------------------------------------------------------------------------------------------------------------*/ + await ((ITopicLazyLoadable)target).EnsureLoaded(TopicPayload.Children).ConfigureAwait(false); + /*-------------------------------------------------------------------------------------------------------------------------- | Establish target collection to store mapped topics \-------------------------------------------------------------------------------------------------------------------------*/ @@ -458,6 +473,14 @@ private async Task SetNestedTopicsAsync( container.IsHidden = true; } + /*-------------------------------------------------------------------------------------------------------------------------- + | Warm container's children + >--------------------------------------------------------------------------------------------------------------------------- + | The container can be NotLoaded even when target is loaded; PopulateTargetCollectionAsync()'s Contains() check for existing + | children as well as it's check for orphans require the complete set + \-------------------------------------------------------------------------------------------------------------------------*/ + await ((ITopicLazyLoadable)container).EnsureLoaded(TopicPayload.Children).ConfigureAwait(false); + /*-------------------------------------------------------------------------------------------------------------------------- | Map the topics from the source collection, and add them to the target collection \-------------------------------------------------------------------------------------------------------------------------*/ From 59a8b85c440ad84e87185db6fd8acf37c345ddb5 Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 18:13:56 -0700 Subject: [PATCH 48/50] Optionally allow tracking load counting On the `TrackingTopicLazyLoader` test double, update it to track every load (via a new `_payloads` field) and, optionally, `SetLoadState()` for the topic, like a normal `ITopicLazyLoader` would be expected to do on `EnsureLoaded()`. This updates `WasCalled` to be readonly, automatically reflecting whether there are any calls recorded in the `_payloads` field. This will be necessary to test the warmup of the target topic(s) via the `ReverseTopicMappingService` (f0c65ea2). This contributes to the concurrency testing of the `ReverseTopicMappingService` (part of #118) in response to the lazy-loading updates (#111). --- .../TestDoubles/TrackingTopicLazyLoader.cs | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/OnTopic.Tests/TestDoubles/TrackingTopicLazyLoader.cs b/OnTopic.Tests/TestDoubles/TrackingTopicLazyLoader.cs index e340a958..793f9d67 100644 --- a/OnTopic.Tests/TestDoubles/TrackingTopicLazyLoader.cs +++ b/OnTopic.Tests/TestDoubles/TrackingTopicLazyLoader.cs @@ -13,8 +13,20 @@ namespace OnTopic.Tests.TestDoubles; /// /// A minimal spy that records whether it was invoked, without performing any actual loading. /// +/// +/// By default, this doesn't mutate , so a stamped topic remains +/// even after a call, letting tests assert that a specific code path either suppresses or triggers autoloading. Pass +/// to instead simulate a real loader's fill, marking the requested payload on each call, when a test needs to confirm that a caller warms a payload exactly once +/// rather than relying on this spy's inertness to inflate the count. +/// [ExcludeFromCodeCoverage] -internal sealed class TrackingTopicLazyLoader : ITopicLazyLoader { +internal sealed class TrackingTopicLazyLoader(bool markLoaded = false) : ITopicLazyLoader { + + /*============================================================================================================================ + | PRIVATE FIELDS + \---------------------------------------------------------------------------------------------------------------------------*/ + private readonly List _payloads = []; /*============================================================================================================================ | PROPERTY: WAS CALLED @@ -22,14 +34,34 @@ internal sealed class TrackingTopicLazyLoader : ITopicLazyLoader { /// /// Returns if was invoked. /// - public bool WasCalled { get; private set; } + public bool WasCalled => _payloads.Count > 0; + + /*============================================================================================================================ + | PROPERTY: CALL COUNT + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Returns the number of times was invoked. + /// + public int CallCount => _payloads.Count; + + /*============================================================================================================================ + | PROPERTY: PAYLOADS + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Returns the passed to each invocation of , in + /// call order. + /// + public IReadOnlyList Payloads => _payloads; /*============================================================================================================================ | METHOD: ENSURE LOADED \---------------------------------------------------------------------------------------------------------------------------*/ /// Task ITopicLazyLoader.EnsureLoaded(Topic topic, TopicPayload payload, CancellationToken cancellationToken) { - WasCalled = true; + _payloads.Add(payload); + if (markLoaded) { + ((ITopicLazyLoadable)topic).SetLoadState(payload, LoadState.Loaded); + } return Task.CompletedTask; } From f2355088aa10149969cfaa630e1ccf321111dc7d Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 18:16:49 -0700 Subject: [PATCH 49/50] Added unit test for topic mapping warmup This tests the warmup of the target topic(s) via the `ReverseTopicMappingService` (f0c65ea2), relying on the updates to the `TrackingTopicLazyLoader` to support counting and marking loads. This contributes to the concurrency testing of the `ReverseTopicMappingService` (part of #118) in response to the lazy-loading updates (#111). --- .../ReverseTopicMappingServiceTest.cs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/OnTopic.Tests/ReverseTopicMappingServiceTest.cs b/OnTopic.Tests/ReverseTopicMappingServiceTest.cs index 4d738526..c1caa9a5 100644 --- a/OnTopic.Tests/ReverseTopicMappingServiceTest.cs +++ b/OnTopic.Tests/ReverseTopicMappingServiceTest.cs @@ -372,6 +372,66 @@ public async Task Map_NestedTopics_StaggeredCompletion_PreservesSourceOrder() { } + /*============================================================================================================================ + | TEST: MAP: SPARSE TOPIC: FILLS EXTENDED ATTRIBUTES ONCE + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Maps a scalar-only binding model onto a target stamped with a whose are . Confirms warms exactly once at the start of the map, rather than leaving it to the attribute + /// collection's own synchronous autoload. + /// + [Fact] + public async Task Map_ScalarProperties_FillsExtendedAttributesOnce() { + + var bindingModel = new TextAttributeTopicBindingModel("Test") { + ContentType = "TextAttributeDescriptor", + DefaultValue = "World" + }; + + var target = new TextAttributeDescriptor("Test", "TextAttributeDescriptor"); + var loader = new TrackingTopicLazyLoader(markLoaded: true); + + ((ITopicLazyLoadable)target).Loader = loader; + target.Attributes.LoadState = LoadState.NotLoaded; + + _ = await _mappingService.MapAsync(bindingModel, target); + + Assert.Equal(1, loader.CallCount); + Assert.Equal(TopicPayload.ExtendedAttributes, loader.Payloads[0]); + + } + + /*============================================================================================================================ + | TEST: MAP: NESTED TOPICS: FILLS CONTAINER CHILDREN + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Maps a nested-topic binding model onto a target whose Attributes container is stamped with its own and left , even though the target's own are already loaded. Confirms warms the container + /// independently before PopulateTargetCollectionAsync probes its existing children. + /// + [Fact] + public async Task Map_NestedTopics_FillsContainerChildren() { + + var bindingModel = new ContentTypeDescriptorTopicBindingModel("Test"); + + bindingModel.Attributes.Add(new TextAttributeTopicBindingModel("Attribute1")); + + var target = new ContentTypeDescriptor("Test", "ContentTypeDescriptor"); + var container = new Topic("Attributes", "List", target); + var containerLoader = new TrackingTopicLazyLoader(markLoaded: true); + + ((ITopicLazyLoadable)container).Loader = containerLoader; + container.Children.LoadState = LoadState.NotLoaded; + + _ = (ContentTypeDescriptor?)await _mappingService.MapAsync(bindingModel, target); + + Assert.Equal(1, containerLoader.CallCount); + Assert.Equal(TopicPayload.Children, containerLoader.Payloads[0]); + + } + /*============================================================================================================================ | TEST: MAP: TOPIC REFERENCES: RETURNS MAPPED TOPIC \---------------------------------------------------------------------------------------------------------------------------*/ From 4857676fe75180e2f40cc1bd0dbb6d00a0cdca4b Mon Sep 17 00:00:00 2001 From: Jeremy Caney Date: Tue, 4 Aug 2026 19:56:14 -0700 Subject: [PATCH 50/50] Defer `GetContentTypeDescriptors()` to first use Instead of blocking construction by potentially eagerly loading the entire `Root:Configuration` tree, instead opt to quickly initialize the `ReverseTopicMappingService`, and then call `GetContentTypeDescriptors()` on its first use. This addresses a blocking call in the `ReverseTopicMappingService` constructor, plus a potentially stale reference to the `Configuration` topic graph. In practice, we generally expect this data will be cached as part of the underlying `ITopicRepository`, even if it's not using e.g., the `CachedTopicRepository`, and so this wasn't actually buying us any performance benefit, while the staleness concern is introduced if the underlying layer doesn't do that (e.g., if it relied and a fast, no-cache persistence layer). This is the final update to make the topic mapping services (and, in this case, the `ReverseTopicMappingService`) compatible (#118) with concurrency concerns introduced by lazy loading (#111). --- .../Reverse/ReverseTopicMappingService.cs | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs b/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs index 6aac76a3..ca892ce5 100644 --- a/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs +++ b/OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs @@ -25,7 +25,6 @@ public class ReverseTopicMappingService : IReverseTopicMappingService { | PRIVATE VARIABLES \---------------------------------------------------------------------------------------------------------------------------*/ readonly ITopicRepository _topicRepository; - readonly ContentTypeDescriptorCollection _contentTypeDescriptors; /*============================================================================================================================ | CONSTRUCTOR @@ -44,16 +43,6 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { | Set dependencies \-------------------------------------------------------------------------------------------------------------------------*/ _topicRepository = topicRepository; - _contentTypeDescriptors = topicRepository.GetContentTypeDescriptors(); - - /*-------------------------------------------------------------------------------------------------------------------------- - | Validate dependencies - \-------------------------------------------------------------------------------------------------------------------------*/ - Contract.Assume( - _contentTypeDescriptors, - $"The {nameof(ITopicRepository.GetContentTypeDescriptors)}() method returned null. This could indicate a corrupt " + - $"or data source." - ); } @@ -133,7 +122,7 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { Contract.Assume(source.ContentType, nameof(source.ContentType)); //Ensure the content type is valid - if (!_contentTypeDescriptors.Contains(source.ContentType)) { + if (!GetContentTypeDescriptors().Contains(source.ContentType)) { throw new MappingModelValidationException( $"The {nameof(source)} object (with the key '{source.Key}') has a content type of '{source.ContentType}'. There " + $"are no matching content types in the ITopicRepository provided. This suggests that the binding model is invalid. " + @@ -168,6 +157,30 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { } + /*============================================================================================================================ + | PRIVATE: GET CONTENT TYPE DESCRIPTORS + \---------------------------------------------------------------------------------------------------------------------------*/ + /// + /// Retrieves the from the . + /// + /// + /// Called per-use rather than cached into a field, since content types can be added after this service is constructed, and + /// a local cache would silently exclude any updates for the life of the service. Further, , + /// the base class for every production in this library, already caches the result after the + /// first call and maintains the live collection in place (e.g. Delete refreshes it), so the per-call access here is + /// expected to be cheap, acknowledging that's a property of that base class, not a guarantee of the interface itself. + /// + private ContentTypeDescriptorCollection GetContentTypeDescriptors() { + var contentTypeDescriptors = _topicRepository.GetContentTypeDescriptors(); + Contract.Assume( + contentTypeDescriptors, + $"The {nameof(ITopicRepository.GetContentTypeDescriptors)}() method returned null. This could indicate a corrupt " + + $"data source." + ); + return contentTypeDescriptors; + } + /*============================================================================================================================ | PRIVATE: MAP (TOPIC) \---------------------------------------------------------------------------------------------------------------------------*/ @@ -209,7 +222,7 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) { | Validate model \-------------------------------------------------------------------------------------------------------------------------*/ var typeAccessor = TypeAccessorCache.GetTypeAccessor(source.GetType()); - var contentTypeDescriptor = _contentTypeDescriptors.GetValue(target.ContentType); + var contentTypeDescriptor = GetContentTypeDescriptors().GetValue(target.ContentType); BindingModelValidator.ValidateModel(typeAccessor, contentTypeDescriptor, attributePrefix); @@ -252,7 +265,7 @@ private async Task SetPropertyAsync( | Establish per-property variables \-------------------------------------------------------------------------------------------------------------------------*/ var configuration = memberAccessor.Configuration; - var contentTypeDescriptor = _contentTypeDescriptors.GetValue(target.ContentType); + var contentTypeDescriptor = GetContentTypeDescriptors().GetValue(target.ContentType); var compositeAttributeKey = configuration.GetCompositeAttributeKey(attributePrefix); Contract.Assume(contentTypeDescriptor, nameof(contentTypeDescriptor));