Skip to content

Commit 8bbee92

Browse files
author
epriestley
committed
Modularize individual Harbormaster build messages
Summary: Ref T13072. Further modularize build messages by applying each one in a separate transaction type. This makes it easier to add new types of messages (although I have no particular plans to do this, offhand) and reduces the amount of switch-boilerplate. This will probably also simplify validating "harbormaster.sendmessage". Test Plan: - Applied all commands. - Ran migration, saw transactions render properly Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13072 Differential Revision: https://secure.phabricator.com/D21690
1 parent 6dfea0a commit 8bbee92

8 files changed

+201
-100
lines changed

‎resources/sql/autopatches/20210715.harborcommand.01.xactions.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
// See T13072. Turn the old "process a command" transaction into modular
4+
// transactions that each handle one particular type of command.
5+
6+
$xactions_table = new HarbormasterBuildTransaction();
7+
$xactions_conn = $xactions_table->establishConnection('w');
8+
$row_iterator = new LiskRawMigrationIterator(
9+
$xactions_conn,
10+
$xactions_table->getTableName());
11+
12+
$map = array(
13+
'"pause"' => 'message/pause',
14+
'"abort"' => 'message/abort',
15+
'"resume"' => 'message/resume',
16+
'"restart"' => 'message/restart',
17+
);
18+
19+
foreach ($row_iterator as $row) {
20+
if ($row['transactionType'] !== 'harbormaster:build:command') {
21+
continue;
22+
}
23+
24+
$raw_value = $row['newValue'];
25+
26+
if (isset($map[$raw_value])) {
27+
queryfx(
28+
$xactions_conn,
29+
'UPDATE %R SET transactionType = %s WHERE id = %d',
30+
$xactions_table,
31+
$map[$raw_value],
32+
$row['id']);
33+
}
34+
}

