From 7acb0939d926ff0437f82e77398492762f9400ca Mon Sep 17 00:00:00 2001 From: Kartik Gupta <88345179+kartikgupta-db@users.noreply.github.com> Date: Wed, 10 Jan 2024 10:42:26 +0100 Subject: [PATCH] Make all models inherit from a common base class (#994) ## Changes ## Tests --- .../rules/mutexSynchronisedDecorator.ts | 6 +- .../bundle/models/BundlePreValidateModel.ts | 110 ++++----- .../src/bundle/models/BundleValidateModel.ts | 102 ++------ .../src/configuration/ConnectionManager.ts | 22 +- .../src/configuration/auth/AuthProvider.ts | 8 +- .../models/BaseModelWithStateCache.ts | 54 +++++ .../src/configuration/models/ConfigModel.ts | 222 +++++++++--------- .../models/OverrideableConfigModel.ts | 55 ++--- .../src/configuration/types.ts | 71 ------ .../src/configuration/ui/AuthTypeComponent.ts | 6 +- .../configuration/ui/BundleTargetComponent.ts | 21 +- .../ui/SyncDestinationComponent.ts | 18 +- .../src/configuration/ui/types.ts | 4 +- .../src/locking/CachedValue.ts | 62 ++++- .../src/vscode-objs/StateStorage.ts | 4 +- 15 files changed, 357 insertions(+), 408 deletions(-) create mode 100644 packages/databricks-vscode/src/configuration/models/BaseModelWithStateCache.ts delete mode 100644 packages/databricks-vscode/src/configuration/types.ts diff --git a/packages/databricks-vscode/eslint-local-rules/rules/mutexSynchronisedDecorator.ts b/packages/databricks-vscode/eslint-local-rules/rules/mutexSynchronisedDecorator.ts index 5984af291..dd6739381 100644 --- a/packages/databricks-vscode/eslint-local-rules/rules/mutexSynchronisedDecorator.ts +++ b/packages/databricks-vscode/eslint-local-rules/rules/mutexSynchronisedDecorator.ts @@ -33,8 +33,10 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ const exists = node.parent.parent.body.some((element) => { return ( - element.type === - AST_NODE_TYPES.PropertyDefinition && + (element.type === + AST_NODE_TYPES.PropertyDefinition || + element.type === + AST_NODE_TYPES.TSAbstractPropertyDefinition) && element.key.type === AST_NODE_TYPES.Identifier && element.key.name === mutexName ); diff --git a/packages/databricks-vscode/src/bundle/models/BundlePreValidateModel.ts b/packages/databricks-vscode/src/bundle/models/BundlePreValidateModel.ts index 02845b511..616f05103 100644 --- a/packages/databricks-vscode/src/bundle/models/BundlePreValidateModel.ts +++ b/packages/databricks-vscode/src/bundle/models/BundlePreValidateModel.ts @@ -1,48 +1,30 @@ -import {Disposable, Uri} from "vscode"; +import {Uri} from "vscode"; import {BundleFileSet, BundleWatcher} from ".."; import {BundleTarget} from "../types"; -import {CachedValue} from "../../locking/CachedValue"; -import { - BundlePreValidateConfig, - isBundlePreValidateConfigKey, -} from "../../configuration/types"; +import {BaseModelWithStateCache} from "../../configuration/models/BaseModelWithStateCache"; +import {UrlUtils} from "../../utils"; +import {onError} from "../../utils/onErrorDecorator"; +import {Mutex} from "../../locking"; + +export type BundlePreValidateState = { + host?: URL; + mode?: "development" | "staging" | "production"; + authParams?: Record; +} & BundleTarget; + /** * Reads and writes bundle configs. This class does not notify when the configs change. * We use the BundleWatcher to notify when the configs change. */ -export class BundlePreValidateModel implements Disposable { - private disposables: Disposable[] = []; - - private readonly stateCache = new CachedValue< - BundlePreValidateConfig | undefined - >(async () => { - if (this.target === undefined) { - return undefined; - } - return this.readState(this.target); - }); - - public readonly onDidChange = this.stateCache.onDidChange; - +export class BundlePreValidateModel extends BaseModelWithStateCache { + protected mutex = new Mutex(); private target: string | undefined; - private readonly readerMapping: Record< - keyof BundlePreValidateConfig, - ( - t?: BundleTarget - ) => Promise< - BundlePreValidateConfig[keyof BundlePreValidateConfig] | undefined - > - > = { - authParams: this.getAuthParams, - mode: this.getMode, - host: this.getHost, - }; - constructor( private readonly bundleFileSet: BundleFileSet, private readonly bunldeFileWatcher: BundleWatcher ) { + super(); this.disposables.push( this.bunldeFileWatcher.onDidChange(async () => { await this.stateCache.refresh(); @@ -50,20 +32,6 @@ export class BundlePreValidateModel implements Disposable { ); } - private async getHost(target?: BundleTarget) { - return target?.workspace?.host; - } - - private async getMode(target?: BundleTarget) { - return target?.mode; - } - - /* eslint-disable @typescript-eslint/no-unused-vars */ - private async getAuthParams(target?: BundleTarget) { - return undefined; - } - /* eslint-enable @typescript-eslint/no-unused-vars */ - get targets() { return this.bundleFileSet.bundleDataCache.value.then( (data) => data?.targets @@ -87,33 +55,41 @@ export class BundlePreValidateModel implements Disposable { await this.stateCache.refresh(); } - private async readState(target: string) { - const configs = {} as any; - const targetObject = (await this.bundleFileSet.bundleDataCache.value) - .targets?.[target]; + protected readStateFromTarget( + target: BundleTarget + ): BundlePreValidateState { + return { + ...target, + host: UrlUtils.normalizeHost(target?.workspace?.host ?? ""), + mode: target?.mode as BundlePreValidateState["mode"], + authParams: undefined, + }; + } - for (const key of Object.keys(this.readerMapping)) { - if (!isBundlePreValidateConfigKey(key)) { - continue; - } - configs[key] = await this.readerMapping[key](targetObject); + @onError({popup: {prefix: "Failed to parse bundle yaml"}}) + @Mutex.synchronise("mutex") + protected async readState() { + if (this.target === undefined) { + return {}; } - return configs as BundlePreValidateConfig; + + const targetObject = (await this.bundleFileSet.bundleDataCache.value) + .targets?.[this.target]; + + return this.readStateFromTarget(targetObject ?? {}); } - public async getFileToWrite( - key: T - ) { + public async getFileToWrite(key: string) { const filesWithTarget: Uri[] = []; const filesWithConfig = ( await this.bundleFileSet.findFile(async (data, file) => { const bundleTarget = data.targets?.[this.target ?? ""]; - if (bundleTarget) { - filesWithTarget.push(file); + if (bundleTarget === undefined) { + return false; } - if ( - (await this.readerMapping[key](bundleTarget)) === undefined - ) { + filesWithTarget.push(file); + + if (this.readStateFromTarget(bundleTarget) === undefined) { return false; } return true; @@ -135,10 +111,6 @@ export class BundlePreValidateModel implements Disposable { return [...filesWithConfig, ...filesWithTarget][0]; } - public async load() { - return await this.stateCache.value; - } - public dispose() { this.disposables.forEach((d) => d.dispose()); } diff --git a/packages/databricks-vscode/src/bundle/models/BundleValidateModel.ts b/packages/databricks-vscode/src/bundle/models/BundleValidateModel.ts index 0b6addaf9..f7898b2b0 100644 --- a/packages/databricks-vscode/src/bundle/models/BundleValidateModel.ts +++ b/packages/databricks-vscode/src/bundle/models/BundleValidateModel.ts @@ -1,78 +1,37 @@ -import {Disposable, Uri, EventEmitter} from "vscode"; +import {Uri} from "vscode"; import {BundleWatcher} from "../BundleWatcher"; import {AuthProvider} from "../../configuration/auth/AuthProvider"; import {Mutex} from "../../locking"; import {CliWrapper} from "../../cli/CliWrapper"; import {BundleTarget} from "../types"; -import {CachedValue} from "../../locking/CachedValue"; import {onError} from "../../utils/onErrorDecorator"; import lodash from "lodash"; import {workspaceConfigs} from "../../vscode-objs/WorkspaceConfigs"; -import {BundleValidateConfig} from "../../configuration/types"; +import {BaseModelWithStateCache} from "../../configuration/models/BaseModelWithStateCache"; -type BundleValidateState = BundleValidateConfig & BundleTarget; - -export class BundleValidateModel implements Disposable { - private disposables: Disposable[] = []; - private mutex = new Mutex(); +export type BundleValidateState = { + clusterId?: string; + remoteRootPath?: string; +} & BundleTarget; +export class BundleValidateModel extends BaseModelWithStateCache { private target: string | undefined; private authProvider: AuthProvider | undefined; - - private readonly stateCache = new CachedValue< - BundleValidateState | undefined - >(this.readState.bind(this)); - - private readonly onDidChangeKeyEmitters = new Map< - keyof BundleValidateState, - EventEmitter - >(); - - onDidChangeKey(key: keyof BundleValidateState) { - if (!this.onDidChangeKeyEmitters.has(key)) { - this.onDidChangeKeyEmitters.set(key, new EventEmitter()); - } - return this.onDidChangeKeyEmitters.get(key)!.event; - } - - public onDidChange = this.stateCache.onDidChange; + protected mutex = new Mutex(); constructor( private readonly bundleWatcher: BundleWatcher, private readonly cli: CliWrapper, private readonly workspaceFolder: Uri ) { + super(); this.disposables.push( this.bundleWatcher.onDidChange(async () => { await this.stateCache.refresh(); - }), - // Emit an event for each key that changes - this.stateCache.onDidChange(async ({oldValue, newValue}) => { - for (const key of Object.keys({ - ...oldValue, - ...newValue, - }) as (keyof BundleValidateState)[]) { - if ( - oldValue === null || - !lodash.isEqual(oldValue?.[key], newValue?.[key]) - ) { - this.onDidChangeKeyEmitters.get(key)?.fire(); - } - } }) ); } - private readerMapping: { - [K in keyof BundleValidateState]: ( - t?: BundleTarget - ) => BundleValidateState[K]; - } = { - clusterId: (target) => target?.bundle?.compute_id, - workspaceFsPath: (target) => target?.workspace?.file_path, - resources: (target) => target?.resources, - }; - @Mutex.synchronise("mutex") public async setTarget(target: string | undefined) { if (this.target === target) { @@ -93,14 +52,13 @@ export class BundleValidateModel implements Disposable { } } - @onError({popup: {prefix: "Failed to read bundle config."}}) - @Mutex.synchronise("mutex") - private async readState() { + @onError({popup: {prefix: "Failed to parse bundle validate ouput"}}) + protected async readState(): Promise { if (this.target === undefined || this.authProvider === undefined) { - return; + return {}; } - const targetObject = JSON.parse( + const validateOutput = JSON.parse( await this.cli.bundleValidate( this.target, this.authProvider, @@ -109,35 +67,11 @@ export class BundleValidateModel implements Disposable { ) ) as BundleTarget; - const configs: any = {}; - - for (const key of Object.keys( - this.readerMapping - ) as (keyof BundleValidateState)[]) { - configs[key] = this.readerMapping[key]?.(targetObject); - } - - return {...configs, ...targetObject} as BundleValidateState; - } - - @Mutex.synchronise("mutex") - public async load( - keys: T[] = [] - ): Promise> | undefined> { - if (keys.length === 0) { - return await this.stateCache.value; - } - - const target = await this.stateCache.value; - const configs: Partial<{ - [K in T]: BundleValidateState[K]; - }> = {}; - - for (const key of keys) { - configs[key] = this.readerMapping[key]?.(target); - } - - return configs; + return { + clusterId: validateOutput?.bundle?.compute_id, + remoteRootPath: validateOutput?.workspace?.file_path, + ...validateOutput, + }; } dispose() { diff --git a/packages/databricks-vscode/src/configuration/ConnectionManager.ts b/packages/databricks-vscode/src/configuration/ConnectionManager.ts index f4defae65..9771b0c0c 100644 --- a/packages/databricks-vscode/src/configuration/ConnectionManager.ts +++ b/packages/databricks-vscode/src/configuration/ConnectionManager.ts @@ -57,7 +57,7 @@ export class ConnectionManager implements Disposable { popup: {prefix: "Error attaching sync destination: "}, }) private async updateSyncDestinationMapper() { - const workspacePath = await this.configModel.get("workspaceFsPath"); + const workspacePath = await this.configModel.get("remoteRootPath"); const remoteUri = workspacePath ? new RemoteUri(workspacePath) : undefined; @@ -116,19 +116,16 @@ export class ConnectionManager implements Disposable { await this.loginWithSavedAuth(); this.disposables.push( - this.configModel.onDidChange("workspaceFsPath")( + this.configModel.onDidChangeKey("remoteRootPath")( this.updateSyncDestinationMapper, this ), - this.configModel.onDidChange("clusterId")( + this.configModel.onDidChangeKey("clusterId")( this.updateClusterManager, this ), - this.configModel.onDidChange("target")( - this.loginWithSavedAuth, - this - ), - this.configModel.onDidChange("authParams")(async () => { + this.configModel.onDidChangeTarget(this.loginWithSavedAuth, this), + this.configModel.onDidChangeKey("authParams")(async () => { const config = await this.configModel.getS("authParams"); if (config === undefined) { return; @@ -290,7 +287,10 @@ export class ConnectionManager implements Disposable { return; } - const config = await configureWorkspaceWizard(this.cli, host); + const config = await configureWorkspaceWizard( + this.cli, + host.toString() + ); if (!config) { return; } @@ -326,14 +326,14 @@ export class ConnectionManager implements Disposable { ); return; } - await this.configModel.set("workspaceFsPath", remoteWorkspace.path); + await this.configModel.set("remoteRootPath", remoteWorkspace.path); } @onError({ popup: {prefix: "Can't detach sync destination. "}, }) async detachSyncDestination(): Promise { - await this.configModel.set("workspaceFsPath", undefined); + await this.configModel.set("remoteRootPath", undefined); } private updateState(newState: ConnectionState) { diff --git a/packages/databricks-vscode/src/configuration/auth/AuthProvider.ts b/packages/databricks-vscode/src/configuration/auth/AuthProvider.ts index 66ae81b6f..0dfc9eb81 100644 --- a/packages/databricks-vscode/src/configuration/auth/AuthProvider.ts +++ b/packages/databricks-vscode/src/configuration/auth/AuthProvider.ts @@ -38,7 +38,7 @@ export abstract class AuthProvider { * Used to display the auth method in the UI */ abstract describe(): string; - abstract toJSON(): Record; + abstract toJSON(): Record; abstract toEnv(): Record; getWorkspaceClient(): WorkspaceClient { @@ -110,7 +110,7 @@ export class ProfileAuthProvider extends AuthProvider { return `Profile '${this.profile}'`; } - toJSON(): Record { + toJSON(): Record { return { host: this.host.toString(), authType: this.authType, @@ -167,7 +167,7 @@ export class DatabricksCliAuthProvider extends AuthProvider { return "OAuth U2M"; } - toJSON(): Record { + toJSON(): Record { return { host: this.host.toString(), authType: this.authType, @@ -220,7 +220,7 @@ export class AzureCliAuthProvider extends AuthProvider { return "Azure CLI"; } - toJSON(): Record { + toJSON(): Record { return { host: this.host.toString(), authType: this.authType, diff --git a/packages/databricks-vscode/src/configuration/models/BaseModelWithStateCache.ts b/packages/databricks-vscode/src/configuration/models/BaseModelWithStateCache.ts new file mode 100644 index 000000000..862b7ab8b --- /dev/null +++ b/packages/databricks-vscode/src/configuration/models/BaseModelWithStateCache.ts @@ -0,0 +1,54 @@ +import {Disposable} from "vscode"; +import {CachedValue} from "../../locking/CachedValue"; +import {Mutex} from "../../locking"; + +export abstract class BaseModelWithStateCache + implements Disposable +{ + protected disposables: Disposable[] = []; + protected abstract mutex: Mutex; + + constructor() { + this.disposables.push(this.stateCache); + } + + protected readonly stateCache: CachedValue = new CachedValue( + this.readState.bind(this) + ); + + public onDidChange = this.stateCache.onDidChange.bind(this.stateCache); + public onDidChangeKey = this.stateCache.onDidChangeKey.bind( + this.stateCache + ); + + protected abstract readState(): Promise; + + @Mutex.synchronise("mutex") + public async get( + key: T + ): Promise { + return (await this.stateCache.value)[key]; + } + + @Mutex.synchronise("mutex") + public async load( + keys: T[] = [] + ): Promise>> { + if (keys.length === 0) { + return await this.stateCache.value; + } + + const stateValue = await this.stateCache.value; + return Object.fromEntries( + Object.entries(stateValue).filter(([key]) => { + return keys.includes(key as T); + }) + ) as Partial<{ + [K in T]: StateType[K]; + }>; + } + + dispose() { + this.disposables.forEach((i) => i.dispose()); + } +} diff --git a/packages/databricks-vscode/src/configuration/models/ConfigModel.ts b/packages/databricks-vscode/src/configuration/models/ConfigModel.ts index 0ab7cb4d0..66c433906 100644 --- a/packages/databricks-vscode/src/configuration/models/ConfigModel.ts +++ b/packages/databricks-vscode/src/configuration/models/ConfigModel.ts @@ -1,26 +1,57 @@ import {Disposable, EventEmitter, Uri, Event} from "vscode"; -import { - DATABRICKS_CONFIG_KEYS, - DatabricksConfig, - isBundlePreValidateConfigKey, - isOverrideableConfigKey, - DatabricksConfigSourceMap, - BUNDLE_VALIDATE_CONFIG_KEYS, -} from "../types"; import {Mutex} from "../../locking"; import {CachedValue} from "../../locking/CachedValue"; import {StateStorage} from "../../vscode-objs/StateStorage"; -import lodash from "lodash"; import {onError} from "../../utils/onErrorDecorator"; import {AuthProvider} from "../auth/AuthProvider"; -import {OverrideableConfigModel} from "./OverrideableConfigModel"; -import {BundlePreValidateModel} from "../../bundle/models/BundlePreValidateModel"; -import {BundleValidateModel} from "../../bundle/models/BundleValidateModel"; +import { + OverrideableConfigModel, + OverrideableConfigState, + isOverrideableConfigKey, +} from "./OverrideableConfigModel"; +import { + BundlePreValidateModel, + BundlePreValidateState, +} from "../../bundle/models/BundlePreValidateModel"; +import { + BundleValidateModel, + BundleValidateState, +} from "../../bundle/models/BundleValidateModel"; -const defaults: DatabricksConfig = { +const defaults: ConfigState = { mode: "development", }; +const SELECTED_BUNDLE_VALIDATE_CONFIG_KEYS = [ + "clusterId", + "remoteRootPath", +] as const; + +const SELECTED_BUNDLE_PRE_VALIDATE_CONFIG_KEYS = [ + "host", + "mode", + "authParams", +] as const; + +type ConfigState = Pick< + BundleValidateState, + (typeof SELECTED_BUNDLE_VALIDATE_CONFIG_KEYS)[number] +> & + Pick< + BundlePreValidateState, + (typeof SELECTED_BUNDLE_PRE_VALIDATE_CONFIG_KEYS)[number] + > & + OverrideableConfigState; + +export type ConfigSource = "bundle" | "override" | "default"; + +type ConfigSourceMap = { + [K in keyof ConfigState]: { + config: ConfigState[K]; + source: ConfigSource; + }; +}; + /** * In memory view of the databricks configs loaded from overrides and bundle. */ @@ -28,57 +59,59 @@ export class ConfigModel implements Disposable { private disposables: Disposable[] = []; private readonly configsMutex = new Mutex(); - private readonly configCache = new CachedValue<{ - config: DatabricksConfig; - source: DatabricksConfigSourceMap; - }>(async () => { - if (this.target === undefined) { - return {config: {}, source: {}}; - } - const bundleValidateConfig = await this.bundleValidateModel.load([ - ...BUNDLE_VALIDATE_CONFIG_KEYS, - ]); - const overrides = await this.overrideableConfigModel.load(); - const bundleConfigs = await this.bundlePreValidateModel.load(); - const newValue: DatabricksConfig = { - ...bundleConfigs, - ...bundleValidateConfig, - ...overrides, - }; + private readonly configCache = new CachedValue( + async () => { + if (this.target === undefined) { + return {config: {}, source: {}}; + } + const bundleValidateConfig = await this.bundleValidateModel.load([ + ...SELECTED_BUNDLE_VALIDATE_CONFIG_KEYS, + ]); + const overrides = await this.overrideableConfigModel.load(); + const bundleConfigs = await this.bundlePreValidateModel.load([ + ...SELECTED_BUNDLE_PRE_VALIDATE_CONFIG_KEYS, + ]); + const newConfigs = { + ...bundleConfigs, + ...bundleValidateConfig, + ...overrides, + }; - const source: DatabricksConfigSourceMap = {}; + const newValue: any = {}; + (Object.keys(newConfigs) as (keyof typeof newConfigs)[]).forEach( + (key) => { + newValue[key] = { + config: newConfigs[key], + source: + overrides !== undefined && key in overrides + ? "override" + : "bundle", + }; + } + ); - /* By default undefined values are considered to have come from bundle. - This is because when override for a key is undefined, it means that the key - is not overridden and we want to get the value from bundle. - */ - DATABRICKS_CONFIG_KEYS.forEach((key) => { - source[key] = - overrides !== undefined && key in overrides - ? "override" - : "bundle"; - }); + return newValue; + } + ); - return { - config: newValue, - source: source, - }; - }); + public onDidChange = this.configCache.onDidChange.bind(this.configCache); + public onDidChangeKey = this.configCache.onDidChangeKey.bind( + this.configCache + ); - private readonly changeEmitters = new Map< - keyof DatabricksConfig | "target", - { - emitter: EventEmitter; - onDidEmit: Event; - } - >(); - public onDidChangeAny = this.configCache.onDidChange; + private onDidChangeTargetEmitter = new EventEmitter(); + public readonly onDidChangeTarget: Event = + this.onDidChangeTargetEmitter.event; + private onDidChangeAuthProviderEmitter = new EventEmitter(); + public readonly onDidChangeAuthProvider: Event = + this.onDidChangeAuthProviderEmitter.event; private _target: string | undefined; + private _authProvider: AuthProvider | undefined; constructor( - private readonly bundleValidateModel: BundleValidateModel, - private readonly overrideableConfigModel: OverrideableConfigModel, + public readonly bundleValidateModel: BundleValidateModel, + public readonly overrideableConfigModel: OverrideableConfigModel, public readonly bundlePreValidateModel: BundlePreValidateModel, private readonly stateStorage: StateStorage ) { @@ -92,25 +125,12 @@ export class ConfigModel implements Disposable { //refresh cache to trigger onDidChange event await this.configCache.refresh(); }), - ...BUNDLE_VALIDATE_CONFIG_KEYS.map((key) => + ...SELECTED_BUNDLE_VALIDATE_CONFIG_KEYS.map((key) => this.bundleValidateModel.onDidChangeKey(key)(async () => { //refresh cache to trigger onDidChange event this.configCache.refresh(); }) - ), - this.configCache.onDidChange(({oldValue, newValue}) => { - DATABRICKS_CONFIG_KEYS.forEach((key) => { - if ( - oldValue === null || - !lodash.isEqual( - oldValue.config[key], - newValue.config[key] - ) - ) { - this.changeEmitters.get(key)?.emitter.fire(); - } - }); - }) + ) ); } @@ -119,17 +139,6 @@ export class ConfigModel implements Disposable { await this.readTarget(); } - public onDidChange(key: T) { - if (!this.changeEmitters.has(key)) { - const emitter = new EventEmitter(); - this.changeEmitters.set(key, { - emitter: emitter, - onDidEmit: emitter.event, - }); - } - - return this.changeEmitters.get(key)!.onDidEmit; - } /** * Try to read target from bundle config. * If not found, try to read from state storage. @@ -182,7 +191,7 @@ export class ConfigModel implements Disposable { await this.configsMutex.synchronise(async () => { this._target = target; await this.stateStorage.set("databricks.bundle.target", target); - this.changeEmitters.get("target")?.emitter.fire(); + this.onDidChangeTargetEmitter.fire(); await Promise.all([ this.bundlePreValidateModel.setTarget(target), this.bundleValidateModel.setTarget(target), @@ -192,15 +201,22 @@ export class ConfigModel implements Disposable { } @onError({popup: {prefix: "Failed to set auth provider."}}) + @Mutex.synchronise("configsMutex") public async setAuthProvider(authProvider: AuthProvider | undefined) { await this.bundleValidateModel.setAuthProvider(authProvider); + this._authProvider = authProvider; + this.onDidChangeAuthProviderEmitter.fire(); + } + + get authProvider(): AuthProvider | undefined { + return this._authProvider; } @Mutex.synchronise("configsMutex") - public async get( + public async get( key: T - ): Promise { - return (await this.configCache.value).config[key] ?? defaults[key]; + ): Promise { + return (await this.configCache.value)[key]?.config ?? defaults[key]; } /** @@ -208,33 +224,25 @@ export class ConfigModel implements Disposable { * Refer to {@link DatabricksConfigSource} for possible values. */ @Mutex.synchronise("configsMutex") - public async getS( + public async getS( key: T - ): Promise< - | { - config: DatabricksConfig[T]; - source: DatabricksConfigSourceMap[T]; - } - | undefined - > { - const {config: fullConfig, source: fullSource} = - await this.configCache.value; - const config = fullConfig[key] ?? defaults[key]; - const source = - fullConfig[key] !== undefined ? fullSource[key] : "default"; + ): Promise { + let {config, source} = (await this.configCache.value)[key]; + config = config ?? defaults[key]; + source = source ?? "default"; return config - ? { + ? ({ config, source, - } + } as ConfigSourceMap[T]) : undefined; } @onError({popup: {prefix: "Failed to set config."}}) @Mutex.synchronise("configsMutex") - public async set( + public async set( key: T, - value?: DatabricksConfig[T], + value?: ConfigState[T], handleInteractiveWrite?: (file: Uri) => Promise ) { if (this.target === undefined) { @@ -243,9 +251,13 @@ export class ConfigModel implements Disposable { ); } if (isOverrideableConfigKey(key)) { - return this.overrideableConfigModel.write(key, this.target, value); + return this.overrideableConfigModel.write( + key, + this.target, + value as any + ); } - if (isBundlePreValidateConfigKey(key) && handleInteractiveWrite) { + if (handleInteractiveWrite) { await handleInteractiveWrite( await this.bundlePreValidateModel.getFileToWrite(key) ); diff --git a/packages/databricks-vscode/src/configuration/models/OverrideableConfigModel.ts b/packages/databricks-vscode/src/configuration/models/OverrideableConfigModel.ts index 1454fcc87..930030981 100644 --- a/packages/databricks-vscode/src/configuration/models/OverrideableConfigModel.ts +++ b/packages/databricks-vscode/src/configuration/models/OverrideableConfigModel.ts @@ -1,28 +1,25 @@ import {StateStorage} from "../../vscode-objs/StateStorage"; -import {OverrideableConfig} from "../types"; -import {CachedValue} from "../../locking/CachedValue"; -import {Disposable} from "vscode"; import {Mutex} from "../../locking"; +import {BaseModelWithStateCache} from "./BaseModelWithStateCache"; +import {onError} from "../../utils/onErrorDecorator"; -export class OverrideableConfigModel implements Disposable { - private writeMutex = new Mutex(); +export type OverrideableConfigState = { + authProfile?: string; + clusterId?: string; +}; - private disposables: Disposable[] = []; - - private readonly stateCache = new CachedValue< - OverrideableConfig | undefined - >(async () => { - if (this.target === undefined) { - return undefined; - } - return this.readState(this.target); - }); - - public readonly onDidChange = this.stateCache.onDidChange; +export function isOverrideableConfigKey( + key: string +): key is keyof OverrideableConfigState { + return ["authProfile", "clusterId"].includes(key); +} +export class OverrideableConfigModel extends BaseModelWithStateCache { + protected mutex = new Mutex(); private target: string | undefined; constructor(private readonly storage: StateStorage) { + super(); this.disposables.push( this.storage.onDidChange("databricks.bundle.overrides")( async () => await this.stateCache.refresh() @@ -34,12 +31,14 @@ export class OverrideableConfigModel implements Disposable { this.target = target; } - private async readState(target: string) { - return this.storage.get("databricks.bundle.overrides")[target]; - } - - public async load() { - return this.stateCache.value; + @onError({popup: {prefix: "Error while reading config overrides"}}) + protected async readState() { + if (this.target === undefined) { + return {}; + } + return ( + this.storage.get("databricks.bundle.overrides")[this.target] ?? {} + ); } /** @@ -49,20 +48,16 @@ export class OverrideableConfigModel implements Disposable { * @param value the value to write. If undefined, the override is removed. * @returns status of the write */ - @Mutex.synchronise("writeMutex") - async write( + @Mutex.synchronise("mutex") + async write( key: T, target: string, - value?: OverrideableConfig[T] + value?: OverrideableConfigState[T] ) { const data = this.storage.get("databricks.bundle.overrides"); if (data[target] === undefined) { data[target] = {}; } - const oldValue = JSON.stringify(data[target][key]); - if (oldValue === JSON.stringify(value)) { - return; - } data[target][key] = value; await this.storage.set("databricks.bundle.overrides", data); } diff --git a/packages/databricks-vscode/src/configuration/types.ts b/packages/databricks-vscode/src/configuration/types.ts deleted file mode 100644 index 0d5f480ac..000000000 --- a/packages/databricks-vscode/src/configuration/types.ts +++ /dev/null @@ -1,71 +0,0 @@ -export type DatabricksConfig = { - host?: string; - - mode?: "development" | "staging" | "production"; - authParams?: Record; - - clusterId?: string; - workspaceFsPath?: string; -}; - -export type DatabricksConfigSource = "bundle" | "override" | "default"; - -export type DatabricksConfigSourceMap = { - [key in keyof DatabricksConfig]: DatabricksConfigSource; -}; - -export const OVERRIDEABLE_CONFIG_KEYS = ["clusterId", "authParams"] as const; - -export type OverrideableConfig = Pick< - DatabricksConfig, - (typeof OVERRIDEABLE_CONFIG_KEYS)[number] ->; - -export const BUNDLE_PRE_VALIDATE_CONFIG_KEYS = [ - "authParams", - "mode", - "host", -] as const; - -/** These are configs which can be loaded from the bundle */ -export type BundlePreValidateConfig = Pick< - DatabricksConfig, - (typeof BUNDLE_PRE_VALIDATE_CONFIG_KEYS)[number] ->; - -export const BUNDLE_VALIDATE_CONFIG_KEYS = [ - "clusterId", - "workspaceFsPath", -] as const; - -/** These are configs which can be loaded from the bundle */ -export type BundleValidateConfig = Pick< - DatabricksConfig, - (typeof BUNDLE_VALIDATE_CONFIG_KEYS)[number] ->; - -export const DATABRICKS_CONFIG_KEYS: (keyof DatabricksConfig)[] = Array.from( - new Set([ - ...OVERRIDEABLE_CONFIG_KEYS, - ...BUNDLE_PRE_VALIDATE_CONFIG_KEYS, - ...BUNDLE_VALIDATE_CONFIG_KEYS, - ]) -); - -export function isOverrideableConfigKey( - key: any -): key is keyof OverrideableConfig { - return OVERRIDEABLE_CONFIG_KEYS.includes(key); -} - -export function isBundlePreValidateConfigKey( - key: any -): key is keyof BundlePreValidateConfig { - return BUNDLE_PRE_VALIDATE_CONFIG_KEYS.includes(key); -} - -export function isBundleValidateConfigKey( - key: any -): key is keyof BundleValidateConfig { - return BUNDLE_VALIDATE_CONFIG_KEYS.includes(key); -} diff --git a/packages/databricks-vscode/src/configuration/ui/AuthTypeComponent.ts b/packages/databricks-vscode/src/configuration/ui/AuthTypeComponent.ts index ae6ee82a3..1de39ad6a 100644 --- a/packages/databricks-vscode/src/configuration/ui/AuthTypeComponent.ts +++ b/packages/databricks-vscode/src/configuration/ui/AuthTypeComponent.ts @@ -18,7 +18,7 @@ export class AuthTypeComponent extends BaseComponent { this.connectionManager.onDidChangeState(() => { this.onDidChangeEmitter.fire(); }), - this.configModel.onDidChange("target")(() => { + this.configModel.onDidChangeTarget(() => { this.onDidChangeEmitter.fire(); }) ); @@ -54,7 +54,9 @@ export class AuthTypeComponent extends BaseComponent { ]; } - const config = await this.configModel.getS("authParams"); + const config = + (await this.configModel.getS("authProfile")) ?? + (await this.configModel.getS("authParams")); if (config === undefined) { // This case can never happen. This is just to make ts happy. return []; diff --git a/packages/databricks-vscode/src/configuration/ui/BundleTargetComponent.ts b/packages/databricks-vscode/src/configuration/ui/BundleTargetComponent.ts index d0f9b252d..c7480783c 100644 --- a/packages/databricks-vscode/src/configuration/ui/BundleTargetComponent.ts +++ b/packages/databricks-vscode/src/configuration/ui/BundleTargetComponent.ts @@ -1,7 +1,6 @@ import {ThemeIcon, ThemeColor, TreeItemCollapsibleState, window} from "vscode"; import {ConfigModel} from "../models/ConfigModel"; import {BaseComponent} from "./BaseComponent"; -import {DatabricksConfig} from "../types"; import {ConfigurationTreeItem} from "./types"; const TREE_ICON_ID = "TARGET"; @@ -10,24 +9,18 @@ function getTreeIconId(key: string) { return `${TREE_ICON_ID}.${key}`; } -function humaniseMode(mode: DatabricksConfig["mode"]) { - switch (mode) { - case "production": - return "Production"; - case "staging": - return "Staging"; - case "development": - return "Development"; - default: - return mode; +function humaniseMode(mode?: string) { + if (mode === undefined) { + return ""; } + return mode.charAt(0).toUpperCase() + mode.slice(1); } export class BundleTargetComponent extends BaseComponent { constructor(private readonly configModel: ConfigModel) { super(); this.disposables.push( - this.configModel.onDidChange("target")(() => { + this.configModel.onDidChangeTarget(() => { this.onDidChangeEmitter.fire(); }) ); @@ -98,9 +91,9 @@ export class BundleTargetComponent extends BaseComponent { { label: "Host", id: getTreeIconId("host"), - description: host, + description: host?.toString(), collapsibleState: TreeItemCollapsibleState.None, - url: host, + url: host?.toString(), }, { label: "Mode", diff --git a/packages/databricks-vscode/src/configuration/ui/SyncDestinationComponent.ts b/packages/databricks-vscode/src/configuration/ui/SyncDestinationComponent.ts index 8c558b00b..ae7d9be4b 100644 --- a/packages/databricks-vscode/src/configuration/ui/SyncDestinationComponent.ts +++ b/packages/databricks-vscode/src/configuration/ui/SyncDestinationComponent.ts @@ -62,17 +62,20 @@ export class SyncDestinationComponent extends BaseComponent { ) { super(); this.disposables.push( - this.codeSynchronizer.onDidChangeState(() => { + this.configModel.onDidChangeTarget(() => { this.onDidChangeEmitter.fire(); }), - this.configModel.onDidChange("workspaceFsPath")(() => { + this.connectionManager.onDidChangeState(() => { + this.onDidChangeEmitter.fire(); + }), + this.configModel.onDidChangeKey("remoteRootPath")(() => { this.onDidChangeEmitter.fire(); }) ); } private async getRoot(): Promise { - const config = await this.configModel.get("workspaceFsPath"); + const config = await this.configModel.get("remoteRootPath"); if (config === undefined) { // Workspace folder is not set in bundle and override // We are not logged in @@ -86,7 +89,7 @@ export class SyncDestinationComponent extends BaseComponent { return [ { label: "Sync", - description: posix.basename(config), + description: posix.basename(posix.dirname(config)), collapsibleState: TreeItemCollapsibleState.Expanded, contextValue: contextValue, iconPath: icon, @@ -113,18 +116,17 @@ export class SyncDestinationComponent extends BaseComponent { if (parent.id !== TREE_ICON_ID) { return []; } - const workspaceFsPath = await this.configModel.get("workspaceFsPath"); + const workspaceFsPath = await this.configModel.get("remoteRootPath"); if (workspaceFsPath === undefined) { return []; } - //TODO: Disable syncing for prod/staging - //TODO: Read sync destination from bundle. Infer from CLI if not set. + //TODO: Disable syncing for all targets (dev/staging/prod) const children: ConfigurationTreeItem[] = [ { label: "Workspace Folder", - description: posix.basename(workspaceFsPath), + description: workspaceFsPath, collapsibleState: TreeItemCollapsibleState.None, }, ]; diff --git a/packages/databricks-vscode/src/configuration/ui/types.ts b/packages/databricks-vscode/src/configuration/ui/types.ts index 6cbfbf241..bfcf414d8 100644 --- a/packages/databricks-vscode/src/configuration/ui/types.ts +++ b/packages/databricks-vscode/src/configuration/ui/types.ts @@ -1,7 +1,7 @@ import {TreeItem} from "vscode"; -import {DatabricksConfigSource} from "../types"; +import {ConfigSource} from "../models/ConfigModel"; export interface ConfigurationTreeItem extends TreeItem { - source?: DatabricksConfigSource; + source?: ConfigSource; url?: string; } diff --git a/packages/databricks-vscode/src/locking/CachedValue.ts b/packages/databricks-vscode/src/locking/CachedValue.ts index 2f42ff7d0..96b50834c 100644 --- a/packages/databricks-vscode/src/locking/CachedValue.ts +++ b/packages/databricks-vscode/src/locking/CachedValue.ts @@ -1,8 +1,10 @@ -import {EventEmitter} from "vscode"; +import {EventEmitter, Disposable} from "vscode"; import {Mutex} from "."; import lodash from "lodash"; -export class CachedValue { +export class CachedValue implements Disposable { + private disposables: Disposable[] = []; + private _value: T | null = null; private _dirty = true; private readonly mutex = new Mutex(); @@ -12,12 +14,48 @@ export class CachedValue { }>(); public readonly onDidChange = this.onDidChangeEmitter.event; - constructor(private readonly getter: (value: T | null) => Promise) {} + constructor(private readonly getter: () => Promise) { + this.disposables.push( + this.onDidChange(async ({oldValue, newValue}) => { + function isObject( + value: unknown + ): value is object | undefined | null { + return ( + typeof value === "object" || + typeof value === "undefined" || + value === null + ); + } + if (!isObject(oldValue) || !isObject(newValue)) { + return; + } + + if (oldValue === null || oldValue === undefined) { + oldValue = {} as T; + } + if (newValue === null || newValue === undefined) { + newValue = {} as T; + } + + for (const key of Object.keys({ + ...oldValue, + ...newValue, + } as any) as (keyof T)[]) { + if ( + oldValue === null || + !lodash.isEqual(oldValue?.[key], newValue?.[key]) + ) { + this.onDidChangeKeyEmitters.get(key)?.fire(); + } + } + }) + ); + } get value(): Promise { if (this._dirty || this._value === null) { return this.mutex.synchronise(async () => { - const newValue = await this.getter(this._value); + const newValue = await this.getter(); if (!lodash.isEqual(newValue, this._value)) { this.onDidChangeEmitter.fire({ oldValue: this._value, @@ -33,6 +71,18 @@ export class CachedValue { return Promise.resolve(this._value); } + private readonly onDidChangeKeyEmitters = new Map< + keyof T, + EventEmitter + >(); + + onDidChangeKey(key: T extends object ? keyof T : never) { + if (!this.onDidChangeKeyEmitters.has(key)) { + this.onDidChangeKeyEmitters.set(key, new EventEmitter()); + } + return this.onDidChangeKeyEmitters.get(key)!.event; + } + @Mutex.synchronise("mutex") async invalidate() { this._dirty = true; @@ -42,4 +92,8 @@ export class CachedValue { await this.invalidate(); await this.value; } + + dispose() { + this.disposables.forEach((d) => d.dispose()); + } } diff --git a/packages/databricks-vscode/src/vscode-objs/StateStorage.ts b/packages/databricks-vscode/src/vscode-objs/StateStorage.ts index 2728f403e..eb5a8e6d3 100644 --- a/packages/databricks-vscode/src/vscode-objs/StateStorage.ts +++ b/packages/databricks-vscode/src/vscode-objs/StateStorage.ts @@ -1,6 +1,6 @@ import {randomUUID} from "crypto"; import {EventEmitter, ExtensionContext, Event} from "vscode"; -import {OverrideableConfig} from "../configuration/types"; +import {OverrideableConfigState} from "../configuration/models/OverrideableConfigModel"; import {Mutex} from "../locking"; import lodash from "lodash"; @@ -20,7 +20,7 @@ function withType() { const StorageConfigurations = { "databricks.bundle.overrides": withType<{ - [k: string]: OverrideableConfig; + [k: string]: OverrideableConfigState; }>()({ location: "workspace", defaultValue: {},