Remove unused baseDoc arg and iterate fowards in local serializer

Author: Brian Chen

common/api-review/app-exp.api.md

@@ -5,9 +5,9 @@
 ```ts
 
 import { Component } from '@firebase/component';
-import { FirebaseApp } from '@firebase/app-types';
-import { FirebaseAppConfig } from '@firebase/app-types';
-import { FirebaseOptions } from '@firebase/app-types';
+import { FirebaseApp } from '@firebase/app-types-exp';
+import { FirebaseAppConfig } from '@firebase/app-types-exp';
+import { FirebaseOptions } from '@firebase/app-types-exp';
 import { LogCallback } from '@firebase/logger';
 import { LogLevelString } from '@firebase/logger';
 import { LogOptions } from '@firebase/logger';

common/api-review/firestore-lite.api.md

@@ -201,7 +201,9 @@ export class FirebaseFirestore implements FirestoreService {
     protected _terminate(): Promise<void>;
     // (undocumented)
     get _terminated(): boolean;
-    }
+    // (undocumented)
+    toJSON(): object;
+}
 
 // @public
 export interface FirestoreDataConverter<T> {

common/api-review/installations-exp.api.md

@@ -10,6 +10,8 @@ import { FirebaseInstallations } from '@firebase/installations-types-exp';
 // @public
 export function deleteInstallations(installations: FirebaseInstallations): Promise<void>;
 
+export { FirebaseInstallations }
+
 // @public
 export function getId(installations: FirebaseInstallations): Promise<string>;
 

common/api-review/messaging-exp.api.md

@@ -14,6 +14,8 @@ import { Unsubscribe } from '@firebase/util';
 // @public
 export function deleteToken(messaging: FirebaseMessaging): Promise<boolean>;
 
+export { FirebaseMessaging }
+
 // @public
 export function getMessaging(app: FirebaseApp): FirebaseMessaging;
 
@@ -23,6 +25,8 @@ export function getToken(messaging: FirebaseMessaging, options?: {
     swReg?: ServiceWorkerRegistration;
 }): Promise<string>;
 
+export { MessagePayload }
+
 // @public
 export function onMessage(messaging: FirebaseMessaging, nextOrObserver: NextFn<MessagePayload> | Observer<MessagePayload>): Unsubscribe;
 

common/api-review/performance-exp.api.md

@@ -9,9 +9,15 @@ import { FirebasePerformance } from '@firebase/performance-types-exp';
 import { PerformanceSettings } from '@firebase/performance-types-exp';
 import { PerformanceTrace } from '@firebase/performance-types-exp';
 
+export { FirebasePerformance }
+
 // @public
 export function getPerformance(app: FirebaseApp, settings?: PerformanceSettings): FirebasePerformance;
 
+export { PerformanceSettings }
+
+export { PerformanceTrace }
+
 // @public
 export function trace(performance: FirebasePerformance, name: string): PerformanceTrace;
 

common/api-review/remote-config-exp.api.md

@@ -5,9 +5,9 @@
 ```ts
 
 import { FirebaseApp } from '@firebase/app-types-exp';
-import { LogLevel } from '@firebase/remote-config-types-exp';
 import { RemoteConfig } from '@firebase/remote-config-types-exp';
-import { Value } from '@firebase/remote-config-types-exp';
+import { LogLevel as RemoteConfigLogLevel } from '@firebase/remote-config-types-exp';
+import { Value as ValueType } from '@firebase/remote-config-types-exp';
 
 // @public
 export function activate(remoteConfig: RemoteConfig): Promise<boolean>;
@@ -22,7 +22,7 @@ export function fetchAndActivate(remoteConfig: RemoteConfig): Promise<boolean>;
 export function fetchConfig(remoteConfig: RemoteConfig): Promise<void>;
 
 // @public
-export function getAll(remoteConfig: RemoteConfig): Record<string, Value>;
+export function getAll(remoteConfig: RemoteConfig): Record<string, ValueType>;
 
 // @public
 export function getBoolean(remoteConfig: RemoteConfig, key: string): boolean;
@@ -37,10 +37,16 @@ export function getRemoteConfig(app: FirebaseApp): RemoteConfig;
 export function getString(remoteConfig: RemoteConfig, key: string): string;
 
 // @public
-export function getValue(remoteConfig: RemoteConfig, key: string): Value;
+export function getValue(remoteConfig: RemoteConfig, key: string): ValueType;
+
+export { RemoteConfig }
+
+export { RemoteConfigLogLevel }
 
 // @public
