Skip to content

Commit

Permalink
Changed how "forgeign repository name" is stored in entity ids.
Browse files Browse the repository at this point in the history 
Follow up to #678

This removes the addition of static code to EntityId and pushes
the responsibility of parsing serializations out of the class.

I did not touch various other issues and have created a number of
small ones, so this leaves the code in a state, that while it works,
still needs some work before it is mergeable. That includes making
names consistent again and adding some edge case tests.

The first constructor argument contains what the existing getter
has named the "local part", which can also include prefixes defined
on other sites. It's not immedidately clear to me why we need this
here, so I'd be nice if someone could explain that. Should this
not already have been resolved by the time the id is constructed?
  • Loading branch information
@JeroenDeDauw
JeroenDeDauw committed Sep 22, 2016
1 parent 41e8f75 commit 874a834
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 203 deletions.
105 changes: 13 additions & 92 deletions src/Entity/EntityId.php
View file
Expand Up @@ -3,7 +3,6 @@
namespace Wikibase\DataModel\Entity;

use Comparable;
use InvalidArgumentException;
use Serializable;

/**
Expand All @@ -17,30 +16,7 @@
abstract class EntityId implements Comparable, Serializable {

protected $serialization;

const PATTERN = '/^:?(\w+:)*[^:]+\z/';

/**
* @param string $serialization
*/
public function __construct( $serialization ) {
self::assertValidSerialization( $serialization );
$this->serialization = self::normalizeIdSerialization( $serialization );
}

private static function assertValidSerialization( $serialization ) {
if ( !is_string( $serialization ) ) {
throw new InvalidArgumentException( '$serialization must be a string' );
}

if ( $serialization === '' ) {
throw new InvalidArgumentException( '$serialization must not be an empty string' );
}

if ( !preg_match( self::PATTERN, $serialization ) ) {
throw new InvalidArgumentException( '$serialization must match ' . self::PATTERN );
}
}
protected $repositoryName;

/**
* @return string
Expand All @@ -51,52 +27,20 @@ public abstract function getEntityType();
* @return string
*/
public function getSerialization() {
return $this->serialization;
}

/**
* Returns an array with 3 elements: the foreign repository name as the first element, the local ID as the last
* element and everything that is in between as the second element.
*
* EntityId::joinSerialization can be used to restore the original serialization from the parts returned.
*
* @param string $serialization
* @return string[] Array containing the serialization split into 3 parts.
*/
public static function splitSerialization( $serialization ) {
self::assertValidSerialization( $serialization );

$parts = explode( ':', self::normalizeIdSerialization( $serialization ) );
$localPart = array_pop( $parts );
$repoName = array_shift( $parts );
$prefixRemainder = implode( ':', $parts );
if ( $this->isForeign() ) {
return $this->repositoryName . ':' . $this->serialization;
}

return array(
is_string( $repoName ) ? $repoName : '',
$prefixRemainder,
$localPart
);
return $this->serialization;
}

