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

Reply via email to