diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3716d1ba9e..87463605d9 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -687,6 +687,9 @@ phutil_register_library_map(array( 'HarbormasterPlanViewController' => 'applications/harbormaster/controller/HarbormasterPlanViewController.php', 'HarbormasterRemarkupRule' => 'applications/harbormaster/remarkup/HarbormasterRemarkupRule.php', 'HarbormasterScratchTable' => 'applications/harbormaster/storage/HarbormasterScratchTable.php', + 'HarbormasterStepAddController' => 'applications/harbormaster/controller/HarbormasterStepAddController.php', + 'HarbormasterStepDeleteController' => 'applications/harbormaster/controller/HarbormasterStepDeleteController.php', + 'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php', 'HeraldAction' => 'applications/herald/storage/HeraldAction.php', 'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php', 'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php', @@ -2919,6 +2922,9 @@ phutil_register_library_map(array( 'HarbormasterPlanViewController' => 'HarbormasterPlanController', 'HarbormasterRemarkupRule' => 'PhabricatorRemarkupRuleObject', 'HarbormasterScratchTable' => 'HarbormasterDAO', + 'HarbormasterStepAddController' => 'HarbormasterController', + 'HarbormasterStepDeleteController' => 'HarbormasterController', + 'HarbormasterStepEditController' => 'HarbormasterController', 'HeraldAction' => 'HeraldDAO', 'HeraldApplyTranscript' => 'HeraldDAO', 'HeraldCapabilityManageGlobalRules' => 'PhabricatorPolicyCapability', diff --git a/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php b/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php index 6bcb87d7b1..bf0d7c21ac 100644 --- a/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php +++ b/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php @@ -46,6 +46,11 @@ final class PhabricatorApplicationHarbormaster extends PhabricatorApplication { 'edit/(?:(?P\d+)/)?' => 'HarbormasterBuildableEditController', 'apply/(?:(?P\d+)/)?' => 'HarbormasterBuildableApplyController', ), + 'step/' => array( + 'add/(?:(?P\d+)/)?' => 'HarbormasterStepAddController', + 'edit/(?:(?P\d+)/)?' => 'HarbormasterStepEditController', + 'delete/(?:(?P\d+)/)?' => 'HarbormasterStepDeleteController', + ), 'plan/' => array( '(?:query/(?P[^/]+)/)?' => 'HarbormasterPlanListController', diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php index afe43b1423..a5c9c0b34d 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -62,6 +62,10 @@ final class HarbormasterBuildableViewController $item->setBarColor('red'); $item->addAttribute(pht('Failed')); break; + case HarbormasterBuild::STATUS_ERROR: + $item->setBarColor('red'); + $item->addAttribute(pht('Unexpected Error')); + break; } $build_list->addItem($item); } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php index 58eefbe4ab..9cc6944202 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -55,10 +55,13 @@ final class HarbormasterPlanViewController id(new PhabricatorCrumbView()) ->setName(pht("Plan %d", $id))); + $step_list = $this->buildStepList($plan); + return $this->buildApplicationPage( array( $crumbs, $box, + $step_list, $xaction_view, ), array( @@ -67,6 +70,56 @@ final class HarbormasterPlanViewController )); } + private function buildStepList(HarbormasterBuildPlan $plan) { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $steps = id(new HarbormasterBuildStepQuery()) + ->setViewer($viewer) + ->withBuildPlanPHIDs(array($plan->getPHID())) + ->execute(); + + $can_edit = $this->hasApplicationCapability( + HarbormasterCapabilityManagePlans::CAPABILITY); + + $i = 1; + $step_list = id(new PHUIObjectItemListView()) + ->setUser($viewer); + foreach ($steps as $step) { + $implementation = $step->getStepImplementation(); + $item = id(new PHUIObjectItemView()) + ->setObjectName("Step ".$i++) + ->setHeader($implementation->getName()); + + if (!$implementation->validateSettings()) { + $item + ->setBarColor('red') + ->addAttribute(pht('This step is not configured correctly.')); + } else { + $item->addAttribute($implementation->getDescription()); + } + + if ($can_edit) { + $edit_uri = $this->getApplicationURI("step/edit/".$step->getID()."/"); + $item + ->setHref($edit_uri) + ->addAction( + id(new PHUIListItemView()) + ->setIcon('delete') + ->addSigil('harbormaster-build-step-delete') + ->setWorkflow(true) + ->setRenderNameAsTooltip(true) + ->setName(pht("Delete")) + ->setHref( + $this->getApplicationURI("step/delete/".$step->getID()."/"))); + } + + $step_list->addItem($item); + } + + return $step_list; + } + private function buildActionList(HarbormasterBuildPlan $plan) { $request = $this->getRequest(); $viewer = $request->getUser(); @@ -88,6 +141,14 @@ final class HarbormasterPlanViewController ->setDisabled(!$can_edit) ->setIcon('edit')); + $list->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Add Build Step')) + ->setHref($this->getApplicationURI("step/add/{$id}/")) + ->setWorkflow($can_edit) + ->setDisabled(!$can_edit) + ->setIcon('new')); + return $list; } diff --git a/src/applications/harbormaster/controller/HarbormasterStepAddController.php b/src/applications/harbormaster/controller/HarbormasterStepAddController.php new file mode 100644 index 0000000000..095f6149ff --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterStepAddController.php @@ -0,0 +1,84 @@ +id = $data['id']; + } + + public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $this->requireApplicationCapability( + HarbormasterCapabilityManagePlans::CAPABILITY); + + $id = $this->id; + + $plan = id(new HarbormasterBuildPlanQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if ($plan === null) { + throw new Exception("Build plan not found!"); + } + + $implementations = BuildStepImplementation::getImplementations(); + + $cancel_uri = $this->getApplicationURI('plan/'.$plan->getID().'/'); + + if ($request->isDialogFormPost()) { + $class = $request->getStr('step-type'); + if (!in_array($class, $implementations)) { + return $this->createDialog($implementations); + } + + $step = new HarbormasterBuildStep(); + $step->setBuildPlanPHID($plan->getPHID()); + $step->setClassName($class); + $step->setDetails(array()); + $step->save(); + + $edit_uri = $this->getApplicationURI("step/edit/".$step->getID()."/"); + + return id(new AphrontRedirectResponse())->setURI($edit_uri); + } + + return $this->createDialog($implementations, $cancel_uri); + } + + function createDialog(array $implementations, $cancel_uri) { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $control = id(new AphrontFormRadioButtonControl()) + ->setName('step-type'); + + foreach ($implementations as $implementation_name) { + $implementation = new $implementation_name(); + $control + ->addButton( + $implementation_name, + $implementation->getName(), + $implementation->getGenericDescription()); + } + + $dialog = new AphrontDialogView(); + $dialog->setTitle(pht('Add New Step')) + ->setUser($viewer) + ->addSubmitButton(pht('Add Build Step')) + ->addCancelButton($cancel_uri); + $dialog->appendChild( + phutil_tag( + 'p', + array(), + pht( + 'Select what type of build step you want to add: '))); + $dialog->appendChild($control); + return id(new AphrontDialogResponse())->setDialog($dialog); + } + +} diff --git a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php new file mode 100644 index 0000000000..a11d565ba9 --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php @@ -0,0 +1,52 @@ +id = $data['id']; + } + + public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $this->requireApplicationCapability( + HarbormasterCapabilityManagePlans::CAPABILITY); + + $id = $this->id; + + $step = id(new HarbormasterBuildStepQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if ($step === null) { + throw new Exception("Build step not found!"); + } + + $plan_id = $step->getBuildPlan()->getID(); + $done_uri = $this->getApplicationURI('plan/'.$plan_id.'/'); + + if ($request->isDialogFormPost()) { + $step->delete(); + return id(new AphrontRedirectResponse())->setURI($done_uri); + } + + $dialog = new AphrontDialogView(); + $dialog->setTitle(pht('Really Delete Step?')) + ->setUser($viewer) + ->addSubmitButton(pht('Delete Build Step')) + ->addCancelButton($done_uri); + $dialog->appendChild( + phutil_tag( + 'p', + array(), + pht( + 'Are you sure you want to delete this '. + 'step? This can\'t be undone!'))); + return id(new AphrontDialogResponse())->setDialog($dialog); + } + +} diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php new file mode 100644 index 0000000000..81d3f0b2a8 --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -0,0 +1,156 @@ +id = idx($data, 'id'); + } + + public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $this->requireApplicationCapability( + HarbormasterCapabilityManagePlans::CAPABILITY); + + $step = id(new HarbormasterBuildStepQuery()) + ->setViewer($viewer) + ->withIDs(array($this->id)) + ->executeOne(); + if (!$step) { + return new Aphront404Response(); + } + + $plan = id(new HarbormasterBuildPlanQuery()) + ->setViewer($viewer) + ->withPHIDs(array($step->getBuildPlanPHID())) + ->executeOne(); + if (!$plan) { + return new Aphront404Response(); + } + + $implementation = $step->getStepImplementation(); + $implementation->validateSettingDefinitions(); + $settings = $implementation->getSettings(); + + $errors = array(); + if ($request->isFormPost()) { + foreach ($implementation->getSettingDefinitions() as $name => $opt) { + $readable_name = $this->getReadableName($name, $opt); + $value = $this->getValueFromRequest($request, $name, $opt['type']); + + // TODO: This won't catch any validation issues unless the field + // is missing completely. How should we check if the user is + // required to enter an integer? + if ($value === null) { + $errors[] = $readable_name.' is not valid.'; + } else { + $step->setDetail($name, $value); + } + } + + if (count($errors) === 0) { + $step->save(); + + return id(new AphrontRedirectResponse()) + ->setURI($this->getApplicationURI('plan/'.$plan->getID().'/')); + } + } + + $form = id(new AphrontFormView()) + ->setUser($viewer); + + // We need to render out all of the fields for the settings that + // the implementation has. + foreach ($implementation->getSettingDefinitions() as $name => $opt) { + if ($request->isFormPost()) { + $value = $this->getValueFromRequest($request, $name, $opt['type']); + } else { + $value = $settings[$name]; + } + + switch ($opt['type']) { + case BuildStepImplementation::SETTING_TYPE_STRING: + case BuildStepImplementation::SETTING_TYPE_INTEGER: + $control = id(new AphrontFormTextControl()) + ->setLabel($this->getReadableName($name, $opt)) + ->setName($name) + ->setValue($value); + break; + case BuildStepImplementation::SETTING_TYPE_BOOLEAN: + $control = id(new AphrontFormCheckboxControl()) + ->setLabel($this->getReadableName($name, $opt)) + ->setName($name) + ->setValue($value); + break; + default: + throw new Exception("Unable to render field with unknown type."); + } + + if (isset($opt['description'])) { + $control->setCaption($opt['description']); + } + + $form->appendChild($control); + } + + $form->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Save Step Configuration')) + ->addCancelButton( + $this->getApplicationURI('plan/'.$plan->getID().'/'))); + + $box = id(new PHUIObjectBoxView()) + ->setHeaderText('Edit Step: '.$implementation->getName()) + ->setValidationException(null) + ->setForm($form); + + $crumbs = $this->buildApplicationCrumbs(); + $id = $plan->getID(); + $crumbs->addCrumb( + id(new PhabricatorCrumbView()) + ->setName(pht("Plan %d", $id)) + ->setHref($this->getApplicationURI("plan/{$id}/"))); + $crumbs->addCrumb( + id(new PhabricatorCrumbView()) + ->setName(pht('Edit Step'))); + + return $this->buildApplicationPage( + array( + $crumbs, + $box, + ), + array( + 'title' => $implementation->getName(), + 'device' => true, + )); + } + + public function getReadableName($name, $opt) { + $readable_name = $name; + if (isset($opt['name'])) { + $readable_name = $opt['name']; + } + return $readable_name; + } + + public function getValueFromRequest(AphrontRequest $request, $name, $type) { + switch ($type) { + case BuildStepImplementation::SETTING_TYPE_STRING: + return $request->getStr($name); + break; + case BuildStepImplementation::SETTING_TYPE_INTEGER: + return $request->getInt($name); + break; + case BuildStepImplementation::SETTING_TYPE_BOOLEAN: + return $request->getBool($name); + break; + default: + throw new Exception("Unsupported setting type '".$type."'."); + } + } + +} diff --git a/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php b/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php index 61190b3285..3bcc057f16 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php @@ -22,6 +22,14 @@ final class HarbormasterBuildStepQuery return $this; } + public function getPagingColumn() { + return 'id'; + } + + public function getReversePaging() { + return true; + } + protected function loadPage() { $table = new HarbormasterBuildStep(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/harbormaster/step/BuildStepImplementation.php b/src/applications/harbormaster/step/BuildStepImplementation.php index b782496a6f..6a524f4e39 100644 --- a/src/applications/harbormaster/step/BuildStepImplementation.php +++ b/src/applications/harbormaster/step/BuildStepImplementation.php @@ -4,13 +4,32 @@ abstract class BuildStepImplementation { private $settings; + const SETTING_TYPE_STRING = 'string'; + const SETTING_TYPE_INTEGER = 'integer'; + const SETTING_TYPE_BOOLEAN = 'boolean'; + + public static function getImplementations() { + $symbols = id(new PhutilSymbolLoader()) + ->setAncestorClass("BuildStepImplementation") + ->setConcreteOnly(true) + ->selectAndLoadSymbols(); + return ipull($symbols, 'name'); + } + /** * The name of the implementation. */ abstract public function getName(); /** - * The description of the implementation. + * The generic description of the implementation. + */ + public function getGenericDescription() { + return ''; + } + + /** + * The description of the implementation, based on the current settings. */ public function getDescription() { return ''; @@ -24,21 +43,42 @@ abstract class BuildStepImplementation { /** * Gets the settings for this build step. */ - protected function getSettings() { + public function getSettings() { return $this->settings; } + /** + * Validate the current settings of this build step. + */ + public function validate() { + return true; + } + /** * Loads the settings for this build step implementation from the build step. */ public final function loadSettings(HarbormasterBuildStep $build_step) { $this->settings = array(); + $this->validateSettingDefinitions(); foreach ($this->getSettingDefinitions() as $name => $opt) { $this->settings[$name] = $build_step->getDetail($name); } return $this->settings; } + /** + * Validates that the setting definitions for this implementation are valid. + */ + public final function validateSettingDefinitions() { + foreach ($this->getSettingDefinitions() as $name => $opt) { + if (!isset($opt['type'])) { + throw new Exception( + 'Setting definition \''.$name. + '\' is missing type requirement.'); + } + } + } + /** * Return an array of settings for this step implementation. */ diff --git a/src/applications/harbormaster/step/SleepBuildStepImplementation.php b/src/applications/harbormaster/step/SleepBuildStepImplementation.php index bf16b22d23..e80f559e29 100644 --- a/src/applications/harbormaster/step/SleepBuildStepImplementation.php +++ b/src/applications/harbormaster/step/SleepBuildStepImplementation.php @@ -6,19 +6,40 @@ final class SleepBuildStepImplementation extends BuildStepImplementation { return pht('Sleep'); } - public function getDescription() { + public function getGenericDescription() { return pht('Sleep for a specified number of seconds.'); } + public function getDescription() { + $settings = $this->getSettings(); + + return pht('Sleep for %s seconds.', $settings['seconds']); + } + public function execute(HarbormasterBuild $build) { $settings = $this->getSettings(); sleep($settings['seconds']); } + public function validateSettings() { + $settings = $this->getSettings(); + + if ($settings['seconds'] === null) { + return false; + } + if (!is_int($settings['seconds'])) { + return false; + } + return true; + } + public function getSettingDefinitions() { return array( - 'seconds' => array()); + 'seconds' => array( + 'name' => 'Seconds', + 'description' => 'The number of seconds to sleep for.', + 'type' => BuildStepImplementation::SETTING_TYPE_INTEGER)); } } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index c7d5aa7181..aa8e6c4f53 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -40,6 +40,11 @@ final class HarbormasterBuild extends HarbormasterDAO */ const STATUS_FAILED = 'failed'; + /** + * The build encountered an unexpected error. + */ + const STATUS_ERROR = 'error'; + public static function initializeNewBuild(PhabricatorUser $actor) { return id(new HarbormasterBuild()) ->setBuildStatus(self::STATUS_INACTIVE); diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php index 42a7e89fa9..cb9310d2aa 100644 --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php @@ -46,9 +46,16 @@ final class HarbormasterBuildStep extends HarbormasterDAO throw new Exception("No implementation set for the given step."); } - // TODO: We should look up the class in phutil's system to ensure - // that it actually extends BuildStepImplementation. + static $implementations = null; + if ($implementations === null) { + $implementations = BuildStepImplementation::getImplementations(); + } + $class = $this->className; + if (!in_array($class, $implementations)) { + throw new Exception( + "Class name '".$class."' does not extend BuildStepImplementation."); + } $implementation = newv($class, array()); $implementation->loadSettings($this); return $implementation; diff --git a/src/applications/harbormaster/worker/HarbormasterBuildWorker.php b/src/applications/harbormaster/worker/HarbormasterBuildWorker.php index 347119e348..0306c18d8f 100644 --- a/src/applications/harbormaster/worker/HarbormasterBuildWorker.php +++ b/src/applications/harbormaster/worker/HarbormasterBuildWorker.php @@ -40,6 +40,10 @@ final class HarbormasterBuildWorker extends PhabricatorWorker { // Perform the build. foreach ($steps as $step) { $implementation = $step->getStepImplementation(); + if (!$implementation->validateSettings()) { + $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR); + break; + } $implementation->execute($build); if ($build->getBuildStatus() !== HarbormasterBuild::STATUS_BUILDING) { break; @@ -56,7 +60,7 @@ final class HarbormasterBuildWorker extends PhabricatorWorker { // If any exception is raised, the build is marked as a failure and // the exception is re-thrown (this ensures we don't leave builds // in an inconsistent state). - $build->setBuildStatus(HarbormasterBuild::STATUS_FAILED); + $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR); $build->save(); throw $e; }