From 5194ff3db595e2dff678eee990584dfa50a1a04e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Oct 2015 15:57:41 -0700
Subject: [PATCH] (stable) Fix several error handling issues with Subversion
 commits in Diffusion

Summary:
Ref T9513. I checked this briefly but didn't do a very thorough job of it.

  - Don't try to query merges for Subversion, since it doesn't support them.
  - Fix up "existsquery" to work properly (and efficiently) for both hosted and imported repositories.
  - Fix up "parentsquery" to have similar behavior on invalid commits to other VCSes (throw an exception).

Test Plan:
  - No more merges warning on SVN.
  - Hosted SVN gets the right exists result now.
  - Visiting "r23980283789287" now 404's instead of "not parsed yet".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9513

Differential Revision: https://secure.phabricator.com/D14239
---
 .../DiffusionExistsQueryConduitAPIMethod.php  | 17 +++++++----------
 .../controller/DiffusionCommitController.php  | 19 +++++++++++--------
 .../DiffusionLowLevelParentsQuery.php         | 16 +++++++++++++++-
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
index 3c6d09ebda..4280230002 100644
--- a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php
@@ -33,16 +33,13 @@ final class DiffusionExistsQueryConduitAPIMethod
   protected function getSVNResult(ConduitAPIRequest $request) {
     $repository = $this->getDiffusionRequest()->getRepository();
     $commit = $request->getValue('commit');
-    list($info) = $repository->execxRemoteCommand(
-      'info %s',
-      $repository->getRemoteURI());
-    $exists = false;
-    $matches = null;
-    if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
-      $base_revision = $matches[1];
-      $exists = $base_revision >= $commit;
-    }
-    return $exists;
+
+    $refs = id(new DiffusionCachedResolveRefsQuery())
+      ->setRepository($repository)
+      ->withRefs(array($commit))
+      ->execute();
+
+    return (bool)$refs;
   }
 
   protected function getMercurialResult(ConduitAPIRequest $request) {
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index fe2f539f09..a698eba5c0 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -1136,14 +1136,17 @@ final class DiffusionCommitController extends DiffusionController {
     $merge_limit = $this->getMergeDisplayLimit();
 
     try {
-      $merges = $this->callConduitWithDiffusionRequest(
-        'diffusion.mergedcommitsquery',
-        array(
-          'commit' => $commit,
-          'limit' => $merge_limit + 1,
-        ));
-
-      $this->commitMerges = DiffusionPathChange::newFromConduit($merges);
+      if ($repository->isSVN()) {
+        $this->commitMerges = array();
+      } else {
+        $merges = $this->callConduitWithDiffusionRequest(
+          'diffusion.mergedcommitsquery',
+          array(
+            'commit' => $commit,
+            'limit' => $merge_limit + 1,
+          ));
+        $this->commitMerges = DiffusionPathChange::newFromConduit($merges);
+      }
     } catch (Exception $ex) {
       $this->commitMerges = false;
       $exceptions[] = $ex;
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
index 60ee4b5fe2..f49f828d79 100644
--- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
@@ -70,7 +70,21 @@ final class DiffusionLowLevelParentsQuery
   }
 
   private function loadSubversionParents() {
-    $n = (int)$this->identifier;
+    $repository = $this->getRepository();
+    $identifier = $this->identifier;
+
+    $refs = id(new DiffusionCachedResolveRefsQuery())
+      ->setRepository($repository)
+      ->withRefs(array($identifier))
+      ->execute();
+    if (!$refs) {
+      throw new Exception(
+        pht(
+          'No commit "%s" in this repository.',
+          $identifier));
+    }
+
+    $n = (int)$identifier;
     if ($n > 1) {
       $ids = array($n - 1);
     } else {