From c24c452c5b2df534a41571ecb25b3c49e761d55c Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Fri, 11 Oct 2019 08:15:24 -0700 Subject: [PATCH 1/3] update lint to not fix warnings by default (#1715) while we have a bunch of warnings that we do not want to act on, let's keep eslint from touching EVERY file (and this should make it pass `npm test`, though `npm run lint` will error with warnings. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index deffe3c41d2..6976c3a4c3b 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "build": "tsc", "build:watch": "tsc --watch", "clean": "rm -rf lib", - "format": "npm run lint -- --fix", + "format": "npm run lint -- --fix --quiet", "lint": "eslint '**/*.{js,ts}'", "mocha": "nyc mocha --opts mocha.opts", "prepare": "npm run clean && npm run build -- --build tsconfig.publish.json", From 526dc1b593887284dc07e0942a3c70890d64ce1f Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Fri, 11 Oct 2019 09:10:54 -0700 Subject: [PATCH 2/3] javascript lint cleanup x3 (#1710) * remove one-var exception * split declarations onto multiple lines * remove guard-for-in * fix various issues * remove no-undef * clean up and fix variables * use functionName in delete options * use Map for hash cache data --- .eslintrc.js | 3 --- src/deploy/functions/release.js | 3 ++- src/deploy/hosting/hashcache.js | 6 +++--- src/deploy/hosting/uploader.js | 6 +++--- src/functionsDelete.js | 10 ++++++---- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 889d9ad9496..1114b72e50f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -36,13 +36,10 @@ module.exports = { { "files": ["*.js"], "rules": { - "guard-for-in": "warn", // TODO(bkendall): remove, allow to error. "no-extra-boolean-cast": "warn", // TODO(bkendall): remove, allow to error. "no-invalid-this": "warn", // TODO(bkendall): remove, allow to error. "no-redeclare": "warn", // TODO(bkendall): remove, allow to error. - "no-undef": "warn", // TODO(bkendall): remove, allow to error. "no-var": "warn", // TODO(bkendall): remove, allow to error. - "one-var": "warn", // TODO(bkendall): remove, allow to error. "prefer-rest-params": "warn", // TODO(bkendall): remove, allow to error. }, }, diff --git a/src/deploy/functions/release.js b/src/deploy/functions/release.js index 4910647551c..96dd78d351b 100644 --- a/src/deploy/functions/release.js +++ b/src/deploy/functions/release.js @@ -138,7 +138,8 @@ module.exports = function(context, options, payload) { }); var uploadedNames = _.map(functionsInfo, "name"); var functionFilterGroups = helper.getFilterGroups(options); - var deleteReleaseNames, existingScheduledFunctions; + var deleteReleaseNames; + var existingScheduledFunctions; delete payload.functions; return gcp.cloudfunctions diff --git a/src/deploy/hosting/hashcache.js b/src/deploy/hosting/hashcache.js index ba360fcaf3b..0930d6610e7 100644 --- a/src/deploy/hosting/hashcache.js +++ b/src/deploy/hosting/hashcache.js @@ -9,7 +9,7 @@ function cachePath(cwd, name) { exports.load = function(cwd, name) { try { const out = {}; - lines = fs.readFileSync(cachePath(cwd, name), { + const lines = fs.readFileSync(cachePath(cwd, name), { encoding: "utf8", }); lines.split("\n").forEach(function(line) { @@ -32,9 +32,9 @@ exports.load = function(cwd, name) { exports.dump = function(cwd, name, data) { let st = ""; let count = 0; - for (let path in data) { + for (const [path, d] of data) { count++; - st += path + "," + data[path].mtime + "," + data[path].hash + "\n"; + st += path + "," + d.mtime + "," + d.hash + "\n"; } try { fs.outputFileSync(cachePath(cwd, name), st, { encoding: "utf8" }); diff --git a/src/deploy/hosting/uploader.js b/src/deploy/hosting/uploader.js index 8e35f25f183..7f4f1be47c7 100644 --- a/src/deploy/hosting/uploader.js +++ b/src/deploy/hosting/uploader.js @@ -55,7 +55,7 @@ class Uploader { this.fileCount = this.files.length; this.cache = hashcache.load(this.projectRoot, this.hashcacheName()); - this.cacheNew = {}; + this.cacheNew = new Map(); this.sizeMap = {}; this.hashMap = {}; @@ -141,7 +141,7 @@ class Uploader { this.sizeMap[filePath] = stats.size; const cached = this.cache[filePath]; if (cached && cached.mtime === mtime) { - this.cacheNew[filePath] = cached; + this.cacheNew.set(filePath, cached); this.addHash(filePath, cached.hash); return Promise.resolve(); } @@ -155,7 +155,7 @@ class Uploader { return new Promise(function(resolve, reject) { fstream.on("end", function() { const hashVal = hash.read().toString("hex"); - self.cacheNew[filePath] = { mtime: mtime, hash: hashVal }; + self.cacheNew.set(filePath, { mtime: mtime, hash: hashVal }); self.addHash(filePath, hashVal); resolve(); }); diff --git a/src/functionsDelete.js b/src/functionsDelete.js index 70743916bb6..98bc323a7b7 100644 --- a/src/functionsDelete.js +++ b/src/functionsDelete.js @@ -5,9 +5,10 @@ var clc = require("cli-color"); var cloudfunctions = require("./gcp/cloudfunctions"); var cloudscheduler = require("./gcp/cloudscheduler"); -var pubsub = require("./gcp/pubsub"); +var FirebaseError = require("./error"); var helper = require("./functionsDeployHelper"); var logger = require("./logger"); +var pubsub = require("./gcp/pubsub"); var track = require("./track"); var utils = require("./utils"); @@ -50,8 +51,9 @@ var printTooManyOps = function(projectId) { module.exports = function(functionsToDelete, projectId, appEngineLocation) { deletes = _.map(functionsToDelete, function(name) { - var scheduleName = helper.getScheduleName(name, appEngineLocation); - var topicName = helper.getTopicName(name); + const scheduleName = helper.getScheduleName(name, appEngineLocation); + const topicName = helper.getTopicName(name); + const functionName = helper.getFunctionName(name); return { name: name, retryFunction: function() { @@ -84,7 +86,7 @@ module.exports = function(functionsToDelete, projectId, appEngineLocation) { return cloudfunctions.delete({ projectId: projectId, region: helper.getRegion(name), - functionName: helper.getFunctionName(name), + functionName, }); }); }, From 77b0182ec8943c66891740a5aa9b21a1dab86a2a Mon Sep 17 00:00:00 2001 From: Sam Stern Date: Fri, 11 Oct 2019 10:30:34 -0700 Subject: [PATCH 3/3] Integration with firebase-functions 3.3.0 (#1714) --- CHANGELOG.md | 3 +- src/emulator/emulatorLogger.ts | 17 +++- src/emulator/functionsEmulator.ts | 3 + src/emulator/functionsEmulatorRuntime.ts | 88 +++++++++++-------- src/emulator/functionsEmulatorUtils.ts | 59 ++++++++++++- src/emulator/types.ts | 2 +- .../emulators/functionsEmulatorUtils.spec.ts | 24 +++++ 7 files changed, 153 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10af7795b0e..2a8ab4a71d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ * Added support for managing the Realtime Database setting `strictTriggerValidation`. * Fixes trigger parser to not rely on prototype methods (#1687). -* Fixes bug where standalone CLI would hang in the Android Studio integrated terminal. \ No newline at end of file +* Fixes bug where standalone CLI would hang in the Android Studio integrated terminal. +* Fixes a bug where accessing refs from background function arguments would point to prod (#1682). \ No newline at end of file diff --git a/src/emulator/emulatorLogger.ts b/src/emulator/emulatorLogger.ts index 3c7afacd291..78a857e2267 100644 --- a/src/emulator/emulatorLogger.ts +++ b/src/emulator/emulatorLogger.ts @@ -7,8 +7,9 @@ import * as logger from "../logger"; * SUCCESS - useful to humans, similar to bullet but in a 'success' style. * USER - logged by user code, always show to humans. * WARN - warnings from our code that humans need. + * WARN_ONCE - warnings from our code that humans need, but only once per session. */ -type LogType = "DEBUG" | "INFO" | "BULLET" | "SUCCESS" | "USER" | "WARN"; +type LogType = "DEBUG" | "INFO" | "BULLET" | "SUCCESS" | "USER" | "WARN" | "WARN_ONCE"; const TYPE_VERBOSITY: { [type in LogType]: number } = { DEBUG: 0, @@ -17,6 +18,7 @@ const TYPE_VERBOSITY: { [type in LogType]: number } = { SUCCESS: 1, USER: 2, WARN: 2, + WARN_ONCE: 2, }; export enum Verbosity { @@ -27,6 +29,7 @@ export enum Verbosity { export class EmulatorLogger { static verbosity: Verbosity = Verbosity.DEBUG; + static warnOnceCache = new Set(); /** * Within this file, utils.logFoo() or logger.Foo() should not be called directly, @@ -54,6 +57,12 @@ export class EmulatorLogger { case "WARN": utils.logWarning(text); break; + case "WARN_ONCE": + if (!this.warnOnceCache.has(text)) { + utils.logWarning(text); + this.warnOnceCache.add(text); + } + break; case "SUCCESS": utils.logSuccess(text); break; @@ -80,6 +89,12 @@ export class EmulatorLogger { case "WARN": utils.logLabeledWarning(label, text); break; + case "WARN_ONCE": + if (!this.warnOnceCache.has(text)) { + utils.logLabeledWarning(label, text); + this.warnOnceCache.add(text); + } + break; } } diff --git a/src/emulator/functionsEmulator.ts b/src/emulator/functionsEmulator.ts index 5f8ac895984..8e0ab15e8a1 100644 --- a/src/emulator/functionsEmulator.ts +++ b/src/emulator/functionsEmulator.ts @@ -411,6 +411,9 @@ You can probably fix this by running "npm install ${ case "WARN": EmulatorLogger.logLabeled("WARN", "functions", log.text); break; + case "WARN_ONCE": + EmulatorLogger.logLabeled("WARN_ONCE", "functions", log.text); + break; case "FATAL": EmulatorLogger.logLabeled("WARN", "functions", log.text); break; diff --git a/src/emulator/functionsEmulatorRuntime.ts b/src/emulator/functionsEmulatorRuntime.ts index 0c86e8bc39e..b3256266674 100644 --- a/src/emulator/functionsEmulatorRuntime.ts +++ b/src/emulator/functionsEmulatorRuntime.ts @@ -11,6 +11,7 @@ import { getEmulatedTriggersFromDefinitions, getTemporarySocketPath, } from "./functionsEmulatorShared"; +import { parseVersionString, compareVersionStrings } from "./functionsEmulatorUtils"; import * as express from "express"; import * as path from "path"; import * as admin from "firebase-admin"; @@ -85,10 +86,11 @@ interface ModuleResolution { resolution?: string; } -interface ModuleVersion { - major: number; - minor: number; - patch: number; +interface SuccessfulModuleResolution { + declared: true; + installed: true; + version: string; + resolution: string; } interface ProxyTarget extends Object { @@ -246,10 +248,26 @@ async function resolveDeveloperNodeModule( return moduleResolution; } +async function assertResolveDeveloperNodeModule( + frb: FunctionsRuntimeBundle, + name: string +): Promise { + const resolution = await resolveDeveloperNodeModule(frb, name); + if ( + !(resolution.installed && resolution.declared && resolution.resolution && resolution.version) + ) { + throw new Error( + `Assertion failure: could not fully resolve ${name}: ${JSON.stringify(resolution)}` + ); + } + + return resolution as SuccessfulModuleResolution; +} + async function verifyDeveloperNodeModules(frb: FunctionsRuntimeBundle): Promise { const modBundles = [ - { name: "firebase-admin", isDev: false, minVersion: 8 }, - { name: "firebase-functions", isDev: false, minVersion: 3 }, + { name: "firebase-admin", isDev: false, minVersion: "8.0.0" }, + { name: "firebase-functions", isDev: false, minVersion: "3.0.0" }, ]; for (const modBundle of modBundles) { @@ -268,8 +286,7 @@ async function verifyDeveloperNodeModules(frb: FunctionsRuntimeBundle): Promise< return false; } - const versionInfo = parseVersionString(resolution.version); - if (versionInfo.major < modBundle.minVersion) { + if (compareVersionStrings(resolution.version, modBundle.minVersion) < 0) { new EmulatorLog("SYSTEM", "out-of-date-module", "", modBundle).log(); return false; } @@ -299,23 +316,6 @@ function requirePackageJson(frb: FunctionsRuntimeBundle): PackageJSON | undefine } } -/** - * Parse a semver version string into parts, filling in 0s where empty. - */ -function parseVersionString(version?: string): ModuleVersion { - const parts = (version || "0").split("."); - - // Make sure "parts" always has 3 elements. Extras are ignored. - parts.push("0"); - parts.push("0"); - - return { - major: parseInt(parts[0], 10), - minor: parseInt(parts[1], 10), - patch: parseInt(parts[2], 10), - }; -} - /* We mock out a ton of different paths that we can take to network I/O. It doesn't matter if they overlap (like TLS and HTTPS) because the dev will either whitelist, block, or allow for one @@ -433,11 +433,10 @@ function InitializeNetworkFiltering(frb: FunctionsRuntimeBundle): void { https://github.com/firebase/firebase-functions/blob/9e3bda13565454543b4c7b2fd10fb627a6a3ab97/src/providers/https.ts#L66 */ async function InitializeFirebaseFunctionsStubs(frb: FunctionsRuntimeBundle): Promise { - const firebaseFunctionsResolution = await resolveDeveloperNodeModule(frb, "firebase-functions"); - if (!firebaseFunctionsResolution.resolution) { - throw new Error("Could not resolve 'firebase-functions'"); - } - + const firebaseFunctionsResolution = await assertResolveDeveloperNodeModule( + frb, + "firebase-functions" + ); const firebaseFunctionsRoot = findModuleRoot( "firebase-functions", firebaseFunctionsResolution.resolution @@ -445,9 +444,8 @@ async function InitializeFirebaseFunctionsStubs(frb: FunctionsRuntimeBundle): Pr const httpsProviderResolution = path.join(firebaseFunctionsRoot, "lib/providers/https"); // TODO: Remove this logic and stop relying on internal APIs. See #1480 for reasoning. - const functionsVersion = parseVersionString(firebaseFunctionsResolution.version!); let methodName = "_onRequestWithOpts"; - if (functionsVersion.major >= 3 && functionsVersion.minor >= 1) { + if (compareVersionStrings(firebaseFunctionsResolution.version, "3.1.0") >= 0) { methodName = "_onRequestWithOptions"; } @@ -514,12 +512,12 @@ function getDefaultConfig(): any { * We also mock out firestore.settings() so we can merge the emulator settings with the developer's. */ async function InitializeFirebaseAdminStubs(frb: FunctionsRuntimeBundle): Promise { - const adminResolution = await resolveDeveloperNodeModule(frb, "firebase-admin"); - if (!adminResolution.resolution) { - throw new Error("Could not resolve 'firebase-admin'"); - } + const adminResolution = await assertResolveDeveloperNodeModule(frb, "firebase-admin"); const localAdminModule = require(adminResolution.resolution); + const functionsResolution = await assertResolveDeveloperNodeModule(frb, "firebase-functions"); + const localFunctionsModule = require(functionsResolution.resolution); + // Set up global proxied Firestore proxiedFirestore = await makeProxiedFirestore(frb, localAdminModule); @@ -558,6 +556,20 @@ async function InitializeFirebaseAdminStubs(frb: FunctionsRuntimeBundle): Promis databaseApp = adminModuleTarget.initializeApp(databaseAppOptions, DATABASE_APP); proxiedDatabase = makeProxiedDatabase(adminModuleTarget); + // Tell the Firebase Functions SDK to use the proxied app so that things like "change.after.ref" + // point to the right place. + if (localFunctionsModule.app.setEmulatedAdminApp) { + localFunctionsModule.app.setEmulatedAdminApp(defaultApp); + } else { + new EmulatorLog( + "WARN_ONCE", + "runtime-status", + `You're using firebase-functions v${ + functionsResolution.version + }, please upgrade to firebase-functions v3.3.0 or higher for best results.` + ).log(); + } + return defaultApp; }) .when("firestore", (target) => { @@ -679,6 +691,7 @@ function warnAboutFirestoreProd(): void { "runtime-status", "The Cloud Firestore emulator is not running, so calls to Firestore will affect production." ).log(); + hasAccessedFirestore = true; } @@ -688,10 +701,11 @@ function warnAboutDatabaseProd(): void { } new EmulatorLog( - "WARN", + "WARN_ONCE", "runtime-status", "The Realtime Database emulator is not running, so calls to Realtime Database will affect production." ).log(); + hasAccessedDatabase = true; } diff --git a/src/emulator/functionsEmulatorUtils.ts b/src/emulator/functionsEmulatorUtils.ts index ef4295f485a..ad034c99d61 100644 --- a/src/emulator/functionsEmulatorUtils.ts +++ b/src/emulator/functionsEmulatorUtils.ts @@ -1,11 +1,20 @@ -/* -Please be careful when adding require/imports to this file, it is pulled into functionsEmulatorRuntime -which is ran in a separate node process, so it is likely to have unintended side-effects for you. +/** + * Please be careful when adding require/imports to this file, it is pulled into functionsEmulatorRuntime + * which is ran in a separate node process, so it is likely to have unintended side-effects for you. + * + * TODO(samstern): Merge this with functionsEmulatorShared + * TODO(samstern): Audit dependencies of functionsEmulatorShared */ const wildcardRegex = new RegExp("{[^/{}]*}"); const wildcardKeyRegex = new RegExp("^{(.+)}$"); +export interface ModuleVersion { + major: number; + minor: number; + patch: number; +} + export function extractParamsFromPath( wildcardPath: string, snapshotPath: string @@ -63,3 +72,47 @@ export function removePathSegments(path: string, count: number): string { .slice(count) .join("/"); } + +/** + * Parse a semver version string into parts, filling in 0s where empty. + */ +export function parseVersionString(version?: string): ModuleVersion { + const parts = (version || "0").split("."); + + // Make sure "parts" always has 3 elements. Extras are ignored. + parts.push("0"); + parts.push("0"); + + return { + major: parseInt(parts[0], 10), + minor: parseInt(parts[1], 10), + patch: parseInt(parts[2], 10), + }; +} + +/** + * Compare two SemVer version strings. + * + * Returns: + * - Positive number if a is greater. + * - Negative number if b is greater. + * - Zero if they are the same. + */ +export function compareVersionStrings(a?: string, b?: string) { + const versionA = parseVersionString(a); + const versionB = parseVersionString(b); + + if (versionA.major != versionB.major) { + return versionA.major - versionB.major; + } + + if (versionA.minor != versionB.minor) { + return versionA.minor - versionB.minor; + } + + if (versionA.patch != versionB.patch) { + return versionA.patch - versionB.patch; + } + + return 0; +} diff --git a/src/emulator/types.ts b/src/emulator/types.ts index 2030b51de00..55ff4c81a73 100644 --- a/src/emulator/types.ts +++ b/src/emulator/types.ts @@ -130,7 +130,7 @@ export class EmulatorLog { private static LOG_BUFFER: string[] = []; constructor( - public level: "DEBUG" | "INFO" | "WARN" | "ERROR" | "FATAL" | "SYSTEM" | "USER", + public level: "DEBUG" | "INFO" | "WARN" | "WARN_ONCE" | "ERROR" | "FATAL" | "SYSTEM" | "USER", public type: string, public text: string, public data?: any, diff --git a/src/test/emulators/functionsEmulatorUtils.spec.ts b/src/test/emulators/functionsEmulatorUtils.spec.ts index 96974c7f997..1bba247bc77 100644 --- a/src/test/emulators/functionsEmulatorUtils.spec.ts +++ b/src/test/emulators/functionsEmulatorUtils.spec.ts @@ -3,6 +3,7 @@ import { extractParamsFromPath, isValidWildcardMatch, trimSlashes, + compareVersionStrings, } from "../../emulator/functionsEmulatorUtils"; describe("FunctionsEmulatorUtils", () => { @@ -93,4 +94,27 @@ describe("FunctionsEmulatorUtils", () => { expect(trimSlashes("///a////b//c/")).to.equal("a/b/c"); }); }); + + describe("compareVersonStrings", () => { + it("should detect a higher major version", () => { + expect(compareVersionStrings("4.0.0", "3.2.1")).to.be.gt(0); + expect(compareVersionStrings("3.2.1", "4.0.0")).to.be.lt(0); + }); + + it("should detect a higher minor version", () => { + expect(compareVersionStrings("4.1.0", "4.0.1")).to.be.gt(0); + expect(compareVersionStrings("4.0.1", "4.1.0")).to.be.lt(0); + }); + + it("should detect a higher patch version", () => { + expect(compareVersionStrings("4.0.1", "4.0.0")).to.be.gt(0); + expect(compareVersionStrings("4.0.0", "4.0.1")).to.be.lt(0); + }); + + it("should detect the same version", () => { + expect(compareVersionStrings("4.0.0", "4.0.0")).to.eql(0); + expect(compareVersionStrings("4.0", "4.0.0")).to.eql(0); + expect(compareVersionStrings("4", "4.0.0")).to.eql(0); + }); + }); });