From 4ca38612f2b0afc9529469b9c5b3fc1771d3e89c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 25 Jun 2013 16:30:44 -0700 Subject: [PATCH] Add DoorkeeperExternalObjectQuery, expose more ref/import APIs Summary: Ref T2852. - Broadly, we support "I have a Ref, I need a PHID" well but not "I have a PHID, I need a Ref". - Add DoorkeeperExternalObjectQuery, and use it to query ExternalObjects. - Allow external objects to be imported by their internal PHIDs. Basically, if we have an edge pointing at an ExternalObject, we can say "load all the data about this" from just the PHID and have it hit all the same code. - Allow construction of Refs from ExternalObjects. This makes the "I have a PHID, I need a Ref" easier. Test Plan: - Verified Asana links still enrich properly at display time. - Used in future revision. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2852 Differential Revision: https://secure.phabricator.com/D6294 --- src/__phutil_library_map__.php | 2 + .../doorkeeper/bridge/DoorkeeperBridge.php | 3 + .../bridge/DoorkeeperBridgeAsana.php | 10 +++- .../engine/DoorkeeperImportEngine.php | 37 ++++++++++--- .../query/DoorkeeperExternalObjectQuery.php | 55 +++++++++++++++++++ .../storage/DoorkeeperExternalObject.php | 15 +++-- 6 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 70a92287dc..3c57f00fba 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -537,6 +537,7 @@ phutil_register_library_map(array( 'DoorkeeperBridgeAsana' => 'applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php', 'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php', 'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php', + 'DoorkeeperExternalObjectQuery' => 'applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php', 'DoorkeeperImportEngine' => 'applications/doorkeeper/engine/DoorkeeperImportEngine.php', 'DoorkeeperObjectRef' => 'applications/doorkeeper/engine/DoorkeeperObjectRef.php', 'DoorkeeperRemarkupRuleAsana' => 'applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php', @@ -2426,6 +2427,7 @@ phutil_register_library_map(array( 0 => 'DoorkeeperDAO', 1 => 'PhabricatorPolicyInterface', ), + 'DoorkeeperExternalObjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DoorkeeperImportEngine' => 'Phobject', 'DoorkeeperObjectRef' => 'Phobject', 'DoorkeeperRemarkupRuleAsana' => 'PhutilRemarkupRule', diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridge.php b/src/applications/doorkeeper/bridge/DoorkeeperBridge.php index 5a7459ab5a..5b23721d00 100644 --- a/src/applications/doorkeeper/bridge/DoorkeeperBridge.php +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridge.php @@ -20,4 +20,7 @@ abstract class DoorkeeperBridge extends Phobject { abstract public function canPullRef(DoorkeeperObjectRef $ref); abstract public function pullRefs(array $refs); + public function fillObjectFromData(DoorkeeperExternalObject $obj, $result) { + } + } diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php index 21ed1f5427..ead1ab7843 100644 --- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php @@ -77,9 +77,7 @@ final class DoorkeeperBridgeAsana extends DoorkeeperBridge { continue; } - $id = $result['id']; - $uri = "https://app.asana.com/0/{$id}/{$id}"; - $obj->setObjectURI($uri); + $this->fillObjectFromData($obj, $result); $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $obj->save(); @@ -87,4 +85,10 @@ final class DoorkeeperBridgeAsana extends DoorkeeperBridge { } } + public function fillObjectFromData(DoorkeeperExternalObject $obj, $result) { + $id = $result['id']; + $uri = "https://app.asana.com/0/{$id}/{$id}"; + $obj->setObjectURI($uri); + } + } diff --git a/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php b/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php index 8c069342e2..b93c19dbf0 100644 --- a/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php +++ b/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php @@ -3,7 +3,8 @@ final class DoorkeeperImportEngine extends Phobject { private $viewer; - private $refs; + private $refs = array(); + private $phids = array(); public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; @@ -24,26 +25,45 @@ final class DoorkeeperImportEngine extends Phobject { return $this->refs; } + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + public function execute() { $refs = $this->getRefs(); $viewer = $this->getViewer(); $keys = mpull($refs, 'getObjectKey'); if ($keys) { - $xobjs = id(new DoorkeeperExternalObject())->loadAllWhere( - 'objectKey IN (%Ls)', - $keys); + $xobjs = id(new DoorkeeperExternalObjectQuery()) + ->setViewer($viewer) + ->withObjectKeys($keys) + ->execute(); $xobjs = mpull($xobjs, null, 'getObjectKey'); foreach ($refs as $ref) { $xobj = idx($xobjs, $ref->getObjectKey()); if (!$xobj) { - $xobj = $ref->newExternalObject() + $xobj = $ref + ->newExternalObject() ->setImporterPHID($viewer->getPHID()); } $ref->attachExternalObject($xobj); } } + if ($this->phids) { + $xobjs = id(new DoorkeeperExternalObjectQuery()) + ->setViewer($viewer) + ->withPHIDs($this->phids) + ->execute(); + foreach ($xobjs as $xobj) { + $ref = $xobj->getRef(); + $ref->attachExternalObject($xobj); + $refs[$ref->getObjectKey()] = $ref; + } + } + $bridges = id(new PhutilSymbolLoader()) ->setAncestorClass('DoorkeeperBridge') ->loadObjects(); @@ -55,12 +75,13 @@ final class DoorkeeperImportEngine extends Phobject { $bridge->setViewer($viewer); } + $working_set = $refs; foreach ($bridges as $bridge) { $bridge_refs = array(); - foreach ($refs as $key => $ref) { + foreach ($working_set as $key => $ref) { if ($bridge->canPullRef($ref)) { $bridge_refs[$key] = $ref; - unset($refs[$key]); + unset($working_set[$key]); } } if ($bridge_refs) { @@ -68,7 +89,7 @@ final class DoorkeeperImportEngine extends Phobject { } } - return $this->getRefs(); + return $refs; } } diff --git a/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php b/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php new file mode 100644 index 0000000000..0246fdf210 --- /dev/null +++ b/src/applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php @@ -0,0 +1,55 @@ +phids = $phids; + return $this; + } + + public function withObjectKeys(array $keys) { + $this->objectKeys = $keys; + return $this; + } + + public 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); + } + + private function buildWhereClause(AphrontDatabaseConnection $conn_r) { + $where = array(); + + if ($this->phids) { + $where[] = qsprintf( + $conn_r, + 'phid IN (%Ls)', + $this->phids); + } + + if ($this->objectKeys) { + $where[] = qsprintf( + $conn_r, + 'objectKey IN (%Ls)', + $this->objectKeys); + } + + $where[] = $this->buildPagingClause($conn_r); + return $this->formatWhereClause($where); + } + +} diff --git a/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php b/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php index acdf040029..af46a3e543 100644 --- a/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php +++ b/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php @@ -39,16 +39,19 @@ final class DoorkeeperExternalObject extends DoorkeeperDAO public function getObjectKey() { $key = parent::getObjectKey(); if ($key === null) { - $key = id(new DoorkeeperObjectRef()) - ->setApplicationType($this->getApplicationType()) - ->setApplicationDomain($this->getApplicationDomain()) - ->setObjectType($this->getObjectType()) - ->setObjectID($this->getObjectID()) - ->getObjectKey(); + $key = $this->getRef()->getObjectKey(); } return $key; } + public function getRef() { + return id(new DoorkeeperObjectRef()) + ->setApplicationType($this->getApplicationType()) + ->setApplicationDomain($this->getApplicationDomain()) + ->setObjectType($this->getObjectType()) + ->setObjectID($this->getObjectID()); + } + public function save() { if (!$this->objectKey) { $this->objectKey = $this->getObjectKey();