From 825fb9c85ae7b12f1e20be0328d4cba220ca757f Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 3 Sep 2013 17:27:38 -0700 Subject: [PATCH] Add JIRA doorkeeper and remarkup support Summary: Ref T3687. Adds a Doorkeeper bridge for JIRA issues, plus remarkup support. In particular: - The Asana and JIRA remarkup rules shared most of their implementation, so I refactored what I could into a base class. - Actual bridge implementation is straightforward and similar to Asana, although probably not similar enough to really justify refactoring. Test Plan: - When logged in as a JIRA-connected user, pasted a JIRA issue link and saw it enriched at rendering time. - Logged in and out with JIRA. - Tested an Asana link, too (seems I haven't broken anything). Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3687 Differential Revision: https://secure.phabricator.com/D6878 --- src/__phutil_library_map__.php | 9 +- .../PhabricatorAuthProviderOAuth1JIRA.php | 29 +++++ .../PhabricatorApplicationDoorkeeper.php | 1 + .../bridge/DoorkeeperBridgeJIRA.php | 119 ++++++++++++++++++ .../remarkup/DoorkeeperRemarkupRule.php | 65 ++++++++++ .../remarkup/DoorkeeperRemarkupRuleAsana.php | 82 +++--------- .../remarkup/DoorkeeperRemarkupRuleJIRA.php | 44 +++++++ 7 files changed, 281 insertions(+), 68 deletions(-) create mode 100644 src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php create mode 100644 src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRule.php create mode 100644 src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6eb5c8befe..a1893c9f65 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -545,6 +545,7 @@ phutil_register_library_map(array( 'DivinerWorkflow' => 'applications/diviner/workflow/DivinerWorkflow.php', 'DoorkeeperBridge' => 'applications/doorkeeper/bridge/DoorkeeperBridge.php', 'DoorkeeperBridgeAsana' => 'applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php', + 'DoorkeeperBridgeJIRA' => 'applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php', 'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php', 'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php', 'DoorkeeperExternalObjectQuery' => 'applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php', @@ -552,7 +553,9 @@ phutil_register_library_map(array( 'DoorkeeperFeedWorkerAsana' => 'applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php', 'DoorkeeperImportEngine' => 'applications/doorkeeper/engine/DoorkeeperImportEngine.php', 'DoorkeeperObjectRef' => 'applications/doorkeeper/engine/DoorkeeperObjectRef.php', + 'DoorkeeperRemarkupRule' => 'applications/doorkeeper/remarkup/DoorkeeperRemarkupRule.php', 'DoorkeeperRemarkupRuleAsana' => 'applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php', + 'DoorkeeperRemarkupRuleJIRA' => 'applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php', 'DoorkeeperTagsController' => 'applications/doorkeeper/controller/DoorkeeperTagsController.php', 'DrydockAllocatorWorker' => 'applications/drydock/worker/DrydockAllocatorWorker.php', 'DrydockApacheWebrootInterface' => 'applications/drydock/interface/webroot/DrydockApacheWebrootInterface.php', @@ -2026,6 +2029,7 @@ phutil_register_library_map(array( 'function' => array( '_phabricator_date_format' => 'view/viewutils.php', + '_phabricator_time_format' => 'view/viewutils.php', 'celerity_generate_unique_node_id' => 'infrastructure/celerity/api.php', 'celerity_get_resource_uri' => 'infrastructure/celerity/api.php', 'celerity_register_resource_map' => 'infrastructure/celerity/map.php', @@ -2575,6 +2579,7 @@ phutil_register_library_map(array( 'DivinerWorkflow' => 'PhutilArgumentWorkflow', 'DoorkeeperBridge' => 'Phobject', 'DoorkeeperBridgeAsana' => 'DoorkeeperBridge', + 'DoorkeeperBridgeJIRA' => 'DoorkeeperBridge', 'DoorkeeperDAO' => 'PhabricatorLiskDAO', 'DoorkeeperExternalObject' => array( @@ -2585,7 +2590,9 @@ phutil_register_library_map(array( 'DoorkeeperFeedWorkerAsana' => 'FeedPushWorker', 'DoorkeeperImportEngine' => 'Phobject', 'DoorkeeperObjectRef' => 'Phobject', - 'DoorkeeperRemarkupRuleAsana' => 'PhutilRemarkupRule', + 'DoorkeeperRemarkupRule' => 'PhutilRemarkupRule', + 'DoorkeeperRemarkupRuleAsana' => 'DoorkeeperRemarkupRule', + 'DoorkeeperRemarkupRuleJIRA' => 'DoorkeeperRemarkupRule', 'DoorkeeperTagsController' => 'PhabricatorController', 'DrydockAllocatorWorker' => 'PhabricatorWorker', 'DrydockApacheWebrootInterface' => 'DrydockWebrootInterface', diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php index b0f6f69f31..5653b4da89 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php @@ -3,6 +3,10 @@ final class PhabricatorAuthProviderOAuth1JIRA extends PhabricatorAuthProviderOAuth1 { + public function getJIRABaseURI() { + return $this->getProviderConfig()->getProperty(self::PROPERTY_JIRA_URI); + } + public function getProviderName() { return pht('JIRA'); } @@ -245,4 +249,29 @@ final class PhabricatorAuthProviderOAuth1JIRA return true; } + public static function getJIRAProvider() { + $providers = self::getAllEnabledProviders(); + + foreach ($providers as $provider) { + if ($provider instanceof PhabricatorAuthProviderOAuth1JIRA) { + return $provider; + } + } + + return null; + } + + public function newJIRAFuture( + PhabricatorExternalAccount $account, + $path, + $method, + $params = array()) { + + $adapter = clone $this->getAdapter(); + $adapter->setToken($account->getProperty('oauth1.token')); + $adapter->setTokenSecret($account->getProperty('oauth1.token.secret')); + + return $adapter->newJIRAFuture($path, $method, $params); + } + } diff --git a/src/applications/doorkeeper/application/PhabricatorApplicationDoorkeeper.php b/src/applications/doorkeeper/application/PhabricatorApplicationDoorkeeper.php index a86d7d09e8..24b605c1cc 100644 --- a/src/applications/doorkeeper/application/PhabricatorApplicationDoorkeeper.php +++ b/src/applications/doorkeeper/application/PhabricatorApplicationDoorkeeper.php @@ -17,6 +17,7 @@ final class PhabricatorApplicationDoorkeeper extends PhabricatorApplication { public function getRemarkupRules() { return array( new DoorkeeperRemarkupRuleAsana(), + new DoorkeeperRemarkupRuleJIRA(), ); } diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php new file mode 100644 index 0000000000..d259e40ebb --- /dev/null +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php @@ -0,0 +1,119 @@ +getApplicationType() != self::APPTYPE_JIRA) { + return false; + } + + $types = array( + self::OBJTYPE_ISSUE => true, + ); + + return isset($types[$ref->getObjectType()]); + } + + public function pullRefs(array $refs) { + + $id_map = mpull($refs, 'getObjectID', 'getObjectKey'); + $viewer = $this->getViewer(); + + $provider = PhabricatorAuthProviderOAuth1JIRA::getJIRAProvider(); + if (!$provider) { + return; + } + + $accounts = id(new PhabricatorExternalAccountQuery()) + ->setViewer($viewer) + ->withUserPHIDs(array($viewer->getPHID())) + ->withAccountTypes(array($provider->getProviderType())) + ->execute(); + + if (!$accounts) { + return; + } + + // TODO: When we support multiple JIRA instances, we need to disambiguate + // issues (perhaps with additional configuration) or cast a wide net + // (by querying all instances). For now, just query the one instance. + $account = head($accounts); + + $futures = array(); + foreach ($id_map as $key => $id) { + $futures[$key] = $provider->newJIRAFuture( + $account, + 'rest/api/2/issue/'.phutil_escape_uri($id), + 'GET'); + } + + $results = array(); + $failed = array(); + foreach (Futures($futures) as $key => $future) { + try { + $results[$key] = $future->resolveJSON(); + } catch (Exception $ex) { + if (($ex instanceof HTTPFutureResponseStatus) && + ($ex->getStatusCode() == 404)) { + // This indicates that the object has been deleted (or never existed, + // or isn't visible to the current user) but it's a successful sync of + // an object which isn't visible. + } else { + // This is something else, so consider it a synchronization failure. + phlog($ex); + $failed[$key] = $ex; + } + } + } + + foreach ($refs as $ref) { + $ref->setAttribute('name', pht('JIRA %s', $ref->getObjectID())); + + $did_fail = idx($failed, $ref->getObjectKey()); + if ($did_fail) { + $ref->setSyncFailed(true); + continue; + } + + $result = idx($results, $ref->getObjectKey()); + if (!$result) { + continue; + } + + $fields = idx($result, 'fields', array()); + + $ref->setIsVisible(true); + $ref->setAttribute( + 'fullname', + pht('JIRA %s %s', $result['key'], idx($fields, 'summary'))); + + $ref->setAttribute('title', idx($fields, 'summary')); + $ref->setAttribute('description', idx($result, 'description')); + + $obj = $ref->getExternalObject(); + if ($obj->getID()) { + continue; + } + + $this->fillObjectFromData($obj, $result); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $obj->save(); + unset($unguarded); + } + } + + public function fillObjectFromData(DoorkeeperExternalObject $obj, $result) { + // 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()); + + $obj->setObjectURI((string)$uri); + } + +} diff --git a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRule.php b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRule.php new file mode 100644 index 0000000000..815ac56b69 --- /dev/null +++ b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRule.php @@ -0,0 +1,65 @@ +getEngine(); + $token = $engine->storeText(get_class($this)); + + $tags = $engine->getTextMetadata($key, array()); + + $tags[] = array( + 'token' => $token, + ) + $spec + array( + 'extra' => array(), + ); + + $engine->setTextMetadata($key, $tags); + return $token; + } + + public function didMarkupText() { + $key = self::KEY_TAGS; + $engine = $this->getEngine(); + $tags = $engine->getTextMetadata($key, array()); + + if (!$tags) { + return; + } + + $refs = array(); + foreach ($tags as $spec) { + $tag_id = celerity_generate_unique_node_id(); + + $refs[] = array( + 'id' => $tag_id, + ) + $spec['tag']; + + if ($this->getEngine()->isTextMode()) { + $view = $spec['href']; + } else { + $view = id(new PhabricatorTagView()) + ->setID($tag_id) + ->setName($spec['href']) + ->setHref($spec['href']) + ->setType(PhabricatorTagView::TYPE_OBJECT) + ->setExternal(true); + } + + $engine->overwriteStoredText($spec['token'], $view); + } + + Javelin::initBehavior('doorkeeper-tag', array('tags' => $refs)); + + $engine->setTextMetadata($key, array()); + } + +} diff --git a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php index 7307d0e8f3..a2ce76d2e8 100644 --- a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php +++ b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php @@ -1,13 +1,7 @@ getEngine(); - $token = $engine->storeText('AsanaDoorkeeper'); - - $tags = $engine->getTextMetadata($key, array()); - - $tags[] = array( - 'token' => $token, - 'href' => $matches[0], - 'tag' => array( - 'ref' => array( - DoorkeeperBridgeAsana::APPTYPE_ASANA, - DoorkeeperBridgeAsana::APPDOMAIN_ASANA, - DoorkeeperBridgeAsana::OBJTYPE_TASK, - $matches[2], + return $this->addDoorkeeperTag( + array( + 'href' => $matches[0], + 'tag' => array( + 'ref' => array( + DoorkeeperBridgeAsana::APPTYPE_ASANA, + DoorkeeperBridgeAsana::APPDOMAIN_ASANA, + DoorkeeperBridgeAsana::OBJTYPE_TASK, + $matches[2], + ), + 'extra' => array( + 'asana.context' => $matches[1], + ), ), - 'extra' => array( - 'asana.context' => $matches[1], - ), - ), - ); - - $engine->setTextMetadata($key, $tags); - - return $token; - } - - public function didMarkupText() { - $key = self::KEY_TAGS; - $engine = $this->getEngine(); - $tags = $engine->getTextMetadata($key, array()); - - if (!$tags) { - return; - } - - $refs = array(); - foreach ($tags as $spec) { - $tag_id = celerity_generate_unique_node_id(); - - $refs[] = array( - 'id' => $tag_id, - ) + $spec['tag']; - - if ($this->getEngine()->isTextMode()) { - $view = $spec['href']; - } else { - $view = id(new PhabricatorTagView()) - ->setID($tag_id) - ->setName($spec['href']) - ->setHref($spec['href']) - ->setType(PhabricatorTagView::TYPE_OBJECT) - ->setExternal(true); - } - - $engine->overwriteStoredText($spec['token'], $view); - } - - Javelin::initBehavior('doorkeeper-tag', array('tags' => $refs)); - - $engine->setTextMetadata($key, array()); + )); } } diff --git a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php new file mode 100644 index 0000000000..d09433b23d --- /dev/null +++ b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php @@ -0,0 +1,44 @@ +getJIRABaseURI(); + if ($match_domain != rtrim($jira_base, '/')) { + return $matches[0]; + } + + return $this->addDoorkeeperTag( + array( + 'href' => $matches[0], + 'tag' => array( + 'ref' => array( + DoorkeeperBridgeJIRA::APPTYPE_JIRA, + $provider->getProviderDomain(), + DoorkeeperBridgeJIRA::OBJTYPE_ISSUE, + $match_issue, + ), + ), + )); + } + + +}