Change Details

`CheckingResultsBuilder::getResults()` first gets all check results from `DelegatingConstraintChecker`, then filters them for just the statuses that were requested (by default: discarding everything except `violation`, `warning`, `bad-parameters` – see T184937), and //then// collects the results, including the metadata. This is wrong – we must collect the metadata //before// filtering, otherwise we’re discarding possible depended entity IDs. This is the root cause for the [2018-02-26 WBQC incident](https://wikitech.wikimedia.org/wiki/Incident_documentation/20180226-WikibaseQualityConstraints) – for constraint checks with only successful results (where the filtering step resulted in an empty set of depended entity IDs), `CachingResultsBuilder` would ask for `getLatestRevisionIds( [ /* empty array */ ] )`. However, this incorrect request only caused large-scale problems due to a combination of other bugs.

`CheckingResultsBuilder::getResults()` first gets all check results from `DelegatingConstraintChecker`, then filters them for just the statuses that were requested (by default: discarding everything except `violation`, `warning`, `bad-parameters` – see T184937), and //then// collects the results, including the metadata. This is wrong – we must collect the metadata //before// filtering, otherwise we’re discarding possible depended entity IDs. This is the root cause for the [2018-02-26 WBQC incident](https://wikitech.wikimedia.org/wiki/Incident_documentation/20180226-WikibaseQualityConstraints) – for constraint checks with only successful results (where the filtering step resulted in an empty set of depended entity IDs), `CachingResultsBuilder` would ask for `getLatestRevisionIds( [ /* empty array */ ] )`. However, this incorrect request only caused large-scale problems due to a combination of other bugs. Note that this means the `latestRevisionIds` of all cached results are incorrect (even if they’re not empty, they can still be incomplete), so a full fix to this bug must make sure that we’re not using those incorrect results. The easiest solution is probably to tweak the cache key (e. g. change the `v2` component to `v2.1`).

`CheckingResultsBuilder::getResults()` first gets all check results from `DelegatingConstraintChecker`, then filters them for just the statuses that were requested (by default: discarding everything except `violation`, `warning`, `bad-parameters` – see T184937), and //then// collects the results, including the metadata. This is wrong – we must collect the metadata //before// filtering, otherwise we’re discarding possible depended entity IDs. This is the root cause for the [2018-02-26 WBQC incident](https://wikitech.wikimedia.org/wiki/Incident_documentation/20180226-WikibaseQualityConstraints) – for constraint checks with only successful results (where the filtering step resulted in an empty set of depended entity IDs), `CachingResultsBuilder` would ask for `getLatestRevisionIds( [ /* empty array */ ] )`. However, this incorrect request only caused large-scale problems due to a combination of other bugs. Note that this means the `latestRevisionIds` of all cached results are incorrect (even if they’re not empty, they can still be incomplete), so a full fix to this bug must make sure that we’re not using those incorrect results. The easiest solution is probably to tweak the cache key (e. g. change the `v2` component to `v2.1`).