Migrate the "badcommit" table to use the less-hacky "hint" mechanism

Summary: Ref T11522. This migrates any "badcommit" data (which probably only exists at Facebook and on 1-2 other installs in the wild) to the new "hint" table.

Test Plan:
  - Wrote some bad commit annotations to the badcommit table.
  - Viewed them in the web UI and used `bin/repository reparse --change ...` to reparse them. Saw "this is bad" messages.
  - Ran migration, verified that valid "badcommit" rows were successfully migrated to become "hint" rows.
  - Viewed the new web UI and re-parsed the change, saw "unreadable commit" messages.
  - Viewed a good commit; reparsed a good commit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11522

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

resources/sql/autopatches/20160824.repohint.02.movebad.php

@@ -0,0 +1,39 @@
+<?php
+
+$table = new PhabricatorRepositoryCommit();
+$conn = $table->establishConnection('w');
+
+$rows = queryfx_all(
+  $conn,
+  'SELECT fullCommitName FROM repository_badcommit');
+
+$viewer = PhabricatorUser::getOmnipotentUser();
+
+foreach ($rows as $row) {
+  $identifier = $row['fullCommitName'];
+
+  $commit = id(new DiffusionCommitQuery())
+    ->setViewer($viewer)
+    ->withIdentifiers(array($identifier))
+    ->executeOne();
+
+  if (!$commit) {
+    echo tsprintf(
+      "%s\n",
+      pht(
+        'Skipped hint for "%s", this is not a valid commit.',
+        $identifier));
+  } else {
+    PhabricatorRepositoryCommitHint::updateHint(
+      $commit->getRepository()->getPHID(),
+      $commit->getCommitIdentifier(),
+      null,
+      PhabricatorRepositoryCommitHint::HINT_UNREADABLE);
+
+    echo tsprintf(
+      "%s\n",
+      pht(
+        'Updated commit hint for "%s".',
+        $identifier));
+  }
+}

resources/sql/autopatches/20160824.repohint.03.nukebad.sql

@@ -0,0 +1 @@
+DROP TABLE {$NAMESPACE}_repository.repository_badcommit;

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -167,23 +167,29 @@ final class DiffusionCommitController extends DiffusionController {
 
     $count = count($changes);
 
-    $bad_commit = null;
+    $is_unreadable = false;
-    if ($count == 0) {
+    if (!$count) {
-      $bad_commit = queryfx_one(
+      $hint = id(new DiffusionCommitHintQuery())
-        id(new PhabricatorRepository())->establishConnection('r'),
+        ->setViewer($viewer)
-        'SELECT * FROM %T WHERE fullCommitName = %s',
+        ->withRepositoryPHIDs(array($repository->getPHID()))
-        PhabricatorRepository::TABLE_BADCOMMIT,
+        ->withOldCommitIdentifiers(array($commit->getCommitIdentifier()))
-        $commit->getMonogram());
+        ->executeOne();
+      if ($hint) {
+        $is_unreadable = $hint->isUnreadable();
+      }
     }
 
     $show_changesets = false;
     $info_panel = null;
     $change_list = null;
     $change_table = null;
-    if ($bad_commit) {
+    if ($is_unreadable) {
       $info_panel = $this->renderStatusMessage(
-        pht('Bad Commit'),
+        pht('Unreadable Commit'),
-        $bad_commit['description']);
+        pht(
+          'This commit has been marked as unreadable by an administrator. '.
+          'It may have been corrupted or created improperly by an external '.
+          'tool.'));
     } else if ($is_foreign) {
       // Don't render anything else.
     } else if (!$commit->isImported()) {

src/applications/diffusion/query/DiffusionCommitHintQuery.php

@@ -43,7 +43,7 @@ final class DiffusionCommitHintQuery
     if ($this->repositoryPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'reositoryPHID IN (%Ls)',
+        'repositoryPHID IN (%Ls)',
         $this->repositoryPHIDs);
     }
 

src/applications/repository/storage/PhabricatorRepository.php

@@ -35,7 +35,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
   const TABLE_PATHCHANGE = 'repository_pathchange';
   const TABLE_FILESYSTEM = 'repository_filesystem';
   const TABLE_SUMMARY = 'repository_summary';
-  const TABLE_BADCOMMIT = 'repository_badcommit';
   const TABLE_LINTMESSAGE = 'repository_lintmessage';
   const TABLE_PARENTS = 'repository_parents';
   const TABLE_COVERAGE = 'repository_coverage';

src/applications/repository/storage/PhabricatorRepositoryCommitHint.php

@@ -101,6 +101,11 @@ final class PhabricatorRepositoryCommitHint
   }
 
 
+  public function isUnreadable() {
+    return ($this->getHintType() == self::HINT_UNREADABLE);
+  }
+
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 

src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php

@@ -6,20 +6,6 @@ final class PhabricatorRepositorySchemaSpec
   public function buildSchemata() {
     $this->buildEdgeSchemata(new PhabricatorRepository());
 
-    $this->buildRawSchema(
-      id(new PhabricatorRepository())->getApplicationName(),
-      PhabricatorRepository::TABLE_BADCOMMIT,
-      array(
-        'fullCommitName' => 'text64',
-        'description' => 'text',
-      ),
-      array(
-        'PRIMARY' => array(
-          'columns' => array('fullCommitName'),
-          'unique' => true,
-        ),
-      ));
-
     $this->buildRawSchema(
       id(new PhabricatorRepository())->getApplicationName(),
       PhabricatorRepository::TABLE_COVERAGE,

src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php

@@ -95,16 +95,16 @@ abstract class PhabricatorRepositoryCommitParserWorker
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit);
 
-  protected function isBadCommit(PhabricatorRepositoryCommit $commit) {
+  protected function loadCommitHint(PhabricatorRepositoryCommit $commit) {
-    $repository = new PhabricatorRepository();
+    $viewer = PhabricatorUser::getOmnipotentUser();
 
-    $bad_commit = queryfx_one(
+    $repository = $commit->getRepository();
-      $repository->establishConnection('w'),
-      'SELECT * FROM %T WHERE fullCommitName = %s',
-      PhabricatorRepository::TABLE_BADCOMMIT,
-      $commit->getMonogram());
 
-    return (bool)$bad_commit;
+    return id(new DiffusionCommitHintQuery())
+      ->setViewer($viewer)
+      ->withRepositoryPHIDs(array($repository->getPHID()))
+      ->withOldCommitIdentifiers(array($commit->getCommitIdentifier()))
+      ->executeOne();
   }
 
   public function renderForDisplay(PhabricatorUser $viewer) {

src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php

@@ -22,8 +22,13 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker
     PhabricatorRepositoryCommit $commit) {
 
     $this->log("%s\n", pht('Parsing "%s"...', $commit->getMonogram()));
-    if ($this->isBadCommit($commit)) {
+
-      $this->log(pht('This commit is marked bad!'));
+    $hint = $this->loadCommitHint($commit);
+    if ($hint && $hint->isUnreadable()) {
+      $this->log(
+        pht(
+          'This commit is marked as unreadable, so changes will not be '.
+          'parsed.'));
       return;
     }