-
Notifications
You must be signed in to change notification settings - Fork 216
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
Fix JSONDictionary of keys query responses #1707
Conversation
bcd5e6f
to
fb0815f
Compare
fb0815f
to
fa7334b
Compare
@@ -35,7 +35,7 @@ public extension MXRestClient { | |||
failure: @escaping (_ error: NSError?) -> Void) -> MXHTTPOperation { | |||
|
|||
// Do not chunk if not needed | |||
if users.count < chunkSize { | |||
if users.count <= chunkSize { |
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.
Naughty naughty. Will write a test for this in a separate PR as it requires some integration test stubbing
Codecov ReportBase: 17.40% // Head: 37.32% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1707 +/- ##
============================================
+ Coverage 17.40% 37.32% +19.91%
============================================
Files 610 611 +1
Lines 95428 95876 +448
Branches 40198 41447 +1249
============================================
+ Hits 16613 35785 +19172
+ Misses 78309 59043 -19266
- Partials 506 1048 +542
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. |
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.
LGTM
When downloading user keys via
keys/query
we sometimes usedownloadByChunks
method that makes several HTTP requests and combines the result into a singleMXKeysQueryResponse
. The Crypto V2 relies on getting the value of this response viaJSONDictionary
property, however this property was previously just returning whatever initial value was used to create it, and is nil when combining multiple responses together.To solve this we cannot rely on stored property but rather have to compute the
JSONDictionary
out of the current values of the response. Additionally there was a minor bug indownloadByChunks
that would trigger this problem even if we are downloading exactly chunk-sized number of users