diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f9369a3fe..e26aa211b66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,78 @@ +Changes in [0.10.0-rc.1](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.10.0-rc.1) (2018-03-19) +============================================================================================================ +[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.9.2...v0.10.0-rc.1) + + * Fix duplicated state events in timeline from peek + [\#630](https://github.com/matrix-org/matrix-js-sdk/pull/630) + * Create indexeddb worker when starting the store + [\#627](https://github.com/matrix-org/matrix-js-sdk/pull/627) + * Fix indexeddb logging + [\#626](https://github.com/matrix-org/matrix-js-sdk/pull/626) + * Don't do /keys/changes on incremental sync + [\#625](https://github.com/matrix-org/matrix-js-sdk/pull/625) + * Don't mark devicelist dirty unnecessarily + [\#623](https://github.com/matrix-org/matrix-js-sdk/pull/623) + * Cache the joined member count for a room state + [\#619](https://github.com/matrix-org/matrix-js-sdk/pull/619) + * Fix JS doc + [\#618](https://github.com/matrix-org/matrix-js-sdk/pull/618) + * Precompute push actions for state events + [\#617](https://github.com/matrix-org/matrix-js-sdk/pull/617) + * Fix bug where global "Never send to unverified..." is ignored + [\#616](https://github.com/matrix-org/matrix-js-sdk/pull/616) + * Intern legacy top-level 'membership' field + [\#615](https://github.com/matrix-org/matrix-js-sdk/pull/615) + * Don't synthesize RR for m.room.redaction as causes the RR to go missing. + [\#598](https://github.com/matrix-org/matrix-js-sdk/pull/598) + * Make Events create Dates on demand + [\#613](https://github.com/matrix-org/matrix-js-sdk/pull/613) + * Stop cloning events when adding to state + [\#612](https://github.com/matrix-org/matrix-js-sdk/pull/612) + * De-dup code: use the initialiseState function + [\#611](https://github.com/matrix-org/matrix-js-sdk/pull/611) + * Create sentinel members on-demand + [\#610](https://github.com/matrix-org/matrix-js-sdk/pull/610) + * Some more doc on how sentinels work + [\#609](https://github.com/matrix-org/matrix-js-sdk/pull/609) + * Migrate room encryption store to crypto store + [\#597](https://github.com/matrix-org/matrix-js-sdk/pull/597) + * add parameter to getIdentityServerUrl to strip the protocol for invites + [\#600](https://github.com/matrix-org/matrix-js-sdk/pull/600) + * Move Device Tracking Data to Crypto Store + [\#594](https://github.com/matrix-org/matrix-js-sdk/pull/594) + * Optimise pushprocessor + [\#591](https://github.com/matrix-org/matrix-js-sdk/pull/591) + * Set event error before emitting + [\#592](https://github.com/matrix-org/matrix-js-sdk/pull/592) + * Add event type for stickers [WIP] + [\#590](https://github.com/matrix-org/matrix-js-sdk/pull/590) + * Migrate inbound sessions to cryptostore + [\#587](https://github.com/matrix-org/matrix-js-sdk/pull/587) + * Disambiguate names if they contain an mxid + [\#588](https://github.com/matrix-org/matrix-js-sdk/pull/588) + * Check for sessions in indexeddb before migrating + [\#585](https://github.com/matrix-org/matrix-js-sdk/pull/585) + * Emit an event for crypto store migration + [\#586](https://github.com/matrix-org/matrix-js-sdk/pull/586) + * Supporting fixes For making UnknownDeviceDialog not pop up automatically + [\#575](https://github.com/matrix-org/matrix-js-sdk/pull/575) + * Move sessions to the crypto store + [\#584](https://github.com/matrix-org/matrix-js-sdk/pull/584) + * Change crypto store transaction API + [\#582](https://github.com/matrix-org/matrix-js-sdk/pull/582) + * Add some missed copyright notices + [\#581](https://github.com/matrix-org/matrix-js-sdk/pull/581) + * Move Olm account to IndexedDB + [\#579](https://github.com/matrix-org/matrix-js-sdk/pull/579) + * Fix logging of DecryptionErrors to be more useful + [\#580](https://github.com/matrix-org/matrix-js-sdk/pull/580) + * [BREAKING] Change the behaviour of the unverfied devices blacklist flag + [\#568](https://github.com/matrix-org/matrix-js-sdk/pull/568) + * Support set_presence=offline for syncing + [\#557](https://github.com/matrix-org/matrix-js-sdk/pull/557) + * Consider cases where the sender may not redact their own event + [\#556](https://github.com/matrix-org/matrix-js-sdk/pull/556) + Changes in [0.9.2](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.9.2) (2017-12-04) ================================================================================================ [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.9.1...v0.9.2) diff --git a/package.json b/package.json index 0a0e8ffc473..cccb161998c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-js-sdk", - "version": "0.9.2", + "version": "0.10.0-rc.1", "description": "Matrix Client-Server SDK for Javascript", "main": "index.js", "scripts": { diff --git a/spec/integ/matrix-client-syncing.spec.js b/spec/integ/matrix-client-syncing.spec.js index 9b737aaed12..a74eea53a88 100644 --- a/spec/integ/matrix-client-syncing.spec.js +++ b/spec/integ/matrix-client-syncing.spec.js @@ -437,7 +437,11 @@ describe("MatrixClient syncing", function() { }); }); - it("should correctly interpret state in incremental sync.", function() { + // XXX: This test asserts that the js-sdk obeys the spec and treats state + // events that arrive in the incremental sync as if they preceeded the + // timeline events, however this breaks peeking, so it's disabled + // (see sync.js) + xit("should correctly interpret state in incremental sync.", function() { httpBackend.when("GET", "/sync").respond(200, syncData); httpBackend.when("GET", "/sync").respond(200, nextSyncData); diff --git a/spec/unit/matrix-client.spec.js b/spec/unit/matrix-client.spec.js index c7bc164284f..8ef22f6b018 100644 --- a/spec/unit/matrix-client.spec.js +++ b/spec/unit/matrix-client.spec.js @@ -132,11 +132,12 @@ describe("MatrixClient", function() { ].reduce((r, k) => { r[k] = expect.createSpy(); return r; }, {}); store = [ "getRoom", "getRooms", "getUser", "getSyncToken", "scrollback", - "save", "setSyncToken", "storeEvents", "storeRoom", "storeUser", + "save", "wantsSave", "setSyncToken", "storeEvents", "storeRoom", "storeUser", "getFilterIdByName", "setFilterIdByName", "getFilter", "storeFilter", "getSyncAccumulator", "startup", "deleteAllData", ].reduce((r, k) => { r[k] = expect.createSpy(); return r; }, {}); store.getSavedSync = expect.createSpy().andReturn(Promise.resolve(null)); + store.getSavedSyncToken = expect.createSpy().andReturn(Promise.resolve(null)); store.setSyncData = expect.createSpy().andReturn(Promise.resolve(null)); client = new MatrixClient({ baseUrl: "https://my.home.server", diff --git a/src/client.js b/src/client.js index 6c2cef03f02..109bab32836 100644 --- a/src/client.js +++ b/src/client.js @@ -196,6 +196,9 @@ function MatrixClient(opts) { // we still want to know which rooms are encrypted even if crypto is disabled: // we don't want to start sending unencrypted events to them. this._roomList = new RoomList(this._cryptoStore, this._sessionStore); + + // The pushprocessor caches useful things, so keep one and re-use it + this._pushProcessor = new PushProcessor(this); } utils.inherits(MatrixClient, EventEmitter); utils.extend(MatrixClient.prototype, MatrixBaseApis.prototype); @@ -635,6 +638,16 @@ MatrixClient.prototype.isEventSenderVerified = async function(event) { return device.isVerified(); }; +/** + * Cancel a room key request for this event if one is ongoing and resend the + * request. + * @param {MatrxEvent} event event of which to cancel and resend the room + * key request. + */ +MatrixClient.prototype.cancelAndResendEventRoomKeyRequest = function(event) { + event.cancelAndResendKeyRequest(this._crypto); +}; + /** * Enable end-to-end encryption for a room. * @param {string} roomId The room ID to enable encryption in. @@ -1773,8 +1786,7 @@ function _membershipChange(client, roomId, userId, membership, reason, callback) */ MatrixClient.prototype.getPushActionsForEvent = function(event) { if (!event.getPushActions()) { - const pushProcessor = new PushProcessor(this); - event.setPushActions(pushProcessor.actionsForEvent(event)); + event.setPushActions(this._pushProcessor.actionsForEvent(event)); } return event.getPushActions(); }; diff --git a/src/crypto/DeviceList.js b/src/crypto/DeviceList.js index d913b426a81..fa55f2fa65b 100644 --- a/src/crypto/DeviceList.js +++ b/src/crypto/DeviceList.js @@ -92,6 +92,12 @@ export default class DeviceList { // Promise resolved when device data is saved this._savePromise = null; + // Function that resolves the save promise + this._resolveSavePromise = null; + // The time the save is scheduled for + this._savePromiseTime = null; + // The timer used to delay the save + this._saveTimer = null; } /** @@ -146,25 +152,55 @@ export default class DeviceList { * The actual save will be delayed by a short amount of time to * aggregate multiple writes to the database. * + * @param {integer} delay Time in ms before which the save actually happens. + * By default, the save is delayed for a short period in order to batch + * multiple writes, but this behaviour can be disabled by passing 0. + * * @return {Promise} true if the data was saved, false if * it was not (eg. because no changes were pending). The promise * will only resolve once the data is saved, so may take some time * to resolve. */ - async saveIfDirty() { + async saveIfDirty(delay) { if (!this._dirty) return Promise.resolve(false); + // Delay saves for a bit so we can aggregate multiple saves that happen + // in quick succession (eg. when a whole room's devices are marked as known) + if (delay === undefined) delay = 500; + + const targetTime = Date.now + delay; + if (this._savePromiseTime && targetTime < this._savePromiseTime) { + // There's a save scheduled but for after we would like: cancel + // it & schedule one for the time we want + clearTimeout(this._saveTimer); + this._saveTimer = null; + this._savePromiseTime = null; + // (but keep the save promise since whatever called save before + // will still want to know when the save is done) + } + + let savePromise = this._savePromise; + if (savePromise === null) { + savePromise = new Promise((resolve, reject) => { + this._resolveSavePromise = resolve; + }); + this._savePromise = savePromise; + } - if (this._savePromise === null) { - // Delay saves for a bit so we can aggregate multiple saves that happen - // in quick succession (eg. when a whole room's devices are marked as known) - this._savePromise = Promise.delay(500).then(() => { + if (this._saveTimer === null) { + const resolveSavePromise = this._resolveSavePromise; + this._savePromiseTime = targetTime; + this._saveTimer = setTimeout(() => { console.log('Saving device tracking data at token ' + this._syncToken); // null out savePromise now (after the delay but before the write), // otherwise we could return the existing promise when the save has // actually already happened. Likewise for the dirty flag. + this._savePromiseTime = null; + this._saveTimer = null; this._savePromise = null; + this._resolveSavePromise = null; + this._dirty = false; - return this._cryptoStore.doTxn( + this._cryptoStore.doTxn( 'readwrite', [IndexedDBCryptoStore.STORE_DEVICE_DATA], (txn) => { this._cryptoStore.storeEndToEndDeviceData({ devices: this._devices, @@ -172,12 +208,12 @@ export default class DeviceList { syncToken: this._syncToken, }, txn); }, - ); - }).then(() => { - return true; - }); + ).then(() => { + resolveSavePromise(); + }); + }, delay); } - return this._savePromise; + return savePromise; } /** @@ -418,11 +454,11 @@ export default class DeviceList { if (this._deviceTrackingStatus[userId]) { console.log('No longer tracking device list for ' + userId); this._deviceTrackingStatus[userId] = TRACKING_STATUS_NOT_TRACKED; - } - // we don't yet persist the tracking status, since there may be a lot - // of calls; we save all data together once the sync is done - this._dirty = true; + // we don't yet persist the tracking status, since there may be a lot + // of calls; we save all data together once the sync is done + this._dirty = true; + } } /** @@ -453,11 +489,11 @@ export default class DeviceList { if (this._deviceTrackingStatus[userId]) { console.log("Marking device list outdated for", userId); this._deviceTrackingStatus[userId] = TRACKING_STATUS_PENDING_DOWNLOAD; - } - // we don't yet persist the tracking status, since there may be a lot - // of calls; we save all data together once the sync is done - this._dirty = true; + // we don't yet persist the tracking status, since there may be a lot + // of calls; we save all data together once the sync is done + this._dirty = true; + } } /** diff --git a/src/crypto/OutgoingRoomKeyRequestManager.js b/src/crypto/OutgoingRoomKeyRequestManager.js index 8bc860d62b9..75ab35454d5 100644 --- a/src/crypto/OutgoingRoomKeyRequestManager.js +++ b/src/crypto/OutgoingRoomKeyRequestManager.js @@ -35,13 +35,19 @@ const SEND_KEY_REQUESTS_DELAY_MS = 500; * * The state machine looks like: * - * | - * V (cancellation requested) - * UNSENT -----------------------------+ - * | | - * | (send successful) | - * V | - * SENT | + * | (cancellation sent) + * | .-------------------------------------------------. + * | | | + * V V (cancellation requested) | + * UNSENT -----------------------------+ | + * | | | + * | | | + * | (send successful) | CANCELLATION_PENDING_AND_WILL_RESEND + * V | Λ + * SENT | | + * |-------------------------------- | --------------' + * | | (cancellation requested with intent + * | | to resend the original request) * | | * | (cancellation requested) | * V | @@ -62,6 +68,12 @@ const ROOM_KEY_REQUEST_STATES = { /** reply received, cancellation not yet sent */ CANCELLATION_PENDING: 2, + + /** + * Cancellation not yet sent and will transition to UNSENT instead of + * being deleted once the cancellation has been sent. + */ + CANCELLATION_PENDING_AND_WILL_RESEND: 3, }; export default class OutgoingRoomKeyRequestManager { @@ -130,14 +142,16 @@ export default class OutgoingRoomKeyRequestManager { } /** - * Cancel room key requests, if any match the given details + * Cancel room key requests, if any match the given requestBody * * @param {module:crypto~RoomKeyRequestBody} requestBody + * @param {boolean} andResend if true, transition to UNSENT instead of + * deleting after sending cancellation. * * @returns {Promise} resolves when the request has been updated in our * pending list. */ - cancelRoomKeyRequest(requestBody) { + cancelRoomKeyRequest(requestBody, andResend=false) { return this._cryptoStore.getOutgoingRoomKeyRequest( requestBody, ).then((req) => { @@ -147,6 +161,7 @@ export default class OutgoingRoomKeyRequestManager { } switch (req.state) { case ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING: + case ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING_AND_WILL_RESEND: // nothing to do here return; @@ -166,11 +181,16 @@ export default class OutgoingRoomKeyRequestManager { req.requestId, ROOM_KEY_REQUEST_STATES.UNSENT, ); - case ROOM_KEY_REQUEST_STATES.SENT: + case ROOM_KEY_REQUEST_STATES.SENT: { + // If `andResend` is set, transition to UNSENT once the + // cancellation has successfully been sent. + const state = andResend ? + ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING_AND_WILL_RESEND : + ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING; // send a cancellation. return this._cryptoStore.updateOutgoingRoomKeyRequest( req.requestId, ROOM_KEY_REQUEST_STATES.SENT, { - state: ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING, + state, cancellationTxnId: this._baseApis.makeTxnId(), }, ).then((updatedReq) => { @@ -200,15 +220,23 @@ export default class OutgoingRoomKeyRequestManager { // do.) this._sendOutgoingRoomKeyRequestCancellation( updatedReq, + andResend, ).catch((e) => { console.error( "Error sending room key request cancellation;" + " will retry later.", e, ); this._startTimer(); - }).done(); + }).then(() => { + if (!andResend) return; + // The request has transitioned from + // CANCELLATION_PENDING_AND_WILL_RESEND to UNSENT. We + // still need to resend the request which is now UNSENT, so + // start the timer if it isn't already started. + this._startTimer(); + }); }); - + } default: throw new Error('unhandled state: ' + req.state); } @@ -258,6 +286,7 @@ export default class OutgoingRoomKeyRequestManager { return this._cryptoStore.getOutgoingRoomKeyRequestByState([ ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING, + ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING_AND_WILL_RESEND, ROOM_KEY_REQUEST_STATES.UNSENT, ]).then((req) => { if (!req) { @@ -267,10 +296,16 @@ export default class OutgoingRoomKeyRequestManager { } let prom; - if (req.state === ROOM_KEY_REQUEST_STATES.UNSENT) { - prom = this._sendOutgoingRoomKeyRequest(req); - } else { // must be a cancellation - prom = this._sendOutgoingRoomKeyRequestCancellation(req); + switch (req.state) { + case ROOM_KEY_REQUEST_STATES.UNSENT: + prom = this._sendOutgoingRoomKeyRequest(req); + break; + case ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING: + prom = this._sendOutgoingRoomKeyRequestCancellation(req); + break; + case ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING_AND_WILL_RESEND: + prom = this._sendOutgoingRoomKeyRequestCancellation(req, true); + break; } return prom.then(() => { @@ -309,8 +344,9 @@ export default class OutgoingRoomKeyRequestManager { }); } - // given a RoomKeyRequest, cancel it and delete the request record - _sendOutgoingRoomKeyRequestCancellation(req) { + // Given a RoomKeyRequest, cancel it and delete the request record unless + // andResend is set, in which case transition to UNSENT. + _sendOutgoingRoomKeyRequestCancellation(req, andResend) { console.log( `Sending cancellation for key request for ` + `${stringifyRequestBody(req.requestBody)} to ` + @@ -327,6 +363,14 @@ export default class OutgoingRoomKeyRequestManager { return this._sendMessageToDevices( requestMessage, req.recipients, req.cancellationTxnId, ).then(() => { + if (andResend) { + // We want to resend, so transition to UNSENT + return this._cryptoStore.updateOutgoingRoomKeyRequest( + req.requestId, + ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING_AND_WILL_RESEND, + { state: ROOM_KEY_REQUEST_STATES.UNSENT }, + ); + } return this._cryptoStore.deleteOutgoingRoomKeyRequest( req.requestId, ROOM_KEY_REQUEST_STATES.CANCELLATION_PENDING, ); diff --git a/src/crypto/index.js b/src/crypto/index.js index d773285dbed..64f863eb972 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -444,6 +444,22 @@ Crypto.prototype.getStoredDevice = function(userId, deviceId) { return this._deviceList.getStoredDevice(userId, deviceId); }; +/** + * Save the device list, if necessary + * + * @param {integer} delay Time in ms before which the save actually happens. + * By default, the save is delayed for a short period in order to batch + * multiple writes, but this behaviour can be disabled by passing 0. + * + * @return {Promise} true if the data was saved, false if + * it was not (eg. because no changes were pending). The promise + * will only resolve once the data is saved, so may take some time + * to resolve. + */ +Crypto.prototype.saveDeviceList = function(delay) { + return this._deviceList.saveIfDirty(delay); +}; + /** * Update the blocked/verified state of the given device * @@ -811,27 +827,18 @@ Crypto.prototype.decryptEvent = function(event) { */ Crypto.prototype.handleDeviceListChanges = async function(syncData, syncDeviceLists) { // Initial syncs don't have device change lists. We'll either get the complete list - // of changes for the interval or invalidate everything in onSyncComplete + // of changes for the interval or will have invalidated everything in willProcessSync if (!syncData.oldSyncToken) return; - if (syncData.oldSyncToken === this._deviceList.getSyncToken()) { - // the point the db is at matches where the sync started from, so - // we can safely write the changes - this._evalDeviceListChanges(syncDeviceLists); - } else { - // the db is at a different point to where this sync started from, so - // additionally fetch the changes between where the db is and where the - // sync started - console.log( - "Device list sync gap detected - fetching key changes between " + - this._deviceList.getSyncToken() + " and " + syncData.oldSyncToken, - ); - const gapDeviceLists = await this._baseApis.getKeyChanges( - this._deviceList.getSyncToken(), syncData.oldSyncToken, - ); - this._evalDeviceListChanges(gapDeviceLists); - this._evalDeviceListChanges(syncDeviceLists); - } + // Here, we're relying on the fact that we only ever save the sync data after + // sucessfully saving the device list data, so we're guaranteed that the device + // list store is at least as fresh as the sync token from the sync store, ie. + // any device changes received in sync tokens prior to the 'next' token here + // have been processed and are reflected in the current device list. + // If we didn't make this assumption, we'd have to use the /keys/changes API + // to get key changes between the sync token in the device list and the 'old' + // sync token used here to make sure we didn't miss any. + this._evalDeviceListChanges(syncDeviceLists); }; /** @@ -856,9 +863,11 @@ Crypto.prototype.requestRoomKey = function(requestBody, recipients) { * * @param {module:crypto~RoomKeyRequestBody} requestBody * parameters to match for cancellation + * @param {boolean} andResend + * if true, resend the key request after cancelling. */ -Crypto.prototype.cancelRoomKeyRequest = function(requestBody) { - this._outgoingRoomKeyRequestManager.cancelRoomKeyRequest(requestBody) +Crypto.prototype.cancelRoomKeyRequest = function(requestBody, andResend) { + this._outgoingRoomKeyRequestManager.cancelRoomKeyRequest(requestBody, andResend) .catch((e) => { console.warn("Error clearing pending room key requests", e); }).done(); diff --git a/src/models/event.js b/src/models/event.js index 26559c688cf..0bc2a3d4033 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -378,6 +378,21 @@ utils.extend(module.exports.MatrixEvent.prototype, { return this._decryptionPromise; }, + /** + * Cancel any room key request for this event and resend another. + * + * @param {module:crypto} crypto crypto module + */ + cancelAndResendKeyRequest: function(crypto) { + const wireContent = this.getWireContent(); + crypto.cancelRoomKeyRequest({ + algorithm: wireContent.algorithm, + room_id: this.getRoomId(), + session_id: wireContent.session_id, + sender_key: wireContent.sender_key, + }, true); + }, + _decryptionLoop: async function(crypto) { // make sure that this method never runs completely synchronously. // (doing so would mean that we would clear _decryptionPromise *before* diff --git a/src/store/indexeddb-local-backend.js b/src/store/indexeddb-local-backend.js index 453156134c5..a928e1aa6bb 100644 --- a/src/store/indexeddb-local-backend.js +++ b/src/store/indexeddb-local-backend.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -101,6 +102,7 @@ const LocalIndexedDBStoreBackend = function LocalIndexedDBStoreBackend( this.indexedDB = indexedDBInterface; this._dbName = "matrix-js-sdk:" + (dbName || "default"); this.db = null; + this._disconnected = true; this._syncAccumulator = new SyncAccumulator(); }; @@ -112,15 +114,17 @@ LocalIndexedDBStoreBackend.prototype = { * @return {Promise} Resolves if successfully connected. */ connect: function() { - if (this.db) { + if (!this._disconnected) { console.log( - `LocalIndexedDBStoreBackend.connect: already connected`, + `LocalIndexedDBStoreBackend.connect: already connected or connecting`, ); return Promise.resolve(); } + this._disconnected = false; + console.log( - `LocalIndexedDBStoreBackend.connect: connecting`, + `LocalIndexedDBStoreBackend.connect: connecting...`, ); const req = this.indexedDB.open(this._dbName, VERSION); req.onupgradeneeded = (ev) => { @@ -142,7 +146,7 @@ LocalIndexedDBStoreBackend.prototype = { }; console.log( - `LocalIndexedDBStoreBackend.connect: awaiting connection`, + `LocalIndexedDBStoreBackend.connect: awaiting connection...`, ); return promiseifyRequest(req).then((ev) => { console.log( @@ -240,6 +244,10 @@ LocalIndexedDBStoreBackend.prototype = { } }, + getNextBatchToken: function() { + return Promise.resolve(this._syncAccumulator.getNextBatchToken()); + }, + setSyncData: function(syncData) { return Promise.resolve().then(() => { this._syncAccumulator.accumulate(syncData); @@ -341,16 +349,18 @@ LocalIndexedDBStoreBackend.prototype = { */ _loadAccountData: function() { console.log( - `LocalIndexedDBStoreBackend: loading account data`, + `LocalIndexedDBStoreBackend: loading account data...`, ); return Promise.try(() => { - console.log( - `LocalIndexedDBStoreBackend: loaded account data`, - ); const txn = this.db.transaction(["accountData"], "readonly"); const store = txn.objectStore("accountData"); return selectQuery(store, undefined, (cursor) => { return cursor.value; + }).then((result) => { + console.log( + `LocalIndexedDBStoreBackend: loaded account data`, + ); + return result; }); }); }, @@ -361,17 +371,17 @@ LocalIndexedDBStoreBackend.prototype = { */ _loadSyncData: function() { console.log( - `LocalIndexedDBStoreBackend: loaded sync data`, + `LocalIndexedDBStoreBackend: loading sync data...`, ); return Promise.try(() => { - console.log( - `LocalIndexedDBStoreBackend: loaded sync data`, - ); const txn = this.db.transaction(["sync"], "readonly"); const store = txn.objectStore("sync"); return selectQuery(store, undefined, (cursor) => { return cursor.value; }).then((results) => { + console.log( + `LocalIndexedDBStoreBackend: loaded sync data`, + ); if (results.length > 1) { console.warn("loadSyncData: More than 1 sync row found."); } diff --git a/src/store/indexeddb-remote-backend.js b/src/store/indexeddb-remote-backend.js index e5c1cdd8420..2221633e423 100644 --- a/src/store/indexeddb-remote-backend.js +++ b/src/store/indexeddb-remote-backend.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -26,25 +27,23 @@ import Promise from 'bluebird'; * @param {string} workerScript URL to the worker script * @param {string=} dbName Optional database name. The same name must be used * to open the same database. - * @param {Object} WorkerApi The web worker compatible interface object + * @param {Object} workerApi The web worker compatible interface object */ const RemoteIndexedDBStoreBackend = function RemoteIndexedDBStoreBackend( - workerScript, dbName, WorkerApi, + workerScript, dbName, workerApi, ) { + this._workerScript = workerScript; this._dbName = dbName; - this._worker = new WorkerApi(workerScript); + this._workerApi = workerApi; + this._worker = null; this._nextSeq = 0; // The currently in-flight requests to the actual backend this._inFlight = { // seq: promise, }; - - this._worker.onmessage = this._onWorkerMessage.bind(this); - - // tell the worker the db name. - this._startPromise = this._doCmd('_setupWorker', [this._dbName]).then(() => { - console.log("IndexedDB worker is ready"); - }); + // Once we start connecting, we keep the promise and re-use it + // if we try to connect again + this._startPromise = null; }; @@ -55,7 +54,7 @@ RemoteIndexedDBStoreBackend.prototype = { * @return {Promise} Resolves if successfully connected. */ connect: function() { - return this._startPromise.then(() => this._doCmd('connect')); + return this._ensureStarted().then(() => this._doCmd('connect')); }, /** @@ -64,7 +63,7 @@ RemoteIndexedDBStoreBackend.prototype = { * @return {Promise} Resolved when the database is cleared. */ clearDatabase: function() { - return this._startPromise.then(() => this._doCmd('clearDatabase')); + return this._ensureStarted().then(() => this._doCmd('clearDatabase')); }, /** @@ -76,6 +75,10 @@ RemoteIndexedDBStoreBackend.prototype = { return this._doCmd('getSavedSync'); }, + getNextBatchToken: function() { + return this._doCmd('getNextBatchToken'); + }, + setSyncData: function(syncData) { return this._doCmd('setSyncData', [syncData]); }, @@ -93,6 +96,19 @@ RemoteIndexedDBStoreBackend.prototype = { return this._doCmd('getUserPresenceEvents'); }, + _ensureStarted: function() { + if (this._startPromise === null) { + this._worker = new this._workerApi(this._workerScript); + this._worker.onmessage = this._onWorkerMessage.bind(this); + + // tell the worker the db name. + this._startPromise = this._doCmd('_setupWorker', [this._dbName]).then(() => { + console.log("IndexedDB worker is ready"); + }); + } + return this._startPromise; + }, + _doCmd: function(cmd, args) { // wrap in a q so if the postMessage throws, // the promise automatically gets rejected diff --git a/src/store/indexeddb-store-worker.js b/src/store/indexeddb-store-worker.js index 3dc16e85544..d32df7274e5 100644 --- a/src/store/indexeddb-store-worker.js +++ b/src/store/indexeddb-store-worker.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -88,6 +89,9 @@ class IndexedDBStoreWorker { case 'getUserPresenceEvents': prom = this.backend.getUserPresenceEvents(); break; + case 'getNextBatchToken': + prom = this.backend.getNextBatchToken(); + break; } if (prom === undefined) { diff --git a/src/store/indexeddb.js b/src/store/indexeddb.js index 335f496d4c8..d47e3554469 100644 --- a/src/store/indexeddb.js +++ b/src/store/indexeddb.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -145,6 +146,14 @@ IndexedDBStore.prototype.getSavedSync = function() { return this.backend.getSavedSync(); }; +/** + * @return {Promise} If there is a saved sync, the nextBatch token + * for this sync, otherwise null. + */ +IndexedDBStore.prototype.getSavedSyncToken = function() { + return this.backend.getNextBatchToken(); +}, + /** * Delete all data from this store. * @return {Promise} Resolves if the data was deleted from the database. @@ -159,13 +168,27 @@ IndexedDBStore.prototype.deleteAllData = function() { }); }; +/** + * Whether this store would like to save its data + * Note that obviously whether the store wants to save or + * not could change between calling this function and calling + * save(). + * + * @return {boolean} True if calling save() will actually save + * (at the time this function is called). + */ +IndexedDBStore.prototype.wantsSave = function() { + const now = Date.now(); + return now - this._syncTs > WRITE_DELAY_MS; +}; + /** * Possibly write data to the database. - * @return {Promise} Promise resolves after the write completes. + * @return {Promise} Promise resolves after the write completes + * (or immediately if no write is performed) */ IndexedDBStore.prototype.save = function() { - const now = Date.now(); - if (now - this._syncTs > WRITE_DELAY_MS) { + if (this.wantsSave()) { return this._reallySave(); } return Promise.resolve(); diff --git a/src/store/memory.js b/src/store/memory.js index d25d2ea8960..4f2f7c7546a 100644 --- a/src/store/memory.js +++ b/src/store/memory.js @@ -1,6 +1,7 @@ /* Copyright 2015, 2016 OpenMarket Ltd Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -315,6 +316,15 @@ module.exports.MatrixInMemoryStore.prototype = { return Promise.resolve(); }, + /** + * We never want to save becase we have nothing to save to. + * + * @return {boolean} If the store wants to save + */ + wantsSave: function() { + return false; + }, + /** * Save does nothing as there is no backing data store. */ @@ -337,6 +347,14 @@ module.exports.MatrixInMemoryStore.prototype = { return Promise.resolve(null); }, + /** + * @return {Promise} If there is a saved sync, the nextBatch token + * for this sync, otherwise null. + */ + getSavedSyncToken: function() { + return Promise.resolve(null); + }, + /** * Delete all data from this store. * @return {Promise} An immediately resolved promise. diff --git a/src/store/stub.js b/src/store/stub.js index 964ca4a815a..f41e0c5e058 100644 --- a/src/store/stub.js +++ b/src/store/stub.js @@ -1,6 +1,7 @@ /* Copyright 2015, 2016 OpenMarket Ltd Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -216,6 +217,15 @@ StubStore.prototype = { return Promise.resolve(); }, + /** + * We never want to save becase we have nothing to save to. + * + * @return {boolean} If the store wants to save + */ + wantsSave: function() { + return false; + }, + /** * Save does nothing as there is no backing data store. */ @@ -238,6 +248,14 @@ StubStore.prototype = { return Promise.resolve(null); }, + /** + * @return {Promise} If there is a saved sync, the nextBatch token + * for this sync, otherwise null. + */ + getSavedSyncToken: function() { + return Promise.resolve(null); + }, + /** * Delete all data from this store. Does nothing since this store * doesn't store anything. diff --git a/src/sync-accumulator.js b/src/sync-accumulator.js index c1266a9b01d..7369c9b8a89 100644 --- a/src/sync-accumulator.js +++ b/src/sync-accumulator.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -520,6 +521,10 @@ class SyncAccumulator { accountData: accData, }; } + + getNextBatchToken() { + return this.nextBatch; + } } function setState(eventMap, event) { diff --git a/src/sync.js b/src/sync.js index 3928875f2bb..2636460b91c 100644 --- a/src/sync.js +++ b/src/sync.js @@ -396,6 +396,17 @@ SyncApi.prototype.getSyncState = function() { return this._syncState; }; +SyncApi.prototype.recoverFromSyncStartupError = async function(savedSyncPromise, err) { + // Wait for the saved sync to complete - we send the pushrules and filter requests + // before the saved sync has finished so they can run in parallel, but only process + // the results after the saved sync is done. Equivalently, we wait for it to finish + // before reporting failures from these functions. + await savedSyncPromise; + const keepaliveProm = this._startKeepAlives(); + this._updateSyncState("ERROR", { error: err }); + await keepaliveProm; +}; + /** * Main entry point */ @@ -410,28 +421,32 @@ SyncApi.prototype.sync = function() { global.document.addEventListener("online", this._onOnlineBound, false); } + let savedSyncPromise = Promise.resolve(); + let savedSyncToken = null; + // We need to do one-off checks before we can begin the /sync loop. // These are: // 1) We need to get push rules so we can check if events should bing as we get // them from /sync. // 2) We need to get/create a filter which we can use for /sync. - function getPushRules() { - client.getPushRules().done((result) => { + async function getPushRules() { + try { + const result = await client.getPushRules(); debuglog("Got push rules"); client.pushRules = result; - - getFilter(); // Now get the filter and start syncing - }, (err) => { - self._startKeepAlives().done(() => { - getPushRules(); - }); - self._updateSyncState("ERROR", { error: err }); - }); + } catch (err) { + // wait for saved sync to complete before doing anything else, + // otherwise the sync state will end up being incorrect + await self.recoverFromSyncStartupError(savedSyncPromise, err); + getPushRules(); + return; + } + getFilter(); // Now get the filter and start syncing } - function getFilter() { + async function getFilter() { let filter; if (self.opts.filter) { filter = self.opts.filter; @@ -440,40 +455,55 @@ SyncApi.prototype.sync = function() { filter.setTimelineLimit(self.opts.initialSyncLimit); } - client.getOrCreateFilter( - getFilterName(client.credentials.userId), filter, - ).done((filterId) => { - // reset the notifications timeline to prepare it to paginate from - // the current point in time. - // The right solution would be to tie /sync pagination tokens into - // /notifications API somehow. - client.resetNotifTimelineSet(); - - self._sync({ filterId }); - }, (err) => { - self._startKeepAlives().done(function() { - getFilter(); - }); - self._updateSyncState("ERROR", { error: err }); - }); + let filterId; + try { + filterId = await client.getOrCreateFilter( + getFilterName(client.credentials.userId), filter, + ); + } catch (err) { + // wait for saved sync to complete before doing anything else, + // otherwise the sync state will end up being incorrect + await self.recoverFromSyncStartupError(savedSyncPromise, err); + getFilter(); + return; + } + // reset the notifications timeline to prepare it to paginate from + // the current point in time. + // The right solution would be to tie /sync pagination tokens into + // /notifications API somehow. + client.resetNotifTimelineSet(); + + if (self._currentSyncRequest === null) { + // Send this first sync request here so we can then wait for the saved + // sync data to finish processing before we process the results of this one. + console.log("Sending first sync request..."); + self._currentSyncRequest = self._doSyncRequest({ filterId }, savedSyncToken); + } + + // Now wait for the saved sync to finish... + await savedSyncPromise; + self._sync({ filterId }); } if (client.isGuest()) { // no push rules for guests, no access to POST filter for guests. self._sync({}); } else { - // Before fetching push rules, fetching the filter and syncing, check - // for persisted /sync data and use that if present. - client.store.getSavedSync().then((savedSync) => { + // Pull the saved sync token out first, before the worker starts sending + // all the sync data which could take a while. This will let us send our + // first incremental sync request before we've processed our saved data. + savedSyncPromise = client.store.getSavedSyncToken().then((tok) => { + savedSyncToken = tok; + return client.store.getSavedSync(); + }).then((savedSync) => { if (savedSync) { return self._syncFromCache(savedSync); } - }).then(() => { - // Get push rules and start syncing after getting the saved sync - // to handle the case where we needed the `nextBatch` token to - // start syncing from. - getPushRules(); }); + // Now start the first incremental sync request: this can also + // take a while so if we set it going now, we can wait for it + // to finish while we process our saved sync data. + getPushRules(); } }; @@ -565,70 +595,20 @@ SyncApi.prototype._sync = async function(syncOptions) { return; } - let filterId = syncOptions.filterId; - if (client.isGuest() && !filterId) { - filterId = this._getGuestFilter(); - } - const syncToken = client.store.getSyncToken(); - let pollTimeout = this.opts.pollTimeout; - - if (this.getSyncState() !== 'SYNCING' || this._catchingUp) { - // unless we are happily syncing already, we want the server to return - // as quickly as possible, even if there are no events queued. This - // serves two purposes: - // - // * When the connection dies, we want to know asap when it comes back, - // so that we can hide the error from the user. (We don't want to - // have to wait for an event or a timeout). - // - // * We want to know if the server has any to_device messages queued up - // for us. We do that by calling it with a zero timeout until it - // doesn't give us any more to_device messages. - this._catchingUp = true; - pollTimeout = 0; - } - - // normal timeout= plus buffer time - const clientSideTimeoutMs = pollTimeout + BUFFER_PERIOD_MS; - - const qps = { - filter: filterId, - timeout: pollTimeout, - }; - - if (this.opts.disablePresence) { - qps.set_presence = "offline"; - } - - if (syncToken) { - qps.since = syncToken; - } else { - // use a cachebuster for initialsyncs, to make sure that - // we don't get a stale sync - // (https://github.com/vector-im/vector-web/issues/1354) - qps._cacheBuster = Date.now(); - } - - if (this.getSyncState() == 'ERROR' || this.getSyncState() == 'RECONNECTING') { - // we think the connection is dead. If it comes back up, we won't know - // about it till /sync returns. If the timeout= is high, this could - // be a long time. Set it to 0 when doing retries so we don't have to wait - // for an event or a timeout before emiting the SYNCING event. - qps.timeout = 0; - } - let data; try { //debuglog('Starting sync since=' + syncToken); - this._currentSyncRequest = client._http.authedRequest( - undefined, "GET", "/sync", qps, undefined, clientSideTimeoutMs, - ); + if (this._currentSyncRequest === null) { + this._currentSyncRequest = this._doSyncRequest(syncOptions, syncToken); + } data = await this._currentSyncRequest; } catch (e) { this._onSyncError(e, syncOptions); return; + } finally { + this._currentSyncRequest = null; } //debuglog('Completed sync, next_batch=' + data.next_batch); @@ -681,13 +661,85 @@ SyncApi.prototype._sync = async function(syncOptions) { // keep emitting SYNCING -> SYNCING for clients who want to do bulk updates this._updateSyncState("SYNCING", syncEventData); - // tell databases that everything is now in a consistent state and can be saved. - client.store.save(); + if (client.store.wantsSave()) { + // We always save the device list (if it's dirty) before saving the sync data: + // this means we know the saved device list data is at least as fresh as the + // stored sync data which means we don't have to worry that we may have missed + // device changes. We can also skip the delay since we're not calling this very + // frequently (and we don't really want to delay the sync for it). + if (this.opts.crypto) { + await this.opts.crypto.saveDeviceList(0); + } + + // tell databases that everything is now in a consistent state and can be saved. + client.store.save(); + } // Begin next sync this._sync(syncOptions); }; +SyncApi.prototype._doSyncRequest = function(syncOptions, syncToken) { + const qps = this._getSyncParams(syncOptions, syncToken); + return this.client._http.authedRequest( + undefined, "GET", "/sync", qps, undefined, + qps.timeout + BUFFER_PERIOD_MS, + ); +}; + +SyncApi.prototype._getSyncParams = function(syncOptions, syncToken) { + let pollTimeout = this.opts.pollTimeout; + + if (this.getSyncState() !== 'SYNCING' || this._catchingUp) { + // unless we are happily syncing already, we want the server to return + // as quickly as possible, even if there are no events queued. This + // serves two purposes: + // + // * When the connection dies, we want to know asap when it comes back, + // so that we can hide the error from the user. (We don't want to + // have to wait for an event or a timeout). + // + // * We want to know if the server has any to_device messages queued up + // for us. We do that by calling it with a zero timeout until it + // doesn't give us any more to_device messages. + this._catchingUp = true; + pollTimeout = 0; + } + + let filterId = syncOptions.filterId; + if (this.client.isGuest() && !filterId) { + filterId = this._getGuestFilter(); + } + + const qps = { + filter: filterId, + timeout: pollTimeout, + }; + + if (this.opts.disablePresence) { + qps.set_presence = "offline"; + } + + if (syncToken) { + qps.since = syncToken; + } else { + // use a cachebuster for initialsyncs, to make sure that + // we don't get a stale sync + // (https://github.com/vector-im/vector-web/issues/1354) + qps._cacheBuster = Date.now(); + } + + if (this.getSyncState() == 'ERROR' || this.getSyncState() == 'RECONNECTING') { + // we think the connection is dead. If it comes back up, we won't know + // about it till /sync returns. If the timeout= is high, this could + // be a long time. Set it to 0 when doing retries so we don't have to wait + // for an event or a timeout before emiting the SYNCING event. + qps.timeout = 0; + } + + return qps; +}; + SyncApi.prototype._onSyncError = function(err, syncOptions) { if (!this._running) { debuglog("Sync no longer running: exiting"); @@ -1329,8 +1381,19 @@ SyncApi.prototype._processRoomEvents = function(room, stateEventList, // If the timeline wasn't empty, we process the state events here: they're // defined as updates to the state before the start of the timeline, so this // starts to roll the state forward. + // XXX: That's what we *should* do, but this can happen if we were previously + // peeking in a room, in which case we obviously do *not* want to add the + // state events here onto the end of the timeline. Historically, the js-sdk + // has just set these new state events on the old and new state. This seems + // very wrong because there could be events in the timeline that diverge the + // state, in which case this is going to leave things out of sync. However, + // for now I think it;s best to behave the same as the code has done previously. if (!timelineWasEmpty) { - room.addLiveEvents(stateEventList || []); + // XXX: As above, don't do this... + //room.addLiveEvents(stateEventList || []); + // Do this instead... + room.oldState.setStateEvents(stateEventList || []); + room.currentState.setStateEvents(stateEventList || []); } // execute the timeline events. This will continue to diverge the current state // if the timeline has any state events in it.