From 557c7816a701dd4e669190a6c749677d3d8c8718 Mon Sep 17 00:00:00 2001 From: Huihui Wu Date: Wed, 11 Dec 2024 11:02:32 +0800 Subject: [PATCH 1/4] feat: support env variables in localization.json --- .../driver/teamsApp/createAppPackage.ts | 19 +++++++++--- .../driver/teamsApp/utils/ManifestUtils.ts | 30 +++++++++++++++++-- .../src/component/driver/teamsApp/validate.ts | 8 ++--- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/packages/fx-core/src/component/driver/teamsApp/createAppPackage.ts b/packages/fx-core/src/component/driver/teamsApp/createAppPackage.ts index 79ab167922..f520a955d8 100644 --- a/packages/fx-core/src/component/driver/teamsApp/createAppPackage.ts +++ b/packages/fx-core/src/component/driver/teamsApp/createAppPackage.ts @@ -175,8 +175,13 @@ export class CreateAppPackageDriver implements StepDriver { if (relativePath.startsWith("..")) { return err(new InvalidFileOutsideOfTheDirectotryError(fileName)); } - const dir = path.dirname(file); - zip.addLocalFile(fileName, dir === "." ? "" : dir); + const resolvedLocFileRes = await manifestUtils.resolveLocFile(fileName); + if (resolvedLocFileRes.isErr()) { + return err(resolvedLocFileRes.error); + } + if (resolvedLocFileRes.value) { + zip.addFile(relativePath, Buffer.from(resolvedLocFileRes.value)); + } } } if (manifest.localizationInfo && manifest.localizationInfo.defaultLanguageFile) { @@ -186,8 +191,14 @@ export class CreateAppPackageDriver implements StepDriver { if (relativePath.startsWith("..")) { return err(new InvalidFileOutsideOfTheDirectotryError(fileName)); } - const dir = path.dirname(file); - zip.addLocalFile(fileName, dir === "." ? "" : dir); + + const resolvedLocFileRes = await manifestUtils.resolveLocFile(fileName); + if (resolvedLocFileRes.isErr()) { + return err(resolvedLocFileRes.error); + } + if (resolvedLocFileRes.value) { + zip.addFile(relativePath, Buffer.from(resolvedLocFileRes.value)); + } } // API ME, API specification and Adaptive card templates diff --git a/packages/fx-core/src/component/driver/teamsApp/utils/ManifestUtils.ts b/packages/fx-core/src/component/driver/teamsApp/utils/ManifestUtils.ts index c6f9d11fc5..14eb5a6abd 100644 --- a/packages/fx-core/src/component/driver/teamsApp/utils/ManifestUtils.ts +++ b/packages/fx-core/src/component/driver/teamsApp/utils/ManifestUtils.ts @@ -22,11 +22,16 @@ import { v4 } from "uuid"; import isUUID from "validator/lib/isUUID"; import { ErrorContextMW } from "../../../../common/globalVars"; import { getCapabilities as checkManifestCapabilities } from "../../../../common/projectTypeChecker"; -import { FileNotFoundError, JSONSyntaxError, ReadFileError } from "../../../../error/common"; +import { + FileNotFoundError, + JSONSyntaxError, + MissingEnvironmentVariablesError, + ReadFileError, +} from "../../../../error/common"; import { CapabilityOptions } from "../../../../question/constants"; import { BotScenario } from "../../../constants"; import { convertManifestTemplateToV2, convertManifestTemplateToV3 } from "../../../migrate"; -import { expandEnvironmentVariable } from "../../../utils/common"; +import { expandEnvironmentVariable, getEnvironmentVariables } from "../../../utils/common"; import { ManifestType } from "../../../utils/envFunctionUtils"; import { DriverContext } from "../../interface/commonArgs"; import { @@ -432,6 +437,27 @@ export class ManifestUtils { } return ok(undefined); } + + async resolveLocFile(locFilePath: string): Promise> { + if (!(await fs.pathExists(locFilePath))) { + return err(new FileNotFoundError("teamsApp", locFilePath)); + } + + const locFileString = await fs.readFile(locFilePath, "utf8"); + const resolvedLocFileString = expandEnvironmentVariable(locFileString); + const unresolvedEnvVariables = getEnvironmentVariables(resolvedLocFileString); + if (unresolvedEnvVariables && unresolvedEnvVariables.length > 0) { + return err( + new MissingEnvironmentVariablesError( + "teamsApp", + unresolvedEnvVariables.join(","), + locFilePath + ) + ); + } + + return ok(resolvedLocFileString); + } } export const manifestUtils = new ManifestUtils(); diff --git a/packages/fx-core/src/component/driver/teamsApp/validate.ts b/packages/fx-core/src/component/driver/teamsApp/validate.ts index a87deb2c8a..5e4dd7279b 100644 --- a/packages/fx-core/src/component/driver/teamsApp/validate.ts +++ b/packages/fx-core/src/component/driver/teamsApp/validate.ts @@ -431,11 +431,11 @@ export class ValidateManifestDriver implements StepDriver { ); const localizationFilePath = getAbsolutePath(filePath, localizationFileDir); - const manifestRes = await manifestUtils._readAppManifest(localizationFilePath); - if (manifestRes.isErr()) { - return err(manifestRes.error); + const resolvedLocFileRes = await manifestUtils.resolveLocFile(localizationFilePath); + if (resolvedLocFileRes.isErr()) { + return err(resolvedLocFileRes.error); } - const localizationFile = manifestRes.value; + const localizationFile = JSON.parse(resolvedLocFileRes.value) as TeamsAppManifest; try { const schema = await ManifestUtil.fetchSchema(localizationFile); // the current localization schema has invalid regex sytax, we need to manually fix the properties temporarily From 778510b1feb49ed05638938aa036b735b309491a Mon Sep 17 00:00:00 2001 From: Huihui Wu Date: Wed, 11 Dec 2024 14:32:31 +0800 Subject: [PATCH 2/4] test: add ut --- .../driver/teamsApp/createAppPackage.test.ts | 48 +++++++++++++++++- .../driver/teamsApp/manifestUtils.test.ts | 50 +++++++++++++++++++ .../driver/teamsApp/validate.test.ts | 16 ++++-- 3 files changed, 108 insertions(+), 6 deletions(-) diff --git a/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts b/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts index f2e4b20b51..0a9a3d5405 100644 --- a/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts +++ b/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts @@ -12,7 +12,14 @@ import { CreateAppPackageArgs } from "../../../../src/component/driver/teamsApp/ import { MockedLogProvider, MockedUserInteraction } from "../../../plugins/solution/util"; import { FileNotFoundError, JSONSyntaxError } from "../../../../src/error/common"; import { manifestUtils } from "../../../../src/component/driver/teamsApp/utils/ManifestUtils"; -import { ok, Platform, PluginManifestSchema, TeamsAppManifest } from "@microsoft/teamsfx-api"; +import { + err, + ok, + Platform, + PluginManifestSchema, + SystemError, + TeamsAppManifest, +} from "@microsoft/teamsfx-api"; import AdmZip from "adm-zip"; import { InvalidFileOutsideOfTheDirectotryError } from "../../../../src/error/teamsApp"; import { MockedM365Provider } from "../../../core/utils"; @@ -1469,6 +1476,44 @@ describe("teamsApp/createAppPackage", async () => { } }); + it("resolve localization file error", async () => { + const args: CreateAppPackageArgs = { + manifestPath: + "./tests/plugins/resource/appstudio/resources-multi-env/templates/appPackage/v3.manifest.template.json", + outputZipPath: + "./tests/plugins/resource/appstudio/resources-multi-env/build/appPackage/appPackage.dev.zip", + outputFolder: "./tests/plugins/resource/appstudio/resources-multi-env/build/appPackage", + }; + + const manifest = new TeamsAppManifest(); + manifest.localizationInfo = { + defaultLanguageTag: "en", + additionalLanguages: [ + { + languageTag: "de", + file: "migrate.manifest.json", + }, + ], + defaultLanguageFile: "de.json", + }; + manifest.icons = { + color: "resources/color.png", + outline: "resources/outline.png", + }; + sinon.stub(manifestUtils, "getManifestV3").resolves(ok(manifest)); + sinon.stub(fs, "pathExists").resolves(true); + sinon.stub(fs, "chmod").callsFake(async () => {}); + sinon.stub(fs, "writeFile").callsFake(async () => {}); + sinon + .stub(manifestUtils, "resolveLocFile") + .resolves(err(new FileNotFoundError("teamsapp", "faked_loc_path"))); + + const result = (await teamsAppDriver.execute(args, mockedDriverContext)).result; + if (result.isErr()) { + chai.assert.isTrue(result.error instanceof FileNotFoundError); + } + }); + it("relative path error 2", async () => { const args: CreateAppPackageArgs = { manifestPath: @@ -1546,6 +1591,7 @@ describe("teamsApp/createAppPackage", async () => { sinon.stub(fs, "chmod").callsFake(async () => {}); const writeFileStub = sinon.stub(fs, "writeFile").callsFake(async () => {}); + sinon.stub(manifestUtils, "resolveLocFile").resolves(ok("{}")); const result = (await teamsAppDriver.execute(args, mockedDriverContext)).result; chai.assert(result.isOk()); diff --git a/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts b/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts index ca26f7feb0..43402bdc41 100644 --- a/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts +++ b/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts @@ -520,3 +520,53 @@ describe("trimManifestShortName", () => { assert.isTrue(writeFileStub.notCalled); }); }); + +describe("resolveLocFile", () => { + const sandbox = sinon.createSandbox(); + afterEach(() => { + sandbox.restore(); + }); + + it("returns error when loc file doesn't exist", async () => { + sandbox.stub(fs, "pathExists").resolves(false); + + const locFile = await manifestUtils.resolveLocFile("loc_file_path"); + + assert.isTrue(locFile.isErr()); + if (locFile.isErr()) { + assert.equal(locFile.error.name, "FileNotFoundError"); + } + }); + + it("returns error when there're unresolved env variables", async () => { + sandbox.stub(fs, "pathExists").resolves(true); + const fakedLocManifest = new TeamsAppManifest(); + fakedLocManifest.name.short = "shortname ${{APP_NAME_SUFFIX}}"; + sandbox.stub(fs, "readFile").resolves(JSON.stringify(fakedLocManifest) as any); + + const locFile = await manifestUtils.resolveLocFile("loc_file_path"); + + assert.isTrue(locFile.isErr()); + if (locFile.isErr()) { + assert.equal(locFile.error.name, "MissingEnvironmentVariablesError"); + } + }); + + it("happy pass", async () => { + sandbox.stub(fs, "pathExists").resolves(true); + const fakedLocManifest = new TeamsAppManifest(); + fakedLocManifest.name.short = "shortname ${{APP_NAME_SUFFIX}}"; + process.env.APP_NAME_SUFFIX = "- hello world"; + sandbox.stub(fs, "readFile").resolves(JSON.stringify(fakedLocManifest) as any); + + const locFile = await manifestUtils.resolveLocFile("loc_file_path"); + + assert.isTrue(locFile.isOk()); + if (locFile.isOk()) { + assert.equal( + (JSON.parse(locFile.value) as TeamsAppManifest).name.short, + "shortname - hello world" + ); + } + }); +}); diff --git a/packages/fx-core/tests/component/driver/teamsApp/validate.test.ts b/packages/fx-core/tests/component/driver/teamsApp/validate.test.ts index 01dc884bff..504315ea97 100644 --- a/packages/fx-core/tests/component/driver/teamsApp/validate.test.ts +++ b/packages/fx-core/tests/component/driver/teamsApp/validate.test.ts @@ -275,7 +275,7 @@ describe("teamsApp/validateManifest", async () => { const manifest = { localizationInfo: { additionalLanguages: [{ file: "filePath" }] } } as any; sinon - .stub(manifestUtils, "_readAppManifest") + .stub(manifestUtils, "resolveLocFile") .resolves(err(new SystemError("error", "error", "", ""))); const result = await teamsAppDriver.validateLocalizatoinFiles( @@ -297,7 +297,9 @@ describe("teamsApp/validateManifest", async () => { "https://developer.microsoft.com/en-us/json-schemas/teams/v1.16/MicrosoftTeams.Localization.schema.json", }; - sinon.stub(manifestUtils, "_readAppManifest").resolves(ok(fakeLocalizationFile as any)); + sinon + .stub(manifestUtils, "resolveLocFile") + .resolves(ok(JSON.stringify(fakeLocalizationFile))); sinon.stub(ManifestUtil, "validateManifestAgainstSchema").resolves(["Validation error"]); const result = await teamsAppDriver.validateLocalizatoinFiles( @@ -365,7 +367,9 @@ describe("teamsApp/validateManifest", async () => { const manifest = { localizationInfo: { additionalLanguages: [{ file: "filePath" }] } } as any; const fakeLocalizationFile = {}; - sinon.stub(manifestUtils, "_readAppManifest").resolves(ok(fakeLocalizationFile as any)); + sinon + .stub(manifestUtils, "resolveLocFile") + .resolves(ok(JSON.stringify(fakeLocalizationFile))); sinon .stub(ManifestUtil, "validateManifestAgainstSchema") .throws(new Error("validation exception")); @@ -385,7 +389,7 @@ describe("teamsApp/validateManifest", async () => { const args: ValidateManifestArgs = { manifestPath: "fakepath" }; const manifest = { localizationInfo: { additionalLanguages: [{ file: "filePath" }] } } as any; sinon.stub(ManifestUtil, "fetchSchema").resolves({} as any); - sinon.stub(manifestUtils, "_readAppManifest").resolves(ok({} as any)); + sinon.stub(manifestUtils, "resolveLocFile").resolves(ok("{}")); sinon.stub(ManifestUtil, "validateManifestAgainstSchema").resolves([] as any); const result = await teamsAppDriver.validateLocalizatoinFiles( args, @@ -409,7 +413,9 @@ describe("teamsApp/validateManifest", async () => { "activities.activityTypes[0].description": "aa", }; - sinon.stub(manifestUtils, "_readAppManifest").resolves(ok(fakeLocalizationFile as any)); + sinon + .stub(manifestUtils, "resolveLocFile") + .resolves(ok(JSON.stringify(fakeLocalizationFile))); const result = await teamsAppDriver.validateLocalizatoinFiles( args, mockedDriverContext, From d737e542a0af484e1eb1550a62e52e4d24196999 Mon Sep 17 00:00:00 2001 From: Huihui Wu Date: Wed, 11 Dec 2024 15:12:24 +0800 Subject: [PATCH 3/4] test: fix ut --- .../component/driver/teamsApp/manifestUtils.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts b/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts index 43402bdc41..1242d9e1e2 100644 --- a/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts +++ b/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts @@ -27,6 +27,7 @@ import { } from "../../../../src/component/driver/teamsApp/constants"; import { AppStudioError } from "../../../../src/component/driver/teamsApp/errors"; import { FileNotFoundError, JSONSyntaxError, ReadFileError } from "../../../../src/error"; +import mockedEnv, { RestoreFn } from "mocked-env"; const latestManifestVersion = "1.17"; const oldManifestVersion = "1.16"; @@ -523,7 +524,12 @@ describe("trimManifestShortName", () => { describe("resolveLocFile", () => { const sandbox = sinon.createSandbox(); + let mockedEnvRestore: RestoreFn; + afterEach(() => { + if (mockedEnvRestore) { + mockedEnvRestore(); + } sandbox.restore(); }); @@ -556,7 +562,9 @@ describe("resolveLocFile", () => { sandbox.stub(fs, "pathExists").resolves(true); const fakedLocManifest = new TeamsAppManifest(); fakedLocManifest.name.short = "shortname ${{APP_NAME_SUFFIX}}"; - process.env.APP_NAME_SUFFIX = "- hello world"; + mockedEnvRestore = mockedEnv({ + ["APP_NAME_SUFFIX"]: "- hello world", + }); sandbox.stub(fs, "readFile").resolves(JSON.stringify(fakedLocManifest) as any); const locFile = await manifestUtils.resolveLocFile("loc_file_path"); From ff5cd9b45da7e86d7d2f3b90fc3ffa5b261e2af9 Mon Sep 17 00:00:00 2001 From: Huihui Wu Date: Wed, 11 Dec 2024 15:47:12 +0800 Subject: [PATCH 4/4] test: add ut --- .../driver/teamsApp/createAppPackage.test.ts | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts b/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts index 0a9a3d5405..24c336f4bc 100644 --- a/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts +++ b/packages/fx-core/tests/component/driver/teamsApp/createAppPackage.test.ts @@ -1476,7 +1476,7 @@ describe("teamsApp/createAppPackage", async () => { } }); - it("resolve localization file error", async () => { + it("resolve additional localization file error", async () => { const args: CreateAppPackageArgs = { manifestPath: "./tests/plugins/resource/appstudio/resources-multi-env/templates/appPackage/v3.manifest.template.json", @@ -1514,6 +1514,48 @@ describe("teamsApp/createAppPackage", async () => { } }); + it("resolve default localization file error", async () => { + const args: CreateAppPackageArgs = { + manifestPath: + "./tests/plugins/resource/appstudio/resources-multi-env/templates/appPackage/v3.manifest.template.json", + outputZipPath: + "./tests/plugins/resource/appstudio/resources-multi-env/build/appPackage/appPackage.dev.zip", + outputFolder: "./tests/plugins/resource/appstudio/resources-multi-env/build/appPackage", + }; + + const manifest = new TeamsAppManifest(); + manifest.localizationInfo = { + defaultLanguageTag: "en", + additionalLanguages: [ + { + languageTag: "de", + file: "migrate.manifest.json", + }, + ], + defaultLanguageFile: "de.json", + }; + manifest.icons = { + color: "resources/color.png", + outline: "resources/outline.png", + }; + sinon.stub(manifestUtils, "getManifestV3").resolves(ok(manifest)); + sinon.stub(fs, "pathExists").resolves(true); + sinon.stub(fs, "chmod").callsFake(async () => {}); + sinon.stub(fs, "writeFile").callsFake(async () => {}); + sinon.stub(manifestUtils, "resolveLocFile").callsFake(async (path) => { + if (path.includes("migrate.manifest.json")) { + return ok("{}"); + } else { + return err(new FileNotFoundError("teamsapp", "faked_loc_path")); + } + }); + + const result = (await teamsAppDriver.execute(args, mockedDriverContext)).result; + if (result.isErr()) { + chai.assert.isTrue(result.error instanceof FileNotFoundError); + } + }); + it("relative path error 2", async () => { const args: CreateAppPackageArgs = { manifestPath: