Provide a more flexible script for administrative management of audits

Summary: Fixes T3679. This comes up every so often and the old script is extremely broad (nuke everything in a repository). Provide a more surgical tool.

Test Plan: Ran a bunch of variations of the script and they all seemed to work OK.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, staticshock

Maniphest Tasks: T3679

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

bin/audit

@@ -0,0 +1 @@
+../scripts/setup/manage_audit.php

scripts/repository/audit.php

@@ -1,83 +1,5 @@
 #!/usr/bin/env php
 <?php
 
-$root = dirname(dirname(dirname(__FILE__)));
-require_once $root.'/scripts/__init_script__.php';
-
-$args = new PhutilArgumentParser($argv);
-$args->setTagline('manage open Audit requests');
-$args->setSynopsis(<<<EOSYNOPSIS
-**audit.php** __repository_callsign__
-    Close all open audit requests in a repository. This is intended to
-    reset the state of an imported repository which triggered a bunch of
-    spurious audit requests during import.
-
-EOSYNOPSIS
-  );
-$args->parseStandardArguments();
-$args->parse(
-  array(
-    array(
-      'name'      => 'more',
-      'wildcard'  => true,
-    ),
-  ));
-
-$more = $args->getArg('more');
-if (count($more) !== 1) {
-  $args->printHelpAndExit();
-}
-$callsign = reset($more);
-
-$repository = id(new PhabricatorRepository())->loadOneWhere(
-  'callsign = %s',
-  $callsign);
-if (!$repository) {
-  throw new Exception("No repository exists with callsign '{$callsign}'!");
-}
-
-$ok = phutil_console_confirm(
-  'This will reset all open audit requests ("Audit Required" or "Concern '.
-  'Raised") for commits in this repository to "Audit Not Required". This '.
-  'operation destroys information and can not be undone! Are you sure '.
-  'you want to proceed?');
-if (!$ok) {
-  echo "OK, aborting.\n";
-  die(1);
-}
-
-echo "Loading commits...\n";
-$all_commits = id(new PhabricatorRepositoryCommit())->loadAllWhere(
-  'repositoryID = %d',
-  $repository->getID());
-
-echo "Clearing audit requests...\n";
-
-foreach ($all_commits as $commit) {
-  $query = id(new PhabricatorAuditQuery())
-    ->withStatus(PhabricatorAuditQuery::STATUS_OPEN)
-    ->withCommitPHIDs(array($commit->getPHID()));
-  $requests = $query->execute();
-
-  echo "Clearing ".$commit->getPHID()."... ";
-
-  if (!$requests) {
-    echo "nothing to do.\n";
-    continue;
-  } else {
-    echo count($requests)." requests to clear";
-  }
-
-  foreach ($requests as $request) {
-    $request->setAuditStatus(
-      PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED);
-    $request->save();
-    echo ".";
-  }
-
-  $commit->setAuditStatus(PhabricatorAuditCommitStatusConstants::NONE);
-  $commit->save();
-  echo "\n";
-}
-
-echo "Done.\n";
+echo "This script has been replaced with `bin/audit`.\n";
+exit(1);

scripts/setup/manage_audit.php

@@ -0,0 +1,22 @@
+#!/usr/bin/env php
+<?php
+
+$root = dirname(dirname(dirname(__FILE__)));
+require_once $root.'/scripts/__init_script__.php';
+
+$args = new PhutilArgumentParser($argv);
+$args->setTagline('manage audits');
+$args->setSynopsis(<<<EOSYNOPSIS
+**audit** __command__ [__options__]
+    Manage Phabricator audits.
+
+EOSYNOPSIS
+  );
+$args->parseStandardArguments();
+
+$workflows = array(
+  new PhabricatorAuditManagementDeleteWorkflow(),
+  new PhutilHelpArgumentWorkflow(),
+);
+
+$args->parseWorkflows($workflows);

src/__phutil_library_map__.php

