Provide a low-level SVN commit query, and merge the VCS query types

Summary: Ref T4195. Even though we use `svnlook` in the hook itself, I need this query elsewhere, so provide it and merge the classes into one which does the right thing.

Test Plan:
  - Used `reparse.php` to reparse messages for Git, SVN and Mercurial commits, using `var_dump()` to examine the commit refs for sanity.
  - Used `reparse.php` to reparse changes for an SVN commit.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

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

src/__phutil_library_map__.php

@@ -507,10 +507,9 @@ phutil_register_library_map(array(
     'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
     'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php',
     'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
-    'DiffusionLowLevelGitCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php',
+    'DiffusionLowLevelCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php',
     'DiffusionLowLevelGitRefQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php',
     'DiffusionLowLevelMercurialBranchesQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php',
-    'DiffusionLowLevelMercurialCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php',
     'DiffusionLowLevelQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php',
     'DiffusionLowLevelResolveRefsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php',
     'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php',
@@ -2891,10 +2890,9 @@ phutil_register_library_map(array(
     'DiffusionLastModifiedController' => 'DiffusionController',
     'DiffusionLintController' => 'DiffusionController',
     'DiffusionLintDetailsController' => 'DiffusionController',
-    'DiffusionLowLevelGitCommitQuery' => 'DiffusionLowLevelQuery',
+    'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery',
     'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery',
     'DiffusionLowLevelMercurialBranchesQuery' => 'DiffusionLowLevelQuery',
-    'DiffusionLowLevelMercurialCommitQuery' => 'DiffusionLowLevelQuery',
     'DiffusionLowLevelQuery' => 'Phobject',
     'DiffusionLowLevelResolveRefsQuery' => 'DiffusionLowLevelQuery',
     'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery',

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

@@ -939,12 +939,8 @@ final class DiffusionCommitHookEngine extends Phobject {
     $vcs = $repository->getVersionControlSystem();
     switch ($vcs) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-        return id(new DiffusionLowLevelGitCommitQuery())
-          ->setRepository($repository)
-          ->withIdentifier($identifier)
-          ->execute();
       case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        return id(new DiffusionLowLevelMercurialCommitQuery())
+        return id(new DiffusionLowLevelCommitQuery())
           ->setRepository($repository)
           ->withIdentifier($identifier)
           ->execute();

src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php

@@ -0,0 +1,137 @@
+<?php
+
+/**
+ * Populate a @{class:DiffusionCommitRef} with information about a specific
+ * commit in a repository. This is a low-level query which talks directly to
+ * the underlying VCS.
+ */
+final class DiffusionLowLevelCommitQuery
+  extends DiffusionLowLevelQuery {
+
+  private $identifier;
+
+  public function withIdentifier($identifier) {
+    $this->identifier = $identifier;
+    return $this;
+  }
+
+  public function executeQuery() {
+    if (!strlen($this->identifier)) {
+      throw new Exception(
+        pht('You must provide an identifier with withIdentifier()!'));
+    }
+
+    $type = $this->getRepository()->getVersionControlSystem();
+    switch ($type) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        $result = $this->loadGitCommitRef();
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $result = $this->loadMercurialCommitRef();
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        $result = $this->loadSubversionCommitRef();
+        break;
+      default:
+        throw new Exception(pht('Unsupported repository type "%s"!', $type));
+    }
+
+    return $result;
+  }
+
+  private function loadGitCommitRef() {
+    $repository = $this->getRepository();
+
+    // NOTE: %B was introduced somewhat recently in git's history, so pull
+    // commit message information with %s and %b instead.
+
+    // Even though we pass --encoding here, git doesn't always succeed, so
+    // we try a little harder, since git *does* tell us what the actual encoding
+    // is correctly (unless it doesn't; encoding is sometimes empty).
+    list($info) = $repository->execxLocalCommand(
+      'log -n 1 --encoding=%s --format=%s %s --',
+      'UTF-8',
+      implode('%x00', array('%e', '%cn', '%ce', '%an', '%ae', '%s%n%n%b')),
+      $this->identifier);
+
+    $parts = explode("\0", $info);
+    $encoding = array_shift($parts);
+
+    foreach ($parts as $key => $part) {
+      if ($encoding) {
+        $part = phutil_utf8_convert($part, 'UTF-8', $encoding);
+      }
+      $parts[$key] = phutil_utf8ize($part);
+      if (!strlen($parts[$key])) {
+        $parts[$key] = null;
+      }
+    }
+
+    return id(new DiffusionCommitRef())
+      ->setCommitterName($parts[0])
+      ->setCommitterEmail($parts[1])
+      ->setAuthorName($parts[2])
+      ->setAuthorEmail($parts[3])
+      ->setMessage($parts[4]);
+  }
+
+  private function loadMercurialCommitRef() {
+    $repository = $this->getRepository();
+
+    list($stdout) = $repository->execxLocalCommand(
+      'log --template %s --rev %s',
+      '{author}\\n{desc}',
+      hgsprintf('%s', $this->identifier));
+
+    list($author, $message) = explode("\n", $stdout, 2);
+
+    $author = phutil_utf8ize($author);
+    $message = phutil_utf8ize($message);
+
+    list($author_name, $author_email) = $this->splitUserIdentifier($author);
+
+    return id(new DiffusionCommitRef())
+      ->setAuthorName($author_name)
+      ->setAuthorEmail($author_email)
+      ->setMessage($message);
+  }
+
+  private function loadSubversionCommitRef() {
+    $repository = $this->getRepository();
+
+    list($xml) = $repository->execxRemoteCommand(
+      'log --xml --limit 1 %s',
+      $repository->getSubversionPathURI(null, $this->identifier));
+
+    // Subversion may send us back commit messages which won't parse because
+    // they have non UTF-8 garbage in them. Slam them into valid UTF-8.
+    $xml = phutil_utf8ize($xml);
+    $log = new SimpleXMLElement($xml);
+    $entry = $log->logentry[0];
+
+    $author = (string)$entry->author;
+    $message = (string)$entry->msg;
+
+    list($author_name, $author_email) = $this->splitUserIdentifier($author);
+
+    return id(new DiffusionCommitRef())
+      ->setAuthorName($author_name)
+      ->setAuthorEmail($author_email)
+      ->setMessage($message);
+  }
+
+  private function splitUserIdentifier($user) {
+    $email = new PhutilEmailAddress($user);
+
+    if ($email->getDisplayName() || $email->getDomainName()) {
+      $user_name = $email->getDisplayName();
+      $user_email = $email->getAddress();
+    } else {
+      $user_name = $email->getAddress();
+      $user_email = null;
+    }
+
+    return array($user_name, $user_email);
+  }
+
+}

src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitCommitQuery.php

@@ -1,48 +0,0 @@
-<?php
-
-final class DiffusionLowLevelGitCommitQuery extends DiffusionLowLevelQuery {
-
-  private $identifier;
-
-  public function withIdentifier($identifier) {
-    $this->identifier = $identifier;
-    return $this;
-  }
-
-  protected function executeQuery() {
-    $repository = $this->getRepository();
-
-    // NOTE: %B was introduced somewhat recently in git's history, so pull
-    // commit message information with %s and %b instead.
-
-    // Even though we pass --encoding here, git doesn't always succeed, so
-    // we try a little harder, since git *does* tell us what the actual encoding
-    // is correctly (unless it doesn't; encoding is sometimes empty).
-    list($info) = $repository->execxLocalCommand(
-      'log -n 1 --encoding=%s --format=%s %s --',
-      'UTF-8',
-      implode('%x00', array('%e', '%cn', '%ce', '%an', '%ae', '%s%n%n%b')),
-      $this->identifier);
-
-    $parts = explode("\0", $info);
-    $encoding = array_shift($parts);
-
-    foreach ($parts as $key => $part) {
-      if ($encoding) {
-        $part = phutil_utf8_convert($part, 'UTF-8', $encoding);
-      }
-      $parts[$key] = phutil_utf8ize($part);
-      if (!strlen($parts[$key])) {
-        $parts[$key] = null;
-      }
-    }
-
-    return id(new DiffusionCommitRef())
-      ->setCommitterName($parts[0])
-      ->setCommitterEmail($parts[1])
-      ->setAuthorName($parts[2])
-      ->setAuthorEmail($parts[3])
-      ->setMessage($parts[4]);
-  }
-
-}

src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialCommitQuery.php

@@ -1,41 +0,0 @@
-<?php
-
-final class DiffusionLowLevelMercurialCommitQuery
-  extends DiffusionLowLevelQuery {
-
-  private $identifier;
-
-  public function withIdentifier($identifier) {
-    $this->identifier = $identifier;
-    return $this;
-  }
-
-  protected function executeQuery() {
-    $repository = $this->getRepository();
-
-    list($stdout) = $repository->execxLocalCommand(
-      'log --template %s --rev %s',
-      '{author}\\n{desc}',
-      hgsprintf('%s', $this->identifier));
-
-    list($author, $message) = explode("\n", $stdout, 2);
-
-    $author = phutil_utf8ize($author);
-    $message = phutil_utf8ize($message);
-
-    $email = new PhutilEmailAddress($author);
-    if ($email->getDisplayName() || $email->getDomainName()) {
-      $author_name = $email->getDisplayName();
-      $author_email = $email->getAddress();
-    } else {
-      $author_name = $email->getAddress();
-      $author_email = null;
-    }
-
-    return id(new DiffusionCommitRef())
-      ->setAuthorName($author_name)
-      ->setAuthorEmail($author_email)
-      ->setMessage($message);
-  }
-
-}

src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php

@@ -52,28 +52,6 @@ abstract class PhabricatorRepositoryCommitParserWorker
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit);
 
-  /**
-   * This method is kind of awkward here but both the SVN message and
-   * change parsers use it.
-   */
-  protected function getSVNLogXMLObject($uri, $revision, $verbose = false) {
-
-    if ($verbose) {
-      $verbose = '--verbose';
-    }
-
-    list($xml) = $this->repository->execxRemoteCommand(
-      "log --xml {$verbose} --limit 1 %s@%d",
-      $uri,
-      $revision);
-
-    // Subversion may send us back commit messages which won't parse because
-    // they have non UTF-8 garbage in them. Slam them into valid UTF-8.
-    $xml = phutil_utf8ize($xml);
-
-    return new SimpleXMLElement($xml);
-  }
-
   protected function isBadCommit($full_commit_name) {
     $repository = new PhabricatorRepository();
 

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

@@ -29,7 +29,7 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker
 
     // Pull the top-level path changes out of "svn log". This is pretty
     // straightforward; just parse the XML log.
-    $log = $this->getSVNLogXMLObject($uri, $svn_commit, $verbose = true);
+    $log = $this->getSVNLogXMLObject($uri, $svn_commit);
 
     $entry = $log->logentry[0];
 
@@ -769,4 +769,21 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker
     return $parents;
   }
 
+  /**
+   * This method is kind of awkward here but both the SVN message and
+   * change parsers use it.
+   */
+  private function getSVNLogXMLObject($uri, $revision) {
+    list($xml) = $this->repository->execxRemoteCommand(
+      "log --xml --verbose --limit 1 %s@%d",
+      $uri,
+      $revision);
+
+    // Subversion may send us back commit messages which won't parse because
+    // they have non UTF-8 garbage in them. Slam them into valid UTF-8.
+    $xml = phutil_utf8ize($xml);
+
+    return new SimpleXMLElement($xml);
+  }
+
 }

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

@@ -7,7 +7,7 @@ final class PhabricatorRepositoryGitCommitMessageParserWorker
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit) {
 
-    $ref = id(new DiffusionLowLevelGitCommitQuery())
+    $ref = id(new DiffusionLowLevelCommitQuery())
       ->setRepository($repository)
       ->withIdentifier($commit->getCommitIdentifier())
       ->execute();

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

@@ -7,7 +7,7 @@ final class PhabricatorRepositoryMercurialCommitMessageParserWorker
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit) {
 
-    $ref = id(new DiffusionLowLevelMercurialCommitQuery())
+    $ref = id(new DiffusionLowLevelCommitQuery())
       ->setRepository($repository)
       ->withIdentifier($commit->getCommitIdentifier())
       ->execute();

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

@@ -7,17 +7,13 @@ final class PhabricatorRepositorySvnCommitMessageParserWorker
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit) {
 
-    $uri = $repository->getSubversionPathURI();
+    $ref = id(new DiffusionLowLevelCommitQuery())
+      ->setRepository($repository)
+      ->withIdentifier($commit->getCommitIdentifier())
+      ->execute();
 
-    $log = $this->getSVNLogXMLObject(
+    $author = $ref->getAuthor();
-      $uri,
+    $message = $ref->getMessage();
-      $commit->getCommitIdentifier(),
-      $verbose = false);
-
-    $entry = $log->logentry[0];
-
-    $author = (string)$entry->author;
-    $message = (string)$entry->msg;
 
     $this->updateCommitData($author, $message);