jenkins-bot has submitted this change and it was merged.
Change subject: Fix removal/addition of categorylinks
......................................................................
Fix removal/addition of categorylinks
A couple of things lead to `categorylinks` not properly being updated
when categories changed:
* The listener wanted to update categorylinks (via LinksTableUpdater)
before the data was actually stored to cache. Should usually be
fine since we're writing to cache (and even reading that data as
long as it's not committed), but safer would be to defer until data
has been committed to DB (for when there's no cache, it's cleared
elsewhere, it fails, ...)
If it has to read from DB before it was committed, it would read
the existing data & not pick up the changes.
* Cache was not properly being updated. We currently have a nullable
column in production (ref_src_wiki) to prepare for upcoming changes.
Current code is not yet aware of that column. When filling cache
with data read from DB, ref_src_wiki is included. When changing
the data, the code doesn't know about it. It tries to array_find()
different things and doesn't find the existing row and is unable
to update it.
Changed it to fetch the schema from the first row in cache, and
make the new row match the schema. Should be ok for nullable columms
that are otherwise of no importance. And if there are meaningful
schema changes, we should alter cache key anyway.
* Since we have bad data in cache ATM, I updated the cache keys.
* Not entirely related, but when reading from DB & writing to cache,
we should also normalize (roundtrip to- & fromStorageRow) the
data. It should store what the code knows about, not what the DB
schema happens to look like.
* Maintenance script to fix incorrect existing categorylinks etc.
* Removed an expensive & potentially erroneous fallback path in
TopicHistoryIndex::findTopicRootId. It was used in some test
(which I fixed), and I couldn't find other occurences anymore.
Most important changes are in:
FeatureIndex.php (#4)
TopKIndex.php (#2)
ReferenceRecorder.php (#1)
FlowFixLinks.php (#5)
The rest of the changes are mostly because I had to pass $mapper
along to the indices (#4)
Bug: T94569
Change-Id: I23147fd711425e770369f092ad25d0de2d354c2f
---
M Hooks.php
M container.php
M includes/Data/Index/BoardHistoryIndex.php
M includes/Data/Index/FeatureIndex.php
M includes/Data/Index/TopKIndex.php
M includes/Data/Index/TopicHistoryIndex.php
M includes/Data/Listener/ReferenceRecorder.php
M includes/Data/Mapper/BasicObjectMapper.php
M includes/Data/Mapper/CachingObjectMapper.php
M includes/Data/Storage/BasicDbStorage.php
A maintenance/FlowFixLinks.php
M tests/phpunit/Data/Index/FeatureIndexTest.php
M tests/phpunit/Data/IndexTest.php
M tests/phpunit/Data/Pager/PagerTest.php
M tests/phpunit/PostRevisionTestCase.php
M tests/phpunit/api/ApiFlowViewTopicListTest.php
16 files changed, 277 insertions(+), 39 deletions(-)
Approvals:
Mattflaschen: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Hooks.php b/Hooks.php
index f387602..6bf3a96 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -228,6 +228,9 @@
require_once __DIR__.'/maintenance/FlowCreateTemplates.php';
$updater->addPostDatabaseUpdateMaintenance(
'FlowCreateTemplates' );
+ require_once __DIR__.'/maintenance/FlowFixLinks.php';
+ $updater->addPostDatabaseUpdateMaintenance( 'FlowFixLinks' );
+
return true;
}
diff --git a/container.php b/container.php
index d2534c2..f912cf5 100644
--- a/container.php
+++ b/container.php
@@ -172,6 +172,12 @@
$c['storage.workflow.class'] = 'Flow\Model\Workflow';
$c['storage.workflow.table'] = 'flow_workflow';
$c['storage.workflow.primary_key'] = array( 'workflow_id' );
+$c['storage.workflow.mapper'] = function( $c ) {
+ return CachingObjectMapper::model(
+ $c['storage.workflow.class'],
+ $c['storage.workflow.primary_key']
+ );
+};
$c['storage.workflow.backend'] = function( $c ) {
return new BasicDbStorage(
$c['db.factory'],
@@ -179,16 +185,11 @@
$c['storage.workflow.primary_key']
);
};
-$c['storage.workflow.mapper'] = function( $c ) {
- return CachingObjectMapper::model(
- $c['storage.workflow.class'],
- $c['storage.workflow.primary_key']
- );
-};
$c['storage.workflow.indexes.primary'] = function( $c ) {
return new UniqueFeatureIndex(
$c['memcache.local_buffered'],
$c['storage.workflow.backend'],
+ $c['storage.workflow.mapper'],
'flow_workflow:v2:pk',
$c['storage.workflow.primary_key']
);
@@ -197,6 +198,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.workflow.backend'],
+ $c['storage.workflow.mapper'],
'flow_workflow:title:v2:',
array( 'workflow_wiki', 'workflow_namespace',
'workflow_title_text', 'workflow_type' ),
array(
@@ -269,6 +271,8 @@
$c['memcache.local_buffered'],
// backend storage
$c['storage.board_history.backend'],
+ // data mapper
+ $c['storage.board_history.mapper'],
// key prefix
'flow_revision:topic_list_history',
// primary key
@@ -354,6 +358,7 @@
return new UniqueFeatureIndex(
$c['memcache.local_buffered'],
$c['storage.header.backend'],
+ $c['storage.header.mapper'],
'flow_header:v2:pk',
$c['storage.header.primary_key']
);
@@ -362,6 +367,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.header.backend'],
+ $c['storage.header.mapper'],
'flow_header:workflow',
array( 'rev_type_id' ),
array(
@@ -432,6 +438,7 @@
return new UniqueFeatureIndex(
$c['memcache.local_buffered'],
$c['storage.post_summary.backend'],
+ $c['storage.post_summary.mapper'],
'flow_post_summary:v2:pk',
$c['storage.post_summary.primary_key']
);
@@ -440,6 +447,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.post_summary.backend'],
+ $c['storage.post_summary.mapper'],
'flow_post_summary:workflow',
array( 'rev_type_id' ),
array(
@@ -498,6 +506,7 @@
return new UniqueFeatureIndex(
$c['memcache.local_buffered'],
$c['storage.topic_list.backend'],
+ $c['storage.topic_list.mapper'],
'flow_topic_list:topic',
array( 'topic_id' )
);
@@ -509,6 +518,7 @@
return new TopicListTopKIndex(
$c['memcache.local_buffered'],
$c['storage.topic_list.backend'],
+ $c['storage.topic_list.mapper'],
'flow_topic_list:list',
array( 'topic_list_id' ),
array( 'sort' => 'topic_id' )
@@ -519,6 +529,7 @@
return new TopicListTopKIndex(
$c['memcache.local_buffered'],
$c['storage.topic_list.indexes.last_updated.backend'],
+ $c['storage.topic_list.mapper'],
'flow_topic_list_last_updated:list',
array( 'topic_list_id' ),
array(
@@ -611,6 +622,7 @@
return new UniqueFeatureIndex(
$c['memcache.local_buffered'],
$c['storage.post.backend'],
+ $c['storage.post.mapper'],
'flow_revision:v4:pk',
$c['storage.post.primary_key']
);
@@ -620,6 +632,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.post.backend'],
+ $c['storage.post.mapper'],
'flow_revision:descendant',
array( 'rev_type_id' ),
array(
@@ -651,10 +664,9 @@
};
$c['storage.topic_history.primary_key'] = array( 'rev_id' );
$c['storage.topic_history.backend'] = function( $c ) {
- global $wgFlowExternalStore;
return new TopicHistoryStorage(
- new PostRevisionStorage( $c['db.factory'],
$wgFlowExternalStore, $c['repository.tree'] ),
- new PostSummaryRevisionStorage( $c['db.factory'],
$wgFlowExternalStore ),
+ $c['storage.post.backend'],
+ $c['storage.post_summary.backend'],
$c['repository.tree']
);
};
@@ -662,6 +674,7 @@
return new UniqueFeatureIndex(
$c['memcache.local_buffered'],
$c['storage.topic_history.backend'],
+ $c['storage.topic_history.mapper'],
'flow_revision:v4:pk',
$c['storage.topic_history.primary_key']
);
@@ -670,6 +683,7 @@
return new TopicHistoryIndex(
$c['memcache.local_buffered'],
$c['storage.topic_history.backend'],
+ $c['storage.topic_history.mapper'],
$c['repository.tree'],
'flow_revision:topic:v2',
array( 'topic_root_id' ),
@@ -1048,7 +1062,7 @@
'ref_target_title'
);
$c['storage.wiki_reference.mapper'] = function( $c ) {
- return Flow\Data\Mapper\BasicObjectMapper::model(
+ return BasicObjectMapper::model(
$c['storage.wiki_reference.class']
);
};
@@ -1063,6 +1077,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.wiki_reference.backend'],
+ $c['storage.wiki_reference.mapper'],
'flow_ref:wiki:by-source:v3',
array(
'ref_src_wiki',
@@ -1079,6 +1094,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.wiki_reference.backend'],
+ $c['storage.wiki_reference.mapper'],
'flow_ref:wiki:by-revision:v3',
array(
'ref_src_wiki',
@@ -1106,7 +1122,8 @@
new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.wiki_reference.backend'],
- 'flow_ref:wiki:by-source',
+ $c['storage.wiki_reference.mapper'],
+ 'flow_ref:wiki:by-source:v2b',
array(
'ref_src_namespace',
'ref_src_title',
@@ -1119,7 +1136,8 @@
new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.wiki_reference.backend'],
- 'flow_ref:wiki:by-revision:v2',
+ $c['storage.wiki_reference.mapper'],
+ 'flow_ref:wiki:by-revision:v2b',
array(
'ref_src_object_type',
'ref_src_object_id',
@@ -1153,7 +1171,7 @@
'ref_target'
);
$c['storage.url_reference.mapper'] = function( $c ) {
- return Flow\Data\Mapper\BasicObjectMapper::model(
+ return BasicObjectMapper::model(
$c['storage.url_reference.class']
);
};
@@ -1170,6 +1188,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.url_reference.backend'],
+ $c['storage.url_reference.mapper'],
'flow_ref:url:by-source:v3',
array(
'ref_src_wiki',
@@ -1186,6 +1205,7 @@
return new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.url_reference.backend'],
+ $c['storage.url_reference.mapper'],
'flow_ref:url:by-revision:v3',
array(
'ref_src_wiki',
@@ -1210,7 +1230,8 @@
new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.url_reference.backend'],
- 'flow_ref:url:by-source',
+ $c['storage.url_reference.mapper'],
+ 'flow_ref:url:by-source:v2b',
array(
'ref_src_namespace',
'ref_src_title',
@@ -1223,7 +1244,8 @@
new TopKIndex(
$c['memcache.local_buffered'],
$c['storage.url_reference.backend'],
- 'flow_ref:url:by-revision:v2',
+ $c['storage.url_reference.mapper'],
+ 'flow_ref:url:by-revision:v2b',
array(
'ref_src_object_type',
'ref_src_object_id',
@@ -1280,7 +1302,8 @@
$c['reference.extractor'],
$c['reference.updater.links-tables'],
$c['storage'],
- $c['repository.tree']
+ $c['repository.tree'],
+ $c['deferred_queue']
);
};
diff --git a/includes/Data/Index/BoardHistoryIndex.php
b/includes/Data/Index/BoardHistoryIndex.php
index 9767f32..ef7f93f 100644
--- a/includes/Data/Index/BoardHistoryIndex.php
+++ b/includes/Data/Index/BoardHistoryIndex.php
@@ -4,6 +4,7 @@
use Flow\Data\BufferedCache;
use Flow\Data\ObjectManager;
+use Flow\Data\ObjectMapper;
use Flow\Data\Storage\BoardHistoryStorage;
use Flow\Exception\DataModelException;
use Flow\Exception\InvalidInputException;
@@ -30,6 +31,7 @@
public function __construct(
BufferedCache $cache,
BoardHistoryStorage $storage,
+ ObjectMapper $mapper,
$prefix,
array $indexed,
array $options = array(),
@@ -38,7 +40,7 @@
if ( $indexed !== array( 'topic_list_id' ) ) {
throw new DataModelException( __CLASS__ . ' is
hardcoded to only index topic_list_id: ' . print_r( $indexed, true ),
'process-data' );
}
- parent::__construct( $cache, $storage, $prefix, $indexed,
$options );
+ parent::__construct( $cache, $storage, $mapper, $prefix,
$indexed, $options );
$this->om = $om;
}
diff --git a/includes/Data/Index/FeatureIndex.php
b/includes/Data/Index/FeatureIndex.php
index a7a7f87..adda6c1 100644
--- a/includes/Data/Index/FeatureIndex.php
+++ b/includes/Data/Index/FeatureIndex.php
@@ -9,6 +9,7 @@
use Flow\Data\Compactor\ShallowCompactor;
use Flow\Data\Index;
use Flow\Data\ObjectManager;
+use Flow\Data\ObjectMapper;
use Flow\Data\ObjectStorage;
use Flow\Model\UUID;
use FormatJson;
@@ -28,6 +29,11 @@
* @var ObjectStorage
*/
protected $storage;
+
+ /**
+ * @var ObjectMapper
+ */
+ protected $mapper;
/**
* @var string
@@ -94,12 +100,14 @@
/**
* @param BufferedCache $cache
* @param ObjectStorage $storage
+ * @param ObjectMapper $mapper
* @param string $prefix Prefix to utilize for all cache keys
* @param array $indexedColumns List of columns to index,
*/
- public function __construct( BufferedCache $cache, ObjectStorage
$storage, $prefix, array $indexedColumns ) {
+ public function __construct( BufferedCache $cache, ObjectStorage
$storage, ObjectMapper $mapper, $prefix, array $indexedColumns ) {
$this->cache = $cache;
$this->storage = $storage;
+ $this->mapper = $mapper;
$this->prefix = $prefix;
$this->rowCompactor = new FeatureCompactor( $indexedColumns );
$this->indexed = $indexedColumns;
@@ -347,7 +355,8 @@
if ( !$indexed ) {
throw new DataModelException( 'Unindexable row: ' .
FormatJson::encode( $old ), 'process-data' );
}
- $this->removeFromIndex( $indexed, $old );
+ $compacted = $this->rowCompactor->compactRow(
UUID::convertUUIDs( $old, 'alphadecimal' ) );
+ $this->removeFromIndex( $indexed, $compacted );
}
/**
@@ -412,6 +421,11 @@
continue;
}
+ // normalize rows fetched from DB: schema may
be different than the
+ // real data (e.g. nullable columns no longer
used or in preparation
+ // for changes)
+ $rows = array_map( array( $this->mapper,
'normalizeRow' ), $rows );
+
$compacted = $this->rowCompactor->compactRows(
$rows );
$callback = function( \BagOStuff $cache, $key,
$value ) use ( $compacted ) {
if ( $value !== false ) {
diff --git a/includes/Data/Index/TopKIndex.php
b/includes/Data/Index/TopKIndex.php
index b79e93f..9d6b493 100644
--- a/includes/Data/Index/TopKIndex.php
+++ b/includes/Data/Index/TopKIndex.php
@@ -5,10 +5,12 @@
use BagOStuff;
use Flow\Data\BufferedCache;
use Flow\Data\ObjectManager;
+use Flow\Data\ObjectMapper;
use Flow\Data\ObjectStorage;
use Flow\Data\Compactor\ShallowCompactor;
use Flow\Data\Utils\SortArrayByKeys;
use Flow\Exception\InvalidInputException;
+use Flow\Model\UUID;
/**
* Holds the top k items with matching $indexed columns. List is sorted and
truncated to specified size.
@@ -19,12 +21,12 @@
*/
protected $options = array();
- public function __construct( BufferedCache $cache, ObjectStorage
$storage, $prefix, array $indexed, array $options = array() ) {
+ public function __construct( BufferedCache $cache, ObjectStorage
$storage, ObjectMapper $mapper, $prefix, array $indexed, array $options =
array() ) {
if ( empty( $options['sort'] ) ) {
throw new InvalidInputException( 'TopKIndex must be
sorted', 'invalid-input' );
}
- parent::__construct( $cache, $storage, $prefix, $indexed );
+ parent::__construct( $cache, $storage, $mapper, $prefix,
$indexed );
$this->options = $options + array(
'limit' => 500,
@@ -68,6 +70,7 @@
protected function addToIndex( array $indexed, array $row ) {
$self = $this;
+
// If this used redis instead of memcached, could it add to
index in position
// without retry possibility? need a single number that will
properly sort rows.
$this->cache->merge(
@@ -76,6 +79,26 @@
if ( $value === false ) {
return false;
}
+
+ if ( count( $value ) > 0 ) {
+ /*
+ * Ideally, we'd expand the rows
currently in cache, run them
+ * through $this->storage->normalize &
then re-compact them.
+ * And then we can reliably locate $row
in there.
+ * However, that may require additional
cache lookups for
+ * the expand info.
+ *
+ * Instead of doing that, Let's just
make the current row
+ * columns conform the rows in cache
($schema)
+ *
+ * This is mostly to fight useless
nullable columns in DB
+ * (either in preparation for schema
change or no longer needed)
+ * Meaningful changes in data will need
a cache key change, so
+ * we're good here.
+ */
+ $row = $self->normalizeCompressed(
$row, array_keys( reset( $value ) ) );
+ }
+
$idx = array_search( $row, $value );
if ( $idx !== false ) {
return false; // This row already
exists somehow
@@ -95,12 +118,20 @@
}
protected function removeFromIndex( array $indexed, array $row ) {
+ $self = $this;
+
$this->cache->merge(
$this->cacheKey( $indexed ),
- function( BagOStuff $cache, $key, $value ) use( $row ) {
+ function( BagOStuff $cache, $key, $value ) use( $self,
$row ) {
if ( $value === false ) {
return false;
}
+
+ if ( count( $value ) > 0 ) {
+ // see comment in self::addToIndex on
why to normalize
+ $row = $self->normalizeCompressed(
$row, array_keys( reset( $value ) ) );
+ }
+
$idx = array_search( $row, $value );
if ( $idx === false ) {
return false;
@@ -120,6 +151,13 @@
return false;
}
$retval = $value;
+
+ if ( count( $value ) > 0 ) {
+ // see comment in self::addToIndex on
why to normalize
+ $oldRow = $self->normalizeCompressed(
$oldRow, array_keys( reset( $value ) ) );
+ $newRow = $self->normalizeCompressed(
$newRow, array_keys( reset( $value ) ) );
+ }
+
$idx = array_search( $oldRow, $retval );
if ( $idx !== false ) {
unset( $retval[$idx] );
@@ -128,7 +166,7 @@
$retval = $self->sortIndex( $retval );
$retval = $self->limitIndexSize( $retval );
if ( $value === $retval ) {
- // new item didnt fit in index and old
item wasnt found in index
+ // new item didn't fit in index and old
item wasn't found in index
return false;
} else {
return $retval;
@@ -137,9 +175,31 @@
);
}
+ /**
+ * In order to be able to reliably find a row in an array of
+ * cached rows, we need to normalize those to make sure the
+ * columns match: they may be outdated.
+ *
+ * @param array $row Array in [column => value] format
+ * @param array $schema Array of column names to be present in $row
+ * @return array
+ */
+ // INTERNAL: in 5.4 it can be protected
+ public function normalizeCompressed( array $row, array $schema ) {
+ $schema = array_fill_keys( $schema, null );
+
+ // add null value for columns currently in cache
+ $row = array_merge( $schema, $row );
+
+ // remove unknown columns from the row
+ $row = array_intersect_key( $row, $schema );
+
+ return $row;
+ }
+
// INTERNAL: in 5.4 it can be protected
public function sortIndex( array $values ) {
- // I dont think this is a valid way to sort a 128bit integer
string
+ // I don't think this is a valid way to sort a 128bit integer
string
$callback = new SortArrayByKeys( $this->options['sort'], true );
/** @noinspection PhpParamsInspection */
usort( $values, $callback );
diff --git a/includes/Data/Index/TopicHistoryIndex.php
b/includes/Data/Index/TopicHistoryIndex.php
index 8fec832..c196c36 100644
--- a/includes/Data/Index/TopicHistoryIndex.php
+++ b/includes/Data/Index/TopicHistoryIndex.php
@@ -3,6 +3,7 @@
namespace Flow\Data\Index;
use Flow\Data\BufferedCache;
+use Flow\Data\ObjectMapper;
use Flow\Data\Storage\TopicHistoryStorage;
use Flow\Exception\InvalidInputException;
use Flow\Model\PostRevision;
@@ -19,11 +20,11 @@
protected $treeRepository;
- public function __construct( BufferedCache $cache, TopicHistoryStorage
$storage, TreeRepository $treeRepo, $prefix, array $indexed, array $options =
array() ) {
+ public function __construct( BufferedCache $cache, TopicHistoryStorage
$storage, ObjectMapper $mapper, TreeRepository $treeRepo, $prefix, array
$indexed, array $options = array() ) {
if ( $indexed !== array( 'topic_root_id' ) ) {
throw new \MWException( __CLASS__ . ' is hardcoded to
only index topic_root_id: ' . print_r( $indexed, true ) );
}
- parent::__construct( $cache, $storage, $prefix, $indexed,
$options );
+ parent::__construct( $cache, $storage, $mapper, $prefix,
$indexed, $options );
$this->treeRepository = $treeRepo;
}
@@ -77,7 +78,11 @@
if ( isset( $metadata['workflow'] ) && $metadata['workflow']
instanceof Workflow ) {
return $metadata['workflow']->getId();
} elseif ( $object instanceof PostRevision ) {
- return
$object->getRootPost()->getPostId()->getAlphadecimal();
+ // We used to figure out the root post from a
PostRevision object.
+ // Now we should just make sure we pass in the correct
metadata.
+ // Resolving workflow is intensive and at this point,
the data in
+ // storage may already have been deleted...
+ throw new InvalidInputException( 'Missing "workflow"
metadata: ' . get_class( $object ) );
} elseif ( $object instanceof PostSummary ) {
return
$object->getCollection()->getWorkflowId()->getAlphadecimal();
} else {
diff --git a/includes/Data/Listener/ReferenceRecorder.php
b/includes/Data/Listener/ReferenceRecorder.php
index 18be59c..99ceb05 100644
--- a/includes/Data/Listener/ReferenceRecorder.php
+++ b/includes/Data/Listener/ReferenceRecorder.php
@@ -14,6 +14,7 @@
use Flow\Model\Reference;
use Flow\Parsoid\ReferenceExtractor;
use Flow\Repository\TreeRepository;
+use SplQueue;
/**
* Listens for new revisions to be inserted. Calculates the difference in
@@ -42,16 +43,23 @@
*/
protected $treeRepository;
+ /**
+ * @var SplQueue
+ */
+ protected $deferredQueue;
+
public function __construct(
ReferenceExtractor $referenceExtractor,
LinksTableUpdater $linksTableUpdater,
ManagerGroup $storage,
- TreeRepository $treeRepository
+ TreeRepository $treeRepository,
+ SplQueue $deferredQueue
) {
$this->referenceExtractor = $referenceExtractor;
$this->linksTableUpdater = $linksTableUpdater;
$this->storage = $storage;
$this->treeRepository = $treeRepository;
+ $this->deferredQueue = $deferredQueue;
}
public function onAfterLoad( $object, array $old ) {
@@ -65,6 +73,7 @@
if ( !$revision instanceof AbstractRevision ) {
throw new InvalidDataException( 'ReferenceRecorder can
only attach to AbstractRevision storage');
}
+ /** @var Workflow $workflow */
$workflow = $metadata['workflow'];
if ( $revision instanceof PostRevision &&
$revision->isTopicTitle() ) {
@@ -76,8 +85,12 @@
$this->storage->multiPut( $added );
$this->storage->multiRemove( $removed );
- // Data updates
- $this->linksTableUpdater->doUpdate( $workflow );
+ // Data has not yet been committed at this point, so let's delay
+ // updating `categorylinks`, `externallinks`, etc.
+ $linksTableUpdater = $this->linksTableUpdater;
+ $this->deferredQueue->push( function() use (
$linksTableUpdater, $workflow ) {
+ $linksTableUpdater->doUpdate( $workflow );
+ } );
}
/**
diff --git a/includes/Data/Mapper/BasicObjectMapper.php
b/includes/Data/Mapper/BasicObjectMapper.php
index 4e74abf..4d01613 100644
--- a/includes/Data/Mapper/BasicObjectMapper.php
+++ b/includes/Data/Mapper/BasicObjectMapper.php
@@ -42,6 +42,9 @@
return null;
}
+ /**
+ * {@inheritDoc}
+ */
public function normalizeRow( array $row ) {
$object = $this->fromStorageRow( $row );
return $this->toStorageRow( $object );
diff --git a/includes/Data/Mapper/CachingObjectMapper.php
b/includes/Data/Mapper/CachingObjectMapper.php
index 87a42a9..bbb41f1 100644
--- a/includes/Data/Mapper/CachingObjectMapper.php
+++ b/includes/Data/Mapper/CachingObjectMapper.php
@@ -120,6 +120,9 @@
}
}
+ /**
+ * {@inheritDoc}
+ */
public function normalizeRow( array $row ) {
$object = call_user_func( $this->fromStorageRow, $row );
return call_user_func( $this->toStorageRow, $object );
diff --git a/includes/Data/Storage/BasicDbStorage.php
b/includes/Data/Storage/BasicDbStorage.php
index d1bb3ef..9c6be5f 100644
--- a/includes/Data/Storage/BasicDbStorage.php
+++ b/includes/Data/Storage/BasicDbStorage.php
@@ -164,7 +164,7 @@
public function findMulti( array $queries, array $options = array() ) {
$keys = array_keys( reset( $queries ) );
- $pks = $this->getPrimaryKeyColumns();
+ $pks = $this->getPrimaryKeyColumns();
if ( count( $keys ) !== count( $pks ) || array_diff( $keys,
$pks ) ) {
return $this->fallbackFindMulti( $queries, $options );
}
diff --git a/maintenance/FlowFixLinks.php b/maintenance/FlowFixLinks.php
new file mode 100644
index 0000000..6070b2b
--- /dev/null
+++ b/maintenance/FlowFixLinks.php
@@ -0,0 +1,72 @@
+<?php
+
+use Flow\Container;
+use Flow\LinksTableUpdater;
+use Flow\Model\Workflow;
+
+require_once ( getenv( 'MW_INSTALL_PATH' ) !== false
+ ? getenv( 'MW_INSTALL_PATH' ) . '/maintenance/Maintenance.php'
+ : dirname( __FILE__ ) . '/../../../maintenance/Maintenance.php' );
+// extending these - autoloader not yet wired up at the point these are
interpreted
+require_once( __DIR__ . '/../../../includes/utils/BatchRowWriter.php' );
+require_once( __DIR__ . '/../../../includes/utils/RowUpdateGenerator.php' );
+
+/**
+ * Fixes Flow entries in categorylinks & related tables.
+ *
+ * @ingroup Maintenance
+ */
+class FlowFixLinks extends LoggedUpdateMaintenance {
+ public function __construct() {
+ parent::__construct();
+
+ $this->mDescription = 'Fixes Flow entries in categorylinks &
related tables';
+
+ $this->setBatchSize( 300 );
+ }
+
+ protected function getUpdateKey() {
+ return __CLASS__;
+ }
+
+ protected function doDBUpdates() {
+ $dbw = wfGetDB( DB_MASTER );
+ $dbr = Container::get( 'db.factory' )->getDB( DB_SLAVE );
+ $linksTableUpdater = Container::get(
'reference.updater.links-tables' );
+
+ $iterator = new BatchRowIterator( $dbr, 'flow_workflow',
'workflow_id', $this->mBatchSize );
+ $iterator->setFetchColumns( array( '*' ) );
+ $iterator->addConditions( array( 'workflow_wiki' => wfWikiId()
) );
+
+ foreach ( $iterator as $rows ) {
+ $dbw->begin();
+
+ foreach ( $rows as $row ) {
+ $workflow = Workflow::fromStorageRow( (array)
$row );
+ $id =
$workflow->getArticleTitle()->getArticleID();
+
+ // delete existing links from DB
+ $dbw->delete( 'pagelinks', array( 'pl_from' =>
$id ), __METHOD__ );
+ $dbw->delete( 'imagelinks', array( 'il_from' =>
$id ), __METHOD__ );
+ $dbw->delete( 'categorylinks', array( 'cl_from'
=> $id ), __METHOD__ );
+ $dbw->delete( 'templatelinks', array( 'tl_from'
=> $id ), __METHOD__ );
+ $dbw->delete( 'externallinks', array( 'el_from'
=> $id ), __METHOD__ );
+ $dbw->delete( 'langlinks', array( 'll_from' =>
$id ), __METHOD__ );
+ $dbw->delete( 'iwlinks', array( 'iwl_from' =>
$id ), __METHOD__ );
+
+ // regenerate & store those links
+ $linksTableUpdater->doUpdate( $workflow );
+ }
+
+ $dbw->commit();
+ wfWaitForSlaves();
+ }
+
+ $this->output( "Completed\n" );
+
+ return true;
+ }
+}
+
+$maintClass = 'FlowFixLinks';
+require_once( RUN_MAINTENANCE_IF_MAIN );
diff --git a/tests/phpunit/Data/Index/FeatureIndexTest.php
b/tests/phpunit/Data/Index/FeatureIndexTest.php
index c2e4105..faa8b26 100644
--- a/tests/phpunit/Data/Index/FeatureIndexTest.php
+++ b/tests/phpunit/Data/Index/FeatureIndexTest.php
@@ -3,6 +3,7 @@
namespace Flow\Tests\Data\Index;
use Flow\Data\Index\FeatureIndex;
+use Flow\Data\ObjectMapper;
/**
* @group Flow
@@ -17,6 +18,13 @@
$storage = $this->getMockBuilder( 'Flow\Data\ObjectStorage' )
->disableOriginalConstructor()
->getMock();
+ // fake ObjectMapper that doesn't roundtrip to- & fromStorageRow
+ $mapper = $this->getMockBuilder(
'Flow\Data\Mapper\BasicObjectMapper' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mapper->expects( $this->any() )
+ ->method( 'normalizeRow' )
+ ->will( $this->returnArgument( 0 ) );
$dbId = FeatureIndex::cachedDbId();
$cache->expects( $this->any() )
@@ -33,7 +41,7 @@
$storage->expects( $this->never() )
->method( 'findMulti' );
- $index = new MockFeatureIndex( $cache, $storage, 'foo', array(
'bar' ) );
+ $index = new MockFeatureIndex( $cache, $storage, $mapper,
'foo', array( 'bar' ) );
$res = $index->find(
array( 'bar' => 5 ),
@@ -73,8 +81,15 @@
) ) );
$storage->expects( $this->never() )
->method( 'findMulti' );
+ // fake ObjectMapper that doesn't roundtrip to- & fromStorageRow
+ $mapper = $this->getMockBuilder(
'Flow\Data\Mapper\BasicObjectMapper' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mapper->expects( $this->any() )
+ ->method( 'normalizeRow' )
+ ->will( $this->returnArgument( 0 ) );
- $index = new MockFeatureIndex( $cache, $storage, 'foo', array(
'bar' ) );
+ $index = new MockFeatureIndex( $cache, $storage, $mapper,
'foo', array( 'bar' ) );
$res = $index->find(
array( 'bar' => 5 ),
diff --git a/tests/phpunit/Data/IndexTest.php b/tests/phpunit/Data/IndexTest.php
index 02f6949..d9665fc 100644
--- a/tests/phpunit/Data/IndexTest.php
+++ b/tests/phpunit/Data/IndexTest.php
@@ -21,17 +21,25 @@
$bag = new BufferedBagOStuff( new \HashBagOStuff );
$cache = new BufferedCache( $bag, $wgFlowCacheTime );
+ // fake ObjectMapper that doesn't roundtrip to- & fromStorageRow
+ $mapper = $this->getMockBuilder(
'Flow\Data\Mapper\BasicObjectMapper' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mapper->expects( $this->any() )
+ ->method( 'normalizeRow' )
+ ->will( $this->returnArgument( 0 ) );
+
// As we are only testing the cached result, storage should
never be called
// not sure how to test that
$storage = $this->getMock( 'Flow\\Data\\ObjectStorage' );
$unique = new UniqueFeatureIndex(
- $cache, $storage, 'unique',
+ $cache, $storage, $mapper, 'unique',
array( 'id' )
);
$secondary = new TopKIndex(
- $cache, $storage, 'secondary',
+ $cache, $storage, $mapper, 'secondary',
array( 'name' ), // keys indexed in this array
array(
'shallow' => $unique,
@@ -67,13 +75,21 @@
$cache = new BufferedCache( $bag, $wgFlowCacheTime );
$storage = $this->getMock( 'Flow\\Data\\ObjectStorage' );
+ // fake ObjectMapper that doesn't roundtrip to- & fromStorageRow
+ $mapper = $this->getMockBuilder(
'Flow\Data\Mapper\BasicObjectMapper' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mapper->expects( $this->any() )
+ ->method( 'normalizeRow' )
+ ->will( $this->returnArgument( 0 ) );
+
$unique = new UniqueFeatureIndex(
- $cache, $storage, 'unique',
+ $cache, $storage, $mapper, 'unique',
array( 'id', 'ot' )
);
$secondary = new TopKIndex(
- $cache, $storage, 'secondary',
+ $cache, $storage, $mapper, 'secondary',
array( 'name' ), // keys indexed in this array
array(
'shallow' => $unique,
diff --git a/tests/phpunit/Data/Pager/PagerTest.php
b/tests/phpunit/Data/Pager/PagerTest.php
index 038bfcf..b24f436 100644
--- a/tests/phpunit/Data/Pager/PagerTest.php
+++ b/tests/phpunit/Data/Pager/PagerTest.php
@@ -460,10 +460,18 @@
) );
$storage = $this->getMock( 'Flow\Data\ObjectStorage' );
+ // fake ObjectMapper that doesn't roundtrip to- & fromStorageRow
+ $mapper = $this->getMockBuilder(
'Flow\Data\Mapper\BasicObjectMapper' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mapper->expects( $this->any() )
+ ->method( 'normalizeRow' )
+ ->will( $this->returnArgument( 0 ) );
$index = new TopKIndex(
$cache,
$storage,
+ $mapper,
'prefix',
array( 'foo' ),
array(
diff --git a/tests/phpunit/PostRevisionTestCase.php
b/tests/phpunit/PostRevisionTestCase.php
index b9eff75..81c8099 100644
--- a/tests/phpunit/PostRevisionTestCase.php
+++ b/tests/phpunit/PostRevisionTestCase.php
@@ -60,7 +60,8 @@
foreach ( $this->revisions as $revision ) {
try {
- $this->getStorage()->multiRemove( array(
$revision ) );
+ $workflow =
$revision->getCollection()->getWorkflow();
+ $this->getStorage()->multiRemove( array(
$revision ), array( 'workflow' => $workflow ) );
} catch ( \MWException $e ) {
// ignore - lifecyclehandlers may cause issues
with tests, where
// not all related stuff is loaded
diff --git a/tests/phpunit/api/ApiFlowViewTopicListTest.php
b/tests/phpunit/api/ApiFlowViewTopicListTest.php
index 4f275b9..d9d6f75 100644
--- a/tests/phpunit/api/ApiFlowViewTopicListTest.php
+++ b/tests/phpunit/api/ApiFlowViewTopicListTest.php
@@ -168,7 +168,7 @@
// Make it so update order is chronologically (1, 0, 2)
// We then expect it to be returned reverse chronologically (2,
0)
- $updateList = array( 1, 0, 2);
+ $updateList = array( 1, 0, 2 );
foreach ( $updateList as $updateListInd => $topicDataInd ) {
$replyResponse = $this->doApiRequest(
--
To view, visit https://gerrit.wikimedia.org/r/233432
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I23147fd711425e770369f092ad25d0de2d354c2f
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org>
Gerrit-Reviewer: Mattflaschen <mflasc...@wikimedia.org>
Gerrit-Reviewer: Matthias Mullie <mmul...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
