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

Lazy loading: avoid loading members at initial sync for e2e rooms #699

Merged
merged 14 commits into from
Aug 28, 2018

Conversation

bwindels
Copy link
Contributor

No description provided.

…ate events

we were only bailing out when receiving a non JSON-identical m.room.encryption event.
When receiving an identical event, the algorithm in _roomEncryptors would be reset,
generating a new megolm session every time this happens (there is a LL synapse bug
where this happens on every sync).

As the _roomList is backed by indexeddb you might already have a config without the algorithm being present though,
so we first check for the room encryptor algorithm being present. If so, always bail out as setRoomEncryption was
already called for the given room.

If no algorithm is present, still check if the config is not being changed.
Also setup the roomlist and room encryption synchronously before awaiting
the indexeddb operation to store the room encryption config in roomlist.
@bwindels bwindels force-pushed the bwindels/fixlle2ememberfetch-bis branch from 0cda95d to 4cfdaaf Compare August 24, 2018 16:49
@bwindels bwindels force-pushed the bwindels/fixlle2ememberfetch-bis branch from 4cfdaaf to 7d00c0b Compare August 27, 2018 08:54
without LL, we could refresh the device list before all members have been tracked.
as promises, even resolved ones (in case of no LL), always continue async
@bwindels bwindels requested a review from a team August 27, 2018 10:57
@bwindels
Copy link
Contributor Author

This should be ready to have a look now

@bwindels
Copy link
Contributor Author

Ideally I wanted to add some devicelist integration tests to this as well, with LL enabled, but couldn't figure out how the http mocking is supposed to work, so I'll do it in another PR.

@bwindels
Copy link
Contributor Author

Addresses element-hq/element-web#7182 (Item 2 in the list)

@ara4n
Copy link
Member

ara4n commented Aug 27, 2018

why would you need http mocking for e2e tests btw?

@bwindels
Copy link
Contributor Author

@ara4n Not sure if you're think of the e2e tests. I mean the integration tests for e2e encryption device lists (e.g. spec/integ/devicelist-integ-spec). We use MockHttpBackend there...

@bwindels
Copy link
Contributor Author

Started writing a test on bwindels/lldevicelisttest btw, I get Error: Timed out after flushing 1 requests; 3 remaining. Plan to continue on it later on.

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Looks plausible otherwise!

@@ -71,6 +71,9 @@ export default class RoomList {
}

async setRoomEncryption(roomId, roomInfo) {
// important that this happens before calling into the store
// as it prevents the Crypto::setRoomEncryption for calling
Copy link
Member

Choose a reason for hiding this comment

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

s/for/from/

@bwindels bwindels merged commit fcd6dd3 into develop Aug 28, 2018
@bwindels bwindels deleted the bwindels/fixlle2ememberfetch-bis branch August 28, 2018 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants