From cfebf99bcd74cfbfaf9a2b9543079a99bfd8ac84 Mon Sep 17 00:00:00 2001 From: Ross Stenersen Date: Thu, 21 Jul 2022 14:50:43 -0500 Subject: [PATCH] refactor: move organization handling into APIOrganizationCommand class --- .changeset/quick-brooms-develop.md | 6 ++ package-lock.json | 4 +- .../lib/src/__tests__/api-command.test.ts | 35 -------- .../api-organization-command.test.ts | 79 +++++++++++++++++++ packages/lib/src/api-command.ts | 56 +++++-------- packages/lib/src/api-organization-command.ts | 22 +++++- packages/testlib/package.json | 2 +- 7 files changed, 130 insertions(+), 74 deletions(-) create mode 100644 .changeset/quick-brooms-develop.md create mode 100644 packages/lib/src/__tests__/api-organization-command.test.ts diff --git a/.changeset/quick-brooms-develop.md b/.changeset/quick-brooms-develop.md new file mode 100644 index 000000000..363a88d24 --- /dev/null +++ b/.changeset/quick-brooms-develop.md @@ -0,0 +1,6 @@ +--- +"@smartthings/cli-lib": patch +"@smartthings/cli-testlib": patch +--- + +refactor handling of headers on initialization diff --git a/package-lock.json b/package-lock.json index 1a3c6d804..2254c1a8b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18773,7 +18773,7 @@ "license": "Apache-2.0", "dependencies": { "@smartthings/cli-lib": "^1.0.0-beta.10", - "@smartthings/core-sdk": "^5.0.0" + "@smartthings/core-sdk": "^5.1.0" }, "devDependencies": { "@types/jest": "^28.1.5", @@ -21918,7 +21918,7 @@ "version": "file:packages/testlib", "requires": { "@smartthings/cli-lib": "^1.0.0-beta.10", - "@smartthings/core-sdk": "^5.0.0", + "@smartthings/core-sdk": "^5.1.0", "@types/jest": "^28.1.5", "@types/js-yaml": "^4.0.5", "@types/node": "^16.11.44", diff --git a/packages/lib/src/__tests__/api-command.test.ts b/packages/lib/src/__tests__/api-command.test.ts index 5f9e97305..815ef7acc 100644 --- a/packages/lib/src/__tests__/api-command.test.ts +++ b/packages/lib/src/__tests__/api-command.test.ts @@ -91,28 +91,6 @@ describe('api-command', () => { expect(configUsed?.headers).toContainEntry(['Accept-Language', 'es-US']) }) - it('passes organization flag on to client', async () => { - parseSpy.mockResolvedValueOnce({ args: {}, flags: { organization: 'organization-id-from-flag' } } as ParserOutputType) - await apiCommand.init() - - expect(stClientSpy).toHaveBeenCalledTimes(1) - - const configUsed = stClientSpy.mock.calls[0][1] - expect(configUsed?.headers).toContainEntry(['X-ST-Organization', 'organization-id-from-flag']) - }) - - it('passes organization config on to client', async () => { - const profile: Profile = { organization: 'organization-id-from-config' } - loadConfigMock.mockResolvedValueOnce({ profile } as CLIConfig) - - await apiCommand.init() - - expect(stClientSpy).toHaveBeenCalledTimes(1) - - const configUsed = stClientSpy.mock.calls[0][1] - expect(configUsed?.headers).toContainEntry(['X-ST-Organization', 'organization-id-from-config']) - }) - it('returns oclif config User-Agent and default if undefined', () => { expect(apiCommand.userAgent).toBe('@smartthings/cli') @@ -146,19 +124,6 @@ describe('api-command', () => { expect(LoginAuthenticator).toBeCalledWith(expect.anything(), expect.anything(), expect.any(String)) }) - it('prefers organization flag over config', async () => { - const profile: Profile = { organization: 'organization-id-from-config' } - loadConfigMock.mockResolvedValueOnce({ profile } as CLIConfig) - parseSpy.mockResolvedValueOnce({ args: {}, flags: { organization: 'organization-id-from-flag' } } as ParserOutputType) - - await apiCommand.init() - - expect(stClientSpy).toHaveBeenCalledTimes(1) - - const configUsed = stClientSpy.mock.calls[0][1] - expect(configUsed?.headers).toContainEntry(['X-ST-Organization', 'organization-id-from-flag']) - }) - describe('warningLogger', () => { it('uses string as-is', async () => { parseSpy.mockResolvedValueOnce({ args: {}, flags: { language: 'es-US' } } as ParserOutputType) diff --git a/packages/lib/src/__tests__/api-organization-command.test.ts b/packages/lib/src/__tests__/api-organization-command.test.ts new file mode 100644 index 000000000..6d77b0474 --- /dev/null +++ b/packages/lib/src/__tests__/api-organization-command.test.ts @@ -0,0 +1,79 @@ +import { Config, Interfaces } from '@oclif/core' + +import * as coreSDK from '@smartthings/core-sdk' + +import { APIOrganizationCommand } from '../api-organization-command' +import { CLIConfig, loadConfig, Profile } from '../cli-config' + + +jest.mock('@smartthings/core-sdk') +jest.mock('../cli-config') +jest.mock('../login-authenticator') + +describe('APIOrganizationCommand', () => { + const stClientSpy = jest.spyOn(coreSDK, 'SmartThingsClient') + + class TestCommand extends APIOrganizationCommand { + async run(): Promise { + // eslint-disable-line @typescript-eslint/no-empty-function + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unused-vars + async parse(options?: Interfaces.Input, argv?: string[]): Promise> { + return { + flags: {}, + args: {}, + argv: [], + raw: [], + metadata: { flags: {} }, + } + } + } + + const loadConfigMock = jest.mocked(loadConfig) + const parseSpy = jest.spyOn(TestCommand.prototype, 'parse') + // eslint-disable-next-line @typescript-eslint/no-explicit-any + type ParserOutputType = Interfaces.ParserOutput + + let apiCommand: TestCommand + + beforeEach(() => { + apiCommand = new TestCommand([], {} as Config) + apiCommand.warn = jest.fn() + }) + + it('passes organization flag on to client', async () => { + parseSpy.mockResolvedValueOnce({ args: {}, flags: { organization: 'organization-id-from-flag' } } as ParserOutputType) + await apiCommand.init() + + expect(stClientSpy).toHaveBeenCalledTimes(1) + + const configUsed = stClientSpy.mock.calls[0][1] + expect(configUsed?.headers).toContainEntry(['X-ST-Organization', 'organization-id-from-flag']) + }) + + it('passes organization config on to client', async () => { + const profile: Profile = { organization: 'organization-id-from-config' } + loadConfigMock.mockResolvedValueOnce({ profile } as CLIConfig) + + await apiCommand.init() + + expect(stClientSpy).toHaveBeenCalledTimes(1) + + const configUsed = stClientSpy.mock.calls[0][1] + expect(configUsed?.headers).toContainEntry(['X-ST-Organization', 'organization-id-from-config']) + }) + + it('prefers organization flag over config', async () => { + const profile: Profile = { organization: 'organization-id-from-config' } + loadConfigMock.mockResolvedValueOnce({ profile } as CLIConfig) + parseSpy.mockResolvedValueOnce({ args: {}, flags: { organization: 'organization-id-from-flag' } } as ParserOutputType) + + await apiCommand.init() + + expect(stClientSpy).toHaveBeenCalledTimes(1) + + const configUsed = stClientSpy.mock.calls[0][1] + expect(configUsed?.headers).toContainEntry(['X-ST-Organization', 'organization-id-from-flag']) + }) +}) diff --git a/packages/lib/src/api-command.ts b/packages/lib/src/api-command.ts index 42f1b3701..d8e8c6a2d 100644 --- a/packages/lib/src/api-command.ts +++ b/packages/lib/src/api-command.ts @@ -1,25 +1,19 @@ +import log4js from '@log4js-node/log4js-api' import { Flags } from '@oclif/core' import osLocale from 'os-locale' + import { Authenticator, BearerTokenAuthenticator, HttpClientHeaders, SmartThingsClient, WarningFromHeader } from '@smartthings/core-sdk' + import { ClientIdProvider, defaultClientIdProvider, LoginAuthenticator } from './login-authenticator' import { SmartThingsCommand } from './smartthings-command' -import log4js from '@log4js-node/log4js-api' -import { APIOrganizationCommand } from './api-organization-command' const LANGUAGE_HEADER = 'Accept-Language' -const ORGANIZATION_HEADER = 'X-ST-Organization' - -/** - * The command being parsed will not always have {@link APIOrganizationCommand.flags}. - * Therefore, we make them all optional to be safely accessible in init below. - */ -type InputFlags = typeof APICommand.flags & Partial /** * Base class for commands that need to use Rest API via the SmartThings Core SDK. */ -export abstract class APICommand extends SmartThingsCommand { +export abstract class APICommand extends SmartThingsCommand { static flags = { ...SmartThingsCommand.flags, token: Flags.string({ @@ -36,7 +30,6 @@ export abstract class APICommand extends SmartThingsComman protected token?: string private _authenticator!: Authenticator private _client!: SmartThingsClient - private _headers!: HttpClientHeaders get authenticator(): Authenticator { return this._authenticator @@ -46,14 +39,25 @@ export abstract class APICommand extends SmartThingsComman return this._client } - protected get headers(): HttpClientHeaders { - return this._headers - } - get userAgent(): string { return this.config.userAgent ?? '@smartthings/cli' } + async initHeaders(): Promise { + // eslint-disable-next-line @typescript-eslint/naming-convention + const headers: HttpClientHeaders = { 'User-Agent': this.userAgent } + + if (this.flags.language) { + if (this.flags.language !== 'NONE') { + headers[LANGUAGE_HEADER] = this.flags.language + } + } else { + headers[LANGUAGE_HEADER] = await osLocale() + } + + return headers + } + async init(): Promise { await super.init() @@ -77,25 +81,7 @@ export abstract class APICommand extends SmartThingsComman const logger = log4js.getLogger('rest-client') - // eslint-disable-next-line @typescript-eslint/naming-convention - this._headers = { 'User-Agent': this.userAgent } - - if (this.flags.language) { - if (this.flags.language !== 'NONE') { - this._headers[LANGUAGE_HEADER] = this.flags.language - } - } else { - this._headers[LANGUAGE_HEADER] = await osLocale() - } - - if (this.flags.organization) { - this._headers[ORGANIZATION_HEADER] = this.flags.organization - } else { - const configOrganization = this.stringConfigValue('organization') - if (configOrganization) { - this._headers[ORGANIZATION_HEADER] = configOrganization - } - } + const headers = await this.initHeaders() this._authenticator = this.token ? new BearerTokenAuthenticator(this.token) @@ -109,6 +95,6 @@ export abstract class APICommand extends SmartThingsComman this.warn(message) } this._client = new SmartThingsClient(this._authenticator, - { urlProvider: this.clientIdProvider, logger, headers: this.headers, warningLogger }) + { urlProvider: this.clientIdProvider, logger, headers, warningLogger }) } } diff --git a/packages/lib/src/api-organization-command.ts b/packages/lib/src/api-organization-command.ts index 67e3c95b7..3795393a8 100644 --- a/packages/lib/src/api-organization-command.ts +++ b/packages/lib/src/api-organization-command.ts @@ -1,7 +1,12 @@ import { Flags } from '@oclif/core' + +import { HttpClientHeaders } from '@smartthings/core-sdk' + import { APICommand } from './api-command' +const ORGANIZATION_HEADER = 'X-ST-Organization' + /** * Base class for commands that need to use Rest API commands via the * SmartThings Core SDK and can act on the behalf of different organizations. @@ -11,7 +16,22 @@ export abstract class APIOrganizationCommand { + const headers = await super.initHeaders() + + if (this.flags.organization) { + headers[ORGANIZATION_HEADER] = this.flags.organization + } else { + const configOrganization = this.stringConfigValue('organization') + if (configOrganization) { + headers[ORGANIZATION_HEADER] = configOrganization + } + } + + return headers + } } diff --git a/packages/testlib/package.json b/packages/testlib/package.json index 8c9403fb4..e9d1246ab 100644 --- a/packages/testlib/package.json +++ b/packages/testlib/package.json @@ -29,7 +29,7 @@ }, "dependencies": { "@smartthings/cli-lib": "^1.0.0-beta.10", - "@smartthings/core-sdk": "^5.0.0" + "@smartthings/core-sdk": "^5.1.0" }, "devDependencies": { "@types/jest": "^28.1.5",