Skip to content

Commit

Permalink
fix: content page tree calculation improvments (#1555)
Browse files Browse the repository at this point in the history
* prevent infinite loop can occur in the mapping of the content page tree.
* remove potentially circular references in content page tree path calculation

Co-authored-by: Danilo Hoffmann <dhhyi@aol.com>
  • Loading branch information
tbouliere-datasolution and dhhyi authored Mar 27, 2024
1 parent 564d5c8 commit 2e84cb7
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { ContentPageTreeHelper } from 'ish-core/models/content-page-tree/content-page-tree.helper';
import { ContentPageTreeElement } from 'ish-core/models/content-page-tree/content-page-tree.model';

import { createContentPageTreeView } from './content-page-tree-view.model';

describe('Content Page Tree View Model', () => {
it('should not loop forever with recursive tree', () => {
const rootElement = { contentPageId: 'A', path: ['A'] } as ContentPageTreeElement;
const child1 = { contentPageId: 'A.1', path: ['A', 'A.1'] } as ContentPageTreeElement;
const child2 = { contentPageId: 'A.2', path: ['A', 'A.2'] } as ContentPageTreeElement;
const loopingChild = { contentPageId: 'A.1.1', path: ['A', 'A.1', 'A.1'] } as ContentPageTreeElement;

const empty = ContentPageTreeHelper.empty();
const tree0 = ContentPageTreeHelper.add(empty, rootElement);
const tree1 = ContentPageTreeHelper.add(tree0, child1);
const tree2 = ContentPageTreeHelper.add(tree1, child2);
const loopingTree = ContentPageTreeHelper.add(tree2, loopingChild);

const tree = createContentPageTreeView(loopingTree, 'A', 'A');

expect(tree).toEqual({
children: [
{
children: [],
contentPageId: 'A.1',
parent: 'A',
path: ['A', 'A.1'],
pathElements: [
{ contentPageId: 'A', path: ['A'] },
{ contentPageId: 'A.1', path: ['A', 'A.1'] },
],
},
{
children: [],
contentPageId: 'A.2',
parent: 'A',
path: ['A', 'A.2'],
pathElements: [
{ contentPageId: 'A', path: ['A'] },
{ contentPageId: 'A.2', path: ['A', 'A.2'] },
],
},
],
contentPageId: 'A',
parent: undefined,
path: ['A'],
pathElements: [{ contentPageId: 'A', path: ['A'] }],
});
});

it('should not loop forever with recursive tree (subtree case)', () => {
const rootElement = { contentPageId: 'A', path: ['A'] } as ContentPageTreeElement;
const child1 = { contentPageId: 'A.1', path: ['A', 'A.1'] } as ContentPageTreeElement;
const child2 = { contentPageId: 'A.2', path: ['A', 'A.2'] } as ContentPageTreeElement;
const loopingChild = { contentPageId: 'A.1.1', path: ['A', 'A.1', 'A.1'] } as ContentPageTreeElement;

const empty = ContentPageTreeHelper.empty();
const tree0 = ContentPageTreeHelper.add(empty, rootElement);
const tree1 = ContentPageTreeHelper.add(tree0, child1);
const tree2 = ContentPageTreeHelper.add(tree1, child2);
const loopingTree = ContentPageTreeHelper.add(tree2, loopingChild);

const tree = createContentPageTreeView(loopingTree, 'A', 'A.1');

expect(tree).toEqual({
children: [
{
children: [],
contentPageId: 'A.1',
parent: 'A',
path: ['A', 'A.1'],
pathElements: [
{ contentPageId: 'A', path: ['A'] },
{ contentPageId: 'A.1', path: ['A', 'A.1'] },
],
},
{
children: [],
contentPageId: 'A.2',
parent: 'A',
path: ['A', 'A.2'],
pathElements: [
{ contentPageId: 'A', path: ['A'] },
{ contentPageId: 'A.2', path: ['A', 'A.2'] },
],
},
],
contentPageId: 'A',
parent: undefined,
path: ['A'],
pathElements: [{ contentPageId: 'A', path: ['A'] }],
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,9 @@ function getContentPageTreeElements(
// If current content page is part of element subtree or equals the element itself, then all children of elements should be analyzed
if (tree.edges[elementId] && isContentPagePartOfPageTreeElement(tree, currentContentPageId, elementId)) {
treeElements = tree.edges[elementId]
.filter(child => child !== elementId)
.map(child => getContentPageTreeElements(tree, child, rootId, currentContentPageId))
.reduce((a, b) => {
b.map(element => a.push(element));
return a;
});
.flat();
}

const parent = tree.nodes[elementId].path[tree.nodes[elementId].path.length - 2];
Expand Down Expand Up @@ -75,6 +73,7 @@ function isContentPagePartOfPageTreeElement(
result = true;
} else {
result = tree.edges[elementId]
.filter(e => e !== elementId)
.map(e => isContentPagePartOfPageTreeElement(tree, currentContentPageId, e))
.find(res => res);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class ContentPageTreeMapper {
// yield page tree element
return {
name: pathElement.title,
path: [...newTreeElementPath],
path: [...newTreeElementPath].filter((v, i, a) => a.indexOf(v) === i),
contentPageId: pathElement.itemId,
};
})
Expand Down

0 comments on commit 2e84cb7

Please sign in to comment.