Improve Diffusion parser linking of author names

Summary: See T502. Under some VCS setups, we get full email addresses instead of
usernames or real names. Try harder to find matches, by falling back to email
address parsing if we don't get hits on the straight-up token parsing.

Test Plan:
This is difficult to test because it depends on the account state and repository
state, and hard to pull out so it's more testable without better mocking
facilities. I just dumped this into the parser to verify the behavior:

  foreach (array(
    'epriestley',
    'Evan Priestley',
    'epriestley@epriestley.com',
    'derp <epriestley@epriestley.com>',
    '"Evan Priestley" <derpderpderp@derpderpderp.com>',
    'quackderp <derpderpderp@derpderpderp.com>',
    ) as $email) {
    echo "{$email} = ".$this->resolveUserPHID($email)."\n";
  }
  die();

  Running PhabricatorRepositoryGitCommitMessageParserWorker...
  epriestley = PHID-USER-79f25616ea2635089a31
  Evan Priestley = PHID-USER-79f25616ea2635089a31
  epriestley@epriestley.com = PHID-USER-1bec59b91be6223f07fd
  derp <epriestley@epriestley.com> = PHID-USER-1bec59b91be6223f07fd
  "Evan Priestley" <derpderpderp@derpderpderp.com> =
PHID-USER-79f25616ea2635089a31
  quackderp <derpderpderp@derpderpderp.com> =

This is expected (all variations of my identity parsed correctly, and the bogus
one failed). There are two different user PHIDs in the result set because I have
like 30 different similar accounts on my local, including one called "derp" and
another one with address "derp@derp.com", which prevented an earlier version of
this test case from working correctly.

Reviewers: zachallia, aran, Makinde, jungejason, nh, tuomaspelkonen

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 968

src/applications/repository/parser/base/PhabricatorRepositoryCommitMessageDetailParser.php

@@ -36,26 +36,43 @@ abstract class PhabricatorRepositoryCommitMessageDetailParser {
     return $this->commitData;
   }
 
+  /**
+   * Try to link a commit name to a Phabricator account. Basically we throw it
+   * at the wall and see if something sticks.
+   */
   public function resolveUserPHID($user_name) {
     if (!strlen($user_name)) {
       return null;
     }
 
-    $by_username = id(new PhabricatorUser())->loadOneWhere(
+    $phid = $this->findUserByUserName($user_name);
-      'userName = %s',
+    if ($phid) {
-      $user_name);
+      return $phid;
-    if ($by_username) {
+    }
-      return $by_username->getPHID();
+    $phid = $this->findUserByEmailAddress($user_name);
+    if ($phid) {
+      return $phid;
+    }
+    $phid = $this->findUserByRealName($user_name);
+    if ($phid) {
+      return $phid;
     }
 
-    // Note, real names are not guaranteed unique, which is why we do it this
+    // No hits yet, try to parse it as an email address.
-    // way.
+
-    $by_realname = id(new PhabricatorUser())->loadAllWhere(
+    $email = new PhutilEmailAddress($user_name);
-      'realName = %s LIMIT 1',
+
-      $user_name);
+    $phid = $this->findUserByEmailAddress($email->getAddress());
-    if ($by_realname) {
+    if ($phid) {
-      $by_realname = reset($by_realname);
+      return $phid;
-      return $by_realname->getPHID();
+    }
+
+    $display_name = $email->getDisplayName();
+    if ($display_name) {
+      $phid = $this->findUserByRealName($display_name);
+      if ($phid) {
+        return $phid;
+      }
     }
 
     return null;
@@ -63,4 +80,36 @@ abstract class PhabricatorRepositoryCommitMessageDetailParser {
 
   abstract public function parseCommitDetails();
 
+  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())->loadOneWhere(
+      'realName = %s LIMIT 1',
+      $real_name);
+    if ($by_realname) {
+      return $by_realname->getPHID();
+    }
+    return null;
+  }
+
+  private function findUserByEmailAddress($email_address) {
+    $by_email = id(new PhabricatorUser())->loadOneWhere(
+      'email = %s',
+      $email_address);
+    if ($by_email) {
+      return $by_email->getPHID();
+    }
+    return null;
+  }
+
 }

src/applications/repository/parser/base/__init__.php

@@ -8,6 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/people/storage/user');
 
+phutil_require_module('phutil', 'parser/emailaddress');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/repository/parser/default/PhabricatorRepositoryDefaultCommitMessageDetailParser.php

@@ -53,12 +53,19 @@ class PhabricatorRepositoryDefaultCommitMessageDetailParser
       $reviewer_phid = $this->resolveUserPHID($details['reviewerName']);
       if ($reviewer_phid) {
         $details['reviewerPHID'] = $reviewer_phid;
+      } else {
+        unset($details['reviewerPHID']);
       }
+    } else {
+      unset($details['reviewerName']);
+      unset($details['reviewerPHID']);
     }
 
     $author_phid = $this->resolveUserPHID($author_name);
     if ($author_phid) {
       $details['authorPHID'] = $author_phid;
+    } else {
+      unset($details['authorPHID']);
     }
 
     $data->setCommitDetails($details);