Skip to content

Commit

Permalink
feat: globalState abstraction
Browse files Browse the repository at this point in the history
Problem:
We have a settings.ts module which abstracts the vscode settings
interface, to gain important features such as type-checking, validation,
and error handling.
https://github.com/aws/aws-toolkit-vscode/blob/b9d5534c0879382baa5c900d14d0a2c3fdd529c5/packages/core/src/shared/settings.ts

The vscode globalState interface is very similar to the vscode settings
interface, and has the same limitations and potential for bugs: the data
is user-defined and arbitrary, thus the types are unknown and must
always be runtime-checked, which is a verbose and often overlooked step.

Examples:

- `redshiftState.ts` https://github.com/aws/aws-toolkit-vscode-staging/pull/1034/files
- https://github.com/aws/aws-toolkit-vscode/blob/8f55e40cab47ef7d25ed5faac274c3fac3f9f91c/src/shared/filesystemUtilities.ts#L228-L253
- globalState type issues related to a codewhisperer bug: #3060
- `aws.lastUploadedToS3Folder` https://github.com/aws/aws-toolkit-vscode/pull/3183/files
- `ExtensionUse` class https://github.com/aws/aws-toolkit-vscode/pull/3634/files
- `codewhisperer/util/globalStateUtil.test.ts` https://github.com/aws/aws-toolkit-vscode/blob/80e715bbf3e6eb354a9b6e5e327c732b89df38e3/packages/amazonq/test/unit/codewhisperer/util/globalStateUtil.test.ts

Solution:
- Introduce a `globalState` wrapper, similar to `src/shared/settings.ts`.
- Migrate the `redshiftState.ts` module into the centralized `globalState`
  module.
  • Loading branch information
justinmk3 committed Jul 11, 2024
1 parent 31bd1f9 commit f88250f
Show file tree
Hide file tree
Showing 15 changed files with 314 additions and 93 deletions.
11 changes: 7 additions & 4 deletions packages/core/src/awsService/redshift/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import { RedshiftNotebookController } from './notebook/redshiftNotebookControlle
import { CellStatusBarItemProvider } from './notebook/cellStatusBarItemProvider'
import { Commands } from '../../shared/vscode/commands2'
import { NotebookConnectionWizard, RedshiftNodeConnectionWizard } from './wizards/connectionWizard'
import { ConnectionParams, ConnectionType } from './models/models'
import { deleteConnection, ConnectionParams, ConnectionType } from './models/models'
import { DefaultRedshiftClient } from '../../shared/clients/redshiftClient'
import { localize } from '../../shared/utilities/vsCodeUtils'
import { RedshiftWarehouseNode } from './explorer/redshiftWarehouseNode'
import { ToolkitError } from '../../shared/errors'
import { deleteConnection, updateConnectionParamsState } from './explorer/redshiftState'
import { showViewLogsMessage } from '../../shared/utilities/messages'
import { showConnectionMessage } from './messageUtils'
import globals from '../../shared/extensionGlobals'