-export function setLogLevel(remoteConfig: RemoteConfig, logLevel: LogLevel): void;
+export function setLogLevel(remoteConfig: RemoteConfig, logLevel: RemoteConfigLogLevel): void;
+
+export { ValueType }
 
 
 // (No @packageDocumentation comment for this package)

packages/firestore/src/local/local_documents_view.ts

@@ -268,7 +268,6 @@ export class LocalDocumentsView {
               const mutatedDoc = applyMutationToLocalView(
                 mutation,
                 baseDoc,
-                baseDoc,
                 batch.localWriteTime
               );
               if (mutatedDoc instanceof Document) {

packages/firestore/src/local/local_serializer.ts

@@ -201,19 +201,21 @@ export function fromDbMutationBatch(
   // on the SDK means that old `transform` mutations stored in IndexedDB need
   // to be updated to `update_transforms`.
   // TODO(b/174608374): Remove this code once we perform a schema migration.
-  for (let i = dbBatch.mutations.length - 1; i >= 0; --i) {
-    const mutationProto = dbBatch.mutations[i];
-    if (mutationProto?.transform !== undefined) {
+  for (let i = 0; i < dbBatch.mutations.length - 1; ++i) {
+    const currentMutation = dbBatch.mutations[i];
+    const hasTransform =
+      i + 1 < dbBatch.mutations.length &&
+      dbBatch.mutations[i + 1].transform !== undefined;
+    if (hasTransform) {
       debugAssert(
-        i >= 1 &&
-          dbBatch.mutations[i - 1].transform === undefined &&
-          dbBatch.mutations[i - 1].update !== undefined,
+        dbBatch.mutations[i].transform === undefined &&
+          dbBatch.mutations[i].update !== undefined,
         'TransformMutation should be preceded by a patch or set mutation'
       );
-      const mutationToJoin = dbBatch.mutations[i - 1];
-      mutationToJoin.updateTransforms = mutationProto.transform.fieldTransforms;
-      dbBatch.mutations.splice(i, 1);
-      --i;
+      const transformMutation = dbBatch.mutations[i + 1];
+      currentMutation.updateTransforms = transformMutation.transform!.fieldTransforms;
+      dbBatch.mutations.splice(i + 1, 1);
+      ++i;
     }
   }
 

packages/firestore/src/model/mutation.ts

@@ -270,9 +270,6 @@ export function applyMutationToRemoteDocument(
  * @param mutation - The mutation to apply.
  * @param maybeDoc - The document to mutate. The input document can be null if
  *     the client has no knowledge of the pre-mutation state of the document.
- * @param baseDoc - The state of the document prior to this mutation batch. The
- *     input document can be null if the client has no knowledge of the
- *     pre-mutation state of the document.
  * @param localWriteTime - A timestamp indicating the local write time of the
  *     batch this mutation is a part of.
  * @returns The mutated document. The returned document may be null, but only
@@ -281,25 +278,14 @@ export function applyMutationToRemoteDocument(
 export function applyMutationToLocalView(
   mutation: Mutation,
   maybeDoc: MaybeDocument | null,
-  baseDoc: MaybeDocument | null,
   localWriteTime: Timestamp
 ): MaybeDocument | null {
   verifyMutationKeyMatches(mutation, maybeDoc);
 
   if (mutation instanceof SetMutation) {
-    return applySetMutationToLocalView(
-      mutation,
-      maybeDoc,
-      localWriteTime,
-      baseDoc
-    );
+    return applySetMutationToLocalView(mutation, maybeDoc, localWriteTime);
   } else if (mutation instanceof PatchMutation) {
-    return applyPatchMutationToLocalView(
-      mutation,
-      maybeDoc,
-      localWriteTime,
-      baseDoc
-    );
+    return applyPatchMutationToLocalView(mutation, maybeDoc, localWriteTime);
   } else {
     debugAssert(
       mutation instanceof DeleteMutation,
@@ -469,8 +455,7 @@ function applySetMutationToRemoteDocument(
 function applySetMutationToLocalView(
   mutation: SetMutation,
   maybeDoc: MaybeDocument | null,
-  localWriteTime: Timestamp,
-  baseDoc: MaybeDocument | null
+  localWriteTime: Timestamp
 ): MaybeDocument | null {
   if (!preconditionIsValidForDocument(mutation.precondition, maybeDoc)) {
     return maybeDoc;
@@ -480,8 +465,7 @@ function applySetMutationToLocalView(
   const transformResults = localTransformResults(
     mutation.fieldTransforms,
     localWriteTime,
-    maybeDoc,
-    baseDoc
+    maybeDoc
   );
   newData = transformObject(
     mutation.fieldTransforms,
@@ -551,8 +535,7 @@ function applyPatchMutationToRemoteDocument(
 function applyPatchMutationToLocalView(
   mutation: PatchMutation,
   maybeDoc: MaybeDocument | null,
-  localWriteTime: Timestamp,
-  baseDoc: MaybeDocument | null
+  localWriteTime: Timestamp
 ): MaybeDocument | null {
   if (!preconditionIsValidForDocument(mutation.precondition, maybeDoc)) {
     return maybeDoc;
@@ -562,8 +545,7 @@ function applyPatchMutationToLocalView(
   const transformResults = localTransformResults(
     mutation.fieldTransforms,
     localWriteTime,
-    maybeDoc,
-    baseDoc
+    maybeDoc
   );
   const newData = patchDocument(mutation, maybeDoc, transformResults);
   return new Document(mutation.key, version, newData, {
@@ -613,13 +595,14 @@ function patchObject(mutation: PatchMutation, data: ObjectValue): ObjectValue {
  * containing transforms has been acknowledged by the server.
  *
  * @param fieldTransforms - The field transforms to apply the result to.
- * @param baseDoc - The document prior to applying this mutation batch.
+ * @param maybeDoc - The current state of the document after applying all
+ * previous mutations.
  * @param serverTransformResults - The transform results received by the server.
  * @returns The transform results list.
  */
 function serverTransformResults(
   fieldTransforms: FieldTransform[],
-  baseDoc: MaybeDocument | null,
+  maybeDoc: MaybeDocument | null,
   serverTransformResults: Array<ProtoValue | null>
 ): ProtoValue[] {
   const transformResults: ProtoValue[] = [];
@@ -633,8 +616,8 @@ function serverTransformResults(
     const fieldTransform = fieldTransforms[i];
     const transform = fieldTransform.transform;
     let previousValue: ProtoValue | null = null;
-    if (baseDoc instanceof Document) {
-      previousValue = baseDoc.field(fieldTransform.field);
+    if (maybeDoc instanceof Document) {
+      previousValue = maybeDoc.field(fieldTransform.field);
     }
     transformResults.push(
       applyTransformOperationToRemoteDocument(
@@ -657,14 +640,12 @@ function serverTransformResults(
  *     generate ServerTimestampValues).
  * @param maybeDoc - The current state of the document after applying all
  *     previous mutations.
- * @param baseDoc - The document prior to applying this mutation batch.
  * @returns The transform results list.
  */
 function localTransformResults(
   fieldTransforms: FieldTransform[],
   localWriteTime: Timestamp,
-  maybeDoc: MaybeDocument | null,
-  baseDoc: MaybeDocument | null
+  maybeDoc: MaybeDocument | null
 ): ProtoValue[] {
   const transformResults: ProtoValue[] = [];
   for (const fieldTransform of fieldTransforms) {

packages/firestore/src/model/mutation_batch.ts

@@ -132,21 +132,17 @@ export class MutationBatch {
         maybeDoc = applyMutationToLocalView(
           mutation,
           maybeDoc,
-          maybeDoc,
           this.localWriteTime
         );
       }
     }
 
-    const baseDoc = maybeDoc;
-
     // Second, apply all user-provided mutations.
     for (const mutation of this.mutations) {
       if (mutation.key.isEqual(docKey)) {
         maybeDoc = applyMutationToLocalView(
           mutation,
           maybeDoc,
-          baseDoc,
           this.localWriteTime
         );
       }

packages/firestore/test/unit/local/local_serializer.test.ts

@@ -129,21 +129,6 @@ describe('Local Serializer', () => {
     );
   });
 
-  it('TransformMutation (legacy) on its own throws assertion', () => {
-    const dbMutationBatch = new DbMutationBatch(
-      userId,
-      batchId,
-      1000,
-      [],
-      [transformMutationWrite]
-    );
-    expect(() =>
-      fromDbMutationBatch(localSerializer, dbMutationBatch)
-    ).to.throw(
-      'TransformMutation should be preceded by a patch or set mutation'
-    );
-  });
-
   it('DeleteMutation + TransformMutation (legacy) on its own throws assertion', () => {
     const dbMutationBatch = new DbMutationBatch(
       userId,