Page MenuHomePhabricator

D18015.diff
No OneTemporary

D18015.diff

diff --git a/src/applications/nuance/command/NuanceCommandImplementation.php b/src/applications/nuance/command/NuanceCommandImplementation.php
--- a/src/applications/nuance/command/NuanceCommandImplementation.php
+++ b/src/applications/nuance/command/NuanceCommandImplementation.php
@@ -19,6 +19,12 @@
abstract public function getCommandName();
abstract public function canApplyToItem(NuanceItem $item);
+ public function canApplyImmediately(
+ NuanceItem $item,
+ NuanceItemCommand $command) {
+ return false;
+ }
+
abstract protected function executeCommand(
NuanceItem $item,
NuanceItemCommand $command);
diff --git a/src/applications/nuance/command/NuanceTrashCommand.php b/src/applications/nuance/command/NuanceTrashCommand.php
--- a/src/applications/nuance/command/NuanceTrashCommand.php
+++ b/src/applications/nuance/command/NuanceTrashCommand.php
@@ -14,6 +14,12 @@
return ($type instanceof NuanceFormItemType);
}
+ public function canApplyImmediately(
+ NuanceItem $item,
+ NuanceItemCommand $command) {
+ return true;
+ }
+
protected function executeCommand(
NuanceItem $item,
NuanceItemCommand $command) {
diff --git a/src/applications/nuance/controller/NuanceItemActionController.php b/src/applications/nuance/controller/NuanceItemActionController.php
--- a/src/applications/nuance/controller/NuanceItemActionController.php
+++ b/src/applications/nuance/controller/NuanceItemActionController.php
@@ -53,6 +53,25 @@
$impl->setViewer($viewer);
$impl->setController($this);
+ $executors = NuanceCommandImplementation::getAllCommands();
+ $executor = idx($executors, $action);
+ if (!$executor) {
+ return new Aphront404Response();
+ }
+
+ $executor = id(clone $executor)
+ ->setActor($viewer);
+
+ if (!$executor->canApplyToItem($item)) {
+ return $this->newDialog()
+ ->setTitle(pht('Command Not Supported'))
+ ->appendParagraph(
+ pht(
+ 'This item does not support the specified command ("%s").',
+ $action))
+ ->addCancelButton($cancel_uri);
+ }
+
$command = NuanceItemCommand::initializeNewCommand()
->setItemPHID($item->getPHID())
->setAuthorPHID($viewer->getPHID())
@@ -64,17 +83,29 @@
$command->save();
- // TODO: Here, we should check if the command should be tried immediately,
- // and just defer it to the daemons if not. If we're going to try to apply
- // the command directly, we should first acquire the worker lock. If we
- // can not, we should defer the command even if it's an immediate command.
- // For the moment, skip all this stuff by deferring unconditionally.
+ // If this command can be applied immediately, try to apply it now.
- $should_defer = true;
- if ($should_defer) {
+ // In most cases, local commands (like closing an item) can be applied
+ // immediately.
+
+ // Commands that require making a call to a remote system (for example,
+ // to reply to a tweet or close a remote object) are usually done in the
+ // background so the user doesn't have to wait for the operation to
+ // complete before they can continue work.
+
+ $did_apply = false;
+ $immediate = $executor->canApplyImmediately($item, $command);
+ if ($immediate) {
+ // TODO: Move this stuff to a new Engine, and have the controller and
+ // worker both call into the Engine.
+ $worker = new NuanceItemUpdateWorker(array());
+ $did_apply = $worker->executeCommands($item, array($command));
+ }
+
+ // If this can't be applied immediately or we were unable to get a lock
+ // fast enough, do the update in the background instead.
+ if (!$did_apply) {
$item->scheduleUpdate();
- } else {
- // ...
}
if ($queue) {
diff --git a/src/applications/nuance/worker/NuanceItemUpdateWorker.php b/src/applications/nuance/worker/NuanceItemUpdateWorker.php
--- a/src/applications/nuance/worker/NuanceItemUpdateWorker.php
+++ b/src/applications/nuance/worker/NuanceItemUpdateWorker.php
@@ -6,9 +6,7 @@
protected function doWork() {
$item_phid = $this->getTaskDataValue('itemPHID');
- $hash = PhabricatorHash::digestForIndex($item_phid);
- $lock_key = "nuance.item.{$hash}";
- $lock = PhabricatorGlobalLock::newLock($lock_key);
+ $lock = $this->newLock($item_phid);
$lock->lock(1);
try {
@@ -55,9 +53,6 @@
private function applyCommands(NuanceItem $item) {
$viewer = $this->getViewer();
- $impl = $item->getImplementation();
- $impl->setViewer($viewer);
-
$commands = id(new NuanceItemCommandQuery())
->setViewer($viewer)
->withItemPHIDs(array($item->getPHID()))
@@ -68,8 +63,60 @@
->execute();
$commands = msort($commands, 'getID');
+ $this->executeCommandList($item, $commands);
+ }
+
+ public function executeCommands(NuanceItem $item, array $commands) {
+ if (!$commands) {
+ return true;
+ }
+
+ $item_phid = $item->getPHID();
+ $viewer = $this->getViewer();
+
+ $lock = $this->newLock($item_phid);
+ try {
+ $lock->lock(1);
+ } catch (PhutilLockException $ex) {
+ return false;
+ }
+
+ try {
+ $item = $this->loadItem($item_phid);
+
+ // Reload commands now that we have a lock, to make sure we don't
+ // execute any commands twice by mistake.
+ $commands = id(new NuanceItemCommandQuery())
+ ->setViewer($viewer)
+ ->withIDs(mpull($commands, 'getID'))
+ ->execute();
+
+ $this->executeCommandList($item, $commands);
+ } catch (Exception $ex) {
+ $lock->unlock();
+ throw $ex;
+ }
+
+ $lock->unlock();
+
+ return true;
+ }
+
+ private function executeCommandList(NuanceItem $item, array $commands) {
+ $viewer = $this->getViewer();
+
$executors = NuanceCommandImplementation::getAllCommands();
foreach ($commands as $command) {
+ if ($command->getItemPHID() !== $item->getPHID()) {
+ throw new Exception(
+ pht('Trying to apply a command to the wrong item!'));
+ }
+
+ if ($command->getStatus() !== NuanceItemCommand::STATUS_ISSUED) {
+ // Never execute commands which have already been issued.
+ continue;
+ }
+
$command
->setStatus(NuanceItemCommand::STATUS_EXECUTING)
->save();
@@ -105,4 +152,10 @@
}
}
+ private function newLock($item_phid) {
+ $hash = PhabricatorHash::digestForIndex($item_phid);
+ $lock_key = "nuance.item.{$hash}";
+ return PhabricatorGlobalLock::newLock($lock_key);
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 23, 2:57 AM (10 h, 54 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6776905
Default Alt Text
D18015.diff (6 KB)

Event Timeline