Skip to content

Commit

Permalink
fix: Check for circular references (#13783)
Browse files Browse the repository at this point in the history
Co-authored-by: Konrad-Simso <konrad.simso@digdir.no>
  • Loading branch information
TomasEng and Konrad-Simso authored Oct 30, 2024
1 parent 951d041 commit 3f0ff04
Show file tree
Hide file tree
Showing 10 changed files with 417 additions and 24 deletions.
1 change: 1 addition & 0 deletions frontend/language/src/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@
"schema_editor.enum_error_duplicate": "Verdiene må være unike.",
"schema_editor.enum_legend": "Liste med gyldige verdier",
"schema_editor.enum_value": "Gyldig verdi nummer {{index}}",
"schema_editor.error_circular_references": "Du kan ikke utføre denne operasjonen fordi Studio ikke støtter sirkulære referanser.",
"schema_editor.error_could_not_detect_taskType": "Kunne ikke hente oppgavetype for {{layout}}",
"schema_editor.error_could_not_detect_taskType_description": "Dette gjør at vi ikke kan vise riktig liste over tilgjengelige komponenter. Velg en annen sidegruppe eller prøv å last siden på nytt.",
"schema_editor.error_data_type_name_exists": "Modellen kan ikke ha samme navn som datatyper i løsningen.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,24 @@ describe('ArrayUtils', () => {
});
});

describe('hasIntersection', () => {
it('Returns true when arrays have one common element', () => {
expect(ArrayUtils.hasIntersection([1, 2, 3], [3, 4, 5])).toBe(true);
});

it('Returns true when arrays have multiple common elements', () => {
expect(ArrayUtils.hasIntersection([1, 2, 3], [3, 2, 5])).toBe(true);
});

it('Returns false when arrays have no common elements', () => {
expect(ArrayUtils.hasIntersection([1, 2, 3], [4, 5, 6])).toBe(false);
});

it('Returns false when the arrays are empty', () => {
expect(ArrayUtils.hasIntersection([], [])).toBe(false);
});
});

