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

Complete MXCryptoV2 implementation #1620

Merged
merged 5 commits into from
Oct 31, 2022
Merged

Complete MXCryptoV2 implementation #1620

merged 5 commits into from
Oct 31, 2022

Conversation

Anderas
Copy link
Contributor

@Anderas Anderas commented Oct 26, 2022

A large final round of changes to the MXCryptoV2 implementation to make it ready for alpha testing. The changes in this PR include:

  • Extract additional methods out of MXCrypto protocol and into MXLegacyCrypto implementation, as they are not needed with V2 (the functionality is handled internally by CryptoMachine)
  • Annotate all of MXCrypto with nullability to make swift code safer and more explicit. At times this means making more things nullable than necessary just to handle any rogue objective-c nils
  • Update MatrixSDKCrypto to the latest version which includes EncryptionSetting
  • Completely reorganize MXCryptoV2 to group related functionality (apologies for impact on code review)

@Anderas Anderas requested review from a team and ismailgulek and removed request for a team October 26, 2022 15:56
@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Base: 15.90% // Head: 36.37% // Increases project coverage by +20.46% 🎉

Coverage data is based on head (befcd80) compared to base (d74b476).
Patch coverage: 14.67% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #1620       +/-   ##
============================================
+ Coverage    15.90%   36.37%   +20.46%     
============================================
  Files          577      577               
  Lines        90876    90876               
  Branches     38355    39535     +1180     
============================================
+ Hits         14454    33056    +18602     
+ Misses       75953    56837    -19116     
- Partials       469      983      +514     
Impacted Files Coverage Δ
...trixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift 21.25% <0.00%> (-1.12%) ⬇️
...trixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift 0.00% <0.00%> (ø)
.../Crypto/KeySharing/MXSharedHistoryKeyManager.swift 90.41% <0.00%> (ø)
MatrixSDK/Crypto/MXCrypto.m 64.69% <ø> (+62.65%) ⬆️
MatrixSDK/Crypto/MXCryptoV2.swift 0.92% <0.00%> (+0.02%) ⬆️
...K/Crypto/SecretStorage/MXCryptoSecretStoreV2.swift 0.00% <0.00%> (ø)
...sts/Crypto/MXLegacyCrypto+LegacyCrossSigning.swift 57.14% <ø> (+57.14%) ⬆️
MatrixSDKTests/MXBackgroundSyncServiceTests.swift 1.84% <0.00%> (+1.84%) ⬆️
MatrixSDKTests/MXCryptoShareTests.m 39.90% <0.00%> (+39.90%) ⬆️
MatrixSDKTests/MXLazyLoadingTests.m 0.00% <0.00%> (ø)
... and 198 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

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.

LGTM! This is a huge improvement 👏

Copy link

@phloux phloux left a comment

Choose a reason for hiding this comment

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

LGTM

@Anderas Anderas mentioned this pull request Oct 31, 2022
@Anderas Anderas merged commit 4dc8446 into develop Oct 31, 2022
@Anderas Anderas deleted the andy/complete_crypto branch October 31, 2022 17:55
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