Skip to content

Notify changes from Refresh() via a new TopicUpdated event #151

Description

@JeremyCaney

ITopicRepository.Refresh() merges topics changed since a given time into an in-memory topic graph, but it doesn't raise an event and simply returns a bare Task, so a consumer that polls it cannot determine which topics were changed. This proposes a dedicated TopicUpdated event—with a corresponding OnTopicUpdated() method and a simple TopicUpdateEventArgs—that fires for each topic whose that Refresh() updates with newer data. As an immediate consumer, this will also allow for the per-topic cache eviction called for as part of the Output Caching update (#150).

Background

Every change a repository observes falls into one of three independent categories, with this event filling the one gap in that model:

CategoryTriggerEvent(s)
Local writeSave(), Move(), Delete(), Rollback()TopicSaved, TopicMoved, TopicDeleted
Remote updateRefresh()TopicUpdated (proposed)
MaterializationLoad, EnsureLoadedTopicLoaded

TopicLoaded cannot serve this role as it conflates cold materialization and lazy loading (as EnsureLoaded() raises it per child), neither of which is a change in data. The new event should therefore fire only if Refresh() merged newer data into a topic that was already in memory, not from any Load() overload, not from Rollback() (which persists via Save(), and is thus already covered by TopicSaved), and not from EnsureLoaded() (a fill based on LoadState, whereas Refresh() is a replacement of already loaded data).

Implementation Notes

Interface

  • New TopicUpdateEventArgs : TopicEventArgs, mirroring TopicLoadEventArgs, only including the base Topic and passing isRecursive: false (Since and the data modified will be deferred until they're needed)
  • New TopicUpdated event, backing field, and protected OnTopicUpdated() on ObservableTopicRepository, declared on ITopicRepository
  • TopicRepositoryDecorator should re-subscribe and re-raise so a subscriber on CachedTopicRepository receives events raised by the inner SqlTopicRepository; CachedTopicRepository needs no override because Refresh() mutates its cached instances by reference

The event should fire once per topic (matching TopicSaved and TopicLoaded), not as a single batch: The same evict($"topic:{args.Topic.Id}") handler from #150 will then serve both local and poll-detected edits. Only genuinely updated (pre-existing) topics are reported; new topics pulled by the same batch evict nothing and handled by the coarse cache invalidation.

Collecting the change set

The change set must be the union of the first five result sets of GetTopicUpdates—core Topics (keyed TopicID), indexed Attributes, ExtendedAttributes, Relationships (keyed Source_TopicID), and TopicReferences (keyed Source_TopicID)—intersected with the topics in-memory prior to the merge. Result set 1 covers only core-field changes, so collecting from it alone would miss every attribute-only and association-only update, leaving those topics stale.

Collection should live in LoadTopicGraph(), via an optional output collection that Refresh() supplies:

internalstaticasyncTask<Topic?>LoadTopicGraph(thisDbDataReaderreader,intseedTopicId=-1,Topic?referenceTopic=null,bool?markDirty=null,ICollection<Topic>?updatedTopics=null,CancellationTokencancellationToken=default){

IDs should accumulate into a HashSet<int> (as a topic can appear in multiple result sets), allocated only when updatedTopics is supplied, so every non-Refresh() callers can just use null-conditional guards. After all result sets are read, the union is filtered by preExistingIds (the updated-only filter) and materialized via the live index. SqlTopicRepository.Refresh() should pass a fresh List<Topic> and raise OnTopicUpdated() for each entry after the existing try/catch, so a SqlException fires nothing.

Structural limitation

Refresh() cannot (currently) notify about remote deletes or reorders: A deleted topic is simply absent from the batch, and HasChildren is NULL for every row. This event therefore covers value-updates to in-memory topics only. Downstream cross-server invalidation (the output-caching stage in #150) must therefore retain a periodic coarse sweep for structural changes.

Tasks

  • Add a simple TopicUpdateEventArgs : TopicEventArgs with a TopicUpdateEventArgs(Topic topic) constructor
  • Add the TopicUpdated event, backing field, and OnTopicUpdated() to ObservableTopicRepository, and declare TopicUpdated on ITopicRepository
  • Add the corresponding line for relaying the TopicUpdated event to TopicRepositoryDecorator
  • Add the updatedTopics output collection to LoadTopicGraph(), populated from the union of content result sets, filtered by preExistingIds
  • Raise OnTopicUpdated() in SqlTopicRepository.Refresh() after a successful merge
  • Add tests against the LoadTopicGraph() merge:
    • Fires for updated in-memory topics, not for untouched topics
    • Fires for an attribute-only or association-only change absent from result set 1
    • Does not fire for a new topic pulled by the same batch
    • Does not fire on EnsureLoaded() or Rollback()
    • Bubbles through TopicRepositoryDecorator

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: RepositoriesRelates to the `ITopicRepository` interface or one of its implementations.Priority: 2Severity 1: MinorStatus 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