From b48d4fabaf9ee614747009c4f465e4232a2f36bb Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 13 Jul 2021 15:28:49 -0700 Subject: [PATCH] Merge the "HarbormasterBuildCommand" table into "HarbormasterBuildMessage" Summary: Ref T13072. These two similar tables don't make sense to keep separate. Instead, make Build a valid receiver for BuildMessage objects. These tables are practically the same, so this is straightforward: just copy the rows in and then drop the old table. (This table was trivial and ephemeral anyway, so I'm not bothering to do the usual "keep it around for a couple years just in case".) Test Plan: - Populated BuildCommand table, ran migration, saw Builds end up in the proper transitional state (e.g., pausing, aborting, restarting) with appropriate queued messages. - Queued new messages by clicking UI buttons. - Ran BuildWorkers, saw them process messages and mark them as consumed. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13072 Differential Revision: https://secure.phabricator.com/D21684 --- .../20210713.harborcommand.01.migrate.sql | 4 ++++ .../20210713.harborcommand.02.drop.sql | 1 + src/__phutil_library_map__.php | 2 +- .../HarbormasterBuildTransactionEditor.php | 6 +++--- .../query/HarbormasterBuildQuery.php | 6 +++--- .../storage/HarbormasterBuildCommand.php | 20 ++----------------- .../storage/build/HarbormasterBuild.php | 19 +++++++++++------- 7 files changed, 26 insertions(+), 32 deletions(-) create mode 100644 resources/sql/autopatches/20210713.harborcommand.01.migrate.sql create mode 100644 resources/sql/autopatches/20210713.harborcommand.02.drop.sql diff --git a/resources/sql/autopatches/20210713.harborcommand.01.migrate.sql b/resources/sql/autopatches/20210713.harborcommand.01.migrate.sql new file mode 100644 index 0000000000..7acf3b33c8 --- /dev/null +++ b/resources/sql/autopatches/20210713.harborcommand.01.migrate.sql @@ -0,0 +1,4 @@ +INSERT IGNORE INTO {$NAMESPACE}_harbormaster.harbormaster_buildmessage + (authorPHID, receiverPHID, type, isConsumed, dateCreated, dateModified) + SELECT authorPHID, targetPHID, command, 0, dateCreated, dateModified + FROM {$NAMESPACE}_harbormaster.harbormaster_buildcommand; diff --git a/resources/sql/autopatches/20210713.harborcommand.02.drop.sql b/resources/sql/autopatches/20210713.harborcommand.02.drop.sql new file mode 100644 index 0000000000..0d04376c13 --- /dev/null +++ b/resources/sql/autopatches/20210713.harborcommand.02.drop.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS {$NAMESPACE}_harbormaster.harbormaster_buildcommand; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2c5bb3341f..999b4bf973 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -7597,7 +7597,7 @@ phutil_register_library_map(array( 'HarbormasterBuildArtifactPHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildArtifactQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildAutoplan' => 'Phobject', - 'HarbormasterBuildCommand' => 'HarbormasterDAO', + 'HarbormasterBuildCommand' => 'Phobject', 'HarbormasterBuildDependencyDatasource' => 'PhabricatorTypeaheadDatasource', 'HarbormasterBuildEngine' => 'Phobject', 'HarbormasterBuildFailureException' => 'Exception', diff --git a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php index 2615f3c5be..d4c2a44957 100644 --- a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php @@ -93,10 +93,10 @@ final class HarbormasterBuildTransactionEditor return; } - id(new HarbormasterBuildCommand()) + HarbormasterBuildMessage::initializeNewMessage($actor) ->setAuthorPHID($xaction->getAuthorPHID()) - ->setTargetPHID($build->getPHID()) - ->setCommand($command) + ->setReceiverPHID($build->getPHID()) + ->setType($command) ->save(); PhabricatorWorker::scheduleTask( diff --git a/src/applications/harbormaster/query/HarbormasterBuildQuery.php b/src/applications/harbormaster/query/HarbormasterBuildQuery.php index 3e474bd81d..5ff5fe2eb3 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildQuery.php @@ -104,10 +104,10 @@ final class HarbormasterBuildQuery } $build_phids = mpull($page, 'getPHID'); - $messages = id(new HarbormasterBuildCommand())->loadAllWhere( - 'targetPHID IN (%Ls) ORDER BY id ASC', + $messages = id(new HarbormasterBuildMessage())->loadAllWhere( + 'receiverPHID IN (%Ls) AND isConsumed = 0 ORDER BY id ASC', $build_phids); - $messages = mgroup($messages, 'getTargetPHID'); + $messages = mgroup($messages, 'getReceiverPHID'); foreach ($page as $build) { $unprocessed_messages = idx($messages, $build->getPHID(), array()); $build->attachUnprocessedMessages($unprocessed_messages); diff --git a/src/applications/harbormaster/storage/HarbormasterBuildCommand.php b/src/applications/harbormaster/storage/HarbormasterBuildCommand.php index 50a40d8e98..4d5d7f7cc3 100644 --- a/src/applications/harbormaster/storage/HarbormasterBuildCommand.php +++ b/src/applications/harbormaster/storage/HarbormasterBuildCommand.php @@ -1,27 +1,11 @@ array( - 'command' => 'text128', - ), - self::CONFIG_KEY_SCHEMA => array( - 'key_target' => array( - 'columns' => array('targetPHID'), - ), - ), - ) + parent::getConfiguration(); - } - } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 292ccd99cf..954c710d90 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -230,6 +230,7 @@ final class HarbormasterBuild extends HarbormasterDAO } public function attachUnprocessedMessages(array $messages) { + assert_instances_of($messages, 'HarbormasterBuildMessage'); $this->unprocessedMessages = $messages; return $this; } @@ -331,7 +332,7 @@ final class HarbormasterBuild extends HarbormasterDAO public function isPausing() { $is_pausing = false; foreach ($this->getUnprocessedMessages() as $message_object) { - $message_type = $message_object->getCommand(); + $message_type = $message_object->getType(); switch ($message_type) { case HarbormasterBuildCommand::COMMAND_PAUSE: $is_pausing = true; @@ -352,7 +353,7 @@ final class HarbormasterBuild extends HarbormasterDAO public function isResuming() { $is_resuming = false; foreach ($this->getUnprocessedMessages() as $message_object) { - $message_type = $message_object->getCommand(); + $message_type = $message_object->getType(); switch ($message_type) { case HarbormasterBuildCommand::COMMAND_RESTART: case HarbormasterBuildCommand::COMMAND_RESUME: @@ -373,7 +374,7 @@ final class HarbormasterBuild extends HarbormasterDAO public function isRestarting() { $is_restarting = false; foreach ($this->getUnprocessedMessages() as $message_object) { - $message_type = $message_object->getCommand(); + $message_type = $message_object->getType(); switch ($message_type) { case HarbormasterBuildCommand::COMMAND_RESTART: $is_restarting = true; @@ -387,7 +388,7 @@ final class HarbormasterBuild extends HarbormasterDAO public function isAborting() { $is_aborting = false; foreach ($this->getUnprocessedMessages() as $message_object) { - $message_type = $message_object->getCommand(); + $message_type = $message_object->getType(); switch ($message_type) { case HarbormasterBuildCommand::COMMAND_ABORT: $is_aborting = true; @@ -399,9 +400,13 @@ final class HarbormasterBuild extends HarbormasterDAO } public function markUnprocessedMessagesAsProcessed() { - // TODO: See T13072. This is a placeholder until BuildCommand and - // BuildMessage merge. - return $this->deleteUnprocessedMessages(); + foreach ($this->getUnprocessedMessages() as $key => $message_object) { + $message_object + ->setIsConsumed(1) + ->save(); + } + + return $this; } public function deleteUnprocessedMessages() {