Change Details

@Ladsgroup pointed out that we store localized violation messages in the cache, but use the same cache for requests independent of language, so whichever request to miss the cache writes messages in that request’s language to the cache, and all other requests for the same entity then get cached violation messages in the first request’s language instead of their own. The quick-and-dirty fix is to add the language to the cache key. Purging gets slightly tricky (we need to delete the cache entry for all possible languages), but otherwise this is fairly simple. However, caching results independently per-language both bloats the cache (caching effectively the same results several times) and slows down constraint checks for lesser-used languages (since they’re less likely to get cached results). Alternatively, we could change what we cache: instead of caching essentially the full `wbcheckconstraints` response, cache something closer to a list of `CheckResult` serializations, after changing `CheckResult` to hold a `Message` object instead of a `string`. This also opens up other opportunities: the `wbcheckconstraints` response includes entries even for statements with no violations (cf. T178160), which we can’t remove from the API response, but could remove from the value that we cache. (That is: don’t store `NullResult`s, instead recreate them when reading from cache.)

@Ladsgroup pointed out that we store localized violation messages in the cache, but use the same cache for requests independent of language, so whichever request to miss the cache writes messages in that request’s language to the cache, and all other requests for the same entity then get cached violation messages in the first request’s language instead of their own. The quick-and-dirty fix is to add the language to the cache key. Purging gets slightly tricky (we need to delete the cache entry for all possible languages), but otherwise this is fairly simple. However, caching results independently per-language both bloats the cache (caching effectively the same results several times) and slows down constraint checks for lesser-used languages (since they’re less likely to get cached results). Alternatively, we could change what we cache: instead of caching essentially the full `wbcheckconstraints` response, cache something closer to a list of `CheckResult` serializations, after changing `CheckResult` to hold a `Message` object instead of a `string`. This also opens up other opportunities: the `wbcheckconstraints` response includes entries even for statements with no violations (cf. T178160), which we can’t remove from the API response, but could remove from the value that we cache. (That is: don’t store `NullResult`s, instead recreate them when reading from cache.) And this will also allow us to store results from requests with a wider `status` parameter in the cache, and use the cache for requests with a narrower parameter, instead of only using the cache if the parameter is exactly `violation|warning|bad-parameters`.

@Ladsgroup pointed out that we store localized violation messages in the cache, but use the same cache for requests independent of language, so whichever request to miss the cache writes messages in that request’s language to the cache, and all other requests for the same entity then get cached violation messages in the first request’s language instead of their own. The quick-and-dirty fix is to add the language to the cache key. Purging gets slightly tricky (we need to delete the cache entry for all possible languages), but otherwise this is fairly simple. However, caching results independently per-language both bloats the cache (caching effectively the same results several times) and slows down constraint checks for lesser-used languages (since they’re less likely to get cached results). Alternatively, we could change what we cache: instead of caching essentially the full `wbcheckconstraints` response, cache something closer to a list of `CheckResult` serializations, after changing `CheckResult` to hold a `Message` object instead of a `string`. This also opens up other opportunities: the `wbcheckconstraints` response includes entries even for statements with no violations (cf. T178160), which we can’t remove from the API response, but could remove from the value that we cache. (That is: don’t store `NullResult`s, instead recreate them when reading from cache.) And this will also allow us to store results from requests with a wider `status` parameter in the cache, and use the cache for requests with a narrower parameter, instead of only using the cache if the parameter is exactly `violation|warning|bad-parameters`.