Add harbormaster.createartifact

Summary:
Ref T8659. In the general case, this eventually allows build processes to do things like:

  - Upload build results (like a ".app" or ".exe" or other binary).
  - Pass complex results between build steps (e.g., build step A does something hard and build step B uses it to do something else).

Today, we're a long way away from having the infrastructure for that. However, it is useful to let third party build processes (like Jenkins) upload URIs that link back to the external build results.

This adds `harbormaster.createartifact` so they can do that. The only useful thing to do with this method today is have your Jenkins build do this:

  params = array(
    "uri": "https://jenkins.mycompany.com/build/23923/details/",
    "name": "View Build Results in Jenkins",
    "ui.external": true,
  );
  harbormaster.createartifact(target, 'uri', params);

Then (after the next diff) we'll show a link in Differential and a prominent link in Harbormaster. I didn't actually do the UI stuff in this diff since it's already pretty big.

This change moves a lot of code around, too:

  - Adds PHIDs to artifacts.
  - It modularizes build artifact types (currently "file", "host" and "URI").
  - It formalizes build artifact parameters and construction:
    - This lets me generate usable documentation about how to create artifacts.
    - This prevents users from doing dangerous or policy-violating things.
  - It does some other general modernization.

Test Plan:
{F715633}

{F715634}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8659

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

resources/sql/autopatches/20150814.harbormater.artifact.phid.sql

@@ -0,0 +1,5 @@
+ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildartifact
+  ADD phid VARBINARY(64) NOT NULL AFTER id;
+
+UPDATE {$NAMESPACE}_harbormaster.harbormaster_buildartifact
+  SET phid = CONCAT('PHID-HMBA-', id) WHERE phid = '';

src/__phutil_library_map__.php

