Page MenuHomePhabricator
Create Task
Maniphest T281342

Migrate EntityRevisionLookup to service container
Closed, ResolvedPublic
Actions

Description

The EntityRevisionLookup service in Multi/SingleEntitySourceServices (DataAccessServices) should be migrated to the new service wiring files in repo and client respectively. (See docs on how to migrate services into the new service container)

This will allow us to stop passing a SingleEntitySourceServicesFactory into PrefetchingTermLookupFactory, and will finally allow us to remove the SingleEntitySourceServiceFactory service (T277731).

The SingleEntitySourceServices method getEntityRevisionLookup is bound to one entity source, so we might want to also create a kind of EntityRevisionLookupFactory, which is given an entity source (or its database name?) and returns a EntityRevisionLookup.

Details

SubjectRepoBranchLines +/-
Get EntityRevisionLookup from WikibaseClient servicesmediawiki/extensions/Mathmaster+1 -1
Add EntityRevisionLookup to client service containermediawiki/extensions/Wikibasemaster+49 -25
Customize query in gerrit

Related Objects
Search...

Event Timeline

ItamarWMDE created this task.Apr 28 2021, 9:54 AM
ItamarWMDE mentioned this in T281343: Stop Passing SingleEntitySourceServicesFactory into PrefetchingTermLookup callbacks.Apr 28 2021, 10:01 AM
ItamarWMDE updated the task description. (Show Details)May 3 2021, 3:12 PM
Lucas_Werkmeister_WMDE mentioned this in T277731: WikibaseRepo + WikibaseClient: Remove SingleEntitySourceServicesFactory Service.May 3 2021, 4:12 PM
Addshore added a project: [DEPRECATED] wdwb-tech.Nov 5 2021, 4:44 PM
Lucas_Werkmeister_WMDE subscribed.Jul 8 2022, 10:35 AM

The SingleEntitySourceServices method getEntityRevisionLookup is bound to one entity source, so we might want to also create a kind of EntityRevisionLookupFactory, which is given an entity source (or its database name?) and returns a EntityRevisionLookup.

I’m not sure we actually need this. In Repo, we already have an EntityRevisionLookup service, and it seems to work fine for all entity types as far as I can tell, including on Commons for MediaInfo entities (local) as well as Items (other entity source). In Client, the service doesn’t directly exist yet, but you can get it from the Store service, and the lookup from there likewise seems to support all entity types.

I think we can just add the store lookup directly to the client service container, use that in the .entitytypes.php files, and thus remove the need for the SingleEntitySourceServicesFactory. Ideally, test this on mwdebug before it rolls out with the train (perhaps even before merging) – next week (no regular train) might be a good opportunity for this.

Lucas_Werkmeister_WMDE added a comment.Jul 12 2022, 10:58 AM

I just tested the following diffs on mwdebug1002:

