Allow a HarbormasterBuildMessage to be sent to any object

Summary:
See T13054. This prepares for Buildables to be sent messages ("attach", "done scheduling builds") to fix races between Harbormaster and Differential.

The `buildTargetPHID` is replaced with a `recipientPHID` in the API. An additional change will fix the storage.

In the future, this table could probably also replace `HarbormasterBuildCommand` now, which is approximately the same bus, but for Builds.

Test Plan: Viewed builds with messages. Sent messages with `harbormaster.sendmessage`. Processed messages with `bin/phd debug task`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19062
This commit is contained in:
epriestley
2018-02-12 10:16:23 -08:00
parent 4fa99374be
commit ed0ba41cd2
5 changed files with 54 additions and 48 deletions

View File

@@ -245,7 +245,7 @@ final class HarbormasterSendMessageConduitAPIMethod
} }
$save[] = HarbormasterBuildMessage::initializeNewMessage($viewer) $save[] = HarbormasterBuildMessage::initializeNewMessage($viewer)
->setBuildTargetPHID($build_target->getPHID()) ->setReceiverPHID($build_target->getPHID())
->setType($message_type); ->setType($message_type);
$build_target->openTransaction(); $build_target->openTransaction();

View File

@@ -65,9 +65,9 @@ final class HarbormasterBuildViewController
if ($build_targets) { if ($build_targets) {
$messages = id(new HarbormasterBuildMessageQuery()) $messages = id(new HarbormasterBuildMessageQuery())
->setViewer($viewer) ->setViewer($viewer)
->withBuildTargetPHIDs(mpull($build_targets, 'getPHID')) ->withReceiverPHIDs(mpull($build_targets, 'getPHID'))
->execute(); ->execute();
$messages = mgroup($messages, 'getBuildTargetPHID'); $messages = mgroup($messages, 'getReceiverPHID');
} else { } else {
$messages = array(); $messages = array();
} }

View File

