diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 2f4de3fa..f7d44126 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,5 +1,11 @@ # Wikibase DataModel release notes +## Version 7.2.0 (2017-10-23) + +* Performance optimizations on methods critical for dump generation: + * `DispatchingEntityIdParser::parse` + * `SnakList::orderByProperty` + ## Version 7.1.0 (2017-09-01) * Changed `EntityIdValue::getArrayValue` to allow it handle foreign entity IDs and entity IDs that diff --git a/WikibaseDataModel.php b/WikibaseDataModel.php index 55db3834..3f25b06e 100644 --- a/WikibaseDataModel.php +++ b/WikibaseDataModel.php @@ -11,4 +11,4 @@ return; } -define( 'WIKIBASE_DATAMODEL_VERSION', '7.1.0' ); +define( 'WIKIBASE_DATAMODEL_VERSION', '7.2.0' ); diff --git a/composer.json b/composer.json index 93cb6855..1cc27d85 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,7 @@ }, "extra": { "branch-alias": { - "dev-master": "7.1.x-dev" + "dev-master": "7.2.x-dev" } }, "scripts": { diff --git a/src/Entity/DispatchingEntityIdParser.php b/src/Entity/DispatchingEntityIdParser.php index 06b25d2d..8e061ebb 100644 --- a/src/Entity/DispatchingEntityIdParser.php +++ b/src/Entity/DispatchingEntityIdParser.php @@ -37,20 +37,19 @@ public function __construct( array $idBuilders ) { * @return EntityId */ public function parse( $idSerialization ) { - $this->assertIdIsString( $idSerialization ); - - if ( empty( $this->idBuilders ) ) { + if ( $this->idBuilders === [] ) { throw new EntityIdParsingException( 'No id builders are configured' ); } + try { + list( , , $localId ) = EntityId::splitSerialization( $idSerialization ); + } catch ( InvalidArgumentException $ex ) { + // EntityId::splitSerialization performs some sanity checks which + // might result in an exception. Should this happen, re-throw the exception message + throw new EntityIdParsingException( $ex->getMessage(), 0, $ex ); + } + foreach ( $this->idBuilders as $idPattern => $idBuilder ) { - try { - list( , , $localId ) = EntityId::splitSerialization( $idSerialization ); - } catch ( InvalidArgumentException $ex ) { - // EntityId::splitSerialization performs some sanity checks which - // might result in an exception. Should this happen, re-throw the exception message - throw new EntityIdParsingException( $ex->getMessage(), 0, $ex ); - } if ( preg_match( $idPattern, $localId ) ) { return $this->buildId( $idBuilder, $idSerialization ); } @@ -61,21 +60,6 @@ public function parse( $idSerialization ) { ); } - /** - * @param string $idSerialization - * - * @throws EntityIdParsingException - */ - private function assertIdIsString( $idSerialization ) { - if ( !is_string( $idSerialization ) ) { - throw new EntityIdParsingException( - '$idSerialization must be a string, got ' . ( is_object( $idSerialization ) - ? get_class( $idSerialization ) - : getType( $idSerialization ) ) - ); - } - } - /** * @param callable $idBuilder * @param string $idSerialization diff --git a/src/Snak/SnakList.php b/src/Snak/SnakList.php index e1eb0dae..29960487 100644 --- a/src/Snak/SnakList.php +++ b/src/Snak/SnakList.php @@ -144,53 +144,33 @@ public function getHash() { } /** - * Orders the snaks in the list grouping them by property. + * Groups snaks by property, and optionally orders them. * - * @param string[] $order List of serliazed property ids to order by. + * @param string[] $order List of property ID strings to order by. Snaks with other properties + * will also be grouped, but put at the end, in the order each property appeared first in the + * original list. * * @since 0.5 */ public function orderByProperty( array $order = [] ) { - $snaksByProperty = $this->getSnaksByProperty(); - $orderedProperties = array_unique( array_merge( $order, array_keys( $snaksByProperty ) ) ); - - foreach ( $orderedProperties as $property ) { - if ( array_key_exists( $property, $snaksByProperty ) ) { - $snaks = $snaksByProperty[$property]; - $this->moveSnaksToBottom( $snaks ); - } + $byProperty = array_fill_keys( $order, [] ); + + /** @var Snak $snak */ + foreach ( $this as $snak ) { + $byProperty[$snak->getPropertyId()->getSerialization()][] = $snak; } - } - /** - * @param Snak[] $snaks to remove and re add - */ - private function moveSnaksToBottom( array $snaks ) { - foreach ( $snaks as $snak ) { - $this->removeSnak( $snak ); - $this->addSnak( $snak ); + $ordered = []; + foreach ( $byProperty as $snaks ) { + $ordered = array_merge( $ordered, $snaks ); } - } - /** - * Gets the snaks in the current object in an array - * grouped by property id - * - * @return array[] - */ - private function getSnaksByProperty() { - $snaksByProperty = []; + $this->exchangeArray( $ordered ); - foreach ( $this as $snak ) { - /** @var Snak $snak */ - $propertyId = $snak->getPropertyId()->getSerialization(); - if ( !isset( $snaksByProperty[$propertyId] ) ) { - $snaksByProperty[$propertyId] = []; - } - $snaksByProperty[$propertyId][] = $snak; + $index = 0; + foreach ( $ordered as $snak ) { + $this->offsetHashes[$snak->getHash()] = $index++; } - - return $snaksByProperty; } } diff --git a/tests/unit/Snak/SnakListTest.php b/tests/unit/Snak/SnakListTest.php index 04a9b745..a4e7f1cd 100644 --- a/tests/unit/Snak/SnakListTest.php +++ b/tests/unit/Snak/SnakListTest.php @@ -175,6 +175,7 @@ public function orderByPropertyProvider() { $id1 = new PropertyId( 'P1' ); $id2 = new PropertyId( 'P2' ); $id3 = new PropertyId( 'P3' ); + $id4 = new PropertyId( 'P4' ); /** * List of test data containing snaks to initialize SnakList objects. The first list of @@ -248,6 +249,27 @@ public function orderByPropertyProvider() { ], [ 'P1' ] ], + 'Multiple IDs in order' => [ + [ + new PropertyValueSnak( $id1, new StringValue( 'a' ) ), + new PropertyValueSnak( $id2, new StringValue( 'b' ) ), + new PropertyValueSnak( $id1, new StringValue( 'c' ) ), + new PropertyValueSnak( $id3, new StringValue( 'd' ) ), + new PropertyValueSnak( $id4, new StringValue( 'e' ) ), + new PropertyValueSnak( $id2, new StringValue( 'f' ) ), + new PropertyValueSnak( $id4, new StringValue( 'g' ) ), + ], + [ + new PropertyValueSnak( $id2, new StringValue( 'b' ) ), + new PropertyValueSnak( $id2, new StringValue( 'f' ) ), + new PropertyValueSnak( $id3, new StringValue( 'd' ) ), + new PropertyValueSnak( $id1, new StringValue( 'a' ) ), + new PropertyValueSnak( $id1, new StringValue( 'c' ) ), + new PropertyValueSnak( $id4, new StringValue( 'e' ) ), + new PropertyValueSnak( $id4, new StringValue( 'g' ) ), + ], + [ 'P2', 'P3', 'P1' ] + ], ]; $arguments = []; @@ -282,6 +304,16 @@ public function testOrderByProperty( SnakList $snakList, SnakList $expected, arr } else { $this->assertNotSame( $initialSnakList->getHash(), $snakList->getHash() ); } + + /** @var Snak $snak */ + foreach ( $snakList as $snak ) { + $hash = $snak->getHash(); + $this->assertSame( + $hash, + $snakList->getSnak( $hash )->getHash(), + 'Reordering must not mess up the lists internal state' + ); + } } public function testComparableInterface() {