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

@@ -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',

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');
 

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(

src/applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php

@@ -0,0 +1,103 @@
+<?php
+
+final class DiffusionCommitHasTaskEdgeType extends PhabricatorEdgeType {
+
+  const EDGECONST = 2;
+
+  public function shouldWriteInverseTransactions() {
+    // TODO: This should happen after T4896, but Diffusion does not support
+    // transactions yet.
+    return false;
+  }
+
+  public function getTransactionAddString(
+    $actor,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s task(s): %s.',
+      $actor,
+      $add_count,
+      $add_edges);
+  }
+
+  public function getTransactionRemoveString(
+    $actor,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s task(s): %s.',
+      $actor,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getTransactionEditString(
+    $actor,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited %s task(s), added %s: %s; removed %s: %s.',
+      $actor,
+      $total_count,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getFeedAddString(
+    $actor,
+    $object,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s task(s) to %s: %s.',
+      $actor,
+      $add_count,
+      $object,
+      $add_edges);
+  }
+
+  public function getFeedRemoveString(
+    $actor,
+    $object,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s task(s) from %s: %s.',
+      $actor,
+      $rem_count,
+      $object,
+      $rem_edges);
+  }
+
+  public function getFeedEditString(
+    $actor,
+    $object,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited %s task(s) for %s, added %s: %s; removed %s: %s.',
+      $actor,
+      $total_count,
+      $object,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+}

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -51,7 +51,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
       ->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())

src/applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php

@@ -0,0 +1,103 @@
+<?php
+
+final class ManiphestTaskHasCommitEdgeType extends PhabricatorEdgeType {
+
+  const EDGECONST = 1;
+
+  public function shouldWriteInverseTransactions() {
+    // TODO: This should happen after T4896, but Diffusion does not support
+    // transactions yet.
+    return false;
+  }
+
+  public function getTransactionAddString(
+    $actor,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s commit(s): %s.',
+      $actor,
+      $add_count,
+      $add_edges);
+  }
+
+  public function getTransactionRemoveString(
+    $actor,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s commit(s): %s.',
+      $actor,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getTransactionEditString(
+    $actor,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited %s commit(s), added %s: %s; removed %s: %s.',
+      $actor,
+      $total_count,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getFeedAddString(
+    $actor,
+    $object,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s commit(s) to %s: %s.',
+      $actor,
+      $add_count,
+      $object,
+      $add_edges);
+  }
+
+  public function getFeedRemoveString(
+    $actor,
+    $object,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s commit(s) from %s: %s.',
+      $actor,
+      $rem_count,
+      $object,
+      $rem_edges);
+  }
+
+  public function getFeedEditString(
+    $actor,
+    $object,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited %s commit(s) for %s, added %s: %s; removed %s: %s.',
+      $actor,
+      $total_count,
+      $object,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+}

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -448,28 +448,16 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     foreach ($tasks as $task_id => $task) {
       $xactions = array();
 
-      // TODO: Swap this for a real edge transaction once the weirdness in
+      $edge_type = ManiphestTaskHasCommitEdgeType::EDGECONST;
-      // 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.
-
       $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) {

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,

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: