From 0c42cd0be45c4a725d1ccf09897086876e8cf257 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Fri, 18 Oct 2019 16:59:10 -0400 Subject: [PATCH 1/3] new count API --- Sources/MongoSwift/MongoCollection+Read.swift | 62 ++++--------------- ...on.swift => CountDocumentsOperation.swift} | 23 ++----- .../EstimatedDocumentCountOperation.swift | 54 ++++++++++++++++ .../MongoSwiftTests/ClientSessionTests.swift | 3 +- Tests/MongoSwiftTests/CodecTests.swift | 16 ++--- .../CommandMonitoringTests.swift | 7 ++- Tests/MongoSwiftTests/CrudTests.swift | 47 +++++++++++--- .../MongoCollectionTests.swift | 28 ++++----- Tests/MongoSwiftTests/OptionsTests.swift | 5 +- .../MongoSwiftTests/ReadPreferenceTests.swift | 6 +- .../ReadWriteConcernTests.swift | 8 ++- .../SpecTestRunner/TestOperation.swift | 12 ++-- .../crud/tests/read/count-collation.json | 20 +++++- Tests/Specs/crud/tests/read/count-empty.json | 39 ++++++++++++ Tests/Specs/crud/tests/read/count.json | 58 ++++++++++++++++- 15 files changed, 272 insertions(+), 116 deletions(-) rename Sources/MongoSwift/Operations/{CountOperation.swift => CountDocumentsOperation.swift} (76%) create mode 100644 Sources/MongoSwift/Operations/EstimatedDocumentCountOperation.swift create mode 100644 Tests/Specs/crud/tests/read/count-empty.json diff --git a/Sources/MongoSwift/MongoCollection+Read.swift b/Sources/MongoSwift/MongoCollection+Read.swift index 9a22ad4f3..2825fa4fb 100644 --- a/Sources/MongoSwift/MongoCollection+Read.swift +++ b/Sources/MongoSwift/MongoCollection+Read.swift @@ -79,50 +79,27 @@ extension SyncMongoCollection { } } - // TODO: SWIFT-133: mark this method deprecated https://jira.mongodb.org/browse/SWIFT-133 /** - * Counts the number of documents in this collection matching the provided filter. + * Counts the number of documents in this collection matching the provided filter. Note that an empty filter will + * force a scan of the entire collection. For a fast count of the total documents in a collection see + * `estimatedDocumentCount`. * * - Parameters: * - filter: a `Document`, the filter that documents must match in order to be counted - * - options: Optional `CountOptions` to use when executing the command + * - options: Optional `CountDocumentsOptions` to use when executing the command * - session: Optional `SyncClientSession` to use when executing this command * * - Returns: The count of the documents that matched the filter - * - * - Throws: - * - `ServerError.commandError` if an error occurs that prevents the command from performing the write. - * - `UserError.invalidArgumentError` if the options passed in form an invalid combination. - * - `EncodingError` if an error occurs while encoding the options to BSON. */ - public func count( + public func countDocuments( _ filter: Document = [:], - options: CountOptions? = nil, + options: CountDocumentsOptions? = nil, session: SyncClientSession? = nil ) throws -> Int { - let operation = CountOperation(collection: self, filter: filter, options: options) + let operation = CountDocumentsOperation(collection: self, filter: filter, options: options) return try self._client.executeOperation(operation, session: session) } - /** - * Counts the number of documents in this collection matching the provided filter. - * - * - Parameters: - * - filter: a `Document`, the filter that documents must match in order to be counted - * - options: Optional `CountDocumentsOptions` to use when executing the command - * - session: Optional `SyncClientSession` to use when executing this command - * - * - Returns: The count of the documents that matched the filter - */ - private func countDocuments( - _: Document = [:], - options _: CountDocumentsOptions? = nil, - session _: SyncClientSession? = nil - ) throws -> Int { - // TODO: SWIFT-133: implement this https://jira.mongodb.org/browse/SWIFT-133 - throw UserError.logicError(message: "Unimplemented command") - } - /** * Gets an estimate of the count of documents in this collection using collection metadata. * @@ -132,12 +109,12 @@ extension SyncMongoCollection { * * - Returns: an estimate of the count of documents in this collection */ - private func estimatedDocumentCount( - options _: EstimatedDocumentCountOptions? = nil, - session _: SyncClientSession? = nil + public func estimatedDocumentCount( + options: EstimatedDocumentCountOptions? = nil, + session: SyncClientSession? = nil ) throws -> Int { - // TODO: SWIFT-133: implement this https://jira.mongodb.org/browse/SWIFT-133 - throw UserError.logicError(message: "Unimplemented command") + let operation = EstimatedDocumentCountOperation(collection: self, options: options) + return try self._client.executeOperation(operation, session: session) } /** @@ -263,21 +240,6 @@ public struct AggregateOptions: Codable { } } -/// The `countDocuments` command takes the same options as the deprecated `count`. -private typealias CountDocumentsOptions = CountOptions - -/// Options to use when executing an `estimatedDocumentCount` command on a `MongoCollection` or a -/// `SyncMongoCollection`. -private struct EstimatedDocumentCountOptions { - /// The maximum amount of time to allow the query to run. - public let maxTimeMS: Int64? - - /// Initializer allowing any/all parameters to be omitted or optional. - public init(maxTimeMS: Int64? = nil) { - self.maxTimeMS = maxTimeMS - } -} - /// The possible types of `MongoCursor` or `SyncMongoCursor` an operation can return. public enum CursorType { /** diff --git a/Sources/MongoSwift/Operations/CountOperation.swift b/Sources/MongoSwift/Operations/CountDocumentsOperation.swift similarity index 76% rename from Sources/MongoSwift/Operations/CountOperation.swift rename to Sources/MongoSwift/Operations/CountDocumentsOperation.swift index 43e6f41ed..4ec8c20b3 100644 --- a/Sources/MongoSwift/Operations/CountOperation.swift +++ b/Sources/MongoSwift/Operations/CountDocumentsOperation.swift @@ -1,7 +1,7 @@ import mongoc -/// Options to use when executing a `count` command on a `MongoCollection` or a `SyncMongoCollection`. -public struct CountOptions: Codable { +/// Options to use when executing a `countDocuments` command on a `MongoCollection`. +public struct CountDocumentsOptions: Codable { /// Specifies a collation. public var collation: Document? @@ -50,12 +50,12 @@ public struct CountOptions: Codable { } /// An operation corresponding to a "count" command on a collection. -internal struct CountOperation: Operation { +internal struct CountDocumentsOperation: Operation { private let collection: SyncMongoCollection private let filter: Document - private let options: CountOptions? + private let options: CountDocumentsOptions? - internal init(collection: SyncMongoCollection, filter: Document, options: CountOptions?) { + internal init(collection: SyncMongoCollection, filter: Document, options: CountDocumentsOptions?) { self.collection = collection self.filter = filter self.options = options @@ -66,18 +66,7 @@ internal struct CountOperation: Operation { let rp = self.options?.readPreference?._readPreference var error = bson_error_t() let count = self.collection.withMongocCollection(from: connection) { collPtr in - // because we already encode skip and limit in the options, - // pass in 0s so we don't get duplicate parameter errors. - mongoc_collection_count_with_opts( - collPtr, - MONGOC_QUERY_NONE, - self.filter._bson, - 0, // skip - 0, // limit - opts?._bson, - rp, - &error - ) + mongoc_collection_count_documents(collPtr, self.filter._bson, opts?._bson, rp, nil, &error) } guard count != -1 else { throw extractMongoError(error: error) } diff --git a/Sources/MongoSwift/Operations/EstimatedDocumentCountOperation.swift b/Sources/MongoSwift/Operations/EstimatedDocumentCountOperation.swift new file mode 100644 index 000000000..7eb50989a --- /dev/null +++ b/Sources/MongoSwift/Operations/EstimatedDocumentCountOperation.swift @@ -0,0 +1,54 @@ +import mongoc + +/// Options to use when executing an `estimatedDocumentCount` command on a `MongoCollection`. +public struct EstimatedDocumentCountOptions: Codable { + /// The maximum amount of time to allow the query to run. + public var maxTimeMS: Int64? + + /// A ReadConcern to use for this operation. + public var readConcern: ReadConcern? + + // swiftlint:disable redundant_optional_initialization + /// A ReadPreference to use for this operation. + public var readPreference: ReadPreference? = nil + // swiftlint:enable redundant_optional_initialization + + /// Convenience initializer allowing any/all parameters to be optional + public init( + maxTimeMS: Int64? = nil, + readConcern: ReadConcern? = nil, + readPreference: ReadPreference? = nil + ) { + self.maxTimeMS = maxTimeMS + self.readConcern = readConcern + self.readPreference = readPreference + } + + private enum CodingKeys: String, CodingKey { + case maxTimeMS, readConcern + } +} + +/// An operation corresponding to a "count" command on a collection. +internal struct EstimatedDocumentCountOperation: Operation { + private let collection: SyncMongoCollection + private let options: EstimatedDocumentCountOptions? + + internal init(collection: SyncMongoCollection, options: EstimatedDocumentCountOptions?) { + self.collection = collection + self.options = options + } + + internal func execute(using connection: Connection, session: SyncClientSession?) throws -> Int { + let opts = try encodeOptions(options: options, session: session) + let rp = self.options?.readPreference?._readPreference + var error = bson_error_t() + let count = self.collection.withMongocCollection(from: connection) { collPtr in + mongoc_collection_estimated_document_count(collPtr, opts?._bson, rp, nil, &error) + } + + guard count != -1 else { throw extractMongoError(error: error) } + + return Int(count) + } +} diff --git a/Tests/MongoSwiftTests/ClientSessionTests.swift b/Tests/MongoSwiftTests/ClientSessionTests.swift index b454d0761..caa0b2a36 100644 --- a/Tests/MongoSwiftTests/ClientSessionTests.swift +++ b/Tests/MongoSwiftTests/ClientSessionTests.swift @@ -35,7 +35,8 @@ final class ClientSessionTests: MongoSwiftTestCase { (name: "find", body: { _ = try $0.find([:], session: $1).nextOrError() }), (name: "aggregate", body: { _ = try $0.aggregate([], session: $1).nextOrError() }), (name: "distinct", body: { _ = try $0.distinct(fieldName: "x", session: $1) }), - (name: "count", body: { _ = try $0.count(session: $1) }) + (name: "countDocuments", body: { _ = try $0.countDocuments(session: $1) }), + (name: "estimatedDocumentCount", body: { _ = try $0.estimatedDocumentCount(session: $1) }) ] // list of write operations on SyncMongoCollection that take in a session diff --git a/Tests/MongoSwiftTests/CodecTests.swift b/Tests/MongoSwiftTests/CodecTests.swift index fa387e112..716d00a8d 100644 --- a/Tests/MongoSwiftTests/CodecTests.swift +++ b/Tests/MongoSwiftTests/CodecTests.swift @@ -805,14 +805,14 @@ final class CodecTests: MongoSwiftTestCase { "writeConcern" ])) - let count = CountOptions( - collation: Document(), - hint: .indexName("hint"), - limit: 123, - maxTimeMS: 12, - readConcern: rc, - readPreference: rp, - skip: 123 + let count = CountDocumentsOptions( + collation: Document(), + hint: .indexName("hint"), + limit: 123, + maxTimeMS: 12, + readConcern: rc, + readPreference: rp, + skip: 123 ) expect(try encoder.encode(count).keys.sorted()).to(equal([ "collation", diff --git a/Tests/MongoSwiftTests/CommandMonitoringTests.swift b/Tests/MongoSwiftTests/CommandMonitoringTests.swift index 5476244c1..97fa87310 100644 --- a/Tests/MongoSwiftTests/CommandMonitoringTests.swift +++ b/Tests/MongoSwiftTests/CommandMonitoringTests.swift @@ -22,9 +22,12 @@ final class CommandMonitoringTests: MongoSwiftTestCase { // read in the file data and parse into a struct let name = filename.components(separatedBy: ".")[0] - // remove this if/when bulkwrite is supported + // TODO: SWIFT-346: remove this skip if name.lowercased().contains("bulkwrite") { continue } + // remove this when command.json is updated with the new count API (see SPEC-1272) + if name.lowercased() == "command" { continue } + print("-----------------------") print("Executing tests for file \(name)...\n") @@ -149,7 +152,7 @@ private struct CMTest: Decodable { switch self.op.name { case "count": - _ = try? collection.count(filter) + _ = try? collection.countDocuments(filter) case "deleteMany": _ = try? collection.deleteMany(filter) case "deleteOne": diff --git a/Tests/MongoSwiftTests/CrudTests.swift b/Tests/MongoSwiftTests/CrudTests.swift index 4a29acbf8..743fa7014 100644 --- a/Tests/MongoSwiftTests/CrudTests.swift +++ b/Tests/MongoSwiftTests/CrudTests.swift @@ -30,7 +30,7 @@ final class CrudTests: MongoSwiftTestCase { print("\n------------\nExecuting tests from file \(dir)/\(filename)...\n") // For each file, execute the test cases contained in it - for (i, test) in file.tests.enumerated() { + for (i, test) in try file.makeTests().enumerated() { print("Executing test: \(test.description)") // for each test case: @@ -40,10 +40,18 @@ final class CrudTests: MongoSwiftTestCase { // 4) verify that expected data is present // 5) drop the collection to clean up let collection = db.collection(self.getCollectionName(suffix: "\(filename)_\(i)")) - try collection.insertMany(file.data) + if !file.data.isEmpty { + try collection.insertMany(file.data) + } try test.execute(usingCollection: collection) try test.verifyData(testCollection: collection, db: db) - try collection.drop() + do { + try collection.drop() + } catch let ServerError.commandError(code, _, _, _) where code == 26 { + // ignore ns not found errors + } catch { + throw error + } } } print() // for readability of results @@ -64,7 +72,18 @@ final class CrudTests: MongoSwiftTestCase { private struct CrudTestFile: Decodable { let data: [Document] let testDocs: [Document] - var tests: [CrudTest] { return try! self.testDocs.map { try makeCrudTest($0) } } + func makeTests() throws -> [CrudTest] { + return try testDocs + // skip any test cases within a file that use "count" + .filter { doc in + let op: Document = try doc.get("operation") + return op["name"] != "count" + } + .map { + try makeCrudTest($0) + } + } + let minServerVersion: String? let maxServerVersion: String? @@ -87,10 +106,11 @@ private func makeCrudTest(_ doc: Document) throws -> CrudTest { private var testTypeMap: [String: CrudTest.Type] = [ "aggregate": AggregateTest.self, "bulkWrite": BulkWriteTest.self, - "count": CountTest.self, + "countDocuments": CountDocumentsTest.self, "deleteMany": DeleteTest.self, "deleteOne": DeleteTest.self, "distinct": DistinctTest.self, + "estimatedDocumentCount": EstimatedDocumentCountTest.self, "find": FindTest.self, "findOneAndDelete": FindOneAndDeleteTest.self, "findOneAndUpdate": FindOneAndUpdateTest.self, @@ -268,12 +288,21 @@ private class BulkWriteTest: CrudTest { } } -/// A class for executing `count` tests -private class CountTest: CrudTest { +/// A class for executing `countDocuments` tests +private class CountDocumentsTest: CrudTest { override func execute(usingCollection coll: SyncMongoCollection) throws { let filter: Document = try self.args.get("filter") - let options = CountOptions(collation: self.collation, limit: self.limit, skip: self.skip) - let result = try coll.count(filter, options: options) + let options = CountDocumentsOptions(collation: self.collation, limit: self.limit, skip: self.skip) + let result = try coll.countDocuments(filter, options: options) + expect(result).to(equal(self.result?.asInt())) + } +} + +/// A class for executing `estimatedDocumentCount` tests +private class EstimatedDocumentCountTest: CrudTest { + override func execute(usingCollection coll: SyncMongoCollection) throws { + let options = EstimatedDocumentCountOptions() + let result = try coll.estimatedDocumentCount(options: options) expect(result).to(equal(self.result?.asInt())) } } diff --git a/Tests/MongoSwiftTests/MongoCollectionTests.swift b/Tests/MongoSwiftTests/MongoCollectionTests.swift index e735dc537..7359b331a 100644 --- a/Tests/MongoSwiftTests/MongoCollectionTests.swift +++ b/Tests/MongoSwiftTests/MongoCollectionTests.swift @@ -63,16 +63,16 @@ final class MongoCollectionTests: MongoSwiftTestCase { } func testCount() throws { - expect(try self.coll.count()).to(equal(2)) - let options = CountOptions(limit: 5, maxTimeMS: 1000, skip: 5) - expect(try self.coll.count(options: options)).to(equal(0)) + expect(try self.coll.countDocuments()).to(equal(2)) + let options = CountDocumentsOptions(limit: 5, maxTimeMS: 1000, skip: 5) + expect(try self.coll.countDocuments(options: options)).to(equal(0)) } func testInsertOne() throws { expect(try self.coll.deleteMany([:])).toNot(beNil()) expect(try self.coll.insertOne(self.doc1)?.insertedId).to(equal(1)) expect(try self.coll.insertOne(self.doc2)?.insertedId).to(equal(2)) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // try inserting a document without an ID let docNoID: Document = ["x": 1] @@ -136,7 +136,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { } func testInsertMany() throws { - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // try inserting a mix of documents with and without IDs to verify they are generated let docNoId1: Document = ["x": 1] let docNoId2: Document = ["x": 2] @@ -326,7 +326,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { try coll1.insertOne(b1) try coll1.insertMany([b2, b3]) try coll1.replaceOne(filter: ["x": 2], replacement: b4) - expect(try coll1.count()).to(equal(3)) + expect(try coll1.countDocuments()).to(equal(3)) for doc in try coll1.find() { expect(doc).to(beAnInstanceOf(Basic.self)) @@ -394,13 +394,13 @@ final class MongoCollectionTests: MongoSwiftTestCase { let opts1 = FindOneAndDeleteOptions(maxTimeMS: 100) let result1 = try self.coll.findOneAndDelete(["cat": "cat"], options: opts1) expect(result1).to(equal(self.doc2)) - expect(try self.coll.count()).to(equal(1)) + expect(try self.coll.countDocuments()).to(equal(1)) // test using a write concern let opts2 = FindOneAndDeleteOptions(writeConcern: try WriteConcern(w: .majority)) let result2 = try self.coll.findOneAndDelete([:], options: opts2) expect(result2).to(equal(self.doc1)) - expect(try self.coll.count()).to(equal(0)) + expect(try self.coll.countDocuments()).to(equal(0)) // test invalid maxTimeMS throws error let invalidOpts1 = FindOneAndDeleteOptions(maxTimeMS: 0) @@ -420,7 +420,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { options: opts1 ) expect(result1).to(equal(self.doc2)) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // test using bypassDocumentValidation let opts2 = FindOneAndReplaceOptions(bypassDocumentValidation: true) @@ -430,7 +430,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { options: opts2 ) expect(result2).to(equal(self.doc1)) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // test using a write concern let opts3 = FindOneAndReplaceOptions(writeConcern: try WriteConcern(w: .majority)) @@ -440,7 +440,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { options: opts3 ) expect(result3).to(equal(["_id": 2, "cat": "blah"])) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // test invalid maxTimeMS throws error let invalidOpts1 = FindOneAndReplaceOptions(maxTimeMS: 0) @@ -460,7 +460,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { options: opts1 ) expect(result1).to(equal(self.doc2)) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // test using bypassDocumentValidation let opts2 = FindOneAndUpdateOptions(bypassDocumentValidation: true) @@ -470,7 +470,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { options: opts2 ) expect(result2).to(equal(self.doc1)) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // test using a write concern let opts3 = FindOneAndUpdateOptions(writeConcern: try WriteConcern(w: .majority)) @@ -480,7 +480,7 @@ final class MongoCollectionTests: MongoSwiftTestCase { options: opts3 ) expect(result3).to(equal(["_id": 2, "cat": "blah"])) - expect(try self.coll.count()).to(equal(2)) + expect(try self.coll.countDocuments()).to(equal(2)) // test invalid maxTimeMS throws error let invalidOpts1 = FindOneAndUpdateOptions(maxTimeMS: 0) diff --git a/Tests/MongoSwiftTests/OptionsTests.swift b/Tests/MongoSwiftTests/OptionsTests.swift index 906db2ea7..e6ac386bf 100644 --- a/Tests/MongoSwiftTests/OptionsTests.swift +++ b/Tests/MongoSwiftTests/OptionsTests.swift @@ -28,13 +28,14 @@ final class OptionsTests: MongoSwiftTestCase { DropCollectionOptions(), CollectionOptions(), DropDatabaseOptions(), - CountOptions(), CreateCollectionOptions(), CreateIndexOptions(), DistinctOptions(), DropIndexOptions(), ListCollectionsOptions(), - RunCommandOptions() + RunCommandOptions(), + CountDocumentsOptions(), + EstimatedDocumentCountOptions() ] // This will be useful with Swift 5.1 auto-generated initializers diff --git a/Tests/MongoSwiftTests/ReadPreferenceTests.swift b/Tests/MongoSwiftTests/ReadPreferenceTests.swift index e96da94f9..96c4ce700 100644 --- a/Tests/MongoSwiftTests/ReadPreferenceTests.swift +++ b/Tests/MongoSwiftTests/ReadPreferenceTests.swift @@ -116,8 +116,10 @@ final class ReadPreferenceTests: MongoSwiftTestCase { )) .toNot(throwError()) - expect(try coll.count(options: CountOptions(readPreference: ReadPreference(.secondaryPreferred)))) - .toNot(throwError()) + expect(try coll.countDocuments( + options: + CountDocumentsOptions(readPreference: ReadPreference(.secondaryPreferred)) + )).toNot(throwError()) expect(try coll.distinct( fieldName: "a", diff --git a/Tests/MongoSwiftTests/ReadWriteConcernTests.swift b/Tests/MongoSwiftTests/ReadWriteConcernTests.swift index e0a38019e..533cbee75 100644 --- a/Tests/MongoSwiftTests/ReadWriteConcernTests.swift +++ b/Tests/MongoSwiftTests/ReadWriteConcernTests.swift @@ -423,7 +423,13 @@ final class ReadWriteConcernTests: MongoSwiftTestCase { options: AggregateOptions(readConcern: ReadConcern(.majority)) )).toNot(throwError()) - expect(try coll.count(options: CountOptions(readConcern: ReadConcern(.majority)))).toNot(throwError()) + expect(try coll.countDocuments(options: CountDocumentsOptions(readConcern: ReadConcern(.majority)))) + .toNot(throwError()) + + expect(try coll.estimatedDocumentCount( + options: + EstimatedDocumentCountOptions(readConcern: ReadConcern(.majority)) + )).toNot(throwError()) expect(try coll.distinct( fieldName: "a", diff --git a/Tests/MongoSwiftTests/SpecTestRunner/TestOperation.swift b/Tests/MongoSwiftTests/SpecTestRunner/TestOperation.swift index 5c823cde4..1926c62a4 100644 --- a/Tests/MongoSwiftTests/SpecTestRunner/TestOperation.swift +++ b/Tests/MongoSwiftTests/SpecTestRunner/TestOperation.swift @@ -27,8 +27,8 @@ struct AnyTestOperation: Decodable { switch opName { case "aggregate": self.op = try container.decode(Aggregate.self, forKey: .arguments) - case "count": - self.op = try container.decode(Count.self, forKey: .arguments) + case "countDocuments": + self.op = try container.decode(CountDocuments.self, forKey: .arguments) case "distinct": self.op = try container.decode(Distinct.self, forKey: .arguments) case "find": @@ -88,14 +88,14 @@ struct Aggregate: TestOperation { } } -struct Count: TestOperation { +struct CountDocuments: TestOperation { let filter: Document - let options: CountOptions + let options: CountDocumentsOptions private enum CodingKeys: String, CodingKey { case filter } init(from decoder: Decoder) throws { - self.options = try CountOptions(from: decoder) + self.options = try CountDocumentsOptions(from: decoder) let container = try decoder.container(keyedBy: CodingKeys.self) self.filter = try container.decode(Document.self, forKey: .filter) } @@ -106,7 +106,7 @@ struct Count: TestOperation { collection: SyncMongoCollection, session: SyncClientSession? = nil ) throws -> TestOperationResult? { - return .int(try collection.count(self.filter, options: self.options, session: session)) + return .int(try collection.countDocuments(self.filter, options: self.options, session: session)) } } diff --git a/Tests/Specs/crud/tests/read/count-collation.json b/Tests/Specs/crud/tests/read/count-collation.json index 6ea9ab81c..6f75282fe 100644 --- a/Tests/Specs/crud/tests/read/count-collation.json +++ b/Tests/Specs/crud/tests/read/count-collation.json @@ -8,7 +8,25 @@ "minServerVersion": "3.4", "tests": [ { - "description": "Count with collation", + "description": "Count documents with collation", + "operation": { + "name": "countDocuments", + "arguments": { + "filter": { + "x": "ping" + }, + "collation": { + "locale": "en_US", + "strength": 2 + } + } + }, + "outcome": { + "result": 1 + } + }, + { + "description": "Deprecated count with collation", "operation": { "name": "count", "arguments": { diff --git a/Tests/Specs/crud/tests/read/count-empty.json b/Tests/Specs/crud/tests/read/count-empty.json new file mode 100644 index 000000000..2b8627e0c --- /dev/null +++ b/Tests/Specs/crud/tests/read/count-empty.json @@ -0,0 +1,39 @@ +{ + "data": [], + "tests": [ + { + "description": "Estimated document count with empty collection", + "operation": { + "name": "estimatedDocumentCount", + "arguments": {} + }, + "outcome": { + "result": 0 + } + }, + { + "description": "Count documents with empty collection", + "operation": { + "name": "countDocuments", + "arguments": { + "filter": {} + } + }, + "outcome": { + "result": 0 + } + }, + { + "description": "Deprecated count with empty collection", + "operation": { + "name": "count", + "arguments": { + "filter": {} + } + }, + "outcome": { + "result": 0 + } + } + ] +} diff --git a/Tests/Specs/crud/tests/read/count.json b/Tests/Specs/crud/tests/read/count.json index 5020771d1..9642b2fbd 100644 --- a/Tests/Specs/crud/tests/read/count.json +++ b/Tests/Specs/crud/tests/read/count.json @@ -15,7 +15,59 @@ ], "tests": [ { - "description": "Count without a filter", + "description": "Estimated document count", + "operation": { + "name": "estimatedDocumentCount", + "arguments": {} + }, + "outcome": { + "result": 3 + } + }, + { + "description": "Count documents without a filter", + "operation": { + "name": "countDocuments", + "arguments": { + "filter": {} + } + }, + "outcome": { + "result": 3 + } + }, + { + "description": "Count documents with a filter", + "operation": { + "name": "countDocuments", + "arguments": { + "filter": { + "_id": { + "$gt": 1 + } + } + } + }, + "outcome": { + "result": 2 + } + }, + { + "description": "Count documents with skip and limit", + "operation": { + "name": "countDocuments", + "arguments": { + "filter": {}, + "skip": 1, + "limit": 3 + } + }, + "outcome": { + "result": 2 + } + }, + { + "description": "Deprecated count without a filter", "operation": { "name": "count", "arguments": { @@ -27,7 +79,7 @@ } }, { - "description": "Count with a filter", + "description": "Deprecated count with a filter", "operation": { "name": "count", "arguments": { @@ -43,7 +95,7 @@ } }, { - "description": "Count with skip and limit", + "description": "Deprecated count with skip and limit", "operation": { "name": "count", "arguments": { From 764a5292292e1ac275df72b2ead1661b4cd7c5e5 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Wed, 23 Oct 2019 01:09:57 -0400 Subject: [PATCH 2/3] Use a placeholder for old count API tests --- Tests/MongoSwiftTests/CrudTests.swift | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Tests/MongoSwiftTests/CrudTests.swift b/Tests/MongoSwiftTests/CrudTests.swift index 743fa7014..4d0c7cc16 100644 --- a/Tests/MongoSwiftTests/CrudTests.swift +++ b/Tests/MongoSwiftTests/CrudTests.swift @@ -31,6 +31,10 @@ final class CrudTests: MongoSwiftTestCase { // For each file, execute the test cases contained in it for (i, test) in try file.makeTests().enumerated() { + if type(of: test) == CountTest.self { + print("Skipping test for old count API, no longer supported by the driver") + } + print("Executing test: \(test.description)") // for each test case: @@ -72,16 +76,9 @@ final class CrudTests: MongoSwiftTestCase { private struct CrudTestFile: Decodable { let data: [Document] let testDocs: [Document] + func makeTests() throws -> [CrudTest] { - return try testDocs - // skip any test cases within a file that use "count" - .filter { doc in - let op: Document = try doc.get("operation") - return op["name"] != "count" - } - .map { - try makeCrudTest($0) - } + return try self.testDocs.map { try makeCrudTest($0) } } let minServerVersion: String? @@ -106,6 +103,7 @@ private func makeCrudTest(_ doc: Document) throws -> CrudTest { private var testTypeMap: [String: CrudTest.Type] = [ "aggregate": AggregateTest.self, "bulkWrite": BulkWriteTest.self, + "count": CountTest.self, "countDocuments": CountDocumentsTest.self, "deleteMany": DeleteTest.self, "deleteOne": DeleteTest.self, @@ -288,6 +286,11 @@ private class BulkWriteTest: CrudTest { } } +/// A class for executing `count` tests +private class CountTest: CrudTest { + override func execute(usingCollection coll: SyncMongoCollection) throws {} +} + /// A class for executing `countDocuments` tests private class CountDocumentsTest: CrudTest { override func execute(usingCollection coll: SyncMongoCollection) throws { From e515afe9f50ea60b3bc469ba6ece127314eb9ef9 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Wed, 23 Oct 2019 01:10:50 -0400 Subject: [PATCH 3/3] fmt --- Tests/MongoSwiftTests/CodecTests.swift | 14 +++++++------- Tests/MongoSwiftTests/CrudTests.swift | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Tests/MongoSwiftTests/CodecTests.swift b/Tests/MongoSwiftTests/CodecTests.swift index 716d00a8d..5f69f3f9a 100644 --- a/Tests/MongoSwiftTests/CodecTests.swift +++ b/Tests/MongoSwiftTests/CodecTests.swift @@ -806,13 +806,13 @@ final class CodecTests: MongoSwiftTestCase { ])) let count = CountDocumentsOptions( - collation: Document(), - hint: .indexName("hint"), - limit: 123, - maxTimeMS: 12, - readConcern: rc, - readPreference: rp, - skip: 123 + collation: Document(), + hint: .indexName("hint"), + limit: 123, + maxTimeMS: 12, + readConcern: rc, + readPreference: rp, + skip: 123 ) expect(try encoder.encode(count).keys.sorted()).to(equal([ "collation", diff --git a/Tests/MongoSwiftTests/CrudTests.swift b/Tests/MongoSwiftTests/CrudTests.swift index 4d0c7cc16..6e9f63ac3 100644 --- a/Tests/MongoSwiftTests/CrudTests.swift +++ b/Tests/MongoSwiftTests/CrudTests.swift @@ -288,7 +288,7 @@ private class BulkWriteTest: CrudTest { /// A class for executing `count` tests private class CountTest: CrudTest { - override func execute(usingCollection coll: SyncMongoCollection) throws {} + override func execute(usingCollection _: SyncMongoCollection) throws {} } /// A class for executing `countDocuments` tests