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