diff --git a/packages/firestore/lite/test/dependencies.json b/packages/firestore/lite/test/dependencies.json index 3a59391336b..185bca0c562 100644 --- a/packages/firestore/lite/test/dependencies.json +++ b/packages/firestore/lite/test/dependencies.json @@ -77,6 +77,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -90,6 +91,7 @@ "compareTimestamps", "createMetadata", "debugAssert", + "debugCast", "decodeBase64", "encodeBase64", "errorMessage", @@ -109,6 +111,7 @@ "hardAssert", "invalidClassError", "isArray", + "isDocumentTarget", "isEmpty", "isMapValue", "isNanValue", @@ -130,6 +133,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -147,6 +151,7 @@ "primitiveComparator", "refValue", "registerFirestore", + "targetEquals", "timestampEquals", "toBytes", "toDouble", @@ -223,6 +228,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -232,7 +238,7 @@ ], "variables": [] }, - "sizeInBytes": 97203 + "sizeInBytes": 97396 }, "DocumentReference": { "dependencies": { @@ -1274,6 +1280,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -1311,6 +1318,7 @@ "invalidClassError", "invokeCommitRpc", "isArray", + "isDocumentTarget", "isDouble", "isEmpty", "isInteger", @@ -1335,6 +1343,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -1354,6 +1363,7 @@ "refValue", "registerFirestore", "serverTimestamp", + "targetEquals", "timestampEquals", "toBytes", "toDocumentMask", @@ -1445,6 +1455,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -1455,7 +1466,7 @@ ], "variables": [] }, - "sizeInBytes": 107641 + "sizeInBytes": 107674 }, "arrayRemove": { "dependencies": { @@ -1692,6 +1703,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -1706,6 +1718,7 @@ "compareTimestamps", "createMetadata", "debugAssert", + "debugCast", "decodeBase64", "encodeBase64", "errorMessage", @@ -1725,6 +1738,7 @@ "hardAssert", "invalidClassError", "isArray", + "isDocumentTarget", "isEmpty", "isMapValue", "isNanValue", @@ -1746,6 +1760,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -1763,6 +1778,7 @@ "primitiveComparator", "refValue", "registerFirestore", + "targetEquals", "timestampEquals", "toBytes", "toDouble", @@ -1840,6 +1856,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -1849,7 +1866,7 @@ ], "variables": [] }, - "sizeInBytes": 97831 + "sizeInBytes": 98024 }, "collectionGroup": { "dependencies": { @@ -1866,6 +1883,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -1880,6 +1898,7 @@ "compareTimestamps", "createMetadata", "debugAssert", + "debugCast", "decodeBase64", "encodeBase64", "errorMessage", @@ -1899,6 +1918,7 @@ "hardAssert", "invalidClassError", "isArray", + "isDocumentTarget", "isEmpty", "isMapValue", "isNanValue", @@ -1920,6 +1940,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -1937,6 +1958,7 @@ "primitiveComparator", "refValue", "registerFirestore", + "targetEquals", "timestampEquals", "toBytes", "toDouble", @@ -2012,6 +2034,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -2021,7 +2044,7 @@ ], "variables": [] }, - "sizeInBytes": 97262 + "sizeInBytes": 97455 }, "deleteDoc": { "dependencies": { @@ -2218,6 +2241,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -2231,6 +2255,7 @@ "compareTimestamps", "createMetadata", "debugAssert", + "debugCast", "decodeBase64", "doc", "encodeBase64", @@ -2251,6 +2276,7 @@ "hardAssert", "invalidClassError", "isArray", + "isDocumentTarget", "isEmpty", "isMapValue", "isNanValue", @@ -2272,6 +2298,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -2290,6 +2317,7 @@ "randomBytes", "refValue", "registerFirestore", + "targetEquals", "timestampEquals", "toBytes", "toDouble", @@ -2368,6 +2396,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -2377,7 +2406,7 @@ ], "variables": [] }, - "sizeInBytes": 98652 + "sizeInBytes": 98845 }, "documentId": { "dependencies": { @@ -2799,7 +2828,7 @@ ], "variables": [] }, - "sizeInBytes": 94348 + "sizeInBytes": 94239 }, "increment": { "dependencies": { @@ -2982,6 +3011,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -2995,6 +3025,7 @@ "compareTimestamps", "createMetadata", "debugAssert", + "debugCast", "decodeBase64", "encodeBase64", "errorMessage", @@ -3014,6 +3045,7 @@ "hardAssert", "invalidClassError", "isArray", + "isDocumentTarget", "isEmpty", "isMapValue", "isNanValue", @@ -3035,6 +3067,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -3053,6 +3086,7 @@ "primitiveComparator", "refValue", "registerFirestore", + "targetEquals", "timestampEquals", "toBytes", "toDouble", @@ -3129,6 +3163,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -3138,7 +3173,7 @@ ], "variables": [] }, - "sizeInBytes": 97546 + "sizeInBytes": 97739 }, "queryEqual": { "dependencies": { @@ -3324,6 +3359,7 @@ "canonifyGeoPoint", "canonifyMap", "canonifyReference", + "canonifyTarget", "canonifyTimestamp", "canonifyValue", "cast", @@ -3337,6 +3373,7 @@ "compareTimestamps", "createMetadata", "debugAssert", + "debugCast", "decodeBase64", "encodeBase64", "errorMessage", @@ -3356,6 +3393,7 @@ "hardAssert", "invalidClassError", "isArray", + "isDocumentTarget", "isEmpty", "isMapValue", "isNanValue", @@ -3377,6 +3415,7 @@ "newConnection", "newDatastore", "newSerializer", + "newTarget", "newUserDataReader", "nodePromise", "normalizeByteString", @@ -3395,6 +3434,7 @@ "refEqual", "refValue", "registerFirestore", + "targetEquals", "timestampEquals", "toBytes", "toDouble", @@ -3471,6 +3511,7 @@ "SnapshotVersion", "StreamBridge", "Target", + "TargetImpl", "Timestamp", "TransformMutation", "UnknownDocument", @@ -3480,7 +3521,7 @@ ], "variables": [] }, - "sizeInBytes": 97488 + "sizeInBytes": 97681 }, "runTransaction": { "dependencies": { diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 12ccbfd889b..06f45ea9456 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -47,8 +47,9 @@ export interface Observer { * backend. */ export class EventManager implements SyncEngineListener { - private queries = new ObjectMap(q => - q.canonicalId() + private queries = new ObjectMap( + q => q.canonicalId(), + (l, r) => l.isEqual(r) ); private onlineState = OnlineState.Unknown; diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 5bacdc2c658..6d7f2cd9ed3 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -34,7 +34,14 @@ import { FieldPath, ResourcePath } from '../model/path'; import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { isNullOrUndefined } from '../util/types'; -import { Target } from './target'; +import { + canonifyTarget, + isDocumentTarget, + newTarget, + stringifyTarget, + Target, + targetEquals +} from './target'; export const enum LimitType { First = 'F', @@ -264,18 +271,18 @@ export class Query { // example, use as a dictionary key, but the implementation is subject to // collisions. Make it collision-free. canonicalId(): string { - return `${this.toTarget().canonicalId()}|lt:${this.limitType}`; + return `${canonifyTarget(this.toTarget())}|lt:${this.limitType}`; } toString(): string { - return `Query(target=${this.toTarget().toString()}; limitType=${ + return `Query(target=${stringifyTarget(this.toTarget())}; limitType=${ this.limitType })`; } isEqual(other: Query): boolean { return ( - this.toTarget().isEqual(other.toTarget()) && + targetEquals(this.toTarget(), other.toTarget()) && this.limitType === other.limitType ); } @@ -343,7 +350,7 @@ export class Query { } isDocumentQuery(): boolean { - return this.toTarget().isDocumentQuery(); + return isDocumentTarget(this.toTarget()); } isCollectionGroupQuery(): boolean { @@ -357,7 +364,7 @@ export class Query { toTarget(): Target { if (!this.memoizedTarget) { if (this.limitType === LimitType.First) { - this.memoizedTarget = new Target( + this.memoizedTarget = newTarget( this.path, this.collectionGroup, this.orderBy, @@ -386,7 +393,7 @@ export class Query { : null; // Now return as a LimitType.First query. - this.memoizedTarget = new Target( + this.memoizedTarget = newTarget( this.path, this.collectionGroup, orderBys, diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index b8bbe37cbc1..ba640411957 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -148,8 +148,9 @@ export interface SyncEngineListener { export class SyncEngine implements RemoteSyncer { protected syncEngineListener: SyncEngineListener | null = null; - protected queryViewsByQuery = new ObjectMap(q => - q.canonicalId() + protected queryViewsByQuery = new ObjectMap( + q => q.canonicalId(), + (l, r) => l.isEqual(r) ); protected queriesByTarget = new Map(); /** diff --git a/packages/firestore/src/core/target.ts b/packages/firestore/src/core/target.ts index 4836d01a0bd..656ba77784d 100644 --- a/packages/firestore/src/core/target.ts +++ b/packages/firestore/src/core/target.ts @@ -19,6 +19,7 @@ import { DocumentKey } from '../model/document_key'; import { ResourcePath } from '../model/path'; import { isNullOrUndefined } from '../util/types'; import { Bound, Filter, OrderBy } from './query'; +import { debugCast } from '../util/assert'; /** * A Target represents the WatchTarget representation of a Query, which is used @@ -28,128 +29,163 @@ import { Bound, Filter, OrderBy } from './query'; * in persistence. */ export class Target { - private memoizedCanonicalId: string | null = null; - - /** - * Initializes a Target with a path and optional additional query constraints. - * Path must currently be empty if this is a collection group query. - * - * NOTE: you should always construct `Target` from `Query.toTarget` instead of - * using this constructor, because `Query` provides an implicit `orderBy` - * property. - */ - constructor( + protected constructor( readonly path: ResourcePath, - readonly collectionGroup: string | null = null, - readonly orderBy: OrderBy[] = [], - readonly filters: Filter[] = [], - readonly limit: number | null = null, - readonly startAt: Bound | null = null, - readonly endAt: Bound | null = null + readonly collectionGroup: string | null, + readonly orderBy: OrderBy[], + readonly filters: Filter[], + readonly limit: number | null, + readonly startAt: Bound | null, + readonly endAt: Bound | null ) {} +} - canonicalId(): string { - if (this.memoizedCanonicalId === null) { - let canonicalId = this.path.canonicalString(); - if (this.collectionGroup !== null) { - canonicalId += '|cg:' + this.collectionGroup; - } - canonicalId += '|f:'; - canonicalId += this.filters.map(f => f.canonicalId()).join(','); - canonicalId += '|ob:'; - canonicalId += this.orderBy.map(o => o.canonicalId()).join(','); - - if (!isNullOrUndefined(this.limit)) { - canonicalId += '|l:'; - canonicalId += this.limit!; - } - if (this.startAt) { - canonicalId += '|lb:'; - canonicalId += this.startAt.canonicalId(); - } - if (this.endAt) { - canonicalId += '|ub:'; - canonicalId += this.endAt.canonicalId(); - } - this.memoizedCanonicalId = canonicalId; - } - return this.memoizedCanonicalId; +class TargetImpl extends Target { + memoizedCanonicalId: string | null = null; + constructor( + path: ResourcePath, + collectionGroup: string | null = null, + orderBy: OrderBy[] = [], + filters: Filter[] = [], + limit: number | null = null, + startAt: Bound | null = null, + endAt: Bound | null = null + ) { + super(path, collectionGroup, orderBy, filters, limit, startAt, endAt); } +} - toString(): string { - let str = this.path.canonicalString(); - if (this.collectionGroup !== null) { - str += ' collectionGroup=' + this.collectionGroup; - } - if (this.filters.length > 0) { - str += `, filters: [${this.filters.join(', ')}]`; - } - if (!isNullOrUndefined(this.limit)) { - str += ', limit: ' + this.limit; +/** + * Initializes a Target with a path and optional additional query constraints. + * Path must currently be empty if this is a collection group query. + * + * NOTE: you should always construct `Target` from `Query.toTarget` instead of + * using this factory method, because `Query` provides an implicit `orderBy` + * property. + */ +export function newTarget( + path: ResourcePath, + collectionGroup: string | null = null, + orderBy: OrderBy[] = [], + filters: Filter[] = [], + limit: number | null = null, + startAt: Bound | null = null, + endAt: Bound | null = null +): Target { + return new TargetImpl( + path, + collectionGroup, + orderBy, + filters, + limit, + startAt, + endAt + ); +} + +export function canonifyTarget(target: Target): string { + const targetImpl = debugCast(target, TargetImpl); + + if (targetImpl.memoizedCanonicalId === null) { + let canonicalId = targetImpl.path.canonicalString(); + if (targetImpl.collectionGroup !== null) { + canonicalId += '|cg:' + targetImpl.collectionGroup; } - if (this.orderBy.length > 0) { - str += `, orderBy: [${this.orderBy.join(', ')}]`; + canonicalId += '|f:'; + canonicalId += targetImpl.filters.map(f => f.canonicalId()).join(','); + canonicalId += '|ob:'; + canonicalId += targetImpl.orderBy.map(o => o.canonicalId()).join(','); + + if (!isNullOrUndefined(targetImpl.limit)) { + canonicalId += '|l:'; + canonicalId += targetImpl.limit!; } - if (this.startAt) { - str += ', startAt: ' + this.startAt.canonicalId(); + if (targetImpl.startAt) { + canonicalId += '|lb:'; + canonicalId += targetImpl.startAt.canonicalId(); } - if (this.endAt) { - str += ', endAt: ' + this.endAt.canonicalId(); + if (targetImpl.endAt) { + canonicalId += '|ub:'; + canonicalId += targetImpl.endAt.canonicalId(); } - return `Target(${str})`; + targetImpl.memoizedCanonicalId = canonicalId; } + return targetImpl.memoizedCanonicalId; +} - isEqual(other: Target): boolean { - if (this.limit !== other.limit) { - return false; - } +export function stringifyTarget(target: Target): string { + let str = target.path.canonicalString(); + if (target.collectionGroup !== null) { + str += ' collectionGroup=' + target.collectionGroup; + } + if (target.filters.length > 0) { + str += `, filters: [${target.filters.join(', ')}]`; + } + if (!isNullOrUndefined(target.limit)) { + str += ', limit: ' + target.limit; + } + if (target.orderBy.length > 0) { + str += `, orderBy: [${target.orderBy.join(', ')}]`; + } + if (target.startAt) { + str += ', startAt: ' + target.startAt; + } + if (target.endAt) { + str += ', endAt: ' + target.endAt; + } + return `Target(${str})`; +} - if (this.orderBy.length !== other.orderBy.length) { - return false; - } +export function targetEquals(left: Target, right: Target): boolean { + if (left.limit !== right.limit) { + return false; + } - for (let i = 0; i < this.orderBy.length; i++) { - if (!this.orderBy[i].isEqual(other.orderBy[i])) { - return false; - } - } + if (left.orderBy.length !== right.orderBy.length) { + return false; + } - if (this.filters.length !== other.filters.length) { + for (let i = 0; i < left.orderBy.length; i++) { + if (!left.orderBy[i].isEqual(right.orderBy[i])) { return false; } + } - for (let i = 0; i < this.filters.length; i++) { - if (!this.filters[i].isEqual(other.filters[i])) { - return false; - } - } - - if (this.collectionGroup !== other.collectionGroup) { - return false; - } + if (left.filters.length !== right.filters.length) { + return false; + } - if (!this.path.isEqual(other.path)) { + for (let i = 0; i < left.filters.length; i++) { + if (!left.filters[i].isEqual(right.filters[i])) { return false; } + } - if ( - this.startAt !== null - ? !this.startAt.isEqual(other.startAt) - : other.startAt !== null - ) { - return false; - } + if (left.collectionGroup !== right.collectionGroup) { + return false; + } - return this.endAt !== null - ? this.endAt.isEqual(other.endAt) - : other.endAt === null; + if (!left.path.isEqual(right.path)) { + return false; } - isDocumentQuery(): boolean { - return ( - DocumentKey.isDocumentKey(this.path) && - this.collectionGroup === null && - this.filters.length === 0 - ); + if ( + left.startAt !== null + ? !left.startAt.isEqual(right.startAt) + : right.startAt !== null + ) { + return false; } + + return left.endAt !== null + ? left.endAt.isEqual(right.endAt) + : right.endAt === null; +} + +export function isDocumentTarget(target: Target): boolean { + return ( + DocumentKey.isDocumentKey(target.path) && + target.collectionGroup === null && + target.filters.length === 0 + ); } diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index cf073f2b766..c34ca19587e 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -417,10 +417,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { */ private static RemoteDocumentChangeBuffer = class extends RemoteDocumentChangeBuffer { // A map of document sizes prior to applying the changes in this buffer. - protected documentSizes: ObjectMap< - DocumentKey, - number - > = new ObjectMap(key => key.toString()); + protected documentSizes: ObjectMap = new ObjectMap( + key => key.toString(), + (l, r) => l.isEqual(r) + ); /** * @param documentCache The IndexedDbRemoteDocumentCache to apply the changes to. diff --git a/packages/firestore/src/local/indexeddb_target_cache.ts b/packages/firestore/src/local/indexeddb_target_cache.ts index 1a0ff9b65cb..24b091aa861 100644 --- a/packages/firestore/src/local/indexeddb_target_cache.ts +++ b/packages/firestore/src/local/indexeddb_target_cache.ts @@ -46,7 +46,7 @@ import { PersistencePromise } from './persistence_promise'; import { TargetCache } from './target_cache'; import { TargetData } from './target_data'; import { SimpleDbStore } from './simple_db'; -import { Target } from '../core/target'; +import { canonifyTarget, Target, targetEquals } from '../core/target'; export class IndexedDbTargetCache implements TargetCache { constructor( @@ -252,7 +252,7 @@ export class IndexedDbTargetCache implements TargetCache { // Iterating by the canonicalId may yield more than one result because // canonicalId values are not required to be unique per target. This query // depends on the queryTargets index to be efficient. - const canonicalId = target.canonicalId(); + const canonicalId = canonifyTarget(target); const range = IDBKeyRange.bound( [canonicalId, Number.NEGATIVE_INFINITY], [canonicalId, Number.POSITIVE_INFINITY] @@ -265,7 +265,7 @@ export class IndexedDbTargetCache implements TargetCache { const found = this.serializer.fromDbTarget(value); // After finding a potential match, check that the target is // actually equal to the requested target. - if (target.isEqual(found.target)) { + if (targetEquals(target, found.target)) { result = found; control.done(); } diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index a383b1d3adb..720534facee 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -39,7 +39,7 @@ import { } from '../remote/serializer'; import { debugAssert, fail } from '../util/assert'; import { ByteString } from '../util/byte_string'; -import { Target } from '../core/target'; +import { canonifyTarget, isDocumentTarget, Target } from '../core/target'; import { DbMutationBatch, DbNoDocument, @@ -221,7 +221,7 @@ export class LocalSerializer { targetData.lastLimboFreeSnapshotVersion ); let queryProto: DbQuery; - if (targetData.target.isDocumentQuery()) { + if (isDocumentTarget(targetData.target)) { queryProto = toDocumentsTarget(this.remoteSerializer, targetData.target); } else { queryProto = toQueryTarget(this.remoteSerializer, targetData.target); @@ -234,7 +234,7 @@ export class LocalSerializer { // lastListenSequenceNumber is always 0 until we do real GC. return new DbTarget( targetData.targetId, - targetData.target.canonicalId(), + canonifyTarget(targetData.target), dbTimestamp, resumeToken, targetData.sequenceNumber, diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 25fa30b8c59..3dc97681cc7 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -19,7 +19,7 @@ import { Timestamp } from '../api/timestamp'; import { User } from '../auth/user'; import { Query } from '../core/query'; import { SnapshotVersion } from '../core/snapshot_version'; -import { Target } from '../core/target'; +import { canonifyTarget, Target, targetEquals } from '../core/target'; import { BatchId, TargetId } from '../core/types'; import { DocumentKeySet, @@ -179,8 +179,9 @@ export class LocalStore { /** Maps a target to its targetID. */ // TODO(wuandy): Evaluate if TargetId can be part of Target. - private targetIdByTarget = new ObjectMap(t => - t.canonicalId() + private targetIdByTarget = new ObjectMap( + t => canonifyTarget(t), + targetEquals ); /** diff --git a/packages/firestore/src/local/memory_persistence.ts b/packages/firestore/src/local/memory_persistence.ts index 391f8686565..29173d00ad4 100644 --- a/packages/firestore/src/local/memory_persistence.ts +++ b/packages/firestore/src/local/memory_persistence.ts @@ -308,7 +308,10 @@ export class MemoryLruDelegate implements ReferenceDelegate, LruDelegate { private orphanedSequenceNumbers: ObjectMap< DocumentKey, ListenSequenceNumber - > = new ObjectMap(k => encodeResourcePath(k.path)); + > = new ObjectMap( + k => encodeResourcePath(k.path), + (l, r) => l.isEqual(r) + ); readonly garbageCollector: LruGarbageCollector; diff --git a/packages/firestore/src/local/memory_target_cache.ts b/packages/firestore/src/local/memory_target_cache.ts index 316c655c084..a4ff61eaebf 100644 --- a/packages/firestore/src/local/memory_target_cache.ts +++ b/packages/firestore/src/local/memory_target_cache.ts @@ -30,13 +30,16 @@ import { PersistencePromise } from './persistence_promise'; import { ReferenceSet } from './reference_set'; import { TargetCache } from './target_cache'; import { TargetData } from './target_data'; -import { Target } from '../core/target'; +import { canonifyTarget, Target, targetEquals } from '../core/target'; export class MemoryTargetCache implements TargetCache { /** * Maps a target to the data about that target */ - private targets = new ObjectMap(t => t.canonicalId()); + private targets = new ObjectMap( + t => canonifyTarget(t), + targetEquals + ); /** The last received snapshot version. */ private lastRemoteSnapshotVersion = SnapshotVersion.min(); diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index b434900afdb..c54a0a0a3e7 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -45,7 +45,10 @@ export abstract class RemoteDocumentChangeBuffer { protected changes: ObjectMap< DocumentKey, MaybeDocument | null - > = new ObjectMap(key => key.toString()); + > = new ObjectMap( + key => key.toString(), + (l, r) => l.isEqual(r) + ); // The read time to use for all added documents in this change buffer. private _readTime: SnapshotVersion | undefined; diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 269a56f5631..ea51deb1c9b 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -29,7 +29,7 @@ import { Query } from '../core/query'; import { SnapshotVersion } from '../core/snapshot_version'; -import { Target } from '../core/target'; +import { isDocumentTarget, Target } from '../core/target'; import { TargetId } from '../core/types'; import { TargetData, TargetPurpose } from '../local/target_data'; import { Document, MaybeDocument, NoDocument } from '../model/document'; @@ -958,7 +958,7 @@ export function toTarget( let result: api.Target; const target = targetData.target; - if (target.isDocumentQuery()) { + if (isDocumentTarget(target)) { result = { documents: toDocumentsTarget(serializer, target) }; } else { result = { query: toQueryTarget(serializer, target) }; diff --git a/packages/firestore/src/remote/watch_change.ts b/packages/firestore/src/remote/watch_change.ts index 457173c5b08..89bb1018e7c 100644 --- a/packages/firestore/src/remote/watch_change.ts +++ b/packages/firestore/src/remote/watch_change.ts @@ -35,6 +35,7 @@ import { SortedSet } from '../util/sorted_set'; import { ExistenceFilter } from './existence_filter'; import { RemoteEvent, TargetChange } from './remote_event'; import { ByteString } from '../util/byte_string'; +import { isDocumentTarget } from '../core/target'; /** * Internal representation of the watcher API protocol buffers. @@ -387,7 +388,7 @@ export class WatchChangeAggregator { const targetData = this.targetDataForActiveTarget(targetId); if (targetData) { const target = targetData.target; - if (target.isDocumentQuery()) { + if (isDocumentTarget(target)) { if (expectedCount === 0) { // The existence filter told us the document does not exist. We deduce // that this document does not exist and apply a deleted document to @@ -429,7 +430,7 @@ export class WatchChangeAggregator { this.targetStates.forEach((targetState, targetId) => { const targetData = this.targetDataForActiveTarget(targetId); if (targetData) { - if (targetState.current && targetData.target.isDocumentQuery()) { + if (targetState.current && isDocumentTarget(targetData.target)) { // Document queries for document that don't exist can produce an empty // result set. To update our local cache, we synthesize a document // delete if we have not previously received the document. This diff --git a/packages/firestore/src/util/obj_map.ts b/packages/firestore/src/util/obj_map.ts index 773eb6e5662..dfc56bacbe0 100644 --- a/packages/firestore/src/util/obj_map.ts +++ b/packages/firestore/src/util/obj_map.ts @@ -15,18 +15,17 @@ * limitations under the License. */ -import { Equatable } from './misc'; import { forEach, isEmpty } from './obj'; type Entry = [K, V]; /** - * A map implementation that uses objects as keys. Objects must implement the - * Equatable interface and must be immutable. Entries in the map are stored - * together with the key being produced from the mapKeyFn. This map + * A map implementation that uses objects as keys. Objects must have an + * associated equals function and must be immutable. Entries in the map are + * stored together with the key being produced from the mapKeyFn. This map * automatically handles collisions of keys. */ -export class ObjectMap, ValueType> { +export class ObjectMap { /** * The inner map for a key -> value pair. Due to the possibility of * collisions we keep a list of entries that we do a linear search through @@ -37,7 +36,10 @@ export class ObjectMap, ValueType> { [canonicalId: string]: Array>; } = {}; - constructor(private mapKeyFn: (key: KeyType) => string) {} + constructor( + private mapKeyFn: (key: KeyType) => string, + private equalsFn: (l: KeyType, r: KeyType) => boolean + ) {} /** Get a value for this key, or undefined if it does not exist. */ get(key: KeyType): ValueType | undefined { @@ -47,7 +49,7 @@ export class ObjectMap, ValueType> { return undefined; } for (const [otherKey, value] of matches) { - if (otherKey.isEqual(key)) { + if (this.equalsFn(otherKey, key)) { return value; } } @@ -67,7 +69,7 @@ export class ObjectMap, ValueType> { return; } for (let i = 0; i < matches.length; i++) { - if (matches[i][0].isEqual(key)) { + if (this.equalsFn(matches[i][0], key)) { matches[i] = [key, value]; return; } @@ -85,7 +87,7 @@ export class ObjectMap, ValueType> { return false; } for (let i = 0; i < matches.length; i++) { - if (matches[i][0].isEqual(key)) { + if (this.equalsFn(matches[i][0], key)) { if (matches.length === 1) { delete this.inner[id]; } else { diff --git a/packages/firestore/test/unit/core/query.test.ts b/packages/firestore/test/unit/core/query.test.ts index a1e5f4f20f6..4f177364932 100644 --- a/packages/firestore/test/unit/core/query.test.ts +++ b/packages/firestore/test/unit/core/query.test.ts @@ -33,6 +33,7 @@ import { ref, wrap } from '../../util/helpers'; +import { canonifyTarget } from '../../../src/core/target'; describe('Bound', () => { function makeBound(values: unknown[], before: boolean): Bound { @@ -722,6 +723,6 @@ describe('Query', () => { }); function assertCanonicalId(query: Query, expectedCanonicalId: string): void { - expect(query.toTarget().canonicalId()).to.equal(expectedCanonicalId); + expect(canonifyTarget(query.toTarget())).to.equal(expectedCanonicalId); } }); diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts index a463c9a3183..02bddb52515 100644 --- a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts +++ b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts @@ -73,6 +73,7 @@ import { TEST_PERSISTENCE_PREFIX, TEST_SERIALIZER } from './persistence_test_helpers'; +import { canonifyTarget } from '../../../src/core/target'; import { FakeDocument, testDocument } from '../../util/test_platform'; import { getWindow } from '../../../src/platform/dom'; @@ -808,7 +809,7 @@ describe('IndexedDbSchema: createOrUpgradeDb', () => { const targetsStore = txn.store(DbTarget.store); return targetsStore.iterate((key, value) => { const targetData = TEST_SERIALIZER.fromDbTarget(value).target; - const expectedCanonicalId = targetData.canonicalId(); + const expectedCanonicalId = canonifyTarget(targetData); const actualCanonicalId = value.canonicalId; expect(actualCanonicalId).to.equal(expectedCanonicalId); diff --git a/packages/firestore/test/unit/local/target_cache.test.ts b/packages/firestore/test/unit/local/target_cache.test.ts index 35aaeaa6cfb..1d39cd5927b 100644 --- a/packages/firestore/test/unit/local/target_cache.test.ts +++ b/packages/firestore/test/unit/local/target_cache.test.ts @@ -34,7 +34,7 @@ import { import { Timestamp } from '../../../src/api/timestamp'; import * as persistenceHelpers from './persistence_test_helpers'; import { TestTargetCache } from './test_target_cache'; -import { Target } from '../../../src/core/target'; +import { canonifyTarget, Target, targetEquals } from '../../../src/core/target'; describe('MemoryTargetCache', () => { genericTargetCacheTests(persistenceHelpers.testMemoryEagerPersistence); @@ -106,7 +106,7 @@ describe('IndexedDbTargetCache', () => { function genericTargetCacheTests( persistencePromise: () => Promise ): void { - addEqualityMatcher(); + addEqualityMatcher({ equalsFn: targetEquals, forType: Target }); let cache: TestTargetCache; const QUERY_ROOMS = Query.atPath(path('rooms')).toTarget(); @@ -174,7 +174,7 @@ function genericTargetCacheTests( const q2 = Query.atPath(path('a')) .addFilter(filter('foo', '==', '1')) .toTarget(); - expect(q1.canonicalId()).to.equal(q2.canonicalId()); + expect(canonifyTarget(q1)).to.equal(canonifyTarget(q2)); const data1 = testTargetData(q1, 1, 1); await cache.addTargetData(data1); diff --git a/packages/firestore/test/unit/specs/describe_spec.ts b/packages/firestore/test/unit/specs/describe_spec.ts index a6385a88e44..ed1822a7de7 100644 --- a/packages/firestore/test/unit/specs/describe_spec.ts +++ b/packages/firestore/test/unit/specs/describe_spec.ts @@ -26,6 +26,7 @@ import { SpecBuilder } from './spec_builder'; import { SpecStep } from './spec_test_runner'; import * as stringify from 'json-stable-stringify'; +import { Target, targetEquals } from '../../../src/core/target'; // Disables all other tests; useful for debugging. Multiple tests can have // this tag and they'll all be run (but all others won't). @@ -239,7 +240,7 @@ export function describeSpec( if (!writeJSONFile) { describe(name, () => { - addEqualityMatcher(); + addEqualityMatcher({ equalsFn: targetEquals, forType: Target }); return builder(); }); } else { diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index d5c0f2460ef..af29c65f2dd 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -16,7 +16,7 @@ */ import { FieldFilter, Filter, Query } from '../../../src/core/query'; -import { Target } from '../../../src/core/target'; +import { canonifyTarget, Target, targetEquals } from '../../../src/core/target'; import { TargetIdGenerator } from '../../../src/core/target_id_generator'; import { TargetId } from '../../../src/core/types'; import { @@ -86,7 +86,10 @@ export interface ActiveTargetMap { */ export class ClientMemoryState { activeTargets: ActiveTargetMap = {}; - queryMapping = new ObjectMap(t => t.canonicalId()); + queryMapping = new ObjectMap( + t => canonifyTarget(t), + targetEquals + ); limboMapping: LimboMap = {}; limboIdGenerator: TargetIdGenerator = TargetIdGenerator.forSyncEngine(); @@ -98,7 +101,10 @@ export class ClientMemoryState { /** Reset all internal memory state (as done during a client restart). */ reset(): void { - this.queryMapping = new ObjectMap(t => t.canonicalId()); + this.queryMapping = new ObjectMap( + t => canonifyTarget(t), + targetEquals + ); this.limboMapping = {}; this.activeTargets = {}; this.limboIdGenerator = TargetIdGenerator.forSyncEngine(); @@ -118,7 +124,10 @@ export class ClientMemoryState { */ class CachedTargetIdGenerator { // TODO(wuandy): rename this to targetMapping. - private queryMapping = new ObjectMap(t => t.canonicalId()); + private queryMapping = new ObjectMap( + t => canonifyTarget(t), + targetEquals + ); private targetIdGenerator = TargetIdGenerator.forTargetCache(); /** diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 3b82ecc2cbf..d72d64cebbf 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -165,9 +165,9 @@ abstract class TestRunner { private snapshotsInSyncEvents = 0; protected document = new FakeDocument(); - - private queryListeners = new ObjectMap(q => - q.canonicalId() + private queryListeners = new ObjectMap( + q => q.canonicalId(), + (l, r) => l.isEqual(r) ); private expectedActiveLimboDocs: DocumentKey[]; diff --git a/packages/firestore/test/unit/util/obj_map.test.ts b/packages/firestore/test/unit/util/obj_map.test.ts index 116c90cc5f9..7f1e7688234 100644 --- a/packages/firestore/test/unit/util/obj_map.test.ts +++ b/packages/firestore/test/unit/util/obj_map.test.ts @@ -32,7 +32,10 @@ class TestKey { describe('ObjectMap', () => { it('can get/put/delete values', () => { - const map = new ObjectMap(o => o.mapKey); + const map = new ObjectMap( + o => o.mapKey, + (l, r) => l.isEqual(r) + ); const k1 = new TestKey(4, 4); const k2 = new TestKey(5, 5); const k3 = new TestKey(6, 6); @@ -70,7 +73,10 @@ describe('ObjectMap', () => { }); it('can handle collisions', () => { - const map = new ObjectMap(o => o.mapKey); + const map = new ObjectMap( + o => o.mapKey, + (l, r) => l.isEqual(r) + ); // These all have the same ids, but are different entities. const k1 = new TestKey(4, 4); const k2 = new TestKey(4, 5); diff --git a/packages/firestore/test/util/equality_matcher.ts b/packages/firestore/test/util/equality_matcher.ts index f6569de3567..3ba85f64d9c 100644 --- a/packages/firestore/test/util/equality_matcher.ts +++ b/packages/firestore/test/util/equality_matcher.ts @@ -26,6 +26,15 @@ export interface Equatable { isEqual(other: T): boolean; } +/** + * Custom equals override for types that have a free-standing equals functions + * (such as `queryEquals()`). + */ +export interface CustomMatcher { + equalsFn: (left: T, right: T) => boolean; + forType: Function; +} + /** * @file This file provides a helper function to add a matcher that matches * based on an objects isEqual method. If the isEqual method is present one @@ -33,7 +42,20 @@ export interface Equatable { * implementation is used. */ -function customDeepEqual(left: unknown, right: unknown): boolean { +function customDeepEqual( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + customMatchers: Array>, + left: unknown, + right: unknown +): boolean { + for (const customMatcher of customMatchers) { + if ( + left instanceof customMatcher.forType && + right instanceof customMatcher.forType + ) { + return customMatcher.equalsFn(left, right); + } + } if (typeof left === 'object' && left && 'isEqual' in left) { return (left as Equatable).isEqual(right); } @@ -75,8 +97,10 @@ function customDeepEqual(left: unknown, right: unknown): boolean { if (!Object.prototype.hasOwnProperty.call(right, key)) { return false; } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - if (!customDeepEqual((left as any)[key], (right as any)[key])) { + if ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + !customDeepEqual(customMatchers, (left as any)[key], (right as any)[key]) + ) { return false; } } @@ -86,7 +110,10 @@ function customDeepEqual(left: unknown, right: unknown): boolean { /** The original equality function passed in by chai(). */ let originalFunction: ((r: unknown, l: unknown) => boolean) | null = null; -export function addEqualityMatcher(): void { +export function addEqualityMatcher( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...customMatchers: Array> +): void { let isActive = true; before(() => { @@ -111,7 +138,7 @@ export function addEqualityMatcher(): void { // NOTE: Unlike the top-level chai assert() method, Assertion.assert() // takes the expected value before the actual value. assertion.assert( - customDeepEqual(actual, expected), + customDeepEqual(customMatchers, actual, expected), 'expected #{act} to roughly deeply equal #{exp}', 'expected #{act} to not roughly deeply equal #{exp}', expected,