jenkins-bot has submitted this change and it was merged.
Change subject: Session: Implement ArrayAccess
......................................................................
Session: Implement ArrayAccess
Now that we dropped support for PHP 5.3.3, we can do this.
The behavior of $session['foo'] when that key doesn't already exist is a
little unexpected (it implicitly assigns null), but it's the best we can
do.
Change-Id: Ibef878867d46591a8bf542139a1719dfec3b83ab
(cherry picked from commit 1aedd2df73ad2be69fb24148bfaff31c118fddba)
---
M includes/session/Session.php
M includes/session/SessionBackend.php
M tests/phpunit/includes/session/SessionBackendTest.php
M tests/phpunit/includes/session/SessionTest.php
M tests/phpunit/includes/session/TestUtils.php
5 files changed, 120 insertions(+), 8 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/session/Session.php b/includes/session/Session.php
index 4ad69ae..d654ff1 100644
--- a/includes/session/Session.php
+++ b/includes/session/Session.php
@@ -23,6 +23,7 @@
namespace MediaWiki\Session;
+use Psr\Log\LoggerInterface;
use User;
use WebRequest;
@@ -41,24 +42,28 @@
* The Session object also serves as a replacement for PHP's $_SESSION,
* managing access to per-session data.
*
- * @todo Once we drop support for PHP 5.3.3, implementing ArrayAccess would be
nice.
* @ingroup Session
* @since 1.27
*/
-final class Session implements \Countable, \Iterator {
+final class Session implements \Countable, \Iterator, \ArrayAccess {
/** @var SessionBackend Session backend */
private $backend;
/** @var int Session index */
private $index;
+ /** @var LoggerInterface */
+ private $logger;
+
/**
* @param SessionBackend $backend
* @param int $index
+ * @param LoggerInterface $logger
*/
- public function __construct( SessionBackend $backend, $index ) {
+ public function __construct( SessionBackend $backend, $index,
LoggerInterface $logger ) {
$this->backend = $backend;
$this->index = $index;
+ $this->logger = $logger;
}
public function __destruct() {
@@ -271,7 +276,7 @@
/**
* Fetch a value from the session
* @param string|int $key
- * @param mixed $default
+ * @param mixed $default Returned if $this->exists( $key ) would be
false
* @return mixed
*/
public function get( $key, $default = null ) {
@@ -281,6 +286,7 @@
/**
* Test if a value exists in the session
+ * @note Unlike isset(), null values are considered to exist.
* @param string|int $key
* @return bool
*/
@@ -419,6 +425,39 @@
return key( $data ) !== null;
}
+ /**
+ * @note Despite the name, this seems to be intended to implement
isset()
+ * rather than array_key_exists(). So do that.
+ */
+ public function offsetExists( $offset ) {
+ $data = &$this->backend->getData();
+ return isset( $data[$offset] );
+ }
+
+ /**
+ * @note This supports indirect modifications but can't mark the session
+ * dirty when those happen. SessionBackend::save() checks the hash of
the
+ * data to detect such changes.
+ * @note Accessing a nonexistent key via this mechanism causes that key
to
+ * be created with a null value, and does not raise a PHP warning.
+ */
+ public function &offsetGet( $offset ) {
+ $data = &$this->backend->getData();
+ if ( !array_key_exists( $offset, $data ) ) {
+ $ex = new \Exception( "Undefined index (auto-adds to
session with a null value): $offset" );
+ $this->logger->debug( $ex->getMessage(), array(
'exception' => $ex ) );
+ }
+ return $data[$offset];
+ }
+
+ public function offsetSet( $offset, $value ) {
+ $this->set( $offset, $value );
+ }
+
+ public function offsetUnset( $offset ) {
+ $this->remove( $offset );
+ }
+
/**@}*/
}
diff --git a/includes/session/SessionBackend.php
b/includes/session/SessionBackend.php
index fe446e3..5cf7869 100644
--- a/includes/session/SessionBackend.php
+++ b/includes/session/SessionBackend.php
@@ -170,7 +170,7 @@
public function getSession( WebRequest $request ) {
$index = ++$this->curIndex;
$this->requests[$index] = $request;
- $session = new Session( $this, $index );
+ $session = new Session( $this, $index, $this->logger );
return $session;
}
diff --git a/tests/phpunit/includes/session/SessionBackendTest.php
b/tests/phpunit/includes/session/SessionBackendTest.php
index f08a07d..481b693 100644
--- a/tests/phpunit/includes/session/SessionBackendTest.php
+++ b/tests/phpunit/includes/session/SessionBackendTest.php
@@ -569,6 +569,7 @@
'making sure it did save to backend' );
// Not marked dirty, but dirty data
+ // (e.g. indirect modification from ArrayAccess::offsetGet)
$this->provider = $neverProvider;
$this->onSessionMetadataCalled = false;
$this->mergeMwGlobalArrayValue( 'wgHooks', array(
'SessionMetadata' => array( $this ) ) );
diff --git a/tests/phpunit/includes/session/SessionTest.php
b/tests/phpunit/includes/session/SessionTest.php
index 858996d..3c5090a 100644
--- a/tests/phpunit/includes/session/SessionTest.php
+++ b/tests/phpunit/includes/session/SessionTest.php
@@ -2,6 +2,7 @@
namespace MediaWiki\Session;
+use Psr\Log\LogLevel;
use MediaWikiTestCase;
use User;
@@ -16,7 +17,7 @@
\TestingAccessWrapper::newFromObject( $backend )->requests =
array( -1 => 'dummy' );
\TestingAccessWrapper::newFromObject( $backend )->id = new
SessionId( 'abc' );
- $session = new Session( $backend, 42 );
+ $session = new Session( $backend, 42, new \TestLogger );
$priv = \TestingAccessWrapper::newFromObject( $session );
$this->assertSame( $backend, $priv->backend );
$this->assertSame( 42, $priv->index );
@@ -152,6 +153,71 @@
$this->assertFalse( $backend->dirty );
}
+ public function testArrayAccess() {
+ $logger = new \TestLogger;
+ $session = TestUtils::getDummySession( null, -1, $logger );
+ $backend = \TestingAccessWrapper::newFromObject( $session
)->backend;
+
+ $this->assertEquals( 1, $session['foo'] );
+ $this->assertEquals( 'zero', $session[0] );
+ $this->assertFalse( $backend->dirty );
+
+ $logger->setCollect( true );
+ $this->assertEquals( null, $session['null'] );
+ $logger->setCollect( false );
+ $this->assertFalse( $backend->dirty );
+ $this->assertSame( array(
+ array( LogLevel::DEBUG, 'Undefined index (auto-adds to
session with a null value): null' )
+ ), $logger->getBuffer() );
+ $logger->clearBuffer();
+
+ $session['foo'] = 55;
+ $this->assertEquals( 55, $backend->data['foo'] );
+ $this->assertTrue( $backend->dirty );
+ $backend->dirty = false;
+
+ $session[1] = 'one';
+ $this->assertEquals( 'one', $backend->data[1] );
+ $this->assertTrue( $backend->dirty );
+ $backend->dirty = false;
+
+ $session[1] = 'one';
+ $this->assertFalse( $backend->dirty );
+
+ $session['bar'] = array( 'baz' => array() );
+ $session['bar']['baz']['quux'] = 2;
+ $this->assertEquals( array( 'baz' => array( 'quux' => 2 ) ),
$backend->data['bar'] );
+
+ $logger->setCollect( true );
+ $session['bar2']['baz']['quux'] = 3;
+ $logger->setCollect( false );
+ $this->assertEquals( array( 'baz' => array( 'quux' => 3 ) ),
$backend->data['bar2'] );
+ $this->assertSame( array(
+ array( LogLevel::DEBUG, 'Undefined index (auto-adds to
session with a null value): bar2' )
+ ), $logger->getBuffer() );
+ $logger->clearBuffer();
+
+ $backend->dirty = false;
+ $this->assertTrue( isset( $session['foo'] ) );
+ $this->assertTrue( isset( $session[1] ) );
+ $this->assertFalse( isset( $session['null'] ) );
+ $this->assertFalse( isset( $session['missing'] ) );
+ $this->assertFalse( isset( $session[100] ) );
+ $this->assertFalse( $backend->dirty );
+
+ unset( $session['foo'] );
+ $this->assertArrayNotHasKey( 'foo', $backend->data );
+ $this->assertTrue( $backend->dirty );
+ $backend->dirty = false;
+ unset( $session[1] );
+ $this->assertArrayNotHasKey( 1, $backend->data );
+ $this->assertTrue( $backend->dirty );
+ $backend->dirty = false;
+
+ unset( $session[101] );
+ $this->assertFalse( $backend->dirty );
+ }
+
public function testClear() {
$session = TestUtils::getDummySession();
$priv = \TestingAccessWrapper::newFromObject( $session );
diff --git a/tests/phpunit/includes/session/TestUtils.php
b/tests/phpunit/includes/session/TestUtils.php
index 1619983..cc20ab5 100644
--- a/tests/phpunit/includes/session/TestUtils.php
+++ b/tests/phpunit/includes/session/TestUtils.php
@@ -2,6 +2,8 @@
namespace MediaWiki\Session;
+use Psr\Log\LoggerInterface;
+
/**
* Utility functions for Session unit tests
*/
@@ -67,7 +69,9 @@
);
}
- return $rc->newInstanceWithoutConstructor();
+ $ret = $rc->newInstanceWithoutConstructor();
+ \TestingAccessWrapper::newFromObject( $ret )->logger = new
\TestLogger;
+ return $ret;
}
/**
@@ -75,9 +79,10 @@
* construct one, use this.
* @param object $backend Object to serve as the SessionBackend
* @param int $index Index
+ * @param LoggerInterface $logger
* @return Session
*/
- public static function getDummySession( $backend = null, $index = -1 ) {
+ public static function getDummySession( $backend = null, $index = -1,
$logger = null ) {
$rc = new \ReflectionClass( 'MediaWiki\\Session\\Session' );
if ( !method_exists( $rc, 'newInstanceWithoutConstructor' ) ) {
\PHPUnit_Framework_Assert::markTestSkipped(
@@ -93,6 +98,7 @@
$priv = \TestingAccessWrapper::newFromObject( $session );
$priv->backend = $backend;
$priv->index = $index;
+ $priv->logger = $logger ?: new \TestLogger;
return $session;
}
--
To view, visit https://gerrit.wikimedia.org/r/272827
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibef878867d46591a8bf542139a1719dfec3b83ab
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.27.0-wmf.14
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
