Page MenuHomePhabricator

DispatchingTermSearchInteractor is confusing and should probably go away
Closed, ResolvedPublic

Description

While looking at T246803: Implement new way of abstraction to call remote wbsearchentities API @Jakob_WMDE and I came across some fun mess

At some point the idea was that TermSearchInteractor would be a dispatchable service based on entity type.
In current reality that is not really the case...

MultipleEntitySourceServices::getTermSearchInteractorFactory creates a DispatchingTermSearchInteractorFactory for $factoriesByType
The $factoriesByType come from getEntityTypeToSourceMapping, and singleSourceServices[$source->getSourceName()]->getTermSearchInteractorFactory(); is always called
Always calling getTermSearchInteractorFactory will result in always getting a MatchingTermsSearchInteractorFactory which will always use the term storage for items and properties.

Much further up the chain is the concept of an EntitySearchHelper which actually dispatches to different entity types using a TypeDispatchingEntitySearchHelper.
This is how lexeme etc search works.

  • The EntitySearchHelper should be pushed down to work with all of the type dispatching and entity source dispatching logic that we now use
  • At least DispatchingTermSearchInteractor should be removed, if not, the whole of TermSearchInteractor stuff should be reworked as it is item property specific...

There is more cleanup that probably also needs to happen:

  • Code that calls WikibaseRepo/Client :: newTermSearchInteractor might be doing the wrong things now in places?
  • TBA think harder about more things

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 709388 had a related patch set uploaded (by Tonina Zhelyazkova; author: Tonina Zhelyazkova):

[mediawiki/extensions/Wikibase@master] FP: Introduce getDatabaseSourceForEntityType

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

Change 709389 had a related patch set uploaded (by Tonina Zhelyazkova; author: Tonina Zhelyazkova):

[mediawiki/extensions/Wikibase@master] FP: Stop using DispatchingTermSearchInteractorFactory in TermSearchInteractorFactory

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

Change 709389 abandoned by Tonina Zhelyazkova:

[mediawiki/extensions/Wikibase@master] FP: Stop using DispatchingTermSearchInteractorFactory in TermSearchInteractorFactory

Reason:

We'd rather get rid of the whole TermSearchInterafctorFactory - If11e57d246311fdde1eb4317915a020c4f5fa550

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

Change 709394 had a related patch set uploaded (by Jakob; author: Jakob):

[mediawiki/extensions/Wikibase@master] Remove DispatchingTermSearchInteractor and friends

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

Change 709394 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Remove DispatchingTermSearchInteractor and friends

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

Change 710536 had a related patch set uploaded (by Jakob; author: Jakob):

[mediawiki/extensions/Wikibase@master] FP: Clarify EntitySourceDefinitions::getEntityTypeToSourceMapping

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

Change 709388 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] FP: Introduce getDatabaseSourceForEntityType

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

Change 710536 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] FP: Clarify EntitySourceDefinitions::getEntityTypeToSourceMapping

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