Remove all edge events

Summary: Ref T5245. These were a bad idea. We no longer need actors for edge edits either, so remove those. Generally, edges have fit into the policy model as pure/low-level infrastructure, and they do not have any policy or capability information in and of themselves. Test Plan: `grep` Reviewers: chad, btrahan, joshuaspence Reviewed By: joshuaspence Subscribers: epriestley Maniphest Tasks: T5245 Differential Revision: https://secure.phabricator.com/D9840
2014-07-17 15:41:42 -07:00
parent 533e799c5f
commit 8cbfb49b4e
28 changed files with 19 additions and 125 deletions
--- a/resources/sql/patches/20130201.revisionunsubscribed.php
+++ b/resources/sql/patches/20130201.revisionunsubscribed.php
@@ -19,7 +19,6 @@ foreach ($revs as $rev) {
  }

  $editor = new PhabricatorEdgeEditor();
-  $editor->setSuppressEvents(true);
  foreach ($unsubscribed as $user_phid => $_) {
    $editor->addEdge(
      $rev['phid'],
--- a/resources/sql/patches/20130409.commitdrev.php
+++ b/resources/sql/patches/20130409.commitdrev.php
@@ -3,7 +3,7 @@
 echo "Migrating differential.revisionPHID to edges...\n";
 $commit_table = new PhabricatorRepositoryCommit();
 $data_table = new PhabricatorRepositoryCommitData();
-$editor = id(new PhabricatorEdgeEditor())->setSuppressEvents(true);
+$editor = new PhabricatorEdgeEditor();
 $commit_table->establishConnection('w');
 $edges = 0;

@@ -24,7 +24,7 @@ foreach (new LiskMigrationIterator($commit_table) as $commit) {
  if ($edges % 256 == 0) {
    echo '.';
    $editor->save();
-    $editor = id(new PhabricatorEdgeEditor())->setSuppressEvents(true);
+    $editor = new PhabricatorEdgeEditor();
  }
 }

--- a/resources/sql/patches/20131004.dxreviewers.php
+++ b/resources/sql/patches/20131004.dxreviewers.php
@@ -25,9 +25,7 @@ foreach (new LiskMigrationIterator($table) as $revision) {
    continue;
  }

-  $editor = id(new PhabricatorEdgeEditor())
-    ->setActor(PhabricatorUser::getOmnipotentUser());
-
+  $editor = new PhabricatorEdgeEditor();
  foreach ($reviewer_phids as $dst) {
    if (phid_get_type($dst) == PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) {
      // At least one old install ran into some issues here. Skip the row if we
--- a/resources/sql/patches/20131121.repocredentials.2.mig.php
+++ b/resources/sql/patches/20131121.repocredentials.2.mig.php
@@ -121,7 +121,6 @@ foreach ($map as $credential_type => $credential_usernames) {
        $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_USES_CREDENTIAL;

        id(new PhabricatorEdgeEditor())
-          ->setActor($viewer)
          ->addEdge($repository->getPHID(), $edge_type, $credential->getPHID())
          ->save();
      }
--- a/resources/sql/patches/migrate-differential-dependencies.php
+++ b/resources/sql/patches/migrate-differential-dependencies.php
@@ -15,7 +15,6 @@ foreach (new LiskMigrationIterator($table) as $rev) {
  }

  $editor = new PhabricatorEdgeEditor();
-  $editor->setSuppressEvents(true);
  foreach ($deps as $dep) {
    $editor->addEdge(
      $rev->getPHID(),
--- a/resources/sql/patches/migrate-maniphest-dependencies.php
+++ b/resources/sql/patches/migrate-maniphest-dependencies.php
@@ -15,7 +15,6 @@ foreach (new LiskMigrationIterator($table) as $task) {
  }

  $editor = new PhabricatorEdgeEditor();
-  $editor->setSuppressEvents(true);
  foreach ($deps as $dep) {
    $editor->addEdge(
      $task->getPHID(),
--- a/resources/sql/patches/migrate-maniphest-revisions.php
+++ b/resources/sql/patches/migrate-maniphest-revisions.php
@@ -15,7 +15,6 @@ foreach (new LiskMigrationIterator($table) as $task) {
  }

  $editor = new PhabricatorEdgeEditor();
-  $editor->setSuppressEvents(true);
  foreach ($revs as $rev) {
    $editor->addEdge(
      $task->getPHID(),
--- a/resources/sql/patches/migrate-project-edges.php
+++ b/resources/sql/patches/migrate-project-edges.php
@@ -22,7 +22,6 @@ foreach (new LiskMigrationIterator($table) as $proj) {
  $members = ipull($members, 'userPHID');

  $editor = new PhabricatorEdgeEditor();
-  $editor->setSuppressEvents(true);
  foreach ($members as $user_phid) {
    $editor->addEdge(
      $proj->getPHID(),
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4357,7 +4357,7 @@ phutil_register_library_map(array(
    'PhabricatorDraftDAO' => 'PhabricatorLiskDAO',
    'PhabricatorEdgeConfig' => 'PhabricatorEdgeConstants',
    'PhabricatorEdgeCycleException' => 'Exception',
-    'PhabricatorEdgeEditor' => 'PhabricatorEditor',
+    'PhabricatorEdgeEditor' => 'Phobject',
    'PhabricatorEdgeGraph' => 'AbstractDirectedGraph',
    'PhabricatorEdgeQuery' => 'PhabricatorQuery',
    'PhabricatorEdgeTestCase' => 'PhabricatorTestCase',
--- a/src/applications/conpherence/editor/ConpherenceEditor.php
+++ b/src/applications/conpherence/editor/ConpherenceEditor.php
@@ -216,8 +216,7 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {

    switch ($xaction->getTransactionType()) {
      case ConpherenceTransactionType::TYPE_FILES:
-        $editor = id(new PhabricatorEdgeEditor())
-          ->setActor($this->getActor());
+        $editor = new PhabricatorEdgeEditor();
        $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_FILE;
        $old = array_fill_keys($xaction->getOldValue(), true);
        $new = array_fill_keys($xaction->getNewValue(), true);
--- a/src/applications/differential/customfield/DifferentialJIRAIssuesField.php
+++ b/src/applications/differential/customfield/DifferentialJIRAIssuesField.php
@@ -257,8 +257,7 @@ final class DifferentialJIRAIssuesField
      $revision_phid,
      $edge_type);

-    $editor = id(new PhabricatorEdgeEditor())
-      ->setActor($this->getViewer());
+    $editor = new PhabricatorEdgeEditor();

    foreach (array_diff($edges, $edge_dsts) as $rem_edge) {
      $editor->removeEdge($revision_phid, $edge_type, $rem_edge);
--- a/src/applications/diffusion/controller/DiffusionCommitEditController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitEditController.php
@@ -34,8 +34,8 @@ final class DiffusionCommitEditController extends DiffusionController {
      $new_proj_phids = array_values($proj_phids);
      $rem_proj_phids = array_diff($current_proj_phids,
                                   $new_proj_phids);
-      $editor         = id(new PhabricatorEdgeEditor());
-      $editor->setActor($user);
+
+      $editor = id(new PhabricatorEdgeEditor());
      foreach ($rem_proj_phids as $phid) {
        $editor->removeEdge($commit_phid, $edge_type, $phid);
      }
--- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php
+++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php
@@ -235,7 +235,6 @@ final class DoorkeeperFeedWorkerAsana extends DoorkeeperFeedWorker {
    );

    id(new PhabricatorEdgeEditor())
-      ->setActor($viewer)
      ->addEdge($src_phid, $etype_main, $dst_phid, $edge_options)
      ->save();

@@ -247,8 +246,7 @@ final class DoorkeeperFeedWorkerAsana extends DoorkeeperFeedWorker {

    // Now, handle the subtasks.

-    $sub_editor = id(new PhabricatorEdgeEditor())
-      ->setActor($viewer);
+    $sub_editor = new PhabricatorEdgeEditor();

    // First, find all the object references in Phabricator for tasks that we
    // know about and import their objects from Asana.
--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@@ -859,8 +859,6 @@ final class PhabricatorFile extends PhabricatorFileDAO
    $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_FILE;

    id(new PhabricatorEdgeEditor())
-      ->setActor($actor)
-      ->setSuppressEvents(true)
      ->addEdge($phid, $edge_type, $this->getPHID())
      ->save();

--- a/src/applications/legalpad/editor/LegalpadDocumentEditor.php
+++ b/src/applications/legalpad/editor/LegalpadDocumentEditor.php
@@ -105,7 +105,6 @@ final class LegalpadDocumentEditor
      $type = PhabricatorEdgeConfig::TYPE_CONTRIBUTED_TO_OBJECT;
      id(new PhabricatorEdgeEditor())
        ->addEdge($actor->getPHID(), $type, $object->getPHID())
-        ->setActor($actor)
        ->save();

      $type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_CONTRIBUTOR;
--- a/src/applications/maniphest/controller/ManiphestTaskEditController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php
@@ -330,7 +330,6 @@ final class ManiphestTaskEditController extends ManiphestController {

        if ($parent_task) {
          id(new PhabricatorEdgeEditor())
-            ->setActor($user)
            ->addEdge(
              $parent_task->getPHID(),
              PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK,
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -225,10 +225,7 @@ final class ManiphestTransactionEditor
          return;
        }

-        $editor = id(new PhabricatorEdgeEditor())
-          ->setActor($this->getActor())
-          ->setSuppressEvents(true);
-
+        $editor = new PhabricatorEdgeEditor();
        foreach ($add as $phid) {
          $editor->addEdge($src, $edge_type, $phid);
        }
--- a/src/applications/ponder/editor/PonderVoteEditor.php
+++ b/src/applications/ponder/editor/PonderVoteEditor.php
@@ -34,7 +34,6 @@ final class PonderVoteEditor extends PhabricatorEditor {
    // prepare vote add, or update if this user is amending an
    // earlier vote
    $editor = id(new PhabricatorEdgeEditor())
-      ->setActor($actor)
      ->addEdge(
        $actor->getPHID(),
        $votable->getUserVoteEdgeType(),
--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -231,8 +231,7 @@ final class PhabricatorProjectTransactionEditor

            if ($rem) {
              // When removing members, also remove any watches on the project.
-              $edge_editor = id(new PhabricatorEdgeEditor())
-                ->setSuppressEvents(true);
+              $edge_editor = new PhabricatorEdgeEditor();
              foreach ($rem as $rem_phid) {
                $edge_editor->removeEdge(
                  $object->getPHID(),
--- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php
+++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
@@ -225,8 +225,7 @@ final class PhabricatorRepositoryEditor
        $old_phid = $xaction->getOldValue();
        $new_phid = $xaction->getNewValue();

-        $editor = id(new PhabricatorEdgeEditor())
-          ->setActor($this->requireActor());
+        $editor = new PhabricatorEdgeEditor();

        $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_USES_CREDENTIAL;
        $src_phid = $object->getPHID();
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -90,7 +90,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
      if ($revision) {
        $commit_drev = PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV;
        id(new PhabricatorEdgeEditor())
-          ->setActor($user)
          ->addEdge($commit->getPHID(), $commit_drev, $revision->getPHID())
          ->save();

@@ -453,7 +452,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
      // Maniphest edges is sorted out. Currently, Maniphest reacts to an edge
      // edit on this edge.
      id(new PhabricatorEdgeEditor())
-        ->setActor($actor)
        ->addEdge(
          $task->getPHID(),
          PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT,
--- a/src/applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php
+++ b/src/applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php
@@ -83,8 +83,7 @@ final class PhabricatorSubscriptionsEditor extends PhabricatorEditor {
      $u_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER;
      $s_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER;

-      $editor = id(new PhabricatorEdgeEditor())
-        ->setActor($actor);
+      $editor = new PhabricatorEdgeEditor();

      foreach ($add as $phid => $ignored) {
        $editor->removeEdge($src, $u_type, $phid);
--- a/src/applications/system/engine/PhabricatorDestructionEngine.php
+++ b/src/applications/system/engine/PhabricatorDestructionEngine.php
@@ -62,8 +62,7 @@ final class PhabricatorDestructionEngine extends Phobject {
      return;
    }

-    $editor = id(new PhabricatorEdgeEditor())
-      ->setSuppressEvents(true);
+    $editor = new PhabricatorEdgeEditor();
    foreach ($edges as $type => $type_edges) {
      foreach ($type_edges as $src => $src_edges) {
        foreach ($src_edges as $dst => $edge) {
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -411,8 +411,7 @@ abstract class PhabricatorApplicationTransactionEditor
          $new[$dst_phid]['src'] = $src;
        }

-        $editor = id(new PhabricatorEdgeEditor())
-          ->setActor($this->getActor());
+        $editor = new PhabricatorEdgeEditor();

        foreach ($old as $dst_phid => $edge) {
          if (!empty($new[$dst_phid])) {
@@ -2346,11 +2345,6 @@ abstract class PhabricatorApplicationTransactionEditor
    $editor = id(new PhabricatorEdgeEditor())
      ->setActor($this->getActor());

-    // TODO: Edge-based events were almost certainly a terrible idea. If we
-    // don't suppress this event, the Maniphest listener reenters and adds
-    // more transactions. Just suppress it until that can get cleaned up.
-    $editor->setSuppressEvents(true);
-
    $src = $object->getPHID();
    $type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_FILE;
    foreach ($file_phids as $dst) {
--- a/src/docs/user/userguide/events.diviner
+++ b/src/docs/user/userguide/events.diviner
@@ -236,40 +236,6 @@ Data available on this event:

 Using @{class@libphutil:PhutilEmailAddress} may be helpful in parsing the query.

-== Edge: Will Edit Edges ==
-
-NOTE: Edge events are low-level events deep in the core. It is more difficult to
-correct implement listeners for these events than for higher-level events.
-
-The constant for this event is
-`PhabricatorEventType::TYPE_EDGE_WILLEDITEDGES`.
-
-This event is dispatched before @{class:PhabricatorEdgeEditor} makes an edge
-edit.
-
-  - `id` An identifier for this edit operation.
-  - `add` A list of dictionaries, each representing a new edge.
-  - `rem` A list of dictionaries, each representing a removed edge.
-
-This is similar to the next event (did edit edges) but occurs before the
-edit begins.
-
-== Edge: Did Edit Edges ==
-
-The constant for this event is
-`PhabricatorEventType::TYPE_EDGE_DIDEDITEDGES`.
-
-This event is dispatched after @{class:PhabricatorEdgeEditor} makes an edge
-edit, but before it commits the transactions. Data available on this event:
-
-  - `id` An identifier for this edit operation. This is the same ID as
-    the one included in the corresponding "will edit edges" event.
-  - `add` A list of dictionaries, each representing a new edge.
-  - `rem` A list of dictionaries, each representing a removed edge.
-
-This is similar to the previous event (will edit edges) but occurs after the
-edit completes.
-
 == Search: Did Update Index ==

 The constant for this event is
--- a/src/infrastructure/edges/tests/PhabricatorEdgeTestCase.php
+++ b/src/infrastructure/edges/tests/PhabricatorEdgeTestCase.php
@@ -21,7 +21,6 @@ final class PhabricatorEdgeTestCase extends PhabricatorTestCase {
    $phid2 = $obj2->getPHID();

    $editor = id(new PhabricatorEdgeEditor())
-      ->setActor($user)
      ->addEdge($phid1, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid2)
      ->addEdge($phid2, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid1);

@@ -39,12 +38,10 @@ final class PhabricatorEdgeTestCase extends PhabricatorTestCase {
    // fail (it introduces a cycle).

    $editor = id(new PhabricatorEdgeEditor())
-      ->setActor($user)
      ->addEdge($phid1, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid2)
      ->save();

    $editor = id(new PhabricatorEdgeEditor())
-      ->setActor($user)
      ->addEdge($phid2, PhabricatorEdgeConfig::TYPE_TEST_NO_CYCLE, $phid1);

    $caught = null;
--- a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php
+++ b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php
@@ -5,6 +5,9 @@
 * @{class:PhabricatorEdgeQuery} to load object edges. For more information
 * on edges, see @{article:Using Edges}.
 *
+ * Edges are not directly policy aware, and this editor makes low-level changes
+ * below the policy layer.
+ *
 *    name=Adding Edges
 *    $src  = $earth_phid;
 *    $type = PhabricatorEdgeConfig::TYPE_BODY_HAS_SATELLITE;
@@ -12,19 +15,17 @@
 *
 *    id(new PhabricatorEdgeEditor())
 *      ->addEdge($src, $type, $dst)
- *      ->setActor($user)
 *      ->save();
 *
 * @task edit     Editing Edges
 * @task cycles   Cycle Prevention
 * @task internal Internals
 */
-final class PhabricatorEdgeEditor extends PhabricatorEditor {
+final class PhabricatorEdgeEditor extends Phobject {

  private $addEdges = array();
  private $remEdges = array();
  private $openTransactions = array();
-  private $suppressEvents;


 /* -(  Editing Edges  )------------------------------------------------------ */
@@ -118,8 +119,6 @@ final class PhabricatorEdgeEditor extends PhabricatorEditor {
      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".

@@ -130,8 +129,6 @@ final class PhabricatorEdgeEditor extends PhabricatorEditor {
        $this->detectCycles($src_phids, $cycle_type);
      }

-      $this->sendEvent($id, PhabricatorEventType::TYPE_EDGE_DIDEDITEDGES);
-
      $this->saveTransactions();
    } catch (Exception $ex) {
      $caught = $ex;
@@ -351,37 +348,6 @@ final class PhabricatorEdgeEditor extends PhabricatorEditor {
    }
  }

-  /**
-   * Suppress edge edit events. This prevents listeners from making updates in
-   * response to edits, and is primarily useful when performing migrations. You
-   * should not normally need to use it.
-   *
-   * @param bool True to suppress events related to edits.
-   * @return this
-   * @task internal
-   */
-  public function setSuppressEvents($suppress) {
-    $this->suppressEvents = $suppress;
-    return $this;
-  }
-
-
-  private function sendEvent($edit_id, $event_type) {
-    if ($this->suppressEvents) {
-      return;
-    }
-
-    $event = new PhabricatorEvent(
-      $event_type,
-      array(
-        'id'    => $edit_id,
-        'add'   => $this->addEdges,
-        'rem'   => $this->remEdges,
-      ));
-    $event->setUser($this->getActor());
-    PhutilEventEngine::dispatchEvent($event);
-  }
-

 /* -(  Cycle Prevention  )--------------------------------------------------- */

--- a/src/infrastructure/events/constant/PhabricatorEventType.php
+++ b/src/infrastructure/events/constant/PhabricatorEventType.php
@@ -16,9 +16,6 @@ final class PhabricatorEventType extends PhutilEventType {
  const TYPE_DIFFUSION_DIDDISCOVERCOMMIT    = 'diffusion.didDiscoverCommit';
  const TYPE_DIFFUSION_LOOKUPUSER           = 'diffusion.lookupUser';

-  const TYPE_EDGE_WILLEDITEDGES             = 'edge.willEditEdges';
-  const TYPE_EDGE_DIDEDITEDGES              = 'edge.didEditEdges';
-
  const TYPE_TEST_DIDRUNTEST                = 'test.didRunTest';

  const TYPE_UI_DIDRENDERACTIONS            = 'ui.didRenderActions';