Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-3452): readonly filters not permitted by typings #2927

Merged
merged 6 commits into from Aug 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/mongo_types.ts
Expand Up @@ -95,11 +95,11 @@ export interface FilterOperators<TValue> extends Document {
$eq?: TValue;
$gt?: TValue;
$gte?: TValue;
$in?: TValue[];
$in?: ReadonlyArray<TValue>;
$lt?: TValue;
$lte?: TValue;
$ne?: TValue;
$nin?: TValue[];
$nin?: ReadonlyArray<TValue>;
// Logical
$not?: TValue extends string ? FilterOperators<TValue> | RegExp : FilterOperators<TValue>;
// Element
Expand All @@ -122,8 +122,8 @@ export interface FilterOperators<TValue> extends Document {
$nearSphere?: Document;
$maxDistance?: number;
// Array
$all?: TValue extends ReadonlyArray<any> ? any[] : never;
$elemMatch?: TValue extends ReadonlyArray<any> ? Document : never;
$all?: ReadonlyArray<any>;
$elemMatch?: Document;
$size?: TValue extends ReadonlyArray<any> ? number : never;
// Bitwise
$bitsAllClear?: BitwiseFilter;
Expand All @@ -137,7 +137,7 @@ export interface FilterOperators<TValue> extends Document {
export type BitwiseFilter =
| number /** numeric bit mask */
| Binary /** BinData bit mask */
| number[]; /** `[ <position1>, <position2>, ... ]` */
| ReadonlyArray<number>; /** `[ <position1>, <position2>, ... ]` */

/** @public */
export const BSONType = Object.freeze({
Expand Down Expand Up @@ -286,7 +286,7 @@ export type PullAllOperator<TSchema> = ({
readonly [key in KeysOfAType<TSchema, ReadonlyArray<any>>]?: TSchema[key];
} &
NotAcceptedFields<TSchema, ReadonlyArray<any>>) & {
readonly [key: string]: any[];
readonly [key: string]: ReadonlyArray<any>;
};

/** @public */
Expand Down Expand Up @@ -320,7 +320,7 @@ export type UpdateFilter<TSchema> = {
export type Nullable<AnyType> = AnyType | null | undefined;

/** @public */
export type OneOrMore<T> = T | T[];
export type OneOrMore<T> = T | ReadonlyArray<T>;

/** @public */
export type GenericListener = (...args: any[]) => void;
Expand Down
4 changes: 2 additions & 2 deletions src/utils.ts
Expand Up @@ -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]);
dariakp marked this conversation as resolved.
Show resolved Hide resolved
fieldHash[key] = (indexSpec as any)[key];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Me again, hope I'm not bothering. Can you (in the interim, per the above conversation) fix needing to add the as any bit by iterating over the result of Object.entries instead? The following seems to make the transpiler happy:

    Object.entries(indexSpec).forEach(([key, value]) => {
      indexes.push(key + '_' + value);
      fieldHash[key] = value;
    });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion! thanks :)

});
}

Expand Down
38 changes: 38 additions & 0 deletions test/types/community/collection/findX.test-d.ts
Expand Up @@ -135,3 +135,41 @@ expectNotType<FindOptions<Car>>({

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<string> = Object.freeze(['blue', 'red']);

// Permitted Readonly fields
expectType<FindCursor<{ color: string }>>(colorCollection.find({ color: { $in: colorsFreeze } }));
expectType<FindCursor<{ color: string }>>(colorCollection.find({ color: { $nin: colorsFreeze } }));
// $all and $elemMatch works against single fields (its just redundant)
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
expectType<FindCursor<{ color: string }>>(colorCollection.find({ color: { $all: colorsFreeze } }));
expectType<FindCursor<{ color: string }>>(
colorCollection.find({ color: { $elemMatch: colorsFreeze } })
);

const countCollection = client.db('test_db').collection<{ count: number }>('test_collection');
expectType<FindCursor<{ count: number }>>(
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<FindCursor<{ lists: string[] }>>(listsCollection.find({ lists: { $size: 1 } }));

const rdOnlyListsCollection = client
.db('test_db')
.collection<{ lists: ReadonlyArray<string> }>('test_collection');
expectType<FindCursor<{ lists: ReadonlyArray<string> }>>(
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<FindCursor<{ color: string | { $in: ReadonlyArray<string> } }>>(
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'] } });
14 changes: 13 additions & 1 deletion 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';

Expand Down Expand Up @@ -223,3 +223,15 @@ expectType<PropExists<typeof indexTypeResultMany2, 'ops'>>(false);

expectType<number>(indexTypeResult2.insertedId);
expectType<{ [key: number]: number }>(indexTypeResultMany2.insertedIds);

// Readonly Tests -- NODE-3452
const rdOnlyColl = client.db('test').collection<{ colors: string[] }>('readonlyColors');
dariakp marked this conversation as resolved.
Show resolved Hide resolved
const colorsFreeze: ReadonlyArray<string> = Object.freeze(['blue', 'red']);
// Users must defined their properties as readonly if they want to be able to insert readonly
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
type InsertOneParam = Parameters<typeof rdOnlyColl.insertOne>[0];
expectNotAssignable<InsertOneParam>({ colors: colorsFreeze });
// Correct usage:
const colorsColl = client
.db('test')
.collection<{ colors: ReadonlyArray<string> }>('readonlyColors');
colorsColl.insertOne({ colors: colorsFreeze }); // Just showing no error here
8 changes: 7 additions & 1 deletion test/types/helper_types.test-d.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -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<OnlyFieldsOfType<IndexedSchema, NumericType>>({ a: 2 });

// OneOrMore should accept readonly arrays
expectAssignable<OneOrMore<number>>(1);
expectAssignable<OneOrMore<number>>([1, 2]);
expectAssignable<OneOrMore<number>>(Object.freeze([1, 2]));