Add interface for changes on cached repository

[Ryan J Ollos]

Plugins such as ​TracTicketChangelogPlugin implement an IAdminCommandProvider command to perform an initial synchronization.

$ trac-admin $env ticketlog sync

It would be nice if we could hook into the repository sync and repository resync commands rather than having a separate command for the plugin. I considered adding a changeset_synchronized method to IRepositoryChangeListener, but the pattern didn't seem quite right.

All of the other I*ChangeListener interfaces send notifications when the an instance of the model has changed. IRepositoryChangeListener sends a notification when the repository that is external to Trac has changed, and Trac uses that notification to synchronize the cache with the repository on disk.

That led to my thinking that a better solution would be an interface that sends notifications on changes to CachedRepository.

class ICachedRepositoryChangeListener(Interface):

    def cache_deleted(repos):
        """Called when the cache is deleted."""

    def changeset_added(repos, changeset):
        """Called when a changeset is added."""

    def changeset_modified(repos, changeset, old_changeset):
        """Called when a changeset is modified."""

[Ryan J Ollos]

Proof of concept in [529a64897/rjollos.git] and implemented for ​TracTicketChangelogPlugin in t12898_tracticketchangelogplugin.diff​.

This would also be useful for:

• ​CodeReviewerPlugin.
• ​TracBackLinkPlugin
• #12485: a ticket-revision table will likely be needed

[Peter Suter]

Sounds reasonable.

Was cached_cached_listeners supposed to be cached_change_listeners?

Is it intentional that the method names changeset_added and changeset_modified of the existing IRepositoryChangeListener and the proposed ICachedRepositoryChangeListener are identical? That seems a bit confusing and would make it impossible to implement both interfaces separately on the same component. Should the new interface use cached_changeset_added / cached_changeset_modified instead? What do you think about also renaming cache_deleted to cached_changesets_deleted?

I would love too see a page similar to TracDev/PluginDevelopment/ExtensionPoints/trac.versioncontrol.api.IRepositoryChangeListener with some more background information, motivating examples etc. For example: What exactly is the relationship to IRepositoryChangeListener? Are ICachedRepositoryChangeListener methods always (but not only) called before IRepositoryChangeListener methods? (For trac-admin repository / changeset commands.) When else? When sync_per_request is used? How should a plugin author decide if a new plugin should implement one or the other interface? Do you ever want both? Or does this more or less replace any need for the old one? Maybe we could create one under TracDev/Proposals already now?

A second proof-of-concept patch for one of the other plugins could maybe help ensure the right interface design.

[Jun Omae]

I don't consider we should add new ICachedRepositoryChangeListener. Even though cache option is enabled/disabled, it should have seamless access to the repository.

The new listener would not be useful for ​th:TracBackLinkPlugin because the plugin reads revisions from both non-cached repository and cached repository. doesn't use cache records for repository. The plugin needs a listener for repository to invoke repository add|remove|sync|resync.

At least, ICachedRepositoryChangeListener shouldn't have changeset_added and changeset_modified, which it should implement via IRepositoryChangeListener.

Also, I want changeset_deleted of IRepositoryChangeListener. Revisions in Git repository can be removed.

[Ryan J Ollos]

Milestone renamed

[Ryan J Ollos]

Replying to Jun Omae:

It looks like you are proposing that the new Interface is called on sync and resync for both cached and non-cached repositories. That's effectively what I was aiming for, but my design was wrong. I didn't consider that it should also be called for non-cached repositories.

The plugin needs a listener for repository to invoke repository add|remove|sync|resync.

I don't understand how repository listener for add|remove could work. It would only work for repositories defined in DbRepositoryProvider​? What is the use case?

Also, I want changeset_deleted of IRepositoryChangeListener. Revisions in Git repository can be removed.

Okay that seems to be a separate issue. The new Interface would presumably be independent of IRepositoryChangeListener.

[Jun Omae]

Replying to Ryan J Ollos:

Replying to Jun Omae:

The plugin needs a listener for repository to invoke repository add|remove|sync|resync.

I don't understand how repository listener for add|remove could work. It would only work for repositories defined in DbRepositoryProvider​? What is the use case?

Also, I want changeset_deleted of IRepositoryChangeListener. Revisions in Git repository can be removed.

Okay that seems to be a separate issue. The new Interface would presumably be independent of IRepositoryChangeListener.

I've considered…, for ​​th:TracBackLinkPlugin, if listener of repository is provided, it could automatically scan back links in commit logs of the repository when a repository is added. Currently, trac-admin backlink sync changeset reponame must be manually invoked after the repository is added.

class IRepositoryChangeListener(Interface):

    def repository_added(repos):
        """Called after repository add"""
        # ex) If the repository is non-cached, scan back links in the commit logs

    def repository_removed(repos):
        """Called after repository remove"""
        # ex) Delete backlink records for the repository

    def repository_synchronized(repos):
        """Called after sync() is finished."""
        # ex) If the repository is cached, scan back links in message of revision records
        #     for the repository

    def changeset_added(repos, changeset):
        """Called when a changeset is added."""
        # ex) scan back links in changeset.message

    def changeset_modified(repos, changeset, old_changeset):
        """Called when a changeset is modified."""
        # ex) re-scan back links in changeset.message

[Ryan J Ollos]

I will add code for TracBackLink and TracTicketChangelog in my fork and we can iterate on proposed designs for the new interface that will meet the needs of the two plugins.

[Ryan J Ollos]

Milestone renamed