Aleksey Bekh-Ivanov (WMDE) has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/338356 )

Change subject: [Proposal] Don't use readable strings in type IDs
......................................................................

[Proposal] Don't use readable strings in type IDs

It's impossible to change values of entity type ID for Item and Property
but possible for Lexeme, as soon as it is not in production.

Main benefit is that if one will use this just as string value it will
immediately pop up on review, as soon as it will be very noticeable.
It will force all developers use single constant when refering to the ID,
and will reveal relations, like you can see here in LexemeViewTest: why
internal ID exposed to UI?

Also, you can see how many classes currently tied to this ID and can
easily question need for this.

Change-Id: Iac069f87ca8c85582d861e137b2ef7ac8cffa79f
---
M WikibaseLexeme.entitytypes.php
M src/DataModel/Lexeme.php
M src/DataModel/LexemeId.php
M src/DataModel/Serialization/LexemeDeserializer.php
M src/WikibaseLexeme.hooks.php
M tests/phpunit/composer/DataModel/LexemeIdTest.php
M tests/phpunit/composer/DataModel/LexemeTest.php
M tests/phpunit/composer/DataModel/Serialization/LexemeDeserializerTest.php
M tests/phpunit/composer/DataModel/Serialization/LexemeSerializerTest.php
M tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
M tests/phpunit/mediawiki/View/LexemeViewTest.php
11 files changed, 29 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseLexeme 
refs/changes/56/338356/1

