From 6a46a0a3c20a2130ff4f1044547a9162b577bc8f Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 13 Dec 2023 14:42:45 +0100 Subject: [PATCH 1/7] fix(NODE-5647): type error with $addToSet in bulkWrite --- src/bulk/common.ts | 4 ++-- src/collection.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 078b122c5c..bab23c6b9a 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -712,7 +712,7 @@ export class FindOperators { } /** Add a multiple update operation to the bulk operation */ - update(updateDocument: Document | Document[]): BulkOperationBase { + update(updateDocument: UpdateFilter | Document[]): BulkOperationBase { const currentOp = buildCurrentOp(this.bulkOperation); return this.bulkOperation.addToOperationsList( BatchType.UPDATE, @@ -724,7 +724,7 @@ export class FindOperators { } /** Add a single update operation to the bulk operation */ - updateOne(updateDocument: Document | Document[]): BulkOperationBase { + updateOne(updateDocument: UpdateFilter | Document[]): BulkOperationBase { if (!hasAtomicOperators(updateDocument)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } diff --git a/src/collection.ts b/src/collection.ts index d12bbe31ee..c6dd6bddaa 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -345,7 +345,7 @@ export class Collection { */ async updateOne( filter: Filter, - update: UpdateFilter | Partial, + update: UpdateFilter, options?: UpdateOptions ): Promise> { return executeOperation( From 3dccdf298fdb78646a80f1f74998c62d006b8c09 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 15 Dec 2023 12:20:30 +0100 Subject: [PATCH 2/7] feat: sync types --- src/bulk/common.ts | 22 ++++++++++++++++------ src/collection.ts | 21 +++++++++++++++++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index bab23c6b9a..69818b2a0e 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -87,8 +87,13 @@ export interface ReplaceOneModel { export interface UpdateOneModel { /** The filter to limit the updated documents. */ filter: Filter; - /** A document or pipeline containing update operators. */ - update: UpdateFilter | UpdateFilter[]; + /** + * The modifications to apply. The value can be either: + * UpdateFilter - A document that contains update operator expressions, + * Document - A replacement document with only : pairs, + * Document[] - an aggregation pipeline. + * */ + update: UpdateFilter | OneOrMore; /** A set of filters specifying to which array elements an update should apply. */ arrayFilters?: Document[]; /** Specifies a collation. */ @@ -103,8 +108,13 @@ export interface UpdateOneModel { export interface UpdateManyModel { /** The filter to limit the updated documents. */ filter: Filter; - /** A document or pipeline containing update operators. */ - update: UpdateFilter | UpdateFilter[]; + /** + * The modifications to apply. The value can be either: + * UpdateFilter - A document that contains update operator expressions, + * Document - A replacement document with only : pairs, + * Document[] - an aggregation pipeline. + * */ + update: UpdateFilter | OneOrMore; /** A set of filters specifying to which array elements an update should apply. */ arrayFilters?: Document[]; /** Specifies a collation. */ @@ -712,7 +722,7 @@ export class FindOperators { } /** Add a multiple update operation to the bulk operation */ - update(updateDocument: UpdateFilter | Document[]): BulkOperationBase { + update(updateDocument: Document | Document[]): BulkOperationBase { const currentOp = buildCurrentOp(this.bulkOperation); return this.bulkOperation.addToOperationsList( BatchType.UPDATE, @@ -724,7 +734,7 @@ export class FindOperators { } /** Add a single update operation to the bulk operation */ - updateOne(updateDocument: UpdateFilter | Document[]): BulkOperationBase { + updateOne(updateDocument: Document | Document[]): BulkOperationBase { if (!hasAtomicOperators(updateDocument)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } diff --git a/src/collection.ts b/src/collection.ts index c6dd6bddaa..76e26f5d0e 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -16,6 +16,7 @@ import type { MongoClient, PkFactory } from './mongo_client'; import type { Filter, Flatten, + OneOrMore, OptionalUnlessRequiredId, TODO_NODE_3286, UpdateFilter, @@ -340,12 +341,18 @@ export class Collection { * Update a single document in a collection * * @param filter - The filter used to select the document to update - * @param update - The update operations to be applied to the document + * @param update - The modifications to apply * @param options - Optional settings for the command */ async updateOne( filter: Filter, - update: UpdateFilter, + /** + * The value of update can be either: + * UpdateFilter - A document that contains update operator expressions, + * Document - A replacement document with only : pairs, + * Document[] - an aggregation pipeline. + * */ + update: UpdateFilter | OneOrMore, options?: UpdateOptions ): Promise> { return executeOperation( @@ -386,12 +393,18 @@ export class Collection { * Update multiple documents in a collection * * @param filter - The filter used to select the documents to update - * @param update - The update operations to be applied to the documents + * @param update - The modifications to apply * @param options - Optional settings for the command */ async updateMany( filter: Filter, - update: UpdateFilter, + /** + * The value of update can be either: + * UpdateFilter - A document that contains update operator expressions, + * Document - A replacement document with only : pairs, + * Document[] - an aggregation pipeline. + * */ + update: UpdateFilter | OneOrMore, options?: UpdateOptions ): Promise> { return executeOperation( From ecc9e998223c8700c46007041a03f98114e3c8d3 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Mon, 18 Dec 2023 14:20:17 +0100 Subject: [PATCH 3/7] test: add type tests --- src/bulk/common.ts | 6 +- src/collection.ts | 7 +- src/mongo_types.ts | 13 ++- .../community/collection/bulkWrite.test-d.ts | 105 +++++++++++++++++- .../community/collection/updateX.test-d.ts | 39 +++++++ 5 files changed, 159 insertions(+), 11 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 69818b2a0e..f9217cdc26 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -90,10 +90,9 @@ export interface UpdateOneModel { /** * The modifications to apply. The value can be either: * UpdateFilter - A document that contains update operator expressions, - * Document - A replacement document with only : pairs, * Document[] - an aggregation pipeline. * */ - update: UpdateFilter | OneOrMore; + update: UpdateFilter | Document[]; /** A set of filters specifying to which array elements an update should apply. */ arrayFilters?: Document[]; /** Specifies a collation. */ @@ -111,10 +110,9 @@ export interface UpdateManyModel { /** * The modifications to apply. The value can be either: * UpdateFilter - A document that contains update operator expressions, - * Document - A replacement document with only : pairs, * Document[] - an aggregation pipeline. * */ - update: UpdateFilter | OneOrMore; + update: UpdateFilter | Document[]; /** A set of filters specifying to which array elements an update should apply. */ arrayFilters?: Document[]; /** Specifies a collation. */ diff --git a/src/collection.ts b/src/collection.ts index 76e26f5d0e..bc2c432fb8 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -16,7 +16,6 @@ import type { MongoClient, PkFactory } from './mongo_client'; import type { Filter, Flatten, - OneOrMore, OptionalUnlessRequiredId, TODO_NODE_3286, UpdateFilter, @@ -349,10 +348,9 @@ export class Collection { /** * The value of update can be either: * UpdateFilter - A document that contains update operator expressions, - * Document - A replacement document with only : pairs, * Document[] - an aggregation pipeline. * */ - update: UpdateFilter | OneOrMore, + update: UpdateFilter | Document[], options?: UpdateOptions ): Promise> { return executeOperation( @@ -401,10 +399,9 @@ export class Collection { /** * The value of update can be either: * UpdateFilter - A document that contains update operator expressions, - * Document - A replacement document with only : pairs, * Document[] - an aggregation pipeline. * */ - update: UpdateFilter | OneOrMore, + update: UpdateFilter | Document[], options?: UpdateOptions ): Promise> { return executeOperation( diff --git a/src/mongo_types.ts b/src/mongo_types.ts index adfec0af67..975d4191d3 100644 --- a/src/mongo_types.ts +++ b/src/mongo_types.ts @@ -232,12 +232,23 @@ export type ArrayOperator = { $sort?: Sort; }; +/** @internal */ +type RemoveIndex = { + [K in keyof T as string extends K + ? never + : number extends K + ? never + : symbol extends K + ? never + : K]: T[K]; +}; + /** @public */ export type SetFields = ({ readonly [key in KeysOfAType | undefined>]?: | OptionalId> | AddToSetOperators>>>; -} & NotAcceptedFields | undefined>) & { +} & NotAcceptedFields, ReadonlyArray | undefined>) & { readonly [key: string]: AddToSetOperators | any; }; diff --git a/test/types/community/collection/bulkWrite.test-d.ts b/test/types/community/collection/bulkWrite.test-d.ts index 5c8ee78ecf..62abd5accc 100644 --- a/test/types/community/collection/bulkWrite.test-d.ts +++ b/test/types/community/collection/bulkWrite.test-d.ts @@ -1,6 +1,6 @@ import { expectError } from 'tsd'; -import { MongoClient, ObjectId } from '../../../mongodb'; +import { type Collection, type Document, MongoClient, ObjectId } from '../../../mongodb'; // TODO(NODE-3347): Improve these tests to use more expect assertions @@ -297,3 +297,106 @@ collectionType.bulkWrite([ } } ]); + +{ + // NODE-5647 - Type error with $addToSet in bulkWrite + interface TestDocument { + readonly myId: number; + readonly mySet: number[]; + } + const collection = undefined as unknown as Collection; + collection.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 0 } + } + } + } + ]); + collection.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: [ + { + $addToSet: { mySet: 0 } + } + ] + } + } + ]); + collection.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 0 } + } + } + } + ]); + collection.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: [ + { + $addToSet: { mySet: 0 } + } + ] + } + } + ]); + + interface IndexSingatureTestDocument extends Document { + readonly myId: number; + readonly mySet: number[]; + } + const indexSingatureCollection = undefined as unknown as Collection; + indexSingatureCollection.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 0 } + } + } + } + ]); + indexSingatureCollection.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: [ + { + $addToSet: { mySet: 0 } + } + ] + } + } + ]); + indexSingatureCollection.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 0 } + } + } + } + ]); + indexSingatureCollection.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: [ + { + $addToSet: { mySet: 0 } + } + ] + } + } + ]); +} diff --git a/test/types/community/collection/updateX.test-d.ts b/test/types/community/collection/updateX.test-d.ts index b729b20b4d..d19ccdbde0 100644 --- a/test/types/community/collection/updateX.test-d.ts +++ b/test/types/community/collection/updateX.test-d.ts @@ -439,3 +439,42 @@ export async function testPushWithId(): Promise { collectionWithSchema.updateMany({}, {}) ); } + +{ + // NODE-5647 - Type error with $addToSet in bulkWrite + interface TestDocument { + readonly myId: number; + readonly mySet: number[]; + } + const collection = undefined as unknown as Collection; + collection.updateOne({ myId: 0 }, { $addToSet: { mySet: 0 } }); + collection.updateOne({ myId: 0 }, [ + { + $addToSet: { mySet: 0 } + } + ]); + collection.updateMany({ myId: 0 }, { $addToSet: { mySet: 0 } }); + collection.updateMany({ myId: 0 }, [ + { + $addToSet: { mySet: 0 } + } + ]); + + interface IndexSingatureTestDocument extends Document { + readonly myId: number; + readonly mySet: number[]; + } + const indexSingatureCollection = undefined as unknown as Collection; + indexSingatureCollection.updateOne({ myId: 0 }, { $addToSet: { mySet: 0 } }); + indexSingatureCollection.updateOne({ myId: 0 }, [ + { + $addToSet: { mySet: 0 } + } + ]); + indexSingatureCollection.updateMany({ myId: 0 }, { $addToSet: { mySet: 0 } }); + indexSingatureCollection.updateMany({ myId: 0 }, [ + { + $addToSet: { mySet: 0 } + } + ]); +} From b8c8ebbf20373fd76ae500d1c45ca2eddcbb91af Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 20 Dec 2023 13:29:00 +0100 Subject: [PATCH 4/7] feat: do not validate fields that have any type --- src/mongo_types.ts | 17 ++----- .../community/collection/bulkWrite.test-d.ts | 47 ++++++++++++++++++- .../community/collection/updateX.test-d.ts | 24 ++++++++++ 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/src/mongo_types.ts b/src/mongo_types.ts index 975d4191d3..7de27b75eb 100644 --- a/src/mongo_types.ts +++ b/src/mongo_types.ts @@ -232,23 +232,16 @@ export type ArrayOperator = { $sort?: Sort; }; -/** @internal */ -type RemoveIndex = { - [K in keyof T as string extends K - ? never - : number extends K - ? never - : symbol extends K - ? never - : K]: T[K]; -}; - /** @public */ export type SetFields = ({ readonly [key in KeysOfAType | undefined>]?: | OptionalId> | AddToSetOperators>>>; -} & NotAcceptedFields, ReadonlyArray | undefined>) & { +} & IsAny< + TSchema[keyof TSchema], + object, + NotAcceptedFields | undefined> +>) & { readonly [key: string]: AddToSetOperators | any; }; diff --git a/test/types/community/collection/bulkWrite.test-d.ts b/test/types/community/collection/bulkWrite.test-d.ts index 62abd5accc..f922cdca64 100644 --- a/test/types/community/collection/bulkWrite.test-d.ts +++ b/test/types/community/collection/bulkWrite.test-d.ts @@ -303,8 +303,21 @@ collectionType.bulkWrite([ interface TestDocument { readonly myId: number; readonly mySet: number[]; + readonly myAny: any; } const collection = undefined as unknown as Collection; + expectError( + collection.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 'value of other type' } + } + } + } + ]) + ); collection.bulkWrite([ { updateOne: { @@ -315,18 +328,40 @@ collectionType.bulkWrite([ } } ]); + collection.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: { + $addToSet: { myAny: 1 } + } + } + } + ]); collection.bulkWrite([ { updateOne: { filter: { myId: 0 }, update: [ { - $addToSet: { mySet: 0 } + $addToSet: { myAny: 0 } } ] } } ]); + expectError( + collection.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 'value of other type' } + } + } + } + ]) + ); collection.bulkWrite([ { updateMany: { @@ -337,6 +372,16 @@ collectionType.bulkWrite([ } } ]); + collection.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: { + $addToSet: { myAny: 1 } + } + } + } + ]); collection.bulkWrite([ { updateMany: { diff --git a/test/types/community/collection/updateX.test-d.ts b/test/types/community/collection/updateX.test-d.ts index d19ccdbde0..5ea59cd525 100644 --- a/test/types/community/collection/updateX.test-d.ts +++ b/test/types/community/collection/updateX.test-d.ts @@ -445,15 +445,20 @@ export async function testPushWithId(): Promise { interface TestDocument { readonly myId: number; readonly mySet: number[]; + readonly myAny: any; } const collection = undefined as unknown as Collection; + expectError(collection.updateOne({ myId: 0 }, { $addToSet: { mySet: 'value of other type' } })); collection.updateOne({ myId: 0 }, { $addToSet: { mySet: 0 } }); + collection.updateOne({ myId: 0 }, { $addToSet: { myAny: 1 } }); collection.updateOne({ myId: 0 }, [ { $addToSet: { mySet: 0 } } ]); + expectError(collection.updateMany({ myId: 0 }, { $addToSet: { mySet: 'value of other type' } })); collection.updateMany({ myId: 0 }, { $addToSet: { mySet: 0 } }); + collection.updateMany({ myId: 0 }, { $addToSet: { myAny: 1 } }); collection.updateMany({ myId: 0 }, [ { $addToSet: { mySet: 0 } @@ -463,18 +468,37 @@ export async function testPushWithId(): Promise { interface IndexSingatureTestDocument extends Document { readonly myId: number; readonly mySet: number[]; + readonly myAny: any; } const indexSingatureCollection = undefined as unknown as Collection; indexSingatureCollection.updateOne({ myId: 0 }, { $addToSet: { mySet: 0 } }); + indexSingatureCollection.updateOne({ myId: 0 }, { $addToSet: { myAny: 1 } }); indexSingatureCollection.updateOne({ myId: 0 }, [ { $addToSet: { mySet: 0 } } ]); indexSingatureCollection.updateMany({ myId: 0 }, { $addToSet: { mySet: 0 } }); + indexSingatureCollection.updateMany({ myId: 0 }, { $addToSet: { myAny: 1 } }); indexSingatureCollection.updateMany({ myId: 0 }, [ { $addToSet: { mySet: 0 } } ]); + + const collectionOfAnyType = undefined as unknown as Collection; + collectionOfAnyType.updateOne({ myId: 0 }, { $addToSet: { mySet: 0 } }); + collectionOfAnyType.updateOne({ myId: 0 }, { $addToSet: { myAny: 1 } }); + collectionOfAnyType.updateOne({ myId: 0 }, [ + { + $addToSet: { mySet: 0 } + } + ]); + collectionOfAnyType.updateMany({ myId: 0 }, { $addToSet: { mySet: 0 } }); + collectionOfAnyType.updateMany({ myId: 0 }, { $addToSet: { myAny: 1 } }); + collectionOfAnyType.updateMany({ myId: 0 }, [ + { + $addToSet: { mySet: 0 } + } + ]); } From 820e628fd53ea73534a1403e5f7eac46617bebb1 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 20 Dec 2023 14:31:19 +0100 Subject: [PATCH 5/7] test: add tests for bulkWrite with any --- .../community/collection/bulkWrite.test-d.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/types/community/collection/bulkWrite.test-d.ts b/test/types/community/collection/bulkWrite.test-d.ts index f922cdca64..f47044eb92 100644 --- a/test/types/community/collection/bulkWrite.test-d.ts +++ b/test/types/community/collection/bulkWrite.test-d.ts @@ -444,4 +444,50 @@ collectionType.bulkWrite([ } } ]); + + const collectionOfAnyType = undefined as unknown as Collection; + collectionOfAnyType.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 0 } + } + } + } + ]); + collectionOfAnyType.bulkWrite([ + { + updateOne: { + filter: { myId: 0 }, + update: [ + { + $addToSet: { mySet: 0 } + } + ] + } + } + ]); + collectionOfAnyType.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: { + $addToSet: { mySet: 0 } + } + } + } + ]); + collectionOfAnyType.bulkWrite([ + { + updateMany: { + filter: { myId: 0 }, + update: [ + { + $addToSet: { mySet: 0 } + } + ] + } + } + ]); } From 88d798f9a0d9460b4bf0efe1f542534149a977c8 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 22 Dec 2023 19:09:16 +0100 Subject: [PATCH 6/7] docs: update comments --- src/collection.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index bc2c432fb8..a735a3a082 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -339,17 +339,16 @@ export class Collection { /** * Update a single document in a collection * + * The value of `update` can be either: + * - UpdateFilter - A document that contains update operator expressions, + * - Document[] - an aggregation pipeline. + * * @param filter - The filter used to select the document to update * @param update - The modifications to apply * @param options - Optional settings for the command */ async updateOne( filter: Filter, - /** - * The value of update can be either: - * UpdateFilter - A document that contains update operator expressions, - * Document[] - an aggregation pipeline. - * */ update: UpdateFilter | Document[], options?: UpdateOptions ): Promise> { @@ -390,17 +389,16 @@ export class Collection { /** * Update multiple documents in a collection * - * @param filter - The filter used to select the documents to update + * The value of `update` can be either: + * - UpdateFilter - A document that contains update operator expressions, + * - Document[] - an aggregation pipeline. + * + * @param filter - The filter used to select the document to update * @param update - The modifications to apply * @param options - Optional settings for the command */ async updateMany( filter: Filter, - /** - * The value of update can be either: - * UpdateFilter - A document that contains update operator expressions, - * Document[] - an aggregation pipeline. - * */ update: UpdateFilter | Document[], options?: UpdateOptions ): Promise> { From 044ec4bc52ecd63d3802079ccf629d8972129ac5 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 2 Jan 2024 13:13:25 -0500 Subject: [PATCH 7/7] chore: comment formatting --- src/bulk/common.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index f9217cdc26..0e782b5702 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -91,7 +91,7 @@ export interface UpdateOneModel { * The modifications to apply. The value can be either: * UpdateFilter - A document that contains update operator expressions, * Document[] - an aggregation pipeline. - * */ + */ update: UpdateFilter | Document[]; /** A set of filters specifying to which array elements an update should apply. */ arrayFilters?: Document[]; @@ -111,7 +111,7 @@ export interface UpdateManyModel { * The modifications to apply. The value can be either: * UpdateFilter - A document that contains update operator expressions, * Document[] - an aggregation pipeline. - * */ + */ update: UpdateFilter | Document[]; /** A set of filters specifying to which array elements an update should apply. */ arrayFilters?: Document[];