Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Sapui5MavenSnapshotResolver: Expose cacheMode parameter through all APIs #607

Merged
merged 3 commits into from May 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 12 additions & 6 deletions lib/graph/graph.js
Expand Up @@ -31,6 +31,8 @@ const log = getLogger("generateProjectGraph");
* Whether framework dependencies should be added to the graph
* @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
RandomByte marked this conversation as resolved.
Show resolved Hide resolved
* @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]
Expand All @@ -40,7 +42,7 @@ const log = getLogger("generateProjectGraph");
*/
export async function graphFromPackageDependencies({
cwd, rootConfiguration, rootConfigPath,
versionOverride, resolveFrameworkDependencies = true,
versionOverride, cacheMode, resolveFrameworkDependencies = true,
workspaceName /* TODO 4.0: default workspaceName to "default" ? */,
workspaceConfiguration, workspaceConfigPath = "ui5-workspace.yaml"
}) {
Expand Down Expand Up @@ -71,7 +73,7 @@ export async function graphFromPackageDependencies({
const projectGraph = await projectGraphBuilder(provider, workspace);

if (resolveFrameworkDependencies) {
await ui5Framework.enrichProjectGraph(projectGraph, {versionOverride, workspace});
await ui5Framework.enrichProjectGraph(projectGraph, {versionOverride, cacheMode, workspace});
}

return projectGraph;
Expand All @@ -93,14 +95,16 @@ export async function graphFromPackageDependencies({
* Configuration file to use for the root module instead the default ui5.yaml. Either a path relative to
* <code>cwd</code> or an absolute path. In both case, platform-specific path segment separators must be used.
* @param {string} [options.versionOverride] Framework version to use instead of the one defined in the root project
* @param {module:@ui5/project/ui5Framework/maven/CacheMode} [options.cacheMode]
* Cache mode to use when consuming SNAPSHOT versions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* @param {string} [options.resolveFrameworkDependencies=true]
* Whether framework dependencies should be added to the graph
* @returns {Promise<@ui5/project/graph/ProjectGraph>} Promise resolving to a Project Graph instance
*/
export async function graphFromStaticFile({
filePath = "projectDependencies.yaml", cwd,
rootConfiguration, rootConfigPath,
versionOverride, resolveFrameworkDependencies = true
versionOverride, cacheMode, resolveFrameworkDependencies = true
}) {
log.verbose(`Creating project graph using static file...`);
const {
Expand All @@ -121,7 +125,7 @@ export async function graphFromStaticFile({
const projectGraph = await projectGraphBuilder(provider);

if (resolveFrameworkDependencies) {
await ui5Framework.enrichProjectGraph(projectGraph, {versionOverride});
await ui5Framework.enrichProjectGraph(projectGraph, {versionOverride, cacheMode});
}

return projectGraph;
Expand All @@ -143,14 +147,16 @@ export async function graphFromStaticFile({
* Configuration file to use for the root module instead the default ui5.yaml. Either a path relative to
* <code>cwd</code> or an absolute path. In both case, platform-specific path segment separators must be used.
* @param {string} [options.versionOverride] Framework version to use instead of the one defined in the root project
* @param {module:@ui5/project/ui5Framework/maven/CacheMode} [options.cacheMode]
* Cache mode to use when consuming SNAPSHOT versions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* @param {string} [options.resolveFrameworkDependencies=true]
* Whether framework dependencies should be added to the graph
* @returns {Promise<@ui5/project/graph/ProjectGraph>} Promise resolving to a Project Graph instance
*/
export async function graphFromObject({
dependencyTree, cwd,
rootConfiguration, rootConfigPath,
versionOverride, resolveFrameworkDependencies = true
versionOverride, cacheMode, resolveFrameworkDependencies = true
}) {
log.verbose(`Creating project graph using object...`);
const {
Expand All @@ -169,7 +175,7 @@ export async function graphFromObject({
const projectGraph = await projectGraphBuilder(dependencyTreeProvider);

if (resolveFrameworkDependencies) {
await ui5Framework.enrichProjectGraph(projectGraph, {versionOverride});
await ui5Framework.enrichProjectGraph(projectGraph, {versionOverride, cacheMode});
}

return projectGraph;
Expand Down
6 changes: 3 additions & 3 deletions lib/graph/helpers/ui5Framework.js
Expand Up @@ -280,10 +280,10 @@ export default {
* @param {object} [options]
* @param {string} [options.versionOverride] Framework version to use instead of the root projects framework
* version
* @param {module:@ui5/project/ui5Framework/maven/CacheMode} [options.cacheMode]
* Cache mode to use when consuming SNAPSHOT versions
* @param {@ui5/project/graph/Workspace} [options.workspace]
* Optional workspace instance to use for overriding node resolutions
* @param {object} [options.resolverConfig]
* Optional configuration object to use for the resolvers
* @returns {Promise<@ui5/project/graph/ProjectGraph>}
* Promise resolving with the given graph instance to allow method chaining
*/
Expand Down Expand Up @@ -369,7 +369,7 @@ export default {
cwd: rootProject.getRootPath(),
version,
providedLibraryMetadata,
...options.resolverConfig,
cacheMode: options.cacheMode
RandomByte marked this conversation as resolved.
Show resolved Hide resolved
});

let startTime;
Expand Down
5 changes: 3 additions & 2 deletions lib/ui5Framework/Sapui5MavenSnapshotResolver.js
Expand Up @@ -33,8 +33,8 @@ class Sapui5MavenSnapshotResolver extends AbstractResolver {
* @param {string} [options.cwd=process.cwd()] Current working directory
* @param {string} [options.ui5HomeDir="~/.ui5"] UI5 home directory location. This will be used to store packages,
* metadata and configuration used by the resolvers. Relative to `process.cwd()`
* @param {string} [options.cacheMode="default"] Can be "default" (cache everything, invalidate after 9 hours),
* "off" (do not cache) and "force" (use cache only - no requests)
* @param {module:@ui5/project/ui5Framework/maven/CacheMode} [options.cacheMode=Default]
* Cache mode to use
*/
constructor(options) {
super(options);
Expand Down Expand Up @@ -136,6 +136,7 @@ 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.
RandomByte marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
18 changes: 18 additions & 0 deletions lib/ui5Framework/maven/CacheMode.js
@@ -0,0 +1,18 @@


/**
* Cache modes for maven consumption
*
* @public
flovogt marked this conversation as resolved.
Show resolved Hide resolved
* @readonly
* @enum {string}
* @property {string} Default Cache everything, invalidate after 9 hours
* @property {string} Force Use cache only
* @property {string} Off Do not use the cache
* @module @ui5/project/ui5Framework/maven/CacheMode
*/
export default {
Default: "Default",
Force: "Force",
Off: "Off"
};
16 changes: 10 additions & 6 deletions lib/ui5Framework/maven/Installer.js
Expand Up @@ -6,6 +6,7 @@ const StreamZip = _StreamZip.async;
import {promisify} from "node:util";
import Registry from "./Registry.js";
import AbstractInstaller from "../AbstractInstaller.js";
import CacheMode from "./CacheMode.js";
import {rimraf} from "rimraf";
const stat = promisify(fs.stat);
const readFile = promisify(fs.readFile);
Expand All @@ -26,10 +27,9 @@ class Installer extends AbstractInstaller {
* @param {Function} parameters.snapshotEndpointUrlCb Callback that returns a Promise <string>,
* resolving to the Maven repository URL.
* Example: <code>https://registry.corp/vendor/build-snapshots/</code>
* @param {string} [parameters.cacheMode="default"] Can be "default" (cache everything, invalidate after 9 hours),
* "off" (do not cache), "force" (use cache only - no requests)
* @param {module:@ui5/project/ui5Framework/maven/CacheMode} [parameters.cacheMode=Default] Cache mode to use
*/
constructor({ui5HomeDir, snapshotEndpointUrlCb, cacheMode = "default"}) {
constructor({ui5HomeDir, snapshotEndpointUrlCb, cacheMode = CacheMode.Default}) {
super(ui5HomeDir);

this._artifactsDir = path.join(ui5HomeDir, "framework", "artifacts");
Expand All @@ -43,6 +43,10 @@ class Installer extends AbstractInstaller {
if (!this._snapshotEndpointUrlCb) {
throw new Error(`Installer: Missing Snapshot-Endpoint URL callback parameter`);
}
if (!Object.values(CacheMode).includes(cacheMode)) {
throw new Error(`Installer: Invalid value '${cacheMode}' for cacheMode parameter. ` +
`Must be one of ${Object.values(CacheMode).join(", ")}`);
}

log.verbose(`Installing Maven artifacts to: ${this._artifactsDir}`);
log.verbose(`Installing Packages to: ${this._packagesDir}`);
Expand Down Expand Up @@ -119,16 +123,16 @@ class Installer extends AbstractInstaller {
return this._synchronize("metadata-" + fsId, async () => {
const localMetadata = await this._getLocalArtifactMetadata(fsId);

if (this._cacheMode === "force" && !localMetadata.revision) {
if (this._cacheMode === CacheMode.Force && !localMetadata.revision) {
throw new Error(`Could not find artifact ` +
`${logId} in local cache`);
}

const now = new Date().getTime();
const timeSinceLastCheck = now - localMetadata.lastCheck;

if (this._cacheMode !== "force" &&
(timeSinceLastCheck > CACHE_TIME || this._cacheMode === "off")) {
if (this._cacheMode !== CacheMode.Force &&
(timeSinceLastCheck > CACHE_TIME || this._cacheMode === CacheMode.Off)) {
// No cached metadata (-> timeSinceLastCheck equals time since 1970) or
// too old metadata or disabled cache
// => Retrieve metadata from repository
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -24,6 +24,7 @@
"./ui5Framework/Sapui5MavenSnapshotResolver": "./lib/ui5Framework/Sapui5MavenSnapshotResolver.js",
"./ui5Framework/Openui5Resolver": "./lib/ui5Framework/Openui5Resolver.js",
"./ui5Framework/Sapui5Resolver": "./lib/ui5Framework/Sapui5Resolver.js",
"./ui5Framework/maven/CacheMode": "./lib/ui5Framework/maven/CacheMode.js",
"./validation/validator": "./lib/validation/validator.js",
"./validation/ValidationError": "./lib/validation/ValidationError.js",
"./graph/ProjectGraph": "./lib/graph/ProjectGraph.js",
Expand Down
6 changes: 4 additions & 2 deletions test/lib/graph/graph.integration.js
Expand Up @@ -253,7 +253,8 @@ test.serial("graphFromPackageDependencies with inactive workspace file at custom
rootConfigPath: "/rootConfigPath",
versionOverride: "versionOverride",
workspaceName: "default",
workspaceConfigPath: path.join(libraryHPath, "custom-ui5-workspace.yaml")
workspaceConfigPath: path.join(libraryHPath, "custom-ui5-workspace.yaml"),
cacheMode: "force"
RandomByte marked this conversation as resolved.
Show resolved Hide resolved
});

t.is(res, "graph");
Expand All @@ -276,6 +277,7 @@ test.serial("graphFromPackageDependencies with inactive workspace file at custom
"enrichProjectGraph got called with graph");
t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
versionOverride: "versionOverride",
workspace: null
workspace: null,
cacheMode: "force"
}, "enrichProjectGraph got called with correct options");
});
26 changes: 18 additions & 8 deletions test/lib/graph/graph.js
Expand Up @@ -57,7 +57,8 @@ test.serial("graphFromPackageDependencies", async (t) => {
cwd: "cwd",
rootConfiguration: "rootConfiguration",
rootConfigPath: "/rootConfigPath",
versionOverride: "versionOverride"
versionOverride: "versionOverride",
cacheMode: "off"
RandomByte marked this conversation as resolved.
Show resolved Hide resolved
});

t.is(res, "graph");
Expand All @@ -81,7 +82,8 @@ test.serial("graphFromPackageDependencies", async (t) => {
"enrichProjectGraph got called with graph");
t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
versionOverride: "versionOverride",
workspace: undefined
workspace: undefined,
cacheMode: "off"
}, "enrichProjectGraph got called with correct options");
});

Expand All @@ -98,6 +100,7 @@ test.serial("graphFromPackageDependencies with workspace name", async (t) => {
rootConfigPath: "/rootConfigPath",
versionOverride: "versionOverride",
workspaceName: "dolphin",
cacheMode: "off"
});

t.is(res, "graph");
Expand Down Expand Up @@ -128,7 +131,8 @@ test.serial("graphFromPackageDependencies with workspace name", async (t) => {
"enrichProjectGraph got called with graph");
t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
versionOverride: "versionOverride",
workspace: "workspace"
workspace: "workspace",
cacheMode: "off"
}, "enrichProjectGraph got called with correct options");
});

Expand Down Expand Up @@ -225,6 +229,7 @@ test.serial("graphFromPackageDependencies with empty workspace", async (t) => {
rootConfigPath: "/rootConfigPath",
versionOverride: "versionOverride",
workspaceName: "dolphin",
cacheMode: "off"
});

t.is(res, "graph");
Expand Down Expand Up @@ -255,7 +260,8 @@ test.serial("graphFromPackageDependencies with empty workspace", async (t) => {
"enrichProjectGraph got called with graph");
t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
versionOverride: "versionOverride",
workspace: null
workspace: null,
cacheMode: "off"
}, "enrichProjectGraph got called with correct options");
});

Expand Down Expand Up @@ -290,7 +296,8 @@ test.serial("graphFromStaticFile", async (t) => {
filePath: "file/path",
rootConfiguration: "rootConfiguration",
rootConfigPath: "/rootConfigPath",
versionOverride: "versionOverride"
versionOverride: "versionOverride",
cacheMode: "off"
});

t.is(res, "graph");
Expand All @@ -316,7 +323,8 @@ test.serial("graphFromStaticFile", async (t) => {
t.is(enrichProjectGraphStub.getCall(0).args[0], "graph",
"enrichProjectGraph got called with graph");
t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
versionOverride: "versionOverride"
versionOverride: "versionOverride",
cacheMode: "off"
}, "enrichProjectGraph got called with correct options");
});

Expand Down Expand Up @@ -351,7 +359,8 @@ test.serial("usingObject", async (t) => {
dependencyTree: "dependencyTree",
rootConfiguration: "rootConfiguration",
rootConfigPath: "/rootConfigPath",
versionOverride: "versionOverride"
versionOverride: "versionOverride",
cacheMode: "off"
});

t.is(res, "graph");
Expand All @@ -371,7 +380,8 @@ test.serial("usingObject", async (t) => {
t.is(enrichProjectGraphStub.getCall(0).args[0], "graph",
"enrichProjectGraph got called with graph");
t.deepEqual(enrichProjectGraphStub.getCall(0).args[1], {
versionOverride: "versionOverride"
versionOverride: "versionOverride",
cacheMode: "off"
}, "enrichProjectGraph got called with correct options");
});

Expand Down
14 changes: 11 additions & 3 deletions test/lib/graph/helpers/ui5Framework.js
Expand Up @@ -107,6 +107,7 @@ test.serial("enrichProjectGraph", async (t) => {

t.is(t.context.Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once");
t.deepEqual(t.context.Sapui5ResolverStub.getCall(0).args, [{
cacheMode: undefined,
cwd: dependencyTree.path,
version: dependencyTree.configuration.framework.version,
providedLibraryMetadata: undefined
Expand Down Expand Up @@ -177,7 +178,7 @@ test.serial("enrichProjectGraph: without framework configuration", async (t) =>

test.serial("enrichProjectGraph SNAPSHOT", async (t) => {
const {sinon, ui5Framework, utils, Sapui5MavenSnapshotResolverInstallStub} = t.context;
process.env.UI5_MAVEN_SNAPSHOT_ENDPOINT = "__url__";
process.env.UI5_MAVEN_SNAPSHOT_ENDPOINT_URL = "__url__";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this even required within this test? I wonder why it didn't fail before, as the variable has been renamed already.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


const dependencyTree = {
id: "test1",
Expand Down Expand Up @@ -216,7 +217,9 @@ test.serial("enrichProjectGraph SNAPSHOT", async (t) => {
const provider = new DependencyTreeProvider({dependencyTree});
const projectGraph = await projectGraphBuilder(provider);

await ui5Framework.enrichProjectGraph(projectGraph);
await ui5Framework.enrichProjectGraph(projectGraph, {
cacheMode: "force"
RandomByte marked this conversation as resolved.
Show resolved Hide resolved
});

t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGraph should be called once");

Expand Down Expand Up @@ -263,7 +266,7 @@ test.serial("enrichProjectGraph SNAPSHOT", async (t) => {
"application.a"
], "Traversed graph in correct order");

delete process.env.UI5_MAVEN_SNAPSHOT_ENDPOINT;
delete process.env.UI5_MAVEN_SNAPSHOT_ENDPOINT_URL;
});

test.serial("enrichProjectGraph: With versionOverride", async (t) => {
Expand Down Expand Up @@ -313,6 +316,7 @@ test.serial("enrichProjectGraph: With versionOverride", async (t) => {

t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once");
t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{
cacheMode: undefined,
cwd: dependencyTree.path,
version: "1.99.9",
providedLibraryMetadata: undefined
Expand Down Expand Up @@ -451,6 +455,7 @@ test.serial("enrichProjectGraph should resolve framework project with version an
t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGrap should be called once");
t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once");
t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{
cacheMode: undefined,
cwd: dependencyTree.path,
version: "1.2.3",
providedLibraryMetadata: undefined
Expand Down Expand Up @@ -531,6 +536,7 @@ test.serial("enrichProjectGraph should resolve framework project " +
t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once");
t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGraph should be called once");
t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{
cacheMode: undefined,
cwd: dependencyTree.path,
version: "1.99.9",
providedLibraryMetadata: undefined
Expand Down Expand Up @@ -794,6 +800,7 @@ test.serial("enrichProjectGraph should use framework library metadata from works

t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once");
t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{
cacheMode: undefined,
cwd: dependencyTree.path,
version: "1.111.1",
providedLibraryMetadata: workspaceFrameworkLibraryMetadata
Expand Down Expand Up @@ -851,6 +858,7 @@ test.serial("enrichProjectGraph should allow omitting framework version in case

t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once");
t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{
cacheMode: undefined,
cwd: dependencyTree.path,
version: undefined,
providedLibraryMetadata: workspaceFrameworkLibraryMetadata
Expand Down