From cce57f965e84855a1079fa23589d741a94ba8e8e Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 28 Aug 2026 15:50:04 +0200 Subject: [PATCH] refactor(cli): Consolidate UI5 data dir resolution into dataDir module Introduce lib/dataDir.js exposing getUi5DataDir, getUi5DataDirOrDefault and a formatPath helper that shortens home-directory paths to ~ for display. This consolidates the previously duplicated ui5DataDir resolution logic that framework utils and the cache command each carried their own copy of. The cache command now resolves the data dir via getUi5DataDirOrDefault and lazily imports FrameworkCache and CacheManager, so the modules are not loaded on every CLI invocation. Displayed cache paths use formatPath. This only consolidates resolution within the cli package; a general, cross-package cleanup remains tracked in PR #1456. --- packages/cli/lib/cli/commands/cache.js | 29 ++-- .../lib/cli/commands/helpers/cacheOutput.js | 5 +- packages/cli/lib/dataDir.js | 57 ++++++++ packages/cli/lib/framework/utils.js | 13 +- packages/cli/test/lib/cli/commands/cache.js | 57 +++----- packages/cli/test/lib/dataDir.js | 130 ++++++++++++++++++ packages/cli/test/lib/framework/utils.js | 46 ------- 7 files changed, 219 insertions(+), 118 deletions(-) create mode 100644 packages/cli/lib/dataDir.js create mode 100644 packages/cli/test/lib/dataDir.js diff --git a/packages/cli/lib/cli/commands/cache.js b/packages/cli/lib/cli/commands/cache.js index 5df7d88285a..c580f835e01 100644 --- a/packages/cli/lib/cli/commands/cache.js +++ b/packages/cli/lib/cli/commands/cache.js @@ -1,12 +1,9 @@ import chalk from "chalk"; import path from "node:path"; -import os from "node:os"; import process from "node:process"; import {isLogLevelEnabled} from "@ui5/logger"; import baseMiddleware from "../middlewares/base.js"; -import Configuration from "@ui5/project/config/Configuration"; -import FrameworkCache from "@ui5/project/internal/ui5Framework/cache"; -import CacheManager from "@ui5/project/internal/build/cache/CacheManager"; +import {getUi5DataDirOrDefault, formatPath} from "../../dataDir.js"; import { CACHE_CLEAN_HELP_USAGE, displayCacheCleanWarning, @@ -63,20 +60,6 @@ async function getConfirmation(argv) { }); } -async function resolveCacheUi5DataDir() { - // TODO: Consolidate ui5DataDir resolution once PR #1456 follow-up cleanup is done. - // Keep behavior aligned with existing main-branch resolution order. - let ui5DataDir = process.env.UI5_DATA_DIR; - if (!ui5DataDir) { - const config = await Configuration.fromFile(); - ui5DataDir = config.getUi5DataDir(); - } - if (ui5DataDir) { - return path.resolve(process.cwd(), ui5DataDir); - } - return path.join(os.homedir(), ".ui5"); -} - function withAbsPath(entries, ui5DataDir) { return entries.map((entry) => { return {...entry, absPath: getAbsPath(ui5DataDir, entry)}; @@ -91,12 +74,18 @@ function getAbsPath(ui5DataDir, cacheEntry) { } async function handleCache(argv) { - const ui5DataDir = await resolveCacheUi5DataDir(); + // Lazy loading to prevent unnecessary imports when the command is not executed + const [{default: FrameworkCache}, {default: CacheManager}] = await Promise.all([ + import("@ui5/project/internal/ui5Framework/cache"), + import("@ui5/project/internal/build/cache/CacheManager"), + ]); + + const ui5DataDir = await getUi5DataDirOrDefault({cwd: process.cwd()}); const isVerbose = isLogLevelEnabled("verbose"); if (isVerbose) { // logger.verbose pollutes output with framework noise. - process.stderr.write(`Checking cache at ${chalk.bold(ui5DataDir)} …\n`); + process.stderr.write(`Checking cache at ${chalk.bold(formatPath(ui5DataDir))} …\n`); } const [frameworkInfo, buildInfo] = await Promise.all([ diff --git a/packages/cli/lib/cli/commands/helpers/cacheOutput.js b/packages/cli/lib/cli/commands/helpers/cacheOutput.js index b6a2544be1d..6295a0437ed 100644 --- a/packages/cli/lib/cli/commands/helpers/cacheOutput.js +++ b/packages/cli/lib/cli/commands/helpers/cacheOutput.js @@ -1,5 +1,6 @@ import chalk from "chalk"; import process from "node:process"; +import {formatPath} from "../../../dataDir.js"; const GROUP_FRAMEWORK = "Framework"; const GROUP_BUILD = "Build"; @@ -47,14 +48,14 @@ function writeCategoryHeader(title) { function writePreviewItem(absPath, detail) { process.stderr.write( - ` ${PREVIEW_MARKER} ${chalk.dim(absPath)}` + + ` ${PREVIEW_MARKER} ${chalk.dim(formatPath(absPath))}` + `${detail ? ` ${ITEM_DIVIDER} ${detail}` : ""}\n` ); } function writeCleanupItem(absPath, detail) { process.stderr.write( - ` ${SUCCESS_MARKER} Removed ${chalk.dim(absPath)}` + + ` ${SUCCESS_MARKER} Removed ${chalk.dim(formatPath(absPath))}` + `${detail ? ` ${ITEM_DIVIDER} ${detail}` : ""}\n` ); } diff --git a/packages/cli/lib/dataDir.js b/packages/cli/lib/dataDir.js new file mode 100644 index 00000000000..11b9c2ea1e3 --- /dev/null +++ b/packages/cli/lib/dataDir.js @@ -0,0 +1,57 @@ +import path from "node:path"; +import os from "node:os"; +import process from "node:process"; +import Configuration from "@ui5/project/config/Configuration"; + +// TODO: This module only consolidates ui5DataDir resolution within the cli package. +// A general, cross-package cleanup of ui5DataDir resolution is tracked in PR #1456. + +/** + * Resolves the UI5 data directory from the UI5_DATA_DIR environment variable or the + * UI5 configuration. The environment variable takes precedence over the configured value. + * + * @param {object} options + * @param {string} options.cwd Directory a relative data-dir value is resolved against + * @returns {Promise} Absolute path to the UI5 data directory, + * or undefined when neither source provides a value + */ +export async function getUi5DataDir({cwd}) { + let ui5DataDir = process.env.UI5_DATA_DIR; + if (!ui5DataDir) { + const config = await Configuration.fromFile(); + ui5DataDir = config.getUi5DataDir(); + } + return ui5DataDir ? path.resolve(cwd, ui5DataDir) : undefined; +} + +/** + * Like {@link getUi5DataDir}, but falls back to <home>/.ui5 when neither the + * environment variable nor the configuration provides a value. + * + * @param {object} options + * @param {string} options.cwd Directory a relative data-dir value is resolved against + * @returns {Promise} Absolute path to the UI5 data directory + */ +export async function getUi5DataDirOrDefault({cwd}) { + return (await getUi5DataDir({cwd})) ?? path.join(os.homedir(), ".ui5"); +} + +/** + * Shortens an absolute path for display by replacing the user's home directory with + * ~ (e.g. ~/.ui5). Intended for console and + * error output only — never for values used in actual filesystem operations. + * + * @param {string} filePath Path to format for display + * @returns {string} The path with the home directory replaced by ~, or the + * original path when it does not reside within the home directory + */ +export function formatPath(filePath) { + const home = os.homedir(); + if (filePath === home) { + return "~"; + } + if (filePath.startsWith(home + path.sep)) { + return "~" + filePath.slice(home.length); + } + return filePath; +} diff --git a/packages/cli/lib/framework/utils.js b/packages/cli/lib/framework/utils.js index 799c8a35253..ddbab11ef00 100644 --- a/packages/cli/lib/framework/utils.js +++ b/packages/cli/lib/framework/utils.js @@ -1,6 +1,5 @@ -import path from "node:path"; import {graphFromStaticFile, graphFromPackageDependencies} from "@ui5/project/graph"; -import Configuration from "@ui5/project/config/Configuration"; +import {getUi5DataDir} from "../dataDir.js"; export async function getRootProjectConfiguration(projectGraphOptions) { let graph; @@ -49,16 +48,6 @@ export async function frameworkResolverResolveVersion({frameworkName, frameworkV }); } -async function getUi5DataDir({cwd}) { - // ENV var should take precedence over the dataDir from the configuration. - let ui5DataDir = process.env.UI5_DATA_DIR; - if (!ui5DataDir) { - const config = await Configuration.fromFile(); - ui5DataDir = config.getUi5DataDir(); - } - return ui5DataDir ? path.resolve(cwd, ui5DataDir) : undefined; -} - const utils = { getRootProjectConfiguration, getFrameworkResolver, diff --git a/packages/cli/test/lib/cli/commands/cache.js b/packages/cli/test/lib/cli/commands/cache.js index 2a423cdff4d..713cd4c789d 100644 --- a/packages/cli/test/lib/cli/commands/cache.js +++ b/packages/cli/test/lib/cli/commands/cache.js @@ -22,8 +22,9 @@ function getDefaultArgv() { }; } -// Stable absolute path used as the resolved ui5DataDir in most tests -const TEST_UI5_DATA_DIR = path.resolve("test-ui5-home"); +// Stable absolute path used as the resolved ui5DataDir in most tests. Anchored outside the home +// directory so path assertions are not affected by the ~ shortening applied to home-dir paths. +const TEST_UI5_DATA_DIR = path.join(path.resolve(path.sep), "test-ui5-home"); // Typical framework stub result shape: { path, libraryCount, versionCount } const FRAMEWORK_STUB = {path: "framework", libraryCount: 18, versionCount: 5}; @@ -45,10 +46,7 @@ test.beforeEach(async (t) => { // Prevent real env var from leaking into tests delete process.env.UI5_DATA_DIR; - t.context.configurationGetUi5DataDirStub = sinon.stub().returns(TEST_UI5_DATA_DIR); - t.context.configurationFromFileStub = sinon.stub().resolves({ - getUi5DataDir: t.context.configurationGetUi5DataDirStub, - }); + t.context.getUi5DataDirOrDefaultStub = sinon.stub().resolves(TEST_UI5_DATA_DIR); t.context.frameworkCacheGetCacheInfo = sinon.stub(); t.context.frameworkCacheCleanCache = sinon.stub(); @@ -62,11 +60,6 @@ test.beforeEach(async (t) => { t.context.yesnoStub = sinon.stub(); t.context.cache = await esmock.p("../../../../lib/cli/commands/cache.js", { - "@ui5/project/config/Configuration": { - default: { - fromFile: t.context.configurationFromFileStub, - }, - }, "@ui5/project/internal/ui5Framework/cache": { default: class { static getCacheInfo = t.context.frameworkCacheGetCacheInfo; @@ -86,6 +79,10 @@ test.beforeEach(async (t) => { "yesno": { default: t.context.yesnoStub, }, + }, { + "../../../../lib/dataDir.js": { + getUi5DataDirOrDefault: t.context.getUi5DataDirOrDefaultStub + } }); }); @@ -137,9 +134,9 @@ test.serial("Command definition is correct", (t) => { // ─── ui5DataDir resolution ────────────────────────────────────────────────── -test.serial("ui5 cache clean: uses resolved path from configuration", async (t) => { +test.serial("ui5 cache clean: uses resolved path from getUi5DataDirOrDefault", async (t) => { const {cache, argv, frameworkCacheGetCacheInfo, buildCacheGetCacheInfo, - stderrWriteStub, configurationFromFileStub, configurationGetUi5DataDirStub} = t.context; + stderrWriteStub, getUi5DataDirOrDefaultStub} = t.context; frameworkCacheGetCacheInfo.resolves(null); buildCacheGetCacheInfo.resolves(null); @@ -148,40 +145,21 @@ test.serial("ui5 cache clean: uses resolved path from configuration", async (t) setLogLevel("verbose"); await cache.handler(argv); - t.is(configurationFromFileStub.callCount, 1, "Configuration.fromFile called exactly once"); - t.is(configurationGetUi5DataDirStub.callCount, 1, "Configuration#getUi5DataDir called exactly once"); + t.is(getUi5DataDirOrDefaultStub.callCount, 1, "getUi5DataDirOrDefault called exactly once"); t.is(frameworkCacheGetCacheInfo.firstCall.args[0], TEST_UI5_DATA_DIR, - "getCacheInfo receives the path returned by configuration"); + "getCacheInfo receives the resolved path"); const allOutput = stderrWriteStub.args.map((a) => a[0]).join(""); t.true(allOutput.includes(TEST_UI5_DATA_DIR), "Resolved ui5DataDir shown in checking line"); }); -test.serial("ui5 cache clean: prefers UI5_DATA_DIR env var over configuration", async (t) => { - const {cache, argv, frameworkCacheGetCacheInfo, buildCacheGetCacheInfo, - configurationFromFileStub} = t.context; - - const envUi5DataDir = path.resolve("env-ui5-home"); - process.env.UI5_DATA_DIR = envUi5DataDir; - frameworkCacheGetCacheInfo.resolves(null); - buildCacheGetCacheInfo.resolves(null); - - argv["_"] = ["cache", "clean"]; - await cache.handler(argv); - - t.is(configurationFromFileStub.callCount, 0, - "Configuration.fromFile must not be called when UI5_DATA_DIR is set"); - t.is(frameworkCacheGetCacheInfo.firstCall.args[0], envUi5DataDir, - "getCacheInfo receives value from UI5_DATA_DIR"); -}); - -test.serial("ui5 cache clean: falls back to ~/.ui5 when configuration has no value", async (t) => { +test.serial("ui5 cache clean: uses ~/.ui5 fallback provided by getUi5DataDirOrDefault", async (t) => { const {cache, argv, frameworkCacheGetCacheInfo, buildCacheGetCacheInfo, - stderrWriteStub, configurationGetUi5DataDirStub} = t.context; + stderrWriteStub, getUi5DataDirOrDefaultStub} = t.context; const fallbackUi5DataDir = path.join(os.homedir(), ".ui5"); - configurationGetUi5DataDirStub.returns(undefined); + getUi5DataDirOrDefaultStub.resolves(fallbackUi5DataDir); frameworkCacheGetCacheInfo.resolves(null); buildCacheGetCacheInfo.resolves(null); @@ -193,7 +171,10 @@ test.serial("ui5 cache clean: falls back to ~/.ui5 when configuration has no val "getCacheInfo receives default ~/.ui5 path when no configured value exists"); const allOutput = stderrWriteStub.args.map((a) => a[0]).join(""); - t.true(allOutput.includes(fallbackUi5DataDir), "Fallback ui5DataDir shown in checking line"); + const shortenedDataDir = "~" + path.sep + ".ui5"; + t.true(allOutput.includes(shortenedDataDir), + "Fallback ui5DataDir shown with ~ in checking line"); + t.false(allOutput.includes(os.homedir()), "Full home directory is not printed"); }); // ─── Basic flow ───────────────────────────────────────────────────────────── diff --git a/packages/cli/test/lib/dataDir.js b/packages/cli/test/lib/dataDir.js new file mode 100644 index 00000000000..a8ca6c40136 --- /dev/null +++ b/packages/cli/test/lib/dataDir.js @@ -0,0 +1,130 @@ +import test from "ava"; +import sinonGlobal from "sinon"; +import esmock from "esmock"; +import path from "node:path"; +import os from "node:os"; + +test.beforeEach(async (t) => { + // Tests either rely on not having UI5_DATA_DIR defined, or explicitly define it + t.context.originalUi5DataDirEnv = process.env.UI5_DATA_DIR; + delete process.env.UI5_DATA_DIR; + + const sinon = t.context.sinon = sinonGlobal.createSandbox(); + + t.context.ConfigurationGetUi5DataDirStub = sinon.stub().returns(undefined); + t.context.ConfigurationStub = { + fromFile: sinon.stub().resolves({ + getUi5DataDir: t.context.ConfigurationGetUi5DataDirStub + }) + }; + + t.context.dataDir = await esmock.p("../../lib/dataDir.js", { + "@ui5/project/config/Configuration": t.context.ConfigurationStub + }); +}); + +test.afterEach.always((t) => { + if (typeof t.context.originalUi5DataDirEnv === "undefined") { + delete process.env.UI5_DATA_DIR; + } else { + process.env.UI5_DATA_DIR = t.context.originalUi5DataDirEnv; + } + t.context.sinon.restore(); + esmock.purge(t.context.dataDir); +}); + +test.serial("getUi5DataDir: no value defined", async (t) => { + const {ConfigurationGetUi5DataDirStub, dataDir} = t.context; + + const result = await dataDir.getUi5DataDir({ + cwd: path.resolve("foo") + }); + + t.is(result, undefined); + + t.is(ConfigurationGetUi5DataDirStub.callCount, 1); +}); + +test.serial("getUi5DataDir: from environment variable", async (t) => { + const {ConfigurationGetUi5DataDirStub, dataDir} = t.context; + + // Environment variable must be preferred over configuration value + ConfigurationGetUi5DataDirStub.returns(".ui5-data-dir-from-configuration"); + process.env.UI5_DATA_DIR = ".ui5-data-dir-from-env-variable"; + + const result = await dataDir.getUi5DataDir({ + cwd: path.resolve("foo") + }); + + t.is(result, path.join(path.resolve("foo"), ".ui5-data-dir-from-env-variable")); + + t.is(ConfigurationGetUi5DataDirStub.callCount, 0); +}); + +test.serial("getUi5DataDir: from Configuration", async (t) => { + const {ConfigurationGetUi5DataDirStub, dataDir} = t.context; + + ConfigurationGetUi5DataDirStub.returns(".ui5-data-dir-from-configuration"); + + const result = await dataDir.getUi5DataDir({ + cwd: path.resolve("foo") + }); + + t.is(result, path.join(path.resolve("foo"), ".ui5-data-dir-from-configuration")); + + t.is(ConfigurationGetUi5DataDirStub.callCount, 1); +}); + +test.serial("getUi5DataDirOrDefault: returns resolved value when configured", async (t) => { + const {ConfigurationGetUi5DataDirStub, dataDir} = t.context; + + ConfigurationGetUi5DataDirStub.returns(".ui5-data-dir-from-configuration"); + + const result = await dataDir.getUi5DataDirOrDefault({ + cwd: path.resolve("foo") + }); + + t.is(result, path.join(path.resolve("foo"), ".ui5-data-dir-from-configuration")); +}); + +test.serial("getUi5DataDirOrDefault: falls back to ~/.ui5 when no value defined", async (t) => { + const {dataDir} = t.context; + + const result = await dataDir.getUi5DataDirOrDefault({ + cwd: path.resolve("foo") + }); + + t.is(result, path.join(os.homedir(), ".ui5")); +}); + +test.serial("formatPath: replaces home directory with ~", (t) => { + const {dataDir} = t.context; + + const input = path.join(os.homedir(), ".ui5", "server", "server.key"); + const expected = "~" + path.sep + path.join(".ui5", "server", "server.key"); + + t.is(dataDir.formatPath(input), expected); +}); + +test.serial("formatPath: returns ~ for the home directory itself", (t) => { + const {dataDir} = t.context; + + t.is(dataDir.formatPath(os.homedir()), "~"); +}); + +test.serial("formatPath: leaves paths outside the home directory unchanged", (t) => { + const {dataDir} = t.context; + + const input = path.join(path.resolve(path.sep), "custom", "data-dir"); + + t.is(dataDir.formatPath(input), input); +}); + +test.serial("formatPath: does not shorten a sibling directory sharing the home prefix", (t) => { + const {dataDir} = t.context; + + // A path like "-backup" must not be treated as residing within the home directory. + const input = os.homedir() + "-backup"; + + t.is(dataDir.formatPath(input), input); +}); diff --git a/packages/cli/test/lib/framework/utils.js b/packages/cli/test/lib/framework/utils.js index 0ac6f4aaf37..88cefd1f583 100644 --- a/packages/cli/test/lib/framework/utils.js +++ b/packages/cli/test/lib/framework/utils.js @@ -1,7 +1,6 @@ import test from "ava"; import sinonGlobal from "sinon"; import esmock from "esmock"; -import path from "node:path"; test.beforeEach(async (t) => { // Tests either rely on not having UI5_DATA_DIR defined, or explicitly define it @@ -250,48 +249,3 @@ test.serial("frameworkResolverResolveVersion", async (t) => { } ]); }); - -test.serial("getUi5DataDir: no value defined", async (t) => { - const {ConfigurationGetUi5DataDirStub} = t.context; - const {getUi5DataDir} = t.context._utils; - - const result = await getUi5DataDir({ - cwd: path.resolve("foo") - }); - - t.is(result, undefined); - - t.is(ConfigurationGetUi5DataDirStub.callCount, 1); -}); - -test.serial("getUi5DataDir: from environment variable", async (t) => { - const {ConfigurationGetUi5DataDirStub} = t.context; - const {getUi5DataDir} = t.context._utils; - - // Environment variable must be preferred over configuration value - ConfigurationGetUi5DataDirStub.returns(".ui5-data-dir-from-configuration"); - process.env.UI5_DATA_DIR = ".ui5-data-dir-from-env-variable"; - - const result = await getUi5DataDir({ - cwd: path.resolve("foo") - }); - - t.is(result, path.join(path.resolve("foo"), ".ui5-data-dir-from-env-variable")); - - t.is(ConfigurationGetUi5DataDirStub.callCount, 0); -}); - -test.serial("getUi5DataDir: from Configuration", async (t) => { - const {ConfigurationGetUi5DataDirStub} = t.context; - const {getUi5DataDir} = t.context._utils; - - ConfigurationGetUi5DataDirStub.returns(".ui5-data-dir-from-configuration"); - - const result = await getUi5DataDir({ - cwd: path.resolve("foo") - }); - - t.is(result, path.join(path.resolve("foo"), ".ui5-data-dir-from-configuration")); - - t.is(ConfigurationGetUi5DataDirStub.callCount, 1); -});