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); + } + }