matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-11-28 05:03:59 +03:00
Code Activity

Make tests pass, finally.

Browse Source 
Mostly making tests aware of new storage format or making them
force it to be written. Also some bugfixes like we didn't json
encode some things in the localstorage store and we didn't
correctly check the promise when requesting device data saves.
This commit is contained in:
David Baker
2018-01-16 17:57:49 +00:00
parent 27d28b8247
commit 0174c5674f
4 changed files with 103 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
spec/TestClient.js
View File

@@ -47,9 +47,10 @@ export default function TestClient(
if (sessionStoreBackend === undefined) { if (sessionStoreBackend === undefined) {
sessionStoreBackend = new testUtils.MockStorageApi(); sessionStoreBackend = new testUtils.MockStorageApi();
} }
this.storage = new sdk.WebStorageSessionStore(sessionStoreBackend); const sessionStore = new sdk.WebStorageSessionStore(sessionStoreBackend);
const cryptoStore = new LocalStorageCryptoStore(sessionStoreBackend); // expose this so the tests can get to it
this.cryptoStore = new LocalStorageCryptoStore(sessionStoreBackend);
this.httpBackend = new MockHttpBackend(); this.httpBackend = new MockHttpBackend();
this.client = sdk.createClient({ this.client = sdk.createClient({
@@ -57,8 +58,8 @@ export default function TestClient(
userId: userId, userId: userId,
accessToken: accessToken, accessToken: accessToken,
deviceId: deviceId, deviceId: deviceId,
sessionStore: this.storage, sessionStore: sessionStore,
cryptoStore: cryptoStore, cryptoStore: this.cryptoStore,
request: this.httpBackend.requestFn, request: this.httpBackend.requestFn,
}); });

127
spec/integ/devicelist-integ-spec.js
View File

@@ -151,9 +151,12 @@ describe("DeviceList management:", function() {
aliceTestClient.httpBackend.flush('/keys/query', 1).then( aliceTestClient.httpBackend.flush('/keys/query', 1).then(
() => aliceTestClient.httpBackend.flush('/send/', 1), () => aliceTestClient.httpBackend.flush('/send/', 1),
), ),
aliceTestClient.client._crypto._deviceList.saveIfDirty(),
]); ]);
}).then(() => { }).then(() => {
expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(1); aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
expect(data.syncToken).toEqual(1);
});
// invalidate bob's and chris's device lists in separate syncs // invalidate bob's and chris's device lists in separate syncs
aliceTestClient.httpBackend.when('GET', '/sync').respond(200, { aliceTestClient.httpBackend.when('GET', '/sync').respond(200, {
@@ -185,19 +188,20 @@ describe("DeviceList management:", function() {
return aliceTestClient.httpBackend.flush('/keys/query', 1); return aliceTestClient.httpBackend.flush('/keys/query', 1);
}).then((flushed) => { }).then((flushed) => {
expect(flushed).toEqual(0); expect(flushed).toEqual(0);
const bobStat = aliceTestClient.storage return aliceTestClient.client._crypto._deviceList.saveIfDirty();
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; }).then(() => {
if (bobStat != 1 && bobStat != 2) { aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
throw new Error('Unexpected status for bob: wanted 1 or 2, got ' + const bobStat = data.trackingStatus['@bob:xyz'];
bobStat); if (bobStat != 1 && bobStat != 2) {
} throw new Error('Unexpected status for bob: wanted 1 or 2, got ' +
bobStat);
const chrisStat = aliceTestClient.storage }
.getEndToEndDeviceTrackingStatus()['@chris:abc']; const chrisStat = data.trackingStatus['@chris:abc'];
if (chrisStat != 1 && chrisStat != 2) { if (chrisStat != 1 && chrisStat != 2) {
throw new Error('Unexpected status for chris: wanted 1 or 2, got ' + throw new Error('Unexpected status for chris: wanted 1 or 2, got ' +
chrisStat); chrisStat);
} }
});
// now add an expectation for a query for bob's devices, and let // now add an expectation for a query for bob's devices, and let
// it complete. // it complete.
@@ -216,15 +220,17 @@ describe("DeviceList management:", function() {
// wait for the client to stop processing the response // wait for the client to stop processing the response
return aliceTestClient.client.downloadKeys(['@bob:xyz']); return aliceTestClient.client.downloadKeys(['@bob:xyz']);
}).then(() => { }).then(() => {
const bobStat = aliceTestClient.storage return aliceTestClient.client._crypto._deviceList.saveIfDirty();
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; }).then(() => {
expect(bobStat).toEqual(3); aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
const chrisStat = aliceTestClient.storage const bobStat = data.trackingStatus['@bob:xyz'];
.getEndToEndDeviceTrackingStatus()['@chris:abc']; expect(bobStat).toEqual(3);
if (chrisStat != 1 && chrisStat != 2) { const chrisStat = data.trackingStatus['@chris:abc'];
throw new Error('Unexpected status for chris: wanted 1 or 2, got ' + if (chrisStat != 1 && chrisStat != 2) {
bobStat); throw new Error('Unexpected status for chris: wanted 1 or 2, got ' +
} bobStat);
}
});
// now let the query for chris's devices complete. // now let the query for chris's devices complete.
return aliceTestClient.httpBackend.flush('/keys/query', 1); return aliceTestClient.httpBackend.flush('/keys/query', 1);
@@ -234,16 +240,18 @@ describe("DeviceList management:", function() {
// wait for the client to stop processing the response // wait for the client to stop processing the response
return aliceTestClient.client.downloadKeys(['@chris:abc']); return aliceTestClient.client.downloadKeys(['@chris:abc']);
}).then(() => { }).then(() => {
const bobStat = aliceTestClient.storage return aliceTestClient.client._crypto._deviceList.saveIfDirty();
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; }).then(() => {
const chrisStat = aliceTestClient.storage aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
.getEndToEndDeviceTrackingStatus()['@chris:abc']; const bobStat = data.trackingStatus['@bob:xyz'];
const chrisStat = data.trackingStatus['@bob:xyz'];
expect(bobStat).toEqual(3); expect(bobStat).toEqual(3);
expect(chrisStat).toEqual(3); expect(chrisStat).toEqual(3);
expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(3); expect(data.syncToken).toEqual(3);
});
}); });
}); }).timeout(3000);
// https://github.com/vector-im/riot-web/issues/4983 // https://github.com/vector-im/riot-web/issues/4983
describe("Alice should know she has stale device lists", () => { describe("Alice should know she has stale device lists", () => {
@@ -262,13 +270,16 @@ describe("DeviceList management:", function() {
}, },
); );
await aliceTestClient.httpBackend.flush('/keys/query', 1); await aliceTestClient.httpBackend.flush('/keys/query', 1);
await aliceTestClient.client._crypto._deviceList.saveIfDirty();
const bobStat = aliceTestClient.storage aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; const bobStat = data.trackingStatus['@bob:xyz'];
expect(bobStat).toBeGreaterThan(
0, "Alice should be tracking bob's device list",
);
});
expect(bobStat).toBeGreaterThan(
0, "Alice should be tracking bob's device list",
);
}); });
it("when Bob leaves", async function() { it("when Bob leaves", async function() {
@@ -297,12 +308,15 @@ describe("DeviceList management:", function() {
await aliceTestClient.flushSync(); await aliceTestClient.flushSync();
await aliceTestClient.client._crypto._deviceList.saveIfDirty();
const bobStat = aliceTestClient.storage aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; const bobStat = data.trackingStatus['@bob:xyz'];
expect(bobStat).toEqual(
0, "Alice should have marked bob's device list as untracked", expect(bobStat).toEqual(
); 0, "Alice should have marked bob's device list as untracked",
);
});
}); });
it("when Alice leaves", async function() { it("when Alice leaves", async function() {
@@ -330,12 +344,15 @@ describe("DeviceList management:", function() {
); );
await aliceTestClient.flushSync(); await aliceTestClient.flushSync();
await aliceTestClient.client._crypto._deviceList.saveIfDirty();
const bobStat = aliceTestClient.storage aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; const bobStat = data.trackingStatus['@bob:xyz'];
expect(bobStat).toEqual(
0, "Alice should have marked bob's device list as untracked", expect(bobStat).toEqual(
); 0, "Alice should have marked bob's device list as untracked",
);
});
}); });
it("when Bob leaves whilst Alice is offline", async function() { it("when Bob leaves whilst Alice is offline", async function() {
@@ -344,23 +361,19 @@ describe("DeviceList management:", function() {
const anotherTestClient = await createTestClient(); const anotherTestClient = await createTestClient();
try { try {
anotherTestClient.httpBackend.when('GET', '/keys/changes').respond(
200, {
changed: [],
left: ['@bob:xyz'],
},
);
await anotherTestClient.start(); await anotherTestClient.start();
anotherTestClient.httpBackend.when('GET', '/sync').respond( anotherTestClient.httpBackend.when('GET', '/sync').respond(
200, getSyncResponse([])); 200, getSyncResponse([]));
await anotherTestClient.flushSync(); await anotherTestClient.flushSync();
await aliceTestClient.client._crypto._deviceList.saveIfDirty();
const bobStat = anotherTestClient.storage aliceTestClient.cryptoStore.getEndToEndDeviceData(null, (data) => {
.getEndToEndDeviceTrackingStatus()['@bob:xyz']; const bobStat = data.trackingStatus['@bob:xyz'];
expect(bobStat).toEqual( expect(bobStat).toEqual(
0, "Alice should have marked bob's device list as untracked", 0, "Alice should have marked bob's device list as untracked",
); );
});
} finally { } finally {
anotherTestClient.stop(); anotherTestClient.stop();
} }

38
src/crypto/DeviceList.js
View File

@@ -89,8 +89,6 @@ export default class DeviceList {
// Set whenever changes are made other than setting the sync token // Set whenever changes are made other than setting the sync token
this._dirty = false; this._dirty = false;
// Timeout for a scheduled save
this._saveTimer = null;
// Promise resolved when device data is saved // Promise resolved when device data is saved
this._savePromise = null; this._savePromise = null;
} }
@@ -150,11 +148,10 @@ export default class DeviceList {
async saveIfDirty() { async saveIfDirty() {
if (!this._dirty) return Promise.resolve(false); if (!this._dirty) return Promise.resolve(false);
if (this._saveTimer === null) { if (this._savePromise === null) {
this._saveTimer = setTimeout(() => { this._savePromise = Promise.delay(500).then(() => {
console.log('Saving device tracking data...'); console.log('Saving device tracking data at token ' + this._syncToken);
this._saveTimer = null; return this._cryptoStore.doTxn(
this._savePromise = this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_DEVICE_DATA], (txn) => { 'readwrite', [IndexedDBCryptoStore.STORE_DEVICE_DATA], (txn) => {
this._cryptoStore.storeEndToEndDeviceData({ this._cryptoStore.storeEndToEndDeviceData({
devices: this._devices, devices: this._devices,
@@ -162,11 +159,12 @@ export default class DeviceList {
syncToken: this._syncToken, syncToken: this._syncToken,
}, txn); }, txn);
}, },
).then(() => { );
this._dirty = false; }).then(() => {
return true; this._dirty = false;
}); this._savePromise = null;
}, 500); return true;
});
} else { } else {
return this._savePromise; return this._savePromise;
} }
@@ -571,6 +569,7 @@ class DeviceListUpdateSerialiser {
this._devices = null; // the complete device list this._devices = null; // the complete device list
this._updatedDevices = null; // device list updates we've fetched this._updatedDevices = null; // device list updates we've fetched
this._syncToken = null; // The sync token we send with the requests
} }
/** /**
@@ -596,6 +595,11 @@ class DeviceListUpdateSerialiser {
this._queuedQueryDeferred = Promise.defer(); this._queuedQueryDeferred = Promise.defer();
} }
// We always take the new sync token and just use the latest one we've
// been given, since it just needs to be at least as recent as the
// sync response the device invalidation message arrived in
this._syncToken = syncToken;
if (this._downloadInProgress) { if (this._downloadInProgress) {
// just queue up these users // just queue up these users
console.log('Queued key download for', users); console.log('Queued key download for', users);
@@ -605,10 +609,10 @@ class DeviceListUpdateSerialiser {
this._devices = devices; this._devices = devices;
this._updatedDevices = {}; this._updatedDevices = {};
// start a new download. // start a new download.
return this._doQueuedQueries(syncToken); return this._doQueuedQueries();
} }
_doQueuedQueries(syncToken) { _doQueuedQueries() {
if (this._downloadInProgress) { if (this._downloadInProgress) {
throw new Error( throw new Error(
"DeviceListUpdateSerialiser._doQueuedQueries called with request active", "DeviceListUpdateSerialiser._doQueuedQueries called with request active",
@@ -624,8 +628,8 @@ class DeviceListUpdateSerialiser {
this._downloadInProgress = true; this._downloadInProgress = true;
const opts = {}; const opts = {};
if (syncToken) { if (this._syncToken) {
opts.token = syncToken; opts.token = this._syncToken;
} }
this._baseApis.downloadKeysForUsers( this._baseApis.downloadKeysForUsers(
@@ -654,7 +658,7 @@ class DeviceListUpdateSerialiser {
// if we have queued users, fire off another request. // if we have queued users, fire off another request.
if (this._queuedQueryDeferred) { if (this._queuedQueryDeferred) {
this._doQueuedQueries(syncToken); this._doQueuedQueries();
} }
}, (e) => { }, (e) => {
console.warn('Error downloading keys for ' + downloadUsers + ':', e); console.warn('Error downloading keys for ' + downloadUsers + ':', e);

10
src/crypto/store/localStorage-crypto-store.js
View File

@@ -135,7 +135,9 @@ export default class LocalStorageCryptoStore extends MemoryCryptoStore {
} }
storeEndToEndDeviceData(deviceData, txn) { storeEndToEndDeviceData(deviceData, txn) {
this.store.setItem(KEY_DEVICE_DATA, deviceData); setJsonItem(
this.store, KEY_DEVICE_DATA, deviceData,
);
} }
/** /**
@@ -151,12 +153,14 @@ export default class LocalStorageCryptoStore extends MemoryCryptoStore {
// Olm account // Olm account
getAccount(txn, func) { getAccount(txn, func) {
const account = this.store.getItem(KEY_END_TO_END_ACCOUNT); const account = getJsonItem(this.store, KEY_END_TO_END_ACCOUNT);
func(account); func(account);
} }
storeAccount(txn, newData) { storeAccount(txn, newData) {
this.store.setItem(KEY_END_TO_END_ACCOUNT, newData); setJsonItem(
this.store, KEY_END_TO_END_ACCOUNT, newData,
);
} }
doTxn(mode, stores, func) { doTxn(mode, stores, func) {