describe('replaceLastItem', () => {
it('should replace the last item in an array and return the modified array', () => {
expect(ArrayUtils.replaceLastItem([1, 2, 3], 99)).toEqual([1, 2, 99]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ export class ArrayUtils {
/** Returns the last item of the given array */
public static last = <T>(array: T[]): T => array[array.length - 1];

public static hasIntersection = <T>(arrA: T[], arrB: T[]): boolean => {
return arrA.some((x) => arrB.includes(x));
};

/**
* Returns an array of which the element of arrA are either present or not present in arrB based on the include param.
* @param arrA The first array.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,15 @@ describe('useAddReference', () => {
const addedReferenceNode = addedChild as ReferenceNode;
expect(addedReferenceNode.reference).toEqual(definitionNodeMock.schemaPointer);
});

it('Does not add a reference when the reference would result in a circular reference', () => {
const { add, save } = setup();
const uniquePointerOfDefinition = SchemaModel.getUniquePointer(
definitionNodeMock.schemaPointer,
);
const target: ItemPosition = { parentId: uniquePointerOfDefinition, index: 0 };
jest.spyOn(window, 'alert').mockImplementation(jest.fn());
add(nameOfDefinition, target);
expect(save).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -1,25 +1,49 @@
import type { HandleAdd, ItemPosition } from 'app-shared/types/dndTypes';
import { useCallback } from 'react';
import type { NodePosition } from '@altinn/schema-model';
import { SchemaModel } from '@altinn/schema-model';
import { createDefinitionPointer, SchemaModel } from '@altinn/schema-model';

import { calculatePositionInFullList } from '../utils';
import { useSavableSchemaModel } from '../../../hooks/useSavableSchemaModel';
import { useSchemaEditorAppContext } from '@altinn/schema-editor/hooks/useSchemaEditorAppContext';
import { useTranslation } from 'react-i18next';

export const useAddReference = (): HandleAdd<string> => {
const { setSelectedUniquePointer } = useSchemaEditorAppContext();
const savableModel = useSavableSchemaModel();
const circularReferenceError = useCircularReferenceError();
const circularReferenceAlert = useCircularReferenceAlert();
return useCallback(
(reference: string, position: ItemPosition) => {
const index = calculatePositionInFullList(savableModel, position);
const parentPointer = savableModel.getSchemaPointerByUniquePointer(position.parentId);
if (circularReferenceError(reference, parentPointer)) {
circularReferenceAlert();
return;
}
const target: NodePosition = { parentPointer, index };
const { schemaPointer } = savableModel.getFinalNode(target.parentPointer);
const refName = savableModel.generateUniqueChildName(schemaPointer, 'ref');
const ref = savableModel.addReference(refName, reference, target);
const uniquePointer = SchemaModel.getUniquePointer(ref.schemaPointer);
setSelectedUniquePointer(uniquePointer);
},
[savableModel, setSelectedUniquePointer],
[savableModel, setSelectedUniquePointer, circularReferenceAlert, circularReferenceError],
);
};

function useCircularReferenceError(): (reference: string, targetSchemaPointer: string) => boolean {
const savableModel = useSavableSchemaModel();
return useCallback(
(reference: string, targetSchemaPointer: string) => {
const referencePointer = createDefinitionPointer(reference);
return savableModel.willResultInCircularReferences(referencePointer, targetSchemaPointer);
},
[savableModel],
);
}

function useCircularReferenceAlert(): () => void {
const { t } = useTranslation();
return useCallback(() => alert(t('schema_editor.error_circular_references')), [t]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
objectChildMock,
objectNodeMock,
referenceNodeMock,
referredNodeMock,
rootNodeMock,
toggableNodeMock,
uiSchemaNodesMock,
Expand Down Expand Up @@ -229,4 +230,15 @@ describe('useMoveProperty', () => {
expect(setSelectedUniquePointerMock).toHaveBeenCalledTimes(1);
expect(setSelectedUniquePointerMock).toHaveBeenCalledWith(expectedFinalUniquePointer);
});

it('Does not move the node when it would result in circular references', () => {
const { move, save } = setup();
const pointerOfNodeToMove = referenceNodeMock.schemaPointer;
const pointerOfNewParent = referredNodeMock.schemaPointer;
const indexInNewParent = 0;
const target: ItemPosition = { parentId: pointerOfNewParent, index: indexInNewParent };
jest.spyOn(window, 'alert').mockImplementation(jest.fn());
move(pointerOfNodeToMove, target);
expect(save).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,68 @@ import { useSavableSchemaModel } from '../../../hooks/useSavableSchemaModel';
import { useTranslation } from 'react-i18next';
import { useSchemaEditorAppContext } from '@altinn/schema-editor/hooks/useSchemaEditorAppContext';

type MovementErrorCode = 'circular' | 'colliding_names';

export const useMoveProperty = (): HandleMove => {
const savableModel = useSavableSchemaModel();
const { selectedUniquePointer, setSelectedUniquePointer } = useSchemaEditorAppContext();
const { t } = useTranslation();
const areThereCollidingNames = useCallback(
const movementError = useMovementError();
const movementErrorAlert = useMovementErrorAlert();

return useCallback(
(uniquePointer: string, position: ItemPosition) => {
const schemaPointer = savableModel.getSchemaPointerByUniquePointer(uniquePointer);
const schemaParentPointer = savableModel.getSchemaPointerByUniquePointer(position.parentId);
const index = calculatePositionInFullList(savableModel, position);
const target: NodePosition = { parentPointer: schemaParentPointer, index };
const name = extractNameFromPointer(schemaPointer);
const error = movementError(schemaPointer, schemaParentPointer);
if (error) {
const parent = extractNameFromPointer(schemaParentPointer);
movementErrorAlert(error, { name, parent });
return;
}
const movedNode = savableModel.moveNode(schemaPointer, target);
if (selectedUniquePointer === uniquePointer) {
const movedUniquePointer = SchemaModel.getUniquePointer(
movedNode.schemaPointer,
position.parentId,
);
setSelectedUniquePointer(movedUniquePointer);
}
},
[
savableModel,
movementError,
selectedUniquePointer,
setSelectedUniquePointer,
movementErrorAlert,
],
);
};

function useMovementError(): (
schemaPointer: string,
targetSchemaPointer: string,
) => MovementErrorCode | null {
const savableModel = useSavableSchemaModel();
const areThereCollidingNames = useCollidingNamesError();
return useCallback(
(schemaPointer: string, targetSchemaPointer: string): MovementErrorCode | null => {
if (savableModel.willResultInCircularReferences(schemaPointer, targetSchemaPointer)) {
return 'circular';
} else if (areThereCollidingNames(schemaPointer, targetSchemaPointer)) {
return 'colliding_names';
}
return null;
},
[savableModel, areThereCollidingNames],
);
}

function useCollidingNamesError(): (schemaPointer: string, targetSchemaPointer: string) => boolean {
const savableModel = useSavableSchemaModel();
return useCallback(
(schemaPointer: string, schemaParentPointer: string): boolean => {
const currentParent = savableModel.getParentNode(schemaPointer);
const isMovingWithinSameParent = schemaParentPointer === currentParent.schemaPointer;
Expand All @@ -24,28 +81,38 @@ export const useMoveProperty = (): HandleMove => {
},
[savableModel],
);
}

type AlertMessageInfo = {
name: string;
parent: string;
};

function useMovementErrorAlert(): (errorCode: MovementErrorCode, info: AlertMessageInfo) => void {
const movementErrorMessage = useMovementErrorMessage();
return useCallback(
(uniquePointer: string, position: ItemPosition) => {
const schemaPointer = savableModel.getSchemaPointerByUniquePointer(uniquePointer);
const schemaParentPointer = savableModel.getSchemaPointerByUniquePointer(position.parentId);
const index = calculatePositionInFullList(savableModel, position);
const target: NodePosition = { parentPointer: schemaParentPointer, index };
const name = extractNameFromPointer(schemaPointer);
if (areThereCollidingNames(schemaPointer, schemaParentPointer)) {
const parent = extractNameFromPointer(schemaParentPointer);
alert(t('schema_editor.move_node_same_name_error', { name, parent }));
} else {
const movedNode = savableModel.moveNode(schemaPointer, target);
if (selectedUniquePointer === uniquePointer) {
const movedUniquePointer = SchemaModel.getUniquePointer(
movedNode.schemaPointer,
position.parentId,
);
setSelectedUniquePointer(movedUniquePointer);
}
(errorCode: MovementErrorCode, info: AlertMessageInfo) => {
const message = movementErrorMessage(errorCode, info);
alert(message);
},
[movementErrorMessage],
);
}

function useMovementErrorMessage(): (
errorCode: MovementErrorCode,
info: AlertMessageInfo,
) => string {
const { t } = useTranslation();
return useCallback(
(errorCode: MovementErrorCode, info: AlertMessageInfo) => {
switch (errorCode) {
case 'circular':
return t('schema_editor.error_circular_references');
case 'colliding_names':
return t('schema_editor.move_node_same_name_error', info);
}
},
[savableModel, t, areThereCollidingNames, selectedUniquePointer, setSelectedUniquePointer],
[t],
);
};
}
Loading

0 comments on commit 3f0ff04

Please sign in to comment.