Thiemo Mättig (WMDE) has uploaded a new change for review. https://gerrit.wikimedia.org/r/204249
Change subject: Fix various code smell issues in RDF related code ...................................................................... Fix various code smell issues in RDF related code E.g. outdated documentation, missing use clauses and such. Change-Id: Ic4a3df75e4e96cd90748b631059828975f3a025a --- M repo/includes/rdf/RdfBuilder.php M repo/includes/rdf/RdfSerializer.php M repo/tests/phpunit/includes/Dumpers/JsonDumpGeneratorTest.php M repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php 4 files changed, 63 insertions(+), 30 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/49/204249/1 diff --git a/repo/includes/rdf/RdfBuilder.php b/repo/includes/rdf/RdfBuilder.php index 77c17a2..a5b5f8e 100644 --- a/repo/includes/rdf/RdfBuilder.php +++ b/repo/includes/rdf/RdfBuilder.php @@ -4,6 +4,7 @@ use BagOStuff; use DataValues\DataValue; +use InvalidArgumentException; use SiteList; use Wikibase\DataModel\Entity\BasicEntityIdParser; use Wikibase\DataModel\Entity\Entity; @@ -13,6 +14,9 @@ use Wikibase\DataModel\Entity\Item; use Wikibase\DataModel\Entity\Property; use Wikibase\DataModel\Entity\PropertyId; +use Wikibase\DataModel\Reference; +use Wikibase\DataModel\SiteLink; +use Wikibase\DataModel\Snak\PropertyValueSnak; use Wikibase\DataModel\Snak\Snak; use Wikibase\DataModel\Statement\Statement; use Wikibase\DataModel\StatementListProvider; @@ -497,7 +501,6 @@ ->is( self::NS_ENTITY, $this->getEntityLName( $badge ) ); } } - } /** @@ -512,6 +515,8 @@ $statementList = $entity->getStatements(); $bestList = array(); $produceTruthy = $this->shouldProduce( RdfProducer::PRODUCE_TRUTHY_STATEMENTS ) ; + + /** @var Statement $statement */ foreach ( $statementList->getBestStatementPerProperty() as $statement ) { $bestList[$statement->getGuid()] = true; if ( $produceTruthy ) { @@ -569,6 +574,7 @@ } if ( $this->shouldProduce( RdfProducer::PRODUCE_REFERENCES ) ) { + /** @var Reference $reference */ foreach ( $statement->getReferences() as $reference ) { //FIXME: split body into separate method $hash = $reference->getSnaks()->getHash(); $refLName = $hash; @@ -628,7 +634,6 @@ } else { wfLogWarning( "Unknown rank $rank encountered for $entityId:{$statement->getGuid()}" ); } - } } @@ -639,6 +644,8 @@ * @param Snak $snak Snak object * @param string $propertyNamespace The property namespace for this snak * @param bool $simpleValue + * + * @throws InvalidArgumentException */ private function addSnak( RdfWriter $writer, Snak $snak, $propertyNamespace, $simpleValue = false ) { $propertyId = $snak->getPropertyId(); @@ -658,7 +665,7 @@ $writer->say( $propertyNamespace, $propertyValueLName )->is( self::NS_ONTOLOGY, 'Novalue' ); break; default: - throw new \InvalidArgumentException( 'Unknown snak type: ' . $snak->getType() ); + throw new InvalidArgumentException( 'Unknown snak type: ' . $snak->getType() ); } } @@ -717,15 +724,32 @@ } //FIXME: use a proper registry / dispatching builder - $typeFunc = 'addStatementFor' . preg_replace( '/[^\w]/', '', ucwords( $typeId ) ); - - if ( !is_callable( array( $this, $typeFunc ) ) ) { - wfLogWarning( __METHOD__ . ": Unsupported data type: $typeId" ); - } else { - //TODO: RdfWriter could support aliases -> instead of passing around $propertyNamespace - // and $propertyValueLName, we could define an alias for that and use e.g. '%property' to refer to them. - $this->$typeFunc( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + //TODO: RdfWriter could support aliases -> instead of passing around $propertyNamespace + // and $propertyValueLName, we could define an alias for that and use e.g. '%property' to refer to them. + // TODO: boolean, multilingualtext, number, unknown + switch ( $typeId ) { + case 'wikibase-entityid': + $this->addStatementForWikibaseEntityid( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + break; + case '': + $this->addStatementForString( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + break; + case 'monolingualtext': + $this->addStatementForMonolingualtext( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + break; + case 'time': + $this->addStatementForTime( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + break; + case 'quantity': + $this->addStatementForQuantity( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + break; + case 'globecoordinate': + $this->addStatementForGlobecoordinate( $writer, $propertyNamespace, $propertyValueLName, $dataType, $value, $simpleValue ); + break; + default: + wfLogWarning( __METHOD__ . ": Unsupported data type: $typeId" ); } + // TODO: add special handling like in WDTK? // https://github.com/Wikidata/Wikidata-Toolkit/blob/master/wdtk-rdf/src/main/java/org/wikidata/wdtk/rdf/extensions/SimpleIdExportExtension.java } @@ -742,7 +766,6 @@ */ private function addStatementForWikibaseEntityid( RdfWriter $writer, $propertyValueNamespace, $propertyValueLName, $dataType, EntityIdValue $value, $simpleValue = false ) { - $entityId = $value->getValue()->getEntityId(); $entityLName = $this->getEntityLName( $entityId ); $writer->say( $propertyValueNamespace, $propertyValueLName )->is( self::NS_ENTITY, $entityLName ); @@ -777,7 +800,8 @@ * This function does massaging needed for RDF data types. * * @param RdfWriter $writer - * @param string $propertyName + * @param string $propertyValueNamespace + * @param string $propertyValueLName * @param string $type * @param mixed $value */ @@ -820,7 +844,6 @@ $writer->say( $propertyValueNamespace, $propertyValueLName )->text( $value->getText(), $value->getLanguageCode() ); } - /** * Produce literal that reperesent the date in RDF * If we can convert it to xsd:dateTime, we'll do that. @@ -849,7 +872,6 @@ */ private function addStatementForTime( RdfWriter $writer, $propertyValueNamespace, $propertyValueLName, $dataType, TimeValue $value, $simpleValue = false ) { - $this->addValueToNode( $writer, $propertyValueNamespace, $propertyValueLName, 'dateTime', $value ); if ( !$simpleValue && $this->shouldProduce( RdfProducer::PRODUCE_FULL_VALUES ) ) { //FIXME: register separate generators for different output flavors. @@ -906,7 +928,6 @@ */ private function addStatementForGlobecoordinate( RdfWriter $writer, $propertyValueNamespace, $propertyValueLName, $dataType, GlobeCoordinateValue $value, $simpleValue = false ) { - $point = "Point({$value->getLatitude()} {$value->getLongitude()})"; $writer->say( $propertyValueNamespace, $propertyValueLName )->value( $point, self::NS_GEO, "wktLiteral" ); diff --git a/repo/includes/rdf/RdfSerializer.php b/repo/includes/rdf/RdfSerializer.php index 0f0089e..1e07d84 100644 --- a/repo/includes/rdf/RdfSerializer.php +++ b/repo/includes/rdf/RdfSerializer.php @@ -63,7 +63,7 @@ private $dedupBag; /** - * @param EasyRdf_Format $format + * @param RdfWriter $emitter * @param string $baseUri * @param string $dataUri * @param SiteList $sites; @@ -172,10 +172,10 @@ /** * Start RDF document - * @param int $ts Timestamp (for testing) + * * @return string RDF */ - public function startDocument( ) { + public function startDocument() { $builder = $this->newRdfBuilder(); $builder->startDocument(); return $builder->getRDF(); @@ -195,10 +195,10 @@ /** * Finish RDF document - * @param int $ts Timestamp (for testing) + * * @return string RDF */ - public function finishDocument( ) { + public function finishDocument() { $builder = $this->newRdfBuilder(); $builder->finishDocument(); return $builder->getRDF(); diff --git a/repo/tests/phpunit/includes/Dumpers/JsonDumpGeneratorTest.php b/repo/tests/phpunit/includes/Dumpers/JsonDumpGeneratorTest.php index 315e436..e34ace9 100644 --- a/repo/tests/phpunit/includes/Dumpers/JsonDumpGeneratorTest.php +++ b/repo/tests/phpunit/includes/Dumpers/JsonDumpGeneratorTest.php @@ -15,6 +15,7 @@ use Wikibase\Lib\Serializers\DispatchingEntitySerializer; use Wikibase\Lib\Serializers\SerializationOptions; use Wikibase\Lib\Serializers\SerializerFactory; +use Wikibase\Lib\Store\EntityLookup; use Wikibase\Lib\Store\NullEntityPrefetcher; use Wikibase\Lib\Store\UnresolvedRedirectException; use Wikibase\Repo\Store\EntityIdPager; @@ -215,6 +216,9 @@ return $jsonDumper; } + /** + * @return EntityLookup + */ private function getEntityLookupThrowsMWContentSerializationException() { $entityLookup = $this->getMock( 'Wikibase\Lib\Store\EntityLookup' ); $entityLookup->expects( $this->any() ) diff --git a/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php b/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php index dcbb772..3454e80 100644 --- a/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php +++ b/repo/tests/phpunit/includes/Dumpers/RdfDumpGeneratorTest.php @@ -9,9 +9,10 @@ use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\ItemId; use Wikibase\DataModel\Entity\PropertyId; -use Wikibase\Lib\Store\NullEntityPrefetcher; use Wikibase\Dumpers\RdfDumpGenerator; use Wikibase\EntityRevision; +use Wikibase\Lib\Store\EntityLookup; +use Wikibase\Lib\Store\NullEntityPrefetcher; use Wikibase\Test\RdfBuilderTest; /** @@ -76,16 +77,18 @@ $entityRevisionLookup->expects( $this->any() ) ->method ( 'getEntityRevision' ) - ->will( $this->returnCallback( function( EntityId $id ) use( $entityLookup ) { - $e = $entityLookup->getEntity( $id ); - if ( !$e ) { + ->will( $this->returnCallback( function( EntityId $id ) use ( $entityLookup ) {# + /** @var EntityLookup $entityLookup */ + $entity = $entityLookup->getEntity( $id ); + if ( !$entity ) { return null; } - return new EntityRevision( $e, 12, wfTimestamp( TS_MW, 1000000 ) ); + return new EntityRevision( $entity, 12, wfTimestamp( TS_MW, 1000000 ) ); } )); - return RdfDumpGenerator::createDumpGenerator('ntriples', + return RdfDumpGenerator::createDumpGenerator( + 'ntriples', $out, self::URI_BASE, self::URI_DATA, @@ -162,13 +165,18 @@ /** * @dataProvider loadDataProvider + * @param EntityId[] $ids + * @param string $dumpname */ public function testReferenceDedup( array $ids, $dumpname ) { + $entities = array(); $rdfTest = new RdfBuilderTest(); - foreach( $ids as $id ) { + + foreach ( $ids as $id ) { $id = $id->getSerialization(); $entities[$id] = $rdfTest->getEntityData( $id ); } + $dumper = $this->newDumpGenerator( $entities ); $dumper->setTimestamp(1000000); $jsonTest = new JsonDumpGeneratorTest(); @@ -177,8 +185,8 @@ ob_start(); $dumper->generateDump( $pager ); $dump = ob_get_clean(); - $dump = $this->normalizeData($dump); - $this->assertEquals($this->getSerializedData($dumpname), $dump); + $dump = $this->normalizeData( $dump ); + $this->assertEquals($this->getSerializedData( $dumpname ), $dump); } } -- To view, visit https://gerrit.wikimedia.org/r/204249 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic4a3df75e4e96cd90748b631059828975f3a025a Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase 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