jenkins-bot has submitted this change and it was merged.

Change subject: [DIC]: Move SMWHooks::onParserAfterTidy to SMW\ParserAfterTidy
......................................................................


[DIC]: Move SMWHooks::onParserAfterTidy to SMW\ParserAfterTidy

SMW\ParserAfterTidy
Code coverage: 100%
CRAP: 5

+ Adopt DIC
+ Add appropriate test

Bug 50153: Checks explicitly $title->isSpecialPage() + add test case

Change-Id: If170de666883daa1806c558582d35b9d5c4cd4d1
---
M SemanticMediaWiki.classes.php
M SemanticMediaWiki.hooks.php
M includes/hooks/ArticlePurge.php
A includes/hooks/ParserAfterTidy.php
M tests/phpunit/MockObjectBuilder.php
M tests/phpunit/includes/hooks/ArticlePurgeTest.php
M tests/phpunit/includes/hooks/LinksUpdateConstructedTest.php
A tests/phpunit/includes/hooks/ParserAfterTidyTest.php
8 files changed, 364 insertions(+), 27 deletions(-)

Approvals:
  Mwjames: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/SemanticMediaWiki.classes.php b/SemanticMediaWiki.classes.php
index 15bc739..ff8ba45 100644
--- a/SemanticMediaWiki.classes.php
+++ b/SemanticMediaWiki.classes.php
@@ -102,6 +102,7 @@
        'SMW\CacheIdGenerator'            => 
'includes/cache/CacheIdGenerator.php',
 
        // Hooks
+       'SMW\ParserAfterTidy'           => 'includes/hooks/ParserAfterTidy.php',
        'SMW\LinksUpdateConstructed'    => 
'includes/hooks/LinksUpdateConstructed.php',
        'SMW\BeforePageDisplay'         => 
'includes/hooks/BeforePageDisplay.php',
        'SMW\ArticlePurge'              => 'includes/hooks/ArticlePurge.php',
