jenkins-bot has submitted this change and it was merged.
Change subject: Hygiene: Use commit metadata in one-off indexes
......................................................................
Hygiene: Use commit metadata in one-off indexes
Since we added the commit metadata, including the topic workflow, passed
on into the listeners we no longer need to do as much work to determine
the extra rows these indexes add.
The patch utilizes the 'workflow' parameter of the commit metadata where it
can to avoid doing more complex lookups. Due to the recent addition of
Index::cachePurge() commit metadata is not always available and we cannot
kill the old code.
Change-Id: Ic1485c784bab24ea23e53535e2f465ae5f2a934d
---
M container.php
M includes/Data/Index/BoardHistoryIndex.php
M includes/Data/Index/TopicHistoryIndex.php
M tests/phpunit/PostRevisionTestCase.php
4 files changed, 87 insertions(+), 57 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/container.php b/container.php
index 00c70f0..1188e1c 100644
--- a/container.php
+++ b/container.php
@@ -184,13 +184,22 @@
} );
$c['storage.board_history.index'] = $c->share( function( $c ) {
- return new BoardHistoryIndex( $c['memcache.buffered'],
$c['storage.board_history.backing'], 'flow_revision:topic_list_history',
+ return new BoardHistoryIndex(
+ $c['memcache.buffered'],
+ // backend storage
+ $c['storage.board_history.backing'],
+ // key prefix
+ 'flow_revision:topic_list_history',
+ // primary key
array( 'topic_list_id' ),
+ // index options
array(
'limit' => 500,
'sort' => 'rev_id',
'order' => 'DESC'
- ) );
+ ),
+ $c['storage.topic_list']
+ );
} );
$c['storage.board_history'] = $c->share( function( $c ) {
diff --git a/includes/Data/Index/BoardHistoryIndex.php
b/includes/Data/Index/BoardHistoryIndex.php
index 4baeda6..c904748 100644
--- a/includes/Data/Index/BoardHistoryIndex.php
+++ b/includes/Data/Index/BoardHistoryIndex.php
@@ -4,9 +4,10 @@
use Flow\Container;
use Flow\Data\BufferedCache;
-use Flow\Data\ManagerGroup;
+use Flow\Data\ObjectManager;
use Flow\Data\Storage\BoardHistoryStorage;
use Flow\Exception\DataModelException;
+use Flow\Exception\FlowException;
use Flow\Exception\InvalidInputException;
use Flow\Model\AbstractRevision;
use Flow\Model\Header;
@@ -16,32 +17,49 @@
/**
* Keeps a list of revision ids relevant to the board history bucketed
- * by the owning TopicList id (board workflow)
+ * by the owning TopicList id (board workflow).
+ *
+ * Can be used with Header, PostRevision and PostSummary ObjectMapper's
*/
class BoardHistoryIndex extends TopKIndex {
- public function __construct( BufferedCache $cache, BoardHistoryStorage
$storage, $prefix, array $indexed, array $options = array() ) {
+ /**
+ * @var ObjectManager Manager for the TopicListEntry model
+ */
+ protected $om;
+
+ public function __construct(
+ BufferedCache $cache,
+ BoardHistoryStorage $storage,
+ $prefix,
+ array $indexed,
+ array $options = array(),
+ ObjectManager $om
+ ) {
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 );
+ $this->om = $om;
}
public function findMulti( array $queries, array $options = array() ) {
if ( count( $queries ) > 1 ) {
+ // why?
throw new DataModelException( __METHOD__ . ' expects
only one value in $queries', 'process-data' );
}
return parent::findMulti( $queries, $options );
}
+ /**
+ * @param array $queries
+ * @return array
+ */
public function backingStoreFindMulti( array $queries ) {
- $options = $this->queryOptions();
- $res = $this->storage->findMulti( $queries, $options );
- if ( !$res ) {
- return array();
- }
-
- return $res;
+ return $this->storage->findMulti(
+ $queries,
+ $this->queryOptions()
+ ) ?: array();
}
/**
@@ -49,10 +67,8 @@
* @param string[] $row
*/
public function cachePurge( $object, array $row ) {
- $row['topic_list_id'] = $this->findTopicListId( $object, $row );
- if ( $row['topic_list_id'] ) {
- parent::cachePurge( $object, $row );
- }
+ $row['topic_list_id'] = $this->findTopicListId( $object, $row,
array() );
+ parent::cachePurge( $object, $row );
}
/**
@@ -61,10 +77,8 @@
* @param array $metadata
*/
public function onAfterInsert( $object, array $new, array $metadata ) {
- $new['topic_list_id'] = $this->findTopicListId( $object, $new );
- if ( $new['topic_list_id'] ) {
- parent::onAfterInsert( $object, $new, $metadata );
- }
+ $new['topic_list_id'] = $this->findTopicListId( $object, $new,
$metadata );
+ parent::onAfterInsert( $object, $new, $metadata );
}
/**
@@ -74,10 +88,8 @@
* @param array $metadata
*/
public function onAfterUpdate( $object, array $old, array $new, array
$metadata ) {
- $new['topic_list_id'] = $old['topic_list_id'] =
$this->findTopicListId( $object, $new );
- if ( $new['topic_list_id'] ) {
- parent::onAfterUpdate( $object, $old, $new, $metadata );
- }
+ $new['topic_list_id'] = $old['topic_list_id'] =
$this->findTopicListId( $object, $new, $metadata );
+ parent::onAfterUpdate( $object, $old, $new, $metadata );
}
/**
@@ -86,46 +98,49 @@
* @param array $metadata
*/
public function onAfterRemove( $object, array $old, array $metadata ) {
- $old['topic_list_id'] = $this->findTopicListId( $object, $old );
- if ( $old['topic_list_id'] ) {
- parent::onAfterRemove( $object, $old, $metadata );
- }
+ $old['topic_list_id'] = $this->findTopicListId( $object, $old,
$metadata );
+ parent::onAfterRemove( $object, $old, $metadata );
}
/**
* Find a topic list id related to an abstract revision
*
* @param AbstractRevision $object
- * @param array $row
- * @return string|false Alphadecimal uid of the related board. False
when object is not root post or topic is not found
- * @throws InvalidInputException when $object is not a Header,
PostRevision or
+ * @param string[] $row
+ * @param array $metadata
+ * @return string Alphadecimal uid of the related board
+ * @throws InvalidInputException When $object is not a Header,
PostRevision or
* PostSummary instance.
+ * @throws DataModelException When the related id cannot be located
*/
- protected function findTopicListId( AbstractRevision $object, array
$row ) {
+ protected function findTopicListId( AbstractRevision $object, array
$row, array $metadata ) {
if ( $object instanceof Header ) {
return $row['rev_type_id'];
}
- if ( $object instanceof PostRevision ) {
- $post = $object;
- } elseif ( $object instanceof PostSummary ) {
- $post =
$object->getCollection()->getPost()->getLastRevision();
+ if ( isset( $metadata['workflow'] ) ) {
+ $topicId = $metadata['workflow']->getId();
} else {
- throw new InvalidInputException( 'Unexpected object
type: ' . get_class( $object ) );
+ if ( $object instanceof PostRevision ) {
+ $post = $object;
+ } elseif ( $object instanceof PostSummary ) {
+ $post =
$object->getCollection()->getPost()->getLastRevision();
+ } else {
+ throw new InvalidInputException( 'Unexpected
object type: ' . get_class( $object ) );
+ }
+ $topicId = $post->getRootPost()->getPostId();
}
- /** @var ManagerGroup $storage */
- $storage = Container::get( 'storage' );
- $found = $storage->find(
- 'TopicListEntry',
- array( 'topic_id' => $post->getRootPost()->getPostId() )
- );
- if ( $found ) {
- /** @var TopicListEntry $topicListEntry */
- $topicListEntry = reset( $found );
- return $topicListEntry->getListId()->getAlphadecimal();
- } else {
- return false;
+ $found = $this->om->find( array( 'topic_id' => $topicId ) );
+ if ( !$found ) {
+ throw new DataModelException(
+ "No topic list contains topic " .
$topicId->getAlphadecimal() .
+ ", called for revision " .
$object->getRevisionId()->getAlphadecimal()
+ );
}
+
+ /** @var TopicListEntry $topicListEntry */
+ $topicListEntry = reset( $found );
+ return $topicListEntry->getListId()->getAlphadecimal();
}
}
diff --git a/includes/Data/Index/TopicHistoryIndex.php
b/includes/Data/Index/TopicHistoryIndex.php
index 4cb52df..94ca1d6 100644
--- a/includes/Data/Index/TopicHistoryIndex.php
+++ b/includes/Data/Index/TopicHistoryIndex.php
@@ -31,7 +31,7 @@
* @param array $row
*/
public function cachePurge( $object, array $row ) {
- $row['topic_root_id'] = $this->findTopicRootId( $object );
+ $row['topic_root_id'] = $this->findTopicRootId( $object,
array() );
parent::cachePurge( $object, $row );
}
@@ -41,7 +41,7 @@
* @param array $metadata
*/
public function onAfterInsert( $object, array $new, array $metadata ) {
- $new['topic_root_id'] = $this->findTopicRootId( $object );
+ $new['topic_root_id'] = $this->findTopicRootId( $object,
$metadata );
parent::onAfterInsert( $object, $new, $metadata );
}
@@ -52,7 +52,7 @@
* @param array $metadata
*/
public function onAfterUpdate( $object, array $old, array $new, array
$metadata ) {
- $old['topic_root_id'] = $new['topic_root_id'] =
$this->findTopicRootId( $object );
+ $old['topic_root_id'] = $new['topic_root_id'] =
$this->findTopicRootId( $object, $metadata );
parent::onAfterUpdate( $object, $old, $new, $metadata );
}
@@ -62,7 +62,7 @@
* @param array $metadata
*/
public function onAfterRemove( $object, array $old, array $metadata ) {
- $old['topic_root_id'] = $this->findTopicRootId( $object );
+ $old['topic_root_id'] = $this->findTopicRootId( $object,
$metadata );
parent::onAfterRemove( $object, $old, $metadata );
}
@@ -71,8 +71,10 @@
* @return string alphadecimal uuid
* @throws InvalidInputException When $object is not PostRevision or
PostSummary
*/
- protected function findTopicRootId( $object ) {
- if ( $object instanceof PostRevision ) {
+ protected function findTopicRootId( $object, array $metadata ) {
+ if ( isset( $metadata['workflow'] ) ) {
+ return $metadata['workflow']->getId();
+ } elseif ( $object instanceof PostRevision ) {
return
$object->getRootPost()->getPostId()->getAlphadecimal();
} elseif ( $object instanceof PostSummary ) {
return
$object->getCollection()->getWorkflowId()->getAlphadecimal();
diff --git a/tests/phpunit/PostRevisionTestCase.php
b/tests/phpunit/PostRevisionTestCase.php
index 9b37897..a155938 100644
--- a/tests/phpunit/PostRevisionTestCase.php
+++ b/tests/phpunit/PostRevisionTestCase.php
@@ -3,7 +3,8 @@
namespace Flow\Tests;
use Flow\Container;
-use Flow\Data\NotificationListener;
+use Flow\Data\Index\BoardHistoryIndex;
+use Flow\Data\Listener\NotificationListener;
use Flow\Data\ObjectManager;
use Flow\Data\RecentChanges\RecentChanges as RecentChangesListener;
use Flow\Model\AbstractRevision;
@@ -202,7 +203,10 @@
return !$handler instanceof
RecentChangesListener
// putting together the right
metadata for a commit is beyond the
// scope of these tests
- && !$handler instanceof
NotificationListener;
+ && !$handler instanceof
NotificationListener
+ // BoardHistory requires we
also wire together TopicListEntry objects for
+ // each revision, but thats
also beyond our scope.
+ && !$handler instanceof
BoardHistoryIndex;
}
);
}
--
To view, visit https://gerrit.wikimedia.org/r/168549
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic1485c784bab24ea23e53535e2f465ae5f2a934d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: SG <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
