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

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

Change subject: Consistent SiteLink naming and dead code removed
......................................................................

Consistent SiteLink naming and dead code removed

I started this patch with uppercasing some "L" in "$siteLink"
variable names and cleaned some more code I came accross while
doing this.

Change-Id: I176df356f2f6a48a79d35327c3786597ed9923fd
---
M lib/includes/serializers/AliasSerializer.php
M lib/includes/serializers/ByPropertyListSerializer.php
M lib/includes/serializers/ByPropertyListUnserializer.php
M lib/includes/serializers/ClaimsSerializer.php
M lib/includes/serializers/DescriptionSerializer.php
M lib/includes/serializers/EntitySerializer.php
M lib/includes/serializers/ItemSerializer.php
M lib/includes/serializers/LabelSerializer.php
M lib/includes/serializers/ListSerializer.php
M lib/includes/serializers/ListUnserializer.php
M lib/includes/serializers/MultilingualSerializer.php
M lib/includes/serializers/ReferenceSerializer.php
M lib/includes/serializers/SerializerObject.php
M lib/includes/serializers/SiteLinkSerializer.php
M lib/includes/serializers/SnakSerializer.php
M lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
M lib/tests/phpunit/store/SiteLinkTableTest.php
M lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
M repo/includes/api/EditEntity.php
M repo/includes/api/ResultBuilder.php
M repo/includes/specials/SpecialDispatchStats.php
M repo/includes/specials/SpecialItemResolver.php
M repo/tests/phpunit/includes/api/GetEntitiesTest.php
M repo/tests/phpunit/includes/api/ResultBuilderTest.php
M repo/tests/phpunit/includes/api/SetSiteLinkTest.php
25 files changed, 111 insertions(+), 146 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/41/172541/1

