Provide a standalone query for resolution of commit author/committer into Phabricator users

Summary:
Ref T4195. To implement the "Author" and "Committer" rules, I need to resolve author/committer strings into Phabricator users.

The code to do this is currently buried in the daemons. Extract it into a standalone query.

I also added `bin/repository lookup-users <commit>` to test this query, both to improve confidence I'm getting this right and to provide a diagnostic command for users, since there's occasionally some confusion over how author/committer strings resolve into valid users.

Test Plan:
I tested this using `bin/repository lookup-users` and `reparse.php --message` on Git, Mercurial and SVN commits. Here's the `lookup-users` output:

  >>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rINIS3
  Examining commit rINIS3...
  Raw author string: epriestley
  Phabricator user: epriestley (Evan Priestley   )
  Raw committer string: null
  >>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rPOEMS165b6c54f487c8
  Examining commit rPOEMS165b6c54f487...
  Raw author string: epriestley <git@epriestley.com>
  Phabricator user: epriestley (Evan Priestley   )
  Raw committer string: epriestley <git@epriestley.com>
  Phabricator user: epriestley (Evan Priestley   )
  >>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rINIH6d24c1aee7741e
  Examining commit rINIH6d24c1aee774...
  Raw author string: epriestley <hg@yghe.net>
  Phabricator user: epriestley (Evan Priestley   )
  Raw committer string: null
  >>> orbital ~/devtools/phabricator $

The `reparse.php` output was similar, and all VCSes resolved authors correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1731, T4195

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

scripts/repository/manage_repositories.php

@@ -22,6 +22,7 @@ $workflows = array(
   new PhabricatorRepositoryManagementListWorkflow(),
   new PhabricatorRepositoryManagementDeleteWorkflow(),
   new PhabricatorRepositoryManagementMarkImportedWorkflow(),
+  new PhabricatorRepositoryManagementLookupUsersWorkflow(),
   new PhabricatorRepositoryManagementImportingWorkflow(),
   new PhutilHelpArgumentWorkflow(),
 );

src/__phutil_library_map__.php

