diff --git a/src/fsutils.spec.ts b/src/fsutils.spec.ts index e4b7ede6ad5..cc608b4e879 100644 --- a/src/fsutils.spec.ts +++ b/src/fsutils.spec.ts @@ -2,7 +2,14 @@ import { expect } from "chai"; import * as fs from "fs"; import * as path from "path"; import * as tmp from "tmp"; -import { fileExistsSync, dirExistsSync, readFile, listFiles, moveAll } from "./fsutils"; +import { + fileExistsSync, + dirExistsSync, + readFile, + listFiles, + moveAll, + removeDirectoryIfEmpty, +} from "./fsutils"; describe("fsutils", () => { let tmpDir: tmp.DirResult; @@ -124,4 +131,40 @@ describe("fsutils", () => { expect(fs.existsSync(path.join(destDir, "dest"))).to.be.false; }); }); + + describe("removeDirectoryIfEmpty", () => { + it("should delete directory if it exists and is empty", async () => { + const dirPath = path.join(tmpDir.name, "empty-dir"); + fs.mkdirSync(dirPath); + expect(fs.existsSync(dirPath)).to.be.true; + + await removeDirectoryIfEmpty(dirPath); + + expect(fs.existsSync(dirPath)).to.be.false; + }); + + it("should not delete directory if it contains files", async () => { + const dirPath = path.join(tmpDir.name, "non-empty-dir"); + fs.mkdirSync(dirPath); + fs.writeFileSync(path.join(dirPath, "file.txt"), "content"); + + await removeDirectoryIfEmpty(dirPath); + + expect(fs.existsSync(dirPath)).to.be.true; + }); + + it("should do nothing if path does not exist", async () => { + const nonExistent = path.join(tmpDir.name, "does-not-exist"); + await expect(removeDirectoryIfEmpty(nonExistent)).to.be.fulfilled; + }); + + it("should do nothing if path is not a directory", async () => { + const filePath = path.join(tmpDir.name, "regular-file.txt"); + fs.writeFileSync(filePath, "content"); + + await removeDirectoryIfEmpty(filePath); + + expect(fs.existsSync(filePath)).to.be.true; + }); + }); }); diff --git a/src/fsutils.ts b/src/fsutils.ts index 06c5ea9c1b3..336c4cd5571 100644 --- a/src/fsutils.ts +++ b/src/fsutils.ts @@ -1,7 +1,8 @@ import { existsSync, mkdirSync, readFileSync, readdirSync, statSync } from "fs"; import * as path from "path"; -import { FirebaseError } from "./error"; -import { moveSync } from "fs-extra"; +import * as fs from "fs-extra"; +import { FirebaseError, getErrMsg } from "./error"; +import { logger } from "./logger"; export function fileExistsSync(path: string): boolean { try { @@ -50,6 +51,26 @@ export function moveAll(srcDir: string, destDir: string) { for (const f of files) { const srcPath = path.join(srcDir, f); if (srcPath === destDir) continue; - moveSync(srcPath, path.join(destDir, f)); + fs.moveSync(srcPath, path.join(destDir, f)); + } +} + +/** + * Removes an empty directory if it exists and contains no files or subdirectories, + * suppressing any errors and logging to debug. + */ +export async function removeDirectoryIfEmpty(absDirPath: string): Promise { + try { + if (await fs.pathExists(absDirPath)) { + const stat = await fs.stat(absDirPath); + if (stat.isDirectory()) { + const entries = await fs.readdir(absDirPath); + if (entries.length === 0) { + await fs.remove(absDirPath); + } + } + } + } catch (err: unknown) { + logger.debug(`Failed to clean up directory '${absDirPath}' if empty: ${getErrMsg(err)}`); } } diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 61da86a7b9c..11249c9f7d5 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -35,6 +35,7 @@ import { promptAndWriteKitParams, getKitPackagesToSave, resolveSdkSpecifierToSave, + revertFunctionsConfig, TemplateType, } from "./install"; import * as env from "./env"; @@ -61,6 +62,8 @@ describe("functions/kits/install", () => { let loggerInfoStub: sinon.SinonStub; let loggerWarnStub: sinon.SinonStub; let statStub: sinon.SinonStub; + let fsRemoveStub: sinon.SinonStub; + let fsReaddirStub: sinon.SinonStub; beforeEach(() => { sinon.stub(experiments, "assertEnabled"); @@ -76,6 +79,8 @@ describe("functions/kits/install", () => { sinon.stub(fs, "readJson").resolves({}); sinon.stub(fs, "writeJson").resolves(); sinon.stub(fs, "writeFile").resolves(); + fsRemoveStub = sinon.stub(fs, "remove").resolves(); + fsReaddirStub = sinon.stub(fs, "readdir").resolves([]); seedKitInstanceEnvStub = sinon.stub(env, "seedKitInstanceEnv"); loggerInfoStub = sinon.stub(logger, "info"); loggerWarnStub = sinon.stub(logger, "warn"); @@ -2474,19 +2479,17 @@ describe("functions/kits/install", () => { sinon.stub(prompt, "select").resolves("addInstance"); sinon.stub(prompt, "input").resolves("inst2"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(res).to.deep.equal({ action: "addedInstance", @@ -2540,19 +2543,17 @@ describe("functions/kits/install", () => { }); const writeResolvedParamsStub = sinon.stub(functionsEnv, "writeResolvedParams"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(res.action).to.equal("addedInstance"); expect(delegate.discoverBuild).to.have.been.calledWith( @@ -2580,19 +2581,17 @@ describe("functions/kits/install", () => { sinon.stub(prompt, "select").resolves("addEnv"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(res).to.deep.equal({ action: "configuredEnv", @@ -2620,25 +2619,23 @@ describe("functions/kits/install", () => { sinon.stub(prompt, "select").resolves("addEnv"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - seedEnv: { - projectId: "my-project", - envs: { - FOO: "bar", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + seedEnv: { + projectId: "my-project", + envs: { + FOO: "bar", }, }, existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(seedKitInstanceEnvStub).to.have.been.calledOnceWith({ configDir: path.join( @@ -2695,19 +2692,17 @@ describe("functions/kits/install", () => { }); const writeResolvedParamsStub = sinon.stub(functionsEnv, "writeResolvedParams"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(res).to.deep.equal({ action: "configuredEnv", @@ -2739,21 +2734,19 @@ describe("functions/kits/install", () => { const resolveParamsStub = sinon.stub(params, "resolveParams"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - projectId: "my-project", - configure: false, - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + projectId: "my-project", + configure: false, existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(res).to.deep.equal({ action: "configuredEnv", @@ -2806,20 +2799,18 @@ describe("functions/kits/install", () => { const selectStub = sinon.stub(prompt, "select").resolves("addEnv"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1", "inst2"], }, - ); + }); expect(selectStub).to.have.been.calledOnceWith({ message: @@ -2868,20 +2859,18 @@ describe("functions/kits/install", () => { const selectStub = sinon.stub(prompt, "select"); sinon.stub(prompt, "input").resolves("inst3"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1", "inst2"], }, - ); + }); expect(selectStub).to.not.have.been.called; expect(res.action).to.equal("addedInstance"); @@ -2922,22 +2911,20 @@ describe("functions/kits/install", () => { const selectStub = sinon.stub(prompt, "select"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - instanceId: "inst2", - nonInteractive: true, - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, + instanceId: "inst2", + nonInteractive: true, existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1", "inst2"], }, - ); + }); expect(selectStub).to.not.have.been.called; expect(res.action).to.equal("configuredEnv"); @@ -2969,21 +2956,19 @@ describe("functions/kits/install", () => { } as unknown as Config; await expect( - addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - instanceId: "inst1", - }, + addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, + instanceId: "inst1", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ), + }), ).to.be.rejectedWith( FirebaseError, "Instance 'inst1' is already configured for this project.", @@ -3008,21 +2993,19 @@ describe("functions/kits/install", () => { const selectStub = sinon.stub(prompt, "select"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - instanceId: "inst1", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, + instanceId: "inst1", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(selectStub).to.not.have.been.called; expect(res.action).to.equal("configuredEnv"); @@ -3051,21 +3034,19 @@ describe("functions/kits/install", () => { const selectStub = sinon.stub(prompt, "select"); - const res = await addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - instanceId: "inst-new", - }, + const res = await addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, + instanceId: "inst-new", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export"], existingCodebases: [], existingInstanceIds: ["inst1"], }, - ); + }); expect(selectStub).to.not.have.been.called; expect(res.action).to.equal("addedInstance"); @@ -3089,21 +3070,19 @@ describe("functions/kits/install", () => { } as unknown as Config; await expect( - addKitInstanceOrConfigureProject( - { - config: mockConfig, - project: "my-project", - configure: false, - instanceId: "other-kit-inst", - }, + addKitInstanceOrConfigureProject({ + config: mockConfig, + project: "my-project", + configure: false, + instanceId: "other-kit-inst", existingKit, - { + existingFunctionsInfo: { existingFunctions: [existingKit], existingKitIds: ["firestore-bigquery-export", "other-kit"], existingCodebases: [], existingInstanceIds: ["inst1", "other-kit-inst"], }, - ), + }), ).to.be.rejectedWith( FirebaseError, "functions kit instance ID must be unique across all kits, but 'other-kit-inst' was used more than once.", @@ -3888,5 +3867,332 @@ describe("functions/kits/install", () => { "/mock/project/function-kits/custom-kit/source", ); }); + + it("should clean up package kit directory and revert config if npm install fails", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + wrapSpawnStub.rejects(new Error("npm ERR! code ENOTFOUND")); + + await expect( + installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", + nonInteractive: true, + }), + ).to.be.rejectedWith(FirebaseError, /NPM install failed/); + + expect(fsRemoveStub).to.have.been.calledWith( + path.join("/mock/project", "function-kits/firestore-bigquery-export"), + ); + }); + + it("should clean up package kit directory and empty function-kits dir if param resolution fails", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + const paramList: params.Param[] = [{ name: "DATASET_NAME", type: "string" }]; + const mockBuild: build.Build = { + requiredAPIs: [], + endpoints: {}, + params: paramList, + }; + const delegate = { + discoverBuild: sinon.stub().resolves(mockBuild), + }; + sinon + .stub(runtimes, "getRuntimeDelegate") + .resolves(delegate as unknown as runtimes.RuntimeDelegate); + sinon.stub(params, "resolveParams").rejects(new FirebaseError("Failed to resolve param")); + + (fs.pathExists as sinon.SinonStub) + .withArgs(path.join("/mock/project", "function-kits")) + .resolves(true); + fsReaddirStub.withArgs(path.join("/mock/project", "function-kits")).resolves([]); + + await expect( + installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", + nonInteractive: true, + projectId: "target-proj", + }), + ).to.be.rejectedWith(FirebaseError, "Failed to resolve param"); + + expect(fsRemoveStub).to.have.been.calledWith( + path.join("/mock/project", "function-kits/firestore-bigquery-export"), + ); + expect(fsRemoveStub).to.have.been.calledWith(path.join("/mock/project", "function-kits")); + }); + + it("should clean up directory kit config directory without deleting user source when install fails", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + (fs.pathExists as sinon.SinonStub).withArgs("/mock/project/my-functions").resolves(true); + (fs.pathExists as sinon.SinonStub) + .withArgs(path.join("/mock/project/my-functions", "package.json")) + .resolves(true); + (fs.stat as sinon.SinonStub) + .withArgs("/mock/project/my-functions") + .resolves({ isDirectory: () => true } as fs.Stats); + (fs.readJson as sinon.SinonStub) + .withArgs(path.join("/mock/project/my-functions", "package.json")) + .resolves({ scripts: { build: "tsc" } }); + + wrapSpawnStub.rejects(new Error("Build compilation error")); + + await expect( + installKitOrInstance({ + config: mockConfig, + directory: "./my-functions", + nonInteractive: true, + }), + ).to.be.rejectedWith(FirebaseError, /NPM install failed/); + + expect(fsRemoveStub).to.have.been.calledWith( + path.join("/mock/project", "function-kits/my-functions/config-my-functions"), + ); + expect(fsRemoveStub).to.not.have.been.calledWith("/mock/project/my-functions"); + }); + + it("should revert firebase.json if modified before a failure in installKitOrInstance", async () => { + const initialFunctions = [ + { + codebase: "default", + source: "functions", + }, + ]; + const writtenSnapshots: string[] = []; + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [...initialFunctions] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub().callsFake((_file: string, content: unknown) => { + writtenSnapshots.push(JSON.stringify(content)); + }), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + const mockBuild: build.Build = { + requiredAPIs: [], + endpoints: {}, + params: [], + requiredRoles: ["roles/viewer"], + }; + const delegate = { + discoverBuild: sinon.stub().resolves(mockBuild), + }; + sinon + .stub(runtimes, "getRuntimeDelegate") + .resolves(delegate as unknown as runtimes.RuntimeDelegate); + sinon.stub(iam, "getRoleName").rejects(new Error("Reporting error")); + + await expect( + installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", + nonInteractive: true, + configure: true, + }), + ).to.be.rejectedWith("Reporting error"); + + expect(mockConfig.src.functions).to.deep.equal(initialFunctions); + // Verify the revert was actually persisted, not just applied in memory. + expect(writtenSnapshots).to.not.be.empty; + expect(JSON.parse(writtenSnapshots[writtenSnapshots.length - 1])).to.deep.equal({ + functions: initialFunctions, + }); + }); + }); + + describe("existing kit cleanup on failure in installKitOrInstance", () => { + it("should clean up instance config dir and revert firebase.json if addInstance fails during params prompt", async () => { + const existingKit: ValidatedKitSingle = { + kit: "firestore-bigquery-export", + sourcePackage: { name: "@firebase-function-kits/firestore-bigquery-export" }, + source: "function-kits/firestore-bigquery-export/source", + instances: { + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }, + }; + // Snapshot what is actually serialized on each write. Asserting against a live + // `mockConfig.src` reference would pass trivially, since sinon records the object + // by reference and later mutations would be reflected in the recorded call. + const writtenSnapshots: string[] = []; + const writeProjectFileStub = sinon.stub().callsFake((_file: string, content: unknown) => { + writtenSnapshots.push(JSON.stringify(content)); + }); + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [existingKit] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: writeProjectFileStub, + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + const paramList: params.Param[] = [{ name: "PARAM_A", type: "string" }]; + const mockBuild: build.Build = { + requiredAPIs: [], + endpoints: {}, + params: paramList, + }; + const delegate = { + discoverBuild: sinon.stub().resolves(mockBuild), + }; + sinon + .stub(runtimes, "getRuntimeDelegate") + .resolves(delegate as unknown as runtimes.RuntimeDelegate); + sinon.stub(params, "resolveParams").rejects(new FirebaseError("Required param missing")); + + await expect( + installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export", + instanceId: "inst2", + nonInteractive: true, + projectId: "my-project", + }), + ).to.be.rejectedWith(FirebaseError, "Required param missing"); + + expect(fsRemoveStub).to.have.been.calledWith( + path.join("/mock/project", "function-kits/firestore-bigquery-export/config-inst2"), + ); + // The in-memory config must no longer reference the failed instance. + expect((mockConfig.src.functions as ValidatedKitSingle[])[0].instances).to.deep.equal({ + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }); + // The last thing persisted to disk must also be free of the failed instance, + // otherwise firebase.json is left pointing at a config dir that was deleted. + expect(writtenSnapshots).to.not.be.empty; + const lastWritten = JSON.parse(writtenSnapshots[writtenSnapshots.length - 1]) as { + functions: ValidatedKitSingle[]; + }; + expect(lastWritten.functions[0].instances).to.deep.equal({ + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }); + }); + + it("should clean up created project env file if addEnv fails during params prompt", async () => { + const existingKit: ValidatedKitSingle = { + kit: "firestore-bigquery-export", + sourcePackage: { name: "@firebase-function-kits/firestore-bigquery-export" }, + source: "function-kits/firestore-bigquery-export/source", + instances: { + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }, + }; + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [existingKit] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + (fs.pathExists as sinon.SinonStub) + .withArgs( + path.join( + "/mock/project", + "function-kits/firestore-bigquery-export/config-inst1/.env.my-project", + ), + ) + .resolves(false); + + const paramList: params.Param[] = [{ name: "PARAM_A", type: "string" }]; + const mockBuild: build.Build = { + requiredAPIs: [], + endpoints: {}, + params: paramList, + }; + const delegate = { + discoverBuild: sinon.stub().resolves(mockBuild), + }; + sinon + .stub(runtimes, "getRuntimeDelegate") + .resolves(delegate as unknown as runtimes.RuntimeDelegate); + sinon.stub(params, "resolveParams").rejects(new FirebaseError("Required param missing")); + + await expect( + installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export", + instanceId: "inst1", + nonInteractive: true, + projectId: "my-project", + }), + ).to.be.rejectedWith(FirebaseError, "Required param missing"); + + expect(fsRemoveStub).to.have.been.calledWith( + path.join( + "/mock/project", + "function-kits/firestore-bigquery-export/config-inst1/.env.my-project", + ), + ); + }); + }); + + describe("revertFunctionsConfig", () => { + it("should revert functions and save when config was modified", () => { + const originalFunctions = [{ codebase: "default", source: "functions" }]; + const writeProjectFileStub = sinon.stub(); + const mockConfig = { + src: { + functions: [{ kit: "new-kit", source: "function-kits/new-kit" }], + }, + writeProjectFile: writeProjectFileStub, + } as unknown as Config; + + revertFunctionsConfig({ config: mockConfig, originalFunctions }); + + expect(mockConfig.src.functions).to.deep.equal(originalFunctions); + expect(writeProjectFileStub).to.have.been.calledOnceWith("firebase.json", mockConfig.src); + }); + + it("should delete functions and save when originally undefined", () => { + const writeProjectFileStub = sinon.stub(); + const mockConfig = { + src: { + functions: [{ kit: "new-kit", source: "function-kits/new-kit" }], + }, + writeProjectFile: writeProjectFileStub, + } as unknown as Config; + + revertFunctionsConfig({ config: mockConfig, originalFunctions: undefined }); + + expect(mockConfig.src.functions).to.be.undefined; + expect("functions" in mockConfig.src).to.be.false; + expect(writeProjectFileStub).to.have.been.calledOnceWith("firebase.json", mockConfig.src); + }); + + it("should do nothing when config was not modified", () => { + const originalFunctions = [{ codebase: "default", source: "functions" }]; + const writeProjectFileStub = sinon.stub(); + const mockConfig = { + src: { + functions: [{ codebase: "default", source: "functions" }], + }, + writeProjectFile: writeProjectFileStub, + } as unknown as Config; + + revertFunctionsConfig({ config: mockConfig, originalFunctions }); + + expect(writeProjectFileStub).to.not.have.been.called; + }); }); }); diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index 1514b3428e3..f1bd77c4f60 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -8,7 +8,14 @@ import { Config } from "../../config"; import { FirebaseError, getErrMsg } from "../../error"; import { KitFunctionConfig, FunctionsConfig } from "../../firebaseConfig"; import { getProjectId } from "../../projectUtils"; -import { logLabeledBullet, logLabeledSuccess, logLabeledWarning, resolveWithin } from "../../utils"; +import { + cloneDeep, + deepEqual, + logLabeledBullet, + logLabeledSuccess, + logLabeledWarning, + resolveWithin, +} from "../../utils"; import { addKitPrefix, isKitConfig, @@ -32,10 +39,10 @@ import * as functionsEnv from "../env"; import * as functionsConfig from "../../functionsConfig"; import { partitionUserEnvs } from "../../deploy/functions/prepare"; import { FirebaseConfig } from "../../deploy/functions/args"; -import { cloneDeep } from "../../utils"; import { hasProjectEnv } from "../env"; import { RC } from "../../rc"; import { KitInstanceEnvSeed, seedKitInstanceEnv } from "./env"; +import { removeDirectoryIfEmpty } from "../../fsutils"; export const TEMPLATES = { installation: "init/functions/typescript/index-kit.ts", @@ -130,6 +137,8 @@ export interface PromptExistingInstanceOptions { export interface ExistingKitInstallOptions { config: Config; + existingKit: ValidatedKitSingle; + existingFunctionsInfo: ExistingFunctionsInfo; project?: string; projectId?: string; nonInteractive?: boolean; @@ -139,6 +148,7 @@ export interface ExistingKitInstallOptions { instanceId?: string; defaultInstanceId?: string; seedEnv?: KitInstanceEnvSeed; + createdPaths?: string[]; } export interface PromptAndWriteKitParamsOptions { @@ -939,6 +949,29 @@ export function addInstanceToKitConfig( config.writeProjectFile("firebase.json", config.src); } +export interface RevertFunctionsConfigOptions { + config: Config; + originalFunctions: FunctionsConfig | undefined; +} + +/** + * Reverts the functions configuration in firebase.json to its original state if it was modified. + */ +export function revertFunctionsConfig(options: RevertFunctionsConfigOptions): void { + try { + if (!deepEqual(options.config.src.functions, options.originalFunctions)) { + if (options.originalFunctions === undefined) { + delete options.config.src.functions; + } else { + options.config.src.functions = options.originalFunctions; + } + options.config.writeProjectFile("firebase.json", options.config.src); + } + } catch (err: unknown) { + logger.debug(`Failed to revert firebase.json: ${getErrMsg(err)}`); + } +} + /** * Scaffolds a new kit and its initial instance, optionally seeds the .env. configuration, * and updates firebase.json. @@ -988,6 +1021,7 @@ export async function addInstanceToKit( const configDirPath = path.join(FUNCTION_KITS_DIR, options.kitId, `config-${options.instanceId}`); const absConfigDirPath = options.config.path(configDirPath); + await fs.ensureDir(absConfigDirPath); if (options.seedEnv?.envs && Object.keys(options.seedEnv.envs).length > 0) { @@ -1163,9 +1197,8 @@ export async function printKitFirstDeployReport( */ export async function addKitInstanceOrConfigureProject( options: ExistingKitInstallOptions, - existingKit: ValidatedKitSingle, - existingFunctionsInfo: ExistingFunctionsInfo, ): Promise { + const { existingKit, existingFunctionsInfo, createdPaths = [] } = options; const projectId = getProjectId(options) || options.projectId; const projectAlias = options.rc?.hasProjects && options.project && options.rc.hasProjectAlias(options.project) @@ -1234,6 +1267,14 @@ export async function addKitInstanceOrConfigureProject( options.instanceId, ); + const expectedConfigDirPath = path.join( + FUNCTION_KITS_DIR, + existingKit.kit, + `config-${instanceId}`, + ); + absConfigDirPath = options.config.path(expectedConfigDirPath); + createdPaths.push(absConfigDirPath); + const result = await addInstanceToKit({ config: options.config, kitId: existingKit.kit, @@ -1257,6 +1298,14 @@ export async function addKitInstanceOrConfigureProject( ); } absConfigDirPath = options.config.path(configDirPath); + + if (projectId) { + const envPath = path.join(absConfigDirPath, `.env.${projectId}`); + if (!(await fs.pathExists(envPath))) { + createdPaths.push(envPath); + } + } + if (options.seedEnv?.envs && Object.keys(options.seedEnv.envs).length > 0) { await fs.ensureDir(absConfigDirPath); seedKitInstanceEnv({ @@ -1454,108 +1503,143 @@ export async function installKitOrInstance( throw new FirebaseError("Cannot specify --template with --directory."); } - const existingFunctionsInfo = extractExistingFunctionsInfo(options.config.src.functions); - const existingKit = findExistingKit(existingFunctionsInfo.existingFunctions, options); - if (existingKit) { - return addKitInstanceOrConfigureProject(options, existingKit, existingFunctionsInfo); - } + const originalFunctions = cloneDeep(options.config.src.functions); + const createdPaths: string[] = []; + let kitId: string | undefined; - const source = options.directory - ? await resolveDirectorySource(options) - : await resolvePackageSource(options); + try { + const existingFunctionsInfo = extractExistingFunctionsInfo(options.config.src.functions); + const existingKit = findExistingKit(existingFunctionsInfo.existingFunctions, options); + if (existingKit) { + kitId = existingKit.kit; + return await addKitInstanceOrConfigureProject({ + ...options, + existingKit, + existingFunctionsInfo, + createdPaths, + }); + } - const kitId = await promptKitId( - source.defaultKitName, - existingFunctionsInfo.existingKitIds, - options.nonInteractive, - options.kitId, - ); + const source = options.directory + ? await resolveDirectorySource(options) + : await resolvePackageSource(options); - const instanceId = await promptKitInstanceId( - options.defaultInstanceId ?? kitId, - existingFunctionsInfo.existingInstanceIds, - existingFunctionsInfo.existingCodebases, - options.nonInteractive, - options.instanceId, - ); + kitId = await promptKitId( + source.defaultKitName, + existingFunctionsInfo.existingKitIds, + options.nonInteractive, + options.kitId, + ); - const { sourcePath, configDirPath, absSourcePath, absConfigDirPath } = await source.setup( - kitId, - instanceId, - ); + const instanceId = await promptKitInstanceId( + options.defaultInstanceId ?? kitId, + existingFunctionsInfo.existingInstanceIds, + existingFunctionsInfo.existingCodebases, + options.nonInteractive, + options.instanceId, + ); - if (options.seedEnv?.envs && Object.keys(options.seedEnv.envs).length > 0) { - seedKitInstanceEnv({ - configDir: absConfigDirPath, - functionsSource: absSourcePath, - projectDir: options.config.projectDir, - projectId: options.seedEnv.projectId, - projectAlias: options.seedEnv.projectAlias, - envs: options.seedEnv.envs, - }); - } + const isPackageKit = !options.directory; + const absKitDir = options.config.path(path.join(FUNCTION_KITS_DIR, kitId)); + const expectedConfigDirPath = path.join(FUNCTION_KITS_DIR, kitId, `config-${instanceId}`); + const absConfigDirPath = options.config.path(expectedConfigDirPath); - await source.buildAndInstall(absSourcePath); + if (isPackageKit) { + createdPaths.push(absKitDir); + } else { + createdPaths.push(absConfigDirPath); + } - const projectId = getProjectId(options) || options.projectId; - const projectAlias = - options.rc?.hasProjects && options.project && options.rc.hasProjectAlias(options.project) - ? options.project - : undefined; + const setupResult = await source.setup(kitId, instanceId); + const { sourcePath, configDirPath, absSourcePath } = setupResult; - if (projectId) { - fs.ensureFileSync(path.join(absConfigDirPath, `.env.${projectId}`)); - } + if (options.seedEnv?.envs && Object.keys(options.seedEnv.envs).length > 0) { + seedKitInstanceEnv({ + configDir: setupResult.absConfigDirPath, + functionsSource: absSourcePath, + projectDir: options.config.projectDir, + projectId: options.seedEnv.projectId, + projectAlias: options.seedEnv.projectAlias, + envs: options.seedEnv.envs, + }); + } - let discoveredBuild: build.Build | undefined; + await source.buildAndInstall(absSourcePath); - const shouldConfigure = options.configure !== false; - if (shouldConfigure) { - try { - discoveredBuild = await discoverKitBuild({ ...options, instanceId }, absSourcePath); - } catch (err: unknown) { - logger.debug(`Could not discover kit build for params prompting: ${getErrMsg(err)}`); + const projectId = getProjectId(options) || options.projectId; + const projectAlias = + options.rc?.hasProjects && options.project && options.rc.hasProjectAlias(options.project) + ? options.project + : undefined; + + if (projectId) { + fs.ensureFileSync(path.join(setupResult.absConfigDirPath, `.env.${projectId}`)); } - if (discoveredBuild?.params && discoveredBuild.params.length > 0) { - await promptAndWriteKitParams({ - config: options.config, - projectId, - projectAlias, - absConfigDirPath, - absSourcePath, - instanceId, - nonInteractive: options.nonInteractive, - force: options.force, - params: discoveredBuild.params, - }); + let discoveredBuild: build.Build | undefined; + + const shouldConfigure = options.configure !== false; + if (shouldConfigure) { + try { + discoveredBuild = await discoverKitBuild({ ...options, instanceId }, absSourcePath); + } catch (err: unknown) { + logger.debug(`Could not discover kit build for params prompting: ${getErrMsg(err)}`); + } + + if (discoveredBuild?.params && discoveredBuild.params.length > 0) { + await promptAndWriteKitParams({ + config: options.config, + projectId, + projectAlias, + absConfigDirPath: setupResult.absConfigDirPath, + absSourcePath, + instanceId, + nonInteractive: options.nonInteractive, + force: options.force, + params: discoveredBuild.params, + }); + } } - } - addKitToConfig(options.config, { - kitId, - instanceId, - packageName: source.sourcePackageName, - sourcePath, - configDirPath, - hasBuildScript: source.hasBuildScript, - }); + addKitToConfig(options.config, { + kitId, + instanceId, + packageName: source.sourcePackageName, + sourcePath, + configDirPath, + hasBuildScript: source.hasBuildScript, + }); - logLabeledSuccess("functions", `Function kit ${clc.bold(kitId)} successfully installed.`); - await printKitFirstDeployReport({ - config: options.config, - project: options.project, - projectId: options.projectId, - instanceId, - absSourcePath, - preDiscoveredBuild: discoveredBuild, - }); + logLabeledSuccess("functions", `Function kit ${clc.bold(kitId)} successfully installed.`); + await printKitFirstDeployReport({ + config: options.config, + project: options.project, + projectId: options.projectId, + instanceId, + absSourcePath, + preDiscoveredBuild: discoveredBuild, + }); - return { - action: "installedKit", - kitId, - instanceId, - sourcePath, - configDirPath, - }; + return { + action: "installedKit", + kitId, + instanceId, + sourcePath, + configDirPath, + }; + } catch (err: unknown) { + await Promise.all( + createdPaths.map((targetPath) => + fs.remove(targetPath).catch((cleanupErr: unknown) => { + logger.debug(`Failed to clean up path '${targetPath}': ${getErrMsg(cleanupErr)}`); + }), + ), + ); + if (kitId) { + await removeDirectoryIfEmpty(options.config.path(path.join(FUNCTION_KITS_DIR, kitId))); + } + await removeDirectoryIfEmpty(options.config.path(FUNCTION_KITS_DIR)); + revertFunctionsConfig({ config: options.config, originalFunctions }); + throw err; + } }