refactor: move organization handling into APIOrganizationCommand class

.changeset/quick-brooms-develop.md

@@ -0,0 +1,6 @@
+---
+"@smartthings/cli-lib": patch
+"@smartthings/cli-testlib": patch
+---
+
+refactor handling of headers on initialization

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)

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<typeof TestCommand.flags> {
+		async run(): Promise<void> {
+			// 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<any>, argv?: string[]): Promise<Interfaces.ParserOutput<any, any>> {
+			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<any, any>
+
+	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'])
+	})
+})

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<typeof APIOrganizationCommand.flags>
 
 /**
  * Base class for commands that need to use Rest API via the SmartThings Core SDK.
  */
-export abstract class APICommand<T extends InputFlags> extends SmartThingsCommand<T> {
+export abstract class APICommand<T extends typeof APICommand.flags> extends SmartThingsCommand<T> {
 	static flags = {
 		...SmartThingsCommand.flags,
 		token: Flags.string({
@@ -36,7 +30,6 @@ export abstract class APICommand<T extends InputFlags> 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<T extends InputFlags> extends SmartThingsComman
 		return this._client
 	}
 
-	protected get headers(): HttpClientHeaders {
-		return this._headers
-	}
-
 	get userAgent(): string {
 		return this.config.userAgent ?? '@smartthings/cli'
 	}
 
+	async initHeaders(): Promise<HttpClientHeaders> {
+		// 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<void> {
 		await super.init()
 
@@ -77,25 +81,7 @@ export abstract class APICommand<T extends InputFlags> 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<T extends InputFlags> 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 })
 	}
 }

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<T extends typeof APIOrganizationCom
 		...APICommand.flags,
 		organization: Flags.string({
 			char: 'O',
-			description: 'The organization ID to use for this command',
+			description: 'the organization ID to use for this command',
 		}),
 	}
+
+	async initHeaders(): Promise<HttpClientHeaders> {
+		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
+	}
 }

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",