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

Store sharedHistory flag for inbound Megolm sessions #1444

Merged
merged 7 commits into from
Apr 26, 2022

Conversation

Anderas
Copy link
Contributor

@Anderas Anderas commented Apr 21, 2022

Relates to element-hq/element-ios#4947 which is an implementation of matrix-org/matrix-spec-proposals#3061

This PR stores a sharedHistory flag on all MXOlmInboundGroupSessions, which reflects the current history visibility of a room. In a future PR this flag will be used to determine which sessions can be shared with other users when invited.

The changes in this PR:

  • add sharedHistory flag to MXOlmInboundGroupSession and other related objects, so that it can be both persisted as well as backed up to the server
  • add roomId to MXRealmOlmInboundGroupSession so that inbound sessions can be fetched by roomId in the future (i.e. "share keys per room"). Otherwise we would need to fetch them all, deserialize the data and only then filter in memory.
  • extact a method out MXMegolmEncryption.m which checks whether a session should be reset and add another check based on room history visibility change

@Anderas Anderas requested review from a team, ismailgulek and ariskotsomitopoulos and removed request for a team April 21, 2022 14:28
@Anderas Anderas requested a review from uhoreg April 21, 2022 14:42
@Anderas Anderas requested a review from ismailgulek April 22, 2022 15:13
@@ -47,8 +47,14 @@
"MXHTTPAdditionalHeadersUnitTests",
"MXJSONModelUnitTests",
"MXKeyProviderUnitTests",
"MXLoggerUnitTests\/testDeleteLogFiles",
Copy link
Contributor

@ismailgulek ismailgulek Apr 22, 2022

Choose a reason for hiding this comment

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

Can you manually edit the file to just include the class names? And can you also include them in UnitTestsWithSanitizers.xctestplan?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, thx

Copy link
Contributor

@ismailgulek ismailgulek left a comment

Choose a reason for hiding this comment

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

Just one comment about test plan file, otherwise LGTM 👍

@Anderas Anderas requested a review from ismailgulek April 22, 2022 15:48
@Anderas Anderas merged commit e373d15 into develop Apr 26, 2022
@Anderas Anderas deleted the andy/4947_shared_history branch April 26, 2022 07:21
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.

2 participants