diff --git a/SemanticMediaWiki.hooks.php b/SemanticMediaWiki.hooks.php
index dbb2fd0..72c32df 100644
--- a/SemanticMediaWiki.hooks.php
+++ b/SemanticMediaWiki.hooks.php
@@ -495,27 +495,7 @@
         * @return true
         */
        public static function onParserAfterTidy( &$parser, &$text ) {
-
-               $settings   = \SMW\Settings::newFromGlobals();
-               $parserData = new SMW\ParserData( $parser->getTitle(), 
$parser->getOutput() );
-
-               $complementor = new \SMW\BasePropertyAnnotator( 
$parserData->getData(), $settings );
-               $complementor->attach( $parserData );
-
-               $complementor->addCategories( 
$parser->getOutput()->getCategoryLinks() );
-               $complementor->addDefaultSort( $parser->getDefaultSort() );
-
-               // If an article was was manually purged/moved ensure that the 
store is
-               // updated as well for all other cases onLinksUpdateConstructed 
will
-               // initiate the store update
-               $cache = \SMW\CacheHandler::newFromId()->key( 'autorefresh', 
$parser->getTitle()->getArticleID() );
-
-               if( $cache->get() ) {
-                       $parserData->setObservableDispatcher( new 
\SMW\ObservableSubjectDispatcher( new \SMW\UpdateObserver() ) )->updateStore();
-                       $cache->delete();
-               }
-
-               return true;
+               return \SMW\FunctionHookRegistry::register( new 
\SMW\ParserAfterTidy( $parser, $text ) )->process();
        }
 
        /**
@@ -596,7 +576,7 @@
         */
        public static function onTitleMoveComplete( &$oldTitle, &$newTitle, 
&$user, $oldId, $newId ) {
                \SMW\CacheHandler::newFromId()
-                       ->key( 'autorefresh', $newTitle->getArticleID() )
+                       ->setKey( \SMW\ArticlePurge::newIdGenerator( 
$newTitle->getArticleID() ) )
                        ->set( $GLOBALS['smwgAutoRefreshOnPageMove'] );
 
                smwfGetStore()->changeTitle( $oldTitle, $newTitle, $oldId, 
$newId );
diff --git a/includes/hooks/ArticlePurge.php b/includes/hooks/ArticlePurge.php
index 62c9c31..7d09cf6 100644
--- a/includes/hooks/ArticlePurge.php
+++ b/includes/hooks/ArticlePurge.php
@@ -40,6 +40,17 @@
        }
 
        /**
+        * Returns a CacheIdGenerator object
+        *
+        * @since 1.9
+        *
+        * @return CacheIdGenerator
+        */
+       public static function newIdGenerator( $pageId ) {
+               return new CacheIdGenerator( $pageId, 'autorefresh' );
+       }
+
+       /**
         * @see HookBase::process
         *
         * @since 1.9
@@ -61,7 +72,7 @@
                $cache = $this->getDependencyBuilder()->newObject( 
'CacheHandler' );
 
                $cache->setCacheEnabled( $pageId > 0 )
-                       ->key( 'autorefresh', $pageId )
+                       ->setKey( $this->newIdGenerator( $pageId ) )
                        ->set( $settings->get( 'smwgAutoRefreshOnPurge' ) );
 
                return true;
diff --git a/includes/hooks/ParserAfterTidy.php 
b/includes/hooks/ParserAfterTidy.php
new file mode 100644
index 0000000..d7ef83c
--- /dev/null
+++ b/includes/hooks/ParserAfterTidy.php
@@ -0,0 +1,104 @@
+<?php
+
+namespace SMW;
+
+use Parser;
+
+/**
+ * ParserAfterTidy hook
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @since   1.9
+ *
+ * @author mwjames
+ */
+
+/**
+ * Hook: ParserAfterTidy to add some final processing to the
+ * fully-rendered page output
+ *
+ * @see http://www.mediawiki.org/wiki/Manual:Hooks/ParserAfterTidy
+ *
+ * @ingroup Hook
+ */
+class ParserAfterTidy extends FunctionHook {
+
+       /** @var Parser */
+       protected $parser = null;
+
+       /** @var string */
+       protected $text;
+
+       /**
+        * @since  1.9
+        *
+        * @param Parser $parser
+        * @param string $text
+        */
+       public function __construct( Parser &$parser, &$text ) {
+               $this->parser = $parser;
+               $this->text = $text;
+       }
+
+       /**
+        * @see FunctionHook::process
+        *
+        * @note Article purge: In case an article was manually purged/moved
+        * the store is updated as well and for all other cases 
LinksUpdateConstructed
+        * will handle the store update
+        *
+        * @note Store update: For NS_FILE ParserAfterTidy is initiated several
+        * times and somewhere in-between an empty ParserOuput object is
+        * returned which would cause the existing output properties being 
overridden
+        * therefore purge refresh for NS_FILE is not supported
+        *
+        * @since 1.9
+        *
+        * @return true
+        */
+       public function process() {
+
+               $title = $this->parser->getTitle();
+
+               if ( $title->isSpecialPage() ) {
+                       return true;
+               }
+
+               /**
+                * @var Settings $settings
+                */
+               $settings = $this->getDependencyBuilder()->newObject( 
'Settings' );
+
+               /**
+                * @var CacheHandler $cache
+                */
+               $cache = $this->getDependencyBuilder()->newObject( 
'CacheHandler' );
+
+               /**
+                * @var ParserData $parserData
+                */
+               $parserData = $this->getDependencyBuilder()->newObject( 
'ParserData', array(
+                       'Title'        => $title,
+                       'ParserOutput' => $this->parser->getOutput()
+               ) );
+
+               $annotator = new BasePropertyAnnotator( $parserData->getData(), 
$settings );
+               $annotator->attach( $parserData );
+
+               $annotator->addCategories( 
$this->parser->getOutput()->getCategoryLinks() );
+               $annotator->addDefaultSort( $this->parser->getDefaultSort() );
+
+               $cache->setKey( ArticlePurge::newIdGenerator( 
$title->getArticleID() ) );
+
+               if( $cache->get() && !$title->inNamespace( NS_FILE ) ) {
+                       $parserData->updateStore();
+               }
+
+               $cache->delete();
+
+               return true;
+       }
+
+}
diff --git a/tests/phpunit/MockObjectBuilder.php 
b/tests/phpunit/MockObjectBuilder.php
index 48d44eb..0367ee7 100644
--- a/tests/phpunit/MockObjectBuilder.php
+++ b/tests/phpunit/MockObjectBuilder.php
@@ -746,6 +746,12 @@
         */
        public function getMockTitle() {
 
+               // When interacting with a "real" Parser object, the Parser 
expects in
+               // in 1.21+ a content model to be present while in MW 1.19/1.20 
such
+               // object is not required. In order to avoid operational 
obstruction a
+               // model is set as default and can if necessary individually be 
overridden
+               $contentModel = class_exists( 'ContentHandler') ? 
CONTENT_MODEL_WIKITEXT : null;
+
                $title = $this->getMockBuilder( 'Title' )
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -796,7 +802,7 @@
 
                $title->expects( $this->any() )
                        ->method( 'getContentModel' )
-                       ->will( $this->returnValue( $this->setValue( 
'getContentModel' ) ) );
+                       ->will( $this->returnValue( $this->setValue( 
'getContentModel', $contentModel ) ) );
 
                $title->expects( $this->any() )
                        ->method( 'getPageLanguage' )
diff --git a/tests/phpunit/includes/hooks/ArticlePurgeTest.php 
b/tests/phpunit/includes/hooks/ArticlePurgeTest.php
index 7801e6f..3b96ad5 100644
--- a/tests/phpunit/includes/hooks/ArticlePurgeTest.php
+++ b/tests/phpunit/includes/hooks/ArticlePurgeTest.php
@@ -89,11 +89,11 @@
                $instance->setDependencyBuilder( $dependencyBuilder );
                $cache = $dependencyBuilder->newObject( 'CacheHandler' );
 
-               $this->assertFalse( $cache->key( 'autorefresh', $pageId 
)->get() );
+               $this->assertFalse( $cache->setKey( $instance->newIdGenerator( 
$pageId ) )->get() );
                $result = $instance->process();
 
                $this->assertTrue( $result );
-               $this->assertEquals( $expected['result'], $cache->key( 
'autorefresh', $pageId )->get() );
+               $this->assertEquals( $expected['result'], $cache->setKey( 
$instance->newIdGenerator( $pageId ) )->get() );
 
        }
 
