[INTERNAL] Apply feedback from code review

lib/graph/graph.js

@@ -32,7 +32,7 @@ const log = getLogger("generateProjectGraph");
  * @param {string} [options.workspaceName]
  * 		Name of the workspace configuration that should be used if any is provided
  * @param {module:@ui5/project/ui5Framework/maven/CacheMode} [options.cacheMode]
- *      Cache mode to use when consuming SNAPSHOT versions
+ *      Cache mode to use when consuming SNAPSHOT versions of a framework
  * @param {string} [options.workspaceConfigPath=ui5-workspace.yaml]
  * 		Workspace configuration file to use if no object has been provided
  * @param {@ui5/project/graph/Workspace~Configuration} [options.workspaceConfiguration]

lib/graph/helpers/ui5Framework.js

@@ -288,7 +288,7 @@ export default {
 	 *   Promise resolving with the given graph instance to allow method chaining
 	 */
 	enrichProjectGraph: async function(projectGraph, options = {}) {
-		const {workspace} = options;
+		const {workspace, cacheMode} = options;
 		const rootProject = projectGraph.getRoot();
 		const frameworkName = rootProject.getFrameworkName();
 		const frameworkVersion = rootProject.getFrameworkVersion();
@@ -369,7 +369,7 @@ export default {
 			cwd: rootProject.getRootPath(),
 			version,
 			providedLibraryMetadata,
-			cacheMode: options.cacheMode
+			cacheMode
 		});
 
 		let startTime;

lib/ui5Framework/Sapui5MavenSnapshotResolver.js

@@ -136,11 +136,10 @@ class Sapui5MavenSnapshotResolver extends AbstractResolver {
 		snapshotEndpointUrl = process.env.UI5_MAVEN_SNAPSHOT_ENDPOINT_URL || snapshotEndpointUrl;
 
 		if (!snapshotEndpointUrl) {
-			// Return an unexecuted promise instead of the resolved URL here.
-			// If we resolve the settings.xml at this point, we'd need to always ask the end
-			// user for confirmation. In some cases where the resources are already cached,
-			// this is not necessary and we could skip it as a real request to the repository won't
-			// be made.
+			// Here we return a function which returns a promise that resolves with the URL.
+			// If we would already start resolving the settings.xml at this point, we'd need to always ask the
+			// end user for confirmation whether the resolved URL should be used. In some cases where the resources
+			// are already cached, this is actually not necessary and could be skipped
 			return Sapui5MavenSnapshotResolver._resolveSnapshotEndpointUrl;
 		} else {
 			return () => Promise.resolve(snapshotEndpointUrl);

test/lib/graph/graph.integration.js

@@ -4,6 +4,7 @@ import path from "node:path";
 import sinonGlobal from "sinon";
 import esmock from "esmock";
 import Workspace from "../../../lib/graph/Workspace.js";
+import CacheMode from "../../../lib/ui5Framework/maven/CacheMode.js";
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
 const fixturesPath = path.join(__dirname, "..", "..", "fixtures");
@@ -254,7 +255,7 @@ test.serial("graphFromPackageDependencies with inactive workspace file at custom
 		versionOverride: "versionOverride",
 		workspaceName: "default",
 		workspaceConfigPath: path.join(libraryHPath, "custom-ui5-workspace.yaml"),
-		cacheMode: "force"
+		cacheMode: CacheMode.Force
 	});
 
 	t.is(res, "graph");
@@ -278,6 +279,6 @@ test.serial("graphFromPackageDependencies with inactive workspace file at custom
 	t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
 		versionOverride: "versionOverride",
 		workspace: null,
-		cacheMode: "force"
+		cacheMode: "Force"
 	}, "enrichProjectGraph got called with correct options");
 });

test/lib/graph/graph.js

@@ -3,6 +3,7 @@ import {fileURLToPath} from "node:url";
 import path from "node:path";
 import sinonGlobal from "sinon";
 import esmock from "esmock";
