Add settings() method (#257)

Author: schmidt-sebastian

package.json

@@ -41,7 +41,7 @@
     "generate-scaffolding": "repo-tools generate all",
     "system-test": "mocha build/system-test --timeout 600000",
     "conformance": "mocha build/conformance",
-    "test-only": "nyc mocha build/test",
+    "test-only": "GOOGLE_APPLICATION_CREDENTIALS=build/test/fake-certificate.json nyc mocha build/test",
     "test": "npm run test-only",
     "check": "gts check",
     "clean": "gts clean",

src/index.js

@@ -220,15 +220,15 @@ const GRPC_UNAVAILABLE = 14;
  */
 class Firestore {
   /**
-   * @param {Object=} options - [Configuration object](#/docs).
-   * @param {string=} options.projectId The Firestore Project ID. Can be
+   * @param {Object=} settings - [Configuration object](#/docs).
+   * @param {string=} settings.projectId The Firestore Project ID. Can be
    * omitted in environments that support `Application Default Credentials`
    * {@see https://cloud.google.com/docs/authentication}
-   * @param {string=} options.keyFilename Local file containing the Service
+   * @param {string=} settings.keyFilename Local file containing the Service
    * Account credentials. Can be omitted in environments that support
    * `Application Default Credentials`
    * {@see https://cloud.google.com/docs/authentication}
-   * @param {boolean=} options.timestampsInSnapshots Enables the use of
+   * @param {boolean=} settings.timestampsInSnapshots Enables the use of
    * `Timestamp`s for timestamp fields in `DocumentSnapshots`.<br/>
    * Currently, Firestore returns timestamp fields as `Date` but `Date` only
    * supports millisecond precision, which leads to truncation and causes
@@ -242,8 +242,8 @@ class Firestore {
    * default and this option will be removed so you should change your code to
    * use `Timestamp` now and opt-in to this new behavior as soon as you can.
    */
-  constructor(options) {
-    options = extend({}, options, {
+  constructor(settings) {
+    settings = extend({}, settings, {
       libName: 'gccl',
       libVersion: libVersion,
     });
@@ -257,11 +257,12 @@ class Firestore {
     this._clientPool = null;
 
     /**
-     * The configuration options for the GAPIC client.
+     * Whether the initialization settings can still be changed by invoking
+     * `settings()`.
      * @private
-     * @type {Object}
+     * @type {boolean}
      */
-    this._initalizationOptions = options;
+    this._settingsFrozen = false;
 
     /**
      * A Promise that resolves when client initialization completes. Can be
@@ -271,6 +272,15 @@ class Firestore {
      */
     this._clientInitialized = null;
 
+    /**
+     * The configuration options for the GAPIC client.
+     * @private
+     * @type {Object}
+     */
+    this._initalizationSettings = null;
+
+    this.validateAndApplySettings(settings);
+
     // GCF currently tears down idle connections after two minutes. Requests
     // that are issued after this period may fail. On GCF, we therefore issue
     // these requests as part of a transaction so that we can safely retry until
@@ -285,43 +295,59 @@ class Firestore {
       Firestore.log('Firestore', null, 'Detected GCF environment');
     }
 
-    this._timestampsInSnapshotsEnabled = !!options.timestampsInSnapshots;
+    Firestore.log('Firestore', null, 'Initialized Firestore');
+  }
 
-    if (!this._timestampsInSnapshotsEnabled) {
-      console.error(`
-The behavior for Date objects stored in Firestore is going to change
-AND YOUR APP MAY BREAK.
-To hide this warning and ensure your app does not break, you need to add the
-following code to your app before calling any other Cloud Firestore methods:
+  /**
+   * Specifies custom settings to be used to configure the `Firestore`
+   * instance. Can only be invoked once and before any other Firestore method.
+   *
+   * If settings are provided via both `settings()` and the `Firestore`
+   * constructor, both settings objects are merged and any settings provided via
+   * `settings()` take precedence.
+   *
+   * @param {object} settings The settings to use for all Firestore operations.
+   */
+  settings(settings) {
+    validate.isObject('settings', settings);
+    validate.isOptionalString('settings.projectId', settings.projectId);
+    validate.isOptionalBoolean(
+        'settings.timestampsInSnapshots', settings.timestampsInSnapshots);
 
-  const settings = {/* your settings... */ timestampsInSnapshots: true};
-  const firestore = new Firestore(settings);
+    if (this._clientInitialized) {
+      throw new Error(
+          'Firestore has already been started and its settings can no longer ' +
+          'be changed. You can only call settings() before calling any other ' +
+          'methods on a Firestore object.');
+    }
 
-With this change, timestamps stored in Cloud Firestore will be read back as
-Firebase Timestamp objects instead of as system Date objects. So you will also
-need to update code expecting a Date to instead expect a Timestamp. For example:
+    if (this._settingsFrozen) {
+      throw new Error(
+          'Firestore.settings() has already be called. You can only call ' +
+          'settings() once, and only before calling any other methods on a ' +
+          'Firestore object.');
+    }
 
-  // Old:
-  const date = snapshot.get('created_at');
-  // New:
-  const timestamp = snapshot.get('created_at');
-  const date = timestamp.toDate();
+    const mergedSettings = extend({}, this._initalizationSettings, settings);
+    this.validateAndApplySettings(mergedSettings);
+    this._settingsFrozen = true;
+  }
 
-Please audit all existing usages of Date when you enable the new behavior. In a
-future release, the behavior will change to the new behavior, so if you do not
-follow these steps, YOUR APP MAY BREAK.`);
-    }
+  validateAndApplySettings(settings) {
+    validate.isOptionalBoolean(
+        'settings.timestampsInSnapshots', settings.timestampsInSnapshots);
+    this._timestampsInSnapshotsEnabled = !!settings.timestampsInSnapshots;
 
-    if (options && options.projectId) {
-      validate.isString('options.projectId', options.projectId);
-      this._referencePath = new ResourcePath(options.projectId, '(default)');
+    if (settings && settings.projectId) {
+      validate.isString('settings.projectId', settings.projectId);
+      this._referencePath = new ResourcePath(settings.projectId, '(default)');
     } else {
       // Initialize a temporary reference path that will be overwritten during
       // project ID detection.
       this._referencePath = new ResourcePath('{{projectId}}', '(default)');
     }
 
-    Firestore.log('Firestore', null, 'Initialized Firestore');
+    this._initalizationSettings = settings;
   }
 
   /**
@@ -429,6 +455,12 @@ follow these steps, YOUR APP MAY BREAK.`);
    * for existing documents, otherwise a DocumentSnapshot.
    */
   snapshot_(documentOrName, readTime, encoding) {
+    if (!this._initalizationSettings.projectId) {
+      throw new Error(
+          'Cannot use `snapshot_()` without a Project ID. Please provide a ' +
+          'Project ID via `Firestore.settings()`.');
+    }
+
     let convertTimestamp;
     let convertDocument;
 
@@ -750,9 +782,36 @@ follow these steps, YOUR APP MAY BREAK.`);
     // Initialize the client pool if this is the first request.
     if (!this._clientInitialized) {
       common = require('@google-cloud/common');
+
+      if (!this._timestampsInSnapshotsEnabled) {
+        console.error(`
+The behavior for Date objects stored in Firestore is going to change
+AND YOUR APP MAY BREAK.
+To hide this warning and ensure your app does not break, you need to add the
+following code to your app before calling any other Cloud Firestore methods:
+
+  const firestore = new Firestore();
+  const settings = {/* your settings... */ timestampsInSnapshots: true};
+  firestore.settings(settings);
+
+With this change, timestamps stored in Cloud Firestore will be read back as
+Firebase Timestamp objects instead of as system Date objects. So you will also
+need to update code expecting a Date to instead expect a Timestamp. For example:
+
+  // Old:
+  const date = snapshot.get('created_at');
+  // New:
+  const timestamp = snapshot.get('created_at');
+  const date = timestamp.toDate();
+
+Please audit all existing usages of Date when you enable the new behavior. In a
+future release, the behavior will change to the new behavior, so if you do not
+follow these steps, YOUR APP MAY BREAK.`);
+      }
+
       this._clientInitialized = this._initClientPool().then(clientPool => {
         this._clientPool = clientPool;
-      })
+      });
     }
 
     return this._clientInitialized.then(() => this._clientPool.run(op));

src/validate.js

@@ -75,6 +75,7 @@ module.exports = validators => {
         },
         object: is.object,
         string: is.string,
+        boolean: is.boolean
       },
       validators);
 

test/index.js

@@ -32,6 +32,12 @@ const createInstance = require('../test/util/helpers').createInstance;
 
 const PROJECT_ID = 'test-project';
 const DATABASE_ROOT = `projects/${PROJECT_ID}/databases/(default)`;
+const DEFAULT_SETTINGS = {
+  projectId: PROJECT_ID,
+  sslCreds: grpc.credentials.createInsecure(),
+  keyFilename: './test/fake-certificate.json',
+  timestampsInSnapshots: true
+};
 
 // Change the argument to 'console.log' to enable debug output.
 Firestore.setLogFunction(() => {});
@@ -305,22 +311,64 @@ function stream() {
 
 describe('instantiation', function() {
   it('creates instance', function() {
-    let firestore = new Firestore({
-      projectId: PROJECT_ID,
-      sslCreds: grpc.credentials.createInsecure(),
-      timestampsInSnapshots: true,
-      keyFilename: './test/fake-certificate.json',
-    });
+    let firestore = new Firestore(DEFAULT_SETTINGS);
     assert(firestore instanceof Firestore);
   });
 
+  it('merges settings', function() {
+    let firestore = new Firestore(DEFAULT_SETTINGS);
+    firestore.settings({foo: 'bar'});
+
+    assert.equal(firestore._initalizationSettings.projectId, PROJECT_ID);
+    assert.equal(firestore._initalizationSettings.foo, 'bar');
+  });
+
+  it('can only call settings() once', function() {
+    let firestore = new Firestore(DEFAULT_SETTINGS);
+    firestore.settings({timestampsInSnapshots: true});
+
+    assert.throws(
+        () => firestore.settings({}),
+        /Firestore.settings\(\) has already be called. You can only call settings\(\) once, and only before calling any other methods on a Firestore object./);
+  });
+
+  it('cannot change settings after client initialized', function() {
+    let firestore = new Firestore(DEFAULT_SETTINGS);
+    firestore._runRequest(() => Promise.resolve());
+
+    assert.throws(
+        () => firestore.settings({}),
+        /Firestore has already been started and its settings can no longer be changed. You can only call settings\(\) before calling any other methods on a Firestore object./);
+  });
+
+  it('validates project ID is string', function() {
+    assert.throws(() => {
+      const settings = Object.assign({}, DEFAULT_SETTINGS, {
+        projectId: 1337,
+      });
+      new Firestore(settings);
+    }, /Argument "settings.projectId" is not a valid string/);
+
+    assert.throws(() => {
+      new Firestore(DEFAULT_SETTINGS).settings({projectId: 1337});
+    }, /Argument "settings.projectId" is not a valid string/);
+  });
+
+  it('validates timestampsInSnapshots is boolean', function() {
+    assert.throws(() => {
+      const settings = Object.assign({}, DEFAULT_SETTINGS, {
+        timestampsInSnapshots: 1337,
+      });
+      new Firestore(settings);
+    }, /Argument "settings.timestampsInSnapshots" is not a valid boolean/);
+
+    assert.throws(() => {
+      new Firestore(DEFAULT_SETTINGS).settings({timestampsInSnapshots: 1337});
+    }, /Argument "settings.timestampsInSnapshots" is not a valid boolean/);
+  });
+
   it('uses project id from constructor', () => {
-    let firestore = new Firestore({
-      projectId: PROJECT_ID,
-      sslCreds: grpc.credentials.createInsecure(),
-      timestampsInSnapshots: true,
-      keyFilename: './test/fake-certificate.json',
-    });
+    let firestore = new Firestore(DEFAULT_SETTINGS);
 
     return firestore._runRequest(() => {
       assert.equal(
@@ -367,7 +415,20 @@ describe('instantiation', function() {
     });
   });
 
-  it('handles error from project ID detection', () => {
+  it('uses project ID from settings()', function() {
+    let firestore = new Firestore({
+      sslCreds: grpc.credentials.createInsecure(),
+      timestampsInSnapshots: true,
+      keyFilename: './test/fake-certificate.json',
+    });
+
+    firestore.settings({projectId: PROJECT_ID});
+
+    assert.equal(
+        firestore.formattedName, `projects/${PROJECT_ID}/databases/(default)`);
+  });
+
+  it('handles error from project ID detection', function() {
     let firestore = new Firestore({
       sslCreds: grpc.credentials.createInsecure(),
       timestampsInSnapshots: true,
@@ -479,6 +540,17 @@ describe('snapshot_() method', function() {
     });
   });
 
+  it('validates Project ID provided', function() {
+    firestore = new Firestore({
+      sslCreds: grpc.credentials.createInsecure(),
+      keyFilename: './test/fake-certificate.json'
+    });
+
+    assert.throws(
+        () => firestore.snapshot_(),
+        /Cannot use `snapshot_\(\)` without a Project ID. Please provide a Project ID via `Firestore.settings\(\)`./);
+  });
+
   it('handles ProtobufJS', function() {
     let doc = firestore.snapshot_(
         document('doc', {

test/typescript.ts

@@ -53,6 +53,12 @@ xdescribe('firestore.d.ts', function() {
   FirebaseFirestore.setLogFunction(console.log);
 
   it('has typings for Firestore', () => {
+    firestore.settings({
+      keyFilename: 'foo',
+      projectId: 'foo',
+      timestampsInSnapshots: true,
+      otherOption: 'foo'
+    });
     const collRef: CollectionReference = firestore.collection('coll');
     const docRef1: DocumentReference = firestore.doc('coll/doc');
     const docRef2: DocumentReference = firestore.doc('coll/doc');

test/util/helpers.ts

@@ -80,7 +80,8 @@ export function createInstance(
       },
       firestoreSettings);
 
-  const firestore = new Firestore(initializationOptions);
+  const firestore = new Firestore();
+  firestore.settings(initializationOptions);
 
   const clientPool = new ClientPool(/* concurrentRequestLimit= */ 1, () => {
     const gapicClient: GapicClient = v1beta1(initializationOptions);

types/firestore.d.ts

@@ -85,10 +85,24 @@ declare namespace FirebaseFirestore {
    */
   export class Firestore {
     /**
-     * @param options - Configuration object. See [Firestore Documentation]
+     * @param settings - Configuration object. See [Firestore Documentation]
      * {@link https://firebase.google.com/docs/firestore/}
      */
-    public constructor(options?: Settings);
+    public constructor(settings?: Settings);
+
+    /**
+     * Specifies custom settings to be used to configure the `Firestore`
+     * instance. Can only be invoked once and before any other Firestore
+     * method.
+     *
+     * If settings are provided via both `settings()` and the `Firestore`
+     * constructor, both settings objects are merged and any settings provided
+     * via `settings()` take precedence.
+     *
+     * @param {object} settings The settings to use for all Firestore
+     * operations.
+     */
+    settings(settings: Settings);
 
     /**
      * Gets a `CollectionReference` instance that refers to the collection at