Mailing List Archive

Lucas Werkmeister (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/406050 )

Change subject: Split up and test CheckConstraints::newFromGlobalState
......................................................................

Split up and test CheckConstraints::newFromGlobalState

CheckConstraints::newFromGlobalState was one giant method that
initialized the dependency injection for everything else. This commit
splits it up into several methods and adds tests for all of them. I’ll
be the first to admit that this still isn’t great, and that the tests
don’t do very much, but at least we’ll now get test failures if one of
the classes instantiated here gains a parameter, which previously would
only be detected by manual tests.

Bug: T183373
Change-Id: Ie075eade0f9155128898564874bea9f38a86f443
---
M src/Api/CheckConstraints.php
M tests/phpunit/Api/CheckConstraintsTest.php
2 files changed, 221 insertions(+), 72 deletions(-)


git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints refs/changes/50/406050/1

diff --git a/src/Api/CheckConstraints.php b/src/Api/CheckConstraints.php
index 46b5303..6d092ba 100644
--- a/src/Api/CheckConstraints.php
+++ b/src/Api/CheckConstraints.php
@@ -4,9 +4,11 @@

use ApiBase;
use ApiMain;
+use Config;
use IBufferingStatsdDataFactory;
use MediaWiki\MediaWikiServices;
use RequestContext;
+use TitleParser;
use ValueFormatters\FormatterOptions;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
@@ -19,6 +21,7 @@
use Wikibase\Repo\Api\ResultBuilder;
use Wikibase\Repo\EntityIdLabelFormatterFactory;
use Wikibase\Repo\WikibaseRepo;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\DelegatingConstraintChecker;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
@@ -75,32 +78,91 @@
* @param string $prefix
*
* @return self
+ *
+ * @codeCoverageIgnore hard to test and only delegates to other methods
*/
public static function newFromGlobalState( ApiMain $main, $name, $prefix = '' ) {
- $repo = WikibaseRepo::getDefaultInstance();
+ return self::getCheckConstraints(
+ MediaWikiServices::getInstance(),
+ WikibaseRepo::getDefaultInstance(),
+ $main,
+ $name,
+ $prefix
+ );
+ }

