From 41d28abfcc46f8c139046fd909e148ec23d95135 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 9 Feb 2018 11:48:41 -0800 Subject: [PATCH] Trigger all "Firehose" webhooks on all transactional edits Summary: Depends on D19047. Ref T11330. Triggers every firehose hook on every edit; prepares for Herald triggers. Test Plan: Configured a firehose hook, edited some objects, saw callbacks. Maniphest Tasks: T11330 Differential Revision: https://secure.phabricator.com/D19048 --- .../HeraldWebhookTestController.php | 1 + .../herald/editor/HeraldWebhookEditor.php | 9 +++ .../HeraldWebhookCallManagementWorkflow.php | 3 + .../phid/HeraldWebhookRequestPHIDType.php | 3 +- .../herald/storage/HeraldWebhookRequest.php | 8 +++ .../herald/worker/HeraldWebhookWorker.php | 10 +++- ...habricatorApplicationTransactionEditor.php | 57 +++++++++++++++++++ 7 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/applications/herald/controller/HeraldWebhookTestController.php b/src/applications/herald/controller/HeraldWebhookTestController.php index b791ba783b..510e7f3f81 100644 --- a/src/applications/herald/controller/HeraldWebhookTestController.php +++ b/src/applications/herald/controller/HeraldWebhookTestController.php @@ -52,6 +52,7 @@ final class HeraldWebhookTestController $request = HeraldWebhookRequest::initializeNewWebhookRequest($hook) ->setObjectPHID($object->getPHID()) + ->setTriggerPHIDs(array($viewer->getPHID())) ->setIsTestAction(true) ->setTransactionPHIDs(mpull($xactions, 'getPHID')) ->save(); diff --git a/src/applications/herald/editor/HeraldWebhookEditor.php b/src/applications/herald/editor/HeraldWebhookEditor.php index 2206a59411..1f138e5028 100644 --- a/src/applications/herald/editor/HeraldWebhookEditor.php +++ b/src/applications/herald/editor/HeraldWebhookEditor.php @@ -19,4 +19,13 @@ final class HeraldWebhookEditor return pht('%s created %s.', $author, $object); } + public function getTransactionTypes() { + $types = parent::getTransactionTypes(); + + $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; + $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; + + return $types; + } + } diff --git a/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php b/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php index abc9a40e17..10249cca68 100644 --- a/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php +++ b/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php @@ -78,11 +78,14 @@ final class HeraldWebhookCallManagementWorkflow ->setLimit(10) ->execute(); + $application_phid = id(new PhabricatorHeraldApplication())->getPHID(); + $request = HeraldWebhookRequest::initializeNewWebhookRequest($hook) ->setObjectPHID($object->getPHID()) ->setIsTestAction(true) ->setIsSilentAction((bool)$args->getArg('silent')) ->setIsSecureAction((bool)$args->getArg('secure')) + ->setTriggerPHIDs(array($application_phid)) ->setTransactionPHIDs(mpull($xactions, 'getPHID')) ->save(); diff --git a/src/applications/herald/phid/HeraldWebhookRequestPHIDType.php b/src/applications/herald/phid/HeraldWebhookRequestPHIDType.php index 8263ba7484..bcf5afb0d3 100644 --- a/src/applications/herald/phid/HeraldWebhookRequestPHIDType.php +++ b/src/applications/herald/phid/HeraldWebhookRequestPHIDType.php @@ -31,8 +31,7 @@ final class HeraldWebhookRequestPHIDType extends PhabricatorPHIDType { foreach ($handles as $phid => $handle) { $request = $objects[$phid]; - - // TODO: Fill this in. + $handle->setName(pht('Webhook Request %d', $request->getID())); } } diff --git a/src/applications/herald/storage/HeraldWebhookRequest.php b/src/applications/herald/storage/HeraldWebhookRequest.php index d6e1364aaa..5db5b2916e 100644 --- a/src/applications/herald/storage/HeraldWebhookRequest.php +++ b/src/applications/herald/storage/HeraldWebhookRequest.php @@ -116,6 +116,14 @@ final class HeraldWebhookRequest return $this->getProperty('transactionPHIDs', array()); } + public function setTriggerPHIDs(array $phids) { + return $this->setProperty('triggerPHIDs', $phids); + } + + public function getTriggerPHIDs() { + return $this->getProperty('triggerPHIDs', array()); + } + public function setIsSilentAction($bool) { return $this->setProperty('silent', $bool); } diff --git a/src/applications/herald/worker/HeraldWebhookWorker.php b/src/applications/herald/worker/HeraldWebhookWorker.php index fdf4bf7dc1..f74268c694 100644 --- a/src/applications/herald/worker/HeraldWebhookWorker.php +++ b/src/applications/herald/worker/HeraldWebhookWorker.php @@ -138,11 +138,19 @@ final class HeraldWebhookWorker ); } + $trigger_data = array(); + foreach ($request->getTriggerPHIDs() as $trigger_phid) { + $trigger_data[] = array( + 'phid' => $trigger_phid, + ); + } + $payload = array( - 'triggers' => array(), 'object' => array( + 'type' => phid_get_type($object->getPHID()), 'phid' => $object->getPHID(), ), + 'triggers' => $trigger_data, 'action' => array( 'test' => $request->getIsTestAction(), 'silent' => $request->getIsSilentAction(), diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 5e6a311a85..09c6af62b7 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -79,6 +79,7 @@ abstract class PhabricatorApplicationTransactionEditor private $mailRemovedPHIDs = array(); private $mailUnexpandablePHIDs = array(); private $mailMutedPHIDs = array(); + private $webhookMap = array(); private $transactionQueue = array(); @@ -1307,6 +1308,8 @@ abstract class PhabricatorApplicationTransactionEditor $mail->save(); } + $this->queueWebhooks($object, $xactions); + return $xactions; } @@ -3660,6 +3663,8 @@ abstract class PhabricatorApplicationTransactionEditor 'mailStamps', 'mailUnexpandablePHIDs', 'mailMutedPHIDs', + 'webhookMap', + 'silent', ); } @@ -4240,4 +4245,56 @@ abstract class PhabricatorApplicationTransactionEditor return $this; } + private function queueWebhooks($object, array $xactions) { + $hook_viewer = PhabricatorUser::getOmnipotentUser(); + + $webhook_map = $this->webhookMap; + if (!is_array($webhook_map)) { + $webhook_map = array(); + } + + // Add any "Firehose" hooks to the list of hooks we're going to call. + $firehose_hooks = id(new HeraldWebhookQuery()) + ->setViewer($hook_viewer) + ->withStatuses( + array( + HeraldWebhook::HOOKSTATUS_FIREHOSE, + )) + ->execute(); + foreach ($firehose_hooks as $firehose_hook) { + // This is "the hook itself is the reason this hook is being called", + // since we're including it because it's configured as a firehose + // hook. + $hook_phid = $firehose_hook->getPHID(); + $webhook_map[$hook_phid][] = $hook_phid; + } + + if (!$webhook_map) { + return; + } + + // NOTE: We're going to queue calls to disabled webhooks, they'll just + // immediately fail in the worker queue. This makes the behavior more + // visible. + + $call_hooks = id(new HeraldWebhookQuery()) + ->setViewer($hook_viewer) + ->withPHIDs(array_keys($webhook_map)) + ->execute(); + + foreach ($call_hooks as $call_hook) { + $trigger_phids = idx($webhook_map, $call_hook->getPHID()); + + $request = HeraldWebhookRequest::initializeNewWebhookRequest($call_hook) + ->setObjectPHID($object->getPHID()) + ->setTransactionPHIDs(mpull($xactions, 'getPHID')) + ->setTriggerPHIDs($trigger_phids) + ->setIsSilentAction((bool)$this->getIsSilent()) + ->setIsSecureAction((bool)$this->getMustEncrypt()) + ->save(); + + $request->queueCall(); + } + } + }