Mwjames has uploaded a new change for review.
https://gerrit.wikimedia.org/r/86849
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 the 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, 81 insertions(+), 117 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/SemanticMediaWiki
refs/changes/49/86849/1
diff --git a/includes/UpdateObserver.php b/includes/UpdateObserver.php
index 1828c2b..c75c510 100644
--- a/includes/UpdateObserver.php
+++ b/includes/UpdateObserver.php
@@ -21,37 +21,35 @@
* 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
*/
-class UpdateObserver extends Observer implements DependencyRequestor {
+class UpdateObserver extends Observer implements ContextAware {
- /** @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 +57,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 +78,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 +112,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 8de063b..4003e21 100644
--- a/includes/dic/SharedDependencyContainer.php
+++ b/includes/dic/SharedDependencyContainer.php
@@ -284,7 +284,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..3a05bd8 100644
--- a/includes/jobs/JobBase.php
+++ b/includes/jobs/JobBase.php
@@ -11,40 +11,37 @@
* @license GNU GPL v2+
* @author mwjames
*/
-abstract class JobBase extends Job implements DependencyRequestor {
+abstract class JobBase extends Job implements ContextAware {
- /** @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 269b6a5..ac8416c 100644
--- a/includes/jobs/UpdateDispatcherJob.php
+++ b/includes/jobs/UpdateDispatcherJob.php
@@ -99,7 +99,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 +171,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..f702d03 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
) );
@@ -133,13 +128,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/tests/phpunit/includes/UpdateObserverTest.php
b/tests/phpunit/includes/UpdateObserverTest.php
index f18a6e1..aa3f660 100644
--- a/tests/phpunit/includes/UpdateObserverTest.php
+++ b/tests/phpunit/includes/UpdateObserverTest.php
@@ -3,6 +3,7 @@
namespace SMW\Test;
use SMW\UpdateObserver;
+use SMW\BaseContext;
/**
* Tests for the UpdateObserver class
@@ -43,23 +44,23 @@
*
* @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;
}
/**
@@ -79,15 +80,11 @@
*/
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'
);
}
@@ -99,15 +96,11 @@
*/
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..5a2800b 100644
--- a/tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
+++ b/tests/phpunit/includes/jobs/UpdateDispatcherJobTest.php
@@ -2,8 +2,8 @@
namespace SMW\Test;
-use SMW\SharedDependencyContainer;
use SMW\UpdateDispatcherJob;
+use SMW\BaseContext;
use SMW\DIProperty;
use Title;
@@ -69,12 +69,14 @@
'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;
}
diff --git a/tests/phpunit/includes/jobs/UpdateJobTest.php
b/tests/phpunit/includes/jobs/UpdateJobTest.php
index ef907d8..a31414b 100644
--- a/tests/phpunit/includes/jobs/UpdateJobTest.php
+++ b/tests/phpunit/includes/jobs/UpdateJobTest.php
@@ -2,7 +2,7 @@
namespace SMW\Test;
-use SMW\SharedDependencyContainer;
+use SMW\BaseContext;
use SMW\UpdateJob;
use Title;
@@ -45,34 +45,29 @@
*
* @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;
-
}
/**
@@ -116,7 +111,8 @@
$instance = $this->newInstance( $setup['title'] );
- $instance->getDependencyBuilder()
+ $instance->withContext()
+ ->getDependencyBuilder()
->getContainer()
->registerObject( 'ContentParser',
$setup['contentParser'] );
--
To view, visit https://gerrit.wikimedia.org/r/86849
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie5e74dfd56f5fecd7391285585622f80681db97a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <jamesin.hongkon...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
