jenkins-bot has submitted this change and it was merged.
Change subject: Put parser cache js config fallback in
MakeGlobalVariablesScript hook handler
......................................................................
Put parser cache js config fallback in MakeGlobalVariablesScript hook handler
If we need to do fallback, then we need OutputPage::getRevisionId which
returns null when called in OutputPageBeforeHTML (which occurs during
OutputPage::addParserOutput).
addParserOutput is called in Article, just before OutputPage::setRevisionId
gets called by Article.
When MakeGlobalVariablesScript is called, output page has revision id set.
Change-Id: Ideb2a4dbf8228f3163d3432157894fabd4760c20
(cherry picked from commit d9ae8e66ba1fe54d537441b0ee7079851d194751)
---
M repo/Wikibase.hooks.php
M repo/Wikibase.php
A repo/includes/Hook/MakeGlobalVariablesScriptHandler.php
M repo/includes/Hook/OutputPageJsConfigHookHandler.php
A repo/tests/phpunit/includes/Hook/MakeGlobalVariablesScriptHandlerTest.php
M repo/tests/phpunit/includes/Hook/OutputPageJsConfigHookHandlerTest.php
6 files changed, 318 insertions(+), 187 deletions(-)
Approvals:
Aude: Looks good to me, approved
WikidataJenkins: Verified
jenkins-bot: Verified
diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php
index e9617cd..ed3c188 100644
--- a/repo/Wikibase.hooks.php
+++ b/repo/Wikibase.hooks.php
@@ -28,6 +28,7 @@
use SplFileInfo;
use Title;
use User;
+use Wikibase\Hook\MakeGlobalVariablesScriptHandler;
use Wikibase\Hook\OutputPageJsConfigHookHandler;
use Wikibase\Repo\WikibaseRepo;
use WikiPage;
@@ -1150,6 +1151,33 @@
return true;
}
+ $settings = WikibaseRepo::getDefaultInstance()->getSettings();
+ $hookHandler = new OutputPageJsConfigHookHandler( $settings );
+
+ $isExperimental = defined( 'WB_EXPERIMENTAL_FEATURES' ) &&
WB_EXPERIMENTAL_FEATURES;
+
+ $hookHandler->handle( $out, $isExperimental );
+
+ return true;
+ }
+
+ /**
+ * Provides fallback for output page js config vars that are stored in
parser cache.
+ *
+ * In some cases, e.g. stale parser cache contents, variables including
wbEntity might be
+ * missing, so we add them here as a fallback. This hook is called
after
+ * OutputPage::setRevisionId is called. Revision id is needed to
retrieve the correct entity.
+ *
+ * @param array $vars
+ * @param OutputPage $out
+ *
+ * @return boolean
+ */
+ public static function onMakeGlobalVariablesScript( $vars, $out ) {
+ if ( !self::isTitleInEntityNamespace( $out->getTitle() ) ) {
+ return true;
+ }
+
$wikibaseRepo = WikibaseRepo::getDefaultInstance();
$langCode = $out->getContext()->getLanguage()->getCode();
@@ -1158,16 +1186,13 @@
$langCodes = Utils::getLanguageCodes() + array( $langCode =>
$fallbackChain );
- $hookHandler = new OutputPageJsConfigHookHandler(
+ $hookHandler = new MakeGlobalVariablesScriptHandler(
$wikibaseRepo->getEntityContentFactory(),
$wikibaseRepo->getParserOutputJsConfigBuilder(
$langCode ),
- $wikibaseRepo->getSettings(),
$langCodes
);
- $isExperimental = defined( 'WB_EXPERIMENTAL_FEATURES' ) &&
WB_EXPERIMENTAL_FEATURES;
-
- $hookHandler->handle( $out, $isExperimental );
+ $hookHandler->handle( $out );
return true;
}
diff --git a/repo/Wikibase.php b/repo/Wikibase.php
index 53cf7e5..a7de07b 100644
--- a/repo/Wikibase.php
+++ b/repo/Wikibase.php
@@ -177,6 +177,7 @@
$wgHooks['ContentModelCanBeUsedOn'][] =
'Wikibase\RepoHooks::onContentModelCanBeUsedOn';
$wgHooks['OutputPageBeforeHTML'][] =
'Wikibase\RepoHooks::onOutputPageBeforeHTML';
$wgHooks['OutputPageBeforeHTML'][] =
'Wikibase\RepoHooks::onOutputPageBeforeHtmlRegisterConfig';
+ $wgHooks['MakeGlobalVariablesScript'][] =
'Wikibase\RepoHooks::onMakeGlobalVariablesScript';
// Resource Loader Modules:
$wgResourceModules = array_merge( $wgResourceModules, include( __DIR__
. "/resources/Resources.php" ) );
diff --git a/repo/includes/Hook/MakeGlobalVariablesScriptHandler.php
b/repo/includes/Hook/MakeGlobalVariablesScriptHandler.php
new file mode 100644
index 0000000..763f17f
--- /dev/null
+++ b/repo/includes/Hook/MakeGlobalVariablesScriptHandler.php
@@ -0,0 +1,135 @@
+<?php
+
+namespace Wikibase\Hook;
+
+use OutputPage;
+use Title;
+use Wikibase\DataModel\Entity\EntityIdParser;
+use Wikibase\DataModel\Entity\EntityIdParsingException;
+use Wikibase\EntityContent;
+use Wikibase\EntityContentFactory;
+use Wikibase\LanguageFallbackChainFactory;
+use Wikibase\Lib\Serializers\SerializationOptions;
+use Wikibase\NamespaceUtils;
+use Wikibase\OutputPageJsConfigBuilder;
+use Wikibase\ParserOutputJsConfigBuilder;
+use Wikibase\Settings;
+
+/**
+ * @since 0.5
+ *
+ * @licence GNU GPL v2+
+ * @author Katie Filbert < [email protected] >
+ */
+class MakeGlobalVariablesScriptHandler {
+
+ /**
+ * @var EntityContentFactory
+ */
+ protected $entityContentFactory;
+
+ /**
+ * @var ParserOutputJsConfigBuilder
+ */
+ protected $parserOutputConfigBuilder;
+
+ /**
+ * @param EntityContentFactory $entityContentFactory
+ * @param ParserOutputJsConfigBuilder $configBuilder
+ * @param array $langCodes
+ */
+ public function __construct(
+ EntityContentFactory $entityContentFactory,
+ ParserOutputJsConfigBuilder $parserOutputConfigBuilder,
+ array $langCodes
+ ) {
+ $this->entityContentFactory = $entityContentFactory;
+ $this->parserOutputConfigBuilder = $parserOutputConfigBuilder;
+ $this->langCodes = $langCodes;
+ }
+
+ /**
+ * @param OutputPage $out
+ */
+ public function handle( OutputPage $out ) {
+ $configVars = $out->getJsConfigVars();
+
+ // backwards compatibility, in case config is not in parser
cache and output page
+ if ( !$this->hasParserConfigVars( $configVars ) ) {
+ $this->addConfigVars( $out );
+ }
+ }
+
+ /**
+ * Regenerates and adds parser config variables (e.g. wbEntity), in case
+ * they are missing in OutputPage (e.g. not in parser cache)
+ *
+ * @param OutputPage $out
+ */
+ private function addConfigVars( OutputPage $out ) {
+ $revisionId = $out->getRevisionId();
+
+ // this will not be set for deleted pages
+ if ( !$revisionId ) {
+ return;
+ }
+
+ $parserConfigVars = $this->getParserConfigVars( $revisionId );
+ $out->addJsConfigVars( $parserConfigVars );
+ }
+
+ /**
+ * @param mixed
+ *
+ * @return boolean
+ */
+ private function hasParserConfigVars( $configVars ) {
+ return is_array( $configVars ) && array_key_exists(
'wbEntityId', $configVars );
+ }
+
+ /**
+ * @param int $revisionId
+ *
+ * @return array
+ */
+ private function getParserConfigVars( $revisionId ) {
+ $entityContent = $this->entityContentFactory->getFromRevision(
$revisionId );
+
+ if ( $entityContent === null || ! $entityContent instanceof
\Wikibase\EntityContent ) {
+ // entity or revision deleted, or non-entity content in
entity namespace
+ wfDebugLog( __CLASS__, "No entity content found for
revision $revisionId" );
+ return array();
+ }
+
+ return $this->buildParserConfigVars( $entityContent );
+ }
+
+ /**
+ * @param EntityContent $entityContent
+ *
+ * @return array
+ */
+ private function buildParserConfigVars( EntityContent $entityContent ) {
+ $options = $this->makeSerializationOptions();
+
+ $entity = $entityContent->getEntity();
+
+ $parserConfigVars = $this->parserOutputConfigBuilder->build(
+ $entity,
+ $options
+ );
+
+ return $parserConfigVars;
+ }
+
+ /**
+ * @return SerializationOptions
+ */
+ private function makeSerializationOptions() {
+ $options = new SerializationOptions();
+ $options->setLanguages( $this->langCodes );
+
+ return $options;
+ }
+
+}
diff --git a/repo/includes/Hook/OutputPageJsConfigHookHandler.php
b/repo/includes/Hook/OutputPageJsConfigHookHandler.php
index 52d6d9b..2eed4c4 100644
--- a/repo/includes/Hook/OutputPageJsConfigHookHandler.php
+++ b/repo/includes/Hook/OutputPageJsConfigHookHandler.php
@@ -24,16 +24,6 @@
class OutputPageJsConfigHookHandler {
/**
- * @var EntityContentFactory
- */
- protected $entityContentFactory;
-
- /**
- * @var ParserOutputJsConfigBuilder
- */
- protected $parserOutputConfigBuilder;
-
- /**
* @var Settings
*/
protected $settings;
@@ -44,47 +34,21 @@
protected $outputPageConfigBuilder;
/**
- * @param EntityContentFactory $entityContentFactory
* @param Settings $settings
- * @param ParserOutputJsConfigBuilder $configBuilder
- * @param array $langCodes
*/
- public function __construct(
- EntityContentFactory $entityContentFactory,
- ParserOutputJsConfigBuilder $parserOutputConfigBuilder,
- Settings $settings,
- array $langCodes
- ) {
- $this->entityContentFactory = $entityContentFactory;
- $this->parserOutputConfigBuilder = $parserOutputConfigBuilder;
+ public function __construct( Settings $settings ) {
$this->settings = $settings;
- $this->langCodes = $langCodes;
-
$this->outputPageConfigBuilder = new
OutputPageJsConfigBuilder();
}
/**
* @param OutputPage &$out
* @param boolean $isExperimental
- *
- * @return OutputPage
*/
public function handle( OutputPage $out, $isExperimental ) {
- $configVars = $out->getJsConfigVars();
-
- // backwards compatibility, in case config is not in parser
cache and output page
- if ( !$this->hasParserConfigVars( $configVars ) ) {
- // gets from parser cache, with fallback to generate it
if not cached
- $revisionId = $out->getRevisionId();
- $parserConfigVars = $this->getParserConfigVars(
$revisionId );
- $out->addJsConfigVars( $parserConfigVars );
- }
-
$outputConfigVars = $this->buildConfigVars( $out,
$isExperimental );
$out->addJsConfigVars( $outputConfigVars );
-
- return true;
}
/**
@@ -105,60 +69,6 @@
);
return $configVars;
- }
-
- /**
- * @param mixed
- *
- * @return boolean
- */
- private function hasParserConfigVars( $configVars ) {
- return is_array( $configVars ) && array_key_exists(
'wbEntityId', $configVars );
- }
-
- /**
- * @param int $revisionId
- *
- * @return array
- */
- private function getParserConfigVars( $revisionId ) {
- $entityContent = $this->entityContentFactory->getFromRevision(
$revisionId );
-
- if ( $entityContent === null || ! $entityContent instanceof
\Wikibase\EntityContent ) {
- // entity or revision deleted, or non-entity content in
entity namespace
- wfDebugLog( __CLASS__, "No entity content found for
revision $revisionId" );
- return array();
- }
-
- return $this->buildParserConfigVars( $entityContent );
- }
-
- /**
- * @param EntityContent $entityContent
- *
- * @return array
- */
- private function buildParserConfigVars( EntityContent $entityContent ) {
- $options = $this->makeSerializationOptions();
-
- $entity = $entityContent->getEntity();
-
- $parserConfigVars = $this->parserOutputConfigBuilder->build(
- $entity,
- $options
- );
-
- return $parserConfigVars;
- }
-
- /**
- * @return SerializationOptions
- */
- private function makeSerializationOptions() {
- $options = new SerializationOptions();
- $options->setLanguages( $this->langCodes );
-
- return $options;
}
}
diff --git
a/repo/tests/phpunit/includes/Hook/MakeGlobalVariablesScriptHandlerTest.php
b/repo/tests/phpunit/includes/Hook/MakeGlobalVariablesScriptHandlerTest.php
new file mode 100644
index 0000000..cf014a2
--- /dev/null
+++ b/repo/tests/phpunit/includes/Hook/MakeGlobalVariablesScriptHandlerTest.php
@@ -0,0 +1,144 @@
+<?php
+
+namespace Wikibase\Test;
+
+use DataValues\StringValue;
+use RequestContext;
+use Title;
+use Wikibase\DataModel\Claim\Claim;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
+use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Hook\MakeGlobalVariablesScriptHandler;
+use Wikibase\ItemContent;
+use Wikibase\LanguageFallbackChainFactory;
+use Wikibase\ParserOutputJsConfigBuilder;
+
+/**
+ * @covers Wikibase\Hook\MakeGlobalVariablesScriptHandler
+ *
+ * @since 0.5
+ *
+ * @group WikibaseRepo
+ * @group Wikibase
+ *
+ * @licence GNU GPL v2+
+ * @author Katie Filbert < [email protected] >
+ */
+class MakeGlobalVariablesScriptHandlerTest extends \PHPUnit_Framework_TestCase
{
+
+ /**
+ * @dataProvider handleProvider
+ */
+ public function testHandle( array $expected, Title $title, array
$parserConfigVars, $message ) {
+ $hookHandler = new MakeGlobalVariablesScriptHandler(
+ $this->getEntityContentFactory(),
+ $this->getParserOutputJsConfigBuilder(
$parserConfigVars ),
+ array( 'de', 'en', 'es', 'fr' )
+ );
+
+ $context = new RequestContext();
+ $context->setTitle( $title );
+
+ $output = $context->getOutput();
+ $output->setRevisionId( 9320 );
+
+ $hookHandler->handle( $output );
+
+ $configVars = $output->getJsConfigVars();
+
+ $this->assertEquals( $expected, array_keys( $configVars ),
$message );
+ }
+
+ public function handleProvider() {
+ $entityId = new ItemId( 'Q4' );
+ $title = $this->getTitleForId( $entityId );
+
+ $expected = array(
+ 'wbEntityId'
+ );
+
+ $parserConfigVars = array(
+ 'wbEntityId' => 'Q4'
+ );
+
+ return array(
+ array( $expected, $title, $parserConfigVars, 'config
vars without parser cache' )
+ );
+ }
+
+ /**
+ * @param EntityId $entityId
+ *
+ * @return Title
+ */
+ public function getTitleForId( EntityId $entityId ) {
+ $name = $entityId->getEntityType() . ':' .
$entityId->getPrefixedId();
+ return Title::makeTitle( NS_MAIN, $name );
+ }
+
+ /**
+ * @return ParserOutputJsConfigBuilder
+ */
+ private function getParserOutputJsConfigBuilder( $configVars ) {
+ $configBuilder = $this->getMockBuilder(
'Wikibase\ParserOutputJsConfigBuilder' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $configBuilder->expects( $this->any() )
+ ->method( 'build' )
+ ->will( $this->returnCallback(
+ function() use( $configVars ) {
+ return $configVars;
+ }
+ )
+ );
+
+ return $configBuilder;
+ }
+
+ /**
+ * @return EntityContentFactory
+ */
+ private function getEntityContentFactory() {
+ $entityContentFactory = $this->getMockBuilder(
'Wikibase\EntityContentFactory' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $entityContentFactory->expects( $this->any() )
+ ->method( 'getFromRevision' )
+ ->will( $this->returnCallback( array( $this,
'getEntityContent' ) ) );
+
+ $entityContentFactory->expects( $this->any() )
+ ->method( 'getTitleForId' )
+ ->will( $this->returnCallback( array( $this,
'getTitleForId' ) ) );
+
+ return $entityContentFactory;
+ }
+
+ /**
+ * @return EntityContent
+ */
+ public function getEntityContent() {
+ $item = Item::newFromArray( array() );
+
+ $itemId = new ItemId( 'Q5881' );
+ $item->setId( $itemId );
+ $item->setLabel( 'en', 'Cake' );
+
+ $snak = new PropertyValueSnak( new PropertyId( 'P794' ), new
StringValue( 'a' ) );
+
+ $claim = new Claim( $snak );
+ $claim->setGuid( 'P794$muahahaha' );
+
+ $item->addClaim( $claim );
+
+ $entityContent = new ItemContent( $item );
+
+ return $entityContent;
+ }
+
+}
diff --git
a/repo/tests/phpunit/includes/Hook/OutputPageJsConfigHookHandlerTest.php
b/repo/tests/phpunit/includes/Hook/OutputPageJsConfigHookHandlerTest.php
index 33f089b..546facb 100644
--- a/repo/tests/phpunit/includes/Hook/OutputPageJsConfigHookHandlerTest.php
+++ b/repo/tests/phpunit/includes/Hook/OutputPageJsConfigHookHandlerTest.php
@@ -2,20 +2,11 @@
namespace Wikibase\Test;
-use DataValues\StringValue;
use RequestContext;
use Title;
-use Wikibase\DataModel\Claim\Claim;
-use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
-use Wikibase\DataModel\Snak\PropertyValueSnak;
-use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\Hook\OutputPageJsConfigHookHandler;
-use Wikibase\ItemContent;
-use Wikibase\LanguageFallbackChainFactory;
-use Wikibase\ParserOutputJsConfigBuilder;
use Wikibase\Settings;
/**
@@ -34,23 +25,15 @@
/**
* @dataProvider handleProvider
*/
- public function testHandle( array $expected, EntityId $entityId, array
$cachedConfig,
- array $parserConfig, Settings $settings, $experimental, $message
+ public function testHandle( array $expected, Title $title, Settings
$settings, $experimental,
+ $message
) {
- $hookHandler = new OutputPageJsConfigHookHandler(
- $this->getEntityContentFactory(),
- $this->getParserOutputJsConfigBuilder( $parserConfig ),
- $settings,
- array( 'de', 'en', 'es', 'fr' )
- );
-
- $title = $this->getTitleForId( $entityId );
+ $hookHandler = new OutputPageJsConfigHookHandler( $settings );
$context = new RequestContext();
$context->setTitle( $title );
$output = $context->getOutput();
- $output->addJsConfigVars( $cachedConfig );
$hookHandler->handle( $output, $experimental );
@@ -62,25 +45,19 @@
public function handleProvider() {
$settings = $this->getSettings();
- $entityId = new ItemId( 'Q4' );
-
- $parserConfig = array(
- 'wbEntityId' => $entityId->getSerialization()
- );
$expected = array(
- 'wbEntityId',
'wbUserIsBlocked',
'wbUserCanEdit',
'wbCopyright',
'wbExperimentalFeatures'
);
+ $entityId = new ItemId( 'Q4' );
+ $title = $this->getTitleForId( $entityId );
+
return array(
- array( $expected, $entityId, $parserConfig,
$parserConfig,
- $settings, true, 'config vars with parser
cache' ),
- array( $expected, $entityId, array(), $parserConfig,
- $settings, true, 'config vars without parser
cache' )
+ array( $expected, $title, $settings, true, 'config vars
added to OutputPage' )
);
}
@@ -103,67 +80,6 @@
public function getTitleForId( EntityId $entityId ) {
$name = $entityId->getEntityType() . ':' .
$entityId->getPrefixedId();
return Title::makeTitle( NS_MAIN, $name );
- }
-
- /**
- * @return ParserOutputJsConfigBuilder
- */
- private function getParserOutputJsConfigBuilder( $configVars ) {
- $configBuilder = $this->getMockBuilder(
'Wikibase\ParserOutputJsConfigBuilder' )
- ->disableOriginalConstructor()
- ->getMock();
-
- $configBuilder->expects( $this->any() )
- ->method( 'build' )
- ->will( $this->returnCallback(
- function() use( $configVars ) {
- return $configVars;
- }
- )
- );
-
- return $configBuilder;
- }
-
- /**
- * @return EntityContentFactory
- */
- private function getEntityContentFactory() {
- $entityContentFactory = $this->getMockBuilder(
'Wikibase\EntityContentFactory' )
- ->disableOriginalConstructor()
- ->getMock();
-
- $entityContentFactory->expects( $this->any() )
- ->method( 'getFromRevision' )
- ->will( $this->returnCallback( array( $this,
'getEntityContent' ) ) );
-
- $entityContentFactory->expects( $this->any() )
- ->method( 'getTitleForId' )
- ->will( $this->returnCallback( array( $this,
'getTitleForId' ) ) );
-
- return $entityContentFactory;
- }
-
- /**
- * @return EntityContent
- */
- public function getEntityContent() {
- $item = Item::newFromArray( array() );
-
- $itemId = new ItemId( 'Q5881' );
- $item->setId( $itemId );
- $item->setLabel( 'en', 'Cake' );
-
- $snak = new PropertyValueSnak( new PropertyId( 'P794' ), new
StringValue( 'a' ) );
-
- $claim = new Claim( $snak );
- $claim->setGuid( 'P794$muahahaha' );
-
- $item->addClaim( $claim );
-
- $entityContent = new ItemContent( $item );
-
- return $entityContent;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/120831
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ideb2a4dbf8228f3163d3432157894fabd4760c20
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: mw1.23-wmf19
Gerrit-Owner: Aude <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: WikidataJenkins <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
