From 1885c4e03bb62bc71c119299010ca4b84dcdd15b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 06:51:22 -0700 Subject: [PATCH] Add an ItemCommand queue to Nuance Summary: Ref T10537. Generally, when users interact with Nuance items we'll dump a command into a queue and apply it in the background. This avoids race conditions with multiple users interacting with an item, which Nuance is more subject to than other applications because it has an import/external component. The "sync" command doesn't actually do anything yet. Test Plan: {F1186365} Reviewers: chad Reviewed By: chad Subscribers: Luke081515.2 Maniphest Tasks: T10537 Differential Revision: https://secure.phabricator.com/D15506 --- .../20160322.nuance.01.itemcommand.sql | 8 +++ src/__phutil_library_map__.php | 7 +++ .../controller/NuanceItemViewController.php | 45 ++++++++++++++- .../nuance/editor/NuanceItemEditor.php | 7 +++ .../nuance/item/NuanceGitHubEventItemType.php | 14 +++++ .../nuance/item/NuanceItemType.php | 39 +++++++++++++ .../nuance/query/NuanceItemCommandQuery.php | 47 ++++++++++++++++ .../nuance/storage/NuanceItem.php | 17 ++++++ .../nuance/storage/NuanceItemCommand.php | 55 +++++++++++++++++++ .../nuance/storage/NuanceItemTransaction.php | 7 +++ .../nuance/worker/NuanceItemUpdateWorker.php | 19 +++++++ 11 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 resources/sql/autopatches/20160322.nuance.01.itemcommand.sql create mode 100644 src/applications/nuance/query/NuanceItemCommandQuery.php create mode 100644 src/applications/nuance/storage/NuanceItemCommand.php diff --git a/resources/sql/autopatches/20160322.nuance.01.itemcommand.sql b/resources/sql/autopatches/20160322.nuance.01.itemcommand.sql new file mode 100644 index 0000000000..f356db4947 --- /dev/null +++ b/resources/sql/autopatches/20160322.nuance.01.itemcommand.sql @@ -0,0 +1,8 @@ +CREATE TABLE {$NAMESPACE}_nuance.nuance_itemcommand ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + itemPHID VARBINARY(64) NOT NULL, + authorPHID VARBINARY(64) NOT NULL, + command VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT}, + parameters LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}, + KEY `key_item` (itemPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 92290ecd77..3932c6a4c1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1440,6 +1440,8 @@ phutil_register_library_map(array( 'NuanceImportCursorPHIDType' => 'applications/nuance/phid/NuanceImportCursorPHIDType.php', 'NuanceItem' => 'applications/nuance/storage/NuanceItem.php', 'NuanceItemActionController' => 'applications/nuance/controller/NuanceItemActionController.php', + 'NuanceItemCommand' => 'applications/nuance/storage/NuanceItemCommand.php', + 'NuanceItemCommandQuery' => 'applications/nuance/query/NuanceItemCommandQuery.php', 'NuanceItemController' => 'applications/nuance/controller/NuanceItemController.php', 'NuanceItemEditor' => 'applications/nuance/editor/NuanceItemEditor.php', 'NuanceItemListController' => 'applications/nuance/controller/NuanceItemListController.php', @@ -5728,6 +5730,11 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', ), 'NuanceItemActionController' => 'NuanceController', + 'NuanceItemCommand' => array( + 'NuanceDAO', + 'PhabricatorPolicyInterface', + ), + 'NuanceItemCommandQuery' => 'NuanceQuery', 'NuanceItemController' => 'NuanceController', 'NuanceItemEditor' => 'PhabricatorApplicationTransactionEditor', 'NuanceItemListController' => 'NuanceItemController', diff --git a/src/applications/nuance/controller/NuanceItemViewController.php b/src/applications/nuance/controller/NuanceItemViewController.php index e8e3967545..091ade2d6b 100644 --- a/src/applications/nuance/controller/NuanceItemViewController.php +++ b/src/applications/nuance/controller/NuanceItemViewController.php @@ -26,6 +26,17 @@ final class NuanceItemViewController extends NuanceController { $curtain = $this->buildCurtain($item); $content = $this->buildContent($item); + $commands = $this->buildCommands($item); + + $timeline = $this->buildTransactionTimeline( + $item, + new NuanceItemTransactionQuery()); + + $main = array( + $commands, + $content, + $timeline, + ); $header = id(new PHUIHeaderView()) ->setHeader($name); @@ -33,7 +44,7 @@ final class NuanceItemViewController extends NuanceController { $view = id(new PHUITwoColumnView()) ->setHeader($header) ->setCurtain($curtain) - ->setMainColumn($content); + ->setMainColumn($main); return $this->newPage() ->setTitle($title) @@ -76,4 +87,36 @@ final class NuanceItemViewController extends NuanceController { return $impl->buildItemView($item); } + private function buildCommands(NuanceItem $item) { + $viewer = $this->getViewer(); + + $commands = id(new NuanceItemCommandQuery()) + ->setViewer($viewer) + ->withItemPHIDs(array($item->getPHID())) + ->execute(); + $commands = msort($commands, 'getID'); + + if (!$commands) { + return null; + } + + $rows = array(); + foreach ($commands as $command) { + $rows[] = array( + $command->getCommand(), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Command'), + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Pending Commands')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setTable($table); + } + } diff --git a/src/applications/nuance/editor/NuanceItemEditor.php b/src/applications/nuance/editor/NuanceItemEditor.php index a331a65196..45288052c2 100644 --- a/src/applications/nuance/editor/NuanceItemEditor.php +++ b/src/applications/nuance/editor/NuanceItemEditor.php @@ -19,6 +19,7 @@ final class NuanceItemEditor $types[] = NuanceItemTransaction::TYPE_REQUESTOR; $types[] = NuanceItemTransaction::TYPE_PROPERTY; $types[] = NuanceItemTransaction::TYPE_QUEUE; + $types[] = NuanceItemTransaction::TYPE_COMMAND; $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorTransactions::TYPE_COMMENT; @@ -45,6 +46,8 @@ final class NuanceItemEditor $key = $xaction->getMetadataValue( NuanceItemTransaction::PROPERTY_KEY); return $object->getNuanceProperty($key); + case NuanceItemTransaction::TYPE_COMMAND: + return null; } return parent::getCustomTransactionOldValue($object, $xaction); @@ -60,6 +63,7 @@ final class NuanceItemEditor case NuanceItemTransaction::TYPE_OWNER: case NuanceItemTransaction::TYPE_PROPERTY: case NuanceItemTransaction::TYPE_QUEUE: + case NuanceItemTransaction::TYPE_COMMAND: return $xaction->getNewValue(); } @@ -88,6 +92,8 @@ final class NuanceItemEditor NuanceItemTransaction::PROPERTY_KEY); $object->setNuanceProperty($key, $xaction->getNewValue()); break; + case NuanceItemTransaction::TYPE_COMMAND: + break; } } @@ -101,6 +107,7 @@ final class NuanceItemEditor case NuanceItemTransaction::TYPE_OWNER: case NuanceItemTransaction::TYPE_PROPERTY: case NuanceItemTransaction::TYPE_QUEUE: + case NuanceItemTransaction::TYPE_COMMAND: return; } diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index dd52dc1c15..3ee2e91489 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -147,6 +147,12 @@ final class NuanceGitHubEventItemType public function getItemActions(NuanceItem $item) { $actions = array(); + $actions[] = $this->newItemAction($item, 'sync') + ->setName(pht('Import to Maniphest')) + ->setIcon('fa-anchor') + ->setWorkflow(true) + ->setRenderAsForm(true); + $actions[] = $this->newItemAction($item, 'raw') ->setName(pht('View Raw Event')) ->setWorkflow(true) @@ -156,6 +162,7 @@ final class NuanceGitHubEventItemType } protected function handleAction(NuanceItem $item, $action) { + $viewer = $this->getViewer(); $controller = $this->getController(); switch ($action) { @@ -181,6 +188,9 @@ final class NuanceGitHubEventItemType ->setTitle(pht('GitHub Raw Event')) ->appendForm($form) ->addCancelButton($item->getURI(), pht('Done')); + case 'sync': + $item->issueCommand($viewer->getPHID(), $action); + return id(new AphrontRedirectResponse())->setURI($item->getURI()); } return null; @@ -228,5 +238,9 @@ final class NuanceGitHubEventItemType ->appendChild($property_list); } + protected function handleCommand(NuanceItem $item, $action) { + return null; + } + } diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php index 14b2fc5324..144d6e86f5 100644 --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -96,4 +96,43 @@ abstract class NuanceItemType return null; } + final public function applyCommand( + NuanceItem $item, + NuanceItemCommand $command) { + + $result = $this->handleCommand($item, $command); + + if ($result === null) { + return; + } + + $xaction = id(new NuanceItemTransaction()) + ->setTransactionType(NuanceItemTransaction::TYPE_COMMAND) + ->setNewValue( + array( + 'command' => $command->getCommand(), + 'parameters' => $command->getParameters(), + 'result' => $result, + )); + + $viewer = $this->getViewer(); + + // TODO: Maybe preserve the actor's original content source? + $source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_DAEMON, + array()); + + $editor = id(new NuanceItemEditor()) + ->setActor($viewer) + ->setActingAsPHID($command->getAuthorPHID()) + ->setContentSource($source) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($item, array($xaction)); + } + + protected function handleCommand(NuanceItem $item, $action) { + return null; + } + } diff --git a/src/applications/nuance/query/NuanceItemCommandQuery.php b/src/applications/nuance/query/NuanceItemCommandQuery.php new file mode 100644 index 0000000000..cb20610187 --- /dev/null +++ b/src/applications/nuance/query/NuanceItemCommandQuery.php @@ -0,0 +1,47 @@ +ids = $ids; + return $this; + } + + public function withItemPHIDs(array $item_phids) { + $this->itemPHIDs = $item_phids; + return $this; + } + + public function newResultObject() { + return new NuanceItemCommand(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->itemPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'itemPHID IN (%Ls)', + $this->itemPHIDs); + } + + return $where; + } + +} diff --git a/src/applications/nuance/storage/NuanceItem.php b/src/applications/nuance/storage/NuanceItem.php index a83db3ec70..a21a5443c2 100644 --- a/src/applications/nuance/storage/NuanceItem.php +++ b/src/applications/nuance/storage/NuanceItem.php @@ -154,6 +154,23 @@ final class NuanceItem )); } + public function issueCommand( + $author_phid, + $command, + array $parameters = array()) { + + $command = id(NuanceItemCommand::initializeNewCommand()) + ->setItemPHID($this->getPHID()) + ->setAuthorPHID($author_phid) + ->setCommand($command) + ->setParameters($parameters) + ->save(); + + $this->scheduleUpdate(); + + return $this; + } + public function getImplementation() { return $this->assertAttached($this->implementation); } diff --git a/src/applications/nuance/storage/NuanceItemCommand.php b/src/applications/nuance/storage/NuanceItemCommand.php new file mode 100644 index 0000000000..94409dd89a --- /dev/null +++ b/src/applications/nuance/storage/NuanceItemCommand.php @@ -0,0 +1,55 @@ + false, + self::CONFIG_SERIALIZATION => array( + 'parameters' => self::SERIALIZATION_JSON, + ), + self::CONFIG_COLUMN_SCHEMA => array( + 'command' => 'text64', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_item' => array( + 'columns' => array('itemPHID'), + ), + ), + ) + parent::getConfiguration(); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + return PhabricatorPolicies::getMostOpenPolicy(); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + public function describeAutomaticCapability($capability) { + return null; + } + +} diff --git a/src/applications/nuance/storage/NuanceItemTransaction.php b/src/applications/nuance/storage/NuanceItemTransaction.php index 183596402f..77471fcdb9 100644 --- a/src/applications/nuance/storage/NuanceItemTransaction.php +++ b/src/applications/nuance/storage/NuanceItemTransaction.php @@ -10,6 +10,7 @@ final class NuanceItemTransaction const TYPE_SOURCE = 'nuance.item.source'; const TYPE_PROPERTY = 'nuance.item.property'; const TYPE_QUEUE = 'nuance.item.queue'; + const TYPE_COMMAND = 'nuance.item.command'; public function getApplicationTransactionType() { return NuanceItemPHIDType::TYPECONST; @@ -65,6 +66,12 @@ final class NuanceItemTransaction '%s routed this item to the %s queue.', $this->renderHandleLink($author_phid), $this->renderHandleLink($new)); + case self::TYPE_COMMAND: + // TODO: Give item types a chance to render this properly. + return pht( + '%s applied command "%s" to this item.', + $this->renderHandleLink($author_phid), + idx($new, 'command')); } return parent::getTitle(); diff --git a/src/applications/nuance/worker/NuanceItemUpdateWorker.php b/src/applications/nuance/worker/NuanceItemUpdateWorker.php index c8c65e33ea..57be20edae 100644 --- a/src/applications/nuance/worker/NuanceItemUpdateWorker.php +++ b/src/applications/nuance/worker/NuanceItemUpdateWorker.php @@ -15,6 +15,7 @@ final class NuanceItemUpdateWorker $item = $this->loadItem($item_phid); $this->updateItem($item); $this->routeItem($item); + $this->applyCommands($item); } catch (Exception $ex) { $lock->unlock(); throw $ex; @@ -51,4 +52,22 @@ final class NuanceItemUpdateWorker ->save(); } + private function applyCommands(NuanceItem $item) { + $viewer = $this->getViewer(); + + $impl = $item->getImplementation(); + $impl->setViewer($viewer); + + $commands = id(new NuanceItemCommandQuery()) + ->setViewer($viewer) + ->withItemPHIDs(array($item->getPHID())) + ->execute(); + $commands = msort($commands, 'getID'); + + foreach ($commands as $command) { + $impl->applyCommand($item, $command); + $command->delete(); + } + } + }