Migrate "PhabricatorOwnersPackageCommitRelationship" to "PhabricatorRepositoryAuditRequest"

Summary: - Move table to Repository, since we have no Owners joins in the application anymore but would like to do a Repository join. - Rename "packagePHID" to "auditorPHID", since this column may contain package, project, or user PHIDs. Test Plan: - Browsed Owners, Audit, and Differential interfaces to the Audit tool. - Made comments and state changes. - Ran "reparse.php --herald --owners" on several commits. Reviewers: btrahan, jungejason Reviewed By: btrahan CC: aran, epriestley, nh, vrana Maniphest Tasks: T904 Differential Revision: https://secure.phabricator.com/D1787
2024-11-10 00:42:41 +01:00 · 2012-03-05 13:17:29 -08:00 · 2012-03-05 13:17:29 -08:00 · d94129b739
commit d94129b739
parent 3a251f9b16
19 changed files with 113 additions and 94 deletions
--- a/resources/sql/patches/114.auditrequest.sql
+++ b/resources/sql/patches/114.auditrequest.sql
@ -0,0 +1,16 @@
+CREATE TABLE phabricator_repository.repository_auditrequest (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  auditorPHID varchar(64) BINARY NOT NULL,
+  commitPHID varchar(64) BINARY NOT NULL,
+  auditStatus varchar(64) NOT NULL,
+  auditReasons LONGBLOB NOT NULL,
+  KEY (commitPHID),
+  KEY (auditorPHID, auditStatus)
+) ENGINE=InnoDB;
+
+INSERT INTO phabricator_repository.repository_auditrequest
+    (auditorPHID, commitPHID, auditStatus, auditReasons)
+  SELECT packagePHID, commitPHID, auditStatus, auditReasons
+    FROM phabricator_owners.owners_packagecommitrelationship;
+
+DROP TABLE phabricator_owners.owners_packagecommitrelationship;
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -666,7 +666,6 @@ phutil_register_library_map(array(
    'PhabricatorOwnersListController' => 'applications/owners/controller/list',
    'PhabricatorOwnersOwner' => 'applications/owners/storage/owner',
    'PhabricatorOwnersPackage' => 'applications/owners/storage/package',
-    'PhabricatorOwnersPackageCommitRelationship' => 'applications/owners/storage/packagecommitrelationship',
    'PhabricatorOwnersPath' => 'applications/owners/storage/path',
    'PhabricatorPHID' => 'applications/phid/storage/phid',
    'PhabricatorPHIDConstants' => 'applications/phid/constants',
@ -722,6 +721,7 @@ phutil_register_library_map(array(
    'PhabricatorRepository' => 'applications/repository/storage/repository',
    'PhabricatorRepositoryArcanistProject' => 'applications/repository/storage/arcanistproject',
    'PhabricatorRepositoryArcanistProjectEditController' => 'applications/repository/controller/arcansistprojectedit',
+    'PhabricatorRepositoryAuditRequest' => 'applications/repository/storage/auditrequest',
    'PhabricatorRepositoryCommit' => 'applications/repository/storage/commit',
    'PhabricatorRepositoryCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/base',
    'PhabricatorRepositoryCommitData' => 'applications/repository/storage/commitdata',
@ -1424,7 +1424,6 @@ phutil_register_library_map(array(
    'PhabricatorOwnersListController' => 'PhabricatorOwnersController',
    'PhabricatorOwnersOwner' => 'PhabricatorOwnersDAO',
    'PhabricatorOwnersPackage' => 'PhabricatorOwnersDAO',
-    'PhabricatorOwnersPackageCommitRelationship' => 'PhabricatorOwnersDAO',
    'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO',
    'PhabricatorPHID' => 'PhabricatorPHIDDAO',
    'PhabricatorPHIDController' => 'PhabricatorController',
@ -1473,6 +1472,7 @@ phutil_register_library_map(array(
    'PhabricatorRepository' => 'PhabricatorRepositoryDAO',
    'PhabricatorRepositoryArcanistProject' => 'PhabricatorRepositoryDAO',
    'PhabricatorRepositoryArcanistProjectEditController' => 'PhabricatorRepositoryController',
+    'PhabricatorRepositoryAuditRequest' => 'PhabricatorRepositoryDAO',
    'PhabricatorRepositoryCommit' => 'PhabricatorRepositoryDAO',
    'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
    'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO',
--- a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php
+++ b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php
@ -54,7 +54,7 @@ final class PhabricatorAuditCommentEditor {
    $audit_phids = self::loadAuditPHIDsForUser($this->user);
    $audit_phids = array_fill_keys($audit_phids, true);

-    $relationships = id(new PhabricatorOwnersPackageCommitRelationship())
+    $requests = id(new PhabricatorRepositoryAuditRequest())
      ->loadAllWhere(
        'commitPHID = %s',
        $commit->getPHID());
@ -65,34 +65,34 @@ final class PhabricatorAuditCommentEditor {

    // Status may be empty for updates which don't affect status, like
    // "comment".
-    $have_any_relationship = false;
-    foreach ($relationships as $relationship) {
-      if (empty($audit_phids[$relationship->getPackagePHID()])) {
+    $have_any_requests = false;
+    foreach ($requests as $request) {
+      if (empty($audit_phids[$request->getAuditorPHID()])) {
        continue;
      }
-      $have_any_relationship = true;
+      $have_any_requests = true;
      if ($status) {
-        $relationship->setAuditStatus($status);
-        $relationship->save();
+        $request->setAuditStatus($status);
+        $request->save();
      }
    }

-    if (!$have_any_relationship) {
+    if (!$have_any_requests) {
      // If the user has no current authority over any audit trigger, make a
      // new one to represent their audit state.
-      $relationship = id(new PhabricatorOwnersPackageCommitRelationship())
+      $request = id(new PhabricatorRepositoryAuditRequest())
        ->setCommitPHID($commit->getPHID())
-        ->setPackagePHID($user->getPHID())
+        ->setAuditorPHID($user->getPHID())
        ->setAuditStatus(
            $status
              ? $status
              : PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED)
        ->setAuditReasons(array("Voluntary Participant"))
        ->save();
-      $relationships[] = $relationship;
+      $requests[] = $request;
    }

-    $commit->updateAuditStatus($relationships);
+    $commit->updateAuditStatus($requests);
    $commit->save();

    $this->publishFeedStory($comment, array_keys($audit_phids));
--- a/src/applications/audit/editor/comment/init.php
+++ b/src/applications/audit/editor/comment/init.php
@ -7,16 +7,16 @@


 phutil_require_module('phabricator', 'applications/audit/constants/action');
-phutil_require_module('phabricator', 'applications/feed/constants/story');
-phutil_require_module('phabricator', 'applications/feed/publisher');
 phutil_require_module('phabricator', 'applications/audit/constants/status');
 phutil_require_module('phabricator', 'applications/audit/storage/auditcomment');
+phutil_require_module('phabricator', 'applications/feed/constants/story');
+phutil_require_module('phabricator', 'applications/feed/publisher');
 phutil_require_module('phabricator', 'applications/metamta/storage/mail');
 phutil_require_module('phabricator', 'applications/owners/storage/owner');
 phutil_require_module('phabricator', 'applications/owners/storage/package');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'applications/project/query/project');
+phutil_require_module('phabricator', 'applications/repository/storage/auditrequest');
 phutil_require_module('phabricator', 'applications/search/index/indexer/repository');
 phutil_require_module('phabricator', 'infrastructure/env');

--- a/src/applications/audit/query/audit/PhabricatorAuditQuery.php
+++ b/src/applications/audit/query/audit/PhabricatorAuditQuery.php
@ -54,7 +54,7 @@ final class PhabricatorAuditQuery {
  }

  public function execute() {
-    $table = new PhabricatorOwnersPackageCommitRelationship();
+    $table = new PhabricatorRepositoryAuditRequest();
    $conn_r = $table->establishConnection('r');

    $where = $this->buildWhereClause($conn_r);
@ -85,7 +85,7 @@ final class PhabricatorAuditQuery {
    if ($this->auditorPHIDs) {
      $where[] = qsprintf(
        $conn_r,
-        'packagePHID IN (%Ls)',
+        'auditorPHID IN (%Ls)',
        $this->auditorPHIDs);
    }

--- a/src/applications/audit/query/audit/init.php
+++ b/src/applications/audit/query/audit/init.php
@ -7,7 +7,7 @@


 phutil_require_module('phabricator', 'applications/audit/constants/status');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
+phutil_require_module('phabricator', 'applications/repository/storage/auditrequest');
 phutil_require_module('phabricator', 'storage/qsprintf');
 phutil_require_module('phabricator', 'storage/queryfx');

--- a/src/applications/audit/query/commit/PhabricatorAuditCommitQuery.php
+++ b/src/applications/audit/query/commit/PhabricatorAuditCommitQuery.php
@ -69,11 +69,13 @@ final class PhabricatorAuditCommitQuery {
      // on the same database as the commits. Once they're migrated we can
      // resolve this via JOIN.

-      $table = new PhabricatorOwnersPackageCommitRelationship();
+      // TODO: Clean this up now that we've moved the table.
+
+      $table = new PhabricatorRepositoryAuditRequest();
      $conn_r = $table->establishConnection('r');
      $phids = queryfx_all(
        $conn_r,
-        'SELECT DISTINCT commitPHID FROM %T WHERE packagePHID IN (%Ls)
+        'SELECT DISTINCT commitPHID FROM %T WHERE auditorPHID IN (%Ls)
          ORDER BY id DESC %Q',
        $table->getTableName(),
        $this->packagePHIDs,
--- a/src/applications/audit/query/commit/init.php
+++ b/src/applications/audit/query/commit/init.php
@ -7,7 +7,7 @@


 phutil_require_module('phabricator', 'applications/audit/constants/commitstatus');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
+phutil_require_module('phabricator', 'applications/repository/storage/auditrequest');
 phutil_require_module('phabricator', 'applications/repository/storage/commit');
 phutil_require_module('phabricator', 'applications/repository/storage/commitdata');
 phutil_require_module('phabricator', 'storage/qsprintf');
--- a/src/applications/audit/view/list/PhabricatorAuditListView.php
+++ b/src/applications/audit/view/list/PhabricatorAuditListView.php
@ -51,7 +51,7 @@ final class PhabricatorAuditListView extends AphrontView {
    $phids = array();
    foreach ($this->audits as $audit) {
      $phids[$audit->getCommitPHID()] = true;
-      $phids[$audit->getPackagePHID()] = true;
+      $phids[$audit->getAuditorPHID()] = true;
    }
    return array_keys($phids);
  }
@ -90,7 +90,7 @@ final class PhabricatorAuditListView extends AphrontView {
      $status_code = $audit->getAuditStatus();
      $status = PhabricatorAuditStatusConstants::getStatusName($status_code);

-      $auditor_handle = $this->getHandle($audit->getPackagePHID());
+      $auditor_handle = $this->getHandle($audit->getAuditorPHID());
      $rows[] = array(
        $commit_name,
        $auditor_handle->renderLink(),
@ -98,7 +98,7 @@ final class PhabricatorAuditListView extends AphrontView {
        $reasons,
      );

-      if (empty($authority[$audit->getPackagePHID()])) {
+      if (empty($authority[$audit->getAuditorPHID()])) {
        $rowc[] = null;
      } else {
        $rowc[] = 'highlighted';
--- a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php
+++ b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php
@ -87,17 +87,17 @@ class HeraldCommitAdapter extends HeraldObjectAdapter {

  public function loadAuditNeededPackage() {
    if ($this->auditNeededPackages === null) {
-      $status_arr = array (
+      $status_arr = array(
        PhabricatorAuditStatusConstants::AUDIT_REQUIRED,
-          PhabricatorAuditStatusConstants::CONCERNED,
-        );
-      $relationships = id(new PhabricatorOwnersPackageCommitRelationship())
+        PhabricatorAuditStatusConstants::CONCERNED,
+      );
+      $requests = id(new PhabricatorRepositoryAuditRequest())
          ->loadAllWhere(
        "commitPHID = %s AND auditStatus IN (%Ls)",
        $this->commit->getPHID(),
        $status_arr);

-      $packages = mpull($relationships, 'getPackagePHID');
+      $packages = mpull($requests, 'getAuditorPHID');
      $this->auditNeededPackages = $packages;
    }
    return $this->auditNeededPackages;
--- a/src/applications/herald/adapter/commit/init.php
+++ b/src/applications/herald/adapter/commit/init.php
@ -16,7 +16,7 @@ phutil_require_module('phabricator', 'applications/herald/storage/transcript/app
 phutil_require_module('phabricator', 'applications/owners/query/path');
 phutil_require_module('phabricator', 'applications/owners/storage/owner');
 phutil_require_module('phabricator', 'applications/owners/storage/package');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
+phutil_require_module('phabricator', 'applications/repository/storage/auditrequest');

 phutil_require_module('phutil', 'utils');

--- a/src/applications/owners/storage/packagecommitrelationship/init.php
+++ b/src/applications/owners/storage/packagecommitrelationship/init.php
@ -1,12 +0,0 @@
-<?php
-/**
- * This file is automatically generated. Lint this module to rebuild it.
- * @generated
- */
-
-
-
-phutil_require_module('phabricator', 'applications/owners/storage/base');
-
-
-phutil_require_source('PhabricatorOwnersPackageCommitRelationship.php');
--- a/src/applications/owners/storage/packagecommitrelationship/PhabricatorOwnersPackageCommitRelationship.php
+++ b/src/applications/owners/storage/packagecommitrelationship/PhabricatorOwnersPackageCommitRelationship.php
@ -1,7 +1,7 @@
 <?php

 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@ -16,9 +16,9 @@
 * limitations under the License.
 */

-class PhabricatorOwnersPackageCommitRelationship extends PhabricatorOwnersDAO {
+final class PhabricatorRepositoryAuditRequest extends PhabricatorRepositoryDAO {

-  protected $packagePHID;
+  protected $auditorPHID;
  protected $commitPHID;
  protected $auditReasons = array();
  protected $auditStatus;
--- a/src/applications/repository/storage/auditrequest/init.php
+++ b/src/applications/repository/storage/auditrequest/init.php
@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/repository/storage/base');
+
+
+phutil_require_source('PhabricatorRepositoryAuditRequest.php');
--- a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php
@ -85,13 +85,13 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO {
   * Synchronize a commit's overall audit status with the individual audit
   * triggers.
   */
-  public function updateAuditStatus(array $rships) {
+  public function updateAuditStatus(array $requests) {
    $any_concern = false;
    $any_accept = false;
    $any_need = false;

-    foreach ($rships as $rship) {
-      switch ($rship->getAuditStatus()) {
+    foreach ($requests as $request) {
+      switch ($request->getAuditStatus()) {
        case PhabricatorAuditStatusConstants::AUDIT_REQUIRED:
          $any_need = true;
          break;
--- a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php
+++ b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php
@ -173,16 +173,16 @@ EOBODY;
    array $map,
    array $rules) {

-    $table = new PhabricatorOwnersPackageCommitRelationship();
-    $rships = $table->loadAllWhere(
+    $table = new PhabricatorRepositoryAuditRequest();
+    $requests = $table->loadAllWhere(
      'commitPHID = %s',
      $commit->getPHID());
-    $rships = mpull($rships, null, 'getPackagePHID');
+    $requests = mpull($requests, null, 'getAuditorPHID');

    $rules = mpull($rules, null, 'getID');
    foreach ($map as $phid => $rule_ids) {
-      $rship = idx($rships, $phid);
-      if ($rship) {
+      $request = idx($requests, $phid);
+      if ($request) {
        continue;
      }
      $reasons = array();
@ -194,15 +194,15 @@ EOBODY;
        $reasons[] = 'Herald Rule #'.$id.' "'.$rule_name.'" Triggered Audit';
      }

-      $rship = new PhabricatorOwnersPackageCommitRelationship();
-      $rship->setCommitPHID($commit->getPHID());
-      $rship->setPackagePHID($phid);
-      $rship->setAuditStatus(PhabricatorAuditStatusConstants::AUDIT_REQUIRED);
-      $rship->setAuditReasons($reasons);
-      $rship->save();
+      $request = new PhabricatorRepositoryAuditRequest();
+      $request->setCommitPHID($commit->getPHID());
+      $request->setAuditorPHID($phid);
+      $request->setAuditStatus(PhabricatorAuditStatusConstants::AUDIT_REQUIRED);
+      $request->setAuditReasons($reasons);
+      $request->save();
    }

-    $commit->updateAuditStatus($rships);
+    $commit->updateAuditStatus($requests);
    $commit->save();
  }
 }
--- a/src/applications/repository/worker/herald/init.php
+++ b/src/applications/repository/worker/herald/init.php
@ -13,8 +13,8 @@ phutil_require_module('phabricator', 'applications/herald/config/contenttype');
 phutil_require_module('phabricator', 'applications/herald/engine/engine');
 phutil_require_module('phabricator', 'applications/herald/storage/rule');
 phutil_require_module('phabricator', 'applications/metamta/storage/mail');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
+phutil_require_module('phabricator', 'applications/repository/storage/auditrequest');
 phutil_require_module('phabricator', 'applications/repository/storage/commitdata');
 phutil_require_module('phabricator', 'applications/repository/worker/base');
 phutil_require_module('phabricator', 'infrastructure/env');
--- a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php
@ -30,44 +30,45 @@ class PhabricatorRepositoryCommitOwnersWorker
      $affected_paths);

    if ($affected_packages) {
-      $rships = id(new PhabricatorOwnersPackageCommitRelationship())
+      $requests = id(new PhabricatorRepositoryAuditRequest())
        ->loadAllWhere(
          'commitPHID = %s',
          $commit->getPHID());
-      $rships = mpull($rships, null, 'getPackagePHID');
+      $requests = mpull($requests, null, 'getAuditorPHID');

      foreach ($affected_packages as $package) {
-        $relationship = idx($rships, $package->getPHID());
-
-        // Don't update relationship if it exists already
-        if (!$relationship) {
-          if ($package->getAuditingEnabled()) {
-            $reasons = $this->checkAuditReasons($commit, $package);
-            if ($reasons) {
-              $audit_status =
-                PhabricatorAuditStatusConstants::AUDIT_REQUIRED;
-            } else {
-              $audit_status =
-                PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED;
-            }
-          } else {
-            $reasons = array();
-            $audit_status = PhabricatorAuditStatusConstants::NONE;
-          }
-
-          $relationship = new PhabricatorOwnersPackageCommitRelationship();
-          $relationship->setPackagePHID($package->getPHID());
-          $relationship->setCommitPHID($commit->getPHID());
-          $relationship->setAuditReasons($reasons);
-          $relationship->setAuditStatus($audit_status);
-
-          $relationship->save();
-
-          $rships[] = $relationship;
+        $request = idx($requests, $package->getPHID());
+        if ($request) {
+          // Don't update request if it exists already.
+          continue;
        }
+
+        if ($package->getAuditingEnabled()) {
+          $reasons = $this->checkAuditReasons($commit, $package);
+          if ($reasons) {
+            $audit_status =
+              PhabricatorAuditStatusConstants::AUDIT_REQUIRED;
+          } else {
+            $audit_status =
+              PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED;
+          }
+        } else {
+          $reasons = array();
+          $audit_status = PhabricatorAuditStatusConstants::NONE;
+        }
+
+        $relationship = new PhabricatorRepositoryAuditRequest();
+        $relationship->setAuditorPHID($package->getPHID());
+        $relationship->setCommitPHID($commit->getPHID());
+        $relationship->setAuditReasons($reasons);
+        $relationship->setAuditStatus($audit_status);
+
+        $relationship->save();
+
+        $requests[$package->getPHID()] = $relationship;
      }

-      $commit->updateAuditStatus($rships);
+      $commit->updateAuditStatus($requests);
      $commit->save();
    }

--- a/src/applications/repository/worker/owner/init.php
+++ b/src/applications/repository/worker/owner/init.php
@ -11,7 +11,7 @@ phutil_require_module('phabricator', 'applications/differential/storage/revision
 phutil_require_module('phabricator', 'applications/owners/query/path');
 phutil_require_module('phabricator', 'applications/owners/storage/owner');
 phutil_require_module('phabricator', 'applications/owners/storage/package');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
+phutil_require_module('phabricator', 'applications/repository/storage/auditrequest');
 phutil_require_module('phabricator', 'applications/repository/storage/commitdata');
 phutil_require_module('phabricator', 'applications/repository/worker/base');
 phutil_require_module('phabricator', 'infrastructure/daemon/workers/storage/task');