Skip to content

Topic Mapping: Addressed concurrency issues introduced by Lazy-Loading - #144

Merged
JeremyCaney merged 50 commits into
feature/ITopicRepository-lazy-loadingfrom
improvement/lazy-loading/mapping
Aug 5, 2026
Merged

Topic Mapping: Addressed concurrency issues introduced by Lazy-Loading#144
JeremyCaney merged 50 commits into
feature/ITopicRepository-lazy-loadingfrom
improvement/lazy-loading/mapping

Conversation

@JeremyCaney

Copy link
Copy Markdown
Member

This addresses issues related to the TopicMappingService and ReverseTopicMappingService introduced by lazy-loading (#111) and the possibility of genuine I/O-bound concurrency, thus closing #118. (Many of these issues were preexisting, but would never occur with an eager-loaded tree with mapping being a CPU-bound operation.)

TopicMappingService

  • Fixed lazy-loading defects that either silently forced eager loads or returned incomplete data
    • Deferred GetSourceCollectionAsync()'s collection until after preconditionsMet (7777ed1, 6e064a1)
    • Made AttributeCollection.GetValue() await its extended attributes synchronously (d4b4020)
    • Made AsAttributeDictionary() load extended attributes before enumerating (03ed27c, 51d0f4e, faf3639)
    • Loaded metadata lookup list children before mapping [Metadata] properties (b88d436)
  • Prevented properties from being remapped when a view model is assessed with a broader association set (f287c1d, ec102ab, 49a6048)
  • Made concurrent mapping of the same topic/view-model pair (i.e., MappedTopicCacheEntry) safe
    • Made association "claims" atomic, so overlapping expansions no longer map the same associations (0f51264, a971aeb, bc73f87)
    • Introduced MapPath to track the MapAsync() constructor chain, distinguishing circular constructor references from valid sibling overlap
      • MapPath class (b42673a), tested via Contains() (a64b1f8)
      • Threaded through the MapAsync() and its private dependency chain (61b1487)
      • Circular reference detection tested with new CircularConstructorTopicViewModel (dbb4798, dc88d41, 4a67bab)
    • Added completion semantics to MappedTopicCacheEntry, so concurrent calls await construction instead of duplicating or colliding
      • Complete()/Fault() and Completion property (7a5bd2c), tested via Complete(), Fault(), Completion, and IsInitializing (28f0b07)
      • Wired up Complete() via Register() (ee9a9a5)
      • Wired up Fault() via MapAsync() (b197889)
      • New TryGetValue(…, includeInitializing) lookup (5a6b8a9), centralized in resolveCachedEntry() (1334a2b)
    • Tests confirm concurrent siblings mapping the same topic share one instance (678bf64) via new Concurrent topic view models (73e2f3a) and CreateGatedMappingService() helper (91d0a21)
      • A faulted creator faults concurrent joiners without hanging (6d59060)
    • Locked shared target collections, preventing races when the same member is populated from multiple sources
      • Creation, preventing two passes from installing competing collection instances (86c6f7e)
      • Population (addToList), preventing concurrent List<T> corruption when adding items (9dfc24c)
      • Tested with new RendezvousTopicLazyLoader and a fault-simulating repository (a101412, 96eae01, f6a85a2, 3a50d6b)
  • Replaced WhenAny with WhenAll to ensure sibling order is preserved (c5f191b, 487ecf2)
    • Tested with new StaggeredTopicLazyLoader under staggered loads (3359853, 1a631ab)

ReverseTopicMappingService

  • Fixed Topic.Relationships.Clear() to also purge Deferred entries (3a16836)
    • Tested that Clear() removes Deferred as well as resolved items (e308bc5)
    • Tested that EnsureLoaded() doesn't resurrect Clear()ed items afterward (7d5cad0)
  • Serialized PopulateTargetCollectionAsync(), since TopicMultiMap isn't thread-safe (c6e644d)
    • Tested (23f4adf) using new StaggeredStubTopicRepository (277edab) and NestedReferenceAttributeTopicBindingModel (909121d) fixtures
  • Warmed the target topic's extended attributes and children (for nested topics) (f0c65ea)
    • Tested (f235508) using updated load-counting on TrackingTopicLazyLoader (59a8b85)
  • Deferred the constructor's blocking GetContentTypeDescriptors() call to first use (4857676)

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.
These confirm the previous fix (7777ed1) 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).
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()` (9e712ff).
`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.
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 (03ed27c) introduced to patch the `TopicMappingService` (#118) in response to lazy loading (#111).
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 (51d0f4e) as part of #118 is correctly testing the right mapping method.
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).
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!
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 (f287c1d). 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.
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 (f287c1d). This relies on the two newly introduced view models (ec102ab) 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.
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).
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).
This updates the existing unit tests for `AddMissingAssociations()` to account for the concurrency fix (0f51264, a971aeb) related to #118.
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.
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).
This adds the new `MapPath` data object (b42673a) 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.
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` (7a5bd2c), 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.
When calling `MappedTopicCache.Register()`, call the new `MappedTopicCacheEntry.Complete()` method (7a5bd2c). 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).
When calling (the final private overload of) `TopicMappingService.MapAsync()`, call the new `MappedTopicCacheEntry.Fault()` method (7a5bd2c) 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).
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 ((7a5bd2c) that were previously integrated (ee9a9a5, b197889) by relying on the new `TryGetValue(…, includeInitializing)` parameter (5a6b8a9) to lookup a pending constructor.
It also calculates the `MapPath` class (61b1487, b42673a) 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.
These tests evaluate the new completion semantics (7a5bd2c), including `Complete()` and `Fault()` as well as the `Completion` and `IsInitializing` (ee9a9a5) 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`.
This provides a unit test for the new `MapPath` data container (b42673a) and, specifically, it's `Contain()` method, testing a similar situation as implemented in `resolveCacheEntry()` (1334a2b).
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`.
This will be used in a new unit test to confirm that the circular constructor reference detection, including the completion semantics (7a5bd2c, ee9a9a5, b197889, 5a6b8a9) and `MapPath` tracking (b42673a, 61b1487) as implemented in `resolveCachedEntry()` (1334a2b).
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`.
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` (dbb4798) 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.
This unit test utilizes the newly introduced `CircularConstructorTopicViewModel` (dbb4798) to create a scenario where a circular constructor reference occurs.
This confirms that the circular constructor reference detection, including the completion semantics (7a5bd2c, ee9a9a5, b197889, 5a6b8a9) and `MapPath` tracking (b42673a, 61b1487) as implemented in `resolveCachedEntry()` (1334a2b).
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`.
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` (b42673a, 61b1487), as detected via `resolveCacheEntry()` (1334a2b).
This will also evaluate the closely related completion semantics (7a5bd2c, ee9a9a5, b197889, 5a6b8a9).
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`.
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`.
This test utilizes the new `ConcurrentReferenceTopicViewModel` and `SharedConcurrentTopicViewModel` view models (73e2f3a) as well as the `CreateGatedMappingService()` helper (91d0a21) 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` (b42673a, 61b1487), as detected via `resolveCacheEntry()` (1334a2b).
This also evaluate the closely related completion semantics (7a5bd2c, ee9a9a5, b197889, 5a6b8a9).
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`.
This test utilizes the new `ConcurrentReferenceTopicViewModel` and `SharedConcurrentTopicViewModel` view models (73e2f3a) as well as the `CreateGatedMappingService()` helper (91d0a21) to confirm that a `Fault()` that occurs during mapping, similar to what's implemented in `MapAsync()` (b197889), throws an exception without hanging during concurrent construction of the same view model reference.
This also relies on the broader completion semantics (7a5bd2c, ee9a9a5, 5a6b8a9) 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).
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).
This complements the existing `ArmEnsureLoadedGate()` and `ReleaseEnsureLoadedGate()` (2875725, 8b56f3c) with a new `FaultEnsureLoadedGate()`, which will allow us to simulate an exception as part of the testing of the mapped collection concurrently (86c6f7e, 9dfc24c) tests (#118).
These two view models setup the conditions that will allow us to test the locks on `SetCollectionValueAsync()` (86c6f7e), which prevents duplicate collections from being created, and `PopulateTargetCollectionAsync()` (9dfc24c), which prevents two items from being added concurrently. This contributes to the testing of the `TopicMappingService` concurrency (#118).
The `RendezvousTopicLazyLoader` allows us to test the locks on `SetCollectionValueAsync()` (86c6f7e), which prevents duplicate collections from being created, and `PopulateTargetCollectionAsync()` (9dfc24c), 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).
The `RendezvousTopicLazyLoader` allows us to test the locks on `SetCollectionValueAsync()` (86c6f7e), which prevents duplicate collections from being created, and `PopulateTargetCollectionAsync()` (9dfc24c), which prevents two items from being added concurrently.
This relies on the `RendezvousTopicLazyLoader` (f6a85a2), the `ConcurrentExpansion` topic view models (96eae01), and the new `FaultEnsureLoadedGate()` (a101412). It also introduces a new `BuildConcurrentExpansionGraph()` helper to construct the graph itself.
This contributes to the testing of the mapped collection concurrently (86c6f7e, 9dfc24c) tests (#118).
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).
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 (c5f191b).
This contributes to the testing of the `ITopicMappingService` concurrency (#118).
This utilizes the newly introduced `StaggeredTopicLazyLoader` (3359853) to validate the `WhenAll()` fix (c5f191b) for ensuring source order is maintained when mapping collections.
This contributes to the concurrency fixes for `TopicMappingService` (#118).
Previously, the queue was being wrapped in another array unnecessarily.
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.
This confirms that the `Topic.Relationships.Clear()` call successfully removes `Deferred` items as well as resolved items, as per the recent fix (3a16836).
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.
In the previous unit test (e308bc5), I validated the base `Clear()` fix, ensuring that `Deferred` items are cleared alongside resolved targets (3a16836). 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).
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).
The `StaggeredStubTopicRepository` performs the same task as `StaggeredTopicLazyLoader` (3359853), 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 (c6e644d).
This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111).
The `NestedReferenceAttributeTopicBindingModel` provides a binding model for testing the serialization fix of the `ReverseTopicMappingService`'s `PopulateTargetCollectionAsync()` method (c6e644d).
This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111).
This provides a unit test for serialization fix of the `ReverseTopicMappingService`'s `PopulateTargetCollectionAsync()` method (c6e644d), using the newly introduced `StaggeredStubTopicRepository` (277edab) and `NestedReferenceAttributeTopicBindingModel` (909121d).
This contributes to the concurrency updates to the topic mapping services (#118) in response to the lazy-loading updates (#111).
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).
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` (f0c65ea).
This contributes to the concurrency testing of the `ReverseTopicMappingService` (part of #118) in response to the lazy-loading updates (#111).
This tests the warmup of the target topic(s) via the `ReverseTopicMappingService` (f0c65ea), 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).
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).
@JeremyCaneyJeremyCaney added this to the OnTopic 6.0.0 milestone Aug 5, 2026
@JeremyCaneyJeremyCaney self-assigned this Aug 5, 2026
@JeremyCaneyJeremyCaney added Area: Mapping Relates to one of the `ITopicMappingService` interfaces or implementations. Severity 3: Blocking Priority: 0 Type: Improvement Improves the functionality or interface of an existing feature. Status 5: Complete Task is considered complete, and ready for deployment. labels Aug 5, 2026
@JeremyCaney
JeremyCaney merged commit 3acee3b into feature/ITopicRepository-lazy-loadingAug 5, 2026
1 check passed
@JeremyCaney
JeremyCaney deleted the improvement/lazy-loading/mapping branch August 5, 2026 03:57
@JeremyCaneyJeremyCaney linked an issue Aug 14, 2026 that may be closed by this pull request
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: MappingRelates to one of the `ITopicMappingService` interfaces or implementations.Priority: 0Severity 3: BlockingStatus 5: CompleteTask is considered complete, and ready for deployment.Type: ImprovementImproves the functionality or interface of an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mapping Services Lazy-Loading Integration

1 participant

@JeremyCaney