Thiemo Mättig (WMDE) has uploaded a new change for review.

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

Change subject: Rename duplicate/outdated methods and parameters
......................................................................

Rename duplicate/outdated methods and parameters

Focus is on two renames:
1. The duplicate "crossCheckStatement(s|List)" methods in CrossCheckInteractor.
2. Outdated "claimGuids".

Change-Id: I45449638aca5ff41f2dd5d9ea69b5c90af7ef45d
---
M api/RunCrossCheck.php
M includes/CrossCheck/CrossCheckInteractor.php
M includes/DumpMetaInformation/DumpMetaInformation.php
M includes/DumpMetaInformation/SqlDumpMetaInformationRepo.php
M includes/ExternalDataRepo.php
M specials/SpecialCrossCheck.php
M tests/phpunit/Api/RunCrossCheckTest.php
M tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
M tests/phpunit/CrossCheck/Result/CrossCheckResultTest.php
M tests/phpunit/Specials/SpecialCrossCheckTest.php
10 files changed, 58 insertions(+), 56 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityExternalValidation
 refs/changes/25/243125/1

diff --git a/api/RunCrossCheck.php b/api/RunCrossCheck.php
index 06b2651..8bc75c2 100644
--- a/api/RunCrossCheck.php
+++ b/api/RunCrossCheck.php
@@ -122,9 +122,9 @@
                                $resultLists = 
