From 318d83f5de5d516591c52edb4e8665c2aeeea549 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Fri, 16 Dec 2022 11:24:51 -0700 Subject: [PATCH 1/4] Updated the canonical ID of flat conjunctions to match the canonical ID of old style implicit flat conjunctions. --- packages/firestore/src/core/filter.ts | 8 ++++++++ packages/firestore/test/unit/core/filter.test.ts | 14 +++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/core/filter.ts b/packages/firestore/src/core/filter.ts index 4322c926658..16bb627f2dc 100644 --- a/packages/firestore/src/core/filter.ts +++ b/packages/firestore/src/core/filter.ts @@ -325,6 +325,14 @@ export function canonifyFilter(filter: Filter): string { filter.op.toString() + canonicalId(filter.value) ); + } else if (compositeFilterIsFlatConjunction(filter)) { + // Older SDK versions use an implicit AND operation between their filters. + // In the new SDK versions, the developer may use an explicit AND filter. + // To stay consistent with the old usages, we add a special case to ensure + // the canonical ID for these two are the same. For example: + // `col.whereEquals("a", 1).whereEquals("b", 2)` should have the same + // canonical ID as `col.where(and(equals("a",1), equals("b",2)))`. + return filter.filters.map(filter => canonifyFilter(filter)).join(','); } else { // filter instanceof CompositeFilter const canonicalIdsString = filter.filters diff --git a/packages/firestore/test/unit/core/filter.test.ts b/packages/firestore/test/unit/core/filter.test.ts index 3011e3f1647..896e368e30f 100644 --- a/packages/firestore/test/unit/core/filter.test.ts +++ b/packages/firestore/test/unit/core/filter.test.ts @@ -25,7 +25,9 @@ import { FieldFilter, Operator } from '../../../src/core/filter'; -import { andFilter, filter, orFilter } from '../../util/helpers'; +import { queryToTarget } from '../../../src/core/query'; +import { canonifyTarget } from '../../../src/core/target'; +import { andFilter, filter, orFilter, query } from '../../util/helpers'; describe('FieldFilter', () => { it('exposes field filter members', () => { @@ -93,4 +95,14 @@ describe('CompositeFilter', () => { expect(compositeFilterIsFlat(orFilter2)).false; expect(compositeFilterIsFlatConjunction(orFilter2)).false; }); + + it('computes canonical id of flat conjunctions', () => { + const target1 = query('col', a, b, c); + + const target2 = query('col', andFilter(a, b, c)); + + expect(canonifyTarget(queryToTarget(target1))).to.equal( + canonifyTarget(queryToTarget(target2)) + ); + }); }); From bbb22b39af1268341bd968653970e88fd1846ac0 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Fri, 16 Dec 2022 11:25:28 -0700 Subject: [PATCH 2/4] Updated documentation of or() and and() to match the latest API docs in the Android implementation. --- packages/firestore/src/lite-api/query.ts | 26 +++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/firestore/src/lite-api/query.ts b/packages/firestore/src/lite-api/query.ts index 2f1d7ecbf8f..0b3de2268d0 100644 --- a/packages/firestore/src/lite-api/query.ts +++ b/packages/firestore/src/lite-api/query.ts @@ -364,13 +364,14 @@ export type QueryFilterConstraint = | QueryCompositeFilterConstraint; /** - * Creates a {@link QueryCompositeFilterConstraint} that performs a logical OR - * of all the provided {@link QueryFilterConstraint}s. + * Creates a new {@link QueryCompositeFilterConstraint} that is a disjunction of + * the given filter constraints. A disjunction filter includes a document if it + * satisfies any of the given filters. * - * @param queryConstraints - Optional. The {@link QueryFilterConstraint}s - * for OR operation. These must be created with calls to {@link where}, - * {@link or}, or {@link and}. - * @returns The created {@link QueryCompositeFilterConstraint}. + * @param queryConstraints - Optional. The list of + * {@link QueryFilterConstraint}s to perform a disjunction for. These must be + * created with calls to {@link where}, {@link or}, or {@link and}. + * @returns The newly created {@link QueryCompositeFilterConstraint}. * @internal TODO remove this internal tag with OR Query support in the server */ export function or( @@ -388,13 +389,14 @@ export function or( } /** - * Creates a {@link QueryCompositeFilterConstraint} that performs a logical AND - * of all the provided {@link QueryFilterConstraint}s. + * Creates a new {@link QueryCompositeFilterConstraint} that is a conjunction of + * the given filter constraints. A conjunction filter includes a document if it + * satisfies all of the given filters. * - * @param queryConstraints - Optional. The {@link QueryFilterConstraint}s - * for AND operation. These must be created with calls to {@link where}, - * {@link or}, or {@link and}. - * @returns The created {@link QueryCompositeFilterConstraint}. + * @param queryConstraints - Optional. The list of + * {@link QueryFilterConstraint}s to perform a conjunction for. These must be + * created with calls to {@link where}, {@link or}, or {@link and}. + * @returns The newly created {@link QueryCompositeFilterConstraint}. * @internal TODO remove this internal tag with OR Query support in the server */ export function and( From b0d2c7e373e9c55e52f102fc6fb93ecebd0be9e5 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Mon, 19 Dec 2022 08:04:08 -0700 Subject: [PATCH 3/4] Formatting fix. --- packages/firestore/test/unit/core/filter.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/unit/core/filter.test.ts b/packages/firestore/test/unit/core/filter.test.ts index 896e368e30f..a8ea6277c9a 100644 --- a/packages/firestore/test/unit/core/filter.test.ts +++ b/packages/firestore/test/unit/core/filter.test.ts @@ -25,7 +25,7 @@ import { FieldFilter, Operator } from '../../../src/core/filter'; -import { queryToTarget } from '../../../src/core/query'; +import { queryToTarget } from '../../../src/core/query'; import { canonifyTarget } from '../../../src/core/target'; import { andFilter, filter, orFilter, query } from '../../util/helpers'; From 76de87e2759c977835d70968dacbf73efba5d219 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Mon, 19 Dec 2022 08:43:58 -0700 Subject: [PATCH 4/4] Create quick-radios-obey.md --- .changeset/quick-radios-obey.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/quick-radios-obey.md diff --git a/.changeset/quick-radios-obey.md b/.changeset/quick-radios-obey.md new file mode 100644 index 00000000000..83e452264a5 --- /dev/null +++ b/.changeset/quick-radios-obey.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Update canonifyFilter to compute the canonization for flat conjunctions the same as implicit AND queries.