Page MenuHomePhabricator

Move WikibaseRepo EntityChangeOpProvider to service container
Closed, ResolvedPublic

Event Timeline

Change 667900 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] repo: move EntityChangeOpProvider to service container

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

In the above change, EditEntityFingerprintUniquenessIntegrationTest fails, and the error seems to be that in DatabaseTermsCollisionDetector::detectLabelCollision(), during the second API call (the one that’s supposed to fail because the property fingerprint is not unique), the $labelTypeId (the wby_id of the “label” term type) is null, because the NameTableStore inside the DatabaseTypeIdsStore has a cached empty version of the wbt_type table; this means that the conflict can’t be detected, because the “label” type ID is unknown. I assume this was previously not a problem because the two API calls would get different EntityChangeOpProviders, which I suppose contained separate DatabaseTypeIdsStore and NameTableStore instances, and so the wbt_type table wasn’t cached as much, but now the service is cached inside the service container. Let’s see if T276312: Move WikibaseRepo TermsCollisionDetectorFactory to service container fixes the problem.

Nope, T276312 doesn’t fix the problem. I suspect we need to refactor TermStoreWriterFactory so it doesn’t create a new DatabaseTypeIdsStore every time.

Nope, T276312 doesn’t fix the problem. I suspect we need to refactor TermStoreWriterFactory so it doesn’t create a new DatabaseTypeIdsStore every time.

Doing that in T276358: Move WikibaseRepo TypeIdsStore to service container.

Nope, T276312 doesn’t fix the problem. I suspect we need to refactor TermStoreWriterFactory so it doesn’t create a new DatabaseTypeIdsStore every time.

Doing that in T276358: Move WikibaseRepo TypeIdsStore to service container.

Hm, it seems like an alternative solution would be to use MediaWiki core’s NameTableStoreFactory, which takes care of caching the stores. In fact, I think we have to do that anyways, since NameTableStore isn’t marked as @newable (i.e. by currently constructing that class directly, our code is not covered by MediaWiki’s stable interface policy).

In fact, I think we have to do that anyways, since NameTableStore isn’t marked as @newable (i.e. by currently constructing that class directly, our code is not covered by MediaWiki’s stable interface policy).

Uh, nevermind. NameTableStoreFactory only supports a specific set of tables (those covered by its getTableInfo()), and that set isn’t (yet?) configurable.

See T276551: Allow extensions to use NameTableStore for their own tables for potential NameTableStore improvements. For the time being, I guess the current code is acceptable.

Change 667900 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] repo: move EntityChangeOpProvider to service container

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