Page MenuHomePhabricator

Add cacheable layer between CheckConstraints and DelegatingConstraintChecker
Closed, ResolvedPublic

Description

We need some element, some layer between the CheckConstraints API class (and also SpecialConstraintReport) and the current DelegatingConstraintChecker, where we can introduce the caching.

Input: A list of entity IDs and statement IDs to check, along with an optional list of constraint IDs to filter for (otherwise: check all constraints). Or, much more elegantly: a ConstraintCheckPlan encapsulating all that, if we pull T180796: Add ConstraintCheckPlan class into this task.

Output: A JSON blob (as PHP array structure, not string) of constraint check results. We could also return a list of CheckResults, but that would mean that in the most common case (API request with cache hit), we would deserialize those CheckResults from the cache only to serialize them into JSON again immediately, which seems rather unnecessary to me. I’d rather return JSON at this level directly, make the API simply pass it through, and update the special page to operate on the JSON instead of a CheckResult[]. (We still have a JSON string → PHP array → JSON string roundtrip, but that can’t be avoided since the API request might also have requested XML results.)

If we do implement T180796: Add ConstraintCheckPlan class as part of this, then I think we could just turn DelegatingConstraintChecker into this layer. Its current interface (checkAgainstConstraintsOnEntityId, -ClaimId) doesn’t make too much sense in the presence of ConstraintCheckPlan anyways.

Patch-For-Review:

Event Timeline

Or, much more elegantly: a ConstraintCheckPlan encapsulating all that

Hm, I didn’t really think that through :) because the current ConstraintCheckPlan is an opaque interface, so how would the caching layer know what to cache? The current ConstraintCheckPlan just has a list of contexts, it doesn’t remember that those contexts comprise a full constraint check on a certain entity.

Change 395007 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] WIP: Add layer where constraint check results can be cached

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

Change 395558 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add tests for CheckingResultsBuilder::checkResultToArray()

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

Change 395559 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add tests for CheckingResultsBuilder::getResults()

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

Change 395007 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add layer where constraint check results can be cached

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

Change 395558 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add tests for CheckingResultsBuilder::checkResultToArray()

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

Jonas updated the task description. (Show Details)Dec 6 2017, 1:16 PM
Jonas moved this task from Review to Done on the Wikidata-Sprint-2017-12-06 board.

Change 395559 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add tests for CheckingResultsBuilder::getResults()

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

Lucas_Werkmeister_WMDE closed this task as Resolved.Dec 6 2017, 1:24 PM