-
Notifications
You must be signed in to change notification settings - Fork 65
SWIFT-137 Sketch remainder of CRUD API #83
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
import libmongoc | ||
|
||
/// An extension of `MongoCollection` encapsulating bulk write operations. | ||
extension MongoCollection { | ||
/** | ||
* Execute multiple write operations. | ||
* | ||
* - Parameters: | ||
* - requests: a `[WriteModel]` containing the writes to perform | ||
* - options: optional `BulkWriteOptions` to use while executing the operation | ||
* | ||
* - Returns: a `BulkWriteResult`, or `nil` if the write concern is unacknowledged. | ||
* | ||
* - Throws: | ||
* - `MongoError.invalidArgument` if `requests` is empty | ||
* - `MongoError.bulkWriteError` if any error occurs while performing the writes | ||
*/ | ||
@discardableResult | ||
public func bulkWrite(_ requests: [WriteModel], options: BulkWriteOptions? = nil) throws -> BulkWriteResult? { | ||
throw MongoError.commandError(message: "Unimplemented command") | ||
} | ||
} | ||
|
||
/// Options to use when performing a bulk write operation on a `MongoCollection`. | ||
public struct BulkWriteOptions: Encodable { | ||
/// If `true` (the default), when an insert fails, return without performing the remaining writes. | ||
/// If `false`, when a write fails, continue with the remaining writes, if any. | ||
public var ordered: Bool = true | ||
|
||
/// If `true`, allows the write to opt-out of document level validation. | ||
public let bypassDocumentValidation: Bool? | ||
} | ||
|
||
/// A protocol indicating write types that can be batched together using `MongoCollection.bulkWrite`. | ||
public protocol WriteModel: Encodable {} | ||
|
||
/// A model for an `insertOne` command. | ||
public struct InsertOneModel: WriteModel { | ||
/// The `Document` to insert. | ||
public let document: Document | ||
} | ||
|
||
/// A model for a `deleteOne` command. | ||
public struct DeleteOneModel: WriteModel { | ||
/// A `Document` representing the match criteria. | ||
public let filter: Document | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
} | ||
|
||
/// A model for a `deleteMany` command. | ||
public struct DeleteManyModel: WriteModel { | ||
/// A `Document` representing the match criteria. | ||
public let filter: Document | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
} | ||
|
||
/// A model for a `replaceOne` command. | ||
public struct ReplaceOneModel: WriteModel { | ||
/// A `Document` representing the match criteria. | ||
public let filter: Document | ||
|
||
/// The `Document` with which to replace the matched document. | ||
public let replacement: Document | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
|
||
/// When `true`, creates a new document if no document matches the query. | ||
public let upsert: Bool? | ||
} | ||
|
||
/// A model for an `updateOne` command. | ||
public struct UpdateOneModel: WriteModel { | ||
/// A `Document` representing the match criteria. | ||
public let filter: Document | ||
|
||
/// A `Document` containing update operators. | ||
public let update: Document | ||
|
||
/// A set of filters specifying to which array elements an update should apply. | ||
public let arrayFilters: [Document]? | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
|
||
/// When `true`, creates a new document if no document matches the query. | ||
public let upsert: Bool? | ||
} | ||
|
||
/// A model for an `updateMany` command. | ||
public struct UpdateManyModel: WriteModel { | ||
/// A `Document` representing the match criteria. | ||
public let filter: Document | ||
|
||
/// A `Document` containing update operators. | ||
public let update: Document | ||
|
||
/// A set of filters specifying to which array elements an update should apply. | ||
public let arrayFilters: [Document]? | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
|
||
/// When `true`, creates a new document if no document matches the query. | ||
public let upsert: Bool? | ||
} | ||
|
||
/// The result of a bulk write operation on a `MongoCollection`. | ||
public struct BulkWriteResult: Decodable { | ||
/// Number of documents inserted. | ||
public let insertedCount: Int | ||
|
||
/// Map of the index of the operation to the id of the inserted document. | ||
public let insertedIds: [Int: AnyBsonValue] | ||
|
||
/// Number of documents matched for update. | ||
public let matchedCount: Int | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to the above comment, the update and deleted counts originate from the server and won't be available if the write concern was unacknowledged. I think everything apart from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I just remembered that you do have |
||
|
||
/// Number of documents modified. | ||
public let modifiedCount: Int | ||
|
||
/// Number of documents deleted. | ||
public let deletedCount: Int | ||
|
||
/// Number of documents upserted. | ||
public let upsertedCount: Int | ||
|
||
/// Map of the index of the operation to the id of the upserted document. | ||
public let upsertedIds: [Int: AnyBsonValue] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
import libmongoc | ||
|
||
/// An extension of `MongoCollection` encapsulating find and modify operations. | ||
extension MongoCollection { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note that I have modified all the find and modify commands to support using the |
||
/** | ||
* Finds a single document and deletes it, returning the original. | ||
* | ||
* - Parameters: | ||
* - filter: `Document` representing the match criteria | ||
* - options: Optional `FindOneAndDeleteOptions` to use when executing the command | ||
* | ||
* - Returns: The deleted document, represented as a `CollectionType`, or `nil` if no document was deleted. | ||
* - Throws: | ||
* - `MongoError.commandError` if there are any errors executing the command. | ||
* - A `DecodingError` if the deleted document cannot be decoded to a `CollectionType` value | ||
*/ | ||
@discardableResult | ||
public func findOneAndDelete(_ filter: Document, options: FindOneAndDeleteOptions? = nil) throws -> CollectionType? { | ||
throw MongoError.commandError(message: "Unimplemented command") | ||
} | ||
|
||
/** | ||
* Finds a single document and replaces it, returning either the original or the replaced document. | ||
* | ||
* - Parameters: | ||
* - filter: `Document` representing the match criteria | ||
* - replacement: a `CollectionType` to replace the found document | ||
* - options: Optional `FindOneAndReplaceOptions` to use when executing the command | ||
* | ||
* - Returns: A `CollectionType`, representing either the original document or its replacement, | ||
* depending on selected options, or `nil` if there was no match. | ||
* - Throws: | ||
* - `MongoError.commandError` if there are any errors executing the command. | ||
* - An `EncodingError` if `replacement` cannot be encoded to a `Document` | ||
* - A `DecodingError` if the replaced document cannot be decoded to a `CollectionType` value | ||
*/ | ||
@discardableResult | ||
public func findOneAndReplace(filter: Document, replacement: CollectionType, | ||
options: FindOneAndDeleteOptions? = nil) throws -> CollectionType? { | ||
throw MongoError.commandError(message: "Unimplemented command") | ||
} | ||
|
||
/** | ||
* Finds a single document and updates it, returning either the original or the updated document. | ||
* | ||
* - Parameters: | ||
* - filter: `Document` representing the match criteria | ||
* - update: a `Document` containing updates to apply | ||
* - options: Optional `FindOneAndUpdateOptions` to use when executing the command | ||
* | ||
* - Returns: A `CollectionType` representing either the original or updated document, | ||
* depending on selected options, or `nil` if there was no match. | ||
* - Throws: | ||
* - `MongoError.commandError` if there are any errors executing the command. | ||
* - A `DecodingError` if the updated document cannot be decoded to a `CollectionType` value | ||
*/ | ||
@discardableResult | ||
public func findOneAndUpdate(filter: Document, update: Document, | ||
options: FindOneAndUpdateOptions? = nil) throws -> CollectionType? { | ||
throw MongoError.commandError(message: "Unimplemented command") | ||
} | ||
} | ||
|
||
/// Indicates which document to return in a find and modify operation. | ||
public enum ReturnDocument: Encodable { | ||
/// Indicates to return the document before the update, replacement, or insert occured. | ||
case before | ||
|
||
/// Indicates to return the document after the update, replacement, or insert occured. | ||
case after | ||
|
||
public func encode(to encoder: Encoder) throws { | ||
// fill in later on | ||
} | ||
} | ||
|
||
/// Options to use when executing a `findOneAndDelete` command on a `MongoCollection`. | ||
public struct FindOneAndDeleteOptions: Encodable { | ||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
|
||
/// The maximum amount of time to allow the query to run. | ||
public let maxTimeMS: Int64? | ||
|
||
/// Limits the fields to return for the matching document. | ||
public let projection: Document? | ||
|
||
/// Determines which document the operation modifies if the query selects multiple documents. | ||
public let sort: Document? | ||
|
||
/// An optional `WriteConcern` to use for the command. | ||
public let writeConcern: WriteConcern? | ||
} | ||
|
||
/// Options to use when executing a `findOneAndReplace` command on a `MongoCollection`. | ||
public struct FindOneAndReplaceOptions: Encodable { | ||
/// If `true`, allows the write to opt-out of document level validation. | ||
public let bypassDocumentValidation: Bool? | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
|
||
/// The maximum amount of time to allow the query to run. | ||
public let maxTimeMS: Int64? | ||
|
||
/// Limits the fields to return for the matching document. | ||
public let projection: Document? | ||
|
||
/// When `ReturnDocument.After`, returns the replaced or inserted document rather than the original. | ||
public let returnDocument: ReturnDocument? | ||
|
||
/// Determines which document the operation modifies if the query selects multiple documents. | ||
public let sort: Document? | ||
|
||
/// When `true`, creates a new document if no document matches the query. | ||
public let upsert: Bool? | ||
|
||
/// An optional `WriteConcern` to use for the command. | ||
public let writeConcern: WriteConcern? | ||
} | ||
|
||
/// Options to use when executing a `findOneAndUpdate` command on a `MongoCollection`. | ||
public struct FindOneAndUpdateOptions: Encodable { | ||
/// A set of filters specifying to which array elements an update should apply. | ||
public let arrayFilters: [Document]? | ||
|
||
/// If `true`, allows the write to opt-out of document level validation. | ||
public let bypassDocumentValidation: Bool? | ||
|
||
/// Specifies a collation to use. | ||
public let collation: Document? | ||
|
||
/// The maximum amount of time to allow the query to run. | ||
public let maxTimeMS: Int64? | ||
|
||
/// Limits the fields to return for the matching document. | ||
public let projection: Document? | ||
|
||
/// When`ReturnDocument.After`, returns the updated or inserted document rather than the original. | ||
public let returnDocument: ReturnDocument? | ||
|
||
/// Determines which document the operation modifies if the query selects multiple documents. | ||
public let sort: Document? | ||
|
||
/// When `true`, creates a new document if no document matches the query. | ||
public let upsert: Bool? | ||
|
||
/// An optional `WriteConcern` to use for the command. | ||
public let writeConcern: WriteConcern? | ||
} |
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.
Does it make sense to provide accessors for write errors, write concern errors, and whether the write was acknowledged? PHP does that with WriteResult.
If a bulk write execution results in an error, libmongoc will create an error message for us but users should still have a programmatic API to inspect the errors. This is particularly relevant if there are both write and write concern errors.
On a separate note, users may want to know whether a write was acknowledged as some fields should not be accessible otherwise (or they should be
nil
).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.
given our conversation earlier, not totally sure how we want to handle this....
would it make sense to throw from
MongoCollection.bulkWrite
if there is a write error or write concern error? that would mean if we actually get to the point of returning aBulkWriteResult
, there are no errors.Uh oh!
There was an error while loading. Please reload this page.
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.
PHP determines whether to throw an exception based on the return value of
mongoc_bulk_operation_execute()
. That function does return0
in the event of a write or write concern error.I think it's easier to convert the entire libmongoc reply document into a write result struct and stuff that onto an error rather than split the document into separate entities (document for count/ID fields,
writeErrors
array, andwriteConcernError
document).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.
so I think for an unacknowledged write we should just return nil. we are already doing (or supposed to be doing) that in some other places in the driver, and since we have optionals I think it's more idiomatic. so this method should actually return a
BulkWriteResult?
.re: errors, IMO it's kind of confusing to users if there's an error field when there doesn't need to be - they might think they need to manually check each
BulkWriteResult
they get back to see ifwriteErrors
orwriteConcernErrors
have anything in them.instead I propose we add these cases to
MongoError
:so then when
mongoc_bulk_operation_execute
fails, we will throw abulkWriteError
that stores all the info. when it succeeds (and the write is acknowledged), we'll just return aBulkWriteResult
.it does require a bit of work to split up the info, but I think our BSON decoder will automate some of that, and it saves users having to go down two nested levels (error -> result struct -> error fields) to actually see the errors.
thoughts?
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.
If the Swift driver has no intention of reporting inserted IDs, I suppose
BulkWriteResult?
makes sensein place of aBulkWriteResult
consisting entirely of unset optionals and a singleisAcknowledged = false
property.I personally think it's useful for drivers to report inserted IDs, since those are often generated by the driver when a document without an
_id
field is inserted; however, a reasonable counter-argument is that any users that actually care about the inserted ID could simply set_id
themselves before the insert operation.I think that
bulkWriteError
should still have a message and/or code. libmongoc is already going to provide abson_error_t
for us and we can use that as we normally do. The only extra behavior is that we also want to stuff thebson_t
reply into the error as well.One thought is that BulkWriteResult may need to be optional. Consider the scenario where an unacknowledged write encounters a network error. libmongoc is still going to fill out a
bson_error_t
even if there isn't anything to report in thebson_t
reply.I'm personally in favor of BulkWriteResult matching the
bson_t
reply provided by libmongoc, but if that complicates things I won't object. I imagine not splitting out thewriteErrors
andwriteConcernError
fields might prevent us from being able to re-use the same BulkWriteResult struct for both the successfulbulkWrite()
return value and thebulkWriteError
type. In that case, let's aim for re-use.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.
^ just updated my 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.
Should
writeErrors: [MongoError]
be[MongoError]?
, or would you ensure thatwriteErrors
is an empty array even if the only bulk write error was thewriteConcernError
?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.
I was imagining we'd just use an empty array, but thinking about it more, I think making it an optional is better. it's nicer to be able to say like
if let writeErrs = error.writeErrors { ... }
instead ofif errors.writeErrors.count > 0 { ... }
.also saves us creating a bunch of unused arrays
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.
let's discuss the bulkWrite / unacknowledged stuff in triage as this is blocking further progress on our CRUD work
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 concluded to keep the existing insertOne/insertMany behavior and have operations return an optional result object (i.e.
nil
for{w:0}
).