Page MenuHomePhabricator

Replace CachingResultsBuilder with CachingResultsSource, (de)serializing CheckResults
Closed, ResolvedPublic

Description

We don’t need any hybrid phase for T185709: Cache CheckResult serializations per-entity in ObjectCache, so we can probably just exchange the CachingResultsBuilder implementation for one that reads and writes CheckResult serializations instead (with a separate cache key format… v2.2 I guess).

The transition is complicated enough that it’s split up across several patches. Essentially, were replacing ResultsBuilder with ResultsSource plus CheckResultsRenderer.

Patch-For-Review:

Event Timeline

Lucas_Werkmeister_WMDE triaged this task as Normal priority.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 13 2018, 3:13 PM

Hm, we don’t need a hybrid phase, but this is still a big change – and it pretty much invalidates the current interface of CachedCheckConstraintsResponse and ResultsBuilder, so perhaps it would be better to build up a new interface with new implementations in several commits, and then remove the old implementations afterwards, so it’s not in one huge, monolithic commit.

So what do we have instead?

  • CachedCheckResults: wrapper around CheckResult[] + Metadata. (Note: since the results may be filtered, the overall metadata may be more than the merge of the individual CheckResult metadatas!)
  • something which takes the same parameters as ResultsBuilder::getResults (entity IDs, claim IDs, constraint IDs, statuses), but returns CachedCheckResults instead of a CachedCheckConstraintsResponse. I don’t have a good name for this yet… it does even less than ResultsBuilder did (at least the implementation that doesn’t do caching). A ResultsGatherer, perhaps? But CachingResultsGatherer doesn’t really make too much sense.

Perhaps a CheckResultsSource? But that doesn’t really yield any good names for the implementation (a CheckingCheckResultsSource?).

I suppose it could be just a ResultsSource. Very generic, but matches ResultsBuilder. CheckingResultsSource and CachingResultsSource don’t sound too bad to me.

Oh, and we need a place where we actually do the conversion of CheckResults to arrays and store them all in the giant result structure, since ResultsSource doesn’t do that anymore. (This is also the step where ViolationMessages are rendered.) Is that a CheckResultsRenderer?

Change 420706 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedCheckResults

https://gerrit.wikimedia.org/r/420706

Change 420707 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ResultsSource and CheckingResultsSource

https://gerrit.wikimedia.org/r/420707

Change 420708 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CheckResultsRenderer

https://gerrit.wikimedia.org/r/420708

Lucas_Werkmeister_WMDE renamed this task from (De)serialize CheckResults in CachingResultsBuilder to Replace CachingResultsBuilder with CachingResultsSource, (de)serializing CheckResults.Mar 20 2018, 2:03 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)

Change 420719 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Make ResultsCache format version configurable

https://gerrit.wikimedia.org/r/420719

make the "v2.1" part of the cache key in ResultsCache configurable – we need "v2.1" for CachingResultsBuilder but "v2.2" for CachingResultsSource

Um. Except that the whole point of this entire exercise is to reuse the same cache for all languages, which means a change to the cache key anyways… 🤦

(It still makes sense to bump the key to v2.2, but it’s pretty stupid how long it took me to realize this ^^ )

Change 420834 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingResultsSource

https://gerrit.wikimedia.org/r/420834

Change 420835 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] DNM: Use ResultsSource instead of ResultsBuilder

https://gerrit.wikimedia.org/r/420835

Change 420706 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedCheckResults

https://gerrit.wikimedia.org/r/420706

Change 420707 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ResultsSource and CheckingResultsSource

https://gerrit.wikimedia.org/r/420707

Change 420708 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CheckResultsRenderer

https://gerrit.wikimedia.org/r/420708

Change 420719 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Make ResultsCache format version configurable

https://gerrit.wikimedia.org/r/420719

Is everything here merged?

Change 421873 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Update help message for status parameter

https://gerrit.wikimedia.org/r/421873

Change 421875 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Remove ResultsBuilder and implementations

https://gerrit.wikimedia.org/r/421875

Change 420834 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingResultsSource

https://gerrit.wikimedia.org/r/420834

Change 420835 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Use ResultsSource instead of ResultsBuilder

https://gerrit.wikimedia.org/r/420835

Change 421873 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Update help message for status parameter

https://gerrit.wikimedia.org/r/421873

Change 421875 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Remove ResultsBuilder and implementations

https://gerrit.wikimedia.org/r/421875

Lucas_Werkmeister_WMDE closed this task as Resolved.Mar 26 2018, 12:54 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)