Use "w" transactions for reading if they exist, so we can do transactional reads

Summary: Generally moves us toward having a sane approach to transaction handling.

Test Plan: See test case, which fails before this patch and passes afterwards.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2394

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
+    // TODO: There is currently no protection on 'r' queries against writing.
-    // or on 'w' queries against reading
+
+    $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()) {

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;
+    }
+  }
+
+
+
 }

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');