diff --git a/resources/sql/patches/migrate-maniphest-revisions.php b/resources/sql/patches/migrate-maniphest-revisions.php --- a/resources/sql/patches/migrate-maniphest-revisions.php +++ b/resources/sql/patches/migrate-maniphest-revisions.php @@ -19,7 +19,7 @@ foreach ($revs as $rev) { $editor->addEdge( $task->getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV, + ManiphestTaskHasRevisionEdgeType::EDGECONST, $rev); } $editor->save(); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -438,6 +438,7 @@ 'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php', 'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php', 'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php', + 'DifferentialRevisionHasTaskEdgeType' => 'applications/differential/edge/DifferentialRevisionHasTaskEdgeType.php', 'DifferentialRevisionIDField' => 'applications/differential/customfield/DifferentialRevisionIDField.php', 'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php', 'DifferentialRevisionListController' => 'applications/differential/controller/DifferentialRevisionListController.php', @@ -944,6 +945,7 @@ 'ManiphestTaskDescriptionPreviewController' => 'applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php', 'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php', 'ManiphestTaskEditController' => 'applications/maniphest/controller/ManiphestTaskEditController.php', + 'ManiphestTaskHasRevisionEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php', 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', 'ManiphestTaskListView' => 'applications/maniphest/view/ManiphestTaskListView.php', 'ManiphestTaskMailReceiver' => 'applications/maniphest/mail/ManiphestTaskMailReceiver.php', @@ -3153,6 +3155,7 @@ ), 'DifferentialRevisionDetailView' => 'AphrontView', 'DifferentialRevisionEditController' => 'DifferentialController', + 'DifferentialRevisionHasTaskEdgeType' => 'PhabricatorEdgeType', 'DifferentialRevisionIDField' => 'DifferentialCustomField', 'DifferentialRevisionLandController' => 'DifferentialController', 'DifferentialRevisionListController' => 'DifferentialController', @@ -3700,6 +3703,7 @@ 'ManiphestTaskDescriptionPreviewController' => 'ManiphestController', 'ManiphestTaskDetailController' => 'ManiphestController', 'ManiphestTaskEditController' => 'ManiphestController', + 'ManiphestTaskHasRevisionEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListView' => 'ManiphestView', 'ManiphestTaskMailReceiver' => 'PhabricatorObjectMailReceiver', diff --git a/src/applications/differential/customfield/DifferentialManiphestTasksField.php b/src/applications/differential/customfield/DifferentialManiphestTasksField.php --- a/src/applications/differential/customfield/DifferentialManiphestTasksField.php +++ b/src/applications/differential/customfield/DifferentialManiphestTasksField.php @@ -42,7 +42,7 @@ return PhabricatorEdgeQuery::loadDestinationPHIDs( $revision->getPHID(), - PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK); + DifferentialRevisionHasTaskEdgeType::EDGECONST); } public function getApplicationTransactionType() { @@ -51,7 +51,7 @@ public function getApplicationTransactionMetadata() { return array( - 'edge:type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK, + 'edge:type' => DifferentialRevisionHasTaskEdgeType::EDGECONST, ); } diff --git a/src/applications/differential/edge/DifferentialRevisionHasTaskEdgeType.php b/src/applications/differential/edge/DifferentialRevisionHasTaskEdgeType.php new file mode 100644 --- /dev/null +++ b/src/applications/differential/edge/DifferentialRevisionHasTaskEdgeType.php @@ -0,0 +1,105 @@ +execute(); if ($tasks) { - $edge_related = PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK; + $edge_related = DifferentialRevisionHasTaskEdgeType::EDGECONST; $edges[$edge_related] = mpull($tasks, 'getPHID', 'getPHID'); } } diff --git a/src/applications/differential/event/DifferentialHovercardEventListener.php b/src/applications/differential/event/DifferentialHovercardEventListener.php --- a/src/applications/differential/event/DifferentialHovercardEventListener.php +++ b/src/applications/differential/event/DifferentialHovercardEventListener.php @@ -28,7 +28,7 @@ $rev->loadRelationships(); $reviewer_phids = $rev->getReviewers(); - $e_task = PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK; + $e_task = DifferentialRevisionHasTaskEdgeType::EDGECONST; $edge_query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs(array($phid)) ->withEdgeTypes( diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -54,7 +54,7 @@ $e_commit = PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT; $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK; $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK; - $e_rev = PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV; + $e_rev = ManiphestTaskHasRevisionEdgeType::EDGECONST; $e_mock = PhabricatorEdgeConfig::TYPE_TASK_HAS_MOCK; $phid = $task->getPHID(); @@ -590,13 +590,13 @@ $edge_types = array( PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK - => pht('Blocks'), + => pht('Blocks'), PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK - => pht('Blocked By'), - PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV - => pht('Differential Revisions'), + => pht('Blocked By'), + ManiphestTaskHasRevisionEdgeType::EDGECONST + => pht('Differential Revisions'), PhabricatorEdgeConfig::TYPE_TASK_HAS_MOCK - => pht('Pholio Mocks'), + => pht('Pholio Mocks'), ); $revisions_commits = array(); @@ -616,7 +616,7 @@ $revision_phid = key($drev_edges[$phid][$commit_drev]); $revision_handle = idx($handles, $revision_phid); if ($revision_handle) { - $task_drev = PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV; + $task_drev = ManiphestTaskHasRevisionEdgeType::EDGECONST; unset($edges[$task_drev][$revision_phid]); $revisions_commits[$phid] = hsprintf( '%s / %s', diff --git a/src/applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php b/src/applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php @@ -0,0 +1,105 @@ + array( $t_cmit => PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT, $t_task => PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, - $t_drev => PhabricatorEdgeConfig::TYPE_TASK_HAS_RELATED_DREV, + $t_drev => ManiphestTaskHasRevisionEdgeType::EDGECONST, $t_mock => PhabricatorEdgeConfig::TYPE_TASK_HAS_MOCK, ), $t_drev => array( $t_drev => PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV, - $t_task => PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK, + $t_task => DifferentialRevisionHasTaskEdgeType::EDGECONST, ), $t_mock => array( $t_task => PhabricatorEdgeConfig::TYPE_MOCK_HAS_TASK, diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -24,6 +24,7 @@ private $isPreview; private $isHeraldEditor; + private $isInverseEdgeEditor; private $actingAsPHID; private $disableEmail; @@ -120,6 +121,15 @@ return $this->isPreview; } + public function setIsInverseEdgeEditor($is_inverse_edge_editor) { + $this->isInverseEdgeEditor = $is_inverse_edge_editor; + return $this; + } + + public function getIsInverseEdgeEditor() { + return $this->isInverseEdgeEditor; + } + public function setIsHeraldEditor($is_herald_editor) { $this->isHeraldEditor = $is_herald_editor; return $this; @@ -377,10 +387,25 @@ break; case PhabricatorTransactions::TYPE_EDGE: + if ($this->getIsInverseEdgeEditor()) { + // If we're writing an inverse edge transaction, don't actually + // do anything. The initiating editor on the other side of the + // transaction will take care of the edge writes. + break; + } + $old = $xaction->getOldValue(); $new = $xaction->getNewValue(); $src = $object->getPHID(); - $type = $xaction->getMetadataValue('edge:type'); + $const = $xaction->getMetadataValue('edge:type'); + + $type = PhabricatorEdgeType::getByConstant($const); + if ($type->shouldWriteInverseTransactions()) { + $this->applyInverseEdgeTransactions( + $object, + $xaction, + $type->getInverseEdgeConstant()); + } foreach ($new as $dst_phid => $edge) { $new[$dst_phid]['src'] = $src; @@ -395,7 +420,7 @@ continue; } } - $editor->removeEdge($src, $type, $dst_phid); + $editor->removeEdge($src, $const, $dst_phid); } foreach ($new as $dst_phid => $edge) { @@ -409,7 +434,7 @@ 'data' => $edge['data'], ); - $editor->addEdge($src, $type, $dst_phid, $data); + $editor->addEdge($src, $const, $dst_phid, $data); } $editor->save(); @@ -2335,4 +2360,59 @@ $editor->save(); } + private function applyInverseEdgeTransactions( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction, + $inverse_type) { + + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + + $add = array_keys(array_diff_key($new, $old)); + $rem = array_keys(array_diff_key($old, $new)); + + $add = array_fuse($add); + $rem = array_fuse($rem); + $all = $add + $rem; + + $nodes = id(new PhabricatorObjectQuery()) + ->setViewer($this->requireActor()) + ->withPHIDs($all) + ->execute(); + + foreach ($nodes as $node) { + if (!($node instanceof PhabricatorApplicationTransactionInterface)) { + continue; + } + + $editor = $node->getApplicationTransactionEditor(); + $template = $node->getApplicationTransactionTemplate(); + $target = $node->getApplicationTransactionObject(); + + if (isset($add[$node->getPHID()])) { + $edge_edit_type = '+'; + } else { + $edge_edit_type = '-'; + } + + $template + ->setTransactionType($xaction->getTransactionType()) + ->setMetadataValue('edge:type', $inverse_type) + ->setNewValue( + array( + $edge_edit_type => array($object->getPHID() => $object->getPHID()), + )); + + $editor + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->setParentMessageID($this->getParentMessageID()) + ->setIsInverseEdgeEditor(true) + ->setActor($this->requireActor()) + ->setContentSource($this->getContentSource()); + + $editor->applyTransactions($target, array($template)); + } + } + } diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -19,9 +19,6 @@ const TYPE_BLOG_HAS_BLOGGER = 9; const TYPE_BLOGGER_HAS_BLOG = 10; - const TYPE_TASK_HAS_RELATED_DREV = 11; - const TYPE_DREV_HAS_RELATED_TASK = 12; - const TYPE_PROJ_MEMBER = 13; const TYPE_MEMBER_OF_PROJ = 14; @@ -137,7 +134,7 @@ return $map; } - public static function getInverse($edge_type) { + 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, @@ -153,9 +150,6 @@ self::TYPE_BLOG_HAS_BLOGGER => self::TYPE_BLOGGER_HAS_BLOG, self::TYPE_BLOGGER_HAS_BLOG => self::TYPE_BLOG_HAS_BLOGGER, - self::TYPE_TASK_HAS_RELATED_DREV => self::TYPE_DREV_HAS_RELATED_TASK, - self::TYPE_DREV_HAS_RELATED_TASK => self::TYPE_TASK_HAS_RELATED_DREV, - self::TYPE_PROJ_MEMBER => self::TYPE_MEMBER_OF_PROJ, self::TYPE_MEMBER_OF_PROJ => self::TYPE_PROJ_MEMBER, @@ -226,7 +220,7 @@ return idx($map, $edge_type); } - public static function shouldPreventCycles($edge_type) { + private static function shouldPreventCycles($edge_type) { static $map = array( self::TYPE_TEST_NO_CYCLE => true, self::TYPE_TASK_DEPENDS_ON_TASK => true, @@ -272,12 +266,10 @@ case self::TYPE_COMMIT_HAS_TASK: case self::TYPE_TASK_DEPENDS_ON_TASK: case self::TYPE_TASK_DEPENDED_ON_BY_TASK: - case self::TYPE_DREV_HAS_RELATED_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s edited task(s), added %d: %s; removed %d: %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: case self::TYPE_DREV_DEPENDED_ON_BY_DREV: - case self::TYPE_TASK_HAS_RELATED_DREV: case self::TYPE_COMMIT_HAS_DREV: case self::TYPE_REVIEWER_FOR_DREV: return '%s edited revision(s), added %d: %s; removed %d: %s.'; @@ -354,11 +346,9 @@ 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_DREV_HAS_RELATED_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s added %d task(s): %s.'; case self::TYPE_DREV_DEPENDED_ON_BY_DREV: - case self::TYPE_TASK_HAS_RELATED_DREV: case self::TYPE_COMMIT_HAS_DREV: case self::TYPE_REVIEWER_FOR_DREV: return '%s added %d revision(s): %s.'; @@ -432,12 +422,10 @@ 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_DREV_HAS_RELATED_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s removed %d task(s): %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: case self::TYPE_DREV_DEPENDED_ON_BY_DREV: - case self::TYPE_TASK_HAS_RELATED_DREV: case self::TYPE_COMMIT_HAS_DREV: case self::TYPE_REVIEWER_FOR_DREV: return '%s removed %d revision(s): %s.'; @@ -507,12 +495,10 @@ case self::TYPE_COMMIT_HAS_TASK: case self::TYPE_TASK_DEPENDS_ON_TASK: case self::TYPE_TASK_DEPENDED_ON_BY_TASK: - case self::TYPE_DREV_HAS_RELATED_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s updated tasks of %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: case self::TYPE_DREV_DEPENDED_ON_BY_DREV: - case self::TYPE_TASK_HAS_RELATED_DREV: case self::TYPE_COMMIT_HAS_DREV: case self::TYPE_REVIEWER_FOR_DREV: return '%s updated revisions of %s.'; diff --git a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php --- a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php +++ b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php @@ -180,8 +180,9 @@ 'data' => $data, ); - $inverse = PhabricatorEdgeConfig::getInverse($type); - if ($inverse) { + $type_obj = PhabricatorEdgeType::getByConstant($type); + $inverse = $type_obj->getInverseEdgeConstant(); + if ($inverse !== null) { // If `inverse_data` is set, overwrite the edge data. Normally, just // write the same data to the inverse edge. @@ -398,7 +399,8 @@ $edge_types[$edge['type']] = true; } foreach ($edge_types as $type => $ignored) { - if (!PhabricatorEdgeConfig::shouldPreventCycles($type)) { + $type_obj = PhabricatorEdgeType::getByConstant($type); + if (!$type_obj->shouldPreventCycles()) { unset($edge_types[$type]); } } diff --git a/src/infrastructure/edges/type/PhabricatorEdgeType.php b/src/infrastructure/edges/type/PhabricatorEdgeType.php --- a/src/infrastructure/edges/type/PhabricatorEdgeType.php +++ b/src/infrastructure/edges/type/PhabricatorEdgeType.php @@ -42,6 +42,10 @@ return false; } + public function shouldWriteInverseTransactions() { + return false; + } + public function getTransactionAddString( $actor, $add_count,