diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ed7a44915d..dfd985b953 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -453,6 +453,7 @@ phutil_register_library_map(array( 'JavelinViewExampleServerView' => 'applications/uiexample/examples/client', 'LiskDAO' => 'storage/lisk/dao', 'LiskEphemeralObjectException' => 'storage/lisk/dao', + 'LiskFixtureTestCase' => 'storage/lisk/dao/__tests__', 'LiskIsolationTestCase' => 'storage/lisk/dao/__tests__', 'LiskIsolationTestDAO' => 'storage/lisk/dao/__tests__', 'LiskIsolationTestDAOException' => 'storage/lisk/dao/__tests__', @@ -896,6 +897,7 @@ phutil_register_library_map(array( 'PhabricatorSortTableExample' => 'applications/uiexample/examples/sorttable', 'PhabricatorStandardPageView' => 'view/page/standard', 'PhabricatorStatusController' => 'applications/status/base', + 'PhabricatorStorageFixtureScopeGuard' => 'infrastructure/testing/fixture/storage', 'PhabricatorStorageManagementAPI' => 'infrastructure/setup/storage/management', 'PhabricatorStorageManagementDatabasesWorkflow' => 'infrastructure/setup/storage/workflow/databases', 'PhabricatorStorageManagementDestroyWorkflow' => 'infrastructure/setup/storage/workflow/destroy', @@ -1395,6 +1397,7 @@ phutil_register_library_map(array( 'JavelinReactorExample' => 'PhabricatorUIExample', 'JavelinViewExample' => 'PhabricatorUIExample', 'JavelinViewExampleServerView' => 'AphrontView', + 'LiskFixtureTestCase' => 'PhabricatorTestCase', 'LiskIsolationTestCase' => 'PhabricatorTestCase', 'LiskIsolationTestDAO' => 'LiskDAO', 'ManiphestAction' => 'PhrictionConstants', diff --git a/src/applications/base/storage/lisk/PhabricatorLiskDAO.php b/src/applications/base/storage/lisk/PhabricatorLiskDAO.php index e77b5a9861..3a1cdf4803 100644 --- a/src/applications/base/storage/lisk/PhabricatorLiskDAO.php +++ b/src/applications/base/storage/lisk/PhabricatorLiskDAO.php @@ -72,7 +72,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO { /** * @task config */ - public static function popStorageNamespace($namespace) { + public static function popStorageNamespace() { array_pop(self::$namespaceStack); } diff --git a/src/infrastructure/testing/fixture/storage/PhabricatorStorageFixtureScopeGuard.php b/src/infrastructure/testing/fixture/storage/PhabricatorStorageFixtureScopeGuard.php new file mode 100644 index 0000000000..f1cda5344b --- /dev/null +++ b/src/infrastructure/testing/fixture/storage/PhabricatorStorageFixtureScopeGuard.php @@ -0,0 +1,51 @@ +name = $name; + + execx( + '%s upgrade --force --namespace %s', + $this->getStorageBinPath(), + $this->name); + + PhabricatorLiskDAO::pushStorageNamespace($name); + } + + public function __destruct() { + PhabricatorLiskDAO::popStorageNamespace(); + + execx( + '%s destroy --force --namespace %s', + $this->getStorageBinPath(), + $this->name); + } + + private function getStorageBinPath() { + $root = dirname(phutil_get_library_root('phabricator')); + return $root.'/bin/storage'; + } + +} diff --git a/src/infrastructure/testing/fixture/storage/__init__.php b/src/infrastructure/testing/fixture/storage/__init__.php new file mode 100644 index 0000000000..7031306832 --- /dev/null +++ b/src/infrastructure/testing/fixture/storage/__init__.php @@ -0,0 +1,15 @@ +getPhabricatorTestCaseConfiguration() + array( - self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK => true, + $config = $this->getPhabricatorTestCaseConfiguration() + array( + self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK => true, + self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => false, ); + + if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) { + // Fixtures don't make sense with process isolation. + $config[self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK] = false; + } + + return $config; } protected function willRunTests() { @@ -53,6 +80,13 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { LiskDAO::beginIsolateAllLiskEffectsToCurrentProcess(); } + if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) { + ++self::$storageFixtureReferences; + if (!self::$storageFixture) { + self::$storageFixture = $this->newStorageFixture(); + } + } + $this->env = PhabricatorEnv::beginScopedEnv(); } @@ -63,6 +97,13 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { LiskDAO::endIsolateAllLiskEffectsToCurrentProcess(); } + if (self::$storageFixture) { + self::$storageFixtureReferences--; + if (!self::$storageFixtureReferences) { + self::$storageFixture = null; + } + } + try { unset($this->env); } catch (Exception $ex) { @@ -72,4 +113,27 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase { } } + protected function willRunOneTest($test) { + $config = $this->getComputedConfiguration(); + + if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) { + LiskDAO::beginIsolateAllLiskEffectsToTransactions(); + } + } + + protected function didRunOneTest($test) { + $config = $this->getComputedConfiguration(); + + if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) { + LiskDAO::endIsolateAllLiskEffectsToTransactions(); + } + } + + protected function newStorageFixture() { + $bytes = Filesystem::readRandomCharacters(24); + $name = 'phabricator_unittest_'.$bytes; + + return new PhabricatorStorageFixtureScopeGuard($name); + } + } diff --git a/src/infrastructure/testing/testcase/__init__.php b/src/infrastructure/testing/testcase/__init__.php index 79fa8d94f5..dd15da2523 100644 --- a/src/infrastructure/testing/testcase/__init__.php +++ b/src/infrastructure/testing/testcase/__init__.php @@ -9,8 +9,10 @@ phutil_require_module('arcanist', 'unit/engine/phutil/testcase'); phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phabricator', 'infrastructure/testing/fixture/storage'); phutil_require_module('phabricator', 'storage/lisk/dao'); +phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'moduleutils'); diff --git a/src/storage/lisk/dao/LiskDAO.php b/src/storage/lisk/dao/LiskDAO.php index 4c58823d83..2445d2013e 100644 --- a/src/storage/lisk/dao/LiskDAO.php +++ b/src/storage/lisk/dao/LiskDAO.php @@ -190,7 +190,8 @@ abstract class LiskDAO { private $__dirtyFields = array(); private $__missingFields = array(); - private static $processIsolationLevel = 0; + private static $processIsolationLevel = 0; + private static $transactionIsolationLevel = 0; private static $__checkedClasses = array(); private $__ephemeral = false; @@ -809,12 +810,22 @@ abstract class LiskDAO { return $connection; } + if (self::shouldIsolateAllLiskEffectsToTransactions()) { + // If we're doing fixture transaction isolation, force the mode to 'w' + // so we always get the same connection for reads and writes, and thus + // can see the writes inside the transaction. + $mode = 'w'; + } + // TODO There is currently no protection on 'r' queries against writing // or on 'w' queries against reading $connection = $this->getEstablishedConnection($mode); if (!$connection) { $connection = $this->establishLiveConnection($mode); + if (self::shouldIsolateAllLiskEffectsToTransactions()) { + $connection->openTransaction(); + } $this->setEstablishedConnection($mode, $connection); } @@ -1356,6 +1367,44 @@ abstract class LiskDAO { return new AphrontIsolatedDatabaseConnection($config); } + /** + * @task isolate + */ + public static function beginIsolateAllLiskEffectsToTransactions() { + if (self::$transactionIsolationLevel === 0) { + self::closeAllConnections(); + } + self::$transactionIsolationLevel++; + } + + /** + * @task isolate + */ + public static function endIsolateAllLiskEffectsToTransactions() { + self::$transactionIsolationLevel--; + if (self::$transactionIsolationLevel < 0) { + throw new Exception( + "Lisk transaction isolation level was reduced below 0."); + } else if (self::$transactionIsolationLevel == 0) { + foreach (self::$connections as $key => $conn) { + if ($conn) { + $conn->killTransaction(); + } + } + self::closeAllConnections(); + } + } + + /** + * @task isolate + */ + public static function shouldIsolateAllLiskEffectsToTransactions() { + return (bool)self::$transactionIsolationLevel; + } + + public static function closeAllConnections() { + self::$connections = array(); + } /* -( Utilities )---------------------------------------------------------- */ diff --git a/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php b/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php new file mode 100644 index 0000000000..b4a76c3de3 --- /dev/null +++ b/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php @@ -0,0 +1,69 @@ + true, + ); + } + + public function testTransactionalIsolation1of2() { + // NOTE: These tests are verifying that data is destroyed between tests. + // If the user from either test persists, the other test will fail. + $this->assertEqual( + 0, + count(id(new PhabricatorUser())->loadAll())); + + id(new PhabricatorUser()) + ->setUserName('alincoln') + ->setRealName('Abraham Lincoln') + ->setEmail('alincoln@example.com') + ->save(); + } + + public function testTransactionalIsolation2of2() { + $this->assertEqual( + 0, + count(id(new PhabricatorUser())->loadAll())); + + id(new PhabricatorUser()) + ->setUserName('ugrant') + ->setRealName('Ulysses S. Grant') + ->setEmail('ugrant@example.com') + ->save(); + } + + public function testFixturesBasicallyWork() { + $this->assertEqual( + 0, + count(id(new PhabricatorUser())->loadAll())); + + id(new PhabricatorUser()) + ->setUserName('gwashington') + ->setRealName('George Washington') + ->setEmail('gwashington@example.com') + ->save(); + + $this->assertEqual( + 1, + count(id(new PhabricatorUser())->loadAll())); + } + +} diff --git a/src/storage/lisk/dao/__tests__/__init__.php b/src/storage/lisk/dao/__tests__/__init__.php index bb653cebda..d0e9f04537 100644 --- a/src/storage/lisk/dao/__tests__/__init__.php +++ b/src/storage/lisk/dao/__tests__/__init__.php @@ -6,11 +6,15 @@ +phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); phutil_require_module('phabricator', 'infrastructure/testing/testcase'); phutil_require_module('phabricator', 'storage/lisk/dao'); +phutil_require_module('phutil', 'utils'); + +phutil_require_source('LiskFixtureTestCase.php'); phutil_require_source('LiskIsolationTestCase.php'); phutil_require_source('LiskIsolationTestDAO.php'); phutil_require_source('LiskIsolationTestDAOException.php');