Allow Nuance commands to try to apply immediately

Summary:
Ref T12738. By default, we process Nuance commands in the background. The intent is to let the user continue working at full speed if Twitter or GitHub (or whatever) is being a little slow.

Some commands don't do anything heavy and can be processed in the foreground. Let commands choose to try foreground execution.

Test Plan: Threw complaints in the trash, saw them immediately go into the trash.

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Maniphest Tasks: T12738

Differential Revision: https://secure.phabricator.com/D18015

src/applications/nuance/command/NuanceCommandImplementation.php

@@ -19,6 +19,12 @@ abstract class NuanceCommandImplementation
   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);

src/applications/nuance/command/NuanceTrashCommand.php

@@ -14,6 +14,12 @@ final class NuanceTrashCommand
     return ($type instanceof NuanceFormItemType);
   }
 
+  public function canApplyImmediately(
+    NuanceItem $item,
+    NuanceItemCommand $command) {
+    return true;
+  }
+
   protected function executeCommand(
     NuanceItem $item,
     NuanceItemCommand $command) {

src/applications/nuance/controller/NuanceItemActionController.php

@@ -53,6 +53,25 @@ final class NuanceItemActionController extends NuanceController {
     $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 @@ final class NuanceItemActionController extends NuanceController {
 
     $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) {

src/applications/nuance/worker/NuanceItemUpdateWorker.php

@@ -6,9 +6,7 @@ final class NuanceItemUpdateWorker
   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 @@ final class NuanceItemUpdateWorker
   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 @@ final class NuanceItemUpdateWorker
       ->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 @@ final class NuanceItemUpdateWorker
     }
   }
 
+  private function newLock($item_phid) {
+    $hash = PhabricatorHash::digestForIndex($item_phid);
+    $lock_key = "nuance.item.{$hash}";
+    return PhabricatorGlobalLock::newLock($lock_key);
+  }
+
 }