-
Notifications
You must be signed in to change notification settings - Fork 67
SWIFT-936 Use new BSON library #594
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
SWIFT-936 Use new BSON library #594
Conversation
| * Throws an `MongoError.InternalError` if the bson_t isn't proper BSON. | ||
| */ | ||
| internal init(copying bsonPtr: BSONPointer) throws { | ||
| internal init(copying bsonPtr: BSONPointer) { |
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.
This will be updated in a subsequent PR to reduce the amount of validation being done to match what we used to do via bson_copy, namely just verify that the encoded length matches the length of the data.
| // Adds a custom "sortedEqual" predicate that compares two `Document`s and returns true if they | ||
| // have the same key/value pairs in them | ||
| public func sortedEqual(_ expectedValue: BSONDocument?) -> Predicate<BSONDocument> { | ||
| public func sortedEqual<T: SortedEquatable>(_ expectedValue: T?) -> Predicate<T> { |
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.
In order to extend this matcher to BSON (which may contain arrays with documents or documents themselves), I introduced a protocol and made the matcher generic over it. This became more useful as more things became unordered due to the usage of ExtrasJSON instead of libbson.
|
|
||
| public init(from decoder: Decoder) throws { | ||
| self.failPoint = try BSONDocument(from: decoder) | ||
| let container = try decoder.singleValueContainer() |
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.
This is required due to the ordering of the command getting messed up during decoding from JSON.
|
|
||
| /// Return a new `RunCommand` with the command document ordered such that the provided command name | ||
| /// is the first key. | ||
| func withCommandName(_ name: String) -> RunCommand { |
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.
ditto required to reorder the command properly
| } | ||
|
|
||
| extension BSONDocument: SortedEquatable { | ||
| public func sortedEquals(_ other: BSONDocument) -> Bool { |
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.
We have an implementation of this in swift-bson now (called equalsIgnoreKeyOrder), but it currently has a bug where it doesn't compare documents contained in an array via equalsIgnoreKeyOrder. I filed SWIFT-1086 to cover fixing that. Once that's done we can remove this implementation of sortedEquals and just call through to equalsIgnoreKeyOrder.
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 and it's really exciting to finally be doing this 🎉 I just have some small questions/nits
kmahar
left a comment
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!
SWIFT-936
This PR updates the driver to use
swift-bson, the new pure-Swift BSON implementation. It consists of mostly testing changes and should have no effect on the public API. I think the CRUD tests were synced due to a bug in them or something. I did it a while ago though so I'm not exactly sure.