Page MenuHomePhabricator

Don’t reference WikibaseRepo EntityContent class in WikibaseLib’s EntityChange
Closed, ResolvedPublic

Description

The EntityChange class in Lib references the EntityContent class from Repo – it casts Content it receives as an argument to that class. We want to avoid direct Lib→Repo references like that.

Event Timeline

In T255110#6224767, I outlined three ways to achieve this; I’ll summarize them here:

  1. Introduce an EntityIdHolder interface (somewhere outside Repo), make EntityContent implement it, and make EntityChange refer only to that interface rather than EntityContent.
  2. Split EntityChange into two parts: the ones that are expected to be called from Repo and populate the change, and the ones that are expected to be called from Client and consume it.
  3. Add an EntityId parameter to EntityChange::setRevisionInfo() and make the callers get the entity ID from the content (the callers are in Repo).

I still like the concept behind #2, but #3 is so easy to implement that I couldn’t resist it. Patch incoming.

Change 605837 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Move EntityContent access out of EntityChange

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

Just an interesting note here that might impact what is being done.
Content objects, and thus EntityContent objects, are important for ENtity retrieval, and thus should perhaps be alongside whatever code is needed for data retrieval withing Wikibase (Of course this depends on how the client repo dependency continues to evolve.
To get content of an entity you start with a revision, which has slots, which have content.

It appears that our current legacy code goes around this, but any sort of new entity lookup based on the new services in core would likely need content to be accessible for clients that do direct DB access.
(prototype example at https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/605333/)

Change 607019 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Don’t access content in EntityChange::setRevisionInfo

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

Change 607019 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Don’t access content in EntityChange::setRevisionInfo

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

Change 605837 abandoned by Lucas Werkmeister (WMDE):
Move EntityContent access out of EntityChange

Reason:
superseded by I08ea10a8a4

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