From 07fd317afa91851cfa0065e664f1b5a88ec96fa9 Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Thu, 10 Dec 2020 11:40:05 -0500 Subject: [PATCH] refactor: all write commands now inherit from CommandOperation (#2665) This patch moves the command construction for write operations from the wire protocol layer to the operation layer. Specifically, the insert/update/delete operations, which are also shared by the bulk implementation, are now the primary sites for write command construction. This has a few implications, primarily that you can no longer "write" with a `Server` instance, you can only execute a write operation against it. NODE-2953 --- src/bulk/common.ts | 3 +- src/change_stream.ts | 3 +- src/cmap/connection.ts | 16 - src/cmap/wire_protocol/command.ts | 3 +- src/cmap/wire_protocol/index.ts | 38 --- src/cmap/wire_protocol/write_command.ts | 84 ----- src/collection.ts | 68 ++-- src/cursor/find_cursor.ts | 2 +- src/index.ts | 10 +- src/operations/aggregate.ts | 8 +- src/operations/bulk_write.ts | 6 +- src/operations/command.ts | 27 +- src/operations/common_functions.ts | 99 +----- src/operations/delete.ts | 207 +++++------- src/operations/find.ts | 3 +- src/operations/indexes.ts | 8 +- src/operations/insert.ts | 127 +++++--- src/operations/insert_many.ts | 61 ---- src/operations/operation.ts | 3 +- src/operations/replace_one.ts | 72 ---- src/operations/update.ts | 276 ++++++++++++---- src/sdam/server.ts | 96 +----- src/sessions.ts | 7 +- test/functional/aggregation.test.js | 2 + test/functional/apm.test.js | 20 +- test/functional/collection.test.js | 10 +- test/functional/core/tailable_cursor.test.js | 89 ++--- test/functional/core/undefined.test.js | 326 ++++++------------- test/functional/document_validation.test.js | 2 +- test/functional/insert.test.js | 2 +- test/functional/uri.test.js | 20 +- test/spec/apm/README.rst | 36 +- test/spec/apm/bulkWrite.json | 10 +- test/spec/apm/bulkWrite.yml | 8 +- test/spec/apm/insertMany.json | 4 +- test/spec/apm/insertMany.yml | 2 +- test/spec/apm/unacknowledgedBulkWrite.json | 13 +- test/spec/apm/unacknowledgedBulkWrite.yml | 8 +- test/spec/apm/updateMany.json | 6 +- test/spec/apm/updateMany.yml | 2 - test/spec/apm/updateOne.json | 10 +- test/spec/apm/updateOne.yml | 5 - 42 files changed, 679 insertions(+), 1123 deletions(-) delete mode 100644 src/cmap/wire_protocol/write_command.ts delete mode 100644 src/operations/insert_many.ts delete mode 100644 src/operations/replace_one.ts diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 6a2a9ff41a..46278f1909 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -18,8 +18,7 @@ import { DeleteOperation } from '../operations/delete'; import { WriteConcern } from '../write_concern'; import type { Collection } from '../collection'; import type { Topology } from '../sdam/topology'; -import type { CommandOperationOptions } from '../operations/command'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; +import type { CommandOperationOptions, CollationOptions } from '../operations/command'; import type { Hint } from '../operations/operation'; // Error codes diff --git a/src/change_stream.ts b/src/change_stream.ts index 19c5716585..5d8582e3af 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -15,8 +15,7 @@ import { import type { ReadPreference } from './read_preference'; import type { Timestamp, Document } from './bson'; import type { Topology } from './sdam/topology'; -import type { OperationParent } from './operations/command'; -import type { CollationOptions } from './cmap/wire_protocol/write_command'; +import type { OperationParent, CollationOptions } from './operations/command'; import { MongoClient } from './mongo_client'; import { Db } from './db'; import { Collection } from './collection'; diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 20be55169c..ce4f0ef174 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -30,7 +30,6 @@ import type { GetMoreOptions } from './wire_protocol/get_more'; import type { Stream } from './connect'; import type { LoggerOptions } from '../logger'; import type { QueryOptions } from './wire_protocol/query'; -import type { WriteCommandOptions } from './wire_protocol/write_command'; const kStream = Symbol('stream'); const kQueue = Symbol('queue'); @@ -278,21 +277,6 @@ export class Connection extends EventEmitter { killCursors(ns: string, cursorIds: Long[], options: CommandOptions, callback: Callback): void { wp.killCursors(makeServerTrampoline(this), ns, cursorIds, options, callback); } - - /** @internal */ - insert(ns: string, ops: Document[], options: WriteCommandOptions, callback: Callback): void { - wp.insert(makeServerTrampoline(this), ns, ops, options, callback); - } - - /** @internal */ - update(ns: string, ops: Document[], options: WriteCommandOptions, callback: Callback): void { - wp.update(makeServerTrampoline(this), ns, ops, options, callback); - } - - /** @internal */ - remove(ns: string, ops: Document[], options: WriteCommandOptions, callback: Callback): void { - wp.remove(makeServerTrampoline(this), ns, ops, options, callback); - } } /** diff --git a/src/cmap/wire_protocol/command.ts b/src/cmap/wire_protocol/command.ts index 75da460800..80585d94f6 100644 --- a/src/cmap/wire_protocol/command.ts +++ b/src/cmap/wire_protocol/command.ts @@ -9,7 +9,6 @@ import type { Server } from '../../sdam/server'; import type { Topology } from '../../sdam/topology'; import type { ReadPreferenceLike } from '../../read_preference'; import type { WriteConcernOptions, WriteConcern, W } from '../../write_concern'; -import type { WriteCommandOptions } from './write_command'; /** @public */ export interface CommandOptions extends BSONSerializeOptions { @@ -105,7 +104,7 @@ function _command( clusterTime = session.clusterTime; } - const err = applySession(session, finalCmd, options as WriteCommandOptions); + const err = applySession(session, finalCmd, options as CommandOptions); if (err) { return callback(err); } diff --git a/src/cmap/wire_protocol/index.ts b/src/cmap/wire_protocol/index.ts index 1ebb40826a..d4fc9f16ff 100644 --- a/src/cmap/wire_protocol/index.ts +++ b/src/cmap/wire_protocol/index.ts @@ -1,42 +1,4 @@ -import type { Server } from '../../sdam/server'; - export { killCursors } from './kill_cursors'; export { getMore } from './get_more'; export { query } from './query'; export { command } from './command'; - -import { writeCommand, WriteCommandOptions } from './write_command'; -import type { Document } from '../../bson'; -import type { Callback } from '../../utils'; - -export { writeCommand }; - -export function insert( - server: Server, - ns: string, - ops: Document[], - options: WriteCommandOptions, - callback: Callback -): void { - writeCommand(server, 'insert', 'documents', ns, ops, options, callback); -} - -export function update( - server: Server, - ns: string, - ops: Document[], - options: WriteCommandOptions, - callback: Callback -): void { - writeCommand(server, 'update', 'updates', ns, ops, options, callback); -} - -export function remove( - server: Server, - ns: string, - ops: Document[], - options: WriteCommandOptions, - callback: Callback -): void { - writeCommand(server, 'delete', 'deletes', ns, ops, options, callback); -} diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts deleted file mode 100644 index b35668ab5a..0000000000 --- a/src/cmap/wire_protocol/write_command.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { MongoError } from '../../error'; -import { collectionNamespace, Callback, decorateWithExplain } from '../../utils'; -import { command, CommandOptions } from './command'; -import type { Server } from '../../sdam/server'; -import type { Document, BSONSerializeOptions } from '../../bson'; -import type { WriteConcern } from '../../write_concern'; -import { Explain, ExplainOptions } from '../../explain'; - -/** @public */ -export interface CollationOptions { - locale: string; - caseLevel: boolean; - caseFirst: string; - strength: number; - numericOrdering: boolean; - alternate: string; - maxVariable: string; - backwards: boolean; -} - -/** @internal */ -export interface WriteCommandOptions extends BSONSerializeOptions, CommandOptions, ExplainOptions { - ordered?: boolean; - writeConcern?: WriteConcern; - collation?: CollationOptions; - bypassDocumentValidation?: boolean; -} - -export function writeCommand( - server: Server, - type: string, - opsField: string, - ns: string, - ops: Document[], - options: WriteCommandOptions, - callback: Callback -): void { - if (ops.length === 0) throw new MongoError(`${type} must contain at least one document`); - if (typeof options === 'function') { - callback = options; - options = {}; - } - - options = options ?? {}; - const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; - const writeConcern = options.writeConcern; - let writeCommand: Document = {}; - writeCommand[type] = collectionNamespace(ns); - writeCommand[opsField] = ops; - writeCommand.ordered = ordered; - - if (writeConcern && Object.keys(writeConcern).length > 0) { - writeCommand.writeConcern = writeConcern; - } - - if (options.collation) { - for (let i = 0; i < writeCommand[opsField].length; i++) { - if (!writeCommand[opsField][i].collation) { - writeCommand[opsField][i].collation = options.collation; - } - } - } - - if (options.bypassDocumentValidation === true) { - writeCommand.bypassDocumentValidation = options.bypassDocumentValidation; - } - - // If a command is to be explained, we need to reformat the command after - // the other command properties are specified. - const explain = Explain.fromOptions(options); - if (explain) { - writeCommand = decorateWithExplain(writeCommand, explain); - } - - const commandOptions = Object.assign( - { - checkKeys: type === 'insert', - numberToReturn: 1 - }, - options - ); - - command(server, ns, writeCommand, commandOptions, callback); -} diff --git a/src/collection.ts b/src/collection.ts index ebdffc7f72..59bd8290b2 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -51,13 +51,20 @@ import { FindOneAndUpdateOperation, FindAndModifyOptions } from './operations/find_and_modify'; -import { InsertManyOperation, InsertManyResult } from './operations/insert_many'; -import { InsertOneOperation, InsertOneOptions, InsertOneResult } from './operations/insert'; +import { + InsertOneOperation, + InsertOneOptions, + InsertOneResult, + InsertManyOperation, + InsertManyResult +} from './operations/insert'; import { UpdateOneOperation, UpdateManyOperation, UpdateOptions, - UpdateResult + UpdateResult, + ReplaceOneOperation, + ReplaceOptions } from './operations/update'; import { DeleteOneOperation, @@ -74,7 +81,6 @@ import { } from './operations/map_reduce'; import { OptionsOperation } from './operations/options_operation'; import { RenameOperation, RenameOptions } from './operations/rename'; -import { ReplaceOneOperation, ReplaceOptions } from './operations/replace_one'; import { CollStatsOperation, CollStatsOptions } from './operations/stats'; import { executeOperation } from './operations/execute_operation'; import type { Db } from './db'; @@ -387,21 +393,25 @@ export class Collection { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - updateOne(filter: Document, update: Document): Promise; - updateOne(filter: Document, update: Document, callback: Callback): void; - updateOne(filter: Document, update: Document, options: UpdateOptions): Promise; + updateOne(filter: Document, update: Document): Promise; + updateOne(filter: Document, update: Document, callback: Callback): void; + updateOne( + filter: Document, + update: Document, + options: UpdateOptions + ): Promise; updateOne( filter: Document, update: Document, options: UpdateOptions, - callback: Callback + callback: Callback ): void; updateOne( filter: Document, update: Document, - options?: UpdateOptions | Callback, - callback?: Callback - ): Promise | void { + options?: UpdateOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); return executeOperation( @@ -419,25 +429,29 @@ export class Collection { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - replaceOne(filter: Document, replacement: Document): Promise; - replaceOne(filter: Document, replacement: Document, callback: Callback): void; + replaceOne(filter: Document, replacement: Document): Promise; + replaceOne( + filter: Document, + replacement: Document, + callback: Callback + ): void; replaceOne( filter: Document, replacement: Document, options: ReplaceOptions - ): Promise; + ): Promise; replaceOne( filter: Document, replacement: Document, options: ReplaceOptions, - callback: Callback + callback: Callback ): void; replaceOne( filter: Document, replacement: Document, - options?: ReplaceOptions | Callback, - callback?: Callback - ): Promise | void { + options?: ReplaceOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); return executeOperation( @@ -455,21 +469,25 @@ export class Collection { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - updateMany(filter: Document, update: Document): Promise; - updateMany(filter: Document, update: Document, callback: Callback): void; - updateMany(filter: Document, update: Document, options: UpdateOptions): Promise; + updateMany(filter: Document, update: Document): Promise; + updateMany(filter: Document, update: Document, callback: Callback): void; + updateMany( + filter: Document, + update: Document, + options: UpdateOptions + ): Promise; updateMany( filter: Document, update: Document, options: UpdateOptions, - callback: Callback + callback: Callback ): void; updateMany( filter: Document, update: Document, - options?: UpdateOptions | Callback, - callback?: Callback - ): Promise | void { + options?: UpdateOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); return executeOperation( diff --git a/src/cursor/find_cursor.ts b/src/cursor/find_cursor.ts index bfa0645662..d6dbf0567c 100644 --- a/src/cursor/find_cursor.ts +++ b/src/cursor/find_cursor.ts @@ -1,11 +1,11 @@ import type { Document } from '../bson'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import { MongoError } from '../error'; import type { ExplainVerbosityLike } from '../explain'; import { CountOperation, CountOptions } from '../operations/count'; import { executeOperation, ExecutionResult } from '../operations/execute_operation'; import { FindOperation, FindOptions } from '../operations/find'; import type { Hint } from '../operations/operation'; +import type { CollationOptions } from '../operations/command'; import type { Topology } from '../sdam/topology'; import type { ClientSession } from '../sessions'; import { formatSort, Sort, SortDirection } from '../sort'; diff --git a/src/index.ts b/src/index.ts index 1dec8b68f0..f89a571d41 100644 --- a/src/index.ts +++ b/src/index.ts @@ -139,7 +139,6 @@ export type { CommandOptions } from './cmap/wire_protocol/command'; export type { CompressorName, Compressor } from './cmap/wire_protocol/compression'; export type { GetMoreOptions } from './cmap/wire_protocol/get_more'; export type { QueryOptions } from './cmap/wire_protocol/query'; -export type { CollationOptions, WriteCommandOptions } from './cmap/wire_protocol/write_command'; export type { CollectionPrivate, CollectionOptions } from './collection'; export type { AggregationCursorOptions } from './cursor/aggregation_cursor'; export type { @@ -189,7 +188,8 @@ export type { export type { CommandOperationOptions, OperationParent, - CommandOperation + CommandOperation, + CollationOptions } from './operations/command'; export type { IndexInformationOptions } from './operations/common_functions'; export type { CountOptions } from './operations/count'; @@ -211,8 +211,7 @@ export type { ListIndexesOptions, IndexDirection } from './operations/indexes'; -export type { InsertOneResult, InsertOneOptions } from './operations/insert'; -export type { InsertManyResult } from './operations/insert_many'; +export type { InsertOneResult, InsertOneOptions, InsertManyResult } from './operations/insert'; export type { ListCollectionsOptions } from './operations/list_collections'; export type { ListDatabasesResult, ListDatabasesOptions } from './operations/list_databases'; export type { @@ -225,11 +224,10 @@ export type { Hint, OperationOptions, AbstractOperation } from './operations/ope export type { ProfilingLevelOptions } from './operations/profiling_level'; export type { RemoveUserOptions } from './operations/remove_user'; export type { RenameOptions } from './operations/rename'; -export type { ReplaceOptions } from './operations/replace_one'; export type { RunCommandOptions } from './operations/run_command'; export type { ProfilingLevel, SetProfilingLevelOptions } from './operations/set_profiling_level'; export type { CollStatsOptions, DbStatsOptions } from './operations/stats'; -export type { UpdateResult, UpdateOptions } from './operations/update'; +export type { UpdateResult, UpdateOptions, ReplaceOptions } from './operations/update'; export type { ValidateCollectionOptions } from './operations/validate_collection'; export type { ReadConcern, diff --git a/src/operations/aggregate.ts b/src/operations/aggregate.ts index 2607b5fcab..8b3d0a7588 100644 --- a/src/operations/aggregate.ts +++ b/src/operations/aggregate.ts @@ -1,4 +1,9 @@ -import { CommandOperation, CommandOperationOptions, OperationParent } from './command'; +import { + CommandOperation, + CommandOperationOptions, + OperationParent, + CollationOptions +} from './command'; import { ReadPreference } from '../read_preference'; import { MongoError } from '../error'; import { maxWireVersion } from '../utils'; @@ -6,7 +11,6 @@ import { Aspect, defineAspects, Hint } from './operation'; import type { Callback } from '../utils'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ClientSession } from '../sessions'; /** @internal */ diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 19e581eaf0..8ab1f913e0 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -34,9 +34,9 @@ export class BulkWriteOperation extends AbstractOperation { // Create the bulk operation const bulk: BulkOperationBase = - options.ordered === true || options.ordered == null - ? coll.initializeOrderedBulkOp(options) - : coll.initializeUnorderedBulkOp(options); + options.ordered === false + ? coll.initializeUnorderedBulkOp(options) + : coll.initializeOrderedBulkOp(options); // for each op go through and add to the bulk try { diff --git a/src/operations/command.ts b/src/operations/command.ts index 2092578623..bba464adf4 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -8,12 +8,23 @@ import { MongoError } from '../error'; import type { Logger } from '../logger'; import type { Server } from '../sdam/server'; import type { BSONSerializeOptions, Document } from '../bson'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; import { Explain, ExplainOptions } from '../explain'; const SUPPORTS_WRITE_CONCERN_AND_COLLATION = 5; +/** @public */ +export interface CollationOptions { + locale: string; + caseLevel: boolean; + caseFirst: string; + strength: number; + numericOrdering: boolean; + alternate: string; + maxVariable: string; + backwards: boolean; +} + /** @public */ export interface CommandOperationOptions extends OperationOptions, @@ -126,12 +137,16 @@ export abstract class CommandOperation extends AbstractOperation { return; } - if (serverWireVersion >= SUPPORTS_WRITE_CONCERN_AND_COLLATION) { - if (this.writeConcern && this.hasAspect(Aspect.WRITE_OPERATION) && !inTransaction) { - Object.assign(cmd, { writeConcern: this.writeConcern }); - } + if (this.writeConcern && this.hasAspect(Aspect.WRITE_OPERATION) && !inTransaction) { + Object.assign(cmd, { writeConcern: this.writeConcern }); + } - if (options.collation && typeof options.collation === 'object') { + if (serverWireVersion >= SUPPORTS_WRITE_CONCERN_AND_COLLATION) { + if ( + options.collation && + typeof options.collation === 'object' && + !this.hasAspect(Aspect.SKIP_COLLATION) + ) { Object.assign(cmd, { collation: options.collation }); } } diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index 7dcb44c12b..11e4731d85 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -1,19 +1,10 @@ import { MongoError } from '../error'; -import { - applyRetryableWrites, - decorateWithCollation, - Callback, - getTopology, - maxWireVersion -} from '../utils'; +import { Callback, getTopology } from '../utils'; import type { Document } from '../bson'; import type { Db } from '../db'; import type { ClientSession } from '../sessions'; -import type { Server } from '../sdam/server'; import type { ReadPreference } from '../read_preference'; import type { Collection } from '../collection'; -import type { UpdateOptions } from './update'; -import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; /** @public */ export interface IndexInformationOptions { @@ -102,91 +93,3 @@ export function prepareDocs( return doc; }); } - -export function updateDocuments( - server: Server, - coll: Collection, - selector: Document, - document: Document, - callback: Callback -): void; -export function updateDocuments( - server: Server, - coll: Collection, - selector: Document, - document: Document, - options: UpdateOptions, - callback: Callback -): void; -export function updateDocuments( - server: Server, - coll: Collection, - selector: Document, - document: Document, - _options: UpdateOptions | Callback, - _callback?: Callback -): void { - let options = _options as UpdateOptions; - let callback = _callback as Callback; - if ('function' === typeof options) { - callback = options; - options = {}; - } - - // If we are not providing a selector or document throw - if (selector == null || typeof selector !== 'object') - return callback(new TypeError('selector must be a valid JavaScript object')); - if (document == null || typeof document !== 'object') - return callback(new TypeError('document must be a valid JavaScript object')); - - // Final options for retryable writes - let finalOptions = Object.assign({}, options); - finalOptions = applyRetryableWrites(finalOptions, coll.s.db); - - // Execute the operation - const op: Document = { q: selector, u: document }; - op.upsert = options.upsert !== void 0 ? !!options.upsert : false; - op.multi = options.multi !== void 0 ? !!options.multi : false; - - if (options.hint) { - op.hint = options.hint; - } - - if (finalOptions.arrayFilters) { - op.arrayFilters = finalOptions.arrayFilters; - delete finalOptions.arrayFilters; - } - - if (finalOptions.retryWrites && op.multi) { - finalOptions.retryWrites = false; - } - - // Have we specified collation - try { - decorateWithCollation(finalOptions, coll, options); - } catch (err) { - return callback(err, null); - } - - if (options.explain !== undefined && maxWireVersion(server) < 3) { - return callback - ? callback(new MongoError(`server ${server.name} does not support explain on update`)) - : undefined; - } - - // Update options - server.update( - coll.s.namespace.toString(), - [op], - finalOptions as WriteCommandOptions, - (err, result) => { - if (callback == null) return; - if (err) return callback(err); - if (result == null) return callback(); - if (result.code) return callback(new MongoError(result)); - if (result.writeErrors) return callback(new MongoError(result.writeErrors[0])); - // Return the results - callback(undefined, result); - } - ); -} diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 7b4871c7f4..f1e71077d2 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -1,24 +1,26 @@ -import { defineAspects, Aspect, AbstractOperation, Hint } from './operation'; -import { CommandOperation, CommandOperationOptions } from './command'; -import { isObject } from 'util'; -import { - applyRetryableWrites, - Callback, - decorateWithCollation, - maxWireVersion, - MongoDBNamespace -} from '../utils'; +import { defineAspects, Aspect } from './operation'; +import { CommandOperation, CommandOperationOptions, CollationOptions } from './command'; +import { Callback, maxWireVersion, MongoDBNamespace } from '../utils'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ClientSession } from '../sessions'; import { MongoError } from '../error'; +import type { WriteConcernOptions } from '../write_concern'; /** @public */ -export interface DeleteOptions extends CommandOperationOptions { +export interface DeleteOptions extends CommandOperationOptions, WriteConcernOptions { + /** If true, when an insert fails, don't execute the remaining writes. If false, continue with remaining inserts when one fails. */ + ordered?: boolean; + /** A user-provided comment to attach to this command */ + comment?: string | Document; + /** Specifies the collation to use for the operation */ + collation?: CollationOptions; + /** Specify that the update query should only consider plans using the hinted index */ + hint?: string | Document; + + /** @deprecated use `removeOne` or `removeMany` to implicitly specify the limit */ single?: boolean; - hint?: Hint; } /** @public */ @@ -30,53 +32,64 @@ export interface DeleteResult { } /** @internal */ -export class DeleteOperation extends AbstractOperation { +export class DeleteOperation extends CommandOperation { options: DeleteOptions; operations: Document[]; constructor(ns: MongoDBNamespace, ops: Document[], options: DeleteOptions) { - super(options); + super(undefined, options); this.options = options; this.ns = ns; this.operations = ops; } get canRetryWrite(): boolean { + if (super.canRetryWrite === false) { + return false; + } + return this.operations.every(op => (typeof op.limit !== 'undefined' ? op.limit > 0 : true)); } execute(server: Server, session: ClientSession, callback: Callback): void { - server.remove( - this.ns.toString(), - this.operations, - { ...this.options, readPreference: this.readPreference, session } as WriteCommandOptions, - callback - ); + const options = this.options ?? {}; + const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; + const command: Document = { + delete: this.ns.collection, + deletes: this.operations, + ordered + }; + + if (options.explain !== undefined && maxWireVersion(server) < 3) { + return callback + ? callback(new MongoError(`server ${server.name} does not support explain on delete`)) + : undefined; + } + + const unacknowledgedWrite = this.writeConcern && this.writeConcern.w === 0; + if (unacknowledgedWrite || maxWireVersion(server) < 5) { + if (this.operations.find((o: Document) => o.hint)) { + callback(new MongoError(`servers < 3.4 do not support hint on delete`)); + return; + } + } + + super.executeCommand(server, session, command, callback); } } -export class DeleteOneOperation extends CommandOperation { - options: DeleteOptions; - collection: Collection; - filter: Document; - +export class DeleteOneOperation extends DeleteOperation { constructor(collection: Collection, filter: Document, options: DeleteOptions) { - super(collection, options); - - this.options = options; - this.collection = collection; - this.filter = filter; + super(collection.s.namespace, [makeDeleteOperation(filter, { ...options, limit: 1 })], options); } execute(server: Server, session: ClientSession, callback: Callback): void { - const coll = this.collection; - const filter = this.filter; - const options = { ...this.options, ...this.bsonOptions, session }; - - options.single = true; - removeDocuments(server, coll, filter, options, (err, res) => { + super.execute(server, session, (err, res) => { if (err || res == null) return callback(err); - if (typeof options.explain !== 'undefined') return callback(undefined, res); + if (res.code) return callback(new MongoError(res)); + if (res.writeErrors) return callback(new MongoError(res.writeErrors[0])); + if (this.explain) return callback(undefined, res); + callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, deletedCount: res.n @@ -85,36 +98,18 @@ export class DeleteOneOperation extends CommandOperation { } } -export class DeleteManyOperation extends CommandOperation { - options: DeleteOptions; - collection: Collection; - filter: Document; - +export class DeleteManyOperation extends DeleteOperation { constructor(collection: Collection, filter: Document, options: DeleteOptions) { - super(collection, options); - - if (!isObject(filter)) { - throw new TypeError('filter is a required parameter'); - } - - this.options = options; - this.collection = collection; - this.filter = filter; + super(collection.s.namespace, [makeDeleteOperation(filter, options)], options); } execute(server: Server, session: ClientSession, callback: Callback): void { - const coll = this.collection; - const filter = this.filter; - const options = { ...this.options, ...this.bsonOptions, session }; - - // a user can pass `single: true` in to `deleteMany` to remove a single document, theoretically - if (typeof options.single !== 'boolean') { - options.single = false; - } - - removeDocuments(server, coll, filter, options, (err, res) => { + super.execute(server, session, (err, res) => { if (err || res == null) return callback(err); - if (typeof options.explain !== 'undefined') return callback(undefined, res); + if (res.code) return callback(new MongoError(res)); + if (res.writeErrors) return callback(new MongoError(res.writeErrors[0])); + if (this.explain) return callback(undefined, res); + callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, deletedCount: res.n @@ -123,73 +118,43 @@ export class DeleteManyOperation extends CommandOperation { } } -function removeDocuments( - server: Server, - coll: Collection, - selector: Document, - options: DeleteOptions | Document, - callback: Callback -): void { - if (typeof options === 'function') { - (callback = options as Callback), (options = {}); - } else if (typeof selector === 'function') { - callback = selector as Callback; - options = {}; - selector = {}; - } - - // Create an empty options object if the provided one is null - options = options || {}; - - // Final options for retryable writes - let finalOptions = Object.assign({}, options); - finalOptions = applyRetryableWrites(finalOptions, coll.s.db); - - // If selector is null set empty - if (selector == null) selector = {}; +function makeDeleteOperation( + filter: Document, + options: DeleteOptions & { limit?: number } +): Document { + const op: Document = { + q: filter, + limit: typeof options.limit === 'number' ? options.limit : 0 + }; - // Build the op - const op = { q: selector, limit: 0 } as any; - if (options.single) { + if (options.single === true) { op.limit = 1; - } else if (finalOptions.retryWrites) { - finalOptions.retryWrites = false; - } - if (options.hint) { - op.hint = options.hint; } - // Have we specified collation - try { - decorateWithCollation(finalOptions, coll, options); - } catch (err) { - return callback ? callback(err, null) : undefined; + if (options.collation) { + op.collation = options.collation; } - if (options.explain !== undefined && maxWireVersion(server) < 3) { - return callback - ? callback(new MongoError(`server ${server.name} does not support explain on remove`)) - : undefined; + if (options.hint) { + op.hint = options.hint; } - // Execute the remove - server.remove( - coll.s.namespace.toString(), - [op], - finalOptions as WriteCommandOptions, - (err, result) => { - if (err || result == null) return callback(err); - if (result.code) return callback(new MongoError(result)); - if (result.writeErrors) { - return callback(new MongoError(result.writeErrors[0])); - } + if (options.comment) { + op.comment = options.comment; + } - // Return the results - callback(undefined, result); - } - ); + return op; } defineAspects(DeleteOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION]); -defineAspects(DeleteOneOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION, Aspect.EXPLAINABLE]); -defineAspects(DeleteManyOperation, [Aspect.WRITE_OPERATION, Aspect.EXPLAINABLE]); +defineAspects(DeleteOneOperation, [ + Aspect.RETRYABLE, + Aspect.WRITE_OPERATION, + Aspect.EXPLAINABLE, + Aspect.SKIP_COLLATION +]); +defineAspects(DeleteManyOperation, [ + Aspect.WRITE_OPERATION, + Aspect.EXPLAINABLE, + Aspect.SKIP_COLLATION +]); diff --git a/src/operations/find.ts b/src/operations/find.ts index 7cf9f60f20..8d09d066dc 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -10,8 +10,7 @@ import { MongoError } from '../error'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; -import { CommandOperation, CommandOperationOptions } from './command'; +import { CommandOperation, CommandOperationOptions, CollationOptions } from './command'; import { Sort, formatSort } from '../sort'; import { isSharded } from '../cmap/wire_protocol/shared'; import { ReadConcern } from '../read_concern'; diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 51d4a783c7..76fb960c28 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -8,13 +8,17 @@ import { Callback, getTopology } from '../utils'; -import { CommandOperation, CommandOperationOptions, OperationParent } from './command'; +import { + CommandOperation, + CommandOperationOptions, + OperationParent, + CollationOptions +} from './command'; import { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; import type { Document } from '../bson'; import type { Collection } from '../collection'; import type { Db } from '../db'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import { AbstractCursor } from '../cursor/abstract_cursor'; import type { ClientSession } from '../sessions'; import { executeOperation, ExecutionResult } from './execute_operation'; diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 5b2e970783..43418ab434 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -5,32 +5,42 @@ import { prepareDocs } from './common_functions'; import type { Callback, MongoDBNamespace } from '../utils'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ObjectId, Document, BSONSerializeOptions } from '../bson'; import type { BulkWriteOptions } from '../bulk/common'; -import type { WriteConcernOptions } from '../write_concern'; +import { WriteConcern, WriteConcernOptions } from '../write_concern'; import type { ClientSession } from '../sessions'; -import { ReadPreference } from '../read_preference'; +import { BulkWriteOperation } from './bulk_write'; /** @internal */ -export class InsertOperation extends AbstractOperation { +export class InsertOperation extends CommandOperation { options: BulkWriteOptions; - operations: Document[]; + documents: Document[]; - constructor(ns: MongoDBNamespace, ops: Document[], options: BulkWriteOptions) { - super(options); - this.options = options; + constructor(ns: MongoDBNamespace, documents: Document[], options: BulkWriteOptions) { + super(undefined, options); + this.options = { ...options, checkKeys: true }; this.ns = ns; - this.operations = ops; + this.documents = documents; } execute(server: Server, session: ClientSession, callback: Callback): void { - server.insert( - this.ns.toString(), - this.operations, - { ...this.options, readPreference: this.readPreference, session } as WriteCommandOptions, - callback - ); + const options = this.options ?? {}; + const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; + const command: Document = { + insert: this.ns.collection, + documents: this.documents, + ordered + }; + + if (typeof options.bypassDocumentValidation === 'boolean') { + command.bypassDocumentValidation = options.bypassDocumentValidation; + } + + if (typeof options.comment !== 'undefined') { + command.comment = options.comment; + } + + super.executeCommand(server, session, command, callback); } } @@ -50,47 +60,74 @@ export interface InsertOneResult { insertedId: ObjectId; } -export class InsertOneOperation extends CommandOperation { - options: InsertOneOptions; +export class InsertOneOperation extends InsertOperation { + constructor(collection: Collection, doc: Document, options: InsertOneOptions) { + super(collection.s.namespace, prepareDocs(collection, [doc], options), options); + } + + execute(server: Server, session: ClientSession, callback: Callback): void { + super.execute(server, session, (err, res) => { + if (err || res == null) return callback(err); + if (res.code) return callback(new MongoError(res)); + if (res.writeErrors) return callback(new MongoError(res.writeErrors[0])); + + callback(undefined, { + acknowledged: this.writeConcern?.w !== 0 ?? true, + insertedId: this.documents[0]._id + }); + }); + } +} + +/** @public */ +export interface InsertManyResult { + /** Indicates whether this write result was acknowledged. If not, then all other members of this result will be undefined */ + acknowledged: boolean; + /** The number of inserted documents for this operations */ + insertedCount: number; + /** Map of the index of the inserted document to the id of the inserted document */ + insertedIds: { [key: number]: ObjectId }; +} + +/** @internal */ +export class InsertManyOperation extends AbstractOperation { + options: BulkWriteOptions; collection: Collection; - doc: Document; + docs: Document[]; - constructor(collection: Collection, doc: Document, options: InsertOneOptions) { - super(collection, options); + constructor(collection: Collection, docs: Document[], options: BulkWriteOptions) { + super(options); + + if (!Array.isArray(docs)) { + throw new TypeError('docs parameter must be an array of documents'); + } this.options = options; this.collection = collection; - this.doc = doc; + this.docs = docs; } - execute(server: Server, session: ClientSession, callback: Callback): void { + execute(server: Server, session: ClientSession, callback: Callback): void { const coll = this.collection; - const doc = this.doc; - const options = { - ...this.options, - ...this.bsonOptions, - readPreference: ReadPreference.primary, - session - }; - - // File inserts - server.insert( - coll.s.namespace.toString(), - prepareDocs(coll, [this.doc], options), - options as WriteCommandOptions, - (err, result) => { - if (err || result == null) return callback(err); - if (result.code) return callback(new MongoError(result)); - if (result.writeErrors) return callback(new MongoError(result.writeErrors[0])); - - callback(undefined, { - acknowledged: this.writeConcern?.w !== 0 ?? true, - insertedId: doc._id - }); - } + const options = { ...this.options, ...this.bsonOptions, readPreference: this.readPreference }; + const writeConcern = WriteConcern.fromOptions(options); + const bulkWriteOperation = new BulkWriteOperation( + coll, + [{ insertMany: prepareDocs(coll, this.docs, options) }], + options ); + + bulkWriteOperation.execute(server, session, (err, res) => { + if (err || res == null) return callback(err); + callback(undefined, { + acknowledged: writeConcern?.w !== 0 ?? true, + insertedCount: res.insertedCount, + insertedIds: res.insertedIds + }); + }); } } defineAspects(InsertOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION]); defineAspects(InsertOneOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION]); +defineAspects(InsertManyOperation, [Aspect.WRITE_OPERATION]); diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts deleted file mode 100644 index a9bf728f23..0000000000 --- a/src/operations/insert_many.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { Aspect, defineAspects, AbstractOperation } from './operation'; -import { BulkWriteOperation } from './bulk_write'; -import { prepareDocs } from './common_functions'; -import type { Callback } from '../utils'; -import type { Collection } from '../collection'; -import type { ObjectId, Document } from '../bson'; -import type { BulkWriteOptions } from '../bulk/common'; -import type { Server } from '../sdam/server'; -import { WriteConcern } from '../write_concern'; -import type { ClientSession } from '../sessions'; - -/** @public */ -export interface InsertManyResult { - /** Indicates whether this write result was acknowledged. If not, then all other members of this result will be undefined */ - acknowledged: boolean; - /** The number of inserted documents for this operations */ - insertedCount: number; - /** Map of the index of the inserted document to the id of the inserted document */ - insertedIds: { [key: number]: ObjectId }; -} - -/** @internal */ -export class InsertManyOperation extends AbstractOperation { - options: BulkWriteOptions; - collection: Collection; - docs: Document[]; - - constructor(collection: Collection, docs: Document[], options: BulkWriteOptions) { - super(options); - - if (!Array.isArray(docs)) { - throw new TypeError('docs parameter must be an array of documents'); - } - - this.options = options; - this.collection = collection; - this.docs = docs; - } - - execute(server: Server, session: ClientSession, callback: Callback): void { - const coll = this.collection; - const options = { ...this.options, ...this.bsonOptions, readPreference: this.readPreference }; - const writeConcern = WriteConcern.fromOptions(options); - const bulkWriteOperation = new BulkWriteOperation( - coll, - [{ insertMany: prepareDocs(coll, this.docs, options) }], - options - ); - - bulkWriteOperation.execute(server, session, (err, res) => { - if (err || res == null) return callback(err); - callback(undefined, { - acknowledged: writeConcern?.w !== 0 ?? true, - insertedCount: res.insertedCount, - insertedIds: res.insertedIds - }); - }); - } -} - -defineAspects(InsertManyOperation, [Aspect.WRITE_OPERATION]); diff --git a/src/operations/operation.ts b/src/operations/operation.ts index db6c3b7047..7c062cb864 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -8,7 +8,8 @@ export const Aspect = { READ_OPERATION: Symbol('READ_OPERATION'), WRITE_OPERATION: Symbol('WRITE_OPERATION'), RETRYABLE: Symbol('RETRYABLE'), - EXPLAINABLE: Symbol('EXPLAINABLE') + EXPLAINABLE: Symbol('EXPLAINABLE'), + SKIP_COLLATION: Symbol('SKIP_COLLATION') } as const; /** @public */ diff --git a/src/operations/replace_one.ts b/src/operations/replace_one.ts deleted file mode 100644 index 9f97138554..0000000000 --- a/src/operations/replace_one.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { defineAspects, Aspect } from './operation'; -import { updateDocuments } from './common_functions'; -import { hasAtomicOperators, Callback } from '../utils'; -import { CommandOperation, CommandOperationOptions } from './command'; -import type { Document } from '../bson'; -import type { Server } from '../sdam/server'; -import type { Collection } from '../collection'; -import type { CollationOptions } from '../cmap/wire_protocol/write_command'; -import type { UpdateResult } from './update'; -import type { ClientSession } from '../sessions'; - -/** @public */ -export interface ReplaceOptions extends CommandOperationOptions { - /** If true, allows the write to opt-out of document level validation */ - bypassDocumentValidation?: boolean; - /** Specifies a collation */ - collation?: CollationOptions; - /** Specify that the update query should only consider plans using the hinted index */ - hint?: string | Document; - /** When true, creates a new document if no document matches the query */ - upsert?: boolean; - - // FIXME: - multi?: boolean; -} - -/** @internal */ -export class ReplaceOneOperation extends CommandOperation { - options: ReplaceOptions; - collection: Collection; - filter: Document; - replacement: Document; - - constructor( - collection: Collection, - filter: Document, - replacement: Document, - options: ReplaceOptions - ) { - super(collection, options); - - if (hasAtomicOperators(replacement)) { - throw new TypeError('Replacement document must not contain atomic operators'); - } - - this.options = options; - this.collection = collection; - this.filter = filter; - this.replacement = replacement; - } - - execute(server: Server, session: ClientSession, callback: Callback): void { - const coll = this.collection; - const filter = this.filter; - const replacement = this.replacement; - const options = { ...this.options, ...this.bsonOptions, session, multi: false }; - - updateDocuments(server, coll, filter, replacement, options, (err, r) => { - if (err || !r) return callback(err); - if (typeof this.explain !== 'undefined') return callback(undefined, r); - callback(undefined, { - acknowledged: this.writeConcern?.w !== 0 ?? true, - modifiedCount: r.nModified != null ? r.nModified : r.n, - upsertedId: Array.isArray(r.upserted) && r.upserted.length > 0 ? r.upserted[0]._id : null, - upsertedCount: Array.isArray(r.upserted) && r.upserted.length ? r.upserted.length : 0, - matchedCount: Array.isArray(r.upserted) && r.upserted.length > 0 ? 0 : r.n - }); - }); - } -} - -defineAspects(ReplaceOneOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION]); diff --git a/src/operations/update.ts b/src/operations/update.ts index c102e85923..3ae2902bbc 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -1,12 +1,17 @@ -import { defineAspects, Aspect, AbstractOperation } from './operation'; -import { updateDocuments } from './common_functions'; -import { hasAtomicOperators, MongoDBNamespace, Callback } from '../utils'; -import { CommandOperation, CommandOperationOptions } from './command'; +import { defineAspects, Aspect } from './operation'; +import { + hasAtomicOperators, + MongoDBNamespace, + Callback, + collationNotSupported, + maxWireVersion +} from '../utils'; +import { CommandOperation, CommandOperationOptions, CollationOptions } from './command'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import type { CollationOptions, WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ObjectId, Document } from '../bson'; import type { ClientSession } from '../sessions'; +import { MongoError } from '../error'; /** @public */ export interface UpdateOptions extends CommandOperationOptions { @@ -20,10 +25,6 @@ export interface UpdateOptions extends CommandOperationOptions { hint?: string | Document; /** When true, creates a new document if no document matches the query */ upsert?: boolean; - - // non-standard options - retryWrites?: boolean; - multi?: boolean; } /** @public */ @@ -41,107 +42,244 @@ export interface UpdateResult { } /** @internal */ -export class UpdateOperation extends AbstractOperation { - options: UpdateOptions; +export class UpdateOperation extends CommandOperation { + options: UpdateOptions & { ordered?: boolean }; operations: Document[]; - constructor(ns: MongoDBNamespace, ops: Document[], options: UpdateOptions) { - super(options); + constructor( + ns: MongoDBNamespace, + ops: Document[], + options: UpdateOptions & { ordered?: boolean } + ) { + super(undefined, options); this.options = options; this.ns = ns; + this.operations = ops; } get canRetryWrite(): boolean { + if (super.canRetryWrite === false) { + return false; + } + return this.operations.every(op => op.multi == null || op.multi === false); } execute(server: Server, session: ClientSession, callback: Callback): void { - server.update( - this.ns.toString(), - this.operations, - { ...this.options, readPreference: this.readPreference, session } as WriteCommandOptions, - callback - ); + const options = this.options ?? {}; + const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; + const command: Document = { + update: this.ns.collection, + updates: this.operations, + ordered + }; + + if (typeof options.bypassDocumentValidation === 'boolean') { + command.bypassDocumentValidation = options.bypassDocumentValidation; + } + + if (collationNotSupported(server, options)) { + callback(new MongoError(`server ${server.name} does not support collation`)); + return; + } + + const unacknowledgedWrite = this.writeConcern && this.writeConcern.w === 0; + if (unacknowledgedWrite || maxWireVersion(server) < 5) { + if (this.operations.find((o: Document) => o.hint)) { + callback(new MongoError(`servers < 3.4 do not support hint on update`)); + return; + } + } + + if (this.explain && maxWireVersion(server) < 3) { + callback(new MongoError(`server ${server.name} does not support explain on update`)); + return; + } + + super.executeCommand(server, session, command, callback); } } /** @internal */ -export class UpdateOneOperation extends CommandOperation { - options: UpdateOptions; - collection: Collection; - filter: Document; - update: Document; - +export class UpdateOneOperation extends UpdateOperation { constructor(collection: Collection, filter: Document, update: Document, options: UpdateOptions) { - super(collection, options); + super( + collection.s.namespace, + [makeUpdateOperation(filter, update, { ...options, multi: false })], + options + ); if (!hasAtomicOperators(update)) { throw new TypeError('Update document requires atomic operators'); } - - this.options = options; - this.collection = collection; - this.filter = filter; - this.update = update; } - execute(server: Server, session: ClientSession, callback: Callback): void { - const coll = this.collection; - const filter = this.filter; - const update = this.update; - const options = { ...this.options, ...this.bsonOptions, session, multi: false }; + execute( + server: Server, + session: ClientSession, + callback: Callback + ): void { + super.execute(server, session, (err, res) => { + if (err || !res) return callback(err); + if (typeof this.explain !== 'undefined') return callback(undefined, res); + if (res.code) return callback(new MongoError(res)); + if (res.writeErrors) return callback(new MongoError(res.writeErrors[0])); - updateDocuments(server, coll, filter, update, options, (err, r) => { - if (err || !r) return callback(err); - if (typeof this.explain !== 'undefined') return callback(undefined, r); callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, - modifiedCount: r.nModified != null ? r.nModified : r.n, - upsertedId: Array.isArray(r.upserted) && r.upserted.length > 0 ? r.upserted[0]._id : null, - upsertedCount: Array.isArray(r.upserted) && r.upserted.length ? r.upserted.length : 0, - matchedCount: Array.isArray(r.upserted) && r.upserted.length > 0 ? 0 : r.n + modifiedCount: res.nModified != null ? res.nModified : res.n, + upsertedId: + Array.isArray(res.upserted) && res.upserted.length > 0 ? res.upserted[0]._id : null, + upsertedCount: Array.isArray(res.upserted) && res.upserted.length ? res.upserted.length : 0, + matchedCount: Array.isArray(res.upserted) && res.upserted.length > 0 ? 0 : res.n }); }); } } /** @internal */ -export class UpdateManyOperation extends CommandOperation { - options: UpdateOptions; - collection: Collection; - filter: Document; - update: Document; - +export class UpdateManyOperation extends UpdateOperation { constructor(collection: Collection, filter: Document, update: Document, options: UpdateOptions) { - super(collection, options); + super( + collection.s.namespace, + [makeUpdateOperation(filter, update, { ...options, multi: true })], + options + ); - this.options = options; - this.collection = collection; - this.filter = filter; - this.update = update; + if (!hasAtomicOperators(update)) { + throw new TypeError('Update document requires atomic operators'); + } } - execute(server: Server, session: ClientSession, callback: Callback): void { - const coll = this.collection; - const filter = this.filter; - const update = this.update; - const options = { ...this.options, ...this.bsonOptions, session, multi: true }; + execute( + server: Server, + session: ClientSession, + callback: Callback + ): void { + super.execute(server, session, (err, res) => { + if (err || !res) return callback(err); + if (typeof this.explain !== 'undefined') return callback(undefined, res); + if (res.code) return callback(new MongoError(res)); + if (res.writeErrors) return callback(new MongoError(res.writeErrors[0])); - updateDocuments(server, coll, filter, update, options, (err, r) => { - if (err || !r) return callback(err); - if (typeof this.explain !== 'undefined') return callback(undefined, r); callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, - modifiedCount: r.nModified != null ? r.nModified : r.n, - upsertedId: Array.isArray(r.upserted) && r.upserted.length > 0 ? r.upserted[0]._id : null, - upsertedCount: Array.isArray(r.upserted) && r.upserted.length ? r.upserted.length : 0, - matchedCount: Array.isArray(r.upserted) && r.upserted.length > 0 ? 0 : r.n + modifiedCount: res.nModified != null ? res.nModified : res.n, + upsertedId: + Array.isArray(res.upserted) && res.upserted.length > 0 ? res.upserted[0]._id : null, + upsertedCount: Array.isArray(res.upserted) && res.upserted.length ? res.upserted.length : 0, + matchedCount: Array.isArray(res.upserted) && res.upserted.length > 0 ? 0 : res.n }); }); } } -defineAspects(UpdateOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION]); -defineAspects(UpdateOneOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION, Aspect.EXPLAINABLE]); -defineAspects(UpdateManyOperation, [Aspect.WRITE_OPERATION, Aspect.EXPLAINABLE]); +/** @public */ +export interface ReplaceOptions extends CommandOperationOptions { + /** If true, allows the write to opt-out of document level validation */ + bypassDocumentValidation?: boolean; + /** Specifies a collation */ + collation?: CollationOptions; + /** Specify that the update query should only consider plans using the hinted index */ + hint?: string | Document; + /** When true, creates a new document if no document matches the query */ + upsert?: boolean; +} + +/** @internal */ +export class ReplaceOneOperation extends UpdateOperation { + constructor( + collection: Collection, + filter: Document, + replacement: Document, + options: ReplaceOptions + ) { + super( + collection.s.namespace, + [makeUpdateOperation(filter, replacement, { ...options, multi: false })], + options + ); + + if (hasAtomicOperators(replacement)) { + throw new TypeError('Replacement document must not contain atomic operators'); + } + } + + execute( + server: Server, + session: ClientSession, + callback: Callback + ): void { + super.execute(server, session, (err, res) => { + if (err || !res) return callback(err); + if (typeof this.explain !== 'undefined') return callback(undefined, res); + if (res.code) return callback(new MongoError(res)); + if (res.writeErrors) return callback(new MongoError(res.writeErrors[0])); + + callback(undefined, { + acknowledged: this.writeConcern?.w !== 0 ?? true, + modifiedCount: res.nModified != null ? res.nModified : res.n, + upsertedId: + Array.isArray(res.upserted) && res.upserted.length > 0 ? res.upserted[0]._id : null, + upsertedCount: Array.isArray(res.upserted) && res.upserted.length ? res.upserted.length : 0, + matchedCount: Array.isArray(res.upserted) && res.upserted.length > 0 ? 0 : res.n + }); + }); + } +} + +function makeUpdateOperation( + filter: Document, + update: Document, + options: UpdateOptions & { multi?: boolean } +): Document { + if (filter == null || typeof filter !== 'object') { + throw new TypeError('selector must be a valid JavaScript object'); + } + + if (update == null || typeof update !== 'object') { + throw new TypeError('document must be a valid JavaScript object'); + } + + const op: Document = { q: filter, u: update }; + if (typeof options.upsert === 'boolean') { + op.upsert = options.upsert; + } + + if (typeof options.multi === 'boolean') { + op.multi = options.multi; + } + + if (options.hint) { + op.hint = options.hint; + } + + if (options.arrayFilters) { + op.arrayFilters = options.arrayFilters; + } + + if (options.collation) { + op.collation = options.collation; + } + + return op; +} + +defineAspects(UpdateOperation, [Aspect.RETRYABLE, Aspect.WRITE_OPERATION, Aspect.SKIP_COLLATION]); +defineAspects(UpdateOneOperation, [ + Aspect.RETRYABLE, + Aspect.WRITE_OPERATION, + Aspect.EXPLAINABLE, + Aspect.SKIP_COLLATION +]); +defineAspects(UpdateManyOperation, [ + Aspect.WRITE_OPERATION, + Aspect.EXPLAINABLE, + Aspect.SKIP_COLLATION +]); +defineAspects(ReplaceOneOperation, [ + Aspect.RETRYABLE, + Aspect.WRITE_OPERATION, + Aspect.SKIP_COLLATION +]); diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 2d2731132b..1ba78abf5f 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -40,7 +40,6 @@ import type { ServerHeartbeatSucceededEvent } from './events'; import type { ClientSession } from '../sessions'; import type { CommandOptions } from '../cmap/wire_protocol/command'; import type { GetMoreOptions } from '../cmap/wire_protocol/get_more'; -import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Document, Long } from '../bson'; import type { AutoEncrypter } from '../deps'; import type { QueryOptions } from '../cmap/wire_protocol/query'; @@ -371,41 +370,6 @@ export class Server extends EventEmitter { ); }, callback); } - - /** - * Insert one or more documents - * @internal - * - * @param ns - The MongoDB fully qualified namespace (ex: db1.collection1) - * @param ops - An array of documents to insert - */ - insert(ns: string, ops: Document[], options: WriteCommandOptions, callback: Callback): void { - executeWriteOperation({ server: this, op: 'insert', ns, ops }, options, callback); - } - - /** - * Perform one or more update operations - * @internal - * - * @param ns - The MongoDB fully qualified namespace (ex: db1.collection1) - * @param ops - An array of updates - */ - update(ns: string, ops: Document[], options: WriteCommandOptions, callback: Callback): void { - executeWriteOperation({ server: this, op: 'update', ns, ops }, options, callback); - } - - /** - * Perform one or more remove operations - * @internal - * - * @param ns - The MongoDB fully qualified namespace (ex: db1.collection1) - * @param ops - An array of removes - * @param options - options for removal - * @param callback - A callback function - */ - remove(ns: string, ops: Document[], options: WriteCommandOptions, callback: Callback): void { - executeWriteOperation({ server: this, op: 'remove', ns, ops }, options, callback); - } } Object.defineProperty(Server.prototype, 'clusterTime', { @@ -434,64 +398,6 @@ function calculateRoundTripTime(oldRtt: number, duration: number): number { return alpha * duration + (1 - alpha) * oldRtt; } -function executeWriteOperation( - args: { server: Server; op: string; ns: string; ops: Document[] | Document }, - options: WriteCommandOptions, - callback: Callback -) { - options = options ?? {}; - - const { server, op, ns } = args; - const ops = Array.isArray(args.ops) ? args.ops : [args.ops]; - if (server.s.state === STATE_CLOSING || server.s.state === STATE_CLOSED) { - callback(new MongoError('server is closed')); - return; - } - - if (collationNotSupported(server, options)) { - callback(new MongoError(`server ${server.name} does not support collation`)); - return; - } - - const unacknowledgedWrite = options.writeConcern && options.writeConcern.w === 0; - if (unacknowledgedWrite || maxWireVersion(server) < 5) { - if ((op === 'update' || op === 'remove') && ops.find((o: Document) => o.hint)) { - callback(new MongoError(`servers < 3.4 do not support hint on ${op}`)); - return; - } - } - - server.s.pool.withConnection((err, conn, cb) => { - if (err || !conn) { - markServerUnknown(server, err); - return cb(err); - } - - if (op === 'insert') { - conn.insert( - ns, - ops, - options, - makeOperationHandler(server, conn, ops, options, cb) as Callback - ); - } else if (op === 'update') { - conn.update( - ns, - ops, - options, - makeOperationHandler(server, conn, ops, options, cb) as Callback - ); - } else { - conn.remove( - ns, - ops, - options, - makeOperationHandler(server, conn, ops, options, cb) as Callback - ); - } - }, callback); -} - function markServerUnknown(server: Server, error?: MongoError) { if (error instanceof MongoNetworkError && !(error instanceof MongoNetworkTimeoutError)) { server[kMonitor].reset(); @@ -531,7 +437,7 @@ function makeOperationHandler( server: Server, connection: Connection, cmd: Document, - options: CommandOptions | WriteCommandOptions | GetMoreOptions | undefined, + options: CommandOptions | GetMoreOptions | undefined, callback: Callback ): CallbackWithType { const session = options?.session; diff --git a/src/sessions.ts b/src/sessions.ts index 0b46cad74b..aa86f2cdd0 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -17,10 +17,11 @@ import { } from './utils'; import type { Topology } from './sdam/topology'; import type { MongoClientOptions } from './mongo_client'; -import type { WriteCommandOptions } from './cmap/wire_protocol/write_command'; import { executeOperation } from './operations/execute_operation'; import { RunAdminCommandOperation } from './operations/run_command'; import type { AbstractCursor } from './cursor/abstract_cursor'; +import type { CommandOptions } from './cmap/wire_protocol/command'; +import type { WriteConcern } from './write_concern'; const minWireVersionForShardedTransactions = 8; @@ -744,7 +745,7 @@ function commandSupportsReadConcern(command: Document, options?: Document): bool function applySession( session: ClientSession, command: Document, - options?: WriteCommandOptions + options?: CommandOptions ): MongoError | undefined { // TODO: merge this with `assertAlive`, did not want to throw a try/catch here if (session.hasEnded) { @@ -758,7 +759,7 @@ function applySession( // SPEC-1019: silently ignore explicit session with unacknowledged write for backwards compatibility // FIXME: NODE-2781, this check for write concern shouldn't be happening here, but instead during command construction - if (options && options.writeConcern && options.writeConcern.w === 0) { + if (options && options.writeConcern && (options.writeConcern as WriteConcern).w === 0) { if (session && session.explicit) { return new MongoError('Cannot have explicit session with unacknowledged writes'); } diff --git a/test/functional/aggregation.test.js b/test/functional/aggregation.test.js index 939cbd93bc..4779e401ee 100644 --- a/test/functional/aggregation.test.js +++ b/test/functional/aggregation.test.js @@ -54,6 +54,8 @@ describe('Aggregation', function () { var collection = db.collection('shouldCorrectlyExecuteSimpleAggregationPipelineUsingArray'); // Insert the docs collection.insertMany(docs, { w: 1 }, function (err, result) { + if (err) console.dir(err); + expect(result).to.exist; expect(err).to.not.exist; diff --git a/test/functional/apm.test.js b/test/functional/apm.test.js index b1f748bd79..a80e40826c 100644 --- a/test/functional/apm.test.js +++ b/test/functional/apm.test.js @@ -952,12 +952,23 @@ describe('APM', function () { ); // Unpack the operation + if (args.options) options = args.options; if (args.filter) params.push(args.filter); if (args.deletes) params.push(args.deletes); if (args.document) params.push(args.document); if (args.documents) params.push(args.documents); if (args.update) params.push(args.update); - if (args.requests) params.push(args.requests); + if (args.requests) { + if (operation.name !== 'bulkWrite') { + params.push(args.requests); + } else { + params.push( + args.requests.map(r => { + return { [r.name]: r.arguments.document || r.arguments }; + }) + ); + } + } if (args.writeConcern) { if (options == null) { @@ -1013,12 +1024,15 @@ describe('APM', function () { ) ); } - // Add options if they exists if (options) params.push(options); // Execute the operation - const promise = collection[commandName].apply(collection, params); + const coll = operation.collectionOptions + ? db.collection(scenario.collection_name, operation.collectionOptions) + : db.collection(scenario.collection_name); + + const promise = coll[commandName].apply(coll, params); return promise .catch(() => {} /* ignore */) .then(() => diff --git a/test/functional/collection.test.js b/test/functional/collection.test.js index 46e8f88f79..7c4f95dfe1 100644 --- a/test/functional/collection.test.js +++ b/test/functional/collection.test.js @@ -381,12 +381,10 @@ describe('Collection', function () { it('should throw error due to illegal update', function (done) { db.createCollection('shouldThrowErrorDueToIllegalUpdate', {}, (err, coll) => { - coll.update({}, null, err => { - expect(err.message).to.equal('document must be a valid JavaScript object'); - }); - coll.update(null, null, err => { - expect(err.message).to.equal('selector must be a valid JavaScript object'); - }); + expect(() => coll.update({}, null)).to.throw(/document must be a valid JavaScript object/); + expect(() => coll.update(null, null)).to.throw( + /selector must be a valid JavaScript object/ + ); done(); }); diff --git a/test/functional/core/tailable_cursor.test.js b/test/functional/core/tailable_cursor.test.js index 9450210953..b8243cf4a4 100644 --- a/test/functional/core/tailable_cursor.test.js +++ b/test/functional/core/tailable_cursor.test.js @@ -1,10 +1,6 @@ 'use strict'; - -const { MongoDBNamespace } = require('../../../src/utils'); -const { FindCursor } = require('../../../src/cursor/find_cursor'); - -const expect = require('chai').expect; -const setupDatabase = require('../shared').setupDatabase; +const { expect } = require('chai'); +const { setupDatabase, withClientV2 } = require('../shared'); describe('Tailable cursor tests', function () { before(function () { @@ -12,72 +8,35 @@ describe('Tailable cursor tests', function () { }); it('should correctly perform awaitData', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded'], mongodb: '>=3.2' } - }, - - test: function (done) { - const self = this; - const topology = this.configuration.newTopology(); - const ns = `${this.configuration.db}.cursor_tailable`; - - topology.connect(err => { - expect(err).to.not.exist; - this.defer(() => topology.close()); - - topology.selectServer('primary', (err, server) => { + metadata: { requires: { mongodb: '>=3.2' } }, + test: withClientV2((client, done) => { + const db = client.db(); + db.collection('cursor_tailable').drop(() => { + db.createCollection('cursor_tailable', { capped: true, size: 10000 }, (err, coll) => { expect(err).to.not.exist; - // Create a capped collection - server.command( - `${self.configuration.db}.$cmd`, - { create: 'cursor_tailable', capped: true, size: 10000 }, - (cmdErr, cmdRes) => { - expect(cmdErr).to.not.exist; - expect(cmdRes).to.exist; - - // Execute the write - server.insert( - ns, - [{ a: 1 }], - { - writeConcern: { w: 1 }, - ordered: true - }, - (insertErr, results) => { - expect(insertErr).to.not.exist; - expect(results.n).to.equal(1); - - // Execute find - const cursor = new FindCursor( - topology, - MongoDBNamespace.fromString(ns), - {}, - { batchSize: 2, tailable: true, awaitData: true } - ); + coll.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).property('insertedId').to.exist; - // Execute next - cursor.next((cursorErr, cursorD) => { - expect(cursorErr).to.not.exist; - expect(cursorD).to.exist; + const cursor = coll.find({}, { batchSize: 2, tailable: true, awaitData: true }); + cursor.next((err, doc) => { + expect(err).to.not.exist; + expect(doc).to.exist; - const s = new Date(); - cursor.next(() => { - const e = new Date(); - expect(e.getTime() - s.getTime()).to.be.at.least(300); + const s = new Date(); + cursor.next(() => { + const e = new Date(); + expect(e.getTime() - s.getTime()).to.be.at.least(300); - // Destroy the server connection - server.destroy(done); - }); + done(); + }); - setTimeout(() => cursor.close(), 300); - }); - } - ); - } - ); + setTimeout(() => cursor.close(), 300); + }); + }); }); }); - } + }) }); }); diff --git a/test/functional/core/undefined.test.js b/test/functional/core/undefined.test.js index 83826cc54a..6ba21f2ba4 100644 --- a/test/functional/core/undefined.test.js +++ b/test/functional/core/undefined.test.js @@ -1,256 +1,114 @@ 'use strict'; const { expect } = require('chai'); -const { format: f } = require('util'); const { ObjectId } = require('bson'); -const { FindCursor } = require('../../../src/cursor/find_cursor'); -const { MongoDBNamespace } = require('../../../src/utils'); +const { withClientV2 } = require('../shared'); describe('A server', function () { it('should correctly execute insert culling undefined', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded'], mongodb: '>=3.2' } - }, - - test: function (done) { - const self = this; - const topology = this.configuration.newTopology(); - - topology.connect(err => { - expect(err).to.not.exist; - this.defer(() => topology.close()); - - topology.selectServer('primary', (err, server) => { - expect(err).to.not.exist; - - // Drop collection - server.command(f('%s.$cmd', self.configuration.db), { drop: 'insert1' }, () => { - const ns = f('%s.insert1', self.configuration.db); - const objectId = new ObjectId(); - // Execute the write - server.insert( - ns, - [{ _id: objectId, a: 1, b: undefined }], - { - writeConcern: { w: 1 }, - ordered: true, - ignoreUndefined: true - }, - (insertErr, results) => { - expect(insertErr).to.not.exist; - expect(results.n).to.eql(1); - - // Execute find - const cursor = new FindCursor( - topology, - MongoDBNamespace.fromString(ns), - { _id: objectId }, - { batchSize: 2 } - ); - - // Execute next - cursor.next((nextErr, d) => { - expect(nextErr).to.not.exist; - expect(d.b).to.be.undefined; - - // Destroy the connection - server.destroy(done); - }); - } - ); - }); - }); + metadata: { requires: { mongodb: '>=3.2' } }, + test: withClientV2(function (client, done) { + const coll = client.db().collection('insert1'); + coll.drop(() => { + const objectId = new ObjectId(); + coll.insertOne( + { _id: objectId, a: 1, b: undefined }, + { ignoreUndefined: true }, + (err, res) => { + expect(err).to.not.exist; + expect(res).property('insertedId').to.exist; + + const cursor = coll.find({ _id: objectId }); + this.defer(() => cursor.close()); + + cursor.next((err, doc) => { + expect(err).to.not.exist; + expect(doc).to.not.have.property('b'); + done(); + }); + } + ); }); - } + }) }); it('should correctly execute update culling undefined', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded'], mongodb: '>=3.2' } - }, - - test: function (done) { - var self = this; - const topology = this.configuration.newTopology(); - - topology.connect(err => { - expect(err).to.not.exist; - this.defer(() => topology.close()); - - topology.selectServer('primary', (err, server) => { - expect(err).to.not.exist; - - // Drop collection - server.command(f('%s.$cmd', self.configuration.db), { drop: 'update1' }, () => { - const ns = f('%s.update1', self.configuration.db); - const objectId = new ObjectId(); - // Execute the write - server.update( - ns, - { - q: { _id: objectId, a: 1, b: undefined }, - u: { $set: { a: 1, b: undefined } }, - upsert: true - }, - { - writeConcern: { w: 1 }, - ordered: true, - ignoreUndefined: true - }, - (insertErr, results) => { - expect(insertErr).to.not.exist; - expect(results.n).to.eql(1); - - // Execute find - const cursor = new FindCursor( - topology, - MongoDBNamespace.fromString(ns), - { _id: objectId }, - { batchSize: 2 } - ); - - // Execute next - cursor.next((nextErr, d) => { - expect(nextErr).to.not.exist; - expect(d.b).to.be.undefined; - - // Destroy the connection - server.destroy(done); - }); - } - ); - }); - }); + metadata: { requires: { mongodb: '>=3.2' } }, + test: withClientV2(function (client, done) { + const coll = client.db().collection('update1'); + coll.drop(() => { + const objectId = new ObjectId(); + coll.updateOne( + { _id: objectId, a: 1, b: undefined }, + { $set: { a: 1, b: undefined } }, + { ignoreUndefined: true, upsert: true }, + (err, res) => { + expect(err).to.not.exist; + expect(res).property('upsertedCount').to.equal(1); + + const cursor = coll.find({ _id: objectId }); + this.defer(() => cursor.close()); + + cursor.next((err, doc) => { + expect(err).to.not.exist; + expect(doc).to.not.have.property('b'); + done(); + }); + } + ); }); - } + }) }); it('should correctly execute remove culling undefined', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded'] } - }, - - test: function (done) { - const self = this; - const topology = this.configuration.newTopology(); - const ns = f('%s.remove1', this.configuration.db); - - topology.connect(err => { - expect(err).to.not.exist; - this.defer(() => topology.close()); - - topology.selectServer('primary', (err, server) => { - expect(err).to.not.exist; - - const objectId = new ObjectId(); - server.command(f('%s.$cmd', self.configuration.db), { drop: 'remove1' }, () => { - // Execute the write - server.insert( - ns, - [ - { id: objectId, a: 1, b: undefined }, - { id: objectId, a: 2, b: 1 } - ], - { - writeConcern: { w: 1 }, - ordered: true - }, - (insertErr, results) => { - expect(insertErr).to.not.exist; - expect(results.n).to.eql(2); - - // Execute the write - server.remove( - ns, - [ - { - q: { b: undefined }, - limit: 0 - } - ], - { - writeConcern: { w: 1 }, - ordered: true, - ignoreUndefined: true - }, - (removeErr, removeResults) => { - expect(removeErr).to.not.exist; - expect(removeResults.n).to.eql(2); - - // Destroy the connection - server.destroy(done); - } - ); - } - ); - }); - }); + metadata: { requires: { mongodb: '>=3.2' } }, + test: withClientV2(function (client, done) { + const coll = client.db().collection('remove1'); + coll.drop(() => { + const objectId = new ObjectId(); + coll.insertMany( + [ + { id: objectId, a: 1, b: undefined }, + { id: objectId, a: 2, b: 1 } + ], + (err, res) => { + expect(err).to.not.exist; + expect(res).property('insertedCount').to.equal(2); + + coll.removeMany({ b: undefined }, { ignoreUndefined: true }, (err, res) => { + expect(err).to.not.exist; + expect(res).property('deletedCount').to.equal(2); + done(); + }); + } + ); }); - } + }) }); it('should correctly execute remove not culling undefined', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded'] } - }, - - test: function (done) { - const self = this; - const topology = this.configuration.newTopology(); - const ns = f('%s.remove2', this.configuration.db); - - topology.connect(err => { - expect(err).to.not.exist; - this.defer(() => topology.close()); - - topology.selectServer('primary', (err, server) => { - expect(err).to.not.exist; - - const objectId = new ObjectId(); - server.command(f('%s.$cmd', self.configuration.db), { drop: 'remove2' }, () => { - // Execute the write - server.insert( - ns, - [ - { id: objectId, a: 1, b: undefined }, - { id: objectId, a: 2, b: 1 } - ], - { - writeConcern: { w: 1 }, - ordered: true - }, - (insertErr, results) => { - expect(insertErr).to.not.exist; - expect(results.n).to.eql(2); - - // Execute the write - server.remove( - ns, - [ - { - q: { b: null }, - limit: 0 - } - ], - { - writeConcern: { w: 1 }, - ordered: true - }, - (removeErr, removeResults) => { - expect(removeErr).to.not.exist; - expect(removeResults.n).to.eql(1); - - // Destroy the connection - server.destroy(); - // Finish the test - done(); - } - ); - } - ); - }); - }); + metadata: { requires: { mongodb: '>=3.2' } }, + test: withClientV2(function (client, done) { + const coll = client.db().collection('remove1'); + coll.drop(() => { + const objectId = new ObjectId(); + coll.insertMany( + [ + { id: objectId, a: 1, b: undefined }, + { id: objectId, a: 2, b: 1 } + ], + (err, res) => { + expect(err).to.not.exist; + expect(res).property('insertedCount').to.equal(2); + + coll.removeMany({ b: null }, (err, res) => { + expect(err).to.not.exist; + expect(res).property('deletedCount').to.equal(1); + done(); + }); + } + ); }); - } + }) }); }); diff --git a/test/functional/document_validation.test.js b/test/functional/document_validation.test.js index 48f564e84f..b9ecf3a211 100644 --- a/test/functional/document_validation.test.js +++ b/test/functional/document_validation.test.js @@ -96,7 +96,7 @@ describe('Document Validation', function () { // Should fail col.update({ b: 1 }, { $set: { b: 1 } }, { upsert: true }, function (err) { - test.ok(err != null); + expect(err).to.exist; // Ensure validation was correctly applied col.update( diff --git a/test/functional/insert.test.js b/test/functional/insert.test.js index d480d73e69..0ad97c97a9 100644 --- a/test/functional/insert.test.js +++ b/test/functional/insert.test.js @@ -1439,7 +1439,7 @@ describe('Insert', function () { client.connect(function (err, client) { var db = client.db(configuration.db); var collection = db.collection('gh-completely3'); - collection.update({ a: 1 }, { a: 2 }, { upsert: true, w: 0 }, cb); + collection.update({ a: 1 }, { $set: { a: 2 } }, { upsert: true, w: 0 }, cb); }); } }); diff --git a/test/functional/uri.test.js b/test/functional/uri.test.js index 407280ff36..d5c7cf1cb1 100644 --- a/test/functional/uri.test.js +++ b/test/functional/uri.test.js @@ -22,15 +22,17 @@ describe('URI', function () { expect(err).to.not.exist; var db = client.db(self.configuration.db); - db.collection('mongoclient_test').update({ a: 1 }, { b: 1 }, { upsert: true }, function ( - err, - result - ) { - expect(err).to.not.exist; - expect(result).to.exist; - expect(result).property('acknowledged').to.be.false; - client.close(done); - }); + db.collection('mongoclient_test').update( + { a: 1 }, + { $set: { b: 1 } }, + { upsert: true }, + function (err, result) { + expect(err).to.not.exist; + expect(result).to.exist; + expect(result).property('acknowledged').to.be.false; + client.close(done); + } + ); }); } }); diff --git a/test/spec/apm/README.rst b/test/spec/apm/README.rst index 20a4cd6164..9f0562364a 100644 --- a/test/spec/apm/README.rst +++ b/test/spec/apm/README.rst @@ -14,6 +14,12 @@ Testing Tests are provided in YML and JSON format to assert proper upconversion of commands. +Database and Collection Names +----------------------------- + +The collection under test is specified in each test file with the fields +``database_name`` and ``collection_name``. + Data ---- @@ -98,19 +104,25 @@ the YAML, then the command does *not* pass the test:: readConcern: level: majority -Ignoring Tests Based On Server Version -`````````````````````````````````````` +Ignoring Tests Based On Server Version or Topology Type +``````````````````````````````````````````````````````` + +Due to variations in server behavior, some tests may not be valid and MUST NOT be run on +certain server versions or topology types. These tests are indicated with any of the +following fields, which will be optionally provided at the ``description`` level of each +test: -Due to variations in server behaviour, some tests may not be valid on a specific range -of server versions and MUST NOT be run. These tests are indicated with the fields -``ignore_if_server_version_greater_than`` and ``ignore_if_server_version_less_than`` which -are optionally provided at the description level of the test. When determining if the test -must be run or not, use only the minor server version. +- ``ignore_if_server_version_greater_than`` (optional): If specified, the test MUST be + skipped if the minor version of the server is greater than this minor version. The + server's patch version MUST NOT be considered. For example, a value of ``3.0`` implies + that the test can run on server version ``3.0.15`` but not ``3.1.0``. -Example: +- ``ignore_if_server_version_less_than`` (optional): If specified, the test MUST be + skipped if the minor version of the server is less than this minor version. The + server's patch version MUST NOT be considered. For example, a value of ``3.2`` implies + that the test can run on server version ``3.2.0`` but not ``3.0.15``. -If ``ignore_if_server_version_greater_than`` is ``3.0``, then the tests MUST NOT run on -``3.1`` and higher, but MUST run on ``3.0.3``. +- ``ignore_if_topology_type`` (optional): An array of server topologies for which the test + MUST be skipped. Valid topologies are "single", "replicaset", and "sharded". -Tests which do not have either one of these fields MUST run on all supported server -versions. +Tests that have none of these fields MUST be run on all supported server versions. diff --git a/test/spec/apm/bulkWrite.json b/test/spec/apm/bulkWrite.json index d8281a6790..ca5a9a105c 100644 --- a/test/spec/apm/bulkWrite.json +++ b/test/spec/apm/bulkWrite.json @@ -23,7 +23,8 @@ "arguments": { "requests": [ { - "insertOne": { + "name": "insertOne", + "arguments": { "document": { "_id": 4, "x": 44 @@ -31,7 +32,8 @@ } }, { - "updateOne": { + "name": "updateOne", + "arguments": { "filter": { "_id": 3 }, @@ -84,9 +86,7 @@ "$set": { "x": 333 } - }, - "upsert": false, - "multi": false + } } ], "ordered": true diff --git a/test/spec/apm/bulkWrite.yml b/test/spec/apm/bulkWrite.yml index 715bd15d48..b2e4a1b9a4 100644 --- a/test/spec/apm/bulkWrite.yml +++ b/test/spec/apm/bulkWrite.yml @@ -13,9 +13,11 @@ tests: name: "bulkWrite" arguments: requests: - - insertOne: + - name: "insertOne" + arguments: document: { _id: 4, x: 44 } - - updateOne: + - name: "updateOne" + arguments: filter: { _id: 3 } update: { $set: { x: 333 } } expectations: @@ -37,7 +39,7 @@ tests: command: update: *collection_name updates: - - { q: {_id: 3 }, u: { $set: { x: 333 } }, upsert: false, multi: false } + - { q: {_id: 3 }, u: { $set: { x: 333 } } } ordered: true command_name: "update" database_name: *database_name diff --git a/test/spec/apm/insertMany.json b/test/spec/apm/insertMany.json index f2605e3c0f..0becf928e4 100644 --- a/test/spec/apm/insertMany.json +++ b/test/spec/apm/insertMany.json @@ -108,7 +108,9 @@ "x": 22 } ], - "ordered": false + "options": { + "ordered": false + } } }, "expectations": [ diff --git a/test/spec/apm/insertMany.yml b/test/spec/apm/insertMany.yml index 3670793397..8c6dc1603a 100644 --- a/test/spec/apm/insertMany.yml +++ b/test/spec/apm/insertMany.yml @@ -58,7 +58,7 @@ tests: arguments: documents: - { _id: 2, x: 22 } - ordered: false + options: { ordered: false } expectations: - command_started_event: diff --git a/test/spec/apm/unacknowledgedBulkWrite.json b/test/spec/apm/unacknowledgedBulkWrite.json index 16d5a151dc..ae116289eb 100644 --- a/test/spec/apm/unacknowledgedBulkWrite.json +++ b/test/spec/apm/unacknowledgedBulkWrite.json @@ -13,10 +13,16 @@ "comment": "On a 2.4 server, no GLE is sent and requires a client-side manufactured reply", "operation": { "name": "bulkWrite", + "collectionOptions": { + "writeConcern": { + "w": 0 + } + }, "arguments": { "requests": [ { - "insertOne": { + "name": "insertOne", + "arguments": { "document": { "_id": "unorderedBulkWriteInsertW0", "x": 44 @@ -24,9 +30,8 @@ } } ], - "ordered": false, - "writeConcern": { - "w": 0 + "options": { + "ordered": false } } }, diff --git a/test/spec/apm/unacknowledgedBulkWrite.yml b/test/spec/apm/unacknowledgedBulkWrite.yml index 3d135ec16a..faf7852050 100644 --- a/test/spec/apm/unacknowledgedBulkWrite.yml +++ b/test/spec/apm/unacknowledgedBulkWrite.yml @@ -10,12 +10,14 @@ tests: comment: "On a 2.4 server, no GLE is sent and requires a client-side manufactured reply" operation: name: "bulkWrite" + collectionOptions: + writeConcern: { w: 0 } arguments: requests: - - insertOne: + - name: "insertOne" + arguments: document: { _id: "unorderedBulkWriteInsertW0", x: 44 } - ordered: false - writeConcern: { w: 0 } + options: { ordered: false } expectations: - command_started_event: diff --git a/test/spec/apm/updateMany.json b/test/spec/apm/updateMany.json index 8e98fc92fd..d82792fc4e 100644 --- a/test/spec/apm/updateMany.json +++ b/test/spec/apm/updateMany.json @@ -51,8 +51,7 @@ "x": 1 } }, - "multi": true, - "upsert": false + "multi": true } ] }, @@ -106,8 +105,7 @@ "x": 1 } }, - "multi": true, - "upsert": false + "multi": true } ] }, diff --git a/test/spec/apm/updateMany.yml b/test/spec/apm/updateMany.yml index c09602ce36..8e1a90677f 100644 --- a/test/spec/apm/updateMany.yml +++ b/test/spec/apm/updateMany.yml @@ -27,7 +27,6 @@ tests: q: { _id: { $gt: 1 }} u: { $inc: { x: 1 }} multi: true - upsert: false command_name: "update" database_name: *database_name - @@ -54,7 +53,6 @@ tests: q: { _id: { $gt: 1 }} u: { $nothing: { x: 1 }} multi: true - upsert: false command_name: "update" database_name: *database_name - diff --git a/test/spec/apm/updateOne.json b/test/spec/apm/updateOne.json index 272aa00ad3..ba41dbb0c0 100644 --- a/test/spec/apm/updateOne.json +++ b/test/spec/apm/updateOne.json @@ -50,9 +50,7 @@ "$inc": { "x": 1 } - }, - "multi": false, - "upsert": false + } } ] }, @@ -103,7 +101,6 @@ "x": 1 } }, - "multi": false, "upsert": true } ] @@ -131,7 +128,6 @@ }, { "description": "A successful update one command with write errors", - "ignore_if_server_version_greater_than": "4.5", "operation": { "name": "updateOne", "arguments": { @@ -164,9 +160,7 @@ "$nothing": { "x": 1 } - }, - "multi": false, - "upsert": false + } } ] }, diff --git a/test/spec/apm/updateOne.yml b/test/spec/apm/updateOne.yml index d9fe28921f..70a1ae1bbb 100644 --- a/test/spec/apm/updateOne.yml +++ b/test/spec/apm/updateOne.yml @@ -26,8 +26,6 @@ tests: - q: { _id: { $gt: 1 }} u: { $inc: { x: 1 }} - multi: false - upsert: false command_name: "update" database_name: *database_name - @@ -54,7 +52,6 @@ tests: - q: { _id: 4 } u: { $inc: { x: 1 } } - multi: false upsert: true command_name: "update" database_name: *database_name @@ -81,8 +78,6 @@ tests: - q: { _id: { $gt: 1 }} u: { $nothing: { x: 1 }} - multi: false - upsert: false command_name: "update" database_name: *database_name -