Move most Herald condition config into dynamic adapters

Summary: Ref T2769. Pushes most condition configuration into Adapters, out of the hard-coded class.

Test Plan: Looked at, edited, and dry-run'd Herald rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

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

src/applications/herald/adapter/HeraldAdapter.php

@@ -16,6 +16,24 @@ abstract class HeraldAdapter {
   const FIELD_AFFECTED_PACKAGE       = 'affected-package';
   const FIELD_AFFECTED_PACKAGE_OWNER = 'affected-package-owner';
 
+  const CONDITION_CONTAINS        = 'contains';
+  const CONDITION_NOT_CONTAINS    = '!contains';
+  const CONDITION_IS              = 'is';
+  const CONDITION_IS_NOT          = '!is';
+  const CONDITION_IS_ANY          = 'isany';
+  const CONDITION_IS_NOT_ANY      = '!isany';
+  const CONDITION_INCLUDE_ALL     = 'all';
+  const CONDITION_INCLUDE_ANY     = 'any';
+  const CONDITION_INCLUDE_NONE    = 'none';
+  const CONDITION_IS_ME           = 'me';
+  const CONDITION_IS_NOT_ME       = '!me';
+  const CONDITION_REGEXP          = 'regexp';
+  const CONDITION_RULE            = 'conditions';
+  const CONDITION_NOT_RULE        = '!conditions';
+  const CONDITION_EXISTS          = 'exists';
+  const CONDITION_NOT_EXISTS      = '!exists';
+  const CONDITION_REGEXP_PAIR     = 'regexp-pair';
+
   abstract public function getPHID();
   abstract public function getHeraldName();
   abstract public function getHeraldTypeName();
@@ -56,6 +74,83 @@ abstract class HeraldAdapter {
     );
   }
 
+  public function getConditionNameMap() {
+    return array(
+      self::CONDITION_CONTAINS        => pht('contains'),
+      self::CONDITION_NOT_CONTAINS    => pht('does not contain'),
+      self::CONDITION_IS              => pht('is'),
+      self::CONDITION_IS_NOT          => pht('is not'),
+      self::CONDITION_IS_ANY          => pht('is any of'),
+      self::CONDITION_IS_NOT_ANY      => pht('is not any of'),
+      self::CONDITION_INCLUDE_ALL     => pht('include all of'),
+      self::CONDITION_INCLUDE_ANY     => pht('include any of'),
+      self::CONDITION_INCLUDE_NONE    => pht('include none of'),
+      self::CONDITION_IS_ME           => pht('is myself'),
+      self::CONDITION_IS_NOT_ME       => pht('is not myself'),
+      self::CONDITION_REGEXP          => pht('matches regexp'),
+      self::CONDITION_RULE            => pht('matches:'),
+      self::CONDITION_NOT_RULE        => pht('does not match:'),
+      self::CONDITION_EXISTS          => pht('exists'),
+      self::CONDITION_NOT_EXISTS      => pht('does not exist'),
+      self::CONDITION_REGEXP_PAIR     => pht('matches regexp pair'),
+    );
+  }
+
+  public function getConditionsForField($field) {
+    switch ($field) {
+      case self::FIELD_TITLE:
+      case self::FIELD_BODY:
+        return array(
+          self::CONDITION_CONTAINS,
+          self::CONDITION_NOT_CONTAINS,
+          self::CONDITION_IS,
+          self::CONDITION_IS_NOT,
+          self::CONDITION_REGEXP,
+        );
+      case self::FIELD_AUTHOR:
+      case self::FIELD_REPOSITORY:
+      case self::FIELD_REVIEWER:
+        return array(
+          self::CONDITION_IS_ANY,
+          self::CONDITION_IS_NOT_ANY,
+        );
+      case self::FIELD_TAGS:
+      case self::FIELD_REVIEWERS:
+      case self::FIELD_CC:
+        return array(
+          self::CONDITION_INCLUDE_ALL,
+          self::CONDITION_INCLUDE_ANY,
+          self::CONDITION_INCLUDE_NONE,
+        );
+      case self::FIELD_DIFF_FILE:
+        return array(
+          self::CONDITION_CONTAINS,
+          self::CONDITION_REGEXP,
+        );
+      case self::FIELD_DIFF_CONTENT:
+        return array(
+          self::CONDITION_CONTAINS,
+          self::CONDITION_REGEXP,
+          self::CONDITION_REGEXP_PAIR,
+        );
+      case self::FIELD_RULE:
+        return array(
+          self::CONDITION_RULE,
+          self::CONDITION_NOT_RULE,
+        );
+      case self::FIELD_AFFECTED_PACKAGE:
+      case self::FIELD_AFFECTED_PACKAGE_OWNER:
+        return array(
+          self::CONDITION_INCLUDE_ANY,
+          self::CONDITION_INCLUDE_NONE,
+        );
+      default:
+        throw new Exception(
+          "This adapter does not define conditions for field '{$field}'!");
+    }
+  }
+
+
   public static function applyFlagEffect(HeraldEffect $effect, $phid) {
     $color = $effect->getTarget();
 

src/applications/herald/adapter/HeraldCommitAdapter.php

@@ -63,6 +63,29 @@ final class HeraldCommitAdapter extends HeraldAdapter {
     );
   }
 
+  public function getConditionsForField($field) {
+    switch ($field) {
+      case self::FIELD_DIFFERENTIAL_REVIEWERS:
+      case self::FIELD_DIFFERENTIAL_CCS:
+        return array(
+          self::CONDITION_INCLUDE_ALL,
+          self::CONDITION_INCLUDE_ANY,
+          self::CONDITION_INCLUDE_NONE,
+        );
+      case self::FIELD_DIFFERENTIAL_REVISION:
+        return array(
+          self::CONDITION_EXISTS,
+          self::CONDITION_NOT_EXISTS,
+        );
+      case self::FIELD_NEED_AUDIT_FOR_PACKAGE:
+        return array(
+          self::CONDITION_INCLUDE_ANY,
+          self::CONDITION_INCLUDE_NONE,
+        );
+    }
+    return parent::getConditionsForField($field);
+  }
+
   public static function newLegacyAdapter(
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit,
@@ -182,17 +205,17 @@ final class HeraldCommitAdapter extends HeraldAdapter {
   public function getHeraldField($field) {
     $data = $this->commitData;
     switch ($field) {
-      case HeraldFieldConfig::FIELD_BODY:
+      case self::FIELD_BODY:
         return $data->getCommitMessage();
-      case HeraldFieldConfig::FIELD_AUTHOR:
+      case self::FIELD_AUTHOR:
         return $data->getCommitDetail('authorPHID');
-      case HeraldFieldConfig::FIELD_REVIEWER:
+      case self::FIELD_REVIEWER:
         return $data->getCommitDetail('reviewerPHID');
-      case HeraldFieldConfig::FIELD_DIFF_FILE:
+      case self::FIELD_DIFF_FILE:
         return $this->loadAffectedPaths();
-      case HeraldFieldConfig::FIELD_REPOSITORY:
+      case self::FIELD_REPOSITORY:
         return $this->repository->getPHID();
-      case HeraldFieldConfig::FIELD_DIFF_CONTENT:
+      case self::FIELD_DIFF_CONTENT:
         try {
           $diff = $this->loadCommitDiff();
         } catch (Exception $ex) {
@@ -211,28 +234,28 @@ final class HeraldCommitAdapter extends HeraldAdapter {
           $dict[$change->getFilename()] = implode("\n", $lines);
         }
         return $dict;
-      case HeraldFieldConfig::FIELD_AFFECTED_PACKAGE:
+      case self::FIELD_AFFECTED_PACKAGE:
         $packages = $this->loadAffectedPackages();
         return mpull($packages, 'getPHID');
-      case HeraldFieldConfig::FIELD_AFFECTED_PACKAGE_OWNER:
+      case self::FIELD_AFFECTED_PACKAGE_OWNER:
         $packages = $this->loadAffectedPackages();
         $owners = PhabricatorOwnersOwner::loadAllForPackages($packages);
         return mpull($owners, 'getUserPHID');
-      case HeraldFieldConfig::FIELD_NEED_AUDIT_FOR_PACKAGE:
+      case self::FIELD_NEED_AUDIT_FOR_PACKAGE:
         return $this->loadAuditNeededPackage();
-      case HeraldFieldConfig::FIELD_DIFFERENTIAL_REVISION:
+      case self::FIELD_DIFFERENTIAL_REVISION:
         $revision = $this->loadDifferentialRevision();
         if (!$revision) {
           return null;
         }
         return $revision->getID();
-      case HeraldFieldConfig::FIELD_DIFFERENTIAL_REVIEWERS:
+      case self::FIELD_DIFFERENTIAL_REVIEWERS:
         $revision = $this->loadDifferentialRevision();
         if (!$revision) {
           return array();
         }
         return $revision->getReviewers();
-      case HeraldFieldConfig::FIELD_DIFFERENTIAL_CCS:
+      case self::FIELD_DIFFERENTIAL_CCS:
         $revision = $this->loadDifferentialRevision();
         if (!$revision) {
           return array();

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

@@ -198,42 +198,42 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
 
   public function getHeraldField($field) {
     switch ($field) {
-      case HeraldFieldConfig::FIELD_TITLE:
+      case self::FIELD_TITLE:
         return $this->revision->getTitle();
         break;
-      case HeraldFieldConfig::FIELD_BODY:
+      case self::FIELD_BODY:
         return $this->revision->getSummary()."\n".
                $this->revision->getTestPlan();
         break;
-      case HeraldFieldConfig::FIELD_AUTHOR:
+      case self::FIELD_AUTHOR:
         return $this->revision->getAuthorPHID();
         break;
-      case HeraldFieldConfig::FIELD_DIFF_FILE:
+      case self::FIELD_DIFF_FILE:
         return $this->loadAffectedPaths();
-      case HeraldFieldConfig::FIELD_CC:
+      case self::FIELD_CC:
         if (isset($this->explicitCCs)) {
           return array_keys($this->explicitCCs);
         } else {
           return $this->revision->getCCPHIDs();
         }
-      case HeraldFieldConfig::FIELD_REVIEWERS:
+      case self::FIELD_REVIEWERS:
         if (isset($this->explicitReviewers)) {
           return array_keys($this->explicitReviewers);
         } else {
           return $this->revision->getReviewers();
         }
-      case HeraldFieldConfig::FIELD_REPOSITORY:
+      case self::FIELD_REPOSITORY:
         $repository = $this->loadRepository();
         if (!$repository) {
           return null;
         }
         return $repository->getPHID();
-      case HeraldFieldConfig::FIELD_DIFF_CONTENT:
+      case self::FIELD_DIFF_CONTENT:
         return $this->loadContentDictionary();
-      case HeraldFieldConfig::FIELD_AFFECTED_PACKAGE:
+      case self::FIELD_AFFECTED_PACKAGE:
         $packages = $this->loadAffectedPackages();
         return mpull($packages, 'getPHID');
-      case HeraldFieldConfig::FIELD_AFFECTED_PACKAGE_OWNER:
+      case self::FIELD_AFFECTED_PACKAGE_OWNER:
         $packages = $this->loadAffectedPackages();
         return PhabricatorOwnersOwner::loadAffiliatedUserPHIDs(
           mpull($packages, 'getID'));

src/applications/herald/config/HeraldConditionConfig.php

@@ -2,6 +2,7 @@
 
 final class HeraldConditionConfig {
 
+  // TODO: Remove; still used by Engine/etc.
   const CONDITION_CONTAINS        = 'contains';
   const CONDITION_NOT_CONTAINS    = '!contains';
   const CONDITION_IS              = 'is';
@@ -20,6 +21,7 @@ final class HeraldConditionConfig {
   const CONDITION_NOT_EXISTS      = '!exists';
   const CONDITION_REGEXP_PAIR     = 'regexp-pair';
 
+  // TODO: Remove; still used by Transcripts.
   public static function getConditionMap() {
     $map = array(
       self::CONDITION_CONTAINS        => pht('contains'),
@@ -44,82 +46,4 @@ final class HeraldConditionConfig {
     return $map;
   }
 
-  public static function getConditionMapForField($field) {
-    $map = self::getConditionMap();
-    switch ($field) {
-      case HeraldFieldConfig::FIELD_TITLE:
-      case HeraldFieldConfig::FIELD_BODY:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_CONTAINS,
-            self::CONDITION_NOT_CONTAINS,
-            self::CONDITION_IS,
-            self::CONDITION_IS_NOT,
-            self::CONDITION_REGEXP,
-          ));
-      case HeraldFieldConfig::FIELD_AUTHOR:
-      case HeraldFieldConfig::FIELD_REPOSITORY:
-      case HeraldFieldConfig::FIELD_REVIEWER:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_IS_ANY,
-            self::CONDITION_IS_NOT_ANY,
-          ));
-      case HeraldFieldConfig::FIELD_TAGS:
-      case HeraldFieldConfig::FIELD_REVIEWERS:
-      case HeraldFieldConfig::FIELD_CC:
-      case HeraldFieldConfig::FIELD_DIFFERENTIAL_REVIEWERS:
-      case HeraldFieldConfig::FIELD_DIFFERENTIAL_CCS:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_INCLUDE_ALL,
-            self::CONDITION_INCLUDE_ANY,
-            self::CONDITION_INCLUDE_NONE,
-          ));
-      case HeraldFieldConfig::FIELD_DIFF_FILE:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_CONTAINS,
-            self::CONDITION_REGEXP,
-          ));
-      case HeraldFieldConfig::FIELD_DIFF_CONTENT:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_CONTAINS,
-            self::CONDITION_REGEXP,
-            self::CONDITION_REGEXP_PAIR,
-          ));
-      case HeraldFieldConfig::FIELD_RULE:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_RULE,
-            self::CONDITION_NOT_RULE,
-          ));
-      case HeraldFieldConfig::FIELD_AFFECTED_PACKAGE:
-      case HeraldFieldConfig::FIELD_AFFECTED_PACKAGE_OWNER:
-      case HeraldFieldConfig::FIELD_NEED_AUDIT_FOR_PACKAGE:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_INCLUDE_ANY,
-            self::CONDITION_INCLUDE_NONE,
-          ));
-      case HeraldFieldConfig::FIELD_DIFFERENTIAL_REVISION:
-        return array_select_keys(
-          $map,
-          array(
-            self::CONDITION_EXISTS,
-            self::CONDITION_NOT_EXISTS,
-          ));
-      default:
-        throw new Exception("Unknown field type '{$field}'.");
-    }
-  }
-
 }

src/applications/herald/controller/HeraldRuleController.php

@@ -387,18 +387,22 @@ final class HeraldRuleController extends HeraldController {
     $all_rules = mpull($all_rules, 'getName', 'getID');
     asort($all_rules);
 
+    $all_fields = $adapter->getFieldNameMap();
+    $all_conditions = $adapter->getConditionNameMap();
+
     $fields = $adapter->getFields();
-    $field_map = array_select_keys($adapter->getFieldNameMap(), $fields);
+    $field_map = array_select_keys($all_fields, $fields);
 
     $config_info = array();
     $config_info['fields'] = $field_map;
-    $config_info['conditions'] = HeraldConditionConfig::getConditionMap();
+    $config_info['conditions'] = $all_conditions;
     foreach ($config_info['fields'] as $field => $name) {
-      $config_info['conditionMap'][$field] = array_keys(
+      $field_conditions = $adapter->getConditionsForField($field);
-        HeraldConditionConfig::getConditionMapForField($field));
+      $config_info['conditionMap'][$field] = $field_conditions;
     }
+
     foreach ($config_info['fields'] as $field => $fname) {
-      foreach ($config_info['conditions'] as $condition => $cname) {
+      foreach ($config_info['conditionMap'][$field] as $condition) {
         $config_info['values'][$field][$condition] =
           HeraldValueTypeConfig::getValueTypeForFieldAndCondition(
             $field,