‎src/__phutil_library_map__.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,7 +1408,11 @@
14081408
'HarbormasterBuildLogView' => 'applications/harbormaster/view/HarbormasterBuildLogView.php',
14091409
'HarbormasterBuildLogViewController' => 'applications/harbormaster/controller/HarbormasterBuildLogViewController.php',
14101410
'HarbormasterBuildMessage' => 'applications/harbormaster/storage/HarbormasterBuildMessage.php',
1411+
'HarbormasterBuildMessageAbortTransaction' => 'applications/harbormaster/xaction/build/HarbormasterBuildMessageAbortTransaction.php',
1412+
'HarbormasterBuildMessagePauseTransaction' => 'applications/harbormaster/xaction/build/HarbormasterBuildMessagePauseTransaction.php',
14111413
'HarbormasterBuildMessageQuery' => 'applications/harbormaster/query/HarbormasterBuildMessageQuery.php',
1414+
'HarbormasterBuildMessageRestartTransaction' => 'applications/harbormaster/xaction/build/HarbormasterBuildMessageRestartTransaction.php',
1415+
'HarbormasterBuildMessageResumeTransaction' => 'applications/harbormaster/xaction/build/HarbormasterBuildMessageResumeTransaction.php',
14121416
'HarbormasterBuildMessageTransaction' => 'applications/harbormaster/xaction/build/HarbormasterBuildMessageTransaction.php',
14131417
'HarbormasterBuildPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPHIDType.php',
14141418
'HarbormasterBuildPlan' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php',
@@ -7629,7 +7633,11 @@
76297633
'PhabricatorPolicyInterface',
76307634
'PhabricatorDestructibleInterface',
76317635
),
7636+
'HarbormasterBuildMessageAbortTransaction' => 'HarbormasterBuildMessageTransaction',
7637+
'HarbormasterBuildMessagePauseTransaction' => 'HarbormasterBuildMessageTransaction',
76327638
'HarbormasterBuildMessageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
7639+
'HarbormasterBuildMessageRestartTransaction' => 'HarbormasterBuildMessageTransaction',
7640+
'HarbormasterBuildMessageResumeTransaction' => 'HarbormasterBuildMessageTransaction',
76337641
'HarbormasterBuildMessageTransaction' => 'HarbormasterBuildTransactionType',
76347642
'HarbormasterBuildPHIDType' => 'PhabricatorPHIDType',
76357643
'HarbormasterBuildPlan' => array(

‎src/applications/harbormaster/engine/HarbormasterBuildEngine.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,18 @@ private function updateBuild(HarbormasterBuild $build) {
124124

125125
$xactions = array();
126126

127-
$message_xaction = HarbormasterBuildMessageTransaction::TRANSACTIONTYPE;
128-
129127
$messages = $build->getUnprocessedMessagesForApply();
130128
foreach ($messages as $message) {
131129
$message_type = $message->getType();
132130

131+
$message_xaction =
132+
HarbormasterBuildMessageTransaction::getTransactionTypeForMessageType(
133+
$message_type);
134+
135+
if (!$message_xaction) {
136+
continue;
137+
}
138+
133139
$xactions[] = $build->getApplicationTransactionTemplate()
134140
->setAuthorPHID($message->getAuthorPHID())
135141
->setTransactionType($message_xaction)

‎src/applications/harbormaster/xaction/build/HarbormasterBuildMessageAbortTransaction.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
final class HarbormasterBuildMessageAbortTransaction
4+
extends HarbormasterBuildMessageTransaction {
5+
6+
const TRANSACTIONTYPE = 'message/abort';
7+
8+
public function getMessageType() {
9+
return 'abort';
10+
}
11+
12+
public function getTitle() {
13+
return pht(
14+
'%s aborted this build.',
15+
$this->renderAuthor());
16+
}
17+
18+
public function getIcon() {
19+
return 'fa-exclamation-triangle';
20+
}
21+
22+
public function getColor() {
23+
return 'red';
24+
}
25+
26+
public function applyInternalEffects($object, $value) {
27+
$actor = $this->getActor();
28+
$build = $object;
29+
30+
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED);
31+
}
32+
33+
public function applyExternalEffects($object, $value) {
34+
$actor = $this->getActor();
35+
$build = $object;
36+
37+
$build->releaseAllArtifacts($actor);
38+
}
39+
40+
41+
}

‎src/applications/harbormaster/xaction/build/HarbormasterBuildMessagePauseTransaction.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
final class HarbormasterBuildMessagePauseTransaction
4+
extends HarbormasterBuildMessageTransaction {
5+
6+
const TRANSACTIONTYPE = 'message/pause';
7+
8+
public function getMessageType() {
9+
return 'pause';
10+
}
11+
12+
public function getTitle() {
13+
return pht(
14+
'%s paused this build.',
15+
$this->renderAuthor());
16+
}
17+
18+
public function getIcon() {
19+
return 'fa-pause';
20+
}
21+
22+
public function getColor() {
23+
return 'red';
24+
}
25+
26+
public function applyInternalEffects($object, $value) {
27+
$actor = $this->getActor();
28+
$build = $object;
29+
30+
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED);
31+
}
32+
33+
}

‎src/applications/harbormaster/xaction/build/HarbormasterBuildMessageRestartTransaction.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
final class HarbormasterBuildMessageRestartTransaction
4+
extends HarbormasterBuildMessageTransaction {
5+
6+
const TRANSACTIONTYPE = 'message/restart';
7+
8+
public function getMessageType() {
9+
return 'restart';
10+
}
11+
12+
public function getTitle() {
13+
return pht(
14+
'%s restarted this build.',
15+
$this->renderAuthor());
16+
}
17+
18+
public function getIcon() {
19+
return 'fa-backward';
20+
}
21+
22+
public function applyInternalEffects($object, $value) {
23+
$actor = $this->getActor();
24+
$build = $object;
25+
26+
$build->restartBuild($actor);
27+
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
28+
}
29+
30+
}

‎src/applications/harbormaster/xaction/build/HarbormasterBuildMessageResumeTransaction.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
final class HarbormasterBuildMessageResumeTransaction
4+
extends HarbormasterBuildMessageTransaction {
5+
6+
const TRANSACTIONTYPE = 'message/resume';
7+
8+
public function getMessageType() {
9+
return 'resume';
10+
}
11+
12+
public function getTitle() {
13+
return pht(
14+
'%s resumed this build.',
15+
$this->renderAuthor());
16+
}
17+
18+
public function getIcon() {
19+
return 'fa-play';
20+
}
21+
22+
public function applyInternalEffects($object, $value) {
23+
$actor = $this->getActor();
24+
$build = $object;
25+
26+
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
27+
}
28+
29+
}

‎src/applications/harbormaster/xaction/build/HarbormasterBuildMessageTransaction.php‎

Lines changed: 18 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,80 +1,37 @@
11
<?php
22

3-
final class HarbormasterBuildMessageTransaction
3+
abstract class HarbormasterBuildMessageTransaction
44
extends HarbormasterBuildTransactionType {
55

6-
const TRANSACTIONTYPE = 'harbormaster:build:command';
7-
8-
public function generateOldValue($object) {
6+
final public function generateOldValue($object) {
97
return null;
108
}
119

12-
public function getTitle() {
13-
$new = $this->getNewValue();
14-
15-
switch ($new) {
16-
case HarbormasterBuildCommand::COMMAND_RESTART:
17-
return pht(
18-
'%s restarted this build.',
19-
$this->renderAuthor());
20-
case HarbormasterBuildCommand::COMMAND_ABORT:
21-
return pht(
22-
'%s aborted this build.',
23-
$this->renderAuthor());
24-
case HarbormasterBuildCommand::COMMAND_RESUME:
25-
return pht(
26-
'%s resumed this build.',
27-
$this->renderAuthor());
28-
case HarbormasterBuildCommand::COMMAND_PAUSE:
29-
return pht(
30-
'%s paused this build.',
31-
$this->renderAuthor());
32-
}
33-
34-
return pht(
35-
'%s issued an unknown command ("%s") to this build.',
36-
$this->renderAuthor(),
37-
$this->renderValue($new));
10+
final public function getTransactionTypeForConduit($xaction) {
11+
return 'message';
3812
}
3913

40-
public function getIcon() {
41-
$new = $this->getNewValue();
42-
43-
switch ($new) {
44-
case HarbormasterBuildCommand::COMMAND_RESTART:
45-
return 'fa-backward';
46-
case HarbormasterBuildCommand::COMMAND_RESUME:
47-
return 'fa-play';
48-
case HarbormasterBuildCommand::COMMAND_PAUSE:
49-
return 'fa-pause';
50-
case HarbormasterBuildCommand::COMMAND_ABORT:
51-
return 'fa-exclamation-triangle';
52-
default:
53-
return 'fa-question';
54-
}
14+
final public function getFieldValuesForConduit($xaction, $data) {
15+
return array(
16+
'type' => $xaction->getNewValue(),
17+
);
5518
}
5619

57-
public function getColor() {
58-
$new = $this->getNewValue();
20+
final public static function getTransactionTypeForMessageType($message_type) {
21+
$message_xactions = id(new PhutilClassMapQuery())
22+
->setAncestorClass(__CLASS__)
23+
->execute();
5924

60-
switch ($new) {
61-
case HarbormasterBuildCommand::COMMAND_PAUSE:
62-
case HarbormasterBuildCommand::COMMAND_ABORT:
63-
return 'red';
25+
foreach ($message_xactions as $message_xaction) {
26+
if ($message_xaction->getMessageType() === $message_type) {
27+
return $message_xaction->getTransactionTypeConstant();
28+
}
6429
}
6530

66-
return parent::getColor();
31+
return null;
6732
}
6833

69-
public function getTransactionTypeForConduit($xaction) {
70-
return 'message';
71-
}
72-
73-
public function getFieldValuesForConduit($xaction, $data) {
74-
return array(
75-
'type' => $xaction->getNewValue(),
76-
);
77-
}
34+
abstract public function getMessageType();
7835

7936
public function validateTransactions($object, array $xactions) {
8037
$errors = array();
@@ -88,41 +45,4 @@ public function validateTransactions($object, array $xactions) {
8845
return $errors;
8946
}
9047

91-
public function applyInternalEffects($object, $value) {
92-
$actor = $this->getActor();
93-
$build = $object;
94-
95-
$new = $this->getNewValue();
96-
97-
switch ($new) {
98-
case HarbormasterBuildCommand::COMMAND_ABORT:
99-
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED);
100-
break;
101-
case HarbormasterBuildCommand::COMMAND_RESTART:
102-
$build->restartBuild($actor);
103-
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
104-
break;
105-
case HarbormasterBuildCommand::COMMAND_RESUME:
106-
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
107-
break;
108-
case HarbormasterBuildCommand::COMMAND_PAUSE:
109-
$build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED);
110-
break;
111-
}
112-
}
113-
114-
public function applyExternalEffects($object, $value) {
115-
$actor = $this->getActor();
116-
$build = $object;
117-
118-
$new = $this->getNewValue();
119-
120-
switch ($new) {
121-
case HarbormasterBuildCommand::COMMAND_ABORT:
122-
$build->releaseAllArtifacts($actor);
123-
break;
124-
}
125-
}
126-
127-
12848
}

0 commit comments

Comments
 (0)