From 7209c4b2581f5db96ab494fc23e3ca6736838aab Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 1 Jun 2021 17:46:02 +0100 Subject: [PATCH 1/4] Follow up to #3420 --- src/checkFirebaseSDKVersion.ts | 5 +- src/commands/remoteconfig-get.ts | 9 +-- src/config.ts | 63 ++++++++----------- src/database/rulesConfig.ts | 17 +++-- src/deploy/firestore/prepare.ts | 11 ++-- src/deploy/functions/deploy.ts | 9 ++- .../discovery/jsexports/parseTriggers.ts | 4 +- src/deploy/functions/prepare.ts | 11 ++-- .../functions/prepareFunctionsUpload.ts | 10 ++- src/deploy/remoteconfig/prepare.ts | 5 +- src/deploy/storage/prepare.ts | 5 +- src/emulator/constants.ts | 8 --- src/emulator/controller.ts | 25 +++----- src/extensions/emulator/optionsHelper.ts | 2 +- src/firebaseConfig.ts | 18 +++++- src/functionsShellCommandAction.ts | 9 +-- src/serve/functions.ts | 20 +++--- 17 files changed, 115 insertions(+), 116 deletions(-) diff --git a/src/checkFirebaseSDKVersion.ts b/src/checkFirebaseSDKVersion.ts index f0c0eca428a..047e6912a2d 100644 --- a/src/checkFirebaseSDKVersion.ts +++ b/src/checkFirebaseSDKVersion.ts @@ -7,6 +7,7 @@ import * as spawn from "cross-spawn"; import * as utils from "./utils"; import { FirebaseError } from "./error"; import { logger } from "./logger"; +import { Options } from "./options"; interface NpmListResult { name: string; @@ -54,12 +55,12 @@ export function getFunctionsSDKVersion(sourceDir: string): string | void { * If it is unable to do the check, it does nothing. * @param options Options object from "firebase deploy" command. */ -export function checkFunctionsSDKVersion(options: any): void { +export function checkFunctionsSDKVersion(options: Options): void { if (!options.config.has("functions")) { return; } - const sourceDirName = options.config.get("functions.source"); + const sourceDirName = options.config.src.functions?.source; if (!sourceDirName) { throw new FirebaseError( `No functions code detected at default location ("./functions"), and no "functions.source" defined in "firebase.json"` diff --git a/src/commands/remoteconfig-get.ts b/src/commands/remoteconfig-get.ts index baff2614054..2a2c67c5251 100644 --- a/src/commands/remoteconfig-get.ts +++ b/src/commands/remoteconfig-get.ts @@ -10,6 +10,7 @@ import { parseTemplateForTable } from "../remoteconfig/get"; import Table = require("cli-table"); import * as fs from "fs"; import util = require("util"); +import { Options } from "../options"; const tableHead = ["Entry Name", "Value"]; @@ -32,10 +33,10 @@ module.exports = new Command("remoteconfig:get") ) .before(requireAuth) .before(requirePermissions, ["cloudconfig.configs.get"]) - .action(async (options) => { + .action(async (options: Options) => { const template: RemoteConfigTemplate = await rcGet.getTemplate( getProjectId(options), - checkValidNumber(options.versionNumber) + checkValidNumber(options.versionNumber as string) ); const table = new Table({ head: tableHead, style: { head: ["green"] } }); if (template.conditions) { @@ -60,8 +61,8 @@ module.exports = new Command("remoteconfig:get") if (fileOut) { const shouldUseDefaultFilename = options.output === true || options.output === ""; const filename = shouldUseDefaultFilename - ? options.config.get("remoteconfig.template") - : options.output; + ? options.config.src.remoteconfig!.template + : (options.output as string); const outTemplate = { ...template }; delete outTemplate.version; fs.writeFileSync(filename, JSON.stringify(outTemplate, null, 2)); diff --git a/src/config.ts b/src/config.ts index f9d873d0cf2..352692c6ebf 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,24 +2,25 @@ import { FirebaseConfig } from "./firebaseConfig"; -const _ = require("lodash"); -const clc = require("cli-color"); -const cjson = require("cjson"); -const fs = require("fs-extra"); -const path = require("path"); +import * as _ from "lodash"; +import * as clc from "cli-color"; +import * as fs from "fs-extra"; +import * as path from "path"; + +import { detectProjectRoot } from "./detectProjectRoot"; +import { FirebaseError } from "./error"; +import * as fsutils from "./fsutils"; +import { promptOnce } from "./prompt"; +import { resolveProjectPath } from "./projectPath"; +import * as utils from "./utils"; -const detectProjectRoot = require("./detectProjectRoot").detectProjectRoot; -const { FirebaseError } = require("./error"); -const fsutils = require("./fsutils"); +const cjson = require("cjson"); const loadCJSON = require("./loadCJSON"); const parseBoltRules = require("./parseBoltRules"); -const { promptOnce } = require("./prompt"); -const { resolveProjectPath } = require("./projectPath"); -const utils = require("./utils"); - -type PlainObject = Record; export class Config { + static DEFAULT_FUNCTIONS_SOURCE = "functions"; + static FILENAME = "firebase.json"; static MATERIALIZE_TARGETS = [ "database", @@ -60,7 +61,7 @@ export class Config { Config.MATERIALIZE_TARGETS.forEach((target) => { if (_.get(this._src, target)) { - _.set(this.data, target, this._materialize(target)); + _.set(this.data, target, this.materialize(target)); } }); @@ -70,36 +71,22 @@ export class Config { !this.get("functions.source") && fsutils.fileExistsSync(this.path("functions/package.json")) ) { - this.set("functions.source", "functions"); - } - } - - _hasDeepKey(obj: PlainObject, key: string) { - if (_.has(obj, key)) { - return true; - } - - for (const k in obj) { - if (obj.hasOwnProperty(k)) { - if (_.isPlainObject(obj[k]) && this._hasDeepKey(obj[k] as PlainObject, key)) { - return true; - } - } + this.set("functions.source", Config.DEFAULT_FUNCTIONS_SOURCE); } - return false; } - _materialize(target: string) { + private materialize(target: string) { const val = _.get(this._src, target); - if (_.isString(val)) { - let out = this._parseFile(target, val); + if (typeof val === "string") { + let out = this.parseFile(target, val); // if e.g. rules.json has {"rules": {}} use that - const lastSegment = _.last(target.split(".")); - if (_.size(out) === 1 && _.has(out, lastSegment)) { + const segments = target.split("."); + const lastSegment = segments[segments.length - 1]; + if (Object.keys(out).length === 1 && _.has(out, lastSegment)) { out = out[lastSegment]; } return out; - } else if (_.isPlainObject(val) || _.isArray(val)) { + } else if (val !== null && typeof val === "object") { return val; } @@ -108,7 +95,7 @@ export class Config { }); } - _parseFile(target: string, filePath: string) { + private parseFile(target: string, filePath: string) { const fullPath = resolveProjectPath(this.options, filePath); const ext = path.extname(filePath); if (!fsutils.fileExistsSync(fullPath)) { @@ -167,7 +154,7 @@ export class Config { path(pathName: string) { const outPath = path.normalize(path.join(this.projectDir, pathName)); - if (_.includes(path.relative(this.projectDir, outPath), "..")) { + if (path.relative(this.projectDir, outPath).includes("..")) { throw new FirebaseError(clc.bold(pathName) + " is outside of project directory", { exit: 1 }); } return outPath; diff --git a/src/database/rulesConfig.ts b/src/database/rulesConfig.ts index 17422452378..2f1acef021c 100644 --- a/src/database/rulesConfig.ts +++ b/src/database/rulesConfig.ts @@ -1,6 +1,7 @@ import { FirebaseError } from "../error"; import { Config } from "../config"; import { logger } from "../logger"; +import { Options } from "../options"; export interface RulesInstanceConfig { instance: string; @@ -29,19 +30,16 @@ export function normalizeRulesConfig( }); } -export function getRulesConfig(projectId: string, options: any): RulesInstanceConfig[] { - // TODO(samstern): Config should be typed - const config = options.config as any; - - const dbConfig: { rules?: string } | DatabaseConfig[] | undefined = config.get("database"); - +export function getRulesConfig(projectId: string, options: Options): RulesInstanceConfig[] { + const dbConfig = options.config.src.database; if (dbConfig === undefined) { return []; } if (!Array.isArray(dbConfig)) { if (dbConfig && dbConfig.rules) { - const instance = options.instance || `${options.project}-default-rtdb`; + const instance = + (options.instance as string | undefined) || `${options.project}-default-rtdb`; return [{ rules: dbConfig.rules, instance }]; } else { logger.debug("Possibly invalid database config: ", JSON.stringify(dbConfig)); @@ -50,13 +48,14 @@ export function getRulesConfig(projectId: string, options: any): RulesInstanceCo } const results: RulesInstanceConfig[] = []; + const rc = options.rc as any; for (const c of dbConfig) { if (c.target) { // Make sure the target exists (this will throw otherwise) - options.rc.requireTarget(projectId, "database", c.target); + rc.requireTarget(projectId, "database", c.target); // Get a list of db instances the target maps to - const instances: string[] = options.rc.target(projectId, "database", c.target); + const instances: string[] = rc.target(projectId, "database", c.target); for (const i of instances) { results.push({ instance: i, rules: c.rules }); } diff --git a/src/deploy/firestore/prepare.ts b/src/deploy/firestore/prepare.ts index be2bd7b863b..f6e4807b8d9 100644 --- a/src/deploy/firestore/prepare.ts +++ b/src/deploy/firestore/prepare.ts @@ -4,14 +4,15 @@ import * as clc from "cli-color"; import loadCJSON = require("../../loadCJSON"); import { RulesDeploy, RulesetServiceType } from "../../rulesDeploy"; import utils = require("../../utils"); +import { Options } from "../../options"; /** * Prepares Firestore Rules deploys. * @param context The deploy context. * @param options The CLI options object. */ -async function prepareRules(context: any, options: any): Promise { - const rulesFile = options.config.get("firestore.rules"); +async function prepareRules(context: any, options: Options): Promise { + const rulesFile = options.config.src.firestore?.rules; if (context.firestoreRules && rulesFile) { const rulesDeploy = new RulesDeploy(options, RulesetServiceType.CLOUD_FIRESTORE); @@ -25,12 +26,12 @@ async function prepareRules(context: any, options: any): Promise { * @param context The deploy context. * @param options The CLI options object. */ -function prepareIndexes(context: any, options: any): void { - if (!context.firestoreIndexes || !options.config.get("firestore.indexes")) { +function prepareIndexes(context: any, options: Options): void { + if (!context.firestoreIndexes || !options.config.src.firestore?.indexes) { return; } - const indexesFileName = options.config.get("firestore.indexes"); + const indexesFileName = options.config.src.firestore.indexes; const indexesPath = options.config.path(indexesFileName); const parsedSrc = loadCJSON(indexesPath); diff --git a/src/deploy/functions/deploy.ts b/src/deploy/functions/deploy.ts index 0375d788601..b1a6a4e93b7 100644 --- a/src/deploy/functions/deploy.ts +++ b/src/deploy/functions/deploy.ts @@ -10,6 +10,7 @@ import * as fs from "fs"; import * as gcs from "../../gcp/storage"; import * as gcf from "../../gcp/cloudfunctions"; import { Options } from "../../options"; +import { Config } from "../../config"; const GCP_REGION = functionsUploadRegion; @@ -45,7 +46,7 @@ export async function deploy( options: Options, payload: args.Payload ): Promise { - if (!options.config.get("functions")) { + if (!options.config.src.functions) { return; } @@ -66,11 +67,9 @@ export async function deploy( } await Promise.all(uploads); + const srcDir = options.config.src.functions.source || Config.DEFAULT_FUNCTIONS_SOURCE; logSuccess( - clc.green.bold("functions:") + - " " + - clc.bold(options.config.get("functions.source")) + - " folder uploaded successfully" + clc.green.bold("functions:") + " " + clc.bold(srcDir) + " folder uploaded successfully" ); } catch (err) { logWarning(clc.yellow("functions:") + " Upload Error: " + err.message); diff --git a/src/deploy/functions/discovery/jsexports/parseTriggers.ts b/src/deploy/functions/discovery/jsexports/parseTriggers.ts index 4db0dabcd01..106af20c0a9 100644 --- a/src/deploy/functions/discovery/jsexports/parseTriggers.ts +++ b/src/deploy/functions/discovery/jsexports/parseTriggers.ts @@ -9,6 +9,7 @@ import * as api from "../../../../api"; import * as proto from "../../../../gcp/proto"; import * as args from "../../args"; import { Options } from "../../../../options"; +import { Config } from "../../../../config"; const TRIGGER_PARSER = path.resolve(__dirname, "./triggerParser.js"); @@ -126,7 +127,8 @@ export async function discoverBackend( options: Options, configValues: backend.RuntimeConfigValues ): Promise { - const sourceDir = options.config.path(options.config.get("functions.source") as string); + const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; + const sourceDir = options.config.path(functionsSource); const triggerAnnotations = await parseTriggers(context.projectId, sourceDir, configValues); const want: backend.Backend = backend.empty(); for (const annotation of triggerAnnotations) { diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index f878712e9f1..aa22bbb208c 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -14,17 +14,18 @@ import * as functionsConfig from "../../functionsConfig"; import * as getProjectId from "../../getProjectId"; import * as validate from "./validate"; import { Options } from "../../options"; +import { Config } from "../../config"; export async function prepare( context: args.Context, options: Options, payload: args.Payload ): Promise { - if (!options.config.has("functions")) { + if (!options.config.src.functions) { return; } - const sourceDirName = options.config.get("functions.source") as string; + const sourceDirName = options.config.src.functions.source; if (!sourceDirName) { throw new FirebaseError( `No functions code detected at default location (./functions), and no functions.source defined in firebase.json` @@ -56,11 +57,9 @@ export async function prepare( context.firebaseConfig = firebaseConfig; // Prepare the functions directory for upload, and set context.triggers. + const srcDir = options.config.src.functions.source || Config.DEFAULT_FUNCTIONS_SOURCE; logBullet( - clc.cyan.bold("functions:") + - " preparing " + - clc.bold(options.config.get("functions.source")) + - " directory for uploading..." + clc.cyan.bold("functions:") + " preparing " + clc.bold(srcDir) + " directory for uploading..." ); context.functionsSource = await prepareFunctionsUpload(context, options); diff --git a/src/deploy/functions/prepareFunctionsUpload.ts b/src/deploy/functions/prepareFunctionsUpload.ts index a07b9408d41..98f34d03c1e 100644 --- a/src/deploy/functions/prepareFunctionsUpload.ts +++ b/src/deploy/functions/prepareFunctionsUpload.ts @@ -15,6 +15,7 @@ import * as utils from "../../utils"; import * as fsAsync from "../../fsAsync"; import * as args from "./args"; import { Options } from "../../options"; +import { Config } from "../../config"; const CONFIG_DEST_FILE = ".runtimeconfig.json"; @@ -66,7 +67,7 @@ async function packageSource(options: Options, sourceDir: string, configValues: // you're in the public dir when you deploy. // We ignore any CONFIG_DEST_FILE that already exists, and write another one // with current config values into the archive in the "end" handler for reader - const ignore = options.config.get("functions.ignore", ["node_modules", ".git"]) as string[]; + const ignore = options.config.src.functions?.ignore || ["node_modules", ".git"]; ignore.push( "firebase-debug.log", "firebase-debug.*.log", @@ -95,10 +96,12 @@ async function packageSource(options: Options, sourceDir: string, configValues: } ); } + + const srcDir = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; utils.logBullet( clc.cyan.bold("functions:") + " packaged " + - clc.bold(options.config.get("functions.source")) + + clc.bold(srcDir) + " (" + filesize(archive.pointer()) + ") for uploading" @@ -110,7 +113,8 @@ export async function prepareFunctionsUpload( context: args.Context, options: Options ): Promise { - const sourceDir = options.config.path(options.config.get("functions.source") as string); + const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; + const sourceDir = options.config.path(functionsSource); const configValues = await getFunctionsConfig(context); const backend = await discoverBackendSpec(context, options, configValues); options.config.set("functions.backend", backend); diff --git a/src/deploy/remoteconfig/prepare.ts b/src/deploy/remoteconfig/prepare.ts index dc9503c066f..34ee2a674bc 100644 --- a/src/deploy/remoteconfig/prepare.ts +++ b/src/deploy/remoteconfig/prepare.ts @@ -2,12 +2,13 @@ import { getProjectNumber } from "../../getProjectNumber"; import loadCJSON = require("../../loadCJSON"); import { getEtag } from "./functions"; import { validateInputRemoteConfigTemplate } from "./functions"; +import { Options } from "../../options"; -export default async function (context: any, options: any): Promise { +export default async function (context: any, options: Options): Promise { if (!context) { return; } - const filePath = options.config.get("remoteconfig.template"); + const filePath = options.config.src.remoteconfig?.template; if (!filePath) { return; } diff --git a/src/deploy/storage/prepare.ts b/src/deploy/storage/prepare.ts index f849758c36d..a65c8c5538c 100644 --- a/src/deploy/storage/prepare.ts +++ b/src/deploy/storage/prepare.ts @@ -2,13 +2,14 @@ import * as _ from "lodash"; import gcp = require("../../gcp"); import { RulesDeploy, RulesetServiceType } from "../../rulesDeploy"; +import { Options } from "../../options"; /** * Prepares for a Firebase Storage deployment. * @param context The deploy context. * @param options The CLI options object. */ -export default async function (context: any, options: any): Promise { +export default async function (context: any, options: Options): Promise { let rulesConfig = options.config.get("storage"); if (!rulesConfig) { return; @@ -27,7 +28,7 @@ export default async function (context: any, options: any): Promise { rulesConfig.forEach((ruleConfig: any) => { if (ruleConfig.target) { - options.rc.requireTarget(context.projectId, "storage", ruleConfig.target); + (options.rc as any).requireTarget(context.projectId, "storage", ruleConfig.target); } rulesDeploy.addFile(ruleConfig.rules); }); diff --git a/src/emulator/constants.ts b/src/emulator/constants.ts index 4d22831325a..8813aeacd6f 100644 --- a/src/emulator/constants.ts +++ b/src/emulator/constants.ts @@ -115,14 +115,6 @@ export class Constants { return DEFAULT_PORTS[emulator]; } - static getHostKey(emulator: Emulators): string { - return `emulators.${emulator.toString()}.host`; - } - - static getPortKey(emulator: Emulators): string { - return `emulators.${emulator.toString()}.port`; - } - static description(name: Emulators): string { return EMULATOR_DESCRIPTION[name]; } diff --git a/src/emulator/controller.ts b/src/emulator/controller.ts index cd768aa1bb5..0eaa02c061b 100644 --- a/src/emulator/controller.ts +++ b/src/emulator/controller.ts @@ -42,12 +42,10 @@ import { fileExistsSync } from "../fsutils"; import { StorageEmulator } from "./storage"; import { getDefaultDatabaseInstance } from "../getDefaultDatabaseInstance"; import { getProjectDefaultAccount } from "../auth"; +import { Options } from "../options"; -async function getAndCheckAddress(emulator: Emulators, options: any): Promise
{ - let host = Constants.normalizeHost( - options.config.get(Constants.getHostKey(emulator), Constants.getDefaultHost(emulator)) - ); - +async function getAndCheckAddress(emulator: Emulators, options: Options): Promise
{ + let host = options.config.src.emulators?.[emulator]?.host || Constants.getDefaultHost(emulator); if (host === "localhost" && utils.isRunningInWSL()) { // HACK(https://github.com/firebase/firebase-tools-ui/issues/332): Use IPv4 // 127.0.0.1 instead of localhost. This, combined with the hack in @@ -58,7 +56,7 @@ async function getAndCheckAddress(emulator: Emulators, options: any): Promise { - if (options.port) { +export const actionFunction = async (options: Options) => { + if (typeof options.port === "string") { options.port = parseInt(options.port, 10); } @@ -28,7 +29,7 @@ export const actionFunction = async (options: any) => { debugPort = commandUtils.parseInspectionPort(options); } - const hubClient = new EmulatorHubClient(options.project); + const hubClient = new EmulatorHubClient(options.project as string); let remoteEmulators: Record = {}; if (hubClient.foundHub()) { @@ -52,7 +53,7 @@ export const actionFunction = async (options: any) => { } else if (!options.port) { // If the user did not pass in any port and the functions emulator is not already running, we can // use the port defined for the Functions emulator in their firebase.json - options.port = options.config.get(Constants.getPortKey(Emulators.FUNCTIONS), undefined); + options.port = options.config.src.emulators?.functions?.port; } // If the port was not set by the --port flag or determined from 'firebase.json', just scan diff --git a/src/serve/functions.ts b/src/serve/functions.ts index cafd1967a50..7e8f90ccbd1 100644 --- a/src/serve/functions.ts +++ b/src/serve/functions.ts @@ -4,6 +4,8 @@ import { EmulatorServer } from "../emulator/emulatorServer"; import { parseRuntimeVersion } from "../emulator/functionsEmulatorUtils"; import * as getProjectId from "../getProjectId"; import { getProjectDefaultAccount } from "../auth"; +import { Options } from "../options"; +import { Config } from "../config"; // TODO(samstern): It would be better to convert this to an EmulatorServer // but we don't have the "options" object until start() is called. @@ -16,12 +18,10 @@ export class FunctionsServer { } } - async start(options: any, partialArgs: Partial): Promise { + async start(options: Options, partialArgs: Partial): Promise { const projectId = getProjectId(options, false); - const functionsDir = path.join( - options.config.projectDir, - options.config.get("functions.source") - ); + const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; + const functionsDir = path.join(options.config.projectDir, functionsSource); const account = getProjectDefaultAccount(options.config.projectDir); const nodeMajorVersion = parseRuntimeVersion(options.config.get("functions.runtime")); @@ -37,18 +37,20 @@ export class FunctionsServer { }; if (options.host) { - args.host = options.host; + args.host = options.host as string | undefined; } // If hosting emulator is not being served but Functions is, // we can use the port argument. Otherwise it goes to hosting and // we use port + 1. if (options.port) { - const hostingRunning = options.targets && options.targets.indexOf("hosting") >= 0; + const targets = options.targets as string[] | undefined; + const port = options.port as number; + const hostingRunning = targets && targets.indexOf("hosting") >= 0; if (hostingRunning) { - args.port = options.port + 1; + args.port = port + 1; } else { - args.port = options.port; + args.port = port; } } From 933ed5581fda9e994a4e0f517fd803a323777940 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 1 Jun 2021 17:55:34 +0100 Subject: [PATCH 2/4] Fix tests --- src/config.ts | 4 ++-- src/test/config.spec.js | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/config.ts b/src/config.ts index 352692c6ebf..d08a8daef90 100644 --- a/src/config.ts +++ b/src/config.ts @@ -75,7 +75,7 @@ export class Config { } } - private materialize(target: string) { + materialize(target: string) { const val = _.get(this._src, target); if (typeof val === "string") { let out = this.parseFile(target, val); @@ -95,7 +95,7 @@ export class Config { }); } - private parseFile(target: string, filePath: string) { + parseFile(target: string, filePath: string) { const fullPath = resolveProjectPath(this.options, filePath); const ext = path.extname(filePath); if (!fsutils.fileExistsSync(fullPath)) { diff --git a/src/test/config.spec.js b/src/test/config.spec.js index 9febcb7fa34..fc6f3a29210 100644 --- a/src/test/config.spec.js +++ b/src/test/config.spec.js @@ -24,36 +24,36 @@ describe("Config", function () { }); }); - describe("#_parseFile", function () { + describe("#parseFile", function () { it("should load a cjson file", function () { var config = new Config({}, { cwd: _fixtureDir("config-imports") }); - expect(config._parseFile("hosting", "hosting.json").public).to.equal("."); + expect(config.parseFile("hosting", "hosting.json").public).to.equal("."); }); it("should error out for an unknown file", function () { var config = new Config({}, { cwd: _fixtureDir("config-imports") }); expect(function () { - config._parseFile("hosting", "i-dont-exist.json"); + config.parseFile("hosting", "i-dont-exist.json"); }).to.throw("Imported file i-dont-exist.json does not exist"); }); it("should error out for an unrecognized extension", function () { var config = new Config({}, { cwd: _fixtureDir("config-imports") }); expect(function () { - config._parseFile("hosting", "unsupported.txt"); + config.parseFile("hosting", "unsupported.txt"); }).to.throw("unsupported.txt is not of a supported config file type"); }); }); - describe("#_materialize", function () { + describe("#materialize", function () { it("should assign unaltered if an object is found", function () { var config = new Config({ example: { foo: "bar" } }, {}); - expect(config._materialize("example").foo).to.equal("bar"); + expect(config.materialize("example").foo).to.equal("bar"); }); it("should prevent top-level key duplication", function () { var config = new Config({ rules: "rules.json" }, { cwd: _fixtureDir("dup-top-level") }); - expect(config._materialize("rules")).to.deep.equal({ ".read": true }); + expect(config.materialize("rules")).to.deep.equal({ ".read": true }); }); }); }); From 23042ff4f75025e16f48dcf6db2ba46767bad462 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 2 Jun 2021 11:39:41 +0100 Subject: [PATCH 3/4] Type assertions --- src/commands/logout.ts | 3 ++- src/commands/remoteconfig-get.ts | 18 ++++++++++++----- src/config.ts | 3 +-- src/database/rulesConfig.ts | 9 +++++---- src/emulator/controller.ts | 18 ++++++++++++----- src/functionsShellCommandAction.ts | 3 ++- src/serve/functions.ts | 7 +++++-- src/utils.ts | 32 ++++++++++++++++++++++++++++++ 8 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/commands/logout.ts b/src/commands/logout.ts index 387b98e19c4..cfc126d3399 100644 --- a/src/commands/logout.ts +++ b/src/commands/logout.ts @@ -9,7 +9,8 @@ import { promptOnce } from "../prompt"; module.exports = new Command("logout [email]") .description("log the CLI out of Firebase") .action(async (email: string | undefined, options: any) => { - const globalToken = utils.getInheritedOption(options, "token") as string | undefined; + const globalToken = utils.getInheritedOption(options, "token"); + utils.assertIsStringOrUndefined(globalToken); const allAccounts = auth.getAllAccounts(); if (allAccounts.length === 0 && !globalToken) { diff --git a/src/commands/remoteconfig-get.ts b/src/commands/remoteconfig-get.ts index 2a2c67c5251..12f18f50006 100644 --- a/src/commands/remoteconfig-get.ts +++ b/src/commands/remoteconfig-get.ts @@ -6,11 +6,12 @@ import { RemoteConfigTemplate } from "../remoteconfig/interfaces"; import getProjectId = require("../getProjectId"); import { requirePermissions } from "../requirePermissions"; import { parseTemplateForTable } from "../remoteconfig/get"; +import { Options } from "../options"; +import * as utils from "../utils"; import Table = require("cli-table"); import * as fs from "fs"; import util = require("util"); -import { Options } from "../options"; const tableHead = ["Entry Name", "Value"]; @@ -34,9 +35,10 @@ module.exports = new Command("remoteconfig:get") .before(requireAuth) .before(requirePermissions, ["cloudconfig.configs.get"]) .action(async (options: Options) => { + utils.assertIsString(options.versionNumber); const template: RemoteConfigTemplate = await rcGet.getTemplate( getProjectId(options), - checkValidNumber(options.versionNumber as string) + checkValidNumber(options.versionNumber) ); const table = new Table({ head: tableHead, style: { head: ["green"] } }); if (template.conditions) { @@ -60,9 +62,15 @@ module.exports = new Command("remoteconfig:get") const fileOut = !!options.output; if (fileOut) { const shouldUseDefaultFilename = options.output === true || options.output === ""; - const filename = shouldUseDefaultFilename - ? options.config.src.remoteconfig!.template - : (options.output as string); + + let filename = undefined; + if (shouldUseDefaultFilename) { + filename = options.config.src.remoteconfig!.template; + } else { + utils.assertIsString(options.output); + filename = options.output; + } + const outTemplate = { ...template }; delete outTemplate.version; fs.writeFileSync(filename, JSON.stringify(outTemplate, null, 2)); diff --git a/src/config.ts b/src/config.ts index d08a8daef90..6c9e04024ee 100644 --- a/src/config.ts +++ b/src/config.ts @@ -6,6 +6,7 @@ import * as _ from "lodash"; import * as clc from "cli-color"; import * as fs from "fs-extra"; import * as path from "path"; +const cjson = require("cjson"); import { detectProjectRoot } from "./detectProjectRoot"; import { FirebaseError } from "./error"; @@ -13,8 +14,6 @@ import * as fsutils from "./fsutils"; import { promptOnce } from "./prompt"; import { resolveProjectPath } from "./projectPath"; import * as utils from "./utils"; - -const cjson = require("cjson"); const loadCJSON = require("./loadCJSON"); const parseBoltRules = require("./parseBoltRules"); diff --git a/src/database/rulesConfig.ts b/src/database/rulesConfig.ts index 2f1acef021c..6299d225895 100644 --- a/src/database/rulesConfig.ts +++ b/src/database/rulesConfig.ts @@ -2,6 +2,7 @@ import { FirebaseError } from "../error"; import { Config } from "../config"; import { logger } from "../logger"; import { Options } from "../options"; +import * as utils from "../utils"; export interface RulesInstanceConfig { instance: string; @@ -19,9 +20,9 @@ interface DatabaseConfig { */ export function normalizeRulesConfig( rulesConfig: RulesInstanceConfig[], - options: any + options: Options ): RulesInstanceConfig[] { - const config = options.config as Config; + const config = options.config; return rulesConfig.map((rc) => { return { instance: rc.instance, @@ -38,8 +39,8 @@ export function getRulesConfig(projectId: string, options: Options): RulesInstan if (!Array.isArray(dbConfig)) { if (dbConfig && dbConfig.rules) { - const instance = - (options.instance as string | undefined) || `${options.project}-default-rtdb`; + utils.assertIsStringOrUndefined(options.instance); + const instance = options.instance || `${options.project}-default-rtdb`; return [{ rules: dbConfig.rules, instance }]; } else { logger.debug("Possibly invalid database config: ", JSON.stringify(dbConfig)); diff --git a/src/emulator/controller.ts b/src/emulator/controller.ts index 0eaa02c061b..bc7ceb1f4c8 100644 --- a/src/emulator/controller.ts +++ b/src/emulator/controller.ts @@ -43,6 +43,7 @@ import { StorageEmulator } from "./storage"; import { getDefaultDatabaseInstance } from "../getDefaultDatabaseInstance"; import { getProjectDefaultAccount } from "../auth"; import { Options } from "../options"; +import { ParsedTriggerDefinition } from "./functionsEmulatorShared"; async function getAndCheckAddress(emulator: Emulators, options: Options): Promise
{ let host = options.config.src.emulators?.[emulator]?.host || Constants.getDefaultHost(emulator); @@ -305,7 +306,7 @@ function findExportMetadata(importPath: string): ExportMetadata | undefined { } } -export async function startAll(options: any, showUI: boolean = true): Promise { +export async function startAll(options: Options, showUI: boolean = true): Promise { // Emulators config is specified in firebase.json as: // "emulators": { // "firestore": { @@ -384,6 +385,7 @@ export async function startAll(options: any, showUI: boolean = true): Promise | undefined), }, - predefinedTriggers: options.extensionTriggers, + predefinedTriggers: options.extensionTriggers as ParsedTriggerDefinition[] | undefined, nodeMajorVersion: parseRuntimeVersion( options.extensionNodeVersion || options.config.get("functions.runtime") ), @@ -464,6 +468,7 @@ export async function startAll(options: any, showUI: boolean = true): Promise { debugPort = commandUtils.parseInspectionPort(options); } - const hubClient = new EmulatorHubClient(options.project as string); + utils.assertIsString(options.project); + const hubClient = new EmulatorHubClient(options.project); let remoteEmulators: Record = {}; if (hubClient.foundHub()) { diff --git a/src/serve/functions.ts b/src/serve/functions.ts index 7e8f90ccbd1..c2ce55c9d35 100644 --- a/src/serve/functions.ts +++ b/src/serve/functions.ts @@ -6,6 +6,7 @@ import * as getProjectId from "../getProjectId"; import { getProjectDefaultAccount } from "../auth"; import { Options } from "../options"; import { Config } from "../config"; +import * as utils from "../utils"; // TODO(samstern): It would be better to convert this to an EmulatorServer // but we don't have the "options" object until start() is called. @@ -37,15 +38,17 @@ export class FunctionsServer { }; if (options.host) { - args.host = options.host as string | undefined; + utils.assertIsStringOrUndefined(options.host); + args.host = options.host; } // If hosting emulator is not being served but Functions is, // we can use the port argument. Otherwise it goes to hosting and // we use port + 1. if (options.port) { + utils.assertIsNumber(options.port); const targets = options.targets as string[] | undefined; - const port = options.port as number; + const port = options.port; const hostingRunning = targets && targets.indexOf("hosting") >= 0; if (hostingRunning) { args.port = port + 1; diff --git a/src/utils.ts b/src/utils.ts index 9cdb941d189..be0532f664c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -7,6 +7,7 @@ import * as process from "process"; import { Readable } from "stream"; import * as winston from "winston"; import { SPLAT } from "triple-beam"; +import { AssertionError } from "assert"; const ansiStrip = require("cli-color/strip") as (input: string) => string; import { configstore } from "./configstore"; @@ -499,3 +500,34 @@ export function isRunningInWSL(): boolean { export function thirtyDaysFromNow(): Date { return new Date(Date.now() + THIRTY_DAYS_IN_MILLISECONDS); } + +/** + * See: + * https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#assertion-functions + */ +export function assertIsString(val: any, message?: string): asserts val is string { + if (typeof val !== "string") { + throw new AssertionError({ + message: message || `expected "string" but got "${typeof val}"`, + }); + } +} + +export function assertIsNumber(val: any, message?: string): asserts val is number { + if (typeof val !== "number") { + throw new AssertionError({ + message: message || `expected "number" but got "${typeof val}"`, + }); + } +} + +export function assertIsStringOrUndefined( + val: any, + message?: string +): asserts val is string | undefined { + if (!(val === undefined || typeof val === "string")) { + throw new AssertionError({ + message: message || `expected "string" or "undefined" but got "${typeof val}"`, + }); + } +} From ff259ddbc0c24ce8b53371eff004ef62a6bb5d01 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 3 Jun 2021 12:58:46 +0100 Subject: [PATCH 4/4] Change defaults handline --- src/config.ts | 17 +++++++++++++++-- src/deploy/functions/deploy.ts | 11 +++++++++-- .../discovery/jsexports/parseTriggers.ts | 9 +++++++-- src/deploy/functions/prepare.ts | 11 +++++++++-- src/deploy/functions/prepareFunctionsUpload.ts | 17 +++++++++++++---- src/emulator/controller.ts | 9 +++++++-- src/functionsShellCommandAction.ts | 2 +- src/options.ts | 10 ++++++++-- src/serve/functions.ts | 9 +++++++-- src/test/deploy/functions/prompts.spec.ts | 3 +++ src/utils.ts | 8 ++++++++ 11 files changed, 87 insertions(+), 19 deletions(-) diff --git a/src/config.ts b/src/config.ts index 6c9e04024ee..6ea5063c9f0 100644 --- a/src/config.ts +++ b/src/config.ts @@ -21,7 +21,7 @@ export class Config { static DEFAULT_FUNCTIONS_SOURCE = "functions"; static FILENAME = "firebase.json"; - static MATERIALIZE_TARGETS = [ + static MATERIALIZE_TARGETS: Array = [ "database", "emulators", "firestore", @@ -39,6 +39,10 @@ export class Config { private _src: any; + /** + * @param src incoming firebase.json source, parsed by not validated. + * @param options command-line options. + */ constructor(src: any, options: any) { this.options = options || {}; this.projectDir = options.projectDir || detectProjectRoot(options); @@ -54,17 +58,22 @@ export class Config { ); } + // Move the deprecated top-level "rules" ket into the "database" object if (_.has(this._src, "rules")) { _.set(this._src, "database.rules", this._src.rules); } + // If a top-level key contains a string path pointing to a suported file + // type (JSON or Bolt), we read the file. + // + // TODO: This is janky and confusing behavior, we should remove it ASAP. Config.MATERIALIZE_TARGETS.forEach((target) => { if (_.get(this._src, target)) { _.set(this.data, target, this.materialize(target)); } }); - // auto-detect functions from package.json in directory + // Auto-detect functions from package.json in directory if ( this.projectDir && !this.get("functions.source") && @@ -144,6 +153,10 @@ export class Config { } set(key: string, value: any) { + // TODO: We should really remove all instances of config.set() around the + // codebase but until we do we need this to prevent src from going stale. + _.set(this._src, key, value); + return _.set(this.data, key, value); } diff --git a/src/deploy/functions/deploy.ts b/src/deploy/functions/deploy.ts index b1a6a4e93b7..8b23d1aaae5 100644 --- a/src/deploy/functions/deploy.ts +++ b/src/deploy/functions/deploy.ts @@ -11,6 +11,7 @@ import * as gcs from "../../gcp/storage"; import * as gcf from "../../gcp/cloudfunctions"; import { Options } from "../../options"; import { Config } from "../../config"; +import * as utils from "../../utils"; const GCP_REGION = functionsUploadRegion; @@ -67,9 +68,15 @@ export async function deploy( } await Promise.all(uploads); - const srcDir = options.config.src.functions.source || Config.DEFAULT_FUNCTIONS_SOURCE; + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); logSuccess( - clc.green.bold("functions:") + " " + clc.bold(srcDir) + " folder uploaded successfully" + clc.green.bold("functions:") + + " " + + clc.bold(options.config.src.functions.source) + + " folder uploaded successfully" ); } catch (err) { logWarning(clc.yellow("functions:") + " Upload Error: " + err.message); diff --git a/src/deploy/functions/discovery/jsexports/parseTriggers.ts b/src/deploy/functions/discovery/jsexports/parseTriggers.ts index 106af20c0a9..f824041ec05 100644 --- a/src/deploy/functions/discovery/jsexports/parseTriggers.ts +++ b/src/deploy/functions/discovery/jsexports/parseTriggers.ts @@ -10,6 +10,7 @@ import * as proto from "../../../../gcp/proto"; import * as args from "../../args"; import { Options } from "../../../../options"; import { Config } from "../../../../config"; +import * as utils from "../../../../utils"; const TRIGGER_PARSER = path.resolve(__dirname, "./triggerParser.js"); @@ -127,8 +128,12 @@ export async function discoverBackend( options: Options, configValues: backend.RuntimeConfigValues ): Promise { - const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; - const sourceDir = options.config.path(functionsSource); + utils.assertDefined(options.config.src.functions); + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); + const sourceDir = options.config.path(options.config.src.functions.source); const triggerAnnotations = await parseTriggers(context.projectId, sourceDir, configValues); const want: backend.Backend = backend.empty(); for (const annotation of triggerAnnotations) { diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index aa22bbb208c..e952ae4f93a 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -15,6 +15,7 @@ import * as getProjectId from "../../getProjectId"; import * as validate from "./validate"; import { Options } from "../../options"; import { Config } from "../../config"; +import * as utils from "../../utils"; export async function prepare( context: args.Context, @@ -57,9 +58,15 @@ export async function prepare( context.firebaseConfig = firebaseConfig; // Prepare the functions directory for upload, and set context.triggers. - const srcDir = options.config.src.functions.source || Config.DEFAULT_FUNCTIONS_SOURCE; + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); logBullet( - clc.cyan.bold("functions:") + " preparing " + clc.bold(srcDir) + " directory for uploading..." + clc.cyan.bold("functions:") + + " preparing " + + clc.bold(options.config.src.functions.source) + + " directory for uploading..." ); context.functionsSource = await prepareFunctionsUpload(context, options); diff --git a/src/deploy/functions/prepareFunctionsUpload.ts b/src/deploy/functions/prepareFunctionsUpload.ts index 98f34d03c1e..ed4409620fb 100644 --- a/src/deploy/functions/prepareFunctionsUpload.ts +++ b/src/deploy/functions/prepareFunctionsUpload.ts @@ -97,11 +97,15 @@ async function packageSource(options: Options, sourceDir: string, configValues: ); } - const srcDir = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; + utils.assertDefined(options.config.src.functions); + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); utils.logBullet( clc.cyan.bold("functions:") + " packaged " + - clc.bold(srcDir) + + clc.bold(options.config.src.functions.source) + " (" + filesize(archive.pointer()) + ") for uploading" @@ -113,8 +117,13 @@ export async function prepareFunctionsUpload( context: args.Context, options: Options ): Promise { - const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; - const sourceDir = options.config.path(functionsSource); + utils.assertDefined(options.config.src.functions); + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); + + const sourceDir = options.config.path(options.config.src.functions.source); const configValues = await getFunctionsConfig(context); const backend = await discoverBackendSpec(context, options, configValues); options.config.set("functions.backend", backend); diff --git a/src/emulator/controller.ts b/src/emulator/controller.ts index bc7ceb1f4c8..39f32215879 100644 --- a/src/emulator/controller.ts +++ b/src/emulator/controller.ts @@ -403,12 +403,17 @@ export async function startAll(options: Options, showUI: boolean = true): Promis const functionsLogger = EmulatorLogger.forEmulator(Emulators.FUNCTIONS); const functionsAddr = await getAndCheckAddress(Emulators.FUNCTIONS, options); const projectId = getProjectId(options, false); - const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; + + utils.assertDefined(options.config.src.functions); + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); utils.assertIsStringOrUndefined(options.extensionDir); const functionsDir = path.join( options.extensionDir || options.config.projectDir, - functionsSource + options.config.src.functions.source ); let inspectFunctions: number | undefined; diff --git a/src/functionsShellCommandAction.ts b/src/functionsShellCommandAction.ts index fca47b5daa9..7f7dd859857 100644 --- a/src/functionsShellCommandAction.ts +++ b/src/functionsShellCommandAction.ts @@ -29,7 +29,7 @@ export const actionFunction = async (options: Options) => { debugPort = commandUtils.parseInspectionPort(options); } - utils.assertIsString(options.project); + utils.assertDefined(options.project); const hubClient = new EmulatorHubClient(options.project); let remoteEmulators: Record = {}; diff --git a/src/options.ts b/src/options.ts index 1ad4a0c3c48..1ff547c543d 100644 --- a/src/options.ts +++ b/src/options.ts @@ -5,13 +5,19 @@ import { Config } from "./config"; export interface Options { cwd: string; configPath: string; - // OMITTED: project. Use context.projectId instead only: string; config: Config; filteredTargets: string[]; - nonInteractive: boolean; force: boolean; + // Options which are present on every command + project?: string; + account?: string; + json: boolean; + nonInteractive: boolean; + interactive: boolean; + debug: boolean; + // TODO(samstern): Remove this once options is better typed [key: string]: unknown; } diff --git a/src/serve/functions.ts b/src/serve/functions.ts index c2ce55c9d35..6fa3bffd42a 100644 --- a/src/serve/functions.ts +++ b/src/serve/functions.ts @@ -21,8 +21,13 @@ export class FunctionsServer { async start(options: Options, partialArgs: Partial): Promise { const projectId = getProjectId(options, false); - const functionsSource = options.config.src.functions?.source || Config.DEFAULT_FUNCTIONS_SOURCE; - const functionsDir = path.join(options.config.projectDir, functionsSource); + utils.assertDefined(options.config.src.functions); + utils.assertDefined( + options.config.src.functions.source, + "Error: 'functions.source' is not defined" + ); + + const functionsDir = path.join(options.config.projectDir, options.config.src.functions.source); const account = getProjectDefaultAccount(options.config.projectDir); const nodeMajorVersion = parseRuntimeVersion(options.config.get("functions.runtime")); diff --git a/src/test/deploy/functions/prompts.spec.ts b/src/test/deploy/functions/prompts.spec.ts index c6e36b3f3c7..a3f1613da21 100644 --- a/src/test/deploy/functions/prompts.spec.ts +++ b/src/test/deploy/functions/prompts.spec.ts @@ -35,6 +35,9 @@ const SAMPLE_OPTIONS: Options = { config: {} as any, only: "functions", nonInteractive: false, + json: false, + interactive: false, + debug: false, force: false, filteredTargets: ["functions"], }; diff --git a/src/utils.ts b/src/utils.ts index be0532f664c..5bd6b96eec2 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -505,6 +505,14 @@ export function thirtyDaysFromNow(): Date { * See: * https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#assertion-functions */ +export function assertDefined(val: T, message?: string): asserts val is NonNullable { + if (val === undefined || val === null) { + throw new AssertionError({ + message: message || `expected value to be defined but got "${val}"`, + }); + } +} + export function assertIsString(val: any, message?: string): asserts val is string { if (typeof val !== "string") { throw new AssertionError({