Skip to content

Commit 083893c

Browse files
schmidt-sebastianschmidt-sebastian
committed
Tree-shakeable Bound
1 parent d609b63 commit 083893c

File tree

3 files changed

+84
-83
lines changed

3 files changed

+84
-83
lines changed

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

Lines changed: 68 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -452,10 +452,10 @@ export class Query {
452452
* Makes sure a document is within the bounds, if provided.
453453
*/
454454
private matchesBounds(doc: Document): boolean {
455-
if (this.startAt && !this.startAt.sortsBeforeDocument(this.orderBy, doc)) {
455+
if (this.startAt && !sortsBeforeDocument(this.startAt, this.orderBy, doc)) {
456456
return false;
457457
}
458-
if (this.endAt && this.endAt.sortsBeforeDocument(this.orderBy, doc)) {
458+
if (this.endAt && sortsBeforeDocument(this.endAt, this.orderBy, doc)) {
459459
return false;
460460
}
461461
return true;
@@ -732,73 +732,80 @@ export const enum Direction {
732732
*/
733733
export class Bound {
734734
constructor(readonly position: api.Value[], readonly before: boolean) {}
735+
}
735736

736-
canonicalId(): string {
737-
// TODO(b/29183165): Make this collision robust.
738-
return `${this.before ? 'b' : 'a'}:${this.position
739-
.map(p => canonicalId(p))
740-
.join(',')}`;
741-
}
737+
export function canonifyBound(bound: Bound): string {
738+
// TODO(b/29183165): Make this collision robust.
739+
return `${bound.before ? 'b' : 'a'}:${bound.position
740+
.map(p => canonicalId(p))
741+
.join(',')}`;
742+
}
742743

743-
/**
744-
* Returns true if a document sorts before a bound using the provided sort
745-
* order.
746-
*/
747-
sortsBeforeDocument(orderBy: OrderBy[], doc: Document): boolean {
748-
debugAssert(
749-
this.position.length <= orderBy.length,
750-
"Bound has more components than query's orderBy"
751-
);
752-
let comparison = 0;
753-
for (let i = 0; i < this.position.length; i++) {
754-
const orderByComponent = orderBy[i];
755-
const component = this.position[i];
756-
if (orderByComponent.field.isKeyField()) {
757-
debugAssert(
758-
isReferenceValue(component),
759-
'Bound has a non-key value where the key path is being used.'
760-
);
761-
comparison = DocumentKey.comparator(
762-
DocumentKey.fromName(component.referenceValue),
763-
doc.key
764-
);
765-
} else {
766-
const docValue = doc.field(orderByComponent.field);
767-
debugAssert(
768-
docValue !== null,
769-
'Field should exist since document matched the orderBy already.'
770-
);
771-
comparison = valueCompare(component, docValue);
772-
}
773-
if (orderByComponent.dir === Direction.DESCENDING) {
774-
comparison = comparison * -1;
775-
}
776-
if (comparison !== 0) {
777-
break;
778-
}
744+
/**
745+
* Returns true if a document sorts before a bound using the provided sort
746+
* order.
747+
*/
748+
export function sortsBeforeDocument(
749+
bound: Bound,
750+
orderBy: OrderBy[],
751+
doc: Document
752+
): boolean {
753+
debugAssert(
754+
bound.position.length <= orderBy.length,
755+
"Bound has more components than query's orderBy"
756+
);
757+
let comparison = 0;
758+
for (let i = 0; i < bound.position.length; i++) {
759+
const orderByComponent = orderBy[i];
760+
const component = bound.position[i];
761+
if (orderByComponent.field.isKeyField()) {
762+
debugAssert(
763+
isReferenceValue(component),
764+
'Bound has a non-key value where the key path is being used.'
765+
);
766+
comparison = DocumentKey.comparator(
767+
DocumentKey.fromName(component.referenceValue),
768+
doc.key
769+
);
770+
} else {
771+
const docValue = doc.field(orderByComponent.field);
772+
debugAssert(
773+
docValue !== null,
774+
'Field should exist since document matched the orderBy already.'
775+
);
776+
comparison = valueCompare(component, docValue);
777+
}
778+
if (orderByComponent.dir === Direction.DESCENDING) {
779+
comparison = comparison * -1;
780+
}
781+
if (comparison !== 0) {
782+
break;
779783
}
780-
return this.before ? comparison <= 0 : comparison < 0;
781784
}
785+
return bound.before ? comparison <= 0 : comparison < 0;
786+
}
782787

783-
isEqual(other: Bound | null): boolean {
784-
if (other === null) {
785-
return false;
786-
}
787-
if (
788-
this.before !== other.before ||
789-
this.position.length !== other.position.length
790-
) {
788+
export function boundEquals(left: Bound | null, right: Bound | null): boolean {
789+
if (left === null) {
790+
return right === null;
791+
} else if (right === null) {
792+
return false;
793+
}
794+
795+
if (
796+
left.before !== right.before ||
797+
left.position.length !== right.position.length
798+
) {
799+
return false;
800+
}
801+
for (let i = 0; i < left.position.length; i++) {
802+
const thisPosition = left.position[i];
803+
const otherPosition = right.position[i];
804+
if (!valueEquals(thisPosition, otherPosition)) {
791805
return false;
792806
}
793-
for (let i = 0; i < this.position.length; i++) {
794-
const thisPosition = this.position[i];
795-
const otherPosition = other.position[i];
796-
if (!valueEquals(thisPosition, otherPosition)) {
797-
return false;
798-
}
799-
}
800-
return true;
801807
}
808+
return true;
802809
}
803810

804811
/**

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import { DocumentKey } from '../model/document_key';
1919
import { ResourcePath } from '../model/path';
2020
import { isNullOrUndefined } from '../util/types';
21-
import { Bound, Filter, OrderBy } from './query';
21+
import { Bound, boundEquals, canonifyBound, Filter, OrderBy } from './query';
2222
import { debugCast } from '../util/assert';
2323

2424
/**
@@ -102,11 +102,11 @@ export function canonifyTarget(target: Target): string {
102102
}
103103
if (targetImpl.startAt) {
104104
canonicalId += '|lb:';
105-
canonicalId += targetImpl.startAt.canonicalId();
105+
canonicalId += canonifyBound(targetImpl.startAt);
106106
}
107107
if (targetImpl.endAt) {
108108
canonicalId += '|ub:';
109-
canonicalId += targetImpl.endAt.canonicalId();
109+
canonicalId += canonifyBound(targetImpl.endAt);
110110
}
111111
targetImpl.memoizedCanonicalId = canonicalId;
112112
}
@@ -128,10 +128,10 @@ export function stringifyTarget(target: Target): string {
128128
str += `, orderBy: [${target.orderBy.join(', ')}]`;
129129
}
130130
if (target.startAt) {
131-
str += ', startAt: ' + target.startAt;
131+
str += ', startAt: ' + canonifyBound(target.startAt);
132132
}
133133
if (target.endAt) {
134-
str += ', endAt: ' + target.endAt;
134+
str += ', endAt: ' + canonifyBound(target.endAt);
135135
}
136136
return `Target(${str})`;
137137
}
@@ -169,17 +169,11 @@ export function targetEquals(left: Target, right: Target): boolean {
169169
return false;
170170
}
171171

172-
if (
173-
left.startAt !== null
174-
? !left.startAt.isEqual(right.startAt)
175-
: right.startAt !== null
176-
) {
172+
if (!boundEquals(left.startAt, right.startAt)) {
177173
return false;
178174
}
179175

180-
return left.endAt !== null
181-
? left.endAt.isEqual(right.endAt)
182-
: right.endAt === null;
176+
return boundEquals(left.endAt, right.endAt);
183177
}
184178

185179
export function isDocumentTarget(target: Target): boolean {

‎packages/firestore/test/unit/core/query.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { expect } from 'chai';
1919
import { Blob } from '../../../src/api/blob';
2020
import { Timestamp } from '../../../src/api/timestamp';
2121
import { GeoPoint } from '../../../src/api/geo_point';
22-
import { Bound, Query } from '../../../src/core/query';
22+
import { Bound, boundEquals, Query } from '../../../src/core/query';
2323
import { DOCUMENT_KEY_NAME, ResourcePath } from '../../../src/model/path';
2424
import { addEqualityMatcher } from '../../util/equality_matcher';
2525
import {
@@ -45,23 +45,23 @@ describe('Bound', () => {
4545

4646
it('implements isEqual', () => {
4747
let bound = makeBound([1, 2], true);
48-
expect(bound.isEqual(makeBound([1, 2], true))).to.be.true;
48+
expect(boundEquals(bound, makeBound([1, 2], true))).to.be.true;
4949

5050
// Mismatch values
51-
expect(bound.isEqual(makeBound([2, 2], true))).to.be.false;
52-
expect(bound.isEqual(makeBound([1, 3], true))).to.be.false;
51+
expect(boundEquals(bound, makeBound([2, 2], true))).to.be.false;
52+
expect(boundEquals(bound, makeBound([1, 3], true))).to.be.false;
5353

5454
// Mismatch before
55-
expect(bound.isEqual(makeBound([1, 2], false))).to.be.false;
55+
expect(boundEquals(bound, makeBound([1, 2], false))).to.be.false;
5656

5757
// Unequal lengths
58-
expect(bound.isEqual(makeBound([], true))).to.be.false;
59-
expect(bound.isEqual(makeBound([1], true))).to.be.false;
60-
expect(bound.isEqual(makeBound([1, 2, 3], true))).to.be.false;
58+
expect(boundEquals(bound, makeBound([], true))).to.be.false;
59+
expect(boundEquals(bound, makeBound([1], true))).to.be.false;
60+
expect(boundEquals(bound, makeBound([1, 2, 3], true))).to.be.false;
6161

6262
// Zero length
6363
bound = makeBound([], false);
64-
expect(bound.isEqual(makeBound([], false))).to.be.true;
64+
expect(boundEquals(bound, makeBound([], false))).to.be.true;
6565
});
6666
});
6767

0 commit comments

Comments
 (0)