Skip to content

Support moved and deleted topics via Refresh() #152

Description

@JeremyCaney

Today, ITopicRepository.Refresh() synchronizes an in-memory topic graph against the store but, by its own contract, handles value changes only: "It is not expected to handle deletes or reordering of topics." This proposal closes that gap: Refresh() should detect and apply the four structural remote changes it cannot see today—i.e., deletes, moves, reorders, and new topics—both by re-syncing the in-memory graph and by raising events for the affected topics, so e.g., caches can be invalidated per-topic.

This builds off of the establishment of the Refresh() and TopicUpdated scaffolding (#151), and it in turn unlocks the proposed Topic Output Caching's (#150) ability to rely exclusively on per-topic invalidation, instead of a full graph invalidation.

Implementation Notes

SQL

Renamed and new topics already bump Topics.LastModified, so they are already included in the first result set from the GetTopicUpdates stored procedure. Two items are missing:

  • MoveTopic should bump LastModified on the moved topic (a single UPDATE). It currently shifts the nested set and updates ParentID, but never touches LastModified, so a pure move or reorder is invisible to GetTopicUpdates. The moved row already carries its (new or unchanged) ParentID, which is all the consumer needs for both moves and reorders.
  • A DeletedTopics table, written by DeleteTopic inside its existing transaction (reusing its @Topics subtree table variable) containing the deleted Topic ID, its Parent ID, and a timestamp.
  • GetTopicUpdates includes a deleted result set filtered by DeletedDate > @Since, appended after the existing sets.

Merge and re-sync (OnTopic.Data.Sql)

Classification happens inside AddTopic(), where the old and new core fields are both visible before the overwrite:

  • current.Key != key is the rename trigger
  • current.Parent?.Id != parentId is the move trigger
  • A previously missing topic is new
  • An in-memory topic with an unchanged identity is a reorder

These generate a structural out collection (including deleted IDs, affected parents, and renamed or moved subtree roots) on LoadTopicGraph(), consumed exclusively by Refresh().

AddTopic() and SetParent() already handle moves (for both parents) as well as new topics, so only two things are genuinely new here:

  • Deletes: The deleted topics result set, which is independent of the merge
    • This must mirror the local Delete() in-memory deconstruction, not merely detach from the parent's Children, because that also severs the subtree's associations in both directions (outbound targets off X.Relationships and X.References, and X off each Y.IncomingRelationships)
  • Reorder: Ensure that a moved or reordered topic is correctly positioned within its parent's Children collection
    • AddTopic() and SetParent() append a new or moved child to the end, so a reorder is not currently reflected
    • Because in-memory topics don't carry a RangeLeft, we don't have enough information to sort them
    • Resetting Children to NotLoaded would only work if all descendants were removed, which would be potentially expensive
    • Therefore, we should refetch the parent's children (via GetTopics @Depth=1) and re-sort the in-memory collection to match

These run after the whole merge is read (a topic that moved and changed attributes in the same window must be merged first). The deleted result set must be conditioned on the structural out collection being present, which only Refresh() retrieves, never on the shared Load() or GetTopics() path. Events should fire only after a successful merge, as they do elsewhere.

Events (OnTopic core)

Refresh() should raise the same event that the equivalent local operation raises, so a subscriber reacts identically whether a change arrived locally via Save(), Move(), Delete() or remotely via Refresh(). Each event carries only the root affected topic; the subscriber makes any implied descendant changes (such as invalidating the subtree), exactly as it does for the TopicMoved and TopicRenamed events today.

Structural change Event Handler's implied action
Rename TopicRenamed Invalidate the topic and its subtree (paths are derived)
Move TopicMoved (Source and Target = old and new parent) Invalidate the subtree and both parents
Reorder TopicMoved (Source == Target) Invalidate the parent only
New topic TopicLoaded Invalidate the parent (its child list changed)
Delete TopicDeleted Invalidate the subtree
Value change TopicUpdated (#151, unchanged) Invalidate the topic
  • TopicUpdated remains as Notify changes from Refresh() via a new TopicUpdated event #151 defines it: Value changes to an already-in-memory topic. Identity and structure are not folded into it.
  • TopicMoveEventArgs already carries Source, Target, and Sibling, so both a move and a pure reorder (where Source == Target) both raise TopicMoved; the subscriber distinguishes them by comparing Source and Target.
  • TopicLoaded also fires on ordinary Load() and EnsureLoaded(), but that's harmless: A topic being added to the graph has never been rendered, so evicting its own tag is a no-op. The subscriber should ignore version loads (args.Version is not null). See the parent-eviction note under Limitations.
  • TopicDeleted should be raised before the in-memory deconstruction, not after. The persistence-layer deletion (e.g., the DeleteTopic stored procedure) already runs first, so the event still means the topic has been deleted from the store; firing it before the deconstruction lets a subscriber read the intact subtree and its associations in both directions—which is required for Add server-side output caching for topic pages based on CacheProfile #150's association eviction—without adding anything to DeleteEventArgs. Existing TopicDeleted subscribers must tolerate the topic still being attached at event time.
  • The decorator (TopicRepositoryDecorator) already forwards all five events, so no new forwarding is required.

Association eviction (built in #150)

Relationship and reference invalidation—i.e., evicting a page that renders a changed or deleted topic through an association, in either direction—is enabled by this proposal, but built in #150, using the events above. This proposal's only obligation is to make a deleted topic's associations readable at event time (i.e., the TopicDeleted ordering note above); looping over a topic's inbound IncomingRelationships and outbound Relationships and References and evicting each is #150's responsibility based on those events.

Affected Files

  • OnTopic.Data.Sql.Database:
    • New DeletedTopics table
    • Changes to the DeleteTopic, MoveTopic, and GetTopicUpdates stored procedures
    • A new stored procedure for cleaning up the DeletedTopics table
  • OnTopic.Data.Sql/SqlDataReaderExtensions: Classification, structural out collection, child re-ordering, and handling the deleted result set (Refresh() path only)
  • OnTopic.Data.Sql/SqlTopicRepository: Refresh() deletions, reconciling moves and reorders, and raising the structural events (TopicLoaded, TopicRenamed, TopicMoved, and TopicDeleted)
  • OnTopic/Repositories/ITopicRepository: Update the Refresh() remarks (e.g., remove "not expected to handle deletes or reordering") and document that Refresh() now raises these existing events
  • OnTopic/Repositories/TopicRepository: Extract the Delete() in-memory deconstruction into a helper the remote Refresh() can reuse, and raise TopicDeleted before it runs

Limitations

  • A rename or move co-occurring with a reorder of the same topic in one Refresh() window triggers an identity change but misses the reorder; sibling order stays stale until the next structural change
  • A cached page whose own topic is unchanged but whose navigation (or another shared menu) renders a renamed, moved, or deleted topic from outside its own ancestor chain stays stale until its own Duration expires
    • Note: Breadcrumbs are unaffected: A rename, move, or delete of an ancestor evicts that ancestor's whole subtree, which includes the page
  • A new topic causes a one-time, unnecessary eviction of its parent the first time a non-Refresh() caller loads that parent's children; after that first load, TopicLoaded no longer fires for those children, so the cost is limited and acceptable
  • The two mapping-layer caches (CachedHierarchicalTopicMappingService<T>, CachedTopicMappingService) aren't evicted today; these can optionally subscribe to these same events to evict by Topic.Id as a future follow-up

Tasks

  • Add the DeletedTopics table
  • Write deleted rows from DeleteTopic inside its existing transaction
  • Bump LastModified on the moved topic in MoveTopic
  • Append the deleted result set to GetTopicUpdates (DeletedDate > @Since)
  • Add a DeletedTopics cleanup stored procedure for retention
  • Extract the Delete() deconstruction into a shared helper, and raise TopicDeleted before it runs
  • Add an in-place child re-ordering capability to SqlDataReaderExtensions
  • Classify each topic from the first result set in AddTopic() into the structural out collections
  • Wire Refresh() to remove deleted subtrees (via the shared helper), re-order affected parents, and raise the structural events
  • Update the Refresh() remarks and document the events it now raises
  • Add Refresh()-path tests for delete, move, reorder, new, and rename, plus a value-only change that raises no structural events

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: RepositoriesRelates to the `ITopicRepository` interface or one of its implementations.Priority: 2Severity 2: MajorStatus 2: ScheduledPlanned for an upcoming release.Type: ImprovementImproves the functionality or interface of an existing feature.

    Type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions