diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'ce06b6f6', - 'core.pkg.js' => '08b41036', + 'core.pkg.js' => '172ae180', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '7ba78475', 'differential.pkg.js' => 'd0cd0df6', @@ -232,7 +232,7 @@ 'rsrc/externals/javelin/lib/DOM.js' => '805b806a', 'rsrc/externals/javelin/lib/History.js' => 'd4505101', 'rsrc/externals/javelin/lib/JSON.js' => '69adf288', - 'rsrc/externals/javelin/lib/Leader.js' => '331b1611', + 'rsrc/externals/javelin/lib/Leader.js' => 'f2fc8712', 'rsrc/externals/javelin/lib/Mask.js' => '8a41885b', 'rsrc/externals/javelin/lib/Quicksand.js' => '6b8ef10b', 'rsrc/externals/javelin/lib/Request.js' => '94b750d2', @@ -700,7 +700,7 @@ 'javelin-history' => 'd4505101', 'javelin-install' => '05270951', 'javelin-json' => '69adf288', - 'javelin-leader' => '331b1611', + 'javelin-leader' => 'f2fc8712', 'javelin-magical-init' => '3010e992', 'javelin-mask' => '8a41885b', 'javelin-quicksand' => '6b8ef10b', @@ -1108,9 +1108,6 @@ 'javelin-dom', 'javelin-workflow', ), - '331b1611' => array( - 'javelin-install', - ), '340c8eff' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2073,6 +2070,9 @@ 'javelin-workflow', 'javelin-json', ), + 'f2fc8712' => array( + 'javelin-install', + ), 'f411b6ae' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2039,6 +2039,7 @@ 'PhabricatorConfigApplication' => 'applications/config/application/PhabricatorConfigApplication.php', 'PhabricatorConfigCacheController' => 'applications/config/controller/PhabricatorConfigCacheController.php', 'PhabricatorConfigClusterDatabasesController' => 'applications/config/controller/PhabricatorConfigClusterDatabasesController.php', + 'PhabricatorConfigClusterNotificationsController' => 'applications/config/controller/PhabricatorConfigClusterNotificationsController.php', 'PhabricatorConfigCollectorsModule' => 'applications/config/module/PhabricatorConfigCollectorsModule.php', 'PhabricatorConfigColumnSchema' => 'applications/config/schema/PhabricatorConfigColumnSchema.php', 'PhabricatorConfigConfigPHIDType' => 'applications/config/phid/PhabricatorConfigConfigPHIDType.php', @@ -2710,7 +2711,8 @@ 'PhabricatorNotificationPanelController' => 'applications/notification/controller/PhabricatorNotificationPanelController.php', 'PhabricatorNotificationQuery' => 'applications/notification/query/PhabricatorNotificationQuery.php', 'PhabricatorNotificationSearchEngine' => 'applications/notification/query/PhabricatorNotificationSearchEngine.php', - 'PhabricatorNotificationStatusController' => 'applications/notification/controller/PhabricatorNotificationStatusController.php', + 'PhabricatorNotificationServerRef' => 'applications/notification/client/PhabricatorNotificationServerRef.php', + 'PhabricatorNotificationServersConfigOptionType' => 'applications/notification/config/PhabricatorNotificationServersConfigOptionType.php', 'PhabricatorNotificationStatusView' => 'applications/notification/view/PhabricatorNotificationStatusView.php', 'PhabricatorNotificationTestController' => 'applications/notification/controller/PhabricatorNotificationTestController.php', 'PhabricatorNotificationTestFeedStory' => 'applications/notification/feed/PhabricatorNotificationTestFeedStory.php', @@ -6465,6 +6467,7 @@ 'PhabricatorConfigApplication' => 'PhabricatorApplication', 'PhabricatorConfigCacheController' => 'PhabricatorConfigController', 'PhabricatorConfigClusterDatabasesController' => 'PhabricatorConfigController', + 'PhabricatorConfigClusterNotificationsController' => 'PhabricatorConfigController', 'PhabricatorConfigCollectorsModule' => 'PhabricatorConfigModule', 'PhabricatorConfigColumnSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigConfigPHIDType' => 'PhabricatorPHIDType', @@ -7225,7 +7228,8 @@ 'PhabricatorNotificationPanelController' => 'PhabricatorNotificationController', 'PhabricatorNotificationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorNotificationSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'PhabricatorNotificationStatusController' => 'PhabricatorNotificationController', + 'PhabricatorNotificationServerRef' => 'Phobject', + 'PhabricatorNotificationServersConfigOptionType' => 'PhabricatorConfigJSONOptionType', 'PhabricatorNotificationStatusView' => 'AphrontTagView', 'PhabricatorNotificationTestController' => 'PhabricatorNotificationController', 'PhabricatorNotificationTestFeedStory' => 'PhabricatorFeedStory', diff --git a/src/applications/config/application/PhabricatorConfigApplication.php b/src/applications/config/application/PhabricatorConfigApplication.php --- a/src/applications/config/application/PhabricatorConfigApplication.php +++ b/src/applications/config/application/PhabricatorConfigApplication.php @@ -64,6 +64,7 @@ ), 'cluster/' => array( 'databases/' => 'PhabricatorConfigClusterDatabasesController', + 'notifications/' => 'PhabricatorConfigClusterNotificationsController', ), ), ); diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -307,6 +307,9 @@ 'notification.ssl-key' => $aphlict_reason, 'notification.pidfile' => $aphlict_reason, 'notification.log' => $aphlict_reason, + 'notification.enabled' => $aphlict_reason, + 'notification.client-uri' => $aphlict_reason, + 'notification.server-uri' => $aphlict_reason, ); return $ancient_config; diff --git a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php b/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php --- a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php +++ b/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php @@ -7,11 +7,11 @@ $nav = $this->buildSideNavView(); $nav->selectFilter('cluster/databases/'); - $title = pht('Cluster Databases'); + $title = pht('Database Servers'); $crumbs = $this ->buildApplicationCrumbs($nav) - ->addTextCrumb(pht('Cluster Databases')); + ->addTextCrumb(pht('Database Servers')); $database_status = $this->buildClusterDatabaseStatus(); diff --git a/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php b/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php new file mode 100644 --- /dev/null +++ b/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php @@ -0,0 +1,163 @@ +buildSideNavView(); + $nav->selectFilter('cluster/notifications/'); + + $title = pht('Cluster Notifications'); + + $crumbs = $this + ->buildApplicationCrumbs($nav) + ->addTextCrumb(pht('Cluster Notifications')); + + $notification_status = $this->buildClusterNotificationStatus(); + + $view = id(new PHUITwoColumnView()) + ->setNavigation($nav) + ->setMainColumn($notification_status); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + private function buildClusterNotificationStatus() { + $viewer = $this->getViewer(); + + $servers = PhabricatorNotificationServerRef::newRefs(); + Javelin::initBehavior('phabricator-tooltips'); + + $rows = array(); + foreach ($servers as $server) { + if ($server->isAdminServer()) { + $type_icon = 'fa-database sky'; + $type_tip = pht('Admin Server'); + } else { + $type_icon = 'fa-bell sky'; + $type_tip = pht('Client Server'); + } + + $type_icon = id(new PHUIIconView()) + ->setIcon($type_icon) + ->addSigil('has-tooltip') + ->setMetadata( + array( + 'tip' => $type_tip, + )); + + $messages = array(); + + $details = array(); + if ($server->isAdminServer()) { + try { + $details = $server->loadServerStatus(); + $status_icon = 'fa-exchange green'; + $status_label = pht('Version %s', idx($details, 'version')); + } catch (Exception $ex) { + $status_icon = 'fa-times red'; + $status_label = pht('Connection Error'); + $messages[] = $ex->getMessage(); + } + } else { + try { + $server->testClient(); + $status_icon = 'fa-exchange green'; + $status_label = pht('Connected'); + } catch (Exception $ex) { + $status_icon = 'fa-times red'; + $status_label = pht('Connection Error'); + $messages[] = $ex->getMessage(); + } + } + + if ($details) { + $uptime = idx($details, 'uptime'); + $uptime = $uptime / 1000; + $uptime = phutil_format_relative_time_detailed($uptime); + + $clients = pht( + '%s Active / %s Total', + new PhutilNumber(idx($details, 'clients.active')), + new PhutilNumber(idx($details, 'clients.total'))); + + $stats = pht( + '%s In / %s Out', + new PhutilNumber(idx($details, 'messages.in')), + new PhutilNumber(idx($details, 'messages.out'))); + + } else { + $uptime = null; + $clients = null; + $stats = null; + } + + $status_view = array( + id(new PHUIIconView())->setIcon($status_icon), + ' ', + $status_label, + ); + + $messages = phutil_implode_html(phutil_tag('br'), $messages); + + $rows[] = array( + $type_icon, + $server->getProtocol(), + $server->getHost(), + $server->getPort(), + $status_view, + $uptime, + $clients, + $stats, + $messages, + ); + } + + $table = id(new AphrontTableView($rows)) + ->setNoDataString( + pht('No notification servers are configured.')) + ->setHeaders( + array( + null, + pht('Proto'), + pht('Host'), + pht('Port'), + pht('Status'), + pht('Uptime'), + pht('Clients'), + pht('Messages'), + null, + )) + ->setColumnClasses( + array( + null, + null, + null, + null, + null, + null, + null, + null, + 'wide', + )); + + $doc_href = PhabricatorEnv::getDoclink('Cluster: Notifications'); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Cluster Notification Status')) + ->addActionLink( + id(new PHUIButtonView()) + ->setIcon('fa-book') + ->setHref($doc_href) + ->setTag('a') + ->setText(pht('Documentation'))); + + return id(new PHUIObjectBoxView()) + ->setHeader($header) + ->setTable($table); + } + +} diff --git a/src/applications/config/controller/PhabricatorConfigController.php b/src/applications/config/controller/PhabricatorConfigController.php --- a/src/applications/config/controller/PhabricatorConfigController.php +++ b/src/applications/config/controller/PhabricatorConfigController.php @@ -23,7 +23,8 @@ $nav->addLabel(pht('Cache')); $nav->addFilter('cache/', pht('Cache Status')); $nav->addLabel(pht('Cluster')); - $nav->addFilter('cluster/databases/', pht('Cluster Databases')); + $nav->addFilter('cluster/databases/', pht('Database Servers')); + $nav->addFilter('cluster/notifications/', pht('Notification Servers')); $nav->addLabel(pht('Welcome')); $nav->addFilter('welcome/', pht('Welcome Screen')); $nav->addLabel(pht('Modules')); diff --git a/src/applications/config/option/PhabricatorNotificationConfigOptions.php b/src/applications/config/option/PhabricatorNotificationConfigOptions.php --- a/src/applications/config/option/PhabricatorNotificationConfigOptions.php +++ b/src/applications/config/option/PhabricatorNotificationConfigOptions.php @@ -20,30 +20,43 @@ } public function getOptions() { + $servers_type = 'custom:PhabricatorNotificationServersConfigOptionType'; + $servers_help = $this->deformat(pht(<< 'client', + 'host' => 'phabricator.mycompany.com', + 'port' => 22280, + 'protocol' => 'https', + ), + array( + 'type' => 'admin', + 'host' => '127.0.0.1', + 'port' => 22281, + 'protocol' => 'http', + ), + ); + + $servers_example1 = id(new PhutilJSON())->encodeAsList( + $servers_example1); + return array( - $this->newOption('notification.enabled', 'bool', false) - ->setBoolOptions( - array( - pht('Enable Real-Time Notifications'), - pht('Disable Real-Time Notifications'), - )) - ->setSummary(pht('Enable real-time notifications.')) - ->setDescription( - pht( - "Enable real-time notifications. You must also run a Node.js ". - "based notification server for this to work. Consult the ". - "documentation in 'Notifications User Guide: Setup and ". - "Configuration' for instructions.")), - $this->newOption( - 'notification.client-uri', - 'string', - 'http://localhost:22280/') - ->setDescription(pht('Location of the client server.')), - $this->newOption( - 'notification.server-uri', - 'string', - 'http://localhost:22281/') - ->setDescription(pht('Location of the notification receiver server.')), + $this->newOption('notification.servers', $servers_type, array()) + ->setSummary(pht('Configure real-time notifications.')) + ->setDescription($servers_help) + ->addExample( + $servers_example1, + pht('Simple Example')), ); } diff --git a/src/applications/notification/application/PhabricatorNotificationsApplication.php b/src/applications/notification/application/PhabricatorNotificationsApplication.php --- a/src/applications/notification/application/PhabricatorNotificationsApplication.php +++ b/src/applications/notification/application/PhabricatorNotificationsApplication.php @@ -25,7 +25,6 @@ => 'PhabricatorNotificationListController', 'panel/' => 'PhabricatorNotificationPanelController', 'individual/' => 'PhabricatorNotificationIndividualController', - 'status/' => 'PhabricatorNotificationStatusController', 'clear/' => 'PhabricatorNotificationClearController', 'test/' => 'PhabricatorNotificationTestController', ), diff --git a/src/applications/notification/client/PhabricatorNotificationClient.php b/src/applications/notification/client/PhabricatorNotificationClient.php --- a/src/applications/notification/client/PhabricatorNotificationClient.php +++ b/src/applications/notification/client/PhabricatorNotificationClient.php @@ -2,62 +2,31 @@ final class PhabricatorNotificationClient extends Phobject { - const EXPECT_VERSION = 7; + public static function tryAnyConnection() { + $servers = PhabricatorNotificationServerRef::getEnabledAdminServers(); - public static function getServerStatus() { - $uri = PhabricatorEnv::getEnvConfig('notification.server-uri'); - $uri = id(new PhutilURI($uri)) - ->setPath('/status/') - ->setQueryParam('instance', self::getInstance()); - - // We always use HTTP to connect to the server itself: it's simpler and - // there's no meaningful security benefit to securing this link today. - // Force the protocol to HTTP in case users have set it to something else. - $uri->setProtocol('http'); - - list($body) = id(new HTTPSFuture($uri)) - ->setTimeout(3) - ->resolvex(); - - $status = phutil_json_decode($body); - if (!is_array($status)) { - throw new Exception( - pht( - 'Expected JSON response from notification server, received: %s', - $body)); - } - - return $status; - } - - public static function tryToPostMessage(array $data) { - if (!PhabricatorEnv::getEnvConfig('notification.enabled')) { + if (!$servers) { return; } - try { - self::postMessage($data); - } catch (Exception $ex) { - // Just ignore any issues here. - phlog($ex); + foreach ($servers as $server) { + $server->loadServerStatus(); + return; } - } - private static function postMessage(array $data) { - $server_uri = PhabricatorEnv::getEnvConfig('notification.server-uri'); - $server_uri = id(new PhutilURI($server_uri)) - ->setPath('/') - ->setQueryParam('instance', self::getInstance()); - - id(new HTTPSFuture($server_uri, json_encode($data))) - ->setMethod('POST') - ->setTimeout(1) - ->resolvex(); + return; } - private static function getInstance() { - $client_uri = PhabricatorEnv::getEnvConfig('notification.client-uri'); - return id(new PhutilURI($client_uri))->getPath(); + public static function tryToPostMessage(array $data) { + $servers = PhabricatorNotificationServerRef::getEnabledAdminServers(); + foreach ($servers as $server) { + try { + $server->postMessage($data); + return; + } catch (Exception $ex) { + // Just ignore any issues here. + } + } } } diff --git a/src/applications/notification/client/PhabricatorNotificationServerRef.php b/src/applications/notification/client/PhabricatorNotificationServerRef.php new file mode 100644 --- /dev/null +++ b/src/applications/notification/client/PhabricatorNotificationServerRef.php @@ -0,0 +1,234 @@ +type = $type; + return $this; + } + + public function getType() { + return $this->type; + } + + public function setHost($host) { + $this->host = $host; + return $this; + } + + public function getHost() { + return $this->host; + } + + public function setPort($port) { + $this->port = $port; + return $this; + } + + public function getPort() { + return $this->port; + } + + public function setProtocol($protocol) { + $this->protocol = $protocol; + return $this; + } + + public function getProtocol() { + return $this->protocol; + } + + public function setPath($path) { + $this->path = $path; + return $this; + } + + public function getPath() { + return $this->path; + } + + public function setIsDisabled($is_disabled) { + $this->isDisabled = $is_disabled; + return $this; + } + + public function getIsDisabled() { + return $this->isDisabled; + } + + public static function getLiveServers() { + $cache = PhabricatorCaches::getRequestCache(); + + $refs = $cache->getKey(self::KEY_REFS); + if (!$refs) { + $refs = self::newRefs(); + $cache->setKey(self::KEY_REFS, $refs); + } + + return $refs; + } + + public static function newRefs() { + $configs = PhabricatorEnv::getEnvConfig('notification.servers'); + + $refs = array(); + foreach ($configs as $config) { + $ref = id(new self()) + ->setType($config['type']) + ->setHost($config['host']) + ->setPort($config['port']) + ->setProtocol($config['protocol']) + ->setPath(idx($config, 'path')) + ->setIsDisabled(idx($config, 'disabled', false)); + $refs[] = $ref; + } + + return $refs; + } + + public static function getEnabledServers() { + $servers = self::getLiveServers(); + + foreach ($servers as $key => $server) { + if ($server->getIsDisabled()) { + unset($servers[$key]); + } + } + + return array_values($servers); + } + + public static function getEnabledAdminServers() { + $servers = self::getEnabledServers(); + + foreach ($servers as $key => $server) { + if (!$server->isAdminServer()) { + unset($servers[$key]); + } + } + + return array_values($servers); + } + + public static function getEnabledClientServers($with_protocol) { + $servers = self::getEnabledServers(); + + foreach ($servers as $key => $server) { + if ($server->isAdminServer()) { + unset($servers[$key]); + continue; + } + + $protocol = $server->getProtocol(); + if ($protocol != $with_protocol) { + unset($servers[$key]); + continue; + } + } + + return array_values($servers); + } + + public function isAdminServer() { + return ($this->type == 'admin'); + } + + public function getURI($to_path = null) { + $full_path = rtrim($this->getPath(), '/').'/'.ltrim($to_path, '/'); + + $uri = id(new PhutilURI('http://'.$this->getHost())) + ->setProtocol($this->getProtocol()) + ->setPort($this->getPort()) + ->setPath($full_path); + + $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); + if (strlen($instance)) { + $uri->setQueryParam('instance', $instance); + } + + return $uri; + } + + public function getWebsocketURI($to_path = null) { + $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); + if (strlen($instance)) { + $to_path = $to_path.$instance.'/'; + } + + $uri = $this->getURI($to_path); + + if ($this->getProtocol() == 'https') { + $uri->setProtocol('wss'); + } else { + $uri->setProtocol('ws'); + } + + return $uri; + } + + public function testClient() { + if ($this->isAdminServer()) { + throw new Exception( + pht('Unable to test client on an admin server!')); + } + + $server_uri = $this->getURI(); + + try { + id(new HTTPSFuture($server_uri)) + ->setTimeout(2) + ->resolvex(); + } catch (HTTPFutureHTTPResponseStatus $ex) { + // This is what we expect when things are working correctly. + if ($ex->getStatusCode() == 501) { + return true; + } + throw $ex; + } + + throw new Exception( + pht('Got HTTP 200, but expected HTTP 501 (WebSocket Upgrade)!')); + } + + public function loadServerStatus() { + if (!$this->isAdminServer()) { + throw new Exception( + pht( + 'Unable to load server status: this is not an admin server!')); + } + + $server_uri = $this->getURI('/status/'); + + list($body) = id(new HTTPSFuture($server_uri)) + ->setTimeout(2) + ->resolvex(); + + return phutil_json_decode($body); + } + + public function postMessage(array $data) { + if (!$this->isAdminServer()) { + throw new Exception( + pht('Unable to post message: this is not an admin server!')); + } + + $server_uri = $this->getURI('/'); + $payload = phutil_json_encode($data); + + id(new HTTPSFuture($server_uri, $payload)) + ->setMethod('POST') + ->setTimeout(2) + ->resolvex(); + } + +} diff --git a/src/applications/notification/config/PhabricatorNotificationServersConfigOptionType.php b/src/applications/notification/config/PhabricatorNotificationServersConfigOptionType.php new file mode 100644 --- /dev/null +++ b/src/applications/notification/config/PhabricatorNotificationServersConfigOptionType.php @@ -0,0 +1,140 @@ + $spec) { + if (!is_array($spec)) { + throw new Exception( + pht( + 'Notification server configuration is not valid: each entry in '. + 'the list must be a dictionary describing a service, but '. + 'the value with index "%s" is not a dictionary.', + $index)); + } + } + + $has_admin = false; + $has_client = false; + $map = array(); + foreach ($value as $index => $spec) { + try { + PhutilTypeSpec::checkMap( + $spec, + array( + 'type' => 'string', + 'host' => 'string', + 'port' => 'int', + 'protocol' => 'string', + 'path' => 'optional string', + 'disabled' => 'optional bool', + )); + } catch (Exception $ex) { + throw new Exception( + pht( + 'Notification server configuration has an invalid service '. + 'specification (at index "%s"): %s.', + $index, + $ex->getMessage())); + } + + $type = $spec['type']; + $host = $spec['host']; + $port = $spec['port']; + $protocol = $spec['protocol']; + $disabled = idx($spec, 'disabled'); + + switch ($type) { + case 'admin': + if (!$disabled) { + $has_admin = true; + } + break; + case 'client': + if (!$disabled) { + $has_client = true; + } + break; + default: + throw new Exception( + pht( + 'Notification server configuration describes an invalid '. + 'host ("%s", at index "%s") with an unrecognized type ("%s"). '. + 'Valid types are "%s" or "%s".', + $host, + $index, + $type, + 'admin', + 'client')); + } + + switch ($protocol) { + case 'http': + case 'https': + break; + default: + throw new Exception( + pht( + 'Notification server configuration describes an invalid '. + 'host ("%s", at index "%s") with an invalid protocol ("%s"). '. + 'Valid protocols are "%s" or "%s".', + $host, + $index, + $protocol, + 'http', + 'https')); + } + + $path = idx($spec, 'path'); + if ($type == 'admin' && strlen($path)) { + throw new Exception( + pht( + 'Notification server configuration describes an invalid host '. + '("%s", at index "%s"). This is an "admin" service but it has a '. + '"path" property. This property is only valid for "client" '. + 'services.')); + } + + // We can't guarantee that you didn't just give the same host two + // different names in DNS, but this check can catch silly copy/paste + // mistakes. + $key = "{$host}:{$port}"; + if (isset($map[$key])) { + throw new Exception( + pht( + 'Notification server configuration is invalid: it describes the '. + 'same host and port ("%s") multiple times. Each host and port '. + 'combination should appear only once in the list.', + $key)); + } + $map[$key] = true; + } + + if ($value) { + if (!$has_admin) { + throw new Exception( + pht( + 'Notification server configuration is invalid: it does not '. + 'specify any enabled servers with type "admin". Notifications '. + 'require at least one active "admin" server.')); + } + + if (!$has_client) { + throw new Exception( + pht( + 'Notification server configuration is invalid: it does not '. + 'specify any enabled servers with type "client". Notifications '. + 'require at least one active "client" server.')); + } + } + } + +} diff --git a/src/applications/notification/controller/PhabricatorNotificationPanelController.php b/src/applications/notification/controller/PhabricatorNotificationPanelController.php --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -44,17 +44,8 @@ ), pht('Notifications')); - if (PhabricatorEnv::getEnvConfig('notification.enabled')) { - $connection_status = new PhabricatorNotificationStatusView(); - } else { - $connection_status = phutil_tag( - 'a', - array( - 'href' => PhabricatorEnv::getDoclink( - 'Notifications User Guide: Setup and Configuration'), - ), - pht('Notification Server not enabled.')); - } + $connection_status = new PhabricatorNotificationStatusView(); + $connection_ui = phutil_tag( 'div', array( diff --git a/src/applications/notification/controller/PhabricatorNotificationStatusController.php b/src/applications/notification/controller/PhabricatorNotificationStatusController.php deleted file mode 100644 --- a/src/applications/notification/controller/PhabricatorNotificationStatusController.php +++ /dev/null @@ -1,82 +0,0 @@ -renderServerStatus($status); - } catch (Exception $ex) { - $status = new PHUIInfoView(); - $status->setTitle(pht('Notification Server Issue')); - $status->appendChild(hsprintf( - '%s

'. - '%s %s', - pht( - 'Unable to determine server status. This probably means the server '. - 'is not in great shape. The specific issue encountered was:'), - get_class($ex), - phutil_escape_html_newlines($ex->getMessage()))); - } - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Status')); - - $title = pht('Notification Server Status'); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($status); - } - - private function renderServerStatus(array $status) { - - $rows = array(); - foreach ($status as $key => $value) { - switch ($key) { - case 'uptime': - $value /= 1000; - $value = phutil_format_relative_time_detailed($value); - break; - case 'log': - case 'instance': - break; - default: - $value = number_format($value); - break; - } - - $rows[] = array($key, $value); - } - - $table = new AphrontTableView($rows); - $table->setColumnClasses( - array( - 'header', - 'wide', - )); - - $test_icon = id(new PHUIIconView()) - ->setIcon('fa-exclamation-triangle'); - - $test_button = id(new PHUIButtonView()) - ->setTag('a') - ->setWorkflow(true) - ->setText(pht('Send Test Notification')) - ->setHref($this->getApplicationURI('test/')) - ->setIcon($test_icon); - - $header = id(new PHUIHeaderView()) - ->setHeader(pht('Notification Server Status')) - ->addActionLink($test_button); - - $box = id(new PHUIObjectBoxView()) - ->setHeader($header) - ->appendChild($table); - - return $box; - } -} diff --git a/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php b/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php --- a/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php +++ b/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php @@ -3,14 +3,8 @@ final class PhabricatorAphlictSetupCheck extends PhabricatorSetupCheck { protected function executeChecks() { - $enabled = PhabricatorEnv::getEnvConfig('notification.enabled'); - if (!$enabled) { - // Notifications aren't set up, so just ignore all of these checks. - return; - } - try { - $status = PhabricatorNotificationClient::getServerStatus(); + PhabricatorNotificationClient::tryAnyConnection(); } catch (Exception $ex) { $message = pht( "Phabricator is configured to use a notification server, but is ". @@ -31,8 +25,7 @@ ->setShortName(pht('Notification Server Down')) ->setName(pht('Unable to Connect to Notification Server')) ->setMessage($message) - ->addRelatedPhabricatorConfig('notification.enabled') - ->addRelatedPhabricatorConfig('notification.server-uri') + ->addRelatedPhabricatorConfig('notification.servers') ->addCommand( pht( "(To start the server, run this command.)\n%s", @@ -40,23 +33,5 @@ return; } - - $expect_version = PhabricatorNotificationClient::EXPECT_VERSION; - $have_version = idx($status, 'version', 1); - if ($have_version != $expect_version) { - $message = pht( - 'The notification server is out of date. You are running server '. - 'version %d, but Phabricator expects version %d. Restart the server '. - 'to update it, using the command below:', - $have_version, - $expect_version); - - $this->newIssue('aphlict.version') - ->setShortName(pht('Notification Server Version')) - ->setName(pht('Notification Server Out of Date')) - ->setMessage($message) - ->addCommand('phabricator/ $ ./bin/aphlict restart'); - } - } } diff --git a/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php b/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php --- a/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php @@ -4,9 +4,13 @@ extends PhabricatorSettingsPanel { public function isEnabled() { - return PhabricatorEnv::getEnvConfig('notification.enabled') && - PhabricatorApplication::isClassInstalled( - 'PhabricatorNotificationsApplication'); + $servers = PhabricatorNotificationServerRef::getEnabledAdminServers(); + if (!$servers) { + return false; + } + + return PhabricatorApplication::isClassInstalled( + 'PhabricatorNotificationsApplication'); } public function getPanelKey() { diff --git a/src/docs/user/cluster/cluster_databases.diviner b/src/docs/user/cluster/cluster_databases.diviner --- a/src/docs/user/cluster/cluster_databases.diviner +++ b/src/docs/user/cluster/cluster_databases.diviner @@ -69,7 +69,7 @@ Monitoring Replicas =================== -You can monitor replicas in {nav Config > Cluster Databases}. This interface +You can monitor replicas in {nav Config > Database Servers}. This interface shows you a quick overview of replicas and their health, and can detect some common issues with replication. @@ -146,7 +146,7 @@ Once satisfied, turn the master back on. After a brief delay, Phabricator should recognize that the master is healthy again and recover fully. -Throughout this process, the {nav Cluster Databases} console will show a +Throughout this process, the {nav Database Servers} console will show a current view of the world from the perspective of the web server handling the request. You can use it to monitor state. @@ -249,7 +249,7 @@ This improves performance during a service interruption and reduces load on the master, which may help it recover from load problems. -You can monitor the status of health checks in the {nav Cluster Databases} +You can monitor the status of health checks in the {nav Database Servers} console. The "Health" column shows how many checks have run recently and how many have succeeded. diff --git a/src/docs/user/configuration/notifications.diviner b/src/docs/user/configuration/notifications.diviner --- a/src/docs/user/configuration/notifications.diviner +++ b/src/docs/user/configuration/notifications.diviner @@ -11,13 +11,13 @@ notifications. Phabricator can also be configured to deliver notifications in real time, by -popping up a message in any open browser windows if something has -happened or an object has been updated. +popping up a message in any open browser windows if something has happened or +an object has been updated. To enable real-time notifications: - - Set `notification.enabled` in your configuration to true. - - Run the notification server, as described below. + - Configure and start the notification server, as described below. + - Adjust `notification.servers` to point at it. This document describes the process in detail. @@ -104,23 +104,42 @@ Configuring Phabricator ======================= -You may also want to adjust these settings: +After starting the server, configure Phabricator to connect to it by adjusting +`notification.servers`. This configuration option should have a list of servers +that Phabricator should interact with. + +Normally, you'll list one client server and one admin server, like this: + +```lang=json +[ + { + "type": "client", + "host": "phabricator.mycompany.com", + "port": 22280, + "protocol": "https" + }, + { + "type": "admin", + "host": "127.0.0.1", + "port": 22281, + "protocol": "http" + } +] +``` - - `notification.client-uri` Externally-facing host and port that browsers will - connect to in order to listen for notifications. - - `notification.server-uri` Internally-facing host and port that Phabricator - will connect to in order to publish notifications. +This definition defines which services the user's browser will attempt to +connect to. Most of the time, it will be very similar to the services defined +in the Aphlict configuration. However, if you are sending traffic through a +load balancer or terminating SSL somewhere before traffic reaches Aphlict, +the services the browser connects to may need to have different hosts, ports +or protocols than the underlying server listens on. Verifying Server Status ======================= -Access `/notification/status/` to verify the server is operational. You should -see a table showing stats like "uptime" and connection/message counts if the -server is working. If it isn't working, you should see an error. - -You can also send a test notification by clicking the button in the upper right -corner of this screen. +After configuring `notification.servers`, navigate to +{nav Config > Notification Servers} to verify that things are operational. Troubleshooting @@ -134,31 +153,61 @@ may also have information that is useful in figuring out what's wrong. The server also generates a log, by default in `/var/log/aphlict.log`. You can -change this location by changing `notification.log` in your configuration. The -log may contain information useful in resolving issues. +change this location by adjusting configuration. The log may contain +information that is useful in resolving issues. + + +SSL and HTTPS +============= + +If you serve Phabricator over HTTPS, you must also serve websockets over HTTPS. +Browsers will refuse to connect to `ws://` websockets from HTTPS pages. + +If a client connects to Phabricator over HTTPS, Phabricator will automatically +select an appropriate HTTPS service from `notification.servers` and instruct +the browser to open a websocket connection with `wss://`. + +The simplest way to do this is configure Aphlict with an SSL key and +certificate and let it terminate SSL directly. + +If you prefer not to do this, two other options are: + + - run the websocket through a websocket-capable loadbalancer and terminate + SSL there; or + - run the websokket through `nginx` over the same socket as the rest of + your web traffic. + +See the next sections for more detail. -Advanced Usage -============== +Terminating SSL with a Load Balancer +==================================== -It is possible to route the WebSockets traffic for Aphlict through a reverse -proxy such as `nginx` (see @{article:Configuration Guide} for instructions on -configuring `nginx`). In order to do this with `nginx`, you will require at -least version 1.3. You can read some more information about using `nginx` with -WebSockets at http://nginx.com/blog/websocket-nginx/. +If you want to terminate SSL in front of the notification server with a +traditional load balancer or a similar device, do this: -There are a few benefits of this approach: + - Point `notification.servers` at your load balancer or reverse proxy, + specifying that the protocol is `https`. + - On the load balancer or proxy, terminate SSL and forward traffic to the + Aphlict server. + - In the Aphlict configuration, listen on the target port with `http`. - - SSL is terminated at the `nginx` layer and consequently there is no need to - configure `notificaton.ssl-cert` and `notification.ssl-key` (in fact, with - this approach you should //not// configure these options because otherwise - the Aphlict server will not accept HTTP traffic). - - You don't have to open up a separate port on the server. - - Clients don't need to be able to connect to Aphlict over a non-standard - port which may be blocked by a firewall or anti-virus software. -The following files show an example `nginx` configuration. Note that this is an -example only and you may need to adjust this to suit your own setup. +Terminating SSL with Nginx +========================== + +If you use `nginx`, you can send websocket traffic to the same port as normal +HTTP traffic and have `nginx` proxy it selectively based on the request path. + +This requires `nginx` 1.3 or greater. See the `nginx` documentation for +details: + +> http://nginx.com/blog/websocket-nginx/ + +This is very complex, but allows you to support notifications without opening +additional ports. + +An example `nginx` configuration might look something like this: ```lang=nginx, name=/etc/nginx/conf.d/connection_upgrade.conf map $http_upgrade $connection_upgrade { @@ -191,8 +240,23 @@ } ``` -With this approach, you should set `notification.client-uri` to -`http://localhost/ws/`. Additionally, there is no need for the Aphlict server -to bind to `0.0.0.0` anymore (which is the default behavior), so you could -start the Aphlict server with `./bin/aphlict start --client-host=localhost` -instead. +With this approach, you should make these additional adjustments: + +**Phabricator Configuration**: The entry in `notification.servers` with type +`"client"` should have these adjustments made: + + - Set `host` to the Phabricator host. + - Set `port` to the standard HTTPS port (usually `443`). + - Set `protocol` to `"https"`. + - Set `path` to `/ws/`, so it matches the special `location` in your + `nginx` config. + +You do not need to adjust the `"admin"` server. + +**Aphlict**: Your Aphlict configuration should make these adjustments to +the `"client"` server: + + - The `protocol` should be `"http"`: `nginx` will send plain HTTP traffic + to Aphlict. + - Optionally, you can `listen` on `127.0.0.1` instead of `0.0.0.0`, because + the server will no longer receive external traffic. diff --git a/src/infrastructure/testing/PhabricatorTestCase.php b/src/infrastructure/testing/PhabricatorTestCase.php --- a/src/infrastructure/testing/PhabricatorTestCase.php +++ b/src/infrastructure/testing/PhabricatorTestCase.php @@ -113,8 +113,8 @@ // We can't stub this service right now, and it's not generally useful // to publish notifications about test execution. $this->env->overrideEnvConfig( - 'notification.enabled', - false); + 'notification.servers', + array()); $this->env->overrideEnvConfig( 'phabricator.base-uri', diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -528,22 +528,22 @@ $response = CelerityAPI::getStaticResourceResponse(); - if (PhabricatorEnv::getEnvConfig('notification.enabled')) { - if ($user && $user->isLoggedIn()) { + if ($request->isHTTPS()) { + $with_protocol = 'https'; + } else { + $with_protocol = 'http'; + } - $client_uri = PhabricatorEnv::getEnvConfig('notification.client-uri'); - $client_uri = new PhutilURI($client_uri); - if ($client_uri->getDomain() == 'localhost') { - $this_host = $this->getRequest()->getHost(); - $this_host = new PhutilURI('http://'.$this_host.'/'); - $client_uri->setDomain($this_host->getDomain()); - } + $servers = PhabricatorNotificationServerRef::getEnabledClientServers( + $with_protocol); - if ($request->isHTTPS()) { - $client_uri->setProtocol('wss'); - } else { - $client_uri->setProtocol('ws'); - } + if ($servers) { + if ($user && $user->isLoggedIn()) { + // TODO: We could be smarter about selecting a server if there are + // multiple options available. + $server = head($servers); + + $client_uri = $server->getWebsocketURI(); Javelin::initBehavior( 'aphlict-listen', diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js --- a/support/aphlict/server/aphlict_server.js +++ b/support/aphlict/server/aphlict_server.js @@ -82,6 +82,7 @@ require('./lib/AphlictAdminServer'); require('./lib/AphlictClientServer'); + var ii; var logs = config.logs || []; diff --git a/support/aphlict/server/lib/AphlictAdminServer.js b/support/aphlict/server/lib/AphlictAdminServer.js --- a/support/aphlict/server/lib/AphlictAdminServer.js +++ b/support/aphlict/server/lib/AphlictAdminServer.js @@ -58,7 +58,7 @@ _onrequest: function(request, response) { var self = this; var u = url.parse(request.url, true); - var instance = u.query.instance || '/'; + var instance = u.query.instance || 'default'; // Publishing a notification. if (u.pathname == '/') { diff --git a/support/aphlict/server/lib/AphlictClientServer.js b/support/aphlict/server/lib/AphlictClientServer.js --- a/support/aphlict/server/lib/AphlictClientServer.js +++ b/support/aphlict/server/lib/AphlictClientServer.js @@ -26,11 +26,11 @@ _server: null, _lists: null, - getListenerList: function(path) { - if (!this._lists[path]) { - this._lists[path] = new JX.AphlictListenerList(path); + getListenerList: function(instance) { + if (!this._lists[instance]) { + this._lists[instance] = new JX.AphlictListenerList(instance); } - return this._lists[path]; + return this._lists[instance]; }, log: function() { @@ -58,8 +58,14 @@ var wss = new WebSocket.Server({server: server}); wss.on('connection', function(ws) { - var path = url.parse(ws.upgradeReq.url).pathname; - var listener = self.getListenerList(path).addListener(ws); + var instance = url.parse(ws.upgradeReq.url).pathname; + + instance = instance.replace(/\//g, ''); + if (!instance.length) { + instance = 'default'; + } + + var listener = self.getListenerList(instance).addListener(ws); function log() { self.log( @@ -104,23 +110,12 @@ }); ws.on('close', function() { - self.getListenerList(path).removeListener(listener); - log('Disconnected.'); - }); - - wss.on('close', function() { - self.getListenerList(path).removeListener(listener); + self.getListenerList(instance).removeListener(listener); log('Disconnected.'); }); - - wss.on('error', function(err) { - log('Error: %s', err.message); - }); - }); - }, - + } } }); diff --git a/support/aphlict/server/lib/AphlictListener.js b/support/aphlict/server/lib/AphlictListener.js --- a/support/aphlict/server/lib/AphlictListener.js +++ b/support/aphlict/server/lib/AphlictListener.js @@ -50,7 +50,7 @@ }, getDescription: function() { - return 'Listener/' + this.getID() + this._path; + return 'Listener/' + this.getID() + '/' + this._path; }, writeMessage: function(message) { diff --git a/webroot/rsrc/externals/javelin/lib/Leader.js b/webroot/rsrc/externals/javelin/lib/Leader.js --- a/webroot/rsrc/externals/javelin/lib/Leader.js +++ b/webroot/rsrc/externals/javelin/lib/Leader.js @@ -63,7 +63,7 @@ */ start: function() { var self = JX.Leader; - self.callIfLeader(JX.bag); + self.call(JX.bag); }, /** @@ -132,8 +132,15 @@ self._becomeLeader(); leader_callback(); } else { + + // Set a callback to try to become the leader shortly after the + // current lease expires. This lets us recover from cases where the + // leader goes missing quickly. + window.setTimeout(self._usurp, (lease.until - now) + 50); + follower_callback(); } + return; } @@ -285,6 +292,16 @@ new JX.Leader().invoke('onBecomeLeader'); }, + + /** + * Try to usurp leadership position after a lease expiration. + */ + _usurp: function() { + var self = JX.Leader; + self.call(JX.bag); + }, + + /** * Mark a message as seen. *