@@ -554,6 +554,7 @@ phutil_register_library_map(array(
     'DiffusionRepositoryRef' => 'applications/diffusion/data/DiffusionRepositoryRef.php',
     'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php',
     'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php',
+    'DiffusionResolveUserQuery' => 'applications/diffusion/query/DiffusionResolveUserQuery.php',
     'DiffusionSSHGitReceivePackWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitReceivePackWorkflow.php',
     'DiffusionSSHGitUploadPackWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitUploadPackWorkflow.php',
     'DiffusionSSHGitWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitWorkflow.php',
@@ -1794,6 +1795,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryManagementEditWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementEditWorkflow.php',
     'PhabricatorRepositoryManagementImportingWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementImportingWorkflow.php',
     'PhabricatorRepositoryManagementListWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php',
+    'PhabricatorRepositoryManagementLookupUsersWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementLookupUsersWorkflow.php',
     'PhabricatorRepositoryManagementMarkImportedWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementMarkImportedWorkflow.php',
     'PhabricatorRepositoryManagementPullWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php',
     'PhabricatorRepositoryManagementWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementWorkflow.php',
@@ -2935,6 +2937,7 @@ phutil_register_library_map(array(
       1 => 'PhabricatorApplicationSearchResultsControllerInterface',
     ),
     'DiffusionRepositoryNewController' => 'DiffusionController',
+    'DiffusionResolveUserQuery' => 'Phobject',
     'DiffusionSSHGitReceivePackWorkflow' => 'DiffusionSSHGitWorkflow',
     'DiffusionSSHGitUploadPackWorkflow' => 'DiffusionSSHGitWorkflow',
     'DiffusionSSHGitWorkflow' => 'DiffusionSSHWorkflow',
@@ -4362,6 +4365,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryManagementEditWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
     'PhabricatorRepositoryManagementImportingWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
     'PhabricatorRepositoryManagementListWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
+    'PhabricatorRepositoryManagementLookupUsersWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
     'PhabricatorRepositoryManagementMarkImportedWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
     'PhabricatorRepositoryManagementPullWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
     'PhabricatorRepositoryManagementWorkflow' => 'PhutilArgumentWorkflow',

src/applications/diffusion/query/DiffusionResolveUserQuery.php

@@ -0,0 +1,130 @@
+<?php
+
+/**
+ * Resolve an author or committer name, like
+ * `"Abraham Lincoln <alincoln@logcab.in>"`, into a valid Phabricator user
+ * account, like `@alincoln`.
+ */
+final class DiffusionResolveUserQuery extends Phobject {
+
+  private $name;
+  private $commit;
+
+  public function withName($name) {
+    $this->name = $name;
+    return $this;
+  }
+
+  public function withCommit($commit) {
+    $this->commit = $commit;
+    return $this;
+  }
+
+  public function execute() {
+    $user_name = $this->name;
+
+    $phid = $this->findUserPHID($this->name);
+    $phid = $this->fireLookupEvent($phid);
+
+    return $phid;
+  }
+
+  private function findUserPHID($user_name) {
+    if (!strlen($user_name)) {
+      return null;
+    }
+
+    $phid = $this->findUserByUserName($user_name);
+    if ($phid) {
+      return $phid;
+    }
+
+    $phid = $this->findUserByEmailAddress($user_name);
+    if ($phid) {
+      return $phid;
+    }
+
+    $phid = $this->findUserByRealName($user_name);
+    if ($phid) {
+      return $phid;
+    }
+
+    // No hits yet, try to parse it as an email address.
+
+    $email = new PhutilEmailAddress($user_name);
+
+    $phid = $this->findUserByEmailAddress($email->getAddress());
+    if ($phid) {
+      return $phid;
+    }
+
+    $display_name = $email->getDisplayName();
+    if ($display_name) {
+      $phid = $this->findUserByUserName($display_name);
+      if ($phid) {
+        return $phid;
+      }
+
+      $phid = $this->findUserByRealName($display_name);
+      if ($phid) {
+        return $phid;
+      }
+    }
+
+    return null;
+  }
+
+
+  /**
+   * Emit an event so installs can do custom lookup of commit authors who may
+   * not be naturally resolvable.
+   */
+  private function fireLookupEvent($guess) {
+
+    $type = PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER;
+    $data = array(
+      'commit'  => $this->commit,
+      'query'   => $this->name,
+      'result'  => $guess,
+    );
+
+    $event = new PhabricatorEvent($type, $data);
+    PhutilEventEngine::dispatchEvent($event);
+
+    return $event->getValue('result');
+  }
+
+
+  private function findUserByUserName($user_name) {
+    $by_username = id(new PhabricatorUser())->loadOneWhere(
+      'userName = %s',
+      $user_name);
+    if ($by_username) {
+      return $by_username->getPHID();
+    }
+    return null;
+  }
+
+
+  private function findUserByRealName($real_name) {
+    // Note, real names are not guaranteed unique, which is why we do it this
+    // way.
+    $by_realname = id(new PhabricatorUser())->loadAllWhere(
+      'realName = %s',
+      $real_name);
+    if (count($by_realname) == 1) {
+      return reset($by_realname)->getPHID();
+    }
+    return null;
+  }
+
+
+  private function findUserByEmailAddress($email_address) {
+    $by_email = PhabricatorUser::loadOneWithEmailAddress($email_address);
+    if ($by_email) {
+      return $by_email->getPHID();
+    }
+    return null;
+  }
+
+}

src/applications/repository/management/PhabricatorRepositoryManagementLookupUsersWorkflow.php

@@ -0,0 +1,97 @@
+<?php
+
+final class PhabricatorRepositoryManagementLookupUsersWorkflow
+  extends PhabricatorRepositoryManagementWorkflow {
+
+  public function didConstruct() {
+    $this
+      ->setName('lookup-users')
+      ->setExamples('**lookup-users** __commit__ ...')
+      ->setSynopsis('Resolve user accounts for users attached to __commit__.')
+      ->setArguments(
+        array(
+          array(
+            'name'        => 'commits',
+            'wildcard'    => true,
+          ),
+        ));
+  }
+
+  public function execute(PhutilArgumentParser $args) {
+    $commits = $this->loadCommits($args, 'commits');
+    if (!$commits) {
+      throw new PhutilArgumentUsageException(
+        "Specify one or more commits to resolve users for.");
+    }
+
+    $console = PhutilConsole::getConsole();
+    foreach ($commits as $commit) {
+      $repo = $commit->getRepository();
+      $name =  $repo->formatCommitName($commit->getCommitIdentifier());
+
+      $console->writeOut(
+        "%s\n",
+        pht("Examining commit %s...", $name));
+
+      $ref = id(new DiffusionLowLevelCommitQuery())
+        ->setRepository($repo)
+        ->withIdentifier($commit->getCommitIdentifier())
+        ->execute();
+
+      $author = $ref->getAuthor();
+      $console->writeOut(
+        "%s\n",
+        pht('Raw author string: %s', coalesce($author, 'null')));
+
+      if ($author !== null) {
+        $handle = $this->resolveUser($commit, $author);
+        if ($handle) {
+          $console->writeOut(
+            "%s\n",
+            pht('Phabricator user: %s', $handle->getFullName()));
+        } else {
+          $console->writeOut(
+            "%s\n",
+            pht('Unable to resolve a corresponding Phabricator user.'));
+        }
+      }
+
+      $committer = $ref->getCommitter();
+      $console->writeOut(
+        "%s\n",
+        pht('Raw committer string: %s', coalesce($committer, 'null')));
+
+      if ($committer !== null) {
+        $handle = $this->resolveUser($commit, $committer);
+        if ($handle) {
+          $console->writeOut(
+            "%s\n",
+            pht('Phabricator user: %s', $handle->getFullName()));
+        } else {
+          $console->writeOut(
+            "%s\n",
+            pht('Unable to resolve a corresponding Phabricator user.'));
+        }
+      }
+    }
+
+    return 0;
+  }
+
+  private function resolveUser(PhabricatorRepositoryCommit $commit, $name) {
+    $phid = id(new DiffusionResolveUserQuery())
+      ->withCommit($commit)
+      ->withName($name)
+      ->execute();
+
+    if (!$phid) {
+      return null;
+    }
+
+    return id(new PhabricatorHandleQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withPHIDs(array($phid))
+      ->executeOne();
+  }
+
+}

src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php

@@ -30,5 +30,27 @@ abstract class PhabricatorRepositoryManagementWorkflow
     return $repos;
   }
 
+  protected function loadCommits(PhutilArgumentParser $args, $param) {
+    $names = $args->getArg($param);
+    if (!$names) {
+      return null;
+    }
+
+    $query = id(new DiffusionCommitQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withIdentifiers($names);
+
+    $query->execute();
+    $map = $query->getIdentifierMap();
+
+    foreach ($names as $name) {
+      if (empty($map[$name])) {
+        throw new PhutilArgumentUsageException(
+          pht('Commit "%s" does not exist or is ambiguous.', $name));
+      }
+    }
+
+    return $map;
+  }
 
 }

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -22,7 +22,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     $data->setAuthorName($author);
     $data->setCommitDetail(
       'authorPHID',
-      $this->resolveUserPHID($author));
+      $this->resolveUserPHID($commit, $author));
 
     $data->setCommitMessage($message);
 
@@ -30,16 +30,13 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       $data->setCommitDetail('committer', $committer);
       $data->setCommitDetail(
         'committerPHID',
-        $this->resolveUserPHID($committer));
+        $this->resolveUserPHID($commit, $committer));
     }
 
     $repository = $this->repository;
 
