From 2930733ac99f1bc0966dd8cc296d0a9438eaef81 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 Apr 2016 12:07:48 -0700 Subject: [PATCH] Complete modernization of Aphlict configuration Summary: Fixes T10697. This finishes bringing the rest of the config up to cluster power levels. Phabricator is now given an arbitrarily long list of notification servers. Each Aphlict server is given an arbitrarily long list of ports to run services on. Users are free to make them meet in the middle by proxying whatever they want to whatever else they want. This should also accommodate clustering fairly easily in the future. Also rewrote the status UI and changed a million other things. :boar: Test Plan: {F1217864} {F1217865} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10697 Differential Revision: https://secure.phabricator.com/D15703 --- resources/celerity/map.php | 12 +- src/__phutil_library_map__.php | 8 +- .../PhabricatorConfigApplication.php | 1 + .../PhabricatorExtraConfigSetupCheck.php | 3 + ...icatorConfigClusterDatabasesController.php | 4 +- ...orConfigClusterNotificationsController.php | 163 ++++++++++++ .../PhabricatorConfigController.php | 3 +- .../PhabricatorNotificationConfigOptions.php | 59 +++-- .../PhabricatorNotificationsApplication.php | 1 - .../client/PhabricatorNotificationClient.php | 71 ++---- .../PhabricatorNotificationServerRef.php | 234 ++++++++++++++++++ ...torNotificationServersConfigOptionType.php | 140 +++++++++++ ...PhabricatorNotificationPanelController.php | 13 +- ...habricatorNotificationStatusController.php | 82 ------ .../setup/PhabricatorAphlictSetupCheck.php | 29 +-- ...catorDesktopNotificationsSettingsPanel.php | 10 +- .../user/cluster/cluster_databases.diviner | 6 +- .../user/configuration/notifications.diviner | 142 ++++++++--- .../testing/PhabricatorTestCase.php | 4 +- src/view/page/PhabricatorStandardPageView.php | 28 +-- support/aphlict/server/aphlict_server.js | 1 + .../aphlict/server/lib/AphlictAdminServer.js | 2 +- .../aphlict/server/lib/AphlictClientServer.js | 33 ++- support/aphlict/server/lib/AphlictListener.js | 2 +- webroot/rsrc/externals/javelin/lib/Leader.js | 26 +- 25 files changed, 788 insertions(+), 289 deletions(-) create mode 100644 src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php create mode 100644 src/applications/notification/client/PhabricatorNotificationServerRef.php create mode 100644 src/applications/notification/config/PhabricatorNotificationServersConfigOptionType.php delete mode 100644 src/applications/notification/controller/PhabricatorNotificationStatusController.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index e722117be2..29df3a60e2 100644 --- 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' => 'e526f428', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '7ba78475', 'differential.pkg.js' => 'd0cd0df6', @@ -232,7 +232,7 @@ return array( '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' => 'b4ba945c', '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 @@ return array( 'javelin-history' => 'd4505101', 'javelin-install' => '05270951', 'javelin-json' => '69adf288', - 'javelin-leader' => '331b1611', + 'javelin-leader' => 'b4ba945c', 'javelin-magical-init' => '3010e992', 'javelin-mask' => '8a41885b', 'javelin-quicksand' => '6b8ef10b', @@ -1108,9 +1108,6 @@ return array( 'javelin-dom', 'javelin-workflow', ), - '331b1611' => array( - 'javelin-install', - ), '340c8eff' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1752,6 +1749,9 @@ return array( 'javelin-typeahead-preloaded-source', 'javelin-util', ), + 'b4ba945c' => array( + 'javelin-install', + ), 'b59e1e96' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1b86548f5c..fffd0424be 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2040,6 +2040,7 @@ phutil_register_library_map(array( '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', @@ -2711,7 +2712,8 @@ phutil_register_library_map(array( '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', @@ -6467,6 +6469,7 @@ phutil_register_library_map(array( 'PhabricatorConfigApplication' => 'PhabricatorApplication', 'PhabricatorConfigCacheController' => 'PhabricatorConfigController', 'PhabricatorConfigClusterDatabasesController' => 'PhabricatorConfigController', + 'PhabricatorConfigClusterNotificationsController' => 'PhabricatorConfigController', 'PhabricatorConfigCollectorsModule' => 'PhabricatorConfigModule', 'PhabricatorConfigColumnSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigConfigPHIDType' => 'PhabricatorPHIDType', @@ -7227,7 +7230,8 @@ phutil_register_library_map(array( '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 index 65e1486ce1..bdaf0b2cc0 100644 --- a/src/applications/config/application/PhabricatorConfigApplication.php +++ b/src/applications/config/application/PhabricatorConfigApplication.php @@ -64,6 +64,7 @@ final class PhabricatorConfigApplication extends PhabricatorApplication { ), 'cluster/' => array( 'databases/' => 'PhabricatorConfigClusterDatabasesController', + 'notifications/' => 'PhabricatorConfigClusterNotificationsController', ), ), ); diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index e054918c1d..5e5b493591 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -307,6 +307,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { '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 index c60b24b089..453ebfe742 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php +++ b/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php @@ -7,11 +7,11 @@ final class PhabricatorConfigClusterDatabasesController $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 index 0000000000..7f42c323e6 --- /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 index 6efe8923f5..c390688930 100644 --- a/src/applications/config/controller/PhabricatorConfigController.php +++ b/src/applications/config/controller/PhabricatorConfigController.php @@ -23,7 +23,8 @@ abstract class PhabricatorConfigController extends PhabricatorController { $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 index 5855aafb60..34fae7c184 100644 --- a/src/applications/config/option/PhabricatorNotificationConfigOptions.php +++ b/src/applications/config/option/PhabricatorNotificationConfigOptions.php @@ -20,30 +20,43 @@ final class PhabricatorNotificationConfigOptions } 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 index b4cc2b45b6..e068db50a8 100644 --- a/src/applications/notification/application/PhabricatorNotificationsApplication.php +++ b/src/applications/notification/application/PhabricatorNotificationsApplication.php @@ -25,7 +25,6 @@ final class PhabricatorNotificationsApplication extends PhabricatorApplication { => '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 index f527a64bc4..292fda7f49 100644 --- 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; + } + + return; + } + + 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. + } } } - 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(); - } - - private static function getInstance() { - $client_uri = PhabricatorEnv::getEnvConfig('notification.client-uri'); - return id(new PhutilURI($client_uri))->getPath(); - } - } diff --git a/src/applications/notification/client/PhabricatorNotificationServerRef.php b/src/applications/notification/client/PhabricatorNotificationServerRef.php new file mode 100644 index 0000000000..cfdbb5e8eb --- /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 index 0000000000..07cdf7eece --- /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 index c16069a0e6..be68cc2de7 100644 --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -44,17 +44,8 @@ final class PhabricatorNotificationPanelController ), 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 index 49d3fb9d9b..0000000000 --- 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 index 99dba45ec4..b42ff85c2e 100644 --- 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 @@ final class PhabricatorAphlictSetupCheck extends PhabricatorSetupCheck { ->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 @@ final class PhabricatorAphlictSetupCheck extends PhabricatorSetupCheck { 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 index bc7538f6bd..8da45a518c 100644 --- a/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php @@ -4,9 +4,13 @@ final class PhabricatorDesktopNotificationsSettingsPanel 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 index 234db4d86a..7e9f2c8a03 100644 --- a/src/docs/user/cluster/cluster_databases.diviner +++ b/src/docs/user/cluster/cluster_databases.diviner @@ -69,7 +69,7 @@ effect, then continue to "Monitoring Replicas" to verify the configuration. 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 @@ works internally, see "Unreachable Masters" below. 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 @@ unhealthy and stop sending all traffic (except for more health checks) to it. 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 index 0b828f7072..e1f5cacaed 100644 --- a/src/docs/user/configuration/notifications.diviner +++ b/src/docs/user/configuration/notifications.diviner @@ -11,13 +11,13 @@ tasks or commenting on code reviews) through email and in-application 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 @@ if you are running a more complex configuration. 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. - - `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. +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" + } +] +``` + +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 @@ Because the notification server uses WebSockets, your browser error console 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. -Advanced Usage -============== +SSL and HTTPS +============= -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 serve Phabricator over HTTPS, you must also serve websockets over HTTPS. +Browsers will refuse to connect to `ws://` websockets from HTTPS pages. -There are a few benefits of this approach: +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://`. - - 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 simplest way to do this is configure Aphlict with an SSL key and +certificate and let it terminate SSL directly. -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. +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. + + +Terminating SSL with a Load Balancer +==================================== + +If you want to terminate SSL in front of the notification server with a +traditional load balancer or a similar device, do this: + + - 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`. + + +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 @@ server { } ``` -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 index 4f2fedeca6..4af75d157e 100644 --- a/src/infrastructure/testing/PhabricatorTestCase.php +++ b/src/infrastructure/testing/PhabricatorTestCase.php @@ -113,8 +113,8 @@ abstract class PhabricatorTestCase extends PhutilTestCase { // 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 index 56f8f8a4fd..072e921bf6 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -528,22 +528,22 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView $response = CelerityAPI::getStaticResourceResponse(); - if (PhabricatorEnv::getEnvConfig('notification.enabled')) { + if ($request->isHTTPS()) { + $with_protocol = 'https'; + } else { + $with_protocol = 'http'; + } + + $servers = PhabricatorNotificationServerRef::getEnabledClientServers( + $with_protocol); + + 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 = 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()); - } - - if ($request->isHTTPS()) { - $client_uri->setProtocol('wss'); - } else { - $client_uri->setProtocol('ws'); - } + $client_uri = $server->getWebsocketURI(); Javelin::initBehavior( 'aphlict-listen', diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js index 7244ee7593..263a355542 100644 --- a/support/aphlict/server/aphlict_server.js +++ b/support/aphlict/server/aphlict_server.js @@ -82,6 +82,7 @@ try { 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 index c08f09b70c..97dca15bde 100644 --- a/support/aphlict/server/lib/AphlictAdminServer.js +++ b/support/aphlict/server/lib/AphlictAdminServer.js @@ -58,7 +58,7 @@ JX.install('AphlictAdminServer', { _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 index 1d8051a65c..4d173171c2 100644 --- a/support/aphlict/server/lib/AphlictClientServer.js +++ b/support/aphlict/server/lib/AphlictClientServer.js @@ -26,11 +26,11 @@ JX.install('AphlictClientServer', { _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 @@ JX.install('AphlictClientServer', { 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 @@ JX.install('AphlictClientServer', { }); ws.on('close', function() { - self.getListenerList(path).removeListener(listener); + self.getListenerList(instance).removeListener(listener); log('Disconnected.'); }); - - wss.on('close', function() { - self.getListenerList(path).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 index 4c62e4aa7e..94a5c4fb50 100644 --- a/support/aphlict/server/lib/AphlictListener.js +++ b/support/aphlict/server/lib/AphlictListener.js @@ -50,7 +50,7 @@ JX.install('AphlictListener', { }, 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 index aaa37846e9..e0b71e2e48 100644 --- a/webroot/rsrc/externals/javelin/lib/Leader.js +++ b/webroot/rsrc/externals/javelin/lib/Leader.js @@ -34,6 +34,7 @@ JX.install('Leader', { statics: { _interval: null, + _timeout: null, _broadcastKey: 'JX.Leader.broadcast', _leaderKey: 'JX.Leader.id', @@ -63,7 +64,7 @@ JX.install('Leader', { */ start: function() { var self = JX.Leader; - self.callIfLeader(JX.bag); + self.call(JX.bag); }, /** @@ -132,8 +133,21 @@ JX.install('Leader', { 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. + if (self._timeoout) { + window.clearTimeout(self._timeout); + self._timeout = null; + } + self._timeout = window.setTimeout( + self._usurp, + (lease.until - now) + 50); + follower_callback(); } + return; } @@ -285,6 +299,16 @@ JX.install('Leader', { 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. *