Skip to content

Commit

Permalink
refactor: unify feature flag access (#11817)
Browse files Browse the repository at this point in the history
* refactor: unify feature flag access

* refactor: merge

* refactor: ut

* fix: ut

* fix: ut

* fix: ut

* fix: ut

* fix: ut
  • Loading branch information
jayzhang authored Jun 17, 2024
1 parent 1e072b3 commit f31cd0e
Show file tree
Hide file tree
Showing 18 changed files with 121 additions and 208 deletions.
36 changes: 0 additions & 36 deletions packages/fx-core/src/common/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,6 @@ export function isFeatureFlagEnabled(featureFlagName: string, defaultValue = fal
}
}

export function isCLIDotNetEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.CLIDotNet);
}

export function enableTestToolByDefault(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.TestTool);
}

export function enableMETestToolByDefault(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.METestTool);
}

export function isNewGeneratorEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.NewGenerator);
}

export function isOfficeJSONAddinEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.OfficeAddin);
}

export function isTdpTemplateCliTestEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.TdpTemplateCliTest);
}

export function isAsyncAppValidationEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.AsyncAppValidation);
}

export function isNewProjectTypeEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.NewProjectType);
}

export function isChatParticipantEnabled(): boolean {
return featureFlagManager.getBooleanValue(FeatureFlags.ChatParticipant);
}

///////////////////////////////////////////////////////////////////////////////
// Notes for Office Addin Feature flags:
// Case 1: TEAMSFX_OFFICE_ADDIN = false, TEAMSFX_OFFICE_XML_ADDIN = false
Expand Down
4 changes: 2 additions & 2 deletions packages/fx-core/src/component/coordinator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import * as path from "path";
import * as uuid from "uuid";
import * as xml2js from "xml2js";
import { AppStudioScopes, getResourceGroupInPortal } from "../../common/constants";
import { FeatureFlags, featureFlagManager, isNewGeneratorEnabled } from "../../common/featureFlags";
import { FeatureFlags, featureFlagManager } from "../../common/featureFlags";
import { ErrorContextMW, globalVars } from "../../common/globalVars";
import { getLocalizedString } from "../../common/localizeUtils";
import { convertToAlphanumericOnly } from "../../common/stringUtils";
Expand Down Expand Up @@ -174,7 +174,7 @@ class Coordinator {
});
}