export async function activate(ctx: ExtContext): Promise<void> {
if ('NotebookEdit' in vscode) {
Expand Down Expand Up @@ -123,7 +123,10 @@ function getEditConnectionHandler() {
connectionParams.secret = secretArnFetched
}
redshiftWarehouseNode.setConnectionParams(connectionParams)
await updateConnectionParamsState(redshiftWarehouseNode.arn, redshiftWarehouseNode.connectionParams)
await globals.globalState.saveRedshiftConnection(
redshiftWarehouseNode.arn,
redshiftWarehouseNode.connectionParams
)
await vscode.commands.executeCommand('aws.refreshAwsExplorerNode', redshiftWarehouseNode)
}
} catch (error) {
Expand All @@ -135,7 +138,7 @@ function getEditConnectionHandler() {
function getDeleteConnectionHandler() {
return async (redshiftWarehouseNode: RedshiftWarehouseNode) => {
redshiftWarehouseNode.connectionParams = undefined
await updateConnectionParamsState(redshiftWarehouseNode.arn, deleteConnection)
await globals.globalState.saveRedshiftConnection(redshiftWarehouseNode.arn, deleteConnection)
await vscode.commands.executeCommand('aws.refreshAwsExplorerNode', redshiftWarehouseNode)
}
}
45 changes: 0 additions & 45 deletions packages/core/src/awsService/redshift/explorer/redshiftState.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@
import { AWSResourceNode } from '../../../shared/treeview/nodes/awsResourceNode'
import { AWSTreeNodeBase } from '../../../shared/treeview/nodes/awsTreeNodeBase'
import * as vscode from 'vscode'
import globals from '../../../shared/extensionGlobals'
import { RedshiftNode } from './redshiftNode'
import { makeChildrenNodes } from '../../../shared/treeview/utils'
import { RedshiftDatabaseNode } from './redshiftDatabaseNode'
import { localize } from '../../../shared/utilities/vsCodeUtils'
import { LoadMoreNode } from '../../../shared/treeview/nodes/loadMoreNode'
import { ChildNodeLoader, ChildNodePage } from '../../../awsexplorer/childNodeLoader'
import { DefaultRedshiftClient } from '../../../shared/clients/redshiftClient'
import { ConnectionParams, ConnectionType, RedshiftWarehouseType } from '../models/models'
import { deleteConnection, ConnectionParams, ConnectionType, RedshiftWarehouseType } from '../models/models'
import { RedshiftNodeConnectionWizard } from '../wizards/connectionWizard'
import { ListDatabasesResponse } from 'aws-sdk/clients/redshiftdata'
import { getIcon } from '../../../shared/icons'
import { AWSCommandTreeNode } from '../../../shared/treeview/nodes/awsCommandTreeNode'
import { telemetry } from '../../../shared/telemetry/telemetry'
import { deleteConnection, getConnectionParamsState, updateConnectionParamsState } from './redshiftState'
import { createLogsConnectionMessage, showConnectionMessage } from '../messageUtils'

export class CreateNotebookNode extends AWSCommandTreeNode {
Expand Down Expand Up @@ -50,7 +50,7 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
this.arn = redshiftWarehouse.arn
this.name = redshiftWarehouse.name
this.redshiftClient = parent.redshiftClient
const existingConnectionParams = getConnectionParamsState(this.arn)
const existingConnectionParams = globals.globalState.getRedshiftConnection(this.arn)
if (existingConnectionParams && existingConnectionParams !== deleteConnection) {
this.connectionParams = existingConnectionParams as ConnectionParams
this.iconPath = getIcon('aws-redshift-cluster-connected')
Expand Down Expand Up @@ -108,14 +108,14 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
return await makeChildrenNodes({
getChildNodes: async () => {
this.childLoader.clearChildren()
const existingConnectionParams = getConnectionParamsState(this.arn)
const existingConnectionParams = globals.globalState.getRedshiftConnection(this.arn)
if (existingConnectionParams && existingConnectionParams === deleteConnection) {
// connection is deleted but explorer is not refreshed: return clickToEstablishConnectionNode
await updateConnectionParamsState(this.arn, undefined)
await globals.globalState.saveRedshiftConnection(this.arn, undefined)
return this.getClickToEstablishConnectionNode()
} else if (existingConnectionParams && existingConnectionParams !== deleteConnection) {
} else if (existingConnectionParams) {
// valid connectionParams: update the redshiftWarehouseNode
this.connectionParams = existingConnectionParams as ConnectionParams
this.connectionParams = existingConnectionParams
} else {
// No connectionParams: trigger connection wizard to get user input
this.connectionParams = await new RedshiftNodeConnectionWizard(this).run()
Expand All @@ -137,11 +137,11 @@ export class RedshiftWarehouseNode extends AWSTreeNodeBase implements AWSResourc
const childNodes = await this.childLoader.getChildren()
const startButtonNode = new CreateNotebookNode(this)
childNodes.unshift(startButtonNode)
await updateConnectionParamsState(this.arn, this.connectionParams)
await globals.globalState.saveRedshiftConnection(this.arn, this.connectionParams)
return childNodes
} catch (error) {
showConnectionMessage(this.redshiftWarehouse.name, error as Error)
await updateConnectionParamsState(this.arn, undefined)
await globals.globalState.saveRedshiftConnection(this.arn, undefined)
return this.getRetryNode()
}
},
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/awsService/redshift/models/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

import { Region } from '../../../shared/regions/endpoints'

// Sigil treated such that the connection wizard is not triggered during explorer node refresh after
// connection deletion.
export const deleteConnection = 'DELETE_CONNECTION' as const

export class ConnectionParams {
constructor(
public readonly connectionType: ConnectionType,
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/extensionCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { LoginManager } from './auth/deprecated/loginManager'
import { CredentialsStore } from './auth/credentials/store'
import { initializeAwsCredentialsStatusBarItem } from './auth/ui/statusBarItem'
import { RegionProvider, getEndpointsFromFetcher } from './shared/regions/regionProvider'
import { getMachineId } from './shared/vscode/env'
import { getMachineId, isAutomation } from './shared/vscode/env'
import { registerCommandErrorHandler } from './shared/vscode/commands2'
import { registerWebviewErrorHandler } from './webviews/server'
import { showQuickStartWebview } from './shared/extensionStartup'
Expand Down Expand Up @@ -77,7 +77,7 @@ export async function activateCommon(
const homeDirLogs = await fs.init(context, homeDir => {
void showViewLogsMessage(`Invalid home directory (check $HOME): "${homeDir}"`)
})
errors.init(fs.getUsername())
errors.init(fs.getUsername(), isAutomation())
await initializeComputeRegion()

globals.contextPrefix = '' //todo: disconnect supplied argument
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/shared/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import { isNonNullable } from './utilities/tsUtils'
import type * as nodefs from 'fs'
import type * as os from 'os'
import { CodeWhispererStreamingServiceException } from '@amzn/codewhisperer-streaming'
import { isAutomation } from './vscode/env'
import { driveLetterRegex } from './utilities/pathUtils'

let _username = 'unknown-user'
let _isAutomation = false

/** One-time initialization for this module. */
export function init(username: string) {
/** Performs one-time initialization, to avoid circular dependencies. */
export function init(username: string, isAutomation: boolean) {
_username = username
_isAutomation = isAutomation
}

export const errorCode = {
Expand Down Expand Up @@ -667,7 +668,7 @@ function vscodeModeToString(mode: vscode.FileStat['permissions']) {
}

// XXX: future-proof in case vscode.FileStat.permissions gains more granularity.
if (isAutomation()) {
if (_isAutomation) {
throw new Error('vscode.FileStat.permissions gained new fields, update this logic')
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/extensionGlobals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export function initialize(context: ExtensionContext, isWeb: boolean = false): T
context,
clock: copyClock(),
didReload: checkDidReload(context),
globalState: new GlobalState(context),
globalState: new GlobalState(context.globalState),
manifestPaths: {} as ToolkitGlobals['manifestPaths'],
visualizationResourcePaths: {} as ToolkitGlobals['visualizationResourcePaths'],
isWeb,
Expand Down
124 changes: 110 additions & 14 deletions packages/core/src/shared/globalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@

import * as vscode from 'vscode'
import { getLogger } from './logger/logger'
import * as redshift from '../awsService/redshift/models/models'
import { TypeConstructor, cast } from './utilities/typeConstructors'

type globalKey =
| 'aws.downloadPath'
| 'aws.lastTouchedS3Folder'
| 'aws.lastUploadedToS3Folder'
| 'aws.redshift.connections'
| 'aws.toolkit.amazonq.dismissed'
| 'aws.toolkit.amazonqInstall.dismissed'
// Deprecated/legacy names. New keys should start with "aws.".
Expand All @@ -19,41 +22,134 @@ type globalKey =
| 'hasAlreadyOpenedAmazonQ'

/**
* Extension-local, shared state which persists after IDE restart. Shared with all instances (or
* tabs, in a web browser) of this extension for a given user, but not visible to other vscode
* extensions. Global state should be avoided, except when absolutely necessary.
* Extension-local (not visible to other vscode extensions) shared state which persists after IDE
* restart. Shared with all instances (or tabs, in a web browser) of this extension for a given
* user, including "remote" instances!
*
* Note: Global state should be avoided, except when absolutely necessary.
*
* This wrapper adds structure and visibility to the vscode `globalState` interface. It also opens
* the door for:
* - validation
* - garbage collection
*/
export class GlobalState implements vscode.Memento {
public constructor(private readonly extContext: vscode.ExtensionContext) {}
constructor(private readonly memento: vscode.Memento) {}

public keys(): readonly string[] {
return this.extContext.globalState.keys()
keys(): readonly string[] {
return this.memento.keys()
}

public get<T>(key: globalKey, defaultValue?: T): T | undefined {
return this.extContext.globalState.get(key) ?? defaultValue
/**
* Gets the value for `key` if it satisfies the `type` specification, or fails.
*
* @param key Key name
* @param type Type validator function, or primitive type constructor such as {@link Object},
* {@link String}, {@link Boolean}, etc.
* @param defaultVal Value returned if `key` has no value.
*/
getStrict<T>(key: globalKey, type: TypeConstructor<T>, defaulVal?: T) {
try {
const val = this.memento.get<T>(key) ?? defaulVal
return !type || val === undefined ? val : cast(val, type)
} catch (e) {
const msg = `GlobalState: invalid state (or read failed) for key: "${key}"`
// XXX: ToolkitError causes circular dependency
// throw ToolkitError.chain(e, `Failed to read globalState: "${key}"`)
const err = new Error(msg) as Error & {
code: string
cause: unknown
}
err.cause = e
err.code = 'GlobalState'
throw err
}
}

/**
* Gets the value at `key`, without type-checking. See {@link tryGet} and {@link getStrict} for type-checking variants.
*
* @param key Key name
* @param defaultVal Value returned if `key` has no value.
*/
get<T>(key: globalKey, defaulVal?: T): T | undefined {
const skip = (o: any) => o as T // Don't type check.
return this.getStrict(key, skip, defaulVal)
}

/**
* Gets the value for `key` if it satisfies the `type` specification, else logs an error and returns `defaulVal`.
*
* @param key Key name
* @param type Type validator function, or primitive type constructor such as {@link Object},
* {@link String}, {@link Boolean}, etc.
* @param defaultVal Value returned if `key` has no value.
*/
tryGet<T>(key: globalKey, type: TypeConstructor<T>, defaulVal?: T): T | undefined {
try {
return this.getStrict(key, type, defaulVal)
} catch (e) {
getLogger().error('%s', (e as Error).message)
return defaulVal
}
}

/** Asynchronously updates globalState, or logs an error on failure. */
public tryUpdate(key: globalKey, value: any): void {
this.extContext.globalState.update(key, value).then(
tryUpdate(key: globalKey, value: any): void {
this.memento.update(key, value).then(
undefined, // TODO: log.debug() ?
e => {
getLogger().error('GlobalState: failed to set "%s": %s', key, (e as Error).message)
}
)
}

public update(key: globalKey, value: any): Thenable<void> {
return this.extContext.globalState.update(key, value)
update(key: globalKey, value: any): Thenable<void> {
return this.memento.update(key, value)
}

/**
* Stores Redshift connection info for the specified warehouse ARN.
*
* TODO: this never garbage-collects old connections, so the state will grow forever...
*
* @param warehouseArn redshift warehouse ARN
* @param cxnInfo Connection info. Value is 'DELETE_CONNECTION' when the connection is deleted
* but the explorer node is not refreshed yet.
*/
async saveRedshiftConnection(
warehouseArn: string,
cxnInfo: redshift.ConnectionParams | undefined | 'DELETE_CONNECTION'
) {
const allCxns = this.tryGet('aws.redshift.connections', Object, {})
await this.update('aws.redshift.connections', {
...allCxns,
[warehouseArn]: cxnInfo,
})
}

public samAndCfnSchemaDestinationUri() {
return vscode.Uri.joinPath(this.extContext.globalStorageUri, 'sam.schema.json')
/**
* Get the Redshift connection info for the specified warehouse ARN.
*
* @param warehouseArn redshift warehouse ARN
* @returns Connection info. Value is 'DELETE_CONNECTION' when the connection is deleted but the
* explorer node is not refreshed yet.
*/
getRedshiftConnection(warehouseArn: string): redshift.ConnectionParams | undefined | 'DELETE_CONNECTION' {
const allCxns = this.tryGet(
'aws.redshift.connections',
v => {
if (v !== undefined && typeof v !== 'object') {
throw new Error()
}
const cxn = (v as any)?.[warehouseArn]
if (cxn !== undefined && typeof cxn !== 'object' && cxn !== 'DELETE_CONNECTION') {
throw new Error()
}
return v
},
undefined
)
return (allCxns as any)?.[warehouseArn]
}
}
10 changes: 8 additions & 2 deletions packages/core/src/shared/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ import * as codecatalyst from './clients/codecatalystClient'
import * as codewhisperer from '../codewhisperer/client/codewhisperer'
import packageJson from '../../package.json'
import { getLogger } from './logger'
import { cast, FromDescriptor, Record, TypeConstructor, TypeDescriptor } from './utilities/typeConstructors'
import {
cast,
FromDescriptor,
isNameMangled,
Record,
TypeConstructor,
TypeDescriptor,
} from './utilities/typeConstructors'
import { assertHasProps, ClassToInterfaceType, keys } from './utilities/tsUtils'
import { toRecord } from './utilities/collectionUtils'
import { isNameMangled } from './vscode/env'
import { once, onceChanged } from './utilities/functionUtils'
import { ToolkitError } from './errors'
import { telemetry } from './telemetry/telemetry'
Expand Down
Loading

0 comments on commit f88250f

Please sign in to comment.