From d4b2bfa2f4c1354661c607af71cb794ed03632e5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 17 Jul 2014 15:42:06 -0700 Subject: [PATCH] Modernize commit/edge transaction when parsing commit messages Summary: Ref T5245. With work elsewhere (notably, D9839) we can remove this TODO and use real transactions. Test Plan: Pushed a `closes Txxx` commit and got a close + transaction. Reviewers: chad, btrahan, joshuaspence Reviewed By: joshuaspence Subscribers: epriestley Maniphest Tasks: T5245 Differential Revision: https://secure.phabricator.com/D9848 --- src/__phutil_library_map__.php | 4 + ...ConduitAPI_diffusion_getcommits_Method.php | 2 +- .../controller/DiffusionCommitController.php | 4 +- .../edge/DiffusionCommitHasTaskEdgeType.php | 103 ++++++++++++++++++ .../ManiphestTaskDetailController.php | 4 +- .../edge/ManiphestTaskHasCommitEdgeType.php | 103 ++++++++++++++++++ ...torRepositoryCommitMessageParserWorker.php | 16 +-- .../PhabricatorSearchAttachController.php | 4 +- .../edges/constants/PhabricatorEdgeConfig.php | 14 --- 9 files changed, 219 insertions(+), 35 deletions(-) create mode 100644 src/applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php create mode 100644 src/applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f4171d374f..e0ccee3f2d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -486,6 +486,7 @@ phutil_register_library_map(array( 'DiffusionCommitChangeTableView' => 'applications/diffusion/view/DiffusionCommitChangeTableView.php', 'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php', 'DiffusionCommitEditController' => 'applications/diffusion/controller/DiffusionCommitEditController.php', + 'DiffusionCommitHasTaskEdgeType' => 'applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php', 'DiffusionCommitHash' => 'applications/diffusion/data/DiffusionCommitHash.php', 'DiffusionCommitHookEngine' => 'applications/diffusion/engine/DiffusionCommitHookEngine.php', 'DiffusionCommitHookRejectException' => 'applications/diffusion/exception/DiffusionCommitHookRejectException.php', @@ -945,6 +946,7 @@ phutil_register_library_map(array( 'ManiphestTaskDescriptionPreviewController' => 'applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php', 'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php', 'ManiphestTaskEditController' => 'applications/maniphest/controller/ManiphestTaskEditController.php', + 'ManiphestTaskHasCommitEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php', 'ManiphestTaskHasRevisionEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php', 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', 'ManiphestTaskListView' => 'applications/maniphest/view/ManiphestTaskListView.php', @@ -3199,6 +3201,7 @@ phutil_register_library_map(array( 'DiffusionCommitChangeTableView' => 'DiffusionView', 'DiffusionCommitController' => 'DiffusionController', 'DiffusionCommitEditController' => 'DiffusionController', + 'DiffusionCommitHasTaskEdgeType' => 'PhabricatorEdgeType', 'DiffusionCommitHash' => 'Phobject', 'DiffusionCommitHookEngine' => 'Phobject', 'DiffusionCommitHookRejectException' => 'Exception', @@ -3703,6 +3706,7 @@ phutil_register_library_map(array( 'ManiphestTaskDescriptionPreviewController' => 'ManiphestController', 'ManiphestTaskDetailController' => 'ManiphestController', 'ManiphestTaskEditController' => 'ManiphestController', + 'ManiphestTaskHasCommitEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasRevisionEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListView' => 'ManiphestView', diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php index 46139ba66b..05a6a33dcf 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php @@ -269,7 +269,7 @@ final class ConduitAPI_diffusion_getcommits_Method * Enhances the commits list with Maniphest information. */ private function addManiphestInformation(array $commits) { - $task_type = PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK; + $task_type = DiffusionCommitHasTaskEdgeType::EDGECONST; $commit_phids = ipull($commits, 'commitPHID'); diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index c7dfdfd652..6716f0e033 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -420,7 +420,7 @@ final class DiffusionCommitController extends DiffusionController { $edge_query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs(array($commit_phid)) ->withEdgeTypes(array( - PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK, + DiffusionCommitHasTaskEdgeType::EDGECONST, PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT, PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV, )); @@ -428,7 +428,7 @@ final class DiffusionCommitController extends DiffusionController { $edges = $edge_query->execute(); $task_phids = array_keys( - $edges[$commit_phid][PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK]); + $edges[$commit_phid][DiffusionCommitHasTaskEdgeType::EDGECONST]); $proj_phids = array_keys( $edges[$commit_phid][PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT]); $revision_phid = key( diff --git a/src/applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php b/src/applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php new file mode 100644 index 0000000000..516fcdf023 --- /dev/null +++ b/src/applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php @@ -0,0 +1,103 @@ +setViewer($user) ->readFieldsFromStorage($task); - $e_commit = PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT; + $e_commit = ManiphestTaskHasCommitEdgeType::EDGECONST; $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK; $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK; $e_rev = ManiphestTaskHasRevisionEdgeType::EDGECONST; @@ -603,7 +603,7 @@ final class ManiphestTaskDetailController extends ManiphestController { $handles = $this->getLoadedHandles(); $commit_phids = array_keys( - $edges[PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT]); + $edges[ManiphestTaskHasCommitEdgeType::EDGECONST]); if ($commit_phids) { $commit_drev = PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV; $drev_edges = id(new PhabricatorEdgeQuery()) diff --git a/src/applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php b/src/applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php new file mode 100644 index 0000000000..a9842f8ff1 --- /dev/null +++ b/src/applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php @@ -0,0 +1,103 @@ + $task) { $xactions = array(); - // TODO: Swap this for a real edge transaction once the weirdness in - // Maniphest edges is sorted out. Currently, Maniphest reacts to an edge - // edit on this edge. - id(new PhabricatorEdgeEditor()) - ->addEdge( - $task->getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT, - $commit->getPHID()) - ->save(); - - /* TODO: Do this instead of the above. - + $edge_type = ManiphestTaskHasCommitEdgeType::EDGECONST; $xactions[] = id(new ManiphestTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue('edge:type', $edge_task_has_commit) + ->setMetadataValue('edge:type', $edge_type) ->setNewValue( array( '+' => array( $commit->getPHID() => $commit->getPHID(), ), )); - */ $status = $task_statuses[$task_id]; if ($status) { diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php index 0218e8658f..b6519a3e0b 100644 --- a/src/applications/search/controller/PhabricatorSearchAttachController.php +++ b/src/applications/search/controller/PhabricatorSearchAttachController.php @@ -290,10 +290,10 @@ final class PhabricatorSearchAttachController $map = array( $t_cmit => array( - $t_task => PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK, + $t_task => DiffusionCommitHasTaskEdgeType::EDGECONST, ), $t_task => array( - $t_cmit => PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT, + $t_cmit => ManiphestTaskHasCommitEdgeType::EDGECONST, $t_task => PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, $t_drev => ManiphestTaskHasRevisionEdgeType::EDGECONST, $t_mock => PhabricatorEdgeConfig::TYPE_TASK_HAS_MOCK, diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php index bfff37a813..8bcd37efb0 100644 --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -5,9 +5,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { const TABLE_NAME_EDGE = 'edge'; const TABLE_NAME_EDGEDATA = 'edgedata'; - const TYPE_TASK_HAS_COMMIT = 1; - const TYPE_COMMIT_HAS_TASK = 2; - const TYPE_TASK_DEPENDS_ON_TASK = 3; const TYPE_TASK_DEPENDED_ON_BY_TASK = 4; @@ -136,9 +133,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { private static function getInverse($edge_type) { static $map = array( - self::TYPE_TASK_HAS_COMMIT => self::TYPE_COMMIT_HAS_TASK, - self::TYPE_COMMIT_HAS_TASK => self::TYPE_TASK_HAS_COMMIT, - self::TYPE_TASK_DEPENDS_ON_TASK => self::TYPE_TASK_DEPENDED_ON_BY_TASK, self::TYPE_TASK_DEPENDED_ON_BY_TASK => self::TYPE_TASK_DEPENDS_ON_TASK, @@ -259,11 +253,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { public static function getEditStringForEdgeType($type) { switch ($type) { - case self::TYPE_TASK_HAS_COMMIT: case self::TYPE_PROJECT_HAS_COMMIT: case self::TYPE_DREV_HAS_COMMIT: return '%s edited commit(s), added %d: %s; removed %d: %s.'; - case self::TYPE_COMMIT_HAS_TASK: case self::TYPE_TASK_DEPENDS_ON_TASK: case self::TYPE_TASK_DEPENDED_ON_BY_TASK: case self::TYPE_MOCK_HAS_TASK: @@ -335,7 +327,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { public static function getAddStringForEdgeType($type) { switch ($type) { - case self::TYPE_TASK_HAS_COMMIT: case self::TYPE_PROJECT_HAS_COMMIT: case self::TYPE_DREV_HAS_COMMIT: return '%s added %d commit(s): %s.'; @@ -345,7 +336,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { return '%s added %d dependencie(s): %s.'; case self::TYPE_TASK_DEPENDED_ON_BY_TASK: return '%s added %d blocked task(s): %s.'; - case self::TYPE_COMMIT_HAS_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s added %d task(s): %s.'; case self::TYPE_DREV_DEPENDED_ON_BY_DREV: @@ -413,7 +403,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { public static function getRemoveStringForEdgeType($type) { switch ($type) { - case self::TYPE_TASK_HAS_COMMIT: case self::TYPE_PROJECT_HAS_COMMIT: case self::TYPE_DREV_HAS_COMMIT: return '%s removed %d commit(s): %s.'; @@ -421,7 +410,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { return '%s removed %d blocking task(s): %s.'; case self::TYPE_TASK_DEPENDED_ON_BY_TASK: return '%s removed %d blocked task(s): %s.'; - case self::TYPE_COMMIT_HAS_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s removed %d task(s): %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: @@ -488,11 +476,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { public static function getFeedStringForEdgeType($type) { switch ($type) { - case self::TYPE_TASK_HAS_COMMIT: case self::TYPE_PROJECT_HAS_COMMIT: case self::TYPE_DREV_HAS_COMMIT: return '%s updated commits of %s.'; - case self::TYPE_COMMIT_HAS_TASK: case self::TYPE_TASK_DEPENDS_ON_TASK: case self::TYPE_TASK_DEPENDED_ON_BY_TASK: case self::TYPE_MOCK_HAS_TASK: