ParserAfterTidy avoid Parser::lock (Parser state cleared while parsin…

…g) (#2294)
SemanticMediaWiki · Mar 4, 2017 · 0514f55 · 0514f55
1 parent 9025cea
commit 0514f55
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 50 deletions.
diff --git a/src/MediaWiki/Hooks/HookRegistry.php b/src/MediaWiki/Hooks/HookRegistry.php
@@ -138,11 +138,14 @@ private function addCallbackHandlers( $basePath, $globalVars ) {
 		$this->handlers['ParserAfterTidy'] = function ( &$parser, &$text ) {
 
 			$parserAfterTidy = new ParserAfterTidy(
-				$parser,
-				$text
+				$parser
+			);
+
+			$parserAfterTidy->isCommandLineMode(
+				$GLOBALS['wgCommandLineMode']
 			);
 
-			return $parserAfterTidy->process();
+			return $parserAfterTidy->process( $text );
 		};
 
 		/**

diff --git a/src/MediaWiki/Hooks/ParserAfterTidy.php b/src/MediaWiki/Hooks/ParserAfterTidy.php
@@ -17,39 +17,55 @@
  *
  * @author mwjames
  */
-class ParserAfterTidy {
+class ParserAfterTidy extends HookHandler {
 
 	/**
 	 * @var Parser
 	 */
 	private $parser = null;
 
 	/**
-	 * @var string
+	 * @var boolean
 	 */
-	private $text;
+	private $isCommandLineMode = false;
 
 	/**
 	 * @since  1.9
 	 *
 	 * @param Parser $parser
-	 * @param string $text
 	 */
-	public function __construct( Parser &$parser, &$text ) {
+	public function __construct( Parser &$parser ) {
 		$this->parser = $parser;
-		$this->text =& $text;
+	}
+
+	/**
+	 * @see https://www.mediawiki.org/wiki/Manual:$wgCommandLineMode
+	 *
+	 * @since 2.5
+	 *
+	 * @param boolean $isCommandLineMode
+	 */
+	public function isCommandLineMode( $isCommandLineMode ) {
+		$this->isCommandLineMode = (bool)$isCommandLineMode;
 	}
 
 	/**
 	 * @since 1.9
 	 *
+	 * @param string $text
+	 *
 	 * @return true
 	 */
-	public function process() {
-		return $this->canPerformUpdate() ? $this->performUpdate() : true;
+	public function process( &$text ) {
+
+		if ( $this->canPerformUpdate() ) {
+			$this->performUpdate( $text );
+		}
+
+		return true;
 	}
 
-	protected function canPerformUpdate() {
+	private function canPerformUpdate() {
 
 		// ParserOptions::getInterfaceMessage is being used to identify whether a
 		// parse was initiated by `Message::parse`
@@ -69,7 +85,7 @@ protected function canPerformUpdate() {
 		return false;
 	}
 
-	protected function performUpdate() {
+	private function performUpdate( &$text ) {
 
 		$applicationFactory = ApplicationFactory::getInstance();
 
@@ -78,19 +94,17 @@ protected function performUpdate() {
 			$this->parser->getOutput()
 		);
 
-		$this->updateAnnotationsForAfterParse(
+		$this->updateAnnotationsOnAfterParse(
 			$applicationFactory->singleton( 'PropertyAnnotatorFactory' ),
 			$parserData->getSemanticData()
 		);
 
 		$parserData->pushSemanticDataToParserOutput();
 
-		$this->checkForRequestedUpdateByPagePurge( $parserData );
-
-		return true;
+		$this->checkOnPurgeRequest( $parserData );
 	}
 
-	private function updateAnnotationsForAfterParse( $propertyAnnotatorFactory, $semanticData ) {
+	private function updateAnnotationsOnAfterParse( $propertyAnnotatorFactory, $semanticData ) {
 
 		$propertyAnnotator = $propertyAnnotatorFactory->newNullPropertyAnnotator(
 			$semanticData
@@ -138,7 +152,7 @@ private function updateAnnotationsForAfterParse( $propertyAnnotatorFactory, $sem
 	 * a static variable or any other messaging that is not persistent will not
 	 * work hence the reliance on the cache as temporary persistence marker
 	 */
-	private function checkForRequestedUpdateByPagePurge( $parserData ) {
+	private function checkOnPurgeRequest( $parserData ) {
 
 		// Only carry out a purge where InTextAnnotationParser have set
 		// an appropriate context reference otherwise it is assumed that the hook
@@ -157,9 +171,15 @@ private function checkForRequestedUpdateByPagePurge( $parserData ) {
 		if( $cache->contains( $key ) && $cache->fetch( $key ) ) {
 			$cache->delete( $key );
 
+			// Avoid a Parser::lock for when a PurgeRequest remains intact
+			// during an update process while being executed from the cmdLine
+			if ( $this->isCommandLineMode ) {
+				return true;
+			}
+
 			$parserData->setOrigin( 'ParserAfterTidy' );
 
-			// Set a timestamp explicitly to create a new hash for the property
+			// Set an explicit timestamp to create a new hash for the property
 			// table change row differ and force a data comparison (this doesn't
 			// change the _MDAT annotation)
 			$parserData->getSemanticData()->setOption(
@@ -174,8 +194,6 @@ private function checkForRequestedUpdateByPagePurge( $parserData ) {
 				number_format( ( microtime( true ) - $start ), 3 )
 			);
 		}
-
-		return true;
 	}
 
 }
diff --git a/tests/phpunit/Unit/MediaWiki/Hooks/ParserAfterTidyTest.php b/tests/phpunit/Unit/MediaWiki/Hooks/ParserAfterTidyTest.php
@@ -5,7 +5,8 @@
 use SMW\ApplicationFactory;
 use SMW\MediaWiki\Hooks\ParserAfterTidy;
 use SMW\Tests\Utils\Mock\MockTitle;
-use SMW\Tests\Utils\UtilityFactory;
+use SMW\Tests\TestEnvironment;
+use SMW\DataItemFactory;
 use Title;
 
 /**
@@ -22,35 +23,36 @@ class ParserAfterTidyTest extends \PHPUnit_Framework_TestCase {
 	private $semanticDataValidator;
 	private $applicationFactory;
 	private $parserFactory;
+	private $spyLogger;
+	private $testEnvironment;
 
 	protected function setUp() {
 		parent::setUp();
 
-		$this->semanticDataValidator = UtilityFactory::getInstance()->newValidatorFactory()->newSemanticDataValidator();
-		$this->parserFactory =  UtilityFactory::getInstance()->newParserFactory();
+		$settings = array(
+			'smwgDeclarationProperties' => array(),
+			'smwgCacheType'        => 'hash',
+			'smwgEnableUpdateJobs' => false
+		);
 
-		$this->applicationFactory = ApplicationFactory::getInstance();
+		$this->testEnvironment = new TestEnvironment( $settings );
+		$this->dataItemFactory = new DataItemFactory();
+
+		$this->spyLogger = $this->testEnvironment->getUtilityFactory()->newSpyLogger();
+		$this->semanticDataValidator = $this->testEnvironment->getUtilityFactory()->newValidatorFactory()->newSemanticDataValidator();
+		$this->parserFactory = $this->testEnvironment->getUtilityFactory()->newParserFactory();
 
 		$store = $this->getMockBuilder( '\SMW\Store' )
 			->disableOriginalConstructor()
 			->getMockForAbstractClass();
 
-		$this->applicationFactory->registerObject( 'Store', $store );
+		$this->testEnvironment->registerObject( 'Store', $store );
 
-		$settings = array(
-			'smwgDeclarationProperties' => array(),
-			'smwgCacheType'        => 'hash',
-			'smwgEnableUpdateJobs' => false
-		);
-
-		foreach ( $settings as $key => $value ) {
-			$this->applicationFactory->getSettings()->set( $key, $value );
-		}
+		$this->applicationFactory = ApplicationFactory::getInstance();
 	}
 
 	protected function tearDown() {
-		$this->applicationFactory->clear();
-
+		$this->testEnvironment->tearDown();
 		parent::tearDown();
 	}
 
@@ -60,11 +62,9 @@ public function testCanConstruct() {
 			->disableOriginalConstructor()
 			->getMock();
 
-		$text  = '';
-
 		$this->assertInstanceOf(
 			'\SMW\MediaWiki\Hooks\ParserAfterTidy',
-			new ParserAfterTidy( $parser, $text )
+			new ParserAfterTidy( $parser )
 		);
 	}
 
@@ -94,15 +94,15 @@ private function newMockCache( $id, $containsStatus, $fetchStatus ) {
 	 */
 	public function testProcess( $parameters ) {
 
-		$this->applicationFactory->registerObject( 'Store', $parameters['store'] );
+		$this->testEnvironment->registerObject( 'Store', $parameters['store'] );
 
 		$cache = $this->newMockCache(
 			$parameters['title']->getArticleID(),
 			$parameters['cache-contains'],
 			$parameters['cache-fetch']
 		);
 
-		$this->applicationFactory->registerObject( 'Cache', $cache );
+		$this->testEnvironment->registerObject( 'Cache', $cache );
 
 		$parser = $this->parserFactory->newFromTitle( $parameters['title'] );
 
@@ -118,10 +118,10 @@ public function testProcess( $parameters ) {
 
 		$text   = '';
 
-		$instance = new ParserAfterTidy( $parser, $text );
+		$instance = new ParserAfterTidy( $parser );
 
 		$this->assertTrue(
-			$instance->process()
+			$instance->process( $text )
 		);
 	}
 
@@ -135,9 +135,7 @@ public function testSemanticDataParserOuputUpdateIntegration() {
 			'smwgShowHiddenCategories'  => true
 		);
 
-		foreach ( $settings as $key => $value ) {
-			$this->applicationFactory->getSettings()->set( $key, $value );
-		}
+		$this->testEnvironment->withConfiguration( $settings );
 
 		$text   = '';
 		$title  = Title::newFromText( __METHOD__ );
@@ -148,8 +146,11 @@ public function testSemanticDataParserOuputUpdateIntegration() {
 		$parser->getOutput()->addCategory( 'Bar', 'Bar' );
 		$parser->getOutput()->setProperty( 'smw-semanticdata-status', true );
 
-		$instance = new ParserAfterTidy( $parser, $text );
-		$this->assertTrue( $instance->process() );
+		$instance = new ParserAfterTidy( $parser );
+
+		$this->assertTrue(
+			$instance->process( $text  )
+		);
 
 		$expected = array(
 			'propertyCount'  => 2,