Uh oh!
There was an error while loading. Please reload this page.
Lazy Loading Topic Repository - #145
Merged
Merged
Conversation
When I originally introduced (55241c6) and implemented (a9035bb) `ITopicBackingAccessor`, I hadn't included `Attributes` because a) it doesn't have a backing field, and b) it doesn't trigger lazy-loading based on access to the `Attributes` property. But this is inconsistent, leaks internal implementation details to understand, and fails to insulate us in case we decide to change how attributes are lazy-loaded in the future. Based on that, I've retrofitted `ITopicBackingAccessor` with `Attributes`, and added the explicit interface implementation for it to `Topic`.
Throughout the lazy-loading infrastructure (#111), I ensured that access to the lazy-loaded properties on the `Topic` object are accessed through `ITopicBackingAccessor` (55241c6, 819e373) to ensure that they don't trip a loop on triggering the lazy-loading contract they're attempting to fulfill. As part of this, I established the convention of calling this `rawTopic`. There were several places where this was the only topic reference needed, and I could have changed the signature, but I kept the boxing from `topic` to `rawTopic` and variable name to reinforce that this is going through the backing fields.
Previously, when restoring a topic to a previous version, we cleared the `Relationships` and `References` before establishing the database connection, so that they could be repopulated with new content. This moves that within the connection and after the reader is executed so that this isn't performed unless we have successfully connected to the server and (presumably) retrieved the requested content. As part of this, I also clear the `Deferred` collections, as related to #111. They will be repopulated if there are any misses from the versioned associations.
In the `EnsureLoaded()` and `EnsureLoadedAsync()` in the `SqlTopicRepository`, we only need to include `@IncludeRelationships` or `@IncludeAssociations` if the caller requested `TopicPayload.Children`, as we ALWAYS load those when introducing new topics to the topic graph (3c27813), and then store them in the `Deferred` collections (f210866, fce368b). If we're including new children in the call, however, those won't have that data and so, just like with `@IncludeExtendedAttributes`, they need to be accounted for. This gets confusing because `EnsureLoaded(Relationships)` or `EnsureLoaded(References)` don't actually do anything in the `SqlTopicRepository` right now. Those take the `Deferred` values, load them via `Load()≈, and incorporate them into the topic graph. Perhaps that should be in `SqlTopicRepository`, and I'll be revisiting that, but for now it means that those have no purpose here. As a result, I also added an early check-out at the top of `EnsureLoaded()` and `EnsureLoadedAsync()` if `TopicPayload` doesn't include `Children` or `ExtendedAttributes` so we avoid an unnecessary SQL calls. Finally, I also preemptively clear the `Deferred` collections on the parent/seed node if children are being loaded since, in that one case, it'll be processing those entries fresh. By clearing them, we avoid the possibility of creating duplicate entries—which doesn't hurt anything, but also doesn't add any value. As part of this, I made updates to a lot of the comments to help clarify this behavior. This contributes to #111.
Previously, I ensured that ascendants were loaded if we were calling a topic other than the root and the `referenceTopic` was null (1968876). The assumption was that `referenceTopic` would include the parent and, therefore, the loaded topic would have an existing home in the topic graph without needing the ascendant context. That assumption is invalid, and especially in context of lazy loading (#111). The `referenceTopic` is intended to connect this back to a graph, but it could still be well below any parent nodes that it needs to connect to that graph. As such, we _always_ need this, unless we're pulling the root node. (Even with a root node, this wouldn't hurt anything—there simply wouldn't be any data—but might as well save that quick query if we know to that to be the case, and especially given this is an existing capability.)
This is the core implementation of `EnsureLoaded()` and `EnsureLoadedAsync()` for `TopicPayload.Relationships` and/or `TopicPayload.References`, and thus a key deliverable for the lazy-loading project (#111). In `Load()`, the `Relationships` and `References` arguments instruct the stored procedure to return the keys and topic IDs for those alongside topics (3c27813), which is a cheap call and we can store orphans in `Deferred` (f210866, fce368b) without needing to look them up again. In `EnsureLoaded()`, instead, we want to ensure that those topic IDs resolve to actual topics in the topic tree. That's a different task entirely. For now, this is is exclusively being implemented in `CachedTopicRepository`, not the underlying e.g., `SqlTopicRepository` (dc975bf), though I'll almost certainly want to revisit that later, ideally by some type of base class that supports all repositories. In the meanwhile, however, this establishes the business logic and allows this to work in real-world scenarios.
This is the final wiring required for lazy-loading associations (#111)! Now, calls to `Topic.Relationships` or `Topic.References()` will trigger the lazy-loading implementation in e.g., `CachedTopicRepository` (257545e), assuming it's being used. I need to revisit the strategy here for other cases (e.g., where someone is using `SqlTopicRepository` without the `CachedTopicRepository` for some reason, such as a temporary in-memory programmatic crawl), but this satisfies the core use case that most implementations will follow.
This clears the new `Deferred` collection (f210866, fce368b) at the end of each `EnsureLoaded()` call to prevent test data from infecting subsequent calls to this shared resource. If this becomes a problem, we'll need to instead spin up a new instance for each test. This lays the foundation for testing the lazy-loading implementation (#111).
The prior setup of the tests will now trigger lazy-loading. In practice, at this point, that doesn't hurt anything—but it should, and will in subsequent test versions. This prevents that eventuality. This contributes to the testing of #111.
In xUnit 3.x, the `Assert.NotNull()` establishes that the variable isn't null (as it should) and thus we can remove the null-forgiving or assurance operators.
The `parentId` parameter of `AddRow()` is `null` by default, so doesn't need to be explicitly defined.
Still not clear if this is just a JetBrains Rider issue or a legitimate issue with how XML docblocks are parsed. Regardless, keeping quoted identifiers on the same line addresses the issue. Also removed some implicit values while I was at it.
These pertain to #111.
This is a massive architectural shift and, obviously, a breaking change, but a) allows the lazy-loading (#111) of associations to happen in parallel, and b) will support migrating to .NET built-in dependency injection and, thus, taking advantage of its support for asynchronous loading, which isn't supported by the activator model today. This also required moving `EnsureLoaded()` to async only, and thus removing the original `EnsureLoaded()` and replacing it with the `EnsureLoadedAsync()`. A number of these cases still require `GetAwaiter().GetResult()`, which is unfortunately, though quite a few of those will be resolved in subsequent commits. One case that will be persisted is the synchronous lazy-loading when calling lazy-loading properties (#111) directly. That said, typically we're working through the `TopicMappingService` and, thus, the hope is that we can forecast those calls and asynchronously load them before the `TopicMappingService` gets that far. This also required rehoming the `SqlDataReaderExtensions` off of `DbDataReader` instead of the `IDataReader`. That's an unfortunate compromise, but needed to permit asynchronous operation.
Following the lead of `Load()`, moved `Save()` (and `SaveTopic()`), `Move()` (and `MoveTopic()`), `Delete()` (and `DeleteTopic()`), as well as `Rollback()` and `Refresh()` from synchronous to asynchronous. The benefit of this won't be as strong as with `Load()` in most cases, but it's more consistent and, since these are typically called from controllers, will be compatible with their existing call structure. This is tangentially related to #111.
This continues a change that was first implemented narrowly (aff6491).
This continues a one-off implementation previously (56379ac).
This may just be a particularity of Jetbrains Rider, but it doesn't otherwise accept `cref` references unless the entire quoted identifier is on its own line. An irritating limitation, and especially because it can otherwise lead to awkward wrapping, but it is what it is. I just hope Visual Studio is cool with this, and it's not a mutual incompatibility.
This partially undoes work committed previously (5c6bd85) in preferring implicit constructors where the type is known. There were some false positives!
If defining a constructor, the base default constructor will always be called. This isn't intuitive to me, but you learn something new every day. (Also, a word of caution when implementing base default constructors!).
Using the same lock used to synchronize the creation of the collection itself (86c6f7e), 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.)
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).
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).
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.) See PR #144 for a detailed breakdown of updates.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This implements the lazy-loading project (#111), so that
ITopicRepository.Load()returns a sparsely populated topic graph by default and fills in extended attributes, associations, children, and version history on demand, instead of eagerly loading the entire tree top-to-bottom.It also folds in the related tasks that grew out of that work:
TopicIndexcache #116Topic.Children#119ITopicRepository.Load()#120This is a large, deeply interdependent change targeting OnTopic 6.0.0, and will be the primary feature shaping that major update.
Because the effort spanned several sub-branches, the merged tasks (#116–#120) are summarized and cross-referenced to their corresponding issue and merge commit rather than re-enumerated their individual commits; see the linked issues for their internal breakdown.
Breaking Changes
Major
Load(),EnsureLoaded(),Save(),Move(),Delete(),Rollback(), andRefresh()now returnTaskLoad()now returns just the root topic, with everything else resolved on demand, rather than eagerly loading the whole treeLoad()signature (5852408, cac32ba): ATopicPayloadflags argument selects which content to load, and anint depthreplaces theisRecursivebooleanITopicRepository.Load()#120 (c1e80a5, 9351513)Save()andDelete()retain theirbool isRecursiveparameterLoad(topicId, version)is now detached (142b966, f22adb7): The version overload no longer merges into a live graph—that responsibility moved intoRollback()—and the now-unusedreferenceTopicparameter was removedincludeExternalReferencesparameter (10c5144)GetTopicsstored procedure parameters (8c2c81d): Conditional-load@Include…flags replace the old@DeepLoad, and a@Depthparameter replaces@LoadChildren/@LoadDescendants@Depthintroduced by Allow specifying depth ofITopicRepository.Load()#120OnTopic.Queryinglimitations (4d83e0e, 92a9995):FindFirst()andFindAll()only traverse topics already resident in memory, returning incomplete results on a sparse graphStubTopicRepositoryID offset (4f11cf8, 6ae375e): Seeded IDs and keys now start at 1 rather than 0 to avoid e.g.,00,000, and00000all collapsing to0Minor
TopicRelationshipMultiMapconstructor madeinternal(50f4376): This isTopic-specific infrastructure, so external callers should use the underlyingTopicMultiMapSetTopic()/RemoveTopic()overloads that pointed at already-removedSetValue()/Remove()overloadsTopicIsDirty()/MarkClean()overloads not required byITrackDirtyKeysTopicCollectionExtensions.AnyDirty()extensionExtendedaction and itsincludeMetadataplumbing are goneLazy Loading
Features
Extended Attributes
Extended attributes resolve on demand the first time an unindexed attribute is requested.
GetValue()trigger (9e712ff): WhenAttributeCollection.LoadStateisNotLoaded, a miss triggersEnsureLoaded()for the extended attributesHasExtendedAttributes(9475ec3, 5adf48c, aa19e64): Returned by theGetTopicsstored procedure soLoadStatecan be setLoadedwhen there are noneNotLoadedotherwiseautoLoadbypass (aecee85, 42495b9): A newGetValue(…, autoLoad)overload lets indexed reads skip the lazy loadGetBoolean(),GetInteger(),GetDouble(),GetDateTime(), andGetUri(), plusTitleandView, presume their attributes are indexedParentand/orBaseTopicTopic.Attributesdoesn't trigger a load, unlike the other lazy collectionsAssociations
Relationships and references are always returned cheaply (as
keyandtopicIdpairs) withLoad(), now held internally on aDeferredcollection when their target isn't yet in the graph, and connected to real topics on demand.DeferredAssociationrecord (ba72f23) andDeferredcollections (f210866) onTopicRelationshipMultiMapandTopicReferenceCollectionDeferredAssociationCollectionwhoseSetValue()deduplicates entriesDeferredAssociationCollectionLoad()(3c27813)Deferredif unavailable in the topic graph (fce368b)Deferredonce the association is resolved (520ce3c)LoadStatederives from theDeferredcount rather than manual trackingIsFullyLoadedbit (c65307f)EnsureLoaded(Relationships|References)loads theDeferredtargetsTopic.RelationshipsorTopic.References(f7a86d8)Deferred Targets
ResolveAssociations(): Connects deferred targets to any in-memory topicsfallBackToLoadflag decides whether an unresolved target is loaded viaLoad()or left deferredLazyLoadingTopicRepositorybelowILazyLoaderviaStampAscendants()Relationships & References
SqlTopicRepository(dc975bf):EnsureLoaded()only requests associations when loading new children, since they always accompanyLoad()ed topicsLoadStateofRelationshipsandReferencesisn't set directly viaEnsureLoaded()LoadStateis computed from theDeferredcount;NotLoadedwhile any target remains deferred,Loadedonce all are resolvedBaseTopicis always lazy loaded on reference to ensure integrity of attribute inheritance (951eba0)Children
Accessing an unloaded
Topic.Childrencollection loads its immediate children on demand._childrenlets internal callers (e.g.,SetParent()) reach the collection without tripping a load, even when it isNotLoadedTopic.ChildrenwhileNotLoadedinvokes the loader'sEnsureLoaded()SqlTopicRepositoryfills children by via theGetTopicsstored procedure (via the new@Depthparameter)AddChildTopic()(0224cea) andFillChildren()(f7fc266, f4a3e53): Internal methods for processing children duringEnsureLoaded()Save()(110584b) andFindFirst()/FindAll()(4d83e0e, 92a9995, a304e90) checkLoadStatebefore recursing, so never trigger cascading lazy loadingVersion History
VersionHistoryCollection(7251476, 609b5aa): A dedicated backing collection withLoadStateITopicBackingAccessor(10959ed)TopicPayload(8fa4a93)IsLoaded()andSetLoadState()cover version historySetVersionHistory()(155a605) marks itLoadedEnsureLoaded()(b28a31a) andLoad()(dea4bc9) conditionally set theGetTopicsstored procedure's@IncludeHistoryparameterImplementation
Topic
IsLoaded(payload)(e5ddc26) andIsLoaded(payload, depth)(b339d61): Check whether the requested payload is already loadedSetLoadState(payload, state)(03a1e24, 5fc6da9): Sets the load state for each site named in a payload, so callers don't need to know where each lives or risk tripping a loadFilterPayload(payload)(3e3d538, 3ea5d45, 31b986a): Strips already-loaded flags from a payloadrawTopicconvention (2c4e23d): Lazy-loaded properties are reached throughITopicBackingAccessor(see below) internally to avoid re-tripping the contract they fulfillInterfaces
ITopicBackingAccessor(55241c6, a9035bb, 951eba0): Direct access to the backing fields of lazy-loaded properties onTopicwithout triggering a loadChildren, associations (Relationships,References) (e1b28c7), (extended)Attributes(d081413), andVersionHistoryITopicLazyLoadable(1007d6a, 248411e, e66cea7): The lazy-loading surface onTopic, exposed as explicit default interface members so it's hidden from adopters but available to infrastructureLoader(819e373),EnsureLoaded()(9df2db7),IsLoaded()/IsLoaded(depth), andSetLoadState()ITopicLazyLoader(59fa20d, a159b61, 9c88454): A narrow, per-repository loader exposing onlyEnsureLoaded(), modeled on EF Core'sILazyLoaderITopicRepository(e25f843)ITopicLazyLoadableonTopicviaEnsureLoaded()(9df2db7) and aLoaderpropertyLoaderis written to byStampLoader()(819e373) and, forChildren,EnsureLoaded()(5b293fd)Enums
LoadState(94c8666, 486f963, a013fe0): Replaces theIsFullyLoadedbit,TopicReferenceCollection,TopicRelationshipMultiMap,AttributeCollection, andKeyedTopicCollection<T>TopicPayload(a0071d6, 07a01cf, f242fcb, c7b356a): A flags enum naming data what to load per topicChildren,Relationships,References,ExtendedAttributes, andVersionHistoryStored Procedures
GetTopics(9c91f29): Extended for composable, partial loads@Include…flags (8c2c81d) such as@IncludeExtendedAttributesTopicPayload@LoadAscendants(8c2c81d)@Depthvia Allow specifying depth ofITopicRepository.Load()#120 (-1replaces@DeepLoad)@Depthof1supportsLoad(TopicPayload.Children)(5da1190)HasChildrenbit so childrenLoadStatecan be set (e3e7cfb)HasExtendedAttributesbit so attributesLoadStatecan be set (9475ec3, 5adf48c, aa19e64)HasChildrenandHasExtendedAttributesbackfilled toGetTopicUpdatesandGetTopicVersion(asnull) to keep the result shape consistent (8be260e)GetSitemap: A new lean projection for the sitemapGetTopicUpdates: Ordering fixed so ascendants precede descendantsTopicIndexcache #116Repositories
LazyLoadingTopicRepositoryA new base sitting between
ObservableTopicRepositoryand bothTopicRepositoryandTopicRepositoryDecorator, centralizing lazy-loading logic (f93a982).StampLoader()(2a1ce8e, 6cd4e80, 70005a1, cd21597): Associates aTopicwith theITopicLazyLoaderthat most recently loaded it, so it can resolve missing content on demandTopicLoadedandTopicSavedevents, so consumers never invoke it directlyStampAscendants()(29969d1): Stamps a loaded topic's ancestors so they, too, lazy-loadLoadDeferredAssociations(): Resolves a topic's deferred associations, implemented on both this base (b787626, 1935995) andSqlTopicRepository(cf376a7)ResolveAssociations()(167067f): Connects resident associations without loading missing onesTopicLoadedfor children (4caa4b4):EnsureLoaded(Children)now firesTopicLoaded, matchingLoad()and association loadsSemaphoreSlimgates viaWithLoadGate()(9351513) inLoad(topicId),Load(uniqueKey), andEnsureLoaded()prevent concurrent lazy loads of the same topic from racingSqlTopicRepositoryEnsureLoaded()(de60afb): Fills the requested payload from theGetTopicsstored procedures, wiring the enum to the parameters viaAddEnsureLoadedParameters()seedTopicId) to conditionally setChildren.LoadStatefor ancestors (5f92c5f, 5b4bf6b, 1b602d0)LoadDeferredAssociations(): Resolves a topic's deferred associations (cf376a7)ConvergeLoadState()(47769ad, b5da944): Reconciles a collection'sLoadStatewhen a subsequentLoad()merges a differentTopicPayloadinto an existing topicAddChildTopic()(c998de1) andLoadTopicGraph()(9a7594a)Delete()lazy awareness (665e7f5): Check for children viaLoadStateandITopicBackingAccessorto prevent lazy-loading children when validating theisRecursiveAddTopic()root lookup (fa2dd93): Returns the topic directly instead of re-looking it up by IDCachedTopicRepositoryOnTopicLoaded()(7ad38df),OnTopicSaved(),OnTopicDeleted(),OnTopicMoved(), andOnTopicRenamed()handlers with a sharedRekeyTopicSubtree()helper (d508408)IndexTopic()(f50cd8b)TopicIndexRegistryTopicIndexRegistry, Create staticTopicIndexcache #116uniqueKeynormalized against the cached root (a31a054)EnsureLoaded()delegates to the inner loader (3cc30d4) and stamps any loaded children (78b314b)EnsureLoaded()overload re-runs and merges a query when a cached topic doesn't satisfy the requested payload (012c37c)Root:Configurationeager-loaded since it's commonly referenced and richly associatedCachedTopicRepository(4c14f31), including removal of its_topicRepositoryinitialization (2804a55), with the seed's unit test updated to match (8d0af7d)MergeIntoCache()LoadTopicGraph()andConvergeLoadState()came to merge divergent payloads into thereferenceTopicdirectlyCollections
ChildTopicCollection: A dedicated collection type forTopic.ChildrenLoadStatehere from the sharedKeyedTopicCollection<T>base, since it's uniquely relevant to childrenKeyedTopicCollection<T>is also used byContentTypeDescriptorCollectionandAttributeDescriptorCollection, which don't us thisTopicIndexRegistryhooks for maintaining the index when children are attached or detachedTopic.Children#119 (ed34ca1)TopicRelationshipMultiMap/TopicReferenceCollection: Gained theDeferredcollections andLoadStateSourcetightened toprivate protectedto preserve the read-only promise (482a4ff)isIncomingnow comes exclusively from the constructor rather than per-call parametersTopicRelationshipMultiMap.Clear()(2589d79): Removes an entire relationship set at once, honoring dirty state and reciprocalIncomingRelationshipsremoval (see Bug Fixes)DeferredAssociationCollection(687d1ef, f2add3a): TheDeferredproperty type, with deduplicatingSetValue()andRemove()DeferredAssociationis dirty-aware for rollback (see Version Management)ReplaceAll()swaps the full set forRollback()(41be0d8)TopicIndexRegistry: AConditionalWeakTableof one liveTopicIndexper graph, keyed by the root topic instanceGetTopicIndex()recomputation withGetLiveTopicIndex()TopicIndexconverted toConcurrentDictionaryand filtering outIsNewtopics (4037de9)TopicIndexcache #116 (cb8e2ca)VersionHistoryCollection: See Version HistoryMapping (#118)
Lazy loading turned mapping into a potentially I/O-bound, concurrent operation, surfacing defects that never occurred with an eager-loaded tree. This work was developed on its own branch and is summarized here.
AssociationMap.PayloadMappings(5253452, e7d8ec4): Maps theCollectionTypeenum toTopicPayloadso the mapper firesEnsureLoaded()as early in the chain as possible, the practical hook through which most lazy loading is triggeredTopicPayloadas part ofMapAsync()#133 in the futureTopicMappingService: Closed lazy-loading defects that:ReverseTopicMappingService:Relationships.Clear()to purgeDeferredHierarchicalTopicMappingService:Load(…, depth)in one round-trip instead of anEnsureLoaded()per unloadedChildrenDepth-limited loading (#120)
Load()now accepts anint depth(replacingisRecursive)TopicLoadEventArgs,IsLoaded(), and every stubGetTopicsstored procedure's@Depthparameter (replacing@LoadChildren/@LoadDescendants) and a newParentIDindexHierarchicalTopicMappingServicewarm a full tier in one round-tripITopicRepository.Load()#120 (c1e80a5, 9351513)Testing
New infrastructure for genuinely exercising partial and lazy loads, where prior tests only covered eager graphs.
StubLazyLoadingTopicRepository(aa2c476): Actual partial- and lazy-loading test doubleStubLazyLoadingTopicRepositoryBuilder(198ae09): Fluent builder that materializesTopicRecordfixtures into real topicsTopicRecord(864d712): Barebones topic metadata for configuring the builder to load new topicsFakeSqlTopicRepository(de1784e, 14eae80): MimicsSqlTopicRepositoryviaLoadTopicGraph(), returning fresh instances to test cross-instance mergesLazyLoadingTopicRepositoryTest(db8fbe8): Tests specific to the new baseTopicRepositoryBaseTestcases were removed (fa60597) and the remainder migrated and improved (3338fed)ITopicLazyLoadableTest(8893273, d578cbf): Hosts theIsLoaded(),SetLoadState(), andEnsureLoaded()testsSqlTopicRepositoryTest: Coverage for the SQL load pathHasExtendedAttributesschema coverage (5e4f923), withLoadTopicGraph()extended/blob load-state tests (421c7e0)HasChildrencolumn added toTopicsDataTable(78602c2), with basic (7da41f5) and load-state (d347e3c) testsStoredProcedures.resx(5e93790)LoadTopicGraph()andConvergeLoadState()tests (86aaf98)Refresh()-path tests (3ff79e3)Load()overloads (a05ff70) andLoad(TopicPayload.Children)(f247c23)IsLoaded()andEnsureLoaded()tests (c8bb082) andITopicLazyLoadercoverage (dedb2af), backed by aLoadoverload forTestTopicRepository(305c35c)RelationshipsandRelatedadded to the stub repository (6bf7650) andDeferredcleared during setup (6e9cde8)FindFirst()andFindAll()gates (7fa99cc), with test expectations updated after the gate fixes (a8a95bb)VersionHistory(c5c1267)(ITopicLazyLoadable)cast into locals (0202356) and threadedCancellationTokenthrough the async test calls (ba648d7)Version Management
Beyond loading, this untangles version handling from the live graph.
Load(topicId, version)(142b966, f22adb7): Now only fetches a detached historical topic, no longer merging it into the live topic graphRollback()(142b966): Now responsible for merging the detached topic into the live graph via a new privateMergeVersion()onTopicRepository, shareable by any persistence implementationDeferredAssociation.IsDirty(eb15c3d) propagates through resolution (03445b3) and into the collection'sIsDirty()(3cd78b3), so associations merged during a rollback persist correctlySave()includesDeferredentries in itsRelationships/Referencesmappings@DeleteUnmatchedis now always set, since the full association set is available even when unresolved in memory (2986685)PersistRelationships()only callsUpdateRelationshipsfor keys that actually have dirty associations, avoiding a round-trip per keyLoadTopicGraph()intermediary (ff134c0) and aLoad(topicId, version)fake (30deeb3) that registers historical relationships (3e4e019), exercising rollback (bc98eb3) and dirty-deferred resolution (a0b2c3c)Sitemap
The
SitemapControllerneeds the entire tree, which is exactly what lazy loading avoids — loading and caching the whole graph would defeat the purpose. This gives the sitemap its own lean read path.ISitemapTopicRepository(f9aa14d): A minimal read-only repository exposing onlyLoad()SqlSitemapTopicRepository(bfd6fe2): A SQL implementation backed by the new leanGetSitemapstored procedure (1eb817d), returning the bare minimum the sitemap needsSitemapController.Index()is now async and drops itsrootTopicguard, since the new contract guarantees non-nullCacheOutput()policy — one hour, varying by hostname, query string, and scheme — since sitemaps are polled every few minutesMapTopicSitemap(cacheDuration)StubSitemapTopicRepository(755c060) wrapping an existingITopicRepository, wired intoHost(79ec7a6, 3b81cf1) andSitemapControllerTest(966d5c7)Bug Fixes
Fixes not otherwise specific to this branch's new code.
Refresh()ordering and orphans: A long-standingGetTopicUpdatesordering bug, where ascendants weren't guaranteed before descendants, potentially orphaning newly created topicsAddTopic()now skips the orphaned (parentless) rows that lazy loading can surfaceTopicIndexcache #116 (cb8e2ca)Clear()(7bfd09e): Clearing a relationship set left stale entries in the reciprocalIncomingRelationships. Now each is removed individually, reusing the existing reciprocal deletion logic (tested by 60e0cf9)00000to0, causing duplicate-key exceptionsCleanup
Largely mechanical passes, batched here to keep the substantive history readable.
#regionover per-member flowerboxes (ffdf518)usings (f6bfb68)CountoverAny()(a3b9a21)ToList()(7b56929)crefidentifiers on one line for Rider (6e8fb7d, 45feabe, e9bcbc8, 5745766)Load()call sites reformatted (43ba647)docs/,.idea/, and.DS_Store(fe4efe1)