Wikibase.git
diff --git a/client/WikibaseClient.entitytypes.php b/client/WikibaseClient.entitytypes.php
index d3f5ddc8d5..f4705c2287 100644
--- a/client/WikibaseClient.entitytypes.php
+++ b/client/WikibaseClient.entitytypes.php
@@ -40,8 +40,6 @@
 	'property' => [
 		Def::PREFETCHING_TERM_LOOKUP_CALLBACK => function ( DatabaseEntitySource $entitySource ) {
 			$mwServices = MediaWikiServices::getInstance();
-			$entitySourceServices = WikibaseClient::getSingleEntitySourceServicesFactory( $mwServices )
-				->getServicesForSource( $entitySource );
 
 			$cacheSecret = hash( 'sha256', $mwServices->getMainConfig()->get( 'SecretKey' ) );
 			$bagOStuff = $mwServices->getLocalServerObjectCache();
@@ -69,7 +67,7 @@
 				]
 			);
 			$redirectResolvingRevisionLookup = new RedirectResolvingLatestRevisionLookup(
-				$entitySourceServices->getEntityRevisionLookup()
+				WikibaseClient::getStore( $mwServices )->getEntityRevisionLookup()
 			);
 
 			return new CachingPrefetchingTermLookup(
diff --git a/repo/WikibaseRepo.entitytypes.php b/repo/WikibaseRepo.entitytypes.php
index c8c2d11bf2..6480726d4c 100644
--- a/repo/WikibaseRepo.entitytypes.php
+++ b/repo/WikibaseRepo.entitytypes.php
@@ -464,8 +464,6 @@
 		},
 		Def::PREFETCHING_TERM_LOOKUP_CALLBACK => function ( DatabaseEntitySource $entitySource ) {
 			$mwServices = MediaWikiServices::getInstance();
-			$entitySourceServices = WikibaseRepo::getSingleEntitySourceServicesFactory( $mwServices )
-				->getServicesForSource( $entitySource );
 
 			$cacheSecret = hash( 'sha256', $mwServices->getMainConfig()->get( 'SecretKey' ) );
 			$bagOStuff = $mwServices->getLocalServerObjectCache();
@@ -493,7 +491,7 @@
 				]
 			);
 			$redirectResolvingRevisionLookup = new RedirectResolvingLatestRevisionLookup(
-				$entitySourceServices->getEntityRevisionLookup()
+				WikibaseRepo::getEntityRevisionLookup( $mwServices )
 			);
 
 			return new CachingPrefetchingTermLookup(
WikibaseMediaInfo.git
diff --git a/WikibaseMediaInfo.entitytypes.php b/WikibaseMediaInfo.entitytypes.php
index 4b8e98e96f..779a90468b 100644
--- a/WikibaseMediaInfo.entitytypes.php
+++ b/WikibaseMediaInfo.entitytypes.php
@@ -320,10 +320,7 @@
 			);
 		},
 		Def::PREFETCHING_TERM_LOOKUP_CALLBACK => static function ( EntitySource $entitySource ) {
-			$services = WikibaseRepo::getSingleEntitySourceServicesFactory()
-					->getServicesForSource( $entitySource );
-
-			return new MediaInfoPrefetchingTermLookup( $services->getEntityRevisionLookup() );
+			return new MediaInfoPrefetchingTermLookup( WikibaseRepo::getEntityRevisionLookup() );
 		},
 		Def::ENTITY_ID_LOOKUP_CALLBACK => static function () {
 			return MediaInfoServices::getMediaInfoIdLookup();

And as far as I can tell, everything worked fine; there aren’t any unusual messages in the mwdebug logstash either. I think we can really just add the EntityRevisionLookup to the client service container (just like the repo one: get it from the Store service), use that instead of the one from the $entitySourceServices, then see how much of the SingleEntitySourceServicesFactory that lets us remove.

gerritbot added a comment.Jul 15 2022, 11:52 AM

Change 814146 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Add EntityRevisionLookup to client service container

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

gerritbot added a project: Patch-For-Review.Jul 15 2022, 11:52 AM

Change 814148 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Math@master] Get EntityRevisionLookup from WikibaseClient services

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

Lucas_Werkmeister_WMDE claimed this task.Jul 15 2022, 11:53 AM
gerritbot added a comment.Jul 20 2022, 3:17 PM

Change 814146 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Add EntityRevisionLookup to client service container

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.22; 2022-07-25).Jul 20 2022, 4:00 PM
gerritbot added a comment.Jul 21 2022, 1:25 PM

Change 814148 abandoned by Physikerwelt:

[mediawiki/extensions/Math@master] Get EntityRevisionLookup from WikibaseClient services

Reason:

Replaced by I38d6425eb5e2c52ae4362c4b8656223a8d9d90a5

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

Maintenance_bot removed a project: Patch-For-Review.Jul 21 2022, 1:30 PM
Lucas_Werkmeister_WMDE closed this task as Resolved.Jul 21 2022, 1:54 PM
Lucas_Werkmeister_WMDE moved this task from Incoming to Done on the Wikibase Service Migration board.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL