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

Reply via email to