FDC Parsing Improvements

common/api-review/data-connect.api.md

@@ -259,8 +259,6 @@ export interface TransportOptions {
     // (undocumented)
     host: string;
     // (undocumented)
-    port?: number;
-    // (undocumented)
     sslEnabled?: boolean;
 }
 

packages/data-connect/src/api/DataConnect.ts

@@ -58,7 +58,6 @@ export interface ConnectorConfig {
 export interface TransportOptions {
   host: string;
   sslEnabled?: boolean;
-  port?: number;
 }
 
 const FIREBASE_DATA_CONNECT_EMULATOR_HOST_VAR =
@@ -71,11 +70,15 @@ const FIREBASE_DATA_CONNECT_EMULATOR_HOST_VAR =
  * @internal
  */
 export function parseOptions(fullHost: string): TransportOptions {
-  const [protocol, hostName] = fullHost.split('://');
-  const isSecure = protocol === 'https';
-  const [host, portAsString] = hostName.split(':');
-  const port = Number(portAsString);
-  return { host, port, sslEnabled: isSecure };
+  if (fullHost.startsWith('http://') || fullHost.startsWith('https://')) {
+    const [protocol, host] = fullHost.split('://');
+    const isSecure = protocol === 'https';
+    return { host, sslEnabled: isSecure };
+  }
+  return {
+    host: fullHost,
+    sslEnabled: false
+  };
 }
 /**
  * DataConnectOptions including project id
@@ -182,7 +185,6 @@ export class DataConnect {
     if (this._transportOptions) {
       this._transport.useEmulator(
         this._transportOptions.host,
-        this._transportOptions.port,
         this._transportOptions.sslEnabled
       );
     }
@@ -219,7 +221,6 @@ export function areTransportOptionsEqual(
 ): boolean {
   return (
     transportOptions1.host === transportOptions2.host &&
-    transportOptions1.port === transportOptions2.port &&
     transportOptions1.sslEnabled === transportOptions2.sslEnabled
   );
 }
@@ -237,7 +238,8 @@ export function connectDataConnectEmulator(
   port?: number,
   sslEnabled = false
 ): void {
-  dc.enableEmulator({ host, port, sslEnabled });
+  const hostWithPort = port ? `${host}:${port}` : host;
+  dc.enableEmulator({ host: hostWithPort, sslEnabled });
 }
 
 /**

packages/data-connect/src/core/QueryManager.ts

@@ -221,9 +221,6 @@ export class QueryManager {
 
     return newR;
   }
-  enableEmulator(host: string, port: number): void {
-    this.transport.useEmulator(host, port);
-  }
 }
 function compareDates(str1: string, str2: string): boolean {
   const date1 = new Date(str1);

packages/data-connect/src/network/transport/index.ts

@@ -51,7 +51,7 @@ export interface DataConnectTransport {
     queryName: string,
     body?: U
   ): Promise<{ data: T; errors: Error[] }>;
-  useEmulator(host: string, port?: number, sslEnabled?: boolean): void;
+  useEmulator(host: string, sslEnabled?: boolean): void;
   onTokenChanged: (token: string | null) => void;
   _setCallerSdkType(callerSdkType: CallerSdkType): void;
 }

packages/data-connect/src/network/transport/rest.ts

@@ -27,7 +27,6 @@ import { CallerSdkType, CallerSdkTypeEnum, DataConnectTransport } from '.';
 
 export class RESTTransport implements DataConnectTransport {
   private _host = '';
-  private _port: number | undefined;
   private _location = 'l';
   private _connectorName = '';
   private _secure = true;
@@ -47,9 +46,6 @@ export class RESTTransport implements DataConnectTransport {
     private _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base
   ) {
     if (transportOptions) {
-      if (typeof transportOptions.port === 'number') {
-        this._port = transportOptions.port;
-      }
       if (typeof transportOptions.sslEnabled !== 'undefined') {
         this._secure = transportOptions.sslEnabled;
       }
@@ -88,14 +84,11 @@ export class RESTTransport implements DataConnectTransport {
         projectId: this._project,
         service: this._serviceName
       },
-      { host: this._host, sslEnabled: this._secure, port: this._port }
+      { host: this._host, sslEnabled: this._secure }
     );
   }
-  useEmulator(host: string, port?: number, isSecure?: boolean): void {
+  useEmulator(host: string, isSecure?: boolean): void {
     this._host = host;
-    if (typeof port === 'number') {
-      this._port = port;
-    }
     if (typeof isSecure !== 'undefined') {
       this._secure = isSecure;
     }

packages/data-connect/src/util/url.ts

@@ -16,27 +16,16 @@
  */
 
 import { DataConnectOptions, TransportOptions } from '../api/DataConnect';
-import { Code, DataConnectError } from '../core/error';
-import { logError } from '../logger';
 
 export function urlBuilder(
   projectConfig: DataConnectOptions,
   transportOptions: TransportOptions
 ): string {
   const { connector, location, projectId: project, service } = projectConfig;
-  const { host, sslEnabled, port } = transportOptions;
+  const { host, sslEnabled } = transportOptions;
   const protocol = sslEnabled ? 'https' : 'http';
   const realHost = host || `firebasedataconnect.googleapis.com`;
-  let baseUrl = `${protocol}://${realHost}`;
-  if (typeof port === 'number') {
-    baseUrl += `:${port}`;
-  } else if (typeof port !== 'undefined') {
-    logError('Port type is of an invalid type');
-    throw new DataConnectError(
-      Code.INVALID_ARGUMENT,
-      'Incorrect type for port passed in!'
-    );
-  }
+  const baseUrl = `${protocol}://${realHost}`;
   return `${baseUrl}/v1/projects/${project}/locations/${location}/services/${service}/connectors/${connector}`;
 }
 export function addToken(url: string, apiKey?: string): string {

packages/data-connect/test/unit/dataconnect.test.ts

@@ -18,7 +18,7 @@
 import { deleteApp, initializeApp } from '@firebase/app';
 import { expect } from 'chai';
 
-import { getDataConnect } from '../../src';
+import { getDataConnect, parseOptions } from '../../src';
 
 describe('Data Connect Test', () => {
   beforeEach(() => {});
@@ -60,4 +60,26 @@ describe('Data Connect Test', () => {
     expect(dc.app.options.projectId).to.eq(projectId);
     await deleteApp(customApp);
   });
+  it('should parse env var correctly with http://', async () => {
+    const parsedHost = parseOptions('http://localhost');
+    expect(parsedHost.host).to.eq('localhost');
+    expect(parsedHost.sslEnabled).to.be.false;
+  });
+  it('should parse env var correctly with port', async () => {
+    const parsedHost = parseOptions('localhost:8080');
+    expect(parsedHost.host).to.eq('localhost:8080');
+    expect(parsedHost.sslEnabled).to.be.false;
+  });
+  it('should parse env var correctly with https://', async () => {
+    const parsedHost = parseOptions('https://localhost');
+    expect(parsedHost.host).to.eq('localhost');
+    expect(parsedHost.sslEnabled).to.be.true;
+  });
+  it('should parse ipv6 addresses correctly', async () => {
+    const host = '2001:0db8:85a3:0000:0000:8a2e:0370:7334';
+    const parsedHost = parseOptions(host);
+    expect(parsedHost.host).to.eq(host);
+    expect(parsedHost.sslEnabled).to.be.false;
+  });
 });
+

packages/data-connect/test/unit/transportoptions.test.ts

@@ -29,12 +29,10 @@ describe('Transport Options', () => {
   it('should return false if transport options are not equal', () => {
     const transportOptions1: TransportOptions = {
       host: 'h',
-      port: 1,
       sslEnabled: false
     };
     const transportOptions2: TransportOptions = {
       host: 'h2',
-      port: 2,
       sslEnabled: false
     };
     expect(
@@ -44,11 +42,9 @@ describe('Transport Options', () => {
   it('should return true if transport options are equal', () => {
     const transportOptions1: TransportOptions = {
       host: 'h',
-      port: 1,
       sslEnabled: false
     };
     const transportOptions2: TransportOptions = {
-      port: 1,
       host: 'h',
       sslEnabled: false
     };