Allow structured destruction of Differential Revisions

Summary:
Ref T4749. Ref T3265. Ref T4909.

  - Remove old "destroy revision" script.
  - Move to structured `bin/remove` destruction.
  - Fix some edge issues.
  - Add transaction destruction support.

Test Plan:
  - Destroyed a bunch of revisions.
  - Saw diffs, changesets, hunks, transactions, edges, and inlines also get wiped out.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4749, T4909, T3265

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

scripts/differential/destroy_revision.php

@@ -1,51 +0,0 @@
-#!/usr/bin/env php
-<?php
-
-$root = dirname(dirname(dirname(__FILE__)));
-require_once $root.'/scripts/__init_script__.php';
-
-$args = new PhutilArgumentParser($argv);
-$args->setTagline('permanently destroy a Differential Revision');
-$args->setSynopsis(<<<EOHELP
-**destroy_revision.php** __D123__
-  Permanently destroy the specified Differential Revision (for example,
-  because it contains secrets that the world is not ready to know).
-
-  Normally, you can just "Abandon" unwanted revisions, but in dire
-  circumstances this script can be used to completely destroy a
-  revision. Destroying a revision may cause some glitches in
-  linked objects.
-
-  The revision is utterly destroyed and can not be recovered unless you
-  have backups.
-EOHELP
-);
-$args->parseStandardArguments();
-$args->parse(
-  array(
-    array(
-      'name'      => 'revision',
-      'wildcard'  => true,
-    ),
-  ));
-
-$revisions = $args->getArg('revision');
-if (count($revisions) != 1) {
-  $args->printHelpAndExit();
-}
-
-$id = trim(strtolower(head($revisions)), 'd ');
-$revision = id(new DifferentialRevision())->load($id);
-
-if (!$revision) {
-  throw new Exception("No revision '{$id}' exists!");
-}
-
-$title = $revision->getTitle();
-$ok = phutil_console_confirm("Really destroy 'D{$id}: {$title}' forever?");
-if (!$ok) {
-  throw new Exception("User aborted workflow.");
-}
-
-$revision->delete();
-echo "OK, destroyed revision.\n";

src/__phutil_library_map__.php

