diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 0dd4458e560..8b2e4410050 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -1515,7 +1515,7 @@ export class Query implements firestore.Query { value ); } - const filter = Filter.create(fieldPath, operator, fieldValue); + const filter = FieldFilter.create(fieldPath, operator, fieldValue); this.validateNewFilter(filter); return new Query(this._query.addFilter(filter), this.firestore); } diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index dfb44a053db..221803bb95f 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -443,57 +443,6 @@ export abstract class Filter { abstract matches(doc: Document): boolean; abstract canonicalId(): string; abstract isEqual(filter: Filter): boolean; - - /** - * Creates a filter based on the provided arguments. - */ - static create(field: FieldPath, op: Operator, value: FieldValue): Filter { - if (field.isKeyField()) { - assert( - value instanceof RefValue, - 'Comparing on key, but filter value not a RefValue' - ); - assert( - op !== Operator.ARRAY_CONTAINS && - op !== Operator.ARRAY_CONTAINS_ANY && - op !== Operator.IN, - `'${op.toString()}' queries don't make sense on document keys.` - ); - return new KeyFieldFilter(field, op, value as RefValue); - } else if (value.isEqual(NullValue.INSTANCE)) { - if (op !== Operator.EQUAL) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You can only perform equals comparisons on null.' - ); - } - return new FieldFilter(field, op, value); - } else if (value.isEqual(DoubleValue.NAN)) { - if (op !== Operator.EQUAL) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. You can only perform equals comparisons on NaN.' - ); - } - return new FieldFilter(field, op, value); - } else if (op === Operator.ARRAY_CONTAINS) { - return new ArrayContainsFilter(field, value); - } else if (op === Operator.IN) { - assert( - value instanceof ArrayValue, - 'IN filter has invalid value: ' + value.toString() - ); - return new InFilter(field, value as ArrayValue); - } else if (op === Operator.ARRAY_CONTAINS_ANY) { - assert( - value instanceof ArrayValue, - 'ARRAY_CONTAINS_ANY filter has invalid value: ' + value.toString() - ); - return new ArrayContainsAnyFilter(field, value as ArrayValue); - } else { - return new FieldFilter(field, op, value); - } - } } export class Operator { @@ -541,7 +490,7 @@ export class Operator { } export class FieldFilter extends Filter { - constructor( + protected constructor( public field: FieldPath, public op: Operator, public value: FieldValue @@ -549,6 +498,61 @@ export class FieldFilter extends Filter { super(); } + /** + * Creates a filter based on the provided arguments. + */ + static create( + field: FieldPath, + op: Operator, + value: FieldValue + ): FieldFilter { + if (field.isKeyField()) { + assert( + value instanceof RefValue, + 'Comparing on key, but filter value not a RefValue' + ); + assert( + op !== Operator.ARRAY_CONTAINS && + op !== Operator.ARRAY_CONTAINS_ANY && + op !== Operator.IN, + `'${op.toString()}' queries don't make sense on document keys.` + ); + return new KeyFieldFilter(field, op, value as RefValue); + } else if (value.isEqual(NullValue.INSTANCE)) { + if (op !== Operator.EQUAL) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You can only perform equals comparisons on null.' + ); + } + return new FieldFilter(field, op, value); + } else if (value.isEqual(DoubleValue.NAN)) { + if (op !== Operator.EQUAL) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + 'Invalid query. You can only perform equals comparisons on NaN.' + ); + } + return new FieldFilter(field, op, value); + } else if (op === Operator.ARRAY_CONTAINS) { + return new ArrayContainsFilter(field, value); + } else if (op === Operator.IN) { + assert( + value instanceof ArrayValue, + 'IN filter has invalid value: ' + value.toString() + ); + return new InFilter(field, value as ArrayValue); + } else if (op === Operator.ARRAY_CONTAINS_ANY) { + assert( + value instanceof ArrayValue, + 'ARRAY_CONTAINS_ANY filter has invalid value: ' + value.toString() + ); + return new ArrayContainsAnyFilter(field, value as ArrayValue); + } else { + return new FieldFilter(field, op, value); + } + } + matches(doc: Document): boolean { const other = doc.field(this.field); diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index bb864ad1572..307b84bab9e 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -1282,7 +1282,7 @@ export class JsonProtoSerializer { } fromFieldFilter(filter: api.Filter): Filter { - return new FieldFilter( + return FieldFilter.create( this.fromFieldPathReference(filter.fieldFilter!.field!), this.fromOperatorName(filter.fieldFilter!.op!), this.fromValue(filter.fieldFilter!.value!) @@ -1323,12 +1323,16 @@ export class JsonProtoSerializer { const nanField = this.fromFieldPathReference( filter.unaryFilter!.field! ); - return new FieldFilter(nanField, Operator.EQUAL, DoubleValue.NAN); + return FieldFilter.create(nanField, Operator.EQUAL, DoubleValue.NAN); case 'IS_NULL': const nullField = this.fromFieldPathReference( filter.unaryFilter!.field! ); - return new FieldFilter(nullField, Operator.EQUAL, NullValue.INSTANCE); + return FieldFilter.create( + nullField, + Operator.EQUAL, + NullValue.INSTANCE + ); case 'OPERATOR_UNSPECIFIED': return fail('Unspecified filter'); default: diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index 146d6aaaeed..17917b9200d 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -25,8 +25,12 @@ import { GeoPoint } from '../../../../src/api/geo_point'; import { Timestamp } from '../../../../src/api/timestamp'; import { DatabaseId } from '../../../../src/core/database_info'; import { + ArrayContainsAnyFilter, + ArrayContainsFilter, Direction, FieldFilter, + InFilter, + KeyFieldFilter, Operator, OrderBy, Query @@ -684,7 +688,7 @@ describe('Serializer', () => { it('makes dotted-property names', () => { const path = new FieldPath(['item', 'part', 'top']); - const input = new FieldFilter(path, Operator.EQUAL, wrap('food')); + const input = FieldFilter.create(path, Operator.EQUAL, wrap('food')); const actual = s.toUnaryOrFieldFilter(input); expect(actual).to.deep.equal({ fieldFilter: { @@ -693,7 +697,9 @@ describe('Serializer', () => { value: { stringValue: 'food' } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(FieldFilter); }); it('converts LessThan', () => { @@ -706,7 +712,9 @@ describe('Serializer', () => { value: { integerValue: '42' } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(FieldFilter); }); it('converts LessThanOrEqual', () => { @@ -719,7 +727,9 @@ describe('Serializer', () => { value: { stringValue: 'food' } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(FieldFilter); }); it('converts GreaterThan', () => { @@ -732,7 +742,9 @@ describe('Serializer', () => { value: { booleanValue: false } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(FieldFilter); }); it('converts GreaterThanOrEqual', () => { @@ -745,7 +757,31 @@ describe('Serializer', () => { value: { doubleValue: 1e100 } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(FieldFilter); + }); + + it('converts key field', () => { + const input = filter( + DOCUMENT_KEY_NAME, + '==', + ref('project/database', 'coll/doc') + ); + const actual = s.toUnaryOrFieldFilter(input); + expect(actual).to.deep.equal({ + fieldFilter: { + field: { fieldPath: '__name__' }, + op: 'EQUAL', + value: { + referenceValue: + 'projects/project/databases/database/documents/coll/doc' + } + } + }); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(KeyFieldFilter); }); it('converts array-contains', () => { @@ -758,7 +794,9 @@ describe('Serializer', () => { value: { integerValue: '42' } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(ArrayContainsFilter); }); it('converts IN', () => { @@ -779,7 +817,9 @@ describe('Serializer', () => { } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(InFilter); }); it('converts array-contains-any', () => { @@ -800,7 +840,9 @@ describe('Serializer', () => { } } }); - expect(s.fromFieldFilter(actual)).to.deep.equal(input); + const roundtripped = s.fromFieldFilter(actual); + expect(roundtripped).to.deep.equal(input); + expect(roundtripped).to.be.instanceof(ArrayContainsAnyFilter); }); }); diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index d706ba00867..d02de62d35b 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -607,6 +607,28 @@ describeSpec('Listens:', [], () => { .expectEvents(query, {}); }); + specTest('Array-contains queries support resuming', [], () => { + const query = Query.atPath(path('collection')).addFilter( + filter('array', 'array-contains', 42) + ); + const docA = doc('collection/a', 2000, { foo: 'bar', array: [1, 42, 3] }); + return spec() + .withGCEnabled(false) + .userListens(query) + .watchAcksFull(query, 1000) + .expectEvents(query, {}) + .watchSends({ affects: [query] }, docA) + .watchSnapshots(2000, [query], 'resume-token-2000') + .watchSnapshots(2000) + .expectEvents(query, { added: [docA] }) + .userUnlistens(query) + .watchRemoves(query) + .userListens(query, 'resume-token-2000') + .expectEvents(query, { added: [docA], fromCache: true }) + .watchAcksFull(query, 3000) + .expectEvents(query, {}); + }); + specTest('Persists global resume tokens on unlisten', [], () => { const query = Query.atPath(path('collection')); const docA = doc('collection/a', 1000, { key: 'a' }); diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index d03c7276f59..1deb75bb1f5 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -30,7 +30,6 @@ import { Bound, Direction, FieldFilter, - Filter, Operator, OrderBy } from '../../src/core/query'; @@ -190,7 +189,7 @@ export function blob(...bytes: number[]): Blob { export function filter(path: string, op: string, value: unknown): FieldFilter { const dataValue = wrap(value); const operator = Operator.fromString(op); - const filter = Filter.create(field(path), operator, dataValue); + const filter = FieldFilter.create(field(path), operator, dataValue); if (filter instanceof FieldFilter) { return filter;