-    $author_phid = $this->lookupUser(
+    $author_phid = $data->getCommitDetail('authorPHID');
-      $commit,
+    $committer_phid = $data->getCommitDetail('committerPHID');
-      $data->getAuthorName(),
-      $data->getCommitDetail('authorPHID'));
-    $data->setCommitDetail('authorPHID', $author_phid);
 
     $user = new PhabricatorUser();
     if ($author_phid) {
@@ -87,12 +84,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       'differential.revisionID',
       $revision_id);
 
-    $committer_phid = $this->lookupUser(
-      $commit,
-      $data->getCommitDetail('committer'),
-      $data->getCommitDetail('committerPHID'));
-    $data->setCommitDetail('committerPHID', $committer_phid);
-
     if ($author_phid != $commit->getAuthorPHID()) {
       $commit->setAuthorPHID($author_phid);
     }
@@ -449,7 +440,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
 
   /**
    * Given a set of revisions, returns the revision with the latest
-   * updated time.   This is ostensibly the most recent revision.
+   * updated time. This is ostensibly the most recent revision.
    */
   private function identifyMostRecentRevision(array $revisions) {
     assert_instances_of($revisions, 'DifferentialRevision');
@@ -457,98 +448,14 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     return end($revisions);
   }
 
-  /**
+  private function resolveUserPHID(
-   * Emit an event so installs can do custom lookup of commit authors who may
-   * not be naturally resolvable.
-   */
-  private function lookupUser(
     PhabricatorRepositoryCommit $commit,
-    $query,
+    $user_name) {
-    $guess) {
 
-    $type = PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER;
+    return id(new DiffusionResolveUserQuery())
-    $data = array(
+      ->withCommit($commit)
-      'commit'  => $commit,
+      ->withName($user_name)
-      'query'   => $query,
+      ->execute();
-      'result'  => $guess,
-    );
-
-    $event = new PhabricatorEvent($type, $data);
-    PhutilEventEngine::dispatchEvent($event);
-
-    return $event->getValue('result');
-  }
-
-  private function resolveUserPHID($user_name) {
-    if (!strlen($user_name)) {
-      return null;
-    }
-
-    $phid = $this->findUserByUserName($user_name);
-    if ($phid) {
-      return $phid;
-    }
-    $phid = $this->findUserByEmailAddress($user_name);
-    if ($phid) {
-      return $phid;
-    }
-    $phid = $this->findUserByRealName($user_name);
-    if ($phid) {
-      return $phid;
-    }
-
-    // No hits yet, try to parse it as an email address.
-
-    $email = new PhutilEmailAddress($user_name);
-
-    $phid = $this->findUserByEmailAddress($email->getAddress());
-    if ($phid) {
-      return $phid;
-    }
-
-    $display_name = $email->getDisplayName();
-    if ($display_name) {
-      $phid = $this->findUserByUserName($display_name);
-      if ($phid) {
-        return $phid;
-      }
-      $phid = $this->findUserByRealName($display_name);
-      if ($phid) {
-        return $phid;
-      }
-    }
-
-    return null;
-  }
-
-  private function findUserByUserName($user_name) {
-    $by_username = id(new PhabricatorUser())->loadOneWhere(
-      'userName = %s',
-      $user_name);
-    if ($by_username) {
-      return $by_username->getPHID();
-    }
-    return null;
-  }
-
-  private function findUserByRealName($real_name) {
-    // Note, real names are not guaranteed unique, which is why we do it this
-    // way.
-    $by_realname = id(new PhabricatorUser())->loadAllWhere(
-      'realName = %s',
-      $real_name);
-    if (count($by_realname) == 1) {
-      return reset($by_realname)->getPHID();
-    }
-    return null;
-  }
-
-  private function findUserByEmailAddress($email_address) {
-    $by_email = PhabricatorUser::loadOneWithEmailAddress($email_address);
-    if ($by_email) {
-      return $by_email->getPHID();
-    }
-    return null;
   }
 
 }