Skip to content

Commit b1854ab

Browse files
author
Brian Chen
authored
Fix NotInFilter matches (#3752)
1 parent e81c429 commit b1854ab

File tree

4 files changed

+174
-188
lines changed

4 files changed

+174
-188
lines changed

‎packages/firestore/src/core/query.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,11 @@ export class NotInFilter extends FieldFilter {
855855
}
856856

857857
matches(doc: Document): boolean {
858+
if (
859+
arrayValueContains(this.value.arrayValue!, { nullValue: 'NULL_VALUE' })
860+
) {
861+
return false;
862+
}
858863
const other = doc.field(this.field);
859864
return other !== null && !arrayValueContains(this.value.arrayValue!, other);
860865
}

‎packages/firestore/test/integration/api/query.test.ts

Lines changed: 145 additions & 182 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import { EventsAccumulator } from '../util/events_accumulator';
2424
import * as firebaseExport from '../util/firebase_export';
2525
import {
2626
apiDescribe,
27-
isRunningAgainstEmulator,
2827
notEqualOp,
2928
notInOp,
3029
toChangesArray,
@@ -687,101 +686,83 @@ apiDescribe('Queries', (persistence: boolean) => {
687686
});
688687
});
689688

690-
// eslint-disable-next-line no-restricted-properties
691-
(isRunningAgainstEmulator() ? it : it.skip)(
692-
'can use != filters',
693-
async () => {
694-
const testDocs = {
695-
a: { zip: 98101 },
696-
b: { zip: 91102 },
697-
c: { zip: '98101' },
698-
d: { zip: [98101] },
699-
e: { zip: ['98101', { zip: 98101 }] },
700-
f: { zip: { code: 500 } },
701-
g: { zip: [98101, 98102] },
702-
h: { code: 500 },
703-
i: { zip: null },
704-
j: { zip: Number.NaN }
705-
};
689+
it('can use != filters', async () => {
690+
const testDocs = {
691+
a: { zip: 98101 },
692+
b: { zip: 91102 },
693+
c: { zip: '98101' },
694+
d: { zip: [98101] },
695+
e: { zip: ['98101', { zip: 98101 }] },
696+
f: { zip: { code: 500 } },
697+
g: { zip: [98101, 98102] },
698+
h: { code: 500 },
699+
i: { zip: null },
700+
j: { zip: Number.NaN }
701+
};
706702

707-
await withTestCollection(persistence, testDocs, async coll => {
708-
let expected = { ...testDocs };
709-
// @ts-expect-error
710-
delete expected.a;
711-
// @ts-expect-error
712-
delete expected.h;
713-
// @ts-expect-error
714-
delete expected.i;
715-
const snapshot = await coll.where('zip', notEqualOp, 98101).get();
716-
expect(toDataArray(snapshot)).to.have.deep.members(
717-
Object.values(expected)
718-
);
703+
await withTestCollection(persistence, testDocs, async coll => {
704+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
705+
let expected: { [name: string]: any } = { ...testDocs };
706+
delete expected.a;
707+
delete expected.h;
708+
delete expected.i;
709+
const snapshot = await coll.where('zip', notEqualOp, 98101).get();
710+
expect(toDataArray(snapshot)).to.have.deep.members(
711+
Object.values(expected)
712+
);
719713

720-
// With objects.
721-
const snapshot2 = await coll
722-
.where('zip', notEqualOp, { code: 500 })
723-
.get();
724-
expected = { ...testDocs };
725-
// @ts-expect-error
726-
delete expected.f;
727-
// @ts-expect-error
728-
delete expected.h;
729-
// @ts-expect-error
730-
delete expected.i;
731-
expect(toDataArray(snapshot2)).to.have.deep.members(
732-
Object.values(expected)
733-
);
714+
// With objects.
715+
const snapshot2 = await coll
716+
.where('zip', notEqualOp, { code: 500 })
717+
.get();
718+
expected = { ...testDocs };
719+
delete expected.f;
720+
delete expected.h;
721+
delete expected.i;
722+
expect(toDataArray(snapshot2)).to.have.deep.members(
723+
Object.values(expected)
724+
);
734725

735-
// With null.
736-
const snapshot3 = await coll.where('zip', notEqualOp, null).get();
737-
expected = { ...testDocs };
738-
// @ts-expect-error
739-
delete expected.h;
740-
// @ts-expect-error
741-
delete expected.i;
742-
expect(toDataArray(snapshot3)).to.have.deep.members(
743-
Object.values(expected)
744-
);
726+
// With null.
727+
const snapshot3 = await coll.where('zip', notEqualOp, null).get();
728+
expected = { ...testDocs };
729+
delete expected.h;
730+
delete expected.i;
731+
expect(toDataArray(snapshot3)).to.have.deep.members(
732+
Object.values(expected)
733+
);
745734

746-
// With NaN.
747-
const snapshot4 = await coll.where('zip', notEqualOp, Number.NaN).get();
748-
expected = { ...testDocs };
749-
// @ts-expect-error
750-
delete expected.h;
751-
// @ts-expect-error
752-
delete expected.i;
753-
// @ts-expect-error
754-
delete expected.j;
755-
expect(toDataArray(snapshot4)).to.have.deep.members(
756-
Object.values(expected)
757-
);
758-
});
759-
}
760-
);
735+
// With NaN.
736+
const snapshot4 = await coll.where('zip', notEqualOp, Number.NaN).get();
737+
expected = { ...testDocs };
738+
delete expected.h;
739+
delete expected.i;
740+
delete expected.j;
741+
expect(toDataArray(snapshot4)).to.have.deep.members(
742+
Object.values(expected)
743+
);
744+
});
745+
});
761746

762-
// eslint-disable-next-line no-restricted-properties
763-
(isRunningAgainstEmulator() ? it : it.skip)(
764-
'can use != filters by document ID',
765-
async () => {
766-
const testDocs = {
767-
aa: { key: 'aa' },
768-
ab: { key: 'ab' },
769-
ba: { key: 'ba' },
770-
bb: { key: 'bb' }
771-
};
772-
await withTestCollection(persistence, testDocs, async coll => {
773-
const snapshot = await coll
774-
.where(FieldPath.documentId(), notEqualOp, 'aa')
775-
.get();
776-
777-
expect(toDataArray(snapshot)).to.deep.equal([
778-
{ key: 'ab' },
779-
{ key: 'ba' },
780-
{ key: 'bb' }
781-
]);
782-
});
783-
}
784-
);
747+
it('can use != filters by document ID', async () => {
748+
const testDocs = {
749+
aa: { key: 'aa' },
750+
ab: { key: 'ab' },
751+
ba: { key: 'ba' },
752+
bb: { key: 'bb' }
753+
};
754+
await withTestCollection(persistence, testDocs, async coll => {
755+
const snapshot = await coll
756+
.where(FieldPath.documentId(), notEqualOp, 'aa')
757+
.get();
758+
759+
expect(toDataArray(snapshot)).to.deep.equal([
760+
{ key: 'ab' },
761+
{ key: 'ba' },
762+
{ key: 'bb' }
763+
]);
764+
});
765+
});
785766

786767
it('can use array-contains filters', async () => {
787768
const testDocs = {
@@ -851,100 +832,82 @@ apiDescribe('Queries', (persistence: boolean) => {
851832
});
852833
});
853834

835+
// TODO(ne-queries): re-enable in next PR to make public.
854836
// eslint-disable-next-line no-restricted-properties
855-
(isRunningAgainstEmulator() ? it : it.skip)(
856-
'can use NOT_IN filters',
857-
async () => {
858-
const testDocs = {
859-
a: { zip: 98101 },
860-
b: { zip: 91102 },
861-
c: { zip: 98103 },
862-
d: { zip: [98101] },
863-
e: { zip: ['98101', { zip: 98101 }] },
864-
f: { zip: { code: 500 } },
865-
g: { zip: [98101, 98102] },
866-
h: { code: 500 },
867-
i: { zip: null },
868-
j: { zip: Number.NaN }
869-
};
837+
it.skip('can use NOT_IN filters', async () => {
838+
const testDocs = {
839+
a: { zip: 98101 },
840+
b: { zip: 91102 },
841+
c: { zip: 98103 },
842+
d: { zip: [98101] },
843+
e: { zip: ['98101', { zip: 98101 }] },
844+
f: { zip: { code: 500 } },
845+
g: { zip: [98101, 98102] },
846+
h: { code: 500 },
847+
i: { zip: null },
848+
j: { zip: Number.NaN }
849+
};
870850

871-
await withTestCollection(persistence, testDocs, async coll => {
872-
let expected = { ...testDocs };
873-
// @ts-expect-error
874-
delete expected.a;
875-
// @ts-expect-error
876-
delete expected.c;
877-
// @ts-expect-error
878-
delete expected.g;
879-
// @ts-expect-error
880-
delete expected.h;
881-
const snapshot = await coll
882-
.where('zip', notInOp, [98101, 98103, [98101, 98102]])
883-
.get();
884-
expect(toDataArray(snapshot)).to.deep.equal(Object.values(expected));
885-
886-
// With objects.
887-
const snapshot2 = await coll
888-
.where('zip', notInOp, [{ code: 500 }])
889-
.get();
890-
expected = { ...testDocs };
891-
// @ts-expect-error
892-
delete expected.f;
893-
// @ts-expect-error
894-
delete expected.h;
895-
expect(toDataArray(snapshot2)).to.deep.equal(Object.values(expected));
896-
897-
// With null.
898-
const snapshot3 = await coll.where('zip', notInOp, [null]).get();
899-
expect(toDataArray(snapshot3)).to.deep.equal([]);
900-
901-
// With NaN.
902-
const snapshot4 = await coll.where('zip', notInOp, [Number.NaN]).get();
903-
expected = { ...testDocs };
904-
// @ts-expect-error
905-
delete expected.h;
906-
// @ts-expect-error
907-
delete expected.j;
908-
expect(toDataArray(snapshot4)).to.deep.equal(Object.values(expected));
909-
910-
// With NaN and a number.
911-
const snapshot5 = await coll
912-
.where('zip', notInOp, [Number.NaN, 98101])
913-
.get();
914-
expected = { ...testDocs };
915-
// @ts-expect-error
916-
delete expected.a;
917-
// @ts-expect-error
918-
delete expected.h;
919-
// @ts-expect-error
920-
delete expected.j;
921-
expect(toDataArray(snapshot5)).to.deep.equal(Object.values(expected));
922-
});
923-
}
924-
);
851+
await withTestCollection(persistence, testDocs, async coll => {
852+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
853+
let expected: { [name: string]: any } = { ...testDocs };
854+
delete expected.a;
855+
delete expected.c;
856+
delete expected.g;
857+
delete expected.h;
858+
const snapshot = await coll
859+
.where('zip', notInOp, [98101, 98103, [98101, 98102]])
860+
.get();
861+
expect(toDataArray(snapshot)).to.deep.equal(Object.values(expected));
925862

926-
// eslint-disable-next-line no-restricted-properties
927-
(isRunningAgainstEmulator() ? it : it.skip)(
928-
'can use NOT_IN filters by document ID',
929-
async () => {
930-
const testDocs = {
931-
aa: { key: 'aa' },
932-
ab: { key: 'ab' },
933-
ba: { key: 'ba' },
934-
bb: { key: 'bb' }
935-
};
936-
await withTestCollection(persistence, testDocs, async coll => {
937-
const snapshot = await coll
938-
.where(FieldPath.documentId(), notInOp, ['aa', 'ab'])
939-
.get();
940-
941-
expect(toDataArray(snapshot)).to.deep.equal([
942-
{ key: 'ba' },
943-
{ key: 'bb' }
944-
]);
945-
});
946-
}
947-
);
863+
// With objects.
864+
const snapshot2 = await coll.where('zip', notInOp, [{ code: 500 }]).get();
865+
expected = { ...testDocs };
866+
delete expected.f;
867+
delete expected.h;
868+
expect(toDataArray(snapshot2)).to.deep.equal(Object.values(expected));
869+
870+
// With null.
871+
const snapshot3 = await coll.where('zip', notInOp, [null]).get();
872+
expect(toDataArray(snapshot3)).to.deep.equal([]);
873+
874+
// With NaN.
875+
const snapshot4 = await coll.where('zip', notInOp, [Number.NaN]).get();
876+
expected = { ...testDocs };
877+
delete expected.h;
878+
delete expected.j;
879+
expect(toDataArray(snapshot4)).to.deep.equal(Object.values(expected));
880+
881+
// With NaN and a number.
882+
const snapshot5 = await coll
883+
.where('zip', notInOp, [Number.NaN, 98101])
884+
.get();
885+
expected = { ...testDocs };
886+
delete expected.a;
887+
delete expected.h;
888+
delete expected.j;
889+
expect(toDataArray(snapshot5)).to.deep.equal(Object.values(expected));
890+
});
891+
});
892+
893+
it('can use NOT_IN filters by document ID', async () => {
894+
const testDocs = {
895+
aa: { key: 'aa' },
896+
ab: { key: 'ab' },
897+
ba: { key: 'ba' },
898+
bb: { key: 'bb' }
899+
};
900+
await withTestCollection(persistence, testDocs, async coll => {
901+
const snapshot = await coll
902+
.where(FieldPath.documentId(), notInOp, ['aa', 'ab'])
903+
.get();
904+
905+
expect(toDataArray(snapshot)).to.deep.equal([
906+
{ key: 'ba' },
907+
{ key: 'bb' }
908+
]);
909+
});
910+
});
948911

949912
it('can use array-contains-any filters', async () => {
950913
const testDocs = {

‎packages/firestore/test/integration/util/helpers.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ import * as firestore from '@firebase/firestore-types';
1919
import {
2020
ALT_PROJECT_ID,
2121
DEFAULT_PROJECT_ID,
22-
DEFAULT_SETTINGS,
23-
USE_EMULATOR
22+
DEFAULT_SETTINGS
2423
} from './settings';
2524
import * as firebaseExport from './firebase_export';
2625

@@ -287,7 +286,3 @@ export const notEqualOp = '!=' as firestore.WhereFilterOp;
287286
// repeatedly. Once we expose 'not-in' publicly we can remove it and just use 'in'
288287
// in all the tests.
289288
export const notInOp = 'not-in' as firestore.WhereFilterOp;
290-
291-
export function isRunningAgainstEmulator(): boolean {
292-
return USE_EMULATOR;
293-
}

0 commit comments

Comments
 (0)