-
Notifications
You must be signed in to change notification settings - Fork 4
refactor: refactored strategies to use the template design pattern #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
088300a
8a03170
d252cc5
f14150e
378eaa6
88052bc
bf2bdd6
e6713ae
f65c1cc
56e217b
7355ee1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { Strategy } from "unleash-client"; | ||
| import { AppStudioMultiContext } from "../appstudio_context"; | ||
| import { log } from "../logger"; | ||
| import { AppStudioStrategy } from "./appStudioStrategies"; | ||
|
|
||
| // The name of the str in the server | ||
| const APP_STUDIO_MULTI = "AppStudioMulti"; | ||
|
|
||
| export interface AppStudioMultiParameters { | ||
| environments: string; // apps | ||
| infrastructures: string; //iaass | ||
| landscapes: string; // regions | ||
| subaccounts: string; | ||
| users: string; | ||
| wss: string; | ||
| tenantids: string; | ||
| } | ||
|
|
||
| export class AppStudioMultiStrategy extends Strategy { | ||
| private strategies: AppStudioStrategy[] = []; | ||
|
|
||
| constructor() { | ||
| super(APP_STUDIO_MULTI); // for testing the name is: StrategyMulti | ||
| } | ||
|
|
||
| public register(appStudioStrategies: AppStudioStrategy[]): void { | ||
| appStudioStrategies.forEach((appStudioStrategy) => { | ||
| const strategyName = appStudioStrategy.getName(); | ||
| const isExist = this.strategies.some((strategy) => strategy.getName() === strategyName); | ||
| if (isExist) { | ||
| throw new Error(`Strategy ${strategyName} already registered`); | ||
| } | ||
|
|
||
| log(`Added strategy: ${strategyName}`); | ||
| this.strategies.push(appStudioStrategy); | ||
| }); | ||
| } | ||
|
|
||
| // Check only the strategy parameters. the default strategy is calculated at the server side | ||
| /** Template method */ | ||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| // Check if the user has any restrictions defined in the strategy (defined in the FT server) | ||
| const isNotDefined = this.strategies.every((strategy) => !strategy.isDefined(parameters)); | ||
| if (isNotDefined) { | ||
| // No restriction from the strategy side | ||
| return true; | ||
| } | ||
|
|
||
| // context.currentEnvironment - The Environment that the extension is running on (calculated in this client) | ||
| // parameters.environments // The Environments that the extension is allowed to be enabled on (list is defined on the server) | ||
| // Check if strategy meet one of the terms | ||
| const isStrategyMet = this.strategies.some((strategy) => strategy.isEnabled(parameters, context)); | ||
| if (isStrategyMet) { | ||
| return true; | ||
| } | ||
|
|
||
| // no matches -> send "disable the feature" to the FT server | ||
| return false; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { AppStudioMultiContext } from "../appstudio_context"; | ||
| import { AppStudioMultiParameters, AppStudioMultiStrategy } from "./appStudioMultiStrategy"; | ||
| import { EnvironmentsStrategy, InfrastructuresStrategy, LandscapesStrategy, SubAccountsStrategy, TenantIdsStrategy, UsersStrategy, WssStrategy } from "./strategies"; | ||
|
|
||
| export interface AppStudioStrategy { | ||
| getName(): string; | ||
|
|
||
| // Check if the user has any restrictions defined in the strategy (defined in the FT server) | ||
| isDefined(parameters: AppStudioMultiParameters): boolean; | ||
|
|
||
| // context.currentEnvironment - The Environment that the extension is running on (calculated in this client) | ||
| // parameters.environments // The Environments that the extension is allowed to be enabled on (list is defined on the server) | ||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean; | ||
| } | ||
|
|
||
| export function registerStrategies(appStudioMultiStrategy: AppStudioMultiStrategy): void { | ||
| appStudioMultiStrategy.register([new EnvironmentsStrategy(), new InfrastructuresStrategy(), new LandscapesStrategy(), new SubAccountsStrategy(), new UsersStrategy(), new WssStrategy(), new TenantIdsStrategy()]); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { AppStudioMultiContext } from "../appstudio_context"; | ||
| import { AppStudioStrategy } from "./appStudioStrategies"; | ||
| import { AppStudioMultiParameters } from "./appStudioMultiStrategy"; | ||
|
|
||
| export class EnvironmentsStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `Environments_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.environments; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentEnvironment && parameters.environments && parameters.environments.indexOf(context.currentEnvironment) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export class InfrastructuresStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `Infrastructures_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.infrastructures; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentInfrastructure && parameters.infrastructures && parameters.infrastructures.indexOf(context.currentInfrastructure) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export class LandscapesStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `Landscapes_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.landscapes; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentLandscape && parameters.landscapes && parameters.landscapes.indexOf(context.currentLandscape) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export class SubAccountsStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `SubAccounts_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.subaccounts; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentCfSubAccount && parameters.subaccounts && parameters.subaccounts.indexOf(context.currentCfSubAccount) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export class UsersStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `Users_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.users; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentUser && parameters.users && parameters.users.indexOf(context.currentUser) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export class WssStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `Wss_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.wss; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentWs && parameters.wss && parameters.wss.indexOf(context.currentWs) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export class TenantIdsStrategy implements AppStudioStrategy { | ||
| getName(): string { | ||
| return `TenantIds_Strategy`; | ||
| } | ||
|
|
||
| isDefined(parameters: AppStudioMultiParameters): boolean { | ||
| return !!parameters.tenantids; | ||
| } | ||
|
|
||
| isEnabled(parameters: AppStudioMultiParameters, context: AppStudioMultiContext): boolean { | ||
| if (!!context.currentTenantId && parameters.tenantids && parameters.tenantids.indexOf(context.currentTenantId) !== -1) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| import { Unleash } from "unleash-client"; | ||
| import * as ServerArgs from "./server_arguments"; | ||
| import { initializeUnleashClient } from "./unleash_client_wrapper"; | ||
| import { AppStudioMultiStrategy } from "./appstudio_strategy"; | ||
| import { AppStudioMultiStrategy } from "./strategy/appStudioMultiStrategy"; | ||
| import { registerStrategies } from "./strategy/appStudioStrategies"; | ||
|
|
||
| // Map of Unleash clients. | ||
| // map key = extensionName | ||
|
|
@@ -13,7 +14,9 @@ async function createNewUnleashClient(extensionName: string, unleashClientMap: M | |
| const serverArgs = ServerArgs.getServerArgs(); | ||
|
|
||
| //init client | ||
| const client = await initializeUnleashClient(extensionName, serverArgs, [new AppStudioMultiStrategy()]); | ||
| const appStudioMultiStrategy = new AppStudioMultiStrategy(); | ||
| const client = await initializeUnleashClient(extensionName, serverArgs, [appStudioMultiStrategy]); | ||
| registerStrategies(appStudioMultiStrategy); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer instance method: appStudioMultiStrategy.registerStrategies()
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| //add the client to the map | ||
| unleashClientMap.set(extensionName, client); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method doesnt feel in the right place.
You define abstract class here for strategy and then comes this specific method which use instances of classes which implement this abstract class.
I would do it as instance method of AppStudioMultiStrategy class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppStudioMultiStrategyis a generic class that should not know the concrete implementation.unleash_client_manager.tsmanageAppStudioMultiStrategy, initialize and register the strategies