jenkins-bot has submitted this change and it was merged.
Change subject: Some clean-up
......................................................................
Some clean-up
Change-Id: Ic88c2f26c6b2da5dbe86a4782d88a30c7afdd232
---
M includes/FactboxCache.php
M includes/StoreUpdater.php
M includes/UpdateObserver.php
M includes/hooks/ParserAfterTidy.php
M includes/serializer/Serializers/SemanticDataSerializer.php
M tests/phpunit/includes/hooks/SkinAfterContentTest.php
6 files changed, 107 insertions(+), 71 deletions(-)
Approvals:
Mwjames: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/FactboxCache.php b/includes/FactboxCache.php
index 10e306b..d6a5393 100644
--- a/includes/FactboxCache.php
+++ b/includes/FactboxCache.php
@@ -42,12 +42,11 @@
/**
* Prepare and update the OutputPage property
*
- * Factbox content is either retrived from CacheStore or re-parsed from
- * the invoked Factbox object
+ * Factbox content is either retrived from a CacheStore or re-parsed
from
+ * the Factbox object
*
- * Altered content is tracked using the revision id (getTouched() is
not a
- * good measure for comparison where getLatestRevID() only changes
after a
- * content modification has occurred).
+ * Altered content is tracked using the revision Id, getLatestRevID()
only
+ * changes after a content modification has occurred.
*
* Cached content is stored in an associative array following:
* { 'revId' => $revisionId, 'text' => (...) }
@@ -62,7 +61,7 @@
$title = $this->outputPage->getTitle();
$revId = $this->getRevisionId( $title );
- $resultMapper = $this->getResultMapper( $title->getArticleID()
);
+ $resultMapper = $this->newResultMapper( $title->getArticleID()
);
$content = $resultMapper->fetchFromCache();
if ( isset( $content['revId'] ) && ( $content['revId'] ===
$revId ) && $content['text'] !== null ) {
@@ -73,11 +72,19 @@
} else {
$this->isCached = false;
- $this->outputPage->mSMWFactboxText = $this->rebuild(
$parserOutput );
+
+ $text = $this->rebuild(
+ $title,
+ $parserOutput,
+ $this->outputPage->getContext()
+ );
+
$resultMapper->recache( array(
'revId' => $revId,
- 'text' => $this->outputPage->mSMWFactboxText
+ 'text' => $text
) );
+
+ $this->outputPage->mSMWFactboxText = $text;
}
Profiler::Out( __METHOD__ );
@@ -99,7 +106,7 @@
$text = $this->outputPage->mSMWFactboxText;
} else if ( $this->outputPage->getTitle() instanceof Title &&
!$this->outputPage->getTitle()->isSpecialPage() ) {
- $content = $this->getResultMapper(
$this->outputPage->getTitle()->getArticleID() )->fetchFromCache();
+ $content = $this->newResultMapper(
$this->outputPage->getTitle()->getArticleID() )->fetchFromCache();
$text = isset( $content['text'] ) ? $content['text'] :
'';
}
@@ -137,7 +144,7 @@
*
* @return CacheableResultMapper
*/
- public function getResultMapper( $pageId ) {
+ public function newResultMapper( $pageId ) {
/**
* @var Settings $settings
@@ -158,6 +165,7 @@
* revision or permalink etc.) or from the title object
*
* @since 1.9
+ *
* @param Title $title
*
* @return integer
@@ -180,20 +188,20 @@
*
* @return string|null
*/
- protected function rebuild( ParserOutput $parserOutput ) {
+ protected function rebuild( Title $title, ParserOutput $parserOutput,
$requestContext ) {
$text = null;
/**
* @var RequestContext
*/
- $this->getDependencyBuilder()->getContainer()->registerObject(
'RequestContext', $this->outputPage->getContext() );
+ $this->getDependencyBuilder()->getContainer()->registerObject(
'RequestContext', $requestContext );
/**
* @var Factbox $factbox
*/
$factbox = $this->getDependencyBuilder()->newObject( 'Factbox',
array(
- 'Title' => $this->outputPage->getTitle(),
+ 'Title' => $title,
'ParserOutput' => $parserOutput
) );
@@ -203,7 +211,7 @@
* @var ContentParser $contentParser
*/
$contentParser =
$this->getDependencyBuilder()->newObject( 'ContentParser', array(
- 'Title' => $this->outputPage->getTitle()
+ 'Title' => $title
) );
$contentParser->parse( $factbox->getContent() );
diff --git a/includes/StoreUpdater.php b/includes/StoreUpdater.php
index b0788a3..a8aef8d 100644
--- a/includes/StoreUpdater.php
+++ b/includes/StoreUpdater.php
@@ -98,6 +98,10 @@
}
/**
+ * @note Make sure to have a valid revision (null means delete etc.) and
+ * check if semantic data should be processed and displayed for a page
in
+ * the given namespace
+ *
* @since 1.9
*
* @param WikiPage $wikiPage
@@ -106,13 +110,10 @@
*/
protected function performUpdate( WikiPage $wikiPage ) {
- Profiler::In( __METHOD__, true );
+ Profiler::In();
$revision = $wikiPage->getRevision();
- // Make sure to have a valid revision (null means delete etc.)
- // Check if semantic data should be processed and displayed for
a page in
- // the given namespace
$processSemantics = $revision !== null && $this->isValid(
$wikiPage );
if ( $processSemantics ) {
@@ -130,8 +131,20 @@
$this->semanticData = new SemanticData(
$this->getSubject() );
}
- // Comparison must happen *before* the storage update;
- // even finding uses of a property fails after its type changed.
+ Profiler::Out();
+ return $this->updateStore( $this->detectChanges(
$processSemantics ) );
+ }
+
+ /**
+ * @note Comparison must happen *before* the storage update;
+ * even finding uses of a property fails after its type changed.
+ *
+ * @since 1.9
+ *
+ * @return boolean
+ */
+ protected function detectChanges( $processSemantics ) {
+
if ( $this->updateJobs ) {
$changeNotifier =
$this->withContext()->getDependencyBuilder()->newObject(
'PropertyChangeNotifier', array(
@@ -141,6 +154,18 @@
$changeNotifier->detectChanges();
}
+ return $processSemantics;
+ }
+
+ /**
+ * @since 1.9
+ *
+ * @return boolean
+ */
+ protected function updateStore( $processSemantics ) {
+
+ Profiler::In();
+
// Actually store semantic data, or at least clear it if needed
if ( $processSemantics ) {
$this->withContext()->getStore()->updateData(
$this->semanticData );
@@ -148,7 +173,7 @@
$this->withContext()->getStore()->clearData(
$this->semanticData->getSubject() );
}
- Profiler::Out( __METHOD__, true );
+ Profiler::Out();
return true;
}
diff --git a/includes/UpdateObserver.php b/includes/UpdateObserver.php
index 38b6952..73960f1 100644
--- a/includes/UpdateObserver.php
+++ b/includes/UpdateObserver.php
@@ -40,15 +40,6 @@
*/
public function withContext() {
- // This is not as clean as it should be but to avoid to make
- // multiple changes at once we determine a default builder here
- // which at some point should vanish after pending changes have
- // been merged
-
- // LinksUpdateConstructed is injecting the builder via the
setter
- // UpdateJob does not
- // ParserAfterTidy does not
-
if ( $this->context === null ) {
$this->context = new BaseContext();
}
diff --git a/includes/hooks/ParserAfterTidy.php
b/includes/hooks/ParserAfterTidy.php
index 58d87c5..0edc9de 100644
--- a/includes/hooks/ParserAfterTidy.php
+++ b/includes/hooks/ParserAfterTidy.php
@@ -79,9 +79,9 @@
'SemanticData' => $parserData->getData(),
) );
- $propertyAnnotator->attach( $parserData )
- ->addCategories(
$this->parser->getOutput()->getCategoryLinks() )
- ->addDefaultSort( $this->parser->getDefaultSort() );
+ $propertyAnnotator->attach( $parserData );
+ $propertyAnnotator->addCategories(
$this->parser->getOutput()->getCategoryLinks() );
+ $propertyAnnotator->addDefaultSort(
$this->parser->getDefaultSort() );
/**
* @var CacheHandler $cache
diff --git a/includes/serializer/Serializers/SemanticDataSerializer.php
b/includes/serializer/Serializers/SemanticDataSerializer.php
index 5d6733b..3e4ee08 100644
--- a/includes/serializer/Serializers/SemanticDataSerializer.php
+++ b/includes/serializer/Serializers/SemanticDataSerializer.php
@@ -20,21 +20,21 @@
class SemanticDataSerializer implements Serializer {
/**
- * @see Serializers::serialize
+ * @see Serializer::serialize
*
* @since 1.9
*/
- public function serialize( $object ) {
+ public function serialize( $semanticData ) {
- if ( !$this->isSerializerFor( $object ) ) {
+ if ( !$this->isSerializerFor( $semanticData ) ) {
throw new OutOfBoundsException( 'Object is not
supported' );
}
- return $this->serializeSemanticData( $object ) + array(
'serializer' => __CLASS__, 'version' => 0.1 );
+ return $this->serializeSemanticData( $semanticData ) + array(
'serializer' => __CLASS__, 'version' => 0.1 );
}
/**
- * @see Serializers::isSerializerFor
+ * @see Serializer::isSerializerFor
*
* @since 1.9
*/
@@ -49,46 +49,37 @@
*/
protected function serializeSemanticData( SemanticData $semanticData ) {
- $output = array();
+ $data = array(
+ 'subject' =>
$semanticData->getSubject()->getSerialization(),
+ 'data' => $this->serializeProperty( $semanticData )
+ );
- $output['subject'] =
$semanticData->getSubject()->getSerialization();
+ $subobjects = $this->serializeSubobject(
$semanticData->getSubSemanticData() );
- /**
- * Build property and dataItem serialization record
- */
- foreach ( $semanticData->getProperties() as $property ) {
-
- $prop = array();
-
- $prop['property'] = $property->getSerialization();
-
- foreach ( $semanticData->getPropertyValues( $property )
as $dataItem ) {
- $prop['dataitem'][] = $this->serializeDataItem(
$dataItem );
- }
-
- $output['data'][] = $prop;
-
+ if ( $subobjects !== array() ) {
+ $data['sobj'] = $subobjects;
}
- $this->serializeSubobject( $semanticData->getSubSemanticData(),
$output );
-
- return $output;
+ return $data;
}
/**
- * Returns all subobjects of a SemanticData instance
- *
- * @note The subobject name is used as reference key as it is the only
- * reliable unique key to allow a performable lookup during
unserialization
+ * Build property and dataItem serialization record
*
* @return array
*/
- protected function serializeSubobject( $subSemanticData, &$output ) {
+ protected function serializeProperty( $semanticData ) {
- foreach ( $subSemanticData as $semanticData ) {
- $output['sobj'][] = $this->serializeSemanticData(
$semanticData );
+ $properties = array();
+
+ foreach ( $semanticData->getProperties() as $property ) {
+ $properties[] = array(
+ 'property' => $property->getSerialization(),
+ 'dataitem' => $this->serializeDataItem(
$semanticData, $property )
+ );
}
+ return $properties;
}
/**
@@ -101,13 +92,34 @@
*
* @return array
*/
- protected function serializeDataItem( DataItem $dataItem ) {
+ protected function serializeDataItem( $semanticData, $property ) {
- return array(
- 'type' => $dataItem->getDIType(),
- 'item' => $dataItem->getSerialization()
- );
+ $dataItems = array();
+ foreach ( $semanticData->getPropertyValues( $property ) as
$dataItem ) {
+ $dataItems[] = array(
+ 'type' => $dataItem->getDIType(),
+ 'item' => $dataItem->getSerialization()
+ );
+ }
+
+ return $dataItems;
+ }
+
+ /**
+ * Returns all subobjects of a SemanticData instance
+ *
+ * @return array
+ */
+ protected function serializeSubobject( $subSemanticData ) {
+
+ $subobjects = array();
+
+ foreach ( $subSemanticData as $semanticData ) {
+ $subobjects[] = $this->serializeSemanticData(
$semanticData );
+ }
+
+ return $subobjects;
}
}
diff --git a/tests/phpunit/includes/hooks/SkinAfterContentTest.php
b/tests/phpunit/includes/hooks/SkinAfterContentTest.php
index 0adf3a0..14f69ea 100644
--- a/tests/phpunit/includes/hooks/SkinAfterContentTest.php
+++ b/tests/phpunit/includes/hooks/SkinAfterContentTest.php
@@ -76,7 +76,7 @@
'OutputPage' => $setup['skin']->getOutput()
) );
- $resultMapper = $factboxCache->getResultMapper(
$setup['title']->getArticleID() );
+ $resultMapper = $factboxCache->newResultMapper(
$setup['title']->getArticleID() );
$resultMapper->recache( array(
'revId' => null,
'text' => $setup['text']
--
To view, visit https://gerrit.wikimedia.org/r/91559
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic88c2f26c6b2da5dbe86a4782d88a30c7afdd232
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <[email protected]>
Gerrit-Reviewer: Mwjames <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits