Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/239835
Change subject: Pre-format comments in ChangeHandler
......................................................................
Pre-format comments in ChangeHandler
This avoids re-parsing comments when displaying RecentChanges or the
Watchlist. Formatting edit summaries may involve lookupas for entity
labels, potenitally causing performance issues.
Change-Id: I5439a76ceb626a7a8f3195bb3001bb9e2f6cb233
---
M client/includes/Changes/ChangeHandler.php
M client/includes/WikibaseClient.php
M client/includes/recentchanges/ChangeLineFormatter.php
M client/includes/recentchanges/ExternalChange.php
M client/includes/recentchanges/ExternalChangeFactory.php
M client/includes/recentchanges/ExternalRecentChange.php
M client/includes/recentchanges/RevisionData.php
M client/tests/phpunit/includes/Changes/ChangeHandlerTest.php
M client/tests/phpunit/includes/recentchanges/ChangeLineFormatterTest.php
M client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php
10 files changed, 165 insertions(+), 56 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/35/239835/1
diff --git a/client/includes/Changes/ChangeHandler.php
b/client/includes/Changes/ChangeHandler.php
index 5e6f372..df3484b 100644
--- a/client/includes/Changes/ChangeHandler.php
+++ b/client/includes/Changes/ChangeHandler.php
@@ -88,6 +88,11 @@
private $localSiteId;
/**
+ * @var string
+ */
+ private $repoId;
+
+ /**
* @var bool
*/
private $injectRecentChanges;
@@ -99,8 +104,8 @@
* @param ChangeListTransformer $changeListTransformer
* @param Language $language
* @param string $localSiteId
+ * @param string $repoId
* @param bool $injectRecentChanges
- *
*/
public function __construct(
AffectedPagesFinder $affectedPagesFinder,
@@ -109,6 +114,7 @@
ChangeListTransformer $changeListTransformer,
Language $language,
$localSiteId,
+ $repoId,
$injectRecentChanges = true
) {
if ( !is_string( $localSiteId ) ) {
@@ -125,6 +131,7 @@
$this->changeListTransformer = $changeListTransformer;
$this->language = $language;
$this->localSiteId = $localSiteId;
+ $this->repoId = $repoId;
$this->injectRecentChanges = $injectRecentChanges;
}
@@ -345,6 +352,10 @@
unset( $fields['info'] );
$changeParams = array_merge( $fields, $rcinfo );
+ if ( !isset( $changeParams['site_id'] ) ) {
+ $changeParams['site_id'] = $this->repoId;
+ }
+
$comment = $this->getEditCommentMulti( $changesForComment );
// Use keys known to ExternalRecentChange::buildAttributes.
@@ -377,7 +388,7 @@
if ( $c === 0 ) {
return '';
} elseif ( $c === 1 ) {
- return $comments[1];
+ return reset( $comments );
} else {
//@todo: handle overly long lists nicely!
return $this->language->semicolonList( $comments );
diff --git a/client/includes/WikibaseClient.php
b/client/includes/WikibaseClient.php
index e282ccb..7d66b02 100644
--- a/client/includes/WikibaseClient.php
+++ b/client/includes/WikibaseClient.php
@@ -895,6 +895,7 @@
*/
public function getChangeHandler() {
$siteId = $this->getSite()->getGlobalId();
+ $repoId = $this->settings->getSetting( 'repoSiteId' );
return new ChangeHandler(
$this->getAffectedPagesFinder(),
@@ -907,6 +908,7 @@
),
$this->getContentLanguage(),
$siteId,
+ $repoId,
$this->settings->getSetting( 'injectRecentChanges' )
);
}
diff --git a/client/includes/recentchanges/ChangeLineFormatter.php
b/client/includes/recentchanges/ChangeLineFormatter.php
index 6c6db6d..d290689 100644
--- a/client/includes/recentchanges/ChangeLineFormatter.php
+++ b/client/includes/recentchanges/ChangeLineFormatter.php
@@ -75,12 +75,30 @@
$line .= $this->formatTimestamp( $rev->getTimestamp() );
$line .= $this->formatUserLinks( $rev->getUserName() );
- $line .= Linker::commentBlock( $rev->getComment(), $title,
false, $externalChange->getSiteId() );
+
+ $commentHtml = $rev->getCommentHtml();
+
+ if ( $commentHtml === null || $commentHtml === '' ) {
+ $commentHtml = Linker::formatComment(
$rev->getComment(), $title, false, $externalChange->getSiteId() );
+ }
+
+ $line .= $this->wrapCommentBlock( $commentHtml );
return $line;
}
/**
+ * @param string $commentHtml Formatted comment HTML
+ *
+ * @return string Formatted comment HTML wrapped as comment block
+ */
+ public static function wrapCommentBlock( $commentHtml ) {
+ //NOTE: keep in sync with Linker::commentBlock
+ $formatted = wfMessage( 'parentheses' )->rawParams(
$commentHtml )->escaped();
+ return " <span class=\"comment\">$formatted</span>";
+ }
+
+ /**
* @return string HTML
*/
private function changeSeparator() {
diff --git a/client/includes/recentchanges/ExternalChange.php
b/client/includes/recentchanges/ExternalChange.php
index 7dce4cc..ec826e5 100644
--- a/client/includes/recentchanges/ExternalChange.php
+++ b/client/includes/recentchanges/ExternalChange.php
@@ -15,11 +15,6 @@
class ExternalChange {
/**
- * @var string
- */
- private $siteId;
-
- /**
* @var EntityId
*/
private $entityId;
diff --git a/client/includes/recentchanges/ExternalChangeFactory.php
b/client/includes/recentchanges/ExternalChangeFactory.php
index 355c284..d4511a4 100644
--- a/client/includes/recentchanges/ExternalChangeFactory.php
+++ b/client/includes/recentchanges/ExternalChangeFactory.php
@@ -2,6 +2,7 @@
namespace Wikibase\Client\RecentChanges;
+use Content;
use InvalidArgumentException;
use Language;
use RecentChange;
@@ -45,11 +46,20 @@
* @return ExternalChange
*/
public function newFromRecentChange( RecentChange $recentChange ) {
- $changeParams = $this->extractChangeData( $recentChange );
+ $rc_params = $recentChange->parseParams();
+
+ if ( !is_array( $rc_params ) || !array_key_exists(
'wikibase-repo-change', $rc_params ) ) {
+ throw new UnexpectedValueException( 'Not a Wikibase
change' );
+ }
+
+ $changeParams = $this->extractChangeData( $rc_params );
+
+ // If a pre-formatted comment exists, pass it on.
+ $changeHtml = isset( $rc_params['comment-html'] ) ?
$rc_params['comment-html'] : null;
$itemId = $this->extractItemId( $changeParams['object_id'] );
$changeType = $this->extractChangeType( $changeParams['type'] );
- $rev = $this->newRevisionData( $recentChange, $changeParams );
+ $rev = $this->newRevisionData( $recentChange, $changeParams,
$changeHtml );
return new ExternalChange( $itemId, $rev, $changeType );
}
@@ -57,10 +67,11 @@
/**
* @param RecentChange $recentChange
* @param array $changeParams
+ * @param string|null $commentHtml Pre-formatted comment HTML
*
* @return RevisionData
*/
- private function newRevisionData( RecentChange $recentChange, array
$changeParams ) {
+ private function newRevisionData( RecentChange $recentChange, array
$changeParams, $commentHtml = null ) {
$repoId = isset( $changeParams['site_id'] )
? $changeParams['site_id'] : $this->repoSiteId;
@@ -77,24 +88,19 @@
$changeParams['parent_id'],
$recentChange->getAttribute( 'rc_timestamp' ),
$comment,
+ $commentHtml,
$repoId
);
}
/**
- * @param RecentChange $recentChange
+ * @param array Content of rc_params
*
* @throws UnexpectedValueException
* @return array
*/
- private function extractChangeData( RecentChange $recentChange ) {
- $params = unserialize( $recentChange->getAttribute( 'rc_params'
) );
-
- if ( !is_array( $params ) || !array_key_exists(
'wikibase-repo-change', $params ) ) {
- throw new UnexpectedValueException( 'Not a Wikibase
change' );
- }
-
- $changeParams = $params['wikibase-repo-change'];
+ private function extractChangeData( array $rc_params ) {
+ $changeParams = $rc_params['wikibase-repo-change'];
$this->validateChangeData( $changeParams );
diff --git a/client/includes/recentchanges/ExternalRecentChange.php
b/client/includes/recentchanges/ExternalRecentChange.php
index 737dd35..ce2b853 100644
--- a/client/includes/recentchanges/ExternalRecentChange.php
+++ b/client/includes/recentchanges/ExternalRecentChange.php
@@ -3,6 +3,7 @@
namespace Wikibase\Client\RecentChanges;
use DatabaseBase;
+use Linker;
use MWException;
use Title;
@@ -62,13 +63,12 @@
$userText = $metadata['rc_user_text'];
}
+ $repoId = isset( $metadata['site_id'] ) ? $metadata['site_id']
: null;
$time = isset( $metadata['time'] ) ? $metadata['time'] :
wfTimestamp( TS_MW );
$comment = isset( $attribs['comment'] ) ? $attribs['comment'] :
'';
- // Unset the 'comment' field, so
ExternalChangeFactory::extractCommentOverride doesn't
- // trigger and replaces the actual comment.
- // TODO: put $metadata into rc_params, not $attribs; See
ExternalChangeFactory::extractChangeData
- unset( $attribs['comment'] );
+ //XXX: wrap Linker in something we can inject here
+ $attribs['comment-html'] = Linker::formatComment( $comment,
$title, false, $repoId );
$this->mAttribs = array(
'rc_namespace' => $title->getNamespace(),
diff --git a/client/includes/recentchanges/RevisionData.php
b/client/includes/recentchanges/RevisionData.php
index 73b1d38..7600428 100644
--- a/client/includes/recentchanges/RevisionData.php
+++ b/client/includes/recentchanges/RevisionData.php
@@ -38,9 +38,14 @@
protected $timestamp;
/**
- * @var string
+ * @var string wikitext
*/
protected $comment;
+
+ /**
+ * @var string HTML
+ */
+ protected $commentHtml;
/**
* @param string $userName
@@ -49,10 +54,18 @@
* @param int $parentId
* @param string $timestamp
* @param string $comment
+ * @param string|null $commentHtml
* @param string $siteId
*/
- public function __construct( $userName, $pageId, $revId, $parentId,
$timestamp,
- $comment, $siteId
+ public function __construct(
+ $userName,
+ $pageId,
+ $revId,
+ $parentId,
+ $timestamp,
+ $comment,
+ $commentHtml,
+ $siteId
) {
$this->userName = $userName;
$this->pageId = $pageId;
@@ -60,6 +73,7 @@
$this->parentId = $parentId;
$this->timestamp = $timestamp;
$this->comment = $comment;
+ $this->commentHtml = $commentHtml;
$this->siteId = $siteId;
}
@@ -108,6 +122,13 @@
/**
* @return string
*/
+ public function getCommentHtml() {
+ return $this->commentHtml;
+ }
+
+ /**
+ * @return string
+ */
public function getSiteId() {
return $this->siteId;
}
diff --git a/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php
b/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php
index d941317..cf7dceb 100644
--- a/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php
+++ b/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php
@@ -80,6 +80,7 @@
$changeListTransformer,
Language::factory( 'qqx' ),
'enwiki',
+ 'repowiki',
true
);
diff --git
a/client/tests/phpunit/includes/recentchanges/ChangeLineFormatterTest.php
b/client/tests/phpunit/includes/recentchanges/ChangeLineFormatterTest.php
index 6e01952..ddbc54d 100644
--- a/client/tests/phpunit/includes/recentchanges/ChangeLineFormatterTest.php
+++ b/client/tests/phpunit/includes/recentchanges/ChangeLineFormatterTest.php
@@ -98,11 +98,15 @@
}
public function formatProvider() {
+ $commentHtml = '<span><a href="http://acme.test";>Linky</a>
<script>we can run scripts here</script><span/>';
+
return array(
'edit-change' => array(
$this->getEditSiteLinkChangeTagMatchers(),
$this->getEditSiteLinkPatterns(),
- $this->getEditSiteLinkRecentChange()
+ $this->getEditSiteLinkRecentChange(
+ '/* wbsetclaim-update:2||1 */
[[Property:P213]]: [[Q850]]'
+ )
),
'log-change' => array(
$this->getLogChangeTagMatchers(),
@@ -114,12 +118,27 @@
'comment-fallback' => array(
array(),
array(
- '/\(Associated .*? item deleted\.
Language links removed\.\)/'
+ '/<span
class=\"comment\">.*\(Associated .*? item deleted\. Language links removed\.\)/'
),
$this->getEditSiteLinkRecentChange(
'',
+ null,
array(
'message' =>
'wikibase-comment-remove',
+ ),
+ null
+ )
+ ),
+ 'comment-html' => array(
+ array(),
+ array(
+ '/<span class=\"comment\">.*' .
preg_quote( $commentHtml, '/' ) . '/',
+ ),
+ $this->getEditSiteLinkRecentChange(
+ 'this shall be ignored',
+ $commentHtml,
+ array(
+ 'message' =>
'this-shall-be-ignored',
),
null
)
@@ -216,7 +235,7 @@
);
}
- protected function getEditSiteLinkRecentChange( $comment = null,
$legacyComment = null, $compositeLegacyComment = null ) {
+ protected function getEditSiteLinkRecentChange( $comment, $commentHtml
= null, $legacyComment = null, $compositeLegacyComment = null ) {
$params = array(
'wikibase-repo-change' => array(
'id' => 4,
@@ -234,14 +253,14 @@
)
);
- if ( !is_string( $comment ) ) {
- $comment = '/* wbsetclaim-update:2||1 */
[[Property:P213]]: [[Q850]]';
- }
-
if ( $legacyComment ) {
$params['wikibase-repo-change']['comment'] =
$legacyComment;
}
+ if ( $commentHtml ) {
+ $params['comment-html'] = $commentHtml;
+ }
+
if ( $compositeLegacyComment ) {
$params['wikibase-repo-change']['composite-comment'] =
$compositeLegacyComment;
}
diff --git
a/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php
b/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php
index 172c996..58734b9 100644
--- a/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php
+++ b/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php
@@ -30,6 +30,7 @@
public function testNewFromRecentChange_itemUpdated() {
$recentChange = $this->makeRecentChange(
'',
+ null,
'wikibase-comment-update',
null,
'wikibase-item~update',
@@ -39,7 +40,7 @@
$externalChangeFactory = $this->getExternalChangeFactory();
$this->assertEquals(
- $this->makeExpectedExternalChange(
'(wikibase-comment-update)', 'update' ),
+ $this->makeExpectedExternalChange(
'(wikibase-comment-update)', null, 'update' ),
$externalChangeFactory->newFromRecentChange(
$recentChange )
);
}
@@ -49,6 +50,7 @@
// 'wikibase-comment-update' for these changes.
$recentChange = $this->makeRecentChange(
'',
+ null,
array(
'message' => 'wikibase-comment-sitelink-add',
'sitelink' => array(
@@ -63,7 +65,7 @@
$externalChangeFactory = $this->getExternalChangeFactory();
$this->assertEquals(
- $this->makeExpectedExternalChange(
'(wikibase-comment-update)', 'update' ),
+ $this->makeExpectedExternalChange(
'(wikibase-comment-update)', null, 'update' ),
$externalChangeFactory->newFromRecentChange(
$recentChange )
);
}
@@ -71,6 +73,7 @@
public function testNewFromRecentChange_pageLinkedOnRepo() {
$recentChange = $this->makeRecentChange(
'',
+ null,
array(
'message' => 'wikibase-comment-linked'
),
@@ -82,7 +85,7 @@
$externalChangeFactory = $this->getExternalChangeFactory();
$this->assertEquals(
- $this->makeExpectedExternalChange(
'(wikibase-comment-linked)', 'add' ),
+ $this->makeExpectedExternalChange(
'(wikibase-comment-linked)', null, 'add' ),
$externalChangeFactory->newFromRecentChange(
$recentChange )
);
}
@@ -92,6 +95,7 @@
$recentChange = $this->makeRecentChange(
$comment,
+ null,
array(
'message' => 'this-shall-be-ignored'
),
@@ -103,7 +107,30 @@
$externalChangeFactory = $this->getExternalChangeFactory();
$this->assertEquals(
- $this->makeExpectedExternalChange( $comment, 'update' ),
+ $this->makeExpectedExternalChange( $comment, null,
'update' ),
+ $externalChangeFactory->newFromRecentChange(
$recentChange )
+ );
+ }
+
+ public function testNewFromRecentChange_withHtmlComment() {
+ $comment = '/* wbsetclaim-update:2||1 */ this shall be ignored';
+ $commentHtml = '<span><a href="http://acme.test";>Linky</a>
<script>we can run scripts here</script><span/>';
+
+ $recentChange = $this->makeRecentChange(
+ $comment,
+ $commentHtml,
+ array(
+ 'message' => 'this-shall-be-ignored'
+ ),
+ null,
+ 'wikibase-item~update',
+ false
+ );
+
+ $externalChangeFactory = $this->getExternalChangeFactory();
+
+ $this->assertEquals(
+ $this->makeExpectedExternalChange( $comment,
$commentHtml, 'update' ),
$externalChangeFactory->newFromRecentChange(
$recentChange )
);
}
@@ -111,6 +138,7 @@
public function testNewFromRecentChange_compositeComment() {
$recentChange = $this->makeRecentChange(
'',
+ null,
null,
array(
'wikibase-comment-update',
@@ -122,7 +150,7 @@
$expected = new ExternalChange(
new ItemId( 'Q4' ),
- $this->makeRevisionData( '(wikibase-comment-multi: 2)'
),
+ $this->makeRevisionData( '(wikibase-comment-multi: 2)',
null ),
'update'
);
@@ -137,6 +165,7 @@
public function testNewFromRecentChange_botEdit() {
$recentChange = $this->makeRecentChange(
'',
+ null,
'wikibase-comment-update',
null,
'wikibase-item~update',
@@ -146,7 +175,7 @@
$externalChangeFactory = $this->getExternalChangeFactory();
$this->assertEquals(
- $this->makeExpectedExternalChange(
'(wikibase-comment-update)', 'update' ),
+ $this->makeExpectedExternalChange(
'(wikibase-comment-update)', null, 'update' ),
$externalChangeFactory->newFromRecentChange(
$recentChange )
);
}
@@ -154,6 +183,7 @@
public function testNewFromRecentChange_nonBotEdit() {
$recentChange = $this->makeRecentChange(
'',
+ null,
'wikibase-comment-update',
null,
'wikibase-item~update',
@@ -163,7 +193,7 @@
$externalChangeFactory = $this->getExternalChangeFactory();
$this->assertEquals(
- $this->makeExpectedExternalChange(
'(wikibase-comment-update)', 'update' ),
+ $this->makeExpectedExternalChange(
'(wikibase-comment-update)', null, 'update' ),
$externalChangeFactory->newFromRecentChange(
$recentChange )
);
}
@@ -174,15 +204,15 @@
*
* @return ExternalChange
*/
- private function makeExpectedExternalChange( $expectedComment,
$expectedType ) {
+ private function makeExpectedExternalChange( $expectedComment,
$commentHtml, $expectedType ) {
return new ExternalChange(
new ItemId( 'Q4' ),
- $this->makeRevisionData( $expectedComment ),
+ $this->makeRevisionData( $expectedComment, $commentHtml
),
$expectedType
);
}
- private function makeRevisionData( $comment ) {
+ private function makeRevisionData( $comment, $commentHtml = null ) {
return new RevisionData(
'Cat',
5,
@@ -190,25 +220,26 @@
90,
'20130819111741',
strval( $comment ),
+ $commentHtml,
'testrepo'
);
}
/**
* @param string $comment
- * @param null|string|array $commentOverride
- * @param null|string|array $compositeCommentOverride
+ * @param null|string|array $legacyComment
+ * @param null|string|array $compositeLegacyComment
* @param string $changeType
* @param bool $isBot
*
* @return RecentChange
*/
- private function makeRecentChange( $comment, $commentOverride,
$compositeCommentOverride, $changeType, $isBot ) {
+ private function makeRecentChange( $comment, $commentHtml,
$legacyComment, $compositeLegacyComment, $changeType, $isBot ) {
$recentChange = new RecentChange();
$recentChange->counter = 2;
$attribs = $this->makeAttribs(
- $this->makeRCParams( $comment, $commentOverride,
$compositeCommentOverride, $changeType, $isBot ),
+ $this->makeRCParams( $comment, $commentHtml,
$legacyComment, $compositeLegacyComment, $changeType, $isBot ),
$isBot
);
@@ -220,14 +251,15 @@
/**
* @param string $comment
- * @param null|string|array $commentOverride
- * @param null|string|array $compositeCommentOverride
+ * @param null|string $commentHtml
+ * @param null|string|array $legacyComment
+ * @param null|string|array $compositeLegacyComment
* @param string $changeType
* @param boolean $bot
*
* @return array
*/
- private function makeRCParams( $comment, $commentOverride,
$compositeCommentOverride, $changeType, $bot ) {
+ private function makeRCParams( $comment, $commentHtml, $legacyComment,
$compositeLegacyComment, $changeType, $bot ) {
$params = array(
'wikibase-repo-change' => array(
'id' => 4,
@@ -243,15 +275,19 @@
'rev_id' => 92,
'parent_id' => 90,
),
- 'comment' => $comment
+ 'comment' => $comment,
);
- if ( $commentOverride ) {
- $params['wikibase-repo-change']['comment'] =
$commentOverride;
+ if ( $commentHtml !== null ) {
+ $params['comment-html'] = $commentHtml;
}
- if ( $compositeCommentOverride ) {
- $params['wikibase-repo-change']['composite-comment'] =
$compositeCommentOverride;
+ if ( $legacyComment ) {
+ $params['wikibase-repo-change']['comment'] =
$legacyComment;
+ }
+
+ if ( $compositeLegacyComment ) {
+ $params['wikibase-repo-change']['composite-comment'] =
$compositeLegacyComment;
}
return $params;
--
To view, visit https://gerrit.wikimedia.org/r/239835
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5439a76ceb626a7a8f3195bb3001bb9e2f6cb233
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
