Merge branch 'master' into mrschmidt/pendingwrites

Author: schmidt-sebastian

packages-exp/app-types-exp/package.json

@@ -7,6 +7,7 @@
   "license": "Apache-2.0",
   "scripts": {
     "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js",
     "api-report": "api-extractor run --local --verbose",
     "predoc": "node ../../scripts/exp/remove-exp.js temp",
     "doc": "api-documenter markdown --input temp --output docs",

packages/analytics-interop-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts"

packages/analytics-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts"

packages/app-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts",

packages/auth-interop-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts"

packages/auth-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts"

packages/database-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts"

packages/database/CHANGELOG.md

@@ -1,6 +1,7 @@
 # Unreleased
 - [feature] Added ServerValue.increment() to support atomic field value increments
   without transactions.
+- [fixed] Fixed Realtime Database URL parsing bug to support domains with more than 3 components.
 
 # Released
 - [fixed] Fixed an issue that caused large numeric values with leading zeros to

packages/database/src/core/util/libs/parser.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -72,7 +72,7 @@ export const parseRepoInfo = function(
   const parsedUrl = parseDatabaseURL(dataURL),
     namespace = parsedUrl.namespace;
 
-  if (parsedUrl.domain === 'firebase') {
+  if (parsedUrl.domain === 'firebase.com') {
     fatal(
       parsedUrl.host +
         ' is no longer supported. ' +
@@ -171,20 +171,21 @@ export const parseDatabaseURL = function(
       secure = scheme === 'https' || scheme === 'wss';
       port = parseInt(host.substring(colonInd + 1), 10);
     } else {
-      colonInd = dataURL.length;
+      colonInd = host.length;
     }
 
-    const parts = host.split('.');
-    if (parts.length === 3) {
+    const hostWithoutPort = host.slice(0, colonInd);
+    if (hostWithoutPort.toLowerCase() === 'localhost') {
+      domain = 'localhost';
+    } else if (hostWithoutPort.split('.').length <= 2) {
+      domain = hostWithoutPort;
+    } else {
+      // Interpret the subdomain of a 3 or more component URL as the namespace name.
+      const dotInd = host.indexOf('.');
+      subdomain = host.substring(0, dotInd).toLowerCase();
+      domain = host.substring(dotInd + 1);
       // Normalize namespaces to lowercase to share storage / connection.
-      domain = parts[1];
-      subdomain = parts[0].toLowerCase();
-      // We interpret the subdomain of a 3 component URL as the namespace name.
       namespace = subdomain;
-    } else if (parts.length === 2) {
-      domain = parts[0];
-    } else if (parts[0].slice(0, colonInd).toLowerCase() === 'localhost') {
-      domain = 'localhost';
     }
     // Always treat the value of the `ns` as the namespace name if it is present.
     if ('ns' in queryParams) {

packages/database/test/database.test.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -62,18 +62,49 @@ describe('Database Tests', () => {
     expect(db.ref().toString()).to.equal('https://foo.bar.com/');
   });
 
+  it('Can get database with multi-region URL', () => {
+    const db = defaultApp.database('http://foo.euw1.firebasedatabase.app');
+    expect(db).to.be.ok;
+    expect(db.repo_.repoInfo_.namespace).to.equal('foo');
+    expect(db.ref().toString()).to.equal(
+      'https://foo.euw1.firebasedatabase.app/'
+    );
+  });
+
+  it('Can get database with upper case URL', () => {
+    const db = defaultApp.database('http://fOO.EUW1.firebaseDATABASE.app');
+    expect(db).to.be.ok;
+    expect(db.repo_.repoInfo_.namespace).to.equal('foo');
+    expect(db.ref().toString()).to.equal(
+      'https://foo.euw1.firebasedatabase.app/'
+    );
+  });
+
+  it('Can get database with localhost URL', () => {
+    const db = defaultApp.database('http://localhost');
+    expect(db).to.be.ok;
+    expect(db.ref().toString()).to.equal('https://localhost/');
+  });
+
   it('Can get database with localhost URL and port', () => {
     const db = defaultApp.database('http://localhost:80');
     expect(db).to.be.ok;
     expect(db.ref().toString()).to.equal('http://localhost:80/');
   });
 
-  it('Can get database with localhost URL', () => {
-    const db = defaultApp.database('http://localhost');
+  it('Can get database with a upper case localhost URL', () => {
+    const db = defaultApp.database('http://LOCALHOST');
     expect(db).to.be.ok;
     expect(db.ref().toString()).to.equal('https://localhost/');
   });
 
+  it('Can get database with a upper case localhost URL and ns', () => {
+    const db = defaultApp.database('http://LOCALHOST?ns=foo');
+    expect(db).to.be.ok;
+    expect(db.repo_.repoInfo_.namespace).to.equal('foo');
+    expect(db.ref().toString()).to.equal('https://localhost/');
+  });
+
   it('Can read ns query param', () => {
     const db = defaultApp.database('http://localhost:80/?ns=foo&unused=true');
     expect(db).to.be.ok;
@@ -111,10 +142,19 @@ describe('Database Tests', () => {
     }).to.throw(/Database initialized multiple times/i);
   });
 
+  it('Databases with legacy domain', () => {
+    expect(() => {
+      defaultApp.database('http://foo.firebase.com/');
+    }).to.throw(/is no longer supported/i);
+  });
+
   it('Databases with invalid custom URLs', () => {
     expect(() => {
       defaultApp.database('not-a-url');
     }).to.throw(/Cannot parse Firebase url/i);
+    expect(() => {
+      defaultApp.database('http://foo.com');
+    }).to.throw(/Cannot parse Firebase url/i);
     expect(() => {
       defaultApp.database('http://fblocal.com');
     }).to.throw(/Cannot parse Firebase url/i);

packages/firestore-types/package.json

@@ -5,7 +5,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "license": "Apache-2.0",
   "scripts": {
-    "test": "tsc"
+    "test": "tsc",
+    "test:ci": "node ../../scripts/run_tests_in_ci.js"
   },
   "files": [
     "index.d.ts"

packages/firestore/CHANGELOG.md

@@ -1,4 +1,6 @@
 # Unreleased
+- [fixed] Fixed a regression introduced in v7.14.2 that incorrectly applied
+  a `FieldValue.increment` in combination with `set({...}, {merge: true})`.
 - [fixed] Firestore now rejects `onSnapshot()` listeners if they cannot be
   registered in IndexedDB. Previously, these errors crashed the client.
 - [fixed] Firestore now rejects write operations if they cannot be persisted

packages/firestore/src/api/field_value.ts

@@ -102,12 +102,17 @@ export class ArrayUnionFieldValueImpl extends FieldValueImpl {
     // Although array transforms are used with writes, the actual elements
     // being uniomed or removed are not considered writes since they cannot
     // contain any FieldValue sentinels, etc.
-    const parseContext = context.contextWith({
-      dataSource: UserDataSource.Argument,
-      methodName: this._methodName
-    });
+    const parseContext = new ParseContext(
+      {
+        dataSource: UserDataSource.Argument,
+        methodName: this._methodName,
+        arrayElement: true
+      },
+      context.databaseId,
+      context.serializer
+    );
     const parsedElements = this._elements.map(
-      (element, i) => parseData(element, parseContext.childContextForArray(i))!
+      element => parseData(element, parseContext)!
     );
     const arrayUnion = new ArrayUnionTransformOperation(parsedElements);
     return new FieldTransform(context.path!, arrayUnion);
@@ -128,12 +133,17 @@ export class ArrayRemoveFieldValueImpl extends FieldValueImpl {
     // Although array transforms are used with writes, the actual elements
     // being unioned or removed are not considered writes since they cannot
     // contain any FieldValue sentinels, etc.
-    const parseContext = context.contextWith({
-      dataSource: UserDataSource.Argument,
-      methodName: this._methodName
-    });
+    const parseContext = new ParseContext(
+      {
+        dataSource: UserDataSource.Argument,
+        methodName: this._methodName,
+        arrayElement: true
+      },
+      context.databaseId,
+      context.serializer
+    );
     const parsedElements = this._elements.map(
-      (element, i) => parseData(element, parseContext.childContextForArray(i))!
+      element => parseData(element, parseContext)!
     );
     const arrayUnion = new ArrayRemoveTransformOperation(parsedElements);
     return new FieldTransform(context.path!, arrayUnion);
@@ -151,8 +161,15 @@ export class NumericIncrementFieldValueImpl extends FieldValueImpl {
   }
 
   toFieldTransform(context: ParseContext): FieldTransform {
-    context.contextWith({ methodName: this._methodName });
-    const operand = parseData(this._operand, context)!;
+    const parseContext = new ParseContext(
+      {
+        dataSource: UserDataSource.Argument,
+        methodName: this._methodName
+      },
+      context.databaseId,
+      context.serializer
+    );
+    const operand = parseData(this._operand, parseContext)!;
     const numericIncrement = new NumericIncrementTransformOperation(
       context.serializer,
       operand

packages/firestore/src/api/user_data_reader.ts

@@ -140,9 +140,12 @@ interface ContextSettings {
    * nonempty path (indicating the context represents a nested location within
    * the data).
    */
-  readonly path: FieldPath | null;
-  /** Whether or not this context corresponds to an element of an array. */
-  readonly arrayElement: boolean;
+  readonly path?: FieldPath;
+  /**
+   * Whether or not this context corresponds to an element of an array.
+   * If not set, elements are treated as if they were outside of arrays.
+   */
+  readonly arrayElement?: boolean;
 }
 
 /** A "context" object passed around while parsing user data. */
@@ -181,7 +184,7 @@ export class ParseContext {
     this.fieldMask = fieldMask || [];
   }
 
-  get path(): FieldPath | null {
+  get path(): FieldPath | undefined {
     return this.settings.path;
   }
 
@@ -201,28 +204,28 @@ export class ParseContext {
   }
 
   childContextForField(field: string): ParseContext {
-    const childPath = this.path == null ? null : this.path.child(field);
+    const childPath = this.path?.child(field);
     const context = this.contextWith({ path: childPath, arrayElement: false });
     context.validatePathSegment(field);
     return context;
   }
 
   childContextForFieldPath(field: FieldPath): ParseContext {
-    const childPath = this.path == null ? null : this.path.child(field);
+    const childPath = this.path?.child(field);
     const context = this.contextWith({ path: childPath, arrayElement: false });
     context.validatePath();
     return context;
   }
 
   childContextForArray(index: number): ParseContext {
     // TODO(b/34871131): We don't support array paths right now; so make path
-    // null.
-    return this.contextWith({ path: null, arrayElement: true });
+    // undefined.
+    return this.contextWith({ path: undefined, arrayElement: true });
   }
 
   createError(reason: string): Error {
     const fieldDescription =
-      this.path === null || this.path.isEmpty()
+      !this.path || this.path.isEmpty()
         ? ''
         : ` (found in field ${this.path.toString()})`;
     return new FirestoreError(
@@ -246,7 +249,7 @@ export class ParseContext {
   private validatePath(): void {
     // TODO(b/34871131): Remove null check once we have proper paths for fields
     // within arrays.
-    if (this.path === null) {
+    if (!this.path) {
       return;
     }
     for (let i = 0; i < this.path.length; i++) {