diff --git a/WikibaseLexeme.entitytypes.php b/WikibaseLexeme.entitytypes.php
index 494fe77..82cb8dd 100644
--- a/WikibaseLexeme.entitytypes.php
+++ b/WikibaseLexeme.entitytypes.php
@@ -35,7 +35,7 @@
 use Wikibase\View\Template\TemplateFactory;
 
 return [
-       'lexeme' => [
+       Lexeme::ENTITY_TYPE => [
                'serializer-factory-callback' => function( SerializerFactory 
$serializerFactory ) {
                        return new LexemeSerializer(
                                $serializerFactory->newTermListSerializer(),
diff --git a/src/DataModel/Lexeme.php b/src/DataModel/Lexeme.php
index 2f266df..a5d0e1a 100644
--- a/src/DataModel/Lexeme.php
+++ b/src/DataModel/Lexeme.php
@@ -24,7 +24,8 @@
                LabelsProvider, DescriptionsProvider, LemmasProvider, 
LexicalCategoryProvider,
                LanguageProvider {
 
-       const ENTITY_TYPE = 'lexeme';
+       /** Should be NEVER changed */
+       const ENTITY_TYPE = 'xpipo15we3';
 
        /**
         * @var LexemeId|null
diff --git a/src/DataModel/LexemeId.php b/src/DataModel/LexemeId.php
index 3f3261c..a325b9f 100644
--- a/src/DataModel/LexemeId.php
+++ b/src/DataModel/LexemeId.php
@@ -70,7 +70,7 @@
         * @return string
         */
        public function getEntityType() {
-               return 'lexeme';
+               return Lexeme::ENTITY_TYPE;
        }
 
        /**
diff --git a/src/DataModel/Serialization/LexemeDeserializer.php 
b/src/DataModel/Serialization/LexemeDeserializer.php
index 0d064f3..49fa227 100644
--- a/src/DataModel/Serialization/LexemeDeserializer.php
+++ b/src/DataModel/Serialization/LexemeDeserializer.php
@@ -43,7 +43,7 @@
                TermListDeserializer $termListDeserializer,
                StatementListDeserializer $statementListDeserializer
        ) {
-               parent::__construct( 'lexeme', 'type' );
+               parent::__construct( Lexeme::ENTITY_TYPE, 'type' );
                $this->termListDeserializer = $termListDeserializer;
                $this->statementListDeserializer = $statementListDeserializer;
                $this->entityIdDeserializer = $entityIdDeserializer;
diff --git a/src/WikibaseLexeme.hooks.php b/src/WikibaseLexeme.hooks.php
index 2c9a040..c7331eb 100644
--- a/src/WikibaseLexeme.hooks.php
+++ b/src/WikibaseLexeme.hooks.php
@@ -3,6 +3,7 @@
 namespace Wikibase\Lexeme;
 
 use MediaWiki\MediaWikiServices;
+use Wikibase\Lexeme\DataModel\Lexeme;
 
 /**
  * MediaWiki hook handlers for the Wikibase Lexeme extension.
@@ -22,7 +23,7 @@
                $config = MediaWikiServices::getInstance()->getMainConfig();
 
                // Setting the namespace to false disabled automatic 
registration.
-               $entityNamespacesSetting['lexeme'] = $config->get( 
'LexemeNamespace' );
+               $entityNamespacesSetting[Lexeme::ENTITY_TYPE] = $config->get( 
'LexemeNamespace' );
        }
 
        /**
diff --git a/tests/phpunit/composer/DataModel/LexemeIdTest.php 
b/tests/phpunit/composer/DataModel/LexemeIdTest.php
index 1a6baf4..09ff241 100644
--- a/tests/phpunit/composer/DataModel/LexemeIdTest.php
+++ b/tests/phpunit/composer/DataModel/LexemeIdTest.php
@@ -2,6 +2,7 @@
 
 namespace Wikibase\Lexeme\Tests\DataModel;
 
+use Wikibase\Lexeme\DataModel\Lexeme;
 use Wikibase\Lexeme\DataModel\LexemeId;
 use InvalidArgumentException;
 use RuntimeException;
@@ -77,7 +78,7 @@
        }
 
        public function testGetEntityType() {
-               $this->assertSame( 'lexeme', ( new LexemeId( 'L1' ) 
)->getEntityType() );
+               $this->assertSame( Lexeme::ENTITY_TYPE, ( new LexemeId( 'L1' ) 
)->getEntityType() );
        }
 
        public function testSerialize() {
diff --git a/tests/phpunit/composer/DataModel/LexemeTest.php 
b/tests/phpunit/composer/DataModel/LexemeTest.php
index 87ada3c..e78d3e7 100644
--- a/tests/phpunit/composer/DataModel/LexemeTest.php
+++ b/tests/phpunit/composer/DataModel/LexemeTest.php
@@ -51,7 +51,7 @@
        }
 
        public function testGetEntityType() {
-               $this->assertSame( 'lexeme', ( new Lexeme() )->getType() );
+               $this->assertSame( Lexeme::ENTITY_TYPE, ( new Lexeme() 
)->getType() );
        }
 
        public function testSetNewId() {
diff --git 
a/tests/phpunit/composer/DataModel/Serialization/LexemeDeserializerTest.php 
b/tests/phpunit/composer/DataModel/Serialization/LexemeDeserializerTest.php
index 3709bd7..7515b14 100644
--- a/tests/phpunit/composer/DataModel/Serialization/LexemeDeserializerTest.php
+++ b/tests/phpunit/composer/DataModel/Serialization/LexemeDeserializerTest.php
@@ -76,13 +76,13 @@
                $serializations = [];
 
                $serializations['empty'] = [
-                       [ 'type' => 'lexeme' ],
+                       [ 'type' => Lexeme::ENTITY_TYPE ],
                        new Lexeme()
                ];
 
                $serializations['empty lists'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'claims' => []
                        ],
                        new Lexeme()
@@ -90,7 +90,7 @@
 
                $serializations['with id'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'id' => 'L1'
                        ],
                        new Lexeme( new LexemeId( 'L1' ) )
@@ -98,7 +98,7 @@
 
                $serializations['with id and empty lists'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'id' => 'L1',
                                'claims' => []
                        ],
@@ -110,7 +110,7 @@
 
                $serializations['with content'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'claims' => [ 42 ]
                        ],
                        $lexeme
@@ -121,7 +121,7 @@
 
                $serializations['with content and id'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'id' => 'L2',
                                'claims' => [ 42 ]
                        ],
@@ -133,7 +133,7 @@
 
                $serializations['with content and lemmas'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'id' => 'L2',
                                'lemmas' => [ 'el'  => 'Hey' ],
                        ],
@@ -144,7 +144,7 @@
                $lexeme->setLexicalCategory( new ItemId( 'Q33' ) );
                $serializations['with lexical category and id'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'id' => 'L2',
                                'lexicalCategory' => 'Q33'
                        ],
@@ -155,7 +155,7 @@
                $lexeme->setLanguage( new ItemId( 'Q11' ) );
                $serializations['with language and id'] = [
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'id' => 'L3',
                                'language' => 'Q11'
                        ],
diff --git 
a/tests/phpunit/composer/DataModel/Serialization/LexemeSerializerTest.php 
b/tests/phpunit/composer/DataModel/Serialization/LexemeSerializerTest.php
index bfeaf8b..955cff2 100644
--- a/tests/phpunit/composer/DataModel/Serialization/LexemeSerializerTest.php
+++ b/tests/phpunit/composer/DataModel/Serialization/LexemeSerializerTest.php
@@ -56,7 +56,7 @@
                $serializations['empty'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' => Lexeme::ENTITY_TYPE,
                                'claims' => '',
                        ]
                ];
@@ -66,7 +66,7 @@
                $serializations['with id'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' =>  Lexeme::ENTITY_TYPE,
                                'id' => 'L1',
                                'claims' => '',
                        ]
@@ -78,7 +78,7 @@
                $serializations['with content'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' =>  Lexeme::ENTITY_TYPE,
                                'claims' => 'P42',
                        ]
                ];
@@ -89,7 +89,7 @@
                $serializations['with content and id'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' =>  Lexeme::ENTITY_TYPE,
                                'id' => 'L2',
                                'claims' => 'P42',
                        ]
@@ -101,7 +101,7 @@
                $serializations['with lemmas and id'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' =>  Lexeme::ENTITY_TYPE,
                                'id' => 'L2',
                                'lemmas' => [ 'ja' => 'Tokyo' ],
                                'claims' => '',
@@ -114,7 +114,7 @@
                $serializations['with lexical category and id'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' =>  Lexeme::ENTITY_TYPE,
                                'id' => 'L2',
                                'lexicalCategory' => 'Q32',
                                'claims' => '',
@@ -127,7 +127,7 @@
                $serializations['with language and id'] = [
                        $lexeme,
                        [
-                               'type' => 'lexeme',
+                               'type' =>  Lexeme::ENTITY_TYPE,
                                'id' => 'L3',
                                'language' => 'Q11',
                                'claims' => '',
diff --git 
a/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php 
b/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
index 13f019a..e8530cd 100644
--- a/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
+++ b/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
@@ -41,7 +41,7 @@
 
        public function testCanPatchEntityType() {
                $patcher = new LexemePatcher();
-               $this->assertTrue( $patcher->canPatchEntityType( 'lexeme' ) );
+               $this->assertTrue( $patcher->canPatchEntityType(  
Lexeme::ENTITY_TYPE ) );
                $this->assertFalse( $patcher->canPatchEntityType( 'property' ) 
);
                $this->assertFalse( $patcher->canPatchEntityType( '' ) );
                $this->assertFalse( $patcher->canPatchEntityType( null ) );
diff --git a/tests/phpunit/mediawiki/View/LexemeViewTest.php 
b/tests/phpunit/mediawiki/View/LexemeViewTest.php
index b1293e5..bae32d4 100644
--- a/tests/phpunit/mediawiki/View/LexemeViewTest.php
+++ b/tests/phpunit/mediawiki/View/LexemeViewTest.php
@@ -145,8 +145,8 @@
 
                $html = $view->getHtml( $lexeme );
                $this->assertInternalType( 'string', $html );
-               $this->assertContains( 'id="wb-lexeme-' . ( $lexeme->getId() ?: 
'new' ) . '"', $html );
-               $this->assertContains( 'class="wikibase-entityview wb-lexeme"', 
$html );
+               $this->assertContains( 'id="wb-' . Lexeme::ENTITY_TYPE . '-' . 
( $lexeme->getId() ?: 'new' ) . '"', $html );
+               $this->assertContains( 'class="wikibase-entityview wb-' . 
Lexeme::ENTITY_TYPE . '"', $html );
                $this->assertContains( 'statementSectionsView->getHtml', $html 
);
        }
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iac069f87ca8c85582d861e137b2ef7ac8cffa79f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseLexeme
Gerrit-Branch: master
Gerrit-Owner: Aleksey Bekh-Ivanov (WMDE) <aleksey.bekh-iva...@wikimedia.de>

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

Reply via email to