jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/373056 )
Change subject: Do not pass the WikibaseClient instance to
PerRepositoryServiceContainer
......................................................................
Do not pass the WikibaseClient instance to PerRepositoryServiceContainer
PerRepositoryServiceContainer gets the array of callbacks instantiating
entity deserializers in the constructor, instead of WikibaseClient
being passed.
PerRepositoryServiceContainerFactory gets the instance of
EntityTypeDefinitions that is then used the get the array
of deserializer "factory" callbacks.
Note this does not yet entirely remove the dependency of data access
component on the Client component.
Change-Id: I6338881ae4140894b2cb869f0216431dec32c87b
---
M client/includes/WikibaseClient.php
M data-access/src/PerRepositoryServiceContainer.php
M data-access/src/PerRepositoryServiceContainerFactory.php
M data-access/tests/phpunit/MultiRepositoryServiceWiringTest.php
M data-access/tests/phpunit/NoReverseDependencyTest.php
M data-access/tests/phpunit/PerRepositoryServiceContainerFactoryTest.php
M data-access/tests/phpunit/PerRepositoryServiceContainerTest.php
M data-access/tests/phpunit/PerRepositoryServiceWiringTest.php
8 files changed, 22 insertions(+), 31 deletions(-)
Approvals:
WMDE-leszek: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/WikibaseClient.php
b/client/includes/WikibaseClient.php
index cc3e24d..e237046 100644
--- a/client/includes/WikibaseClient.php
+++ b/client/includes/WikibaseClient.php
@@ -424,7 +424,7 @@
$clientSettings->getSetting(
'perRepositoryServiceWiringFiles' ),
$genericServices,
$dataAccessSettings,
- $this
+ $this->entityTypeDefinitions
);
}
diff --git a/data-access/src/PerRepositoryServiceContainer.php
b/data-access/src/PerRepositoryServiceContainer.php
index 4127387..8b1d2a2 100644
--- a/data-access/src/PerRepositoryServiceContainer.php
+++ b/data-access/src/PerRepositoryServiceContainer.php
@@ -6,7 +6,6 @@
use Deserializers\Deserializer;
use Deserializers\DispatchingDeserializer;
use MediaWiki\Services\ServiceContainer;
-use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\DeserializerFactory;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
@@ -62,7 +61,7 @@
* @param DataValueDeserializer $dataValueDeserializer
* @param GenericServices $genericServices
* @param DataAccessSettings $settings
- * @param WikibaseClient $client Top-level factory passed to service
instantiators // TODO: fix this!
+ * @param callable[] $deserializerFactoryCallbacks
*/
public function __construct(
$databaseName,
@@ -72,7 +71,7 @@
DataValueDeserializer $dataValueDeserializer,
GenericServices $genericServices,
DataAccessSettings $settings,
- WikibaseClient $client
+ array $deserializerFactoryCallbacks
) {
parent::__construct( [ $genericServices, $settings ] );
@@ -81,8 +80,7 @@
$this->entityIdParser = $entityIdParser;
$this->entityIdComposer = $entityIdComposer;
$this->dataValueDeserializer = $dataValueDeserializer;
- // TODO: pass EntityTypeDefinitions to get those callbacks? or
two arrays at least?
- $this->deserializerFactoryCallbacks =
$client->getEntityDeserializerFactoryCallbacks();
+ $this->deserializerFactoryCallbacks =
$deserializerFactoryCallbacks;
}
/**
diff --git a/data-access/src/PerRepositoryServiceContainerFactory.php
b/data-access/src/PerRepositoryServiceContainerFactory.php
index 21daf5b..f82195e 100644
--- a/data-access/src/PerRepositoryServiceContainerFactory.php
+++ b/data-access/src/PerRepositoryServiceContainerFactory.php
@@ -2,10 +2,10 @@
namespace Wikibase\DataAccess;
-use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Services\EntityId\PrefixMappingEntityIdParserFactory;
use Wikibase\DataModel\Services\Lookup\UnknownForeignRepositoryException;
use Wikibase\Lib\EntityIdComposer;
+use Wikibase\Lib\EntityTypeDefinitions;
use Wikibase\Lib\Serialization\RepositorySpecificDataValueDeserializerFactory;
/**
@@ -49,23 +49,16 @@
private $genericServices;
/**
- * @var WikibaseClient
- */
- private $client;
- /**
* @var DataAccessSettings
*/
private $settings;
/**
- * FIXME: injecting of the top-level factory (WikibaseClient) here is
only a temporary solution.
- * The instance of the top-level factory is being passed to
instantiators of services
- * stored in the PerRepositoryServiceContainer so they can get the
service they depend on.
- *
- * This approach is not clean, the class should not depend on the
top-level factory.
- * This should be changed after some refactoring: Instantiators in
PerRepositoryServiceWiring should
- * rather be getting some other service container, not the whole
top-level factory.
- *
+ * @var EntityTypeDefinitions
+ */
+ private $entityTypeDefinitions;
+
+ /**
* @param PrefixMappingEntityIdParserFactory $idParserFactory
* @param EntityIdComposer $idComposer
* @param RepositorySpecificDataValueDeserializerFactory
$dataValueDeserializerFactory
@@ -73,7 +66,7 @@
* @param string[] $wiringFiles
* @param GenericServices $genericServices
* @param DataAccessSettings $settings
- * @param WikibaseClient $client
+ * @param EntityTypeDefinitions $entityTypeDefinitions
*/
public function __construct(
PrefixMappingEntityIdParserFactory $idParserFactory,
@@ -83,7 +76,7 @@
array $wiringFiles,
GenericServices $genericServices,
DataAccessSettings $settings,
- WikibaseClient $client
+ EntityTypeDefinitions $entityTypeDefinitions
) {
$this->idParserFactory = $idParserFactory;
$this->idComposer = $idComposer;
@@ -92,7 +85,7 @@
$this->wiringFiles = $wiringFiles;
$this->genericServices = $genericServices;
$this->settings = $settings;
- $this->client = $client;
+ $this->entityTypeDefinitions = $entityTypeDefinitions;
}
/**
@@ -115,7 +108,7 @@
$this->dataValueDeserializerFactory->getDeserializer(
$repositoryName ),
$this->genericServices,
$this->settings,
- $this->client
+
$this->entityTypeDefinitions->getDeserializerFactoryCallbacks()
);
$container->loadWiringFiles( $this->wiringFiles );
diff --git a/data-access/tests/phpunit/MultiRepositoryServiceWiringTest.php
b/data-access/tests/phpunit/MultiRepositoryServiceWiringTest.php
index a6667e0..0cb40a6 100644
--- a/data-access/tests/phpunit/MultiRepositoryServiceWiringTest.php
+++ b/data-access/tests/phpunit/MultiRepositoryServiceWiringTest.php
@@ -36,6 +36,7 @@
);
$client = WikibaseClient::getDefaultInstance();
+ $entityTypeDefinitions = new EntityTypeDefinitions( [] );
return new PerRepositoryServiceContainerFactory(
$idParser,
@@ -43,9 +44,9 @@
new RepositorySpecificDataValueDeserializerFactory(
$idParser ),
[ '' => false ],
[ __DIR__ . '/../../src/PerRepositoryServiceWiring.php'
],
- new GenericServices(
$client->getEntityNamespaceLookup(), new EntityTypeDefinitions( [] ) ),
+ new GenericServices(
$client->getEntityNamespaceLookup(), $entityTypeDefinitions ),
new DataAccessSettings( 0, false ),
- $client
+ $entityTypeDefinitions
);
}
diff --git a/data-access/tests/phpunit/NoReverseDependencyTest.php
b/data-access/tests/phpunit/NoReverseDependencyTest.php
index 7169326..239c921 100644
--- a/data-access/tests/phpunit/NoReverseDependencyTest.php
+++ b/data-access/tests/phpunit/NoReverseDependencyTest.php
@@ -19,8 +19,6 @@
public function testNoClientDependency() {
$this->assertEquals(
[
- 'PerRepositoryServiceContainer.php',
- 'PerRepositoryServiceContainerFactory.php',
'PerRepositoryServiceWiring.php',
],
$this->getFilesContainingString( 'Wikibase\\Client\\',
__DIR__ . '/../../src/' )
diff --git
a/data-access/tests/phpunit/PerRepositoryServiceContainerFactoryTest.php
b/data-access/tests/phpunit/PerRepositoryServiceContainerFactoryTest.php
index 3b1b5ab..e8d85d9 100644
--- a/data-access/tests/phpunit/PerRepositoryServiceContainerFactoryTest.php
+++ b/data-access/tests/phpunit/PerRepositoryServiceContainerFactoryTest.php
@@ -29,6 +29,7 @@
);
$client = WikibaseClient::getDefaultInstance();
+ $entityTypeDefinitions = new EntityTypeDefinitions( [] );
return new PerRepositoryServiceContainerFactory(
$idParserFactory,
@@ -36,9 +37,9 @@
new RepositorySpecificDataValueDeserializerFactory(
$idParserFactory ),
[ '' => false ],
[],
- new GenericServices(
$client->getEntityNamespaceLookup(), new EntityTypeDefinitions( [] ) ),
+ new GenericServices(
$client->getEntityNamespaceLookup(), $entityTypeDefinitions ),
new DataAccessSettings( 0, false ),
- $client
+ $entityTypeDefinitions
);
}
diff --git a/data-access/tests/phpunit/PerRepositoryServiceContainerTest.php
b/data-access/tests/phpunit/PerRepositoryServiceContainerTest.php
index 913858f..b43a883 100644
--- a/data-access/tests/phpunit/PerRepositoryServiceContainerTest.php
+++ b/data-access/tests/phpunit/PerRepositoryServiceContainerTest.php
@@ -66,7 +66,7 @@
new DataValueDeserializer( [] ),
new GenericServices(
$client->getEntityNamespaceLookup(), new EntityTypeDefinitions( [] ) ),
new DataAccessSettings( 0, false ),
- $client
+ []
);
}
diff --git a/data-access/tests/phpunit/PerRepositoryServiceWiringTest.php
b/data-access/tests/phpunit/PerRepositoryServiceWiringTest.php
index f37253a..e8051c7 100644
--- a/data-access/tests/phpunit/PerRepositoryServiceWiringTest.php
+++ b/data-access/tests/phpunit/PerRepositoryServiceWiringTest.php
@@ -42,7 +42,7 @@
new DataValueDeserializer( [] ),
new GenericServices(
$client->getEntityNamespaceLookup(), new EntityTypeDefinitions( [] ) ),
new DataAccessSettings( 0, false ),
- $client
+ []
);
$container->loadWiringFiles( [ __DIR__ .
'/../../src/PerRepositoryServiceWiring.php' ] );
--
To view, visit https://gerrit.wikimedia.org/r/373056
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6338881ae4140894b2cb869f0216431dec32c87b
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: WMDE-leszek <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