/**
* Builds an ID serialization from the parts returned by EntityId::splitSerialization.
* Returns the serialization without the first repository prefix.
*
* @param string[] $parts
* @return string
*
* @throws InvalidArgumentException
*/
public static function joinSerialization( array $parts ) {
if ( end( $parts ) === '' ) {
throw new InvalidArgumentException( 'The last element of $parts must not be empty.' );
}

return implode(
':',
array_filter( $parts, function( $part ) {
return $part !== '';
} )
);
public function getLocalPart() {
return $this->serialization;
}

/**
Expand All @@ -106,38 +50,14 @@ public static function joinSerialization( array $parts ) {
* @return string
*/
public function getRepoName() {
$parts = self::splitSerialization( $this->serialization );

return $parts[0];
}

/**
* Returns the serialization without the first repository prefix.
*
* @return string
*/
public function getLocalPart() {
$parts = self::splitSerialization( $this->serialization );

return self::joinSerialization( array( '', $parts[1], $parts[2] ) );
return $this->repositoryName;
}

/**
* Returns true iff EntityId::getRepoName returns a non-empty string.
*
* @return bool
*/
public function isForeign() {
// not actually using EntityId::getRepoName for performance reasons
return strpos( $this->serialization, ':' ) > 0;
}

/**
* @param string $id
* @return string
*/
private static function normalizeIdSerialization( $id ) {
return ltrim( $id, ':' );
return $this->repositoryName !== '';
}

/**
Expand All @@ -148,7 +68,7 @@ private static function normalizeIdSerialization( $id ) {
* @return string
*/
public function __toString() {
return $this->serialization;
return $this->getSerialization();
}

/**
Expand All @@ -166,7 +86,8 @@ public function equals( $target ) {
}

return $target instanceof self
&& $target->serialization === $this->serialization;
&& $target->serialization === $this->serialization
&& $target->repositoryName === $this->repositoryName;
}

}
38 changes: 23 additions & 15 deletions src/Entity/ItemId.php
View file
Expand Up @@ -20,29 +20,31 @@ class ItemId extends EntityId implements Int32EntityId {

/**
* @param string $idSerialization
* @param string $repositoryName
*
* @throws InvalidArgumentException
*/
public function __construct( $idSerialization ) {
$serializationParts = self::splitSerialization( $idSerialization );
$localId = strtoupper( $serializationParts[2] );
$this->assertValidIdFormat( $localId );
parent::__construct( self::joinSerialization(
array( $serializationParts[0], $serializationParts[1], $localId ) )
);
}

private function assertValidIdFormat( $idSerialization ) {
public function __construct( $idSerialization, $repositoryName = '' ) {
if ( !is_string( $idSerialization ) ) {
throw new InvalidArgumentException( '$idSerialization must be a string' );
}

if ( !preg_match( self::PATTERN, $idSerialization ) ) {
$parts = explode( ':', $idSerialization );
$localId = end( $parts );
$this->assertValidIdFormat( $localId );
$parts[count( $parts ) - 1] = strtoupper( $parts[count( $parts ) - 1] );

$this->serialization = implode( ':', $parts );
$this->repositoryName = $repositoryName;
}

private function assertValidIdFormat( $localId ) {
if ( !preg_match( self::PATTERN, $localId ) ) {
throw new InvalidArgumentException( '$idSerialization must match ' . self::PATTERN );
}

if ( strlen( $idSerialization ) > 10
&& substr( $idSerialization, 1 ) > Int32EntityId::MAX
if ( strlen( $localId ) > 10
&& substr( $localId, 1 ) > Int32EntityId::MAX
) {
throw new InvalidArgumentException( '$idSerialization can not exceed '
. Int32EntityId::MAX );
Expand Down Expand Up @@ -75,7 +77,13 @@ public function getEntityType() {
* @return string
*/
public function serialize() {
return json_encode( array( 'item', $this->getSerialization() ) );
$data = [ 'item', $this->serialization ];

if ( $this->isForeign() ) {
$data[] = $this->repositoryName;
}

return json_encode( $data );
}

/**
Expand All @@ -84,7 +92,7 @@ public function serialize() {
* @param string $serialized
*/
public function unserialize( $serialized ) {
list( , $this->serialization ) = json_decode( $serialized );
list( , $this->serialization, $this->repositoryName ) = array_merge( json_decode( $serialized ), [ '' ] );
}

/**
Expand Down
38 changes: 23 additions & 15 deletions src/Entity/PropertyId.php
View file
Expand Up @@ -20,29 +20,31 @@ class PropertyId extends EntityId implements Int32EntityId {

/**
* @param string $idSerialization
* @param string $repositoryName
*
* @throws InvalidArgumentException
*/
public function __construct( $idSerialization ) {
$serializationParts = self::splitSerialization( $idSerialization );
$localId = strtoupper( $serializationParts[2] );
$this->assertValidIdFormat( $localId );
parent::__construct( self::joinSerialization(
array( $serializationParts[0], $serializationParts[1], $localId ) )
);
}

private function assertValidIdFormat( $idSerialization ) {
public function __construct( $idSerialization, $repositoryName = '' ) {
if ( !is_string( $idSerialization ) ) {
throw new InvalidArgumentException( '$idSerialization must be a string' );
}

if ( !preg_match( self::PATTERN, $idSerialization ) ) {
$parts = explode( ':', $idSerialization );
$localId = end( $parts );
$this->assertValidIdFormat( $localId );
$parts[count( $parts ) - 1] = strtoupper( $parts[count( $parts ) - 1] );

$this->serialization = implode( ':', $parts );
$this->repositoryName = $repositoryName;
}

private function assertValidIdFormat( $localId ) {
if ( !preg_match( self::PATTERN, $localId ) ) {
throw new InvalidArgumentException( '$idSerialization must match ' . self::PATTERN );
}

if ( strlen( $idSerialization ) > 10
&& substr( $idSerialization, 1 ) > Int32EntityId::MAX
if ( strlen( $localId ) > 10
&& substr( $localId, 1 ) > Int32EntityId::MAX
) {
throw new InvalidArgumentException( '$idSerialization can not exceed '
. Int32EntityId::MAX );
Expand Down Expand Up @@ -75,7 +77,13 @@ public function getEntityType() {
* @return string
*/
public function serialize() {
return json_encode( array( 'property', $this->getSerialization() ) );
$data = [ 'property', $this->serialization ];

if ( $this->isForeign() ) {
$data[] = $this->repositoryName;
}

return json_encode( $data );
}

/**
Expand All @@ -84,7 +92,7 @@ public function serialize() {
* @param string $serialized
*/
public function unserialize( $serialized ) {
list( , $this->serialization ) = json_decode( $serialized );
list( , $this->serialization, $this->repositoryName ) = array_merge( json_decode( $serialized ), [ '' ] );
}

/**
Expand Down

0 comments on commit 874a834

Please sign in to comment.