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