@@ -382,12 +382,12 @@ final class HarbormasterBuildEngine extends Phobject {
$messages = id(new HarbormasterBuildMessageQuery()) $messages = id(new HarbormasterBuildMessageQuery())
->setViewer($this->getViewer()) ->setViewer($this->getViewer())
->withBuildTargetPHIDs(array_keys($waiting_targets)) ->withReceiverPHIDs(array_keys($waiting_targets))
->withConsumed(false) ->withConsumed(false)
->execute(); ->execute();
foreach ($messages as $message) { foreach ($messages as $message) {
$target = $waiting_targets[$message->getBuildTargetPHID()]; $target = $waiting_targets[$message->getReceiverPHID()];
switch ($message->getType()) { switch ($message->getType()) {
case HarbormasterMessageType::MESSAGE_PASS: case HarbormasterMessageType::MESSAGE_PASS:

View File

@@ -4,7 +4,7 @@ final class HarbormasterBuildMessageQuery
extends PhabricatorCursorPagedPolicyAwareQuery { extends PhabricatorCursorPagedPolicyAwareQuery {
private $ids; private $ids;
private $buildTargetPHIDs; private $receiverPHIDs;
private $consumed; private $consumed;
public function withIDs(array $ids) { public function withIDs(array $ids) {
@@ -12,8 +12,8 @@ final class HarbormasterBuildMessageQuery
return $this; return $this;
} }
public function withBuildTargetPHIDs(array $phids) { public function withReceiverPHIDs(array $phids) {
$this->buildTargetPHIDs = $phids; $this->receiverPHIDs = $phids;
return $this; return $this;
} }
@@ -22,73 +22,67 @@ final class HarbormasterBuildMessageQuery
return $this; return $this;
} }
public function newResultObject() {
return new HarbormasterBuildMessage();
}
protected function loadPage() { protected function loadPage() {
$table = new HarbormasterBuildMessage(); return $this->loadStandardPage($this->newResultObject());
$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);
} }
protected function willFilterPage(array $page) { protected function willFilterPage(array $page) {
$build_target_phids = array_filter(mpull($page, 'getBuildTargetPHID')); $receiver_phids = array_filter(mpull($page, 'getReceiverPHID'));
if ($build_target_phids) { if ($receiver_phids) {
$build_targets = id(new PhabricatorObjectQuery()) $receivers = id(new PhabricatorObjectQuery())
->setViewer($this->getViewer()) ->setViewer($this->getViewer())
->withPHIDs($build_target_phids) ->withPHIDs($receiver_phids)
->setParentQuery($this) ->setParentQuery($this)
->execute(); ->execute();
$build_targets = mpull($build_targets, null, 'getPHID'); $receivers = mpull($receivers, null, 'getPHID');
} else { } else {
$build_targets = array(); $receivers = array();
} }
foreach ($page as $key => $message) { foreach ($page as $key => $message) {
$build_target_phid = $message->getBuildTargetPHID(); $receiver_phid = $message->getReceiverPHID();
if (empty($build_targets[$build_target_phid])) {
if (empty($receivers[$receiver_phid])) {
unset($page[$key]); unset($page[$key]);
$this->didRejectResult($message);
continue; continue;
} }
$message->attachBuildTarget($build_targets[$build_target_phid]);
$message->attachReceiver($receivers[$receiver_phid]);
} }
return $page; return $page;
} }
protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
$where = array(); $where = parent::buildWhereClauseParts($conn);
if ($this->ids) { if ($this->ids !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn,
'id IN (%Ld)', 'id IN (%Ld)',
$this->ids); $this->ids);
} }
if ($this->buildTargetPHIDs) { if ($this->receiverPHIDs !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn,
'buildTargetPHID IN (%Ls)', 'buildTargetPHID IN (%Ls)',
$this->buildTargetPHIDs); $this->receiverPHIDs);
} }
if ($this->consumed !== null) { if ($this->consumed !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn,
'isConsumed = %d', 'isConsumed = %d',
(int)$this->consumed); (int)$this->consumed);
} }
$where[] = $this->buildPagingClause($conn_r); return $where;
return $this->formatWhereClause($where);
} }
public function getQueryApplicationClass() { public function getQueryApplicationClass() {

View File

@@ -14,7 +14,7 @@ final class HarbormasterBuildMessage extends HarbormasterDAO
protected $type; protected $type;
protected $isConsumed; protected $isConsumed;
private $buildTarget = self::ATTACHABLE; private $receiver = self::ATTACHABLE;
public static function initializeNewMessage(PhabricatorUser $actor) { public static function initializeNewMessage(PhabricatorUser $actor) {
$actor_phid = $actor->getPHID(); $actor_phid = $actor->getPHID();
@@ -41,12 +41,24 @@ final class HarbormasterBuildMessage extends HarbormasterDAO
) + parent::getConfiguration(); ) + parent::getConfiguration();
} }
public function getBuildTarget() { public function getReceiverPHID() {
return $this->assertAttached($this->buildTarget); return $this->getBuildTargetPHID();
} }
public function attachBuildTarget(HarbormasterBuildTarget $target) { public function setReceiverPHID($phid) {
$this->buildTarget = $target; return $this->setBuildTargetPHID($phid);
}
public function getReceiver() {
return $this->assertAttached($this->receiver);
}
public function getBuildTarget() {
return $this->getReceiver();
}
public function attachReceiver($receiver) {
$this->receiver = $receiver;
return $this; return $this;
} }
@@ -61,17 +73,17 @@ final class HarbormasterBuildMessage extends HarbormasterDAO
} }
public function getPolicy($capability) { public function getPolicy($capability) {
return $this->getBuildTarget()->getPolicy($capability); return $this->getReceiver()->getPolicy($capability);
} }
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
return $this->getBuildTarget()->hasAutomaticCapability( return $this->getReceiver()->hasAutomaticCapability(
$capability, $capability,
$viewer); $viewer);
} }
public function describeAutomaticCapability($capability) { public function describeAutomaticCapability($capability) {
return pht('Build messages have the same policies as their targets.'); return pht('Build messages have the same policies as their receivers.');
} }
} }