From 7a9d5f8f2de5e44f4014116a47de4ea870361732 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 18 May 2014 05:45:21 -0700
Subject: [PATCH] Fix JIRA issue URI selection for JIRA installs which are not
 on the domain root

Summary: Fixes T4859. See that for details.

Test Plan:
  - Verified things still work on my local (domain root) install.
  - Added some unit tests.
  - Did not verify a non-root install since I don't have one handy, hopefully @salehe can help.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: salehe, epriestley

Maniphest Tasks: T4859

Differential Revision: https://secure.phabricator.com/D8836
---
 src/__phutil_library_map__.php                |  2 +
 .../bridge/DoorkeeperBridgeJIRA.php           | 30 +++++++++++++--
 .../DoorkeeperBridgeJIRATestCase.php          | 37 +++++++++++++++++++
 .../remarkup/DoorkeeperRemarkupRuleJIRA.php   |  4 +-
 4 files changed, 68 insertions(+), 5 deletions(-)
 create mode 100644 src/applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 696021f986..f8ab27945e 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -623,6 +623,7 @@ phutil_register_library_map(array(
     'DoorkeeperBridge' => 'applications/doorkeeper/bridge/DoorkeeperBridge.php',
     'DoorkeeperBridgeAsana' => 'applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php',
     'DoorkeeperBridgeJIRA' => 'applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php',
+    'DoorkeeperBridgeJIRATestCase' => 'applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php',
     'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php',
     'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php',
     'DoorkeeperExternalObjectQuery' => 'applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php',
@@ -3276,6 +3277,7 @@ phutil_register_library_map(array(
     'DoorkeeperBridge' => 'Phobject',
     'DoorkeeperBridgeAsana' => 'DoorkeeperBridge',
     'DoorkeeperBridgeJIRA' => 'DoorkeeperBridge',
+    'DoorkeeperBridgeJIRATestCase' => 'PhabricatorTestCase',
     'DoorkeeperDAO' => 'PhabricatorLiskDAO',
     'DoorkeeperExternalObject' =>
     array(
diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php
index 5149c962dd..3cc8393f7e 100644
--- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php
+++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php
@@ -110,10 +110,34 @@ final class DoorkeeperBridgeJIRA extends DoorkeeperBridge {
     // Convert the "self" URI, which points at the REST endpoint, into a
     // browse URI.
     $self = idx($result, 'self');
-    $uri = new PhutilURI($self);
-    $uri->setPath('browse/'.$obj->getObjectID());
+    $object_id = $obj->getObjectID();
 
-    $obj->setObjectURI((string)$uri);
+    $uri = self::getJIRAIssueBrowseURIFromJIRARestURI($self, $object_id);
+    if ($uri !== null) {
+      $obj->setObjectURI($uri);
+    }
+  }
+
+  public static function getJIRAIssueBrowseURIFromJIRARestURI(
+    $uri,
+    $object_id) {
+
+    $uri = new PhutilURI($uri);
+
+    // The JIRA install might not be at the domain root, so we may need to
+    // keep an initial part of the path, like "/jira/". Find the API specific
+    // part of the URI, strip it off, then replace it with the web version.
+    $path = $uri->getPath();
+    $pos = strrpos($path, 'rest/api/2/issue/');
+    if ($pos === false) {
+      return null;
+    }
+
+    $path = substr($path, 0, $pos);
+    $path = $path.'browse/'.$object_id;
+    $uri->setPath($path);
+
+    return (string)$uri;
   }
 
 }
diff --git a/src/applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php b/src/applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php
new file mode 100644
index 0000000000..a31e8d3974
--- /dev/null
+++ b/src/applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php
@@ -0,0 +1,37 @@
+<?php
+
+final class DoorkeeperBridgeJIRATestCase extends PhabricatorTestCase {
+
+  public function testJIRABridgeRestAPIURIConversion() {
+    $map = array(
+      array(
+        // Installed at domain root.
+        'http://jira.example.com/rest/api/2/issue/1',
+        'TP-1',
+        'http://jira.example.com/browse/TP-1'
+      ),
+      array(
+        // Installed on path.
+        'http://jira.example.com/jira/rest/api/2/issue/1',
+        'TP-1',
+        'http://jira.example.com/jira/browse/TP-1'
+      ),
+      array(
+        // A URI we don't understand.
+        'http://jira.example.com/wake/cli/3/task/1',
+        'TP-1',
+        null,
+      ),
+    );
+
+    foreach ($map as $inputs) {
+      list($rest_uri, $object_id, $expect) = $inputs;
+      $this->assertEqual(
+        $expect,
+        DoorkeeperBridgeJIRA::getJIRAIssueBrowseURIFromJIRARestURI(
+          $rest_uri,
+          $object_id));
+    }
+  }
+
+}
diff --git a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php
index d09433b23d..0eadc6dc45 100644
--- a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php
+++ b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php
@@ -5,13 +5,12 @@ final class DoorkeeperRemarkupRuleJIRA
 
   public function apply($text) {
     return preg_replace_callback(
-      '@(https?://[^/]+)/browse/([A-Z]+-[1-9]\d*)@',
+      '@(https?://\S+?)/browse/([A-Z]+-[1-9]\d*)@',
       array($this, 'markupJIRALink'),
       $text);
   }
 
   public function markupJIRALink($matches) {
-
     $match_domain = $matches[1];
     $match_issue = $matches[2];
 
@@ -21,6 +20,7 @@ final class DoorkeeperRemarkupRuleJIRA
       return $matches[0];
     }
 
+
     $jira_base = $provider->getJIRABaseURI();
     if ($match_domain != rtrim($jira_base, '/')) {
       return $matches[0];