From 57b0353034c582e1fc44390b832abde2dc30af67 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 15 Aug 2015 07:28:56 -0700 Subject: [PATCH] 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 --- .../20150814.harbormater.artifact.phid.sql | 5 + src/__phutil_library_map__.php | 12 ++ .../drydock/query/DrydockLeaseQuery.php | 41 +++--- .../artifact/HarbormasterArtifact.php | 88 ++++++++++++ .../artifact/HarbormasterFileArtifact.php | 66 +++++++++ .../artifact/HarbormasterHostArtifact.php | 74 ++++++++++ .../artifact/HarbormasterURIArtifact.php | 100 +++++++++++++ .../conduit/HarbormasterConduitAPIMethod.php | 12 ++ ...ormasterCreateArtifactConduitAPIMethod.php | 129 +++++++++++++++++ .../HarbormasterBuildViewController.php | 56 +++++--- .../engine/HarbormasterBuildEngine.php | 2 +- .../HarbormasterBuildArtifactPHIDType.php | 35 +++++ .../query/HarbormasterBuildArtifactQuery.php | 60 +++----- ...ormasterCommandBuildStepImplementation.php | 12 +- ...masterLeaseHostBuildStepImplementation.php | 15 +- ...PublishFragmentBuildStepImplementation.php | 17 +-- ...rUploadArtifactBuildStepImplementation.php | 20 ++- .../storage/build/HarbormasterBuild.php | 30 ---- .../build/HarbormasterBuildArtifact.php | 135 ++++++------------ .../storage/build/HarbormasterBuildTarget.php | 48 +++++++ 20 files changed, 715 insertions(+), 242 deletions(-) create mode 100644 resources/sql/autopatches/20150814.harbormater.artifact.phid.sql create mode 100644 src/applications/harbormaster/artifact/HarbormasterArtifact.php create mode 100644 src/applications/harbormaster/artifact/HarbormasterFileArtifact.php create mode 100644 src/applications/harbormaster/artifact/HarbormasterHostArtifact.php create mode 100644 src/applications/harbormaster/artifact/HarbormasterURIArtifact.php create mode 100644 src/applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php create mode 100644 src/applications/harbormaster/phid/HarbormasterBuildArtifactPHIDType.php diff --git a/resources/sql/autopatches/20150814.harbormater.artifact.phid.sql b/resources/sql/autopatches/20150814.harbormater.artifact.phid.sql new file mode 100644 index 0000000000..1f03268ff0 --- /dev/null +++ b/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 = ''; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3bb3ad8c5b..4b6d8ac47e 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/drydock/query/DrydockLeaseQuery.php b/src/applications/drydock/query/DrydockLeaseQuery.php index d5d62f950e..23cad90e9c 100644 --- a/src/applications/drydock/query/DrydockLeaseQuery.php +++ b/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(); - $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); + return $this->loadStandardPage($this->newResultObject()); } protected function willFilterPage(array $leases) { @@ -69,40 +62,38 @@ final class DrydockLeaseQuery extends DrydockQuery { return $leases; } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $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 $this->formatWhereClause($where); + return $where; } } diff --git a/src/applications/harbormaster/artifact/HarbormasterArtifact.php b/src/applications/harbormaster/artifact/HarbormasterArtifact.php new file mode 100644 index 0000000000..a0d3a86101 --- /dev/null +++ b/src/applications/harbormaster/artifact/HarbormasterArtifact.php @@ -0,0 +1,88 @@ +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); + } + +} diff --git a/src/applications/harbormaster/artifact/HarbormasterFileArtifact.php b/src/applications/harbormaster/artifact/HarbormasterFileArtifact.php new file mode 100644 index 0000000000..91fc025c21 --- /dev/null +++ b/src/applications/harbormaster/artifact/HarbormasterFileArtifact.php @@ -0,0 +1,66 @@ + '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; + } + +} diff --git a/src/applications/harbormaster/artifact/HarbormasterHostArtifact.php b/src/applications/harbormaster/artifact/HarbormasterHostArtifact.php new file mode 100644 index 0000000000..73d00844af --- /dev/null +++ b/src/applications/harbormaster/artifact/HarbormasterHostArtifact.php @@ -0,0 +1,74 @@ + '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); + } + } + + +} diff --git a/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php b/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php new file mode 100644 index 0000000000..53d3241711 --- /dev/null +++ b/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php @@ -0,0 +1,100 @@ + '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(''); + } + + $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)))); + } + } + +} diff --git a/src/applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php index 33d239dc12..ccd76d2bbc 100644 --- a/src/applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php +++ b/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; + } + } diff --git a/src/applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php new file mode 100644 index 0000000000..56702bf99e --- /dev/null +++ b/src/applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php @@ -0,0 +1,129 @@ +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', + ); + } + + 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)), + ); + } + +} diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index cdc5baec60..189fa76ec2 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -3,17 +3,11 @@ final class HarbormasterBuildViewController extends HarbormasterController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = $data['id']; - } - - public function processRequest() { + public function handleRequest(AphrontRequest $request) { $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( - HarbormasterBuildTarget $build_target) { - - $request = $this->getRequest(); - $viewer = $request->getUser(); + private function buildArtifacts(HarbormasterBuildTarget $build_target) { + $viewer = $this->getViewer(); $artifacts = id(new HarbormasterBuildArtifactQuery()) ->setViewer($viewer) ->withBuildTargetPHIDs(array($build_target->getPHID())) ->execute(); - $list = id(new PHUIObjectItemListView()) - ->setNoDataString(pht('This target has no associated artifacts.')) - ->setFlush(true); + $artifacts = msort($artifacts, 'getArtifactKey'); + $rows = array(); foreach ($artifacts as $artifact) { - $item = $artifact->getObjectItemView($viewer); - if ($item !== null) { - $list->addItem($item); + $impl = $artifact->getArtifactImplementation(); + + if ($impl) { + $summary = $impl->renderArtifactSummary($viewer); + $type_name = $impl->getArtifactTypeName(); + } else { + $summary = pht(''); + $type_name = $artifact->getType(); } + + $rows[] = array( + $artifact->getArtifactKey(), + $type_name, + $summary, + ); } - return $list; + $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( diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php index fd895979cd..f2ed44bb0d 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -483,7 +483,7 @@ final class HarbormasterBuildEngine extends Phobject { ->execute(); foreach ($artifacts as $artifact) { - $artifact->release(); + $artifact->releaseArtifact(); } } diff --git a/src/applications/harbormaster/phid/HarbormasterBuildArtifactPHIDType.php b/src/applications/harbormaster/phid/HarbormasterBuildArtifactPHIDType.php new file mode 100644 index 0000000000..5c3eb992f7 --- /dev/null +++ b/src/applications/harbormaster/phid/HarbormasterBuildArtifactPHIDType.php @@ -0,0 +1,35 @@ +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)); + } + } + +} diff --git a/src/applications/harbormaster/query/HarbormasterBuildArtifactQuery.php b/src/applications/harbormaster/query/HarbormasterBuildArtifactQuery.php index 553780aa76..8f50aa1bb2 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildArtifactQuery.php +++ b/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( - $build_phid, - $build_gen, - array $artifact_keys) { - $this->keyBuildPHID = $build_phid; - $this->keyBuildGeneration = $build_gen; - $this->artifactKeys = $artifact_keys; + public function withArtifactIndexes(array $artifact_indexes) { + $this->artifactIndexes = $artifact_indexes; return $this; } + public function newResultObject() { + return new HarbormasterBuildArtifact(); + } + protected function loadPage() { - $table = new HarbormasterBuildArtifact(); - $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); + return $this->loadStandardPage($this->newResultObject()); } protected function willFilterPage(array $page) { @@ -75,46 +63,38 @@ final class HarbormasterBuildArtifactQuery return $page; } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $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) { - $indexes = array(); - foreach ($this->artifactKeys as $key) { - $indexes[] = PhabricatorHash::digestForIndex( - $this->keyBuildPHID.$this->keyBuildGeneration.$key); - } - + if ($this->artifactIndexes !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'artifactIndex IN (%Ls)', - $indexes); + $this->artifactIndexes); } - $where[] = $this->buildPagingClause($conn_r); - - return $this->formatWhereClause($where); + return $where; } public function getQueryApplicationClass() { diff --git a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php index 931ade75a7..2a6b633a72 100644 --- a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php +++ b/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']); - - $lease = $artifact->loadDrydockLease(); + $artifact = $build_target->loadArtifact($settings['hostartifact']); + $impl = $artifact->getArtifactImplementation(); + $lease = $impl->loadArtifactLease(); $this->platform = $lease->getAttribute('platform'); @@ -120,9 +120,9 @@ final class HarbormasterCommandBuildStepImplementation public function getArtifactInputs() { return array( array( - 'name' => pht('Run on Host'), - 'key' => $this->getSetting('hostartifact'), - 'type' => HarbormasterBuildArtifact::TYPE_HOST, + 'name' => pht('Run on Host'), + 'key' => $this->getSetting('hostartifact'), + 'type' => HarbormasterHostArtifact::ARTIFACTCONST, ), ); } diff --git a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php index d5f461506a..127025fd26 100644 --- a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php @@ -36,14 +36,13 @@ final class HarbormasterLeaseHostBuildStepImplementation $lease->waitUntilActive(); // Create the associated artifact. - $artifact = $build->createArtifact( - $build_target, + $artifact = $build_target->createArtifact( + PhabricatorUser::getOmnipotentUser(), $settings['name'], - HarbormasterBuildArtifact::TYPE_HOST); - $artifact->setArtifactData(array( - 'drydock-lease' => $lease->getID(), - )); - $artifact->save(); + HarbormasterHostArtifact::ARTIFACTCONST, + array( + 'drydockLeasePHID' => $lease->getPHID(), + )); } 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, ), ); } diff --git a/src/applications/harbormaster/step/HarbormasterPublishFragmentBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterPublishFragmentBuildStepImplementation.php index dc59717a23..fc0c830150 100644 --- a/src/applications/harbormaster/step/HarbormasterPublishFragmentBuildStepImplementation.php +++ b/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']); - - $file = $artifact->loadPhabricatorFile(); + $artifact = $build_target->loadArtifact($settings['artifact']); + $impl = $artifact->getArtifactImplementation(); + $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, ), ); } diff --git a/src/applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php index 75b48eb3c6..88651826e7 100644 --- a/src/applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php +++ b/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( - $build_target, + $artifact = $build_target->createArtifact( + PhabricatorUser::getOmnipotentUser(), $settings['name'], - HarbormasterBuildArtifact::TYPE_FILE); - $artifact->setArtifactData(array( - 'filePHID' => $file->getPHID(), - )); - $artifact->save(); + HarbormasterFileArtifact::ARTIFACTCONST, + array( + 'filePHID' => $file->getPHID(), + )); } 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, ), ); } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 5ea9f2f07b..e35744ecc9 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/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, diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php index f96ed6243c..0b195db88f 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php @@ -10,19 +10,18 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO protected $artifactData = array(); private $buildTarget = self::ATTACHABLE; - - const TYPE_FILE = 'file'; - const TYPE_HOST = 'host'; - const TYPE_URI = 'uri'; + private $artifactImplementation; 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) { - $this->artifactIndex = - PhabricatorHash::digestForIndex($build_phid.$build_gen.$key); + public function setArtifactKey($key) { + $target = $this->getBuildTarget(); + $this->artifactIndex = self::getArtifactIndex($target, $key); $this->artifactKey = $key; return $this; } - public function getObjectItemView(PhabricatorUser $viewer) { - $data = $this->getArtifactData(); - switch ($this->getArtifactType()) { - case self::TYPE_FILE: - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($viewer) - ->withPHIDs($data) - ->executeOne(); - - return id(new PHUIObjectItemView()) - ->setObjectName(pht('File')) - ->setHeader($handle->getFullName()) - ->setHref($handle->getURI()); - case self::TYPE_HOST: - $leases = id(new DrydockLeaseQuery()) - ->setViewer($viewer) - ->withIDs(array($data['drydock-lease'])) - ->execute(); - $lease = idx($leases, $data['drydock-lease']); - - 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 loadDrydockLease() { - if ($this->getArtifactType() !== self::TYPE_HOST) { - throw new Exception( - pht( - '`%s` may only be called on host artifacts.', - __FUNCTION__)); - } + public static function getArtifactIndex( + HarbormasterBuildTarget $target, + $artifact_key) { - $data = $this->getArtifactData(); + $build = $target->getBuild(); - // 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); + $parts = array( + $build->getPHID(), + $target->getBuildGeneration(), + $artifact_key, + ); + $parts = implode("\0", $parts); - return $lease; + return PhabricatorHash::digestForIndex($parts); } - public function loadPhabricatorFile() { - if ($this->getArtifactType() !== self::TYPE_FILE) { - throw new Exception( - pht( - '`%s` may only be called on file artifacts.', - __FUNCTION__)); + public function releaseArtifact() { + $impl = $this->getArtifactImplementation(); + + if ($impl) { + $impl->releaseArtifact(PhabricatorUser::getOmnipotentUser()); } - $data = $this->getArtifactData(); + return null; + } - // The data for TYPE_FILE is an array with a single PHID in it. - $phid = $data['filePHID']; + public function getArtifactImplementation() { + if ($this->artifactImplementation === null) { + $type = $this->getArtifactType(); + $impl = HarbormasterArtifact::getArtifactType($type); + if (!$impl) { + return null; + } - $file = id(new PhabricatorFileQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withPHIDs(array($phid)) - ->executeOne(); - if ($file === null) { - throw new Exception(pht('Associated file not found!')); + $impl = clone $impl; + $impl->setBuildArtifact($this); + $this->artifactImplementation = $impl; } - return $file; - } - public function release() { - switch ($this->getArtifactType()) { - case self::TYPE_HOST: - $this->releaseDrydockLease(); - break; - } + return $this->artifactImplementation; } - public function releaseDrydockLease() { - $lease = $this->loadDrydockLease(); - $resource = $lease->getResource(); - $blueprint = $resource->getBlueprint(); - if ($lease->isActive()) { - $blueprint->releaseLease($resource, $lease); - } + public function getProperty($key, $default = null) { + return idx($this->artifactData, $key, $default); } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php index b2e0a2aa83..7f08292787 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php +++ b/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 )------------------------------------------------------------- */ -- 2.51.2