-
Notifications
You must be signed in to change notification settings - Fork 270
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
feat: Introduce EncryptionState
#4777
Conversation
8019667
to
69e4edd
Compare
c02659c
to
cf7e678
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4777 +/- ##
==========================================
- Coverage 86.38% 86.37% -0.02%
==========================================
Files 291 291
Lines 34293 34303 +10
==========================================
+ Hits 29624 29629 +5
- Misses 4669 4674 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, you left in a merge conflict marker which we should get rid of, but that's about it.
if !self.encryption_state_synced { | ||
EncryptionState::Unknown | ||
} else if self.base_info.encryption.is_some() { | ||
EncryptionState::Encrypted | ||
} else { | ||
EncryptionState::NotEncrypted | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be nice to replace encryption_state_synced
and base_info.encryption
with EncryptionState
where Encrypted
keeps the m.room.encryption
content as associated data.
But that's probably something for a different PR and might not even be worth the trouble.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked about that in a meeting and agreed to address that in another PR.
7460970
to
45ae5f5
Compare
This patch introduces the new `EncryptionState` to represent the 3 possible states: `Encrypted`, `NotEncrypted` or `Unknown`. All the `is_encrypted` methods have been replaced by `encryption_state`. The most noticable change is in `matrix_sdk::Room` where `async fn is_encrypted(&self) -> Result<bool>` has been replaced by `fn fn encryption_state(&self) -> EncryptionState`. However, a new `async fn latest_encryption_state(&self) -> Result<EncryptionState>` method “restores” the previous behaviour by calling `request_encryption_state` if necessary. The idea is that the caller is now responsible to call `request_encryption_state` if desired, or use `latest_encryption_state` to automate the call if necessary. `encryption_state` is now non-async and infallible everywhere. `matrix-sdk-ffi` has been updated but no methods have been added for the moment.
d007455
to
161eb8d
Compare
161eb8d
to
3c6ae5a
Compare
This patch introduces the new
EncryptionState
to represent the 3possible states:
Encrypted
,NotEncrypted
orUnknown
. All theis_encrypted
methods have been replaced byencryption_state
.The most noticable change is in
matrix_sdk::Room
whereasync fn is_encrypted(&self) -> Result<bool>
has been replaced byfn fn encryption_state(&self) -> EncryptionState
. However, a newasync fn latest_encryption_state(&self) -> Result<EncryptionState>
method“restores” the previous behaviour by calling
request_encryption_state
if necessary.
The idea is that the caller is now responsible to call
request_encryption_state
if desired, or uselatest_encryption_state
to automate the call if necessary.
encryption_state
is now non-asyncand infallible everywhere.
matrix-sdk-ffi
has been updated but no methods have been added forthe moment.
Room::is_encrypted
is now async #4756