@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Implement an "Attachments" behavior for Conduit Search APIs

Summary:
Ref T9964. We have various kinds of secondary data on objects (like subscribers, projects, paste content, Owners paths, file attachments, etc) which is somewhat slow, or somewhat large, or both.

Some approaches to handling this in the API include:

- Always return all of it (very easy, but slow).
- Require users to make separate API calls to get each piece of data (very simple, but inefficient and really cumbersome to use).
- Implement a hierarchical query language like GraphQL (powerful, but very complex).
- Kind of mix-and-match a half-power query language and some extra calls? (fairly simple, not too terrible?)

We currently mix-and-match internally, with `->needStuff(true)`. This is not a general-purpose, full-power graph query language like GraphQL, and it occasionally does limit us.

For example, there is no way to do this sort of thing:

$conpherence_thread_query = id(new ConpherenceThreadQuery())
->setViewer($viewer)
// ...
->setNeedMessages(true)
->setWhenYouLoadTheMessagesTheyNeedProfilePictures(true);

However, we almost never actually need to do this and when we do want to do it we usually don't //really// want to do it, so I don't think this is a major limit to the practical power of the system for the kinds of things we really want to do with it.

Put another way, we have a lot of 1-level hierarchical queries (get pictures or repositories or projects or files or content for these objects) but few-to-no 2+ level queries (get files for these objects, then get all the projects for those files).

So even though 1-level hierarchies are not a beautiful, general-purpose, fully-abstract system, they've worked well so far in practice and I'm comfortable moving forward with them in the API.

If we do need N-level queries in the future, there is no technical reason we can't put GraphQL (or something similar) on top of this eventually, and this would represent a solid step toward that. However, I suspect we'll never need them.

Upshot: I'm pretty happy with "->needX()" for all practical purposes, so this is just adding a way to say "->needX()" to the API.

Specifically, you say:

```
{
"attachments": {
"subscribers": true,
}
}
```

...and get back subscriber data. In the future (or for certain attachments), `true` might become a dictionary of extra parameters, if necessary, and could do so without breaking the API.

Test Plan:
- Ran queries to get attachments.

{F1025449}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14772

