jenkins-bot has submitted this change and it was merged. Change subject: Restoring "Implement multiPut method." ......................................................................
Restoring "Implement multiPut method." This restores https://gerrit.wikimedia.org/r/#/c/120734/, addresses the rev_content/rev_content_url cache issue Change-Id: Ie59d5db8989dc3b81374f54732b47f6b220fe10b --- M includes/Data/BasicDbStorage.php M includes/Data/DbStorage.php M includes/Data/ManagerGroup.php M includes/Data/ObjectManager.php M includes/Data/ObjectStorage.php M includes/Data/PostRevisionStorage.php M includes/Data/RevisionStorage.php 7 files changed, 149 insertions(+), 44 deletions(-) Approvals: EBernhardson: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/Data/BasicDbStorage.php b/includes/Data/BasicDbStorage.php index 4d5a8d7..d1b09da 100644 --- a/includes/Data/BasicDbStorage.php +++ b/includes/Data/BasicDbStorage.php @@ -24,25 +24,44 @@ $this->primaryKey = $primaryKey; } - // Does not support auto-increment id yet - public function insert( array $row ) { + /** + * Inserts a set of rows into the database + * + * @param array $rows The rows to insert. Also accepts a single row. + * @return array|false An array of the rows that now exist + * in the database. Integrity of keys is guaranteed. + * False if we failed. + */ + public function insert( array $rows ) { // Only allow the row to include key/value pairs. // No raw SQL. - $row = $this->preprocessSqlArray( $row ); + if ( is_array( reset( $rows ) ) ) { + $rows = $this->preprocessNestedSqlArray( $rows ); + } else { + $rows = $this->preprocessSqlArray( $rows ); + } // insert returns boolean true/false $res = $this->dbFactory->getDB( DB_MASTER )->insert( $this->table, - $row, + $rows, __METHOD__ . " ({$this->table})" ); if ( $res ) { - return $row; + return $rows; } else { return false; } } + /** + * Update a single row in the database. + * + * @param array $old The current state of the row. + * @param array $new The desired new state of the row. + * @return boolean Whether or not the operation was successful. + * @throws DataPersistenceException + */ public function update( array $old, array $new ) { $pk = ObjectManager::splitFromRow( $old, $this->primaryKey ); if ( $pk === null ) { diff --git a/includes/Data/DbStorage.php b/includes/Data/DbStorage.php index 9d1327c..998ea34 100644 --- a/includes/Data/DbStorage.php +++ b/includes/Data/DbStorage.php @@ -21,6 +21,23 @@ } /** + * Runs preprocessSqlArray on each element of an array. + * + * @param array $outer The array to check + * @return array Preprocessed SQL array. + * @throws DataModelException + */ + protected function preprocessNestedSqlArray( array $outer ) { + foreach ( $outer as $i => $data ) { + if ( ! is_array( $data) ) { + throw new DataModelException( "Unexpected non-array in nested SQL array" ); + } + $outer[$i] = $this->preprocessSqlArray( $data ); + } + return $outer; + } + + /** * At the moment, does three things: * 1. Finds UUID objects and returns their database representation. * 2. Checks for unarmoured raw SQL and errors out if it exists. diff --git a/includes/Data/ManagerGroup.php b/includes/Data/ManagerGroup.php index 17b8712..e0d3c10 100644 --- a/includes/Data/ManagerGroup.php +++ b/includes/Data/ManagerGroup.php @@ -32,6 +32,27 @@ $this->getStorage( get_class( $object ) )->put( $object ); } + + protected function multiMethod( $method, $objects ) { + $itemsByClass = array(); + + foreach( $objects as $object ) { + $itemsByClass[ get_class( $object ) ][] = $object; + } + + foreach( $itemsByClass as $class => $myObjects ) { + $this->getStorage( $class )->$method( $myObjects ); + } + } + + public function multiPut( $objects ) { + $this->multiMethod( 'multiPut', $objects ); + } + + public function multiRemove( $objects ) { + $this->multiMethod( 'multiRemove', $objects ); + } + protected function call( $method, $args ) { $className = array_shift( $args ); diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php index 33f9016..ea746c1 100644 --- a/includes/Data/ObjectManager.php +++ b/includes/Data/ObjectManager.php @@ -30,10 +30,27 @@ } public function put( $object ) { - if ( isset( $this->loaded[$object] ) ) { - $this->update( $object ); - } else { - $this->insert( $object ); + $this->multiPut( array( $object ) ); + } + + public function multiPut( array $objects ) { + $updateObjects = array(); + $insertObjects = array(); + + foreach( $objects as $object ) { + if ( isset( $this->loaded[$object] ) ) { + $updateObjects[] = $object; + } else { + $insertObjects[] = $object; + } + } + + if ( count( $updateObjects ) ) { + $this->update( $updateObjects ); + } + + if ( count( $insertObjects ) ) { + $this->insert( $insertObjects ); } } @@ -47,32 +64,44 @@ } } - protected function insert( $object ) { + protected function insert( array $objects ) { $section = new \ProfileSection( __METHOD__ ); - $row = $this->mapper->toStorageRow( $object ); - $stored = $this->storage->insert( $row ); - if ( !$stored ) { + $rows = array_map( array( $this->mapper, 'toStorageRow' ), $objects ); + $storedRows = $this->storage->insert( $rows ); + if ( !$storedRows ) { throw new DataModelException( 'failed insert', 'process-data' ); } - // propogate auto-id's and such back into $object - $this->mapper->fromStorageRow( $stored, $object ); - foreach ( $this->lifecycleHandlers as $handler ) { - $handler->onAfterInsert( $object, $stored ); + + $numObjects = count( $objects ); + for( $i = 0; $i < $numObjects; ++$i ) { + $object = $objects[$i]; + $stored = $storedRows[$i]; + + // Propagate stuff that was added to the row by storage back + // into the object. Currently intended for storage URLs etc, + // but may in the future also bring in auto-ids and so on. + $this->mapper->fromStorageRow( $stored, $object ); + + foreach ( $this->lifecycleHandlers as $handler ) { + $handler->onAfterInsert( $object, $stored ); + } + + $this->loaded[$object] = $stored; } - $this->loaded[$object] = $stored; } - protected function update( $object ) { + protected function update( array $objects ) { $section = new \ProfileSection( __METHOD__ ); + foreach( $objects as $object ) { + $this->updateSingle( $object ); + } + } + + protected function updateSingle( $object ) { $old = $this->loaded[$object]; $new = $this->mapper->toStorageRow( $object ); if ( self::arrayEquals( $old, $new ) ) { return; - } - foreach ( $new as $k => $x ) { - if ( $x !== null && !is_scalar( $x ) ) { - throw new DataModelException( "Expected mapper to return all scalars, but '$k' is " . gettype( $x ), 'process-data' ); - } } $this->storage->update( $old, $new ); foreach ( $this->lifecycleHandlers as $handler ) { @@ -168,10 +197,6 @@ } return implode( '|', $offsetFields ); - } - - public function multiPut( array $objects ) { - throw new DataModelException( 'Not Implemented', 'process-data' ); } public function multiDelete( array $objects ) { diff --git a/includes/Data/ObjectStorage.php b/includes/Data/ObjectStorage.php index bb58b7c..4601b77 100644 --- a/includes/Data/ObjectStorage.php +++ b/includes/Data/ObjectStorage.php @@ -35,10 +35,12 @@ /** * Insert the specified row into the data store. * - * @param array $row Map of columns to values + * @param array $row An array of rows, each row is a map of columns => values. + * Currently, the old calling convention of a simple map of columns to values is + * also supported. * @return array|false The resulting $row including any auto-assigned ids or false on failure */ - function insert( array $row ); + function insert( array $rows ); /** * Perform all changes necessary to turn $old into $new in the data store. diff --git a/includes/Data/PostRevisionStorage.php b/includes/Data/PostRevisionStorage.php index 66cb48b..9bb3175 100644 --- a/includes/Data/PostRevisionStorage.php +++ b/includes/Data/PostRevisionStorage.php @@ -26,29 +26,41 @@ return 'post'; } - protected function insertRelated( array $row ) { - $tree = $this->splitUpdate( $row, 'tree' ); + protected function insertRelated( array $rows ) { + if ( ! is_array( reset( $rows ) ) ) { + $rows = array( $rows ); + } + + $trees = array(); + foreach( $rows as $key => $row ) { + $trees[$key] = $this->splitUpdate( $row, 'tree' ); + } + $dbw = $this->dbFactory->getDB( DB_MASTER ); $res = $dbw->insert( $this->joinTable(), - $this->preprocessSqlArray( $tree ), + $this->preprocessNestedSqlArray( $trees ), __METHOD__ ); // If this is a brand new root revision it needs to be added to the tree // If it has a rev_parent_id then its already a part of the tree - if ( $res && $row['rev_parent_id'] === null ) { - $res = (bool) $this->treeRepo->insert( - UUID::create( $tree['tree_rev_descendant_id'] ), - UUID::create( $tree['tree_parent_id'] ) - ); + if ( $res ) { + foreach( $rows as $row ) { + if ( $row['rev_parent_id'] === null ) { + $res = $res && $this->treeRepo->insert( + UUID::create( $row['tree_rev_descendant_id'] ), + UUID::create( $row['tree_parent_id'] ) + ); + } + } } if ( !$res ) { return false; } - return $row; + return $rows; } // Topic split will primarily be done through the TreeRepository directly, but diff --git a/includes/Data/RevisionStorage.php b/includes/Data/RevisionStorage.php index 2dd4605..7ca591f 100644 --- a/includes/Data/RevisionStorage.php +++ b/includes/Data/RevisionStorage.php @@ -348,14 +348,23 @@ } } - public function insert( array $row ) { - $rev = $this->splitUpdate( $row, 'rev' ); - $rev = $this->processExternalStore( $rev ); + public function insert( array $rows ) { + if ( ! is_array( reset( $rows ) ) ) { + $rows = array( $rows ); + } + + // Holds the subset of the row to go into the revision table + $revisions = array(); + + foreach( $rows as $key => $row ) { + $row = $this->processExternalStore( $row ); + $revisions[$key] = $this->splitUpdate( $row, 'rev' ); + } $dbw = $this->dbFactory->getDB( DB_MASTER ); $res = $dbw->insert( 'flow_revision', - $this->preprocessSqlArray( $rev ), + $this->preprocessNestedSqlArray( $revisions ), __METHOD__ ); if ( !$res ) { @@ -363,7 +372,7 @@ return false; } - return $this->insertRelated( $row ); + return $this->insertRelated( $rows ); } protected function processExternalStore( array $row ) { -- To view, visit https://gerrit.wikimedia.org/r/129613 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie59d5db8989dc3b81374f54732b47f6b220fe10b Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: master Gerrit-Owner: Bsitu <bs...@wikimedia.org> Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org> Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Werdna <agarr...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
