From 15ab5f5ad8a3fd26762d2e4b18e06d5bdcaa5d95 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 28 Aug 2014 13:44:32 +0100 Subject: [PATCH 01/16] Merge backfill_ and backfill in federation handler --- synapse/handlers/federation.py | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 9023c3d403..220dbbb3a9 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -188,27 +188,14 @@ class FederationHandler(BaseHandler): @log_function @defer.inlineCallbacks def backfill(self, dest, room_id, limit): - events = yield self._backfill(dest, room_id, limit) - - for event in events: - try: - yield self.store.persist_event(event, backfilled=True) - except: - logger.exception("Failed to persist event: %s", event) - - defer.returnValue(events) - - @defer.inlineCallbacks - def _backfill(self, dest, room_id, limit): pdus = yield self.replication_layer.backfill(dest, room_id, limit) - if not pdus: - defer.returnValue([]) + events = [] - events = [ - self.pdu_codec.event_from_pdu(pdu) - for pdu in pdus - ] + for pdu in pdus: + event = self.pdu_codec.event_from_pdu(pdu) + events.append(event) + yield self.store.persist_event(event, backfilled=True) defer.returnValue(events) From 7acede1e42d2a1b8442bc4b30dba792b034caeed Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 28 Aug 2014 13:51:50 +0100 Subject: [PATCH 02/16] Fix pyflakes warnings --- synapse/handlers/_base.py | 2 +- synapse/handlers/federation.py | 4 +--- synapse/handlers/typing.py | 3 ++- synapse/server.py | 5 ----- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index f141e92ce2..b37c8be964 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -35,7 +35,7 @@ class BaseRoomHandler(BaseHandler): extra_users=[]): snapshot.fill_out_prev_events(event) - store_id = yield self.store.persist_event(event) + yield self.store.persist_event(event) destinations = set(extra_destinations) # Send a PDU to all hosts who have joined the room. diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 220dbbb3a9..606e1f1817 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -22,8 +22,6 @@ from synapse.api.constants import Membership from synapse.util.logutils import log_function from synapse.federation.pdu_codec import PduCodec -from synapse.api.errors import AuthError - from twisted.internet import defer import logging @@ -146,7 +144,7 @@ class FederationHandler(BaseHandler): else: with (yield self.room_lock.lock(event.room_id)): - store_id = yield self.store.persist_event(event, backfilled) + yield self.store.persist_event(event, backfilled) room = yield self.store.get_room(event.room_id) diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index 9d38a7336e..9fab0ff37c 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -17,11 +17,12 @@ from twisted.internet import defer from ._base import BaseHandler +from synapse.api.errors import SynapseError, AuthError + import logging from collections import namedtuple - logger = logging.getLogger(__name__) diff --git a/synapse/server.py b/synapse/server.py index ade8dc6c15..3e72b2bcd5 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -126,11 +126,6 @@ class BaseHomeServer(object): object.""" return UserID.from_string(s, hs=self) - def parse_roomid(self, s): - """Parse the string given by 's' as a Room ID and return a RoomID - object.""" - return RoomID.from_string(s, hs=self) - def parse_roomalias(self, s): """Parse the string given by 's' as a Room Alias and return a RoomAlias object.""" From 52cfdfd5f1b93331eb36faff2204314320352117 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 28 Aug 2014 14:49:15 +0100 Subject: [PATCH 03/16] Fleshed out login spec. --- docs/specification.rst | 344 +++++++++++++++++++++-------------------- 1 file changed, 175 insertions(+), 169 deletions(-) diff --git a/docs/specification.rst b/docs/specification.rst index c271308675..8df5d478a1 100644 --- a/docs/specification.rst +++ b/docs/specification.rst @@ -201,124 +201,133 @@ Clients must register with a home server in order to use Matrix. After registering, the client will be given an access token which must be used in ALL requests to that home server as a query parameter 'access_token'. -- TODO Kegan : Make registration like login +- TODO Kegan : Make registration like login (just omit the "user" key on the + initial request?) - TODO Kegan : Allow alternative forms of login (>1 route) If the client has already registered, they need to be able to login to their account. The home server may provide many different ways of logging in, such -as user/password auth, login via a social network (OAuth), login by confirming +as user/password auth, login via a social network (OAuth2), login by confirming a token sent to their email address, etc. This specification does not define how home servers should authorise their users who want to login to their existing accounts, but instead defines the standard interface which implementations should follow so that ANY client can login to ANY home server. The login process breaks down into the following: - 1. Get login process info. + 1. Determine the requirements for logging in. 2. Submit the login stage credentials. - 3. Get access token or be told the next stage in the login process and repeat + 3. Get credentials or be told the next stage in the login process and repeat step 2. -- What are types? +As each home server may have different ways of logging in, the client needs to know how +they should login. All distinct login stages MUST have a corresponding ``'type'``. +A ``'type'`` is a namespaced string which details the mechanism for logging in. -Matrix-defined login types --------------------------- -- m.login.password -- m.login.oauth2 -- m.login.email.code -- m.login.email.url +A client may be able to login via multiple valid login flows, and should choose a single +flow when logging in. A flow is a series of login stages. The home server MUST respond +with all the valid login flows when requested:: -Password-based --------------- -Type: "m.login.password" -LoginSubmission:: - - { - "type": "m.login.password", - "user": , - "password": - } - -Example: -Assume you are @bob:matrix.org and you wish to login on another mobile device. -First, you GET /login which returns:: - - { - "type": "m.login.password" - } - -Your client knows how to handle this, so your client prompts the user to enter -their username and password. This is then submitted:: - - { - "type": "m.login.password", - "user": "@bob:matrix.org", - "password": "monkey" - } - -The server checks this, finds it is valid, and returns:: + The client can login via 3 paths: 1a and 1b, 2a and 2b, or 3. The client should + select one of these paths. + + [ + { + "type": "", + "stages": [ "", "" ] + }, + { + "type": "", + "stages": [ "", "" ] + }, + { + "type": "" + } + ] + +After the login is completed, the client's fully-qualified user ID and a new access +token MUST be returned:: { + "user_id": "@user:matrix.org", "access_token": "abcdef0123456789" } -The server may optionally return "user_id" to confirm or change the user's ID. -This is particularly useful if the home server wishes to support localpart entry -of usernames (e.g. "bob" rather than "@bob:matrix.org"). +The ``user_id`` key is particularly useful if the home server wishes to support +localpart entry of usernames (e.g. "user" rather than "@user:matrix.org"), as the +client may not be able to determine its ``user_id`` in this case. + +If a login has multiple requests, the home server may wish to create a session. If +a home server responds with a 'session' key to a request, clients MUST submit it in +subsequent requests until the login is completed:: + + { + "session": "" + } + +This specification defines the following login types: + - m.login.password + - m.login.oauth2 + - m.login.email.code + - m.login.email.url + + +Password-based +-------------- +Type: + "m.login.password" +Description: + Login is supported via a username and password. + +To respond to this type, reply with:: + + { + "type": "m.login.password", + "user": "", + "password": "" + } + +The home server MUST respond with either new credentials, the next stage of the login +process, or a standard error response. OAuth2-based ------------ -Type: "m.login.oauth2" -This is a multi-stage login. +Type: + "m.login.oauth2" +Description: + Login is supported via OAuth2 URLs. This login consists of multiple requests. -LoginSubmission:: +To respond to this type, reply with:: { "type": "m.login.oauth2", - "user": + "user": "" } -Returns:: +The server MUST respond with:: { - "uri": + "uri": } -The home server acts as a 'confidential' Client for the purposes of OAuth2. - -If the uri is a "sevice selection uri", it is a simple page which prompts the -user to choose which service to authorize with. On selection of a service, they -link through to Authorization Request URIs. If there is only 1 service which the +The home server acts as a 'confidential' client for the purposes of OAuth2. +If the uri is a ``sevice selection URI``, it MUST point to a webpage which prompts the +user to choose which service to authorize with. On selection of a service, this +MUST link through to an ``Authorization Request URI``. If there is only 1 service which the home server accepts when logging in, this indirection can be skipped and the -"uri" key can be the Authorization Request URI. +"uri" key can be the ``Authorization Request URI``. -The client visits the Authorization Request URI, which then shows the OAuth2 -Allow/Deny prompt. Hitting 'Allow' returns the redirect URI with the auth code. -Home servers can choose any path for the redirect URI. The client should visit -the redirect URI, which will then finish the OAuth2 login process, granting the +The client then visits the ``Authorization Request URI``, which then shows the OAuth2 +Allow/Deny prompt. Hitting 'Allow' returns the ``redirect URI`` with the auth code. +Home servers can choose any path for the ``redirect URI``. The client should visit +the ``redirect URI``, which will then finish the OAuth2 login process, granting the home server an access token for the chosen service. When the home server gets -this access token, it knows that the cilent has authed with the 3rd party, and -so can return a LoginResult. - -The OAuth redirect URI (with auth code) MUST return a LoginResult. +this access token, it verifies that the cilent has authorised with the 3rd party, and +can now complete the login. The OAuth2 ``redirect URI`` (with auth code) MUST respond +with either new credentials, the next stage of the login process, or a standard error +response. -Example: -Assume you are @bob:matrix.org and you wish to login on another mobile device. -First, you GET /login which returns:: - - { - "type": "m.login.oauth2" - } - -Your client knows how to handle this, so your client prompts the user to enter -their username. This is then submitted:: - - { - "type": "m.login.oauth2", - "user": "@bob:matrix.org" - } - -The server only accepts auth from Google, so returns the Authorization Request -URI for Google:: +For example, if a home server accepts OAuth2 from Google, it would return the +Authorization Request URI for Google:: { "uri": "https://accounts.google.com/o/oauth2/auth?response_type=code& @@ -329,145 +338,142 @@ The client then visits this URI and authorizes the home server. The client then visits the REDIRECT_URI with the auth code= query parameter which returns:: { + "user_id": "@user:matrix.org", "access_token": "0123456789abcdef" } Email-based (code) ------------------ -Type: "m.login.email.code" -This is a multi-stage login. +Type: + "m.login.email.code" +Description: + Login is supported by typing in a code which is sent in an email. This login + consists of multiple requests. -First LoginSubmission:: +To respond to this type, reply with:: { "type": "m.login.email.code", - "user": - "email": + "user": "", + "email": "" } -Returns:: - - { - "type": m.login.email.code - "session": - } - -The email contains a code which must be sent in the next LoginSubmission:: +After validating the email address, the home server MUST send an email containing +an authentication code and return:: { "type": "m.login.email.code", - "session": , - "code": + "session": "" } -Returns:: +The second request in this login stage involves sending this authentication code:: { - "access_token": + "type": "m.login.email.code", + "session": "", + "code": "" } +The home server MUST respond to this with either new credentials, the next stage of +the login process, or a standard error response. + Email-based (url) ----------------- -Type: "m.login.email.url" -This is a multi-stage login. +Type: + "m.login.email.url" +Description: + Login is supported by clicking on a URL in an email. This login consists of + multiple requests. -First LoginSubmission:: +To respond to this type, reply with:: { "type": "m.login.email.url", - "user": - "email": + "user": "", + "email": "" } -Returns:: +After validating the email address, the home server MUST send an email containing +an authentication URL and return:: { - "session": + "type": "m.login.email.url", + "session": "" } The email contains a URL which must be clicked. After it has been clicked, the -client should perform a request:: - - { - "type": "m.login.email.code", - "session": - } - -Returns:: - - { - "access_token": - } - -Example: -Assume you are @bob:matrix.org and you wish to login on another mobile device. -First, you GET /login which returns:: - - { - "type": "m.login.email.url" - } - -Your client knows how to handle this, so your client prompts the user to enter -their email address. This is then submitted:: +client should perform another request:: { "type": "m.login.email.url", - "user": "@bob:matrix.org", - "email": "bob@mydomain.com" + "session": "" } -The server confirms that bob@mydomain.com is linked to @bob:matrix.org, then -sends an email to this address and returns:: +The home server MUST respond to this with either new credentials, the next stage of +the login process, or a standard error response. + +A common client implementation will be to periodically poll until the link is clicked. +If the link has not been visited yet, a standard error response with an errcode of +``M_LOGIN_EMAIL_URL_NOT_YET`` should be returned. + + +N-Factor Authentication +----------------------- +Multiple login stages can be combined to create N-factor authentication during login. + +This can be achieved by responding with the ``'next'`` login type on completion of a +previous login stage:: { - "session": "ewuigf7462" + "next": "" } -The client then starts polling the server with the following:: +If a home server implements N-factor authentication, it MUST respond with all +``'stages'`` when initially queried for their login requirements:: { - "type": "m.login.email.url", - "session": "ewuigf7462" + "type": "<1st login type>", + "stages": [ <1st login type>, <2nd login type>, ... , ] } -(Alternatively, the server could send the device a push notification when the -email has been validated). The email arrives and it contains a URL to click on. -The user clicks on the which completes the login process with the server. The -next time the client polls, it returns:: +This can be represented conceptually as:: - { - "access_token": "abcdef0123456789" - } + _______________________ + | Login Stage 1 | + | type: "" | + | ___________________ | + | |_Request_1_________| | <-- Returns "session" key which is used throughout. + | ___________________ | + | |_Request_2_________| | <-- Returns a "next" value of "login type2" + |_______________________| + | + | + _________V_____________ + | Login Stage 2 | + | type: "" | + | ___________________ | + | |_Request_1_________| | + | ___________________ | + | |_Request_2_________| | + | ___________________ | + | |_Request_3_________| | <-- Returns a "next" value of "login type3" + |_______________________| + | + | + _________V_____________ + | Login Stage 3 | + | type: "" | + | ___________________ | + | |_Request_1_________| | <-- Returns user credentials + |_______________________| -N-Factor auth -------------- -Multiple login stages can be combined with the "next" key in the LoginResult. - -Example: -A server demands an email.code then password auth before logging in. First, the -client performs a GET /login which returns:: - - { - "type": "m.login.email.code", - "stages": ["m.login.email.code", "m.login.password"] - } - -The client performs the email login (See "Email-based (code)"), but instead of -returning an access_token, it returns:: - - { - "next": "m.login.password" - } - -The client then presents a user/password screen and the login continues until -this is complete (See "Password-based"), which then returns the "access_token". - Fallback -------- +Clients cannot be expected to be able to know how to process every single +login type. If a client determines it does not know how to handle a given +login type, it should request a login fallback page:: -If the client does NOT know how to handle the given type, they should:: - - GET /login/fallback + GET matrix/client/api/v1/login/fallback This MUST return an HTML page which can perform the entire login process. From 8d7d251c356f74a376053619f23057f0d6d8aa1e Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 28 Aug 2014 14:56:55 +0100 Subject: [PATCH 04/16] Support multiple login flows when deciding how to login. Updated cmdclient and spec. Webclient doesn't need updating for this. --- cmdclient/console.py | 9 +++++++-- docs/specification.rst | 28 +++++++++++++++------------- synapse/rest/login.py | 2 +- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/cmdclient/console.py b/cmdclient/console.py index a4d8145d72..7bda4000fc 100755 --- a/cmdclient/console.py +++ b/cmdclient/console.py @@ -225,8 +225,13 @@ class SynapseCmd(cmd.Cmd): json_res = yield self.http_client.do_request("GET", url) print json_res - if ("type" not in json_res or "m.login.password" != json_res["type"] or - "stages" in json_res): + if "flows" not in json_res: + print "Failed to find any login flows." + defer.returnValue(False) + + flow = json_res["flows"][0] # assume first is the one we want. + if ("type" not in flow or "m.login.password" != flow["type"] or + "stages" in flow): fallback_url = self._url() + "/login/fallback" print ("Unable to login via the command line client. Please visit " "%s to login." % fallback_url) diff --git a/docs/specification.rst b/docs/specification.rst index 8df5d478a1..30e4a7a3fb 100644 --- a/docs/specification.rst +++ b/docs/specification.rst @@ -230,19 +230,21 @@ with all the valid login flows when requested:: The client can login via 3 paths: 1a and 1b, 2a and 2b, or 3. The client should select one of these paths. - [ - { - "type": "", - "stages": [ "", "" ] - }, - { - "type": "", - "stages": [ "", "" ] - }, - { - "type": "" - } - ] + { + "flows": [ + { + "type": "", + "stages": [ "", "" ] + }, + { + "type": "", + "stages": [ "", "" ] + }, + { + "type": "" + } + ] + } After the login is completed, the client's fully-qualified user ID and a new access token MUST be returned:: diff --git a/synapse/rest/login.py b/synapse/rest/login.py index bcf63fd2ab..99e4f10aac 100644 --- a/synapse/rest/login.py +++ b/synapse/rest/login.py @@ -27,7 +27,7 @@ class LoginRestServlet(RestServlet): PASS_TYPE = "m.login.password" def on_GET(self, request): - return (200, {"type": LoginRestServlet.PASS_TYPE}) + return (200, {"flows": [{"type": LoginRestServlet.PASS_TYPE}]}) def on_OPTIONS(self, request): return (200, {}) From 466fbe4c4e034125b9db6f859387ce1141efe425 Mon Sep 17 00:00:00 2001 From: Emmanuel ROHEE Date: Thu, 28 Aug 2014 11:14:36 +0200 Subject: [PATCH 05/16] Cleaned up deps --- webclient/home/home-controller.js | 4 ++-- webclient/recents/recents-controller.js | 4 ++-- webclient/room/room-controller.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/webclient/home/home-controller.js b/webclient/home/home-controller.js index 62f6ef2d95..008dff7422 100644 --- a/webclient/home/home-controller.js +++ b/webclient/home/home-controller.js @@ -17,8 +17,8 @@ limitations under the License. 'use strict'; angular.module('HomeController', ['matrixService', 'eventHandlerService', 'RecentsController']) -.controller('HomeController', ['$scope', '$location', 'matrixService', 'eventHandlerService', 'eventStreamService', - function($scope, $location, matrixService, eventHandlerService, eventStreamService) { +.controller('HomeController', ['$scope', '$location', 'matrixService', + function($scope, $location, matrixService) { $scope.config = matrixService.config(); $scope.public_rooms = []; diff --git a/webclient/recents/recents-controller.js b/webclient/recents/recents-controller.js index bf6a1b8874..e182a3ad23 100644 --- a/webclient/recents/recents-controller.js +++ b/webclient/recents/recents-controller.js @@ -17,8 +17,8 @@ 'use strict'; angular.module('RecentsController', ['matrixService', 'eventHandlerService']) -.controller('RecentsController', ['$scope', 'matrixService', 'eventHandlerService', 'eventStreamService', - function($scope, matrixService, eventHandlerService, eventStreamService) { +.controller('RecentsController', ['$scope', 'matrixService', 'eventHandlerService', + function($scope, matrixService, eventHandlerService) { $scope.rooms = {}; // $scope of the parent where the recents component is included can override this value diff --git a/webclient/room/room-controller.js b/webclient/room/room-controller.js index e775d88570..b30fa9541d 100644 --- a/webclient/room/room-controller.js +++ b/webclient/room/room-controller.js @@ -15,8 +15,8 @@ limitations under the License. */ angular.module('RoomController', ['ngSanitize', 'mFileInput', 'mUtilities']) -.controller('RoomController', ['$scope', '$http', '$timeout', '$routeParams', '$location', 'matrixService', 'eventStreamService', 'eventHandlerService', 'mFileUpload', 'mUtilities', '$rootScope', - function($scope, $http, $timeout, $routeParams, $location, matrixService, eventStreamService, eventHandlerService, mFileUpload, mUtilities, $rootScope) { +.controller('RoomController', ['$scope', '$http', '$timeout', '$routeParams', '$location', 'matrixService', 'eventHandlerService', 'mFileUpload', 'mUtilities', '$rootScope', + function($scope, $http, $timeout, $routeParams, $location, matrixService, eventHandlerService, mFileUpload, mUtilities, $rootScope) { 'use strict'; var MESSAGES_PER_PAGINATION = 30; var THUMBNAIL_SIZE = 320; From 06c79a23d481c45574915fe5ae7088f156e533b3 Mon Sep 17 00:00:00 2001 From: Emmanuel ROHEE Date: Thu, 28 Aug 2014 15:56:16 +0200 Subject: [PATCH 06/16] BF: Made member events parsing work (handleEvents expects an array of events) --- webclient/components/matrix/event-stream-service.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/webclient/components/matrix/event-stream-service.js b/webclient/components/matrix/event-stream-service.js index a1a98b2a36..dc2e359dd0 100644 --- a/webclient/components/matrix/event-stream-service.js +++ b/webclient/components/matrix/event-stream-service.js @@ -96,7 +96,7 @@ angular.module('eventStreamService', []) ); return deferred.promise; - } + }; var startEventStream = function() { settings.shouldPoll = true; @@ -110,18 +110,14 @@ angular.module('eventStreamService', []) for (var i = 0; i < rooms.length; ++i) { var room = rooms[i]; if ("state" in room) { - for (var j = 0; j < room.state.length; ++j) { - eventHandlerService.handleEvents(room.state[j], false); - } + eventHandlerService.handleEvents(room.state, false); } } var presence = response.data.presence; - for (var i = 0; i < presence.length; ++i) { - eventHandlerService.handleEvent(presence[i], false); - } + eventHandlerService.handleEvents(presence, false); - settings.from = response.data.end + settings.from = response.data.end; doEventStream(deferred); }, function(error) { From 7c99ebdbd13c2fc6ac965e939cabd61bd86956d1 Mon Sep 17 00:00:00 2001 From: Emmanuel ROHEE Date: Thu, 28 Aug 2014 16:22:35 +0200 Subject: [PATCH 07/16] Added waitForInitialSyncCompletion so that clients can know when they can access to the data retrieved by the initialSync Request --- .../matrix/event-handler-service.js | 30 +++++++++++++------ .../components/matrix/event-stream-service.js | 3 ++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/webclient/components/matrix/event-handler-service.js b/webclient/components/matrix/event-handler-service.js index 6ea0f58bc5..df61429db5 100644 --- a/webclient/components/matrix/event-handler-service.js +++ b/webclient/components/matrix/event-handler-service.js @@ -27,13 +27,15 @@ Typically, this service will store events or broadcast them to any listeners if typically all the $on method would do is update its own $scope. */ angular.module('eventHandlerService', []) -.factory('eventHandlerService', ['matrixService', '$rootScope', function(matrixService, $rootScope) { +.factory('eventHandlerService', ['matrixService', '$rootScope', '$q', function(matrixService, $rootScope, $q) { var MSG_EVENT = "MSG_EVENT"; var MEMBER_EVENT = "MEMBER_EVENT"; var PRESENCE_EVENT = "PRESENCE_EVENT"; + + var InitialSyncDeferred = $q.defer(); $rootScope.events = { - rooms: {}, // will contain roomId: { messages:[], members:{userid1: event} } + rooms: {} // will contain roomId: { messages:[], members:{userid1: event} } }; $rootScope.presence = {}; @@ -47,11 +49,11 @@ angular.module('eventHandlerService', []) } } - var reInitRoom = function(room_id) { - $rootScope.events.rooms[room_id] = {}; - $rootScope.events.rooms[room_id].messages = []; - $rootScope.events.rooms[room_id].members = {}; - } + var resetRoomMessages = function(room_id) { + if ($rootScope.events.rooms[room_id]) { + $rootScope.events.rooms[room_id].messages = []; + } + }; var handleMessage = function(event, isLiveEvent) { initRoom(event.room_id); @@ -125,8 +127,18 @@ angular.module('eventHandlerService', []) } }, - reInitRoom: function(room_id) { - reInitRoom(room_id); + handleInitialSyncDone: function() { + console.log("# handleInitialSyncDone"); + InitialSyncDeferred.resolve($rootScope.events, $rootScope.presence); }, + + // Returns a promise that resolves when the initialSync request has been processed + waitForInitialSyncCompletion: function() { + return InitialSyncDeferred.promise; + }, + + resetRoomMessages: function(room_id) { + resetRoomMessages(room_id); + } }; }]); diff --git a/webclient/components/matrix/event-stream-service.js b/webclient/components/matrix/event-stream-service.js index dc2e359dd0..4cc2bf4c4e 100644 --- a/webclient/components/matrix/event-stream-service.js +++ b/webclient/components/matrix/event-stream-service.js @@ -117,6 +117,9 @@ angular.module('eventStreamService', []) var presence = response.data.presence; eventHandlerService.handleEvents(presence, false); + // Initial sync is done + eventHandlerService.handleInitialSyncDone(); + settings.from = response.data.end; doEventStream(deferred); }, From c44293db2ff0e40dd46a0f8a6ea6d6fa6ccc7a6a Mon Sep 17 00:00:00 2001 From: Emmanuel ROHEE Date: Thu, 28 Aug 2014 16:23:20 +0200 Subject: [PATCH 08/16] When opening this page, do not join a room already joined --- webclient/room/room-controller.js | 81 ++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 28 deletions(-) diff --git a/webclient/room/room-controller.js b/webclient/room/room-controller.js index b30fa9541d..910168754c 100644 --- a/webclient/room/room-controller.js +++ b/webclient/room/room-controller.js @@ -282,7 +282,7 @@ angular.module('RoomController', ['ngSanitize', 'mFileInput', 'mUtilities']) } if (room_id_or_alias && '!' === room_id_or_alias[0]) { - // Yes. We can start right now + // Yes. We can go on right now $scope.room_id = room_id_or_alias; $scope.room_alias = matrixService.getRoomIdToAliasMapping($scope.room_id); onInit2(); @@ -313,7 +313,7 @@ angular.module('RoomController', ['ngSanitize', 'mFileInput', 'mUtilities']) $scope.room_id = response.data.room_id; console.log(" -> Room ID: " + $scope.room_id); - // Now, we can start + // Now, we can go on onInit2(); }, function () { @@ -323,36 +323,61 @@ angular.module('RoomController', ['ngSanitize', 'mFileInput', 'mUtilities']) }); } }; - + var onInit2 = function() { - eventHandlerService.reInitRoom($scope.room_id); + console.log("onInit2"); + + // Make sure the initialSync has been before going further + eventHandlerService.waitForInitialSyncCompletion().then( + function() { + var needsToJoin = true; + + // The room members is available in the data fetched by initialSync + if ($rootScope.events.rooms[$scope.room_id]) { + var members = $rootScope.events.rooms[$scope.room_id].members; + + // Update the member list + for (var i in members) { + var member = members[i]; + updateMemberList(member); + } + + // Check if the user has already join the room + if ($scope.state.user_id in members) { + if ("join" === members[$scope.state.user_id].membership) { + needsToJoin = false; + } + } + } + + // Do we to join the room before starting? + if (needsToJoin) { + matrixService.join($scope.room_id).then( + function() { + console.log("Joined room "+$scope.room_id); + onInit3(); + }, + function(reason) { + $scope.feedback = "Can't join room: " + reason; + }); + } + else { + onInit3(); + } + } + ); + }; + + var onInit3 = function() { + console.log("onInit3"); + + // TODO: We should be able to keep them + eventHandlerService.resetRoomMessages($scope.room_id); // Make recents highlight the current room $scope.recentsSelectedRoomID = $scope.room_id; - - // Join the room - matrixService.join($scope.room_id).then( - function() { - console.log("Joined room "+$scope.room_id); - - // Get the current member list - matrixService.getMemberList($scope.room_id).then( - function(response) { - for (var i = 0; i < response.data.chunk.length; i++) { - var chunk = response.data.chunk[i]; - updateMemberList(chunk); - } - }, - function(error) { - $scope.feedback = "Failed get member list: " + error.data.error; - } - ); - - paginate(MESSAGES_PER_PAGINATION); - }, - function(reason) { - $scope.feedback = "Can't join room: " + reason; - }); + + paginate(MESSAGES_PER_PAGINATION); }; $scope.inviteUser = function(user_id) { From bddc1d9fff2ab749b5946f44d52ed0670c1ce801 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 28 Aug 2014 14:56:03 +0100 Subject: [PATCH 09/16] use @wraps to set the __name__ __module__ and __doc__ correctly for logged functions --- synapse/util/logutils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/util/logutils.py b/synapse/util/logutils.py index 9270a1790b..021649071b 100644 --- a/synapse/util/logutils.py +++ b/synapse/util/logutils.py @@ -15,6 +15,7 @@ from inspect import getcallargs +from functools import wraps import logging @@ -26,6 +27,7 @@ def log_function(f): lineno = f.func_code.co_firstlineno pathname = f.func_code.co_filename + @wraps(f) def wrapped(*args, **kwargs): name = f.__module__ logger = logging.getLogger(name) From 7b079a26a5bca2c4e22e34f3792d1cdd2230a95e Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 28 Aug 2014 15:32:30 +0100 Subject: [PATCH 10/16] Remove get_state_for_room function from federation handler --- synapse/handlers/federation.py | 26 +++++++++++--------------- tests/handlers/test_federation.py | 10 ++++++++-- tests/utils.py | 10 ++++++++++ 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 606e1f1817..1cc820fb5b 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -84,12 +84,6 @@ class FederationHandler(BaseHandler): yield self.replication_layer.send_pdu(pdu) - @log_function - def get_state_for_room(self, destination, room_id): - return self.replication_layer.get_state_for_context( - destination, room_id - ) - @log_function @defer.inlineCallbacks def on_receive_pdu(self, pdu, backfilled): @@ -139,7 +133,7 @@ class FederationHandler(BaseHandler): yield self.hs.get_handlers().room_member_handler.change_membership( new_event, - True + do_auth=True ) else: @@ -151,8 +145,8 @@ class FederationHandler(BaseHandler): if not room: # Huh, let's try and get the current state try: - yield self.get_state_for_room( - event.origin, event.room_id + yield self.replication_layer.get_state_for_context( + origin, event.room_id ) hosts = yield self.store.get_joined_hosts_for_room( @@ -161,9 +155,9 @@ class FederationHandler(BaseHandler): if self.hs.hostname in hosts: try: yield self.store.store_room( - event.room_id, - "", - is_public=False + room_id=event.room_id, + room_creator_user_id="", + is_public=False, ) except: pass @@ -209,7 +203,9 @@ class FederationHandler(BaseHandler): # First get current state to see if we are already joined. try: - yield self.get_state_for_room(target_host, room_id) + yield self.replication_layer.get_state_for_context( + target_host, room_id + ) hosts = yield self.store.get_joined_hosts_for_room(room_id) if self.hs.hostname in hosts: @@ -239,8 +235,8 @@ class FederationHandler(BaseHandler): try: yield self.store.store_room( - room_id, - "", + room_id=room_id, + room_creator_user_id="", is_public=False ) except: diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index bc260c8aab..fd19442645 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -28,6 +28,8 @@ from mock import NonCallableMock, ANY import logging +from ..utils import get_mock_call_args + logging.getLogger().addHandler(logging.NullHandler()) @@ -99,9 +101,13 @@ class FederationTestCase(unittest.TestCase): mem_handler = self.handlers.room_member_handler self.assertEquals(1, mem_handler.change_membership.call_count) - self.assertEquals(True, mem_handler.change_membership.call_args[0][1]) + call_args = get_mock_call_args( + lambda event, do_auth: None, + mem_handler.change_membership + ) + self.assertEquals(True, call_args["do_auth"]) - new_event = mem_handler.change_membership.call_args[0][0] + new_event = call_args["event"] self.assertEquals(RoomMemberEvent.TYPE, new_event.type) self.assertEquals(room_id, new_event.room_id) self.assertEquals(user_id, new_event.state_key) diff --git a/tests/utils.py b/tests/utils.py index 6666b06931..b32d5ef356 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -28,6 +28,16 @@ from mock import patch, Mock import json import urlparse +from inspect import getcallargs + + +def get_mock_call_args(pattern_func, mock_func): + """ Return the arguments the mock function was called with interpreted + by the pattern functions argument list. + """ + invoked_args, invoked_kargs = mock_func.call_args + return getcallargs(pattern_func, *invoked_args, **invoked_kargs) + # This is a mock /resource/ not an entire server class MockHttpResource(HttpServer): From 62dfa3c7415623c2ea5e49025571fc85325e91c0 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 28 Aug 2014 15:35:20 +0100 Subject: [PATCH 11/16] Flesh out m.room.message msgtypes --- docs/specification.rst | 110 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 101 insertions(+), 9 deletions(-) diff --git a/docs/specification.rst b/docs/specification.rst index 30e4a7a3fb..fa085bac27 100644 --- a/docs/specification.rst +++ b/docs/specification.rst @@ -68,15 +68,108 @@ Non-state messages What are they, when are they used, what do they contain, how should they be used -m.room.message types --------------------- -- m.text -- m.emote -- m.audio -- m.image -- m.video -- m.location +m.room.message msgtypes +----------------------- +Each ``m.room.message`` MUST have a ``msgtype`` key which identifies the type of +message being sent. Each type has their own required and optional keys, as outlined +below: +``m.text`` + Required keys: + - ``body`` : "string" - The body of the message. + Optional keys: + None. + Example: + ``{ "msgtype": "m.text", "body": "I am a fish" }`` + +``m.emote`` + Required keys: + - ``body`` : "string" - The emote action to perform. + Optional keys: + None. + Example: + ``{ "msgtype": "m.emote", "body": "tries to come up with a witty explanation" }`` + +``m.image`` + Required keys: + - ``url`` : "string" - The URL to the image. + Optional keys: + - ``info`` : "string" - info : JSON object (ImageInfo) - The image info for image + referred to in ``url``. + - ``thumbnail_url`` : "string" - The URL to the thumbnail. + - ``thumbnail_info`` : JSON object (ImageInfo) - The image info for the image + referred to in ``thumbnail_url``. + - ``body`` : "string" - The alt text of the image, or some kind of content + description for accessibility e.g. "image attachment". + +ImageInfo: + Information about an image:: + + { + "size" : integer (size of image in bytes), + "w" : integer (width of image in pixels), + "h" : integer (height of image in pixels), + "mimetype" : "string (e.g. image/jpeg)", + } + +``m.audio`` + Required keys: + - ``url`` : "string" - The URL to the audio. + Optional keys: + - ``info`` : JSON object (AudioInfo) - The audio info for the audio referred to in + ``url``. + - ``body`` : "string" - A description of the audio e.g. "Bee Gees - + Stayin' Alive", or some kind of content description for accessibility e.g. + "audio attachment". + +AudioInfo: + Information about a piece of audio:: + + { + "mimetype" : "string (e.g. audio/aac)", + "size" : integer (size of audio in bytes), + "duration" : integer (duration of audio in milliseconds), + } + +``m.video`` + Required keys: + - ``url`` : "string" - The URL to the video. + Optional keys: + - ``info`` : JSON object (VideoInfo) - The video info for the video referred to in + ``url``. + - ``body`` : "string" - A description of the video e.g. "Gangnam style", + or some kind of content description for accessibility e.g. "video attachment". + +VideoInfo: + Information about a video:: + + { + "mimetype" : "string (e.g. video/mp4)", + "size" : integer (size of video in bytes), + "duration" : integer (duration of video in milliseconds), + "w" : integer (width of video in pixels), + "h" : integer (height of video in pixels), + "thumbnail_url" : "string (URL to image)", + "thumbanil_info" : JSON object (ImageInfo) + } + +``m.location`` + Required keys: + - ``geo_uri`` : "string" - The geo URI representing the location. + Optional keys: + - ``thumbnail_url`` : "string" - The URL to a thumnail of the location being + represented. + - ``thumbnail_info`` : JSON object (ImageInfo) - The image info for the image + referred to in ``thumbnail_url``. + - ``body`` : "string" - A description of the location e.g. "Big Ben, + London, UK", or some kind of content description for accessibility e.g. + "location attachment". + +The following keys can be attached to any ``m.room.message``: + + Optional keys: + - ``sender_ts`` : integer - A timestamp (ms resolution) representing the + wall-clock time when the message was sent from the client. Presence ======== @@ -203,7 +296,6 @@ requests to that home server as a query parameter 'access_token'. - TODO Kegan : Make registration like login (just omit the "user" key on the initial request?) -- TODO Kegan : Allow alternative forms of login (>1 route) If the client has already registered, they need to be able to login to their account. The home server may provide many different ways of logging in, such From b09e531159c56a8f08c5ead81dbba40f923db822 Mon Sep 17 00:00:00 2001 From: Emmanuel ROHEE Date: Thu, 28 Aug 2014 16:38:00 +0200 Subject: [PATCH 12/16] Do a smart update of the recents from the events stream rather than hammering initialSync each time --- webclient/recents/recents-controller.js | 28 ++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/webclient/recents/recents-controller.js b/webclient/recents/recents-controller.js index e182a3ad23..803ab420f9 100644 --- a/webclient/recents/recents-controller.js +++ b/webclient/recents/recents-controller.js @@ -25,13 +25,24 @@ angular.module('RecentsController', ['matrixService', 'eventHandlerService']) // in order to highlight a specific room in the list $scope.recentsSelectedRoomID; - // Refresh the list on matrix invitation and message event - $scope.$on(eventHandlerService.MEMBER_EVENT, function(ngEvent, event, isLive) { - refresh(); - }); - $scope.$on(eventHandlerService.MSG_EVENT, function(ngEvent, event, isLive) { - refresh(); - }); + var listenToEventStream = function() { + // Refresh the list on matrix invitation and message event + $scope.$on(eventHandlerService.MEMBER_EVENT, function(ngEvent, event, isLive) { + var config = matrixService.config(); + if (event.state_key === config.user_id && event.content.membership === "invite") { + console.log("Invited to room " + event.room_id); + // FIXME push membership to top level key to match /im/sync + event.membership = event.content.membership; + // FIXME bodge a nicer name than the room ID for this invite. + event.room_display_name = event.user_id + "'s room"; + $scope.rooms[event.room_id] = event; + } + }); + $scope.$on(eventHandlerService.MSG_EVENT, function(ngEvent, event, isLive) { + $scope.rooms[event.room_id].lastMsg = event; + }); + }; + var refresh = function() { // List all rooms joined or been invited to @@ -56,6 +67,9 @@ angular.module('RecentsController', ['matrixService', 'eventHandlerService']) for (var i = 0; i < presence.length; ++i) { eventHandlerService.handleEvent(presence[i], false); } + + // From now, update recents from the stream + listenToEventStream(); }, function(error) { $scope.feedback = "Failure: " + error.data; From c46c8061261c365934d378e52bf721cf400a3303 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 28 Aug 2014 10:50:39 +0100 Subject: [PATCH 13/16] Re-enable presence, un-skip presence tests --- synapse/handlers/presence.py | 8 -------- tests/handlers/test_presence.py | 6 ------ tests/handlers/test_presencelike.py | 3 --- tests/rest/test_presence.py | 2 -- 4 files changed, 19 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index c479908f61..1b3cdcc38c 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -141,10 +141,6 @@ class PresenceHandler(BaseHandler): @defer.inlineCallbacks def is_presence_visible(self, observer_user, observed_user): - defer.returnValue(True) - return - # FIXME (erikj): This code path absolutely kills the database. - assert(observed_user.is_mine) if observer_user == observed_user: @@ -189,10 +185,6 @@ class PresenceHandler(BaseHandler): @defer.inlineCallbacks def set_state(self, target_user, auth_user, state): - return - # TODO (erikj): Turn this back on. Why did we end up sending EDUs - # everywhere? - if not target_user.is_mine: raise SynapseError(400, "User is not hosted on this Home Server") diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 824ed07169..13217d456b 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -192,7 +192,6 @@ class PresenceStateTestCase(unittest.TestCase): ), SynapseError ) - test_get_disallowed_state.skip = "Presence polling is disabled" @defer.inlineCallbacks def test_set_my_state(self): @@ -217,7 +216,6 @@ class PresenceStateTestCase(unittest.TestCase): state={"state": OFFLINE}) self.mock_stop.assert_called_with(self.u_apple) - test_set_my_state.skip = "Presence polling is disabled" class PresenceInvitesTestCase(unittest.TestCase): @@ -657,7 +655,6 @@ class PresencePushTestCase(unittest.TestCase): observed_user=self.u_banana, statuscache=ANY), # self-reflection ]) # and no others... - test_push_local.skip = "Presence polling is disabled" @defer.inlineCallbacks def test_push_remote(self): @@ -709,7 +706,6 @@ class PresencePushTestCase(unittest.TestCase): ) yield put_json.await_calls() - test_push_remote.skip = "Presence polling is disabled" @defer.inlineCallbacks def test_recv_remote(self): @@ -1002,7 +998,6 @@ class PresencePollingTestCase(unittest.TestCase): self.assertFalse("banana" in self.handler._local_pushmap) self.assertFalse("clementine" in self.handler._local_pushmap) - test_push_local.skip = "Presence polling is disabled" @defer.inlineCallbacks @@ -1052,7 +1047,6 @@ class PresencePollingTestCase(unittest.TestCase): put_json.await_calls() self.assertFalse(self.u_potato in self.handler._remote_recvmap) - test_remote_poll_send.skip = "Presence polling is disabled" @defer.inlineCallbacks def test_remote_poll_receive(self): diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py index 1b106fc2b3..da06a06647 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -139,7 +139,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): mocked_set.assert_called_with("apple", {"state": UNAVAILABLE, "status_msg": "Away"}) - test_set_my_state.skip = "Presence polling is disabled" @defer.inlineCallbacks def test_push_local(self): @@ -214,7 +213,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): "displayname": "I am an Apple", "avatar_url": "http://foo", }, statuscache.state) - test_push_local.skip = "Presence polling is disabled" @defer.inlineCallbacks @@ -246,7 +244,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): ], }, ) - test_push_remote.skip = "Presence polling is disabled" @defer.inlineCallbacks def test_recv_remote(self): diff --git a/tests/rest/test_presence.py b/tests/rest/test_presence.py index e15ee38741..7f7347dcf9 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -114,7 +114,6 @@ class PresenceStateTestCase(unittest.TestCase): self.assertEquals(200, code) mocked_set.assert_called_with("apple", {"state": UNAVAILABLE, "status_msg": "Away"}) - test_set_my_status.skip = "Presence polling is disabled" class PresenceListTestCase(unittest.TestCase): @@ -318,4 +317,3 @@ class PresenceEventStreamTestCase(unittest.TestCase): "mtime_age": 0, }}, ]}, response) - test_shortpoll.skip = "Presence polling is disabled" From b1da3fa0a78ab7c6af8c274b6c2beafb7a5a751a Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 28 Aug 2014 16:19:16 +0100 Subject: [PATCH 14/16] Avoid AlreadyCalledError from EDU sending failures --- synapse/federation/replication.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py index 38ae360bcd..7868575a2e 100644 --- a/synapse/federation/replication.py +++ b/synapse/federation/replication.py @@ -541,7 +541,8 @@ class _TransactionQueue(object): ) def eb(failure): - deferred.errback(failure) + if not deferred.called: + deferred.errback(failure) self._attempt_new_transaction(destination).addErrback(eb) return deferred From 113342a7568ff3d019de5099880671417bf4ecf2 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 28 Aug 2014 16:40:06 +0100 Subject: [PATCH 15/16] Ability to assert a DeferredMockCallable has received no calls --- tests/utils.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/utils.py b/tests/utils.py index b32d5ef356..98d4f9ed58 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -248,8 +248,11 @@ class DeferredMockCallable(object): def __init__(self): self.expectations = [] + self.calls = [] def __call__(self, *args, **kwargs): + self.calls.append((args, kwargs)) + if not self.expectations: raise ValueError("%r has no pending calls to handle call(%s)" % ( self, _format_call(args, kwargs)) @@ -272,3 +275,15 @@ class DeferredMockCallable(object): while self.expectations: (_, _, d) = self.expectations.pop(0) yield d + self.calls = [] + + def assert_had_no_calls(self): + if self.calls: + calls = self.calls + self.calls = [] + + raise AssertionError("Expected not to received any calls, got:\n" + + "\n".join([ + "call(%s)" % _format_call(c[0], c[1]) for c in calls + ]) + ) From efc5f3440dc033d9d1713eaa7159b75689704d6c Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 28 Aug 2014 16:43:55 +0100 Subject: [PATCH 16/16] Only send presence "poll"/"unpoll" EDUs when changing from/to zero remotes --- synapse/handlers/presence.py | 16 ++++++++++++-- tests/handlers/test_presence.py | 39 ++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 1b3cdcc38c..bef1508892 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -437,16 +437,22 @@ class PresenceHandler(BaseHandler): ) def _start_polling_remote(self, user, domain, remoteusers): + to_poll = set() + for u in remoteusers: if u not in self._remote_recvmap: self._remote_recvmap[u] = set() + to_poll.add(u) self._remote_recvmap[u].add(user) + if not to_poll: + return defer.succeed(None) + return self.federation.send_edu( destination=domain, edu_type="m.presence", - content={"poll": [u.to_string() for u in remoteusers]} + content={"poll": [u.to_string() for u in to_poll]} ) def stop_polling_presence(self, user, target_user=None): @@ -489,16 +495,22 @@ class PresenceHandler(BaseHandler): del self._local_pushmap[localpart] def _stop_polling_remote(self, user, domain, remoteusers): + to_unpoll = set() + for u in remoteusers: self._remote_recvmap[u].remove(user) if not self._remote_recvmap[u]: del self._remote_recvmap[u] + to_unpoll.add(u) + + if not to_unpoll: + return defer.succeed(None) return self.federation.send_edu( destination=domain, edu_type="m.presence", - content={"unpoll": [u.to_string() for u in remoteusers]} + content={"unpoll": [u.to_string() for u in to_unpoll]} ) @defer.inlineCallbacks diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 13217d456b..8d094fd1f9 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -15,7 +15,7 @@ from twisted.trial import unittest -from twisted.internet import defer +from twisted.internet import defer, reactor from mock import Mock, call, ANY import logging @@ -853,6 +853,7 @@ class PresencePollingTestCase(unittest.TestCase): 'apple': [ "@banana:test", "@clementine:test" ], 'banana': [ "@apple:test" ], 'clementine': [ "@apple:test", "@potato:remote" ], + 'fig': [ "@potato:remote" ], } @@ -902,9 +903,10 @@ class PresencePollingTestCase(unittest.TestCase): # Mocked database state # Local users always start offline self.current_user_state = { - "apple": OFFLINE, - "banana": OFFLINE, - "clementine": OFFLINE, + "apple": OFFLINE, + "banana": OFFLINE, + "clementine": OFFLINE, + "fig": OFFLINE, } def get_presence_state(user_localpart): @@ -934,6 +936,7 @@ class PresencePollingTestCase(unittest.TestCase): self.u_apple = hs.parse_userid("@apple:test") self.u_banana = hs.parse_userid("@banana:test") self.u_clementine = hs.parse_userid("@clementine:test") + self.u_fig = hs.parse_userid("@fig:test") # Remote users self.u_potato = hs.parse_userid("@potato:remote") @@ -1023,10 +1026,32 @@ class PresencePollingTestCase(unittest.TestCase): yield put_json.await_calls() # Gut-wrenching tests - self.assertTrue(self.u_potato in self.handler._remote_recvmap) + self.assertTrue(self.u_potato in self.handler._remote_recvmap, + msg="expected potato to be in _remote_recvmap" + ) self.assertTrue(self.u_clementine in self.handler._remote_recvmap[self.u_potato]) + # fig goes online; shouldn't send a second poll + yield self.handler.set_state( + target_user=self.u_fig, auth_user=self.u_fig, + state={"state": ONLINE} + ) + + reactor.iterate(delay=0) + + put_json.assert_had_no_calls() + + # fig goes offline + yield self.handler.set_state( + target_user=self.u_fig, auth_user=self.u_fig, + state={"state": OFFLINE} + ) + + reactor.iterate(delay=0) + + put_json.assert_had_no_calls() + put_json.expect_call_and_return( call("remote", path="/matrix/federation/v1/send/1000001/", @@ -1046,7 +1071,9 @@ class PresencePollingTestCase(unittest.TestCase): put_json.await_calls() - self.assertFalse(self.u_potato in self.handler._remote_recvmap) + self.assertFalse(self.u_potato in self.handler._remote_recvmap, + msg="expected potato not to be in _remote_recvmap" + ) @defer.inlineCallbacks def test_remote_poll_receive(self):