+308 -19
+4
src/__phutil_library_map__.php
··· 3004 3004 'PhabricatorSearchEditController' => 'applications/search/controller/PhabricatorSearchEditController.php', 3005 3005 'PhabricatorSearchEngine' => 'applications/search/engine/PhabricatorSearchEngine.php', 3006 3006 'PhabricatorSearchEngineAPIMethod' => 'applications/search/engine/PhabricatorSearchEngineAPIMethod.php', 3007 + 'PhabricatorSearchEngineAttachment' => 'applications/search/engineextension/PhabricatorSearchEngineAttachment.php', 3007 3008 'PhabricatorSearchEngineExtension' => 'applications/search/engineextension/PhabricatorSearchEngineExtension.php', 3008 3009 'PhabricatorSearchEngineExtensionModule' => 'applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php', 3009 3010 'PhabricatorSearchEngineTestCase' => 'applications/search/engine/__tests__/PhabricatorSearchEngineTestCase.php', ··· 3149 3150 'PhabricatorSubscriptionsListController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsListController.php', 3150 3151 'PhabricatorSubscriptionsRemoveSelfHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php', 3151 3152 'PhabricatorSubscriptionsRemoveSubscribersHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSubscribersHeraldAction.php', 3153 + 'PhabricatorSubscriptionsSearchEngineAttachment' => 'applications/subscriptions/engineextension/PhabricatorSubscriptionsSearchEngineAttachment.php', 3152 3154 'PhabricatorSubscriptionsSearchEngineExtension' => 'applications/subscriptions/engineextension/PhabricatorSubscriptionsSearchEngineExtension.php', 3153 3155 'PhabricatorSubscriptionsSubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsSubscribeEmailCommand.php', 3154 3156 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php', ··· 7321 7323 'PhabricatorSearchEditController' => 'PhabricatorSearchBaseController', 7322 7324 'PhabricatorSearchEngine' => 'Phobject', 7323 7325 'PhabricatorSearchEngineAPIMethod' => 'ConduitAPIMethod', 7326 + 'PhabricatorSearchEngineAttachment' => 'Phobject', 7324 7327 'PhabricatorSearchEngineExtension' => 'Phobject', 7325 7328 'PhabricatorSearchEngineExtensionModule' => 'PhabricatorConfigModule', 7326 7329 'PhabricatorSearchEngineTestCase' => 'PhabricatorTestCase', ··· 7482 7485 'PhabricatorSubscriptionsListController' => 'PhabricatorController', 7483 7486 'PhabricatorSubscriptionsRemoveSelfHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', 7484 7487 'PhabricatorSubscriptionsRemoveSubscribersHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', 7488 + 'PhabricatorSubscriptionsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', 7485 7489 'PhabricatorSubscriptionsSearchEngineExtension' => 'PhabricatorSearchEngineExtension', 7486 7490 'PhabricatorSubscriptionsSubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand', 7487 7491 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'PhabricatorPolicyRule',
+68 -17
src/applications/search/engine/PhabricatorApplicationSearchEngine.php
··· 1123 1123 1124 1124 $this->saveQuery($saved_query); 1125 1125 1126 - 1127 1126 $query = $this->buildQueryFromSavedQuery($saved_query); 1128 1127 $pager = $this->newPagerForSavedQuery($saved_query); 1129 1128 1129 + $attachments = $this->getConduitSearchAttachments(); 1130 + 1131 + // TODO: Validate this better. 1132 + $attachment_specs = $request->getValue('attachments'); 1133 + $attachments = array_select_keys( 1134 + $attachments, 1135 + array_keys($attachment_specs)); 1136 + 1137 + foreach ($attachments as $key => $attachment) { 1138 + $attachment->setViewer($viewer); 1139 + } 1140 + 1141 + foreach ($attachments as $key => $attachment) { 1142 + $attachment->willLoadAttachmentData($query, $attachment_specs[$key]); 1143 + } 1144 + 1130 1145 $this->setQueryOrderForConduit($query, $request); 1131 1146 $this->setPagerLimitForConduit($pager, $request); 1132 1147 $this->setPagerOffsetsForConduit($pager, $request); ··· 1137 1152 if ($objects) { 1138 1153 $field_extensions = $this->getConduitFieldExtensions(); 1139 1154 1155 + $attachment_data = array(); 1156 + foreach ($attachments as $key => $attachment) { 1157 + $attachment_data[$key] = $attachment->loadAttachmentData( 1158 + $objects, 1159 + $attachment_specs[$key]); 1160 + } 1161 + 1140 1162 foreach ($objects as $object) { 1141 - $data[] = $this->getObjectWireFormatForConduit( 1163 + $field_map = $this->getObjectWireFieldsForConduit( 1142 1164 $object, 1143 1165 $field_extensions); 1166 + 1167 + $attachment_map = array(); 1168 + foreach ($attachments as $key => $attachment) { 1169 + $attachment_map[$key] = $attachment->getAttachmentForObject( 1170 + $object, 1171 + $attachment_data[$key], 1172 + $attachment_specs[$key]); 1173 + } 1174 + 1175 + $id = (int)$object->getID(); 1176 + $phid = $object->getPHID(); 1177 + 1178 + $data[] = array( 1179 + 'id' => $id, 1180 + 'type' => phid_get_type($phid), 1181 + 'phid' => $phid, 1182 + 'fields' => $field_map, 1183 + 'attachments' => $attachment_map, 1184 + ); 1144 1185 } 1145 1186 } 1146 1187 ··· 1264 1305 } 1265 1306 } 1266 1307 1267 - protected function getObjectWireFormatForConduit( 1268 - $object, 1269 - array $field_extensions) { 1270 - $phid = $object->getPHID(); 1271 - 1272 - return array( 1273 - 'id' => (int)$object->getID(), 1274 - 'type' => phid_get_type($phid), 1275 - 'phid' => $phid, 1276 - 'fields' => $this->getObjectWireFieldsForConduit( 1277 - $object, 1278 - $field_extensions), 1279 - ); 1280 - } 1281 - 1282 1308 protected function getObjectWireFieldsForConduit( 1283 1309 $object, 1284 1310 array $field_extensions) { ··· 1289 1315 } 1290 1316 1291 1317 return $fields; 1318 + } 1319 + 1320 + public function getConduitSearchAttachments() { 1321 + $extensions = $this->getEngineExtensions(); 1322 + 1323 + $attachments = array(); 1324 + foreach ($extensions as $extension) { 1325 + $extension_attachments = $extension->getSearchAttachments(); 1326 + foreach ($extension_attachments as $attachment) { 1327 + $attachment_key = $attachment->getAttachmentKey(); 1328 + if (isset($attachments[$attachment_key])) { 1329 + $other = $attachments[$attachment_key]; 1330 + throw new Exception( 1331 + pht( 1332 + 'Two search engine attachments (of classes "%s" and "%s") '. 1333 + 'specify the same attachment key ("%s"); keys must be unique.', 1334 + get_class($attachment), 1335 + get_class($other), 1336 + $attachment_key)); 1337 + } 1338 + $attachments[$attachment_key] = $attachment; 1339 + } 1340 + } 1341 + 1342 + return $attachments; 1292 1343 } 1293 1344 1294 1345 }
+90
src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php
··· 23 23 return array( 24 24 'queryKey' => 'optional string', 25 25 'constraints' => 'optional map<string, wild>', 26 + 'attachments' => 'optional map<string, bool>', 26 27 'order' => 'optional order', 27 28 ) + $this->getPagerParamTypes(); 28 29 } ··· 58 59 $out[] = $this->buildConstraintsBox($engine); 59 60 $out[] = $this->buildOrderBox($engine, $query); 60 61 $out[] = $this->buildFieldsBox($engine); 62 + $out[] = $this->buildAttachmentsBox($engine); 61 63 $out[] = $this->buildPagingBox($engine); 62 64 63 65 return $out; ··· 396 398 397 399 return id(new PHUIObjectBoxView()) 398 400 ->setHeaderText(pht('Object Fields')) 401 + ->setCollapsed(true) 402 + ->appendChild($this->buildRemarkup($info)) 403 + ->appendChild($table); 404 + } 405 + 406 + private function buildAttachmentsBox( 407 + PhabricatorApplicationSearchEngine $engine) { 408 + 409 + $info = pht(<<<EOTEXT 410 + By default, only basic information about objects is returned. If you want 411 + more extensive information, you can use available `attachments` to get more 412 + information in the results (like subscribers and projects). 413 + 414 + Generally, requesting more information means the query executes more slowly 415 + and returns more data (in some cases, much more data). You should normally 416 + request only the data you need. 417 + 418 + To request extra data, specify which attachments you want in the `attachments` 419 + parameter: 420 + 421 + ```lang=json, name="Example Attachments Request" 422 + { 423 + ... 424 + "attachments": { 425 + "subscribers": true 426 + }, 427 + ... 428 + } 429 + ``` 430 + 431 + This example specifies that results should include information about 432 + subscribers. In the return value, each object will now have this information 433 + filled out in the corresponding `attachments` value: 434 + 435 + ```lang=json, name="Example Attachments Result" 436 + { 437 + ... 438 + "data": [ 439 + { 440 + ... 441 + "attachments": { 442 + "subscribers": { 443 + "subscriberPHIDs": [ 444 + "PHID-WXYZ-2222", 445 + ], 446 + "subscriberCount": 1, 447 + "viewerIsSubscribed": false 448 + } 449 + }, 450 + ... 451 + }, 452 + ... 453 + ], 454 + ... 455 + } 456 + ``` 457 + 458 + These attachments are available: 459 + EOTEXT 460 + ); 461 + 462 + $attachments = $engine->getConduitSearchAttachments(); 463 + 464 + $rows = array(); 465 + foreach ($attachments as $key => $attachment) { 466 + $rows[] = array( 467 + $key, 468 + $attachment->getAttachmentName(), 469 + $attachment->getAttachmentDescription(), 470 + ); 471 + } 472 + 473 + $table = id(new AphrontTableView($rows)) 474 + ->setHeaders( 475 + array( 476 + pht('Key'), 477 + pht('Name'), 478 + pht('Description'), 479 + )) 480 + ->setColumnClasses( 481 + array( 482 + 'prewrap', 483 + 'pri', 484 + 'wide', 485 + )); 486 + 487 + return id(new PHUIObjectBoxView()) 488 + ->setHeaderText(pht('Attachments')) 399 489 ->setCollapsed(true) 400 490 ->appendChild($this->buildRemarkup($info)) 401 491 ->appendChild($table);
+50
src/applications/search/engineextension/PhabricatorSearchEngineAttachment.php
··· 1 + <?php 2 + 3 + abstract class PhabricatorSearchEngineAttachment extends Phobject { 4 + 5 + private $attachmentKey; 6 + private $viewer; 7 + private $searchEngine; 8 + 9 + final public function setViewer($viewer) { 10 + $this->viewer = $viewer; 11 + return $this; 12 + } 13 + 14 + final public function getViewer() { 15 + return $this->viewer; 16 + } 17 + 18 + final public function setSearchEngine( 19 + PhabricatorApplicationSearchEngine $engine) { 20 + $this->searchEngine = $engine; 21 + return $this; 22 + } 23 + 24 + final public function getSearchEngine() { 25 + return $this->searchEngine; 26 + } 27 + 28 + public function setAttachmentKey($attachment_key) { 29 + $this->attachmentKey = $attachment_key; 30 + return $this; 31 + } 32 + 33 + public function getAttachmentKey() { 34 + return $this->attachmentKey; 35 + } 36 + 37 + abstract public function getAttachmentName(); 38 + abstract public function getAttachmentDescription(); 39 + 40 + public function willLoadAttachmentData($query, $spec) { 41 + return; 42 + } 43 + 44 + public function loadAttachmentData(array $objects, $spec) { 45 + return null; 46 + } 47 + 48 + abstract public function getAttachmentForObject($object, $data, $spec); 49 + 50 + }
+4
src/applications/search/engineextension/PhabricatorSearchEngineExtension.php
··· 40 40 return array(); 41 41 } 42 42 43 + public function getSearchAttachments() { 44 + return array(); 45 + } 46 + 43 47 public function applyConstraintsToQuery( 44 48 $object, 45 49 $query,
+84
src/applications/subscriptions/engineextension/PhabricatorSubscriptionsSearchEngineAttachment.php
··· 1 + <?php 2 + 3 + final class PhabricatorSubscriptionsSearchEngineAttachment 4 + extends PhabricatorSearchEngineAttachment { 5 + 6 + public function getAttachmentName() { 7 + return pht('Subscribers'); 8 + } 9 + 10 + public function getAttachmentDescription() { 11 + return pht('Get information about subscribers.'); 12 + } 13 + 14 + public function loadAttachmentData(array $objects, $spec) { 15 + $object_phids = mpull($objects, 'getPHID'); 16 + $edge_type = PhabricatorObjectHasSubscriberEdgeType::EDGECONST; 17 + 18 + 19 + $subscribers_query = id(new PhabricatorEdgeQuery()) 20 + ->withSourcePHIDs($object_phids) 21 + ->withEdgeTypes(array($edge_type)); 22 + $subscribers_query->execute(); 23 + 24 + $viewer = $this->getViewer(); 25 + $viewer_phid = $viewer->getPHID(); 26 + if ($viewer) { 27 + $edges = id(new PhabricatorEdgeQuery()) 28 + ->withSourcePHIDs($object_phids) 29 + ->withEdgeTypes(array($edge_type)) 30 + ->withDestinationPHIDs(array($viewer_phid)) 31 + ->execute(); 32 + 33 + $viewer_map = array(); 34 + foreach ($edges as $object_phid => $types) { 35 + if ($types[$edge_type]) { 36 + $viewer_map[$object_phid] = true; 37 + } 38 + } 39 + } else { 40 + $viewer_map = array(); 41 + } 42 + 43 + return array( 44 + 'subscribers.query' => $subscribers_query, 45 + 'viewer.map' => $viewer_map, 46 + ); 47 + } 48 + 49 + public function getAttachmentForObject($object, $data, $spec) { 50 + $subscribers_query = idx($data, 'subscribers.query'); 51 + $viewer_map = idx($data, 'viewer.map'); 52 + $object_phid = $object->getPHID(); 53 + 54 + $subscribed_phids = $subscribers_query->getDestinationPHIDs( 55 + array($object_phid), 56 + array(PhabricatorObjectHasSubscriberEdgeType::EDGECONST)); 57 + $subscribed_count = count($subscribed_phids); 58 + if ($subscribed_count > 10) { 59 + $subscribed_phids = array_slice($subscribed_phids, 0, 10); 60 + } 61 + 62 + $subscribed_phids = array_values($subscribed_phids); 63 + 64 + $viewer = $this->getViewer(); 65 + $viewer_phid = $viewer->getPHID(); 66 + 67 + if (!$viewer_phid) { 68 + $self_subscribed = false; 69 + } else if (isset($viewer_map[$object_phid])) { 70 + $self_subscribed = true; 71 + } else if ($object->isAutomaticallySubscribed($viewer_phid)) { 72 + $self_subscribed = true; 73 + } else { 74 + $self_subscribed = false; 75 + } 76 + 77 + return array( 78 + 'subscriberPHIDs' => $subscribed_phids, 79 + 'subscriberCount' => $subscribed_count, 80 + 'viewerIsSubscribed' => $self_subscribed, 81 + ); 82 + } 83 + 84 + }
+6
src/applications/subscriptions/engineextension/PhabricatorSubscriptionsSearchEngineExtension.php
··· 50 50 return $fields; 51 51 } 52 52 53 + public function getSearchAttachments() { 54 + return array( 55 + id(new PhabricatorSubscriptionsSearchEngineAttachment()) 56 + ->setAttachmentKey('subscribers'), 57 + ); 58 + } 53 59 54 60 }
+2 -2
src/applications/transactions/editfield/PhabricatorEditField.php
··· 516 516 $edit_type = $this->getEditType(); 517 517 518 518 if ($edit_type === null) { 519 - return null; 519 + return array(); 520 520 } 521 521 522 522 return array($edit_type); ··· 526 526 $edit_type = $this->getEditType(); 527 527 528 528 if ($edit_type === null) { 529 - return null; 529 + return array(); 530 530 } 531 531 532 532 return array($edit_type);