-
Notifications
You must be signed in to change notification settings - Fork 937
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor deploy/functions/prepare logic and add tests (#1218)
* Refactor functions prepare step to allow for engines check * Split out validation logic for functions prepare step * Minor fix in wording in the test for consistency * Removing strict from ts files, fixing imports, adding jsdocs * adding sandboxing to sinon tests, adding beforeEach() and afterEach() * Small fix in jsdoc * Add a comment about legacy behavior when package.json not found * switch to non-deprecated sinon sandbox, fix throw() in tests, add some comments about buggy behavior * fix srcmain.js => src/main.js in a test * fix regex for validating function names to allow capital letters, and update jsdoc to match * fix wording in error message and jsdoc on what valid function name should look like * add jsdoc on resolveProjectPath() function * removing legacy behavior, Firebase Functions => Cloud Functions in error message
- Loading branch information
Showing
7 changed files
with
249 additions
and
69 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import * as FirebaseError from "../../error"; | ||
| import * as _ from "lodash"; | ||
| import * as path from "path"; | ||
| import * as clc from "cli-color"; | ||
| import * as logger from "../../logger"; | ||
| import * as projectPath from "../../projectPath"; | ||
| import * as fsutils from "../../fsutils"; | ||
|
|
||
| // have to require this because no @types/cjson available | ||
| // tslint:disable-next-line | ||
| const cjson = require("cjson"); | ||
|
|
||
| /** | ||
| * Check that functions directory exists. | ||
| * @param cwd Working directory. | ||
| * @param sourceDirName Relative path to source directory. | ||
| * @throws { FirebaseError } Functions directory must exist. | ||
| */ | ||
| export function functionsDirectoryExists(cwd: string, sourceDirName: string): void { | ||
| if (!fsutils.dirExistsSync(projectPath.resolveProjectPath(cwd, sourceDirName))) { | ||
| const msg = | ||
| `could not deploy functions because the ${clc.bold('"' + sourceDirName + '"')} ` + | ||
| `directory was not found. Please create it or specify a different source directory in firebase.json`; | ||
| throw new FirebaseError(msg); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validate function names only contain letters, numbers, underscores, and hyphens | ||
| * and not exceed 62 characters in length. | ||
| * @param functionNames Object containing function names as keys. | ||
| * @throws { FirebaseError } Function names must be valid. | ||
| */ | ||
| export function functionNamesAreValid(functionNames: {}): void { | ||
| const validFunctionNameRegex = /^[a-zA-Z0-9_-]{1,62}$/; | ||
| const invalidNames = _.reject( | ||
| _.keys(functionNames), | ||
| (name: string): boolean => { | ||
| return _.startsWith(name, ".") || validFunctionNameRegex.test(name); | ||
| } | ||
| ); | ||
| if (!_.isEmpty(invalidNames)) { | ||
| const msg = | ||
| `${invalidNames.join(", ")} function name(s) can only contain letters, ` + | ||
| `numbers, hyphens, and not exceed 62 characters in length`; | ||
| throw new FirebaseError(msg); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validate contents of package.json to ensure main file is present. | ||
| * @param sourceDirName Name of source directory. | ||
| * @param sourceDir Relative path of source directory. | ||
| * @param projectDir Relative path of project directory. | ||
| * @throws { FirebaseError } Package.json must be present and valid. | ||
| */ | ||
| export function packageJsonIsValid( | ||
| sourceDirName: string, | ||
| sourceDir: string, | ||
| projectDir: string | ||
| ): void { | ||
| const packageJsonFile = path.join(sourceDir, "package.json"); | ||
| if (!fsutils.fileExistsSync(packageJsonFile)) { | ||
| const msg = `No npm package found in functions source directory. Please run 'npm init' inside ${sourceDirName}`; | ||
| throw new FirebaseError(msg); | ||
| } | ||
|
|
||
| try { | ||
| const data = cjson.load(packageJsonFile); | ||
| logger.debug("> [functions] package.json contents:", JSON.stringify(data, null, 2)); | ||
| const indexJsFile = path.join(sourceDir, data.main || "index.js"); | ||
| if (!fsutils.fileExistsSync(indexJsFile)) { | ||
| const msg = `${path.relative( | ||
| projectDir, | ||
| indexJsFile | ||
| )} does not exist, can't deploy Cloud Functions`; | ||
| throw new FirebaseError(msg); | ||
| } | ||
| } catch (e) { | ||
| const msg = `There was an error reading ${sourceDirName}${path.sep}package.json:\n\n ${ | ||
| e.message | ||
| }`; | ||
| throw new FirebaseError(msg); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import * as path from "path"; | ||
| import * as detectProjectRoot from "./detectProjectRoot"; | ||
|
|
||
| /** | ||
| * Returns a fully qualified path to the wanted file/directory inside the project. | ||
| * @param cwd current working directory. | ||
| * @param filePath the target file or directory in the project. | ||
| */ | ||
| export function resolveProjectPath(cwd: string, filePath: string): string { | ||
| return path.resolve(detectProjectRoot(cwd), filePath); | ||
| } |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import { expect } from "chai"; | ||
| import * as fsutils from "../../../fsutils"; | ||
| import * as validate from "../../../deploy/functions/validate"; | ||
| import * as projectPath from "../../../projectPath"; | ||
| import * as FirebaseError from "../../../error"; | ||
| import * as sinon from "sinon"; | ||
|
|
||
| // have to require this because no @types/cjson available | ||
| // tslint:disable-next-line | ||
| const cjson = require("cjson"); | ||
|
|
||
| describe("validate", () => { | ||
| describe("functionsDirectoryExists", () => { | ||
| const sandbox: sinon.SinonSandbox = sinon.createSandbox(); | ||
| let resolvePpathStub: sinon.SinonStub; | ||
| let dirExistsStub: sinon.SinonStub; | ||
|
|
||
| beforeEach(() => { | ||
| resolvePpathStub = sandbox.stub(projectPath, "resolveProjectPath"); | ||
| dirExistsStub = sandbox.stub(fsutils, "dirExistsSync"); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| sandbox.restore(); | ||
| }); | ||
|
|
||
| it("should not throw error if functions directory is present", () => { | ||
| resolvePpathStub.returns("some/path/to/project"); | ||
| dirExistsStub.returns(true); | ||
|
|
||
| expect(() => { | ||
| validate.functionsDirectoryExists("cwd", "sourceDirName"); | ||
| }).to.not.throw(); | ||
| }); | ||
|
|
||
| it("should throw error if the functions directory does not exist", () => { | ||
| resolvePpathStub.returns("some/path/to/project"); | ||
| dirExistsStub.returns(false); | ||
|
|
||
| expect(() => { | ||
| validate.functionsDirectoryExists("cwd", "sourceDirName"); | ||
| }).to.throw(FirebaseError); | ||
| }); | ||
| }); | ||
|
|
||
| describe("functionNamesAreValid", () => { | ||
| it("should allow properly formatted function names", () => { | ||
| const properNames = { "my-function-1": "some field", "my-function-2": "some field" }; | ||
|
|
||
| expect(() => { | ||
| validate.functionNamesAreValid(properNames); | ||
| }).to.not.throw(); | ||
| }); | ||
|
|
||
| it("should throw error on improperly formatted function names", () => { | ||
| const properNames = { | ||
| "my-function-!@#$%": "some field", | ||
| "my-function-!@#$!@#": "some field", | ||
| }; | ||
|
|
||
| expect(() => { | ||
| validate.functionNamesAreValid(properNames); | ||
| }).to.throw(FirebaseError); | ||
| }); | ||
|
|
||
| it("should throw error if some function names are improperly formatted", () => { | ||
| const properNames = { "my-function$%#": "some field", "my-function-2": "some field" }; | ||
|
|
||
| expect(() => { | ||
| validate.functionNamesAreValid(properNames); | ||
| }).to.throw(FirebaseError); | ||
| }); | ||
|
|
||
| // I think it should throw error here but it doesn't error on empty or even undefined functionNames. | ||
| // TODO(b/131331234): fix this test when validation code path is fixed. | ||
| it.skip("should throw error on empty function names", () => { | ||
| const properNames = {}; | ||
|
|
||
| expect(() => { | ||
| validate.functionNamesAreValid(properNames); | ||
| }).to.throw(FirebaseError); | ||
| }); | ||
| }); | ||
|
|
||
| describe("packageJsonIsValid", () => { | ||
| const sandbox: sinon.SinonSandbox = sinon.createSandbox(); | ||
| let cjsonLoadStub: sinon.SinonStub; | ||
| let fileExistsStub: sinon.SinonStub; | ||
|
|
||
| beforeEach(() => { | ||
| fileExistsStub = sandbox.stub(fsutils, "fileExistsSync"); | ||
| cjsonLoadStub = sandbox.stub(cjson, "load"); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| sandbox.restore(); | ||
| }); | ||
|
|
||
| it("should throw error if package.json file is missing", () => { | ||
| fileExistsStub.withArgs("sourceDir/package.json").returns(false); | ||
|
|
||
| expect(() => { | ||
| validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); | ||
| }).to.throw(FirebaseError, "No npm package found"); | ||
| }); | ||
|
|
||
| it("should throw error if functions source file is missing", () => { | ||
| cjsonLoadStub.returns({ name: "my-project" }); | ||
| fileExistsStub.withArgs("sourceDir/package.json").returns(true); | ||
| fileExistsStub.withArgs("sourceDir/index.js").returns(false); | ||
|
|
||
| expect(() => { | ||
| validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); | ||
| }).to.throw(FirebaseError, "does not exist, can't deploy"); | ||
| }); | ||
|
|
||
| it("should throw error if main is defined and that file is missing", () => { | ||
| cjsonLoadStub.returns({ name: "my-project", main: "src/main.js" }); | ||
| fileExistsStub.withArgs("sourceDir/package.json").returns(true); | ||
| fileExistsStub.withArgs("sourceDir/src/main.js").returns(false); | ||
|
|
||
| expect(() => { | ||
| validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); | ||
| }).to.throw(FirebaseError, "does not exist, can't deploy"); | ||
| }); | ||
|
|
||
| it("should not throw error if package.json and functions file exist", () => { | ||
| cjsonLoadStub.returns({ name: "my-project" }); | ||
| fileExistsStub.withArgs("sourceDir/package.json").returns(true); | ||
| fileExistsStub.withArgs("sourceDir/index.js").returns(true); | ||
|
|
||
| expect(() => { | ||
| validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); | ||
| }).to.not.throw(); | ||
| }); | ||
| }); | ||
| }); |