Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect when lazy loading has been toggled in client.startClient #746

Merged
merged 17 commits into from
Sep 27, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions spec/integ/matrix-client-opts.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe("MatrixClient opts", function() {
httpBackend.flush("/txn1", 1);
});

it("should be able to sync / get new events", function(done) {
it("should be able to sync / get new events", async function() {
const expectedEventTypes = [ // from /initialSync
"m.room.message", "m.room.name", "m.room.member", "m.room.member",
"m.room.create",
Expand All @@ -110,20 +110,16 @@ describe("MatrixClient opts", function() {
httpBackend.when("GET", "/pushrules").respond(200, {});
httpBackend.when("POST", "/filter").respond(200, { filter_id: "foo" });
httpBackend.when("GET", "/sync").respond(200, syncData);
client.startClient();
httpBackend.flush("/pushrules", 1).then(function() {
return httpBackend.flush("/filter", 1);
}).then(function() {
return Promise.all([
httpBackend.flush("/sync", 1),
utils.syncPromise(client),
]);
}).done(function() {
expect(expectedEventTypes.length).toEqual(
0, "Expected to see event types: " + expectedEventTypes,
);
done();
});
await client.startClient();
await httpBackend.flush("/pushrules", 1);
await httpBackend.flush("/filter", 1);
await Promise.all([
httpBackend.flush("/sync", 1),
utils.syncPromise(client),
]);
expect(expectedEventTypes.length).toEqual(
0, "Expected to see event types: " + expectedEventTypes,
);
});
});

Expand Down
50 changes: 31 additions & 19 deletions spec/unit/matrix-client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ describe("MatrixClient", function() {
store.getSavedSync = expect.createSpy().andReturn(Promise.resolve(null));
store.getSavedSyncToken = expect.createSpy().andReturn(Promise.resolve(null));
store.setSyncData = expect.createSpy().andReturn(Promise.resolve(null));
store.getClientOptions = expect.createSpy().andReturn(Promise.resolve(null));
store.storeClientOptions = expect.createSpy().andReturn(Promise.resolve(null));
store.isNewlyCreated = expect.createSpy().andReturn(Promise.resolve(true));
client = new MatrixClient({
baseUrl: "https://my.home.server",
idBaseUrl: identityServerUrl,
Expand Down Expand Up @@ -182,7 +185,7 @@ describe("MatrixClient", function() {
});
});

it("should not POST /filter if a matching filter already exists", function(done) {
it("should not POST /filter if a matching filter already exists", async function() {
httpLookups = [];
httpLookups.push(PUSH_RULES_RESPONSE);
httpLookups.push(SYNC_RESPONSE);
Expand All @@ -191,31 +194,40 @@ describe("MatrixClient", function() {
const filter = new sdk.Filter(0, filterId);
filter.setDefinition({"room": {"timeline": {"limit": 8}}});
store.getFilter.andReturn(filter);
client.startClient();

client.on("sync", function syncListener(state) {
if (state === "SYNCING") {
expect(httpLookups.length).toEqual(0);
client.removeListener("sync", syncListener);
done();
}
const syncPromise = new Promise((resolve, reject) => {
client.on("sync", function syncListener(state) {
if (state === "SYNCING") {
expect(httpLookups.length).toEqual(0);
client.removeListener("sync", syncListener);
resolve();
} else if (state === "ERROR") {
reject(new Error("sync error"));
}
});
});
await client.startClient();
await syncPromise;
});

describe("getSyncState", function() {
it("should return null if the client isn't started", function() {
expect(client.getSyncState()).toBe(null);
});

it("should return the same sync state as emitted sync events", function(done) {
client.on("sync", function syncListener(state) {
expect(state).toEqual(client.getSyncState());
if (state === "SYNCING") {
client.removeListener("sync", syncListener);
done();
}
it("should return the same sync state as emitted sync events", async function() {
/* const syncingPromise = new Promise((resolve) => {
throw new Error("fail!!");
client.on("sync", function syncListener(state) {
expect(state).toEqual(client.getSyncState());
if (state === "SYNCING") {
client.removeListener("sync", syncListener);
resolve();
}
});
});
client.startClient();
*/
await client.startClient();
// await syncingPromise;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened to this test?

});
});

Expand Down Expand Up @@ -258,8 +270,8 @@ describe("MatrixClient", function() {
});

describe("retryImmediately", function() {
it("should return false if there is no request waiting", function() {
client.startClient();
it("should return false if there is no request waiting", async function() {
await client.startClient();
expect(client.retryImmediately()).toBe(false);
});

Expand Down
49 changes: 47 additions & 2 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const ContentHelpers = require("./content-helpers");

import ReEmitter from './ReEmitter';
import RoomList from './crypto/RoomList';
import {InvalidStoreError} from './errors';


const LAZY_LOADING_MESSAGES_FILTER = {
Expand Down Expand Up @@ -3115,7 +3116,12 @@ MatrixClient.prototype.startClient = async function(opts) {
opts.lazyLoadMembers = false;
}
}

// need to vape the store when enabling LL and wasn't enabled before
const shouldClear = await this._wasLazyLoadingToggled(opts.lazyLoadMembers);
if (shouldClear) {
const reason = InvalidStoreError.TOGGLED_LAZY_LOADING;
throw new InvalidStoreError(reason, !!opts.lazyLoadMembers);
}
if (opts.lazyLoadMembers && this._crypto) {
this._crypto.enableLazyLoading();
}
Expand All @@ -3128,11 +3134,50 @@ MatrixClient.prototype.startClient = async function(opts) {
return this._canResetTimelineCallback(roomId);
};
this._clientOpts = opts;

await this._storeClientOptions(this._clientOpts);
this._syncApi = new SyncApi(this, opts);
this._syncApi.sync();
};

/**
* Is the lazy loading option different than in previous session?
* @param {bool} lazyLoadMembers current options for lazy loading
* @return {bool} whether or not the option has changed compared to the previous session */
MatrixClient.prototype._wasLazyLoadingToggled = async function(lazyLoadMembers) {
lazyLoadMembers = !!lazyLoadMembers;
// assume it was turned off before
// if we don't know any better
let lazyLoadMembersBefore = false;
const isStoreNewlyCreated = await this.store.isNewlyCreated();
if (!isStoreNewlyCreated) {
const prevClientOptions = await this.store.getClientOptions();
if (prevClientOptions) {
lazyLoadMembersBefore = !!prevClientOptions.lazyLoadMembers;
}
return lazyLoadMembersBefore !== lazyLoadMembers;
}
return false;
};

/**
* store client options with boolean/string/numeric values
* to know in the next session what flags the sync data was
* created with (e.g. lazy loading)
* @param {object} opts the complete set of client options
* @return {Promise} for store operation */
MatrixClient.prototype._storeClientOptions = function(opts) {
const primTypes = ["boolean", "string", "number"];
const serializableOpts = Object.entries(opts)
.filter(([key, value]) => {
return primTypes.includes(typeof value);
})
.reduce((obj, [key, value]) => {
obj[key] = value;
return obj;
}, {});
return this.store.storeClientOptions(serializableOpts);
};

/**
* High level helper method to stop the client from polling and allow a
* clean shutdown.
Expand Down
25 changes: 25 additions & 0 deletions src/errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// can't just do InvalidStoreError extends Error
// because of http://babeljs.io/docs/usage/caveats/#classes
function InvalidStoreError(reason, value) {
const message = `Store is invalid because ${reason}, ` +
`please delete all data and retry`;
const instance = Reflect.construct(Error, [message]);
Reflect.setPrototypeOf(instance, Reflect.getPrototypeOf(this));
instance.reason = reason;
instance.value = value;
return instance;
}

InvalidStoreError.TOGGLED_LAZY_LOADING = "TOGGLED_LAZY_LOADING";

InvalidStoreError.prototype = Object.create(Error.prototype, {
constructor: {
value: Error,
enumerable: false,
writable: true,
configurable: true,
},
});
Reflect.setPrototypeOf(InvalidStoreError, Error);

module.exports.InvalidStoreError = InvalidStoreError;
2 changes: 2 additions & 0 deletions src/matrix.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ module.exports.SyncAccumulator = require("./sync-accumulator");
module.exports.MatrixHttpApi = require("./http-api").MatrixHttpApi;
/** The {@link module:http-api.MatrixError|MatrixError} class. */
module.exports.MatrixError = require("./http-api").MatrixError;
/** The {@link module:errors.InvalidStoreError|InvalidStoreError} class. */
module.exports.InvalidStoreError = require("./errors").InvalidStoreError;
/** The {@link module:client.MatrixClient|MatrixClient} class. */
module.exports.MatrixClient = require("./client").MatrixClient;
/** The {@link module:models/room|Room} class. */
Expand Down
39 changes: 38 additions & 1 deletion src/store/indexeddb-local-backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import Promise from 'bluebird';
import SyncAccumulator from "../sync-accumulator";
import utils from "../utils";

const VERSION = 2;
const VERSION = 3;

function createDatabase(db) {
// Make user store, clobber based on user ID. (userId property of User objects)
Expand All @@ -41,6 +41,12 @@ function upgradeSchemaV2(db) {
oobMembersStore.createIndex("room", "room_id");
}

function upgradeSchemaV3(db) {
db.createObjectStore("client_options",
{ keyPath: ["clobber"]});
}


/**
* Helper method to collect results from a Cursor and promiseify it.
* @param {ObjectStore|Index} store The store to perform openCursor on.
Expand Down Expand Up @@ -123,6 +129,7 @@ const LocalIndexedDBStoreBackend = function LocalIndexedDBStoreBackend(
this.db = null;
this._disconnected = true;
this._syncAccumulator = new SyncAccumulator();
this._isNewlyCreated = false;
};


Expand Down Expand Up @@ -153,11 +160,15 @@ LocalIndexedDBStoreBackend.prototype = {
`LocalIndexedDBStoreBackend.connect: upgrading from ${oldVersion}`,
);
if (oldVersion < 1) { // The database did not previously exist.
this._isNewlyCreated = true;
createDatabase(db);
}
if (oldVersion < 2) {
upgradeSchemaV2(db);
}
if (oldVersion < 3) {
upgradeSchemaV3(db);
}
// Expand as needed.
};

Expand Down Expand Up @@ -185,6 +196,10 @@ LocalIndexedDBStoreBackend.prototype = {
return this._init();
});
},
/** @return {bool} whether or not the database was newly created in this session. */
isNewlyCreated: function() {
return Promise.resolve(this._isNewlyCreated);
},

/**
* Having connected, load initial data from the database and prepare for use
Expand Down Expand Up @@ -529,6 +544,28 @@ LocalIndexedDBStoreBackend.prototype = {
});
});
},

getClientOptions: function() {
return Promise.resolve().then(() => {
const txn = this.db.transaction(["client_options"], "readonly");
const store = txn.objectStore("client_options");
return selectQuery(store, undefined, (cursor) => {
if (cursor.value && cursor.value && cursor.value.options) {
return cursor.value.options;
}
}).then((results) => results[0]);
});
},

storeClientOptions: async function(options) {
const txn = this.db.transaction(["client_options"], "readwrite");
const store = txn.objectStore("client_options");
store.put({
clobber: "-", // constant key so will always clobber
options: options,
}); // put == UPSERT
await txnAsPromise(txn);
},
};

export default LocalIndexedDBStoreBackend;
13 changes: 12 additions & 1 deletion src/store/indexeddb-remote-backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ RemoteIndexedDBStoreBackend.prototype = {
clearDatabase: function() {
return this._ensureStarted().then(() => this._doCmd('clearDatabase'));
},

/** @return {Promise<bool>} whether or not the database was newly created in this session. */
isNewlyCreated: function() {
return this._doCmd('isNewlyCreated');
},
/**
* @return {Promise} Resolves with a sync response to restore the
* client state to where it was at the last save, or null if there
Expand Down Expand Up @@ -114,6 +117,14 @@ RemoteIndexedDBStoreBackend.prototype = {
return this._doCmd('clearOutOfBandMembers', [roomId]);
},

getClientOptions: function() {
return this._doCmd('getClientOptions');
},

storeClientOptions: function(options) {
return this._doCmd('storeClientOptions', [options]);
},

/**
* Load all user presence events from the database. This is not cached.
* @return {Promise<Object[]>} A list of presence events in their raw form.
Expand Down
11 changes: 10 additions & 1 deletion src/store/indexeddb-store-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ class IndexedDBStoreWorker {
case 'connect':
prom = this.backend.connect();
break;
case 'isNewlyCreated':
prom = this.backend.isNewlyCreated();
break;
case 'clearDatabase':
prom = this.backend.clearDatabase().then((result) => {
// This returns special classes which can't be cloned
Expand Down Expand Up @@ -101,10 +104,16 @@ class IndexedDBStoreWorker {
case 'setOutOfBandMembers':
prom = this.backend.setOutOfBandMembers(msg.args[0], msg.args[1]);
break;
case 'getClientOptions':
prom = this.backend.getClientOptions();
break;
case 'storeClientOptions':
prom = this.backend.storeClientOptions(msg.args[0]);
break;
}

if (prom === undefined) {
postMessage({
this.postMessage({
command: 'cmd_fail',
seq: msg.seq,
// Can't be an Error because they're not structured cloneable
Expand Down
Loading