time is now a number. Cleaned up code (#4381)

* time is now a number. Cleaned up code

* Add more type support to cloneDeep

* Changelog

Co-authored-by: Daniel Lee <danielylee@google.com>

CHANGELOG.md

@@ -4,3 +4,4 @@
 - Fixes Storage Emulator ruleset file watcher (#4337).
 - Fixes issue with importing Storage Emulator data exported prior to v10.3.0 (#4358).
 - Adds ergonomic improvements to CF3 secret commands to automatically redeploy functions and delete unused secrets (#4130).
+- Fixes issue with alpha users setting timeouts (#4381)

scripts/emulator-tests/functionsEmulatorRuntime.spec.ts

@@ -2,7 +2,6 @@ import { Change } from "firebase-functions";
 import { DocumentSnapshot } from "firebase-functions/lib/providers/firestore";
 import { expect } from "chai";
 import { IncomingMessage, request } from "http";
-import * as _ from "lodash";
 import * as express from "express";
 import * as fs from "fs";
 import * as sinon from "sinon";
@@ -12,7 +11,7 @@ import { FunctionRuntimeBundles, TIMEOUT_LONG, TIMEOUT_MED, MODULE_ROOT } from "
 import { FunctionsRuntimeBundle, SignatureType } from "../../src/emulator/functionsEmulatorShared";
 import { InvokeRuntimeOpts, FunctionsEmulator } from "../../src/emulator/functionsEmulator";
 import { RuntimeWorker } from "../../src/emulator/functionsRuntimeWorker";
-import { streamToString } from "../../src/utils";
+import { streamToString, cloneDeep } from "../../src/utils";
 import * as registry from "../../src/emulator/registry";
 
 const DO_NOTHING = () => {
@@ -449,7 +448,7 @@ describe("FunctionsEmulator-Runtime", () => {
       }).timeout(TIMEOUT_MED);
 
       it("should return a real databaseURL when RTDB emulator is not running", async () => {
-        const frb = _.cloneDeep(FunctionRuntimeBundles.onRequest);
+        const frb = cloneDeep(FunctionRuntimeBundles.onRequest);
         const worker = await invokeFunction(
           frb,
           () => {

src/deploy/functions/backend.ts

@@ -209,7 +209,7 @@ export interface ServiceConfiguration {
   environmentVariables?: Record<string, string>;
   secretEnvironmentVariables?: SecretEnvVar[];
   availableMemoryMb?: MemoryOptions;
-  timeout?: proto.Duration;
+  timeoutSeconds?: number;
   maxInstances?: number;
   minInstances?: number;
   vpc?: {

src/deploy/functions/runtimes/discovery/v1alpha1.ts

@@ -82,7 +82,7 @@ function parseEndpoints(
     minInstances: "number",
     concurrency: "number",
     serviceAccountEmail: "string",
-    timeout: "string",
+    timeoutSeconds: "number",
     vpc: "object",
     labels: "object",
     ingressSettings: "string",
@@ -205,7 +205,7 @@ function parseEndpoints(
       "minInstances",
       "concurrency",
       "serviceAccountEmail",
-      "timeout",
+      "timeoutSeconds",
       "vpc",
       "labels",
       "ingressSettings",

src/deploy/functions/runtimes/node/parseTriggers.ts

@@ -280,11 +280,17 @@ export function addResourcesToBackend(
       "serviceAccountEmail",
       "labels",
       "ingressSettings",
-      "timeout",
       "maxInstances",
       "minInstances",
       "availableMemoryMb"
     );
+    proto.renameIfPresent(
+      endpoint,
+      annotation,
+      "timeoutSeconds",
+      "timeout",
+      proto.secondsFromDuration
+    );
     want.endpoints[region] = want.endpoints[region] || {};
     want.endpoints[region][endpoint.id] = endpoint;
 

src/emulator/functionsEmulatorShared.ts

@@ -21,7 +21,7 @@ export interface ParsedTriggerDefinition {
   entryPoint: string;
   platform: backend.FunctionsPlatform;
   name: string;
-  timeout?: string | number; // Can be "3s" for some reason lol
+  timeoutSeconds?: number;
   regions?: string[];
   availableMemoryMb?: "128MB" | "256MB" | "512MB" | "1GB" | "2GB" | "4GB";
   httpsTrigger?: any;
@@ -109,11 +109,7 @@ export class EmulatedTrigger {
   }
 
   get timeoutMs(): number {
-    if (typeof this.definition.timeout === "number") {
-      return this.definition.timeout * 1000;
-    } else {
-      return parseInt((this.definition.timeout || "60s").split("s")[0], 10) * 1000;
-    }
+    return (this.definition.timeoutSeconds || 60) * 1000;
   }
 
   getRawFunction(): CloudFunction<any> {
@@ -152,9 +148,9 @@ export function emulatedFunctionsFromEndpoints(
     copyIfPresent(
       def,
       endpoint,
-      "timeout",
       "availableMemoryMb",
       "labels",
+      "timeoutSeconds",
       "platform",
       "secretEnvironmentVariables"
     );

src/extensions/emulator/optionsHelper.ts

@@ -1,5 +1,4 @@
 import * as fs from "fs-extra";
-import * as _ from "lodash";
 import { ParsedTriggerDefinition } from "../../emulator/functionsEmulatorShared";
 import * as path from "path";
 import * as paramHelper from "../paramHelper";
@@ -25,10 +24,7 @@ export async function buildOptions(options: any): Promise<any> {
 
   extensionsHelper.validateCommandLineParams(params, spec.params);
 
-  const functionResources = specHelper.getFunctionResourcesWithParamSubstitution(
-    spec,
-    params
-  ) as Resource[];
+  const functionResources = specHelper.getFunctionResourcesWithParamSubstitution(spec, params);
   let testConfig;
   if (options.testConfig) {
     testConfig = readTestConfigFile(options.testConfig);
@@ -241,11 +237,8 @@ function buildConfig(
 function getFunctionSourceDirectory(functionResources: Resource[]): string {
   let sourceDirectory;
   for (const r of functionResources) {
-    let dir = _.get(r, "properties.sourceDirectory");
     // If not specified, default sourceDirectory to "functions"
-    if (!dir) {
-      dir = "functions";
-    }
+    const dir = r.properties?.sourceDirectory || "functions";
     if (!sourceDirectory) {
       sourceDirectory = dir;
     } else if (sourceDirectory !== dir) {
@@ -254,7 +247,7 @@ function getFunctionSourceDirectory(functionResources: Resource[]): string {
       );
     }
   }
-  return sourceDirectory;
+  return sourceDirectory || "functions";
 }
 
 function shouldEmulateFunctions(resources: Resource[]): boolean {
@@ -263,7 +256,7 @@ function shouldEmulateFunctions(resources: Resource[]): boolean {
 
 function shouldEmulate(emulatorName: string, resources: Resource[]): boolean {
   for (const r of resources) {
-    const eventType: string = _.get(r, "properties.eventTrigger.eventType", "");
+    const eventType: string = r.properties?.eventTrigger?.eventType || "";
     if (eventType.includes(emulatorName)) {
       return true;
     }

src/extensions/emulator/triggerHelper.ts

@@ -1,32 +1,33 @@
-import * as _ from "lodash";
 import {
   ParsedTriggerDefinition,
   getServiceFromEventType,
 } from "../../emulator/functionsEmulatorShared";
 import { EmulatorLogger } from "../../emulator/emulatorLogger";
 import { Emulators } from "../../emulator/types";
+import * as extensionsApi from "../../extensions/extensionsApi";
+import * as proto from "../../gcp/proto";
 
-export function functionResourceToEmulatedTriggerDefintion(resource: any): ParsedTriggerDefinition {
+export function functionResourceToEmulatedTriggerDefintion(
+  resource: extensionsApi.Resource
+): ParsedTriggerDefinition {
   const etd: ParsedTriggerDefinition = {
     name: resource.name,
     entryPoint: resource.name,
     platform: "gcfv1",
   };
-  const properties = _.get(resource, "properties", {});
-  if (properties.timeout) {
-    etd.timeout = properties.timeout;
-  }
-  if (properties.location) {
-    etd.regions = [properties.location];
-  }
-  if (properties.availableMemoryMb) {
-    etd.availableMemoryMb = properties.availableMemoryMb;
-  }
+  const properties = resource.properties || {};
+  proto.renameIfPresent(etd, properties, "timeoutSeconds", "timeout", proto.secondsFromDuration);
+  proto.renameIfPresent(etd, properties, "regions", "location", (str: string) => [str]);
+  proto.copyIfPresent(etd, properties, "availableMemoryMb");
   if (properties.httpsTrigger) {
     etd.httpsTrigger = properties.httpsTrigger;
-  } else if (properties.eventTrigger) {
-    properties.eventTrigger.service = getServiceFromEventType(properties.eventTrigger.eventType);
-    etd.eventTrigger = properties.eventTrigger;
+  }
+  if (properties.eventTrigger) {
+    etd.eventTrigger = {
+      eventType: properties.eventTrigger.eventType,
+      resource: properties.eventTrigger.resource,
+      service: getServiceFromEventType(properties.eventTrigger.eventType),
+    };
   } else {
     EmulatorLogger.forEmulator(Emulators.FUNCTIONS).log(
       "WARN",

src/extensions/extensionsApi.ts

@@ -10,7 +10,11 @@ import { FirebaseError } from "../error";
 import { logger } from "../logger";
 import * as operationPoller from "../operation-poller";
 import * as refs from "./refs";
+import * as proto from "../gcp/proto";
 import { SpecParamType } from "./extensionsHelper";
+import { Runtime } from "../deploy/functions/runtimes";
+import { HttpsTriggered, EventTriggered } from "../deploy/functions/backend";
+import { StringifyOptions } from "querystring";
 
 const VERSION = "v1beta";
 const PAGE_SIZE_MAX = 100;
@@ -132,13 +136,36 @@ export interface Role {
   reason: string;
 }
 
-export interface Resource {
+// Docs at https://firebase.google.com/docs/extensions/alpha/ref-extension-yaml
+export const FUNCTIONS_RESOURCE_TYPE = "firebaseextensions.v1beta.function";
+export interface FunctionResourceProperties {
+  type: typeof FUNCTIONS_RESOURCE_TYPE;
+  properties?: {
+    location?: string;
+    entryPoint?: string;
+    sourceDirectory?: string;
+    timeout?: proto.Duration;
+    availableMemoryMb?: number;
+    runtime?: Runtime;
+    httpsTrigger?: Record<string, never>;
+    eventTrigger?: {
+      eventType: string;
+      resource: string;
+      service?: string;
+    };
+  };
+}
+
+// Union of all valid property types so we can have a strongly typed "property"
+// field depending on the actual value of "type"
+type ResourceProperties = FunctionResourceProperties;
+
+export type Resource = ResourceProperties & {
   name: string;
-  type: string;
   description?: string;
-  properties?: { [key: string]: any };
   propertiesYaml?: string;
-}
+  entryPoint?: string;
+};
 
 export interface Author {
   authorName: string;

src/extensions/paramHelper.ts

@@ -1,4 +1,3 @@
-import * as _ from "lodash";
 import * as path from "path";
 import * as clc from "cli-color";
 import * as fs from "fs-extra";
@@ -15,6 +14,7 @@ import {
 import * as askUserForParam from "./askUserForParam";
 import * as track from "../track";
 import * as env from "../functions/env";
+import { cloneDeep } from "../utils";
 
 /**
  * Interface for holding different param values for different environments/configs.
@@ -80,8 +80,8 @@ export function setNewDefaults(
 export function getParamsWithCurrentValuesAsDefaults(
   extensionInstance: extensionsApi.ExtensionInstance
 ): extensionsApi.Param[] {
-  const specParams = _.cloneDeep(_.get(extensionInstance, "config.source.spec.params", []));
-  const currentParams = _.cloneDeep(_.get(extensionInstance, "config.params", {}));
+  const specParams = cloneDeep(extensionInstance?.config?.source?.spec?.params || []);
+  const currentParams = cloneDeep(extensionInstance?.config?.params || {});
   return setNewDefaults(specParams, currentParams);
 }
 
@@ -101,8 +101,8 @@ export async function getParams(args: {
   nonInteractive?: boolean;
   paramsEnvPath?: string;
   reconfiguring?: boolean;
-}): Promise<{ [key: string]: ParamBindingOptions }> {
-  let params: any;
+}): Promise<Record<string, ParamBindingOptions>> {
+  let params: Record<string, ParamBindingOptions>;
   if (args.nonInteractive && !args.paramsEnvPath) {
     const paramsMessage = args.paramSpecs
       .map((p) => {
@@ -130,7 +130,8 @@ export async function getParams(args: {
       reconfiguring: !!args.reconfiguring,
     });
   }
-  void track("Extension Params", _.isEmpty(params) ? "Not Present" : "Present", _.size(params));
+  const paramNames = Object.keys(params);
+  void track("Extension Params", paramNames.length ? "Not Present" : "Present", paramNames.length);
   return params;
 }
 
@@ -142,8 +143,8 @@ export async function getParamsForUpdate(args: {
   paramsEnvPath?: string;
   nonInteractive?: boolean;
   instanceId: string;
-}): Promise<{ [key: string]: ParamBindingOptions }> {
-  let params: { [key: string]: ParamBindingOptions };
+}): Promise<Record<string, ParamBindingOptions>> {
+  let params: Record<string, ParamBindingOptions>;
   if (args.nonInteractive && !args.paramsEnvPath) {
     const paramsMessage = args.newSpec.params
       .map((p) => {
@@ -170,7 +171,8 @@ export async function getParamsForUpdate(args: {
       instanceId: args.instanceId,
     });
   }
-  void track("Extension Params", _.isEmpty(params) ? "Not Present" : "Present", _.size(params));
+  const paramNames = Object.keys(params);
+  void track("Extension Params", paramNames.length ? "Not Present" : "Present", paramNames.length);
   return params;
 }
 
@@ -192,33 +194,39 @@ export async function promptForNewParams(args: {
   const newParamBindingOptions = buildBindingOptionsWithBaseValue(args.currentParams);
 
   const firebaseProjectParams = await getFirebaseProjectParams(args.projectId);
-  const comparer = (param1: extensionsApi.Param, param2: extensionsApi.Param) => {
+  const sameParam = (param1: extensionsApi.Param) => (param2: extensionsApi.Param) => {
     return param1.type === param2.type && param1.param === param2.param;
   };
+  const paramDiff = (
+    left: extensionsApi.Param[],
+    right: extensionsApi.Param[]
+  ): extensionsApi.Param[] => {
+    return left.filter((aLeft) => !right.find(sameParam(aLeft)));
+  };
 
   // Some params are in the spec but not in currentParams, remove so we can prompt for them.
   const oldParams = args.spec.params.filter((p) =>
     Object.keys(args.currentParams).includes(p.param)
   );
 
-  let paramsDiffDeletions = _.differenceWith(oldParams, args.newSpec.params, comparer);
+  let paramsDiffDeletions = paramDiff(oldParams, args.newSpec.params);
   paramsDiffDeletions = substituteParams<extensionsApi.Param[]>(
     paramsDiffDeletions,
     firebaseProjectParams
   );
 
-  let paramsDiffAdditions = _.differenceWith(args.newSpec.params, oldParams, comparer);
+  let paramsDiffAdditions = paramDiff(args.newSpec.params, oldParams);
   paramsDiffAdditions = substituteParams<extensionsApi.Param[]>(
     paramsDiffAdditions,
     firebaseProjectParams
   );
 
   if (paramsDiffDeletions.length) {
     logger.info("The following params will no longer be used:");
-    paramsDiffDeletions.forEach((param) => {
+    for (const param of paramsDiffDeletions) {
       logger.info(clc.red(`- ${param.param}: ${args.currentParams[param.param.toUpperCase()]}`));
       delete newParamBindingOptions[param.param.toUpperCase()];
-    });
+    }
   }
   if (paramsDiffAdditions.length) {
     logger.info("To update this instance, configure the following new parameters:");