Move structural build publishing logic to BuildEngine, provide "bin/harbormaster publish"

Summary:
Depends on D19278. Ref T13110. This moves most of the structural logic for publishing builds to BuildableEngine and provides a `bin/harbormaster publish` to make publishing easy to retry/debug.

This intentionally removes the bit which actually does anything when builds publish. Followup changes will implement application-specific versions of the publishing logic in Differential and Diffusion.

Test Plan: Ran `bin/harbormaster publish Bxxx`, saw it do nothing (but not crash).

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13110

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

src/__phutil_library_map__.php

@@ -1324,6 +1324,7 @@ phutil_register_library_map(array(
     'HarbormasterLogWorker' => 'applications/harbormaster/worker/HarbormasterLogWorker.php',
     'HarbormasterManagementArchiveLogsWorkflow' => 'applications/harbormaster/management/HarbormasterManagementArchiveLogsWorkflow.php',
     'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php',
+    'HarbormasterManagementPublishWorkflow' => 'applications/harbormaster/management/HarbormasterManagementPublishWorkflow.php',
     'HarbormasterManagementRebuildLogWorkflow' => 'applications/harbormaster/management/HarbormasterManagementRebuildLogWorkflow.php',
     'HarbormasterManagementRestartWorkflow' => 'applications/harbormaster/management/HarbormasterManagementRestartWorkflow.php',
     'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php',
@@ -6675,6 +6676,7 @@ phutil_register_library_map(array(
     'HarbormasterLogWorker' => 'HarbormasterWorker',
     'HarbormasterManagementArchiveLogsWorkflow' => 'HarbormasterManagementWorkflow',
     'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow',
+    'HarbormasterManagementPublishWorkflow' => 'HarbormasterManagementWorkflow',
     'HarbormasterManagementRebuildLogWorkflow' => 'HarbormasterManagementWorkflow',
     'HarbormasterManagementRestartWorkflow' => 'HarbormasterManagementWorkflow',
     'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow',

src/applications/harbormaster/engine/HarbormasterBuildEngine.php

@@ -489,6 +489,8 @@ final class HarbormasterBuildEngine extends Phobject {
       }
     }
 
+    $old = clone $buildable;
+
     // Don't update the buildable status if we're still preparing builds: more
     // builds may still be scheduled shortly, so even if every build we know
     // about so far has passed, that doesn't mean the buildable has actually
@@ -515,8 +517,7 @@ final class HarbormasterBuildEngine extends Phobject {
         $new_status = HarbormasterBuildableStatus::STATUS_BUILDING;
       }
 
-      $old_status = $buildable->getBuildableStatus();
+      $did_update = ($old->getBuildableStatus() !== $new_status);
-      $did_update = ($old_status != $new_status);
       if ($did_update) {
         $buildable->setBuildableStatus($new_status);
         $buildable->save();
@@ -530,81 +531,45 @@ final class HarbormasterBuildEngine extends Phobject {
       return;
     }
 
-    // If we changed the buildable status, try to post a transaction to the
+    $this->publishBuildable($old, $buildable);
-    // object about it. We can safely do this outside of the locked region.
-
-    // NOTE: We only post transactions for automatic buildables, not for
-    // manual ones: manual builds are test builds, whoever is doing tests
-    // can look at the results themselves, and other users generally don't
-    // care about the outcome.
-
-    $should_publish =
-      ($did_update) &&
-      ($new_status != HarbormasterBuildableStatus::STATUS_BUILDING) &&
-      (!$buildable->getIsManualBuildable());
-
-    if (!$should_publish) {
-      return;
   }
 
+  public function publishBuildable(
+    HarbormasterBuildable $old,
+    HarbormasterBuildable $new) {
+
+    $viewer = $this->getViewer();
+
+    // Publish the buildable. We publish buildables even if they haven't
+    // changed status in Harbormaster because applications may care about
+    // different things than Harbormaster does. For example, Differential
+    // does not care about local lint and unit tests when deciding whether
+    // a revision should move out of draft or not.
+
+    // NOTE: We're publishing both automatic and manual buildables. Buildable
+    // objects should generally ignore manual buildables, but it's up to them
+    // to decide.
+
     $object = id(new PhabricatorObjectQuery())
       ->setViewer($viewer)
-      ->withPHIDs(array($buildable->getBuildablePHID()))
+      ->withPHIDs(array($new->getBuildablePHID()))
       ->executeOne();
     if (!$object) {
       return;
     }
 
-    $publish_phid = $object->getHarbormasterPublishablePHID();
+    $engine = HarbormasterBuildableEngine::newForObject($object, $viewer);
-    if (!$publish_phid) {
-      return;
-    }
-
-    if ($publish_phid === $object->getPHID()) {
-      $publish = $object;
-    } else {
-      $publish = id(new PhabricatorObjectQuery())
-        ->setViewer($viewer)
-        ->withPHIDs(array($publish_phid))
-        ->executeOne();
-      if (!$publish) {
-        return;
-      }
-    }
-
-    if (!($publish instanceof PhabricatorApplicationTransactionInterface)) {
-      return;
-    }
-
-    $template = $publish->getApplicationTransactionTemplate();
-    if (!$template) {
-      return;
-    }
-
-    $template
-      ->setTransactionType(PhabricatorTransactions::TYPE_BUILDABLE)
-      ->setMetadataValue(
-        'harbormaster:buildablePHID',
-        $buildable->getPHID())
-      ->setOldValue($old_status)
-      ->setNewValue($new_status);
-
-    $harbormaster_phid = id(new PhabricatorHarbormasterApplication())
-      ->getPHID();
 
     $daemon_source = PhabricatorContentSource::newForSource(
       PhabricatorDaemonContentSource::SOURCECONST);
 
-    $editor = $publish->getApplicationTransactionEditor()
+    $harbormaster_phid = id(new PhabricatorHarbormasterApplication())
-      ->setActor($viewer)
+      ->getPHID();
+
+    $engine
       ->setActingAsPHID($harbormaster_phid)
       ->setContentSource($daemon_source)
-      ->setContinueOnNoEffect(true)
+      ->publishBuildable($old, $new);
-      ->setContinueOnMissingFields(true);
-
-    $editor->applyTransactions(
-      $publish->getApplicationTransactionObject(),
-      array($template));
   }
 
   private function releaseAllArtifacts(HarbormasterBuild $build) {

src/applications/harbormaster/engine/HarbormasterBuildableEngine.php

@@ -1,4 +1,101 @@
 <?php
 
 abstract class HarbormasterBuildableEngine
-  extends Phobject {}
+  extends Phobject {
+
+  private $viewer;
+  private $actingAsPHID;
+  private $contentSource;
+  private $object;
+
+  final public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  final public function getViewer() {
+    return $this->viewer;
+  }
+
+  final public function setActingAsPHID($acting_as_phid) {
+    $this->actingAsPHID = $acting_as_phid;
+    return $this;
+  }
+
+  final public function getActingAsPHID() {
+    return $this->actingAsPHID;
+  }
+
+  final public function setContentSource(
+    PhabricatorContentSource $content_source) {
+    $this->contentSource = $content_source;
+    return $this;
+  }
+
+  final public function getContentSource() {
+    return $this->contentSource;
+  }
+
+  final public function setObject(HarbormasterBuildableInterface $object) {
+    $this->object = $object;
+    return $this;
+  }
+
+  final public function getObject() {
+    return $this->object;
+  }
+
+  final public function publishBuildable(
+    HarbormasterBuildable $old,
+    HarbormasterBuildable $new) {
+    return;
+  }
+
+  final public static function newForObject(
+    HarbormasterBuildableInterface $object,
+    PhabricatorUser $viewer) {
+    return $object->newBuildableEngine()
+      ->setViewer($viewer)
+      ->setObject($object);
+  }
+
+  final protected function newEditor() {
+    $publishable = $this->getObject();
+
+    $viewer = $this->getViewer();
+
+    $editor = $publishable->getApplicationTransactionEditor()
+      ->setActor($viewer)
+      ->setContinueOnNoEffect(true)
+      ->setContinueOnMissingFields(true);
+
+    $acting_as_phid = $this->getActingAsPHID();
+    if ($acting_as_phid !== null) {
+      $editor->setActingAsPHID($acting_as_phid);
+    }
+
+    $content_source = $this->getContentSource();
+    if ($content_source !== null) {
+      $editor->setContentSource($content_source);
+    }
+
+    return $editor;
+  }
+
+  final protected function newTransaction() {
+    $publishable = $this->getObject();
+
+    return $publishable->getApplicationTransactionTemplate();
+  }
+
+  final protected function applyTransactions(array $xactions) {
+    $publishable = $this->getObject();
+    $editor = $this->newEditor();
+
+    $editor->applyTransactions(
+      $publishable->getApplicationTransactionObject(),
+      $xactions);
+  }
+
+
+}

src/applications/harbormaster/management/HarbormasterManagementPublishWorkflow.php

@@ -0,0 +1,87 @@
+<?php
+
+final class HarbormasterManagementPublishWorkflow
+  extends HarbormasterManagementWorkflow {
+
+  protected function didConstruct() {
+    $this
+      ->setName('publish')
+      ->setExamples(pht('**publish** __buildable__ ...'))
+      ->setSynopsis(
+        pht(
+          'Publish a buildable. This is primarily useful for developing '.
+          'and debugging applications which have buildable objects.'))
+      ->setArguments(
+        array(
+          array(
+            'name' =>  'buildable',
+            'wildcard' => true,
+          ),
+        ));
+  }
+
+  public function execute(PhutilArgumentParser $args) {
+    $viewer = $this->getViewer();
+
+    $buildable_names = $args->getArg('buildable');
+    if (!$buildable_names) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Name one or more buildables to publish, like "B123".'));
+    }
+
+    $query = id(new PhabricatorObjectQuery())
+      ->setViewer($viewer)
+      ->withNames($buildable_names);
+
+    $query->execute();
+
+    $result_map = $query->getNamedResults();
+
+    foreach ($buildable_names as $name) {
+      if (!isset($result_map[$name])) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'Argument "%s" does not name a buildable. Provide one or more '.
+            'valid buildable monograms or PHIDs.',
+            $name));
+      }
+    }
+
+    foreach ($result_map as $name => $result) {
+      if (!($result instanceof HarbormasterBuildable)) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'Object "%s" is not a HarbormasterBuildable (it is a "%s"). '.
+            'Name one or more buildables to publish, like "B123".',
+            get_class($result)));
+      }
+    }
+
+    foreach ($result_map as $buildable) {
+      echo tsprintf(
+        "%s\n",
+        pht(
+          'Publishing "%s"...',
+        $buildable->getMonogram()));
+
+      // Reload the buildable to pick up builds.
+      $buildable = id(new HarbormasterBuildableQuery())
+        ->setViewer($viewer)
+        ->withIDs(array($buildable->getID()))
+        ->needBuilds(true)
+        ->executeOne();
+
+      $engine = id(new HarbormasterBuildEngine())
+        ->setViewer($viewer)
+        ->publishBuildable($buildable, $buildable);
+    }
+
+    echo tsprintf(
+      "%s\n",
+      pht('Done.'));
+
+    return 0;
+  }
+
+}