+ public static function getCheckConstraints(
+ MediaWikiServices $services,
+ WikibaseRepo $repo,
+ ApiMain $main,
+ $name,
+ $prefix
+ ) {
+ $config = $services->getMainConfig();
+ $dataFactory = $services->getStatsdDataFactory();
+ $titleParser = $services->getTitleParser();
+
+ $constraintParameterRenderer = self::getConstraintParameterRenderer(
+ $config,
+ $repo
+ );
+ $constraintReportFactory = self::getConstraintReportFactory(
+ $config,
+ $dataFactory,
+ $titleParser,
+ $repo,
+ $constraintParameterRenderer
+ );
+ $resultsBuilder = self::getResultsBuilder(
+ $config,
+ $dataFactory,
+ $repo,
+ $constraintParameterRenderer,
+ $constraintReportFactory->getConstraintChecker()
+ );
+
+ return new CheckConstraints(
+ $main,
+ $name,
+ $prefix,
+ $repo->getEntityIdParser(),
+ $repo->getStatementGuidValidator(),
+ $repo->getApiHelperFactory( RequestContext::getMain() ),
+ $resultsBuilder,
+ $dataFactory
+ );
+ }
+
+ public static function getConstraintParameterRenderer(
+ Config $config,
+ WikibaseRepo $repo
+ ) {
$language = $repo->getUserLanguage();
- $formatterOptions = new FormatterOptions();
- $formatterOptions->setOption( SnakFormatter::OPT_LANG, $language->getCode() );
- $valueFormatterFactory = $repo->getValueFormatterFactory();
- $valueFormatter = $valueFormatterFactory->getValueFormatter( SnakFormatter::FORMAT_HTML, $formatterOptions );
-
- $languageFallbackLabelDescriptionLookupFactory = $repo->getLanguageFallbackLabelDescriptionLookupFactory();
- $labelDescriptionLookup = $languageFallbackLabelDescriptionLookupFactory->newLabelDescriptionLookup( $language );
- $entityIdHtmlLinkFormatterFactory = $repo->getEntityIdHtmlLinkFormatterFactory();
- $entityIdHtmlLinkFormatter = $entityIdHtmlLinkFormatterFactory->getEntityIdFormatter( $labelDescriptionLookup );
- $entityIdLabelFormatterFactory = new EntityIdLabelFormatterFactory();
- $entityIdLabelFormatter = $entityIdLabelFormatterFactory->getEntityIdFormatter( $labelDescriptionLookup );
- $config = MediaWikiServices::getInstance()->getMainConfig();
- $titleParser = MediaWikiServices::getInstance()->getTitleParser();
- $unitConverter = $repo->getUnitConverter();
- $dataFactory = MediaWikiServices::getInstance()->getStatsdDataFactory();
- $constraintParameterRenderer = new ConstraintParameterRenderer(
+ $entityIdHtmlLinkFormatter = $repo->getEntityIdHtmlLinkFormatterFactory()
+ ->getEntityIdFormatter(
+ $repo->getLanguageFallbackLabelDescriptionLookupFactory()
+ ->newLabelDescriptionLookup( $language )
+ );
+ $valueFormatter = $repo->getValueFormatterFactory()
+ ->getValueFormatter(
+ SnakFormatter::FORMAT_HTML,
+ new FormatterOptions( [ SnakFormatter::OPT_LANG => $language->getCode() ] )
+ );
+ return new ConstraintParameterRenderer(
$entityIdHtmlLinkFormatter,
$valueFormatter,
$config
);
- $constraintReportFactory = new ConstraintReportFactory(
+ }
+
+ public static function getConstraintReportFactory(
+ Config $config,
+ IBufferingStatsdDataFactory $dataFactory,
+ TitleParser $titleParser,
+ WikibaseRepo $repo,
+ ConstraintParameterRenderer $constraintParameterRenderer
+ ) {
+ return new ConstraintReportFactory(
$repo->getEntityLookup(),
$repo->getPropertyDataTypeLookup(),
$repo->getStatementGuidParser(),
@@ -114,28 +176,40 @@
$repo->getRdfVocabulary(),
$repo->getEntityIdParser(),
$titleParser,
- $unitConverter,
+ $repo->getUnitConverter(),
$dataFactory
);
+ }

+ public static function getResultsBuilder(
+ Config $config,
+ IBufferingStatsdDataFactory $dataFactory,
+ WikibaseRepo $repo,
+ ConstraintParameterRenderer $constraintParameterRenderer,
+ DelegatingConstraintChecker $delegatingConstraintChecker
+ ) {
+ $entityIdLabelFormatterFactory = new EntityIdLabelFormatterFactory();
+ $entityIdLabelFormatter = $entityIdLabelFormatterFactory->getEntityIdFormatter(
+ $repo->getLanguageFallbackLabelDescriptionLookupFactory()
+ ->newLabelDescriptionLookup( $repo->getUserLanguage() )
+ );
$resultsBuilder = new CheckingResultsBuilder(
- $constraintReportFactory->getConstraintChecker(),
+ $delegatingConstraintChecker,
$repo->getEntityTitleLookup(),
$entityIdLabelFormatter,
$constraintParameterRenderer,
$config
);
+
if ( $config->get( 'WBQualityConstraintsCacheCheckConstraintsResults' ) ) {
$wikiPageEntityMetaDataAccessor = new WikiPageEntityMetaDataLookup(
$repo->getEntityNamespaceLookup()
);
- $entityRevisionLookup = $repo->getEntityRevisionLookup();
- $entityIdParser = $repo->getEntityIdParser();
$resultsBuilder = new CachingResultsBuilder(
$resultsBuilder,
ResultsCache::getDefaultInstance(),
$wikiPageEntityMetaDataAccessor,
- $entityIdParser,
+ $repo->getEntityIdParser(),
$config->get( 'WBQualityConstraintsCacheCheckConstraintsTTLSeconds' ),
[.
$config->get( 'WBQualityConstraintsCommonsLinkConstraintId' ),
@@ -147,16 +221,7 @@
);
}

- return new CheckConstraints(
- $main,
- $name,
- $prefix,
- $repo->getEntityIdParser(),
- $repo->getStatementGuidValidator(),
- $repo->getApiHelperFactory( RequestContext::getMain() ),
- $resultsBuilder,
- $dataFactory
- );
+ return $resultsBuilder;
}

/**
diff --git a/tests/phpunit/Api/CheckConstraintsTest.php b/tests/phpunit/Api/CheckConstraintsTest.php
index 4ae94a2..5890a19 100644
--- a/tests/phpunit/Api/CheckConstraintsTest.php
+++ b/tests/phpunit/Api/CheckConstraintsTest.php
@@ -6,6 +6,7 @@
use DataValues\UnknownValue;
use HashConfig;
use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
use NullStatsdDataFactory;
use RequestContext;
use Wikibase\DataModel\Entity\Item;
@@ -21,6 +22,7 @@
use Wikibase\Repo\Tests\NewItem;
use Wikibase\Repo\Tests\NewStatement;
use Wikibase\Repo\WikibaseRepo;
+use WikibaseQuality\ConstraintReport\Api\CachingResultsBuilder;
use WikibaseQuality\ConstraintReport\Api\CheckConstraints;
use WikibaseQuality\ConstraintReport\Constraint;
use WikibaseQuality\ConstraintReport\ConstraintCheck\ConstraintChecker;
@@ -29,6 +31,7 @@
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\LoggingHelper;
use WikibaseQuality\ConstraintReport\Api\CheckingResultsBuilder;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
+use WikibaseQuality\ConstraintReport\ConstraintReportFactory;
use WikibaseQuality\ConstraintReport\Tests\Fake\FakeChecker;
use WikibaseQuality\ConstraintReport\Tests\Fake\InMemoryConstraintLookup;
use Wikibase\LanguageFallbackChainFactory;
@@ -39,6 +42,7 @@
use Wikimedia\Assert\Assert;
use Language;
use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
+use Wikimedia\TestingAccessWrapper;

/**
* @covers WikibaseQuality\ConstraintReport\Api\CheckConstraints
@@ -83,30 +87,6 @@

$wgAPIModules['wbcheckconstraints']['factory'] = function ( $main, $name ) {
$repo = WikibaseRepo::getDefaultInstance();
- $factory = new EntityIdLabelFormatterFactory();
- $termLookup = $repo->getTermLookup();
- $termBuffer = $repo->getTermBuffer();
- $languageFallbackChainFactory = new LanguageFallbackChainFactory();
- $fallbackLabelDescLookupFactory = new LanguageFallbackLabelDescriptionLookupFactory(
- $languageFallbackChainFactory,
- $termLookup,
- $termBuffer
- );
- $language = new Language();
- $labelLookup = $fallbackLabelDescLookupFactory->newLabelDescriptionLookup( $language );
- $entityIdFormatter = $factory->getEntityIdFormatter( $labelLookup );
-
- $formatterOptions = new FormatterOptions();
- $factoryFunctions = [];
- Assert::parameterElementType( 'callable', $factoryFunctions, '$factoryFunctions' );
- $formatterOptions->setOption( SnakFormatter::OPT_LANG, $language->getCode() );
- $valueFormatterFactory = new OutputFormatValueFormatterFactory(
- $factoryFunctions,
- $language,
- $languageFallbackChainFactory
- );
- $valueFormatter = $valueFormatterFactory->getValueFormatter( SnakFormatter::FORMAT_HTML, $formatterOptions );
-
// we can’t use the DefaultConfig trait because we’re in a static method
$config = new HashConfig( [.
'WBQualityConstraintsPropertyConstraintId' => 'P1',
@@ -119,19 +99,19 @@
'WBQualityConstraintsCheckDurationInfoSeconds' => 1.0,
'WBQualityConstraintsCheckDurationWarningSeconds' => 10.0,
'WBQualityConstraintsIncludeDetailInApi' => true,
+ 'WBQualityConstraintsCacheCheckConstraintsResults' => false,
] );
- $entityIdParser = new ItemIdParser();
- $constraintParameterRenderer = new ConstraintParameterRenderer(
- $entityIdFormatter,
- $valueFormatter,
- $config
+ $dataFactory = new NullStatsdDataFactory();
+
+ $constraintParameterRenderer = CheckConstraints::getConstraintParameterRenderer(
+ $config,
+ $repo
);
$constraintParameterParser = new ConstraintParameterParser(
$config,
$repo->getBaseDataModelDeserializerFactory(),
$constraintParameterRenderer
);
- $dataFactory = new NullStatsdDataFactory();
$constraintChecker = new DelegatingConstraintChecker(
self::$entityLookup,
self::$checkerMap,
@@ -147,21 +127,22 @@
false,
[]
);
+ $resultsBuilder = CheckConstraints::getResultsBuilder(
+ $config,
+ $dataFactory,
+ $repo,
+ $constraintParameterRenderer,
+ $constraintChecker
+ );

return new CheckConstraints(
$main,
$name,
'',
- $entityIdParser,
- new StatementGuidValidator( $entityIdParser ),
+ $repo->getEntityIdParser(),
+ $repo->getStatementGuidValidator(),
$repo->getApiHelperFactory( RequestContext::getMain() ),
- new CheckingResultsBuilder(
- $constraintChecker,
- $repo->getEntityTitleLookup(),
- $entityIdFormatter,
- $constraintParameterRenderer,
- $config
- ),
+ $resultsBuilder,
$dataFactory
);
};
@@ -180,6 +161,109 @@
parent::tearDownAfterClass();
}

+ public function testGetResultsBuilder_WithoutCaching() {
+ $constraintParameterRenderer = $this->getMockBuilder( ConstraintParameterRenderer::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $delegatingConstraintChecker = $this->getMockBuilder( DelegatingConstraintChecker::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $resultsBuilder = CheckConstraints::getResultsBuilder(
+ new HashConfig( [
+ 'WBQualityConstraintsCacheCheckConstraintsResults' => false,
+ ] ),
+ new NullStatsdDataFactory(),
+ WikibaseRepo::getDefaultInstance(),
+ $constraintParameterRenderer,
+ $delegatingConstraintChecker
+ );
+
+ $this->assertInstanceOf( CheckingResultsBuilder::class, $resultsBuilder );
+ }
+
+ public function testGetResultsBuilder_WithCaching() {
+ $constraintParameterRenderer = $this->getMockBuilder( ConstraintParameterRenderer::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $delegatingConstraintChecker = $this->getMockBuilder( DelegatingConstraintChecker::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $resultsBuilder = CheckConstraints::getResultsBuilder(
+ new HashConfig( [.
+ 'WBQualityConstraintsCacheCheckConstraintsResults' => true,
+ 'WBQualityConstraintsCacheCheckConstraintsTTLSeconds' => 10,
+ 'WBQualityConstraintsCommonsLinkConstraintId' => 'Q1',
+ 'WBQualityConstraintsTypeConstraintId' => 'Q2',
+ 'WBQualityConstraintsValueTypeConstraintId' => 'Q3',
+ 'WBQualityConstraintsDistinctValuesConstraintId' => 'Q4',
+ ] ),
+ new NullStatsdDataFactory(),
+ WikibaseRepo::getDefaultInstance(),
+ $constraintParameterRenderer,
+ $delegatingConstraintChecker
+ );
+
+ $this->assertInstanceOf( CachingResultsBuilder::class, $resultsBuilder );
+ $resultsBuilder = TestingAccessWrapper::newFromObject( $resultsBuilder );
+ $this->assertSame( 10, $resultsBuilder->ttlInSeconds );
+ $this->assertSame( [ 'Q1', 'Q2', 'Q3', 'Q4' ], $resultsBuilder->possiblyStaleConstraintTypes );
+ $this->assertInstanceOf( CheckingResultsBuilder::class, $resultsBuilder->resultsBuilder );
+ }
+
+ public function testGetConstraintReportFactory() {
+ $constraintParameterRenderer = $this->getMockBuilder( ConstraintParameterRenderer::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $constraintReportFactory = CheckConstraints::getConstraintReportFactory(
+ new HashConfig(),
+ new NullStatsdDataFactory(),
+ $this->getMock( \TitleParser::class ),
+ WikibaseRepo::getDefaultInstance(),
+ $constraintParameterRenderer
+ );
+
+ $this->assertInstanceOf( ConstraintReportFactory::class, $constraintReportFactory );
+ }
+
+ public function testGetConstraintParameterRenderer() {
+ $constraintParameterRenderer = CheckConstraints::getConstraintParameterRenderer(
+ new HashConfig(),
+ WikibaseRepo::getDefaultInstance()
+ );
+
+ $this->assertInstanceOf( ConstraintParameterRenderer::class, $constraintParameterRenderer );
+ }
+
+ public function testGetCheckConstraints() {
+ $config = $this->getMock( \Config::class );
+ $config->method( 'get' )->willReturnCallback( function ( $key ) {
+ switch ( $key ) {
+ case 'WBQualityConstraintsPropertiesWithViolatingQualifiers':
+ return [];
+ default:
+ return null;
+ }
+ } );
+ $dataFactory = new NullStatsdDataFactory();
+ $titleParser = $this->getMock( \TitleParser::class );
+ $services = $this->getMockBuilder( MediaWikiServices::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $services->expects( $this->once() )->method( 'getMainConfig' )->willReturn( $config );
+ $services->expects( $this->once() )->method( 'getStatsdDataFactory' )->willReturn( $dataFactory );
+ $services->expects( $this->once() )->method( 'getTitleParser' )->willReturn( $titleParser );
+
+ $checkConstraints = CheckConstraints::getCheckConstraints(
+ $services,
+ WikibaseRepo::getDefaultInstance(),
+ new \ApiMain(),
+ 'wbcheckconstraints',
+ ''
+ );
+
+ $this->assertInstanceOf( CheckConstraints::class, $checkConstraints );
+ }
+
public function testReportForNonexistentItemIsEmpty() {
$result = $this->doRequest(
[ CheckConstraints::PARAM_ID => self::NONEXISTENT_ITEM ]

--
To view, visit https://gerrit.wikimedia.org/r/406050
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie075eade0f9155128898564874bea9f38a86f443
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeister@wikimedia.de>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits