[DNM] Optimize EntityId::getRepositoryName and ::getLocalPart

wmde · Nov 2, 2017 · 81f4186 · 81f4186 · brightbyte · Nov 3, 2017
1 parent b54117c
commit 81f4186
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 9 deletions.
diff --git a/src/Entity/EntityId.php b/src/Entity/EntityId.php
@@ -16,6 +16,16 @@ abstract class EntityId implements Comparable, Serializable {
 
 	protected $serialization;
 
+	/**
+	 * @var string
+	 */
+	protected $repositoryName;
+
+	/**
+	 * @var string
+	 */
+	protected $localPart;
+
 	const PATTERN = '/^:?(\w+:)*[^:]+\z/';
 
 	/**
@@ -26,6 +36,8 @@ abstract class EntityId implements Comparable, Serializable {
 	public function __construct( $serialization ) {
 		self::assertValidSerialization( $serialization );
 		$this->serialization = self::normalizeIdSerialization( $serialization );
+
+		list ( $this->repositoryName, $this->localPart ) = $this->getRepositoryNameAndLocalPart( $serialization );
 	}
 
 	private static function assertValidSerialization( $serialization ) {
@@ -68,7 +80,11 @@ public function getSerialization() {
 	public static function splitSerialization( $serialization ) {
 		self::assertValidSerialization( $serialization );
 
-		$parts = explode( ':', self::normalizeIdSerialization( $serialization ) );
+		return self::getSerializationParts( self::normalizeIdSerialization( $serialization ) );
+	}
+
+	private static function getSerializationParts( $serialization ) {
+		$parts = explode( ':', $serialization );
 		$localPart = array_pop( $parts );
 		$repoName = array_shift( $parts );
 		$prefixRemainder = implode( ':', $parts );
@@ -112,9 +128,7 @@ public static function joinSerialization( array $parts ) {
 	 * @return string
 	 */
 	public function getRepositoryName() {
-		$parts = self::splitSerialization( $this->serialization );
-
-		return $parts[0];
+		return $this->repositoryName;
 	}
 
 	/**
@@ -125,9 +139,7 @@ public function getRepositoryName() {
 	 * @return string
 	 */
 	public function getLocalPart() {
-		$parts = self::splitSerialization( $this->serialization );
-
-		return self::joinSerialization( [ '', $parts[1], $parts[2] ] );
+		return $this->localPart;
 	}
 
 	/**
@@ -179,4 +191,10 @@ public function equals( $target ) {
 			&& $target->serialization === $this->serialization;
 	}
 
+	protected function getRepositoryNameAndLocalPart( $serialization ) {
+		list( $repoName, $prefixRemainder, $localId ) = self::getSerializationParts( $serialization );
+		$localPart = self::joinSerialization( [ '', $prefixRemainder, $localId ] );
+		return [ $repoName, $localPart ];
+	}
+
 }
diff --git a/src/Entity/ItemId.php b/src/Entity/ItemId.php
@@ -83,6 +83,7 @@ public function serialize() {
 	public function unserialize( $serialized ) {
 		$array = json_decode( $serialized );
 		$this->serialization = is_array( $array ) ? $array[1] : $serialized;
+		list ( $this->repositoryName, $this->localPart ) = $this->getRepositoryNameAndLocalPart( $this->serialization );
 	}
 
 	/**

diff --git a/src/Entity/PropertyId.php b/src/Entity/PropertyId.php
@@ -83,6 +83,7 @@ public function serialize() {
 	public function unserialize( $serialized ) {
 		$array = json_decode( $serialized );
 		$this->serialization = is_array( $array ) ? $array[1] : $serialized;
+		list ( $this->repositoryName, $this->localPart ) = $this->getRepositoryNameAndLocalPart( $this->serialization );
 	}
 
 	/**

diff --git a/tests/fixtures/CustomEntityId.php b/tests/fixtures/CustomEntityId.php
@@ -28,6 +28,8 @@ public function serialize() {
 	 */
 	public function unserialize( $serialized ) {
 		$this->serialization = $serialized;
+		$this->repositoryName = '';
+		$this->localPart = $serialized;
 	}
 
 	/**

diff --git a/tests/unit/Entity/ItemIdTest.php b/tests/unit/Entity/ItemIdTest.php
@@ -112,8 +112,22 @@ public function serializationProvider() {
 			// All these cases are kind of an injection vector and allow constructing invalid ids.
 			[ '["string","Q2"]', 'Q2' ],
 			[ '["","string"]', 'string' ],
-			[ '["",""]', '' ],
 			[ '["",2]', 2 ],
+		];
+	}
+
+	/**
+	 * @dataProvider invalidSerializationProvider
+	 */
+	public function testGivenInvalidSerialization_unserializeThrowsException( $serialization ) {
+		$id = new ItemId( 'Q1' );
+		$this->setExpectedException( InvalidArgumentException::class );
+		$id->unserialize( $serialization );
+	}
+
+	public function invalidSerializationProvider() {
+		return [
+			[ '["",""]', '' ],
 			[ '["",null]', null ],
 			[ '', '' ],
 		];

diff --git a/tests/unit/Entity/PropertyIdTest.php b/tests/unit/Entity/PropertyIdTest.php
@@ -112,8 +112,22 @@ public function serializationProvider() {
 			// All these cases are kind of an injection vector and allow constructing invalid ids.
 			[ '["string","P2"]', 'P2' ],
 			[ '["","string"]', 'string' ],
-			[ '["",""]', '' ],
 			[ '["",2]', 2 ],
+		];
+	}
+
+	/**
+	 * @dataProvider invalidSerializationProvider
+	 */
+	public function testGivenInvalidSerialization_unserializeThrowsException( $serialization ) {
+		$id = new PropertyId( 'P1' );
+		$this->setExpectedException( InvalidArgumentException::class );
+		$id->unserialize( $serialization );
+	}
+
+	public function invalidSerializationProvider() {
+		return [
+			[ '["",""]', '' ],
 			[ '["",null]', null ],
 			[ '', '' ],
 		];