@@ -915,12 +915,14 @@ phutil_register_library_map(array(
     'FundSchemaSpec' => 'applications/fund/storage/FundSchemaSpec.php',
     'HarbormasterArcLintBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterArcLintBuildStepImplementation.php',
     'HarbormasterArcUnitBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterArcUnitBuildStepImplementation.php',
+    'HarbormasterArtifact' => 'applications/harbormaster/artifact/HarbormasterArtifact.php',
     'HarbormasterAutotargetsTestCase' => 'applications/harbormaster/__tests__/HarbormasterAutotargetsTestCase.php',
     'HarbormasterBuild' => 'applications/harbormaster/storage/build/HarbormasterBuild.php',
     'HarbormasterBuildAbortedException' => 'applications/harbormaster/exception/HarbormasterBuildAbortedException.php',
     'HarbormasterBuildActionController' => 'applications/harbormaster/controller/HarbormasterBuildActionController.php',
     'HarbormasterBuildArcanistAutoplan' => 'applications/harbormaster/autoplan/HarbormasterBuildArcanistAutoplan.php',
     'HarbormasterBuildArtifact' => 'applications/harbormaster/storage/build/HarbormasterBuildArtifact.php',
+    'HarbormasterBuildArtifactPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildArtifactPHIDType.php',
     'HarbormasterBuildArtifactQuery' => 'applications/harbormaster/query/HarbormasterBuildArtifactQuery.php',
     'HarbormasterBuildAutoplan' => 'applications/harbormaster/autoplan/HarbormasterBuildAutoplan.php',
     'HarbormasterBuildCommand' => 'applications/harbormaster/storage/HarbormasterBuildCommand.php',
@@ -980,9 +982,12 @@ phutil_register_library_map(array(
     'HarbormasterCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php',
     'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php',
     'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php',
+    'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php',
     'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php',
     'HarbormasterExternalBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterExternalBuildStepGroup.php',
+    'HarbormasterFileArtifact' => 'applications/harbormaster/artifact/HarbormasterFileArtifact.php',
     'HarbormasterHTTPRequestBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php',
+    'HarbormasterHostArtifact' => 'applications/harbormaster/artifact/HarbormasterHostArtifact.php',
     'HarbormasterLeaseHostBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php',
     'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php',
     'HarbormasterLintPropertyView' => 'applications/harbormaster/view/HarbormasterLintPropertyView.php',
@@ -1018,6 +1023,7 @@ phutil_register_library_map(array(
     'HarbormasterTestBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterTestBuildStepGroup.php',
     'HarbormasterThrowExceptionBuildStep' => 'applications/harbormaster/step/HarbormasterThrowExceptionBuildStep.php',
     'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php',
+    'HarbormasterURIArtifact' => 'applications/harbormaster/artifact/HarbormasterURIArtifact.php',
     'HarbormasterUnitMessagesController' => 'applications/harbormaster/controller/HarbormasterUnitMessagesController.php',
     'HarbormasterUnitPropertyView' => 'applications/harbormaster/view/HarbormasterUnitPropertyView.php',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php',
@@ -4603,6 +4609,7 @@ phutil_register_library_map(array(
     'FundSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'HarbormasterArcLintBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterArcUnitBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
+    'HarbormasterArtifact' => 'Phobject',
     'HarbormasterAutotargetsTestCase' => 'PhabricatorTestCase',
     'HarbormasterBuild' => array(
       'HarbormasterDAO',
@@ -4616,6 +4623,7 @@ phutil_register_library_map(array(
       'HarbormasterDAO',
       'PhabricatorPolicyInterface',
     ),
+    'HarbormasterBuildArtifactPHIDType' => 'PhabricatorPHIDType',
     'HarbormasterBuildArtifactQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'HarbormasterBuildAutoplan' => 'Phobject',
     'HarbormasterBuildCommand' => 'HarbormasterDAO',
@@ -4700,9 +4708,12 @@ phutil_register_library_map(array(
     'HarbormasterCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod',
     'HarbormasterController' => 'PhabricatorController',
+    'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
     'HarbormasterDAO' => 'PhabricatorLiskDAO',
     'HarbormasterExternalBuildStepGroup' => 'HarbormasterBuildStepGroup',
+    'HarbormasterFileArtifact' => 'HarbormasterArtifact',
     'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
+    'HarbormasterHostArtifact' => 'HarbormasterArtifact',
     'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterLintMessagesController' => 'HarbormasterController',
     'HarbormasterLintPropertyView' => 'AphrontView',
@@ -4738,6 +4749,7 @@ phutil_register_library_map(array(
     'HarbormasterTestBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterThrowExceptionBuildStep' => 'HarbormasterBuildStepImplementation',
     'HarbormasterUIEventListener' => 'PhabricatorEventListener',
+    'HarbormasterURIArtifact' => 'HarbormasterArtifact',
     'HarbormasterUnitMessagesController' => 'HarbormasterController',
     'HarbormasterUnitPropertyView' => 'AphrontView',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation',

src/applications/drydock/query/DrydockLeaseQuery.php

@@ -27,19 +27,12 @@ final class DrydockLeaseQuery extends DrydockQuery {
     return $this;
   }
 
+  public function newResultObject() {
+    return new DrydockLease();
+  }
+
   protected function loadPage() {
-    $table = new DrydockLease();
+    return $this->loadStandardPage($this->newResultObject());
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT lease.* FROM %T lease %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
   }
 
   protected function willFilterPage(array $leases) {
@@ -69,40 +62,38 @@ final class DrydockLeaseQuery extends DrydockQuery {
     return $leases;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
-    $where = array();
+    $where = parent::buildWhereClauseParts($conn);
 
-    if ($this->resourceIDs) {
+    if ($this->resourceIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'resourceID IN (%Ld)',
         $this->resourceIDs);
     }
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->statuses) {
+    if ($this->statuses !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'status IN (%Ld)',
         $this->statuses);
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
+    return $where;
-
-    return $this->formatWhereClause($where);
   }
 
 }

src/applications/harbormaster/artifact/HarbormasterArtifact.php

@@ -0,0 +1,88 @@
+<?php
+
+
+abstract class HarbormasterArtifact extends Phobject {
+
+  private $buildArtifact;
+
+  abstract public function getArtifactTypeName();
+
+  public function getArtifactTypeSummary() {
+    return $this->getArtifactTypeDescription();
+  }
+
+  abstract public function getArtifactTypeDescription();
+  abstract public function getArtifactParameterSpecification();
+  abstract public function getArtifactParameterDescriptions();
+  abstract public function willCreateArtifact(PhabricatorUser $actor);
+
+  public function validateArtifactData(array $artifact_data) {
+    $artifact_spec = $this->getArtifactParameterSpecification();
+    PhutilTypeSpec::checkMap($artifact_data, $artifact_spec);
+  }
+
+  public function renderArtifactSummary(PhabricatorUser $viewer) {
+    return null;
+  }
+
+  public function releaseArtifact(PhabricatorUser $actor) {
+    return;
+  }
+
+  public function getArtifactDataExample() {
+    return null;
+  }
+
+  public function setBuildArtifact(HarbormasterBuildArtifact $build_artifact) {
+    $this->buildArtifact = $build_artifact;
+    return $this;
+  }
+
+  public function getBuildArtifact() {
+    return $this->buildArtifact;
+  }
+
+  final public function getArtifactConstant() {
+    $class = new ReflectionClass($this);
+
+    $const = $class->getConstant('ARTIFACTCONST');
+    if ($const === false) {
+      throw new Exception(
+        pht(
+          '"%s" class "%s" must define a "%s" property.',
+          __CLASS__,
+          get_class($this),
+          'ARTIFACTCONST'));
+    }
+
+    $limit = self::getArtifactConstantByteLimit();
+    if (!is_string($const) || (strlen($const) > $limit)) {
+      throw new Exception(
+        pht(
+          '"%s" class "%s" has an invalid "%s" property. Action constants '.
+          'must be strings and no more than %s bytes in length.',
+          __CLASS__,
+          get_class($this),
+          'ARTIFACTCONST',
+          new PhutilNumber($limit)));
+    }
+
+    return $const;
+  }
+
+  final public static function getArtifactConstantByteLimit() {
+    return 32;
+  }
+
+  final public static function getAllArtifactTypes() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getArtifactConstant')
+      ->execute();
+  }
+
+  final public static function getArtifactType($type) {
+    return idx(self::getAllArtifactTypes(), $type);
+  }
+
+}

src/applications/harbormaster/artifact/HarbormasterFileArtifact.php

@@ -0,0 +1,66 @@
+<?php
+
+final class HarbormasterFileArtifact extends HarbormasterArtifact {
+
+  const ARTIFACTCONST = 'file';
+
+  public function getArtifactTypeName() {
+    return pht('File');
+  }
+
+  public function getArtifactTypeDescription() {
+    return pht(
+      'Stores a reference to file data which has been uploaded to '.
+      'Phabricator.');
+  }
+
+  public function getArtifactParameterSpecification() {
+    return array(
+      'filePHID' => 'string',
+    );
+  }
+
+  public function getArtifactParameterDescriptions() {
+    return array(
+      'filePHID' => pht('File to create an artifact from.'),
+    );
+  }
+
+  public function getArtifactDataExample() {
+    return array(
+      'filePHID' => 'PHID-FILE-abcdefghijklmnopqrst',
+    );
+  }
+
+  public function renderArtifactSummary(PhabricatorUser $viewer) {
+    $artifact = $this->getBuildArtifact();
+    $file_phid = $artifact->getProperty('filePHID');
+    return $viewer->renderHandle($file_phid);
+  }
+
+  public function willCreateArtifact(PhabricatorUser $actor) {
+    // NOTE: This is primarily making sure the actor has permission to view the
+    // file. We don't want to let you run builds using files you don't have
+    // permission to see, since this could let you violate permissions.
+    $this->loadArtifactFile($actor);
+  }
+
+  public function loadArtifactFile(PhabricatorUser $viewer) {
+    $artifact = $this->getBuildArtifact();
+    $file_phid = $artifact->getProperty('filePHID');
+
+    $file = id(new PhabricatorFileQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($file_phid))
+      ->executeOne();
+    if (!$file) {
+      throw new Exception(
+        pht(
+          'File PHID "%s" does not correspond to a valid file.',
+          $file_phid));
+    }
+
+    return $file;
+  }
+
+}

src/applications/harbormaster/artifact/HarbormasterHostArtifact.php

@@ -0,0 +1,74 @@
+<?php
+
+final class HarbormasterHostArtifact extends HarbormasterArtifact {
+
+  const ARTIFACTCONST = 'host';
+
+  public function getArtifactTypeName() {
+    return pht('Drydock Host');
+  }
+
+  public function getArtifactTypeDescription() {
+    return pht('References a host lease from Drydock.');
+  }
+
+
+  public function getArtifactParameterSpecification() {
+    return array(
+      'drydockLeasePHID' => 'string',
+    );
+  }
+
+  public function getArtifactParameterDescriptions() {
+    return array(
+      'drydockLeasePHID' => pht(
+        'Drydock host lease to create an artifact from.'),
+    );
+  }
+
+  public function getArtifactDataExample() {
+    return array(
+      'drydockLeasePHID' => 'PHID-DRYL-abcdefghijklmnopqrst',
+    );
+  }
+
+  public function renderArtifactSummary(PhabricatorUser $viewer) {
+    $artifact = $this->getBuildArtifact();
+    $file_phid = $artifact->getProperty('drydockLeasePHID');
+    return $viewer->renderHandle($file_phid);
+  }
+
+  public function willCreateArtifact(PhabricatorUser $actor) {
+    $this->loadArtifactLease($actor);
+  }
+
+  public function loadArtifactLease(PhabricatorUser $viewer) {
+    $artifact = $this->getBuildArtifact();
+    $lease_phid = $artifact->getProperty('drydockLeasePHID');
+
+    $lease = id(new DrydockLeaseQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($lease_phid))
+      ->executeOne();
+    if (!$lease) {
+      throw new Exception(
+        pht(
+          'Drydock lease PHID "%s" does not correspond to a valid lease.',
+          $lease_phid));
+    }
+
+    return $lease;
+  }
+
+  public function releaseArtifact(PhabricatorUser $actor) {
+    $lease = $this->loadArtifactLease($actor);
+    $resource = $lease->getResource();
+    $blueprint = $resource->getBlueprint();
+
+    if ($lease->isActive()) {
+      $blueprint->releaseLease($resource, $lease);
+    }
+  }
+
+
+}

src/applications/harbormaster/artifact/HarbormasterURIArtifact.php

@@ -0,0 +1,100 @@
+<?php
+
+final class HarbormasterURIArtifact extends HarbormasterArtifact {
+
+  const ARTIFACTCONST = 'uri';
+
+  public function getArtifactTypeName() {
+    return pht('URI');
+  }
+
+  public function getArtifactTypeSummary() {
+    return pht('Stores a URI.');
+  }
+
+  public function getArtifactTypeDescription() {
+    return pht(
+      "Stores a URI.\n\n".
+      "With `ui.external`, you can use this artifact type to add links to ".
+      "build results in an external build system.");
+  }
+
+  public function getArtifactParameterSpecification() {
+    return array(
+      'uri' => 'string',
+      'name' => 'optional string',
+      'ui.external' => 'optional bool',
+    );
+  }
+
+  public function getArtifactParameterDescriptions() {
+    return array(
+      'uri' => pht('The URI to store.'),
+      'name' => pht('Optional label for this URI.'),
+      'ui.external' => pht(
+        'If true, display this URI in the UI as an link to '.
+        'additional build details in an external build system.'),
+    );
+  }
+
+  public function getArtifactDataExample() {
+    return array(
+      'uri' => 'https://buildserver.mycompany.com/build/123/',
+      'name' => pht('View External Build Results'),
+      'ui.external' => true,
+    );
+  }
+
+  public function renderArtifactSummary(PhabricatorUser $viewer) {
+    $artifact = $this->getBuildArtifact();
+    $uri = $artifact->getProperty('uri');
+
+    try {
+      $this->validateURI($uri);
+    } catch (Exception $ex) {
+      return pht('<Invalid URI>');
+    }
+
+    $name = $artifact->getProperty('name', $uri);
+
+    return phutil_tag(
+      'a',
+      array(
+        'href' => $uri,
+        'target' => '_blank',
+      ),
+      $name);
+  }
+
+  public function willCreateArtifact(PhabricatorUser $actor) {
+    $artifact = $this->getBuildArtifact();
+    $uri = $artifact->getProperty('uri');
+    $this->validateURI($uri);
+  }
+
+  private function validateURI($raw_uri) {
+    $uri = new PhutilURI($raw_uri);
+
+    $protocol = $uri->getProtocol();
+    if (!strlen($protocol)) {
+      throw new Exception(
+        pht(
+          'Unable to identify the protocol for URI "%s". URIs must be '.
+          'fully qualified and have an identifiable protocol.',
+          $raw_uri));
+    }
+
+    $protocol_key = 'uri.allowed-protocols';
+    $protocols = PhabricatorEnv::getEnvConfig($protocol_key);
+    if (empty($protocols[$protocol])) {
+      throw new Exception(
+        pht(
+          'URI "%s" does not have an allowable protocol. Configure '.
+          'protocols in `%s`. Allowed protocols are: %s.',
+          $raw_uri,
+          $protocol_key,
+          implode(', ', array_keys($protocols))));
+    }
+  }
+
+}

src/applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php

@@ -15,4 +15,16 @@ abstract class HarbormasterConduitAPIMethod extends ConduitAPIMethod {
     return pht('All Harbormaster APIs are new and subject to change.');
   }
 
+  protected function returnArtifactList(array $artifacts) {
+    $list = array();
+
+    foreach ($artifacts as $artifact) {
+      $list[] = array(
+        'phid' => $artifact->getPHID(),
+      );
+    }
+
+    return $list;
+  }
+
 }

src/applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php

@@ -0,0 +1,129 @@
+<?php
+
+final class HarbormasterCreateArtifactConduitAPIMethod
+  extends HarbormasterConduitAPIMethod {
+
+  public function getAPIMethodName() {
+    return 'harbormaster.createartifact';
+  }
+
+  public function getMethodSummary() {
+    return pht('Create a build artifact.');
+  }
+
+  public function getMethodDescription() {
+    $types = HarbormasterArtifact::getAllArtifactTypes();
+    $types = msort($types, 'getArtifactTypeName');
+
+    $head_key = pht('Key');
+    $head_type = pht('Type');
+    $head_desc = pht('Description');
+    $head_atype = pht('Artifact Type');
+    $head_name = pht('Name');
+    $head_summary = pht('Summary');
+
+    $out = array();
+    $out[] = pht(
+      'Use this method to attach artifacts to build targets while running '.
+      'builds. Artifacts can be used to carry data through a complex build '.
+      'workflow, provide extra information to users, or store build results.');
+    $out[] = null;
+    $out[] = pht(
+      'When creating an artifact, you will choose an `artifactType` from '.
+      'this table. These types of artifacts are supported:');
+
+    $out[] = "| {$head_atype} | {$head_name} | {$head_summary} |";
+    $out[] = '|-------------|--------------|--------------|';
+    foreach ($types as $type) {
+      $type_name = $type->getArtifactTypeName();
+      $type_const = $type->getArtifactConstant();
+      $type_summary = $type->getArtifactTypeSummary();
+      $out[] = "| `{$type_const}` | **{$type_name}** | {$type_summary} |";
+    }
+
+    $out[] = null;
+    $out[] = pht(
+      'Each artifact also needs an `artifactKey`, which names the artifact. '.
+      'Finally, you will provide some `artifactData` to fill in the content '.
+      'of the artifact. The data you provide depends on what type of artifact '.
+      'you are creating.');
+
+    foreach ($types as $type) {
+      $type_name = $type->getArtifactTypeName();
+      $type_const = $type->getArtifactConstant();
+
+      $out[] = $type_name;
+      $out[] = '--------------------------';
+      $out[] = null;
+      $out[] = $type->getArtifactTypeDescription();
+      $out[] = null;
+      $out[] = pht(
+        'Create an artifact of this type by passing `%s` as the '.
+        '`artifactType`. When creating an artifact of this type, provide '.
+        'these parameters as a dictionary to `artifactData`:',
+        $type_const);
+
+      $spec = $type->getArtifactParameterSpecification();
+      $desc = $type->getArtifactParameterDescriptions();
+      $out[] = "| {$head_key} | {$head_type} | {$head_desc} |";
+      $out[] = '|-------------|--------------|--------------|';
+      foreach ($spec as $key => $key_type) {
+        $key_desc = idx($desc, $key);
+        $out[] = "| `{$key}` | //{$key_type}// | {$key_desc} |";
+      }
+
+      $example = $type->getArtifactDataExample();
+      if ($example !== null) {
+        $json = new PhutilJSON();
+        $rendered = $json->encodeFormatted($example);
+
+        $out[] = pht('For example:');
+        $out[] = '```lang=json';
+        $out[] = $rendered;
+        $out[] = '```';
+      }
+    }
+
+    return implode("\n", $out);
+  }
+
+  protected function defineParamTypes() {
+    return array(
+      'buildTargetPHID' => 'phid',
+      'artifactKey' => 'string',
+      'artifactType' => 'string',
+      'artifactData' => 'map<string, wild>',
+    );
+  }
+
+  protected function defineReturnType() {
+    return 'wild';
+  }
+
+  protected function execute(ConduitAPIRequest $request) {
+    $viewer = $request->getUser();
+
+    $build_target_phid = $request->getValue('buildTargetPHID');
+    $build_target = id(new HarbormasterBuildTargetQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($build_target_phid))
+      ->executeOne();
+    if (!$build_target) {
+      throw new Exception(
+        pht(
+          'No such build target "%s"!',
+          $build_target_phid));
+    }
+
+    $artifact = $build_target->createArtifact(
+      $viewer,
+      $request->getValue('artifactKey'),
+      $request->getValue('artifactType'),
+      $request->getValue('artifactData'));
+
+    return array(
+      'data' => $this->returnArtifactList(array($artifact)),
+    );
+  }
+
+}

src/applications/harbormaster/controller/HarbormasterBuildViewController.php

@@ -3,17 +3,11 @@
 final class HarbormasterBuildViewController
   extends HarbormasterController {
 
-  private $id;
+  public function handleRequest(AphrontRequest $request) {
-
-  public function willProcessRequest(array $data) {
-    $this->id = $data['id'];
-  }
-
-  public function processRequest() {
     $request = $this->getRequest();
     $viewer = $request->getUser();
 
-    $id = $this->id;
+    $id = $request->getURIData('id');
     $generation = $request->getInt('g');
 
     $build = id(new HarbormasterBuildQuery())
@@ -224,29 +218,51 @@ final class HarbormasterBuildViewController
       ));
   }
 
-  private function buildArtifacts(
+  private function buildArtifacts(HarbormasterBuildTarget $build_target) {
-    HarbormasterBuildTarget $build_target) {
+    $viewer = $this->getViewer();
-
-    $request = $this->getRequest();
-    $viewer = $request->getUser();
 
     $artifacts = id(new HarbormasterBuildArtifactQuery())
       ->setViewer($viewer)
       ->withBuildTargetPHIDs(array($build_target->getPHID()))
       ->execute();
 
-    $list = id(new PHUIObjectItemListView())
+    $artifacts = msort($artifacts, 'getArtifactKey');
-      ->setNoDataString(pht('This target has no associated artifacts.'))
-      ->setFlush(true);
 
+    $rows = array();
     foreach ($artifacts as $artifact) {
-      $item = $artifact->getObjectItemView($viewer);
+      $impl = $artifact->getArtifactImplementation();
-      if ($item !== null) {
+
-        $list->addItem($item);
+      if ($impl) {
-      }
+        $summary = $impl->renderArtifactSummary($viewer);
+        $type_name = $impl->getArtifactTypeName();
+      } else {
+        $summary = pht('<Unknown Artifact Type>');
+        $type_name = $artifact->getType();
       }
 
-    return $list;
+      $rows[] = array(
+        $artifact->getArtifactKey(),
+        $type_name,
+        $summary,
+      );
+    }
+
+    $table = id(new AphrontTableView($rows))
+      ->setNoDataString(pht('This target has no associated artifacts.'))
+      ->setHeaders(
+        array(
+          pht('Key'),
+          pht('Type'),
+          pht('Summary'),
+        ))
+      ->setColumnClasses(
+        array(
+          'pri',
+          '',
+          'wide',
+        ));
+
+    return $table;
   }
 
   private function buildLog(

src/applications/harbormaster/engine/HarbormasterBuildEngine.php

@@ -483,7 +483,7 @@ final class HarbormasterBuildEngine extends Phobject {
       ->execute();
 
     foreach ($artifacts as $artifact) {
-      $artifact->release();
+      $artifact->releaseArtifact();
     }
 
   }

src/applications/harbormaster/phid/HarbormasterBuildArtifactPHIDType.php

@@ -0,0 +1,35 @@
+<?php
+
+final class HarbormasterBuildArtifactPHIDType extends PhabricatorPHIDType {
+
+  const TYPECONST = 'HMBA';
+
+  public function getTypeName() {
+    return pht('Build Artifact');
+  }
+
+  public function newObject() {
+    return new HarbormasterBuildArtifact();
+  }
+
+  protected function buildQueryForObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new HarbormasterBuildArtifactQuery())
+      ->withPHIDs($phids);
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $artifact = $objects[$phid];
+      $artifact_id = $artifact->getID();
+      $handle->setName(pht('Build Artifact %d', $artifact_id));
+    }
+  }
+
+}

src/applications/harbormaster/query/HarbormasterBuildArtifactQuery.php

@@ -6,7 +6,7 @@ final class HarbormasterBuildArtifactQuery
   private $ids;
   private $buildTargetPHIDs;
   private $artifactTypes;
-  private $artifactKeys;
+  private $artifactIndexes;
   private $keyBuildPHID;
   private $keyBuildGeneration;
 
@@ -25,29 +25,17 @@ final class HarbormasterBuildArtifactQuery
     return $this;
   }
 
-  public function withArtifactKeys(
+  public function withArtifactIndexes(array $artifact_indexes) {
-    $build_phid,
+    $this->artifactIndexes = $artifact_indexes;
-    $build_gen,
-    array $artifact_keys) {
-    $this->keyBuildPHID = $build_phid;
-    $this->keyBuildGeneration = $build_gen;
-    $this->artifactKeys = $artifact_keys;
     return $this;
   }
 
+  public function newResultObject() {
+    return new HarbormasterBuildArtifact();
+  }
+
   protected function loadPage() {
-    $table = new HarbormasterBuildArtifact();
+    return $this->loadStandardPage($this->newResultObject());
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
   }
 
   protected function willFilterPage(array $page) {
@@ -75,46 +63,38 @@ final class HarbormasterBuildArtifactQuery
     return $page;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
-    $where = array();
+    $where = parent::buildWhereClauseParts($conn);
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->buildTargetPHIDs) {
+    if ($this->buildTargetPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'buildTargetPHID IN (%Ls)',
         $this->buildTargetPHIDs);
     }
 
-    if ($this->artifactTypes) {
+    if ($this->artifactTypes !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'artifactType in (%Ls)',
         $this->artifactTypes);
     }
 
-    if ($this->artifactKeys) {
+    if ($this->artifactIndexes !== null) {
-      $indexes = array();
-      foreach ($this->artifactKeys as $key) {
-        $indexes[] = PhabricatorHash::digestForIndex(
-          $this->keyBuildPHID.$this->keyBuildGeneration.$key);
-      }
-
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'artifactIndex IN (%Ls)',
-        $indexes);
+        $this->artifactIndexes);
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
+    return $where;
-
-    return $this->formatWhereClause($where);
   }
 
   public function getQueryApplicationClass() {

src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php

@@ -43,9 +43,9 @@ final class HarbormasterCommandBuildStepImplementation
     $settings = $this->getSettings();
     $variables = $build_target->getVariables();
 
-    $artifact = $build->loadArtifact($settings['hostartifact']);
+    $artifact = $build_target->loadArtifact($settings['hostartifact']);
-
+    $impl = $artifact->getArtifactImplementation();
-    $lease = $artifact->loadDrydockLease();
+    $lease = $impl->loadArtifactLease();
 
     $this->platform = $lease->getAttribute('platform');
 
@@ -122,7 +122,7 @@ final class HarbormasterCommandBuildStepImplementation
       array(
         'name' => pht('Run on Host'),
         'key' => $this->getSetting('hostartifact'),
-        'type'  => HarbormasterBuildArtifact::TYPE_HOST,
+        'type' => HarbormasterHostArtifact::ARTIFACTCONST,
       ),
     );
   }

src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php

@@ -36,14 +36,13 @@ final class HarbormasterLeaseHostBuildStepImplementation
     $lease->waitUntilActive();
 
     // Create the associated artifact.
-    $artifact = $build->createArtifact(
+    $artifact = $build_target->createArtifact(
-      $build_target,
+      PhabricatorUser::getOmnipotentUser(),
       $settings['name'],
-      HarbormasterBuildArtifact::TYPE_HOST);
+      HarbormasterHostArtifact::ARTIFACTCONST,
-    $artifact->setArtifactData(array(
+      array(
-      'drydock-lease' => $lease->getID(),
+        'drydockLeasePHID' => $lease->getPHID(),
       ));
-    $artifact->save();
   }
 
   public function getArtifactOutputs() {
@@ -51,7 +50,7 @@ final class HarbormasterLeaseHostBuildStepImplementation
       array(
         'name' => pht('Leased Host'),
         'key' => $this->getSetting('name'),
-        'type' => HarbormasterBuildArtifact::TYPE_HOST,
+        'type' => HarbormasterHostArtifact::ARTIFACTCONST,
       ),
     );
   }

src/applications/harbormaster/step/HarbormasterPublishFragmentBuildStepImplementation.php

@@ -29,33 +29,34 @@ final class HarbormasterPublishFragmentBuildStepImplementation
 
     $settings = $this->getSettings();
     $variables = $build_target->getVariables();
+    $viewer = PhabricatorUser::getOmnipotentUser();
 
     $path = $this->mergeVariables(
       'vsprintf',
       $settings['path'],
       $variables);
 
-    $artifact = $build->loadArtifact($settings['artifact']);
+    $artifact = $build_target->loadArtifact($settings['artifact']);
-
+    $impl = $artifact->getArtifactImplementation();
-    $file = $artifact->loadPhabricatorFile();
+    $file = $impl->loadArtifactFile($viewer);
 
     $fragment = id(new PhragmentFragmentQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->setViewer($viewer)
       ->withPaths(array($path))
       ->executeOne();
 
     if ($fragment === null) {
       PhragmentFragment::createFromFile(
-        PhabricatorUser::getOmnipotentUser(),
+        $viewer,
         $file,
         $path,
         PhabricatorPolicies::getMostOpenPolicy(),
         PhabricatorPolicies::POLICY_USER);
     } else {
       if ($file->getMimeType() === 'application/zip') {
-        $fragment->updateFromZIP(PhabricatorUser::getOmnipotentUser(), $file);
+        $fragment->updateFromZIP($viewer, $file);
       } else {
-        $fragment->updateFromFile(PhabricatorUser::getOmnipotentUser(), $file);
+        $fragment->updateFromFile($viewer, $file);
       }
     }
   }
@@ -65,7 +66,7 @@ final class HarbormasterPublishFragmentBuildStepImplementation
       array(
         'name' => pht('Publishes File'),
         'key' => $this->getSetting('artifact'),
-        'type' => HarbormasterBuildArtifact::TYPE_FILE,
+        'type' => HarbormasterFileArtifact::ARTIFACTCONST,
       ),
     );
   }

src/applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php

@@ -34,8 +34,7 @@ final class HarbormasterUploadArtifactBuildStepImplementation
       $settings['path'],
       $variables);
 
-    $artifact = $build->loadArtifact($settings['hostartifact']);
+    $artifact = $build_target->loadArtifact($settings['hostartifact']);
-
     $lease = $artifact->loadDrydockLease();
 
     $interface = $lease->getInterface('filesystem');
@@ -44,14 +43,13 @@ final class HarbormasterUploadArtifactBuildStepImplementation
     $file = $interface->saveFile($path, $settings['name']);
 
     // Insert the artifact record.
-    $artifact = $build->createArtifact(
+    $artifact = $build_target->createArtifact(
-      $build_target,
+      PhabricatorUser::getOmnipotentUser(),
       $settings['name'],
-      HarbormasterBuildArtifact::TYPE_FILE);
+      HarbormasterFileArtifact::ARTIFACTCONST,
-    $artifact->setArtifactData(array(
+      array(
         'filePHID' => $file->getPHID(),
       ));
-    $artifact->save();
   }
 
   public function getArtifactInputs() {
@@ -59,7 +57,7 @@ final class HarbormasterUploadArtifactBuildStepImplementation
       array(
         'name' => pht('Upload From Host'),
         'key' => $this->getSetting('hostartifact'),
-        'type' => HarbormasterBuildArtifact::TYPE_HOST,
+        'type' => HarbormasterHostArtifact::ARTIFACTCONST,
       ),
     );
   }
@@ -69,7 +67,7 @@ final class HarbormasterUploadArtifactBuildStepImplementation
       array(
         'name' => pht('Uploaded File'),
         'key' => $this->getSetting('name'),
-        'type' => HarbormasterBuildArtifact::TYPE_FILE,
+        'type' => HarbormasterHostArtifact::ARTIFACTCONST,
       ),
     );
   }

src/applications/harbormaster/storage/build/HarbormasterBuild.php

@@ -235,36 +235,6 @@ final class HarbormasterBuild extends HarbormasterDAO
     return $log;
   }
 
-  public function createArtifact(
-    HarbormasterBuildTarget $build_target,
-    $artifact_key,
-    $artifact_type) {
-
-    $artifact =
-      HarbormasterBuildArtifact::initializeNewBuildArtifact($build_target);
-    $artifact->setArtifactKey(
-      $this->getPHID(),
-      $this->getBuildGeneration(),
-      $artifact_key);
-    $artifact->setArtifactType($artifact_type);
-    $artifact->save();
-    return $artifact;
-  }
-
-  public function loadArtifact($name) {
-    $artifact = id(new HarbormasterBuildArtifactQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withArtifactKeys(
-        $this->getPHID(),
-        $this->getBuildGeneration(),
-        array($name))
-      ->executeOne();
-    if ($artifact === null) {
-      throw new Exception(pht('Artifact not found!'));
-    }
-    return $artifact;
-  }
-
   public function retrieveVariablesFromBuild() {
     $results = array(
       'buildable.diff' => null,

src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php

@@ -10,19 +10,18 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO
   protected $artifactData = array();
 
   private $buildTarget = self::ATTACHABLE;
-
+  private $artifactImplementation;
-  const TYPE_FILE = 'file';
-  const TYPE_HOST = 'host';
-  const TYPE_URI = 'uri';
 
   public static function initializeNewBuildArtifact(
     HarbormasterBuildTarget $build_target) {
     return id(new HarbormasterBuildArtifact())
+      ->attachBuildTarget($build_target)
       ->setBuildTargetPHID($build_target->getPHID());
   }
 
   protected function getConfiguration() {
     return array(
+      self::CONFIG_AUX_PHID => true,
       self::CONFIG_SERIALIZATION => array(
         'artifactData' => self::SERIALIZATION_JSON,
       ),
@@ -43,6 +42,11 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO
     ) + parent::getConfiguration();
   }
 
+  public function generatePHID() {
+    return PhabricatorPHID::generateNewPHID(
+      HarbormasterBuildArtifactPHIDType::TYPECONST);
+  }
+
   public function attachBuildTarget(HarbormasterBuildTarget $build_target) {
     $this->buildTarget = $build_target;
     return $this;
@@ -52,113 +56,58 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO
     return $this->assertAttached($this->buildTarget);
   }
 
-  public function setArtifactKey($build_phid, $build_gen, $key) {
+  public function setArtifactKey($key) {
-    $this->artifactIndex =
+    $target = $this->getBuildTarget();
-      PhabricatorHash::digestForIndex($build_phid.$build_gen.$key);
+    $this->artifactIndex = self::getArtifactIndex($target, $key);
     $this->artifactKey = $key;
     return $this;
   }
 
-  public function getObjectItemView(PhabricatorUser $viewer) {
+  public static function getArtifactIndex(
-    $data = $this->getArtifactData();
+    HarbormasterBuildTarget $target,
-    switch ($this->getArtifactType()) {
+    $artifact_key) {
-      case self::TYPE_FILE:
-        $handle = id(new PhabricatorHandleQuery())
-          ->setViewer($viewer)
-          ->withPHIDs($data)
-          ->executeOne();
 
-        return id(new PHUIObjectItemView())
+    $build = $target->getBuild();
-          ->setObjectName(pht('File'))
+
-          ->setHeader($handle->getFullName())
+    $parts = array(
-          ->setHref($handle->getURI());
+      $build->getPHID(),
-      case self::TYPE_HOST:
+      $target->getBuildGeneration(),
-        $leases = id(new DrydockLeaseQuery())
+      $artifact_key,
-          ->setViewer($viewer)
+    );
-          ->withIDs(array($data['drydock-lease']))
+    $parts = implode("\0", $parts);
-          ->execute();
+
-        $lease = idx($leases, $data['drydock-lease']);
+    return PhabricatorHash::digestForIndex($parts);
+  }
+
+  public function releaseArtifact() {
+    $impl = $this->getArtifactImplementation();
+
+    if ($impl) {
+      $impl->releaseArtifact(PhabricatorUser::getOmnipotentUser());
+    }
 
-        return id(new PHUIObjectItemView())
-          ->setObjectName(pht('Drydock Lease'))
-          ->setHeader($lease->getID())
-          ->setHref('/drydock/lease/'.$lease->getID());
-      case self::TYPE_URI:
-        return id(new PHUIObjectItemView())
-          ->setObjectName($data['name'])
-          ->setHeader($data['uri'])
-          ->setHref($data['uri']);
-      default:
     return null;
   }
+
+  public function getArtifactImplementation() {
+    if ($this->artifactImplementation === null) {
+      $type = $this->getArtifactType();
+      $impl = HarbormasterArtifact::getArtifactType($type);
+      if (!$impl) {
+        return null;
       }
 
-  public function loadDrydockLease() {
+      $impl = clone $impl;
-    if ($this->getArtifactType() !== self::TYPE_HOST) {
+      $impl->setBuildArtifact($this);
-      throw new Exception(
+      $this->artifactImplementation = $impl;
-        pht(
-          '`%s` may only be called on host artifacts.',
-          __FUNCTION__));
     }
 
-    $data = $this->getArtifactData();
+    return $this->artifactImplementation;
-
-    // FIXME: Is there a better way of doing this?
-    // TODO: Policy stuff, etc.
-    $lease = id(new DrydockLease())->load(
-      $data['drydock-lease']);
-    if ($lease === null) {
-      throw new Exception(pht('Associated Drydock lease not found!'));
-    }
-    $resource = id(new DrydockResource())->load(
-      $lease->getResourceID());
-    if ($resource === null) {
-      throw new Exception(pht('Associated Drydock resource not found!'));
-    }
-    $lease->attachResource($resource);
-
-    return $lease;
   }
 
-  public function loadPhabricatorFile() {
-    if ($this->getArtifactType() !== self::TYPE_FILE) {
-      throw new Exception(
-        pht(
-          '`%s` may only be called on file artifacts.',
-          __FUNCTION__));
-    }
 
-    $data = $this->getArtifactData();
+  public function getProperty($key, $default = null) {
-
+    return idx($this->artifactData, $key, $default);
-    // The data for TYPE_FILE is an array with a single PHID in it.
-    $phid = $data['filePHID'];
-
-    $file = id(new PhabricatorFileQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withPHIDs(array($phid))
-      ->executeOne();
-    if ($file === null) {
-      throw new Exception(pht('Associated file not found!'));
-    }
-    return $file;
-  }
-
-  public function release() {
-    switch ($this->getArtifactType()) {
-      case self::TYPE_HOST:
-        $this->releaseDrydockLease();
-        break;
-    }
-  }
-
-  public function releaseDrydockLease() {
-    $lease = $this->loadDrydockLease();
-    $resource = $lease->getResource();
-    $blueprint = $resource->getBlueprint();
-
-    if ($lease->isActive()) {
-      $blueprint->releaseLease($resource, $lease);
-    }
   }
 
 

src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php

@@ -201,6 +201,54 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
     );
   }
 
+  public function createArtifact(
+    PhabricatorUser $actor,
+    $artifact_key,
+    $artifact_type,
+    array $artifact_data) {
+
+    $impl = HarbormasterArtifact::getArtifactType($artifact_type);
+    if (!$impl) {
+      throw new Exception(
+        pht(
+          'There is no implementation available for artifacts of type "%s".',
+          $artifact_type));
+    }
+
+    $impl->validateArtifactData($artifact_data);
+
+    $artifact = HarbormasterBuildArtifact::initializeNewBuildArtifact($this)
+      ->setArtifactKey($artifact_key)
+      ->setArtifactType($artifact_type)
+      ->setArtifactData($artifact_data);
+
+    $impl = $artifact->getArtifactImplementation();
+    $impl->willCreateArtifact($actor);
+
+    return $artifact->save();
+  }
+
+  public function loadArtifact($artifact_key) {
+    $indexes = array();
+
+    $indexes[] = HarbormasterBuildArtifact::getArtifactIndex(
+      $this,
+      $artifact_key);
+
+    $artifact = id(new HarbormasterBuildArtifactQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withArtifactIndexes($indexes)
+      ->executeOne();
+    if ($artifact === null) {
+      throw new Exception(
+        pht(
+          'Artifact "%s" not found!',
+          $artifact_key));
+    }
+
+    return $artifact;
+  }
+
 
 /* -(  Status  )------------------------------------------------------------- */