EnabledSourceHandler caches query results in non-volatile storage. But those results never expire. Ever. You have to run a Drush command to clear it out; a regular cache rebuild won't do it, since they're not stored in the regular cache.
This is a problem because it means you can be stuck with outdated results indefinitely, with no way to resolve problem unless you have command-line access.
Cache query results with a normal, volatile cache backend, and continue storing actual project information in a permanent key-value collection. Query results should get a cache tag applied so that cached queries can be invalidated in one fell swoop without affecting project data.
We could quick-fix this in EnabledSourceHandler, but the real crux of the problem here (see #6) is architectural -- EnabledSourceHandler has too many responsibilities. Split it into four classes:
getCurrentSources() to ProjectBrowserSourceManager::getAllEnabledSources().onSave to a dedicated event subscriber that knows when to clear storage and invalidate cache tags.getStoredProject and clearStorage into a dedicated ProjectRepository service.EnabledSourceHandler to QueryManager, since it will only be containing querying-related methods (and caching for those queries).
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
phenaproximaComment #4
phenaproximaComment #5
phenaproximaComment #6
phenaproximaBeen thinking on this a bit more, and the problem comes down to the fact that EnabledSourceHandler stands in deep violation of the single-responsibility principle. It does three things:
Fully cleaning this up is not in scope here. But I think we can create a cleaner separation between points #2 and #3.
Query results are just that: "when you give me these criteria, here are the projects I found". These should use a normal, volatile cache backend.
Stored projects are relied upon by the activators, so they cannot be subject to volatile caching. Storing projects (and clearing that storage) needs to be -- and is -- a separate operation from querying.
So I think my proposed resolution is correct -- use the cache system for query results, and always update stored projects with whatever the source plugins return. Project object storage, though, should stay as it is, in non-expiring key-value stores.
Comment #7
phenaproximaComment #8
phenaproximaAssigning to @tim.plunkett specifically, since this kind of stuff is very much in his wheelhouse.
Comment #9
phenaproximaThis should be backported.
Comment #10
phenaproximaDecided to expand the scope here to encompass the full clean-up of EnabledSourceHandler into better-scoped, single-responsibility classes. Need to update the issue summary to reflect that.
Comment #11
phenaproximaComment #12
phenaproximaComment #13
tim.plunkettWalked through this with @phenaproxima and asked for one additional code-comment, but this is great work!
Comment #17
chrisfromredfin!828 merged to 2.1.x
Comment #19
chrisfromredfinand !828 now in 2.0.x