diff --git a/tests/phpunit/includes/hooks/LinksUpdateConstructedTest.php 
b/tests/phpunit/includes/hooks/LinksUpdateConstructedTest.php
index 0d0f5f4..eccdcba 100644
--- a/tests/phpunit/includes/hooks/LinksUpdateConstructedTest.php
+++ b/tests/phpunit/includes/hooks/LinksUpdateConstructedTest.php
@@ -98,7 +98,7 @@
                $this->assertEquals(
                        'runStoreUpdater',
                        $updateObserver->getNotifier(),
-                       'asserts that the invoked observer was notfied'
+                       'asserts that the invoked observer was notified'
                );
 
        }
diff --git a/tests/phpunit/includes/hooks/ParserAfterTidyTest.php 
b/tests/phpunit/includes/hooks/ParserAfterTidyTest.php
new file mode 100644
index 0000000..9b1d684
--- /dev/null
+++ b/tests/phpunit/includes/hooks/ParserAfterTidyTest.php
@@ -0,0 +1,235 @@
+<?php
+
+namespace SMW\Test;
+
+use SMW\SharedDependencyContainer;
+use SMW\ParserAfterTidy;
+
+/**
+ * Tests for the ParserAfterTidy class
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @since   1.9
+ *
+ * @author mwjames
+ */
+
+/**
+ * @covers \SMW\ParserAfterTidy
+ *
+ * @ingroup Test
+ *
+ * @group SMW
+ * @group SMWExtension
+ */
+class ParserAfterTidyTest extends ParserTestCase {
+
+       /**
+        * Returns the name of the class to be tested
+        *
+        * @return string|false
+        */
+       public function getClass() {
+               return '\SMW\ParserAfterTidy';
+       }
+
+       /**
+        * Helper method that returns a ParserAfterTidy object
+        *
+        * @since 1.9
+        *
+        * @return ParserAfterTidy
+        */
+       private function newInstance( &$parser = null, &$text = '' ) {
+
+               if ( $parser === null ) {
+                       $parser = $this->newParser( $this->newTitle(), 
$this->getUser() );
+               }
+
+               $instance = new ParserAfterTidy( $parser, $text );
+               $instance->setDependencyBuilder( $this->newDependencyBuilder( 
new SharedDependencyContainer() ) );
+
+               return $instance;
+       }
+
+       /**
+        * @test ParserAfterTidy::__construct
+        *
+        * @since 1.9
+        */
+       public function testConstructor() {
+               $this->assertInstanceOf( $this->getClass(), 
$this->newInstance() );
+       }
+
+       /**
+        * @test ParserAfterTidy::process
+        * @dataProvider titleDataProvider
+        *
+        * @since 1.9
+        *
+        * @param $setup
+        * @param $expected
+        */
+       public function testProcess( $setup, $expected ) {
+
+               $parser = $this->newParser( $setup['title'], $this->getUser() );
+               $text   = '';
+
+               $instance = $this->newInstance( $parser, $text );
+               $settings = $this->newSettings( array(
+                       'smwgCacheType'        => 'hash',
+                       'smwgEnableUpdateJobs' => false
+               ) );
+
+               $updateObserver = new MockUpdateObserver();
+               $cacheHandler   = $instance->getDependencyBuilder()->newObject( 
'CacheHandler' );
+
+               $container = $instance->getDependencyBuilder()->getContainer();
+               $container->registerObject( 'Settings', $settings );
+               $container->registerObject( 'Store', 
$this->newMockObject()->getMockStore() );
+               $container->registerObject( 'UpdateObserver', $updateObserver );
+               $container->registerObject( 'CacheHandler', $cacheHandler );
+
+               // Simulates a previous state change did cause a cache entry
+               if ( $setup['cache'] ) {
+                       $cacheHandler->setKey(
+                               \SMW\ArticlePurge::newIdGenerator( 
$setup['title']->getArticleID() )
+                       )->set( __METHOD__ );
+               }
+
+               $this->assertTrue(
+                       $instance->process(),
+                       'asserts that process() always returns true'
+               );
+
+               $this->assertEquals(
+                       $expected['observer'],
+                       $updateObserver->getNotifier(),
+                       'asserts that that the invoked observer was notified'
+               );
+
+       }
+
+       /**
+        * @test ParserAfterTidy::process
+        *
+        * @since 1.9
+        */
+       public function testSemanticDataParserOuputUpdateIntegration() {
+
+               $text   = '';
+               $title  = $this->newTitle( NS_MAIN, __METHOD__ );
+
+               // Set-up categories
+               $parser = $this->newParser( $title, $this->getUser() );
+               $parser->getOutput()->addCategory( 'Foo', 'Foo' );
+               $parser->getOutput()->addCategory( 'Bar', 'Bar' );
+
+               // Expected semantic data
+               $expected = array(
+                       'propertyCount' => 2,
+                       'propertyKey'   => array( '_INST', '_SKEY' ),
+                       'propertyValue' => array( 'Foo', 'Bar', 
$title->getText() ),
+               );
+
+               $instance = $this->newInstance( $parser, $text );
+
+               $settings = $this->newSettings( array(
+                       'smwgCacheType'             => 'hash',
+                       'smwgEnableUpdateJobs'      => false,
+                       'smwgUseCategoryHierarchy'  => false,
+                       'smwgCategoriesAsInstances' => true
+               ) );
+
+               $container = $instance->getDependencyBuilder()->getContainer();
+               $container->registerObject( 'Settings', $settings );
+
+               $this->assertTrue(
+                       $instance->process(),
+                       'asserts that process() always returns true'
+               );
+
+               // Re-read data from the Parser
+               $parserData = $this->getParserData( $title, 
$parser->getOutput() );
+               $this->assertSemanticData(
+                       $parserData->getData(),
+                       $expected,
+                       'asserts whether the container contains expected 
triples'
+               );
+
+       }
+
+       /**
+        * @return array
+        */
+       public function titleDataProvider() {
+
+               $provider = array();
+
+               // #0 Runs store update
+               $title = $this->newMockObject( array(
+                       'inNamespace'     => false,
+               ) )->getMockTitle();
+
+               $provider[] = array(
+                       array(
+                               'title'    => $title,
+                               'cache'    => true
+                       ),
+                       array(
+                               'observer' => 'runStoreUpdater'
+                       )
+               );
+
+               // #1 No cache entry, no store update
+               $title = $this->newMockObject( array(
+                       'inNamespace'     => false,
+               ) )->getMockTitle();
+
+               $provider[] = array(
+                       array(
+                               'title'    => $title,
+                               'cache'    => false
+                       ),
+                       array(
+                               'observer' => null
+                       )
+               );
+
+               // #2 SpecialPage, no store update
+               $title = $this->newMockObject( array(
+                       'isSpecialPage'   => true,
+               ) )->getMockTitle();
+
+               $provider[] = array(
+                       array(
+                               'title'    => $title,
+                               'cache'    => false
+                       ),
+                       array(
+                               'observer' => null
+                       )
+               );
+
+               // #3 NS_FILE, no store update
+               $title = $this->newMockObject( array(
+                       'inNamespace'     => true,
+                       'getNamespace'    => NS_FILE
+               ) )->getMockTitle();
+
+               $provider[] = array(
+                       array(
+                               'title'    => $title,
+                               'cache'    => true
+                       ),
+                       array(
+                               'observer' => null
+                       )
+               );
+
+               return $provider;
+       }
+
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/81167
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If170de666883daa1806c558582d35b9d5c4cd4d1
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[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