Skip to content

Commit

Permalink
Drop support for repo names in entity IDs
Browse files Browse the repository at this point in the history 
My understanding is that these were added years ago when we were still
not sure how to do federation with Structured Data on Commons, but in
the end we didn’t go down that path. Federated Properties have now been
implemented differently as well (see FederatedPropertyId), so at this
point I don’t think there’s any need to support entity IDs prefixed with
repository names anymore; let’s forcibly disable them for now, as fully
removing them from the code will take some more effort.

Bug: T291823
Bug: T338223
Change-Id: I46b7eff7cfe3b472b58e9a45f2f217dc70fafbe7
  • Loading branch information
@lucaswerkmeister @manicki
lucaswerkmeister authored and manicki committed Jun 28, 2023
1 parent 74b74da commit 36dd959
Show file tree
Hide file tree
Showing 20 changed files with 28 additions and 114 deletions.
4 changes: 2 additions & 2 deletions client/tests/phpunit/unit/includes/ServiceWiring/EntityIdComposerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public function testConstruction() {
$entityIdComposer = $this->getService( 'WikibaseClient.EntityIdComposer' );

$this->assertInstanceOf( EntityIdComposer::class, $entityIdComposer );
$this->assertEquals( new ItemId( 'repo:Q123' ),
$entityIdComposer->composeEntityId( 'repo', 'test', 123 ) );
$this->assertEquals( new ItemId( 'Q123' ),
$entityIdComposer->composeEntityId( '', 'test', 123 ) );
}

}
3 changes: 1 addition & 2 deletions client/tests/phpunit/unit/includes/Store/DescriptionLookupTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,7 @@ private function getIdLookup( array $centralDescriptions ) {
if ( !array_key_exists( $title->getArticleID(), $centralDescriptions ) ) {
return null;
}
return new ItemId( ItemId::joinSerialization(
[ 'central', '', 'Q' . $title->getArticleID() ] ) );
return new ItemId( 'Q' . $title->getArticleID() );
}, $titlesByPageId ) );
} );
return $idLookup;
Expand Down
1 change: 1 addition & 0 deletions lib/packages/wikibase/data-model/RELEASE-NOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Added `__serialize()` and `__unserialize()` methods to the `EntityId` interface.
* Added native type hints to the `Statement` and `StatementList` classes
* Added `strict_types=1` to `Statement.php`, `StatementList.php`, and related test files
* Removed support for repository names in entity IDs (e.g. `foo:Q1234`)

## Version 9.6.1 (2021-04-01)

Expand Down
3 changes: 3 additions & 0 deletions lib/packages/wikibase/data-model/src/Entity/ItemId.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ public static function newFromRepositoryAndNumber( $repositoryName, $numericId )
if ( !is_numeric( $numericId ) ) {
throw new InvalidArgumentException( '$numericId must be numeric' );
}
if ( $repositoryName !== '' ) {
throw new InvalidArgumentException( 'repo name no longer supported (T291823)' );
}

return new self( self::joinSerialization( [ $repositoryName, '', 'Q' . $numericId ] ) );
}
Expand Down
3 changes: 3 additions & 0 deletions lib/packages/wikibase/data-model/src/Entity/NumericPropertyId.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ public static function newFromRepositoryAndNumber( $repositoryName, $numericId )
if ( !is_numeric( $numericId ) ) {
throw new InvalidArgumentException( '$numericId must be numeric' );
}
if ( $repositoryName !== '' ) {
throw new InvalidArgumentException( 'repo name no longer supported (T291823)' );
}

