Merge branch 'master' into mrschmidt/initializefirestore

Author: schmidt-sebastian

packages/firestore/exp/index.d.ts

@@ -15,4 +15,4 @@
  * limitations under the License.
  */
 
-export { foo, bar } from './src/api/foobar';
+// TODO(firestorexp): Export API

packages/firestore/exp/index.node.ts

@@ -15,6 +15,8 @@
  * limitations under the License.
  */
 
+// TODO(firestorexp): Replace with actual implementation
+
 export function foo(): string {
   return bar();
 }

packages/firestore/exp/src/api/foobar.ts

@@ -23,7 +23,9 @@ import * as dependencies from './dependencies.json';
 import * as pkg from '../../../package.json';
 import { forEach } from '../../../src/util/obj';
 
-describe('Dependencies', () => {
+// TODO(firestorexp): Enable test
+// eslint-disable-next-line no-restricted-properties
+describe.skip('Dependencies', () => {
   forEach(dependencies, (api, { dependencies }) => {
     it(api, () => {
       return extractDependencies(api, pkg.exp).then(extractedDependencies => {

packages/firestore/exp/test/deps/verify_dependencies.test.ts

@@ -88,7 +88,11 @@ import {
   PartialObserver,
   Unsubscribe
 } from './observer';
-import { fieldPathFromArgument, UserDataReader } from './user_data_reader';
+import {
+  DocumentKeyReference,
+  fieldPathFromArgument,
+  UserDataReader
+} from './user_data_reader';
 import { UserDataWriter } from './user_data_writer';
 import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
 import { Provider } from '@firebase/component';
@@ -935,6 +939,7 @@ export class WriteBatch implements firestore.WriteBatch {
  * A reference to a particular document in a collection in the database.
  */
 export class DocumentReference<T = firestore.DocumentData>
+  extends DocumentKeyReference<T>
   implements firestore.DocumentReference<T> {
   private _firestoreClient: FirestoreClient;
 
@@ -943,6 +948,7 @@ export class DocumentReference<T = firestore.DocumentData>
     readonly firestore: Firestore,
     readonly _converter?: firestore.FirestoreDataConverter<T>
   ) {
+    super(firestore._databaseId, _key, _converter);
     this._firestoreClient = this.firestore.ensureClientConfigured();
   }
 
@@ -1347,10 +1353,10 @@ export class DocumentSnapshot<T = firestore.DocumentData>
         return this._converter.fromFirestore(snapshot, options);
       } else {
         const userDataWriter = new UserDataWriter(
-          this._firestore,
+          this._firestore._databaseId,
           this._firestore._areTimestampsInSnapshotsEnabled(),
-          options.serverTimestamps,
-          /* converter= */ undefined
+          options.serverTimestamps || 'none',
+          key => new DocumentReference(key, this._firestore)
         );
         return userDataWriter.convertValue(this._document.toProto()) as T;
       }
@@ -1369,10 +1375,10 @@ export class DocumentSnapshot<T = firestore.DocumentData>
         .field(fieldPathFromArgument('DocumentSnapshot.get', fieldPath));
       if (value !== null) {
         const userDataWriter = new UserDataWriter(
-          this._firestore,
+          this._firestore._databaseId,
           this._firestore._areTimestampsInSnapshotsEnabled(),
-          options.serverTimestamps,
-          this._converter
+          options.serverTimestamps || 'none',
+          key => new DocumentReference(key, this._firestore, this._converter)
         );
         return userDataWriter.convertValue(value);
       }
@@ -2322,12 +2328,6 @@ export class CollectionReference<T = firestore.DocumentData> extends Query<T>
       1,
       pathString
     );
-    if (pathString === '') {
-      throw new FirestoreError(
-        Code.INVALID_ARGUMENT,
-        'Document path must be a non-empty string'
-      );
-    }
     const path = ResourcePath.fromString(pathString!);
     return DocumentReference.forPath<T>(
       this._query.path.child(path),
@@ -2425,8 +2425,8 @@ function validateReference<T>(
   methodName: string,
   documentRef: firestore.DocumentReference<T>,
   firestore: Firestore
-): DocumentReference<T> {
-  if (!(documentRef instanceof DocumentReference)) {
+): DocumentKeyReference<T> {
+  if (!(documentRef instanceof DocumentKeyReference)) {
     throw invalidClassError(methodName, 'DocumentReference', 1, documentRef);
   } else if (documentRef.firestore !== firestore) {
     throw new FirestoreError(

packages/firestore/src/api/database.ts

@@ -46,10 +46,23 @@ import {
 import { DeleteFieldValueImpl, FieldValueImpl } from './field_value';
 import { GeoPoint } from './geo_point';
 import { PlatformSupport } from '../platform/platform';
-import { DocumentReference } from './database';
 
 const RESERVED_FIELD_REGEX = /^__.*__$/;
 
+/**
+ * A reference to a document in a Firebase project.
+ *
+ * This class serves as a common base class for the public DocumentReferences
+ * exposed in the lite, full and legacy SDK.
+ */
+export class DocumentKeyReference<T> {
+  constructor(
+    public readonly _databaseId: DatabaseId,
+    public readonly _key: DocumentKey,
+    public readonly _converter?: firestore.FirestoreDataConverter<T>
+  ) {}
+}
+
 /** The result of parsing document data (e.g. for a setData call). */
 export class ParsedSetData {
   constructor(
@@ -652,9 +665,9 @@ function parseScalarValue(
     };
   } else if (value instanceof Blob) {
     return { bytesValue: context.serializer.toBytes(value) };
-  } else if (value instanceof DocumentReference) {
+  } else if (value instanceof DocumentKeyReference) {
     const thisDb = context.databaseId;
-    const otherDb = value.firestore._databaseId;
+    const otherDb = value._databaseId;
     if (!otherDb.isEqual(thisDb)) {
       throw context.createError(
         'Document reference is for database ' +
@@ -665,7 +678,7 @@ function parseScalarValue(
     return {
       referenceValue: context.serializer.toResourceName(
         value._key.path,
-        value.firestore._databaseId
+        value._databaseId
       )
     };
   } else if (value === undefined && context.ignoreUndefinedProperties) {
@@ -693,7 +706,7 @@ function looksLikeJsonObject(input: unknown): boolean {
     !(input instanceof Timestamp) &&
     !(input instanceof GeoPoint) &&
     !(input instanceof Blob) &&
-    !(input instanceof DocumentReference) &&
+    !(input instanceof DocumentKeyReference) &&
     !(input instanceof FieldValueImpl)
   );
 }

packages/firestore/src/api/user_data_reader.ts

@@ -19,7 +19,7 @@ import * as firestore from '@firebase/firestore-types';
 
 import * as api from '../protos/firestore_proto_api';
 
-import { DocumentReference, Firestore } from './database';
+import { DocumentKeyReference } from './user_data_reader';
 import { Blob } from './blob';
 import { GeoPoint } from './geo_point';
 import { Timestamp } from './timestamp';
@@ -48,12 +48,14 @@ export type ServerTimestampBehavior = 'estimate' | 'previous' | 'none';
  * Converts Firestore's internal types to the JavaScript types that we expose
  * to the user.
  */
-export class UserDataWriter<T = firestore.DocumentData> {
+export class UserDataWriter {
   constructor(
-    private readonly firestore: Firestore,
+    private readonly databaseId: DatabaseId,
     private readonly timestampsInSnapshots: boolean,
-    private readonly serverTimestampBehavior?: ServerTimestampBehavior,
-    private readonly converter?: firestore.FirestoreDataConverter<T>
+    private readonly serverTimestampBehavior: ServerTimestampBehavior,
+    private readonly referenceFactory: (
+      key: DocumentKey
+    ) => DocumentKeyReference<firestore.DocumentData>
   ) {}
 
   convertValue(value: api.Value): unknown {
@@ -132,7 +134,9 @@ export class UserDataWriter<T = firestore.DocumentData> {
     }
   }
 
-  private convertReference(name: string): DocumentReference<T> {
+  private convertReference(
+    name: string
+  ): DocumentKeyReference<firestore.DocumentData> {
     const resourcePath = ResourcePath.fromString(name);
     hardAssert(
       isValidResourceName(resourcePath),
@@ -141,18 +145,18 @@ export class UserDataWriter<T = firestore.DocumentData> {
     const databaseId = new DatabaseId(resourcePath.get(1), resourcePath.get(3));
     const key = new DocumentKey(resourcePath.popFirst(5));
 
-    if (!databaseId.isEqual(this.firestore._databaseId)) {
+    if (!databaseId.isEqual(this.databaseId)) {
       // TODO(b/64130202): Somehow support foreign references.
       logError(
         `Document ${key} contains a document ` +
           `reference within a different database (` +
           `${databaseId.projectId}/${databaseId.database}) which is not ` +
           `supported. It will be treated as a reference in the current ` +
-          `database (${this.firestore._databaseId.projectId}/${this.firestore._databaseId.database}) ` +
+          `database (${this.databaseId.projectId}/${this.databaseId.database}) ` +
           `instead.`
       );
     }
 
-    return new DocumentReference(key, this.firestore, this.converter);
+    return this.referenceFactory(key);
   }
 }

packages/firestore/src/api/user_data_writer.ts

@@ -133,10 +133,12 @@ export class MemoryComponentProvider implements ComponentProvider {
   }
 
   createPersistence(cfg: ComponentConfiguration): Persistence {
-    debugAssert(
-      !cfg.persistenceSettings.durable,
-      'Can only start memory persistence'
-    );
+    if (cfg.persistenceSettings.durable) {
+      throw new FirestoreError(
+        Code.FAILED_PRECONDITION,
+        MEMORY_ONLY_PERSISTENCE_ERROR_MESSAGE
+      );
+    }
     return new MemoryPersistence(MemoryEagerDelegate.factory);
   }
 
@@ -162,6 +164,7 @@ export class MemoryComponentProvider implements ComponentProvider {
     return new SyncEngine(
       this.localStore,
       this.remoteStore,
+      cfg.datastore,
       this.sharedClientState,
       cfg.initialUser,
       cfg.maxConcurrentLimboResolutions
@@ -219,6 +222,7 @@ export class IndexedDbComponentProvider extends MemoryComponentProvider {
     const syncEngine = new MultiTabSyncEngine(
       this.localStore,
       this.remoteStore,
+      cfg.datastore,
       this.sharedClientState,
       cfg.initialUser,
       cfg.maxConcurrentLimboResolutions

packages/firestore/src/core/component_provider.ts

@@ -74,6 +74,7 @@ import {
 import { ViewSnapshot } from './view_snapshot';
 import { AsyncQueue, wrapInUserErrorIfRecoverable } from '../util/async_queue';
 import { TransactionRunner } from './transaction_runner';
+import { Datastore } from '../remote/datastore';
 
 const LOG_TAG = 'SyncEngine';
 
@@ -185,6 +186,7 @@ export class SyncEngine implements RemoteSyncer {
   constructor(
     protected localStore: LocalStore,
     protected remoteStore: RemoteStore,
+    protected datastore: Datastore,
     // PORTING NOTE: Manages state synchronization in multi-tab environments.
     protected sharedClientState: SharedClientState,
     private currentUser: User,
@@ -390,7 +392,7 @@ export class SyncEngine implements RemoteSyncer {
   ): void {
     new TransactionRunner<T>(
       asyncQueue,
-      this.remoteStore,
+      this.datastore,
       updateFunction,
       deferred
     ).run();
@@ -924,13 +926,15 @@ export class MultiTabSyncEngine extends SyncEngine
   constructor(
     protected localStore: MultiTabLocalStore,
     remoteStore: RemoteStore,
+    datastore: Datastore,
     sharedClientState: SharedClientState,
     currentUser: User,
     maxConcurrentLimboResolutions: number
   ) {
     super(
       localStore,
       remoteStore,
+      datastore,
       sharedClientState,
       currentUser,
       maxConcurrentLimboResolutions
@@ -1110,14 +1114,10 @@ export class MultiTabSyncEngine extends SyncEngine
       const queries = this.queriesByTarget.get(targetId);
 
       if (queries && queries.length !== 0) {
-        // For queries that have a local View, we need to update their state
-        // in LocalStore (as the resume token and the snapshot version
+        // For queries that have a local View, we fetch their current state
+        // from LocalStore (as the resume token and the snapshot version
         // might have changed) and reconcile their views with the persisted
         // state (the list of syncedDocuments may have gotten out of sync).
-        await this.localStore.releaseTarget(
-          targetId,
-          /*keepPersistedTargetData=*/ true
-        );
         targetData = await this.localStore.allocateTarget(
           queries[0].toTarget()
         );
@@ -1136,7 +1136,7 @@ export class MultiTabSyncEngine extends SyncEngine
       } else {
         debugAssert(
           transitionToPrimary,
-          'A secondary tab should never have an active target without an active query.'
+          'A secondary tab should never have an active view without an active target.'
         );
         // For queries that never executed on this client, we need to
         // allocate the target in LocalStore and initialize a new View.