From 0fb307d09b23a7310fbdb44d60a87b03c816f3f5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 3 Nov 2015 10:15:30 +0000 Subject: [PATCH 01/27] Use the history length specified in startClient() for room initial syncs. --- lib/client.js | 3 ++- lib/models/room.js | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index c3c4a93b8..a0e513a73 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2176,7 +2176,8 @@ function _syncRoom(client, room) { } var defer = q.defer(); client._syncingRooms[room.roomId] = defer.promise; - client.roomInitialSync(room.roomId, 8).done(function(res) { + client.roomInitialSync(room.roomId, client._config.initialSyncLimit).done( + function(res) { room.timeline = []; // blow away any previous messages. _processRoomEvents(client, room, res.state, res.messages); room.recalculate(client.credentials.userId); diff --git a/lib/models/room.js b/lib/models/room.js index 15cdf602e..61bbc7ffa 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -326,7 +326,6 @@ Room.prototype.getUsersReadUpTo = function(event) { * have received no read receipts from them. * @param {String} userId The user ID to get read receipt event ID for * @return {String} ID of the latest event that the given user has read, or null. - * an empty list. */ Room.prototype.getEventReadUpTo = function(userId) { if ( From 3b21998d968415c279a80fc05df697271ce15ce9 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 3 Nov 2015 10:18:56 +0000 Subject: [PATCH 02/27] Expose timeout= on /events to clients --- lib/client.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index a0e513a73..bccbe38e8 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1999,6 +1999,8 @@ function doInitialSync(client, historyLen, includeArchived) { * @param {Boolean} opts.resolveInvitesToProfiles True to do /profile requests * on every invite event if the displayname/avatar_url is not known for this user ID. * Default: false. + * @param {Number} opts.pollTimeout The number of milliseconds to wait on /events. + * Default: 30000 (30 seconds). */ MatrixClient.prototype.startClient = function(opts) { if (this.clientRunning) { @@ -2016,6 +2018,7 @@ MatrixClient.prototype.startClient = function(opts) { opts.initialSyncLimit = opts.initialSyncLimit || 8; opts.includeArchivedRooms = opts.includeArchivedRooms || false; opts.resolveInvitesToProfiles = opts.resolveInvitesToProfiles || false; + opts.pollTimeout = opts.pollTimeout || (30 * 1000); this._config = opts; if (CRYPTO_ENABLED && this.sessionStore !== null) { @@ -2054,11 +2057,11 @@ function _pollForEvents(client) { discardResult = true; console.error("/events request timed out."); _pollForEvents(client); - }, 40000); + }, client._config.pollTimeout + (20 * 1000)); // 20s buffer client._http.authedRequest(undefined, "GET", "/events", { from: client.store.getSyncToken(), - timeout: 30000 + timeout: client._config.pollTimeout }).done(function(data) { if (discardResult) { return; From 142ee81e6698ff0f191525a4af8c45dac4f2b71a Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 3 Nov 2015 11:43:52 +0000 Subject: [PATCH 03/27] Emit events for read receipts --- lib/client.js | 2 +- lib/models/room.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 1c597c2cb..69f288d63 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2466,7 +2466,7 @@ function createNewUser(client, userId) { function createNewRoom(client, roomId) { var room = new Room(roomId); - reEmit(client, room, ["Room.name", "Room.timeline"]); + reEmit(client, room, ["Room.name", "Room.timeline", "Room.receipt"]); // we need to also re-emit room state and room member events, so hook it up // to the client now. We need to add a listener for RoomState.members in diff --git a/lib/models/room.js b/lib/models/room.js index 15cdf602e..908e4ef15 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -399,6 +399,10 @@ Room.prototype.addReceipt = function(event) { }); }); }); + + // send events after we've regenerated the cache, otherwise things that + // listened for the event would read from a stale cache + this.emit("Room.receipt", event, this); }; function setEventMetadata(event, stateContext, toStartOfTimeline) { @@ -527,3 +531,14 @@ module.exports = Room; * var newName = room.name; * }); */ + +/** + * Fires whenever a receipt is received for a room + * @event module:client~MatrixClient#"Room.receipt" + * @param {event} event The receipt event + * @param {Room} room The room whose Room.name was updated. + * @example + * matrixClient.on("Room.receipt", function(event, room){ + * var receiptContent = event.getContent(); + * }); + */ From 49f6634d73cd5793b0c69be22d487d7354fd1041 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 3 Nov 2015 14:01:17 +0000 Subject: [PATCH 04/27] Retry /initialSync if it fails (exp backoff up to 2.1min). --- lib/client.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index bccbe38e8..ea4f2339e 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1878,8 +1878,10 @@ MatrixClient.prototype.isLoggedIn = function() { * @param {MatrixClient} client * @param {integer} historyLen * @param {integer} includeArchived + * @param {integer} attempt */ -function doInitialSync(client, historyLen, includeArchived) { +function doInitialSync(client, historyLen, includeArchived, attempt) { + attempt = attempt || 1; var qps = { limit: historyLen }; if (includeArchived) { qps.archived = true; @@ -1980,8 +1982,12 @@ function doInitialSync(client, historyLen, includeArchived) { client.emit("syncComplete"); _pollForEvents(client); }, function(err) { - console.error("/initialSync error: %s", err); + console.error("/initialSync error (%s attempts): %s", attempt, err); client.emit("syncError", err); + attempt += 1; + setTimeout(function() { + doInitialSync(client, historyLen, includeArchived, attempt); + }, Math.pow(2, Math.min(attempt, 7)) * 1000); // max 2^7 secs = 2.1 mins // TODO: Retries. }); } From 27ce0970c50e5375b6e3decacc2a22b2fa2c3265 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 3 Nov 2015 14:35:49 +0000 Subject: [PATCH 05/27] BREAKING: Introduce a formal API for syncing state BREAKING CHANGE: This replaces syncComplete and syncError. --- lib/client.js | 74 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/lib/client.js b/lib/client.js index ea4f2339e..a5394e86a 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2598,23 +2598,65 @@ module.exports.CRYPTO_ENABLED = CRYPTO_ENABLED; */ /** - * Fires whenever the SDK has a problem syncing. This event is experimental - * and may change. - * @event module:client~MatrixClient#"syncError" - * @param {MatrixError} err The matrix error which caused this event to fire. + * Fires whenever the SDK's syncing state is updated. The state can be one of: + *
    + *
  • PREPARED : The client has synced with the server at least once and is + * ready for methods to be called on it. This will be immediately followed by + * a state of SYNCING. This is the equivalent of "syncComplete" in the + * previous API.
  • + *
  • SYNCING : The client is currently polling for new events from the server.
  • + *
  • ERROR : The client has had a problem syncing with the server. If this is + * called before PREPARED then there was a problem performing the initial + * sync. If this is called after PREPARED then there was a problem polling + * the server for updates. This is the equivalent of "syncError" in the previous + * API.
  • + *
+ * State transition diagram: + *
+ *              +----->PREPARED -------> SYNCING <--+
+ *              |        ^                  |       |
+ *   null ------+        |  +----------------+      |
+ *              |        |  V                       |
+ *              +------->ERROR ---------------------+
+ *
+ * NB: 'null' will never be emitted by this event.
+ * 
+ * Transitions: + *
    + *
  • null -> PREPARED : Occurs when the initial sync is completed + * first time. + *
  • null -> ERROR : Occurs when the initial sync failed first time. + *
  • ERROR -> PREPARED : Occurs when the initial sync succeeds + * after previously failing. + *
  • PREPARED -> SYNCING : Occurs immediately after transitioning + * to PREPARED. Starts listening for live updates rather than catching up. + *
  • SYNCING -> ERROR : Occurs the first time a client cannot perform a + * live update. + *
  • ERROR -> SYNCING : Occurs when the client has performed a + * live update after having previously failed. + *
+ * + * @event module:client~MatrixClient#"sync" + * @param {string} state An enum representing the syncing state. One of "PREPARED", + * "SYNCING", "ERROR". + * @param {?string} prevState An enum representing the previous syncing state. + * One of "PREPARED", "SYNCING", "ERROR" or null. + * @param {?Object} data Data about this transition. + * @param {MatrixError} data.err The matrix error if state=ERROR. * @example - * matrixClient.on("syncError", function(err){ - * // update UI to say "Connection Lost" - * }); - */ - -/** - * Fires when the SDK has finished catching up and is now listening for live - * events. This event is experimental and may change. - * @event module:client~MatrixClient#"syncComplete" - * @example - * matrixClient.on("syncComplete", function(){ - * var rooms = matrixClient.getRooms(); + * matrixClient.on("sync", function(state, prevState, data) { + * switch (state) { + * case "ERROR": + * // update UI to say "Connection Lost" + * break; + * case "SYNCING": + * // update UI to remove any "Connection Lost" message + * break; + * case "PREPARED": + * // the client instance is ready to be queried. + * var rooms = matrixClient.getRooms(); + * break; + * } * }); */ From 4b93d801aec1d67cd2a8dfa58e42808f313f145b Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 3 Nov 2015 16:44:19 +0000 Subject: [PATCH 06/27] Implement the new sync state API Also have retry schemes for the rest of the syncing ops (/events, /pushrules) --- lib/client.js | 89 ++++++++++++++----- .../integ/matrix-client-room-timeline.spec.js | 33 ++++--- 2 files changed, 89 insertions(+), 33 deletions(-) diff --git a/lib/client.js b/lib/client.js index a5394e86a..cc2e5f321 100644 --- a/lib/client.js +++ b/lib/client.js @@ -151,7 +151,7 @@ function MatrixClient(opts) { setupCallEventHandler(this); this._supportsVoip = true; } - + this._syncState = null; } utils.inherits(MatrixClient, EventEmitter); @@ -179,6 +179,15 @@ MatrixClient.prototype.supportsVoip = function() { return this._supportsVoip; }; +/** + * Get the current sync state. + * @return {?string} the sync state, which may be null. + * @see module:client~MatrixClient#event:"sync" + */ +MatrixClient.prototype.getSyncState = function() { + return this._syncState; +}; + /** * Is end-to-end crypto enabled for this client. * @return {boolean} True if end-to-end is enabled. @@ -1979,16 +1988,17 @@ function doInitialSync(client, historyLen, includeArchived, attempt) { } client.clientRunning = true; - client.emit("syncComplete"); + updateSyncState(client, "PREPARED"); + // assume success until we fail which may be 30+ secs + updateSyncState(client, "SYNCING"); _pollForEvents(client); }, function(err) { console.error("/initialSync error (%s attempts): %s", attempt, err); - client.emit("syncError", err); + updateSyncState(client, "ERROR", { error: err }); attempt += 1; setTimeout(function() { doInitialSync(client, historyLen, includeArchived, attempt); - }, Math.pow(2, Math.min(attempt, 7)) * 1000); // max 2^7 secs = 2.1 mins - // TODO: Retries. + }, retryTimeMsForAttempt(attempt)); }); } @@ -2040,20 +2050,34 @@ MatrixClient.prototype.startClient = function(opts) { // periodically poll for turn servers if we support voip checkTurnServers(this); - var self = this; - this.pushRules().done(function(result) { - self.pushRules = result; - doInitialSync(self, opts.initialSyncLimit, opts.includeArchivedRooms); - }, function(err) { - self.emit("syncError", err); - }); + prepareForSync(this); }; +function prepareForSync(client, attempt) { + attempt = attempt || 1; + client.pushRules().done(function(result) { + client.pushRules = result; + doInitialSync( + client, + client._config.initialSyncLimit, + client._config.includeArchivedRooms + ); + }, function(err) { + updateSyncState(client, "ERROR", { error: err }); + attempt += 1; + setTimeout(function() { + prepareForSync(client, attempt); + }, retryTimeMsForAttempt(attempt)); + }); +} + /** * This is an internal method. * @param {MatrixClient} client + * @param {Number} attempt The attempt number */ -function _pollForEvents(client) { +function _pollForEvents(client, attempt) { + attempt = attempt || 1; var self = client; if (!client.clientRunning) { return; @@ -2075,6 +2099,11 @@ function _pollForEvents(client) { else { clearTimeout(timeoutObj); } + + if (self._syncState !== "SYNCING") { + updateSyncState(self, "SYNCING"); + } + try { var events = []; if (data) { @@ -2170,12 +2199,12 @@ function _pollForEvents(client) { else { clearTimeout(timeoutObj); } - self.emit("syncError", err); - // retry every few seconds - // FIXME: this should be exponential backoff with an option to nudge + + updateSyncState(self, "ERROR", { error: err }); + attempt += 1; setTimeout(function() { - _pollForEvents(self); - }, 2000); + _pollForEvents(self, attempt); + }, retryTimeMsForAttempt(attempt)); }); } @@ -2455,6 +2484,12 @@ function setupCallEventHandler(client) { }); } +function updateSyncState(client, newState, data) { + var old = client._syncState; + client._syncState = newState; + client.emit("sync", client._syncState, old, data); +} + function checkTurnServers(client) { if (!client._supportsVoip) { return; @@ -2511,6 +2546,12 @@ function createNewRoom(client, roomId) { return room; } +function retryTimeMsForAttempt(attempt) { + // 2,4,8,16,32,64,128,128,128,... seconds + // max 2^7 secs = 2.1 mins + return Math.pow(2, Math.min(attempt, 7)) * 1000; +} + function _reject(callback, defer, err) { if (callback) { callback(err); @@ -2604,18 +2645,20 @@ module.exports.CRYPTO_ENABLED = CRYPTO_ENABLED; * ready for methods to be called on it. This will be immediately followed by * a state of SYNCING. This is the equivalent of "syncComplete" in the * previous API. - *
  • SYNCING : The client is currently polling for new events from the server.
  • + *
  • SYNCING : The client is currently polling for new events from the server. + * The client may fire this before or after processing latest events from a sync.
  • *
  • ERROR : The client has had a problem syncing with the server. If this is * called before PREPARED then there was a problem performing the initial * sync. If this is called after PREPARED then there was a problem polling - * the server for updates. This is the equivalent of "syncError" in the previous + * the server for updates. This may be called multiple times even if the state is + * already ERROR. This is the equivalent of "syncError" in the previous * API.
  • * * State transition diagram: *
      *              +----->PREPARED -------> SYNCING <--+
      *              |        ^                  |       |
    - *   null ------+        |  +----------------+      |
    + *   null ------+        |  +---------------+       |
      *              |        |  V                       |
      *              +------->ERROR ---------------------+
      *
    @@ -2634,8 +2677,10 @@ module.exports.CRYPTO_ENABLED = CRYPTO_ENABLED;
      * live update.
      * 
  • ERROR -> SYNCING : Occurs when the client has performed a * live update after having previously failed. + *
  • ERROR -> ERROR : Occurs when the client has failed to sync + * for a second time or more.
  • * - * + * * @event module:client~MatrixClient#"sync" * @param {string} state An enum representing the syncing state. One of "PREPARED", * "SYNCING", "ERROR". diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index d56658f95..89bdd04ac 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -82,7 +82,8 @@ describe("MatrixClient room timelines", function() { it("should be added immediately after calling MatrixClient.sendEvent " + "with EventStatus.SENDING and the right event.sender", function(done) { - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); expect(room.timeline.length).toEqual(1); @@ -116,7 +117,8 @@ describe("MatrixClient room timelines", function() { ]; eventData.chunk[0].event_id = eventId; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); client.sendTextMessage(roomId, "I am a fish", "txn1").done( function() { @@ -144,7 +146,8 @@ describe("MatrixClient room timelines", function() { ]; eventData.chunk[0].event_id = eventId; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); var promise = client.sendTextMessage(roomId, "I am a fish", "txn1"); httpBackend.flush("/events", 1).done(function() { @@ -180,7 +183,8 @@ describe("MatrixClient room timelines", function() { it("should set Room.oldState.paginationToken to null at the start" + " of the timeline.", function(done) { - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); expect(room.timeline.length).toEqual(1); @@ -219,7 +223,8 @@ describe("MatrixClient room timelines", function() { }) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); expect(room.timeline.length).toEqual(1); @@ -249,7 +254,8 @@ describe("MatrixClient room timelines", function() { }) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); expect(room.timeline.length).toEqual(1); @@ -274,7 +280,8 @@ describe("MatrixClient room timelines", function() { }) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); expect(room.oldState.paginationToken).toBeDefined(); @@ -297,7 +304,8 @@ describe("MatrixClient room timelines", function() { utils.mkMessage({user: userId, room: roomId}), utils.mkMessage({user: userId, room: roomId}) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); var index = 0; @@ -331,7 +339,8 @@ describe("MatrixClient room timelines", function() { }), utils.mkMessage({user: userId, room: roomId}) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); httpBackend.flush("/events", 1).done(function() { var preNameEvent = room.timeline[room.timeline.length - 3]; @@ -352,7 +361,8 @@ describe("MatrixClient room timelines", function() { } }) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); var nameEmitCount = 0; client.on("Room.name", function(rm) { @@ -392,7 +402,8 @@ describe("MatrixClient room timelines", function() { user: userC, room: roomId, mship: "invite", skey: userD }) ]; - client.on("syncComplete", function() { + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } var room = client.getRoom(roomId); httpBackend.flush("/events", 1).done(function() { expect(room.currentState.getMembers().length).toEqual(4); From e98eaaee6e2291cf7ace45b82567e26d267af582 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 3 Nov 2015 17:13:50 +0000 Subject: [PATCH 07/27] Add MatrixClient.retryImmediately() to stop backing off and sync RIGHT NOW --- lib/client.js | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/lib/client.js b/lib/client.js index cc2e5f321..14db9fda7 100644 --- a/lib/client.js +++ b/lib/client.js @@ -152,6 +152,7 @@ function MatrixClient(opts) { this._supportsVoip = true; } this._syncState = null; + this._syncingRetry = null; } utils.inherits(MatrixClient, EventEmitter); @@ -188,6 +189,23 @@ MatrixClient.prototype.getSyncState = function() { return this._syncState; }; +/** + * Retry a backed off syncing request immediately. This should only be used when + * the user explicitly attempts to retry their lost connection. + * @return {boolean} True if this resulted in a request being retried. + */ +MatrixClient.prototype.retryImmediately = function() { + if (!this._syncingRetry) { + return false; + } + // stop waiting + clearTimeout(this._syncingRetry.timeoutId); + // invoke immediately + this._syncingRetry.fn(); + this._syncingRetry = null; + return true; +}; + /** * Is end-to-end crypto enabled for this client. * @return {boolean} True if end-to-end is enabled. @@ -1996,9 +2014,9 @@ function doInitialSync(client, historyLen, includeArchived, attempt) { console.error("/initialSync error (%s attempts): %s", attempt, err); updateSyncState(client, "ERROR", { error: err }); attempt += 1; - setTimeout(function() { + startSyncingRetryTimer(client, attempt, function() { doInitialSync(client, historyLen, includeArchived, attempt); - }, retryTimeMsForAttempt(attempt)); + }); }); } @@ -2065,9 +2083,9 @@ function prepareForSync(client, attempt) { }, function(err) { updateSyncState(client, "ERROR", { error: err }); attempt += 1; - setTimeout(function() { + startSyncingRetryTimer(client, attempt, function() { prepareForSync(client, attempt); - }, retryTimeMsForAttempt(attempt)); + }); }); } @@ -2202,9 +2220,9 @@ function _pollForEvents(client, attempt) { updateSyncState(self, "ERROR", { error: err }); attempt += 1; - setTimeout(function() { + startSyncingRetryTimer(self, attempt, function() { _pollForEvents(self, attempt); - }, retryTimeMsForAttempt(attempt)); + }); }); } @@ -2484,6 +2502,14 @@ function setupCallEventHandler(client) { }); } +function startSyncingRetryTimer(client, attempt, fn) { + client._syncingRetry = {}; + client._syncingRetry.fn = fn; + client._syncingRetry.timeoutId = setTimeout(function() { + fn(); + }, retryTimeMsForAttempt(attempt)); +} + function updateSyncState(client, newState, data) { var old = client._syncState; client._syncState = newState; From 5c3bfa6a83689bafe404041987250c455c82a5f9 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 4 Nov 2015 11:50:32 +0000 Subject: [PATCH 08/27] Add stub unit tests for syncing --- spec/unit/matrix-client.spec.js | 77 +++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 spec/unit/matrix-client.spec.js diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js new file mode 100644 index 000000000..cbb448afd --- /dev/null +++ b/spec/unit/matrix-client.spec.js @@ -0,0 +1,77 @@ +"use strict"; +var sdk = require("../.."); +var MatrixClient = sdk.MatrixClient; +var utils = require("../test-utils"); + +describe("MatrixClient", function() { + var userId = "@alice:bar"; + var client; + + beforeEach(function() { + utils.beforeEach(this); + }); + + describe("getSyncState", function() { + + it("should return null if the client isn't started", function() { + + }); + + it("should return the same sync state as emitted sync events", function() { + + }); + }); + + describe("retryImmediately", function() { + it("should return false if there is no request waiting", function() { + + }); + + it("should return true if there is a request waiting", function() { + + }); + + it("should work on /initialSync", function() { + + }); + + it("should work on /events", function() { + + }); + + it("should work on /pushrules", function() { + + }); + }); + + describe("emitted sync events", function() { + + it("should transition null -> PREPARED after /initialSync", function() { + + }); + + it("should transition null -> ERROR after a failed /initialSync", function() { + + }); + + it("should transition ERROR -> PREPARED after /initialSync if prev failed", function() { + + }); + + it("should transition PREPARED -> SYNCING after /initialSync", function() { + + }); + + it("should transition SYNCING -> ERROR after a failed /events", function() { + + }); + + it("should transition ERROR -> SYNCING after /events if prev failed", function() { + + }); + + it("should transition ERROR -> ERROR if multiple /events fails", function() { + + }); + }); +}); \ No newline at end of file From c9df9c33a8bc245f0dfb4379c850550dcb45a853 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 4 Nov 2015 11:53:10 +0000 Subject: [PATCH 09/27] Linting --- spec/unit/matrix-client.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index cbb448afd..cd02266b4 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -54,7 +54,8 @@ describe("MatrixClient", function() { }); - it("should transition ERROR -> PREPARED after /initialSync if prev failed", function() { + it("should transition ERROR -> PREPARED after /initialSync if prev failed", + function() { }); @@ -74,4 +75,4 @@ describe("MatrixClient", function() { }); }); -}); \ No newline at end of file +}); From 904539df58801a1f99ac94b37cabad1df7896e2a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 4 Nov 2015 12:02:02 +0000 Subject: [PATCH 10/27] Fix c+p fail & add unit test --- lib/models/room.js | 2 +- spec/unit/room.spec.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/models/room.js b/lib/models/room.js index 908e4ef15..724ac559f 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -536,7 +536,7 @@ module.exports = Room; * Fires whenever a receipt is received for a room * @event module:client~MatrixClient#"Room.receipt" * @param {event} event The receipt event - * @param {Room} room The room whose Room.name was updated. + * @param {Room} room The room whose receipts was updated. * @example * matrixClient.on("Room.receipt", function(event, room){ * var receiptContent = event.getContent(); diff --git a/spec/unit/room.spec.js b/spec/unit/room.spec.js index c18b51c8b..b92d89421 100644 --- a/spec/unit/room.spec.js +++ b/spec/unit/room.spec.js @@ -684,6 +684,21 @@ describe("Room", function() { }]); }); + it("should emit an event when a receipt is added", + function() { + var listener = jasmine.createSpy('spy'); + room.on("Room.receipt", listener); + + var ts = 13787898424; + + var receiptEvent = mkReceipt(roomId, [ + mkRecord(eventToAck.getId(), "m.read", userB, ts) + ]); + + room.addReceipt(receiptEvent); + expect(listener).toHaveBeenCalledWith(receiptEvent, room); + }); + it("should clobber receipts based on type and user ID", function() { var nextEventToAck = utils.mkMessage({ room: roomId, user: userA, msg: "I AM HERE YOU KNOW", From bc512a6e4c2e86eb8dac014620fddc37f21d9a14 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 4 Nov 2015 15:20:25 +0000 Subject: [PATCH 11/27] Check m.room.name event actually has a name in the content before using it. This should fix the recent disasters with #android being shown as 'undefined' (or crashing vector). --- lib/models/room.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/room.js b/lib/models/room.js index 724ac559f..9ee1184f4 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -438,7 +438,7 @@ function calculateRoomName(room, userId) { // check for an alias, if any. for now, assume first alias is the // official one. var mRoomName = room.currentState.getStateEvents("m.room.name", ""); - if (mRoomName) { + if (mRoomName && mRoomName.getContent() && mRoomName.getContent().name) { return mRoomName.getContent().name; } From e42f6c0cad768abbcda20261ec0b3c5677fdb266 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 4 Nov 2015 15:35:31 +0000 Subject: [PATCH 12/27] Add http fixings to allow MatrixClient UTs --- spec/unit/matrix-client.spec.js | 108 ++++++++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 4 deletions(-) diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index cd02266b4..596be3464 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -1,24 +1,124 @@ "use strict"; +var q = require("q"); var sdk = require("../.."); var MatrixClient = sdk.MatrixClient; var utils = require("../test-utils"); describe("MatrixClient", function() { var userId = "@alice:bar"; - var client; + var client, store, scheduler; + + var initialSyncData = { + end: "s_5_3", + presence: [], + rooms: [] + }; + + var eventData = { + start: "s_START", + end: "s_END", + chunk: [] + }; + + var httpLookups = [ + // items are objects which look like: + // { + // method: "GET", + // path: "/initialSync", + // data: {}, + // error: { errcode: M_FORBIDDEN } // if present will reject promise + // } + // items are popped off when processed and block if no items left. + ]; + var pendingLookup = {}; + function httpReq(cb, method, path, qp, data, prefix) { + var next = httpLookups.shift(); + var logLine = ( + "MatrixClient[UT] RECV " + method + " " + path + " " + + "EXPECT " + (next ? next.method : next) + " " + (next ? next.path : next) + ); + console.log(logLine); + + if (!next) { // no more things to return + pendingLookup = { + promise: q.defer().promise, + method: method, + path: path + }; + return pendingLookup.promise; + } + if (next.path === path && next.method === method) { + console.log( + "MatrixClient[UT] Matched. Returning " + + (next.error ? "BAD" : "GOOD") + " response" + ); + if (next.error) { + return q.reject({ + errcode: next.error.errcode, + name: next.error.errcode, + message: "Expected testing error", + data: next.error + }); + } + return q(next.data); + } + expect(true).toBe(false, "Expected different request. " + logLine); + return q.defer().promise; + } beforeEach(function() { utils.beforeEach(this); + scheduler = jasmine.createSpyObj("scheduler", [ + "getQueueForEvent", "queueEvent", "removeEventFromQueue", + "setProcessFunction" + ]); + store = jasmine.createSpyObj("store", [ + "getRoom", "getRooms", "getUser", "getSyncToken", "scrollback", + "setSyncToken", "storeEvents", "storeRoom", "storeUser" + ]); + client = new MatrixClient({ + baseUrl: "https://my.home.server", + accessToken: "my.access.token", + request: function() {}, // NOP + store: store, + scheduler: scheduler + }); + // FIXME: We shouldn't be yanking _http like this. + client._http = jasmine.createSpyObj("httpApi", [ + "authedRequest", "authedRequestWithPrefix", "getContentUri", + "request", "requestWithPrefix", "uploadContent" + ]); + client._http.authedRequest.andCallFake(httpReq); + client._http.authedRequestWithPrefix.andCallFake(httpReq); + + // set reasonable working defaults + pendingLookup = {}; + httpLookups = []; + httpLookups.push({ + method: "GET", path: "/pushrules/", data: {} + }); + httpLookups.push({ + method: "GET", path: "/initialSync", data: initialSyncData + }); + httpLookups.push({ + method: "GET", path: "/events", data: eventData + }); }); describe("getSyncState", function() { it("should return null if the client isn't started", function() { - + expect(client.getSyncState()).toBeNull(); }); - it("should return the same sync state as emitted sync events", function() { - + it("should return the same sync state as emitted sync events", function(done) { + client.on("sync", function(state) { + expect(state).toEqual(client.getSyncState()); + if (state === "SYNCING") { + done(); + } + }); + client.startClient(); }); }); From b4c353e65fe21e6fb8f1c631d5f46f3400870c69 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 4 Nov 2015 15:37:10 +0000 Subject: [PATCH 13/27] Linting --- spec/unit/matrix-client.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index 596be3464..4249d2d81 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -81,7 +81,8 @@ describe("MatrixClient", function() { accessToken: "my.access.token", request: function() {}, // NOP store: store, - scheduler: scheduler + scheduler: scheduler, + userId: userId }); // FIXME: We shouldn't be yanking _http like this. client._http = jasmine.createSpyObj("httpApi", [ From af435204a07c5a57540307dfb168504124f47a41 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 4 Nov 2015 15:40:42 +0000 Subject: [PATCH 14/27] More helpful logging --- spec/unit/matrix-client.spec.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index 4249d2d81..ecb3996b9 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -30,7 +30,7 @@ describe("MatrixClient", function() { // } // items are popped off when processed and block if no items left. ]; - var pendingLookup = {}; + var pendingLookup = null; function httpReq(cb, method, path, qp, data, prefix) { var next = httpLookups.shift(); var logLine = ( @@ -40,6 +40,14 @@ describe("MatrixClient", function() { console.log(logLine); if (!next) { // no more things to return + if (pendingLookup) { + // >1 pending thing, whine. + expect(false).toBe( + true, ">1 pending request. You should probably handle them. " + + "PENDING: " + JSON.stringify(pendingLookup) + " JUST GOT: " + + method + " " + path + ); + } pendingLookup = { promise: q.defer().promise, method: method, @@ -93,7 +101,7 @@ describe("MatrixClient", function() { client._http.authedRequestWithPrefix.andCallFake(httpReq); // set reasonable working defaults - pendingLookup = {}; + pendingLookup = null; httpLookups = []; httpLookups.push({ method: "GET", path: "/pushrules/", data: {} From 5d782a317c39871642b78040344f5e2997d43f83 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 4 Nov 2015 16:09:30 +0000 Subject: [PATCH 15/27] Add some sync emission tests. Emit after starting timers. We want to emit AFTER starting the timers so tests can speed up time. We also want to do this because clients may want to retryImmediately() on sync errors (which would be lost unless the timer had already been started) --- lib/client.js | 6 +-- spec/unit/matrix-client.spec.js | 66 ++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/lib/client.js b/lib/client.js index 14db9fda7..101e0d4e5 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2012,11 +2012,11 @@ function doInitialSync(client, historyLen, includeArchived, attempt) { _pollForEvents(client); }, function(err) { console.error("/initialSync error (%s attempts): %s", attempt, err); - updateSyncState(client, "ERROR", { error: err }); attempt += 1; startSyncingRetryTimer(client, attempt, function() { doInitialSync(client, historyLen, includeArchived, attempt); }); + updateSyncState(client, "ERROR", { error: err }); }); } @@ -2081,11 +2081,11 @@ function prepareForSync(client, attempt) { client._config.includeArchivedRooms ); }, function(err) { - updateSyncState(client, "ERROR", { error: err }); attempt += 1; startSyncingRetryTimer(client, attempt, function() { prepareForSync(client, attempt); }); + updateSyncState(client, "ERROR", { error: err }); }); } @@ -2218,11 +2218,11 @@ function _pollForEvents(client, attempt) { clearTimeout(timeoutObj); } - updateSyncState(self, "ERROR", { error: err }); attempt += 1; startSyncingRetryTimer(self, attempt, function() { _pollForEvents(self, attempt); }); + updateSyncState(self, "ERROR", { error: err }); }); } diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index ecb3996b9..a046d76f5 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -20,6 +20,10 @@ describe("MatrixClient", function() { chunk: [] }; + var PUSH_RULES_RESPONSE = { + method: "GET", path: "/pushrules/", data: {} + }; + var httpLookups = [ // items are objects which look like: // { @@ -76,6 +80,7 @@ describe("MatrixClient", function() { beforeEach(function() { utils.beforeEach(this); + jasmine.Clock.useMock(); scheduler = jasmine.createSpyObj("scheduler", [ "getQueueForEvent", "queueEvent", "removeEventFromQueue", "setProcessFunction" @@ -103,9 +108,7 @@ describe("MatrixClient", function() { // set reasonable working defaults pendingLookup = null; httpLookups = []; - httpLookups.push({ - method: "GET", path: "/pushrules/", data: {} - }); + httpLookups.push(PUSH_RULES_RESPONSE); httpLookups.push({ method: "GET", path: "/initialSync", data: initialSyncData }); @@ -155,17 +158,62 @@ describe("MatrixClient", function() { describe("emitted sync events", function() { - it("should transition null -> PREPARED after /initialSync", function() { - + it("should transition null -> PREPARED after /initialSync", function(done) { + // the first sync emitted should be null > prep + client.once("sync", function(state, old) { + expect(state).toEqual("PREPARED"); + expect(old).toBeNull(); + done(); + }); + client.startClient(); }); - it("should transition null -> ERROR after a failed /initialSync", function() { - + it("should transition null -> ERROR after a failed /initialSync", function(done) { + httpLookups = []; + httpLookups.push(PUSH_RULES_RESPONSE); + httpLookups.push({ + method: "GET", path: "/initialSync", error: { errcode: "NOPE_NOPE_NOPE" } + }); + // the first sync emitted should be null > prep + client.once("sync", function(state, old) { + expect(state).toEqual("ERROR"); + expect(old).toBeNull(); + done(); + // FIXME: need to make next req tick else it pollutes other tests + jasmine.Clock.tick(10000); + }); + client.startClient(); }); it("should transition ERROR -> PREPARED after /initialSync if prev failed", - function() { - + function(done) { + httpLookups = []; + httpLookups.push(PUSH_RULES_RESPONSE); + httpLookups.push({ + method: "GET", path: "/initialSync", error: { errcode: "NOPE_NOPE_NOPE" } + }); + httpLookups.push({ + method: "GET", path: "/initialSync", data: initialSyncData + }); + var states = [ + // current, old + ["ERROR", null], + ["PREPARED", "ERROR"] + ]; + client.on("sync", function(state, old) { + var expected = states.shift(); + if (!expected) { + done(); + return; + } + expect(state).toEqual(expected[0]); + expect(old).toEqual(expected[1]); + if (expected.length === 0) { + done(); + } + jasmine.Clock.tick(10000); + }); + client.startClient(); }); it("should transition PREPARED -> SYNCING after /initialSync", function() { From 8500f404a9e1ea03ee2cf176209e516ac3f68fdf Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 5 Nov 2015 13:12:37 +0000 Subject: [PATCH 16/27] Finish implementing UTs --- spec/unit/matrix-client.spec.js | 193 ++++++++++++++++++++++++-------- 1 file changed, 145 insertions(+), 48 deletions(-) diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index a046d76f5..d21d1ea06 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -45,7 +45,10 @@ describe("MatrixClient", function() { if (!next) { // no more things to return if (pendingLookup) { - // >1 pending thing, whine. + if (pendingLookup.method === method && pendingLookup.path === path) { + return pendingLookup.promise; + } + // >1 pending thing, and they are different, whine. expect(false).toBe( true, ">1 pending request. You should probably handle them. " + "PENDING: " + JSON.stringify(pendingLookup) + " JUST GOT: " + @@ -112,8 +115,19 @@ describe("MatrixClient", function() { httpLookups.push({ method: "GET", path: "/initialSync", data: initialSyncData }); - httpLookups.push({ - method: "GET", path: "/events", data: eventData + }); + + afterEach(function() { + // need to re-stub the requests with NOPs because there are no guarantees + // clients from previous tests will be GC'd before the next test. This + // means they may call /events and then fail an expect() which will fail + // a DIFFERENT test (pollution between tests!) - we return unresolved + // promises to stop the client from continuing to run. + client._http.authedRequest.andCallFake(function() { + return q.defer().promise; + }); + client._http.authedRequestWithPrefix.andCallFake(function() { + return q.defer().promise; }); }); @@ -136,35 +150,103 @@ describe("MatrixClient", function() { describe("retryImmediately", function() { it("should return false if there is no request waiting", function() { - + client.startClient(); + expect(client.retryImmediately()).toBe(false); }); - it("should return true if there is a request waiting", function() { + it("should work on /initialSync", function(done) { + httpLookups = []; + httpLookups.push(PUSH_RULES_RESPONSE); + httpLookups.push({ + method: "GET", path: "/initialSync", error: { errcode: "NOPE_NOPE_NOPE" } + }); + httpLookups.push({ + method: "GET", path: "/initialSync", error: { errcode: "NOPE_NOPE_NOPE" } + }); + client.on("sync", function(state) { + if (state === "ERROR" && httpLookups.length > 0) { + expect(httpLookups.length).toEqual(1); + expect(client.retryImmediately()).toBe(true); + expect(httpLookups.length).toEqual(0); + done(); + } + }); + client.startClient(); }); - it("should work on /initialSync", function() { + it("should work on /events", function(done) { + httpLookups.push({ + method: "GET", path: "/events", error: { errcode: "NOPE_NOPE_NOPE" } + }); + httpLookups.push({ + method: "GET", path: "/events", data: eventData + }); + client.on("sync", function(state) { + if (state === "ERROR" && httpLookups.length > 0) { + expect(httpLookups.length).toEqual(1); + expect(client.retryImmediately()).toBe(true); + expect(httpLookups.length).toEqual(0); + done(); + } + }); + client.startClient(); }); - it("should work on /events", function() { - - }); - - it("should work on /pushrules", function() { + it("should work on /pushrules", function(done) { + httpLookups = []; + httpLookups.push({ + method: "GET", path: "/pushrules/", error: { errcode: "NOPE_NOPE_NOPE" } + }); + httpLookups.push({ + method: "GET", path: "/pushrules/", error: { errcode: "NOPE_NOPE_NOPE" } + }); + client.on("sync", function(state) { + if (state === "ERROR" && httpLookups.length > 0) { + expect(httpLookups.length).toEqual(1); + expect(client.retryImmediately()).toBe(true); + expect(httpLookups.length).toEqual(0); + done(); + } + }); + client.startClient(); }); }); describe("emitted sync events", function() { + var expectedStates; + + function syncChecker(done) { + return function(state, old) { + var expected = expectedStates.shift(); + console.log( + "'sync' curr=%s old=%s EXPECT=%s", state, old, expected + ); + if (!expected) { + done(); + return; + } + expect(state).toEqual(expected[0]); + expect(old).toEqual(expected[1]); + if (expectedStates.length === 0) { + done(); + } + // standard retry time is 4s + jasmine.Clock.tick(4001); + }; + } + + beforeEach(function() { + expectedStates = [ + // [current, old] + ]; + }); it("should transition null -> PREPARED after /initialSync", function(done) { - // the first sync emitted should be null > prep - client.once("sync", function(state, old) { - expect(state).toEqual("PREPARED"); - expect(old).toBeNull(); - done(); - }); + expectedStates.push(["PREPARED", null]); + client.on("sync", syncChecker(done)); client.startClient(); }); @@ -174,14 +256,8 @@ describe("MatrixClient", function() { httpLookups.push({ method: "GET", path: "/initialSync", error: { errcode: "NOPE_NOPE_NOPE" } }); - // the first sync emitted should be null > prep - client.once("sync", function(state, old) { - expect(state).toEqual("ERROR"); - expect(old).toBeNull(); - done(); - // FIXME: need to make next req tick else it pollutes other tests - jasmine.Clock.tick(10000); - }); + expectedStates.push(["ERROR", null]); + client.on("sync", syncChecker(done)); client.startClient(); }); @@ -195,41 +271,62 @@ describe("MatrixClient", function() { httpLookups.push({ method: "GET", path: "/initialSync", data: initialSyncData }); - var states = [ - // current, old - ["ERROR", null], - ["PREPARED", "ERROR"] - ]; - client.on("sync", function(state, old) { - var expected = states.shift(); - if (!expected) { - done(); - return; - } - expect(state).toEqual(expected[0]); - expect(old).toEqual(expected[1]); - if (expected.length === 0) { - done(); - } - jasmine.Clock.tick(10000); - }); + + expectedStates.push(["ERROR", null]); + expectedStates.push(["PREPARED", "ERROR"]); + client.on("sync", syncChecker(done)); client.startClient(); }); - it("should transition PREPARED -> SYNCING after /initialSync", function() { - + it("should transition PREPARED -> SYNCING after /initialSync", function(done) { + expectedStates.push(["PREPARED", null]); + expectedStates.push(["SYNCING", "PREPARED"]); + client.on("sync", syncChecker(done)); + client.startClient(); }); - it("should transition SYNCING -> ERROR after a failed /events", function() { + it("should transition SYNCING -> ERROR after a failed /events", function(done) { + httpLookups.push({ + method: "GET", path: "/events", error: { errcode: "NONONONONO" } + }); + expectedStates.push(["PREPARED", null]); + expectedStates.push(["SYNCING", "PREPARED"]); + expectedStates.push(["ERROR", "SYNCING"]); + client.on("sync", syncChecker(done)); + client.startClient(); }); - it("should transition ERROR -> SYNCING after /events if prev failed", function() { + it("should transition ERROR -> SYNCING after /events if prev failed", + function(done) { + httpLookups.push({ + method: "GET", path: "/events", error: { errcode: "NONONONONO" } + }); + httpLookups.push({ + method: "GET", path: "/events", data: eventData + }); + expectedStates.push(["PREPARED", null]); + expectedStates.push(["SYNCING", "PREPARED"]); + expectedStates.push(["ERROR", "SYNCING"]); + client.on("sync", syncChecker(done)); + client.startClient(); }); - it("should transition ERROR -> ERROR if multiple /events fails", function() { + it("should transition ERROR -> ERROR if multiple /events fails", function(done) { + httpLookups.push({ + method: "GET", path: "/events", error: { errcode: "NONONONONO" } + }); + httpLookups.push({ + method: "GET", path: "/events", error: { errcode: "NONONONONO" } + }); + expectedStates.push(["PREPARED", null]); + expectedStates.push(["SYNCING", "PREPARED"]); + expectedStates.push(["ERROR", "SYNCING"]); + expectedStates.push(["ERROR", "ERROR"]); + client.on("sync", syncChecker(done)); + client.startClient(); }); }); }); From 16278892d8f2135d39f60e77c32a2817cb6413ca Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 5 Nov 2015 13:26:16 +0000 Subject: [PATCH 17/27] Modify how detection of the end of pagination is done Synapse may filter down the events resulting in < 'limit' events being returned *but it still has more events*. Change the check to see if the request returned an empty array instead. This may add an extra HTTP hit. --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 84213fbfe..96632ca63 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1638,7 +1638,7 @@ MatrixClient.prototype.scrollback = function(room, limit, callback) { var matrixEvents = utils.map(res.chunk, _PojoToMatrixEventMapper(self)); room.addEventsToTimeline(matrixEvents, true); room.oldState.paginationToken = res.end; - if (res.chunk.length < limit) { + if (res.chunk.length === 0) { room.oldState.paginationToken = null; } self.store.storeEvents(room, matrixEvents, res.end, true); From 0da547a239e652e1c1cf296d4bac2e92b97ceea6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 13:39:03 +0000 Subject: [PATCH 18/27] Implicit read receipts * Inject implicit read receipts into the timeline * Twiddle local echo a bit to make the implicit receipts match the various different stages of local echo. --- lib/client.js | 9 ++++++--- lib/models/room.js | 42 +++++++++++++++++++++++++++++++++++++++++- lib/utils.js | 6 ++++-- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/lib/client.js b/lib/client.js index 69f288d63..2f7c4219d 100644 --- a/lib/client.js +++ b/lib/client.js @@ -946,6 +946,9 @@ function _sendEvent(client, room, event, callback) { // the fake event we made above. If we don't find it, we're still // waiting on the real event and so should assign the fake event // with the real event_id for matching later. + + // FIXME: This manipulation of the room should probably be done + // inside the room class, not by the client. var matchingEvent = utils.findElement(room.timeline, function(ev) { return ev.getId() === eventId; }, true); @@ -958,13 +961,13 @@ function _sendEvent(client, room, event, callback) { matchingEvent.event.content = event.event.content; matchingEvent.event.type = event.event.type; } - utils.removeElement(room.timeline, function(ev) { - return ev.getId() === event.getId(); - }, true); + room.removeEvents([event.getId()]); } else { + room.removeEvents([event.getId()]); event.event.event_id = res.event_id; event.status = null; + room.addEventsToTimeline([event]); } } diff --git a/lib/models/room.js b/lib/models/room.js index 724ac559f..3abf8f580 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -194,7 +194,26 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline) { else { this.timeline.push(events[i]); } - this.emit("Room.timeline", events[i], this, Boolean(toStartOfTimeline)); + + // synthesize and inject implicit read receipts + // Done after adding the event because otherwise the app would get a read receipt + // pointing to an event that wasn't yet in the timeline + + // This is really ugly because JS has no way to express an object literal where the + // name of a key comes from an expression + if (events[i].sender) { + var fakeReceipt = {content: {}}; + fakeReceipt.content[events[i].getId()] = { + 'm.read': { + } + }; + fakeReceipt.content[events[i].getId()]['m.read'][events[i].sender.userId] = { + ts: events[i].getTs() + }; + this.addReceipt(new MatrixEvent(fakeReceipt)); + } + + this.emit("Room.timeline", events[i], this, Boolean(toStartOfTimeline), false); } }; @@ -260,6 +279,26 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { } }; +/** + * Removes events from this room. + * @param {String} event_ids A list of event_ids to remove. + */ +Room.prototype.removeEvents = function(event_ids) { + for (var i = 0; i < event_ids.length; ++i) { + // NB. we supply reverse to search from the end, + // on the assumption that recents events are much + // more likley to be removed than older ones. + var removed = utils.removeElement( + this.timeline, function(e) { + return e.getId() == event_ids[i]; + }, true + ); + if (removed !== false) { + this.emit("Room.timeline", removed, this, undefined, true); + } + } +}; + /** * Recalculate various aspects of the room, including the room name and * room summary. Call this any time the room's current state is modified. @@ -513,6 +552,7 @@ module.exports = Room; * @param {MatrixEvent} event The matrix event which caused this event to fire. * @param {Room} room The room whose Room.timeline was updated. * @param {boolean} toStartOfTimeline True if this event was added to the start + * @param {boolean} removed True if this event has just been removed from the timeline * (beginning; oldest) of the timeline e.g. due to pagination. * @example * matrixClient.on("Room.timeline", function(event, room, toStartOfTimeline){ diff --git a/lib/utils.js b/lib/utils.js index a18a1dc21..141aef734 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -155,16 +155,18 @@ module.exports.removeElement = function(array, fn, reverse) { if (reverse) { for (i = array.length - 1; i >= 0; i--) { if (fn(array[i], i, array)) { + var removed = array[i]; array.splice(i, 1); - return true; + return removed; } } } else { for (i = 0; i < array.length; i++) { if (fn(array[i], i, array)) { + var removed = array[i]; array.splice(i, 1); - return true; + return removed; } } } From ad80d4f0598c8acdc118a68deb1d8c05f57f88f2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 13:57:21 +0000 Subject: [PATCH 19/27] fix lint errors --- lib/models/room.js | 13 ++++++++----- lib/utils.js | 5 +++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index 3abf8f580..eb6ff3ae9 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -199,8 +199,8 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline) { // Done after adding the event because otherwise the app would get a read receipt // pointing to an event that wasn't yet in the timeline - // This is really ugly because JS has no way to express an object literal where the - // name of a key comes from an expression + // This is really ugly because JS has no way to express an object literal + // where the name of a key comes from an expression if (events[i].sender) { var fakeReceipt = {content: {}}; fakeReceipt.content[events[i].getId()] = { @@ -284,14 +284,17 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { * @param {String} event_ids A list of event_ids to remove. */ Room.prototype.removeEvents = function(event_ids) { + // avoids defining a function in the loop, which is a lint error + function eq(a, b) { + return a === b; + } + for (var i = 0; i < event_ids.length; ++i) { // NB. we supply reverse to search from the end, // on the assumption that recents events are much // more likley to be removed than older ones. var removed = utils.removeElement( - this.timeline, function(e) { - return e.getId() == event_ids[i]; - }, true + this.timeline, eq.bind(event_ids[i]), true ); if (removed !== false) { this.emit("Room.timeline", removed, this, undefined, true); diff --git a/lib/utils.js b/lib/utils.js index 141aef734..955903fff 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -152,10 +152,11 @@ module.exports.findElement = function(array, fn, reverse) { */ module.exports.removeElement = function(array, fn, reverse) { var i; + var removed; if (reverse) { for (i = array.length - 1; i >= 0; i--) { if (fn(array[i], i, array)) { - var removed = array[i]; + removed = array[i]; array.splice(i, 1); return removed; } @@ -164,7 +165,7 @@ module.exports.removeElement = function(array, fn, reverse) { else { for (i = 0; i < array.length; i++) { if (fn(array[i], i, array)) { - var removed = array[i]; + removed = array[i]; array.splice(i, 1); return removed; } From 856c34016d947618228c38d16abd508394848916 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 14:13:52 +0000 Subject: [PATCH 20/27] Fix event removal --- lib/models/room.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index eb6ff3ae9..0099ded81 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -285,17 +285,17 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { */ Room.prototype.removeEvents = function(event_ids) { // avoids defining a function in the loop, which is a lint error - function eq(a, b) { - return a === b; - } - - for (var i = 0; i < event_ids.length; ++i) { + function remove_event_with_id(timeline, id) { // NB. we supply reverse to search from the end, // on the assumption that recents events are much // more likley to be removed than older ones. - var removed = utils.removeElement( - this.timeline, eq.bind(event_ids[i]), true - ); + return utils.removeElement(timeline, function(e) { + return e.getId() == id; + }, true); + } + + for (var i = 0; i < event_ids.length; ++i) { + var removed = remove_event_with_id(this.timeline, event_ids[i]); if (removed !== false) { this.emit("Room.timeline", removed, this, undefined, true); } From 483095c3da6bbb863006dcbd3af33a3227c8c7a2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 14:41:35 +0000 Subject: [PATCH 21/27] Fix PR comments --- lib/models/room.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index 0099ded81..2692455ab 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -285,7 +285,7 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { */ Room.prototype.removeEvents = function(event_ids) { // avoids defining a function in the loop, which is a lint error - function remove_event_with_id(timeline, id) { + function removeEventWithId(timeline, id) { // NB. we supply reverse to search from the end, // on the assumption that recents events are much // more likley to be removed than older ones. @@ -295,8 +295,8 @@ Room.prototype.removeEvents = function(event_ids) { } for (var i = 0; i < event_ids.length; ++i) { - var removed = remove_event_with_id(this.timeline, event_ids[i]); - if (removed !== false) { + var removed = removeEventWithId(this.timeline, event_ids[i]); + if (removed) { this.emit("Room.timeline", removed, this, undefined, true); } } From d241f5b3eb6f9f9bbd269e9cd5db146f51c3cf4a Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Thu, 5 Nov 2015 14:51:23 +0000 Subject: [PATCH 22/27] Add login with token method --- lib/client.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/client.js b/lib/client.js index cf67155c7..5d11df353 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1746,6 +1746,18 @@ MatrixClient.prototype.loginWithCas = function(ticket, service, callback) { }, callback); }; +/** + * @param {string} token Login token previously received from homeserver + * @param {module:client.callback} callback Optional. + * @return {module:client.Promise} Resolves: TODO + * @return {module:http-api.MatrixError} Rejects: with an error response. + */ +MatrixClient.prototype.loginWithToken = function(token, callback) { + return this.login("m.login.token", { + token: token + }, callback); +}; + // Push operations // =============== From 21e56d2f53a422d93cb66ac2efd067ee4bdba217 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 5 Nov 2015 15:48:48 +0000 Subject: [PATCH 23/27] Tweak RETRY_BACKOFF_RATELIMIT to take browser-request's CORS failures into account. --- lib/scheduler.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/scheduler.js b/lib/scheduler.js index e78e5dfe3..1f6da08b6 100644 --- a/lib/scheduler.js +++ b/lib/scheduler.js @@ -133,6 +133,12 @@ MatrixScheduler.RETRY_BACKOFF_RATELIMIT = function(event, attempts, err) { // client error; no amount of retrying with save you now. return -1; } + // we ship with browser-request which returns { cors: rejected } when trying + // with no connection, so if we match that, give up since they have no conn. + if (err.cors === "rejected") { + return -1; + } + if (err.name === "M_LIMIT_EXCEEDED") { var waitTime = err.data.retry_after_ms; if (waitTime) { From c3097979f282e03da512326f9f5dd69ad46f9e7e Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Thu, 5 Nov 2015 15:19:04 +0000 Subject: [PATCH 24/27] Change login with CAS to redirect to HS for CAS login --- lib/client.js | 26 +++++--------------------- lib/http-api.js | 9 +++++++++ 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/lib/client.js b/lib/client.js index 5d11df353..685e3d8ca 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1722,28 +1722,12 @@ MatrixClient.prototype.loginWithSAML2 = function(relayState, callback) { }; /** - * @param {module:client.callback} callback Optional. - * @return {module:client.Promise} Resolves: TODO - * @return {module:http-api.MatrixError} Rejects: with an error response. + * @param {string} redirectUrl URL to which to be redirect to after + * HS authenticates with CAS and issues login token + * Will redirect to homeserver to complete CAS login */ -MatrixClient.prototype.getCasServer = function(callback) { - return this._http.request( - callback, "GET", "/login/cas", undefined, undefined - ); -}; - -/** - * @param {string} ticket (Received from CAS) - * @param {string} service Service to which the token was granted - * @param {module:client.callback} callback Optional. - * @return {module:client.Promise} Resolves: TODO - * @return {module:http-api.MatrixError} Rejects: with an error response. - */ -MatrixClient.prototype.loginWithCas = function(ticket, service, callback) { - return this.login("m.login.cas", { - ticket: ticket, - service: service - }, callback); +MatrixClient.prototype.loginWithCas = function(redirectUrl) { + this._http.redirect("/login/cas/redirect", {"redirectUrl": redirectUrl}, httpApi.PREFIX_V1); }; /** diff --git a/lib/http-api.js b/lib/http-api.js index 09b3d4d38..03c498691 100644 --- a/lib/http-api.js +++ b/lib/http-api.js @@ -296,6 +296,15 @@ module.exports.MatrixHttpApi.prototype = { return this._request(callback, method, fullUri, queryParams, data); }, + redirect: function(path, queryParams, prefix) { + var queryString = ""; + if (queryParams) { + queryString = "?" + utils.encodeParams(queryParams); + } + var fullUri = this.opts.baseUrl + prefix + path + queryString; + window.location.href = fullUri; + }, + _request: function(callback, method, uri, queryParams, data) { if (callback !== undefined && !utils.isFunction(callback)) { throw Error( From b963f177cc7422fdf7e710dd5ba2c5852ba9f04a Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Fri, 6 Nov 2015 12:11:50 +0000 Subject: [PATCH 25/27] Update CAS login to return url rather than update location as the JS SDK may not be run within a browser env --- lib/client.js | 10 ++++++---- lib/http-api.js | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/client.js b/lib/client.js index 685e3d8ca..db2cc57a2 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1722,12 +1722,14 @@ MatrixClient.prototype.loginWithSAML2 = function(relayState, callback) { }; /** - * @param {string} redirectUrl URL to which to be redirect to after + * @param {string} redirectUrl URL to which to be redirected to after * HS authenticates with CAS and issues login token - * Will redirect to homeserver to complete CAS login + * @return {string} CAS login URL */ -MatrixClient.prototype.loginWithCas = function(redirectUrl) { - this._http.redirect("/login/cas/redirect", {"redirectUrl": redirectUrl}, httpApi.PREFIX_V1); +MatrixClient.prototype.getCasLoginUrl = function(redirectUrl) { + return this._http.getUrl("/login/cas/redirect", { + "redirectUrl": redirectUrl + }, httpApi.PREFIX_V1); }; /** diff --git a/lib/http-api.js b/lib/http-api.js index 03c498691..797daa426 100644 --- a/lib/http-api.js +++ b/lib/http-api.js @@ -296,13 +296,23 @@ module.exports.MatrixHttpApi.prototype = { return this._request(callback, method, fullUri, queryParams, data); }, - redirect: function(path, queryParams, prefix) { + /** + * Form and return a homeserver request URL based on the given path + * params and prefix. + * @param {string} path The HTTP path after the supplied prefix e.g. + * "/createRoom". + * @param {Object} queryParams A dict of query params (these will NOT be + * urlencoded). + * @param {string} prefix The full prefix to use e.g. + * "/_matrix/client/v2_alpha". + * @return {string} URL + */ + getUrl: function(path, queryParams, prefix) { var queryString = ""; if (queryParams) { queryString = "?" + utils.encodeParams(queryParams); } - var fullUri = this.opts.baseUrl + prefix + path + queryString; - window.location.href = fullUri; + return this.opts.baseUrl + prefix + path + queryString; }, _request: function(callback, method, uri, queryParams, data) { From e71a87c62ccf32af1860156fe1757533d6c3e18e Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Fri, 6 Nov 2015 12:14:24 +0000 Subject: [PATCH 26/27] Update javadoc --- lib/client.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/client.js b/lib/client.js index db2cc57a2..98fdfc04e 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1722,9 +1722,9 @@ MatrixClient.prototype.loginWithSAML2 = function(relayState, callback) { }; /** - * @param {string} redirectUrl URL to which to be redirected to after - * HS authenticates with CAS and issues login token - * @return {string} CAS login URL + * @param {string} redirectUrl The URL to redirect to after the HS + * authenticates with CAS. + * @return {string} The HS URL to hit to begin the CAS login process. */ MatrixClient.prototype.getCasLoginUrl = function(redirectUrl) { return this._http.getUrl("/login/cas/redirect", { From 6736164d98fdf8a588bc90d291ddee678a79721e Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 6 Nov 2015 15:38:46 +0000 Subject: [PATCH 27/27] Make linter happy (space at end of line) --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 2fe555382..e9e68753d 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1749,7 +1749,7 @@ MatrixClient.prototype.loginWithSAML2 = function(relayState, callback) { }; /** - * @param {string} redirectUrl The URL to redirect to after the HS + * @param {string} redirectUrl The URL to redirect to after the HS * authenticates with CAS. * @return {string} The HS URL to hit to begin the CAS login process. */