Thiemo Mättig (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/339154 )
Change subject: [WIP] Introduce dedicated fields in EntityChange ...................................................................... [WIP] Introduce dedicated fields in EntityChange At the moment this is an unfinished code experiment. I will split this patch into smaller ones. Change-Id: I6fc8955034662be78f9d7d7d5cecfc8dac15534a --- M lib/includes/Changes/ChangeRow.php M lib/includes/Changes/EntityChange.php M lib/includes/Changes/EntityChangeFactory.php M repo/includes/Store/Sql/SqlChangeStore.php 4 files changed, 67 insertions(+), 49 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/54/339154/1 diff --git a/lib/includes/Changes/ChangeRow.php b/lib/includes/Changes/ChangeRow.php index d085edf..a31d827 100644 --- a/lib/includes/Changes/ChangeRow.php +++ b/lib/includes/Changes/ChangeRow.php @@ -40,6 +40,13 @@ } /** + * @return string + */ + public function getType() { + throw new \LogicException( 'This should be abstract' ); + } + + /** * @see Change::getAge * * @return integer @@ -63,7 +70,7 @@ * @return string */ public function getObjectId() { - return $this->getField( 'object_id' ); + throw new \LogicException( 'This should be abstract' ); } /** diff --git a/lib/includes/Changes/EntityChange.php b/lib/includes/Changes/EntityChange.php index 5b2cb7f..cc5aa52 100644 --- a/lib/includes/Changes/EntityChange.php +++ b/lib/includes/Changes/EntityChange.php @@ -13,7 +13,6 @@ use User; use Wikibase\Client\WikibaseClient; use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Entity\BasicEntityIdParser; use Wikibase\DataModel\Services\Diff\EntityTypeAwareDiffOpFactory; use Wikibase\DataModel\Statement\Statement; use Wikibase\Repo\WikibaseRepo; @@ -39,54 +38,73 @@ private $entityId = null; /** - * @todo FIXME use uppecase ID, like everywhere else! - * + * @var string|null One of the self::… constants. + */ + private $action = null; + + /** * @param string $name * @param mixed $value * * @throws MWException */ public function setField( $name, $value ) { - if ( $name === 'object_id' && is_string( $value ) ) { - //NOTE: for compatibility with earlier versions, use lower case IDs in the database. - $value = strtolower( $value ); + if ( $name === 'object_id' || $name === 'type' ) { + throw new RuntimeException( 'Use setEntityId and setAction!' ); } parent::setField( $name, $value ); } /** - * @return EntityId + * @return EntityId|null */ public function getEntityId() { - if ( !$this->entityId && $this->hasField( 'object_id' ) ) { - // FIXME: this should not happen - wfWarn( "object_id set in EntityChange, but not entityId" ); - $idParser = new BasicEntityIdParser(); - $this->entityId = $idParser->parse( $this->getObjectId() ); - } - return $this->entityId; } - /** - * Set the Change's entity id (as returned by getEntityId) and the object_id field - * @param EntityId $entityId - */ public function setEntityId( EntityId $entityId ) { $this->entityId = $entityId; - $this->setField( 'object_id', $entityId->getSerialization() ); } /** - * @return string + * @return string|null */ public function getAction() { - // FIXME: This encodes knowledge from EntityChangeFactory::newForEntity. - $type = $this->getField( 'type' ); - list( , $action ) = explode( '~', $type, 2 ); + return $this->action; + } - return $action; + /** + * @param string $action One of the self::… constants. + */ + public function setAction( $action ) { + $this->action = $action; + } + + /** + * @throws RuntimeException if one of the required fields was not set before + * @return string + */ + public function getType() { + if ( !$this->entityId || $this->action === null ) { + throw new RuntimeException( 'Forgot to call setEntityId and setAction!' ); + } + + $entityType = $this->entityId->getEntityType(); + $this->setField( 'type', 'wikibase-' . $entityType . '~' . $this->action ); + } + + /** + * @see Change::getObjectId + * + * @return string + */ + public function getObjectId() { + if ( !$this->entityId ) { + throw new RuntimeException( 'Forgot to call setEntityId!' ); + } + + return $this->entityId->getSerialization(); } /** @@ -147,7 +165,7 @@ if ( !isset( $metadata['comment'] ) ) { // Messages: wikibase-comment-add, wikibase-comment-remove, wikibase-comment-linked, // wikibase-comment-unlink, wikibase-comment-restore, wikibase-comment-update - $metadata['comment'] = 'wikibase-comment-' . $this->getAction(); + $metadata['comment'] = 'wikibase-comment-' . $this->action; } return $metadata['comment']; @@ -209,14 +227,10 @@ 'time' => $revision->getTimestamp(), ) ); - if ( !$this->hasField( 'object_id' ) ) { + if ( !$this->entityId ) { /* @var EntityContent $content */ $content = $revision->getContent(); // potentially expensive! - $entityId = $content->getEntityId(); - - $this->setFields( array( - 'object_id' => $entityId->getSerialization(), - ) ); + $this->entityId = $content->getEntityId(); } $this->setMetadata( array( diff --git a/lib/includes/Changes/EntityChangeFactory.php b/lib/includes/Changes/EntityChangeFactory.php index 95681c7..ee3accf 100644 --- a/lib/includes/Changes/EntityChangeFactory.php +++ b/lib/includes/Changes/EntityChangeFactory.php @@ -57,24 +57,18 @@ $entityType = $entityId->getEntityType(); if ( isset( $this->changeClasses[ $entityType ] ) ) { - $class = $this->changeClasses[$entityType]; + /** @var EntityChange $instance */ + $instance = new $this->changeClasses[$entityType]( $fields ); } else { - $class = EntityChange::class; + $instance = new EntityChange( $fields ); } - /** @var EntityChange $instance */ - $instance = new $class( $fields ); - $instance->setEntityId( $entityId ); + $instance->setAction( $action ); if ( !$instance->hasField( 'info' ) ) { $instance->setField( 'info', [] ); } - - // Note: the change type determines how newForChangeType will - // instantiate and handle the change - $type = 'wikibase-' . $entityId->getEntityType() . '~' . $action; - $instance->setField( 'type', $type ); return $instance; } diff --git a/repo/includes/Store/Sql/SqlChangeStore.php b/repo/includes/Store/Sql/SqlChangeStore.php index 0fa3ed1..d7a4279 100644 --- a/repo/includes/Store/Sql/SqlChangeStore.php +++ b/repo/includes/Store/Sql/SqlChangeStore.php @@ -78,15 +78,18 @@ * @return array */ private function getValues( ChangeRow $change ) { - $fields = $change->getFields(); + $time = $change->hasField( 'time' ) ? $change->getTime() : wfTimestampNow(); + $objectId = $change->hasField( 'object_id' ) ? $change->getObjectId() : ''; + $revisionId = $change->hasField( 'revision_id' ) ? $change->getField( 'revision_id' ) : 0; + $userId = $change->hasField( 'user_id' ) ? $change->getField( 'user_id' ) : 0; return array( - 'change_type' => $fields['type'], - 'change_time' => isset( $fields['time'] ) ? $fields['time'] : wfTimestampNow(), - 'change_object_id' => isset( $fields['object_id'] ) ? $fields['object_id'] : '', - 'change_revision_id' => isset( $fields['revision_id'] ) ? $fields['revision_id'] : '0', - 'change_user_id' => isset( $fields['user_id'] ) ? $fields['user_id'] : '0', - 'change_info' => $change->serializeInfo( $fields['info'] ) + 'change_type' => $change->getType(), + 'change_time' => $time, + 'change_object_id' => $objectId, + 'change_revision_id' => $revisionId, + 'change_user_id' => $userId, + 'change_info' => $change->serializeInfo( $change->getField( 'info' ) ) ); } -- To view, visit https://gerrit.wikimedia.org/r/339154 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6fc8955034662be78f9d7d7d5cecfc8dac15534a Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits