Modularize the Harbormaster "Run build plan" Herald action

Ref T8726. Modularizes "Run build plan" in Differential and Diffusion.

resources/sql/autopatches/20150730.herald.7.sql

@@ -0,0 +1,6 @@
+UPDATE {$NAMESPACE}_herald.herald_action a
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON a.ruleID = r.id
+  SET a.action = 'harbormaster.build'
+  WHERE r.ruleType != 'personal'
+  AND a.action = 'applybuildplans';

src/__phutil_library_map__.php

@@ -963,6 +963,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildWorker' => 'applications/harbormaster/worker/HarbormasterBuildWorker.php',
     'HarbormasterBuildable' => 'applications/harbormaster/storage/HarbormasterBuildable.php',
     'HarbormasterBuildableActionController' => 'applications/harbormaster/controller/HarbormasterBuildableActionController.php',
+    'HarbormasterBuildableAdapterInterface' => 'applications/harbormaster/herald/HarbormasterBuildableAdapterInterface.php',
     'HarbormasterBuildableInterface' => 'applications/harbormaster/interface/HarbormasterBuildableInterface.php',
     'HarbormasterBuildableListController' => 'applications/harbormaster/controller/HarbormasterBuildableListController.php',
     'HarbormasterBuildablePHIDType' => 'applications/harbormaster/phid/HarbormasterBuildablePHIDType.php',