diff --git a/lib/includes/serializers/AliasSerializer.php 
b/lib/includes/serializers/AliasSerializer.php
index 9cf8b4f..8fef354 100644
--- a/lib/includes/serializers/AliasSerializer.php
+++ b/lib/includes/serializers/AliasSerializer.php
@@ -18,17 +18,6 @@
 class AliasSerializer extends SerializerObject implements Unserializer {
 
        /**
-        * Constructor.
-        *
-        * @since 0.4
-        *
-        * @param SerializationOptions $options
-        */
-       public function __construct( SerializationOptions $options = null ) {
-               parent::__construct( $options );
-       }
-
-       /**
         * Returns a serialized array of aliases.
         *
         * @since 0.4
@@ -136,4 +125,5 @@
 
                return $aliases;
        }
+
 }
diff --git a/lib/includes/serializers/ByPropertyListSerializer.php 
b/lib/includes/serializers/ByPropertyListSerializer.php
index 86bce1a..8c4e5e6 100644
--- a/lib/includes/serializers/ByPropertyListSerializer.php
+++ b/lib/includes/serializers/ByPropertyListSerializer.php
@@ -20,29 +20,27 @@
        const OPT_ADD_LOWER_CASE_KEYS = 'addLowerCaseKeys';
 
        /**
-        * @since 0.2
-        *
         * @var string
         */
-       protected $elementName;
+       private $elementName;
 
        /**
-        * @since 0.2
-        *
         * @var Serializer
         */
-       protected $elementSerializer;
+       private $elementSerializer;
 
        /**
-        * Constructor.
-        *
         * @since 0.2
         *
         * @param string $elementName
         * @param Serializer $elementSerializer
         * @param SerializationOptions|null $options
         */
-       public function __construct( $elementName, Serializer 
$elementSerializer, SerializationOptions $options = null ) {
+       public function __construct(
+               $elementName,
+               Serializer $elementSerializer,
+               SerializationOptions $options = null
+       ) {
                parent::__construct( $options );
 
                $this->elementName = $elementName;
diff --git a/lib/includes/serializers/ByPropertyListUnserializer.php 
b/lib/includes/serializers/ByPropertyListUnserializer.php
index 820abf3..a645188 100644
--- a/lib/includes/serializers/ByPropertyListUnserializer.php
+++ b/lib/includes/serializers/ByPropertyListUnserializer.php
@@ -22,11 +22,9 @@
         *
         * @var Serializer
         */
-       protected $elementUnserializer;
+       private $elementUnserializer;
 
        /**
-        * Constructor.
-        *
         * @since 0.2
         *
         * @param Unserializer $elementUnserializer
diff --git a/lib/includes/serializers/ClaimsSerializer.php 
b/lib/includes/serializers/ClaimsSerializer.php
index 4f2b08a..d2e04be 100644
--- a/lib/includes/serializers/ClaimsSerializer.php
+++ b/lib/includes/serializers/ClaimsSerializer.php
@@ -39,7 +39,7 @@
         *
         * @since 0.2
         *
-        * @param mixed $claims
+        * @param Claims $claims
         *
         * @return array
         * @throws InvalidArgumentException
diff --git a/lib/includes/serializers/DescriptionSerializer.php 
b/lib/includes/serializers/DescriptionSerializer.php
index f4360fa..73ad9c1 100644
--- a/lib/includes/serializers/DescriptionSerializer.php
+++ b/lib/includes/serializers/DescriptionSerializer.php
@@ -20,25 +20,25 @@
        /**
         * @var MultilingualSerializer
         */
-       protected $multilingualSerializer;
+       private $multilingualSerializer;
 
        /**
-        * Constructor.
-        *
         * @since 0.4
         *
         * @param SerializationOptions $options
         * @param MultilingualSerializer $multilingualSerializer
         */
-       public function __construct( SerializationOptions $options = null,
+       public function __construct(
+               SerializationOptions $options = null,
                MultilingualSerializer $multilingualSerializer = null
        ) {
+               parent::__construct( $options );
+
                if ( $multilingualSerializer === null ) {
                        $this->multilingualSerializer = new 
MultilingualSerializer( $options );
                } else {
                        $this->multilingualSerializer = $multilingualSerializer;
                }
-               parent::__construct( $options );
        }
 
        /**
@@ -113,4 +113,5 @@
 
                return $descriptions;
        }
+
 }
diff --git a/lib/includes/serializers/EntitySerializer.php 
b/lib/includes/serializers/EntitySerializer.php
index 9d94a17..4d0b9cb 100644
--- a/lib/includes/serializers/EntitySerializer.php
+++ b/lib/includes/serializers/EntitySerializer.php
@@ -58,8 +58,6 @@
        private $claimSerializer;
 
        /**
-        * Constructor.
-        *
         * @since 0.2
         *
         * @param ClaimSerializer $claimSerializer
@@ -68,7 +66,11 @@
         *
         * @todo: make $entityFactory required
         */
-       public function __construct( ClaimSerializer $claimSerializer, 
SerializationOptions $options = null, EntityFactory $entityFactory = null ) {
+       public function __construct(
+               ClaimSerializer $claimSerializer,
+               SerializationOptions $options = null,
+               EntityFactory $entityFactory = null
+       ) {
                if ( $options === null ) {
                        $options = new SerializationOptions();
                }
@@ -109,7 +111,7 @@
        /**
         * @since 0.2
         *
-        * @param mixed $entity
+        * @param Entity $entity
         *
         * @return array
         * @throws InvalidArgumentException
diff --git a/lib/includes/serializers/ItemSerializer.php 
b/lib/includes/serializers/ItemSerializer.php
index 11d57d0..6145ea8 100644
--- a/lib/includes/serializers/ItemSerializer.php
+++ b/lib/includes/serializers/ItemSerializer.php
@@ -28,11 +28,9 @@
         *
         * @var SiteStore
         */
-       protected $siteStore;
+       private $siteStore;
 
        /**
-        * Constructor.
-        *
         * @since 0.4
         *
         * @param ClaimSerializer $claimSerializer
@@ -49,9 +47,9 @@
                SerializationOptions $options = null,
                EntityFactory $entityFactory = null
        ) {
-               $this->siteStore = $siteStore;
-
                parent::__construct( $claimSerializer, $options, $entityFactory 
);
+
+               $this->siteStore = $siteStore;
        }
 
        /**
diff --git a/lib/includes/serializers/LabelSerializer.php 
b/lib/includes/serializers/LabelSerializer.php
index 4f4ea48..8f9e20a 100644
--- a/lib/includes/serializers/LabelSerializer.php
+++ b/lib/includes/serializers/LabelSerializer.php
@@ -20,25 +20,25 @@
        /**
         * @var MultilingualSerializer
         */
-       protected $multilingualSerializer;
+       private $multilingualSerializer;
 
        /**
-        * Constructor.
-        *
         * @since 0.4
         *
         * @param SerializationOptions $options
         * @param MultilingualSerializer $multilingualSerializer
         */
-       public function __construct( SerializationOptions $options = null,
+       public function __construct(
+               SerializationOptions $options = null,
                MultilingualSerializer $multilingualSerializer = null
        ) {
+               parent::__construct( $options );
+
                if ( $multilingualSerializer === null ) {
                        $this->multilingualSerializer = new 
MultilingualSerializer( $options );
                } else {
                        $this->multilingualSerializer = $multilingualSerializer;
                }
-               parent::__construct( $options );
        }
 
        /**
diff --git a/lib/includes/serializers/ListSerializer.php 
b/lib/includes/serializers/ListSerializer.php
index 4db1449..9ac5e74 100644
--- a/lib/includes/serializers/ListSerializer.php
+++ b/lib/includes/serializers/ListSerializer.php
@@ -18,16 +18,18 @@
        /**
         * @var Serializer
         */
-       protected $elementSerializer;
+       private $elementSerializer;
 
        /**
-        * Constructor.
-        *
         * @param string $elementName
         * @param Serializer $elementSerializer
         * @param SerializationOptions|null $options
         */
-       public function __construct( $elementName, Serializer 
$elementSerializer, SerializationOptions $options = null ) {
+       public function __construct(
+               $elementName,
+               Serializer $elementSerializer,
+               SerializationOptions $options = null
+       ) {
                parent::__construct( $options );
 
                $this->elementName = $elementName;
diff --git a/lib/includes/serializers/ListUnserializer.php 
b/lib/includes/serializers/ListUnserializer.php
index fb9557d..01ea673 100644
--- a/lib/includes/serializers/ListUnserializer.php
+++ b/lib/includes/serializers/ListUnserializer.php
@@ -15,11 +15,9 @@
        /**
         * @var Unserializer
         */
-       protected $elementUnserializer;
+       private $elementUnserializer;
 
        /**
-        * Constructor.
-        *
         * @param Unserializer $elementUnserializer
         */
        public function __construct( Unserializer $elementUnserializer ) {
diff --git a/lib/includes/serializers/MultilingualSerializer.php 
b/lib/includes/serializers/MultilingualSerializer.php
index ef01ad5..6a885bc 100644
--- a/lib/includes/serializers/MultilingualSerializer.php
+++ b/lib/includes/serializers/MultilingualSerializer.php
@@ -14,15 +14,11 @@
 class MultilingualSerializer {
 
        /**
-        * @since 0.4
-        *
         * @var SerializationOptions
         */
-       protected $options;
+       private $options;
 
        /**
-        * Constructor.
-        *
         * @since 0.4
         *
         * @param SerializationOptions $options
@@ -45,7 +41,6 @@
         * @return array
         */
        public function serializeMultilingualValues( array $data ) {
-
                $values = array();
                $idx = 0;
 
@@ -104,4 +99,5 @@
 
                return $values;
        }
+
 }
diff --git a/lib/includes/serializers/ReferenceSerializer.php 
b/lib/includes/serializers/ReferenceSerializer.php
index 1ef95de..4c7da15 100644
--- a/lib/includes/serializers/ReferenceSerializer.php
+++ b/lib/includes/serializers/ReferenceSerializer.php
@@ -41,7 +41,7 @@
         *
         * @since 0.3
         *
-        * @param mixed $reference
+        * @param Reference $reference
         *
         * @return array
         * @throws InvalidArgumentException
diff --git a/lib/includes/serializers/SerializerObject.php 
b/lib/includes/serializers/SerializerObject.php
index 5dcd16a..4d2cab8 100644
--- a/lib/includes/serializers/SerializerObject.php
+++ b/lib/includes/serializers/SerializerObject.php
@@ -24,8 +24,6 @@
        protected $options;
 
        /**
-        * Constructor.
-        *
         * @since 0.3
         *
         * @param SerializationOptions|null $options
diff --git a/lib/includes/serializers/SiteLinkSerializer.php 
b/lib/includes/serializers/SiteLinkSerializer.php
index 36b5f96..adbba47 100644
--- a/lib/includes/serializers/SiteLinkSerializer.php
+++ b/lib/includes/serializers/SiteLinkSerializer.php
@@ -25,15 +25,11 @@
 class SiteLinkSerializer extends SerializerObject {
 
        /**
-        * @since 0.4
-        *
         * @var SiteStore $siteStore
         */
-       protected $siteStore;
+       private $siteStore;
 
        /**
-        * Constructor.
-        *
         * @since 0.4
         *
         * @param SerializationOptions $options
@@ -44,8 +40,9 @@
                        'sitelinks',
                ) );
 
-               $this->siteStore = $siteStore;
                parent::__construct( $options );
+
+               $this->siteStore = $siteStore;
        }
 
        /**
@@ -53,9 +50,9 @@
         *
         * @since 0.4
         *
-        * @param array $siteLinks
+        * @param SiteLink[] $siteLinks
         *
-        * @return array
+        * @return array[]
         * @throws InvalidArgumentException
         */
        final public function getSerialized( $siteLinks ) {
@@ -72,9 +69,9 @@
                $includeUrls = in_array( 'sitelinks/urls', $parts );
                $setRemoved = in_array( 'sitelinks/removed' , $parts );
 
-               foreach ( $this->sortSiteLinks( $siteLinks ) as $link ) {
-                       $siteId = $link->getSiteId();
-                       $pageName = $link->getPageName();
+               foreach ( $this->sortSiteLinks( $siteLinks ) as $siteLink ) {
+                       $siteId = $siteLink->getSiteId();
+                       $pageName = $siteLink->getPageName();
 
                        $response = array(
                                'site' => $siteId,
@@ -92,7 +89,7 @@
                        if ( !$setRemoved ) {
                                $badges = array();
 
-                               foreach ( $link->getBadges() as $badge ) {
+                               foreach ( $siteLink->getBadges() as $badge ) {
                                        $badges[] = $badge->getSerialization();
                                }
 
@@ -108,7 +105,7 @@
                        }
 
                        if ( !$this->options->shouldIndexTags() ) {
-                               $serialization[$link->getSiteId()] = $response;
+                               $serialization[$siteLink->getSiteId()] = 
$response;
                        }
                        else {
                                $serialization[] = $response;
@@ -214,4 +211,5 @@
 
                return $badges;
        }
+
 }
diff --git a/lib/includes/serializers/SnakSerializer.php 
b/lib/includes/serializers/SnakSerializer.php
index b51a520..a753dd0 100644
--- a/lib/includes/serializers/SnakSerializer.php
+++ b/lib/includes/serializers/SnakSerializer.php
@@ -50,7 +50,7 @@
         *
         * @since 0.2
         *
-        * @param mixed $snak
+        * @param Snak $snak
         *
         * @return array
         * @throws InvalidArgumentException
diff --git a/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php 
b/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
index abde66b..510adfc 100644
--- a/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
+++ b/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
@@ -23,7 +23,7 @@
  */
 class SiteLinkSerializerTest extends \PHPUnit_Framework_TestCase {
 
-       public function validProvider() {
+       public function validSiteLinksProvider() {
                $validArgs = array();
 
                $options = new SerializationOptions();
@@ -74,7 +74,7 @@
        }
 
        /**
-        * @dataProvider validProvider
+        * @dataProvider validSiteLinksProvider
         */
        public function testGetSerialized( $siteLinks, $options, 
$expectedSerialization ) {
                $siteStore = SiteSQLStore::newInstance();
@@ -85,24 +85,22 @@
                $this->assertEquals( $expectedSerialization, 
$serializedSiteLinks );
        }
 
-       public function invalidProvider() {
-               $invalidArgs = array();
-
-               $invalidArgs[] = array( 'foo' );
-               $invalidArgs[] = array( 42 );
-
-               return $invalidArgs;
+       public function invalidSiteLinksProvider() {
+               return array(
+                       array( 'foo' ),
+                       array( 42 ),
+               );
        }
 
        /**
-        * @dataProvider invalidProvider
+        * @dataProvider invalidSiteLinksProvider
         * @expectedException InvalidArgumentException
         */
-       public function testInvalidGetSerialized( $sitelinks ) {
+       public function testInvalidGetSerialized( $siteLinks ) {
                $options = new SerializationOptions();
                $siteStore = SiteSQLStore::newInstance();
                $siteLinkSerializer = new SiteLinkSerializer( $options, 
$siteStore );
-               $siteLinkSerializer->getSerialized( $sitelinks );
+               $siteLinkSerializer->getSerialized( $siteLinks );
        }
 
        /**
diff --git a/lib/tests/phpunit/store/SiteLinkTableTest.php 
b/lib/tests/phpunit/store/SiteLinkTableTest.php
index 51eb346..46b8d01 100644
--- a/lib/tests/phpunit/store/SiteLinkTableTest.php
+++ b/lib/tests/phpunit/store/SiteLinkTableTest.php
@@ -44,14 +44,14 @@
                $item->setId( new ItemId( 'q1' ) );
                $item->setLabel( 'en', 'Beer' );
 
-               $sitelinks = array(
+               $siteLinks = array(
                        'cswiki' => 'Pivo',
                        'enwiki' => 'Beer',
                        'jawiki' => 'ビール'
                );
 
-               foreach( $sitelinks as $site => $page ) {
-                       $item->addSiteLink( new SiteLink( $site, $page ) );
+               foreach( $siteLinks as $siteId => $pageName ) {
+                       $item->getSiteLinkList()->addNewSiteLink( $siteId, 
$pageName );
                }
 
                $items[] = $item;
diff --git a/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php 
b/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
index 1961cb9..ef66455 100644
--- a/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
+++ b/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
@@ -22,11 +22,10 @@
  */
 class SqlEntityInfoBuilderTest extends EntityInfoBuilderTest {
 
+       /**
+        * @var bool
+        */
        private $useRedirectTargetColumn;
-
-       public function __construct( $name = null, $data = array(), $dataName = 
'' ) {
-               parent::__construct( $name, $data, $dataName );
-       }
 
        public function setUp() {
                parent::setUp();
diff --git a/repo/includes/api/EditEntity.php b/repo/includes/api/EditEntity.php
index 5c36393..8d094ea 100644
--- a/repo/includes/api/EditEntity.php
+++ b/repo/includes/api/EditEntity.php
@@ -142,16 +142,16 @@
        protected function validateParameters( array $params ) {
                $hasId = isset( $params['id'] );
                $hasNew = isset( $params['new'] );
-               $hasSitelink = ( isset( $params['site'] ) && isset( 
$params['title'] ) );
-               $hasSitelinkPart = ( isset( $params['site'] ) || isset( 
$params['title'] ) );
+               $hasSiteLink = isset( $params['site'] ) && isset( 
$params['title'] );
+               $hasSiteLinkPart = isset( $params['site'] ) || isset( 
$params['title'] );
 
-               if ( !( $hasId XOR $hasSitelink XOR $hasNew ) ) {
+               if ( !( $hasId XOR $hasSiteLink XOR $hasNew ) ) {
                        $this->dieError( 'Either provide the item "id" or pairs 
of "site" and "title" or a "new" type for an entity' , 'param-missing' );
                }
-               if( $hasId && $hasSitelink ){
+               if ( $hasId && $hasSiteLink ) {
                        $this->dieError( "Parameter 'id' and 'site', 'title' 
combination are not allowed to be both set in the same request", 
'param-illegal' );
                }
-               if( ( $hasId || $hasSitelinkPart ) && $hasNew ){
+               if ( ( $hasId || $hasSiteLinkPart ) && $hasNew ) {
                        $this->dieError( "Parameters 'id', 'site', 'title' and 
'new' are not allowed to be both set in the same request", 'param-illegal' );
                }
        }
diff --git a/repo/includes/api/ResultBuilder.php 
b/repo/includes/api/ResultBuilder.php
index 1f4e6dc..a05e9c6 100644
--- a/repo/includes/api/ResultBuilder.php
+++ b/repo/includes/api/ResultBuilder.php
@@ -322,8 +322,8 @@
                        $entitySerialization = 
$entitySerializer->getSerialized( $entity );
 
                        if ( !empty( $siteIds ) && array_key_exists( 
'sitelinks', $entitySerialization ) ) {
-                               foreach ( $entitySerialization['sitelinks'] as 
$siteId => $sitelink ) {
-                                       if ( is_array( $sitelink ) && 
!in_array( $sitelink['site'], $siteIds ) ) {
+                               foreach ( $entitySerialization['sitelinks'] as 
$siteId => $siteLink ) {
+                                       if ( is_array( $siteLink ) && 
!in_array( $siteLink['site'], $siteIds ) ) {
                                                unset( 
$entitySerialization['sitelinks'][$siteId] );
                                        }
                                }
@@ -542,21 +542,18 @@
         * @param string|null|array $path Where in the result to put the 
revision id
         */
        public function addRevisionIdFromStatusToResult( Status $status, $path 
) {
-               $statusValue = $status->getValue();
+               $value = $status->getValue();
 
-               /* @var Revision $revision */
-               $revision = isset( $statusValue['revision'] )
-                       ? $statusValue['revision'] : null;
+               if ( isset( $value['revision'] ) ) {
+                       $revision = $value['revision'];
 
-               if ( $revision ) {
-                       //HACK: $revision may be a Revision or EntityRevision
-                       $revId = ( $revision instanceof Revision ) ? 
$revision->getId() : $revision->getRevision();
+                       if ( $revision instanceof Revision ) {
+                               $revisionId = $revision->getId();
+                       } elseif ( $revision instanceof EntityRevision ) {
+                               $revisionId = $revision->getRevision();
+                       }
 
-                       $this->setValue(
-                               $path,
-                               'lastrevid',
-                               intval( $revId )
-                       );
+                       $this->setValue( $path, 'lastrevid', empty( $revisionId 
) ? 0 : $revisionId );
                }
        }
 
diff --git a/repo/includes/specials/SpecialDispatchStats.php 
b/repo/includes/specials/SpecialDispatchStats.php
index 9615dee..b040cad 100644
--- a/repo/includes/specials/SpecialDispatchStats.php
+++ b/repo/includes/specials/SpecialDispatchStats.php
@@ -19,7 +19,6 @@
         */
        public function __construct() {
                parent::__construct( 'DispatchStats' );
-
        }
 
        protected function outputRow( $data, $tag = 'td', $attr = array() ) {
diff --git a/repo/includes/specials/SpecialItemResolver.php 
b/repo/includes/specials/SpecialItemResolver.php
index 1ae7b94..60eb3e2 100644
--- a/repo/includes/specials/SpecialItemResolver.php
+++ b/repo/includes/specials/SpecialItemResolver.php
@@ -14,17 +14,6 @@
        // TODO: would we benefit from using cached page here?
 
        /**
-        * @since 0.1
-        *
-        * @param string $name
-        * @param string $restriction
-        * @param boolean $listed
-        */
-       public function __construct( $name = '', $restriction = '', $listed = 
true ) {
-               parent::__construct( $name, $restriction, $listed );
-       }
-
-       /**
         * @see SpecialWikibasePagePage::$subPage
         *
         * @since 0.1
diff --git a/repo/tests/phpunit/includes/api/GetEntitiesTest.php 
b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
index d7a894b..df854e3 100644
--- a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
+++ b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
@@ -352,9 +352,9 @@
         * @param array $entity
         */
        private function assertEntityPropsSitelinksUrls( $entity ) {
-               foreach ( $entity['sitelinks'] as $sitelink ) {
-                       $this->assertArrayHasKey( 'url', $sitelink );
-                       $this->assertNotEmpty( $sitelink['url'] );
+               foreach ( $entity['sitelinks'] as $siteLink ) {
+                       $this->assertArrayHasKey( 'url', $siteLink );
+                       $this->assertNotEmpty( $siteLink['url'] );
                }
        }
 
@@ -362,11 +362,11 @@
         * @param array $entity
         */
        private function assertEntityPropsSitelinksBadges( $entity ) {
-               foreach ( $entity['sitelinks'] as $sitelink ) {
-                       $this->assertArrayHasKey( 'badges', $sitelink );
-                       $this->assertInternalType( 'array', $sitelink['badges'] 
);
+               foreach ( $entity['sitelinks'] as $siteLink ) {
+                       $this->assertArrayHasKey( 'badges', $siteLink );
+                       $this->assertInternalType( 'array', $siteLink['badges'] 
);
 
-                       foreach ( $sitelink['badges'] as $badge ) {
+                       foreach ( $siteLink['badges'] as $badge ) {
                                $this->assertStringStartsWith( 'Q', $badge );
                                $this->assertGreaterThan( 1, strlen( $badge ) );
                        }
@@ -540,7 +540,7 @@
                $this->assertEquals( $expectedDescriptions, 
$res['entities'][$id]['descriptions'] );
        }
 
-       public function testSitelinkFilter () {
+       public function testSiteLinkFilter () {
                $id = EntityTestHelper::getId( 'Oslo' );
 
                list( $res,, ) = $this->doApiRequest(
@@ -552,7 +552,7 @@
                        )
                );
 
-               $expectedSitelinks = array(
+               $expectedSiteLinks = array(
                        'dewiki' => array(
                                'site' => 'dewiki',
                                'title' => 'Oslo',
@@ -564,6 +564,7 @@
                                'badges' => array(),
                        ),
                );
-               $this->assertEquals( $expectedSitelinks, 
$res['entities'][$id]['sitelinks'] );
+               $this->assertEquals( $expectedSiteLinks, 
$res['entities'][$id]['sitelinks'] );
        }
+
 }
diff --git a/repo/tests/phpunit/includes/api/ResultBuilderTest.php 
b/repo/tests/phpunit/includes/api/ResultBuilderTest.php
index d22742c..00dc8d4 100644
--- a/repo/tests/phpunit/includes/api/ResultBuilderTest.php
+++ b/repo/tests/phpunit/includes/api/ResultBuilderTest.php
@@ -519,7 +519,10 @@
 
        public function testAddSiteLinks(){
                $result = $this->getDefaultResult();
-               $sitelinks = array( new SiteLink( 'enwiki', 'User:Addshore' ), 
new SiteLink( 'dewikivoyage', 'Berlin' ) );
+               $siteLinks = array(
+                       new SiteLink( 'enwiki', 'User:Addshore' ),
+                       new SiteLink( 'dewikivoyage', 'Berlin' ),
+               );
                $path = array( 'entities', 'Q1' );
                $expected = array(
                        'entities' => array(
@@ -541,7 +544,7 @@
                );
 
                $resultBuilder = $this->getResultBuilder( $result );
-               $resultBuilder->addSiteLinks( $sitelinks, $path );
+               $resultBuilder->addSiteLinks( $siteLinks, $path );
 
                $this->assertEquals( $expected, $result->getData() );
        }
diff --git a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php 
b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
index c03ca45..dc88355 100644
--- a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
@@ -229,34 +229,34 @@
 
                // -- check the result only has our changed data (if any)  
------------
                $linkSite = $params['linksite'];
-               $sitelinks = $result['entity']['sitelinks'];
+               $siteLinks = $result['entity']['sitelinks'];
 
-               $this->assertEquals( 1, count( $sitelinks ),
+               $this->assertEquals( 1, count( $siteLinks ),
                        "Entity return contained more than a single site"
                );
 
-               $this->assertArrayHasKey( $linkSite, $sitelinks,
+               $this->assertArrayHasKey( $linkSite, $siteLinks,
                        "Entity doesn't return expected site"
                );
 
-               $sitelink = $sitelinks[$linkSite];
+               $siteLink = $siteLinks[$linkSite];
 
-               $this->assertEquals( $linkSite, $sitelink['site'],
+               $this->assertEquals( $linkSite, $siteLink['site'],
                        "Returned incorrect site"
                );
 
                if ( array_key_exists( $linkSite, $expected['value'] ) ) {
-                       $expSitelink = $expected['value'][ $linkSite ];
+                       $expectedSiteLink = $expected['value'][$linkSite];
 
-                       $this->assertArrayHasKey( 'url', $sitelink );
-                       $this->assertEquals( $expSitelink['title'], 
$sitelink['title'],
+                       $this->assertArrayHasKey( 'url', $siteLink );
+                       $this->assertEquals( $expectedSiteLink['title'], 
$siteLink['title'],
                                "Returned incorrect title"
                        );
 
-                       $this->assertArrayHasKey( 'badges', $sitelink );
-                       $this->assertArrayEquals( $expSitelink['badges'], 
$sitelink['badges'] );
+                       $this->assertArrayHasKey( 'badges', $siteLink );
+                       $this->assertArrayEquals( $expectedSiteLink['badges'], 
$siteLink['badges'] );
                } else if ( empty( $expected['value'] ) ) {
-                       $this->assertArrayHasKey( 'removed', $sitelink,
+                       $this->assertArrayHasKey( 'removed', $siteLink,
                                "Entity doesn't return expected 'removed' 
marker"
                        );
                }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I176df356f2f6a48a79d35327c3786597ed9923fd
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