SWIFT-779 Safe access to ReadPreference pointer #456
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kmahar
commented
Apr 21, 2020
Codecov Report
@@ Coverage Diff @@
## master #456 +/- ##
==========================================
+ Coverage 76.42% 76.62% +0.19%
==========================================
Files 117 117
Lines 12935 12982 +47
==========================================
+ Hits 9886 9947 +61
+ Misses 3049 3035 -14
Continue to review full report at Codecov.
|
nbbeeken
reviewed
Apr 23, 2020
| .target(name: "TestsCommon", dependencies: ["MongoSwift", "Nimble"]), | ||
| .testTarget(name: "BSONTests", dependencies: ["MongoSwift", "TestsCommon", "Nimble", "CLibMongoC"]), | ||
| .testTarget(name: "MongoSwiftTests", dependencies: ["MongoSwift", "TestsCommon", "Nimble", "NIO", "CLibMongoC"]), | ||
| .testTarget(name: "MongoSwiftTests", dependencies: ["MongoSwift", "TestsCommon", "Nimble", "NIO"]), |
nbbeeken
approved these changes
Apr 23, 2020
Contributor
Author
|
whoops sorry i was trying to actually remove your name @mbroadst but i guess since you commented once already you are stuck on the list forever |
Contributor
patrickfreed
left a comment
patrickfreed
left a comment
There was a problem hiding this comment.
looks good, just a few minor comments
patrickfreed
approved these changes
Apr 24, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@mbroadst I'm not sure if you want me to keep including you on these, LMK.
This PR switches
ReadPreferenceover to the "create a temporary libmongoc equivalent, use it to execute a closure, and then clean it up" approach we follow forReadConcern,WriteConcern,MongoDatabase, andMongoCollection.We had existing tests regarding
ReadConcernandWriteConcernthat verified these got set correctly on mongoc clients, dbs, and collections. I think we skipped these forReadPreferenceat the time because it was just wrapping amongoc_read_prefs_tso there was no need to test out the temporary creation of the libmongoc type.I have now added similar tests for
ReadPreference. As part of that I also moved libmongoc-dependent helper methods for getting theReadConcern/WriteConcernon a temporary db/collection into the driver itself. This allowed me to remove the CLibMongoC dependency fromMongoSwiftTests🎉Now we just need to remove it fromBSONTests.I also added some round trip testing between Swift and libmongoc types for all three of RC, WC, RP.