$this->crossCheckInteractor->crossCheckEntitiesByIds( $entityIds );
                        }
                } elseif ( $params['claims'] ) {
-                       $claimGuids = $params['claims'];
-                       $this->assertAreValidClaimGuids( $claimGuids );
-                       $resultLists = 
$this->crossCheckInteractor->crossCheckStatements( $claimGuids );
+                       $guids = $params['claims'];
+                       $this->assertAreValidClaimGuids( $guids );
+                       $resultLists = 
$this->crossCheckInteractor->crossCheckStatementsByGuids( $guids );
                } else {
                        $this->errorReporter->dieError(
                                'Either provide the ids of entities or ids of 
claims, that should be cross-checked.',
@@ -149,11 +149,11 @@
        }
 
        /**
-        * @param array $claimGuids
+        * @param string[] $guids
         */
-       private function assertAreValidClaimGuids( array $claimGuids ) {
-               foreach ( $claimGuids as $claimGuid ) {
-                       if ( $this->statementGuidValidator->validateFormat( 
$claimGuid ) === false ) {
+       private function assertAreValidClaimGuids( array $guids ) {
+               foreach ( $guids as $guid ) {
+                       if ( $this->statementGuidValidator->validateFormat( 
$guid ) === false ) {
                                $this->errorReporter->dieError( 'Invalid claim 
guid.', 'invalid-guid' );
                        }
                }
diff --git a/includes/CrossCheck/CrossCheckInteractor.php 
b/includes/CrossCheck/CrossCheckInteractor.php
index bf77f07..f263237 100644
--- a/includes/CrossCheck/CrossCheckInteractor.php
+++ b/includes/CrossCheck/CrossCheckInteractor.php
@@ -60,7 +60,7 @@
                $entity = $this->entityLookup->getEntity( $entityId );
 
                if ( $entity instanceof StatementListProvider ) {
-                       return $this->crossCheckStatementList( 
$entity->getStatements() );
+                       return $this->crossCheckStatements( 
$entity->getStatements() );
                }
 
                return null;
@@ -91,7 +91,7 @@
         *
         * @return CrossCheckResultList
         */
-       public function crossCheckStatementList( StatementList $statements ) {
+       public function crossCheckStatements( StatementList $statements ) {
                return $this->crossChecker->crossCheckStatements( $statements, 
$statements );
        }
 
@@ -110,7 +110,7 @@
                foreach ( $entities as $entity ) {
                        $entityId = $entity->getId()->getSerialization();
                        if ( $entity instanceof StatementListProvider ) {
-                               $results[$entityId] = 
$this->crossCheckStatementList( $entity->getStatements() );
+                               $results[$entityId] = 
$this->crossCheckStatements( $entity->getStatements() );
                        }
                }
 
@@ -211,16 +211,15 @@
        /**
         * Runs cross-check for a single statement.
         *
-        * @param string $statementGuid
+        * @param string $guid
         *
-        * @param string $claimGuid
         * @return CrossCheckResultList
         * @throws InvalidArgumentException
         */
-       public function crossCheckStatement( $statementGuid ) {
-               $this->assertIsString( $statementGuid, '$claimGuid' );
+       public function crossCheckStatementByGuid( $guid ) {
+               $this->assertIsString( $guid, '$guid' );
 
-               $resultList = $this->crossCheckStatements( array( 
$statementGuid ) );
+               $resultList = $this->crossCheckStatementsByGuids( array( $guid 
) );
 
                return reset( $resultList );
        }
@@ -228,26 +227,26 @@
        /**
         * Runs cross-check for multiple statements.
         *
-        * @param string[] $statementGuids
+        * @param string[] $guids
         *
         * @return CrossCheckResultList[]
         * @throws InvalidArgumentException
         */
-       public function crossCheckStatements( array $statementGuids ) {
-               $this->assertIsArrayOfStrings( $statementGuids, '$claimGuids' );
+       public function crossCheckStatementsByGuids( array $guids ) {
+               $this->assertIsArrayOfStrings( $guids, '$guids' );
 
                $entityIds = array();
                $groupedStatementGuids = array();
-               foreach ( $statementGuids as $statementGuid ) {
-                       $serializedEntityId = 
$this->statementGuidParser->parse( $statementGuid )->getEntityId();
+               foreach ( $guids as $guid ) {
+                       $serializedEntityId = 
$this->statementGuidParser->parse( $guid )->getEntityId();
                        $entityIds[$serializedEntityId->getSerialization()] = 
$serializedEntityId;
-                       
$groupedStatementGuids[$serializedEntityId->getSerialization()][] = 
$statementGuid;
+                       
$groupedStatementGuids[$serializedEntityId->getSerialization()][] = $guid;
                }
 
                $resultLists = array();
-               foreach ( $groupedStatementGuids as $serializedEntityId => 
$claimGuidsOfEntity ) {
+               foreach ( $groupedStatementGuids as $serializedEntityId => 
$guidsOfEntity ) {
                        $entityId = $entityIds[ $serializedEntityId ];
-                       $resultLists[ $serializedEntityId ] = 
$this->crossCheckClaimsOfEntity( $entityId, $claimGuidsOfEntity );
+                       $resultLists[ $serializedEntityId ] = 
$this->crossCheckClaimsOfEntity( $entityId, $guidsOfEntity );
                }
 
                return $resultLists;
@@ -255,16 +254,17 @@
 
        /**
         * @param EntityId $entityId
-        * @param string[] $clamGuids
+        * @param string[] $guids
+        *
         * @return CrossCheckResultList|null
         */
-       private function crossCheckClaimsOfEntity( EntityId $entityId, 
$clamGuids ) {
+       private function crossCheckClaimsOfEntity( EntityId $entityId, array 
$guids ) {
                $entity = $this->entityLookup->getEntity( $entityId );
 
                if ( $entity instanceof StatementListProvider ) {
                        $statements = new StatementList();
                        foreach ( $entity->getStatements()->toArray() as 
$statement ) {
-                               if ( in_array( $statement->getGuid(), 
$clamGuids ) ) {
+                               if ( in_array( $statement->getGuid(), $guids ) 
) {
                                        $statements->addStatement( $statement );
                                }
                        }
diff --git a/includes/DumpMetaInformation/DumpMetaInformation.php 
b/includes/DumpMetaInformation/DumpMetaInformation.php
index 57ac420..fa2cf2a 100644
--- a/includes/DumpMetaInformation/DumpMetaInformation.php
+++ b/includes/DumpMetaInformation/DumpMetaInformation.php
@@ -75,7 +75,7 @@
        /**
         * @param string $dumpId
         * @param ItemId $sourceItemId
-        * @param array $identifierPropertyIds
+        * @param PropertyId[] $identifierPropertyIds
         * @param string $importDate
         * @param string $languageCode
         * @param string $sourceUrl
diff --git a/includes/DumpMetaInformation/SqlDumpMetaInformationRepo.php 
b/includes/DumpMetaInformation/SqlDumpMetaInformationRepo.php
index 3e6dd23..3b8ea19 100644
--- a/includes/DumpMetaInformation/SqlDumpMetaInformationRepo.php
+++ b/includes/DumpMetaInformation/SqlDumpMetaInformationRepo.php
@@ -84,7 +84,9 @@
         * Gets DumpMetaInformation for specific identifier properties from 
database
         * Returns array in the form 'dumpId' => DumpMetaInformation
         *
-        * @param array $identifierPropertyIds
+        * @param PropertyId[] $identifierPropertyIds
+        *
+        * @throws InvalidArgumentException
         * @return DumpMetaInformation[]
         */
        public function getWithIdentifierProperties( array 
$identifierPropertyIds ) {
diff --git a/includes/ExternalDataRepo.php b/includes/ExternalDataRepo.php
index 2aa0c61..62a24e4 100644
--- a/includes/ExternalDataRepo.php
+++ b/includes/ExternalDataRepo.php
@@ -19,8 +19,8 @@
         * Gets external data for specified properties from database that 
matches any
         * combination of given dump and external ids
         *
-        * @param array $dumpIds
-        * @param array $externalIds
+        * @param string[] $dumpIds
+        * @param string[] $externalIds
         * @param PropertyId[] $propertyIds
         * @return array
         */
diff --git a/specials/SpecialCrossCheck.php b/specials/SpecialCrossCheck.php
index b7245cd..7d15857 100644
--- a/specials/SpecialCrossCheck.php
+++ b/specials/SpecialCrossCheck.php
@@ -178,7 +178,7 @@
                                return;
                        }
 
-                       $results = 
$this->crossCheckInteractor->crossCheckStatementList( $entity->getStatements() 
);
+                       $results = 
$this->crossCheckInteractor->crossCheckStatements( $entity->getStatements() );
 
                        if ($results && count($results) > 0) {
                                $out->addHTML(
diff --git a/tests/phpunit/Api/RunCrossCheckTest.php 
b/tests/phpunit/Api/RunCrossCheckTest.php
index 7ad4f02..2d5510f 100644
--- a/tests/phpunit/Api/RunCrossCheckTest.php
+++ b/tests/phpunit/Api/RunCrossCheckTest.php
@@ -94,27 +94,27 @@
 
                        $dataValue = new EntityIdValue( new ItemId( 
IDENTIFIER_PROPERTY_QID ) );
                        $snak = new PropertyValueSnak( new PropertyId( 
INSTANCE_OF_PID ), $dataValue );
-                       $claimGuid = self::$idMap['P3']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
-                       $propertyP3->getStatements()->addNewStatement( $snak, 
null, null, $claimGuid );
+                       $guid = self::$idMap['P3']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
+                       $propertyP3->getStatements()->addNewStatement( $snak, 
null, null, $guid );
                        $store->saveEntity( $propertyP3, 'TestEntityP3',  
$GLOBALS['wgUser'], EDIT_UPDATE );
 
                        $dataValue = new StringValue( 'foo' );
                        $snak = new PropertyValueSnak( self::$idMap['P1'], 
$dataValue );
-                       $claimGuid = self::$idMap['Q1']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
-                       self::$claimGuids['P1'] = $claimGuid;
-                       $itemQ1->getStatements()->addNewStatement( $snak, null, 
null, $claimGuid );
+                       $guid = self::$idMap['Q1']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
+                       self::$claimGuids['P1'] = $guid;
+                       $itemQ1->getStatements()->addNewStatement( $snak, null, 
null, $guid );
 
                        $dataValue = new StringValue( 'baz' );
                        $snak = new PropertyValueSnak( self::$idMap['P2'], 
$dataValue );
-                       $claimGuid = self::$idMap['Q1']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
-                       self::$claimGuids['P2'] = $claimGuid;
-                       $itemQ1->getStatements()->addNewStatement( $snak, null, 
null, $claimGuid );
+                       $guid = self::$idMap['Q1']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
+                       self::$claimGuids['P2'] = $guid;
+                       $itemQ1->getStatements()->addNewStatement( $snak, null, 
null, $guid );
 
                        $dataValue = new StringValue( '1234' );
                        $snak = new PropertyValueSnak( self::$idMap['P3'], 
$dataValue );
-                       $claimGuid = self::$idMap['Q1']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
-                       self::$claimGuids['P3'] = $claimGuid;
-                       $itemQ1->getStatements()->addNewStatement( $snak, null, 
null, $claimGuid );
+                       $guid = self::$idMap['Q1']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
+                       self::$claimGuids['P3'] = $guid;
+                       $itemQ1->getStatements()->addNewStatement( $snak, null, 
null, $guid );
 
                        $store->saveEntity( $itemQ1, 'TestEntityQ1', 
$GLOBALS['wgUser'], EDIT_UPDATE );
 
diff --git a/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php 
b/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
index c1d0094..0750db2 100644
--- a/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
+++ b/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
@@ -47,7 +47,7 @@
                parent::setUp();
 
                $entityLookup = new JsonFileEntityLookup( __DIR__ . '/testdata' 
);
-               $claimGuidParser = new StatementGuidParser( new 
BasicEntityIdParser() );
+               $guidParser = new StatementGuidParser( new 
BasicEntityIdParser() );
                $crossChecker = $this->getMockBuilder( 
'WikibaseQuality\ExternalValidation\CrossCheck\CrossChecker' )
                        ->disableOriginalConstructor()
                        ->setMethods( array( 'crossCheckStatements' ) )
@@ -64,7 +64,7 @@
                                        );
                                }
                        ) );
-               $this->crossCheckInteractor = new CrossCheckInteractor( 
$entityLookup, $claimGuidParser, $crossChecker );
+               $this->crossCheckInteractor = new CrossCheckInteractor( 
$entityLookup, $guidParser, $crossChecker );
        }
 
        public function tearDown() {
@@ -179,7 +179,7 @@
         * @dataProvider crossCheckEntityDataProvider
         */
        public function testCrossCheckEntity( StatementList $statements, array 
$expectedResult ) {
-               $actualResult = 
$this->crossCheckInteractor->crossCheckStatementList( $statements );
+               $actualResult = 
$this->crossCheckInteractor->crossCheckStatements( $statements );
 
                $this->runAssertions( $expectedResult, $actualResult );
        }
@@ -604,7 +604,7 @@
         * @dataProvider crossCheckClaimDataProvider
         */
        public function testCrossCheckClaim(
-               $claimGuid,
+               $guid,
                array $expectedResult = null,
                $expectedException = null
        ) {
@@ -612,7 +612,7 @@
                        $this->setExpectedException( $expectedException );
                }
 
-               $actualResult = 
$this->crossCheckInteractor->crossCheckStatement( $claimGuid );
+               $actualResult = 
$this->crossCheckInteractor->crossCheckStatementByGuid( $guid );
 
                $this->runAssertions( $expectedResult, $actualResult );
        }
@@ -647,7 +647,7 @@
         * @dataProvider crossCheckClaimsDataProvider
         */
        public function testCrossCheckClaims(
-               array $claimGuids,
+               array $guids,
                array $expectedResult = null,
                $expectedException = null
        ) {
@@ -655,7 +655,7 @@
                        $this->setExpectedException( $expectedException );
                }
 
-               $actualResult = 
$this->crossCheckInteractor->crossCheckStatements( $claimGuids );
+               $actualResult = 
$this->crossCheckInteractor->crossCheckStatementsByGuids( $guids );
 
                $this->runAssertions( $expectedResult, $actualResult );
        }
diff --git a/tests/phpunit/CrossCheck/Result/CrossCheckResultTest.php 
b/tests/phpunit/CrossCheck/Result/CrossCheckResultTest.php
index 807d0f4..7822904 100644
--- a/tests/phpunit/CrossCheck/Result/CrossCheckResultTest.php
+++ b/tests/phpunit/CrossCheck/Result/CrossCheckResultTest.php
@@ -23,18 +23,18 @@
 
                // Create test data
                $propertyId = new PropertyId( 'P42' );
-               $claimGuid = 'Q42$fccafc70-07a0-4e82-807f-288a4b21c13c';
+               $guid = 'Q42$fccafc70-07a0-4e82-807f-288a4b21c13c';
                $externalId = 'foobar';
                $dumpMetaInformation = $this->getDumpMetaInformationMock();
                $comparisonResult = $this->getComparisonResultMock();
                $referenceResult = $this->getReferenceResultMock();
 
                // Create instance
-               $crossCheckResult = new CrossCheckResult( $propertyId, 
$claimGuid, $externalId, $dumpMetaInformation, $comparisonResult, 
$referenceResult );
+               $crossCheckResult = new CrossCheckResult( $propertyId, $guid, 
$externalId, $dumpMetaInformation, $comparisonResult, $referenceResult );
 
                // Run assertions
                $this->assertEquals( $propertyId, 
$crossCheckResult->getPropertyId() );
-               $this->assertEquals( $claimGuid, 
$crossCheckResult->getClaimGuid() );
+               $this->assertEquals( $guid, $crossCheckResult->getClaimGuid() );
                $this->assertEquals( $externalId, 
$crossCheckResult->getExternalId() );
                $this->assertEquals( $dumpMetaInformation, 
$crossCheckResult->getDumpMetaInformation() );
                $this->assertEquals( $comparisonResult, 
$crossCheckResult->getComparisonResult() );
@@ -44,10 +44,10 @@
        /**
         * @dataProvider constructInvalidArgumentsDataProvider
         */
-       public function testConstructInvalidArguments( $propertyId, $claimGuid, 
$externalId, $dumpMetaInformation, $comparisonResult, $referenceResult ) {
+       public function testConstructInvalidArguments( $propertyId, $guid, 
$externalId, $dumpMetaInformation, $comparisonResult, $referenceResult ) {
                $this->setExpectedException( 'InvalidArgumentException' );
 
-               new CrossCheckResult( $propertyId, $claimGuid, $externalId, 
$dumpMetaInformation, $comparisonResult, $referenceResult );
+               new CrossCheckResult( $propertyId, $guid, $externalId, 
$dumpMetaInformation, $comparisonResult, $referenceResult );
        }
 
        /**
diff --git a/tests/phpunit/Specials/SpecialCrossCheckTest.php 
b/tests/phpunit/Specials/SpecialCrossCheckTest.php
index 0a2536f..a54cd1c 100644
--- a/tests/phpunit/Specials/SpecialCrossCheckTest.php
+++ b/tests/phpunit/Specials/SpecialCrossCheckTest.php
@@ -121,8 +121,8 @@
 
                        $dataValue = new EntityIdValue(new 
ItemId(IDENTIFIER_PROPERTY_QID));
                        $snak = new PropertyValueSnak(new 
PropertyId(INSTANCE_OF_PID), $dataValue);
-                       $claimGuid = self::$idMap['P3']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
-                       $propertyP3->getStatements()->addNewStatement($snak, 
null, null, $claimGuid);
+                       $guid = self::$idMap['P3']->getSerialization() . 
StatementGuid::SEPARATOR . $guidGenerator->newGuid();
+                       $propertyP3->getStatements()->addNewStatement($snak, 
null, null, $guid);
                        $store->saveEntity($propertyP3, 'TestEntityP3', 
$GLOBALS['wgUser'], EDIT_UPDATE);
 
                        $dataValue = new StringValue('foo');

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I45449638aca5ff41f2dd5d9ea69b5c90af7ef45d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityExternalValidation
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>

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

Reply via email to