(stable) Wrap edge transaction readers in a translation layer

Summary:
Ref T13051. This puts a translation layer between the raw edge data in the transaction table and the UI that uses it.

The intent is to start writing new, more compact data soon. This class give us a consistent API for interacting with either the new or old data format, so we don't have to migrate everything upfront.

Test Plan: Browsed around, saw existing edge transactions render properly in transactions and feed. Added and removed subscribers and projects, saw good transaction rendering.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18946

src/__phutil_library_map__.php

@@ -2747,6 +2747,7 @@ phutil_register_library_map(array(
     'PhabricatorDraftEngine' => 'applications/transactions/draft/PhabricatorDraftEngine.php',
     'PhabricatorDraftInterface' => 'applications/transactions/draft/PhabricatorDraftInterface.php',
     'PhabricatorDrydockApplication' => 'applications/drydock/application/PhabricatorDrydockApplication.php',
+    'PhabricatorEdgeChangeRecord' => 'infrastructure/edges/util/PhabricatorEdgeChangeRecord.php',
     'PhabricatorEdgeConfig' => 'infrastructure/edges/constants/PhabricatorEdgeConfig.php',
     'PhabricatorEdgeConstants' => 'infrastructure/edges/constants/PhabricatorEdgeConstants.php',
     'PhabricatorEdgeCycleException' => 'infrastructure/edges/exception/PhabricatorEdgeCycleException.php',
@@ -8170,6 +8171,7 @@ phutil_register_library_map(array(
     'PhabricatorDraftDAO' => 'PhabricatorLiskDAO',
     'PhabricatorDraftEngine' => 'Phobject',
     'PhabricatorDrydockApplication' => 'PhabricatorApplication',
+    'PhabricatorEdgeChangeRecord' => 'Phobject',
     'PhabricatorEdgeConfig' => 'PhabricatorEdgeConstants',
     'PhabricatorEdgeConstants' => 'Phobject',
     'PhabricatorEdgeCycleException' => 'Exception',

src/applications/differential/storage/DifferentialTransaction.php

@@ -87,19 +87,6 @@ final class DifferentialTransaction
         }
         break;
 
-      case PhabricatorTransactions::TYPE_EDGE:
-        $add = array_diff_key($new, $old);
-        $rem = array_diff_key($old, $new);
-
-        // Hide metadata-only edge transactions. These correspond to users
-        // accepting or rejecting revisions, but the change is always explicit
-        // because of the TYPE_ACTION transaction. Rendering these transactions
-        // just creates clutter.
-
-        if (!$add && !$rem) {
-          return true;
-        }
-        break;
       case DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE:
         // Don't hide the initial "X requested review: ..." transaction from
         // mail or feed even when it occurs during creation. We need this

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -302,8 +302,8 @@ abstract class PhabricatorApplicationTransaction
         $phids[] = $new;
         break;
       case PhabricatorTransactions::TYPE_EDGE:
-        $phids[] = ipull($old, 'dst');
+        $record = PhabricatorEdgeChangeRecord::newFromTransaction($this);
-        $phids[] = ipull($new, 'dst');
+        $phids[] = $record->getChangedPHIDs();
         break;
       case PhabricatorTransactions::TYPE_COLUMNS:
         foreach ($new as $move) {
@@ -632,9 +632,8 @@ abstract class PhabricatorApplicationTransaction
             return true;
             break;
           case PhabricatorObjectMentionedByObjectEdgeType::EDGECONST:
-            $new = ipull($this->getNewValue(), 'dst');
+            $record = PhabricatorEdgeChangeRecord::newFromTransaction($this);
-            $old = ipull($this->getOldValue(), 'dst');
+            $add = $record->getAddedPHIDs();
-            $add = array_diff($new, $old);
             $add_value = reset($add);
             $add_handle = $this->getHandle($add_value);
             if ($add_handle->getPolicyFiltered()) {
@@ -933,10 +932,10 @@ abstract class PhabricatorApplicationTransaction
         }
         break;
       case PhabricatorTransactions::TYPE_EDGE:
-        $new = ipull($new, 'dst');
+        $record = PhabricatorEdgeChangeRecord::newFromTransaction($this);
-        $old = ipull($old, 'dst');
+        $add = $record->getAddedPHIDs();
-        $add = array_diff($new, $old);
+        $rem = $record->getRemovedPHIDs();
-        $rem = array_diff($old, $new);
+
         $type = $this->getMetadata('edge:type');
         $type = head($type);
 
@@ -1172,10 +1171,10 @@ abstract class PhabricatorApplicationTransaction
             $this->renderHandleLink($new));
         }
       case PhabricatorTransactions::TYPE_EDGE:
-        $new = ipull($new, 'dst');
+        $record = PhabricatorEdgeChangeRecord::newFromTransaction($this);
-        $old = ipull($old, 'dst');
+        $add = $record->getAddedPHIDs();
-        $add = array_diff($new, $old);
+        $rem = $record->getRemovedPHIDs();
-        $rem = array_diff($old, $new);
+
         $type = $this->getMetadata('edge:type');
         $type = head($type);
 

src/infrastructure/edges/util/PhabricatorEdgeChangeRecord.php

@@ -0,0 +1,68 @@
+<?php
+
+final class PhabricatorEdgeChangeRecord
+  extends Phobject {
+
+  private $xaction;
+
+  public static function newFromTransaction(
+    PhabricatorApplicationTransaction $xaction) {
+    $record = new self();
+    $record->xaction = $xaction;
+    return $record;
+  }
+
+  public function getChangedPHIDs() {
+    $add = $this->getAddedPHIDs();
+    $rem = $this->getRemovedPHIDs();
+
+    $add = array_fuse($add);
+    $rem = array_fuse($rem);
+
+    return array_keys($add + $rem);
+  }
+
+  public function getAddedPHIDs() {
+    $old = $this->getOldDestinationPHIDs();
+    $new = $this->getNewDestinationPHIDs();
+
+    $old = array_fuse($old);
+    $new = array_fuse($new);
+
+    $add = array_diff_key($new, $old);
+    return array_keys($add);
+  }
+
+  public function getRemovedPHIDs() {
+    $old = $this->getOldDestinationPHIDs();
+    $new = $this->getNewDestinationPHIDs();
+
+    $old = array_fuse($old);
+    $new = array_fuse($new);
+
+    $rem = array_diff_key($old, $new);
+    return array_keys($rem);
+  }
+
+  private function getOldDestinationPHIDs() {
+    if ($this->xaction) {
+      $old = $this->xaction->getOldValue();
+      return ipull($old, 'dst');
+    }
+
+    throw new Exception(
+      pht('Edge change record is not configured with any change data.'));
+  }
+
+  private function getNewDestinationPHIDs() {
+    if ($this->xaction) {
+      $new = $this->xaction->getNewValue();
+      return ipull($new, 'dst');
+    }
+
+    throw new Exception(
+      pht('Edge change record is not configured with any change data.'));
+  }
+
+
+}