From 3a26d5dcca153b09477c815fb0035e1dfc02eea0 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 2 Aug 2021 15:42:55 -0400 Subject: [PATCH 1/6] fix(NODE-3452): readonly filters not permitted by typings --- src/mongo_types.ts | 14 +++---- src/utils.ts | 4 +- .../community/collection/findX.test-d.ts | 38 +++++++++++++++++++ .../community/collection/insertX.test-d.ts | 14 ++++++- test/types/helper_types.test-d.ts | 8 +++- 5 files changed, 67 insertions(+), 11 deletions(-) diff --git a/src/mongo_types.ts b/src/mongo_types.ts index ff1628157f..cf25f75d66 100644 --- a/src/mongo_types.ts +++ b/src/mongo_types.ts @@ -95,11 +95,11 @@ export interface FilterOperators extends Document { $eq?: TValue; $gt?: TValue; $gte?: TValue; - $in?: TValue[]; + $in?: ReadonlyArray; $lt?: TValue; $lte?: TValue; $ne?: TValue; - $nin?: TValue[]; + $nin?: ReadonlyArray; // Logical $not?: TValue extends string ? FilterOperators | RegExp : FilterOperators; // Element @@ -122,8 +122,8 @@ export interface FilterOperators extends Document { $nearSphere?: Document; $maxDistance?: number; // Array - $all?: TValue extends ReadonlyArray ? any[] : never; - $elemMatch?: TValue extends ReadonlyArray ? Document : never; + $all?: ReadonlyArray; + $elemMatch?: Document; $size?: TValue extends ReadonlyArray ? number : never; // Bitwise $bitsAllClear?: BitwiseFilter; @@ -137,7 +137,7 @@ export interface FilterOperators extends Document { export type BitwiseFilter = | number /** numeric bit mask */ | Binary /** BinData bit mask */ - | number[]; /** `[ , , ... ]` */ + | ReadonlyArray; /** `[ , , ... ]` */ /** @public */ export const BSONType = Object.freeze({ @@ -286,7 +286,7 @@ export type PullAllOperator = ({ readonly [key in KeysOfAType>]?: TSchema[key]; } & NotAcceptedFields>) & { - readonly [key: string]: any[]; + readonly [key: string]: ReadonlyArray; }; /** @public */ @@ -320,7 +320,7 @@ export type UpdateFilter = { export type Nullable = AnyType | null | undefined; /** @public */ -export type OneOrMore = T | T[]; +export type OneOrMore = T | ReadonlyArray; /** @public */ export type GenericListener = (...args: any[]) => void; diff --git a/src/utils.ts b/src/utils.ts index 1af2ac2f00..050fc6d2fb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -157,8 +157,8 @@ export function parseIndexOptions(indexSpec: IndexSpecification): IndexOptions { // {location:'2d', type:1} keys = Object.keys(indexSpec); keys.forEach(key => { - indexes.push(key + '_' + indexSpec[key]); - fieldHash[key] = indexSpec[key]; + indexes.push(key + '_' + (indexSpec as any)[key]); + fieldHash[key] = (indexSpec as any)[key]; }); } diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 5a4f162030..738b9c13e2 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -135,3 +135,41 @@ expectNotType>({ printCar(await car.findOne({}, options)); printCar(await car.findOne({}, optionsWithProjection)); + +// Readonly tests -- NODE-3452 +const colorCollection = client.db('test_db').collection<{ color: string }>('test_collection'); +const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); + +// Permitted Readonly fields +expectType>(colorCollection.find({ color: { $in: colorsFreeze } })); +expectType>(colorCollection.find({ color: { $nin: colorsFreeze } })); +// $all and $elemMatch works against single fields (its just redundant) +expectType>(colorCollection.find({ color: { $all: colorsFreeze } })); +expectType>( + colorCollection.find({ color: { $elemMatch: colorsFreeze } }) +); + +const countCollection = client.db('test_db').collection<{ count: number }>('test_collection'); +expectType>( + countCollection.find({ count: { $bitsAnySet: Object.freeze([1, 0, 1]) } }) +); + +const listsCollection = client.db('test_db').collection<{ lists: string[] }>('test_collection'); +await listsCollection.updateOne({}, { list: { $pullAll: Object.freeze(['one', 'two']) } }); +expectType>(listsCollection.find({ lists: { $size: 1 } })); + +const rdOnlyListsCollection = client + .db('test_db') + .collection<{ lists: ReadonlyArray }>('test_collection'); +expectType }>>( + rdOnlyListsCollection.find({ lists: { $size: 1 } }) +); + +// Before NODE-3452's fix we would get this strange result that included the filter shape joined with the actual schema +expectNotType } }>>( + colorCollection.find({ color: { $in: colorsFreeze } }) +); + +// When you use the override, $in doesn't permit readonly +colorCollection.find<{ color: string }>({ color: { $in: colorsFreeze } }); +colorCollection.find<{ color: string }>({ color: { $in: ['regularArray'] } }); diff --git a/test/types/community/collection/insertX.test-d.ts b/test/types/community/collection/insertX.test-d.ts index 8d51559fde..c605025c36 100644 --- a/test/types/community/collection/insertX.test-d.ts +++ b/test/types/community/collection/insertX.test-d.ts @@ -1,4 +1,4 @@ -import { expectError, expectNotType, expectType } from 'tsd'; +import { expectError, expectNotAssignable, expectNotType, expectType } from 'tsd'; import { MongoClient, ObjectId, OptionalId } from '../../../../src'; import type { PropExists } from '../../utility_types'; @@ -223,3 +223,15 @@ expectType>(false); expectType(indexTypeResult2.insertedId); expectType<{ [key: number]: number }>(indexTypeResultMany2.insertedIds); + +// Readonly Tests -- NODE-3452 +const rdOnlyColl = client.db('test').collection<{ colors: string[] }>('readonlyColors'); +const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); +// Users must defined their properties as readonly if they want to be able to insert readonly +type InsertOneParam = Parameters[0]; +expectNotAssignable({ colors: colorsFreeze }); +// Correct usage: +const colorsColl = client + .db('test') + .collection<{ colors: ReadonlyArray }>('readonlyColors'); +colorsColl.insertOne({ colors: colorsFreeze }); // Just showing no error here diff --git a/test/types/helper_types.test-d.ts b/test/types/helper_types.test-d.ts index c71d3c7af2..33253a4e5b 100644 --- a/test/types/helper_types.test-d.ts +++ b/test/types/helper_types.test-d.ts @@ -8,7 +8,8 @@ import type { FilterOperations, OnlyFieldsOfType, IntegerType, - IsAny + IsAny, + OneOrMore } from '../../src/mongo_types'; import { Decimal128, Double, Int32, Long, Document } from '../../src/index'; @@ -97,3 +98,8 @@ interface IndexedSchema { // This means we can't properly enforce the subtype and there doesn't seem to be a way to detect it // and reduce strictness like we can with any, users with indexed schemas will have to use `as any` expectNotAssignable>({ a: 2 }); + +// OneOrMore should accept readonly arrays +expectAssignable>(1); +expectAssignable>([1, 2]); +expectAssignable>(Object.freeze([1, 2])); From 8ab269d0a89b74d54b043b8d248ffebba0abae4c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 2 Aug 2021 16:57:44 -0400 Subject: [PATCH 2/6] defined -> define Co-authored-by: Daria Pardue <81593090+dariakp@users.noreply.github.com> --- test/types/community/collection/insertX.test-d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/types/community/collection/insertX.test-d.ts b/test/types/community/collection/insertX.test-d.ts index c605025c36..7a2846936c 100644 --- a/test/types/community/collection/insertX.test-d.ts +++ b/test/types/community/collection/insertX.test-d.ts @@ -227,7 +227,7 @@ expectType<{ [key: number]: number }>(indexTypeResultMany2.insertedIds); // Readonly Tests -- NODE-3452 const rdOnlyColl = client.db('test').collection<{ colors: string[] }>('readonlyColors'); const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); -// Users must defined their properties as readonly if they want to be able to insert readonly +// Users must define their properties as readonly if they want to be able to insert readonly type InsertOneParam = Parameters[0]; expectNotAssignable({ colors: colorsFreeze }); // Correct usage: From c1078696d494b0dd7fa728823251af01fa450003 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 2 Aug 2021 16:58:01 -0400 Subject: [PATCH 3/6] its -> it's Co-authored-by: Daria Pardue <81593090+dariakp@users.noreply.github.com> --- test/types/community/collection/findX.test-d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 738b9c13e2..cc754491cc 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -143,7 +143,7 @@ const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); // Permitted Readonly fields expectType>(colorCollection.find({ color: { $in: colorsFreeze } })); expectType>(colorCollection.find({ color: { $nin: colorsFreeze } })); -// $all and $elemMatch works against single fields (its just redundant) +// $all and $elemMatch works against single fields (it's just redundant) expectType>(colorCollection.find({ color: { $all: colorsFreeze } })); expectType>( colorCollection.find({ color: { $elemMatch: colorsFreeze } }) From 7c677d423ca74e3089148efb9abc951d5f50468e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 2 Aug 2021 17:00:12 -0400 Subject: [PATCH 4/6] test: var naming --- test/types/community/collection/insertX.test-d.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/types/community/collection/insertX.test-d.ts b/test/types/community/collection/insertX.test-d.ts index 7a2846936c..9b747812dd 100644 --- a/test/types/community/collection/insertX.test-d.ts +++ b/test/types/community/collection/insertX.test-d.ts @@ -225,13 +225,13 @@ expectType(indexTypeResult2.insertedId); expectType<{ [key: number]: number }>(indexTypeResultMany2.insertedIds); // Readonly Tests -- NODE-3452 -const rdOnlyColl = client.db('test').collection<{ colors: string[] }>('readonlyColors'); +const colorsColl = client.db('test').collection<{ colors: string[] }>('readonlyColors'); const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); // Users must define their properties as readonly if they want to be able to insert readonly -type InsertOneParam = Parameters[0]; +type InsertOneParam = Parameters[0]; expectNotAssignable({ colors: colorsFreeze }); // Correct usage: -const colorsColl = client +const rdOnlyColl = client .db('test') .collection<{ colors: ReadonlyArray }>('readonlyColors'); -colorsColl.insertOne({ colors: colorsFreeze }); // Just showing no error here +rdOnlyColl.insertOne({ colors: colorsFreeze }); // Just showing no error here From 451fba021722668401e1ea3a81b2485eb11b64d9 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 3 Aug 2021 16:48:09 -0400 Subject: [PATCH 5/6] fix: tests and using Object.entries --- src/utils.ts | 6 +++--- test/types/community/collection/findX.test-d.ts | 17 +++++++++++++++++ .../community/collection/insertX.test-d.ts | 8 +++++--- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 050fc6d2fb..26600970ee 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -156,9 +156,9 @@ export function parseIndexOptions(indexSpec: IndexSpecification): IndexOptions { } else if (isObject(indexSpec)) { // {location:'2d', type:1} keys = Object.keys(indexSpec); - keys.forEach(key => { - indexes.push(key + '_' + (indexSpec as any)[key]); - fieldHash[key] = (indexSpec as any)[key]; + Object.entries(indexSpec).forEach(([key, value]) => { + indexes.push(key + '_' + value); + fieldHash[key] = value; }); } diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index cc754491cc..bc17e3179e 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -139,20 +139,34 @@ printCar(await car.findOne({}, optionsWithProjection)); // Readonly tests -- NODE-3452 const colorCollection = client.db('test_db').collection<{ color: string }>('test_collection'); const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); +const colorsWritable: Array = ['blue', 'red']; // Permitted Readonly fields expectType>(colorCollection.find({ color: { $in: colorsFreeze } })); +expectType>(colorCollection.find({ color: { $in: colorsWritable } })); expectType>(colorCollection.find({ color: { $nin: colorsFreeze } })); +expectType>( + colorCollection.find({ color: { $nin: colorsWritable } }) +); // $all and $elemMatch works against single fields (it's just redundant) expectType>(colorCollection.find({ color: { $all: colorsFreeze } })); +expectType>( + colorCollection.find({ color: { $all: colorsWritable } }) +); expectType>( colorCollection.find({ color: { $elemMatch: colorsFreeze } }) ); +expectType>( + colorCollection.find({ color: { $elemMatch: colorsWritable } }) +); const countCollection = client.db('test_db').collection<{ count: number }>('test_collection'); expectType>( countCollection.find({ count: { $bitsAnySet: Object.freeze([1, 0, 1]) } }) ); +expectType>( + countCollection.find({ count: { $bitsAnySet: [1, 0, 1] as number[] } }) +); const listsCollection = client.db('test_db').collection<{ lists: string[] }>('test_collection'); await listsCollection.updateOne({}, { list: { $pullAll: Object.freeze(['one', 'two']) } }); @@ -170,6 +184,9 @@ expectNotType } }>>( colorCollection.find({ color: { $in: colorsFreeze } }) ); +// This is related to another bug that will be fixed in NODE-3454 +expectType>(colorCollection.find({ color: { $in: 3 } })); + // When you use the override, $in doesn't permit readonly colorCollection.find<{ color: string }>({ color: { $in: colorsFreeze } }); colorCollection.find<{ color: string }>({ color: { $in: ['regularArray'] } }); diff --git a/test/types/community/collection/insertX.test-d.ts b/test/types/community/collection/insertX.test-d.ts index 9b747812dd..32728342b4 100644 --- a/test/types/community/collection/insertX.test-d.ts +++ b/test/types/community/collection/insertX.test-d.ts @@ -1,4 +1,4 @@ -import { expectError, expectNotAssignable, expectNotType, expectType } from 'tsd'; +import { expectAssignable, expectError, expectNotAssignable, expectNotType, expectType } from 'tsd'; import { MongoClient, ObjectId, OptionalId } from '../../../../src'; import type { PropExists } from '../../utility_types'; @@ -225,7 +225,7 @@ expectType(indexTypeResult2.insertedId); expectType<{ [key: number]: number }>(indexTypeResultMany2.insertedIds); // Readonly Tests -- NODE-3452 -const colorsColl = client.db('test').collection<{ colors: string[] }>('readonlyColors'); +const colorsColl = client.db('test').collection<{ colors: string[] }>('writableColors'); const colorsFreeze: ReadonlyArray = Object.freeze(['blue', 'red']); // Users must define their properties as readonly if they want to be able to insert readonly type InsertOneParam = Parameters[0]; @@ -234,4 +234,6 @@ expectNotAssignable({ colors: colorsFreeze }); const rdOnlyColl = client .db('test') .collection<{ colors: ReadonlyArray }>('readonlyColors'); -rdOnlyColl.insertOne({ colors: colorsFreeze }); // Just showing no error here +rdOnlyColl.insertOne({ colors: colorsFreeze }); +const colorsWritable = ['a', 'b']; +rdOnlyColl.insertOne({ colors: colorsWritable }); From dddc76944088bdf45895840182b7a5e96fa56455 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 3 Aug 2021 16:55:37 -0400 Subject: [PATCH 6/6] fix: lint --- test/types/community/collection/insertX.test-d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/types/community/collection/insertX.test-d.ts b/test/types/community/collection/insertX.test-d.ts index 32728342b4..c2cb43ba4c 100644 --- a/test/types/community/collection/insertX.test-d.ts +++ b/test/types/community/collection/insertX.test-d.ts @@ -1,4 +1,4 @@ -import { expectAssignable, expectError, expectNotAssignable, expectNotType, expectType } from 'tsd'; +import { expectError, expectNotAssignable, expectNotType, expectType } from 'tsd'; import { MongoClient, ObjectId, OptionalId } from '../../../../src'; import type { PropExists } from '../../utility_types';