Skip to content

Conversation

@kmahar
Copy link
Contributor

@kmahar kmahar commented Oct 14, 2019

This implements the auth JSON and prose tests as best as we can right now.

I am putting this up as a PR against the spec file helper branch for now so it can be reviewed separately. I'll switch the base branch to master once that gets merged.

FYI this fails on Travis Swift 4.2 right now. I didn't bother special-casing the call to mongoc_uri_get_mechanism_properties as I expect that #330 dropping 4.2 will be merged before this is.

I also made a somewhat related change to ConnectionString as part of this, will explain more in inline comment.

/// final options set on the `ConnectionString`.
internal init(_ connectionString: String, options: inout ClientOptions) throws {
/// Initializes a new `ConnectionString` with the provided options.
internal init(_ connectionString: String, options: ClientOptions? = nil) throws {
Copy link
Contributor Author

@kmahar kmahar Oct 14, 2019

Choose a reason for hiding this comment

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

for some reason I wrote this clunky inout API where the connection string would update the client options struct to reflect the final values on the URI so that the MongoClient could have correct values for read concern, write concern, and read preference.
however, it's way simpler to just have MongoClient read back those values by calling the appropriate getters on the connection string after it creates it (see MongoClient.init.)

mongoc_uri_set_option_as_bool(self._uri, MONGOC_URI_RETRYREADS, rr)
}

// we can't get values for retryReads and retryWrites individually, so we will read
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the client doesn't get the options struct back anymore, and doesn't actually care what the values of retryReads and retryWrites are, so we don't need to worry about retrieving those values anymore to send back.

@kmahar kmahar force-pushed the SWIFT-599/specfile-helper branch from 1a17a85 to 961959d Compare October 14, 2019 14:43
@kmahar kmahar force-pushed the SWIFT-429/auth-tests branch from cec6caf to 24897d3 Compare October 14, 2019 14:49
Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

one comment about future work, otherwise looks good to me.

@kmahar kmahar force-pushed the SWIFT-429/auth-tests branch from 24897d3 to 56d282f Compare October 14, 2019 16:50
@kmahar kmahar changed the base branch from SWIFT-599/specfile-helper to master October 14, 2019 18:36
@kmahar kmahar force-pushed the SWIFT-429/auth-tests branch from b7ac3b5 to 7eaebb1 Compare October 14, 2019 18:44
@kmahar kmahar force-pushed the SWIFT-429/auth-tests branch from 7eaebb1 to 3e8f1a4 Compare October 15, 2019 17:02
@kmahar kmahar force-pushed the SWIFT-429/auth-tests branch from fb1e24a to 08d4812 Compare October 15, 2019 21:33
@kmahar kmahar merged commit e3dee07 into master Oct 16, 2019
@kmahar kmahar deleted the SWIFT-429/auth-tests branch October 16, 2019 00:42
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.

4 participants