From 9f4cfd40bcf5ba084372bf8da7de1cbab432d562 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 2 Jul 2012 15:42:06 -0700 Subject: [PATCH] Insert Maniphest transactions when edges are edited Summary: - See D2741. - When EdgeEditor performs edits, emit events. - Listen for Maniphest edge events and save the changes as transactions. - Do all this in a reasonably generic way that won't take too much rewriting as we use edges more generally. Test Plan: Attached and detached commits from tasks, saw reasonable-looking transactions spring into existence. Reviewers: btrahan, davidreuss Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D2906 --- src/__phutil_library_map__.php | 2 + .../constants/ManiphestTransactionType.php | 3 +- .../editor/ManiphestTransactionEditor.php | 7 + .../event/ManiphestEdgeEventListener.php | 145 ++++++++++++++++++ .../storage/ManiphestTransaction.php | 2 + .../PhabricatorSearchAttachController.php | 1 + .../edges/editor/PhabricatorEdgeEditor.php | 36 ++++- .../events/PhabricatorEventEngine.php | 2 + .../events/constant/PhabricatorEventType.php | 3 + 9 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 src/applications/maniphest/event/ManiphestEdgeEventListener.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 79e62bb652..ae8b69c4f6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -500,6 +500,7 @@ phutil_register_library_map(array( 'ManiphestController' => 'applications/maniphest/controller/ManiphestController.php', 'ManiphestDAO' => 'applications/maniphest/storage/ManiphestDAO.php', 'ManiphestDefaultTaskExtensions' => 'applications/maniphest/extensions/ManiphestDefaultTaskExtensions.php', + 'ManiphestEdgeEventListener' => 'applications/maniphest/event/ManiphestEdgeEventListener.php', 'ManiphestExportController' => 'applications/maniphest/controller/ManiphestExportController.php', 'ManiphestReplyHandler' => 'applications/maniphest/ManiphestReplyHandler.php', 'ManiphestReportController' => 'applications/maniphest/controller/ManiphestReportController.php', @@ -1527,6 +1528,7 @@ phutil_register_library_map(array( 'ManiphestController' => 'PhabricatorController', 'ManiphestDAO' => 'PhabricatorLiskDAO', 'ManiphestDefaultTaskExtensions' => 'ManiphestTaskExtensions', + 'ManiphestEdgeEventListener' => 'PhutilEventListener', 'ManiphestExportController' => 'ManiphestController', 'ManiphestReplyHandler' => 'PhabricatorMailReplyHandler', 'ManiphestReportController' => 'ManiphestController', diff --git a/src/applications/maniphest/constants/ManiphestTransactionType.php b/src/applications/maniphest/constants/ManiphestTransactionType.php index 1d1444b3c6..b8c623854b 100644 --- a/src/applications/maniphest/constants/ManiphestTransactionType.php +++ b/src/applications/maniphest/constants/ManiphestTransactionType.php @@ -1,7 +1,7 @@ getPriority(); break; + case ManiphestTransactionType::TYPE_EDGE: + $old = $transaction->getOldValue(); + break; case ManiphestTransactionType::TYPE_ATTACH: $old = $task->getAttached(); break; @@ -173,6 +176,10 @@ final class ManiphestTransactionEditor { $aux_key = $transaction->getMetadataValue('aux:key'); $task->setAuxiliaryAttribute($aux_key, $new); break; + case ManiphestTransactionType::TYPE_EDGE: + // Edge edits are accomplished through PhabricatorEdgeEditor, which + // has authority. + break; default: throw new Exception('Unknown action type.'); } diff --git a/src/applications/maniphest/event/ManiphestEdgeEventListener.php b/src/applications/maniphest/event/ManiphestEdgeEventListener.php new file mode 100644 index 0000000000..9ba10311ad --- /dev/null +++ b/src/applications/maniphest/event/ManiphestEdgeEventListener.php @@ -0,0 +1,145 @@ +listen(PhabricatorEventType::TYPE_EDGE_WILLEDITEDGES); + $this->listen(PhabricatorEventType::TYPE_EDGE_DIDEDITEDGES); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_EDGE_WILLEDITEDGES: + return $this->handleWillEditEvent($event); + case PhabricatorEventType::TYPE_EDGE_DIDEDITEDGES: + return $this->handleDidEditEvent($event); + } + } + + private function handleWillEditEvent(PhutilEvent $event) { + // NOTE: Everything is namespaced by `id` so that we aren't left in an + // inconsistent state if an edit fails to complete (e.g., something throws) + // or an edit happens inside another edit. + + $id = $event->getValue('id'); + + $edges = $this->loadAllEdges($event); + $tasks = array(); + if ($edges) { + $tasks = id(new ManiphestTask())->loadAllWhere( + 'phid IN (%Ls)', + array_keys($edges)); + $tasks = mpull($tasks, null, 'getPHID'); + } + + $this->edges[$id] = $edges; + $this->tasks[$id] = $tasks; + } + + private function handleDidEditEvent(PhutilEvent $event) { + $id = $event->getValue('id'); + + $old_edges = $this->edges[$id]; + $tasks = $this->tasks[$id]; + + unset($this->edges[$id]); + unset($this->tasks[$id]); + + $new_edges = $this->loadAllEdges($event); + $editor = new ManiphestTransactionEditor(); + + foreach ($tasks as $phid => $task) { + $xactions = array(); + + $old = $old_edges[$phid]; + $new = $new_edges[$phid]; + + $types = array_keys($old + $new); + foreach ($types as $type) { + $old_type = idx($old, $type, array()); + $new_type = idx($new, $type, array()); + + if ($old_type === $new_type) { + continue; + } + + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransactionType::TYPE_EDGE) + ->setOldValue($old_type) + ->setNewValue($new_type) + ->setMetadataValue('edge:type', $type) + ->setAuthorPHID($event->getUser()->getPHID()); + } + + if ($xactions) { + $editor->applyTransactions($task, $xactions); + } + } + } + + private function filterEdgesBySourceType(array $edges, $type) { + foreach ($edges as $key => $edge) { + if ($edge['src_type'] !== $type) { + unset($edges[$key]); + } + } + return $edges; + } + + private function loadAllEdges(PhutilEvent $event) { + $add_edges = $event->getValue('add'); + $rem_edges = $event->getValue('rem'); + + $type_task = PhabricatorPHIDConstants::PHID_TYPE_TASK; + + $all_edges = array_merge($add_edges, $rem_edges); + $all_edges = $this->filterEdgesBySourceType($all_edges, $type_task); + + if (!$all_edges) { + return; + } + + $all_tasks = array(); + $all_types = array(); + foreach ($all_edges as $edge) { + $all_tasks[$edge['src']] = true; + $all_types[$edge['type']] = true; + } + + $all_tasks = array_keys($all_tasks); + $all_types = array_keys($all_types); + + return id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs($all_tasks) + ->withEdgeTypes($all_types) + ->needEdgeData(true) + ->execute(); + } + +} diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index a82136c2cd..f16e17a262 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -58,6 +58,8 @@ final class ManiphestTransaction extends ManiphestDAO { $phids[] = $this->getOldValue(); $phids[] = $this->getNewValue(); break; + case ManiphestTransactionType::TYPE_EDGE: + return array_keys($this->getOldValue() + $this->getNewValue()); case ManiphestTransactionType::TYPE_ATTACH: $old = $this->getOldValue(); $new = $this->getNewValue(); diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php index 018e722206..670e2103ff 100644 --- a/src/applications/search/controller/PhabricatorSearchAttachController.php +++ b/src/applications/search/controller/PhabricatorSearchAttachController.php @@ -74,6 +74,7 @@ final class PhabricatorSearchAttachController $add_phids = $phids; $rem_phids = array_diff($old_phids, $add_phids); $editor = id(new PhabricatorEdgeEditor()); + $editor->setUser($user); foreach ($add_phids as $phid) { $editor->addEdge($this->phid, $edge_type, $phid); } diff --git a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php index 8eeccd6a2b..77653e89be 100644 --- a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php +++ b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php @@ -28,6 +28,7 @@ * * id(new PhabricatorEdgeEditor()) * ->addEdge($src, $type, $dst) + * ->setUser($user) * ->save(); * * @task edit Editing Edges @@ -38,6 +39,12 @@ final class PhabricatorEdgeEditor { private $addEdges = array(); private $remEdges = array(); private $openTransactions = array(); + private $user; + + public function setUser(PhabricatorUser $user) { + $this->user = $user; + return $this; + } /* -( Editing Edges )------------------------------------------------------ */ @@ -111,12 +118,20 @@ final class PhabricatorEdgeEditor { $this->writeEdgeData(); + static $id = 0; + $id++; + + $this->sendEvent($id, PhabricatorEventType::TYPE_EDGE_WILLEDITEDGES); + // NOTE: Removes first, then adds, so that "remove + add" is a useful // operation meaning "overwrite". $this->executeRemoves(); $this->executeAdds(); + $this->sendEvent($id, PhabricatorEventType::TYPE_EDGE_DIDEDITEDGES); + + $this->saveTransactions(); } @@ -136,11 +151,15 @@ final class PhabricatorEdgeEditor { $data['data'] = $options['data']; } + $src_type = phid_get_type($src); + $dst_type = phid_get_type($dst); + $specs = array(); $specs[] = array( 'src' => $src, - 'src_type' => phid_get_type($src), + 'src_type' => $src_type, 'dst' => $dst, + 'dst_type' => $dst_type, 'type' => $type, 'data' => $data, ); @@ -156,8 +175,9 @@ final class PhabricatorEdgeEditor { $specs[] = array( 'src' => $dst, - 'src_type' => phid_get_type($dst), + 'src_type' => $dst_type, 'dst' => $src, + 'dst_type' => $src_type, 'type' => $inverse, 'data' => $data, ); @@ -307,4 +327,16 @@ final class PhabricatorEdgeEditor { } } + private function sendEvent($edit_id, $event_type) { + $event = new PhabricatorEvent( + $event_type, + array( + 'id' => $edit_id, + 'add' => $this->addEdges, + 'rem' => $this->remEdges, + )); + $event->setUser($this->user); + PhutilEventEngine::dispatchEvent($event); + } + } diff --git a/src/infrastructure/events/PhabricatorEventEngine.php b/src/infrastructure/events/PhabricatorEventEngine.php index fa2ac53e2f..3d63cdd9be 100644 --- a/src/infrastructure/events/PhabricatorEventEngine.php +++ b/src/infrastructure/events/PhabricatorEventEngine.php @@ -26,6 +26,8 @@ final class PhabricatorEventEngine { // Register the DarkConosole event logger. id(new DarkConsoleEventPluginAPI())->register(); + id(new ManiphestEdgeEventListener())->register(); + } } diff --git a/src/infrastructure/events/constant/PhabricatorEventType.php b/src/infrastructure/events/constant/PhabricatorEventType.php index c2f96dbe24..dd7ef4eda9 100644 --- a/src/infrastructure/events/constant/PhabricatorEventType.php +++ b/src/infrastructure/events/constant/PhabricatorEventType.php @@ -23,4 +23,7 @@ final class PhabricatorEventType extends PhutilEventType { const TYPE_DIFFERENTIAL_WILLSENDMAIL = 'differential.willSendMail'; const TYPE_DIFFERENTIAL_WILLMARKGENERATED = 'differential.willMarkGenerated'; + const TYPE_EDGE_WILLEDITEDGES = 'edge.willEditEdges'; + const TYPE_EDGE_DIDEDITEDGES = 'edge.didEditEdges'; + }