jenkins-bot has submitted this change and it was merged.
Change subject: Track the root of a post tree explicitly
......................................................................
Track the root of a post tree explicitly
Querying the TreeRepository from the TopicHistoryIndex is open
to slave-lag issues, the just-added post may not actually
be available in the slave yet.
Anything we are saving we already know the root for, we just didn't
have the information available in the TopicHistoryIndex. This patch
adds another field, rootPost, which is set when loading the posts. That
allows the information to make it into the TopicHistoryIndex
and remove the need for the TreeRepository there.
Bug: 58996
Bug: 59803
Bug: 59804
Bug: 59642
Change-Id: I23854ac0e36d8b892eba0d1202c1fd81dc05bd6e
---
M container.php
M includes/Block/Topic.php
M includes/Data/RecentChanges.php
M includes/Data/RevisionStorage.php
M includes/Data/RootPostLoader.php
M includes/Log/PostModerationLogger.php
M includes/Model/PostRevision.php
M includes/View/History/HistoryRenderer.php
8 files changed, 71 insertions(+), 42 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/container.php b/container.php
index be03b55..6db1690 100644
--- a/container.php
+++ b/container.php
@@ -311,7 +311,7 @@
// topic history -- to keep a history by topic we have to know
what topic every post
// belongs to, not just its parent. TopicHistoryIndex is a
slight tweak to TopKIndex
// using TreeRepository for extra information and stuffing it
into topic_root while indexing
- new TopicHistoryIndex( $cache, $storage, $c['repository.tree'],
'flow_revision:topic',
+ new TopicHistoryIndex( $cache, $storage, 'flow_revision:topic',
array( 'topic_root_id' ),
array(
'limit' => 500,
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index 5b86f97..2a0feed 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -869,6 +869,7 @@
// looking for loadRootPost
$this->topicTitle->setChildren( array() );
$this->topicTitle->setDepth( 0 );
+ $this->topicTitle->setRootPost( $this->topicTitle );
if ( !$this->permissions->isAllowed( $this->topicTitle,
'view' ) ) {
$this->topicTitle = null;
@@ -931,6 +932,7 @@
// using the path to the root post, we can know the
post's depth
$rootPath = $this->rootLoader->treeRepo->findRootPath(
$postId );
$post->setDepth( count( $rootPath ) - 1 );
+ $post->setRootPost( $found['root'] );
}
if ( $this->permissions->isAllowed( $topicTitle, 'view' )
@@ -974,13 +976,16 @@
if ( !$found ) {
throw new InvalidInputException( 'Should have found
revisions', 'missing-revision' );
}
- // Because storage returns a new object for every query
- // We need to find $post in the array and replace it
$revId = $post->getRevisionId();
+ $rootPost = $post->getRootPost();
foreach ( $found as $idx => $revision ) {
if ( $revId->equals( $revision->getRevisionId() ) ) {
+ // Because storage returns a new object for
every query
+ // We need to find $post in the array and
replace it
$found[$idx] = $post;
- break;
+ } else {
+ // Root post needs to propogate from $post to
found revisions
+ $revision->setRootPost( $rootPost );
}
}
return $found;
diff --git a/includes/Data/RecentChanges.php b/includes/Data/RecentChanges.php
index 5c43ee2..8b86223 100644
--- a/includes/Data/RecentChanges.php
+++ b/includes/Data/RecentChanges.php
@@ -158,12 +158,8 @@
}
public function onAfterInsert( $object, array $row ) {
- // There might be a more efficient way to get this workflow id
- $workflowId = $this->tree->findRoot( $object->getPostId() );
- if ( !$workflowId ) {
- wfWarn( __METHOD__ . ": could not locate root for post
" . $object->getPostId()->getHex() );
- return;
- }
+ // The workflow id is the same as the root's post id
+ $workflowId = $object->getRootPost()->getPostId();
// These are likely already in the in-process cache
$workflow = $this->storage->get( 'Workflow', $workflowId );
if ( !$workflow ) {
@@ -187,23 +183,7 @@
}
protected function getTopicTitle( PostRevision $rev ) {
- if ( $rev->isTopicTitle() ) {
- return $rev->getContent( 'wikitext' );
- }
- $topicTitleId = $this->tree->findRoot( $rev->getPostId() );
- if ( $topicTitleId === null ) {
- return null;
- }
- $found = $this->storage->find(
- 'PostRevision',
- array( 'tree_rev_descendant_id' => $topicTitleId ),
- array( 'sort' => 'rev_id', 'order' => 'DESC', 'limit'
=> 1 )
- );
- if ( !$found ) {
- return null;
- }
-
- $content = reset( $found )->getContent( 'wikitext' );
+ $content = $rev->getRootPost()->getContent( 'wikitext' );
if ( is_object( $content ) ) {
// moderated
return null;
diff --git a/includes/Data/RevisionStorage.php
b/includes/Data/RevisionStorage.php
index e21441f..be4e1a0 100644
--- a/includes/Data/RevisionStorage.php
+++ b/includes/Data/RevisionStorage.php
@@ -513,26 +513,25 @@
protected $treeRepository;
- public function __construct( BufferedCache $cache, PostRevisionStorage
$storage, TreeRepository $treeRepo, $prefix, array $indexed, array $options =
array() ) {
+ public function __construct( BufferedCache $cache, PostRevisionStorage
$storage, $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 );
- $this->treeRepository = $treeRepo;
}
public function onAfterInsert( $object, array $new ) {
- $new['topic_root_id'] = $this->treeRepository->findRoot(
UUID::create( $new['tree_rev_descendant_id'] ) )->getBinary();
+ $new['topic_root_id'] =
$object->getRootPost()->getPostId()->getBinary();
parent::onAfterInsert( $object, $new );
}
public function onAfterUpdate( $object, array $old, array $new ) {
- $old['topic_root_id'] = $new['topic_root_id'] =
$this->treeRepository->findRoot( UUID::create( $old['tree_rev_descendant_id'] )
)->getBinary();
+ $old['topic_root_id'] = $new['topic_root_id'] =
$object->getRootPost()->getPostId()->getBinary();
parent::onAfterUpdate( $object, $old, $new );
}
public function onAfterRemove( $object, array $old ) {
- $old['topic_root_id'] = $this->treeRepository->findRoot(
UUID::create( $old['tree_rev_descendant_id'] ) );
+ $old['topic_root_id'] =
$object->getRootPost()->getPostId()->getBinary();
parent::onAfterRemove( $object, $old );
}
diff --git a/includes/Data/RootPostLoader.php b/includes/Data/RootPostLoader.php
index c01a285..f43a4d6 100644
--- a/includes/Data/RootPostLoader.php
+++ b/includes/Data/RootPostLoader.php
@@ -11,7 +11,9 @@
/**
* I'm pretty sure this will generally work for any subtree, not just the topic
- * root. The problem is once you allow any subtree you need to handle things
like
+ * root. The problem is once you allow any subtree you need to handle the
+ * depth and root post setters better, they make the assumption the root
provided
+ * is actually a root.
*/
class RootPostLoader {
public function __construct( ManagerGroup $storage, TreeRepository
$treeRepo ) {
@@ -168,6 +170,11 @@
foreach ( $topicIds as $id ) {
$roots[$id->getHex()] = $posts[$id->getHex()];
}
+ // Attach every post in the tree to its root. setRootPost
+ // recursivly applies it to all children as well.
+ foreach ( $roots as $hex => $post ) {
+ $post->setRootPost( $post );
+ }
return $roots;
}
diff --git a/includes/Log/PostModerationLogger.php
b/includes/Log/PostModerationLogger.php
index 29ffc89..4a56234 100644
--- a/includes/Log/PostModerationLogger.php
+++ b/includes/Log/PostModerationLogger.php
@@ -23,9 +23,8 @@
}
if ( $this->logger->canLog( $object, $object->getChangeType() )
) {
- // This is awful but it's all I can think of
- $rootPost = $this->treeRepo->findRoot(
$object->getPostId() );
- $workflow = $this->storage->get( 'Workflow', $rootPost
);
+ $rootPostId = $object->getRootPost()->getPostId();
+ $workflow = $this->storage->get( 'Workflow',
$rootPostId );
$logParams = array();
if ( $object->isTopicTitle() ) {
@@ -74,4 +73,4 @@
return $changeTypes;
}
-}
\ No newline at end of file
+}
diff --git a/includes/Model/PostRevision.php b/includes/Model/PostRevision.php
index 01cc800..023d602 100644
--- a/includes/Model/PostRevision.php
+++ b/includes/Model/PostRevision.php
@@ -17,9 +17,22 @@
protected $origUserIp;
protected $replyToId;
- // Data that is loaded externally and set
+ /**
+ * @var array|null Optionally loaded list of children for this post.
+ */
protected $children;
+
+ /**
+ * @var int|null Optionally loaded distance of this post from the
+ * root of this post tree.
+ */
protected $depth;
+
+ /**
+ * @var PostRevision|null Optionally loaded root of this posts tree.
+ * This is always a topic title.
+ */
+ protected $rootPost;
/**
* Variables to which callback functions and their results will be
saved.
@@ -57,9 +70,11 @@
$obj = static::newFromId( $topic->getId(), $user, $content );
$obj->changeType = 'new-post';
- // A newly created post has no children and a depth of 0
+ // A newly created post has no children, a depth of 0, and
+ // is the root of its tree.
$obj->setChildren( array() );
$obj->setDepth( 0 );
+ $obj->rootPost = $obj;
return $obj;
}
@@ -139,6 +154,7 @@
$reply->changeType = $changeType;
$reply->setChildren( array() );
$reply->setDepth( $this->getDepth() + 1 );
+ $reply->rootPost = $this->rootPost;
return $reply;
}
@@ -180,6 +196,10 @@
public function setChildren( array $children ) {
$this->children = $children;
+ if ( $this->rootPost ) {
+ // Propagate root post into children.
+ $this->setRootPost( $this->rootPost );
+ }
}
public function getChildren() {
@@ -190,7 +210,7 @@
}
public function setDepth( $depth ) {
- $this->depth = $depth;
+ $this->depth = (int)$depth;
}
public function getDepth() {
@@ -200,6 +220,25 @@
return $this->depth;
}
+ public function setRootPost( PostRevision $root ) {
+ $this->rootPost = $root;
+ if ( $this->children ) {
+ // Propagate root post into children.
+ foreach ( $this->children as $child ) {
+ $child->setRootPost( $root );
+ }
+ }
+ }
+
+ public function getRootPost() {
+ if ( $this->isTopicTitle() ) {
+ return $this;
+ } elseif ( $this->rootPost === null ) {
+ throw new DataModelException( 'Root not loaded for
post: ' . $this->postId->getHex(), 'process-data' );
+ }
+ return $this->rootPost;
+ }
+
/**
* Get the amount of posts in this topic.
*
diff --git a/includes/View/History/HistoryRenderer.php
b/includes/View/History/HistoryRenderer.php
index 01da775..2335af4 100644
--- a/includes/View/History/HistoryRenderer.php
+++ b/includes/View/History/HistoryRenderer.php
@@ -196,9 +196,9 @@
} else {
$revision = $record->getRevision();
if ( isset(
$this->workflows[$revision->getRevisionId()->getHex()] ) ) {
- $workFlowId =
$this->workflows[$revision->getRevisionId()->getHex()];
+ $workflowId =
$this->workflows[$revision->getRevisionId()->getHex()];
$historicalLink =
$this->templating->getUrlGenerator()->generateBlockUrl(
- $workFlowId,
+ $workflowId,
$revision,
true
);
--
To view, visit https://gerrit.wikimedia.org/r/106449
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I23854ac0e36d8b892eba0d1202c1fd81dc05bd6e
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Bsitu <bs...@wikimedia.org>
Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Matthias Mullie <mmul...@wikimedia.org>
Gerrit-Reviewer: Spage <sp...@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
