From aee01c339b8096654566f595094d3377e04ccc42 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 14 Mar 2025 16:33:00 -0400 Subject: [PATCH 01/32] first attempt --- packages/firestore/src/lite-api/snapshot.ts | 13 + packages/firestore/src/model/path.ts | 205 +++++++- packages/firestore/src/util/bundle_builder.ts | 39 ++ .../firestore/src/util/bundle_builder_impl.ts | 275 ++++++++++ .../util/bundle_builder_validation_utils.ts | 468 ++++++++++++++++++ 5 files changed, 999 insertions(+), 1 deletion(-) create mode 100644 packages/firestore/src/util/bundle_builder.ts create mode 100644 packages/firestore/src/util/bundle_builder_impl.ts create mode 100644 packages/firestore/src/util/bundle_builder_validation_utils.ts diff --git a/packages/firestore/src/lite-api/snapshot.ts b/packages/firestore/src/lite-api/snapshot.ts index 3024e2e9db..4da2d9babd 100644 --- a/packages/firestore/src/lite-api/snapshot.ts +++ b/packages/firestore/src/lite-api/snapshot.ts @@ -38,6 +38,7 @@ import { UntypedFirestoreDataConverter } from './user_data_reader'; import { AbstractUserDataWriter } from './user_data_writer'; +import { Timestamp } from '../lite-api/timestamp'; /** * Converter used by `withConverter()` to transform user objects of type @@ -329,6 +330,18 @@ export class DocumentSnapshot< ); } + get createTime(): Timestamp | undefined { + return this._document?.createTime.toTimestamp(); + } + + get updateTime(): Timestamp | undefined { + return this._document?.updateTime.toTimestamp(); + } + + get readTime(): Timestamp | undefined { + return this._document?.readTime.toTimestamp(); + } + /** * Signals whether or not the document at the snapshot's location exists. * diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts index 64cb0376a0..ddd5c1844e 100644 --- a/packages/firestore/src/model/path.ts +++ b/packages/firestore/src/model/path.ts @@ -22,11 +22,22 @@ import { Code, FirestoreError } from '../util/error'; export const DOCUMENT_KEY_NAME = '__name__'; +/*! + * A regular expression to verify an absolute Resource Path in Firestore. It + * extracts the project ID, the database name and the relative resource path + * if available. + * + * @type {RegExp} + */ +const RESOURCE_PATH_RE = + // Note: [\s\S] matches all characters including newlines. + /^projects\/([^/]*)\/databases\/([^/]*)(?:\/documents\/)?([\s\S]*)$/; + /** * Path represents an ordered sequence of string segments. */ abstract class BasePath> { - private segments: string[]; + protected segments: string[]; private offset: number; private len: number; @@ -257,6 +268,21 @@ export class ResourcePath extends BasePath { return this.toArray().map(encodeURIComponent).join('/'); } + /** + * Converts this path to a fully qualified ResourcePath. + * + * @private + * @internal + * @param projectId The project ID of the current Firestore project. + * @return A fully-qualified resource path pointing to the same element. + */ + toQualifiedResourcePath( + projectId: string, + databaseId: string + ): QualifiedResourcePath { + return new QualifiedResourcePath(projectId, databaseId, ...this.segments); + } + /** * Creates a resource path from the given slash-delimited string. If multiple * arguments are provided, all components are combined. Leading and trailing @@ -287,6 +313,183 @@ export class ResourcePath extends BasePath { } } +/** + * A slash-separated path that includes a project and database ID for referring + * to resources in any Firestore project. + * + * @private + * @internal + */ +export class QualifiedResourcePath extends ResourcePath { + /** + * The project ID of this path. + */ + readonly projectId: string; + + /** + * The database ID of this path. + */ + readonly databaseId: string; + + /** + * Constructs a Firestore Resource Path. + * + * @private + * @internal + * @param projectId The Firestore project id. + * @param databaseId The Firestore database id. + * @param segments Sequence of names of the parts of the path. + */ + constructor(projectId: string, databaseId: string, ...segments: string[]) { + super(segments); + + this.projectId = projectId; + this.databaseId = databaseId; + } + + /** + * String representation of the path relative to the database root. + * @private + * @internal + */ + get relativeName(): string { + return this.segments.join('/'); + } + + /** + * Creates a resource path from an absolute Firestore path. + * + * @private + * @internal + * @param absolutePath A string representation of a Resource Path. + * @returns The new ResourcePath. + */ + static fromSlashSeparatedString(absolutePath: string): QualifiedResourcePath { + const elements = RESOURCE_PATH_RE.exec(absolutePath); + + if (elements) { + const project = elements[1]; + const database = elements[2]; + const path = elements[3]; + return new QualifiedResourcePath(project, database).append(path); + } + + throw new Error(`Resource name '${absolutePath}' is not valid.`); + } + + /** + * Create a child path beneath the current level. + * + * @private + * @internal + * @param relativePath Relative path to append to the current path. + * @returns The new path. + */ + append(relativePath: ResourcePath | string): QualifiedResourcePath { + // `super.append()` calls `QualifiedResourcePath.construct()` when invoked + // from here and returns a QualifiedResourcePath. + return super.append(relativePath) as QualifiedResourcePath; + } + + /** + * Create a child path beneath the current level. + * + * @private + * @internal + * @returns The new path. + */ + parent(): QualifiedResourcePath | null { + return super.parent() as QualifiedResourcePath | null; + } + + /** + * String representation of a ResourcePath as expected by the API. + * + * @private + * @internal + * @returns The representation as expected by the API. + */ + get formattedName(): string { + const components = [ + 'projects', + this.projectId, + 'databases', + this.databaseId, + 'documents', + ...this.segments + ]; + return components.join('/'); + } + + /** + * Constructs a new instance of ResourcePath. We need this instead of using + * the normal constructor because polymorphic 'this' doesn't work on static + * methods. + * + * @private + * @internal + * @param segments Sequence of names of the parts of the path. + * @returns The newly created QualifiedResourcePath. + */ + construct(segments: string[]): QualifiedResourcePath { + return new QualifiedResourcePath( + this.projectId, + this.databaseId, + ...segments + ); + } + + /** + * Convenience method to match the ResourcePath API. This method always + * returns the current instance. + * + * @private + * @internal + */ + toQualifiedResourcePath(): QualifiedResourcePath { + return this; + } + + /** + * Compare the current path against another ResourcePath object. + * + * @private + * @internal + * @param other The path to compare to. + * @returns -1 if current < other, 1 if current > other, 0 if equal + */ + compareTo(other: ResourcePath): number { + if (other instanceof QualifiedResourcePath) { + if (this.projectId < other.projectId) { + return -1; + } + if (this.projectId > other.projectId) { + return 1; + } + + if (this.databaseId < other.databaseId) { + return -1; + } + if (this.databaseId > other.databaseId) { + return 1; + } + } + + return super.compareTo(other); + } + + /** + * Converts this ResourcePath to the Firestore Proto representation. + * @private + * @internal + */ + toProto(): api.IValue { + return { + referenceValue: this.formattedName + }; + } +} + const identifierRegExp = /^[_a-zA-Z][_a-zA-Z0-9]*$/; /** diff --git a/packages/firestore/src/util/bundle_builder.ts b/packages/firestore/src/util/bundle_builder.ts new file mode 100644 index 0000000000..20e80db163 --- /dev/null +++ b/packages/firestore/src/util/bundle_builder.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +import { + BundleElement, + BundleMetadata +} from '../protos/firestore_bundle_proto'; +import { JsonProtoSerializer } from '../remote/serializer'; +import { SizedBundleElement } from '../util/bundle_reader'; + + +/** + * A class that can serialize Firestore results into a Firestore bundle. + * + */ +export interface BundleBuilder { + serializer: JsonProtoSerializer; + + close(): Promise; + + getMetadata(): Promise; + + nextElement(): Promise; +} diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts new file mode 100644 index 0000000000..c279c08d2d --- /dev/null +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -0,0 +1,275 @@ +/** + * @license + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +import { Document } from '../protos/firestore_proto_api'; +import { + BundleElement as ProtoBundleElement, + BundleMetadata as ProtoBundleMetadata, + BundledDocumentMetadata as ProtoBundledDocumentMetadata, + NamedQuery as ProtoNamedQuery, +} from '../protos/firestore_bundle_proto'; + +import { DocumentSnapshot } from '../lite-api/snapshot'; +import { QuerySnapshot } from '../lite-api/snapshot'; +import { Timestamp } from '../lite-api/timestamp'; +import { queryToTarget } from '../../src/core/query'; + +import { + JsonProtoSerializer, + toDocument, + toName, + toQueryTarget, + toTimestamp, + toVersion, +} from '../../src/remote/serializer'; + +import { + invalidArgumentMessage, + validateMaxNumberOfArguments, + validateMinNumberOfArguments, + validateString, +} from './bundle_builder_validation_utils'; +import { DatabaseId } from '../core/database_info'; + + + +//import api = google.firestore.v1; + + +const BUNDLE_VERSION = 1; + +/** + * Builds a Firestore data bundle with results from the given document and query snapshots. + */ +export class BundleBuilder { + + // Resulting documents for the bundle, keyed by full document path. + private documents: Map = new Map(); + // Named queries saved in the bundle, keyed by query name. + private namedQueries: Map = new Map(); + + // The latest read time among all bundled documents and queries. + private latestReadTime = new Timestamp(0, 0); + + constructor(readonly bundleId: string) {} + + add(databaseId: DatabaseId, documentSnapshot: DocumentSnapshot): BundleBuilder; + add(databaseId: DatabaseId, queryName: string, querySnapshot: QuerySnapshot): BundleBuilder; + + /** + * Adds a Firestore document snapshot or query snapshot to the bundle. + * Both the documents data and the query read time will be included in the bundle. + * + * @param {DocumentSnapshot | string} documentOrName A document snapshot to add or a name of a query. + * @param {Query=} querySnapshot A query snapshot to add to the bundle, if provided. + * @returns {BundleBuilder} This instance. + * + * @example + * ``` + * const bundle = firestore.bundle('data-bundle'); + * const docSnapshot = await firestore.doc('abc/123').get(); + * const querySnapshot = await firestore.collection('coll').get(); + * + * const bundleBuffer = bundle.add(docSnapshot) // Add a document + * .add('coll-query', querySnapshot) // Add a named query. + * .build() + * // Save `bundleBuffer` to CDN or stream it to clients. + * ``` + */ + add( + databaseId: DatabaseId, + documentOrName: DocumentSnapshot | string, + querySnapshot?: QuerySnapshot + ): BundleBuilder { + // eslint-disable-next-line prefer-rest-params + validateMinNumberOfArguments('BundleBuilder.add', arguments, 1); + // eslint-disable-next-line prefer-rest-params + validateMaxNumberOfArguments('BundleBuilder.add', arguments, 2); + if (arguments.length === 1) { + validateDocumentSnapshot('documentOrName', documentOrName); + this.addBundledDocument(databaseId, documentOrName as DocumentSnapshot); + } else { + validateString('documentOrName', documentOrName); + validateQuerySnapshot('querySnapshot', querySnapshot); + this.addNamedQuery(databaseId, documentOrName as string, querySnapshot!); + } + return this; + } + + private addBundledDocument(databaseId: DatabaseId, snap: DocumentSnapshot, queryName?: string): void { + // TODO: is this a valid shortcut out? + if(!snap._document || !snap._document.isValidDocument()) { + return; + } + const originalDocument = this.documents.get(snap.ref.path); + const originalQueries = originalDocument?.metadata.queries; + const mutableCopy = snap._document.mutableCopy(); + + // Update with document built from `snap` because it is newer. + const snapReadTime = snap.readTime; + if ( !originalDocument || + (!snapReadTime && !originalDocument.metadata.readTime) || + (snapReadTime && originalDocument.metadata.readTime! < snapReadTime) + ) { + const serializer = new JsonProtoSerializer(databaseId, /*useProto3Json=*/ false); + + this.documents.set(snap.ref.path, { + document: snap._document.isFoundDocument() ? toDocument(serializer, mutableCopy) : undefined, + metadata: { + name: toName(serializer, mutableCopy.key), + readTime: snapReadTime, + exists: snap.exists(), + }, + }); + } + + // Update `queries` to include both original and `queryName`. + const newDocument = this.documents.get(snap.ref.path)!; + newDocument.metadata.queries = originalQueries || []; + if (queryName) { + newDocument.metadata.queries!.push(queryName); + } + + const readTime = snap.readTime; + if (readTime && readTime > this.latestReadTime) { + this.latestReadTime = readTime; + } + } + + private addNamedQuery(databaseId: DatabaseId, name: string, querySnap: QuerySnapshot): void { + if (this.namedQueries.has(name)) { + throw new Error(`Query name conflict: ${name} has already been added.`); + } + + const serializer = new JsonProtoSerializer(databaseId, /*useProto3Json=*/ false); + const queryTarget = toQueryTarget(serializer, queryToTarget(querySnap.query._query)); + + for (const snap of querySnap.docs) { + this.addBundledDocument(databaseId, snap, name); + const readTime = snap.readTime; + if (readTime && readTime > this.latestReadTime) { + this.latestReadTime = readTime; + } + } + } + + /** + * Converts a IBundleElement to a Buffer whose content is the length prefixed JSON representation + * of the element. + * @private + * @internal + */ + private elementToLengthPrefixedBuffer( + bundleElement: ProtoBundleElement + ): Buffer { + // Convert to a valid proto message object then take its JSON representation. + // This take cares of stuff like converting internal byte array fields + // to Base64 encodings. + // We lazy-load the Proto file to reduce cold-start times. + const message = require('../protos/firestore_v1_proto_api') + .firestore.BundleElement.fromObject(bundleElement) + .toJSON(); + const buffer = Buffer.from(JSON.stringify(message), 'utf-8'); + const lengthBuffer = Buffer.from(buffer.length.toString()); + return Buffer.concat([lengthBuffer, buffer]); + } + + build(): Buffer { + + let bundleBuffer = Buffer.alloc(0); + + for (const namedQuery of this.namedQueries.values()) { + bundleBuffer = Buffer.concat([ + bundleBuffer, + this.elementToLengthPrefixedBuffer({namedQuery}), + ]); + } + + for (const bundledDocument of this.documents.values()) { + const documentMetadata: ProtoBundledDocumentMetadata = + bundledDocument.metadata; + + bundleBuffer = Buffer.concat([ + bundleBuffer, + this.elementToLengthPrefixedBuffer({documentMetadata}), + ]); + // Write to the bundle if document exists. + const document = bundledDocument.document; + if (document) { + bundleBuffer = Buffer.concat([ + bundleBuffer, + this.elementToLengthPrefixedBuffer({document}), + ]); + } + } + + const metadata: ProtoBundleMetadata = { + id: this.bundleId, + createTime: this.latestReadTime.toProto().timestampValue, + version: BUNDLE_VERSION, + totalDocuments: this.documents.size, + totalBytes: bundleBuffer.length, + }; + // Prepends the metadata element to the bundleBuffer: `bundleBuffer` is the second argument to `Buffer.concat`. + bundleBuffer = Buffer.concat([ + this.elementToLengthPrefixedBuffer({metadata}), + bundleBuffer, + ]); + return bundleBuffer; + } +} + +/** + * Convenient class to hold both the metadata and the actual content of a document to be bundled. + * @private + * @internal + */ +class BundledDocument { + constructor( + readonly metadata: ProtoBundledDocumentMetadata, + readonly document?: Document + ) {} +} + +/** + * Validates that 'value' is DocumentSnapshot. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + */ +function validateDocumentSnapshot(arg: string | number, value: unknown): void { + if (!(value instanceof DocumentSnapshot)) { + throw new Error(invalidArgumentMessage(arg, 'DocumentSnapshot')); + } +} + +/** + * Validates that 'value' is QuerySnapshot. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + */ +function validateQuerySnapshot(arg: string | number, value: unknown): void { + if (!(value instanceof QuerySnapshot)) { + throw new Error(invalidArgumentMessage(arg, 'QuerySnapshot')); + } +} \ No newline at end of file diff --git a/packages/firestore/src/util/bundle_builder_validation_utils.ts b/packages/firestore/src/util/bundle_builder_validation_utils.ts new file mode 100644 index 0000000000..03f5aa6fd9 --- /dev/null +++ b/packages/firestore/src/util/bundle_builder_validation_utils.ts @@ -0,0 +1,468 @@ +/** + * @license + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { FieldPath } from '../lite-api/field_path'; + +/** + * Options to allow argument omission. + * + * @private + * @internal + */ +export interface RequiredArgumentOptions { + optional?: boolean; +} + +/** + * Options to limit the range of numbers. + * + * @private + * @internal + */ +export interface NumericRangeOptions { + minValue?: number; + maxValue?: number; +} + +/** + * Determines whether `value` is a JavaScript function. + * + * @private + * @internal + */ +export function isFunction(value: unknown): boolean { + return typeof value === 'function'; +} + +/** + * Determines whether `value` is a JavaScript object. + * + * @private + * @internal + */ +export function isObject(value: unknown): value is {[k: string]: unknown} { + return Object.prototype.toString.call(value) === '[object Object]'; +} + +/** + * Generates an error message to use with custom objects that cannot be + * serialized. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The value that failed serialization. + * @param path The field path that the object is assigned to. + */ +export function customObjectMessage( + arg: string | number, + value: unknown, + path?: FieldPath +): string { + const fieldPathMessage = path ? ` (found in field "${path}")` : ''; + + if (isObject(value)) { + // We use the base class name as the type name as the sentinel classes + // returned by the public FieldValue API are subclasses of FieldValue. By + // using the base name, we reduce the number of special cases below. + const typeName = value.constructor.name; + switch (typeName) { + case 'DocumentReference': + case 'FieldPath': + case 'FieldValue': + case 'GeoPoint': + case 'Timestamp': + return ( + `${invalidArgumentMessage( + arg, + 'Firestore document' + )} Detected an object of type "${typeName}" that doesn't match the ` + + `expected instance${fieldPathMessage}. Please ensure that the ` + + 'Firestore types you are using are from the same NPM package.)' + ); + case 'Object': + return `${invalidArgumentMessage( + arg, + 'Firestore document' + )} Invalid use of type "${typeof value}" as a Firestore argument${fieldPathMessage}.`; + default: + return ( + `${invalidArgumentMessage( + arg, + 'Firestore document' + )} Couldn't serialize object of type "${typeName}"${fieldPathMessage}. Firestore doesn't support JavaScript ` + + 'objects with custom prototypes (i.e. objects that were created ' + + 'via the "new" operator).' + ); + } + } else { + return `${invalidArgumentMessage( + arg, + 'Firestore document' + )} Input is not a plain JavaScript object${fieldPathMessage}.`; + } +} + +/** + * Validates that 'value' is a function. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the function can be omitted. + */ +export function validateFunction( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + if (!isFunction(value)) { + throw new Error(invalidArgumentMessage(arg, 'function')); + } + } +} + +/** + * Validates that 'value' is an object. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the object can be omitted. + */ +export function validateObject( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + if (!isObject(value)) { + throw new Error(invalidArgumentMessage(arg, 'object')); + } + } +} + +/** + * Validates that 'value' is a string. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the string can be omitted. + */ +export function validateString( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + if (typeof value !== 'string') { + throw new Error(invalidArgumentMessage(arg, 'string')); + } + } +} + +/** + * Validates that 'value' is a host. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the host can be omitted. + */ +export function validateHost( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + validateString(arg, value); + const urlString = `http://${value}/`; + let parsed; + try { + parsed = new URL(urlString); + } catch (e) { + throw new Error(invalidArgumentMessage(arg, 'host')); + } + + if ( + parsed.search !== '' || + parsed.pathname !== '/' || + parsed.username !== '' + ) { + throw new Error(invalidArgumentMessage(arg, 'host')); + } + } +} + +/** + * Validates that 'value' is a boolean. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the boolean can be omitted. + */ +export function validateBoolean( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + if (typeof value !== 'boolean') { + throw new Error(invalidArgumentMessage(arg, 'boolean')); + } + } +} + +/** + * Validates that 'value' is a number. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the number can be omitted. + */ +export function validateNumber( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions & NumericRangeOptions +): void { + const min = + options !== undefined && options.minValue !== undefined + ? options.minValue + : -Infinity; + const max = + options !== undefined && options.maxValue !== undefined + ? options.maxValue + : Infinity; + + if (!validateOptional(value, options)) { + if (typeof value !== 'number' || isNaN(value)) { + throw new Error(invalidArgumentMessage(arg, 'number')); + } else if (value < min || value > max) { + throw new Error( + `${formatArgumentName( + arg + )} must be within [${min}, ${max}] inclusive, but was: ${value}` + ); + } + } +} + +/** + * Validates that 'value' is a integer. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the integer can be omitted. + */ +export function validateInteger( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions & NumericRangeOptions +): void { + const min = + options !== undefined && options.minValue !== undefined + ? options.minValue + : -Infinity; + const max = + options !== undefined && options.maxValue !== undefined + ? options.maxValue + : Infinity; + + if (!validateOptional(value, options)) { + if (typeof value !== 'number' || isNaN(value) || value % 1 !== 0) { + throw new Error(invalidArgumentMessage(arg, 'integer')); + } else if (value < min || value > max) { + throw new Error( + `${formatArgumentName( + arg + )} must be within [${min}, ${max}] inclusive, but was: ${value}` + ); + } + } +} + +/** + * Validates that 'value' is a Timestamp. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the Timestamp can be omitted. + */ +export function validateTimestamp( + arg: string | number, + value: unknown, + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + if (!(value instanceof Timestamp)) { + throw new Error(invalidArgumentMessage(arg, 'Timestamp')); + } + } +} + +/** + * Generates an error message to use with invalid arguments. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param expectedType The expected input type. + */ +export function invalidArgumentMessage( + arg: string | number, + expectedType: string +): string { + return `${formatArgumentName(arg)} is not a valid ${expectedType}.`; +} + +/** + * Enforces the 'options.optional' constraint for 'value'. + * + * @private + * @internal + * @param value The input to validate. + * @param options Whether the function can be omitted. + * @return Whether the object is omitted and is allowed to be omitted. + */ +export function validateOptional( + value: unknown, + options?: RequiredArgumentOptions +): boolean { + return ( + value === undefined && options !== undefined && options.optional === true + ); +} + +/** + * Formats the given word as plural conditionally given the preceding number. + * + * @private + * @internal + * @param num The number to use for formatting. + * @param str The string to format. + */ +function formatPlural(num: number, str: string): string { + return `${num} ${str}` + (num === 1 ? '' : 's'); +} + +/** + * Creates a descriptive name for the provided argument name or index. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @return Either the argument name or its index description. + */ +function formatArgumentName(arg: string | number): string { + return typeof arg === 'string' + ? `Value for argument "${arg}"` + : `Element at index ${arg}`; +} + +/** + * Verifies that 'args' has at least 'minSize' elements. + * + * @private + * @internal + * @param funcName The function name to use in the error message. + * @param args The array (or array-like structure) to verify. + * @param minSize The minimum number of elements to enforce. + * @throws if the expectation is not met. + */ +export function validateMinNumberOfArguments( + funcName: string, + args: IArguments | unknown[], + minSize: number +): void { + if (args.length < minSize) { + throw new Error( + `Function "${funcName}()" requires at least ` + + `${formatPlural(minSize, 'argument')}.` + ); + } +} + +/** + * Verifies that 'args' has at most 'maxSize' elements. + * + * @private + * @internal + * @param funcName The function name to use in the error message. + * @param args The array (or array-like structure) to verify. + * @param maxSize The maximum number of elements to enforce. + * @throws if the expectation is not met. + */ +export function validateMaxNumberOfArguments( + funcName: string, + args: IArguments, + maxSize: number +): void { + if (args.length > maxSize) { + throw new Error( + `Function "${funcName}()" accepts at most ` + + `${formatPlural(maxSize, 'argument')}.` + ); + } +} + +/** + * Validates that the provided named option equals one of the expected values. + * + * @param arg The argument name or argument index (for varargs methods).). + * @param value The input to validate. + * @param allowedValues A list of expected values. + * @param options Whether the input can be omitted. + * @private + * @internal + */ +export function validateEnumValue( + arg: string | number, + value: unknown, + allowedValues: string[], + options?: RequiredArgumentOptions +): void { + if (!validateOptional(value, options)) { + const expectedDescription: string[] = []; + + for (const allowed of allowedValues) { + if (allowed === value) { + return; + } + expectedDescription.push(allowed); + } + + throw new Error( + `${formatArgumentName( + arg + )} is invalid. Acceptable values are: ${expectedDescription.join(', ')}` + ); + } +} \ No newline at end of file From c18ddd5b6cf99c37627f08b05b534e1afd01d32b Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Sat, 15 Mar 2025 11:39:27 -0400 Subject: [PATCH 02/32] Build fixes General type errors. Removed QualifiedResourcePath. --- common/api-review/firestore-lite.api.md | 4 + common/api-review/firestore.api.md | 4 + packages/firestore/src/lite-api/snapshot.ts | 4 - packages/firestore/src/model/path.ts | 192 ------------------ .../firestore/src/util/bundle_builder_impl.ts | 2 +- .../util/bundle_builder_validation_utils.ts | 1 + 6 files changed, 10 insertions(+), 197 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 4a9ef4c017..da2833882d 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -146,10 +146,14 @@ export class DocumentReference { protected constructor(); + // (undocumented) + get createTime(): Timestamp | undefined; data(): AppModelType | undefined; exists(): this is QueryDocumentSnapshot; get(fieldPath: string | FieldPath): any; get id(): string; + // (undocumented) + get readTime(): Timestamp | undefined; get ref(): DocumentReference; } diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 34b56b97f2..1601d9d364 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -172,11 +172,15 @@ export class DocumentReference { protected constructor(); + // (undocumented) + get createTime(): Timestamp; data(options?: SnapshotOptions): AppModelType | undefined; exists(): this is QueryDocumentSnapshot; get(fieldPath: string | FieldPath, options?: SnapshotOptions): any; get id(): string; readonly metadata: SnapshotMetadata; + // (undocumented) + get readTime(): Timestamp; get ref(): DocumentReference; } diff --git a/packages/firestore/src/lite-api/snapshot.ts b/packages/firestore/src/lite-api/snapshot.ts index 4da2d9babd..b2dc6182a8 100644 --- a/packages/firestore/src/lite-api/snapshot.ts +++ b/packages/firestore/src/lite-api/snapshot.ts @@ -334,10 +334,6 @@ export class DocumentSnapshot< return this._document?.createTime.toTimestamp(); } - get updateTime(): Timestamp | undefined { - return this._document?.updateTime.toTimestamp(); - } - get readTime(): Timestamp | undefined { return this._document?.readTime.toTimestamp(); } diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts index ddd5c1844e..16f209d65b 100644 --- a/packages/firestore/src/model/path.ts +++ b/packages/firestore/src/model/path.ts @@ -268,21 +268,6 @@ export class ResourcePath extends BasePath { return this.toArray().map(encodeURIComponent).join('/'); } - /** - * Converts this path to a fully qualified ResourcePath. - * - * @private - * @internal - * @param projectId The project ID of the current Firestore project. - * @return A fully-qualified resource path pointing to the same element. - */ - toQualifiedResourcePath( - projectId: string, - databaseId: string - ): QualifiedResourcePath { - return new QualifiedResourcePath(projectId, databaseId, ...this.segments); - } - /** * Creates a resource path from the given slash-delimited string. If multiple * arguments are provided, all components are combined. Leading and trailing @@ -313,183 +298,6 @@ export class ResourcePath extends BasePath { } } -/** - * A slash-separated path that includes a project and database ID for referring - * to resources in any Firestore project. - * - * @private - * @internal - */ -export class QualifiedResourcePath extends ResourcePath { - /** - * The project ID of this path. - */ - readonly projectId: string; - - /** - * The database ID of this path. - */ - readonly databaseId: string; - - /** - * Constructs a Firestore Resource Path. - * - * @private - * @internal - * @param projectId The Firestore project id. - * @param databaseId The Firestore database id. - * @param segments Sequence of names of the parts of the path. - */ - constructor(projectId: string, databaseId: string, ...segments: string[]) { - super(segments); - - this.projectId = projectId; - this.databaseId = databaseId; - } - - /** - * String representation of the path relative to the database root. - * @private - * @internal - */ - get relativeName(): string { - return this.segments.join('/'); - } - - /** - * Creates a resource path from an absolute Firestore path. - * - * @private - * @internal - * @param absolutePath A string representation of a Resource Path. - * @returns The new ResourcePath. - */ - static fromSlashSeparatedString(absolutePath: string): QualifiedResourcePath { - const elements = RESOURCE_PATH_RE.exec(absolutePath); - - if (elements) { - const project = elements[1]; - const database = elements[2]; - const path = elements[3]; - return new QualifiedResourcePath(project, database).append(path); - } - - throw new Error(`Resource name '${absolutePath}' is not valid.`); - } - - /** - * Create a child path beneath the current level. - * - * @private - * @internal - * @param relativePath Relative path to append to the current path. - * @returns The new path. - */ - append(relativePath: ResourcePath | string): QualifiedResourcePath { - // `super.append()` calls `QualifiedResourcePath.construct()` when invoked - // from here and returns a QualifiedResourcePath. - return super.append(relativePath) as QualifiedResourcePath; - } - - /** - * Create a child path beneath the current level. - * - * @private - * @internal - * @returns The new path. - */ - parent(): QualifiedResourcePath | null { - return super.parent() as QualifiedResourcePath | null; - } - - /** - * String representation of a ResourcePath as expected by the API. - * - * @private - * @internal - * @returns The representation as expected by the API. - */ - get formattedName(): string { - const components = [ - 'projects', - this.projectId, - 'databases', - this.databaseId, - 'documents', - ...this.segments - ]; - return components.join('/'); - } - - /** - * Constructs a new instance of ResourcePath. We need this instead of using - * the normal constructor because polymorphic 'this' doesn't work on static - * methods. - * - * @private - * @internal - * @param segments Sequence of names of the parts of the path. - * @returns The newly created QualifiedResourcePath. - */ - construct(segments: string[]): QualifiedResourcePath { - return new QualifiedResourcePath( - this.projectId, - this.databaseId, - ...segments - ); - } - - /** - * Convenience method to match the ResourcePath API. This method always - * returns the current instance. - * - * @private - * @internal - */ - toQualifiedResourcePath(): QualifiedResourcePath { - return this; - } - - /** - * Compare the current path against another ResourcePath object. - * - * @private - * @internal - * @param other The path to compare to. - * @returns -1 if current < other, 1 if current > other, 0 if equal - */ - compareTo(other: ResourcePath): number { - if (other instanceof QualifiedResourcePath) { - if (this.projectId < other.projectId) { - return -1; - } - if (this.projectId > other.projectId) { - return 1; - } - - if (this.databaseId < other.databaseId) { - return -1; - } - if (this.databaseId > other.databaseId) { - return 1; - } - } - - return super.compareTo(other); - } - - /** - * Converts this ResourcePath to the Firestore Proto representation. - * @private - * @internal - */ - toProto(): api.IValue { - return { - referenceValue: this.formattedName - }; - } -} - const identifierRegExp = /^[_a-zA-Z][_a-zA-Z0-9]*$/; /** diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index c279c08d2d..9c756bedde 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -220,7 +220,7 @@ export class BundleBuilder { const metadata: ProtoBundleMetadata = { id: this.bundleId, - createTime: this.latestReadTime.toProto().timestampValue, + createTime: this.latestReadTime, version: BUNDLE_VERSION, totalDocuments: this.documents.size, totalBytes: bundleBuffer.length, diff --git a/packages/firestore/src/util/bundle_builder_validation_utils.ts b/packages/firestore/src/util/bundle_builder_validation_utils.ts index 03f5aa6fd9..1532414cf4 100644 --- a/packages/firestore/src/util/bundle_builder_validation_utils.ts +++ b/packages/firestore/src/util/bundle_builder_validation_utils.ts @@ -16,6 +16,7 @@ */ import { FieldPath } from '../lite-api/field_path'; +import { Timestamp } from '../lite-api/timestamp'; /** * Options to allow argument omission. From 9ebb63304fdf7933ad12e38fcfb22f994665ee53 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 17 Mar 2025 09:10:12 -0400 Subject: [PATCH 03/32] Add Firestore instance to constructor --- .../firestore/src/util/bundle_builder_impl.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 9c756bedde..b48e29031f 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -23,7 +23,7 @@ import { BundledDocumentMetadata as ProtoBundledDocumentMetadata, NamedQuery as ProtoNamedQuery, } from '../protos/firestore_bundle_proto'; - +import { Firestore } from '../api/database'; import { DocumentSnapshot } from '../lite-api/snapshot'; import { QuerySnapshot } from '../lite-api/snapshot'; import { Timestamp } from '../lite-api/timestamp'; @@ -34,8 +34,6 @@ import { toDocument, toName, toQueryTarget, - toTimestamp, - toVersion, } from '../../src/remote/serializer'; import { @@ -66,10 +64,14 @@ export class BundleBuilder { // The latest read time among all bundled documents and queries. private latestReadTime = new Timestamp(0, 0); - constructor(readonly bundleId: string) {} + private databaseId: DatabaseId; - add(databaseId: DatabaseId, documentSnapshot: DocumentSnapshot): BundleBuilder; - add(databaseId: DatabaseId, queryName: string, querySnapshot: QuerySnapshot): BundleBuilder; + constructor(private firestore: Firestore, readonly bundleId: string) { + this.databaseId = firestore._databaseId; + } + + add(documentSnapshot: DocumentSnapshot): BundleBuilder; + add(queryName: string, querySnapshot: QuerySnapshot): BundleBuilder; /** * Adds a Firestore document snapshot or query snapshot to the bundle. @@ -92,7 +94,6 @@ export class BundleBuilder { * ``` */ add( - databaseId: DatabaseId, documentOrName: DocumentSnapshot | string, querySnapshot?: QuerySnapshot ): BundleBuilder { @@ -102,17 +103,17 @@ export class BundleBuilder { validateMaxNumberOfArguments('BundleBuilder.add', arguments, 2); if (arguments.length === 1) { validateDocumentSnapshot('documentOrName', documentOrName); - this.addBundledDocument(databaseId, documentOrName as DocumentSnapshot); + this.addBundledDocument(documentOrName as DocumentSnapshot); } else { validateString('documentOrName', documentOrName); validateQuerySnapshot('querySnapshot', querySnapshot); - this.addNamedQuery(databaseId, documentOrName as string, querySnapshot!); + this.addNamedQuery(documentOrName as string, querySnapshot!); } return this; } - private addBundledDocument(databaseId: DatabaseId, snap: DocumentSnapshot, queryName?: string): void { - // TODO: is this a valid shortcut out? + private addBundledDocument(snap: DocumentSnapshot, queryName?: string): void { + // TODO: is this a valid shortcircuit? if(!snap._document || !snap._document.isValidDocument()) { return; } @@ -126,7 +127,10 @@ export class BundleBuilder { (!snapReadTime && !originalDocument.metadata.readTime) || (snapReadTime && originalDocument.metadata.readTime! < snapReadTime) ) { - const serializer = new JsonProtoSerializer(databaseId, /*useProto3Json=*/ false); + + // TODO: Should I create on serializer for the bundler instance, or just created one adhoc + // like this? + const serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ false); this.documents.set(snap.ref.path, { document: snap._document.isFoundDocument() ? toDocument(serializer, mutableCopy) : undefined, @@ -151,16 +155,16 @@ export class BundleBuilder { } } - private addNamedQuery(databaseId: DatabaseId, name: string, querySnap: QuerySnapshot): void { + private addNamedQuery(name: string, querySnap: QuerySnapshot): void { if (this.namedQueries.has(name)) { throw new Error(`Query name conflict: ${name} has already been added.`); } - const serializer = new JsonProtoSerializer(databaseId, /*useProto3Json=*/ false); + const serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ false); const queryTarget = toQueryTarget(serializer, queryToTarget(querySnap.query._query)); for (const snap of querySnap.docs) { - this.addBundledDocument(databaseId, snap, name); + this.addBundledDocument(snap, name); const readTime = snap.readTime; if (readTime && readTime > this.latestReadTime) { this.latestReadTime = readTime; From 3c2e83719e90b898a3a00452696c628390526840 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 17 Mar 2025 10:13:41 -0400 Subject: [PATCH 04/32] lint fixes --- packages/firestore/src/lite-api/snapshot.ts | 2 +- packages/firestore/src/model/path.ts | 11 ----------- packages/firestore/src/util/bundle_builder.ts | 1 - 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/packages/firestore/src/lite-api/snapshot.ts b/packages/firestore/src/lite-api/snapshot.ts index b2dc6182a8..efff9987d7 100644 --- a/packages/firestore/src/lite-api/snapshot.ts +++ b/packages/firestore/src/lite-api/snapshot.ts @@ -17,6 +17,7 @@ import { Compat, getModularInstance } from '@firebase/util'; +import { Timestamp } from '../lite-api/timestamp'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { FieldPath as InternalFieldPath } from '../model/path'; @@ -38,7 +39,6 @@ import { UntypedFirestoreDataConverter } from './user_data_reader'; import { AbstractUserDataWriter } from './user_data_writer'; -import { Timestamp } from '../lite-api/timestamp'; /** * Converter used by `withConverter()` to transform user objects of type diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts index 16f209d65b..0a84ee5b83 100644 --- a/packages/firestore/src/model/path.ts +++ b/packages/firestore/src/model/path.ts @@ -22,17 +22,6 @@ import { Code, FirestoreError } from '../util/error'; export const DOCUMENT_KEY_NAME = '__name__'; -/*! - * A regular expression to verify an absolute Resource Path in Firestore. It - * extracts the project ID, the database name and the relative resource path - * if available. - * - * @type {RegExp} - */ -const RESOURCE_PATH_RE = - // Note: [\s\S] matches all characters including newlines. - /^projects\/([^/]*)\/databases\/([^/]*)(?:\/documents\/)?([\s\S]*)$/; - /** * Path represents an ordered sequence of string segments. */ diff --git a/packages/firestore/src/util/bundle_builder.ts b/packages/firestore/src/util/bundle_builder.ts index 20e80db163..4644061282 100644 --- a/packages/firestore/src/util/bundle_builder.ts +++ b/packages/firestore/src/util/bundle_builder.ts @@ -17,7 +17,6 @@ import { - BundleElement, BundleMetadata } from '../protos/firestore_bundle_proto'; import { JsonProtoSerializer } from '../remote/serializer'; From 71d24b44e27dd967c030dc8c6c3435d2c634242c Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 17 Mar 2025 14:48:52 -0400 Subject: [PATCH 05/32] first addQuery impl --- .../firestore/src/util/bundle_builder_impl.ts | 65 ++++++++++--------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index b48e29031f..27455d15b2 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -15,39 +15,29 @@ * limitations under the License. */ - -import { Document } from '../protos/firestore_proto_api'; -import { - BundleElement as ProtoBundleElement, - BundleMetadata as ProtoBundleMetadata, - BundledDocumentMetadata as ProtoBundledDocumentMetadata, - NamedQuery as ProtoNamedQuery, -} from '../protos/firestore_bundle_proto'; -import { Firestore } from '../api/database'; -import { DocumentSnapshot } from '../lite-api/snapshot'; -import { QuerySnapshot } from '../lite-api/snapshot'; -import { Timestamp } from '../lite-api/timestamp'; import { queryToTarget } from '../../src/core/query'; - import { JsonProtoSerializer, toDocument, toName, toQueryTarget, } from '../../src/remote/serializer'; +import { Firestore } from '../api/database'; +import { DatabaseId } from '../core/database_info'; +import { DocumentSnapshot, QuerySnapshot } from '../lite-api/snapshot'; +import { Timestamp } from '../lite-api/timestamp'; +import { + BundleElement as ProtoBundleElement, + BundleMetadata as ProtoBundleMetadata, + BundledDocumentMetadata as ProtoBundledDocumentMetadata, + NamedQuery as ProtoNamedQuery, +} from '../protos/firestore_bundle_proto'; +import { Document } from '../protos/firestore_proto_api'; import { invalidArgumentMessage, - validateMaxNumberOfArguments, - validateMinNumberOfArguments, validateString, } from './bundle_builder_validation_utils'; -import { DatabaseId } from '../core/database_info'; - - - -//import api = google.firestore.v1; - const BUNDLE_VERSION = 1; @@ -97,10 +87,9 @@ export class BundleBuilder { documentOrName: DocumentSnapshot | string, querySnapshot?: QuerySnapshot ): BundleBuilder { - // eslint-disable-next-line prefer-rest-params - validateMinNumberOfArguments('BundleBuilder.add', arguments, 1); - // eslint-disable-next-line prefer-rest-params - validateMaxNumberOfArguments('BundleBuilder.add', arguments, 2); + if (arguments.length < 1 || arguments.length > 2) { + throw new Error( 'Function BundleBuilder.add() requires 1 or 2 arguments.'); + } if (arguments.length === 1) { validateDocumentSnapshot('documentOrName', documentOrName); this.addBundledDocument(documentOrName as DocumentSnapshot); @@ -162,14 +151,29 @@ export class BundleBuilder { const serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ false); const queryTarget = toQueryTarget(serializer, queryToTarget(querySnap.query._query)); - + + // TODO: if we can't resolve the query's readTime then can we set it to the latest + // of the document collection? + let latestReadTime = new Timestamp(0, 0); for (const snap of querySnap.docs) { - this.addBundledDocument(snap, name); const readTime = snap.readTime; - if (readTime && readTime > this.latestReadTime) { - this.latestReadTime = readTime; + if (readTime && readTime > latestReadTime) { + latestReadTime = readTime; } + this.addBundledDocument(snap, name); } + + const bundledQuery = { + parent: queryTarget.parent.canonicalString(), + structuredQuery: queryTarget.queryTarget.structuredQuery, + limitType: null + }; + + this.namedQueries.set(name, { + name, + bundledQuery, + readTime: latestReadTime + }); } /** @@ -184,7 +188,8 @@ export class BundleBuilder { // Convert to a valid proto message object then take its JSON representation. // This take cares of stuff like converting internal byte array fields // to Base64 encodings. - // We lazy-load the Proto file to reduce cold-start times. + + // TODO: This fails. BundleElement doesn't have a toJSON method. const message = require('../protos/firestore_v1_proto_api') .firestore.BundleElement.fromObject(bundleElement) .toJSON(); From 5e9ceb8c391c931777542144003914e5f37700be Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 17 Mar 2025 14:49:09 -0400 Subject: [PATCH 06/32] reduced tools in validation utils. --- .../util/bundle_builder_validation_utils.ts | 351 +----------------- 1 file changed, 3 insertions(+), 348 deletions(-) diff --git a/packages/firestore/src/util/bundle_builder_validation_utils.ts b/packages/firestore/src/util/bundle_builder_validation_utils.ts index 1532414cf4..4d303f906c 100644 --- a/packages/firestore/src/util/bundle_builder_validation_utils.ts +++ b/packages/firestore/src/util/bundle_builder_validation_utils.ts @@ -18,148 +18,6 @@ import { FieldPath } from '../lite-api/field_path'; import { Timestamp } from '../lite-api/timestamp'; -/** - * Options to allow argument omission. - * - * @private - * @internal - */ -export interface RequiredArgumentOptions { - optional?: boolean; -} - -/** - * Options to limit the range of numbers. - * - * @private - * @internal - */ -export interface NumericRangeOptions { - minValue?: number; - maxValue?: number; -} - -/** - * Determines whether `value` is a JavaScript function. - * - * @private - * @internal - */ -export function isFunction(value: unknown): boolean { - return typeof value === 'function'; -} - -/** - * Determines whether `value` is a JavaScript object. - * - * @private - * @internal - */ -export function isObject(value: unknown): value is {[k: string]: unknown} { - return Object.prototype.toString.call(value) === '[object Object]'; -} - -/** - * Generates an error message to use with custom objects that cannot be - * serialized. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The value that failed serialization. - * @param path The field path that the object is assigned to. - */ -export function customObjectMessage( - arg: string | number, - value: unknown, - path?: FieldPath -): string { - const fieldPathMessage = path ? ` (found in field "${path}")` : ''; - - if (isObject(value)) { - // We use the base class name as the type name as the sentinel classes - // returned by the public FieldValue API are subclasses of FieldValue. By - // using the base name, we reduce the number of special cases below. - const typeName = value.constructor.name; - switch (typeName) { - case 'DocumentReference': - case 'FieldPath': - case 'FieldValue': - case 'GeoPoint': - case 'Timestamp': - return ( - `${invalidArgumentMessage( - arg, - 'Firestore document' - )} Detected an object of type "${typeName}" that doesn't match the ` + - `expected instance${fieldPathMessage}. Please ensure that the ` + - 'Firestore types you are using are from the same NPM package.)' - ); - case 'Object': - return `${invalidArgumentMessage( - arg, - 'Firestore document' - )} Invalid use of type "${typeof value}" as a Firestore argument${fieldPathMessage}.`; - default: - return ( - `${invalidArgumentMessage( - arg, - 'Firestore document' - )} Couldn't serialize object of type "${typeName}"${fieldPathMessage}. Firestore doesn't support JavaScript ` + - 'objects with custom prototypes (i.e. objects that were created ' + - 'via the "new" operator).' - ); - } - } else { - return `${invalidArgumentMessage( - arg, - 'Firestore document' - )} Input is not a plain JavaScript object${fieldPathMessage}.`; - } -} - -/** - * Validates that 'value' is a function. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the function can be omitted. - */ -export function validateFunction( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - if (!isFunction(value)) { - throw new Error(invalidArgumentMessage(arg, 'function')); - } - } -} - -/** - * Validates that 'value' is an object. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the object can be omitted. - */ -export function validateObject( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - if (!isObject(value)) { - throw new Error(invalidArgumentMessage(arg, 'object')); - } - } -} - /** * Validates that 'value' is a string. * @@ -171,161 +29,10 @@ export function validateObject( */ export function validateString( arg: string | number, - value: unknown, - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - if (typeof value !== 'string') { - throw new Error(invalidArgumentMessage(arg, 'string')); - } - } -} - -/** - * Validates that 'value' is a host. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the host can be omitted. - */ -export function validateHost( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - validateString(arg, value); - const urlString = `http://${value}/`; - let parsed; - try { - parsed = new URL(urlString); - } catch (e) { - throw new Error(invalidArgumentMessage(arg, 'host')); - } - - if ( - parsed.search !== '' || - parsed.pathname !== '/' || - parsed.username !== '' - ) { - throw new Error(invalidArgumentMessage(arg, 'host')); - } - } -} - -/** - * Validates that 'value' is a boolean. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the boolean can be omitted. - */ -export function validateBoolean( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - if (typeof value !== 'boolean') { - throw new Error(invalidArgumentMessage(arg, 'boolean')); - } - } -} - -/** - * Validates that 'value' is a number. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the number can be omitted. - */ -export function validateNumber( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions & NumericRangeOptions + value: unknown ): void { - const min = - options !== undefined && options.minValue !== undefined - ? options.minValue - : -Infinity; - const max = - options !== undefined && options.maxValue !== undefined - ? options.maxValue - : Infinity; - - if (!validateOptional(value, options)) { - if (typeof value !== 'number' || isNaN(value)) { - throw new Error(invalidArgumentMessage(arg, 'number')); - } else if (value < min || value > max) { - throw new Error( - `${formatArgumentName( - arg - )} must be within [${min}, ${max}] inclusive, but was: ${value}` - ); - } - } -} - -/** - * Validates that 'value' is a integer. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the integer can be omitted. - */ -export function validateInteger( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions & NumericRangeOptions -): void { - const min = - options !== undefined && options.minValue !== undefined - ? options.minValue - : -Infinity; - const max = - options !== undefined && options.maxValue !== undefined - ? options.maxValue - : Infinity; - - if (!validateOptional(value, options)) { - if (typeof value !== 'number' || isNaN(value) || value % 1 !== 0) { - throw new Error(invalidArgumentMessage(arg, 'integer')); - } else if (value < min || value > max) { - throw new Error( - `${formatArgumentName( - arg - )} must be within [${min}, ${max}] inclusive, but was: ${value}` - ); - } - } -} - -/** - * Validates that 'value' is a Timestamp. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the Timestamp can be omitted. - */ -export function validateTimestamp( - arg: string | number, - value: unknown, - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - if (!(value instanceof Timestamp)) { - throw new Error(invalidArgumentMessage(arg, 'Timestamp')); - } + if (typeof value !== 'string') { + throw new Error(invalidArgumentMessage(arg, 'string')); } } @@ -344,24 +51,6 @@ export function invalidArgumentMessage( return `${formatArgumentName(arg)} is not a valid ${expectedType}.`; } -/** - * Enforces the 'options.optional' constraint for 'value'. - * - * @private - * @internal - * @param value The input to validate. - * @param options Whether the function can be omitted. - * @return Whether the object is omitted and is allowed to be omitted. - */ -export function validateOptional( - value: unknown, - options?: RequiredArgumentOptions -): boolean { - return ( - value === undefined && options !== undefined && options.optional === true - ); -} - /** * Formats the given word as plural conditionally given the preceding number. * @@ -432,38 +121,4 @@ export function validateMaxNumberOfArguments( `${formatPlural(maxSize, 'argument')}.` ); } -} - -/** - * Validates that the provided named option equals one of the expected values. - * - * @param arg The argument name or argument index (for varargs methods).). - * @param value The input to validate. - * @param allowedValues A list of expected values. - * @param options Whether the input can be omitted. - * @private - * @internal - */ -export function validateEnumValue( - arg: string | number, - value: unknown, - allowedValues: string[], - options?: RequiredArgumentOptions -): void { - if (!validateOptional(value, options)) { - const expectedDescription: string[] = []; - - for (const allowed of allowedValues) { - if (allowed === value) { - return; - } - expectedDescription.push(allowed); - } - - throw new Error( - `${formatArgumentName( - arg - )} is invalid. Acceptable values are: ${expectedDescription.join(', ')}` - ); - } } \ No newline at end of file From 5dd5ae48663bd3b2c0eaa6ea35b5ebf7dc96f257 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 18 Mar 2025 09:38:47 -0400 Subject: [PATCH 07/32] remove unneeded utils. --- .../firestore/src/util/bundle_builder_impl.ts | 5 +- .../util/bundle_builder_validation_utils.ts | 60 ------------------- 2 files changed, 1 insertion(+), 64 deletions(-) diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 27455d15b2..3bbb47efad 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -60,9 +60,6 @@ export class BundleBuilder { this.databaseId = firestore._databaseId; } - add(documentSnapshot: DocumentSnapshot): BundleBuilder; - add(queryName: string, querySnapshot: QuerySnapshot): BundleBuilder; - /** * Adds a Firestore document snapshot or query snapshot to the bundle. * Both the documents data and the query read time will be included in the bundle. @@ -168,7 +165,7 @@ export class BundleBuilder { structuredQuery: queryTarget.queryTarget.structuredQuery, limitType: null }; - + this.namedQueries.set(name, { name, bundledQuery, diff --git a/packages/firestore/src/util/bundle_builder_validation_utils.ts b/packages/firestore/src/util/bundle_builder_validation_utils.ts index 4d303f906c..440ecf9358 100644 --- a/packages/firestore/src/util/bundle_builder_validation_utils.ts +++ b/packages/firestore/src/util/bundle_builder_validation_utils.ts @@ -15,8 +15,6 @@ * limitations under the License. */ -import { FieldPath } from '../lite-api/field_path'; -import { Timestamp } from '../lite-api/timestamp'; /** * Validates that 'value' is a string. @@ -51,18 +49,6 @@ export function invalidArgumentMessage( return `${formatArgumentName(arg)} is not a valid ${expectedType}.`; } -/** - * Formats the given word as plural conditionally given the preceding number. - * - * @private - * @internal - * @param num The number to use for formatting. - * @param str The string to format. - */ -function formatPlural(num: number, str: string): string { - return `${num} ${str}` + (num === 1 ? '' : 's'); -} - /** * Creates a descriptive name for the provided argument name or index. * @@ -76,49 +62,3 @@ function formatArgumentName(arg: string | number): string { ? `Value for argument "${arg}"` : `Element at index ${arg}`; } - -/** - * Verifies that 'args' has at least 'minSize' elements. - * - * @private - * @internal - * @param funcName The function name to use in the error message. - * @param args The array (or array-like structure) to verify. - * @param minSize The minimum number of elements to enforce. - * @throws if the expectation is not met. - */ -export function validateMinNumberOfArguments( - funcName: string, - args: IArguments | unknown[], - minSize: number -): void { - if (args.length < minSize) { - throw new Error( - `Function "${funcName}()" requires at least ` + - `${formatPlural(minSize, 'argument')}.` - ); - } -} - -/** - * Verifies that 'args' has at most 'maxSize' elements. - * - * @private - * @internal - * @param funcName The function name to use in the error message. - * @param args The array (or array-like structure) to verify. - * @param maxSize The maximum number of elements to enforce. - * @throws if the expectation is not met. - */ -export function validateMaxNumberOfArguments( - funcName: string, - args: IArguments, - maxSize: number -): void { - if (args.length > maxSize) { - throw new Error( - `Function "${funcName}()" accepts at most ` + - `${formatPlural(maxSize, 'argument')}.` - ); - } -} \ No newline at end of file From 3d45407004363c07ed6795171e67e5b246869bea Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 20 Mar 2025 13:32:41 -0400 Subject: [PATCH 08/32] Ideas to support converting bundle protos to Proto3 JSON compatible format (#8855) Co-authored-by: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> --- .../src/lite-api/user_data_reader.ts | 2 +- .../firestore/src/util/bundle_builder_impl.ts | 149 +++++++++++------- 2 files changed, 93 insertions(+), 58 deletions(-) diff --git a/packages/firestore/src/lite-api/user_data_reader.ts b/packages/firestore/src/lite-api/user_data_reader.ts index ebd4b49085..1cd6741dc3 100644 --- a/packages/firestore/src/lite-api/user_data_reader.ts +++ b/packages/firestore/src/lite-api/user_data_reader.ts @@ -778,7 +778,7 @@ export function parseData( } } -function parseObject( +export function parseObject( obj: Dict, context: ParseContextImpl ): { mapValue: ProtoMapValue } { diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 3bbb47efad..6eddc40f76 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -15,29 +15,43 @@ * limitations under the License. */ -import { queryToTarget } from '../../src/core/query'; +import {queryToTarget} from '../../src/core/query'; import { JsonProtoSerializer, toDocument, toName, toQueryTarget, + toTimestamp, } from '../../src/remote/serializer'; -import { Firestore } from '../api/database'; -import { DatabaseId } from '../core/database_info'; -import { DocumentSnapshot, QuerySnapshot } from '../lite-api/snapshot'; -import { Timestamp } from '../lite-api/timestamp'; +import {Firestore} from '../api/database'; +import {DatabaseId} from '../core/database_info'; +import {DocumentSnapshot, QuerySnapshot} from '../lite-api/snapshot'; +import {Timestamp} from '../lite-api/timestamp'; import { + BundledDocumentMetadata as ProtoBundledDocumentMetadata, BundleElement as ProtoBundleElement, BundleMetadata as ProtoBundleMetadata, - BundledDocumentMetadata as ProtoBundledDocumentMetadata, NamedQuery as ProtoNamedQuery, } from '../protos/firestore_bundle_proto'; -import { Document } from '../protos/firestore_proto_api'; +import { + Document as ProtoDocument, + Document +} from '../protos/firestore_proto_api'; import { invalidArgumentMessage, validateString, } from './bundle_builder_validation_utils'; +import {encoder} from "../../test/unit/util/bundle_data"; +import { + parseData, parseObject, + UserDataReader, + UserDataSource +} from "../lite-api/user_data_reader"; +import {AbstractUserDataWriter} from "../lite-api/user_data_writer"; +import {ExpUserDataWriter} from "../api/reference_impl"; +import {MutableDocument} from "../model/document"; +import {debugAssert} from "./assert"; const BUNDLE_VERSION = 1; @@ -45,7 +59,7 @@ const BUNDLE_VERSION = 1; * Builds a Firestore data bundle with results from the given document and query snapshots. */ export class BundleBuilder { - + // Resulting documents for the bundle, keyed by full document path. private documents: Map = new Map(); // Named queries saved in the bundle, keyed by query name. @@ -56,10 +70,21 @@ export class BundleBuilder { private databaseId: DatabaseId; + private readonly serializer: JsonProtoSerializer; + private readonly userDataReader: UserDataReader; + private readonly userDataWriter: AbstractUserDataWriter; + constructor(private firestore: Firestore, readonly bundleId: string) { this.databaseId = firestore._databaseId; + + // useProto3Json is true because the objects will be serialized to JSON string + // before being written to the bundle buffer. + this.serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ true); + + this.userDataWriter = new ExpUserDataWriter(firestore); + this.userDataReader = new UserDataReader(this.databaseId, true, this.serializer); } - + /** * Adds a Firestore document snapshot or query snapshot to the bundle. * Both the documents data and the query read time will be included in the bundle. @@ -97,7 +122,34 @@ export class BundleBuilder { } return this; } - + + toBundleDocument( + document: MutableDocument + ): ProtoDocument { + // TODO handle documents that have mutations + debugAssert( + !document.hasLocalMutations, + "Can't serialize documents with mutations." + ); + + // Convert document fields proto to DocumentData and then back + // to Proto3 JSON objects. This is the same approach used in + // bundling in the nodejs-firestore SDK. It may not be the most + // performant approach. + const documentData = this.userDataWriter.convertObjectMap(document.data.value.mapValue.fields, 'previous'); + // a parse context is typically used for validating and parsing user data, but in this + // case we are using it internally to convert DocumentData to Proto3 JSON + const context = this.userDataReader.createContext(UserDataSource.ArrayArgument, 'internal toBundledDocument'); + const proto3Fields = parseObject(documentData, context); + + return { + name: toName(this.serializer, document.key), + fields: proto3Fields.mapValue.fields, + updateTime: toTimestamp(this.serializer, document.version.toTimestamp()), + createTime: toTimestamp(this.serializer, document.createTime.toTimestamp()) + }; + } + private addBundledDocument(snap: DocumentSnapshot, queryName?: string): void { // TODO: is this a valid shortcircuit? if(!snap._document || !snap._document.isValidDocument()) { @@ -114,15 +166,11 @@ export class BundleBuilder { (snapReadTime && originalDocument.metadata.readTime! < snapReadTime) ) { - // TODO: Should I create on serializer for the bundler instance, or just created one adhoc - // like this? - const serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ false); - this.documents.set(snap.ref.path, { - document: snap._document.isFoundDocument() ? toDocument(serializer, mutableCopy) : undefined, + document: snap._document.isFoundDocument() ? this.toBundleDocument(mutableCopy) : undefined, metadata: { - name: toName(serializer, mutableCopy.key), - readTime: snapReadTime, + name: toName(this.serializer, mutableCopy.key), + readTime: !!snapReadTime ? toTimestamp(this.serializer, snapReadTime) : undefined, exists: snap.exists(), }, }); @@ -145,10 +193,8 @@ export class BundleBuilder { if (this.namedQueries.has(name)) { throw new Error(`Query name conflict: ${name} has already been added.`); } + const queryTarget = toQueryTarget(this.serializer, queryToTarget(querySnap.query._query)); - const serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ false); - const queryTarget = toQueryTarget(serializer, queryToTarget(querySnap.query._query)); - // TODO: if we can't resolve the query's readTime then can we set it to the latest // of the document collection? let latestReadTime = new Timestamp(0, 0); @@ -169,7 +215,7 @@ export class BundleBuilder { this.namedQueries.set(name, { name, bundledQuery, - readTime: latestReadTime + readTime: toTimestamp(this.serializer, latestReadTime) }); } @@ -178,65 +224,54 @@ export class BundleBuilder { * of the element. * @private * @internal + * @param bundleElement A ProtoBundleElement that is expected to be Proto3 JSON compatible. */ - private elementToLengthPrefixedBuffer( + private lengthPrefixedString( bundleElement: ProtoBundleElement - ): Buffer { - // Convert to a valid proto message object then take its JSON representation. - // This take cares of stuff like converting internal byte array fields - // to Base64 encodings. - - // TODO: This fails. BundleElement doesn't have a toJSON method. - const message = require('../protos/firestore_v1_proto_api') - .firestore.BundleElement.fromObject(bundleElement) - .toJSON(); - const buffer = Buffer.from(JSON.stringify(message), 'utf-8'); - const lengthBuffer = Buffer.from(buffer.length.toString()); - return Buffer.concat([lengthBuffer, buffer]); + ): string { + const str = JSON.stringify(bundleElement); + // TODO: it's not ideal to have to re-encode all of these strings multiple times + // It may be more performant to return a UInt8Array that is concatenated to other + // UInt8Arrays instead of returning and concatenating strings and then + // converting the full string to UInt8Array. + const l = encoder.encode(str).byteLength; + return `${l}${str}`; } - build(): Buffer { - - let bundleBuffer = Buffer.alloc(0); + build(): Uint8Array { + let bundleString = ''; for (const namedQuery of this.namedQueries.values()) { - bundleBuffer = Buffer.concat([ - bundleBuffer, - this.elementToLengthPrefixedBuffer({namedQuery}), - ]); + bundleString += this.lengthPrefixedString({namedQuery}); } for (const bundledDocument of this.documents.values()) { const documentMetadata: ProtoBundledDocumentMetadata = bundledDocument.metadata; - bundleBuffer = Buffer.concat([ - bundleBuffer, - this.elementToLengthPrefixedBuffer({documentMetadata}), - ]); + bundleString += this.lengthPrefixedString({documentMetadata}); // Write to the bundle if document exists. const document = bundledDocument.document; if (document) { - bundleBuffer = Buffer.concat([ - bundleBuffer, - this.elementToLengthPrefixedBuffer({document}), - ]); + bundleString += this.lengthPrefixedString({document}); } } const metadata: ProtoBundleMetadata = { id: this.bundleId, - createTime: this.latestReadTime, + createTime: toTimestamp(this.serializer, this.latestReadTime), version: BUNDLE_VERSION, totalDocuments: this.documents.size, - totalBytes: bundleBuffer.length, + // TODO: it's not ideal to have to re-encode all of these strings multiple times + totalBytes: encoder.encode(bundleString).length, }; // Prepends the metadata element to the bundleBuffer: `bundleBuffer` is the second argument to `Buffer.concat`. - bundleBuffer = Buffer.concat([ - this.elementToLengthPrefixedBuffer({metadata}), - bundleBuffer, - ]); - return bundleBuffer; + bundleString = this.lengthPrefixedString({metadata}) + bundleString; + + // TODO: it's not ideal to have to re-encode all of these strings multiple times + // the implementation in nodejs-firestore concatenates Buffers instead of + // concatenating strings. + return encoder.encode(bundleString); } } @@ -278,4 +313,4 @@ function validateQuerySnapshot(arg: string | number, value: unknown): void { if (!(value instanceof QuerySnapshot)) { throw new Error(invalidArgumentMessage(arg, 'QuerySnapshot')); } -} \ No newline at end of file +} From c36196332d72d6f650875b66891fa49ce5faf9d8 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 20 Mar 2025 16:12:39 -0400 Subject: [PATCH 09/32] Format and coalesce utils. --- packages/firestore/src/util/bundle_builder.ts | 38 ---- .../firestore/src/util/bundle_builder_impl.ts | 170 ++++++++++++------ .../util/bundle_builder_validation_utils.ts | 6 +- 3 files changed, 115 insertions(+), 99 deletions(-) delete mode 100644 packages/firestore/src/util/bundle_builder.ts diff --git a/packages/firestore/src/util/bundle_builder.ts b/packages/firestore/src/util/bundle_builder.ts deleted file mode 100644 index 4644061282..0000000000 --- a/packages/firestore/src/util/bundle_builder.ts +++ /dev/null @@ -1,38 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - - -import { - BundleMetadata -} from '../protos/firestore_bundle_proto'; -import { JsonProtoSerializer } from '../remote/serializer'; -import { SizedBundleElement } from '../util/bundle_reader'; - - -/** - * A class that can serialize Firestore results into a Firestore bundle. - * - */ -export interface BundleBuilder { - serializer: JsonProtoSerializer; - - close(): Promise; - - getMetadata(): Promise; - - nextElement(): Promise; -} diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 6eddc40f76..c9fb432ad2 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -15,43 +15,38 @@ * limitations under the License. */ -import {queryToTarget} from '../../src/core/query'; +import { queryToTarget } from '../../src/core/query'; import { JsonProtoSerializer, - toDocument, toName, toQueryTarget, - toTimestamp, + toTimestamp } from '../../src/remote/serializer'; -import {Firestore} from '../api/database'; -import {DatabaseId} from '../core/database_info'; -import {DocumentSnapshot, QuerySnapshot} from '../lite-api/snapshot'; -import {Timestamp} from '../lite-api/timestamp'; +import { encoder } from '../../test/unit/util/bundle_data'; +import { Firestore } from '../api/database'; +import { ExpUserDataWriter } from '../api/reference_impl'; +import { DatabaseId } from '../core/database_info'; +import { DocumentSnapshot, QuerySnapshot } from '../lite-api/snapshot'; +import { Timestamp } from '../lite-api/timestamp'; +import { + parseObject, + UserDataReader, + UserDataSource +} from '../lite-api/user_data_reader'; +import { AbstractUserDataWriter } from '../lite-api/user_data_writer'; +import { MutableDocument } from '../model/document'; import { BundledDocumentMetadata as ProtoBundledDocumentMetadata, BundleElement as ProtoBundleElement, BundleMetadata as ProtoBundleMetadata, - NamedQuery as ProtoNamedQuery, + NamedQuery as ProtoNamedQuery } from '../protos/firestore_bundle_proto'; import { Document as ProtoDocument, Document } from '../protos/firestore_proto_api'; -import { - invalidArgumentMessage, - validateString, -} from './bundle_builder_validation_utils'; -import {encoder} from "../../test/unit/util/bundle_data"; -import { - parseData, parseObject, - UserDataReader, - UserDataSource -} from "../lite-api/user_data_reader"; -import {AbstractUserDataWriter} from "../lite-api/user_data_writer"; -import {ExpUserDataWriter} from "../api/reference_impl"; -import {MutableDocument} from "../model/document"; -import {debugAssert} from "./assert"; +import { debugAssert } from './assert'; const BUNDLE_VERSION = 1; @@ -59,7 +54,6 @@ const BUNDLE_VERSION = 1; * Builds a Firestore data bundle with results from the given document and query snapshots. */ export class BundleBuilder { - // Resulting documents for the bundle, keyed by full document path. private documents: Map = new Map(); // Named queries saved in the bundle, keyed by query name. @@ -79,10 +73,17 @@ export class BundleBuilder { // useProto3Json is true because the objects will be serialized to JSON string // before being written to the bundle buffer. - this.serializer = new JsonProtoSerializer(this.databaseId, /*useProto3Json=*/ true); + this.serializer = new JsonProtoSerializer( + this.databaseId, + /*useProto3Json=*/ true + ); this.userDataWriter = new ExpUserDataWriter(firestore); - this.userDataReader = new UserDataReader(this.databaseId, true, this.serializer); + this.userDataReader = new UserDataReader( + this.databaseId, + true, + this.serializer + ); } /** @@ -110,7 +111,9 @@ export class BundleBuilder { querySnapshot?: QuerySnapshot ): BundleBuilder { if (arguments.length < 1 || arguments.length > 2) { - throw new Error( 'Function BundleBuilder.add() requires 1 or 2 arguments.'); + throw new Error( + 'Function BundleBuilder.add() requires 1 or 2 arguments.' + ); } if (arguments.length === 1) { validateDocumentSnapshot('documentOrName', documentOrName); @@ -123,9 +126,7 @@ export class BundleBuilder { return this; } - toBundleDocument( - document: MutableDocument - ): ProtoDocument { + toBundleDocument(document: MutableDocument): ProtoDocument { // TODO handle documents that have mutations debugAssert( !document.hasLocalMutations, @@ -136,23 +137,35 @@ export class BundleBuilder { // to Proto3 JSON objects. This is the same approach used in // bundling in the nodejs-firestore SDK. It may not be the most // performant approach. - const documentData = this.userDataWriter.convertObjectMap(document.data.value.mapValue.fields, 'previous'); + const documentData = this.userDataWriter.convertObjectMap( + document.data.value.mapValue.fields, + 'previous' + ); // a parse context is typically used for validating and parsing user data, but in this // case we are using it internally to convert DocumentData to Proto3 JSON - const context = this.userDataReader.createContext(UserDataSource.ArrayArgument, 'internal toBundledDocument'); + const context = this.userDataReader.createContext( + UserDataSource.ArrayArgument, + 'internal toBundledDocument' + ); const proto3Fields = parseObject(documentData, context); return { name: toName(this.serializer, document.key), fields: proto3Fields.mapValue.fields, updateTime: toTimestamp(this.serializer, document.version.toTimestamp()), - createTime: toTimestamp(this.serializer, document.createTime.toTimestamp()) + createTime: toTimestamp( + this.serializer, + document.createTime.toTimestamp() + ) }; } private addBundledDocument(snap: DocumentSnapshot, queryName?: string): void { - // TODO: is this a valid shortcircuit? - if(!snap._document || !snap._document.isValidDocument()) { + if ( + !snap._document || + !snap._document.isValidDocument() || + !snap._document.isFoundDocument() + ) { return; } const originalDocument = this.documents.get(snap.ref.path); @@ -161,18 +174,20 @@ export class BundleBuilder { // Update with document built from `snap` because it is newer. const snapReadTime = snap.readTime; - if ( !originalDocument || - (!snapReadTime && !originalDocument.metadata.readTime) || - (snapReadTime && originalDocument.metadata.readTime! < snapReadTime) + if ( + !originalDocument || + (!snapReadTime && !originalDocument.metadata.readTime) || + (snapReadTime && originalDocument.metadata.readTime! < snapReadTime) ) { - this.documents.set(snap.ref.path, { - document: snap._document.isFoundDocument() ? this.toBundleDocument(mutableCopy) : undefined, + document: this.toBundleDocument(mutableCopy), metadata: { name: toName(this.serializer, mutableCopy.key), - readTime: !!snapReadTime ? toTimestamp(this.serializer, snapReadTime) : undefined, - exists: snap.exists(), - }, + readTime: !!snapReadTime + ? toTimestamp(this.serializer, snapReadTime) + : undefined, + exists: snap.exists() + } }); } @@ -183,20 +198,21 @@ export class BundleBuilder { newDocument.metadata.queries!.push(queryName); } - const readTime = snap.readTime; - if (readTime && readTime > this.latestReadTime) { - this.latestReadTime = readTime; + if (snapReadTime && snapReadTime > this.latestReadTime) { + this.latestReadTime = snapReadTime; } } + // TODO: remove this since we're not planning to serialize named queries. private addNamedQuery(name: string, querySnap: QuerySnapshot): void { if (this.namedQueries.has(name)) { throw new Error(`Query name conflict: ${name} has already been added.`); } - const queryTarget = toQueryTarget(this.serializer, queryToTarget(querySnap.query._query)); + const queryTarget = toQueryTarget( + this.serializer, + queryToTarget(querySnap.query._query) + ); - // TODO: if we can't resolve the query's readTime then can we set it to the latest - // of the document collection? let latestReadTime = new Timestamp(0, 0); for (const snap of querySnap.docs) { const readTime = snap.readTime; @@ -226,9 +242,7 @@ export class BundleBuilder { * @internal * @param bundleElement A ProtoBundleElement that is expected to be Proto3 JSON compatible. */ - private lengthPrefixedString( - bundleElement: ProtoBundleElement - ): string { + private lengthPrefixedString(bundleElement: ProtoBundleElement): string { const str = JSON.stringify(bundleElement); // TODO: it's not ideal to have to re-encode all of these strings multiple times // It may be more performant to return a UInt8Array that is concatenated to other @@ -242,18 +256,18 @@ export class BundleBuilder { let bundleString = ''; for (const namedQuery of this.namedQueries.values()) { - bundleString += this.lengthPrefixedString({namedQuery}); + bundleString += this.lengthPrefixedString({ namedQuery }); } for (const bundledDocument of this.documents.values()) { const documentMetadata: ProtoBundledDocumentMetadata = bundledDocument.metadata; - bundleString += this.lengthPrefixedString({documentMetadata}); + bundleString += this.lengthPrefixedString({ documentMetadata }); // Write to the bundle if document exists. const document = bundledDocument.document; if (document) { - bundleString += this.lengthPrefixedString({document}); + bundleString += this.lengthPrefixedString({ document }); } } @@ -263,10 +277,10 @@ export class BundleBuilder { version: BUNDLE_VERSION, totalDocuments: this.documents.size, // TODO: it's not ideal to have to re-encode all of these strings multiple times - totalBytes: encoder.encode(bundleString).length, + totalBytes: encoder.encode(bundleString).length }; // Prepends the metadata element to the bundleBuffer: `bundleBuffer` is the second argument to `Buffer.concat`. - bundleString = this.lengthPrefixedString({metadata}) + bundleString; + bundleString = this.lengthPrefixedString({ metadata }) + bundleString; // TODO: it's not ideal to have to re-encode all of these strings multiple times // the implementation in nodejs-firestore concatenates Buffers instead of @@ -314,3 +328,47 @@ function validateQuerySnapshot(arg: string | number, value: unknown): void { throw new Error(invalidArgumentMessage(arg, 'QuerySnapshot')); } } + +/** + * Validates that 'value' is a string. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param value The input to validate. + * @param options Options that specify whether the string can be omitted. + */ +export function validateString(arg: string | number, value: unknown): void { + if (typeof value !== 'string') { + throw new Error(invalidArgumentMessage(arg, 'string')); + } +} + +/** + * Generates an error message to use with invalid arguments. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @param expectedType The expected input type. + */ +export function invalidArgumentMessage( + arg: string | number, + expectedType: string +): string { + return `${formatArgumentName(arg)} is not a valid ${expectedType}.`; +} + +/** + * Creates a descriptive name for the provided argument name or index. + * + * @private + * @internal + * @param arg The argument name or argument index (for varargs methods). + * @return Either the argument name or its index description. + */ +function formatArgumentName(arg: string | number): string { + return typeof arg === 'string' + ? `Value for argument "${arg}"` + : `Element at index ${arg}`; +} diff --git a/packages/firestore/src/util/bundle_builder_validation_utils.ts b/packages/firestore/src/util/bundle_builder_validation_utils.ts index 440ecf9358..02195ee659 100644 --- a/packages/firestore/src/util/bundle_builder_validation_utils.ts +++ b/packages/firestore/src/util/bundle_builder_validation_utils.ts @@ -15,7 +15,6 @@ * limitations under the License. */ - /** * Validates that 'value' is a string. * @@ -25,10 +24,7 @@ * @param value The input to validate. * @param options Options that specify whether the string can be omitted. */ -export function validateString( - arg: string | number, - value: unknown -): void { +export function validateString(arg: string | number, value: unknown): void { if (typeof value !== 'string') { throw new Error(invalidArgumentMessage(arg, 'string')); } From a1e6a0b1188ea505c38502cb4d2633440f427cde Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 25 Mar 2025 13:20:44 -0400 Subject: [PATCH 10/32] Bundle DocumentSnaps with toJSON Tested the results in loadBundle with a document consisting of all data types, and it succeeded. --- common/api-review/firestore.api.md | 2 + packages/firestore/src/api/snapshot.ts | 44 ++++- .../firestore/src/util/bundle_builder_impl.ts | 172 +++++------------- 3 files changed, 87 insertions(+), 131 deletions(-) diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 1601d9d364..65bbd398d6 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -182,6 +182,8 @@ export class DocumentSnapshot; + // (undocumented) + toJSON(): object; } export { EmulatorMockTokenOptions } diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index 29e1616b61..564ecdc700 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -36,6 +36,7 @@ import { AbstractUserDataWriter } from '../lite-api/user_data_writer'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { debugAssert, fail } from '../util/assert'; +import { BundleBuilder, DocumentBundleData } from '../util/bundle_builder_impl'; import { Code, FirestoreError } from '../util/error'; import { Firestore } from './database'; @@ -496,6 +497,27 @@ export class DocumentSnapshot< } return undefined; } + + toJSON(): object { + if ( + !this._document || + !this._document.isValidDocument() || + !this._document.isFoundDocument() + ) { + return { bundle: '' }; + } + + const builder: BundleBuilder = new BundleBuilder(this._firestore, 'abc123'); + const documentData = this._userDataWriter.convertObjectMap( + this._document.data.value.mapValue.fields, + 'previous' + ); + + builder.addBundleDocument(DocumentToDocumentBundleData(this._firestore, this.ref.path, documentData, this._document)); + return { + bundle: builder.build() + }; + } } /** @@ -637,7 +659,7 @@ export class QuerySnapshot< throw new FirestoreError( Code.INVALID_ARGUMENT, 'To include metadata changes with your document changes, you must ' + - 'also pass { includeMetadataChanges:true } to onSnapshot().' + 'also pass { includeMetadataChanges:true } to onSnapshot().' ); } @@ -673,10 +695,10 @@ export function changesFromSnapshot< ); debugAssert( !lastDoc || - newQueryComparator(querySnapshot._snapshot.query)( - lastDoc, - change.doc - ) < 0, + newQueryComparator(querySnapshot._snapshot.query)( + lastDoc, + change.doc + ) < 0, 'Got added events in wrong order' ); const doc = new QueryDocumentSnapshot( @@ -790,3 +812,15 @@ export function snapshotEqual( return false; } + +function DocumentToDocumentBundleData(firestore: Firestore, path: string, documentData: DocumentData, document: Document): DocumentBundleData { + return { + documentData, + documentKey: document.mutableCopy().key, + documentPath: path, + documentExists: true, + createdTime: document.createTime.toTimestamp(), + readTime: document.readTime.toTimestamp(), + versionTime: document.version.toTimestamp() + }; +} \ No newline at end of file diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index c9fb432ad2..f65ae0ea88 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -15,26 +15,22 @@ * limitations under the License. */ -import { queryToTarget } from '../../src/core/query'; import { JsonProtoSerializer, toName, - toQueryTarget, toTimestamp } from '../../src/remote/serializer'; import { encoder } from '../../test/unit/util/bundle_data'; import { Firestore } from '../api/database'; -import { ExpUserDataWriter } from '../api/reference_impl'; import { DatabaseId } from '../core/database_info'; -import { DocumentSnapshot, QuerySnapshot } from '../lite-api/snapshot'; +import { DocumentData } from '../lite-api/reference'; import { Timestamp } from '../lite-api/timestamp'; import { parseObject, UserDataReader, UserDataSource } from '../lite-api/user_data_reader'; -import { AbstractUserDataWriter } from '../lite-api/user_data_writer'; -import { MutableDocument } from '../model/document'; +import { DocumentKey } from '../model/document_key'; import { BundledDocumentMetadata as ProtoBundledDocumentMetadata, BundleElement as ProtoBundleElement, @@ -66,7 +62,6 @@ export class BundleBuilder { private readonly serializer: JsonProtoSerializer; private readonly userDataReader: UserDataReader; - private readonly userDataWriter: AbstractUserDataWriter; constructor(private firestore: Firestore, readonly bundleId: string) { this.databaseId = firestore._databaseId; @@ -78,7 +73,6 @@ export class BundleBuilder { /*useProto3Json=*/ true ); - this.userDataWriter = new ExpUserDataWriter(firestore); this.userDataReader = new UserDataReader( this.databaseId, true, @@ -86,125 +80,64 @@ export class BundleBuilder { ); } - /** - * Adds a Firestore document snapshot or query snapshot to the bundle. - * Both the documents data and the query read time will be included in the bundle. - * - * @param {DocumentSnapshot | string} documentOrName A document snapshot to add or a name of a query. - * @param {Query=} querySnapshot A query snapshot to add to the bundle, if provided. - * @returns {BundleBuilder} This instance. - * - * @example - * ``` - * const bundle = firestore.bundle('data-bundle'); - * const docSnapshot = await firestore.doc('abc/123').get(); - * const querySnapshot = await firestore.collection('coll').get(); - * - * const bundleBuffer = bundle.add(docSnapshot) // Add a document - * .add('coll-query', querySnapshot) // Add a named query. - * .build() - * // Save `bundleBuffer` to CDN or stream it to clients. - * ``` - */ - add( - documentOrName: DocumentSnapshot | string, - querySnapshot?: QuerySnapshot - ): BundleBuilder { - if (arguments.length < 1 || arguments.length > 2) { - throw new Error( - 'Function BundleBuilder.add() requires 1 or 2 arguments.' - ); - } - if (arguments.length === 1) { - validateDocumentSnapshot('documentOrName', documentOrName); - this.addBundledDocument(documentOrName as DocumentSnapshot); - } else { - validateString('documentOrName', documentOrName); - validateQuerySnapshot('querySnapshot', querySnapshot); - this.addNamedQuery(documentOrName as string, querySnapshot!); - } - return this; - } - - toBundleDocument(document: MutableDocument): ProtoDocument { + toBundleDocument(docBundleData: DocumentBundleData): ProtoDocument { // TODO handle documents that have mutations debugAssert( - !document.hasLocalMutations, + !docBundleData.documentData.hasLocalMutations, "Can't serialize documents with mutations." ); - // Convert document fields proto to DocumentData and then back - // to Proto3 JSON objects. This is the same approach used in - // bundling in the nodejs-firestore SDK. It may not be the most - // performant approach. - const documentData = this.userDataWriter.convertObjectMap( - document.data.value.mapValue.fields, - 'previous' - ); // a parse context is typically used for validating and parsing user data, but in this // case we are using it internally to convert DocumentData to Proto3 JSON const context = this.userDataReader.createContext( UserDataSource.ArrayArgument, 'internal toBundledDocument' ); - const proto3Fields = parseObject(documentData, context); + const proto3Fields = parseObject(docBundleData.documentData, context); return { - name: toName(this.serializer, document.key), + name: toName(this.serializer, docBundleData.documentKey), fields: proto3Fields.mapValue.fields, - updateTime: toTimestamp(this.serializer, document.version.toTimestamp()), - createTime: toTimestamp( - this.serializer, - document.createTime.toTimestamp() - ) + updateTime: toTimestamp(this.serializer, docBundleData.versionTime), + createTime: toTimestamp(this.serializer, docBundleData.createdTime) }; } - private addBundledDocument(snap: DocumentSnapshot, queryName?: string): void { - if ( - !snap._document || - !snap._document.isValidDocument() || - !snap._document.isFoundDocument() - ) { - return; - } - const originalDocument = this.documents.get(snap.ref.path); + addBundleDocument(docBundleData: DocumentBundleData): void { + const originalDocument = this.documents.get(docBundleData.documentPath); const originalQueries = originalDocument?.metadata.queries; - const mutableCopy = snap._document.mutableCopy(); + const readTime = docBundleData.readTime; // Update with document built from `snap` because it is newer. - const snapReadTime = snap.readTime; if ( !originalDocument || - (!snapReadTime && !originalDocument.metadata.readTime) || - (snapReadTime && originalDocument.metadata.readTime! < snapReadTime) + (!readTime && !originalDocument.metadata.readTime) || + (readTime && originalDocument.metadata.readTime! < readTime) ) { - this.documents.set(snap.ref.path, { - document: this.toBundleDocument(mutableCopy), + this.documents.set(docBundleData.documentPath, { + document: this.toBundleDocument(docBundleData), metadata: { - name: toName(this.serializer, mutableCopy.key), - readTime: !!snapReadTime - ? toTimestamp(this.serializer, snapReadTime) + name: toName(this.serializer, docBundleData.documentKey), + readTime: !!readTime + ? toTimestamp(this.serializer, readTime) : undefined, - exists: snap.exists() + exists: docBundleData.documentExists } }); } // Update `queries` to include both original and `queryName`. - const newDocument = this.documents.get(snap.ref.path)!; + const newDocument = this.documents.get(docBundleData.documentPath)!; newDocument.metadata.queries = originalQueries || []; - if (queryName) { - newDocument.metadata.queries!.push(queryName); + if (docBundleData.queryName) { + newDocument.metadata.queries!.push(docBundleData.queryName); } - - if (snapReadTime && snapReadTime > this.latestReadTime) { - this.latestReadTime = snapReadTime; + if (readTime && readTime > this.latestReadTime) { + this.latestReadTime = readTime; } } - // TODO: remove this since we're not planning to serialize named queries. - private addNamedQuery(name: string, querySnap: QuerySnapshot): void { + /*private addNamedQuery(name: string, querySnap: QuerySnapshot): void { if (this.namedQueries.has(name)) { throw new Error(`Query name conflict: ${name} has already been added.`); } @@ -233,7 +166,7 @@ export class BundleBuilder { bundledQuery, readTime: toTimestamp(this.serializer, latestReadTime) }); - } + } */ /** * Converts a IBundleElement to a Buffer whose content is the length prefixed JSON representation @@ -252,7 +185,7 @@ export class BundleBuilder { return `${l}${str}`; } - build(): Uint8Array { + build(): string { let bundleString = ''; for (const namedQuery of this.namedQueries.values()) { @@ -282,51 +215,38 @@ export class BundleBuilder { // Prepends the metadata element to the bundleBuffer: `bundleBuffer` is the second argument to `Buffer.concat`. bundleString = this.lengthPrefixedString({ metadata }) + bundleString; - // TODO: it's not ideal to have to re-encode all of these strings multiple times - // the implementation in nodejs-firestore concatenates Buffers instead of - // concatenating strings. - return encoder.encode(bundleString); + return bundleString; } } /** - * Convenient class to hold both the metadata and the actual content of a document to be bundled. - * @private - * @internal - */ -class BundledDocument { - constructor( - readonly metadata: ProtoBundledDocumentMetadata, - readonly document?: Document - ) {} -} - -/** - * Validates that 'value' is DocumentSnapshot. + * Interface for an object that contains data required to bundle a DocumentSnapshot. + * Accessing the methods of DocumentSnapshot directly to retreivew this data in this + * implementation would create a circular dependency. * - * @private * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. */ -function validateDocumentSnapshot(arg: string | number, value: unknown): void { - if (!(value instanceof DocumentSnapshot)) { - throw new Error(invalidArgumentMessage(arg, 'DocumentSnapshot')); - } +export interface DocumentBundleData { + readonly documentData: DocumentData; + readonly documentKey: DocumentKey; + readonly documentPath: string; + readonly documentExists: boolean; + readonly createdTime: Timestamp; + readonly readTime?: Timestamp; + readonly versionTime: Timestamp; + readonly queryName?: string; } /** - * Validates that 'value' is QuerySnapshot. - * + * Convenient class to hold both the metadata and the actual content of a document to be bundled. * @private * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. */ -function validateQuerySnapshot(arg: string | number, value: unknown): void { - if (!(value instanceof QuerySnapshot)) { - throw new Error(invalidArgumentMessage(arg, 'QuerySnapshot')); - } +class BundledDocument { + constructor( + readonly metadata: ProtoBundledDocumentMetadata, + readonly document?: Document + ) {} } /** From 5e3620ddae81f75cbb574156cf09be106a9cb8a5 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 25 Mar 2025 16:40:48 -0400 Subject: [PATCH 11/32] QuerySnapshot toJSON impl, but it has bugs name and parent fields aren't properly populated. Looks like we have extraneous "orderBy" fields, too, which might be harmless. --- common/api-review/firestore.api.md | 2 + packages/firestore/src/api/snapshot.ts | 63 ++++++++++++++++--- .../firestore/src/util/bundle_builder_impl.ts | 38 ++++++----- 3 files changed, 77 insertions(+), 26 deletions(-) diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 65bbd398d6..471d129298 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -616,6 +616,8 @@ export class QuerySnapshot; get size(): number; + // (undocumented) + toJSON(): object; } // @public diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index 564ecdc700..34e12abe2f 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -38,6 +38,7 @@ import { DocumentKey } from '../model/document_key'; import { debugAssert, fail } from '../util/assert'; import { BundleBuilder, DocumentBundleData } from '../util/bundle_builder_impl'; import { Code, FirestoreError } from '../util/error'; +import { AutoId } from '../util/misc'; import { Firestore } from './database'; import { SnapshotListenOptions } from './reference_impl'; @@ -507,13 +508,23 @@ export class DocumentSnapshot< return { bundle: '' }; } - const builder: BundleBuilder = new BundleBuilder(this._firestore, 'abc123'); + const builder: BundleBuilder = new BundleBuilder( + this._firestore, + AutoId.newId() + ); const documentData = this._userDataWriter.convertObjectMap( this._document.data.value.mapValue.fields, 'previous' ); - builder.addBundleDocument(DocumentToDocumentBundleData(this._firestore, this.ref.path, documentData, this._document)); + builder.addBundleDocument( + DocumentToDocumentBundleData( + this._firestore, + this.ref.path, + documentData, + this._document + ) + ); return { bundle: builder.build() }; @@ -659,7 +670,7 @@ export class QuerySnapshot< throw new FirestoreError( Code.INVALID_ARGUMENT, 'To include metadata changes with your document changes, you must ' + - 'also pass { includeMetadataChanges:true } to onSnapshot().' + 'also pass { includeMetadataChanges:true } to onSnapshot().' ); } @@ -673,6 +684,35 @@ export class QuerySnapshot< return this._cachedChanges; } + + toJSON(): object { + const builder: BundleBuilder = new BundleBuilder( + this._firestore, + AutoId.newId() + ); + const docBundleDataArray: DocumentBundleData[] = []; + const docArray = this.docs; + docArray.forEach(doc => { + if (doc._document === null) { + return; + } + const documentData = this._userDataWriter.convertObjectMap( + doc._document.data.value.mapValue.fields, + 'previous' + ); + docBundleDataArray.push( + DocumentToDocumentBundleData( + this._firestore, + doc.ref.path, + documentData, + doc._document + ) + ); + }); + + builder.addBundleQuery(this.query._query, docBundleDataArray); + return { bundle: builder.build() }; + } } /** Calculates the array of `DocumentChange`s for a given `ViewSnapshot`. */ @@ -695,10 +735,10 @@ export function changesFromSnapshot< ); debugAssert( !lastDoc || - newQueryComparator(querySnapshot._snapshot.query)( - lastDoc, - change.doc - ) < 0, + newQueryComparator(querySnapshot._snapshot.query)( + lastDoc, + change.doc + ) < 0, 'Got added events in wrong order' ); const doc = new QueryDocumentSnapshot( @@ -813,7 +853,12 @@ export function snapshotEqual( return false; } -function DocumentToDocumentBundleData(firestore: Firestore, path: string, documentData: DocumentData, document: Document): DocumentBundleData { +function DocumentToDocumentBundleData( + firestore: Firestore, + path: string, + documentData: DocumentData, + document: Document +): DocumentBundleData { return { documentData, documentKey: document.mutableCopy().key, @@ -823,4 +868,4 @@ function DocumentToDocumentBundleData(firestore: Firestore, path: string, docume readTime: document.readTime.toTimestamp(), versionTime: document.version.toTimestamp() }; -} \ No newline at end of file +} diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index f65ae0ea88..a0adc36078 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -18,11 +18,13 @@ import { JsonProtoSerializer, toName, + toQueryTarget, toTimestamp } from '../../src/remote/serializer'; import { encoder } from '../../test/unit/util/bundle_data'; import { Firestore } from '../api/database'; import { DatabaseId } from '../core/database_info'; +import { Query, queryToTarget } from '../core/query'; import { DocumentData } from '../lite-api/reference'; import { Timestamp } from '../lite-api/timestamp'; import { @@ -103,7 +105,10 @@ export class BundleBuilder { }; } - addBundleDocument(docBundleData: DocumentBundleData): void { + addBundleDocument( + docBundleData: DocumentBundleData, + queryName?: string + ): void { const originalDocument = this.documents.get(docBundleData.documentPath); const originalQueries = originalDocument?.metadata.queries; @@ -127,32 +132,32 @@ export class BundleBuilder { } // Update `queries` to include both original and `queryName`. - const newDocument = this.documents.get(docBundleData.documentPath)!; - newDocument.metadata.queries = originalQueries || []; - if (docBundleData.queryName) { - newDocument.metadata.queries!.push(docBundleData.queryName); + if (queryName) { + const newDocument = this.documents.get(docBundleData.documentPath)!; + newDocument.metadata.queries = originalQueries || []; + if (queryName) { + newDocument.metadata.queries!.push(queryName); + } } if (readTime && readTime > this.latestReadTime) { this.latestReadTime = readTime; } } - /*private addNamedQuery(name: string, querySnap: QuerySnapshot): void { + addBundleQuery(query: Query, docBundleDataArray: DocumentBundleData[]): void { + const queryTarget = toQueryTarget(this.serializer, queryToTarget(query)); + const name = queryTarget.parent.canonicalString(); + if (this.namedQueries.has(name)) { throw new Error(`Query name conflict: ${name} has already been added.`); } - const queryTarget = toQueryTarget( - this.serializer, - queryToTarget(querySnap.query._query) - ); let latestReadTime = new Timestamp(0, 0); - for (const snap of querySnap.docs) { - const readTime = snap.readTime; - if (readTime && readTime > latestReadTime) { - latestReadTime = readTime; + for (const docBundleData of docBundleDataArray) { + this.addBundleDocument(docBundleData, name); + if (docBundleData.readTime && docBundleData.readTime > latestReadTime) { + latestReadTime = docBundleData.readTime; } - this.addBundledDocument(snap, name); } const bundledQuery = { @@ -166,7 +171,7 @@ export class BundleBuilder { bundledQuery, readTime: toTimestamp(this.serializer, latestReadTime) }); - } */ + } /** * Converts a IBundleElement to a Buffer whose content is the length prefixed JSON representation @@ -234,7 +239,6 @@ export interface DocumentBundleData { readonly createdTime: Timestamp; readonly readTime?: Timestamp; readonly versionTime: Timestamp; - readonly queryName?: string; } /** From ffff490c8ebea3e1fa7809c457ebf07990baa232 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 26 Mar 2025 17:30:08 -0400 Subject: [PATCH 12/32] Successfully tested bundling a QuerySnapshot Also includes: - format - lint fixes. - Comments for bundler methods - Addition of DocumentSnapshotBundleData & QuerySnapshotBundleData types. --- packages/firestore/src/api/snapshot.ts | 54 ++++---- .../firestore/src/util/bundle_builder_impl.ts | 116 +++++++----------- 2 files changed, 73 insertions(+), 97 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index 34e12abe2f..e8104ca04e 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -36,7 +36,11 @@ import { AbstractUserDataWriter } from '../lite-api/user_data_writer'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { debugAssert, fail } from '../util/assert'; -import { BundleBuilder, DocumentBundleData } from '../util/bundle_builder_impl'; +import { + BundleBuilder, + DocumentSnapshotBundleData, + QuerySnapshotBundleData +} from '../util/bundle_builder_impl'; import { Code, FirestoreError } from '../util/error'; import { AutoId } from '../util/misc'; @@ -500,34 +504,26 @@ export class DocumentSnapshot< } toJSON(): object { + const document = this._document; if ( - !this._document || - !this._document.isValidDocument() || - !this._document.isFoundDocument() + !document || + !document.isValidDocument() || + !document.isFoundDocument() ) { return { bundle: '' }; } - const builder: BundleBuilder = new BundleBuilder( this._firestore, AutoId.newId() ); const documentData = this._userDataWriter.convertObjectMap( - this._document.data.value.mapValue.fields, + document.data.value.mapValue.fields, 'previous' ); - builder.addBundleDocument( - DocumentToDocumentBundleData( - this._firestore, - this.ref.path, - documentData, - this._document - ) + ToDocumentSnapshotBundleData(this.ref.path, documentData, document) ); - return { - bundle: builder.build() - }; + return { bundle: builder.build() }; } } @@ -690,7 +686,10 @@ export class QuerySnapshot< this._firestore, AutoId.newId() ); - const docBundleDataArray: DocumentBundleData[] = []; + const databaseId = this._firestore._databaseId.database; + const projectId = this._firestore._databaseId.projectId; + const parent = `projects/${projectId}/databases/${databaseId}/documents`; + const docBundleDataArray: DocumentSnapshotBundleData[] = []; const docArray = this.docs; docArray.forEach(doc => { if (doc._document === null) { @@ -701,16 +700,15 @@ export class QuerySnapshot< 'previous' ); docBundleDataArray.push( - DocumentToDocumentBundleData( - this._firestore, - doc.ref.path, - documentData, - doc._document - ) + ToDocumentSnapshotBundleData(doc.ref.path, documentData, doc._document) ); }); - - builder.addBundleQuery(this.query._query, docBundleDataArray); + const bundleData: QuerySnapshotBundleData = { + query: this.query._query, + parent, + docBundleDataArray + }; + builder.addBundleQuery(bundleData); return { bundle: builder.build() }; } } @@ -853,12 +851,12 @@ export function snapshotEqual( return false; } -function DocumentToDocumentBundleData( - firestore: Firestore, +// Formats Document data for bundling a DocumentSnapshot. +function ToDocumentSnapshotBundleData( path: string, documentData: DocumentData, document: Document -): DocumentBundleData { +): DocumentSnapshotBundleData { return { documentData, documentKey: document.mutableCopy().key, diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index a0adc36078..34290588cd 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -43,6 +43,7 @@ import { Document as ProtoDocument, Document } from '../protos/firestore_proto_api'; +import { AutoId } from '../util/misc'; import { debugAssert } from './assert'; @@ -82,7 +83,9 @@ export class BundleBuilder { ); } - toBundleDocument(docBundleData: DocumentBundleData): ProtoDocument { + private toBundleDocument( + docBundleData: DocumentSnapshotBundleData + ): ProtoDocument { // TODO handle documents that have mutations debugAssert( !docBundleData.documentData.hasLocalMutations, @@ -105,8 +108,16 @@ export class BundleBuilder { }; } + /** + * Adds data from a DocumentSnapshot to the bundle. + * @internal + * @param docBundleData A DocumentSnapshotBundleData containing information from the + * DocumentSnapshot. Note we cannot accept a DocumentSnapshot directly due to a circular + * dependency error. + * @param queryName The name of the QuerySnapshot if this document is part of a Query. + */ addBundleDocument( - docBundleData: DocumentBundleData, + docBundleData: DocumentSnapshotBundleData, queryName?: string ): void { const originalDocument = this.documents.get(docBundleData.documentPath); @@ -130,7 +141,6 @@ export class BundleBuilder { } }); } - // Update `queries` to include both original and `queryName`. if (queryName) { const newDocument = this.documents.get(docBundleData.documentPath)!; @@ -144,28 +154,33 @@ export class BundleBuilder { } } - addBundleQuery(query: Query, docBundleDataArray: DocumentBundleData[]): void { - const queryTarget = toQueryTarget(this.serializer, queryToTarget(query)); - const name = queryTarget.parent.canonicalString(); - + /** + * Adds data from a QuerySnapshot to the bundle. + * @internal + * @param docBundleData A QuerySnapshotBundleData containing information from the + * QuerySnapshot. Note we cannot accept a QuerySnapshot directly due to a circular + * dependency error. + */ + addBundleQuery(queryBundleData: QuerySnapshotBundleData): void { + const name = AutoId.newId(); if (this.namedQueries.has(name)) { throw new Error(`Query name conflict: ${name} has already been added.`); } - let latestReadTime = new Timestamp(0, 0); - for (const docBundleData of docBundleDataArray) { + for (const docBundleData of queryBundleData.docBundleDataArray) { this.addBundleDocument(docBundleData, name); if (docBundleData.readTime && docBundleData.readTime > latestReadTime) { latestReadTime = docBundleData.readTime; } } - + const queryTarget = toQueryTarget( + this.serializer, + queryToTarget(queryBundleData.query) + ); const bundledQuery = { - parent: queryTarget.parent.canonicalString(), - structuredQuery: queryTarget.queryTarget.structuredQuery, - limitType: null + parent: queryBundleData.parent, + structuredQuery: queryTarget.queryTarget.structuredQuery }; - this.namedQueries.set(name, { name, bundledQuery, @@ -226,19 +241,26 @@ export class BundleBuilder { /** * Interface for an object that contains data required to bundle a DocumentSnapshot. - * Accessing the methods of DocumentSnapshot directly to retreivew this data in this - * implementation would create a circular dependency. - * * @internal */ -export interface DocumentBundleData { - readonly documentData: DocumentData; - readonly documentKey: DocumentKey; - readonly documentPath: string; - readonly documentExists: boolean; - readonly createdTime: Timestamp; - readonly readTime?: Timestamp; - readonly versionTime: Timestamp; +export interface DocumentSnapshotBundleData { + documentData: DocumentData; + documentKey: DocumentKey; + documentPath: string; + documentExists: boolean; + createdTime: Timestamp; + readTime?: Timestamp; + versionTime: Timestamp; +} + +/** + * Interface for an object that contains data required to bundle a QuerySnapshot. + * @internal + */ +export interface QuerySnapshotBundleData { + query: Query; + parent: string; + docBundleDataArray: DocumentSnapshotBundleData[]; } /** @@ -252,47 +274,3 @@ class BundledDocument { readonly document?: Document ) {} } - -/** - * Validates that 'value' is a string. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the string can be omitted. - */ -export function validateString(arg: string | number, value: unknown): void { - if (typeof value !== 'string') { - throw new Error(invalidArgumentMessage(arg, 'string')); - } -} - -/** - * Generates an error message to use with invalid arguments. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param expectedType The expected input type. - */ -export function invalidArgumentMessage( - arg: string | number, - expectedType: string -): string { - return `${formatArgumentName(arg)} is not a valid ${expectedType}.`; -} - -/** - * Creates a descriptive name for the provided argument name or index. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @return Either the argument name or its index description. - */ -function formatArgumentName(arg: string | number): string { - return typeof arg === 'string' - ? `Value for argument "${arg}"` - : `Element at index ${arg}`; -} From 3bcf31e4a0f0f283954e8c2c45ab39172bfb55ab Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 27 Mar 2025 10:07:54 -0400 Subject: [PATCH 13/32] Remove createTime & readTime from DocumentSnapshot These were added to the API before I knew they could have been queried internally. --- common/api-review/firestore-lite.api.md | 4 ---- common/api-review/firestore.api.md | 4 ---- packages/firestore/src/lite-api/snapshot.ts | 8 -------- 3 files changed, 16 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index da2833882d..4a9ef4c017 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -146,14 +146,10 @@ export class DocumentReference { protected constructor(); - // (undocumented) - get createTime(): Timestamp | undefined; data(): AppModelType | undefined; exists(): this is QueryDocumentSnapshot; get(fieldPath: string | FieldPath): any; get id(): string; - // (undocumented) - get readTime(): Timestamp | undefined; get ref(): DocumentReference; } diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 471d129298..3f17e415e5 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -172,15 +172,11 @@ export class DocumentReference { protected constructor(); - // (undocumented) - get createTime(): Timestamp; data(options?: SnapshotOptions): AppModelType | undefined; exists(): this is QueryDocumentSnapshot; get(fieldPath: string | FieldPath, options?: SnapshotOptions): any; get id(): string; readonly metadata: SnapshotMetadata; - // (undocumented) - get readTime(): Timestamp; get ref(): DocumentReference; // (undocumented) toJSON(): object; diff --git a/packages/firestore/src/lite-api/snapshot.ts b/packages/firestore/src/lite-api/snapshot.ts index efff9987d7..12d60af290 100644 --- a/packages/firestore/src/lite-api/snapshot.ts +++ b/packages/firestore/src/lite-api/snapshot.ts @@ -330,14 +330,6 @@ export class DocumentSnapshot< ); } - get createTime(): Timestamp | undefined { - return this._document?.createTime.toTimestamp(); - } - - get readTime(): Timestamp | undefined { - return this._document?.readTime.toTimestamp(); - } - /** * Signals whether or not the document at the snapshot's location exists. * From ccd5205bea15241167b52aab59dc6c3d6c13f8c1 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 27 Mar 2025 10:08:15 -0400 Subject: [PATCH 14/32] rename documentToDocumentSnapshotBundleData --- packages/firestore/src/api/snapshot.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index e8104ca04e..b06892a727 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -521,7 +521,7 @@ export class DocumentSnapshot< 'previous' ); builder.addBundleDocument( - ToDocumentSnapshotBundleData(this.ref.path, documentData, document) + documentToDocumentSnapshotBundleData(this.ref.path, documentData, document) ); return { bundle: builder.build() }; } @@ -700,7 +700,7 @@ export class QuerySnapshot< 'previous' ); docBundleDataArray.push( - ToDocumentSnapshotBundleData(doc.ref.path, documentData, doc._document) + documentToDocumentSnapshotBundleData(doc.ref.path, documentData, doc._document) ); }); const bundleData: QuerySnapshotBundleData = { @@ -852,7 +852,7 @@ export function snapshotEqual( } // Formats Document data for bundling a DocumentSnapshot. -function ToDocumentSnapshotBundleData( +function documentToDocumentSnapshotBundleData( path: string, documentData: DocumentData, document: Document From 03f3ffbd7a104057631e64f6e6021662c4c09b99 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 27 Mar 2025 10:56:10 -0400 Subject: [PATCH 15/32] Cleanup pass removed extraneous imports and private/protected changes. --- packages/firestore/src/api/snapshot.ts | 12 +++- packages/firestore/src/lite-api/snapshot.ts | 1 - packages/firestore/src/model/path.ts | 2 +- .../firestore/src/util/bundle_builder_impl.ts | 64 +++++++++++-------- 4 files changed, 50 insertions(+), 29 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index b06892a727..c23d03400d 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -521,7 +521,11 @@ export class DocumentSnapshot< 'previous' ); builder.addBundleDocument( - documentToDocumentSnapshotBundleData(this.ref.path, documentData, document) + documentToDocumentSnapshotBundleData( + this.ref.path, + documentData, + document + ) ); return { bundle: builder.build() }; } @@ -700,7 +704,11 @@ export class QuerySnapshot< 'previous' ); docBundleDataArray.push( - documentToDocumentSnapshotBundleData(doc.ref.path, documentData, doc._document) + documentToDocumentSnapshotBundleData( + doc.ref.path, + documentData, + doc._document + ) ); }); const bundleData: QuerySnapshotBundleData = { diff --git a/packages/firestore/src/lite-api/snapshot.ts b/packages/firestore/src/lite-api/snapshot.ts index 12d60af290..3024e2e9db 100644 --- a/packages/firestore/src/lite-api/snapshot.ts +++ b/packages/firestore/src/lite-api/snapshot.ts @@ -17,7 +17,6 @@ import { Compat, getModularInstance } from '@firebase/util'; -import { Timestamp } from '../lite-api/timestamp'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { FieldPath as InternalFieldPath } from '../model/path'; diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts index 0a84ee5b83..64cb0376a0 100644 --- a/packages/firestore/src/model/path.ts +++ b/packages/firestore/src/model/path.ts @@ -26,7 +26,7 @@ export const DOCUMENT_KEY_NAME = '__name__'; * Path represents an ordered sequence of string segments. */ abstract class BasePath> { - protected segments: string[]; + private segments: string[]; private offset: number; private len: number; diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 34290588cd..6a28505725 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -61,8 +61,10 @@ export class BundleBuilder { // The latest read time among all bundled documents and queries. private latestReadTime = new Timestamp(0, 0); + // Database identifier which is part of the serialized bundle. private databaseId: DatabaseId; + // Tools to convert public data types into their serialized form. private readonly serializer: JsonProtoSerializer; private readonly userDataReader: UserDataReader; @@ -83,31 +85,6 @@ export class BundleBuilder { ); } - private toBundleDocument( - docBundleData: DocumentSnapshotBundleData - ): ProtoDocument { - // TODO handle documents that have mutations - debugAssert( - !docBundleData.documentData.hasLocalMutations, - "Can't serialize documents with mutations." - ); - - // a parse context is typically used for validating and parsing user data, but in this - // case we are using it internally to convert DocumentData to Proto3 JSON - const context = this.userDataReader.createContext( - UserDataSource.ArrayArgument, - 'internal toBundledDocument' - ); - const proto3Fields = parseObject(docBundleData.documentData, context); - - return { - name: toName(this.serializer, docBundleData.documentKey), - fields: proto3Fields.mapValue.fields, - updateTime: toTimestamp(this.serializer, docBundleData.versionTime), - createTime: toTimestamp(this.serializer, docBundleData.createdTime) - }; - } - /** * Adds data from a DocumentSnapshot to the bundle. * @internal @@ -188,6 +165,38 @@ export class BundleBuilder { }); } + /** + * Convert data from a DocumentSnapshot into the serialized form within a bundle. + * @private + * @internal + * @param docBundleData a DocumentSnapshotBundleData containing the data required to + * serialize a document. + */ + private toBundleDocument( + docBundleData: DocumentSnapshotBundleData + ): ProtoDocument { + // TODO handle documents that have mutations + debugAssert( + !docBundleData.documentData.hasLocalMutations, + "Can't serialize documents with mutations." + ); + + // a parse context is typically used for validating and parsing user data, but in this + // case we are using it internally to convert DocumentData to Proto3 JSON + const context = this.userDataReader.createContext( + UserDataSource.ArrayArgument, + 'internal toBundledDocument' + ); + const proto3Fields = parseObject(docBundleData.documentData, context); + + return { + name: toName(this.serializer, docBundleData.documentKey), + fields: proto3Fields.mapValue.fields, + updateTime: toTimestamp(this.serializer, docBundleData.versionTime), + createTime: toTimestamp(this.serializer, docBundleData.createdTime) + }; + } + /** * Converts a IBundleElement to a Buffer whose content is the length prefixed JSON representation * of the element. @@ -205,6 +214,11 @@ export class BundleBuilder { return `${l}${str}`; } + /** + * Construct a serialized string containing document and query information that has previously + * been added to the BundleBuilder through the addBundleDocument and addBundleQuery methods. + * @internal + */ build(): string { let bundleString = ''; From 06d84e055a0244603ccebc4b567e77c70c91a300 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 08:48:27 -0400 Subject: [PATCH 16/32] Remove unneeded bundle_builder_validation_utils.ts --- .../util/bundle_builder_validation_utils.ts | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 packages/firestore/src/util/bundle_builder_validation_utils.ts diff --git a/packages/firestore/src/util/bundle_builder_validation_utils.ts b/packages/firestore/src/util/bundle_builder_validation_utils.ts deleted file mode 100644 index 02195ee659..0000000000 --- a/packages/firestore/src/util/bundle_builder_validation_utils.ts +++ /dev/null @@ -1,60 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Validates that 'value' is a string. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param value The input to validate. - * @param options Options that specify whether the string can be omitted. - */ -export function validateString(arg: string | number, value: unknown): void { - if (typeof value !== 'string') { - throw new Error(invalidArgumentMessage(arg, 'string')); - } -} - -/** - * Generates an error message to use with invalid arguments. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @param expectedType The expected input type. - */ -export function invalidArgumentMessage( - arg: string | number, - expectedType: string -): string { - return `${formatArgumentName(arg)} is not a valid ${expectedType}.`; -} - -/** - * Creates a descriptive name for the provided argument name or index. - * - * @private - * @internal - * @param arg The argument name or argument index (for varargs methods). - * @return Either the argument name or its index description. - */ -function formatArgumentName(arg: string | number): string { - return typeof arg === 'string' - ? `Value for argument "${arg}"` - : `Element at index ${arg}`; -} From 1b5335147f78ea62e96940bb2ee1261eac4bcaeb Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 10:56:02 -0400 Subject: [PATCH 17/32] clean up addBundleDocument readTime logic. --- packages/firestore/src/remote/serializer.ts | 5 +++- .../firestore/src/util/bundle_builder_impl.ts | 27 ++++++++++--------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 811c2ac4df..729a73e1fb 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -226,7 +226,10 @@ export function toTimestamp( } } -function fromTimestamp(date: ProtoTimestamp): Timestamp { +/** + * Returns a Timestamp type given timestamp from a protobuf. + */ +export function fromTimestamp(date: ProtoTimestamp): Timestamp { const timestamp = normalizeTimestamp(date); return new Timestamp(timestamp.seconds, timestamp.nanos); } diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 6a28505725..971312bb12 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -17,6 +17,7 @@ import { JsonProtoSerializer, + fromTimestamp, toName, toQueryTarget, toTimestamp @@ -99,24 +100,29 @@ export class BundleBuilder { ): void { const originalDocument = this.documents.get(docBundleData.documentPath); const originalQueries = originalDocument?.metadata.queries; + const docReadTime: Timestamp | undefined = docBundleData.readTime; + const origDocReadTime: Timestamp | null = !!originalDocument?.metadata + .readTime + ? fromTimestamp(originalDocument.metadata.readTime) + : null; - const readTime = docBundleData.readTime; - // Update with document built from `snap` because it is newer. - if ( - !originalDocument || - (!readTime && !originalDocument.metadata.readTime) || - (readTime && originalDocument.metadata.readTime! < readTime) - ) { + const neitherHasReadTime: boolean = !docReadTime && origDocReadTime == null; + const docIsNewer: boolean = docReadTime !== undefined && (origDocReadTime == null || origDocReadTime < docReadTime); + if (neitherHasReadTime || docIsNewer) { + // Store document. this.documents.set(docBundleData.documentPath, { document: this.toBundleDocument(docBundleData), metadata: { name: toName(this.serializer, docBundleData.documentKey), - readTime: !!readTime - ? toTimestamp(this.serializer, readTime) + readTime: !!docReadTime + ? toTimestamp(this.serializer, docReadTime) // Convert Timestamp to proto format. : undefined, exists: docBundleData.documentExists } }); + } + if (docReadTime && docReadTime > this.latestReadTime) { + this.latestReadTime = docReadTime; } // Update `queries` to include both original and `queryName`. if (queryName) { @@ -126,9 +132,6 @@ export class BundleBuilder { newDocument.metadata.queries!.push(queryName); } } - if (readTime && readTime > this.latestReadTime) { - this.latestReadTime = readTime; - } } /** From 5f413b6e5d5218409d186a27fe2592af93a5cf15 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 11:00:59 -0400 Subject: [PATCH 18/32] removed extra `if(queryName)` in addBundleDocument --- packages/firestore/src/util/bundle_builder_impl.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 971312bb12..6e4a0406b5 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -128,9 +128,7 @@ export class BundleBuilder { if (queryName) { const newDocument = this.documents.get(docBundleData.documentPath)!; newDocument.metadata.queries = originalQueries || []; - if (queryName) { - newDocument.metadata.queries!.push(queryName); - } + newDocument.metadata.queries!.push(queryName); } } From e0f3c292ada5e6262716eb705f2685341a85b6bc Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 11:03:46 -0400 Subject: [PATCH 19/32] Update fromTimestamp doc. --- packages/firestore/src/remote/serializer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 729a73e1fb..78a7ceda6f 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -227,7 +227,7 @@ export function toTimestamp( } /** - * Returns a Timestamp type given timestamp from a protobuf. + * Returns a Timestamp typed object given protobuf timestamp value. */ export function fromTimestamp(date: ProtoTimestamp): Timestamp { const timestamp = normalizeTimestamp(date); From fbfa24febcbc2f590b1158e72fb21063b5f01f6b Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 11:28:07 -0400 Subject: [PATCH 20/32] toJSON methods warn of pending writes. --- packages/firestore/src/api/snapshot.ts | 11 +++++++++++ packages/firestore/src/util/bundle_builder_impl.ts | 12 ++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index c23d03400d..1af71bada5 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -42,6 +42,7 @@ import { QuerySnapshotBundleData } from '../util/bundle_builder_impl'; import { Code, FirestoreError } from '../util/error'; +import { logWarn } from '../util/log'; import { AutoId } from '../util/misc'; import { Firestore } from './database'; @@ -520,6 +521,11 @@ export class DocumentSnapshot< document.data.value.mapValue.fields, 'previous' ); + if (documentData.hasPendingWrites) { + logWarn( + 'DocumentSnapshot.toJSON serialized a document with pending writes. The pending writes will not be serialized.' + ); + } builder.addBundleDocument( documentToDocumentSnapshotBundleData( this.ref.path, @@ -703,6 +709,11 @@ export class QuerySnapshot< doc._document.data.value.mapValue.fields, 'previous' ); + if (documentData.hasPendingWrites) { + logWarn( + 'QuerySnapshot.toJSON serialized a document with pending writes. The pending writes will not be serialized.' + ); + } docBundleDataArray.push( documentToDocumentSnapshotBundleData( doc.ref.path, diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 6e4a0406b5..2cbc99703c 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -107,7 +107,9 @@ export class BundleBuilder { : null; const neitherHasReadTime: boolean = !docReadTime && origDocReadTime == null; - const docIsNewer: boolean = docReadTime !== undefined && (origDocReadTime == null || origDocReadTime < docReadTime); + const docIsNewer: boolean = + docReadTime !== undefined && + (origDocReadTime == null || origDocReadTime < docReadTime); if (neitherHasReadTime || docIsNewer) { // Store document. this.documents.set(docBundleData.documentPath, { @@ -120,7 +122,7 @@ export class BundleBuilder { exists: docBundleData.documentExists } }); - } + } if (docReadTime && docReadTime > this.latestReadTime) { this.latestReadTime = docReadTime; } @@ -176,12 +178,6 @@ export class BundleBuilder { private toBundleDocument( docBundleData: DocumentSnapshotBundleData ): ProtoDocument { - // TODO handle documents that have mutations - debugAssert( - !docBundleData.documentData.hasLocalMutations, - "Can't serialize documents with mutations." - ); - // a parse context is typically used for validating and parsing user data, but in this // case we are using it internally to convert DocumentData to Proto3 JSON const context = this.userDataReader.createContext( From 070a84a257bab89df4212d05c85c57047ede883e Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 15:07:56 -0400 Subject: [PATCH 21/32] throw instead of warn. --- packages/firestore/src/api/snapshot.ts | 13 ++++++++----- packages/firestore/src/util/bundle_builder_impl.ts | 8 ++------ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index 1af71bada5..bcf7d01bf7 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -42,7 +42,6 @@ import { QuerySnapshotBundleData } from '../util/bundle_builder_impl'; import { Code, FirestoreError } from '../util/error'; -import { logWarn } from '../util/log'; import { AutoId } from '../util/misc'; import { Firestore } from './database'; @@ -522,8 +521,10 @@ export class DocumentSnapshot< 'previous' ); if (documentData.hasPendingWrites) { - logWarn( - 'DocumentSnapshot.toJSON serialized a document with pending writes. The pending writes will not be serialized.' + throw new FirestoreError( + Code.FAILED_PRECONDITION, + 'DocumentSnapshot.toJSON attempted to serialize a document with pending writes. ' + + 'Await `waitForPendingWrites()` before invoking `toJSON()`.' ); } builder.addBundleDocument( @@ -710,8 +711,10 @@ export class QuerySnapshot< 'previous' ); if (documentData.hasPendingWrites) { - logWarn( - 'QuerySnapshot.toJSON serialized a document with pending writes. The pending writes will not be serialized.' + throw new FirestoreError( + Code.FAILED_PRECONDITION, + 'QuerySnapshot.toJSON attempted to serialize a document with pending writes. ' + + 'Await `waitForPendingWrites()` before invoking `toJSON()`.' ); } docBundleDataArray.push( diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 2cbc99703c..77e2cfb13c 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -46,8 +46,6 @@ import { } from '../protos/firestore_proto_api'; import { AutoId } from '../util/misc'; -import { debugAssert } from './assert'; - const BUNDLE_VERSION = 1; /** @@ -107,9 +105,7 @@ export class BundleBuilder { : null; const neitherHasReadTime: boolean = !docReadTime && origDocReadTime == null; - const docIsNewer: boolean = - docReadTime !== undefined && - (origDocReadTime == null || origDocReadTime < docReadTime); + const docIsNewer: boolean = docReadTime !== undefined && (origDocReadTime == null || origDocReadTime < docReadTime); if (neitherHasReadTime || docIsNewer) { // Store document. this.documents.set(docBundleData.documentPath, { @@ -122,7 +118,7 @@ export class BundleBuilder { exists: docBundleData.documentExists } }); - } + } if (docReadTime && docReadTime > this.latestReadTime) { this.latestReadTime = docReadTime; } From 8e0b3bbe6cb6360f1a83de8f7f5b9d1df4727780 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 15:12:15 -0400 Subject: [PATCH 22/32] format --- packages/firestore/src/util/bundle_builder_impl.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/util/bundle_builder_impl.ts b/packages/firestore/src/util/bundle_builder_impl.ts index 77e2cfb13c..d516e512db 100644 --- a/packages/firestore/src/util/bundle_builder_impl.ts +++ b/packages/firestore/src/util/bundle_builder_impl.ts @@ -105,7 +105,9 @@ export class BundleBuilder { : null; const neitherHasReadTime: boolean = !docReadTime && origDocReadTime == null; - const docIsNewer: boolean = docReadTime !== undefined && (origDocReadTime == null || origDocReadTime < docReadTime); + const docIsNewer: boolean = + docReadTime !== undefined && + (origDocReadTime == null || origDocReadTime < docReadTime); if (neitherHasReadTime || docIsNewer) { // Store document. this.documents.set(docBundleData.documentPath, { @@ -118,7 +120,7 @@ export class BundleBuilder { exists: docBundleData.documentExists } }); - } + } if (docReadTime && docReadTime > this.latestReadTime) { this.latestReadTime = docReadTime; } From 65c3d8be9399859ee2f3e70bf7983cd30e4f8edf Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 15:35:10 -0400 Subject: [PATCH 23/32] metadata contains hasPendingWrites --- packages/firestore/src/api/snapshot.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index bcf7d01bf7..99b43f73f5 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -520,7 +520,7 @@ export class DocumentSnapshot< document.data.value.mapValue.fields, 'previous' ); - if (documentData.hasPendingWrites) { + if (this.metadata.hasPendingWrites) { throw new FirestoreError( Code.FAILED_PRECONDITION, 'DocumentSnapshot.toJSON attempted to serialize a document with pending writes. ' + @@ -710,7 +710,7 @@ export class QuerySnapshot< doc._document.data.value.mapValue.fields, 'previous' ); - if (documentData.hasPendingWrites) { + if (this.metadata.hasPendingWrites) { throw new FirestoreError( Code.FAILED_PRECONDITION, 'QuerySnapshot.toJSON attempted to serialize a document with pending writes. ' + From 73eb8a20f3db75ee4d32997a0235d3b1e0aead2e Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 16:48:53 -0400 Subject: [PATCH 24/32] Unit tests. --- packages/firestore/src/api/snapshot.ts | 8 +- .../firestore/test/unit/api/database.test.ts | 87 +++++++++++++++++++ packages/firestore/test/util/api_helpers.ts | 10 ++- 3 files changed, 98 insertions(+), 7 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index 99b43f73f5..dba5c409ab 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -523,8 +523,8 @@ export class DocumentSnapshot< if (this.metadata.hasPendingWrites) { throw new FirestoreError( Code.FAILED_PRECONDITION, - 'DocumentSnapshot.toJSON attempted to serialize a document with pending writes. ' + - 'Await `waitForPendingWrites()` before invoking `toJSON()`.' + 'DocumentSnapshot.toJSON() attempted to serialize a document with pending writes. ' + + 'Await waitForPendingWrites() before invoking toJSON().' ); } builder.addBundleDocument( @@ -713,8 +713,8 @@ export class QuerySnapshot< if (this.metadata.hasPendingWrites) { throw new FirestoreError( Code.FAILED_PRECONDITION, - 'QuerySnapshot.toJSON attempted to serialize a document with pending writes. ' + - 'Await `waitForPendingWrites()` before invoking `toJSON()`.' + 'QuerySnapshot.toJSON() attempted to serialize a document with pending writes. ' + + 'Await waitForPendingWrites() before invoking toJSON().' ); } docBundleDataArray.push( diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index 1cc1df5106..3838099912 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -19,6 +19,7 @@ import { expect } from 'chai'; import { connectFirestoreEmulator, + loadBundle, refEqual, snapshotEqual, queryEqual @@ -35,6 +36,15 @@ import { } from '../../util/api_helpers'; import { keys } from '../../util/helpers'; +describe('Bundle', () => { + it('loadBundle silently exits with an empty bundle string)', async () => { + const db = newTestFirestore(); + expect(async () => { + await loadBundle(db, ''); + }).to.not.throw; + }); +}); + describe('CollectionReference', () => { it('support equality checking with isEqual()', () => { expect(refEqual(collectionReference('foo'), collectionReference('foo'))).to @@ -107,6 +117,44 @@ describe('DocumentSnapshot', () => { it('JSON.stringify() does not throw', () => { JSON.stringify(documentSnapshot('foo/bar', { a: 1 }, true)); }); + + it('toJSON returns a bundle', () => { + const json = documentSnapshot( + 'foo/bar', + { a: 1 }, + /*fromCache=*/ true + ).toJSON(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle).to.exist; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle.length).to.be.greaterThan(0); + }); + + it('toJSON returns an empty bundle when there are no documents', () => { + const json = documentSnapshot( + 'foo/bar', + /*data=*/ null, + /*fromCache=*/ true + ).toJSON(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle).to.exist; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle.length).to.equal(0); + }); + + it('toJSON throws when there are pending writes', () => { + expect(() => { + documentSnapshot( + 'foo/bar', + {}, + /*fromCache=*/ true, + /*hasPendingWrites=*/ true + ).toJSON(); + }).to.throw( + `DocumentSnapshot.toJSON() attempted to serialize a document with pending writes. ` + + `Await waitForPendingWrites() before invoking toJSON().` + ); + }); }); describe('Query', () => { @@ -229,6 +277,45 @@ describe('QuerySnapshot', () => { querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false) ); }); + + it('toJSON returns a bundle', () => { + const json = querySnapshot( + 'foo', + {}, + { a: { a: 1 } }, + keys(), + false, + false + ).toJSON(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle).to.exist; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle.length).to.be.greaterThan(0); + }); + + it('toJSON returns a bundle when there are no documents', () => { + const json = querySnapshot('foo', {}, {}, keys(), false, false).toJSON(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle).to.exist; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((json as any).bundle.length).to.be.greaterThan(0); + }); + + it('toJSON throws when there are pending writes', () => { + expect(() => + querySnapshot( + 'foo', + {}, + { a: { a: 1 } }, + keys('foo/a'), + true, + true + ).toJSON() + ).to.throw( + `QuerySnapshot.toJSON() attempted to serialize a document with pending writes. ` + + `Await waitForPendingWrites() before invoking toJSON().` + ); + }); }); describe('SnapshotMetadata', () => { diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index 762b5258a2..fe398e4332 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -79,8 +79,12 @@ export function documentReference(path: string): DocumentReference { export function documentSnapshot( path: string, data: JsonObject | null, - fromCache: boolean + fromCache: boolean, + hasPendingWrites?: boolean ): DocumentSnapshot { + if (hasPendingWrites === undefined) { + hasPendingWrites = false; + } const db = firestore(); const userDataWriter = new ExpUserDataWriter(db); if (data) { @@ -89,7 +93,7 @@ export function documentSnapshot( userDataWriter, key(path), doc(path, 1, data), - new SnapshotMetadata(/* hasPendingWrites= */ false, fromCache), + new SnapshotMetadata(hasPendingWrites, fromCache), /* converter= */ null ); } else { @@ -98,7 +102,7 @@ export function documentSnapshot( userDataWriter, key(path), null, - new SnapshotMetadata(/* hasPendingWrites= */ false, fromCache), + new SnapshotMetadata(hasPendingWrites, fromCache), /* converter= */ null ); } From 6251a28e948bae2c069322b18f7306dd6ca5c58e Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 28 Mar 2025 16:58:46 -0400 Subject: [PATCH 25/32] Update database.test.ts --- packages/firestore/test/unit/api/database.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index 3838099912..b5e9dc1b67 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -37,7 +37,7 @@ import { import { keys } from '../../util/helpers'; describe('Bundle', () => { - it('loadBundle silently exits with an empty bundle string)', async () => { + it('loadBundle does not throw with an empty bundle string)', async () => { const db = newTestFirestore(); expect(async () => { await loadBundle(db, ''); From 2a3a7df6e8e948c215553bc10599f4e6c88d211c Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 31 Mar 2025 14:26:37 -0400 Subject: [PATCH 26/32] Add integration test. --- .../test/integration/api/query.test.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 01fd0e47e3..6aa59610e0 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -37,9 +37,11 @@ import { endAt, endBefore, GeoPoint, + getDocFromCache, getDocs, limit, limitToLast, + loadBundle, onSnapshot, or, orderBy, @@ -74,6 +76,44 @@ import { captureExistenceFilterMismatches } from '../util/testing_hooks_util'; apiDescribe('Queries', persistence => { addEqualityMatcher(); + it('QuerySnapshot.toJSON bundle getDocFromCache', async () => { + let path: string | null = null; + let jsonBundle: object | null = null; + const testDocs = { + a: { k: 'a' }, + b: { k: 'b' }, + c: { k: 'c' } + }; + // Write an initial document in an isolated Firestore instance so it's not stored in the cache. + await withTestCollection(persistence, testDocs, async collection => { + await getDocs(query(collection)).then(querySnapshot => { + expect(querySnapshot.docs.length).to.equal(3); + // Find the path to a known doc. + querySnapshot.docs.forEach(docSnapshot => { + if (docSnapshot.ref.path.endsWith('a')) { + path = docSnapshot.ref.path; + } + }); + expect(path).to.not.be.null; + jsonBundle = querySnapshot.toJSON(); + }); + }); + expect(jsonBundle).to.not.be.null; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const json = (jsonBundle as any).bundle; + expect(json).to.exist; + expect(json.length).to.be.greaterThan(0); + // Use a new instance to load the bundle to ensure that the cache is primed by the bundle and + // not the previous getDocs call. + await withTestDb(persistence, async db => { + await loadBundle(db, json); + const docRef = doc(db, path!); + const docSnap = await getDocFromCache(docRef); + expect(docSnap.exists); + expect(docSnap.data()).to.deep.equal(testDocs.a); + }); + }); + it('can issue limit queries', () => { const testDocs = { a: { k: 'a' }, From afd294afe30ab034471d5b8815770ec9555571b0 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Mon, 31 Mar 2025 14:49:51 -0400 Subject: [PATCH 27/32] Docs --- docs-devsite/firestore_.documentsnapshot.md | 12 ++++++++++++ docs-devsite/firestore_.querysnapshot.md | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/docs-devsite/firestore_.documentsnapshot.md b/docs-devsite/firestore_.documentsnapshot.md index 476588b78c..8c4825593d 100644 --- a/docs-devsite/firestore_.documentsnapshot.md +++ b/docs-devsite/firestore_.documentsnapshot.md @@ -41,6 +41,7 @@ export declare class DocumentSnapshotObject. Returns undefined if the document doesn't exist.By default, serverTimestamp() values that have not yet been set to their final value will be returned as null. You can override this by passing an options object. | | [exists()](./firestore_.documentsnapshot.md#documentsnapshotexists) | | Returns whether or not the data exists. True if the document exists. | | [get(fieldPath, options)](./firestore_.documentsnapshot.md#documentsnapshotget) | | Retrieves the field specified by fieldPath. Returns undefined if the document or field doesn't exist.By default, a serverTimestamp() that has not yet been set to its final value will be returned as null. You can override this by passing an options object. | +| [toJSON()](./firestore_.documentsnapshot.md#documentsnapshottojson) | | | ## DocumentSnapshot.(constructor) @@ -144,3 +145,14 @@ any The data at the specified field location or undefined if no such field exists in the document. +## DocumentSnapshot.toJSON() + +Signature: + +```typescript +toJSON(): object; +``` +Returns: + +object + diff --git a/docs-devsite/firestore_.querysnapshot.md b/docs-devsite/firestore_.querysnapshot.md index d9930c68d9..da0913d7b6 100644 --- a/docs-devsite/firestore_.querysnapshot.md +++ b/docs-devsite/firestore_.querysnapshot.md @@ -34,6 +34,7 @@ export declare class QuerySnapshotQuerySnapshot. | +| [toJSON()](./firestore_.querysnapshot.md#querysnapshottojson) | | | ## QuerySnapshot.docs @@ -126,3 +127,14 @@ forEach(callback: (result: QueryDocumentSnapshot) => void +## QuerySnapshot.toJSON() + +Signature: + +```typescript +toJSON(): object; +``` +Returns: + +object + From 06f71c9ee9874ff21ae3ea4ad6456f14dedeeb86 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 1 Apr 2025 09:19:43 -0400 Subject: [PATCH 28/32] debug logs --- packages/firestore/test/integration/api/query.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 6aa59610e0..558771057f 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -64,6 +64,7 @@ import { toChangesArray, toDataArray, PERSISTENCE_MODE_UNSPECIFIED, + withAlternateTestDb, withEmptyTestCollection, withRetry, withTestCollection, @@ -76,7 +77,7 @@ import { captureExistenceFilterMismatches } from '../util/testing_hooks_util'; apiDescribe('Queries', persistence => { addEqualityMatcher(); - it('QuerySnapshot.toJSON bundle getDocFromCache', async () => { + it('DDB4 local QuerySnapshot.toJSON bundle getDocFromCache', async () => { let path: string | null = null; let jsonBundle: object | null = null; const testDocs = { @@ -99,6 +100,7 @@ apiDescribe('Queries', persistence => { }); }); expect(jsonBundle).to.not.be.null; + console.log("DEDB json bundle: ", jsonBundle); // eslint-disable-next-line @typescript-eslint/no-explicit-any const json = (jsonBundle as any).bundle; expect(json).to.exist; @@ -111,6 +113,8 @@ apiDescribe('Queries', persistence => { const docSnap = await getDocFromCache(docRef); expect(docSnap.exists); expect(docSnap.data()).to.deep.equal(testDocs.a); + console.log("DEDB metadata.fromCache: ", docSnap.metadata.fromCache); + //expect(docSnap.metadata.fromCache).to.be.true; }); }); From 4ec62ae78b4d458eee7c348082289ff4c2d22a10 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 1 Apr 2025 09:49:43 -0400 Subject: [PATCH 29/32] Guard test against missing path. --- .../test/integration/api/query.test.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 558771057f..c5fe3ebc10 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -64,7 +64,6 @@ import { toChangesArray, toDataArray, PERSISTENCE_MODE_UNSPECIFIED, - withAlternateTestDb, withEmptyTestCollection, withRetry, withTestCollection, @@ -77,7 +76,7 @@ import { captureExistenceFilterMismatches } from '../util/testing_hooks_util'; apiDescribe('Queries', persistence => { addEqualityMatcher(); - it('DDB4 local QuerySnapshot.toJSON bundle getDocFromCache', async () => { + it('QuerySnapshot.toJSON bundle getDocFromCache', async () => { let path: string | null = null; let jsonBundle: object | null = null; const testDocs = { @@ -100,22 +99,21 @@ apiDescribe('Queries', persistence => { }); }); expect(jsonBundle).to.not.be.null; - console.log("DEDB json bundle: ", jsonBundle); // eslint-disable-next-line @typescript-eslint/no-explicit-any const json = (jsonBundle as any).bundle; expect(json).to.exist; expect(json.length).to.be.greaterThan(0); - // Use a new instance to load the bundle to ensure that the cache is primed by the bundle and - // not the previous getDocs call. - await withTestDb(persistence, async db => { - await loadBundle(db, json); - const docRef = doc(db, path!); - const docSnap = await getDocFromCache(docRef); - expect(docSnap.exists); - expect(docSnap.data()).to.deep.equal(testDocs.a); - console.log("DEDB metadata.fromCache: ", docSnap.metadata.fromCache); - //expect(docSnap.metadata.fromCache).to.be.true; - }); + + if (path !== null) { + await withTestDb(persistence, async db => { + const docRef = doc(db, path!); + await loadBundle(db, json); + + const docSnap = await getDocFromCache(docRef); + expect(docSnap.exists); + expect(docSnap.data()).to.deep.equal(testDocs.a); + }); + } }); it('can issue limit queries', () => { From 36e4fd50d89d59334890a8fdc05e61cf2f960024 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 1 Apr 2025 10:39:07 -0400 Subject: [PATCH 30/32] bundle test --- .../test/integration/api/query.test.ts | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index c5fe3ebc10..01fd0e47e3 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -37,11 +37,9 @@ import { endAt, endBefore, GeoPoint, - getDocFromCache, getDocs, limit, limitToLast, - loadBundle, onSnapshot, or, orderBy, @@ -76,46 +74,6 @@ import { captureExistenceFilterMismatches } from '../util/testing_hooks_util'; apiDescribe('Queries', persistence => { addEqualityMatcher(); - it('QuerySnapshot.toJSON bundle getDocFromCache', async () => { - let path: string | null = null; - let jsonBundle: object | null = null; - const testDocs = { - a: { k: 'a' }, - b: { k: 'b' }, - c: { k: 'c' } - }; - // Write an initial document in an isolated Firestore instance so it's not stored in the cache. - await withTestCollection(persistence, testDocs, async collection => { - await getDocs(query(collection)).then(querySnapshot => { - expect(querySnapshot.docs.length).to.equal(3); - // Find the path to a known doc. - querySnapshot.docs.forEach(docSnapshot => { - if (docSnapshot.ref.path.endsWith('a')) { - path = docSnapshot.ref.path; - } - }); - expect(path).to.not.be.null; - jsonBundle = querySnapshot.toJSON(); - }); - }); - expect(jsonBundle).to.not.be.null; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const json = (jsonBundle as any).bundle; - expect(json).to.exist; - expect(json.length).to.be.greaterThan(0); - - if (path !== null) { - await withTestDb(persistence, async db => { - const docRef = doc(db, path!); - await loadBundle(db, json); - - const docSnap = await getDocFromCache(docRef); - expect(docSnap.exists); - expect(docSnap.data()).to.deep.equal(testDocs.a); - }); - } - }); - it('can issue limit queries', () => { const testDocs = { a: { k: 'a' }, From bad6f72b7bcb018aa1e82dc180f56b696dbe08d9 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 1 Apr 2025 14:21:02 -0400 Subject: [PATCH 31/32] Minify fix. --- packages/firestore/src/api/snapshot.ts | 14 ++++-- .../test/integration/api/query.test.ts | 43 +++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index dba5c409ab..809132ae47 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -505,12 +505,16 @@ export class DocumentSnapshot< toJSON(): object { const document = this._document; + const result : any = { }; + result['bundle'] = ''; + result['source'] = 'DocumentSnapshot'; + if ( !document || !document.isValidDocument() || !document.isFoundDocument() ) { - return { bundle: '' }; + return result; } const builder: BundleBuilder = new BundleBuilder( this._firestore, @@ -534,7 +538,8 @@ export class DocumentSnapshot< document ) ); - return { bundle: builder.build() }; + result['bundle'] = builder.build(); + return result; } } @@ -693,6 +698,8 @@ export class QuerySnapshot< } toJSON(): object { + const result : any = { }; + result['source'] = 'QuerySnapshot'; const builder: BundleBuilder = new BundleBuilder( this._firestore, AutoId.newId() @@ -731,7 +738,8 @@ export class QuerySnapshot< docBundleDataArray }; builder.addBundleQuery(bundleData); - return { bundle: builder.build() }; + result['bundle'] = builder.build(); + return result; } } diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 01fd0e47e3..731805b5a9 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -37,9 +37,11 @@ import { endAt, endBefore, GeoPoint, + getDocFromCache, getDocs, limit, limitToLast, + loadBundle, onSnapshot, or, orderBy, @@ -74,6 +76,47 @@ import { captureExistenceFilterMismatches } from '../util/testing_hooks_util'; apiDescribe('Queries', persistence => { addEqualityMatcher(); + it('QuerySnapshot.toJSON bundle getDocFromCache', async () => { + let path: string | null = null; + let jsonBundle: object | null = null; + const testDocs = { + a: { k: 'a' }, + b: { k: 'b' }, + c: { k: 'c' } + }; + // Write an initial document in an isolated Firestore instance so it's not stored in the cache. + await withTestCollection(persistence, testDocs, async collection => { + await getDocs(query(collection)).then(querySnapshot => { + expect(querySnapshot.docs.length).to.equal(3); + // Find the path to a known doc. + querySnapshot.docs.forEach(docSnapshot => { + if (docSnapshot.ref.path.endsWith('a')) { + path = docSnapshot.ref.path; + } + }); + expect(path).to.not.be.null; + jsonBundle = querySnapshot.toJSON(); + }); + }); + expect(jsonBundle).to.not.be.null; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const json = (jsonBundle as any).bundle; + console.log("DEDB checking if json exists"); + expect(json).to.exist; + expect(json.length).to.be.greaterThan(0); + + if (path !== null) { + await withTestDb(persistence, async db => { + const docRef = doc(db, path!); + await loadBundle(db, json); + const docSnap = await getDocFromCache(docRef); + console.log("DEDB checking if docSnap exists"); + expect(docSnap.exists); + expect(docSnap.data()).to.deep.equal(testDocs.a); + }); + } + }); + it('can issue limit queries', () => { const testDocs = { a: { k: 'a' }, From a5b46b73d69754af712336f49d98e0a4caa6940a Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Tue, 1 Apr 2025 14:42:30 -0400 Subject: [PATCH 32/32] lint fixes, console log removal. --- packages/firestore/src/api/snapshot.ts | 8 +++++--- packages/firestore/test/integration/api/query.test.ts | 2 -- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/firestore/src/api/snapshot.ts b/packages/firestore/src/api/snapshot.ts index 809132ae47..9d2ddf41a7 100644 --- a/packages/firestore/src/api/snapshot.ts +++ b/packages/firestore/src/api/snapshot.ts @@ -505,10 +505,11 @@ export class DocumentSnapshot< toJSON(): object { const document = this._document; - const result : any = { }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result: any = {}; result['bundle'] = ''; result['source'] = 'DocumentSnapshot'; - + if ( !document || !document.isValidDocument() || @@ -698,7 +699,8 @@ export class QuerySnapshot< } toJSON(): object { - const result : any = { }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result: any = {}; result['source'] = 'QuerySnapshot'; const builder: BundleBuilder = new BundleBuilder( this._firestore, diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 731805b5a9..9eb537db4c 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -101,7 +101,6 @@ apiDescribe('Queries', persistence => { expect(jsonBundle).to.not.be.null; // eslint-disable-next-line @typescript-eslint/no-explicit-any const json = (jsonBundle as any).bundle; - console.log("DEDB checking if json exists"); expect(json).to.exist; expect(json.length).to.be.greaterThan(0); @@ -110,7 +109,6 @@ apiDescribe('Queries', persistence => { const docRef = doc(db, path!); await loadBundle(db, json); const docSnap = await getDocFromCache(docRef); - console.log("DEDB checking if docSnap exists"); expect(docSnap.exists); expect(docSnap.data()).to.deep.equal(testDocs.a); });