diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1409,6 +1409,7 @@ 'HarbormasterBuildLogViewController' => 'applications/harbormaster/controller/HarbormasterBuildLogViewController.php', 'HarbormasterBuildMessage' => 'applications/harbormaster/storage/HarbormasterBuildMessage.php', 'HarbormasterBuildMessageQuery' => 'applications/harbormaster/query/HarbormasterBuildMessageQuery.php', + 'HarbormasterBuildMessageTransaction' => 'applications/harbormaster/xaction/build/HarbormasterBuildMessageTransaction.php', 'HarbormasterBuildPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPHIDType.php', 'HarbormasterBuildPlan' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php', 'HarbormasterBuildPlanBehavior' => 'applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php', @@ -1459,6 +1460,7 @@ 'HarbormasterBuildTransaction' => 'applications/harbormaster/storage/HarbormasterBuildTransaction.php', 'HarbormasterBuildTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php', 'HarbormasterBuildTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildTransactionQuery.php', + 'HarbormasterBuildTransactionType' => 'applications/harbormaster/xaction/build/HarbormasterBuildTransactionType.php', 'HarbormasterBuildUnitMessage' => 'applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php', 'HarbormasterBuildUnitMessageQuery' => 'applications/harbormaster/query/HarbormasterBuildUnitMessageQuery.php', 'HarbormasterBuildView' => 'applications/harbormaster/view/HarbormasterBuildView.php', @@ -7628,6 +7630,7 @@ 'PhabricatorDestructibleInterface', ), 'HarbormasterBuildMessageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'HarbormasterBuildMessageTransaction' => 'HarbormasterBuildTransactionType', 'HarbormasterBuildPHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildPlan' => array( 'HarbormasterDAO', @@ -7698,9 +7701,10 @@ 'HarbormasterBuildTargetPHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildTargetQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildTargetSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'HarbormasterBuildTransaction' => 'PhabricatorApplicationTransaction', + 'HarbormasterBuildTransaction' => 'PhabricatorModularTransaction', 'HarbormasterBuildTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'HarbormasterBuildTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'HarbormasterBuildTransactionType' => 'PhabricatorModularTransactionType', 'HarbormasterBuildUnitMessage' => array( 'HarbormasterDAO', 'PhabricatorPolicyInterface', diff --git a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php --- a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php @@ -11,92 +11,4 @@ return pht('Harbormaster Builds'); } - public function getTransactionTypes() { - $types = parent::getTransactionTypes(); - - $types[] = HarbormasterBuildTransaction::TYPE_COMMAND; - - return $types; - } - - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case HarbormasterBuildTransaction::TYPE_COMMAND: - return null; - } - - return parent::getCustomTransactionOldValue($object, $xaction); - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case HarbormasterBuildTransaction::TYPE_COMMAND: - return $xaction->getNewValue(); - } - - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case HarbormasterBuildTransaction::TYPE_COMMAND: - return $this->executeBuildCommand($object, $xaction); - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - private function executeBuildCommand( - HarbormasterBuild $build, - HarbormasterBuildTransaction $xaction) { - - $actor = $this->getActor(); - $message_type = $xaction->getNewValue(); - - // TODO: Restore logic that tests if the command can issue without causing - // anything to lapse into an invalid state. This should not be the same - // as the logic which powers the web UI: for example, if an "abort" is - // queued we want to disable "Abort" in the web UI, but should obviously - // process it here. - - switch ($message_type) { - case HarbormasterBuildCommand::COMMAND_ABORT: - // TODO: This should move to external effects, perhaps. - $build->releaseAllArtifacts($actor); - $build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED); - break; - case HarbormasterBuildCommand::COMMAND_RESTART: - $build->restartBuild($actor); - $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING); - break; - case HarbormasterBuildCommand::COMMAND_RESUME: - $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING); - break; - case HarbormasterBuildCommand::COMMAND_PAUSE: - $build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED); - break; - } - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case HarbormasterBuildTransaction::TYPE_COMMAND: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - } diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -124,13 +124,15 @@ $xactions = array(); + $message_xaction = HarbormasterBuildMessageTransaction::TRANSACTIONTYPE; + $messages = $build->getUnprocessedMessagesForApply(); foreach ($messages as $message) { $message_type = $message->getType(); $xactions[] = $build->getApplicationTransactionTemplate() ->setAuthorPHID($message->getAuthorPHID()) - ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND) + ->setTransactionType($message_xaction) ->setNewValue($message_type); } diff --git a/src/applications/harbormaster/management/HarbormasterManagementRestartWorkflow.php b/src/applications/harbormaster/management/HarbormasterManagementRestartWorkflow.php --- a/src/applications/harbormaster/management/HarbormasterManagementRestartWorkflow.php +++ b/src/applications/harbormaster/management/HarbormasterManagementRestartWorkflow.php @@ -32,10 +32,10 @@ if (!$ids && !$active) { throw new PhutilArgumentUsageException( - pht('Use --id or --active to select builds.')); + pht('Use "--id" or "--active" to select builds.')); } if ($ids && $active) { throw new PhutilArgumentUsageException( - pht('Use one of --id or --active to select builds, but not both.')); + pht('Use one of "--id" or "--active" to select builds, but not both.')); } $query = id(new HarbormasterBuildQuery()) @@ -48,50 +48,38 @@ } $builds = $query->execute(); - $console = PhutilConsole::getConsole(); $count = count($builds); if (!$count) { - $console->writeOut("%s\n", pht('No builds to restart.')); + $this->logSkip( + pht('SKIP'), + pht('No builds to restart.')); return 0; } + $prompt = pht('Restart %s build(s)?', new PhutilNumber($count)); if (!phutil_console_confirm($prompt)) { - $console->writeOut("%s\n", pht('Cancelled.')); - return 1; + throw new ArcanistUserAbortException(); } - $app_phid = id(new PhabricatorHarbormasterApplication())->getPHID(); - $editor = id(new HarbormasterBuildTransactionEditor()) - ->setActor($viewer) - ->setActingAsPHID($app_phid) - ->setContentSource($this->newContentSource()); foreach ($builds as $build) { - $console->writeOut( - " %s %s\n", + $this->logInfo( pht('RESTARTING'), pht('Build %d: %s', $build->getID(), $build->getName())); + if (!$build->canRestartBuild()) { - $console->writeOut( - " %s %s\n", + $this->logWarn( pht('INVALID'), - pht('Cannot be restarted.')); - continue; - } - $xactions = array(); - $xactions[] = id(new HarbormasterBuildTransaction()) - ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND) - ->setNewValue(HarbormasterBuildCommand::COMMAND_RESTART); - try { - $editor->applyTransactions($build, $xactions); - } catch (Exception $e) { - $message = phutil_console_wrap($e->getMessage(), 2); - $console->writeOut( - " %s \n%s\n", - pht('FAILED'), - $message); + pht('Build can not be restarted.')); continue; } - $console->writeOut(" %s \n", pht('SUCCESS')); + + $build->sendMessage( + $viewer, + HarbormasterBuildCommand::COMMAND_RESTART); + + $this->logOkay( + pht('QUEUED'), + pht('Sent a restart message to build.')); } return 0; diff --git a/src/applications/harbormaster/storage/HarbormasterBuildTransaction.php b/src/applications/harbormaster/storage/HarbormasterBuildTransaction.php --- a/src/applications/harbormaster/storage/HarbormasterBuildTransaction.php +++ b/src/applications/harbormaster/storage/HarbormasterBuildTransaction.php @@ -1,9 +1,7 @@ getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_COMMAND: - switch ($new) { - case HarbormasterBuildCommand::COMMAND_RESTART: - return pht( - '%s restarted this build.', - $this->renderHandleLink($author_phid)); - case HarbormasterBuildCommand::COMMAND_ABORT: - return pht( - '%s aborted this build.', - $this->renderHandleLink($author_phid)); - case HarbormasterBuildCommand::COMMAND_RESUME: - return pht( - '%s resumed this build.', - $this->renderHandleLink($author_phid)); - case HarbormasterBuildCommand::COMMAND_PAUSE: - return pht( - '%s paused this build.', - $this->renderHandleLink($author_phid)); - } - } - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'HarbormasterBuildTransactionType'; } - public function getIcon() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_COMMAND: - switch ($new) { - case HarbormasterBuildCommand::COMMAND_RESTART: - return 'fa-backward'; - case HarbormasterBuildCommand::COMMAND_RESUME: - return 'fa-play'; - case HarbormasterBuildCommand::COMMAND_PAUSE: - return 'fa-pause'; - case HarbormasterBuildCommand::COMMAND_ABORT: - return 'fa-exclamation-triangle'; - } - } - - return parent::getIcon(); - } - - public function getColor() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_COMMAND: - switch ($new) { - case HarbormasterBuildCommand::COMMAND_PAUSE: - case HarbormasterBuildCommand::COMMAND_ABORT: - return 'red'; - } - } - return parent::getColor(); - } } diff --git a/src/applications/harbormaster/xaction/build/HarbormasterBuildMessageTransaction.php b/src/applications/harbormaster/xaction/build/HarbormasterBuildMessageTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/xaction/build/HarbormasterBuildMessageTransaction.php @@ -0,0 +1,128 @@ +getNewValue(); + + switch ($new) { + case HarbormasterBuildCommand::COMMAND_RESTART: + return pht( + '%s restarted this build.', + $this->renderAuthor()); + case HarbormasterBuildCommand::COMMAND_ABORT: + return pht( + '%s aborted this build.', + $this->renderAuthor()); + case HarbormasterBuildCommand::COMMAND_RESUME: + return pht( + '%s resumed this build.', + $this->renderAuthor()); + case HarbormasterBuildCommand::COMMAND_PAUSE: + return pht( + '%s paused this build.', + $this->renderAuthor()); + } + + return pht( + '%s issued an unknown command ("%s") to this build.', + $this->renderAuthor(), + $this->renderValue($new)); + } + + public function getIcon() { + $new = $this->getNewValue(); + + switch ($new) { + case HarbormasterBuildCommand::COMMAND_RESTART: + return 'fa-backward'; + case HarbormasterBuildCommand::COMMAND_RESUME: + return 'fa-play'; + case HarbormasterBuildCommand::COMMAND_PAUSE: + return 'fa-pause'; + case HarbormasterBuildCommand::COMMAND_ABORT: + return 'fa-exclamation-triangle'; + default: + return 'fa-question'; + } + } + + public function getColor() { + $new = $this->getNewValue(); + + switch ($new) { + case HarbormasterBuildCommand::COMMAND_PAUSE: + case HarbormasterBuildCommand::COMMAND_ABORT: + return 'red'; + } + + return parent::getColor(); + } + + public function getTransactionTypeForConduit($xaction) { + return 'message'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'type' => $xaction->getNewValue(), + ); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + // TODO: Restore logic that tests if the command can issue without causing + // anything to lapse into an invalid state. This should not be the same + // as the logic which powers the web UI: for example, if an "abort" is + // queued we want to disable "Abort" in the web UI, but should obviously + // process it here. + + return $errors; + } + + public function applyInternalEffects($object, $value) { + $actor = $this->getActor(); + $build = $object; + + $new = $this->getNewValue(); + + switch ($new) { + case HarbormasterBuildCommand::COMMAND_ABORT: + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED); + break; + case HarbormasterBuildCommand::COMMAND_RESTART: + $build->restartBuild($actor); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING); + break; + case HarbormasterBuildCommand::COMMAND_RESUME: + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING); + break; + case HarbormasterBuildCommand::COMMAND_PAUSE: + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED); + break; + } + } + + public function applyExternalEffects($object, $value) { + $actor = $this->getActor(); + $build = $object; + + $new = $this->getNewValue(); + + switch ($new) { + case HarbormasterBuildCommand::COMMAND_ABORT: + $build->releaseAllArtifacts($actor); + break; + } + } + + +} diff --git a/src/applications/harbormaster/xaction/build/HarbormasterBuildTransactionType.php b/src/applications/harbormaster/xaction/build/HarbormasterBuildTransactionType.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/xaction/build/HarbormasterBuildTransactionType.php @@ -0,0 +1,4 @@ +