Matthias Mullie has submitted this change and it was merged.
Change subject: Remove flow_text table
......................................................................
Remove flow_text table
Merged flow_text into flow_revision, it only exists in core for historical
reasons. Any wiki that expects a reasonable amount of content should set
$wgFlowExternalStore to keep the bulk of content outside the revision table.
To allow storage classes to assign auto-ids the WritableObjectStorage::insert
method now returns an array representing the final storage state.
ObjectMapper::fromStorageRow has received an additional nullable param, $object,
which allows us to push those changes from insert back into the object.
Revision content compression happens in AbstractRevision::setContent,
decompression is handled in AbstractRevision::getContent so we lazily decompres
only the content the content that is needed.
Change-Id: I5cf3fe8ef0f6edb4328f4355a4abe5bec439446e
---
M flow.sql
M includes/Data/ObjectManager.php
M includes/Data/RevisionStorage.php
M includes/Model/AbstractRevision.php
M includes/Model/Definition.php
M includes/Model/PostRevision.php
M includes/Model/Summary.php
M includes/Model/TopicListEntry.php
M includes/Model/Workflow.php
M includes/Repository/TreeRepository.php
10 files changed, 228 insertions(+), 171 deletions(-)
Approvals:
Matthias Mullie: Verified; Looks good to me, approved
diff --git a/flow.sql b/flow.sql
index 22aa4af..caad08b 100644
--- a/flow.sql
+++ b/flow.sql
@@ -106,28 +106,18 @@
rev_user_text varchar(255) binary not null default '',
-- rev_id of parent or null if no previous revision
rev_parent_id binary(16),
-
+ -- comma separated set of ascii flags.
+ rev_flags tinyblob not null,
-- content of the revision
- rev_text_id int unsigned not null,
+ rev_content mediumblob not null,
-- comment attached to revision's flag change
rev_comment varchar(255) binary null,
-
PRIMARY KEY (rev_id)
) /*$wgDBTableOptions*/;
-- Prevents inconsistency, but perhaps will hurt inserts?
CREATE UNIQUE INDEX /*i*/flow_revision_unique_parent ON
/*_*/flow_revision (rev_parent_id);
-
-CREATE TABLE /*_*/flow_text (
- -- undecided on uuid, or if table is even neccessary
- -- large wiki should just use external store to distribute
- -- content
- text_id int(10) unsigned not null auto_increment,
- text_content mediumblob not null,
- text_flags tinyblob not null,
- PRIMARY KEY (text_id)
-) /*$wgDBTableOptions*/;
-- Closure table implementation of tree storage in sql
-- We may be able to go simpler than this
diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php
index 3d1d999..d5b2f0a 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -50,6 +50,9 @@
// Note that while ObjectLocator implements the above ObjectStorage interface,
ObjectManger
// cant use this interface because backing stores deal in rows, and OM deals
in objects.
interface WritableObjectStorage extends ObjectStorage {
+ /**
+ * @return array The resulting $row including any auto-assigned ids or
false on failure
+ */
function insert( array $row );
function update( array $old, array $new );
function remove( array $row );
@@ -62,9 +65,15 @@
function toStorageRow( $object );
/**
- * Convert a db row to its domain model.
+ * Convert a db row to its domain model. Object passing is intended for
+ * updating the object to match a changed storage representation.
+ *
+ * @param array $row assoc array representing the domain model
+ * @param object|null $object The domain model to populate, creates
when null
+ * @return object The domain model populated with $row
+ * @throws Exception When object is the wrong class for the mapper
*/
- function fromStorageRow( array $row );
+ function fromStorageRow( array $row, $object = null );
}
// An Index is just a store that receives updates via handler.
@@ -330,12 +339,17 @@
protected function insert( $object ) {
try {
- $new = $this->mapper->toStorageRow( $object );
- $this->storage->insert( $new );
- foreach ( $this->lifecycleHandlers as $handler ) {
- $handler->onAfterInsert( $object, $new );
+ $row = $this->mapper->toStorageRow( $object );
+ $stored = $this->storage->insert( $row );
+ if ( !$stored ) {
+ throw new \Exception( 'failed insert' );
}
- $this->loaded[$object] = $new;
+ // propogate auto-id's and such back into $object
+ $this->mapper->fromStorageRow( $stored, $object );
+ foreach ( $this->lifecycleHandlers as $handler ) {
+ $handler->onAfterInsert( $object, $stored );
+ }
+ $this->loaded[$object] = $stored;
} catch ( \Exception $e ) {
throw new PersistenceException( 'failed insert', null,
$e );
}
@@ -449,12 +463,18 @@
public function toStorageRow( $object ) {
return call_user_func( $this->toStorageRow, $object );
}
- public function fromStorageRow( array $row ) {
- return call_user_func( $this->fromStorageRow, $row );
+ public function fromStorageRow( array $row, $object = null ) {
+ return call_user_func( $this->fromStorageRow, $row, $object );
}
}
-// Doesn't support updating primary key value yet
+/**
+ * Standard backing store for data model with no special cases which is stored
+ * in a single table in mysql.
+ *
+ * Doesn't support updating primary key value yet
+ * Doesn't support auto-increment pk yet
+ */
class BasicDbStorage implements WritableObjectStorage {
public function __construct( DbFactory $dbFactory, $table, array
$primaryKey ) {
if ( !$primaryKey ) {
@@ -465,13 +485,19 @@
$this->primaryKey = $primaryKey;
}
+ // Does not support auto-increment id yet
public function insert( array $row ) {
// insert returns boolean true/false
- return $this->dbFactory->getDB( DB_MASTER )->insert(
+ $res = $this->dbFactory->getDB( DB_MASTER )->insert(
$this->table,
$row,
__METHOD__
);
+ if ( $res ) {
+ return $row;
+ } else {
+ return false;
+ }
}
public function update( array $old, array $new ) {
diff --git a/includes/Data/RevisionStorage.php
b/includes/Data/RevisionStorage.php
index aa05ce4..b50eb0f 100644
--- a/includes/Data/RevisionStorage.php
+++ b/includes/Data/RevisionStorage.php
@@ -10,7 +10,7 @@
use User;
abstract class RevisionStorage implements WritableObjectStorage {
- static protected $allowedUpdateColumns = array( 'text_flags' );
+ static protected $allowedUpdateColumns = array( 'rev_flags' );
protected $dbFactory;
protected $externalStores;
@@ -18,7 +18,7 @@
abstract protected function relatedPk();
abstract protected function joinField();
- abstract protected function insertRelated( array $rev, array $related );
+ abstract protected function insertRelated( array $row, array $related );
abstract protected function updateRelated( array $rev, array $related );
abstract protected function removeRelated( array $row );
@@ -40,15 +40,12 @@
protected function findInternal( array $attributes, array $options =
array() ) {
$dbr = $this->dbFactory->getDB( DB_MASTER );
$res = $dbr->select(
- array( $this->joinTable(), 'rev' => 'flow_revision',
'text' => 'flow_text' ),
+ array( $this->joinTable(), 'rev' => 'flow_revision' ),
'*',
UUID::convertUUIDs( $attributes ),
__METHOD__,
$options,
- array(
- 'rev' => array( 'JOIN', $this->joinField() . '
= rev_id' ),
- 'text' => array( 'JOIN', "text_id =
rev_text_id" ),
- )
+ array( 'rev' => array( 'JOIN', $this->joinField() . ' =
rev_id' ) )
);
if ( !$res ) {
return null;
@@ -66,23 +63,8 @@
} else {
$res = $this->findMultiInternal( $queries, $options );
}
- if ( $this->externalStore ) {
- $res = Merger::mergeMulti(
- $res,
- 'text_content',
- array( 'ExternalStore', 'batchFetchFromURLs' )
- );
- }
-
- // decompress content
- foreach ( $res as &$record ) {
- foreach ( $record as $id => &$row ) {
- $flags = explode( ',', $row['text_flags'] );
- $row['text_content'] =
\Revision::decompressRevisionText( $row['text_content'], $flags );
- }
- }
-
- return $res;
+ // Fetches content for all revisions flagged 'external'
+ return $this->mergeExternalContent( $res );
}
protected function fallbackFindMulti( array $queries, array $options ) {
@@ -167,16 +149,12 @@
// JOIN flow_revision ON tree_rev_id = rev_id
// WHERE tree_rev_id IN (...)
$res = $dbr->select(
- array( 'flow_revision', 'text' => 'flow_text', 'rev' =>
$this->joinTable() ),
+ array( 'flow_revision', 'rev' => $this->joinTable() ),
'*',
array( 'rev_id' => $revisionIds ),
__METHOD__,
array(),
- array(
- 'rev' => array( 'JOIN', "rev_id = $joinField" ),
- // not efficient, likely to pull same content
multiple times.
- 'text' => array( 'JOIN', "text_id =
rev_text_id" ),
- )
+ array( 'rev' => array( 'JOIN', "rev_id = $joinField" ) )
);
if ( !$res ) {
// TODO: dont fail, but dont end up caching bad result
either
@@ -190,6 +168,37 @@
}
return $duplicator->getResult();
+ }
+
+ /**
+ * Handle the injection of externalstore data into a revision
+ * row. All rows exiting this method will have rev_content_url
+ * set to either null or the external url. The rev_content
+ * field will be the final content (possibly compressed still)
+ *
+ * @param array $cacheResult 2d array of rows
+ * @return array 2d array of rows with content merged and
rev_content_url populated
+ */
+ protected function mergeExternalContent( array $cacheResult ) {
+ foreach ( $cacheResult as &$source ) {
+ foreach ( $source as &$row ) {
+ $flags = explode( ',', $row['rev_flags'] );
+ if ( in_array( 'external', $flags ) ) {
+ $row['rev_content_url'] =
$row['rev_content'];
+ $row['rev_content'] = '';
+ } else {
+ $row['rev_content_url'] = null;
+ }
+ }
+ }
+
+ return Merger::mergeMulti(
+ $cacheResult,
+ /* fromKey = */ 'rev_content_url',
+ /* callable = */ array( 'ExternalStore',
'batchFetchFromURLs' ),
+ /* name = */ 'rev_content',
+ /* default = */ ''
+ );
}
protected function buildCompositeInCondition( DatabaseBase $dbr, array
$queries ) {
@@ -214,11 +223,18 @@
}
public function insert( array $row ) {
- if ( !isset( $row['rev_text_id'] ) ) {
- $row['rev_text_id'] = $this->insertContent(
$row['text_content'], $row['text_flags'] );
+ // Check if we need to insert new content
+ if ( $this->externalStore && !isset( $row['rev_content_url'] )
) {
+ $row = $this->insertExternalStore( $row );
}
- unset( $row['text_content'], $row['text_flags'] );
list( $rev, $related ) = $this->splitUpdate( $row );
+ // If a content url is available store that in the db
+ // instead of real content.
+ if ( isset( $rev['rev_content_url'] ) ) {
+ $rev['rev_content'] = $rev['rev_content_url'];
+ }
+ unset( $rev['rev_content_url'] );
+
$dbw = $this->dbFactory->getDB( DB_MASTER );
$res = $dbw->insert(
'flow_revision',
@@ -230,33 +246,22 @@
return false;
}
- return $this->insertRelated( $rev, $related );
+ return $this->insertRelated( $row, $related );
}
- protected function insertContent( $data, $flags = '' ) {
- $compressionFlags = \Revision::compressRevisionText( $data );
- $flags = array_merge( explode( ',', $flags ), explode( ',',
$compressionFlags ) );
- $flags = array_filter( $flags );
-
- if ( $this->externalStore ) {
- // Store and get the URL
- $data = ExternalStore::insertWithFallback(
$this->externalStore, $data );
- if ( !$data ) {
- throw new \MWException( "Unable to store text
to external storage" );
- }
- $flags[] = 'external';
+ protected function insertExternalStore( array $row ) {
+ $url = ExternalStore::insertWithFallback( $this->externalStore,
$row['rev_content'] );
+ if ( !$url ) {
+ throw new \MWException( "Unable to store text to
external storage" );
+ }
+ $row['rev_content_url'] = $url;
+ if ( $row['rev_flags'] ) {
+ $row['rev_flags'] .= ',external';
+ } else {
+ $row['rev_flags'] = 'external';
}
- $dbw = $this->dbFactory->getDB( DB_MASTER );
- $dbw->insert(
- 'flow_text',
- array(
- 'text_content' => $data,
- 'text_flags' => implode( ',', array_unique(
$flags ) ),
- ),
- __METHOD__
- );
- return $dbw->insertId();
+ return $row;
}
// This is to *UPDATE* a revision. It should hardly ever be used.
@@ -357,20 +362,21 @@
UUID::convertUUIDs( $tree ),
__METHOD__
);
- if ( !$res ) {
- return false;
- }
// 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 ( $row['rev_parent_id'] === null ) {
- return (bool) $this->treeRepo->insert(
+ 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'] )
);
}
- return true;
+ if ( !$res ) {
+ return false;
+ }
+
+ return $row;
}
// Topic split will primarily be done through the TreeRepository
directly, but
@@ -407,12 +413,16 @@
return 'summary_rev_id';
}
- protected function insertRelated( array $rev, array $summary ) {
- return (bool) $this->dbFactory->getDB( DB_MASTER )->insert(
+ protected function insertRelated( array $row, array $summary ) {
+ $res = $this->dbFactory->getDB( DB_MASTER )->insert(
$this->joinTable(),
$summary,
__METHOD__
);
+ if ( !$res ) {
+ return false;
+ }
+ return $row;
}
// There is changable data in the summary half, it just points to the
correct workflow
@@ -443,14 +453,22 @@
* @param callable $callable Callable receiving array of foreign keys
returning map
* from foreign key to its value
* @param string $name Name to merge loaded foreign data as. If
null uses $fromKey.
+ * @param string $default Value to use when no matching foreign
value can be located
* @return array $source array with all found foreign key values merged
*/
- static public function merge( array $source, $fromKey, $callable, $name
= null ) {
+ static public function merge( array $source, $fromKey, $callable, $name
= null, $default = '' ) {
if ( $name === null ) {
$name = $fromKey;
}
+ $ids = array();
foreach ( $source as $row ) {
- $ids[] = $row[$fromKey];
+ $id = $row[$fromKey];
+ if ( $id !== null ) {
+ $ids[] = $id;
+ }
+ }
+ if ( !$ids ) {
+ return $source;
}
$res = call_user_func( $callable, $ids );
if ( $res === false ) {
@@ -458,9 +476,10 @@
}
foreach ( $source as $idx => $row ) {
$id = $row[$fromKey];
- if ( isset( $res[$id] ) ) {
- $source[$idx][$name] = $res[$id];
+ if ( $id === null ) {
+ continue;
}
+ $source[$idx][$name] = isset( $res[$id] ) ? $res[$id] :
$default;
}
return $source;
}
@@ -468,14 +487,21 @@
/**
* Same as self::merge, but for 3-dimensional source arrays
*/
- static public function mergeMulti( array $multiSource, $fromKey,
$callable, $name = null ) {
+ static public function mergeMulti( array $multiSource, $fromKey,
$callable, $name = null, $default = '' ) {
if ( $name === null ) {
$name = $fromKey;
}
+ $ids = array();
foreach ( $multiSource as $source ) {
foreach ( $source as $row ) {
- $ids[] = $row[$fromKey];
+ $id = $row[$fromKey];
+ if ( $id !== null ) {
+ $ids[] = $id;
+ }
}
+ }
+ if ( !$ids ) {
+ return $multiSource;
}
$res = call_user_func( $callable, array_unique( $ids ) );
if ( $res === false ) {
@@ -484,9 +510,10 @@
foreach ( $multiSource as $i => $source ) {
foreach ( $source as $j => $row ) {
$id = $row[$fromKey];
- if ( isset( $res[$id] ) ) {
- $multiSource[$i][$j][$name] = $res[$id];
+ if ( $id === null ) {
+ continue;
}
+ $multiSource[$i][$j][$name] = isset( $res[$id]
) ? $res[$id] : $default;
}
}
return $multiSource;
diff --git a/includes/Model/AbstractRevision.php
b/includes/Model/AbstractRevision.php
index c0fedcd..b2517bf 100644
--- a/includes/Model/AbstractRevision.php
+++ b/includes/Model/AbstractRevision.php
@@ -6,7 +6,6 @@
abstract class AbstractRevision {
protected $revId;
- protected $textId;
protected $userId;
protected $userText;
protected $flags = array();
@@ -17,28 +16,28 @@
protected $prevRevision;
// content
- protected $contentModel;
- protected $contentFormat;
protected $content;
+ // Only populated when external store is in use
+ protected $contentUrl;
+ // This is decompressed on-demand from $this->content in
self::getContent()
+ protected $decompressedContent;
- static public function fromStorageRow( array $row ) {
- $obj = new static;
- if ( $row['rev_type'] !== $obj->getRevisionType() ) {
- throw new \MWException( sprintf(
- "Wrong revision type, expected '%s' but
received '%s'",
- $obj->getRevisionType(),
- $row['rev_type']
- ) );
+ static public function fromStorageRow( array $row, $obj = null ) {
+ if ( $obj === null ) {
+ $obj = new static;
+ } elseif ( !$obj instanceof static ) {
+ throw new \Exception( 'wrong object type' );
}
$obj->revId = UUID::create( $row['rev_id'] );
$obj->userId = $row['rev_user_id'];
$obj->userText = $row['rev_user_text'];
$obj->prevRevision = UUID::create( $row['rev_parent_id'] );
$obj->comment = $row['rev_comment'];
-
- $obj->textId = $row['rev_text_id'];
- $obj->content = $row['text_content'];
- $obj->flags = explode( ',', $row['text_flags'] );
+ $obj->flags = array_filter( explode( ',', $row['rev_flags'] ) );
+ $obj->content = $row['rev_content'];
+ // null if external store is not being used
+ $obj->contentUrl = $row['rev_content_url'];
+ $obj->decompressedContent = null;
return $obj;
}
@@ -49,11 +48,11 @@
'rev_user_text' => $obj->userText,
'rev_parent_id' => $obj->prevRevision ?
$obj->prevRevision->getBinary() : null,
'rev_comment' => $obj->comment,
- 'rev_text_id' => $obj->textId,
'rev_type' => $obj->getRevisionType(),
- 'text_content' => $obj->content,
- 'text_flags' => implode( ',', $obj->flags ),
+ 'rev_content' => $obj->content,
+ 'rev_content_url' => $obj->contentUrl,
+ 'rev_flags' => implode( ',', $obj->flags ),
);
}
@@ -70,11 +69,7 @@
public function newNextRevision( User $user, $content ) {
$obj = $this->newNullRevision( $user );
- $obj->flags = array();
- if ( $content !== $obj->content ) {
- $obj->content = $content;
- $obj->textId = null;
- }
+ $obj->setContent( $content );
return $obj;
}
@@ -83,14 +78,19 @@
}
public function getContent() {
- if ( $this->content === null ) {
- throw new \MWException( 'Content not loaded' );
+ if ( $this->decompressedContent === null ) {
+ $this->decompressedContent =
\Revision::decompressRevisionText( $this->content, $this->flags );
}
- return $this->content;
+ return $this->decompressedContent;
}
- public function getTextId() {
- return $this->textId; // not available on creation
+ protected function setContent( $content ) {
+ if ( $content !== $this->getContent() ) {
+ $this->content = $this->decompressedContent = $content;
+ $this->contentUrl = null;
+ // should this only remove a subset of flags?
+ $this->flags = array_filter( explode( ',',
\Revision::compressRevisionText( $this->content ) ) );
+ }
}
public function getPrevRevisionId() {
diff --git a/includes/Model/Definition.php b/includes/Model/Definition.php
index fda4791..325f5db 100644
--- a/includes/Model/Definition.php
+++ b/includes/Model/Definition.php
@@ -10,11 +10,15 @@
protected $name;
protected $options = array();
- static public function fromStorageRow( array $row ) {
- $obj = new self;
+ static public function fromStorageRow( array $row, $obj = null ) {
if ( ! $row['definition_wiki'] ) {
throw new \MWException( "No definition_wiki" );
}
+ if ( $obj === null ) {
+ $obj = new self;
+ } elseif ( !$obj instanceof self ) {
+ throw new \Exception( 'Wrong obj type: ' . get_class(
$obj ) );
+ }
$obj->id = UUID::create( $row['definition_id'] );
$obj->type = $row['definition_type'];
$obj->wiki = $row['definition_wiki'];
diff --git a/includes/Model/PostRevision.php b/includes/Model/PostRevision.php
index 1617faa..0ab36d7 100644
--- a/includes/Model/PostRevision.php
+++ b/includes/Model/PostRevision.php
@@ -24,21 +24,22 @@
$obj = new self;
$obj->revId = UUID::create();
$obj->postId = $topic->getId();
- $obj->content = $content;
$obj->origUserId = $obj->userId = $topic->getUserId();
$obj->origUserText = $obj->userText = $topic->getUserText();
$obj->origCreateTime = wfTimestampNow();
$obj->replyToId = null; // not a reply to anything
$obj->prevRevId = null; // no parent revision
$obj->comment = 'flow-rev-message-new-post';
+ $obj->setContent( $content );
+
return $obj;
}
- static public function fromStorageRow( array $row ) {
+ static public function fromStorageRow( array $row, $obj = null ) {
if ( $row['rev_id'] !== $row['tree_rev_id'] ) {
throw new \MWException( 'tree revision doesn\'t match
provided revision' );
}
- $obj = parent::fromStorageRow( $row );
+ $obj = parent::fromStorageRow( $row, $obj );
$obj->replyToId = UUID::create( $row['tree_parent_id'] );
$obj->postId = UUID::create( $row['tree_rev_descendant_id'] );
@@ -68,7 +69,7 @@
$reply->userId = $reply->origUserId = $user->getId();
$reply->userText = $reply->origUserText = $user->getName();
$reply->origCreateTime = wfTimestampNow();
- $reply->content = $content;
+ $reply->setContent( $content );
$reply->replyToId = $this->postId;
$reply->comment = 'flow-rev-message-reply';
return $reply;
diff --git a/includes/Model/Summary.php b/includes/Model/Summary.php
index 238a874..ffb65a1 100644
--- a/includes/Model/Summary.php
+++ b/includes/Model/Summary.php
@@ -11,15 +11,15 @@
$obj = new self;
$obj->revId = UUID::create();
$obj->workflowId = $workflow->getId();
- $obj->content = $content;
$obj->userId = $user->getId();
$obj->userText = $user->getName();
$obj->prevRevision = null; // no prior revision
+ $obj->setContent( $content );
return $obj;
}
- static public function fromStorageRow( array $row ) {
- $obj = parent::fromStorageRow( $row );
+ static public function fromStorageRow( array $row, $obj = null ) {
+ $obj = parent::fromStorageRow( $row, $obj );
$obj->workflowId = UUID::create( $row['summary_workflow_id'] );
return $obj;
}
diff --git a/includes/Model/TopicListEntry.php
b/includes/Model/TopicListEntry.php
index 6d07f16..0521d89 100644
--- a/includes/Model/TopicListEntry.php
+++ b/includes/Model/TopicListEntry.php
@@ -4,40 +4,44 @@
// TODO: We shouldn't need this class
class TopicListEntry {
- protected $topicListId;
- protected $topicId;
+ protected $topicListId;
+ protected $topicId;
- static public function create( Workflow $topicList, Workflow
$topic ) {
- // die( var_dump( array(
- // 'topicList' => $topicList,
- // 'topic' => $topic,
- // )));
+ static public function create( Workflow $topicList, Workflow $topic ) {
+ // die( var_dump( array(
+ // 'topicList' => $topicList,
+ // 'topic' => $topic,
+ // )));
+ $obj = new self;
+ $obj->topicListId = $topicList->getId();
+ $obj->topicId = $topic->getId();
+ return $obj;
+ }
+
+ static public function fromStorageRow( array $row, $obj = null ) {
+ if ( $obj === null ) {
$obj = new self;
- $obj->topicListId = $topicList->getId();
- $obj->topicId = $topic->getId();
- return $obj;
+ } elseif ( !$obj instanceof self ) {
+ throw new \Exception( 'Wrong obj type: ' . get_class(
$obj ) );
}
+ $obj->topicListId = UUID::create( $row['topic_list_id'] );
+ $obj->topicId = UUID::create( $row['topic_id'] );
+ return $obj;
+ }
- static public function fromStorageRow( array $row ) {
- $obj = new self;
- $obj->topicListId = UUID::create(
$row['topic_list_id'] );
- $obj->topicId = UUID::create( $row['topic_id']
);
- return $obj;
- }
+ static public function toStorageRow( TopicListEntry $obj ) {
+ return array(
+ 'topic_list_id' => $obj->topicListId->getBinary(),
+ 'topic_id' => $obj->topicId->getBinary(),
+ );
+ }
- static public function toStorageRow( TopicListEntry $obj ) {
- return array(
- 'topic_list_id' =>
$obj->topicListId->getBinary(),
- 'topic_id' =>
$obj->topicId->getBinary(),
- );
- }
+ public function getId() {
+ return $this->topicId;
+ }
- public function getId() {
- return $this->topicId;
- }
-
- public function getListId() {
- return $this-topicListId;
- }
+ public function getListId() {
+ return $this-topicListId;
+ }
}
diff --git a/includes/Model/Workflow.php b/includes/Model/Workflow.php
index 0b0ad54..c16efa2 100644
--- a/includes/Model/Workflow.php
+++ b/includes/Model/Workflow.php
@@ -8,6 +8,7 @@
class Workflow {
protected $id;
+ // @var boolean false before writing to storage
protected $isNew;
protected $wiki;
protected $pageId;
@@ -20,9 +21,14 @@
protected $lockState;
protected $definitionId;
- static public function fromStorageRow( array $row ) {
- $obj = new self;
+ static public function fromStorageRow( array $row, $obj = null ) {
+ if ( $obj === null ) {
+ $obj = new self;
+ } elseif ( !$obj instanceof self ) {
+ throw new \Exception( 'Wrong obj type: ' . get_class(
$obj ) );
+ }
$obj->id = UUID::create( $row['workflow_id'] );
+ $obj->isNew = false;
$obj->wiki = $row['workflow_wiki'];
$obj->pageId = $row['workflow_page_id'];
$obj->namespace = (int) $row['workflow_namespace'];
@@ -50,20 +56,19 @@
static public function create( Definition $definition, User $user,
Title $title ) {
if ( $title->isLocal() ) {
- if ( $definition->getWiki() !== wfWikiId() ) {
- throw new \Exception( 'Title and
Definition are from separate wikis' );
- }
+ $wiki = wfWikiId();
} else {
- if ( $definition->getWiki() !==
$title->getTransWikiID() ) {
+ $wiki = $title->getTransWikiID();
+ }
+ if ( $definition->getWiki() !== $wiki ) {
throw new \Exception( 'Title and Definition are
from separate wikis' );
- }
}
$obj = new self;
// Probably unnecessary to create id up front?
// simpler in prototype to give everything an id up front?
$obj->id = UUID::create();
- $obj->isNew = true; // new as of this request
+ $obj->isNew = true; // has not been persisted
$obj->wiki = $definition->getWiki();
$obj->pageId = $title->getArticleID();
$obj->namespace = $title->getNamespace();
diff --git a/includes/Repository/TreeRepository.php
b/includes/Repository/TreeRepository.php
index d88bd94..6c91115 100644
--- a/includes/Repository/TreeRepository.php
+++ b/includes/Repository/TreeRepository.php
@@ -94,8 +94,8 @@
$this->cache->del( $pathKey );
throw new MWEception( 'Failed inserting new tree node'
);
}
-
$this->appendToSubtreeCache( $descendant, $path );
+ return true;
}
protected function appendToSubtreeCache( UUID $descendant, array
$rootPath ) {
--
To view, visit https://gerrit.wikimedia.org/r/78313
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5cf3fe8ef0f6edb4328f4355a4abe5bec439446e
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson (WMF) <[email protected]>
Gerrit-Reviewer: EBernhardson (WMF) <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Werdna <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
