EBernhardson (WMF) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/78313
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 so keep the bulk of content outside the revision table.
Change-Id: I5cf3fe8ef0f6edb4328f4355a4abe5bec439446e
---
M flow.sql
M includes/Data/ObjectManager.php
M includes/Data/RevisionStorage.php
M includes/Model/AbstractRevision.php
4 files changed, 64 insertions(+), 71 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/13/78313/1
diff --git a/flow.sql b/flow.sql
index 22aa4af..161cbf0 100644
--- a/flow.sql
+++ b/flow.sql
@@ -106,9 +106,10 @@
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,
@@ -118,16 +119,6 @@
-- 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..0a5dbaa 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -50,7 +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 {
- function insert( array $row );
+ // this is a reference to allow external store inside RevisionStorage
to utilize
+ // external store and effect updates to the cached values
+ function insert( array &$row );
function update( array $old, array $new );
function remove( array $row );
}
@@ -465,7 +467,7 @@
$this->primaryKey = $primaryKey;
}
- public function insert( array $row ) {
+ public function insert( array &$row ) {
// insert returns boolean true/false
return $this->dbFactory->getDB( DB_MASTER )->insert(
$this->table,
diff --git a/includes/Data/RevisionStorage.php
b/includes/Data/RevisionStorage.php
index aa05ce4..532a12a 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;
@@ -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;
@@ -69,7 +66,7 @@
if ( $this->externalStore ) {
$res = Merger::mergeMulti(
$res,
- 'text_content',
+ 'rev_content',
array( 'ExternalStore', 'batchFetchFromURLs' )
);
}
@@ -77,8 +74,8 @@
// 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 );
+ $flags = explode( ',', $row['rev_flags'] );
+ $row['rev_content'] =
\Revision::decompressRevisionText( $row['rev_content'], $flags );
}
}
@@ -167,16 +164,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
@@ -213,12 +206,28 @@
}
}
- public function insert( array $row ) {
- if ( !isset( $row['rev_text_id'] ) ) {
- $row['rev_text_id'] = $this->insertContent(
$row['text_content'], $row['text_flags'] );
- }
- unset( $row['text_content'], $row['text_flags'] );
+ /**
+ * We take $row as a reference because we *must* be able to effect the
cached content
+ * of $row. This is specifically required so that external store can
change the content
+ * into a url pointing to the content and adjust its flags. The other
alternative would
+ * be to invent an onBeforeInsert lifecycle event.
+ */
+ public function insert( array &$row ) {
+ // Check if we need to insert new content
list( $rev, $related ) = $this->splitUpdate( $row );
+ if ( $this->externalStore ) {
+ // There is no rev_content_url field, it exists only
for external store data
+ if ( isset( $row['rev_content_url'] ) ) {
+ $rev['rev_content'] = $row['rev_content_url'];
+ } else {
+ list( $url, $flags ) = $this->insertContent(
$row['rev_content'], $row['rev_flags'] );
+ // This adds, through the $row reference, the
rev_content_url and the flags to the cached values
+ $row['rev_content_url'] = $rev['rev_content'] =
$url;
+ $row['rev_flags'] = $rev['rev_flags'] = $flags;
+ }
+ }
+ // This is a pseudo-field that never goes to database
+ unset( $rev['rev_content_url'] );
$dbw = $this->dbFactory->getDB( DB_MASTER );
$res = $dbw->insert(
'flow_revision',
@@ -236,27 +245,16 @@
protected function insertContent( $data, $flags = '' ) {
$compressionFlags = \Revision::compressRevisionText( $data );
$flags = array_merge( explode( ',', $flags ), explode( ',',
$compressionFlags ) );
- $flags = array_filter( $flags );
+ $flags = array_unique( 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';
+ // Store and get the URL
+ $url = ExternalStore::insertWithFallback( $this->externalStore,
$data );
+ if ( !$url ) {
+ throw new \MWException( "Unable to store text to
external storage" );
}
+ $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 array( $url, implode( ',', $flags ) );
}
// This is to *UPDATE* a revision. It should hardly ever be used.
@@ -443,9 +441,10 @@
* @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;
}
@@ -458,9 +457,10 @@
}
foreach ( $source as $idx => $row ) {
$id = $row[$fromKey];
- if ( isset( $res[$id] ) ) {
- $source[$idx][$name] = $res[$id];
+ if ( $name === $fromKey ) {
+ $source[$idx]["{$name}_url"] =
$source[$idx][$name];
}
+ $source[$idx][$name] = isset( $res[$id] ) ? $res[$id] :
$default;
}
return $source;
}
@@ -468,7 +468,7 @@
/**
* 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;
}
@@ -484,9 +484,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 ( $name === $fromKey ) {
+ $source[$idx]["{$name}_url"] =
$source[$idx][$name];
}
+ $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..a981866 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();
@@ -20,6 +19,8 @@
protected $contentModel;
protected $contentFormat;
protected $content;
+ // Only populated when external store is in use
+ protected $contentUrl;
static public function fromStorageRow( array $row ) {
$obj = new static;
@@ -35,10 +36,12 @@
$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->content = $row['rev_content'];
+ if ( isset( $row['rev_content_url'] ) ) {
+ // only exists when external store is being used
+ $obj->contentUrl = $row['rev_content_url'];
+ }
+ $obj->flags = explode( ',', $row['rev_flags'] );
return $obj;
}
@@ -49,11 +52,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 ),
);
}
@@ -73,7 +76,7 @@
$obj->flags = array();
if ( $content !== $obj->content ) {
$obj->content = $content;
- $obj->textId = null;
+ $obj->contentUrl = null;
}
return $obj;
}
@@ -87,10 +90,6 @@
throw new \MWException( 'Content not loaded' );
}
return $this->content;
- }
-
- public function getTextId() {
- return $this->textId; // not available on creation
}
public function getPrevRevisionId() {
--
To view, visit https://gerrit.wikimedia.org/r/78313
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5cf3fe8ef0f6edb4328f4355a4abe5bec439446e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson (WMF) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