+import CacheMode from "../../../lib/ui5Framework/maven/CacheMode.js";
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 const fixturesPath = path.join(__dirname, "..", "..", "fixtures");
@@ -58,7 +59,7 @@ test.serial("graphFromPackageDependencies", async (t) => {
 		rootConfiguration: "rootConfiguration",
 		rootConfigPath: "/rootConfigPath",
 		versionOverride: "versionOverride",
-		cacheMode: "off"
+		cacheMode: CacheMode.Off
 	});
 
 	t.is(res, "graph");
@@ -83,7 +84,7 @@ test.serial("graphFromPackageDependencies", async (t) => {
 	t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
 		versionOverride: "versionOverride",
 		workspace: undefined,
-		cacheMode: "off"
+		cacheMode: "Off"
 	}, "enrichProjectGraph got called with correct options");
 });
 
@@ -100,7 +101,7 @@ test.serial("graphFromPackageDependencies with workspace name", async (t) => {
 		rootConfigPath: "/rootConfigPath",
 		versionOverride: "versionOverride",
 		workspaceName: "dolphin",
-		cacheMode: "off"
+		cacheMode: CacheMode.Off
 	});
 
 	t.is(res, "graph");
@@ -132,7 +133,7 @@ test.serial("graphFromPackageDependencies with workspace name", async (t) => {
 	t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
 		versionOverride: "versionOverride",
 		workspace: "workspace",
-		cacheMode: "off"
+		cacheMode: "Off"
 	}, "enrichProjectGraph got called with correct options");
 });
 
@@ -229,7 +230,7 @@ test.serial("graphFromPackageDependencies with empty workspace", async (t) => {
 		rootConfigPath: "/rootConfigPath",
 		versionOverride: "versionOverride",
 		workspaceName: "dolphin",
-		cacheMode: "off"
+		cacheMode: CacheMode.Off
 	});
 
 	t.is(res, "graph");
@@ -261,7 +262,7 @@ test.serial("graphFromPackageDependencies with empty workspace", async (t) => {
 	t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
 		versionOverride: "versionOverride",
 		workspace: null,
-		cacheMode: "off"
+		cacheMode: "Off"
 	}, "enrichProjectGraph got called with correct options");
 });
 
@@ -297,7 +298,7 @@ test.serial("graphFromStaticFile", async (t) => {
 		rootConfiguration: "rootConfiguration",
 		rootConfigPath: "/rootConfigPath",
 		versionOverride: "versionOverride",
-		cacheMode: "off"
+		cacheMode: CacheMode.Off
 	});
 
 	t.is(res, "graph");
@@ -324,7 +325,7 @@ test.serial("graphFromStaticFile", async (t) => {
 		"enrichProjectGraph got called with graph");
 	t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
 		versionOverride: "versionOverride",
-		cacheMode: "off"
+		cacheMode: "Off"
 	}, "enrichProjectGraph got called with correct options");
 });
 
@@ -360,7 +361,7 @@ test.serial("usingObject", async (t) => {
 		rootConfiguration: "rootConfiguration",
 		rootConfigPath: "/rootConfigPath",
 		versionOverride: "versionOverride",
-		cacheMode: "off"
+		cacheMode: "Off"
 	});
 
 	t.is(res, "graph");
@@ -381,7 +382,7 @@ test.serial("usingObject", async (t) => {
 		"enrichProjectGraph got called with graph");
 	t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
 		versionOverride: "versionOverride",
-		cacheMode: "off"
+		cacheMode: "Off"
 	}, "enrichProjectGraph got called with correct options");
 });
 

test/lib/graph/helpers/ui5Framework.js

@@ -6,6 +6,7 @@ import esmock from "esmock";
 import DependencyTreeProvider from "../../../../lib/graph/providers/DependencyTree.js";
 import projectGraphBuilder from "../../../../lib/graph/projectGraphBuilder.js";
 import Specification from "../../../../lib/specifications/Specification.js";
+import CacheMode from "../../../../lib/ui5Framework/maven/CacheMode.js";
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
@@ -218,7 +219,7 @@ test.serial("enrichProjectGraph SNAPSHOT", async (t) => {
 	const projectGraph = await projectGraphBuilder(provider);
 
 	await ui5Framework.enrichProjectGraph(projectGraph, {
-		cacheMode: "force"
+		cacheMode: CacheMode.Force
 	});
 
 	t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGraph should be called once");