Florianschmidtwelzow has uploaded a new change for review.
https://gerrit.wikimedia.org/r/264976
Change subject: Use clone to clone a MutableContext object in DerivativeContext
......................................................................
Use clone to clone a MutableContext object in DerivativeContext
Using local class member variables to hold instances of objects set by one of
the set*-functions works very well for the get*-functions, but doesn't work,
if a function uses the object behind this function in another context. The
example
is the getWikiPage() which uses the title object returned by the getTitle()
method
to construct the WikiPage object. If a user of DerivativeContext uses the object
to set another title as in the source context object (passed to the
DerivativeContext
constructor) without setting a WikiPage object, DerivativeContext::getWikiPage()
will return the result of getWikiPage() of the context object passed in the
constructor.
In this case, the Title object of this context will be used, instead of the
Title
object set in the DerivativeContext (like the user can expect).
To fix this, this change removes the handling of local object and instead
changes
the behaviour of the constructor a little bit. Instead of setting the original
context object as the context of the DerivativeContext, it will clone this
source
context object and set this instead. The set*-functions now simply pass the call
to this newly cloned context object. The get*-functions will do the same. Using
this approach, the DerivativeContext should be much more consistently while
handling
other objects as the source context object without touching the source context
object.
The added test for DerivativeContext checks the behaviour for
setTitle()/getWikiPage(),
and gives a good example of the origin problem.
Bug: T124060
Change-Id: I730255acb361bbdcca26cce2cc5c178db8f24e63
---
M includes/context/DerivativeContext.php
A tests/phpunit/includes/context/DerivativeContextTest.php
2 files changed, 57 insertions(+), 237 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/76/264976/1
diff --git a/includes/context/DerivativeContext.php
b/includes/context/DerivativeContext.php
index 1b881e4..74a7e9d 100644
--- a/includes/context/DerivativeContext.php
+++ b/includes/context/DerivativeContext.php
@@ -27,61 +27,18 @@
* @since 1.19
*/
class DerivativeContext extends ContextSource implements MutableContext {
- /**
- * @var WebRequest
- */
- private $request;
-
- /**
- * @var Title
- */
- private $title;
-
- /**
- * @var WikiPage
- */
- private $wikipage;
-
- /**
- * @var OutputPage
- */
- private $output;
-
- /**
- * @var User
- */
- private $user;
-
- /**
- * @var Language
- */
- private $lang;
-
- /**
- * @var Skin
- */
- private $skin;
-
- /**
- * @var Config
- */
- private $config;
-
- /**
- * @var Stats
- */
- private $stats;
-
- /**
- * @var Timing
- */
- private $timing;
/**
* Constructor
- * @param IContextSource $context Context to inherit from
+ * @param MutableContext $context MutableContext to inherit from
*/
- public function __construct( IContextSource $context ) {
+ public function __construct( MutableContext $context ) {
+ // Clone the original context object so we can change it
without changing the
+ // source context one.
+ $context = clone $context;
+
+ // set this cloned context object as the context of this
DerivativeContext instance,
+ // so any set*-call will be done on this one, like any get*-call
$this->setContext( $context );
}
@@ -91,46 +48,7 @@
* @param Config $s
*/
public function setConfig( Config $s ) {
- $this->config = $s;
- }
-
- /**
- * Get the Config object
- *
- * @return Config
- */
- public function getConfig() {
- if ( !is_null( $this->config ) ) {
- return $this->config;
- } else {
- return $this->getContext()->getConfig();
- }
- }
-
- /**
- * Get the stats object
- *
- * @return BufferingStatsdDataFactory
- */
- public function getStats() {
- if ( !is_null( $this->stats ) ) {
- return $this->stats;
- } else {
- return $this->getContext()->getStats();
- }
- }
-
- /**
- * Get the timing object
- *
- * @return Timing
- */
- public function getTiming() {
- if ( !is_null( $this->timing ) ) {
- return $this->timing;
- } else {
- return $this->getContext()->getTiming();
- }
+ $this->getContext()->setConfig( $s );
}
/**
@@ -139,20 +57,7 @@
* @param WebRequest $r
*/
public function setRequest( WebRequest $r ) {
- $this->request = $r;
- }
-
- /**
- * Get the WebRequest object
- *
- * @return WebRequest
- */
- public function getRequest() {
- if ( !is_null( $this->request ) ) {
- return $this->request;
- } else {
- return $this->getContext()->getRequest();
- }
+ $this->getContext()->setRequest( $r );
}
/**
@@ -161,38 +66,7 @@
* @param Title $t
*/
public function setTitle( Title $t ) {
- $this->title = $t;
- }
-
- /**
- * Get the Title object
- *
- * @return Title|null
- */
- public function getTitle() {
- if ( !is_null( $this->title ) ) {
- return $this->title;
- } else {
- return $this->getContext()->getTitle();
- }
- }
-
- /**
- * Check whether a WikiPage object can be get with getWikiPage().
- * Callers should expect that an exception is thrown from getWikiPage()
- * if this method returns false.
- *
- * @since 1.19
- * @return bool
- */
- public function canUseWikiPage() {
- if ( $this->wikipage !== null ) {
- return true;
- } elseif ( $this->title !== null ) {
- return $this->title->canExist();
- } else {
- return $this->getContext()->canUseWikiPage();
- }
+ $this->getContext()->setTitle( $t );
}
/**
@@ -202,24 +76,7 @@
* @param WikiPage $p
*/
public function setWikiPage( WikiPage $p ) {
- $this->wikipage = $p;
- }
-
- /**
- * Get the WikiPage object.
- * May throw an exception if there's no Title object set or the Title
object
- * belongs to a special namespace that doesn't have WikiPage, so use
first
- * canUseWikiPage() to check whether this method can be called safely.
- *
- * @since 1.19
- * @return WikiPage
- */
- public function getWikiPage() {
- if ( !is_null( $this->wikipage ) ) {
- return $this->wikipage;
- } else {
- return $this->getContext()->getWikiPage();
- }
+ $this->getContext()->setWikiPage( $p );
}
/**
@@ -228,20 +85,7 @@
* @param OutputPage $o
*/
public function setOutput( OutputPage $o ) {
- $this->output = $o;
- }
-
- /**
- * Get the OutputPage object
- *
- * @return OutputPage
- */
- public function getOutput() {
- if ( !is_null( $this->output ) ) {
- return $this->output;
- } else {
- return $this->getContext()->getOutput();
- }
+ $this->getContext()->setOutput( $o );
}
/**
@@ -250,20 +94,7 @@
* @param User $u
*/
public function setUser( User $u ) {
- $this->user = $u;
- }
-
- /**
- * Get the User object
- *
- * @return User
- */
- public function getUser() {
- if ( !is_null( $this->user ) ) {
- return $this->user;
- } else {
- return $this->getContext()->getUser();
- }
+ $this->getContext()->setUser( $u );
}
/**
@@ -274,29 +105,7 @@
* @since 1.19
*/
public function setLanguage( $l ) {
- if ( $l instanceof Language ) {
- $this->lang = $l;
- } elseif ( is_string( $l ) ) {
- $l = RequestContext::sanitizeLangCode( $l );
- $obj = Language::factory( $l );
- $this->lang = $obj;
- } else {
- throw new MWException( __METHOD__ . " was passed an
invalid type of data." );
- }
- }
-
- /**
- * Get the Language object
- *
- * @return Language
- * @since 1.19
- */
- public function getLanguage() {
- if ( !is_null( $this->lang ) ) {
- return $this->lang;
- } else {
- return $this->getContext()->getLanguage();
- }
+ $this->getContext()->setLanguage( $l );
}
/**
@@ -305,36 +114,8 @@
* @param Skin $s
*/
public function setSkin( Skin $s ) {
- $this->skin = clone $s;
- $this->skin->setContext( $this );
- }
-
- /**
- * Get the Skin object
- *
- * @return Skin
- */
- public function getSkin() {
- if ( !is_null( $this->skin ) ) {
- return $this->skin;
- } else {
- return $this->getContext()->getSkin();
- }
- }
-
- /**
- * Get a message using the current context.
- *
- * This can't just inherit from ContextSource, since then
- * it would set only the original context, and not take
- * into account any changes.
- *
- * @param mixed $args,... Arguments to wfMessage
- * @return Message
- */
- public function msg() {
- $args = func_get_args();
-
- return call_user_func_array( 'wfMessage', $args )->setContext(
$this );
+ $skin = clone $s;
+ $skin->setContext( $this );
+ $this->getContext()->setSkin( $skin );
}
}
diff --git a/tests/phpunit/includes/context/DerivativeContextTest.php
b/tests/phpunit/includes/context/DerivativeContextTest.php
new file mode 100644
index 0000000..02a2470
--- /dev/null
+++ b/tests/phpunit/includes/context/DerivativeContextTest.php
@@ -0,0 +1,39 @@
+<?php
+
+class DerivativeContextTest extends MediaWikiTestCase {
+
+ public function testContextTitle() {
+ // the title used for the RequestContext object
+ $title = Title::newFromText( 'TestTitle' );
+ // the Title used for the DerivativeContext object
+ $newTitle = Title::newFromText( 'TestTitle2' );
+
+ $context = new RequestContext();
+ $context->setTitle( $title );
+
+ $derivativeContext = new DerivativeContext( $context );
+ // check, if both WikiPage object are constructed with the same
Title object
+ $this->assertEquals(
+
$derivativeContext->getWikiPage()->getTitle()->getText(),
+ $context->getWikiPage()->getTitle()->getText(),
+ 'The WikiPage returned by
DerivativeContext::getWikiPage() is the same' .
+ 'as the one returned by RequestContext::getWikiPage().'
+ );
+
+ // set the own Title to the DerivativeContext
+ $derivativeContext->setTitle( $newTitle );
+
+ $this->assertEquals(
+
$derivativeContext->getWikiPage()->getTitle()->getText(),
+ 'TestTitle2',
+ 'The WikiPage returned by
DerivativeContext::getWikiPage() is constructed using' .
+ 'the title set by DerivativeContext::setTitle().'
+ );
+ $this->assertNotEquals(
+
$derivativeContext->getWikiPage()->getTitle()->getText(),
+ $context->getWikiPage()->getTitle()->getText(),
+ 'The WikiPage returned by
DerivativeContext::getWikiPage() isn\'t the same' .
+ 'as the one returned by RequestContext::getWikiPage().'
+ );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/264976
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I730255acb361bbdcca26cce2cc5c178db8f24e63
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Florianschmidtwelzow <florian.schmidt.stargatewis...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
