jenkins-bot has submitted this change and it was merged.
Change subject: \SMW\JobBase improve testability and eliminate GLOBALS from
SMW\UpdateJob
......................................................................
\SMW\JobBase improve testability and eliminate GLOBALS from SMW\UpdateJob
Code coverage: 100%
CRAP: 11
* Use JobBase to access general purpose Setter/Getter
* Move SMWUpdateJob to SMW\UpdateJob (SMWUpdateJob object is still accessible)
* Eliminate global wgParser from UpdateJob
* SMW\UpdateJob logic should go in its own class but for now ...
Change-Id: I3b41d4e506a2c425903ec3690f3500dcec939ac4
---
M includes/Setup.php
A includes/jobs/JobBase.php
M includes/jobs/PropertySubjectsUpdateDispatcherJob.php
M includes/jobs/UpdateJob.php
M tests/phpunit/MockObjectBuilder.php
A tests/phpunit/includes/jobs/JobBaseTest.php
M tests/phpunit/includes/jobs/UpdateJobTest.php
7 files changed, 491 insertions(+), 67 deletions(-)
Approvals:
Mwjames: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Setup.php b/includes/Setup.php
index 9a9ff03..17b4054 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -417,9 +417,10 @@
$wgAutoloadClasses['SMW\Test\CompatibilityTestCase'] =
$testsDir . 'CompatibilityTestCase.php';
// Jobs
- $wgJobClasses['SMWUpdateJob'] = 'SMWUpdateJob';
+ $wgAutoloadClasses['SMW\JobBase'] = $smwgIP .
'includes/jobs/JobBase.php';
+ $wgJobClasses['SMW\UpdateJob'] = 'SMW\UpdateJob';
$wgAutoloadClasses['SMWUpdateJob'] = $smwgIP .
'includes/jobs/UpdateJob.php';
- $wgAutoloadClasses['SMW\UpdateJob'] = $smwgIP .
'includes/jobs/UpdateJob.php'; // 1.9
+ $wgAutoloadClasses['SMW\UpdateJob'] = $smwgIP .
'includes/jobs/UpdateJob.php'; // 1.9
$wgJobClasses['SMWRefreshJob'] = 'SMWRefreshJob';
$wgAutoloadClasses['SMWRefreshJob'] = $smwgIP .
'includes/jobs/SMW_RefreshJob.php';
diff --git a/includes/jobs/JobBase.php b/includes/jobs/JobBase.php
new file mode 100644
index 0000000..d129a5c
--- /dev/null
+++ b/includes/jobs/JobBase.php
@@ -0,0 +1,118 @@
+<?php
+
+namespace SMW;
+
+use Job;
+
+/**
+ * Job base class
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @since 1.9
+ *
+ * @author mwjames
+ */
+
+/**
+ * Job base class
+ *
+ * @ingroup Job
+ */
+abstract class JobBase extends Job {
+
+ /** $var Store */
+ protected $store = null;
+
+ /** $var Settings */
+ protected $settings = null;
+
+ /** $var CacheHandler */
+ protected $cache = null;
+
+ /**
+ * Sets Store object
+ *
+ * @since 1.9
+ *
+ * @param Store $store
+ */
+ public function setStore( Store $store ) {
+ $this->store = $store;
+ }
+
+ /**
+ * Returns Store object
+ *
+ * @since 1.9
+ *
+ * @return Store
+ */
+ public function getStore() {
+
+ if ( $this->store === null ) {
+ $this->store = StoreFactory::getStore();
+ }
+
+ return $this->store;
+ }
+
+ /**
+ * Sets Settings object
+ *
+ * @since 1.9
+ *
+ * @param Store $store
+ */
+ public function setSettings( Settings $settings ) {
+ $this->settings = $settings;
+ }
+
+ /**
+ * Returns Settings object
+ *
+ * @since 1.9
+ *
+ * @return Settings
+ */
+ public function getSettings() {
+
+ if ( $this->settings === null ) {
+ $this->settings = Settings::newFromGlobals();
+ }
+
+ return $this->settings;
+ }
+
+ /**
+ * Returns CacheHandler object
+ *
+ * @since 1.9
+ *
+ * @return CacheHandler
+ */
+ public function getCache() {
+
+ if ( $this->cache === null ) {
+ $this->cache = CacheHandler::newFromId(
$this->getSettings()->get( 'smwgCacheType' ) );
+ }
+
+ return $this->cache;
+ }
+}
diff --git a/includes/jobs/PropertySubjectsUpdateDispatcherJob.php
b/includes/jobs/PropertySubjectsUpdateDispatcherJob.php
index e0d4142..284495e 100644
--- a/includes/jobs/PropertySubjectsUpdateDispatcherJob.php
+++ b/includes/jobs/PropertySubjectsUpdateDispatcherJob.php
@@ -39,10 +39,7 @@
* @ingroup Job
* @ingroup Dispatcher
*/
-class PropertySubjectsUpdateDispatcherJob extends Job {
-
- /** $var Store */
- protected $store = null;
+class PropertySubjectsUpdateDispatcherJob extends JobBase {
/** $var Job */
protected $jobs = array();
@@ -60,17 +57,6 @@
public function __construct( Title $title, $params = array(), $id = 0 )
{
parent::__construct( 'SMW\PropertySubjectsUpdateDispatcherJob',
$title, $params, $id );
$this->store = StoreFactory::getStore( isset( $params['store']
) ? $params['store'] : null );
- }
-
- /**
- * Sets Store object
- *
- * @since 1.9
- *
- * @param Store $store
- */
- public function setStore( Store $store ) {
- $this->store = $store;
}
/**
diff --git a/includes/jobs/UpdateJob.php b/includes/jobs/UpdateJob.php
index 7d0e471..843aea9 100644
--- a/includes/jobs/UpdateJob.php
+++ b/includes/jobs/UpdateJob.php
@@ -1,16 +1,48 @@
<?php
+
+namespace SMW;
+
+use ParserOutput;
+use LinkCache;
+use WikiPage;
+use Revision;
+use Title;
+use User;
+use Job;
+
/**
- * File containing SMWUpdateJob.
+ * UpdateJob is responsible for the asynchronous update of semantic data
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @since 1.9
*
* @author Daniel M. Herzig
* @author Markus Krötzsch
- * @file
- * @ingroup SMW
+ * @author mwjames
*/
/**
- * SMWUpdateJob updates the semantic data in the database for a given title
- * using the MediaWiki JobQueue. Update jobs are created if, when saving an
article,
+ * UpdateJob is responsible for the asynchronous update of semantic data
+ * using MediaWiki's JobQueue infrastructure.
+ *
+ * Update jobs are created if, when saving an article,
* it is detected that the content of other pages must be re-parsed as well
(e.g.
* due to some type change).
*
@@ -19,69 +51,98 @@
* formatting in-page values based on a datatype thathas since been changed),
whereas
* the Factbox and query/browsing interfaces might already show the updated
records.
*
- * @ingroup SMW
+ * @ingroup Job
*/
-class SMWUpdateJob extends Job {
+class UpdateJob extends JobBase {
- function __construct( Title $title ) {
- parent::__construct( 'SMWUpdateJob', $title );
- }
+ /** @var WikiPage */
+ protected $wikiPage = null;
+
+ /** @var Revision */
+ protected $revision = null;
+
+ /** @var ParserOutput */
+ protected $parserOutput = null;
/**
- * Returns Title object
+ * @since 1.9
*
- * @since 1.9
- *
- * @return \Title
+ * @param Title $title
*/
- public function getTitle() {
- return $this->title;
+ function __construct( Title $title ) {
+ parent::__construct( 'SMW\UpdateJob', $title );
}
/**
* Run job
* @return boolean success
*/
- function run() {
- wfProfileIn( 'SMWUpdateJob::run (SMW)' );
- global $wgParser;
+ public function run() {
+ Profiler::In( __METHOD__ . '-run' );
LinkCache::singleton()->clear();
- if ( is_null( $this->title ) ) {
- $this->error = "SMWUpdateJob: Invalid title";
- wfProfileOut( 'SMWUpdateJob::run (SMW)' );
+ if ( $this->getTitle() === null ) {
+ $this->setLastError( __METHOD__ . ': Invalid title' );
+ Profiler::Out( __METHOD__ . '-run' );
return false;
- } elseif ( !$this->title->exists() ) {
- smwfGetStore()->deleteSubject( $this->title ); // be
sure to clear the data
- wfProfileOut( 'SMWUpdateJob::run (SMW)' );
+ } elseif ( !$this->getTitle()->exists() ) {
+ $this->getStore()->deleteSubject( $this->getTitle() );
// be sure to clear the data
+ Profiler::Out( __METHOD__ . '-run' );
return true;
}
- $revision = Revision::newFromTitle( $this->title );
- if ( !$revision ) {
- $this->error = 'SMWUpdateJob: Page exists but no
revision was found for "' . $this->title->getPrefixedDBkey() . '"';
- wfProfileOut( 'SMWUpdateJob::run (SMW)' );
+ if ( !$this->getParserOutput() instanceof ParserOutput ) {
return false;
}
- wfProfileIn( __METHOD__ . '-parse' );
- $options = new ParserOptions();
- $output = $wgParser->parse( $revision->getText(), $this->title,
$options, true, true, $revision->getID() );
+ Profiler::In( __METHOD__ . '-update' );
- wfProfileOut( __METHOD__ . '-parse' );
- wfProfileIn( __METHOD__ . '-update' );
-
- // @since 1.9
- // SMWParseData::storeData( $output, $this->title, false );
- $parserData = new SMW\ParserData( $this->title, $output );
+ $parserData = new ParserData( $this->getTitle(),
$this->getParserOutput() );
$parserData->disableUpdateJobs();
$parserData->updateStore();
- wfProfileOut( __METHOD__ . '-update' );
- wfProfileOut( 'SMWUpdateJob::run (SMW)' );
+ Profiler::Out( __METHOD__ . '-update' );
+ Profiler::Out( __METHOD__ . '-run' );
return true;
+ }
+
+ /**
+ * Builds and returns a ParserOutput object
+ *
+ * @since 1.9
+ *
+ * @return ParserOutput|null
+ */
+ public function getParserOutput() {
+
+ if ( $this->parserOutput === null ) {
+ Profiler::In( __METHOD__ );
+
+ if ( $this->wikiPage === null ) {
+ $this->wikiPage = WikiPage::factory(
$this->getTitle() );
+ }
+
+ if ( $this->revision === null && $this->wikiPage !==
null ) {
+ $this->revision =
$this->wikiPage->getRevision();
+ }
+
+ if ( !$this->revision || $this->revision === null ) {
+ $this->setLastError( __METHOD__ . " No revision
available for {$this->getTitle()->getPrefixedDBkey()}" );
+ Profiler::Out( __METHOD__ );
+ return false;
+ }
+
+ $this->parserOutput = $this->wikiPage->getParserOutput(
+ $this->wikiPage->makeParserOptions(
User::newFromId( $this->revision->getUser() ) ),
+ $this->revision->getId()
+ );
+
+ Profiler::Out( __METHOD__ );
+ }
+
+ return $this->parserOutput;
}
/**
@@ -89,19 +150,20 @@
* disables jobs.
* @note Any method that inserts jobs with Job::batchInsert or
otherwise must
* implement this check individually. The below is not called in these
cases.
+ *
+ * @codeCoverageIgnore
*/
function insert() {
- global $smwgEnableUpdateJobs;
- if ( $smwgEnableUpdateJobs ) {
+ if ( $this->getSettings()->get( 'smwgEnableUpdateJobs' ) ) {
parent::insert();
}
}
-
}
/**
- * SMWUpdateJob class alias
+ * SMWUpdateJob
*
- * @since 1.9
+ * @deprecated since 1.9
+ * @codeCoverageIgnore
*/
-class_alias( 'SMWUpdateJob', 'SMW\UpdateJob' );
+class_alias( 'SMW\UpdateJob', 'SMWUpdateJob' );
diff --git a/tests/phpunit/MockObjectBuilder.php
b/tests/phpunit/MockObjectBuilder.php
index a767270..67d4e9d 100644
--- a/tests/phpunit/MockObjectBuilder.php
+++ b/tests/phpunit/MockObjectBuilder.php
@@ -135,6 +135,22 @@
}
/**
+ * Returns a ParserOptions object
+ *
+ * @since 1.9
+ *
+ * @return ParserOptions
+ */
+ public function getMockParserOptions() {
+
+ $parserOptions = $this->getMockBuilder( 'ParserOptions' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ return $parserOptions;
+ }
+
+ /**
* Returns a WikiPage object
*
* @since 1.9
@@ -150,6 +166,18 @@
$wikiPage->expects( $this->any() )
->method( 'getTimestamp' )
->will( $this->returnValue( $this->set( 'getTimestamp'
) ) );
+
+ $wikiPage->expects( $this->any() )
+ ->method( 'getRevision' )
+ ->will( $this->returnValue( $this->set( 'getRevision' )
) );
+
+ $wikiPage->expects( $this->any() )
+ ->method( 'getParserOutput' )
+ ->will( $this->returnValue( $this->set(
'getParserOutput' ) ) );
+
+ $wikiPage->expects( $this->any() )
+ ->method( 'makeParserOptions' )
+ ->will( $this->returnValue( $this->set(
'makeParserOptions' ) ) );
return $wikiPage;
}
@@ -174,6 +202,14 @@
$revision->expects( $this->any() )
->method( 'getParentId' )
->will( $this->returnValue( $this->set( 'getParentId' )
) );
+
+ $revision->expects( $this->any() )
+ ->method( 'getId' )
+ ->will( $this->returnValue( $this->set( 'getId' ) ) );
+
+ $revision->expects( $this->any() )
+ ->method( 'getUser' )
+ ->will( $this->returnValue( $this->set( 'getUser' ) ) );
return $revision;
}
@@ -347,6 +383,10 @@
->will( $this->returnValue( $this->set(
'getPropertiesSpecial' ) ) );
$store->expects( $this->any() )
+ ->method( 'deleteSubject' )
+ ->will( $this->returnValue( $this->set( 'deleteSubject'
) ) );
+
+ $store->expects( $this->any() )
->method( 'getUnusedPropertiesSpecial' )
->will( $this->returnValue( $this->set(
'getUnusedPropertiesSpecial' ) ) );
@@ -439,7 +479,7 @@
$title->expects( $this->any() )
->method( 'getNamespace' )
- ->will( $this->returnValue( $this->set( 'getNamespace'
) ) );
+ ->will( $this->returnValue( $this->set( 'getNamespace',
NS_MAIN ) ) );
$title->expects( $this->any() )
->method( 'isKnown' )
diff --git a/tests/phpunit/includes/jobs/JobBaseTest.php
b/tests/phpunit/includes/jobs/JobBaseTest.php
new file mode 100644
index 0000000..1b90079
--- /dev/null
+++ b/tests/phpunit/includes/jobs/JobBaseTest.php
@@ -0,0 +1,115 @@
+<?php
+
+namespace SMW\Test;
+
+use SMW\JobBase;
+
+/**
+ * Tests for the JobBase class
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @since 1.9
+ *
+ * @author mwjames
+ */
+
+/**
+ * @covers \SMW\JobBase
+ *
+ * @ingroup Test
+ *
+ * @group SMW
+ * @group SMWExtension
+ */
+class JobBaseTest extends SemanticMediaWikiTestCase {
+
+ /**
+ * Returns the name of the class to be tested
+ *
+ * @return string|false
+ */
+ public function getClass() {
+ return '\SMW\JobBase';
+ }
+
+ /**
+ * Helper method that returns a JobBase object
+ *
+ * @since 1.9
+ *
+ * @param $result
+ *
+ * @return JobBase
+ */
+ private function getInstance() {
+ return $this->getMockForAbstractClass( $this->getClass(),
array( $this->newTitle(), array() ) );
+ }
+
+ /**
+ * @test JobBase::__construct
+ *
+ * @since 1.9
+ */
+ public function testConstructor() {
+ $this->assertInstanceOf( $this->getClass(),
$this->getInstance() );
+ }
+
+ /**
+ * @test JobBase::getStore
+ * @test JobBase::setStore
+ *
+ * @since 1.9
+ */
+ public function testGetSetStore() {
+
+ $instance = $this->getInstance();
+
+ $this->assertInstanceOf( '\SMW\Store', $instance->getStore() );
+ $instance->setStore( $this->newMockObject()->getMockStore() );
+ $this->assertInstanceOf( '\SMW\Store', $instance->getStore() );
+ }
+
+ /**
+ * @test JobBase::setSettings
+ * @test JobBase::getSettings
+ *
+ * @since 1.9
+ */
+ public function testGetSetSettings() {
+
+ $instance = $this->getInstance();
+
+ $this->assertInstanceOf( '\SMW\Settings',
$instance->getSettings() );
+ $instance->setSettings( $this->getSettings() );
+ $this->assertInstanceOf( '\SMW\Settings',
$instance->getSettings() );
+ }
+
+ /**
+ * @test JobBase::getCache
+ *
+ * @since 1.9
+ */
+ public function testGetCache() {
+
+ $instance = $this->getInstance();
+ $this->assertInstanceOf( '\SMW\CacheHandler',
$instance->getCache() );
+ }
+
+}
diff --git a/tests/phpunit/includes/jobs/UpdateJobTest.php
b/tests/phpunit/includes/jobs/UpdateJobTest.php
index b3f312b..d4ac56e 100644
--- a/tests/phpunit/includes/jobs/UpdateJobTest.php
+++ b/tests/phpunit/includes/jobs/UpdateJobTest.php
@@ -4,6 +4,7 @@
use SMW\UpdateJob;
+use Title;
/**
* Tests for the UpdateJob class
*
@@ -38,7 +39,7 @@
* @group SMW
* @group SMWExtension
*/
-class UpdateJobTest extends SemanticMediaWikiTestCase {
+class UpdateJobTest extends ParserTestCase {
/**
* Returns the name of the class to be tested
@@ -56,12 +57,20 @@
*
* @return UpdateJob
*/
- private function getInstance() {
- return new UpdateJob( $this->getTitle() );
+ private function getInstance( Title $title = null ) {
+ $instance = new UpdateJob( $title === null ? $this->newTitle()
: $title );
+
+ // Set smwgEnableUpdateJobs to false in order to avoid having
jobs being
+ // inserted as real jobs to the queue
+ $instance->setSettings( $this->getSettings( array(
'smwgEnableUpdateJobs' => false ) ) );
+ return $instance;
}
/**
* @test UpdateJob::__construct
+ *
+ * FIXME Delete SMWUpdateJob assertion after all references to
+ * SMWUpdateJob have been removed
*
* @since 1.9
*/
@@ -70,4 +79,97 @@
$this->assertInstanceOf( 'SMWUpdateJob', $this->getInstance() );
}
+ /**
+ * @test UpdateJob::run
+ * @dataProvider titleWikiPageDataProvider
+ *
+ * @since 1.9
+ */
+ public function testRun( $test, $expected ) {
+
+ $instance = $this->getInstance( $test['title'] );
+ $instance->setStore( $this->newMockObject()->getMockStore() );
+
+ // Provides access to the wikiPage property and
+ // replace it with a controllable mock bject
+ $reflector = $this->newReflector();
+ $wikiPage = $reflector->getProperty( 'wikiPage' );
+ $wikiPage->setAccessible( true );
+ $wikiPage->setValue( $instance, $test['wikiPage'] );
+
+ $this->assertEquals( $expected['result'], $instance->run() );
+ }
+
+ /**
+ * Provides title and wikiPage samples
+ *
+ * @return array
+ */
+ public function titleWikiPageDataProvider() {
+
+ $provider = array();
+ $wikiPage = $this->newMockObject()->getMockWikiPage();
+
+ // #0 Title does not exists, deleteSubject() is being executed
+ $title = $this->newMockObject( array(
+ 'getDBkey' => 'Lila',
+ 'exists' => false
+ ) )->getMockTitle();
+
+ $provider[] = array(
+ array(
+ 'title' => $title,
+ 'wikiPage' => $wikiPage,
+ ),
+ array(
+ 'result' => true
+ )
+ );
+
+ // #1 No revision, no further activities
+ $title = $this->newMockObject( array(
+ 'getDBkey' => 'Lala',
+ 'exists' => true
+ ) )->getMockTitle();
+
+ $provider[] = array(
+ array(
+ 'title' => $title,
+ 'wikiPage' => null
+ ),
+ array(
+ 'result' => false
+ )
+ );
+
+ // #2 Valid revision and parserOuput
+ $title = $this->newMockObject( array(
+ 'getDBkey' => 'Lula',
+ 'exists' => true
+ ) )->getMockTitle();
+
+ $revision = $this->newMockObject( array(
+ 'getId' => 9001,
+ 'getUser' => 'Lala'
+ ) )->getMockRevision();
+
+ $wikiPage = $this->newMockObject( array(
+ 'getRevision' => $revision,
+ 'makeParserOptions' =>
$this->newMockObject()->getMockParserOptions(),
+ 'getParserOutput' => $this->newParserOutput()
+ ) )->getMockWikiPage();
+
+ $provider[] = array(
+ array(
+ 'title' => $title,
+ 'wikiPage' => $wikiPage
+ ),
+ array(
+ 'result' => true
+ )
+ );
+
+ return $provider;
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/74840
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3b41d4e506a2c425903ec3690f3500dcec939ac4
Gerrit-PatchSet: 5
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