if (isNewGeneratorEnabled()) {
if (featureFlagManager.getBooleanValue(FeatureFlags.NewGenerator)) {
// refactored generator
const generator = Generators.find((g) => g.activate(context, inputs));
if (!generator) {
Expand Down
18 changes: 10 additions & 8 deletions packages/fx-core/src/component/generator/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ import {
renderTemplateFileData,
renderTemplateFileName,
} from "./utils";
import {
enableMETestToolByDefault,
enableTestToolByDefault,
isNewProjectTypeEnabled,
} from "../../common/featureFlags";
import { featureFlagManager, FeatureFlags } from "../../common/featureFlags";
import { Utils } from "@microsoft/m365-spec-parser";
import { convertToAlphanumericOnly } from "../../common/stringUtils";

Expand Down Expand Up @@ -80,15 +76,21 @@ export class Generator {
ApiSpecPath: authData?.openapiSpecPath ?? "",
ApiKey: authData?.authType === "apiKey" ? "true" : "",
OAuth: authData?.authType === "oauth2" ? "true" : "",
enableTestToolByDefault: enableTestToolByDefault() ? "true" : "",
enableMETestToolByDefault: enableMETestToolByDefault() ? "true" : "",
enableTestToolByDefault: featureFlagManager.getBooleanValue(FeatureFlags.TestTool)
? "true"
: "",
enableMETestToolByDefault: featureFlagManager.getBooleanValue(FeatureFlags.METestTool)
? "true"
: "",
useOpenAI: llmServiceData?.llmService === "llm-service-openai" ? "true" : "",
useAzureOpenAI: llmServiceData?.llmService === "llm-service-azure-openai" ? "true" : "",
openAIKey: llmServiceData?.openAIKey ?? "",
azureOpenAIKey: llmServiceData?.azureOpenAIKey ?? "",
azureOpenAIEndpoint: llmServiceData?.azureOpenAIEndpoint ?? "",
azureOpenAIDeploymentName: llmServiceData?.azureOpenAIDeploymentName ?? "",
isNewProjectTypeEnabled: isNewProjectTypeEnabled() ? "true" : "",
isNewProjectTypeEnabled: featureFlagManager.getBooleanValue(FeatureFlags.NewProjectType)
? "true"
: "",
NewProjectTypeName: process.env.TEAMSFX_NEW_PROJECT_TYPE_NAME ?? "TeamsApp",
NewProjectTypeExt: process.env.TEAMSFX_NEW_PROJECT_TYPE_EXTENSION ?? "ttkproj",
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
import { Inputs } from "@microsoft/teamsfx-api";
import {
enableMETestToolByDefault,
enableTestToolByDefault,
isNewProjectTypeEnabled,
} from "../../../common/featureFlags";
import { featureFlagManager, FeatureFlags } from "../../../common/featureFlags";
import { convertToAlphanumericOnly } from "../../../common/stringUtils";
import { QuestionNames } from "../../../question/constants";

Expand All @@ -29,15 +25,21 @@ export function getTemplateReplaceMap(inputs: Inputs): { [key: string]: string }
PlaceProjectFileInSolutionDir: placeProjectFileInSolutionDir ? "true" : "",
SafeProjectName: safeProjectName,
SafeProjectNameLowerCase: safeProjectName.toLocaleLowerCase(),
enableTestToolByDefault: enableTestToolByDefault() ? "true" : "",
enableMETestToolByDefault: enableMETestToolByDefault() ? "true" : "",
enableTestToolByDefault: featureFlagManager.getBooleanValue(FeatureFlags.TestTool)
? "true"
: "",
enableMETestToolByDefault: featureFlagManager.getBooleanValue(FeatureFlags.METestTool)
? "true"
: "",
useOpenAI: llmService === "llm-service-openai" ? "true" : "",
useAzureOpenAI: llmService === "llm-service-azure-openai" ? "true" : "",
openAIKey: openAIKey ?? "",
azureOpenAIKey: azureOpenAIKey ?? "",
azureOpenAIEndpoint: azureOpenAIEndpoint ?? "",
azureOpenAIDeploymentName: azureOpenAIDeploymentName ?? "",
isNewProjectTypeEnabled: isNewProjectTypeEnabled() ? "true" : "",
isNewProjectTypeEnabled: featureFlagManager.getBooleanValue(FeatureFlags.NewProjectType)
? "true"
: "",
NewProjectTypeName: process.env.TEAMSFX_NEW_PROJECT_TYPE_NAME ?? "TeamsApp",
NewProjectTypeExt: process.env.TEAMSFX_NEW_PROJECT_TYPE_EXTENSION ?? "ttkproj",
};
Expand Down
7 changes: 1 addition & 6 deletions packages/fx-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,7 @@ export {
getAllowedAppMaps,
} from "./common/constants";
export { Correlator } from "./common/correlator";
export {
FeatureFlags,
featureFlagManager,
isChatParticipantEnabled,
isFeatureFlagEnabled,
} from "./common/featureFlags";
export { FeatureFlags, featureFlagManager, isFeatureFlagEnabled } from "./common/featureFlags";
export { globalStateGet, globalStateUpdate } from "./common/globalState";
export { getDefaultString, getLocalizedString } from "./common/localizeUtils";
export * from "./common/permissionInterface";
Expand Down
16 changes: 5 additions & 11 deletions packages/fx-core/src/question/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@
// Licensed under the MIT license.

import { Inputs, OptionItem, Platform } from "@microsoft/teamsfx-api";
import {
FeatureFlags,
featureFlagManager,
isCLIDotNetEnabled,
isChatParticipantEnabled,
isTdpTemplateCliTestEnabled,
} from "../common/featureFlags";
import { FeatureFlags, featureFlagManager } from "../common/featureFlags";
import { getLocalizedString } from "../common/localizeUtils";
import { OfficeAddinProjectConfig } from "../component/generator/officeXMLAddin/projectConfig";

Expand Down Expand Up @@ -137,7 +131,7 @@ export class RuntimeOptions {

export function getRuntime(inputs: Inputs): string {
let runtime = RuntimeOptions.NodeJS().id;
if (isCLIDotNetEnabled()) {
if (featureFlagManager.getBooleanValue(FeatureFlags.CLIDotNet)) {
runtime = inputs[QuestionNames.Runtime] || runtime;
} else {
if (inputs?.platform === Platform.VS) {
Expand Down Expand Up @@ -169,7 +163,7 @@ export class ScratchOptions {

export class ProjectTypeOptions {
static getCreateGroupName(): string | undefined {
return isChatParticipantEnabled()
return featureFlagManager.getBooleanValue(FeatureFlags.ChatParticipant)
? getLocalizedString("core.createProjectQuestion.projectType.createGroup.title")
: undefined;
}
Expand Down Expand Up @@ -518,7 +512,7 @@ export class CapabilityOptions {
CapabilityOptions.tab(),
...CapabilityOptions.collectMECaps(),
];
if (isTdpTemplateCliTestEnabled()) {
if (featureFlagManager.getBooleanValue(FeatureFlags.TdpTemplateCliTest)) {
capabilities.push(CapabilityOptions.me());
}

Expand Down Expand Up @@ -669,7 +663,7 @@ export class CapabilityOptions {
capabilityOptions.push(...CapabilityOptions.customizeGptOptions());
}
capabilityOptions.push(...CapabilityOptions.customCopilots());
if (isTdpTemplateCliTestEnabled()) {
if (featureFlagManager.getBooleanValue(FeatureFlags.TdpTemplateCliTest)) {
// test templates that are used by TDP integration only
capabilityOptions.push(...CapabilityOptions.tdpIntegrationCapabilities());
}
Expand Down
17 changes: 6 additions & 11 deletions packages/fx-core/src/question/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ import * as os from "os";
import * as path from "path";
import { ConstantString } from "../common/constants";
import { Correlator } from "../common/correlator";
import {
FeatureFlags,
featureFlagManager,
isCLIDotNetEnabled,
isChatParticipantEnabled,
isOfficeJSONAddinEnabled,
} from "../common/featureFlags";
import { FeatureFlags, featureFlagManager } from "../common/featureFlags";
import { createContext } from "../common/globalVars";
import { getLocalizedString } from "../common/localizeUtils";
import { sampleProvider } from "../common/samples";
Expand Down Expand Up @@ -119,7 +113,7 @@ export function projectTypeQuestion(): SingleSelectQuestion {
//only for @office agent, officeXMLAddin are supported
staticOptions.push(ProjectTypeOptions.officeXMLAddin(inputs.platform));
} else {
if (isOfficeJSONAddinEnabled()) {
if (featureFlagManager.getBooleanValue(FeatureFlags.OfficeAddin)) {
staticOptions.push(ProjectTypeOptions.officeAddin(inputs.platform));
} else {
staticOptions.push(ProjectTypeOptions.outlookAddin(inputs.platform));
Expand All @@ -129,7 +123,7 @@ export function projectTypeQuestion(): SingleSelectQuestion {

if (
inputs.platform === Platform.VSCode &&
isChatParticipantEnabled() &&
featureFlagManager.getBooleanValue(FeatureFlags.ChatParticipant) &&
!inputs.teamsAppFromTdp
) {
staticOptions.push(ProjectTypeOptions.startWithGithubCopilot());
Expand Down Expand Up @@ -1524,7 +1518,8 @@ export function createProjectQuestionNode(): IQTreeNode {
children: [
{
condition: (inputs: Inputs) =>
isCLIDotNetEnabled() && CLIPlatforms.includes(inputs.platform),
featureFlagManager.getBooleanValue(FeatureFlags.CLIDotNet) &&
CLIPlatforms.includes(inputs.platform),
data: runtimeQuestion(),
},
{
Expand Down Expand Up @@ -1595,7 +1590,7 @@ export function createProjectCliHelpNode(): IQTreeNode {
QuestionNames.ReplaceBotIds,
QuestionNames.Samples,
];
if (!isCLIDotNetEnabled()) {
if (!featureFlagManager.getBooleanValue(FeatureFlags.CLIDotNet)) {
deleteNames.push(QuestionNames.Runtime);
}
trimQuestionTreeForCliHelp(node, deleteNames);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import { err, Inputs, ok, Platform, SystemError, UserError } from "@microsoft/te
import { assert } from "chai";
import fs from "fs-extra";
import { glob } from "glob";
import mockedEnv, { RestoreFn } from "mocked-env";
import * as sinon from "sinon";
import * as FeatureFlags from "../../../src/common/featureFlags";
import { createContext, setTools } from "../../../src/common/globalVars";
import { MetadataV3 } from "../../../src/common/versionMetadata";
import { coordinator } from "../../../src/component/coordinator";
Expand All @@ -25,6 +23,7 @@ import { TemplateNames } from "../../../src/component/generator/templates/templa
import { settingsUtil } from "../../../src/component/utils/settingsUtil";
import { FxCore } from "../../../src/core/FxCore";
import { InputValidationError, MissingRequiredInputError } from "../../../src/error/common";
import { CreateSampleProjectInputs } from "../../../src/question";
import {
ApiAuthOptions,
CapabilityOptions,
Expand All @@ -36,23 +35,23 @@ import {
QuestionNames,
ScratchOptions,
} from "../../../src/question/constants";
import { validationUtils } from "../../../src/ui/validationUtils";
import { MockTools, randomAppName } from "../../core/utils";
import { MockedUserInteraction } from "../../plugins/solution/util";
import { CreateSampleProjectInputs } from "../../../src/question";
import { validationUtils } from "../../../src/ui/validationUtils";
import mockedEnv, { RestoreFn } from "mocked-env";

const V3Version = MetadataV3.projectVersion;

[false].forEach((newGeneratorFlag) => {
describe(`coordinator create with isNewGeneratorEnabled = ${newGeneratorFlag}`, () => {
const mockedEnvRestore: RestoreFn = () => {};
describe(`coordinator create with new generator enabled = ${newGeneratorFlag}`, () => {
let mockedEnvRestore: RestoreFn = () => {};
const sandbox = sinon.createSandbox();
const tools = new MockTools();
let generator: sinon.SinonStub;
setTools(tools);
beforeEach(() => {
sandbox.stub(fs, "ensureDir").resolves();
sandbox.stub(FeatureFlags, "isNewGeneratorEnabled").returns(newGeneratorFlag);
mockedEnvRestore = mockedEnv({ TEAMSFX_NEW_GENERATOR: `${newGeneratorFlag}` });
generator = newGeneratorFlag
? sandbox
.stub(DefaultTemplateGenerator.prototype, <any>"scaffolding")
Expand Down Expand Up @@ -939,15 +938,15 @@ const V3Version = MetadataV3.projectVersion;
});

describe("Office Addin", async () => {
let mockedEnvRestore: RestoreFn = () => {};
const sandbox = sinon.createSandbox();
const tools = new MockTools();
const mockedEnvRestore: RestoreFn = () => {};
tools.ui = new MockedUserInteraction();
setTools(tools);

beforeEach(() => {
sandbox.stub(fs, "ensureDir").resolves();
sandbox.stub(FeatureFlags, "isNewGeneratorEnabled").returns(false);
mockedEnvRestore = mockedEnv({ TEAMSFX_NEW_GENERATOR: "false" });
});

afterEach(() => {
Expand Down Expand Up @@ -1028,15 +1027,15 @@ describe("Office Addin", async () => {
});

describe("Office XML Addin", async () => {
let mockedEnvRestore: RestoreFn = () => {};
const sandbox = sinon.createSandbox();
const tools = new MockTools();
const mockedEnvRestore: RestoreFn = () => {};
tools.ui = new MockedUserInteraction();
setTools(tools);

beforeEach(() => {
sandbox.stub(fs, "ensureDir").resolves();
sandbox.stub(FeatureFlags, "isNewGeneratorEnabled").returns(false);
mockedEnvRestore = mockedEnv({ TEAMSFX_NEW_GENERATOR: "false" });
});

afterEach(() => {
Expand Down Expand Up @@ -1109,15 +1108,15 @@ describe("Office XML Addin", async () => {
});

describe("Office Addin", async () => {
let mockedEnvRestore: RestoreFn = () => {};
const sandbox = sinon.createSandbox();
const tools = new MockTools();
const mockedEnvRestore: RestoreFn = () => {};
tools.ui = new MockedUserInteraction();
setTools(tools);

beforeEach(() => {
sandbox.stub(fs, "ensureDir").resolves();
sandbox.stub(FeatureFlags, "isNewGeneratorEnabled").returns(false);
mockedEnvRestore = mockedEnv({ TEAMSFX_NEW_GENERATOR: "false" });
});

afterEach(() => {
Expand Down Expand Up @@ -1198,18 +1197,20 @@ describe("Office Addin", async () => {
});

describe("Copilot plugin", async () => {
let mockedEnvRestore: RestoreFn = () => {};
const sandbox = sinon.createSandbox();
const tools = new MockTools();
tools.ui = new MockedUserInteraction();
setTools(tools);

beforeEach(() => {
sandbox.stub(fs, "ensureDir").resolves();
sandbox.stub(FeatureFlags, "isNewGeneratorEnabled").returns(false);
mockedEnvRestore = mockedEnv({ TEAMSFX_NEW_GENERATOR: "false" });
});

afterEach(() => {
sandbox.restore();
mockedEnvRestore();
});

it("should scaffold from API spec successfully", async () => {
Expand Down Expand Up @@ -1253,16 +1254,18 @@ describe("Copilot plugin", async () => {
});
});

describe(`coordinator create with isNewGeneratorEnabled = true`, () => {
describe(`coordinator create with new generator enabled = true`, () => {
let mockedEnvRestore: RestoreFn = () => {};
const sandbox = sinon.createSandbox();
const tools = new MockTools();
setTools(tools);
beforeEach(() => {
sandbox.stub(fs, "ensureDir").resolves();
sandbox.stub(FeatureFlags, "isNewGeneratorEnabled").returns(true);
mockedEnvRestore = mockedEnv({ TEAMSFX_NEW_GENERATOR: "true" });
});
afterEach(() => {
sandbox.restore();
mockedEnvRestore();
});

it("should scaffold by OfficeAddinGeneratorNew successfully", async () => {
Expand Down
Loading

0 comments on commit f31cd0e

Please sign in to comment.