[MediaWiki-commits] [Gerrit] wikimedia...SmashPig[master]: Fix all db subclasses sharing a PDO
jenkins-bot has submitted this change and it was merged.
Change subject: Fix all db subclasses sharing a PDO
..
Fix all db subclasses sharing a PDO
WTF PHP, even static::$thing refers to the same object always?
Even this gets a single shared instance:
$klass = get_called_class();
$db = $klass::$db
We need this to re-apply change I61ef044899ee on deployment, where
it's currently reverted.
Change-Id: Ic770319c8d1f9812e980e08fd833cdbee1bab4d6
---
M Core/DataStores/SmashPigDatabase.php
M Tests/DamagedDatabaseTest.php
A Tests/SmashPigDatabaseTest.php
M Tests/TestingDatabase.php
4 files changed, 73 insertions(+), 14 deletions(-)
Approvals:
Cdentinger: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Core/DataStores/SmashPigDatabase.php
b/Core/DataStores/SmashPigDatabase.php
index 48f78b4..ad8c9b7 100644
--- a/Core/DataStores/SmashPigDatabase.php
+++ b/Core/DataStores/SmashPigDatabase.php
@@ -7,16 +7,23 @@
abstract class SmashPigDatabase {
/**
-* @var PDO
+* @var array
+* key is concrete class name, value is a backing PDO object
* We do the silly singleton thing for convenient testing with in-memory
* databases that would otherwise not be shared between components.
+*
+* Ideally, this would be a scalar variable holding a single PDO object
+* for each concrete subclass. Unfortunately, in PHP the static member
+* variable is shared between all subclasses of the class that declares
+* it, even in an abstract class like this one. See
+*
http://stackoverflow.com/questions/11417681/static-properties-on-base-class-and-inheritance#11418607
*/
- protected static $db;
+ protected static $dbs = array();
protected function __construct() {
$config = Context::get()->getConfiguration();
- if ( !static::$db ) {
- static::$db = $config->object( $this->getConfigKey() );
+ if ( !$this->getDatabase() ) {
+ $this->setDatabase( $config->object(
$this->getConfigKey() ) );
}
}
@@ -25,10 +32,19 @@
}
/**
-* @return PDO
+* @return PDO|null
*/
public function getDatabase() {
- return static::$db;
+ $className = get_called_class();
+ if ( isset ( self::$dbs[$className] ) ) {
+ return self::$dbs[$className];
+ }
+ return null;
+ }
+
+ protected function setDatabase( PDO $db ) {
+ $className = get_called_class();
+ self::$dbs[$className] = $db;
}
public function createTable() {
diff --git a/Tests/DamagedDatabaseTest.php b/Tests/DamagedDatabaseTest.php
index db03227..705ddad 100644
--- a/Tests/DamagedDatabaseTest.php
+++ b/Tests/DamagedDatabaseTest.php
@@ -22,12 +22,7 @@
}
public function tearDown() {
- // Reset PDO static member
- $klass = new \ReflectionClass(
'SmashPig\Core\DataStores\DamagedDatabase' );
- $dbProperty = $klass->getProperty( 'db' );
- $dbProperty->setAccessible( true );
- $dbProperty->setValue( null );
-
+ TestingDatabase::clearStatics( $this->db );
parent::tearDown();
}
diff --git a/Tests/SmashPigDatabaseTest.php b/Tests/SmashPigDatabaseTest.php
new file mode 100644
index 000..118b897
--- /dev/null
+++ b/Tests/SmashPigDatabaseTest.php
@@ -0,0 +1,48 @@
+pendingDb = PendingDatabase::get();
+ $this->pendingDb->createTable();
+ $this->paymentsInitialDb = PaymentsInitialDatabase::get();
+ $this->paymentsInitialDb->createTable();
+ }
+
+ public function tearDown() {
+ TestingDatabase::clearStatics( $this->paymentsInitialDb );
+ TestingDatabase::clearStatics( $this->pendingDb );
+
+ parent::tearDown();
+ }
+
+ public function testDifferentDatabases() {
+ $pendingPdo = $this->pendingDb->getDatabase();
+ $initPdo = $this->paymentsInitialDb->getDatabase();
+ $this->assertNotEquals(
+ spl_object_hash( $pendingPdo ),
+ spl_object_hash( $initPdo ),
+ 'Pending and paymentsInit databases share the same PDO'
+ );
+ }
+}
diff --git a/Tests/TestingDatabase.php b/Tests/TestingDatabase.php
index 6173a22..48e0e87 100644
--- a/Tests/TestingDatabase.php
+++ b/Tests/TestingDatabase.php
@@ -10,8 +10,8 @@
*/
public static function clearStatics( $classish ) {
$klass = new \ReflectionClass( $classish );
- $dbProperty = $klass->getProperty( 'db' );
+ $dbProperty = $klass->getProperty( 'dbs'
[MediaWiki-commits] [Gerrit] wikimedia...SmashPig[master]: Fix all db subclasses sharing a PDO
Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/317277
Change subject: Fix all db subclasses sharing a PDO
..
Fix all db subclasses sharing a PDO
WTF PHP, even static::$thing refers to the same object always?
Even this gets a single shared instance:
$klass = get_called_class();
$db = $klass::$db
We need this to re-apply change I61ef044899ee on deployment, where
it's currently reverted.
Change-Id: Ic770319c8d1f9812e980e08fd833cdbee1bab4d6
---
M Core/DataStores/SmashPigDatabase.php
M Tests/DamagedDatabaseTest.php
A Tests/SmashPigDatabaseTest.php
M Tests/TestingDatabase.php
4 files changed, 68 insertions(+), 14 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/SmashPig
refs/changes/77/317277/1
diff --git a/Core/DataStores/SmashPigDatabase.php
b/Core/DataStores/SmashPigDatabase.php
index 48f78b4..123d66f 100644
--- a/Core/DataStores/SmashPigDatabase.php
+++ b/Core/DataStores/SmashPigDatabase.php
@@ -7,16 +7,18 @@
abstract class SmashPigDatabase {
/**
-* @var PDO
+* @var array
+* key is concrete class name, value is a backing PDO object
* We do the silly singleton thing for convenient testing with in-memory
* databases that would otherwise not be shared between components.
+* This thing is shared between all subclasses because PHP :(
*/
- protected static $db;
+ protected static $dbs = array();
protected function __construct() {
$config = Context::get()->getConfiguration();
- if ( !static::$db ) {
- static::$db = $config->object( $this->getConfigKey() );
+ if ( !$this->getDatabase() ) {
+ $this->setDatabase( $config->object(
$this->getConfigKey() ) );
}
}
@@ -25,10 +27,19 @@
}
/**
-* @return PDO
+* @return PDO|null
*/
public function getDatabase() {
- return static::$db;
+ $className = get_called_class();
+ if ( isset ( self::$dbs[$className] ) ) {
+ return self::$dbs[$className];
+ }
+ return null;
+ }
+
+ protected function setDatabase( PDO $db ) {
+ $className = get_called_class();
+ self::$dbs[$className] = $db;
}
public function createTable() {
diff --git a/Tests/DamagedDatabaseTest.php b/Tests/DamagedDatabaseTest.php
index db03227..705ddad 100644
--- a/Tests/DamagedDatabaseTest.php
+++ b/Tests/DamagedDatabaseTest.php
@@ -22,12 +22,7 @@
}
public function tearDown() {
- // Reset PDO static member
- $klass = new \ReflectionClass(
'SmashPig\Core\DataStores\DamagedDatabase' );
- $dbProperty = $klass->getProperty( 'db' );
- $dbProperty->setAccessible( true );
- $dbProperty->setValue( null );
-
+ TestingDatabase::clearStatics( $this->db );
parent::tearDown();
}
diff --git a/Tests/SmashPigDatabaseTest.php b/Tests/SmashPigDatabaseTest.php
new file mode 100644
index 000..118b897
--- /dev/null
+++ b/Tests/SmashPigDatabaseTest.php
@@ -0,0 +1,48 @@
+pendingDb = PendingDatabase::get();
+ $this->pendingDb->createTable();
+ $this->paymentsInitialDb = PaymentsInitialDatabase::get();
+ $this->paymentsInitialDb->createTable();
+ }
+
+ public function tearDown() {
+ TestingDatabase::clearStatics( $this->paymentsInitialDb );
+ TestingDatabase::clearStatics( $this->pendingDb );
+
+ parent::tearDown();
+ }
+
+ public function testDifferentDatabases() {
+ $pendingPdo = $this->pendingDb->getDatabase();
+ $initPdo = $this->paymentsInitialDb->getDatabase();
+ $this->assertNotEquals(
+ spl_object_hash( $pendingPdo ),
+ spl_object_hash( $initPdo ),
+ 'Pending and paymentsInit databases share the same PDO'
+ );
+ }
+}
diff --git a/Tests/TestingDatabase.php b/Tests/TestingDatabase.php
index 6173a22..48e0e87 100644
--- a/Tests/TestingDatabase.php
+++ b/Tests/TestingDatabase.php
@@ -10,8 +10,8 @@
*/
public static function clearStatics( $classish ) {
$klass = new \ReflectionClass( $classish );
- $dbProperty = $klass->getProperty( 'db' );
+ $dbProperty = $klass->getProperty( 'dbs' );
$dbProperty->setAccessible( true );
- $dbProperty->setValue( null );
+ $dbProperty->setValue( array() );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/317277
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
