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

Added function and tests for cyclic dependency check #25

Merged
merged 2 commits into from
Dec 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,81 @@ function getAffectedVariables(tree, inputVar) {
return Array.from(new Set(values));
}

/**
* Checks for cyclic dependency in the input graph.
* Returns the cyclic path if found otherwise returns `[]`
*
* Iterate over all keys in the graph and run `dfs()`.
* If a cycle is found, error will be thrown and further execution stop.
* In case of error, `.pop()` won't be called after any `dfs()`, so `cycle` array will have the cyclic path.
*
* @param {*} graph The dependency tree created using `createDependencyTree`
* @return {*}
*/
function checkForCyclicDependency(graph) {
let cycle = [];
const UNVISITED = 0;
const VISITING = 1;
const VISITED = 2;
const statusMap = new Map(Object.keys(graph).map(key => [key, UNVISITED]));

/**
* Utility function for depth first search.
* Updates `cycle` to keep track of potential circular dependency.
*
* @param {unknown} node The node to visit
* @param {*} neighbours The neighbours of the node being visited
* @return {undefined} Nothing is returned
* @throws {Error} If a cycle is encountered while doing DFS
*/
function dfs(node, neighbours){
if(statusMap.get(node) === VISITED){
return;
}

if(statusMap.get(node) === VISITING){
throw new Error('The graph contains cyclic dependency for ' + node);
}

statusMap.set(node, VISITING);

neighbours.forEach(neighbour => {
cycle.push(neighbour);
dfs(neighbour, graph[neighbour] || []);
cycle.pop();
});

statusMap.set(node, VISITED);
}

// By using try-catch, we stop error from going up the chain any more and return cycle array
const entries = Object.entries(graph);
try {
entries.forEach(([node, neighbours]) => {
if(statusMap.get(node) === UNVISITED){
cycle.push(node);
dfs(node, neighbours);
cycle.pop();
}
});
} catch (error) {
if (cycle.length) {
// currently cycle contains the entire dfs path
// we need to extract only the cyclic path
const cyclicityStartIndex = cycle.indexOf(cycle[cycle.length - 1]);
cycle = cycle.slice(cyclicityStartIndex);
}
}
// since dependency graph is of format baseVar -> [vars affected by baseVar]
// we need to reverse the cycle array to get a more intuitive cylclic path
return cycle.reverse();
}

module.exports = {
parseAssign,
getTemplates,
createEngine,
createDependencyTree,
getAffectedVariables,
checkForCyclicDependency
};
97 changes: 97 additions & 0 deletions test/dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,100 @@ describe("dependency-graph: Affected Variables", function () {
expect(affectedVars.length).to.equal(7);
});
});

describe("dependency-graph: Cyclic Dependency", function() {

it('Should handle checks with no cyclic dependency', () => {
const expression = `
{% assign x = a | plus: t %}
{% assign y = a | times: t %}
{% assign z = t | times: 3 %}
{% assign z = p | times: 3 %}
{% assign p = q | times: 3 %}
{% assign q = r | times: x %}
`;
const graph = depGraph.createDependencyTree(expression);
const cycle = depGraph.checkForCyclicDependency(graph);
expect(cycle).to.deep.equal([]);
});

it('Should handle simple cyclic dependency check', () => {
// First line ceates cyclicity in the following expression
const expression = `
{% assign x = a | plus: z %}
{% assign y = a | times: t %}
{% assign z = t | times: 3 %}
{% assign z = p | times: 3 %}
{% assign p = q | times: 3 %}
{% assign q = r | times: x %}
`;
const graph = depGraph.createDependencyTree(expression);
const cycle = depGraph.checkForCyclicDependency(graph);
expect(cycle).to.deep.equal([
'x',
'z',
'p',
'q',
'x',
]);
// Explanation
// x depends on z -> (x = a | plus: z)
// z depends on p -> (z = p | times: 3)
// p depends on q -> (p = q | times: 3)
// q depends on x -> (q = r | times: x)
});

it('Should handle self cyclic dependency check', () => {
const expression = `
{% assign x = x | plus:z %}
`;
const graph = depGraph.createDependencyTree(expression);
const cycle = depGraph.checkForCyclicDependency(graph);
expect(cycle).to.deep.equal([
'x',
'x'
]);
});

it("Should handle complex cyclic dependency check", () => {
const expression = `
{% if private_seats %}
{% if yes_discounts_private_seats %}
{% if p_s_p %}
{% assign m_f_p_s_p_o = m_f_p_s | times: d_p_s_p %}
{% assign m_f_p_s_p_t = m_f_p_s_p_o | divided_by: 100.00 %}
{% assign m_f_p_s_p = m_f_p_s | minus: m_f_p_s_p_t %}
{% assign t_m_f_p_s = m_f_p_s_p | times: c_p_s %}
{% else %}
{% assign m_f_p_s_a_o = m_f_p_s | minus: d_s_p_s_a %}
{% assign t_m_f_p_s = m_f_p_s_a_o | times: c_p_s %}
{% endif %}
{% else %}
{% assign t_m_f_p_s = m_f_p_s | times: c_p_s %}
{% endif %}
{% else %}
{% assign t_m_f = t_m_f_d_d | plus: t_m_f_h_d %}
{% endif %}
{% assign t_m_f_c = t_m_f | plus: t_m_f_p_s %}
{% assign m_f_p_s_p_o = t_m_f_c | plus: t_m_f_p_s %}
{% assign s_d = t_m_f_c| times: n_m %}
{% assign s_f = s_f_v | times: t_sea %}
`;
const graph = depGraph.createDependencyTree(expression);
const cycle = depGraph.checkForCyclicDependency(graph);
expect(cycle).to.deep.equal([
'm_f_p_s_p_o',
't_m_f_c',
't_m_f_p_s',
'm_f_p_s_p',
'm_f_p_s_p_t',
'm_f_p_s_p_o',
]);
// Explanation
// m_f_p_s_p_o depends on t_m_f_c -> (m_f_p_s_p_o = t_m_f_c | plus: t_m_f_p_s)
// t_m_f_c depends on t_m_f_p_s -> (t_m_f_c = t_m_f | plus: t_m_f_p_s)
// t_m_f_p_s depends on m_f_p_s_p -> (t_m_f_p_s = m_f_p_s_p | times: c_p_s)
// m_f_p_s_p depends on m_f_p_s_p_t -> (m_f_p_s_p = m_f_p_s | minus: m_f_p_s_p_t)
// m_f_p_s_p_t depends on m_f_p_s_p_o -> (m_f_p_s_p_t = m_f_p_s_p_o | divided_by: 100.00)
});
})