return new self( self::joinSerialization( [ $repositoryName, '', 'P' . $numericId ] ) );
}
Expand Down
4 changes: 4 additions & 0 deletions lib/packages/wikibase/data-model/src/Entity/SerializableEntityId.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ private static function extractSerializationParts( $serialization ) {
$repoName = array_shift( $parts );
$prefixRemainder = implode( ':', $parts );

if ( $repoName !== null ) {
throw new InvalidArgumentException( 'repo name no longer supported (T291823)' );
}

return [
is_string( $repoName ) ? $repoName : '',
$prefixRemainder,
Expand Down
3 changes: 0 additions & 3 deletions lib/packages/wikibase/data-model/tests/unit/Entity/DispatchingEntityIdParserTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ public static function entityIdProvider() {
[ 'Q1337', new ItemId( 'Q1337' ) ],
[ 'p1', new NumericPropertyId( 'p1' ) ],
[ 'P100000', new NumericPropertyId( 'P100000' ) ],
[ 'foo:Q1337', new ItemId( 'foo:Q1337' ) ],
[ 'foo:P123', new NumericPropertyId( 'foo:P123' ) ],
[ 'foo:bar:Q1337', new ItemId( 'foo:bar:Q1337' ) ],
[ ':Q1337', new ItemId( ':Q1337' ) ],
];
}
Expand Down
11 changes: 0 additions & 11 deletions lib/packages/wikibase/data-model/tests/unit/Entity/EntityIdTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ public static function instanceProvider() {
$ids[] = [ new ItemId( 'Q42' ), '' ];
$ids[] = [ new ItemId( 'Q31337' ), '' ];
$ids[] = [ new ItemId( 'Q2147483647' ), '' ];
$ids[] = [ new ItemId( ':Q2147483647' ), '' ];
$ids[] = [ new ItemId( 'foo:Q2147483647' ), 'foo' ];
$ids[] = [ new NumericPropertyId( 'P101010' ), '' ];
$ids[] = [ new NumericPropertyId( 'foo:bar:P101010' ), 'foo' ];

return $ids;
}
Expand Down Expand Up @@ -107,9 +104,7 @@ public function testReturnTypeOfToString( EntityId $id ) {
public function testIsForeign() {
$this->assertFalse( ( new ItemId( 'Q42' ) )->isForeign() );
$this->assertFalse( ( new ItemId( ':Q42' ) )->isForeign() );
$this->assertTrue( ( new ItemId( 'foo:Q42' ) )->isForeign() );
$this->assertFalse( ( new NumericPropertyId( ':P42' ) )->isForeign() );
$this->assertTrue( ( new NumericPropertyId( 'foo:P42' ) )->isForeign() );
}

/**
Expand All @@ -122,9 +117,6 @@ public function testGetRepositoryName( EntityId $id, $repoName ) {
public static function serializationSplitProvider() {
return [
[ 'Q42', [ '', '', 'Q42' ] ],
[ 'foo:Q42', [ 'foo', '', 'Q42' ] ],
[ '0:Q42', [ '0', '', 'Q42' ] ],
[ 'foo:bar:baz:Q42', [ 'foo', 'bar:baz', 'Q42' ] ],
];
}

Expand Down Expand Up @@ -168,15 +160,12 @@ public static function invalidJoinSerializationDataProvider() {

public function testGivenNotNormalizedSerialization_splitSerializationReturnsNormalizedParts() {
$this->assertSame( [ '', '', 'Q42' ], SerializableEntityId::splitSerialization( ':Q42' ) );
$this->assertSame( [ 'foo', 'bar', 'Q42' ], SerializableEntityId::splitSerialization( ':foo:bar:Q42' ) );
}

public static function localPartDataProvider() {
return [
[ 'Q42', 'Q42' ],
[ ':Q42', 'Q42' ],
[ 'foo:Q42', 'Q42' ],
[ 'foo:bar:Q42', 'bar:Q42' ],
];
}

Expand Down
15 changes: 0 additions & 15 deletions lib/packages/wikibase/data-model/tests/unit/Entity/EntityIdValueTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public static function instanceProvider() {
new NumericPropertyId( 'P1' ),
new NumericPropertyId( 'P31337' ),
new CustomEntityId( 'X567' ),
new NumericPropertyId( 'foo:P678' ),
];

$argLists = [];
Expand Down Expand Up @@ -104,14 +103,6 @@ public static function provideGetArrayValue() {
'id' => 'X567',
],
],
'foo:P678' => [
new NumericPropertyId( 'foo:P678' ),
[
'entity-type' => 'property',
'numeric-id' => 678,
'id' => 'foo:P678',
],
],
];
}

Expand All @@ -137,7 +128,6 @@ public function testSerialize() {

public static function provideDeserializationCompatibility() {
$local = new EntityIdValue( new ItemId( 'Q31337' ) );
$foreign = new EntityIdValue( new NumericPropertyId( 'foo:P678' ) );
$custom = new EntityIdValue( new CustomEntityId( 'X567' ) );

return [
Expand All @@ -150,11 +140,6 @@ public static function provideDeserializationCompatibility() {
. '50:{C:32:"Wikibase\DataModel\Entity\ItemId":6:{Q31337}}',
$local,
],
'foreign: Version 7.0 (7fcddfc)' => [
'C:39:"Wikibase\DataModel\Entity\EntityIdValue":'
. '63:{C:43:"Wikibase\DataModel\Entity\NumericPropertyId":8:{foo:P678}}',
$foreign,
],
'custom: Version 7.0 (7fcddfc): custom' => [
'C:39:"Wikibase\DataModel\Entity\EntityIdValue":'
. '58:{C:42:"Wikibase\DataModel\Fixtures\CustomEntityId":4:{X567}}',
Expand Down
14 changes: 5 additions & 9 deletions lib/packages/wikibase/data-model/tests/unit/Entity/ItemIdTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ public static function idSerializationProvider() {
[ 'Q31337', 'Q31337' ],
[ 'Q42', 'Q42' ],
[ ':Q42', 'Q42' ],
[ 'foo:Q42', 'foo:Q42' ],
[ 'foo:bar:q42', 'foo:bar:Q42' ],
[ 'Q2147483647', 'Q2147483647' ],
];
}
Expand Down Expand Up @@ -70,6 +68,9 @@ public static function invalidIdSerializationProvider() {
[ 1 ],
[ 'Q2147483648' ],
[ 'Q99999999999' ],
// no longer supported (T291823, T338223)
[ 'foo:Q42', 'foo:Q42' ],
[ 'foo:bar:q42', 'foo:bar:Q42' ],
];
}

Expand All @@ -78,11 +79,6 @@ public function testGetNumericId() {
$this->assertSame( 1, $id->getNumericId() );
}

public function testGetNumericId_foreignId() {
$id = new ItemId( 'foo:Q1' );
$this->assertSame( 1, $id->getNumericId() );
}

public function testGetEntityType() {
$id = new ItemId( 'Q1' );
$this->assertSame( 'item', $id->getEntityType() );
Expand Down Expand Up @@ -155,8 +151,8 @@ public static function invalidNumericIdProvider() {
}

public function testNewFromRepositoryAndNumber() {
$id = ItemId::newFromRepositoryAndNumber( 'foo', 1 );
$this->assertSame( 'foo:Q1', $id->getSerialization() );
$id = ItemId::newFromRepositoryAndNumber( '', 1 );
$this->assertSame( 'Q1', $id->getSerialization() );
}

public function testNewFromRepositoryAndNumberWithInvalidNumericId() {
Expand Down
14 changes: 5 additions & 9 deletions lib/packages/wikibase/data-model/tests/unit/Entity/NumericPropertyIdTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ public static function idSerializationProvider() {
[ 'P31337', 'P31337' ],
[ 'P42', 'P42' ],
[ ':P42', 'P42' ],
[ 'foo:P42', 'foo:P42' ],
[ 'foo:bar:p42', 'foo:bar:P42' ],
[ 'P2147483647', 'P2147483647' ],
];
}
Expand Down Expand Up @@ -69,6 +67,9 @@ public static function invalidIdSerializationProvider() {
[ 1 ],
[ 'P2147483648' ],
[ 'P99999999999' ],
// no longer supported (T291823, T338223)
[ 'foo:P42', 'foo:P42' ],
[ 'foo:bar:p42', 'foo:bar:P42' ],
];
}

Expand All @@ -77,11 +78,6 @@ public function testGetNumericId() {
$this->assertSame( 1, $id->getNumericId() );
}

public function testGetNumericId_foreignId() {
$id = new NumericPropertyId( 'foo:P1' );
$this->assertSame( 1, $id->getNumericId() );
}

public function testGetEntityType() {
$id = new NumericPropertyId( 'P1' );
$this->assertSame( 'property', $id->getEntityType() );
Expand Down Expand Up @@ -154,8 +150,8 @@ public static function invalidNumericIdProvider() {
}

public function testNewFromRepositoryAndNumber() {
$id = NumericPropertyId::newFromRepositoryAndNumber( 'foo', 1 );
$this->assertSame( 'foo:P1', $id->getSerialization() );
$id = NumericPropertyId::newFromRepositoryAndNumber( '', 1 );
$this->assertSame( 'P1', $id->getSerialization() );
}

public function testNewFromRepositoryAndNumberWithInvalidNumericId() {
Expand Down
7 changes: 0 additions & 7 deletions lib/packages/wikibase/data-model/tests/unit/Snak/PropertyNoValueSnakTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,12 @@ public static function notEqualsProvider() {

public static function provideDataToSerialize() {
$p2 = new NumericPropertyId( 'P2' );
$p2foo = new NumericPropertyId( 'foo:P2' );

return [
'string' => [
'P2',
new PropertyNoValueSnak( $p2 ),
],
'foreign' => [
'foo:P2',
new PropertyNoValueSnak( $p2foo ),
],
];
}

Expand All @@ -139,12 +134,10 @@ public function testSerialize( $expected, Snak $snak ) {

public static function provideDataToUnserialize() {
$p2 = new NumericPropertyId( 'P2' );
$p2foo = new NumericPropertyId( 'foo:P2' );

return [
'legacy' => [ new PropertyNoValueSnak( $p2 ), 'i:2;' ],
'current' => [ new PropertyNoValueSnak( $p2 ), 'P2' ],
'foreign' => [ new PropertyNoValueSnak( $p2foo ), 'foo:P2' ],
];
}

Expand Down
7 changes: 0 additions & 7 deletions lib/packages/wikibase/data-model/tests/unit/Snak/PropertySomeValueSnakTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,12 @@ public static function notEqualsProvider() {

public static function provideDataToSerialize() {
$p2 = new NumericPropertyId( 'P2' );
$p2foo = new NumericPropertyId( 'foo:P2' );

return [
'string' => [
'P2',
new PropertySomeValueSnak( $p2 ),
],
'foreign' => [
'foo:P2',
new PropertySomeValueSnak( $p2foo ),
],
];
}

Expand All @@ -139,12 +134,10 @@ public function testSerialize( $expected, Snak $snak ) {

public static function provideDataToUnserialize() {
$p2 = new NumericPropertyId( 'P2' );
$p2foo = new NumericPropertyId( 'foo:P2' );

return [
'legacy' => [ new PropertySomeValueSnak( $p2 ), 'i:2;' ],
'current' => [ new PropertySomeValueSnak( $p2 ), 'P2' ],
'foreign' => [ new PropertySomeValueSnak( $p2foo ), 'foo:P2' ],
];
}

Expand Down
18 changes: 2 additions & 16 deletions lib/packages/wikibase/data-model/tests/unit/Snak/PropertyValueSnakTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,18 +122,13 @@ public static function notEqualsProvider() {

public static function provideDataToSerialize() {
$p2 = new NumericPropertyId( 'P2' );
$p2foo = new NumericPropertyId( 'foo:P2' );
$value = new StringValue( 'b' );

return [
'string' => [
'a:2:{i:0;s:2:"P2";i:1;O:22:"DataValues\StringValue":1:{i:0;s:1:"b";}}',
new PropertyValueSnak( $p2, $value ),
],
'foreign' => [
'a:2:{i:0;s:6:"foo:P2";i:1;O:22:"DataValues\StringValue":1:{i:0;s:1:"b";}}',
new PropertyValueSnak( $p2foo, $value ),
],
];
}

Expand All @@ -151,30 +146,21 @@ public function testSerialize( $expected, Snak $snak ) {

public static function provideDataToUnserialize() {
$p2 = new NumericPropertyId( 'P2' );
$p2foo = new NumericPropertyId( 'foo:P2' );
$value = new StringValue( 'b' );

return [
'legacy (int property ID)' => [
new PropertyValueSnak( $p2, $value ),
'a:2:{i:0;i:2;i:1;C:22:"DataValues\StringValue":1:{b}}',
],
'legacy (PHP < 7.4 serialization, local property ID)' => [
'legacy (PHP < 7.4 serialization)' => [
new PropertyValueSnak( $p2, $value ),
'a:2:{i:0;s:2:"P2";i:1;C:22:"DataValues\StringValue":1:{b}}',
],
'legacy (PHP < 7.4 serialization, foreign property ID)' => [
new PropertyValueSnak( $p2foo, $value ),
'a:2:{i:0;s:6:"foo:P2";i:1;C:22:"DataValues\StringValue":1:{b}}',
],
'local property ID' => [
'PHP >= 7.4 serialization' => [
new PropertyValueSnak( $p2, $value ),
'a:2:{i:0;s:2:"P2";i:1;O:22:"DataValues\StringValue":1:{i:0;s:1:"b";}}',
],
'foreign property ID' => [
new PropertyValueSnak( $p2foo, $value ),
'a:2:{i:0;s:6:"foo:P2";i:1;O:22:"DataValues\StringValue":1:{i:0;s:1:"b";}}',
],
];
}

Expand Down
8 changes: 0 additions & 8 deletions lib/tests/phpunit/Interactors/TermSearchResultTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@ public static function provideGoodConstruction() {
null,
[ 'datatype' => 'some datatype' ],
],
[
new Term( 'en-gb', 'FooText' ),
'description',
new NumericPropertyId( 'foo:P777' ),
null,
null,
[ 'datatype' => 'some datatype' ],
],
];
}

Expand Down
1 change: 0 additions & 1 deletion lib/tests/phpunit/Store/TermCacheKeyBuilderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public function testBuildCacheKey( $entity, $revision, $language, $termType, $ex
public static function cacheKeyParamsProvider() {
yield [ new ItemId( 'Q123' ), 777, 'en', 'label', 'Q123_777_en_label' ];
yield [ new NumericPropertyId( 'P666' ), 789, 'de', 'alias', 'P666_789_de_alias' ];
yield [ new NumericPropertyId( 'reponame:P666' ), 789, 'de', 'alias', 'reponame_P666_789_de_alias' ];
yield [ new NumericPropertyId( 'P666' ), 789, 'de()', 'alias', 'P666_789_de___alias' ];
yield [ new NumericPropertyId( 'P666' ), 789, 'de{}', 'alias', 'P666_789_de___alias' ];
yield [ new NumericPropertyId( 'P666' ), 789, 'de\/', 'alias', 'P666_789_de___alias' ];
Expand Down
Loading

0 comments on commit 36dd959

Please sign in to comment.