Formalize targets (users and channel) into objects

Summary:
Make users/channels/rooms into objects, so we can later sort out stuff like Campfire user IDs, Phabricator vs chat accounts, etc.

The only change here is that I removed output buffering from the macro handler. We should move throttling/buffering to adapters instead and have it apply globally.

Test Plan: Ran IRC and Campfire bots and interacted with them.

Reviewers: indiefan

Reviewed By: indiefan

CC: aran

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

src/__phutil_library_map__.php

@@ -707,6 +707,7 @@ phutil_register_library_map(array(
     'PhabricatorBaseEnglishTranslation' => 'infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php',
     'PhabricatorBaseProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php',
     'PhabricatorBot' => 'infrastructure/daemon/bot/PhabricatorBot.php',
+    'PhabricatorBotChannel' => 'infrastructure/daemon/bot/target/PhabricatorBotChannel.php',
     'PhabricatorBotDebugLogHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php',
     'PhabricatorBotDifferentialNotificationHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php',
     'PhabricatorBotFeedNotificationHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php',
@@ -716,6 +717,8 @@ phutil_register_library_map(array(
     'PhabricatorBotMessage' => 'infrastructure/daemon/bot/PhabricatorBotMessage.php',
     'PhabricatorBotObjectNameHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php',
     'PhabricatorBotSymbolHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php',
+    'PhabricatorBotTarget' => 'infrastructure/daemon/bot/target/PhabricatorBotTarget.php',
+    'PhabricatorBotUser' => 'infrastructure/daemon/bot/target/PhabricatorBotUser.php',
     'PhabricatorBotWhatsNewHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php',
     'PhabricatorBuiltinPatchList' => 'infrastructure/storage/patch/PhabricatorBuiltinPatchList.php',
     'PhabricatorButtonsExample' => 'applications/uiexample/examples/PhabricatorButtonsExample.php',
@@ -2163,6 +2166,7 @@ phutil_register_library_map(array(
     'PhabricatorBarePageView' => 'AphrontPageView',
     'PhabricatorBaseEnglishTranslation' => 'PhabricatorTranslation',
     'PhabricatorBot' => 'PhabricatorDaemon',
+    'PhabricatorBotChannel' => 'PhabricatorBotTarget',
     'PhabricatorBotDebugLogHandler' => 'PhabricatorBotHandler',
     'PhabricatorBotDifferentialNotificationHandler' => 'PhabricatorBotHandler',
     'PhabricatorBotFeedNotificationHandler' => 'PhabricatorBotHandler',
@@ -2170,6 +2174,7 @@ phutil_register_library_map(array(
     'PhabricatorBotMacroHandler' => 'PhabricatorBotHandler',
     'PhabricatorBotObjectNameHandler' => 'PhabricatorBotHandler',
     'PhabricatorBotSymbolHandler' => 'PhabricatorBotHandler',
+    'PhabricatorBotUser' => 'PhabricatorBotTarget',
     'PhabricatorBotWhatsNewHandler' => 'PhabricatorBotHandler',
     'PhabricatorBuiltinPatchList' => 'PhabricatorSQLPatchList',
     'PhabricatorButtonsExample' => 'PhabricatorUIExample',

src/infrastructure/daemon/bot/PhabricatorBotMessage.php

@@ -13,7 +13,7 @@ final class PhabricatorBotMessage {
     $this->public = true;
   }
 
-  public function setSender($sender) {
+  public function setSender(PhabricatorBotTarget $sender = null) {
     $this->sender = $sender;
     return $this;
   }
@@ -40,7 +40,7 @@ final class PhabricatorBotMessage {
     return $this->body;
   }
 
-  public function setTarget($target) {
+  public function setTarget(PhabricatorBotTarget $target = null) {
     $this->target = $target;
     return $this;
   }
@@ -49,20 +49,4 @@ final class PhabricatorBotMessage {
     return $this->target;
   }
 
-  public function isPublic() {
-    return $this->public;
-  }
-
-  public function setPublic($is_public) {
-    $this->public = $is_public;
-    return $this;
-  }
-
-  public function getReplyTo() {
-    if ($this->public) {
-      return $this->target;
-    } else {
-      return $this->sender;
-    }
-  }
 }

src/infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php

@@ -115,11 +115,34 @@ final class PhabricatorCampfireProtocolAdapter
       $buffer = substr($buffer, $until + 2);
 
       $m_obj = json_decode($message, true);
+      $command = null;
+      switch ($m_obj['type']) {
+        case 'TextMessage':
+          $command = 'MESSAGE';
+          break;
+        case 'PasteMessage':
+          $command = 'PASTE';
+          break;
+        default:
+          // For now, ignore anything which we don't otherwise know about.
+          break;
+      }
+
+      if ($command === null) {
+        continue;
+      }
+
+      // TODO: These should be usernames, not user IDs.
+      $sender = id(new PhabricatorBotUser())
+        ->setName($m_obj['user_id']);
+
+      $target = id(new PhabricatorBotChannel())
+        ->setName($m_obj['room_id']);
 
       return id(new PhabricatorBotMessage())
-        ->setCommand('MESSAGE')
+        ->setCommand($command)
-        ->setSender($m_obj['user_id'])
+        ->setSender($sender)
-        ->setTarget($m_obj['room_id'])
+        ->setTarget($target)
         ->setBody($m_obj['body']);
     }
 
@@ -159,7 +182,13 @@ final class PhabricatorCampfireProtocolAdapter
     unset($this->inRooms[$room_id]);
   }
 
-  private function speak($message, $room_id, $type='TextMessage') {
+  private function speak(
+    $message,
+    PhabricatorBotTarget $channel,
+    $type = 'TextMessage') {
+
+    $room_id = $channel->getName();
+
     $this->performPost(
       "/room/{$room_id}/speak.json",
       array(

src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php

@@ -128,7 +128,7 @@ final class PhabricatorIRCProtocolAdapter
     switch ($message->getCommand()) {
     case 'MESSAGE':
       $data = $irc_command.' '.
-        $message->getTarget().' :'.
+        $message->getTarget()->getName().' :'.
         $message->getBody()."\r\n";
       break;
     default:
@@ -164,16 +164,19 @@ final class PhabricatorIRCProtocolAdapter
     $command = $this->getBotCommand($matches['command']);
     list($target, $body) = $this->parseMessageData($command, $matches['data']);
 
+    if (!strlen($matches['sender'])) {
+      $sender = null;
+    } else {
+      $sender = id(new PhabricatorBotUser())
+       ->setName($matches['sender']);
+    }
+
     $bot_message = id(new PhabricatorBotMessage())
-      ->setSender(idx($matches, 'sender'))
+      ->setSender($sender)
       ->setCommand($command)
       ->setTarget($target)
       ->setBody($body);
 
-    if (!empty($target) && strncmp($target, '#', 1) !== 0) {
-      $bot_message->setPublic(false);
-    }
-
     return $bot_message;
   }
 
@@ -201,8 +204,18 @@ final class PhabricatorIRCProtocolAdapter
     case 'MESSAGE':
       $matches = null;
       if (preg_match('/^(\S+)\s+:?(.*)$/', $data, $matches)) {
+
+        $target_name = $matches[1];
+        if (strncmp($target_name, '#', 1) === 0) {
+          $target = id(new PhabricatorBotChannel())
+            ->setName($target_name);
+        } else {
+          $target = id(new PhabricatorBotUser())
+            ->setName($target_name);
+        }
+
         return array(
-          $matches[1],
+          $target,
           rtrim($matches[2], "\r\n"));
       }
       break;

src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php

@@ -48,15 +48,14 @@ abstract class PhabricatorBotHandler {
     $reply = id(new PhabricatorBotMessage())
       ->setCommand('MESSAGE');
 
-    if ($original_message->isPublic()) {
+    if ($original_message->getTarget()->isPublic()) {
       // This is a public target, like a chatroom. Send the response to the
       // chatroom.
       $reply->setTarget($original_message->getTarget());
     } else {
       // This is a private target, like a private message. Send the response
       // back to the sender (presumably, we are the target).
-      $reply->setTarget($original_message->getSender())
+      $reply->setTarget($original_message->getSender());
-        ->setPublic(false);
     }
 
     $reply->setBody($body);

src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php

@@ -13,21 +13,20 @@ final class PhabricatorBotLogHandler extends PhabricatorBotHandler {
 
     switch ($message->getCommand()) {
       case 'MESSAGE':
-        $reply_to = $message->getReplyTo();
+        $target = $message->getTarget();
-        if (!$reply_to) {
+        if (!$target->isPublic()) {
-          break;
-        }
-        if (!$message->isPublic()) {
           // Don't log private messages, although maybe we should for debugging?
           break;
         }
 
+        $target_name = $target->getName();
+
         $logs = array(
           array(
-            'channel' => $reply_to,
+            'channel' => $target_name,
             'type'    => 'mesg',
             'epoch'   => time(),
-            'author'  => $message->getSender(),
+            'author'  => $message->getSender()->getName(),
             'message' => $message->getBody(),
           ),
         );
@@ -54,7 +53,7 @@ final class PhabricatorBotLogHandler extends PhabricatorBotHandler {
 
         if ($tell) {
           $response = $this->getURI(
-            '/chatlog/channel/'.phutil_escape_uri($reply_to).'/');
+            '/chatlog/channel/'.phutil_escape_uri($target_name).'/');
 
           $this->replyTo($message, $response);
         }

src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php

@@ -8,7 +8,6 @@ final class PhabricatorBotMacroHandler extends PhabricatorBotHandler {
   private $macros;
   private $regexp;
 
-  private $buffer = array();
   private $next   = 0;
 
   private function init() {
@@ -47,11 +46,6 @@ final class PhabricatorBotMacroHandler extends PhabricatorBotHandler {
 
     switch ($message->getCommand()) {
       case 'MESSAGE':
-      $reply_to = $message->getReplyTo();
-      if (!$reply_to) {
-        break;
-      }
-
         $message_body = $message->getBody();
 
         $matches = null;
@@ -74,38 +68,14 @@ final class PhabricatorBotMacroHandler extends PhabricatorBotHandler {
           $ascii = $this->macros[$macro]['ascii'];
         }
 
+        $target_name = $message->getTarget()->getName();
         foreach ($ascii as $line) {
-        $this->buffer[$reply_to][] = $line;
+          $this->replyTo($message, $line);
         }
         break;
     }
   }
 
-  public function runBackgroundTasks() {
-    if (microtime(true) < $this->next) {
-      return;
-    }
-
-    foreach ($this->buffer as $channel => $lines) {
-      if (empty($lines)) {
-        unset($this->buffer[$channel]);
-        continue;
-      }
-      foreach ($lines as $key => $line) {
-        $this->writeMessage(
-          id(new PhabricatorBotMessage())
-          ->setCommand('MESSAGE')
-          ->setTarget($channel)
-          ->setBody($line));
-        unset($this->buffer[$channel][$key]);
-        break 2;
-      }
-    }
-
-    $sleep = $this->getConfig('macro.sleep', 0.25);
-    $this->next = microtime(true) + ((mt_rand(75, 150) / 100) * $sleep);
-  }
-
   public function rasterize($macro, $size, $aspect) {
     $image = HTTPSFuture::loadContent($macro['uri']);
     if (!$image) {

src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php

@@ -170,13 +170,13 @@ final class PhabricatorBotObjectNameHandler extends PhabricatorBotHandler {
         // in public channels, so we avoid spamming the chat over and over
         // again for discsussions of a specific revision, for example.
 
-        $reply_to = $original_message->getReplyTo();
+        $target_name = $original_message->getTarget()->getName();
-        if (empty($this->recentlyMentioned[$reply_to])) {
+        if (empty($this->recentlyMentioned[$target_name])) {
-          $this->recentlyMentioned[$reply_to] = array();
+          $this->recentlyMentioned[$target_name] = array();
         }
 
         $quiet_until = idx(
-          $this->recentlyMentioned[$reply_to],
+          $this->recentlyMentioned[$target_name],
           $phid,
           0) + (60 * 10);
 
@@ -185,7 +185,7 @@ final class PhabricatorBotObjectNameHandler extends PhabricatorBotHandler {
           continue;
         }
 
-        $this->recentlyMentioned[$reply_to][$phid] = time();
+        $this->recentlyMentioned[$target_name][$phid] = time();
         $this->replyTo($original_message, $description);
       }
       break;

src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php

@@ -13,11 +13,6 @@ final class PhabricatorBotWhatsNewHandler extends PhabricatorBotHandler {
 
     switch ($message->getCommand()) {
       case 'MESSAGE':
-        $reply_to = $message->getReplyTo();
-        if (!$reply_to) {
-          break;
-        }
-
         $message_body = $message->getBody();
 
         $prompt = '~what( i|\')?s new\?~i';
@@ -27,13 +22,13 @@ final class PhabricatorBotWhatsNewHandler extends PhabricatorBotHandler {
           }
           $this->floodblock = time() + 60;
 
-          $this->getLatest($reply_to);
+          $this->getLatest($message);
         }
         break;
     }
   }
 
-  public function getLatest($reply_to) {
+  public function getLatest(PhabricatorBotMessage $message) {
     $latest = $this->getConduit()->callMethodSynchronous(
       'feed.query',
       array(
@@ -112,11 +107,7 @@ final class PhabricatorBotWhatsNewHandler extends PhabricatorBotHandler {
       // $content = "{$bold}{$user}{$reset} {$gray}{$action} {$blue}{$bold}".
         // "{$title}{$reset} - {$gray}{$uri}{$reset}";
       $content = "{$user} {$action} {$title} - {$uri}";
-      $this->writeMessage(
+      $this->replyTo($message, $content);
-        id(new PhabricatorBotMessage())
-        ->setCommand('MESSAGE')
-        ->setTarget($reply_to)
-        ->setBody($content));
     }
     return;
   }

src/infrastructure/daemon/bot/target/PhabricatorBotChannel.php

@@ -0,0 +1,12 @@
+<?php
+
+/**
+ * Represents a group/public space, like an IRC channel or a Campfire room.
+ */
+final class PhabricatorBotChannel extends PhabricatorBotTarget {
+
+  public function isPublic() {
+    return true;
+  }
+
+}

src/infrastructure/daemon/bot/target/PhabricatorBotTarget.php

@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * Represents something which can be the target of messages, like a user or
+ * channel.
+ */
+abstract class PhabricatorBotTarget {
+
+  private $name;
+
+  public function setName($name) {
+    $this->name = $name;
+    return $this;
+  }
+
+  public function getName() {
+    return $this->name;
+  }
+
+  abstract public function isPublic();
+
+}

src/infrastructure/daemon/bot/target/PhabricatorBotUser.php

@@ -0,0 +1,12 @@
+<?php
+
+/**
+ * Represents an individual user.
+ */
+final class PhabricatorBotUser extends PhabricatorBotTarget {
+
+  public function isPublic() {
+    return false;
+  }
+
+}