From 6da72d686f02f98dd8857ff236a85ee35174770d Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 19 Jun 2018 16:16:50 +1200 Subject: [PATCH] Maybe fix it? --- src/Dev/State/FixtureTestState.php | 26 +++++- src/ORM/Connect/Database.php | 14 ++++ src/ORM/Connect/MySQLDatabase.php | 9 ++- src/ORM/Connect/TempDatabase.php | 126 ++++++++++++++--------------- tests/php/Security/GroupTest.php | 5 ++ 5 files changed, 113 insertions(+), 67 deletions(-) diff --git a/src/Dev/State/FixtureTestState.php b/src/Dev/State/FixtureTestState.php index a4df4d513..ec5a32d32 100644 --- a/src/Dev/State/FixtureTestState.php +++ b/src/Dev/State/FixtureTestState.php @@ -19,6 +19,13 @@ class FixtureTestState implements TestState */ private $fixtureFactories = []; + /** + * Set if fixtures have been loaded + * + * @var bool + */ + protected $loaded = []; + /** * Called on setup * @@ -29,6 +36,8 @@ class FixtureTestState implements TestState if (!$this->testNeedsDB($test)) { return; } + + // Ensure DB is built $tmpDB = $test::tempDB(); if (!$tmpDB->isUsed()) { // Build base db @@ -40,10 +49,11 @@ class FixtureTestState implements TestState $tmpDB->resetDBSchema($extraObjects); } } + DataObject::singleton()->flushCache(); // Ensure DB is built and populated - if (!$tmpDB->hasStarted()) { + if (!$this->getIsLoaded(get_class($test))) { foreach ($test->getRequireDefaultRecordsFrom() as $className) { $instance = singleton($className); if (method_exists($instance, 'requireDefaultRecords')) { @@ -155,6 +165,8 @@ class FixtureTestState implements TestState foreach ($paths as $fixtureFile) { $this->loadFixture($fixtureFile, $test); } + // Flag as loaded + $this->loaded[get_class($test)] = true; } /** @@ -254,5 +266,17 @@ class FixtureTestState implements TestState protected function resetFixtureFactory($class) { $this->fixtureFactories[strtolower($class)] = Injector::inst()->create(FixtureFactory::class); + $this->loaded[$class] = false; + } + + /** + * Check if fixtures need to be loaded for this class + * + * @param string $class Name of test to check + * @return bool + */ + protected function getIsLoaded($class) + { + return !empty($this->loaded[$class]); } } diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php index add4d6948..4c60ed085 100644 --- a/src/ORM/Connect/Database.php +++ b/src/ORM/Connect/Database.php @@ -668,6 +668,20 @@ abstract class Database */ abstract public function transactionEnd($chain = false); + /** + * Return depth of current transaction + * + * @return int Nesting level, or 0 if not in a transaction + */ + public function transactionDepth() + { + // Placeholder error for transactional DBs that don't expose depth + if ($this->supportsTransactions()) { + user_error(get_class($this) . " does not support transactionDepth", E_USER_WARNING); + } + return 0; + } + /** * Determines if the used database supports application-level locks, * which is different from table- or row-level locking. diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php index 59c0c2b6c..49f6dd62d 100644 --- a/src/ORM/Connect/MySQLDatabase.php +++ b/src/ORM/Connect/MySQLDatabase.php @@ -348,6 +348,11 @@ class MySQLDatabase extends Database return true; } + public function transactionDepth() + { + return $this->transactionNesting; + } + public function transactionEnd($chain = false) { // Fail if transaction isn't available @@ -365,7 +370,7 @@ class MySQLDatabase extends Database /** * In error condition, set transactionNesting to zero */ - protected function discardTransactions() + protected function resetTransactionNesting() { $this->transactionNesting = 0; } @@ -394,7 +399,7 @@ class MySQLDatabase extends Database // on why we need to be over-eager $isDDL = $this->getConnector()->isQueryDDL($sql); if ($isDDL) { - $this->discardTransactions(); + $this->resetTransactionNesting(); } } diff --git a/src/ORM/Connect/TempDatabase.php b/src/ORM/Connect/TempDatabase.php index ce9065bbb..dce37959f 100644 --- a/src/ORM/Connect/TempDatabase.php +++ b/src/ORM/Connect/TempDatabase.php @@ -23,11 +23,6 @@ class TempDatabase */ protected $name = null; - /** - * @var bool If a transaction has been started - */ - protected $hasStarted = false; - /** * Create a new temp database * @@ -73,14 +68,6 @@ class TempDatabase return $this->isDBTemp($selected); } - /** - * @return bool - */ - public function hasStarted() - { - return $this->hasStarted; - } - /** * @return bool */ @@ -94,7 +81,6 @@ class TempDatabase */ public function startTransaction() { - $this->hasStarted = true; if (static::getConn()->supportsTransactions()) { static::getConn()->transactionStart(); } @@ -110,18 +96,21 @@ class TempDatabase */ public function rollbackTransaction() { - $success = $this->hasStarted() && static::getConn()->supportsTransactions(); - if ($success) { - try { - // Explicit false = gnostic error from transactionRollback - if (static::getConn()->transactionRollback() === false) { - $success = false; - } - } catch (DatabaseException $ex) { - $success = false; - } + // Ensure a rollback can be performed + $success = static::getConn()->supportsTransactions() + && static::getConn()->transactionDepth(); + if (!$success) { + return false; + } + try { + // Explicit false = gnostic error from transactionRollback + if (static::getConn()->transactionRollback() === false) { + return false; + } + return true; + } catch (DatabaseException $ex) { + return false; } - return $success; } /** @@ -129,13 +118,14 @@ class TempDatabase */ public function kill() { - $this->hasStarted = false; - - // Delete our temporary database + // Nothing to kill if (!$this->isUsed()) { return; } + // Rollback any transactions (note: Success ignored) + $this->rollbackTransaction(); + // Check the database actually exists $dbConn = $this->getConn(); $dbName = $dbConn->getSelectedDatabase(); @@ -160,7 +150,6 @@ class TempDatabase */ public function clearAllData() { - $this->hasStarted = false; if (!$this->isUsed()) { return; } @@ -218,44 +207,12 @@ class TempDatabase } /** - * Clear all temp DBs on this connection + * Rebuild all database tables * - * Note: This will output results to stdout unless suppressOutput - * is set on the current db schema + * @param array $extraDataObjects */ - public function deleteAll() + protected function rebuildTables($extraDataObjects = []) { - $schema = $this->getConn()->getSchemaManager(); - foreach ($schema->databaseList() as $dbName) { - if ($this->isDBTemp($dbName)) { - $schema->dropDatabase($dbName); - $schema->alterationMessage("Dropped database \"$dbName\"", 'deleted'); - flush(); - } - } - } - - /** - * Reset the testing database's schema. - * - * @param array $extraDataObjects List of extra dataobjects to build - */ - public function resetDBSchema(array $extraDataObjects = []) - { - // pgsql doesn't allow schema updates inside transactions - // so we need to rollback any transactions before commencing a schema reset - if ($this->hasStarted()) { - // Sometimes transactions fail, rebuild - $success = $this->rollbackTransaction(); - if (!$success) { - $this->kill(); - $this->build(); - } - } - if (!$this->isUsed()) { - return; - } - DataObject::reset(); // clear singletons, they're caching old extension info which is used in DatabaseAdmin->doBuild() @@ -291,4 +248,45 @@ class TempDatabase ClassInfo::reset_db_cache(); DataObject::singleton()->flushCache(); } + + /** + * Clear all temp DBs on this connection + * + * Note: This will output results to stdout unless suppressOutput + * is set on the current db schema + */ + public function deleteAll() + { + $schema = $this->getConn()->getSchemaManager(); + foreach ($schema->databaseList() as $dbName) { + if ($this->isDBTemp($dbName)) { + $schema->dropDatabase($dbName); + $schema->alterationMessage("Dropped database \"$dbName\"", 'deleted'); + flush(); + } + } + } + + /** + * Reset the testing database's schema. + * + * @param array $extraDataObjects List of extra dataobjects to build + */ + public function resetDBSchema(array $extraDataObjects = []) + { + // Skip if no DB + if (!$this->isUsed()) { + return; + } + + try { + $this->rebuildTables($extraDataObjects); + } catch (DatabaseException $ex) { + // In case of error during build force a hard reset + // e.g. pgsql doesn't allow schema updates inside transactions + $this->kill(); + $this->build(); + $this->rebuildTables($extraDataObjects); + } + } } diff --git a/tests/php/Security/GroupTest.php b/tests/php/Security/GroupTest.php index 163257602..076e04584 100644 --- a/tests/php/Security/GroupTest.php +++ b/tests/php/Security/GroupTest.php @@ -20,6 +20,11 @@ class GroupTest extends FunctionalTest TestMember::class ]; + protected function setUp() + { + parent::setUp(); + } + public function testGroupCodeDefaultsToTitle() { $g1 = new Group();