jenkins-bot has submitted this change and it was merged.
Change subject: UpdateObserver and JobBase to implement ContextAware
......................................................................
UpdateObserver and JobBase to implement ContextAware
Instead of implementing DependencyRequestor, implement ContextAware to
loosen dependency between DIC and a requestor.
Change-Id: Ie5e74dfd56f5fecd7391285585622f80681db97a
---
M includes/UpdateObserver.php
M includes/dic/SharedDependencyContainer.php
M includes/jobs/JobBase.php
M includes/jobs/UpdateDispatcherJob.php
M includes/jobs/UpdateJob.php
M tests/phpunit/includes/UpdateObserverTest.php
M tests/phpunit/includes/dic/SharedDependencyContainerTest.php
M tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
M tests/phpunit/includes/jobs/UpdateJobTest.php
9 files changed, 109 insertions(+), 216 deletions(-)
Approvals:
Mwjames: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/UpdateObserver.php b/includes/UpdateObserver.php
index 1828c2b..2a50f8b 100644
--- a/includes/UpdateObserver.php
+++ b/includes/UpdateObserver.php
@@ -5,53 +5,43 @@
/**
* Observer for independent update transactions
*
- * @file
- *
- * @license GNU GPL v2+
- * @since 1.9
- *
- * @author mwjames
- */
-
-/**
- * Observer for independent update transactions
- *
* Using this observer can help to enforce loose coupling by having
* a Publisher (ObservableSubject) sent a notification (state change)
* to this observer which will independently act from the source of
* the notification
*
- * @note When testing rountrips, use the MockUpdateObserver instead
+ * @note When testing round-trips, use the MockUpdateObserver instead
*
- * @ingroup Observer
+ * @licence GNU GPL v2+
+ * @since 1.9
+ *
+ * @author mwjames
*/
-class UpdateObserver extends Observer implements DependencyRequestor {
+class UpdateObserver extends Observer implements ContextAware, ContextInjector
{
- /** @var DependencyBuilder */
- protected $dependencyBuilder = null;
+ /** @var ContextResource */
+ protected $context = null;
/**
- * @see DependencyRequestor::setDependencyBuilder
- *
* @since 1.9
*
- * @param DependencyBuilder $builder
+ * @param ContextResource
*/
- public function setDependencyBuilder( DependencyBuilder $builder ) {
- $this->dependencyBuilder = $builder;
+ public function invokeContext( ContextResource $context ) {
+ $this->context = $context;
}
/**
- * @see DependencyRequestor::getDependencyBuilder
+ * @see ContextAware::withContext
*
* @since 1.9
*
- * @return DependencyBuilder
+ * @return ContextResource
*/
- public function getDependencyBuilder() {
+ public function withContext() {
// This is not as clean as it should be but to avoid to make
- // multipe changes at once we determine a default builder here
+ // multiple changes at once we determine a default builder here
// which at some point should vanish after pending changes have
// been merged
@@ -59,11 +49,11 @@
// UpdateJob does not
// ParserAfterTidy does not
- if ( $this->dependencyBuilder === null ) {
- $this->dependencyBuilder = new SimpleDependencyBuilder(
new SharedDependencyContainer() );
+ if ( $this->context === null ) {
+ $this->context = new BaseContext();
}
- return $this->dependencyBuilder;
+ return $this->context;
}
/**
@@ -80,17 +70,12 @@
*/
public function runStoreUpdater( ParserData $subject ) {
- /**
- * @var Settings $settings
- */
- $settings = $this->getDependencyBuilder()->newObject(
'Settings' );
+ $updater = new StoreUpdater(
+ $this->withContext()->getStore(),
+ $subject->getData(),
+ $this->withContext()->getSettings()
+ );
- /**
- * @var Store $store
- */
- $store = $this->getDependencyBuilder()->newObject( 'Store' );
-
- $updater = new StoreUpdater( $store, $subject->getData(),
$settings );
$updater->setUpdateStatus( $subject->getUpdateStatus()
)->doUpdate();
return true;
@@ -119,7 +104,7 @@
public function runUpdateDispatcher( TitleAccess $subject ) {
$dispatcher = new UpdateDispatcherJob( $subject->getTitle() );
- $dispatcher->setDependencyBuilder(
$this->getDependencyBuilder() );
+ $dispatcher->invokeContext( $this->withContext() );
$dispatcher->run();
return true;
diff --git a/includes/dic/SharedDependencyContainer.php
b/includes/dic/SharedDependencyContainer.php
index b0b1527..55fd7b8 100644
--- a/includes/dic/SharedDependencyContainer.php
+++ b/includes/dic/SharedDependencyContainer.php
@@ -280,7 +280,7 @@
protected function getUpdateObserver() {
return function ( DependencyBuilder $builder ) {
$updateObserver = new UpdateObserver();
- $updateObserver->setDependencyBuilder( $builder );
+ $updateObserver->invokeContext( $builder->newObject(
'BaseContext' ) );
return $updateObserver;
};
}
diff --git a/includes/jobs/JobBase.php b/includes/jobs/JobBase.php
index 642232c..a9c1c0a 100644
--- a/includes/jobs/JobBase.php
+++ b/includes/jobs/JobBase.php
@@ -6,45 +6,44 @@
use Title;
/**
+ * Job base class
+ *
+ * @licence GNU GPL v2+
* @since 1.9
*
- * @license GNU GPL v2+
* @author mwjames
*/
-abstract class JobBase extends Job implements DependencyRequestor {
+abstract class JobBase extends Job implements ContextAware, ContextInjector {
- /** @var DependencyBuilder */
- protected $dependencyBuilder = null;
+ /** @var ContextResource */
+ protected $context = null;
/**
- * @see DependencyRequestor::setDependencyBuilder
- *
* @since 1.9
*
- * @param DependencyBuilder $builder
+ * @param ContextResource
*/
- public function setDependencyBuilder( DependencyBuilder $builder ) {
- $this->dependencyBuilder = $builder;
+ public function invokeContext( ContextResource $context ) {
+ $this->context = $context;
}
/**
- * @see DependencyRequestor::getDependencyBuilder
+ * @see ContextAware::withContext
*
* @since 1.9
*
- * @return DependencyBuilder
+ * @return ContextResource
*/
- public function getDependencyBuilder() {
+ public function withContext() {
// JobBase is a top-level class and to avoid a non-instantiated
object
- // a default builder is set as for when Jobs are triggered using
- // command line etc.
-
- if ( $this->dependencyBuilder === null ) {
- $this->dependencyBuilder = new SimpleDependencyBuilder(
new SharedDependencyContainer() );
+ // a default builder is set as for when Jobs are triggered
without
+ // injected context (during command line etc.)
+ if ( $this->context === null ) {
+ $this->context = new BaseContext();
}
- return $this->dependencyBuilder;
+ return $this->context;
}
/**
diff --git a/includes/jobs/UpdateDispatcherJob.php
b/includes/jobs/UpdateDispatcherJob.php
index 0c018e5..baf3e61 100644
--- a/includes/jobs/UpdateDispatcherJob.php
+++ b/includes/jobs/UpdateDispatcherJob.php
@@ -6,22 +6,15 @@
use Job;
/**
- * Dispatcher class to enable to run in deferred or immediate update mode
+ * Dispatcher class to invoke UpdateJob's
*
- * @file
+ * Can be run either in deferred or immediate mode to restore the data parity
+ * between a property and its attached subjects
*
- * @license GNU GPL v2+
- * @since 1.9
+ * @licence GNU GPL v2+
+ * @since 1.9
*
* @author mwjames
- */
-
-/**
- * Dispatcher class to enable to run UpdateJob in deferred or immediate mode
- * that restores the data parity between a property and its attached subjects
- *
- * @ingroup Job
- * @ingroup Dispatcher
*/
class UpdateDispatcherJob extends JobBase {
@@ -99,7 +92,7 @@
/**
* @var Store $store
*/
- $store = $this->getDependencyBuilder()->newObject( 'Store' );
+ $store = $this->withContext()->getStore();
// Array of all subjects that have some value for the given
property
$subjects = $store->getAllPropertySubjects( $property );
@@ -171,13 +164,7 @@
* @codeCoverageIgnore
*/
public function insert() {
-
- /**
- * @var Settings $settings
- */
- $settings = $this->getDependencyBuilder()->newObject(
'Settings' );
-
- if ( $settings->get( 'smwgEnableUpdateJobs' ) ) {
+ if ( $this->withContext()->getSettings()->get(
'smwgEnableUpdateJobs' ) ) {
parent::insert();
}
}
diff --git a/includes/jobs/UpdateJob.php b/includes/jobs/UpdateJob.php
index 9fcbce3..7bbda9d 100644
--- a/includes/jobs/UpdateJob.php
+++ b/includes/jobs/UpdateJob.php
@@ -63,13 +63,7 @@
* @return boolean
*/
private function clearData() {
- /**
- * @var Store $store
- */
- $store = $this->getDependencyBuilder()->newObject( 'Store' );
-
- $store->deleteSubject( $this->getTitle() );
-
+ $this->withContext()->getStore()->deleteSubject(
$this->getTitle() );
return true;
}
@@ -77,10 +71,11 @@
* @return boolean
*/
private function runContentParser() {
+
/**
* @var ContentParser $contentParser
*/
- $contentParser = $this->getDependencyBuilder()->newObject(
'ContentParser', array(
+ $contentParser =
$this->withContext()->getDependencyBuilder()->newObject( 'ContentParser', array(
'Title' => $this->getTitle()
) );
@@ -105,13 +100,13 @@
/**
* @var CacheHandler $cache
*/
- $cache = $this->getDependencyBuilder()->newObject(
'CacheHandler' );
+ $cache =
$this->withContext()->getDependencyBuilder()->newObject( 'CacheHandler' );
$cache->setKey( FactboxCache::newCacheId(
$this->getTitle()->getArticleID() ) )->delete();
/**
* @var ParserData $parserData
*/
- $parserData = $this->getDependencyBuilder()->newObject(
'ParserData', array(
+ $parserData =
$this->withContext()->getDependencyBuilder()->newObject( 'ParserData', array(
'Title' => $this->getTitle(),
'ParserOutput' => $parserOutput
) );
@@ -127,19 +122,14 @@
*
* This actually files the job. This is prevented if the configuration
of SMW
* 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
*/
public function insert() {
-
- /**
- * @var Settings $settings
- */
- $settings = $this->getDependencyBuilder()->newObject(
'Settings' );
-
- if ( $settings->get( 'smwgEnableUpdateJobs' ) ) {
+ if ( $this->withContext()->getSettings()->get(
'smwgEnableUpdateJobs' ) ) {
parent::insert();
}
}
diff --git a/tests/phpunit/includes/UpdateObserverTest.php
b/tests/phpunit/includes/UpdateObserverTest.php
index f18a6e1..781fcfa 100644
--- a/tests/phpunit/includes/UpdateObserverTest.php
+++ b/tests/phpunit/includes/UpdateObserverTest.php
@@ -3,31 +3,22 @@
namespace SMW\Test;
use SMW\UpdateObserver;
-
-/**
- * Tests for the UpdateObserver class
- *
- * @file
- *
- * @license GNU GPL v2+
- * @since 1.9
- *
- * @author mwjames
- */
+use SMW\BaseContext;
/**
* @covers \SMW\UpdateObserver
*
- * @ingroup Test
+ * @licence GNU GPL v2+
+ * @since 1.9
*
* @group SMW
* @group SMWExtension
+ *
+ * @author mwjames
*/
class UpdateObserverTest extends SemanticMediaWikiTestCase {
/**
- * Returns the name of the class to be tested
- *
* @return string|false
*/
public function getClass() {
@@ -35,36 +26,30 @@
}
/**
- * Helper method that returns a UpdateObserver object
- *
* @since 1.9
- *
- * @param $data
*
* @return UpdateObserver
*/
- private function newInstance() {
-
- $observer = new UpdateObserver();
-
- // Use the provided default builder
- $observer->setDependencyBuilder(
$observer->getDependencyBuilder() );
+ private function newInstance( $settings = array() ) {
$mockStore = $this->newMockBuilder()->newObject( 'Store', array(
'getAllPropertySubjects' => array(),
'getPropertySubjects' => array()
) );
- $observer->getDependencyBuilder()
- ->getContainer()
- ->registerObject( 'Store', $mockStore );
+ $context = new BaseContext();
- return $observer;
+ $container = $context->getDependencyBuilder()->getContainer();
+ $container->registerObject( 'Store', $mockStore );
+ $container->registerObject( 'Settings', $this->newSettings(
$settings ) );
+
+ $instance = new UpdateObserver();
+ $instance->invokeContext( $context );
+
+ return $instance;
}
/**
- * @test UpdateObserver::__construct
- *
* @since 1.9
*/
public function testConstructor() {
@@ -72,42 +57,32 @@
}
/**
- * @test UpdateObserver::runUpdateDispatcher
* @dataProvider updateDispatcherDataProvider
*
* @since 1.9
*/
public function testUpdateDispatcherJob( $setup, $expected ) {
- $instance = $this->newInstance();
-
- $instance->getDependencyBuilder()
- ->getContainer()
- ->registerObject( 'Settings', $this->newSettings(
$setup['settings'] ) );
+ $instance = $this->newInstance( $setup['settings'] );
$this->assertTrue(
$instance->runUpdateDispatcher( $setup['title'] ),
- 'asserts that runUpdateDispatcher always returns true'
+ 'Asserts that runUpdateDispatcher always returns true'
);
}
/**
- * @test UpdateObserver::runUpdateDispatcher
* @dataProvider storeUpdaterDataProvider
*
* @since 1.9
*/
public function testStoreUpdater( $setup, $expected ) {
- $instance = $this->newInstance();
-
- $instance->getDependencyBuilder()
- ->getContainer()
- ->registerObject( 'Settings', $this->newSettings(
$setup['settings'] ) );
+ $instance = $this->newInstance( $setup['settings'] );
$this->assertTrue(
$instance->runStoreUpdater( $setup['parserData'] ),
- 'asserts that runStoreUpdater always returns true'
+ 'Asserts that runStoreUpdater always returns true'
);
}
diff --git a/tests/phpunit/includes/dic/SharedDependencyContainerTest.php
b/tests/phpunit/includes/dic/SharedDependencyContainerTest.php
index 8c1d445..6d177b6 100644
--- a/tests/phpunit/includes/dic/SharedDependencyContainerTest.php
+++ b/tests/phpunit/includes/dic/SharedDependencyContainerTest.php
@@ -120,10 +120,10 @@
$provider[] = array( 'Settings', array(
'\SMW\Settings' => array() ) );
$provider[] = array( 'Store', array(
'\SMW\Store' => array() ) );
$provider[] = array( 'CacheHandler', array(
'\SMW\CacheHandler' => array() ) );
+ $provider[] = array( 'BaseContext', array(
'\SMW\ContextResource' => array() ) );
$provider[] = array( 'NamespaceExaminer', array(
'\SMW\NamespaceExaminer' => array() ) );
$provider[] = array( 'UpdateObserver', array(
'\SMW\UpdateObserver' => array() ) );
$provider[] = array( 'ObservableUpdateDispatcher', array(
'\SMW\ObservableSubjectDispatcher' => array() ) );
- $provider[] = array( 'BaseContext', array(
'\SMW\BaseContext' => array() ) );
$provider[] = array( 'RequestContext', array(
'\IContextSource' => array() ) );
diff --git a/tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
b/tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
index 04d4d03..c0a5020 100644
--- a/tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
+++ b/tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
@@ -2,30 +2,22 @@
namespace SMW\Test;
-use SMW\SharedDependencyContainer;
use SMW\UpdateDispatcherJob;
+use SMW\BaseContext;
use SMW\DIProperty;
use Title;
/**
- * Tests for the UpdateDispatcherJob class
- *
- * @file
- *
- * @license GNU GPL v2+
- * @since 1.9
- *
- * @author mwjames
- */
-
-/**
* @covers \SMW\UpdateDispatcherJob
*
- * @ingroup Test
+ * @licence GNU GPL v2+
+ * @since 1.9
*
* @group SMW
* @group SMWExtension
+ *
+ * @author mwjames
*/
class UpdateDispatcherJobTest extends SemanticMediaWikiTestCase {
@@ -36,8 +28,6 @@
protected $subjects;
/**
- * Returns the name of the class to be tested
- *
* @return string|false
*/
public function getClass() {
@@ -45,11 +35,7 @@
}
/**
- * Helper method that returns a UpdateDispatcherJob object
- *
* @since 1.9
- *
- * @param Title|null $title
*
* @return UpdateDispatcherJob
*/
@@ -69,19 +55,19 @@
'smwgEnableUpdateJobs' => false
) );
- $container = new SharedDependencyContainer();
+ $context = new BaseContext();
+
+ $container = $context->getDependencyBuilder()->getContainer();
$container->registerObject( 'Store', $mockStore );
$container->registerObject( 'Settings', $settings );
$instance = new UpdateDispatcherJob( $title );
- $instance->setDependencyBuilder( $this->newDependencyBuilder(
$container ) );
+ $instance->invokeContext( $context );
return $instance;
}
/**
- * @test UpdateDispatcherJob::__construct
- *
* @since 1.9
*/
public function testConstructor() {
@@ -89,10 +75,7 @@
}
/**
- * @test UpdateDispatcherJob::push
- *
- * Just verify that the push method is accessible
- * without inserting any real job
+ * Just verify that the push method is accessible without inserting any
real job
*
* @since 1.9
*/
@@ -101,8 +84,6 @@
}
/**
- * @test UpdateDispatcherJob::run
- *
* @since 1.9
*/
public function testRunOnDB() {
@@ -113,7 +94,6 @@
}
/**
- * @test UpdateDispatcherJob::run
* @dataProvider subjectDataProvider
*
* @since 1.9
diff --git a/tests/phpunit/includes/jobs/UpdateJobTest.php
b/tests/phpunit/includes/jobs/UpdateJobTest.php
index ef907d8..a12dd0a 100644
--- a/tests/phpunit/includes/jobs/UpdateJobTest.php
+++ b/tests/phpunit/includes/jobs/UpdateJobTest.php
@@ -2,36 +2,26 @@
namespace SMW\Test;
-use SMW\SharedDependencyContainer;
+use SMW\BaseContext;
use SMW\UpdateJob;
use Title;
/**
- * Tests for the UpdateJob class
- *
- * @file
- *
- * @license GNU GPL v2+
- * @since 1.9
- *
- * @author mwjames
- */
-
-/**
* @covers \SMW\UpdateJob
* @covers \SMW\JobBase
*
- * @ingroup Test
+ * @licence GNU GPL v2+
+ * @since 1.9
*
* @group SMW
* @group SMWExtension
+ *
+ * @author mwjames
*/
class UpdateJobTest extends ParserTestCase {
/**
- * Returns the name of the class to be tested
- *
* @return string|false
*/
public function getClass() {
@@ -39,45 +29,36 @@
}
/**
- * Helper method that returns a UpdateJob object
- *
* @since 1.9
*
* @return UpdateJob
*/
- private function newInstance( Title $title = null, $settings = null ) {
+ private function newInstance( Title $title = null ) {
if ( $title === null ) {
$title = $this->newTitle();
}
- // Set smwgEnableUpdateJobs to false in order to avoid having
jobs being
- // inserted as real jobs to the queue
- if ( $settings === null ) {
- $settings = $this->newSettings( array(
- 'smwgCacheType' => 'hash',
- 'smwgEnableUpdateJobs' => false
- ) );
- }
+ $settings = $this->newSettings( array(
+ 'smwgCacheType' => 'hash',
+ 'smwgEnableUpdateJobs' => false // false in order to
avoid having jobs being inserted
+ ) );
+
+ $mockStore = $this->newMockBuilder()->newObject( 'Store' );
+
+ $context = new BaseContext();
+
+ $container = $context->getDependencyBuilder()->getContainer();
+ $container->registerObject( 'Store', $mockStore );
+ $container->registerObject( 'Settings', $settings );
$instance = new UpdateJob( $title );
-
- $builder = $instance->getDependencyBuilder();
- $container = $builder->getContainer();
- $container->registerObject( 'Settings', $settings );
- $container->registerObject( 'Store',
$this->newMockBuilder()->newObject( 'Store' ) );
-
- // This seems redundant but it allows to cover
- // all necessary methods provided by the JobBase
- $instance->setDependencyBuilder( $builder );
+ $instance->invokeContext( $context );
return $instance;
-
}
/**
- * @test UpdateJob::__construct
- *
* FIXME Delete SMWUpdateJob assertion after all references to
* SMWUpdateJob have been removed
*
@@ -89,8 +70,6 @@
}
/**
- * @test UpdateJob::__construct
- *
* @since 1.9
*/
public function testRun() {
@@ -101,13 +80,12 @@
$this->assertFalse(
$this->newInstance( $title )->run(),
- 'asserts that the run() returns false due to a missing
ParserOutput object'
+ 'Asserts that the run() returns false due to a missing
ParserOutput object'
);
}
/**
- * @test UpdateJob::run
* @dataProvider titleWikiPageDataProvider
*
* @since 1.9
@@ -116,20 +94,19 @@
$instance = $this->newInstance( $setup['title'] );
- $instance->getDependencyBuilder()
+ $instance->withContext()
+ ->getDependencyBuilder()
->getContainer()
->registerObject( 'ContentParser',
$setup['contentParser'] );
$this->assertEquals(
$expected['result'],
$instance->run(),
- 'asserts run() in terms of the available ContentParser
object'
+ 'Asserts run() in terms of the available ContentParser
object'
);
}
/**
- * Provides title and wikiPage samples
- *
* @return array
*/
public function titleWikiPageDataProvider() {
--
To view, visit https://gerrit.wikimedia.org/r/86849
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie5e74dfd56f5fecd7391285585622f80681db97a
Gerrit-PatchSet: 4
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
