diff --git a/src/storage/lisk/dao/LiskDAO.php b/src/storage/lisk/dao/LiskDAO.php index 2445d2013e..30bf8c3283 100644 --- a/src/storage/lisk/dao/LiskDAO.php +++ b/src/storage/lisk/dao/LiskDAO.php @@ -817,10 +817,20 @@ abstract class LiskDAO { $mode = 'w'; } - // TODO There is currently no protection on 'r' queries against writing - // or on 'w' queries against reading + // TODO: There is currently no protection on 'r' queries against writing. + + $connection = null; + if ($mode == 'r') { + // If we're requesting a read connection but already have a write + // connection, reuse the write connection so that reads can take place + // inside transactions. + $connection = $this->getEstablishedConnection('w'); + } + + if (!$connection) { + $connection = $this->getEstablishedConnection($mode); + } - $connection = $this->getEstablishedConnection($mode); if (!$connection) { $connection = $this->establishLiveConnection($mode); if (self::shouldIsolateAllLiskEffectsToTransactions()) { diff --git a/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php b/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php index b4a76c3de3..ad84abbb9b 100644 --- a/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php +++ b/src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php @@ -66,4 +66,40 @@ final class LiskFixtureTestCase extends PhabricatorTestCase { count(id(new PhabricatorUser())->loadAll())); } + public function testReadableTransactions() { + // TODO: When we have semi-durable fixtures, use those instead. This is + // extremely hacky. + + LiskDAO::endIsolateAllLiskEffectsToTransactions(); + try { + + $phid = 'PHID-TEST-'.Filesystem::readRandomCharacters(32); + + $obj = new PhabricatorPHID(); + $obj->openTransaction(); + + $obj->setPHID($phid); + $obj->setPHIDType('TEST'); + $obj->save(); + + $loaded = id(new PhabricatorPHID())->loadOneWhere( + 'phid = %s', + $phid); + + $obj->killTransaction(); + + $this->assertEqual( + true, + ($loaded !== null), + "Reads inside transactions should have transaction visibility."); + + LiskDAO::beginIsolateAllLiskEffectsToTransactions(); + } catch (Exception $ex) { + LiskDAO::beginIsolateAllLiskEffectsToTransactions(); + throw $ex; + } + } + + + } diff --git a/src/storage/lisk/dao/__tests__/__init__.php b/src/storage/lisk/dao/__tests__/__init__.php index d0e9f04537..21703c010e 100644 --- a/src/storage/lisk/dao/__tests__/__init__.php +++ b/src/storage/lisk/dao/__tests__/__init__.php @@ -11,6 +11,7 @@ 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', 'filesystem'); phutil_require_module('phutil', 'utils');