diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index c83e43fa983..60fa47e270c 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -1436,7 +1436,7 @@ export function newQueryFilter( `Invalid Query. You can't perform '${op}' ` + 'queries on FieldPath.documentId().' ); - } else if (op === Operator.IN) { + } else if (op === Operator.IN || op === Operator.NOT_IN) { validateDisjunctiveFilterElements(value, op); const referenceList: api.Value[] = []; for (const arrayValue of value as api.Value[]) { @@ -1447,14 +1447,18 @@ export function newQueryFilter( fieldValue = parseDocumentIdValue(databaseId, query, value); } } else { - if (op === Operator.IN || op === Operator.ARRAY_CONTAINS_ANY) { + if ( + op === Operator.IN || + op === Operator.NOT_IN || + op === Operator.ARRAY_CONTAINS_ANY + ) { validateDisjunctiveFilterElements(value, op); } fieldValue = parseQueryValue( dataReader, methodName, value, - op === Operator.IN + /* allowArrays= */ op === Operator.IN || op === Operator.NOT_IN ); } const filter = FieldFilter.create(fieldPath, op, fieldValue); @@ -1663,7 +1667,7 @@ function parseDocumentIdValue( } /** - * Validates that the value passed into a disjunctrive filter satisfies all + * Validates that the value passed into a disjunctive filter satisfies all * array requirements. */ function validateDisjunctiveFilterElements( @@ -1684,30 +1688,71 @@ function validateDisjunctiveFilterElements( 'maximum of 10 elements in the value array.' ); } - if (value.indexOf(null) >= 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid Query. '${operator.toString()}' filters cannot contain 'null' ` + - 'in the value array.' - ); + if (operator === Operator.IN || operator === Operator.ARRAY_CONTAINS_ANY) { + if (value.indexOf(null) >= 0) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid Query. '${operator.toString()}' filters cannot contain 'null' ` + + 'in the value array.' + ); + } + if (value.filter(element => Number.isNaN(element)).length > 0) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid Query. '${operator.toString()}' filters cannot contain 'NaN' ` + + 'in the value array.' + ); + } } - if (value.filter(element => Number.isNaN(element)).length > 0) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid Query. '${operator.toString()}' filters cannot contain 'NaN' ` + - 'in the value array.' - ); +} + +/** + * Given an operator, returns the set of operators that cannot be used with it. + * + * Operators in a query must adhere to the following set of rules: + * 1. Only one array operator is allowed. + * 2. Only one disjunctive operator is allowed. + * 3. NOT_EQUAL cannot be used with another NOT_EQUAL operator. + * 4. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators. + * + * Array operators: ARRAY_CONTAINS, ARRAY_CONTAINS_ANY + * Disjunctive operators: IN, ARRAY_CONTAINS_ANY, NOT_IN + */ +function conflictingOps(op: Operator): Operator[] { + switch (op) { + case Operator.NOT_EQUAL: + return [Operator.NOT_EQUAL, Operator.NOT_IN]; + case Operator.ARRAY_CONTAINS: + return [ + Operator.ARRAY_CONTAINS, + Operator.ARRAY_CONTAINS_ANY, + Operator.NOT_IN + ]; + case Operator.IN: + return [Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN]; + case Operator.ARRAY_CONTAINS_ANY: + return [ + Operator.ARRAY_CONTAINS, + Operator.ARRAY_CONTAINS_ANY, + Operator.IN, + Operator.NOT_IN + ]; + case Operator.NOT_IN: + return [ + Operator.ARRAY_CONTAINS, + Operator.ARRAY_CONTAINS_ANY, + Operator.IN, + Operator.NOT_IN, + Operator.NOT_EQUAL + ]; + default: + return []; } } function validateNewFilter(query: InternalQuery, filter: Filter): void { debugAssert(filter instanceof FieldFilter, 'Only FieldFilters are supported'); - const arrayOps = [Operator.ARRAY_CONTAINS, Operator.ARRAY_CONTAINS_ANY]; - const disjunctiveOps = [Operator.IN, Operator.ARRAY_CONTAINS_ANY]; - const isArrayOp = arrayOps.indexOf(filter.op) >= 0; - const isDisjunctiveOp = disjunctiveOps.indexOf(filter.op) >= 0; - if (filter.isInequality()) { const existingField = query.getInequalityFilterField(); if (existingField !== null && !existingField.isEqual(filter.field)) { @@ -1724,31 +1769,23 @@ function validateNewFilter(query: InternalQuery, filter: Filter): void { if (firstOrderByField !== null) { validateOrderByAndInequalityMatch(query, filter.field, firstOrderByField); } - } else if (isDisjunctiveOp || isArrayOp) { - // You can have at most 1 disjunctive filter and 1 array filter. Check if - // the new filter conflicts with an existing one. - let conflictingOp: Operator | null = null; - if (isDisjunctiveOp) { - conflictingOp = query.findFilterOperator(disjunctiveOps); - } - if (conflictingOp === null && isArrayOp) { - conflictingOp = query.findFilterOperator(arrayOps); - } - if (conflictingOp !== null) { - // We special case when it's a duplicate op to give a slightly clearer error message. - if (conflictingOp === filter.op) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` - ); - } else { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. You cannot use '${filter.op.toString()}' filters ` + - `with '${conflictingOp.toString()}' filters.` - ); - } + } + + const conflictingOp = query.findFilterOperator(conflictingOps(filter.op)); + if (conflictingOp !== null) { + // Special case when it's a duplicate op to give a slightly clearer error message. + if (conflictingOp === filter.op) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You cannot use more than one ' + + `'${filter.op.toString()}' filter.` + ); + } else { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Invalid query. You cannot use '${filter.op.toString()}' filters ` + + `with '${conflictingOp.toString()}' filters.` + ); } } } @@ -1806,18 +1843,25 @@ export class Query implements firestore.Query { validateExactNumberOfArgs('Query.where', arguments, 3); validateDefined('Query.where', 3, value); - // Enumerated from the WhereFilterOp type in index.d.ts. - const whereFilterOpEnums = [ - Operator.LESS_THAN, - Operator.LESS_THAN_OR_EQUAL, - Operator.EQUAL, - Operator.GREATER_THAN_OR_EQUAL, - Operator.GREATER_THAN, - Operator.ARRAY_CONTAINS, - Operator.IN, - Operator.ARRAY_CONTAINS_ANY - ]; - const op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr); + // TODO(ne-queries): Add 'not-in' and '!=' to validation. + let op: Operator; + if ((opStr as unknown) === 'not-in' || (opStr as unknown) === '!=') { + op = opStr as Operator; + } else { + // Enumerated from the WhereFilterOp type in index.d.ts. + const whereFilterOpEnums = [ + Operator.LESS_THAN, + Operator.LESS_THAN_OR_EQUAL, + Operator.EQUAL, + Operator.GREATER_THAN_OR_EQUAL, + Operator.GREATER_THAN, + Operator.ARRAY_CONTAINS, + Operator.IN, + Operator.ARRAY_CONTAINS_ANY + ]; + op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr); + } + const fieldPath = fieldPathFromArgument('Query.where', field); const filter = newQueryFilter( this._query, diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 8ed8c2503bb..44455d954aa 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -566,10 +566,12 @@ export const enum Operator { LESS_THAN = '<', LESS_THAN_OR_EQUAL = '<=', EQUAL = '==', + NOT_EQUAL = '!=', GREATER_THAN = '>', GREATER_THAN_OR_EQUAL = '>=', ARRAY_CONTAINS = 'array-contains', IN = 'in', + NOT_IN = 'not-in', ARRAY_CONTAINS_ANY = 'array-contains-any' } @@ -587,16 +589,8 @@ export class FieldFilter extends Filter { */ static create(field: FieldPath, op: Operator, value: api.Value): FieldFilter { if (field.isKeyField()) { - if (op === Operator.IN) { - debugAssert( - isArray(value), - 'Comparing on key with IN, but filter value not an ArrayValue' - ); - debugAssert( - (value.arrayValue.values || []).every(elem => isReferenceValue(elem)), - 'Comparing on key with IN, but an array value was not a RefValue' - ); - return new KeyFieldInFilter(field, value); + if (op === Operator.IN || op === Operator.NOT_IN) { + return this.createKeyFieldInFilter(field, op, value); } else { debugAssert( isReferenceValue(value), @@ -609,7 +603,8 @@ export class FieldFilter extends Filter { return new KeyFieldFilter(field, op, value); } } else if (isNullValue(value)) { - if (op !== Operator.EQUAL) { + if (op !== Operator.EQUAL && op !== Operator.NOT_EQUAL) { + // TODO(ne-queries): Update error message to include != comparison. throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. Null supports only equality comparisons.' @@ -617,7 +612,8 @@ export class FieldFilter extends Filter { } return new FieldFilter(field, op, value); } else if (isNanValue(value)) { - if (op !== Operator.EQUAL) { + if (op !== Operator.EQUAL && op !== Operator.NOT_EQUAL) { + // TODO(ne-queries): Update error message to include != comparison. throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. NaN supports only equality comparisons.' @@ -632,6 +628,12 @@ export class FieldFilter extends Filter { 'IN filter has invalid value: ' + value.toString() ); return new InFilter(field, value); + } else if (op === Operator.NOT_IN) { + debugAssert( + isArray(value), + 'NOT_IN filter has invalid value: ' + value.toString() + ); + return new NotInFilter(field, value); } else if (op === Operator.ARRAY_CONTAINS_ANY) { debugAssert( isArray(value), @@ -643,8 +645,36 @@ export class FieldFilter extends Filter { } } + private static createKeyFieldInFilter( + field: FieldPath, + op: Operator.IN | Operator.NOT_IN, + value: api.Value + ): FieldFilter { + debugAssert( + isArray(value), + `Comparing on key with ${op.toString()}` + + ', but filter value not an ArrayValue' + ); + debugAssert( + (value.arrayValue.values || []).every(elem => isReferenceValue(elem)), + `Comparing on key with ${op.toString()}` + + ', but an array value was not a RefValue' + ); + + return op === Operator.IN + ? new KeyFieldInFilter(field, value) + : new KeyFieldNotInFilter(field, value); + } + matches(doc: Document): boolean { const other = doc.field(this.field); + // Types do not have to match in NOT_EQUAL filters. + if (this.op === Operator.NOT_EQUAL) { + return ( + other !== null && + this.matchesComparison(valueCompare(other!, this.value)) + ); + } // Only compare types with matching backend order (such as double and int). return ( @@ -662,6 +692,8 @@ export class FieldFilter extends Filter { return comparison <= 0; case Operator.EQUAL: return comparison === 0; + case Operator.NOT_EQUAL: + return comparison !== 0; case Operator.GREATER_THAN: return comparison > 0; case Operator.GREATER_THAN_OR_EQUAL: @@ -677,7 +709,8 @@ export class FieldFilter extends Filter { Operator.LESS_THAN, Operator.LESS_THAN_OR_EQUAL, Operator.GREATER_THAN, - Operator.GREATER_THAN_OR_EQUAL + Operator.GREATER_THAN_OR_EQUAL, + Operator.NOT_EQUAL ].indexOf(this.op) >= 0 ); } @@ -747,14 +780,7 @@ export class KeyFieldInFilter extends FieldFilter { constructor(field: FieldPath, value: api.Value) { super(field, Operator.IN, value); - debugAssert(isArray(value), 'KeyFieldInFilter expects an ArrayValue'); - this.keys = (value.arrayValue.values || []).map(v => { - debugAssert( - isReferenceValue(v), - 'Comparing on key with IN, but an array value was not a ReferenceValue' - ); - return DocumentKey.fromName(v.referenceValue); - }); + this.keys = extractDocumentKeysFromArrayValue(Operator.IN, value); } matches(doc: Document): boolean { @@ -762,6 +788,38 @@ export class KeyFieldInFilter extends FieldFilter { } } +/** Filter that matches on key fields not present within an array. */ +export class KeyFieldNotInFilter extends FieldFilter { + private readonly keys: DocumentKey[]; + + constructor(field: FieldPath, value: api.Value) { + super(field, Operator.NOT_IN, value); + this.keys = extractDocumentKeysFromArrayValue(Operator.NOT_IN, value); + } + + matches(doc: Document): boolean { + return !this.keys.some(key => key.isEqual(doc.key)); + } +} + +function extractDocumentKeysFromArrayValue( + op: Operator.IN | Operator.NOT_IN, + value: api.Value +): DocumentKey[] { + debugAssert( + isArray(value), + 'KeyFieldInFilter/KeyFieldNotInFilter expects an ArrayValue' + ); + return (value.arrayValue?.values || []).map(v => { + debugAssert( + isReferenceValue(v), + `Comparing on key with ${op.toString()}, but an array value was not ` + + `a ReferenceValue` + ); + return DocumentKey.fromName(v.referenceValue); + }); +} + /** A Filter that implements the array-contains operator. */ export class ArrayContainsFilter extends FieldFilter { constructor(field: FieldPath, value: api.Value) { @@ -787,6 +845,19 @@ export class InFilter extends FieldFilter { } } +/** A Filter that implements the not-in operator. */ +export class NotInFilter extends FieldFilter { + constructor(field: FieldPath, value: api.Value) { + super(field, Operator.NOT_IN, value); + debugAssert(isArray(value), 'NotInFilter expects an ArrayValue'); + } + + matches(doc: Document): boolean { + const other = doc.field(this.field); + return other !== null && !arrayValueContains(this.value.arrayValue!, other); + } +} + /** A Filter that implements the array-contains-any operator. */ export class ArrayContainsAnyFilter extends FieldFilter { constructor(field: FieldPath, value: api.Value) { diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 3b495d840fb..6095e13e760 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -95,8 +95,10 @@ const OPERATORS = (() => { ops[Operator.GREATER_THAN] = 'GREATER_THAN'; ops[Operator.GREATER_THAN_OR_EQUAL] = 'GREATER_THAN_OR_EQUAL'; ops[Operator.EQUAL] = 'EQUAL'; + ops[Operator.NOT_EQUAL] = 'NOT_EQUAL'; ops[Operator.ARRAY_CONTAINS] = 'ARRAY_CONTAINS'; ops[Operator.IN] = 'IN'; + ops[Operator.NOT_IN] = 'NOT_IN'; ops[Operator.ARRAY_CONTAINS_ANY] = 'ARRAY_CONTAINS_ANY'; return ops; })(); @@ -1062,6 +1064,8 @@ export function fromOperatorName(op: api.FieldFilterOp): Operator { switch (op) { case 'EQUAL': return Operator.EQUAL; + case 'NOT_EQUAL': + return Operator.NOT_EQUAL; case 'GREATER_THAN': return Operator.GREATER_THAN; case 'GREATER_THAN_OR_EQUAL': @@ -1074,6 +1078,8 @@ export function fromOperatorName(op: api.FieldFilterOp): Operator { return Operator.ARRAY_CONTAINS; case 'IN': return Operator.IN; + case 'NOT_IN': + return Operator.NOT_IN; case 'ARRAY_CONTAINS_ANY': return Operator.ARRAY_CONTAINS_ANY; case 'OPERATOR_UNSPECIFIED': @@ -1134,6 +1140,22 @@ export function toUnaryOrFieldFilter(filter: FieldFilter): api.Filter { } }; } + } else if (filter.op === Operator.NOT_EQUAL) { + if (isNanValue(filter.value)) { + return { + unaryFilter: { + field: toFieldPathReference(filter.field), + op: 'IS_NOT_NAN' + } + }; + } else if (isNullValue(filter.value)) { + return { + unaryFilter: { + field: toFieldPathReference(filter.field), + op: 'IS_NOT_NULL' + } + }; + } } return { fieldFilter: { @@ -1156,6 +1178,16 @@ export function fromUnaryFilter(filter: api.Filter): Filter { return FieldFilter.create(nullField, Operator.EQUAL, { nullValue: 'NULL_VALUE' }); + case 'IS_NOT_NAN': + const notNanField = fromFieldPathReference(filter.unaryFilter!.field!); + return FieldFilter.create(notNanField, Operator.NOT_EQUAL, { + doubleValue: NaN + }); + case 'IS_NOT_NULL': + const notNullField = fromFieldPathReference(filter.unaryFilter!.field!); + return FieldFilter.create(notNullField, Operator.NOT_EQUAL, { + nullValue: 'NULL_VALUE' + }); case 'OPERATOR_UNSPECIFIED': return fail('Unspecified filter'); default: diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 2ba3f68d029..45920fde00a 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -25,6 +25,8 @@ import { EventsAccumulator } from '../util/events_accumulator'; import * as firebaseExport from '../util/firebase_export'; import { apiDescribe, + notEqualOp, + notInOp, withTestCollection, withTestDb, withTestDbs, @@ -642,6 +644,14 @@ apiDescribe('Database', (persistence: boolean) => { }); }); + it('inequality and NOT_IN on different fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + coll.where('x', '>=', 32).where('y', notInOp, [1, 2]) + ).not.to.throw(); + }); + }); + it('inequality and array-contains-any on different fields works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => @@ -657,6 +667,17 @@ apiDescribe('Database', (persistence: boolean) => { }); }); + it('!= same as orderBy works.', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + coll.where('x', notEqualOp, 32).orderBy('x') + ).not.to.throw(); + expect(() => + coll.orderBy('x').where('x', notEqualOp, 32) + ).not.to.throw(); + }); + }); + it('inequality same as first orderBy works.', () => { return withTestCollection(persistence, {}, async coll => { expect(() => @@ -668,6 +689,17 @@ apiDescribe('Database', (persistence: boolean) => { }); }); + it('!= same as first orderBy works.', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + coll.where('x', notEqualOp, 32).orderBy('x').orderBy('y') + ).not.to.throw(); + expect(() => + coll.orderBy('x').where('x', notEqualOp, 32).orderBy('y') + ).not.to.throw(); + }); + }); + it('equality different than orderBy works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => coll.orderBy('x').where('y', '==', 'cat')).not.to.throw(); @@ -688,6 +720,14 @@ apiDescribe('Database', (persistence: boolean) => { }); }); + it('NOT_IN different than orderBy works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + coll.orderBy('x').where('y', notInOp, [1, 2]) + ).not.to.throw(); + }); + }); + it('array-contains-any different than orderBy works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 6ea00e54ae2..70087b4bad0 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -24,6 +24,9 @@ import { EventsAccumulator } from '../util/events_accumulator'; import * as firebaseExport from '../util/firebase_export'; import { apiDescribe, + isRunningAgainstEmulator, + notEqualOp, + notInOp, toChangesArray, toDataArray, withTestCollection, @@ -684,6 +687,67 @@ apiDescribe('Queries', (persistence: boolean) => { }); }); + // eslint-disable-next-line no-restricted-properties + (isRunningAgainstEmulator() ? it : it.skip)( + 'can use != filters', + async () => { + const testDocs = { + a: { zip: 98101 }, + b: { zip: 91102 }, + c: { zip: '98101' }, + d: { zip: [98101] }, + e: { zip: ['98101', { zip: 98101 }] }, + f: { zip: { code: 500 } }, + g: { zip: [98101, 98102] }, + h: { code: 500 }, + i: { zip: null }, + j: { zip: Number.NaN } + }; + + await withTestCollection(persistence, testDocs, async coll => { + let expected = { ...testDocs }; + delete expected.a; + delete expected.h; + delete expected.i; + const snapshot = await coll.where('zip', notEqualOp, 98101).get(); + expect(toDataArray(snapshot)).to.have.deep.members( + Object.values(expected) + ); + + // With objects. + const snapshot2 = await coll + .where('zip', notEqualOp, { code: 500 }) + .get(); + expected = { ...testDocs }; + delete expected.f; + delete expected.h; + delete expected.i; + expect(toDataArray(snapshot2)).to.have.deep.members( + Object.values(expected) + ); + + // With null. + const snapshot3 = await coll.where('zip', notEqualOp, null).get(); + expected = { ...testDocs }; + delete expected.h; + delete expected.i; + expect(toDataArray(snapshot3)).to.have.deep.members( + Object.values(expected) + ); + + // With NaN. + const snapshot4 = await coll.where('zip', notEqualOp, Number.NaN).get(); + expected = { ...testDocs }; + delete expected.h; + delete expected.i; + delete expected.j; + expect(toDataArray(snapshot4)).to.have.deep.members( + Object.values(expected) + ); + }); + } + ); + it('can use array-contains filters', async () => { const testDocs = { a: { array: [42] }, @@ -752,6 +816,90 @@ apiDescribe('Queries', (persistence: boolean) => { }); }); + // eslint-disable-next-line no-restricted-properties + (isRunningAgainstEmulator() ? it : it.skip)( + 'can use NOT_IN filters', + async () => { + const testDocs = { + a: { zip: 98101 }, + b: { zip: 91102 }, + c: { zip: 98103 }, + d: { zip: [98101] }, + e: { zip: ['98101', { zip: 98101 }] }, + f: { zip: { code: 500 } }, + g: { zip: [98101, 98102] }, + h: { code: 500 }, + i: { zip: null }, + j: { zip: Number.NaN } + }; + + await withTestCollection(persistence, testDocs, async coll => { + let expected = { ...testDocs }; + delete expected.a; + delete expected.c; + delete expected.g; + delete expected.h; + const snapshot = await coll + .where('zip', notInOp, [98101, 98103, [98101, 98102]]) + .get(); + expect(toDataArray(snapshot)).to.deep.equal(Object.values(expected)); + + // With objects. + const snapshot2 = await coll + .where('zip', notInOp, [{ code: 500 }]) + .get(); + expected = { ...testDocs }; + delete expected.f; + delete expected.h; + expect(toDataArray(snapshot2)).to.deep.equal(Object.values(expected)); + + // With null. + const snapshot3 = await coll.where('zip', notInOp, [null]).get(); + expect(toDataArray(snapshot3)).to.deep.equal([]); + + // With NaN. + const snapshot4 = await coll.where('zip', notInOp, [Number.NaN]).get(); + expected = { ...testDocs }; + delete expected.h; + delete expected.j; + expect(toDataArray(snapshot4)).to.deep.equal(Object.values(expected)); + + // With NaN and a number. + const snapshot5 = await coll + .where('zip', notInOp, [Number.NaN, 98101]) + .get(); + expected = { ...testDocs }; + delete expected.a; + delete expected.h; + delete expected.j; + expect(toDataArray(snapshot5)).to.deep.equal(Object.values(expected)); + }); + } + ); + + // eslint-disable-next-line no-restricted-properties + (isRunningAgainstEmulator() ? it : it.skip)( + 'can use NOT_IN filters by document ID', + async () => { + const testDocs = { + aa: { key: 'aa' }, + ab: { key: 'ab' }, + ba: { key: 'ba' }, + bb: { key: 'bb' } + }; + await withTestCollection(persistence, testDocs, async coll => { + const snapshot = await coll + .where(FieldPath.documentId(), notInOp, ['aa', 'ab']) + .get(); + + expect(toDataArray(snapshot)).to.deep.equal([ + { key: 'ba' }, + { key: 'bb' } + ]); + }); + } + ); + it('can use array-contains-any filters', async () => { const testDocs = { a: { array: [42] }, diff --git a/packages/firestore/test/integration/api/validation.test.ts b/packages/firestore/test/integration/api/validation.test.ts index ab2c93d534c..e192fff49d0 100644 --- a/packages/firestore/test/integration/api/validation.test.ts +++ b/packages/firestore/test/integration/api/validation.test.ts @@ -24,7 +24,9 @@ import { apiDescribe, withAlternateTestDb, withTestCollection, - withTestDb + withTestDb, + notInOp, + notEqualOp } from '../util/helpers'; import { ALT_PROJECT_ID, DEFAULT_PROJECT_ID } from '../util/settings'; @@ -955,6 +957,9 @@ apiDescribe('Validation:', (persistence: boolean) => { expect(() => collection.where('a', 'in', null)).to.throw( "Invalid Query. A non-empty array is required for 'in' filters." ); + expect(() => collection.where('a', notInOp, null)).to.throw( + "Invalid Query. A non-empty array is required for 'not-in' filters." + ); expect(() => collection.where('a', 'array-contains-any', null) ).to.throw( @@ -970,6 +975,9 @@ apiDescribe('Validation:', (persistence: boolean) => { expect(() => collection.where('a', 'in', Number.NaN)).to.throw( "Invalid Query. A non-empty array is required for 'in' filters." ); + expect(() => collection.where('a', notInOp, Number.NaN)).to.throw( + "Invalid Query. A non-empty array is required for 'not-in' filters." + ); expect(() => collection.where('a', 'array-contains-any', Number.NaN) ).to.throw( @@ -1117,6 +1125,28 @@ apiDescribe('Validation:', (persistence: boolean) => { ); }); + validationIt(persistence, 'with more than one != query fail', db => { + const collection = db.collection('test'); + expect(() => + collection.where('x', notEqualOp, 32).where('x', notEqualOp, 33) + ).to.throw("Invalid query. You cannot use more than one '!=' filter."); + }); + + validationIt( + persistence, + 'with != and inequality queries on different fields fail', + db => { + const collection = db.collection('test'); + expect(() => + collection.where('y', '>', 32).where('x', notEqualOp, 33) + ).to.throw( + 'Invalid query. All where filters with an ' + + 'inequality (<, <=, >, or >=) must be on the same field.' + + ` But you have inequality filters on 'y' and 'x` + ); + } + ); + validationIt( persistence, 'with inequality different than first orderBy fail.', @@ -1139,6 +1169,9 @@ apiDescribe('Validation:', (persistence: boolean) => { expect(() => collection.orderBy('y').orderBy('x').where('x', '>', 32) ).to.throw(reason); + expect(() => + collection.where('x', notEqualOp, 32).orderBy('y') + ).to.throw(reason); } ); @@ -1171,6 +1204,36 @@ apiDescribe('Validation:', (persistence: boolean) => { "Invalid query. You cannot use 'array-contains' filters with " + "'array-contains-any' filters." ); + + expect(() => + db + .collection('test') + .where('foo', notInOp, [2, 3]) + .where('foo', 'array-contains', 1) + ).to.throw( + "Invalid query. You cannot use 'array-contains' filters with " + + "'not-in' filters." + ); + }); + + validationIt(persistence, 'with != and not-in filters fail', db => { + expect(() => + db + .collection('test') + .where('foo', notInOp, [2, 3]) + .where('foo', notEqualOp, 4) + ).to.throw( + "Invalid query. You cannot use '!=' filters with 'not-in' filters." + ); + + expect(() => + db + .collection('test') + .where('foo', notEqualOp, 4) + .where('foo', notInOp, [2, 3]) + ).to.throw( + "Invalid query. You cannot use 'not-in' filters with '!=' filters." + ); }); validationIt(persistence, 'with multiple disjunctive filters fail', db => { @@ -1181,6 +1244,15 @@ apiDescribe('Validation:', (persistence: boolean) => { .where('foo', 'in', [2, 3]) ).to.throw("Invalid query. You cannot use more than one 'in' filter."); + expect(() => + db + .collection('test') + .where('foo', notInOp, [1, 2]) + .where('foo', notInOp, [2, 3]) + ).to.throw( + "Invalid query. You cannot use more than one 'not-in' filter." + ); + expect(() => db .collection('test') @@ -1211,6 +1283,44 @@ apiDescribe('Validation:', (persistence: boolean) => { "'in' filters." ); + expect(() => + db + .collection('test') + .where('foo', notInOp, [2, 3]) + .where('foo', 'array-contains-any', [2, 3]) + ).to.throw( + "Invalid query. You cannot use 'array-contains-any' filters with " + + "'not-in' filters." + ); + + expect(() => + db + .collection('test') + .where('foo', 'array-contains-any', [2, 3]) + .where('foo', notInOp, [2, 3]) + ).to.throw( + "Invalid query. You cannot use 'not-in' filters with " + + "'array-contains-any' filters." + ); + + expect(() => + db + .collection('test') + .where('foo', notInOp, [2, 3]) + .where('foo', 'in', [2, 3]) + ).to.throw( + "Invalid query. You cannot use 'in' filters with 'not-in' filters." + ); + + expect(() => + db + .collection('test') + .where('foo', 'in', [2, 3]) + .where('foo', notInOp, [2, 3]) + ).to.throw( + "Invalid query. You cannot use 'not-in' filters with 'in' filters." + ); + // This is redundant with the above tests, but makes sure our validation // doesn't get confused. expect(() => @@ -1220,8 +1330,7 @@ apiDescribe('Validation:', (persistence: boolean) => { .where('foo', 'array-contains', 1) .where('foo', 'array-contains-any', [2]) ).to.throw( - "Invalid query. You cannot use 'array-contains-any' filters with " + - "'in' filters." + "Invalid query. You cannot use 'array-contains-any' filters with 'in' filters." ); expect(() => @@ -1232,7 +1341,29 @@ apiDescribe('Validation:', (persistence: boolean) => { .where('foo', 'array-contains-any', [2]) ).to.throw( "Invalid query. You cannot use 'array-contains-any' filters with " + - "'in' filters." + "'array-contains' filters." + ); + + expect(() => + db + .collection('test') + .where('foo', notInOp, [2, 3]) + .where('foo', 'array-contains', 2) + .where('foo', 'array-contains-any', [2]) + ).to.throw( + "Invalid query. You cannot use 'array-contains' filters with " + + "'not-in' filters." + ); + + expect(() => + db + .collection('test') + .where('foo', 'array-contains', 2) + .where('foo', 'in', [2]) + .where('foo', notInOp, [2, 3]) + ).to.throw( + "Invalid query. You cannot use 'not-in' filters with " + + "'array-contains' filters." ); }); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 16e44ff8a87..abb2e79a3ab 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -19,7 +19,8 @@ import * as firestore from '@firebase/firestore-types'; import { ALT_PROJECT_ID, DEFAULT_PROJECT_ID, - DEFAULT_SETTINGS + DEFAULT_SETTINGS, + USE_EMULATOR } from './settings'; import * as firebaseExport from './firebase_export'; @@ -276,3 +277,17 @@ export function withTestCollectionSettings( } ); } + +// TODO(ne-queries): This exists just so we don't have to do the cast +// repeatedly. Once we expose '!=' publicly we can remove it and just use '!=' +// in all the tests. +export const notEqualOp = '!=' as firestore.WhereFilterOp; + +// TODO(ne-queries): This exists just so we don't have to do the cast +// repeatedly. Once we expose 'not-in' publicly we can remove it and just use 'in' +// in all the tests. +export const notInOp = 'not-in' as firestore.WhereFilterOp; + +export function isRunningAgainstEmulator(): boolean { + return USE_EMULATOR; +} diff --git a/packages/firestore/test/unit/core/query.test.ts b/packages/firestore/test/unit/core/query.test.ts index 0815c56a9cd..3c48d167eb3 100644 --- a/packages/firestore/test/unit/core/query.test.ts +++ b/packages/firestore/test/unit/core/query.test.ts @@ -218,6 +218,64 @@ describe('Query', () => { expect(queryMatches(query1, document)).to.be.true; }); + it('matches not-in filters', () => { + const query1 = query('collection', filter('zip', 'not-in', [12345])); + + // No match. + let document = doc('collection/1', 0, { zip: 23456 }); + expect(queryMatches(query1, document)).to.be.true; + + // Value matches in array. + document = doc('collection/1', 0, { zip: [12345] }); + expect(queryMatches(query1, document)).to.be.true; + + // Non-type match. + document = doc('collection/1', 0, { zip: '12345' }); + expect(queryMatches(query1, document)).to.be.true; + + // Nested match. + document = doc('collection/1', 0, { + zip: [123, '12345', { zip: 12345, b: [42] }] + }); + expect(queryMatches(query1, document)).to.be.true; + + // Null match. + document = doc('collection/1', 0, { + zip: null + }); + expect(queryMatches(query1, document)).to.be.true; + + // NaN match. + document = doc('collection/1', 0, { + zip: Number.NaN + }); + expect(queryMatches(query1, document)).to.be.true; + + // Direct match. + document = doc('collection/1', 0, { zip: 12345 }); + expect(queryMatches(query1, document)).to.be.false; + + // Field not set. + document = doc('collection/1', 0, { chip: 23456 }); + expect(queryMatches(query1, document)).to.be.false; + }); + + it('matches not-in filters with object values', () => { + const query1 = query('collection', filter('zip', 'not-in', [{ a: [42] }])); + + // Containing object in array. + let document = doc('collection/1', 0, { + zip: [{ a: 42 }] + }); + expect(queryMatches(query1, document)).to.be.true; + + // Containing object. + document = doc('collection/1', 0, { + zip: { a: [42] } + }); + expect(queryMatches(query1, document)).to.be.false; + }); + it('matches array-contains-any filters', () => { const query1 = query( 'collection', @@ -268,12 +326,22 @@ describe('Query', () => { const doc3 = doc('collection/3', 0, { sort: 3.1 }); const doc4 = doc('collection/4', 0, { sort: false }); const doc5 = doc('collection/5', 0, { sort: 'string' }); + const doc6 = doc('collection/6', 0, { sort: null }); expect(queryMatches(query1, doc1)).to.equal(true); expect(queryMatches(query1, doc2)).to.equal(false); expect(queryMatches(query1, doc3)).to.equal(false); expect(queryMatches(query1, doc4)).to.equal(false); expect(queryMatches(query1, doc5)).to.equal(false); + expect(queryMatches(query1, doc6)).to.equal(false); + + const query2 = query('collection', filter('sort', '!=', NaN)); + expect(queryMatches(query2, doc1)).to.equal(false); + expect(queryMatches(query2, doc2)).to.equal(true); + expect(queryMatches(query2, doc3)).to.equal(true); + expect(queryMatches(query2, doc4)).to.equal(true); + expect(queryMatches(query2, doc5)).to.equal(true); + expect(queryMatches(query2, doc6)).to.equal(true); }); it('matches null for filters', () => { @@ -283,12 +351,22 @@ describe('Query', () => { const doc3 = doc('collection/3', 0, { sort: 3.1 }); const doc4 = doc('collection/4', 0, { sort: false }); const doc5 = doc('collection/5', 0, { sort: 'string' }); + const doc6 = doc('collection/1', 0, { sort: NaN }); expect(queryMatches(query1, doc1)).to.equal(true); expect(queryMatches(query1, doc2)).to.equal(false); expect(queryMatches(query1, doc3)).to.equal(false); expect(queryMatches(query1, doc4)).to.equal(false); expect(queryMatches(query1, doc5)).to.equal(false); + expect(queryMatches(query1, doc6)).to.equal(false); + + const query2 = query('collection', filter('sort', '!=', null)); + expect(queryMatches(query2, doc1)).to.equal(false); + expect(queryMatches(query2, doc2)).to.equal(true); + expect(queryMatches(query2, doc3)).to.equal(true); + expect(queryMatches(query2, doc4)).to.equal(true); + expect(queryMatches(query2, doc5)).to.equal(true); + expect(queryMatches(query2, doc6)).to.equal(true); }); it('matches complex objects for filters', () => { @@ -573,6 +651,10 @@ describe('Query', () => { query('collection', filter('a', '==', [1, 2, 3])), 'collection|f:a==[1,2,3]|ob:__name__asc' ); + assertCanonicalId( + query('collection', filter('a', '!=', [1, 2, 3])), + 'collection|f:a!=[1,2,3]|ob:aasc,__name__asc' + ); assertCanonicalId( query('collection', filter('a', '==', NaN)), 'collection|f:a==NaN|ob:__name__asc' @@ -592,6 +674,10 @@ describe('Query', () => { query('collection', filter('a', 'in', [1, 2, 3])), 'collection|f:ain[1,2,3]|ob:__name__asc' ); + assertCanonicalId( + query('collection', filter('a', 'not-in', [1, 2, 3])), + 'collection|f:anot-in[1,2,3]|ob:__name__asc' + ); assertCanonicalId( query('collection', filter('a', 'array-contains-any', [1, 2, 3])), 'collection|f:aarray-contains-any[1,2,3]|ob:__name__asc' diff --git a/packages/firestore/test/unit/remote/serializer.helper.ts b/packages/firestore/test/unit/remote/serializer.helper.ts index d01fc50178a..6ee2e6bc3f9 100644 --- a/packages/firestore/test/unit/remote/serializer.helper.ts +++ b/packages/firestore/test/unit/remote/serializer.helper.ts @@ -32,6 +32,7 @@ import { InFilter, KeyFieldFilter, LimitType, + NotInFilter, Operator, OrderBy, queryToTarget, @@ -783,6 +784,21 @@ export function serializerTest( expect(roundtripped).to.be.instanceof(FieldFilter); }); + it('converts NotEqual', () => { + const input = filter('field', '!=', 42); + const actual = toUnaryOrFieldFilter(input); + expect(actual).to.deep.equal({ + fieldFilter: { + field: { fieldPath: 'field' }, + op: 'NOT_EQUAL', + value: { integerValue: '42' } + } + }); + const roundtripped = fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(FieldFilter); + }); + it('converts LessThan', () => { const input = filter('field', '<', 42); const actual = toUnaryOrFieldFilter(input); @@ -899,6 +915,29 @@ export function serializerTest( expect(roundtripped).to.be.instanceof(InFilter); }); + it('converts not-in', () => { + const input = filter('field', 'not-in', [42]); + const actual = toUnaryOrFieldFilter(input); + expect(actual).to.deep.equal({ + fieldFilter: { + field: { fieldPath: 'field' }, + op: 'NOT_IN', + value: { + arrayValue: { + values: [ + { + integerValue: '42' + } + ] + } + } + } + }); + const roundtripped = fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(NotInFilter); + }); + it('converts array-contains-any', () => { const input = filter('field', 'array-contains-any', [42]); const actual = toUnaryOrFieldFilter(input); @@ -949,6 +988,30 @@ export function serializerTest( }); expect(fromUnaryFilter(actual)).to.deep.equal(input); }); + + it('converts not null', () => { + const input = filter('field', '!=', null); + const actual = toUnaryOrFieldFilter(input); + expect(actual).to.deep.equal({ + unaryFilter: { + field: { fieldPath: 'field' }, + op: 'IS_NOT_NULL' + } + }); + expect(fromUnaryFilter(actual)).to.deep.equal(input); + }); + + it('converts not NaN', () => { + const input = filter('field', '!=', NaN); + const actual = toUnaryOrFieldFilter(input); + expect(actual).to.deep.equal({ + unaryFilter: { + field: { fieldPath: 'field' }, + op: 'IS_NOT_NAN' + } + }); + expect(fromUnaryFilter(actual)).to.deep.equal(input); + }); }); it('encodes listen request labels', () => { @@ -1321,10 +1384,12 @@ export function serializerTest( Operator.LESS_THAN, Operator.LESS_THAN_OR_EQUAL, Operator.EQUAL, + Operator.NOT_EQUAL, Operator.GREATER_THAN, Operator.GREATER_THAN_OR_EQUAL, Operator.ARRAY_CONTAINS, Operator.IN, + Operator.NOT_IN, Operator.ARRAY_CONTAINS_ANY ];