@@ -2982,6 +2982,7 @@ phutil_register_library_map(array(
       1 => 'PhabricatorPolicyInterface',
       2 => 'HarbormasterBuildableInterface',
       3 => 'PhabricatorApplicationTransactionInterface',
+      4 => 'PhabricatorDestructableInterface',
     ),
     'DifferentialDiffCreateController' => 'DifferentialController',
     'DifferentialDiffProperty' => 'DifferentialDAO',
@@ -3047,6 +3048,7 @@ phutil_register_library_map(array(
       6 => 'PhabricatorSubscribableInterface',
       7 => 'PhabricatorCustomFieldInterface',
       8 => 'PhabricatorApplicationTransactionInterface',
+      9 => 'PhabricatorDestructableInterface',
     ),
     'DifferentialRevisionDetailView' => 'AphrontView',
     'DifferentialRevisionEditController' => 'DifferentialController',
@@ -3915,12 +3917,14 @@ phutil_register_library_map(array(
     array(
       0 => 'PhabricatorLiskDAO',
       1 => 'PhabricatorPolicyInterface',
+      2 => 'PhabricatorDestructableInterface',
     ),
     'PhabricatorApplicationTransactionComment' =>
     array(
       0 => 'PhabricatorLiskDAO',
       1 => 'PhabricatorMarkupInterface',
       2 => 'PhabricatorPolicyInterface',
+      3 => 'PhabricatorDestructableInterface',
     ),
     'PhabricatorApplicationTransactionCommentEditController' => 'PhabricatorApplicationTransactionController',
     'PhabricatorApplicationTransactionCommentEditor' => 'PhabricatorEditor',

src/applications/differential/storage/DifferentialDiff.php

@@ -5,7 +5,8 @@ final class DifferentialDiff
   implements
     PhabricatorPolicyInterface,
     HarbormasterBuildableInterface,
-    PhabricatorApplicationTransactionInterface {
+    PhabricatorApplicationTransactionInterface,
+    PhabricatorDestructableInterface {
 
   protected $revisionID;
   protected $authorPHID;
@@ -107,24 +108,6 @@ final class DifferentialDiff
     return $ret;
   }
 
-  public function delete() {
-    $this->openTransaction();
-      foreach ($this->loadChangesets() as $changeset) {
-        $changeset->delete();
-      }
-
-      $properties = id(new DifferentialDiffProperty())->loadAllWhere(
-        'diffID = %d',
-        $this->getID());
-      foreach ($properties as $prop) {
-        $prop->delete();
-      }
-
-      $ret = parent::delete();
-    $this->saveTransaction();
-    return $ret;
-  }
-
   public static function newFromRawChanges(array $changes) {
     assert_instances_of($changes, 'ArcanistDiffChange');
     $diff = new DifferentialDiff();
@@ -376,4 +359,28 @@ final class DifferentialDiff
     return $this->getRevision()->getApplicationTransactionTemplate();
   }
 
+
+/* -(  PhabricatorDestructableInterface  )----------------------------------- */
+
+
+  public function destroyObjectPermanently(
+    PhabricatorDestructionEngine $engine) {
+
+    $this->openTransaction();
+      $this->delete();
+
+      foreach ($this->loadChangesets() as $changeset) {
+        $changeset->delete();
+      }
+
+      $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+        'diffID = %d',
+        $this->getID());
+      foreach ($properties as $prop) {
+        $prop->delete();
+      }
+
+    $this->saveTransaction();
+  }
+
 }

src/applications/differential/storage/DifferentialRevision.php

@@ -9,7 +9,8 @@ final class DifferentialRevision extends DifferentialDAO
     HarbormasterBuildableInterface,
     PhabricatorSubscribableInterface,
     PhabricatorCustomFieldInterface,
-    PhabricatorApplicationTransactionInterface {
+    PhabricatorApplicationTransactionInterface,
+    PhabricatorDestructableInterface {
 
   protected $title = '';
   protected $originalTitle;
@@ -174,45 +175,6 @@ final class DifferentialRevision extends DifferentialDAO
     return parent::save();
   }
 
-  public function delete() {
-    $this->openTransaction();
-    $diffs = id(new DifferentialDiffQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withRevisionIDs(array($this->getID()))
-      ->execute();
-      foreach ($diffs as $diff) {
-        $diff->delete();
-      }
-
-      $conn_w = $this->establishConnection('w');
-
-      queryfx(
-        $conn_w,
-        'DELETE FROM %T WHERE revisionID = %d',
-        self::TABLE_COMMIT,
-        $this->getID());
-
-      $inlines = id(new DifferentialInlineCommentQuery())
-        ->withRevisionIDs(array($this->getID()))
-        ->execute();
-      foreach ($inlines as $inline) {
-        $inline->delete();
-      }
-
-      // we have to do paths a little differentally as they do not have
-      // an id or phid column for delete() to act on
-      $dummy_path = new DifferentialAffectedPath();
-      queryfx(
-        $conn_w,
-        'DELETE FROM %T WHERE revisionID = %d',
-        $dummy_path->getTableName(),
-        $this->getID());
-
-      $result = parent::delete();
-    $this->saveTransaction();
-    return $result;
-  }
-
   public function loadRelationships() {
     if (!$this->getID()) {
       $this->relationships = array();
@@ -477,4 +439,53 @@ final class DifferentialRevision extends DifferentialDAO
     return new DifferentialTransaction();
   }
 
+
+/* -(  PhabricatorDestructableInterface  )----------------------------------- */
+
+
+  public function destroyObjectPermanently(
+    PhabricatorDestructionEngine $engine) {
+
+    $this->openTransaction();
+      $diffs = id(new DifferentialDiffQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withRevisionIDs(array($this->getID()))
+        ->execute();
+      foreach ($diffs as $diff) {
+        $engine->destroyObject($diff);
+      }
+
+      $conn_w = $this->establishConnection('w');
+
+      queryfx(
+        $conn_w,
+        'DELETE FROM %T WHERE revisionID = %d',
+        self::TABLE_COMMIT,
+        $this->getID());
+
+      try {
+        $inlines = id(new DifferentialInlineCommentQuery())
+          ->withRevisionIDs(array($this->getID()))
+          ->execute();
+        foreach ($inlines as $inline) {
+          $inline->delete();
+        }
+      } catch (PhabricatorEmptyQueryException $ex) {
+        // TODO: There's still some funky legacy wrapping going on here, and
+        // we might catch a raw query exception.
+      }
+
+      // we have to do paths a little differentally as they do not have
+      // an id or phid column for delete() to act on
+      $dummy_path = new DifferentialAffectedPath();
+      queryfx(
+        $conn_w,
+        'DELETE FROM %T WHERE revisionID = %d',
+        $dummy_path->getTableName(),
+        $this->getID());
+
+      $this->delete();
+    $this->saveTransaction();
+  }
+
 }

src/applications/system/engine/PhabricatorDestructionEngine.php

@@ -41,20 +41,48 @@ final class PhabricatorDestructionEngine extends Phobject {
 
     if ($object_phid) {
       $this->destroyEdges($object_phid);
+
+      if ($object instanceof PhabricatorApplicationTransactionInterface) {
+        $template = $object->getApplicationTransactionTemplate();
+        $this->destroyTransactions($template, $object_phid);
       }
     }
 
+    // TODO: PhabricatorFlaggableInterface
+    // TODO: PhabricatorTokenReceiverInterface
+  }
+
   private function destroyEdges($src_phid) {
+    try {
       $edges = id(new PhabricatorEdgeQuery())
         ->withSourcePHIDs(array($src_phid))
         ->execute();
+    } catch (Exception $ex) {
+      // This is (presumably) a "no edges for this PHID type" exception.
+      return;
+    }
 
     $editor = id(new PhabricatorEdgeEditor())
       ->setSuppressEvents(true);
-    foreach ($edges as $edge) {
+    foreach ($edges as $type => $type_edges) {
+      foreach ($type_edges as $src => $src_edges) {
+        foreach ($src_edges as $dst => $edge) {
           $editor->removeEdge($edge['src'], $edge['type'], $edge['dst']);
         }
+      }
+    }
     $editor->save();
   }
 
+  private function destroyTransactions(
+    PhabricatorApplicationTransaction $template,
+    $object_phid) {
+
+    $xactions = $template->loadAllWhere('objectPHID = %s', $object_phid);
+    foreach ($xactions as $xaction) {
+      $this->destroyObject($xaction);
+    }
+
+  }
+
 }

src/applications/system/management/PhabricatorSystemRemoveLogWorkflow.php

@@ -17,8 +17,9 @@ final class PhabricatorSystemRemoveLogWorkflow
     $table = new PhabricatorSystemDestructionLog();
     foreach (new LiskMigrationIterator($table) as $row) {
       $console->writeOut(
-        "[%s]\t%s\t%s\t%s\n",
+        "[%s]\t%s %s\t%s\t%s\n",
         phabricator_datetime($row->getEpoch(), $this->getViewer()),
+        ($row->getRootLogID() ? ' ' : '*'),
         $row->getObjectClass(),
         $row->getObjectPHID(),
         $row->getObjectMonogram());

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -2,7 +2,9 @@
 
 abstract class PhabricatorApplicationTransaction
   extends PhabricatorLiskDAO
-  implements PhabricatorPolicyInterface {
+  implements
+    PhabricatorPolicyInterface,
+    PhabricatorDestructableInterface {
 
   const TARGET_TEXT = 'text';
   const TARGET_HTML = 'html';
@@ -942,4 +944,32 @@ abstract class PhabricatorApplicationTransaction
   }
 
 
+/* -(  PhabricatorDestructableInterface  )----------------------------------- */
+
+
+  public function destroyObjectPermanently(
+    PhabricatorDestructionEngine $engine) {
+
+    $this->openTransaction();
+      $comment_template = null;
+      try {
+        $comment_template = $this->getApplicationTransactionCommentObject();
+      } catch (Exception $ex) {
+        // Continue; no comments for these transactions.
+      }
+
+      if ($comment_template) {
+        $comments = $comment_template->loadAllWhere(
+          'transactionPHID = %s',
+          $this->getPHID());
+        foreach ($comments as $comment) {
+          $engine->destroyObject($comment);
+        }
+      }
+
+      $this->delete();
+    $this->saveTransaction();
+  }
+
+
 }

src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php

@@ -2,7 +2,10 @@
 
 abstract class PhabricatorApplicationTransactionComment
   extends PhabricatorLiskDAO
-  implements PhabricatorMarkupInterface, PhabricatorPolicyInterface {
+  implements
+    PhabricatorMarkupInterface,
+    PhabricatorPolicyInterface,
+    PhabricatorDestructableInterface {
 
   const MARKUP_FIELD_COMMENT  = 'markup:comment';
 
@@ -118,4 +121,14 @@ abstract class PhabricatorApplicationTransactionComment
     return null;
   }
 
+
+/* -(  PhabricatorDestructableInterface  )----------------------------------- */
+
+  public function destroyObjectPermanently(
+    PhabricatorDestructionEngine $engine) {
+    $this->openTransaction();
+      $this->delete();
+    $this->saveTransaction();
+  }
+
 }