diff --git a/lib/graph/ProjectGraph.js b/lib/graph/ProjectGraph.js
index 8ea683f4f..4d804ef5e 100644
--- a/lib/graph/ProjectGraph.js
+++ b/lib/graph/ProjectGraph.js
@@ -2,7 +2,9 @@ import {getLogger} from "@ui5/logger";
const log = getLogger("graph:ProjectGraph");
/**
- * A rooted, directed graph representing a UI5 project, its dependencies and available extensions
+ * A rooted, directed graph representing a UI5 project, its dependencies and available extensions.
+ *
+ * While it allows defining cyclic dependencies, both traversal functions will throw if they encounter cycles.
*
* @public
* @class
@@ -20,9 +22,9 @@ class ProjectGraph {
}
this._rootProjectName = rootProjectName;
- this._projects = new Map(); // maps project name to instance
- this._adjList = new Map(); // maps project name to edges/dependencies
- this._optAdjList = new Map(); // maps project name to optional dependencies
+ this._projects = new Map(); // maps project name to instance (= nodes)
+ this._adjList = new Map(); // maps project name to dependencies (= edges)
+ this._optAdjList = new Map(); // maps project name to optional dependencies (= edges)
this._extensions = new Map(); // maps extension name to instance
@@ -175,10 +177,6 @@ class ProjectGraph {
declareDependency(fromProjectName, toProjectName) {
this._checkSealed();
try {
- // if (this._optAdjList[fromProjectName] && this._optAdjList[fromProjectName][toProjectName]) {
- // // TODO: Do we even care?
- // throw new Error(`Dependency has already been declared as optional`);
- // }
log.verbose(`Declaring dependency: ${fromProjectName} depends on ${toProjectName}`);
this._declareDependency(this._adjList, fromProjectName, toProjectName);
} catch (err) {
@@ -199,10 +197,6 @@ class ProjectGraph {
declareOptionalDependency(fromProjectName, toProjectName) {
this._checkSealed();
try {
- // if (this._adjList[fromProjectName] && this._adjList[fromProjectName][toProjectName]) {
- // // TODO: Do we even care?
- // throw new Error(`Dependency has already been declared as non-optional`);
- // }
log.verbose(`Declaring optional dependency: ${fromProjectName} depends on ${toProjectName}`);
this._declareDependency(this._optAdjList, fromProjectName, toProjectName);
this._hasUnresolvedOptionalDependencies = true;
@@ -267,6 +261,11 @@ class ProjectGraph {
*/
getTransitiveDependencies(projectName) {
const dependencies = new Set();
+ if (!this._projects.has(projectName)) {
+ throw new Error(
+ `Failed to get transitive dependencies for project ${projectName}: ` +
+ `Unable to find project in project graph`);
+ }
const processDependency = (depName) => {
const adjacencies = this._adjList.get(depName);
@@ -375,7 +374,6 @@ class ProjectGraph {
* graph traversal will wait and only continue once the promise has resolved.
*/
- // TODO: Use generator functions instead?
/**
* Visit every project in the graph that can be reached by the given entry project exactly once.
* The entry project defaults to the root project.
@@ -398,28 +396,28 @@ class ProjectGraph {
const queue = [{
projectNames: [startName],
- predecessors: []
+ ancestors: []
}];
const visited = Object.create(null);
while (queue.length) {
- const {projectNames, predecessors} = queue.shift(); // Get and remove first entry from queue
+ const {projectNames, ancestors} = queue.shift(); // Get and remove first entry from queue
await Promise.all(projectNames.map(async (projectName) => {
- this._checkCycle(predecessors, projectName);
+ this._checkCycle(ancestors, projectName);
if (visited[projectName]) {
return visited[projectName];
}
return visited[projectName] = (async () => {
- const newPredecessors = [...predecessors, projectName];
+ const newAncestors = [...ancestors, projectName];
const dependencies = this.getDependencies(projectName);
queue.push({
projectNames: dependencies,
- predecessors: newPredecessors
+ ancestors: newAncestors
});
await callback({
@@ -453,17 +451,17 @@ class ProjectGraph {
return this._traverseDepthFirst(startName, Object.create(null), [], callback);
}
- async _traverseDepthFirst(projectName, visited, predecessors, callback) {
- this._checkCycle(predecessors, projectName);
+ async _traverseDepthFirst(projectName, visited, ancestors, callback) {
+ this._checkCycle(ancestors, projectName);
if (visited[projectName]) {
return visited[projectName];
}
return visited[projectName] = (async () => {
- const newPredecessors = [...predecessors, projectName];
+ const newAncestors = [...ancestors, projectName];
const dependencies = this.getDependencies(projectName);
await Promise.all(dependencies.map((depName) => {
- return this._traverseDepthFirst(depName, visited, newPredecessors, callback);
+ return this._traverseDepthFirst(depName, visited, newAncestors, callback);
}));
await callback({
@@ -609,13 +607,13 @@ class ProjectGraph {
}
}
- _checkCycle(predecessors, projectName) {
- if (predecessors.includes(projectName)) {
- // We start to run in circles. That's neither expected nor something we can deal with
-
- // Mark first and last occurrence in chain with an asterisk
- predecessors[predecessors.indexOf(projectName)] = `${projectName}*`;
- throw new Error(`Detected cyclic dependency chain: ${predecessors.join(" -> ")} -> ${projectName}*`);
+ _checkCycle(ancestors, projectName) {
+ if (ancestors.includes(projectName)) {
+ // "Back-edge" detected. Neither BFS nor DFS searches should continue
+ // Mark first and last occurrence in chain with an asterisk and throw an error detailing the
+ // problematic dependency chain
+ ancestors[ancestors.indexOf(projectName)] = `*${projectName}*`;
+ throw new Error(`Detected cyclic dependency chain: ${ancestors.join(" -> ")} -> *${projectName}*`);
}
}
diff --git a/test/lib/graph/ProjectGraph.js b/test/lib/graph/ProjectGraph.js
index fdbbe1f41..ea0a3e198 100644
--- a/test/lib/graph/ProjectGraph.js
+++ b/test/lib/graph/ProjectGraph.js
@@ -364,6 +364,20 @@ test("getTransitiveDependencies", async (t) => {
], "Should store and return correct transitive dependencies for library.a");
});
+test("getTransitiveDependencies: Unknown project", (t) => {
+ const {ProjectGraph} = t.context;
+ const graph = new ProjectGraph({
+ rootProjectName: "my root project"
+ });
+
+ const error = t.throws(() => {
+ graph.getTransitiveDependencies("library.x");
+ });
+ t.is(error.message,
+ "Failed to get transitive dependencies for project library.x: Unable to find project in project graph",
+ "Should throw with expected error message");
+});
+
test("declareDependency: Unknown source", async (t) => {
const {ProjectGraph} = t.context;
const graph = new ProjectGraph({
@@ -485,7 +499,6 @@ test("declareDependency: Already declared as optional, now non-optional", async
"Should declare dependency as non-optional");
});
-
test("getDependencies: Project without dependencies", async (t) => {
const {ProjectGraph} = t.context;
const graph = new ProjectGraph({
@@ -775,7 +788,7 @@ test("traverseBreadthFirst: Detect cycle", async (t) => {
const error = await t.throwsAsync(graph.traverseBreadthFirst(() => {}));
t.is(error.message,
- "Detected cyclic dependency chain: library.a* -> library.b -> library.a*",
+ "Detected cyclic dependency chain: *library.a* -> library.b -> *library.a*",
"Should throw with expected error message");
});
@@ -1000,7 +1013,7 @@ test("traverseDepthFirst: Detect cycle", async (t) => {
const error = await t.throwsAsync(graph.traverseDepthFirst(() => {}));
t.is(error.message,
- "Detected cyclic dependency chain: library.a* -> library.b -> library.a*",
+ "Detected cyclic dependency chain: *library.a* -> library.b -> *library.a*",
"Should throw with expected error message");
});
@@ -1020,7 +1033,7 @@ test("traverseDepthFirst: Cycle which does not occur in BFS", async (t) => {
const error = await t.throwsAsync(graph.traverseDepthFirst(() => {}));
t.is(error.message,
- "Detected cyclic dependency chain: library.a -> library.b* -> library.c -> library.b*",
+ "Detected cyclic dependency chain: library.a -> *library.b* -> library.c -> *library.b*",
"Should throw with expected error message");
});
diff --git a/test/lib/graph/graphFromObject.js b/test/lib/graph/graphFromObject.js
index 7efa41072..0e4c90011 100644
--- a/test/lib/graph/graphFromObject.js
+++ b/test/lib/graph/graphFromObject.js
@@ -81,8 +81,8 @@ test("Application Cycle A: Traverse project graph breadth first with cycles", as
t.is(callbackStub.callCount, 4, "Four projects have been visited");
t.is(error.message,
- "Detected cyclic dependency chain: application.cycle.a* -> component.cycle.a " +
- "-> application.cycle.a*",
+ "Detected cyclic dependency chain: *application.cycle.a* -> component.cycle.a " +
+ "-> *application.cycle.a*",
"Threw with expected error message");
const callbackCalls = callbackStub.getCalls().map((call) => call.args[0].project.getName());
@@ -143,8 +143,8 @@ test("Application Cycle A: Traverse project graph depth first with cycles", asyn
t.is(callbackStub.callCount, 0, "Zero projects have been visited");
t.is(error.message,
- "Detected cyclic dependency chain: application.cycle.a* -> component.cycle.a " +
- "-> application.cycle.a*",
+ "Detected cyclic dependency chain: *application.cycle.a* -> component.cycle.a " +
+ "-> *application.cycle.a*",
"Threw with expected error message");
});
@@ -157,8 +157,8 @@ test("Application Cycle B: Traverse project graph depth first with cycles", asyn
t.is(callbackStub.callCount, 0, "Zero projects have been visited");
t.is(error.message,
- "Detected cyclic dependency chain: application.cycle.b -> module.d* " +
- "-> module.e -> module.d*",
+ "Detected cyclic dependency chain: application.cycle.b -> *module.d* " +
+ "-> module.e -> *module.d*",
"Threw with expected error message");
});
diff --git a/test/lib/graph/projectGraphBuilder.js b/test/lib/graph/projectGraphBuilder.js
index 51489cbb9..df042ec74 100644
--- a/test/lib/graph/projectGraphBuilder.js
+++ b/test/lib/graph/projectGraphBuilder.js
@@ -249,6 +249,29 @@ test("Node depends on itself", async (t) => {
"Threw with expected error message");
});
+test("Cyclic dependencies", async (t) => {
+ t.context.getRootNode.resolves(createNode({
+ id: "id1",
+ name: "project-1"
+ }));
+ t.context.getDependencies
+ .onFirstCall().resolves([
+ createNode({
+ id: "id2",
+ name: "project-2"
+ }),
+ ])
+ .onSecondCall().resolves([
+ createNode({
+ id: "id1",
+ name: "project-1"
+ }),
+ ]);
+ const graph = await projectGraphBuilder(t.context.provider);
+ t.deepEqual(graph.getDependencies("project-1"), ["project-2"], "Cyclic dependency has been added");
+ t.deepEqual(graph.getDependencies("project-2"), ["project-1"], "Cyclic dependency has been added");
+});
+
test("Nested node with same id is ignored", async (t) => {
t.context.getRootNode.resolves(createNode({
id: "id1",
@@ -263,7 +286,7 @@ test("Nested node with same id is ignored", async (t) => {
t.context.getDependencies.onSecondCall().resolves([
createNode({
id: "id1",
- name: "project-3"
+ name: "project-3" // name will be ignored, since the first "id1" node is being used
}),
]);
const graph = await projectGraphBuilder(t.context.provider);
diff --git a/test/lib/graph/providers/NodePackageDependencies.integration.js b/test/lib/graph/providers/NodePackageDependencies.integration.js
index 74ea23c0a..2adfa7245 100644
--- a/test/lib/graph/providers/NodePackageDependencies.integration.js
+++ b/test/lib/graph/providers/NodePackageDependencies.integration.js
@@ -219,7 +219,7 @@ test("AppCycleD: cyclic npm deps - Cycles everywhere", async (t) => {
const error = await t.throwsAsync(testGraphCreationDfs(t, npmProvider, []));
t.is(error.message,
- `Detected cyclic dependency chain: application.cycle.d -> module.h* -> module.i -> module.k -> module.h*`);
+ `Detected cyclic dependency chain: application.cycle.d -> *module.h* -> module.i -> module.k -> *module.h*`);
});
test("AppCycleE: cyclic npm deps - Cycle via devDependency", async (t) => {