Page MenuHomePhabricator

Remove ItemAndPropertySource from WikibaseClient
Open, LowPublic


WikibaseClient has three well-known entity sources (available as services in the service container):

  • ItemSource, the source for the item entity type
  • PropertySource, the source for the property entity type
  • ItemAndPropertySource, the source whose name matches the itemAndPropertySourceName setting

This doesn’t really make sense. ItemAndPropertySource presupposes that items and properties always come from the same entity source, which would make the other two sources redundant; but in fact, this is no longer the case, especially with Wikibase - Federated Properties. In reality (if I understand correctly), ItemAndPropertySource is a renamed (T258060) holdover from a time when there was such a thing as the repo to which a client was connected. I think we should remove it, and instead use the right entity source for each entity type everywhere.

Event Timeline

There is also a related TODO in WikibaseRepo::getStore():

// TODO: the idea of local entity source seems not really suitable here. Store should probably
// get source definitions and pass the right source/sources to services it creates accordingly
// (as long as what it creates should not migrate to *SourceServices in the first place)

Observation from the change dispatching hike: the single-repo assumption is still deeply wired into the whole change dispatching system. This will require more than just cosmetic cleanup.

As far as I can tell, this entity source is mainly (only?) used for two broader concepts:

  • the site link lookup
  • change subscription and dispatching

The site link lookup actually feels like it could use the item entity source instead (as long as there’s only a single item entity source…). The change subscription/dispatching is related to T293354: Support dispatching changes for entities in non-main slots, I suppose, because MediaInfo would be the first case in Wikimedia production of a client getting different entities from different sources. (Federated properties don’t support change dispatching at the moment, and that would be a wholly different thing anyways.)