diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -843,12 +843,14 @@ 'DoorkeeperBridgeAsana' => 'applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php', 'DoorkeeperBridgeGitHub' => 'applications/doorkeeper/bridge/DoorkeeperBridgeGitHub.php', 'DoorkeeperBridgeGitHubIssue' => 'applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php', + 'DoorkeeperBridgeGitHubUser' => 'applications/doorkeeper/bridge/DoorkeeperBridgeGitHubUser.php', 'DoorkeeperBridgeJIRA' => 'applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php', 'DoorkeeperBridgeJIRATestCase' => 'applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php', 'DoorkeeperBridgedObjectCurtainExtension' => 'applications/doorkeeper/engineextension/DoorkeeperBridgedObjectCurtainExtension.php', 'DoorkeeperBridgedObjectInterface' => 'applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php', 'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php', 'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php', + 'DoorkeeperExternalObjectPHIDType' => 'applications/doorkeeper/phid/DoorkeeperExternalObjectPHIDType.php', 'DoorkeeperExternalObjectQuery' => 'applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php', 'DoorkeeperFeedStoryPublisher' => 'applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php', 'DoorkeeperFeedWorker' => 'applications/doorkeeper/worker/DoorkeeperFeedWorker.php', @@ -5017,6 +5019,7 @@ 'DoorkeeperBridgeAsana' => 'DoorkeeperBridge', 'DoorkeeperBridgeGitHub' => 'DoorkeeperBridge', 'DoorkeeperBridgeGitHubIssue' => 'DoorkeeperBridgeGitHub', + 'DoorkeeperBridgeGitHubUser' => 'DoorkeeperBridgeGitHub', 'DoorkeeperBridgeJIRA' => 'DoorkeeperBridge', 'DoorkeeperBridgeJIRATestCase' => 'PhabricatorTestCase', 'DoorkeeperBridgedObjectCurtainExtension' => 'PHUICurtainExtension', @@ -5025,6 +5028,7 @@ 'DoorkeeperDAO', 'PhabricatorPolicyInterface', ), + 'DoorkeeperExternalObjectPHIDType' => 'PhabricatorPHIDType', 'DoorkeeperExternalObjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DoorkeeperFeedStoryPublisher' => 'Phobject', 'DoorkeeperFeedWorker' => 'FeedPushWorker', diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubUser.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubUser.php new file mode 100644 --- /dev/null +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubUser.php @@ -0,0 +1,120 @@ +getObjectType() !== self::OBJTYPE_GITHUB_USER) { + return false; + } + + return true; + } + + public function pullRefs(array $refs) { + $token = $this->getGitHubAccessToken(); + if (!strlen($token)) { + return null; + } + + $template = id(new PhutilGitHubFuture()) + ->setAccessToken($token); + + $futures = array(); + $id_map = mpull($refs, 'getObjectID', 'getObjectKey'); + foreach ($id_map as $key => $id) { + // GitHub doesn't provide a way to query for users by ID directly, but we + // can list all users, ordered by ID, starting at some particular ID, + // with a page size of one, which will achieve the desired effect. + $one_less = ($id - 1); + $uri = "/users?since={$one_less}&per_page=1"; + + $data = array(); + $futures[$key] = id(clone $template) + ->setRawGitHubQuery($uri, $data); + } + + $results = array(); + $failed = array(); + foreach (new FutureIterator($futures) as $key => $future) { + try { + $results[$key] = $future->resolve(); + } catch (Exception $ex) { + if (($ex instanceof HTTPFutureResponseStatus) && + ($ex->getStatusCode() == 404)) { + // TODO: Do we end up here for deleted objects and invisible + // objects? + } else { + phlog($ex); + $failed[$key] = $ex; + } + } + } + + $viewer = $this->getViewer(); + + foreach ($refs as $ref) { + $ref->setAttribute('name', pht('GitHub User %s', $ref->getObjectID())); + + $did_fail = idx($failed, $ref->getObjectKey()); + if ($did_fail) { + $ref->setSyncFailed(true); + continue; + } + + $result = idx($results, $ref->getObjectKey()); + if (!$result) { + continue; + } + + $body = $result->getBody(); + if (!is_array($body) || !count($body)) { + $ref->setSyncFailed(true); + continue; + } + + $spec = head($body); + if (!is_array($spec)) { + $ref->setSyncFailed(true); + continue; + } + + // Because we're using a paging query to load each user, if a user (say, + // user ID 123) does not exist for some reason, we might get the next + // user (say, user ID 124) back. Make sure the user we got back is really + // the user we expect. + $id = idx($spec, 'id'); + if ($id !== $ref->getObjectID()) { + $ref->setSyncFailed(true); + continue; + } + + $ref->setIsVisible(true); + $ref->setAttribute('api.raw', $spec); + $ref->setAttribute('name', $spec['login']); + + $obj = $ref->getExternalObject(); + $this->fillObjectFromData($obj, $spec); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $obj->save(); + unset($unguarded); + } + } + + public function fillObjectFromData(DoorkeeperExternalObject $obj, $spec) { + $uri = $spec['html_url']; + $obj->setObjectURI($uri); + + $login = $spec['login']; + + $obj->setDisplayName(pht('%s <%s>', $login, pht('GitHub'))); + } + +} diff --git a/src/applications/doorkeeper/phid/DoorkeeperExternalObjectPHIDType.php b/src/applications/doorkeeper/phid/DoorkeeperExternalObjectPHIDType.php new file mode 100644 --- /dev/null +++ b/src/applications/doorkeeper/phid/DoorkeeperExternalObjectPHIDType.php @@ -0,0 +1,47 @@ +withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + foreach ($handles as $phid => $handle) { + $xobj = $objects[$phid]; + + $uri = $xobj->getObjectURI(); + $name = $xobj->getDisplayName(); + $full_name = $xobj->getDisplayFullName(); + + $handle + ->setURI($uri) + ->setName($name) + ->setFullName($full_name); + } + } + +} diff --git a/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php b/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php --- a/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php +++ b/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php @@ -16,40 +16,32 @@ return $this; } + public function newResultObject() { + return new DoorkeeperExternalObject(); + } + protected function loadPage() { - $table = new DoorkeeperExternalObject(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT * FROM %T %Q %Q %Q', - $table->getTableName(), - $this->buildWhereClause($conn_r), - $this->buildOrderClause($conn_r), - $this->buildLimitClause($conn_r)); - - return $table->loadAllFromArray($data); + return $this->loadStandardPage($this->newResultObject()); } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); - if ($this->phids) { + if ($this->phids !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'phid IN (%Ls)', $this->phids); } - if ($this->objectKeys) { + if ($this->objectKeys !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'objectKey IN (%Ls)', $this->objectKeys); } - $where[] = $this->buildPagingClause($conn_r); - return $this->formatWhereClause($where); + return $where; } public function getQueryApplicationClass() { diff --git a/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php b/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php --- a/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php +++ b/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php @@ -47,7 +47,7 @@ public function generatePHID() { return PhabricatorPHID::generateNewPHID( - PhabricatorPHIDConstants::PHID_TYPE_XOBJ); + DoorkeeperExternalObjectPHIDType::TYPECONST); } public function getProperty($key, $default = null) { @@ -83,6 +83,27 @@ return parent::save(); } + public function setDisplayName($display_name) { + return $this->setProperty('xobj.name.display', $display_name); + } + + public function getDisplayName() { + return $this->getProperty('xobj.name.display', pht('External Object')); + } + + public function setDisplayFullName($full_name) { + return $this->setProperty('xobj.name.display-full', $full_name); + } + + public function getDisplayFullName() { + $full_name = $this->getProperty('xobj.name.display-full'); + + if ($full_name !== null) { + return $full_name; + } + + return $this->getDisplayName(); + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -6,6 +6,7 @@ const ITEMTYPE = 'github.event'; private $externalObject; + private $externalActor; public function getItemTypeDisplayName() { return pht('GitHub Event'); @@ -27,17 +28,18 @@ $viewer = $this->getViewer(); $is_dirty = false; - // TODO: Link up the requestor, etc. - - $is_dirty = false; - $xobj = $this->reloadExternalObject($item); - if ($xobj) { $item->setItemProperty('doorkeeper.xobj.phid', $xobj->getPHID()); $is_dirty = true; } + $actor = $this->reloadExternalActor($item); + if ($actor) { + $item->setRequestorPHID($actor->getPHID()); + $is_dirty = true; + } + if ($item->getStatus() == NuanceItem::STATUS_IMPORTING) { $item->setStatus(NuanceItem::STATUS_ROUTING); $is_dirty = true; @@ -48,6 +50,21 @@ } } + private function getDoorkeeperActorRef(NuanceItem $item) { + $raw = $this->newRawEvent($item); + + $user_id = $raw->getActorGitHubUserID(); + if (!$user_id) { + return null; + } + + $ref_type = DoorkeeperBridgeGitHubUser::OBJTYPE_GITHUB_USER; + + return $this->newDoorkeeperRef() + ->setObjectType($ref_type) + ->setObjectID($user_id); + } + private function getDoorkeeperRef(NuanceItem $item) { $raw = $this->newRawEvent($item); @@ -64,19 +81,52 @@ return null; } - return id(new DoorkeeperObjectRef()) - ->setApplicationType(DoorkeeperBridgeGitHub::APPTYPE_GITHUB) - ->setApplicationDomain(DoorkeeperBridgeGitHub::APPDOMAIN_GITHUB) + return $this->newDoorkeeperRef() ->setObjectType($ref_type) ->setObjectID($full_ref); } + private function newDoorkeeperRef() { + return id(new DoorkeeperObjectRef()) + ->setApplicationType(DoorkeeperBridgeGitHub::APPTYPE_GITHUB) + ->setApplicationDomain(DoorkeeperBridgeGitHub::APPDOMAIN_GITHUB); + } + private function reloadExternalObject(NuanceItem $item, $local = false) { $ref = $this->getDoorkeeperRef($item); if (!$ref) { return null; } + $xobj = $this->reloadExternalRef($item, $ref, $local); + + if ($xobj) { + $this->externalObject = $xobj; + } + + return $xobj; + } + + private function reloadExternalActor(NuanceItem $item, $local = false) { + $ref = $this->getDoorkeeperActorRef($item); + if (!$ref) { + return null; + } + + $xobj = $this->reloadExternalRef($item, $ref, $local); + + if ($xobj) { + $this->externalActor = $xobj; + } + + return $xobj; + } + + private function reloadExternalRef( + NuanceItem $item, + DoorkeeperObjectRef $ref, + $local) { + $source = $item->getSource(); $token = $source->getSourceProperty('github.token'); $token = new PhutilOpaqueEnvelope($token); @@ -97,10 +147,6 @@ $xobj = $ref->getExternalObject(); } - if ($xobj) { - $this->externalObject = $xobj; - } - return $xobj; } @@ -121,6 +167,23 @@ return null; } + private function getExternalActor(NuanceItem $item) { + if ($this->externalActor === null) { + $xobj = $this->reloadExternalActor($item, $local = true); + if ($xobj) { + $this->externalActor = $xobj; + } else { + $this->externalActor = false; + } + } + + if ($this->externalActor) { + return $this->externalActor; + } + + return null; + } + private function newRawEvent(NuanceItem $item) { $type = $item->getItemProperty('api.type'); $raw = $item->getItemProperty('api.raw', array()); @@ -174,6 +237,13 @@ } } + $xactor = $this->getExternalActor($item); + if ($xactor) { + $panels[] = $this->newCurtainPanel($item) + ->setHeaderText(pht('GitHub Actor')) + ->appendChild($viewer->renderHandle($xactor->getPHID())); + } + return $panels; } @@ -363,8 +433,12 @@ } protected function getActingAsPHID(NuanceItem $item) { - // TODO: This should be an external account PHID representing the original - // GitHub user. + $actor_phid = $item->getRequestorPHID(); + + if ($actor_phid) { + return $actor_phid; + } + return parent::getActingAsPHID($item); } diff --git a/src/applications/phid/PhabricatorPHIDConstants.php b/src/applications/phid/PhabricatorPHIDConstants.php --- a/src/applications/phid/PhabricatorPHIDConstants.php +++ b/src/applications/phid/PhabricatorPHIDConstants.php @@ -10,8 +10,6 @@ const PHID_TYPE_XCMT = 'XCMT'; - const PHID_TYPE_XOBJ = 'XOBJ'; - const PHID_TYPE_VOID = 'VOID'; const PHID_VOID = 'PHID-VOID-00000000000000000000'; diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -17,7 +17,6 @@ static $class_map = array( PhabricatorPHIDConstants::PHID_TYPE_TOBJ => 'HarbormasterObject', - PhabricatorPHIDConstants::PHID_TYPE_XOBJ => 'DoorkeeperExternalObject', ); $class = idx($class_map, $phid_type);