diff --git a/packages/config-resolver/test/index.test.ts b/packages/config-resolver/test/index.test.ts index 37d0b72d..5f3536af 100644 --- a/packages/config-resolver/test/index.test.ts +++ b/packages/config-resolver/test/index.test.ts @@ -8,7 +8,7 @@ import assert from 'node:assert/strict' import { mkdtemp, writeFile, rm } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' -import { loadConfig } from '../../../src/config/loader.ts' +import { loadConfig, clearConfigCache } from '../../../src/config/loader.ts' import { containsExpression, resolveString, @@ -21,6 +21,7 @@ import { afterEach(() => { _testResetResolvers() + clearConfigCache() }) // --------------------------------------------------------------------------- diff --git a/src/cli.ts b/src/cli.ts index f3af5c5b..e5d08390 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -7,7 +7,7 @@ import { Command } from 'commander' import { defineCommand, defineGroup, hideBlockedCommands } from './factory.js' import type { OpaqueCommandHandle } from './factory.js' -import { loadConfig, type LoadConfigResult } from './config/loader.ts' +import { loadConfig } from './config/loader.ts' import { BUILT_IN_PROFILES, type BuiltInProfile } from './config/profiles.ts' import { setResolvedConfig } from './config/store.ts' import { renderLogo } from './lib/logo.ts' @@ -33,10 +33,8 @@ program // On error, print a structured message and exit -- never let a config failure // silently propagate into the command handler. // -// When no --config-file or --use-context overrides are specified, the hook -// reuses the cached earlyConfig to avoid a redundant load+resolve cycle. -let earlyConfig: LoadConfigResult | undefined - +// When no --config-file or --use-context overrides are specified, loadConfig() +// returns the in-process cached result from the early load, avoiding redundant I/O. program.hook('preAction', async (thisCommand, actionCommand) => { if (actionCommand.name() === 'version') return // `status` loads the config itself so a partially broken config is reported as @@ -55,16 +53,13 @@ program.hook('preAction', async (thisCommand, actionCommand) => { } const { configFile: configPath, useContext: contextName, commandProfile: profileName } = thisCommand.opts() const typedProfileName = profileName as BuiltInProfile | undefined - - if (configPath == null && contextName == null && profileName == null && earlyConfig?.ok === true) { - setResolvedConfig(earlyConfig.value) - return - } + const hasOverrides = configPath != null || contextName != null || profileName != null const result = await loadConfig({ ...(configPath != null && { configPath }), ...(contextName != null && { contextName }), ...(typedProfileName != null && { profileName: typedProfileName }), + refresh: hasOverrides, }) if (result.ok) { setResolvedConfig(result.value) @@ -208,7 +203,7 @@ if (firstArg === 'status') { // Load config early so --help can hide blocked commands. Skip for commands // that don't need config (e.g. `version`, `sanitize`, or `config` which authors the file) // to avoid unnecessary file I/O and a confusing "no config found" path. -// The result is cached in earlyConfig so the preAction hook can reuse it. +// loadConfig() caches the result in-process; the preAction hook reuses it via the default cache path. const EARLY_CONFIG_COMMANDS = new Set(['version', 'config', 'sanitize', 'extension', 'status']) if (!EARLY_CONFIG_COMMANDS.has(firstArg ?? '')) { // Parse --profile early (before Commander's full parse) so the early config load @@ -216,7 +211,7 @@ if (!EARLY_CONFIG_COMMANDS.has(firstArg ?? '')) { const profileArgIdx = process.argv.indexOf('--command-profile') const earlyProfile = profileArgIdx !== -1 ? process.argv[profileArgIdx + 1] as BuiltInProfile | undefined : undefined - earlyConfig = await loadConfig({ + const earlyConfig = await loadConfig({ ...(earlyProfile != null && { profileName: earlyProfile }), }) if (earlyConfig.ok) { @@ -226,6 +221,7 @@ if (!EARLY_CONFIG_COMMANDS.has(firstArg ?? '')) { } if (process.argv.slice(2).length === 0) { + const earlyConfig = await loadConfig() if (!earlyConfig?.ok || earlyConfig.value.banner !== false) { process.stdout.write(renderLogo(VERSION)) } @@ -244,7 +240,8 @@ if (firstArg != null && !BUILT_IN_COMMANDS.has(firstArg)) { if (ext != null) { const { buildContextEnv } = await import('./extension/context.ts') const { runExtension } = await import('./extension/runner.ts') - const contextEnv = earlyConfig?.ok === true ? buildContextEnv(earlyConfig.value) : {} + const cachedConfig = await loadConfig() + const contextEnv = cachedConfig?.ok === true ? buildContextEnv(cachedConfig.value) : {} const exitCode = await runExtension(ext, process.argv.slice(3), contextEnv) process.exit(exitCode) } diff --git a/src/config/loader.ts b/src/config/loader.ts index 01d06d94..169d4b19 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -219,6 +219,23 @@ export interface LoadConfigOptions { contextName?: string /** Profile name override (`--profile` flag). Overrides any profile set in the config file. */ profileName?: BuiltInProfile + /** + * When `true`, bypass the in-process cache and perform a fresh load (caching + * the new result for subsequent calls). Defaults to `false`. + */ + refresh?: boolean +} + +// --------------------------------------------------------------------------- +// In-process result cache — avoids redundant I/O within a single CLI invocation. +// Tests can call clearConfigCache() between cases to prevent state leakage. +// --------------------------------------------------------------------------- + +let _cachedConfig: LoadConfigResult | undefined + +/** Clears the in-process config cache. Intended for test cleanup only. */ +export function clearConfigCache (): void { + _cachedConfig = undefined } /** Successful result from {@link loadConfig}. */ @@ -251,7 +268,9 @@ export type LoadConfigResult = LoadConfigOk | LoadConfigErr * @returns A `LoadConfigResult` discriminated union. */ export async function loadConfig (options: LoadConfigOptions = {}): Promise { - const { configPath, contextName, profileName } = options + const { configPath, contextName, profileName, refresh = false } = options + + if (!refresh && _cachedConfig !== undefined) return _cachedConfig // Validate profileName early (before any I/O) so the error is immediate and clear if (profileName != null && !(BUILT_IN_PROFILES as readonly string[]).includes(profileName)) { @@ -358,10 +377,13 @@ export async function loadConfig (options: LoadConfigOptions = {}): Promise { + let tmpDir: string + let configPath: string + + before(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'elastic-cli-cache-test-')) + configPath = join(tmpDir, '.elasticrc.yml') + await writeFile(configPath, VALID_CONFIG_YAML, 'utf-8') + }) + + after(async () => rm(tmpDir, { recursive: true })) + + afterEach(() => clearConfigCache()) + + it('returns the same reference on repeated calls (cache hit)', async () => { + const first = await loadConfig({ configPath }) + const second = await loadConfig({ configPath }) + assert.equal(first, second, 'expected same object reference from cache') + }) + + it('returns a fresh result when refresh: true', async () => { + const first = await loadConfig({ configPath }) + const second = await loadConfig({ configPath, refresh: true }) + assert.notEqual(first, second, 'expected a new object when refresh is true') + assert.deepEqual(first, second, 'expected same shape after refresh') + }) + + it('clearConfigCache resets to undefined so next call loads fresh', async () => { + const first = await loadConfig({ configPath }) + clearConfigCache() + const second = await loadConfig({ configPath }) + assert.notEqual(first, second, 'expected a new object after cache was cleared') + assert.deepEqual(first, second, 'expected same shape after re-load') + }) + + it('refresh: true updates the cache so subsequent default calls return the refreshed result', async () => { + await loadConfig({ configPath }) + const refreshed = await loadConfig({ configPath, refresh: true }) + const subsequent = await loadConfig({ configPath }) + assert.equal(refreshed, subsequent, 'expected the refreshed result to be cached') + }) +}) diff --git a/test/config/loader.test.ts b/test/config/loader.test.ts index 583a38a3..cbcac0fe 100644 --- a/test/config/loader.test.ts +++ b/test/config/loader.test.ts @@ -3,14 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, before, after } from 'node:test' +import { describe, it, before, after, afterEach } from 'node:test' import assert from 'node:assert/strict' import { mkdtemp, writeFile, rm, mkdir } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' -import { loadConfigFile, discoverConfigFile, resolveContext, resolveEffectiveCommands, loadConfig } from '../../src/config/loader.ts' +import { loadConfigFile, discoverConfigFile, resolveContext, resolveEffectiveCommands, loadConfig, clearConfigCache } from '../../src/config/loader.ts' import type { ConfigFile, ResolvedConfig } from '../../src/config/types.ts' +afterEach(() => clearConfigCache()) + // --------------------------------------------------------------------------- // Shared fixtures // --------------------------------------------------------------------------- diff --git a/test/status/register.test.ts b/test/status/register.test.ts index 83abc1e2..9a7f25e4 100644 --- a/test/status/register.test.ts +++ b/test/status/register.test.ts @@ -10,6 +10,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { Command } from 'commander' import { registerStatusCommand, runStatusChecks, _testSetFetch } from '../../src/status/register.ts' +import { clearConfigCache } from '../../src/config/loader.ts' import type { ResolvedContext } from '../../src/config/types.ts' const SAMPLE_HEALTH = JSON.stringify({ status: 'green', number_of_nodes: 3 }) @@ -113,6 +114,7 @@ describe('elastic status -- command', () => { afterEach(async () => { await rm(dir, { recursive: true, force: true }) + clearConfigCache() }) async function writeConfig (yaml: string): Promise {