@@ -862,6 +862,8 @@ phutil_register_library_map(array(
     'PhabricatorAuditListController' => 'applications/audit/controller/PhabricatorAuditListController.php',
     'PhabricatorAuditListView' => 'applications/audit/view/PhabricatorAuditListView.php',
     'PhabricatorAuditMailReceiver' => 'applications/audit/mail/PhabricatorAuditMailReceiver.php',
+    'PhabricatorAuditManagementDeleteWorkflow' => 'applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php',
+    'PhabricatorAuditManagementWorkflow' => 'applications/audit/management/PhabricatorAuditManagementWorkflow.php',
     'PhabricatorAuditPreviewController' => 'applications/audit/controller/PhabricatorAuditPreviewController.php',
     'PhabricatorAuditQuery' => 'applications/audit/query/PhabricatorAuditQuery.php',
     'PhabricatorAuditReplyHandler' => 'applications/audit/mail/PhabricatorAuditReplyHandler.php',
@@ -2884,6 +2886,8 @@ phutil_register_library_map(array(
     'PhabricatorAuditListController' => 'PhabricatorAuditController',
     'PhabricatorAuditListView' => 'AphrontView',
     'PhabricatorAuditMailReceiver' => 'PhabricatorObjectMailReceiver',
+    'PhabricatorAuditManagementDeleteWorkflow' => 'PhabricatorAuditManagementWorkflow',
+    'PhabricatorAuditManagementWorkflow' => 'PhutilArgumentWorkflow',
     'PhabricatorAuditPreviewController' => 'PhabricatorAuditController',
     'PhabricatorAuditReplyHandler' => 'PhabricatorMailReplyHandler',
     'PhabricatorAuthAccountView' => 'AphrontView',

src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php

@@ -0,0 +1,282 @@
+<?php
+
+final class PhabricatorAuditManagementDeleteWorkflow
+  extends PhabricatorAuditManagementWorkflow {
+
+  public function didConstruct() {
+    $this
+      ->setName('delete')
+      ->setExamples('**delete** [--dry-run] ...')
+      ->setSynopsis('Delete audit requests matching parameters.')
+      ->setArguments(
+        array(
+          array(
+            'name' => 'dry-run',
+            'help' => 'Show what would be deleted, but do not actually delete '.
+                      'anything.',
+          ),
+          array(
+            'name' => 'users',
+            'param' => 'names',
+            'help' => 'Select only audits by a given list of users.',
+          ),
+          array(
+            'name' => 'repositories',
+            'param' => 'repos',
+            'help' => 'Select only audits in a given list of repositories.',
+          ),
+          array(
+            'name' => 'commits',
+            'param' => 'commits',
+            'help' => 'Select only audits for the given commits.',
+          ),
+          array(
+            'name' => 'min-commit-date',
+            'param' => 'date',
+            'help' => 'Select only audits for commits on or after the given '.
+                      'date.',
+          ),
+          array(
+            'name' => 'max-commit-date',
+            'param' => 'date',
+            'help' => 'Select only audits for commits on or before the given '.
+                      'date.',
+          ),
+          array(
+            'name' => 'status',
+            'param' => 'status',
+            'help' => 'Select only audits in the given status. By default, '.
+                      'only open audits are selected.',
+          ),
+          array(
+            'name' => 'ids',
+            'param' => 'ids',
+            'help' => 'Select only audits with the given IDs.',
+          ),
+        ));
+  }
+
+  public function execute(PhutilArgumentParser $args) {
+    $viewer = $this->getViewer();
+    $users = $this->loadUsers($args->getArg('users'));
+    $repos = $this->loadRepos($args->getArg('repositories'));
+    $commits = $this->loadCommits($args->getArg('commits'));
+    $ids = $this->parseList($args->getArg('ids'));
+
+    $status = $args->getArg('status');
+    if (!$status) {
+      $status = PhabricatorAuditQuery::STATUS_OPEN;
+    }
+
+    $min_date = $this->loadDate($args->getArg('min-commit-date'));
+    $max_date = $this->loadDate($args->getArg('max-commit-date'));
+    if ($min_date && $max_date && ($min_date > $max_date)) {
+      throw new PhutilArgumentUsageException(
+        "Specified max date must come after specified min date.");
+    }
+
+    $is_dry_run = $args->getArg('dry-run');
+
+    $query = id(new PhabricatorAuditQuery())
+      ->needCommits(true);
+
+    if ($status) {
+      $query->withStatus($status);
+    }
+
+    if ($ids) {
+      $query->withIDs($ids);
+    }
+
+    if ($repos) {
+      $query->withRepositoryPHIDs(mpull($repos, 'getPHID'));
+    }
+
+    if ($users) {
+      $query->withAuditorPHIDs(mpull($users, 'getPHID'));
+    }
+
+    if ($commits) {
+      $query->withCommitPHIDs(mpull($commits, 'getPHID'));
+    }
+
+    $audits = $query->execute();
+    $commits = $query->getCommits();
+
+    // TODO: AuditQuery is currently not policy-aware and uses an old query
+    // to load commits. Load them in the modern way to get repositories. Remove
+    // this after modernizing PhabricatorAuditQuery.
+    $commits = id(new DiffusionCommitQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(mpull($commits, 'getPHID'))
+      ->execute();
+    $commits = mpull($commits, null, 'getPHID');
+
+    foreach ($audits as $key => $audit) {
+      $commit = idx($commits, $audit->getCommitPHID());
+      if (!$commit) {
+        unset($audits[$key]);
+        continue;
+      }
+
+      if ($min_date && $commit->getEpoch() < $min_date) {
+        unset($audits[$key]);
+        continue;
+      }
+
+      if ($max_date && $commit->getEpoch() > $max_date) {
+        unset($audits[$key]);
+        continue;
+      }
+    }
+
+    $console = PhutilConsole::getConsole();
+
+    if (!$audits) {
+      $console->writeErr("%s\n", pht("No audits match the query."));
+      return 0;
+    }
+
+    $handles = id(new PhabricatorHandleQuery())
+      ->setViewer($this->getViewer())
+      ->withPHIDs(mpull($audits, 'getAuditorPHID'))
+      ->execute();
+
+
+    foreach ($audits as $audit) {
+      $commit = idx($commits, $audit->getCommitPHID());
+
+      $console->writeOut(
+        "%s\n",
+        sprintf(
+          "%10d %-16s %-16s %s: %s",
+          $audit->getID(),
+          $handles[$audit->getAuditorPHID()]->getName(),
+          PhabricatorAuditStatusConstants::getStatusName(
+            $audit->getAuditStatus()),
+          $commit->getRepository()->formatCommitName(
+            $commit->getCommitIdentifier()),
+          trim($commit->getSummary())));
+    }
+
+    if (!$is_dry_run) {
+      $message = pht(
+        'Really delete these %d audit(s)? They will be permanently deleted '.
+        'and can not be recovered.',
+        count($audits));
+      if ($console->confirm($message)) {
+        foreach ($audits as $audit) {
+          $id = $audit->getID();
+          $console->writeOut("%s\n", pht("Deleting audit %d...", $id));
+          $audit->delete();
+        }
+      }
+    }
+
+    return 0;
+  }
+
+  private function getViewer() {
+    return PhabricatorUser::getOmnipotentUser();
+  }
+
+  private function loadUsers($users) {
+    $users = $this->parseList($users);
+    if (!$users) {
+      return null;
+    }
+
+    $objects = id(new PhabricatorPeopleQuery())
+      ->setViewer($this->getViewer())
+      ->withUsernames($users)
+      ->execute();
+    $objects = mpull($objects, null, 'getUsername');
+
+    foreach ($users as $name) {
+      if (empty($objects[$name])) {
+        throw new PhutilArgumentUsageException(
+          pht('No such user with username "%s"!', $name));
+      }
+    }
+
+    return $objects;
+  }
+
+  private function parseList($list) {
+    $list = preg_split('/\s*,\s*/', $list);
+
+    foreach ($list as $key => $item) {
+      $list[$key] = trim($item);
+    }
+
+    foreach ($list as $key => $item) {
+      if (!strlen($item)) {
+        unset($list[$key]);
+      }
+    }
+
+    return $list;
+  }
+
+  private function loadRepos($callsigns) {
+    $callsigns = $this->parseList($callsigns);
+    if (!$callsigns) {
+      return null;
+    }
+
+    $repos = id(new PhabricatorRepositoryQuery())
+      ->setViewer($this->getViewer())
+      ->withCallsigns($callsigns)
+      ->execute();
+    $repos = mpull($repos, null, 'getCallsign');
+
+    foreach ($callsigns as $sign) {
+      if (empty($repos[$sign])) {
+        throw new PhutilArgumentUsageException(
+          pht('No such repository with callsign "%s"!', $sign));
+      }
+    }
+
+    return $repos;
+  }
+
+  private function loadDate($date) {
+    if (!$date) {
+      return null;
+    }
+
+    $epoch = strtotime($date);
+    if (!$epoch || $epoch < 1) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Unable to parse date "%s". Use a format like "2000-01-01".',
+          $date));
+    }
+
+    return $epoch;
+  }
+
+  private function loadCommits($commits) {
+    $names = $this->parseList($commits);
+    if (!$names) {
+      return null;
+    }
+
+    $query = id(new DiffusionCommitQuery())
+      ->setViewer($this->getViewer())
+      ->withIdentifiers($names);
+
+    $commits = $query->execute();
+
+    $map = $query->getIdentifierMap();
+    foreach ($names as $name) {
+      if (empty($map[$name])) {
+        throw new PhutilArgumentUsageException(
+          pht('No such commit "%s"!', $name));
+      }
+    }
+
+    return $commits;
+  }
+
+}

src/applications/audit/management/PhabricatorAuditManagementWorkflow.php

@@ -0,0 +1,10 @@
+<?php
+
+abstract class PhabricatorAuditManagementWorkflow
+  extends PhutilArgumentWorkflow {
+
+  public function isExecutable() {
+    return true;
+  }
+
+}

src/applications/audit/query/PhabricatorAuditQuery.php

@@ -2,6 +2,7 @@
 
 final class PhabricatorAuditQuery {
 
+  private $ids;
   private $offset;
   private $limit;
 
@@ -22,6 +23,11 @@ final class PhabricatorAuditQuery {
 
   private $commits;
 
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
   public function withCommitPHIDs(array $commit_phids) {
     $this->commitPHIDs = $commit_phids;
     return $this;
@@ -156,6 +162,13 @@ final class PhabricatorAuditQuery {
   private function buildWhereClause($conn_r) {
     $where = array();
 
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'req.id IN (%Ld)',
+        $this->ids);
+    }
+
     if ($this->commitPHIDs) {
       $where[] = qsprintf(
         $conn_r,
@@ -212,7 +225,14 @@ final class PhabricatorAuditQuery {
       case self::STATUS_ANY:
         break;
       default:
-        throw new Exception("Unknown status '{$status}'!");
+        $valid = array(
+          self::STATUS_ANY,
+          self::STATUS_OPEN,
+          self::STATUS_CONCERN,
+        );
+        throw new Exception(
+          "Unknown audit status '{$status}'! Valid statuses are: ".
+          implode(', ', $valid));
     }
 
     if ($where) {