Skip to content

Commit e45d5ff

Browse files
committed
refactor: Cleanup confirmation revised
1 parent 8172c6f commit e45d5ff

4 files changed

Lines changed: 234 additions & 115 deletions

File tree

packages/cli/lib/cli/commands/cache.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,19 @@ function formatSize(bytes) {
6161
}
6262

6363
/**
64-
* Format a count with its singular/plural word, e.g. "340 files" or "1 file".
64+
* Format a library stats detail string, e.g. "2 projects, 3 libraries, 4 versions".
65+
* Each word is independently singular/plural.
6566
*
66-
* @param {number} count
67+
* @param {number} libraryCount
68+
* @param {number} projectCount
69+
* @param {number} versionCount
6770
* @returns {string}
6871
*/
69-
function formatFileCount(count) {
70-
return `${count} ${count === 1 ? "file" : "files"}`;
72+
function formatLibraryStats(libraryCount, projectCount, versionCount) {
73+
const p = `${projectCount} ${projectCount === 1 ? "project" : "projects"}`;
74+
const l = `${libraryCount} ${libraryCount === 1 ? "library" : "libraries"}`;
75+
const v = `${versionCount} ${versionCount === 1 ? "version" : "versions"}`;
76+
return `${p}, ${l}, ${v}`;
7177
}
7278

7379
/**
@@ -97,7 +103,7 @@ async function handleCache(argv) {
97103
return;
98104
}
99105

100-
// Inform the user immediately — getCacheInfo (especially countFiles) may take a moment
106+
// Inform the user immediately — getPackageStats may take a moment on a large cache
101107
process.stderr.write(`Checking cache at ${chalk.bold(ui5DataDir)} …\n`);
102108

103109
// Check what items exist before cleaning (orchestrate both domains)
@@ -120,7 +126,8 @@ async function handleCache(argv) {
120126
// Display items that will be removed
121127
process.stderr.write(chalk.bold("\nThe following cached data will be removed:\n\n"));
122128
if (frameworkInfo) {
123-
const detail = formatFileCount(frameworkInfo.count);
129+
const detail = formatLibraryStats(
130+
frameworkInfo.libraryCount, frameworkInfo.projectCount, frameworkInfo.versionCount);
124131
process.stderr.write(
125132
` ${chalk.yellow("•")} ${padLabel(LABEL_FRAMEWORK)} ${frameworkAbsPath} (${detail})\n`
126133
);
@@ -152,7 +159,8 @@ async function handleCache(argv) {
152159

153160
process.stderr.write("\n");
154161
if (frameworkResult) {
155-
const detail = formatFileCount(frameworkResult.count);
162+
const detail = formatLibraryStats(
163+
frameworkResult.libraryCount, frameworkResult.projectCount, frameworkResult.versionCount);
156164
process.stderr.write(
157165
`${chalk.green("✓")} Removed ${chalk.bold(LABEL_FRAMEWORK)}` +
158166
` (${frameworkAbsPath} · ${detail})\n`

packages/cli/test/lib/cli/commands/cache.js

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ function getDefaultArgv() {
1919
// Stable absolute path used as the resolved ui5DataDir in most tests
2020
const TEST_UI5_DATA_DIR = path.resolve("/test/ui5/home");
2121

22+
// Typical framework stub result shape
23+
const FRAMEWORK_STUB = {path: "framework", projectCount: 2, libraryCount: 18, versionCount: 5};
24+
2225
test.beforeEach(async (t) => {
2326
t.context.argv = getDefaultArgv();
2427
t.context.stderrWriteStub = sinon.stub(process.stderr, "write");
@@ -108,7 +111,6 @@ test.serial("ui5 cache clean: falls back to ~/.ui5 when getUi5DataDir returns un
108111
const {cache, argv, getUi5DataDirStub, frameworkCacheGetCacheInfo, buildCacheGetCacheInfo,
109112
stderrWriteStub} = t.context;
110113

111-
// Simulate no env var, no config — getUi5DataDir returns undefined
112114
getUi5DataDirStub.resolves(undefined);
113115
frameworkCacheGetCacheInfo.resolves(null);
114116
buildCacheGetCacheInfo.resolves(null);
@@ -118,11 +120,7 @@ test.serial("ui5 cache clean: falls back to ~/.ui5 when getUi5DataDir returns un
118120

119121
const expectedDefault = path.join(os.homedir(), ".ui5");
120122
const allOutput = stderrWriteStub.args.map((a) => a[0]).join("");
121-
t.true(allOutput.includes(expectedDefault),
122-
"Falls back to ~/.ui5 and shows it in checking line");
123-
124-
// getCacheInfo called with the default path
125-
t.is(frameworkCacheGetCacheInfo.callCount, 1, "getCacheInfo called");
123+
t.true(allOutput.includes(expectedDefault), "Falls back to ~/.ui5 and shows it in checking line");
126124
t.is(frameworkCacheGetCacheInfo.firstCall.args[0], expectedDefault,
127125
"getCacheInfo receives ~/.ui5 as ui5DataDir");
128126
});
@@ -136,18 +134,14 @@ test.serial("ui5 cache clean: uses resolved path from getUi5DataDir", async (t)
136134
argv["_"] = ["cache", "clean"];
137135
await cache.handler(argv);
138136

139-
// The stub returns TEST_UI5_DATA_DIR — verify it was passed to getCacheInfo
140137
t.is(frameworkCacheGetCacheInfo.firstCall.args[0], TEST_UI5_DATA_DIR,
141138
"getCacheInfo receives the path returned by getUi5DataDir");
142139

143140
const allOutput = stderrWriteStub.args.map((a) => a[0]).join("");
144-
t.true(allOutput.includes(TEST_UI5_DATA_DIR),
145-
"Resolved ui5DataDir shown in checking line");
141+
t.true(allOutput.includes(TEST_UI5_DATA_DIR), "Resolved ui5DataDir shown in checking line");
146142
});
147143

148144
test.serial("ui5 cache clean: relative path from config is resolved via getUi5DataDir", async (t) => {
149-
// getUi5DataDir already resolves relative paths against cwd — verify the cache
150-
// command uses the already-resolved absolute path rather than doing its own resolution.
151145
const {cache, argv, getUi5DataDirStub, frameworkCacheGetCacheInfo, buildCacheGetCacheInfo} = t.context;
152146

153147
const resolvedPath = path.resolve(process.cwd(), "./custom-cache");
@@ -177,20 +171,20 @@ test.serial("ui5 cache clean: nothing to clean", async (t) => {
177171
const allOutput = stderrWriteStub.args.map((a) => a[0]).join("");
178172
t.true(allOutput.includes("Checking cache at"), "Prints checking line");
179173
t.true(allOutput.includes("Nothing to clean"), "Prints nothing to clean");
180-
t.is(frameworkCacheCleanCache.callCount, 0, "frameworkCache.cleanCache should not be called");
181-
t.is(buildCacheCleanCache.callCount, 0, "buildCache.cleanCache should not be called");
174+
t.is(frameworkCacheCleanCache.callCount, 0, "frameworkCache.cleanCache not called");
175+
t.is(buildCacheCleanCache.callCount, 0, "buildCache.cleanCache not called");
182176
});
183177

184178
test.serial("ui5 cache clean: removes both entries and reports", async (t) => {
185179
const {cache, argv, stderrWriteStub, frameworkCacheCleanCache, frameworkCacheGetCacheInfo,
186180
buildCacheCleanCache, buildCacheGetCacheInfo, yesnoStub} = t.context;
187181

188-
frameworkCacheGetCacheInfo.resolves({path: "framework", count: 340});
182+
frameworkCacheGetCacheInfo.resolves(FRAMEWORK_STUB);
189183
buildCacheGetCacheInfo.resolves({path: "buildCache/v0_7", size: 8 * 1024 * 1024});
190184

191185
yesnoStub.resolves(true);
192186

193-
frameworkCacheCleanCache.resolves({path: "framework", count: 340});
187+
frameworkCacheCleanCache.resolves(FRAMEWORK_STUB);
194188
buildCacheCleanCache.resolves({path: "buildCache/v0_7", size: 7 * 1024 * 1024}); // VACUUM freed less
195189

196190
argv["_"] = ["cache", "clean"];
@@ -202,25 +196,26 @@ test.serial("ui5 cache clean: removes both entries and reports", async (t) => {
202196

203197
const allOutput = stderrWriteStub.args.map((a) => a[0]).join("");
204198

205-
// Checking line with absolute path
199+
// Checking line
206200
t.true(allOutput.includes("Checking cache at"), "Prints checking line");
207201
t.true(allOutput.includes(TEST_UI5_DATA_DIR), "Shows resolved ui5DataDir");
208202

209-
// Listing shows absolute paths
203+
// Absolute paths in listing
210204
const expectedFrameworkAbs = path.join(TEST_UI5_DATA_DIR, "framework");
211205
const expectedBuildAbs = path.join(TEST_UI5_DATA_DIR, "buildCache/v0_7");
212206
t.true(allOutput.includes(expectedFrameworkAbs), "Shows absolute framework path");
213207
t.true(allOutput.includes(expectedBuildAbs), "Shows absolute build cache path");
214208

215-
// Labels and detail
216-
t.true(allOutput.includes("UI5 Framework packages"), "Shows framework label");
217-
t.true(allOutput.includes("Build cache (DB)"), "Shows build cache label");
218-
t.true(allOutput.includes("340 files"), "Shows framework file count");
219-
t.true(allOutput.includes("8.0 MB"), "Shows build cache pre-clean size");
220-
t.false(allOutput.includes("7.0 MB"), "Does not show VACUUM-freed size (pre-clean size reused)");
221-
t.false(allOutput.includes("Total:"), "Does not show total line");
209+
// Framework detail: projects, libraries, versions
210+
t.true(allOutput.includes("2 projects"), "Shows project count");
211+
t.true(allOutput.includes("18 libraries"), "Shows library count");
212+
t.true(allOutput.includes("5 versions"), "Shows version count");
222213

223-
// Success
214+
// Build cache detail: pre-clean size reused (not VACUUM-freed 7 MB)
215+
t.true(allOutput.includes("8.0 MB"), "Shows pre-clean build cache size");
216+
t.false(allOutput.includes("7.0 MB"), "Does not show VACUUM-freed size");
217+
218+
t.false(allOutput.includes("Total:"), "Does not show total line");
224219
t.true(allOutput.includes("Cleaned UI5 Framework packages and Build cache (DB)"),
225220
"Shows success summary");
226221
});
@@ -229,9 +224,8 @@ test.serial("ui5 cache clean: user cancels", async (t) => {
229224
const {cache, argv, stderrWriteStub, frameworkCacheCleanCache, frameworkCacheGetCacheInfo,
230225
buildCacheCleanCache, buildCacheGetCacheInfo, yesnoStub} = t.context;
231226

232-
frameworkCacheGetCacheInfo.resolves({path: "framework", count: 10});
227+
frameworkCacheGetCacheInfo.resolves(FRAMEWORK_STUB);
233228
buildCacheGetCacheInfo.resolves(null);
234-
235229
yesnoStub.resolves(false);
236230

237231
argv["_"] = ["cache", "clean"];
@@ -246,24 +240,45 @@ test.serial("ui5 cache clean: user cancels", async (t) => {
246240
t.false(allOutput.includes("Success"), "Does not show success message");
247241
});
248242

249-
test.serial("ui5 cache clean: framework only", async (t) => {
243+
test.serial("ui5 cache clean: framework only — singular labels", async (t) => {
250244
const {cache, argv, stderrWriteStub, frameworkCacheCleanCache, frameworkCacheGetCacheInfo,
251245
buildCacheGetCacheInfo, yesnoStub} = t.context;
252246

253-
frameworkCacheGetCacheInfo.resolves({path: "framework", count: 1});
247+
const singleStub = {path: "framework", projectCount: 1, libraryCount: 1, versionCount: 1};
248+
frameworkCacheGetCacheInfo.resolves(singleStub);
254249
buildCacheGetCacheInfo.resolves(null);
255250
yesnoStub.resolves(true);
256-
frameworkCacheCleanCache.resolves({path: "framework", count: 1});
251+
frameworkCacheCleanCache.resolves(singleStub);
257252

258253
argv["_"] = ["cache", "clean"];
259254
await cache.handler(argv);
260255

261256
const allOutput = stderrWriteStub.args.map((a) => a[0]).join("");
262-
t.true(allOutput.includes("1 file"), "Uses singular 'file'");
257+
t.true(allOutput.includes("1 project,"), "Uses singular 'project'");
258+
t.true(allOutput.includes("1 library,"), "Uses singular 'library'");
259+
t.true(allOutput.includes("1 version"), "Uses singular 'version'");
263260
t.false(allOutput.includes("Build cache (DB)"), "Does not mention build cache");
264261
t.true(allOutput.includes("Cleaned UI5 Framework packages"), "Success mentions framework only");
265262
});
266263

264+
test.serial("ui5 cache clean: framework only — plural labels", async (t) => {
265+
const {cache, argv, stderrWriteStub, frameworkCacheCleanCache, frameworkCacheGetCacheInfo,
266+
buildCacheGetCacheInfo, yesnoStub} = t.context;
267+
268+
frameworkCacheGetCacheInfo.resolves(FRAMEWORK_STUB);
269+
buildCacheGetCacheInfo.resolves(null);
270+
yesnoStub.resolves(true);
271+
frameworkCacheCleanCache.resolves(FRAMEWORK_STUB);
272+
273+
argv["_"] = ["cache", "clean"];
274+
await cache.handler(argv);
275+
276+
const allOutput = stderrWriteStub.args.map((a) => a[0]).join("");
277+
t.true(allOutput.includes("2 projects"), "Uses plural 'projects'");
278+
t.true(allOutput.includes("18 libraries"), "Uses plural 'libraries'");
279+
t.true(allOutput.includes("5 versions"), "Uses plural 'versions'");
280+
});
281+
267282
test.serial("ui5 cache clean: build only", async (t) => {
268283
const {cache, argv, stderrWriteStub,
269284
buildCacheCleanCache, buildCacheGetCacheInfo, yesnoStub} = t.context;
@@ -317,9 +332,9 @@ test.serial("ui5 cache clean --yes: skips confirmation prompt", async (t) => {
317332
const {cache, argv, stderrWriteStub, frameworkCacheCleanCache, frameworkCacheGetCacheInfo,
318333
buildCacheCleanCache, buildCacheGetCacheInfo, yesnoStub} = t.context;
319334

320-
frameworkCacheGetCacheInfo.resolves({path: "framework", count: 100});
335+
frameworkCacheGetCacheInfo.resolves(FRAMEWORK_STUB);
321336
buildCacheGetCacheInfo.resolves({path: "buildCache/v0_7", size: 5 * 1024 * 1024});
322-
frameworkCacheCleanCache.resolves({path: "framework", count: 100});
337+
frameworkCacheCleanCache.resolves(FRAMEWORK_STUB);
323338
buildCacheCleanCache.resolves({path: "buildCache/v0_7", size: 5 * 1024 * 1024});
324339

325340
argv["_"] = ["cache", "clean"];

packages/project/lib/ui5Framework/cache.js

Lines changed: 80 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,51 +8,91 @@ import {
88
} from "./_frameworkPaths.js";
99

1010
/**
11-
* Count all files in a directory tree recursively.
12-
* Returns 0 if the directory does not exist or any entry is unreadable.
11+
* Count unique projects, libraries, and versions in the packages/ subdirectory.
12+
* Uses a 3-level readdir walk (project → library → version) with no recursion into
13+
* package contents. Inner levels are parallelised with Promise.all to avoid serial
14+
* I/O on large caches.
1315
*
14-
* @param {string} dirPath Absolute path to directory
15-
* @returns {Promise<number>} Total file count
16+
*
17+
* @param {string} packagesDir Absolute path to the packages directory
18+
* @returns {Promise<{projects: number, libraries: number, versions: number}|null>}
19+
* Null if the directory does not exist or contains no installed libraries.
1620
*/
17-
async function countFiles(dirPath) {
18-
let total = 0;
19-
let entries;
21+
async function getPackageStats(packagesDir) {
22+
let projectDirs;
2023
try {
21-
entries = await fs.readdir(dirPath, {withFileTypes: true});
24+
projectDirs = await fs.readdir(packagesDir, {withFileTypes: true});
2225
} catch {
23-
return 0;
26+
return null;
2427
}
25-
for (const entry of entries) {
26-
if (entry.isDirectory()) {
27-
total += await countFiles(path.join(dirPath, entry.name));
28-
} else {
29-
total++;
28+
29+
const librarySet = new Set();
30+
const versionSet = new Set();
31+
let totalProjects = 0;
32+
33+
await Promise.all(projectDirs.filter((e) => e.isDirectory()).map(async (project) => {
34+
let libDirs;
35+
try {
36+
libDirs = await fs.readdir(
37+
path.join(packagesDir, project.name), {withFileTypes: true});
38+
} catch {
39+
return;
3040
}
31-
}
32-
return total;
41+
42+
let projectHasLibs = false;
43+
await Promise.all(libDirs.filter((e) => e.isDirectory()).map(async (lib) => {
44+
let versionDirs;
45+
try {
46+
versionDirs = await fs.readdir(
47+
path.join(packagesDir, project.name, lib.name), {withFileTypes: true});
48+
} catch {
49+
return;
50+
}
51+
const installedVersions = versionDirs.filter((v) => v.isDirectory());
52+
if (installedVersions.length > 0) {
53+
librarySet.add(lib.name); // deduplicated: sap.m counts once across all projects
54+
projectHasLibs = true;
55+
for (const v of installedVersions) {
56+
versionSet.add(v.name);
57+
}
58+
}
59+
}));
60+
61+
if (projectHasLibs) {
62+
totalProjects++;
63+
}
64+
}));
65+
66+
return librarySet.size > 0
67+
? {projects: totalProjects, libraries: librarySet.size, versions: versionSet.size}
68+
: null;
3369
}
3470

3571
/**
3672
* Get framework cache info.
3773
*
3874
* @param {string} ui5DataDir Resolved absolute path to UI5 data directory
39-
* @returns {Promise<{path: string, count: number}|null>} Framework cache info or null
75+
* @returns {Promise<{path: string, libraryCount: number, projectCount: number, versionCount: number}|null>}
76+
* Framework cache info, or null if no packages are installed.
4077
*/
4178
export async function getCacheInfo(ui5DataDir) {
4279
const frameworkDir = getFrameworkDir(ui5DataDir);
4380
try {
4481
await fs.access(frameworkDir);
45-
const count = await countFiles(frameworkDir);
46-
if (count > 0) {
47-
return {
48-
path: FRAMEWORK_DIR_NAME,
49-
count,
50-
};
51-
}
5282
} catch {
53-
// Directory doesn't exist
83+
return null;
84+
}
85+
86+
const stats = await getPackageStats(path.join(frameworkDir, "packages"));
87+
if (!stats) {
88+
return null;
5489
}
55-
return null;
90+
return {
91+
path: FRAMEWORK_DIR_NAME,
92+
libraryCount: stats.libraries,
93+
projectCount: stats.projects,
94+
versionCount: stats.versions,
95+
};
5696
}
5797

5898
/**
@@ -73,13 +113,21 @@ export async function isFrameworkLocked(ui5DataDir) {
73113
* deleting files while a download is in progress.
74114
*
75115
* @param {string} ui5DataDir Resolved absolute path to UI5 data directory
76-
* @returns {Promise<{path: string, count: number}|null>} Removal result or null
116+
* @returns {Promise<{path: string, libraryCount: number, projectCount: number, versionCount: number}|null>}
117+
* Removal result, or null if nothing was installed.
77118
* @throws {Error} If a framework operation is currently active (active lockfiles detected)
78119
*/
79120
export async function cleanCache(ui5DataDir) {
80121
const frameworkDir = getFrameworkDir(ui5DataDir);
81-
const count = await countFiles(frameworkDir);
82-
if (count === 0) {
122+
123+
try {
124+
await fs.access(frameworkDir);
125+
} catch {
126+
return null;
127+
}
128+
129+
const stats = await getPackageStats(path.join(frameworkDir, "packages"));
130+
if (!stats) {
83131
return null;
84132
}
85133

@@ -93,6 +141,8 @@ export async function cleanCache(ui5DataDir) {
93141
await fs.rm(frameworkDir, {recursive: true, force: true});
94142
return {
95143
path: FRAMEWORK_DIR_NAME,
96-
count,
144+
libraryCount: stats.libraries,
145+
projectCount: stats.projects,
146+
versionCount: stats.versions,
97147
};
98148
}

0 commit comments

Comments
 (0)