From 86989c9f98ad2d56cfef18f25b606efafda25b9e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 5 Aug 2013 10:35:01 -0700 Subject: [PATCH] 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 | 1 + scripts/repository/audit.php | 82 +---- scripts/setup/manage_audit.php | 22 ++ src/__phutil_library_map__.php | 4 + ...abricatorAuditManagementDeleteWorkflow.php | 282 ++++++++++++++++++ .../PhabricatorAuditManagementWorkflow.php | 10 + .../audit/query/PhabricatorAuditQuery.php | 22 +- 7 files changed, 342 insertions(+), 81 deletions(-) create mode 120000 bin/audit create mode 100755 scripts/setup/manage_audit.php create mode 100644 src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php create mode 100644 src/applications/audit/management/PhabricatorAuditManagementWorkflow.php diff --git a/bin/audit b/bin/audit new file mode 120000 index 0000000000..26939acdec --- /dev/null +++ b/bin/audit @@ -0,0 +1 @@ +../scripts/setup/manage_audit.php \ No newline at end of file diff --git a/scripts/repository/audit.php b/scripts/repository/audit.php index 2ebf249579..c91e05f2a6 100755 --- a/scripts/repository/audit.php +++ b/scripts/repository/audit.php @@ -1,83 +1,5 @@ #!/usr/bin/env php setTagline('manage open Audit requests'); -$args->setSynopsis(<<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); diff --git a/scripts/setup/manage_audit.php b/scripts/setup/manage_audit.php new file mode 100755 index 0000000000..8502db2674 --- /dev/null +++ b/scripts/setup/manage_audit.php @@ -0,0 +1,22 @@ +#!/usr/bin/env php +setTagline('manage audits'); +$args->setSynopsis(<<parseStandardArguments(); + +$workflows = array( + new PhabricatorAuditManagementDeleteWorkflow(), + new PhutilHelpArgumentWorkflow(), +); + +$args->parseWorkflows($workflows); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e0f995b679..d3852bec3b 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php b/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php new file mode 100644 index 0000000000..67c90ffaf1 --- /dev/null +++ b/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php @@ -0,0 +1,282 @@ +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; + } + +} diff --git a/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php b/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php new file mode 100644 index 0000000000..99ad58fc5e --- /dev/null +++ b/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php @@ -0,0 +1,10 @@ +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) {