From 9f38aaa5dec4706b9425483a6a50cac515cfe4b5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 27 Dec 2013 13:16:00 -0800
Subject: [PATCH] Add "raw author name" and "raw committer name" as Herald
 fields for commit content hooks

Summary:
Ref T4195. A legitimate rule which needs this field is "do not allow commits as root". Interestingly, we have exactly one commit as root in each Phabricator, Arcanist and libphutil.

Since the committer and author don't need to be Phabricator accounts (just the Pusher), the existing "Committer" and "Author" fields can't express this rule (they'll be empty).

Test Plan: {F93406}

Reviewers: btrahan

Reviewed By: btrahan

CC: SEJeff, aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7841
---
 .../herald/HeraldPreCommitContentAdapter.php  | 40 +++++++++++++++++++
 .../herald/adapter/HeraldAdapter.php          |  6 +++
 .../herald/storage/HeraldRule.php             |  2 +-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
index 95cd112fad..ac922eaa5b 100644
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -41,7 +41,9 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter {
       array(
         self::FIELD_BODY,
         self::FIELD_AUTHOR,
+        self::FIELD_AUTHOR_RAW,
         self::FIELD_COMMITTER,
+        self::FIELD_COMMITTER_RAW,
         self::FIELD_BRANCHES,
         self::FIELD_DIFF_FILE,
         self::FIELD_DIFF_CONTENT,
@@ -99,8 +101,12 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter {
         return $this->getCommitRef()->getMessage();
       case self::FIELD_AUTHOR:
         return $this->getAuthorPHID();
+      case self::FIELD_AUTHOR_RAW:
+        return $this->getAuthorRaw();
       case self::FIELD_COMMITTER:
         return $this->getCommitterPHID();
+      case self::FIELD_COMMITTER_RAW:
+        return $this->getCommitterRaw();
       case self::FIELD_BRANCHES:
         return $this->getBranches();
       case self::FIELD_DIFF_FILE:
@@ -278,6 +284,40 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter {
     }
   }
 
+  private function getAuthorRaw() {
+    $repository = $this->hookEngine->getRepository();
+    $vcs = $repository->getVersionControlSystem();
+    switch ($vcs) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $ref = $this->getCommitRef();
+        return $ref->getAuthor();
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        // In Subversion, the pusher is always the author.
+        return $this->hookEngine->getViewer()->getUsername();
+    }
+  }
+
+  private function getCommitterRaw() {
+    $repository = $this->hookEngine->getRepository();
+    $vcs = $repository->getVersionControlSystem();
+    switch ($vcs) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        // Here, if there's no committer, we're going to return the author
+        // instead.
+        $ref = $this->getCommitRef();
+        $committer = $ref->getCommitter();
+        if (strlen($committer)) {
+          return $committer;
+        }
+        return $ref->getAuthor();
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        // In Subversion, the pusher is always the committer.
+        return $this->hookEngine->getViewer()->getUsername();
+    }
+  }
+
   private function lookupUser($author) {
     return id(new DiffusionResolveUserQuery())
       ->withName($author)
diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index e71f60d96c..a821d173a1 100644
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -34,6 +34,8 @@ abstract class HeraldAdapter {
   const FIELD_DIFFERENTIAL_ACCEPTED  = 'differential-accepted';
   const FIELD_IS_MERGE_COMMIT        = 'is-merge-commit';
   const FIELD_BRANCHES               = 'branches';
+  const FIELD_AUTHOR_RAW             = 'author-raw';
+  const FIELD_COMMITTER_RAW          = 'committer-raw';
 
   const CONDITION_CONTAINS        = 'contains';
   const CONDITION_NOT_CONTAINS    = '!contains';
@@ -180,6 +182,8 @@ abstract class HeraldAdapter {
         => pht('Accepted Differential revision'),
       self::FIELD_IS_MERGE_COMMIT => pht('Commit is a merge'),
       self::FIELD_BRANCHES => pht('Commit\'s branches'),
+      self::FIELD_AUTHOR_RAW => pht('Raw author name'),
+      self::FIELD_COMMITTER_RAW => pht('Raw committer name'),
     );
   }
 
@@ -218,6 +222,8 @@ abstract class HeraldAdapter {
     switch ($field) {
       case self::FIELD_TITLE:
       case self::FIELD_BODY:
+      case self::FIELD_COMMITTER_RAW:
+      case self::FIELD_AUTHOR_RAW:
         return array(
           self::CONDITION_CONTAINS,
           self::CONDITION_NOT_CONTAINS,
diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php
index 67e615af63..b7e061c520 100644
--- a/src/applications/herald/storage/HeraldRule.php
+++ b/src/applications/herald/storage/HeraldRule.php
@@ -16,7 +16,7 @@ final class HeraldRule extends HeraldDAO
   protected $ruleType;
   protected $isDisabled = 0;
 
-  protected $configVersion = 20;
+  protected $configVersion = 21;
 
   // phids for which this rule has been applied
   private $ruleApplied = self::ATTACHABLE;