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 0000000..118b897
--- /dev/null
+++ b/Tests/SmashPigDatabaseTest.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace SmashPig\Tests;
+
+use SmashPig\Core\Context;
+use SmashPig\Core\DataStores\PaymentsInitialDatabase;
+use SmashPig\Core\DataStores\PendingDatabase;
+
+class SmashPigDatabaseTest extends BaseSmashPigUnitTestCase {
+
+ /**
+ * @var PendingDatabase
+ */
+ protected $pendingDb;
+
+ /**
+ * @var PaymentsInitialDatabase
+ */
+ protected $paymentsInitialDb;
+
+ public function setUp() {
+ parent::setUp();
+ $config = SmashPigDatabaseTestConfiguration::instance();
+ Context::initWithLogger( $config );
+
+ $this->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
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic770319c8d1f9812e980e08fd833cdbee1bab4d6
Gerrit-PatchSet: 3
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