@@ -996,6 +997,7 @@ phutil_register_library_map(array(
     'HarbormasterQueryBuildablesConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php',
     'HarbormasterQueryBuildsConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php',
     'HarbormasterRemarkupRule' => 'applications/harbormaster/remarkup/HarbormasterRemarkupRule.php',
+    'HarbormasterRunBuildPlansHeraldAction' => 'applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php',
     'HarbormasterSchemaSpec' => 'applications/harbormaster/storage/HarbormasterSchemaSpec.php',
     'HarbormasterScratchTable' => 'applications/harbormaster/storage/HarbormasterScratchTable.php',
     'HarbormasterSendMessageConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php',
@@ -4703,6 +4705,7 @@ phutil_register_library_map(array(
     'HarbormasterQueryBuildablesConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
     'HarbormasterQueryBuildsConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
     'HarbormasterRemarkupRule' => 'PhabricatorObjectRemarkupRule',
+    'HarbormasterRunBuildPlansHeraldAction' => 'HeraldAction',
     'HarbormasterSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'HarbormasterScratchTable' => 'HarbormasterDAO',
     'HarbormasterSendMessageConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
@@ -4728,7 +4731,10 @@ phutil_register_library_map(array(
     'HeraldApplicationActionGroup' => 'HeraldActionGroup',
     'HeraldApplyTranscript' => 'Phobject',
     'HeraldBasicFieldGroup' => 'HeraldFieldGroup',
-    'HeraldCommitAdapter' => 'HeraldAdapter',
+    'HeraldCommitAdapter' => array(
+      'HeraldAdapter',
+      'HarbormasterBuildableAdapterInterface',
+    ),
     'HeraldCondition' => 'HeraldDAO',
     'HeraldConditionTranscript' => 'Phobject',
     'HeraldContentSourceField' => 'HeraldField',
@@ -4737,7 +4743,10 @@ phutil_register_library_map(array(
     'HeraldDAO' => 'PhabricatorLiskDAO',
     'HeraldDifferentialAdapter' => 'HeraldAdapter',
     'HeraldDifferentialDiffAdapter' => 'HeraldDifferentialAdapter',
-    'HeraldDifferentialRevisionAdapter' => 'HeraldDifferentialAdapter',
+    'HeraldDifferentialRevisionAdapter' => array(
+      'HeraldDifferentialAdapter',
+      'HarbormasterBuildableAdapterInterface',
+    ),
     'HeraldDisableController' => 'HeraldController',
     'HeraldDoNothingAction' => 'HeraldAction',
     'HeraldEditFieldGroup' => 'HeraldFieldGroup',

src/applications/audit/editor/PhabricatorAuditEditor.php

@@ -901,11 +901,6 @@ final class PhabricatorAuditEditor
           'auditReasonMap', $this->auditReasonMap);
     }
 
-    HarbormasterBuildable::applyBuildPlans(
-      $object->getPHID(),
-      $object->getRepository()->getPHID(),
-      $adapter->getBuildPlans());
-
     $limit = self::MAX_FILES_SHOWN_IN_EMAIL;
     $files = $adapter->loadAffectedPaths();
     sort($files);

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1591,20 +1591,6 @@ final class DifferentialTransactionEditor
     return $adapter;
   }
 
-  protected function didApplyHeraldRules(
-    PhabricatorLiskDAO $object,
-    HeraldAdapter $adapter,
-    HeraldTranscript $transcript) {
-
-    // Apply build plans.
-    HarbormasterBuildable::applyBuildPlans(
-      $adapter->getDiff()->getPHID(),
-      $adapter->getPHID(),
-      $adapter->getBuildPlans());
-
-    return array();
-  }
-
   /**
    * Update the table which links Differential revisions to paths they affect,
    * so Diffusion can efficiently find pending revisions for a given file.

src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php

@@ -1,16 +1,17 @@
 <?php
 
 final class HeraldDifferentialRevisionAdapter
-  extends HeraldDifferentialAdapter {
+  extends HeraldDifferentialAdapter
+  implements HarbormasterBuildableAdapterInterface {
 
   protected $revision;
 
-  protected $buildPlans = array();
-
   protected $affectedPackages;
   protected $changesets;
   private $haveHunks;
 
+  private $buildPlanPHIDs = array();
+
   public function getAdapterApplicationClass() {
     return 'PhabricatorDifferentialApplication';
   }
@@ -79,10 +80,6 @@ final class HeraldDifferentialRevisionAdapter
     return $object;
   }
 
-  public function getBuildPlans() {
-    return $this->buildPlans;
-  }
-
   public function getHeraldName() {
     return $this->revision->getTitle();
   }
@@ -130,44 +127,24 @@ final class HeraldDifferentialRevisionAdapter
     return mpull($reviewers, 'getReviewerPHID');
   }
 
-  public function getActions($rule_type) {
+
-    switch ($rule_type) {
+/* -(  HarbormasterBuildableAdapterInterface  )------------------------------ */
-      case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+
-        return array_merge(
+
-          array(
+  public function getHarbormasterBuildablePHID() {
-            self::ACTION_APPLY_BUILD_PLANS,
+    return $this->getDiff()->getPHID();
-          ),
-          parent::getActions($rule_type));
-      case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
-        return array_merge(
-          array(),
-          parent::getActions($rule_type));
-    }
   }
 
-  public function applyHeraldEffects(array $effects) {
+  public function getHarbormasterContainerPHID() {
-    assert_instances_of($effects, 'HeraldEffect');
+    return $this->getObject()->getPHID();
+  }
 
-    $result = array();
+  public function getQueuedHarbormasterBuildPlanPHIDs() {
+    return $this->buildPlanPHIDs;
+  }
 
-    foreach ($effects as $effect) {
+  public function queueHarbormasterBuildPlanPHID($phid) {
-      $action = $effect->getAction();
+    $this->buildPlanPHIDs[] = $phid;
-      switch ($action) {
-        case self::ACTION_APPLY_BUILD_PLANS:
-          foreach ($effect->getTarget() as $phid) {
-            $this->buildPlans[] = $phid;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Applied build plans.'));
-          break;
-        default:
-          $result[] = $this->applyStandardEffect($effect);
-          break;
-      }
-    }
-    return $result;
   }
 
 }

src/applications/diffusion/herald/HeraldCommitAdapter.php

@@ -1,6 +1,8 @@
 <?php
 
-final class HeraldCommitAdapter extends HeraldAdapter {
+final class HeraldCommitAdapter
+  extends HeraldAdapter
+  implements HarbormasterBuildableAdapterInterface {
 
   protected $diff;
   protected $revision;
@@ -11,13 +13,14 @@ final class HeraldCommitAdapter extends HeraldAdapter {
   private $commitDiff;
 
   protected $auditMap = array();
-  protected $buildPlans = array();
 
   protected $affectedPaths;
   protected $affectedRevision;
   protected $affectedPackages;
   protected $auditNeededPackages;
 
+  private $buildPlanPHIDs = array();
+
   public function getAdapterApplicationClass() {
     return 'PhabricatorDiffusionApplication';
   }
@@ -90,7 +93,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
         return array_merge(
           array(
             self::ACTION_AUDIT,
-            self::ACTION_APPLY_BUILD_PLANS,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
@@ -151,10 +153,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
     return $this->auditMap;
   }
 
-  public function getBuildPlans() {
-    return $this->buildPlans;
-  }
-
   public function getHeraldName() {
     return
       'r'.
@@ -343,15 +341,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
             true,
             pht('Triggered an audit.'));
           break;
-        case self::ACTION_APPLY_BUILD_PLANS:
-          foreach ($effect->getTarget() as $phid) {
-            $this->buildPlans[] = $phid;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Applied build plans.'));
-          break;
         default:
           $result[] = $this->applyStandardEffect($effect);
           break;
@@ -360,4 +349,24 @@ final class HeraldCommitAdapter extends HeraldAdapter {
     return $result;
   }
 
+
+/* -(  HarbormasterBuildableAdapterInterface  )------------------------------ */
+
+
+  public function getHarbormasterBuildablePHID() {
+    return $this->getObject()->getPHID();
+  }
+
+  public function getHarbormasterContainerPHID() {
+    return $this->getObject()->getRepository()->getPHID();
+  }
+
+  public function getQueuedHarbormasterBuildPlanPHIDs() {
+    return $this->buildPlanPHIDs;
+  }
+
+  public function queueHarbormasterBuildPlanPHID($phid) {
+    $this->buildPlanPHIDs[] = $phid;
+  }
+
 }

src/applications/harbormaster/herald/HarbormasterBuildableAdapterInterface.php

@@ -0,0 +1,34 @@
+<?php
+
+interface HarbormasterBuildableAdapterInterface {
+
+  public function getHarbormasterBuildablePHID();
+  public function getHarbormasterContainerPHID();
+  public function getQueuedHarbormasterBuildPlanPHIDs();
+  public function queueHarbormasterBuildPlanPHID($phid);
+
+}
+
+// TEMPLATE IMPLEMENTATION /////////////////////////////////////////////////////
+
+
+/* -(  HarbormasterBuildableAdapterInterface  )------------------------------ */
+/*
+
+  public function getHarbormasterBuildablePHID() {
+    return $this->getObject()->getPHID();
+  }
+
+  public function getHarbormasterContainerPHID() {
+    return null;
+  }
+
+  public function getQueuedHarbormasterBuildPlanPHIDs() {
+    return $this->buildPlanPHIDs;
+  }
+
+  public function queueHarbormasterBuildPlanPHID($phid) {
+    $this->buildPlanPHIDs[] = $phid;
+  }
+
+*/

src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php

@@ -0,0 +1,121 @@
+<?php
+
+final class HarbormasterRunBuildPlansHeraldAction
+  extends HeraldAction {
+
+  const DO_NO_TARGETS = 'do.no-targets';
+  const DO_INVALID = 'do.invalid';
+  const DO_BUILD = 'do.build';
+
+  const ACTIONCONST = 'harbormaster.build';
+
+  public function getActionGroupKey() {
+    return HeraldSupportActionGroup::ACTIONGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    $adapter = $this->getAdapter();
+    return ($adapter instanceof HarbormasterBuildableAdapterInterface);
+  }
+
+  protected function applyBuilds(array $phids) {
+    $adapter = $this->getAdapter();
+
+    $phids = array_fuse($phids);
+    if (!$phids) {
+      $this->logEffect(self::DO_NO_TARGETS);
+      return;
+    }
+
+    $plans = id(new HarbormasterBuildPlanQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withPHIDs($phids)
+      ->execute();
+    $plans = mpull($plans, null, 'getPHID');
+
+    $invalid = array();
+    foreach ($phids as $phid) {
+      if (empty($plans[$phid])) {
+        $invalid[] = $phid;
+        unset($plans[$phid]);
+      }
+    }
+
+    if ($invalid) {
+      $this->logEffect(self::DO_INVALID, $phids);
+    }
+
+    if (!$phids) {
+      return;
+    }
+
+    foreach ($phids as $phid) {
+      $adapter->queueHarbormasterBuildPlanPHID($phid);
+    }
+
+    $this->logEffect(self::DO_BUILD, $phids);
+  }
+
+  protected function getActionEffectMap() {
+    return array(
+      self::DO_NO_TARGETS => array(
+        'icon' => 'fa-ban',
+        'color' => 'grey',
+        'name' => pht('No Targets'),
+      ),
+      self::DO_INVALID => array(
+        'icon' => 'fa-ban',
+        'color' => 'red',
+        'name' => pht('Invalid Targets'),
+      ),
+      self::DO_ALREADY_REQUIRED => array(
+        'icon' => 'fa-play',
+        'color' => 'green',
+        'name' => pht('Building'),
+      ),
+    );
+  }
+
+  public function renderActionEffectDescription($type, $data) {
+    switch ($type) {
+      case self::DO_NO_TARGETS:
+        return pht('Rule lists no targets.');
+      case self::DO_INVALID:
+        return pht(
+          '%s build plan(s) are not valid: %s.',
+          new PhutilNumber(count($data)),
+          $this->renderHandleList($data));
+      case self::DO_REQUIRED:
+        return pht(
+          'Started %s build(s): %s.',
+          new PhutilNumber(count($data)),
+          $this->renderHandleList($data));
+    }
+  }
+
+  public function getHeraldActionName() {
+    return pht('Run build plans');
+  }
+
+  public function supportsRuleType($rule_type) {
+    return ($rule_type != HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
+  }
+
+  public function applyEffect($object, HeraldEffect $effect) {
+    return $this->applyBuilds($effect->getTarget());
+  }
+
+  public function getHeraldActionStandardType() {
+    return self::STANDARD_PHID_LIST;
+  }
+
+  protected function getDatasource() {
+    return new HarbormasterBuildPlanDatasource();
+  }
+
+  public function renderActionDescription($value) {
+    return pht(
+      'Run build plans: %s.',
+      $this->renderHandleList($value));
+  }
+}

src/applications/harbormaster/storage/HarbormasterBuildable.php

@@ -104,7 +104,7 @@ final class HarbormasterBuildable extends HarbormasterDAO
     $container_phid,
     array $plan_phids) {
 
-    if (count($plan_phids) === 0) {
+    if (!$plan_phids) {
       return;
     }
 

src/applications/herald/adapter/HeraldAdapter.php

@@ -27,7 +27,6 @@ abstract class HeraldAdapter extends Phobject {
   const CONDITION_IS_FALSE        = 'false';
 
   const ACTION_AUDIT        = 'audit';
-  const ACTION_APPLY_BUILD_PLANS = 'applybuildplans';
   const ACTION_BLOCK = 'block';
 
   private $contentSource;
@@ -714,7 +713,6 @@ abstract class HeraldAdapter extends Phobject {
       case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
         $standard = array(
           self::ACTION_AUDIT        => pht('Trigger an Audit by'),
-          self::ACTION_APPLY_BUILD_PLANS => pht('Run build plans'),
           self::ACTION_BLOCK => pht('Block change with message'),
         );
         break;
@@ -800,9 +798,6 @@ abstract class HeraldAdapter extends Phobject {
     } else {
       switch ($action) {
         case self::ACTION_AUDIT:
-        case self::ACTION_APPLY_BUILD_PLANS:
-          return $this->buildTokenizerFieldValue(
-            new HarbormasterBuildPlanDatasource());
         case self::ACTION_BLOCK:
           return new HeraldTextFieldValue();
       }

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -2811,6 +2811,13 @@ abstract class PhabricatorApplicationTransactionEditor
     $this->setHeraldAdapter($adapter);
     $this->setHeraldTranscript($xscript);
 
+    if ($adapter instanceof HarbormasterBuildableAdapterInterface) {
+      HarbormasterBuildable::applyBuildPlans(
+        $adapter->getHarbormasterBuildablePHID(),
+        $adapter->getHarbormasterContainerPHID(),
+        $adapter->getQueuedHarbormasterBuildPlanPHIDs());
+    }
+
     return array_merge(
       $this->didApplyHeraldRules($object, $adapter, $xscript),
       $adapter->getQueuedTransactions());