diff --git a/src/components/Pipelines/VariableForm/VariableForm.js b/src/components/Pipelines/VariableForm/VariableForm.js index 1b3365e54..77074912c 100644 --- a/src/components/Pipelines/VariableForm/VariableForm.js +++ b/src/components/Pipelines/VariableForm/VariableForm.js @@ -11,7 +11,7 @@ import Button, { TheButtonGroup } from '../../widgets/TheButton'; import Callout from '../../widgets/Callout'; import SubmitButton from '../../forms/SubmitButton'; import { CloseIcon, SaveIcon, RefreshIcon } from '../../../components/icons'; -import { isArrayType } from '../../../helpers/pipelines'; +import { KNOWN_DATA_TYPES, isArrayType } from '../../../helpers/pipelines'; export const newVariableInitialData = { name: '', @@ -21,10 +21,7 @@ export const newVariableInitialData = { external: false, }; -const variableTypeOptions = ['file', 'remote-file', 'string'] - .reduce((acc, type) => [...acc, type, type + '[]'], []) - .sort() - .map(type => ({ key: type, name: type })); +const variableTypeOptions = KNOWN_DATA_TYPES.map(type => ({ key: type, name: type })); class VariableForm extends Component { render() { diff --git a/src/containers/PipelineEditContainer/PipelineEditContainer.js b/src/containers/PipelineEditContainer/PipelineEditContainer.js index 57ecd5bf6..b2a043b51 100644 --- a/src/containers/PipelineEditContainer/PipelineEditContainer.js +++ b/src/containers/PipelineEditContainer/PipelineEditContainer.js @@ -11,10 +11,9 @@ import VariablesTable from '../../components/Pipelines/VariablesTable'; import VariableForm, { newVariableInitialData } from '../../components/Pipelines/VariableForm'; import BoxForm, { newBoxInitialData } from '../../components/Pipelines/BoxForm'; import Button, { TheButtonGroup } from '../../components/widgets/TheButton'; +import Callout from '../../components/widgets/Callout'; import Icon, { RefreshIcon, SaveIcon, UndoIcon, RedoIcon } from '../../components/icons'; -import { fetchSupplementaryFilesForPipeline } from '../../redux/modules/pipelineFiles'; - import { getVariablesUtilization, isArrayType, @@ -23,12 +22,18 @@ import { getReferenceIdentifier, makeExternalReference, getVariablesTypes, + validatePipeline, } from '../../helpers/pipelines'; import { getBoxTypes } from '../../redux/selectors/boxes'; import { objectMap, arrayToObject, encodeId, deepCompare, identity } from '../../helpers/common'; import styles from '../../components/Pipelines/styles.less'; +const getFormattedErrorAsKey = element => { + const values = element.props.values ? { ...element.props.values } : {}; + delete values.code; + return `${element.props.id}-${Object.values(values).join('_')}`; +}; // TODO /* @@ -90,6 +95,8 @@ class PipelineEditContainer extends Component { version: null, boxes: null, variables: null, + boxTypes: null, + errors: [], history: [], future: [], ...STATE_DEFAULTS, @@ -97,17 +104,32 @@ class PipelineEditContainer extends Component { static getDerivedStateFromProps(nextProps, prevState) { if (prevState.pipelineId !== nextProps.pipeline.id) { + // pipeline was changed whilst component was kept mounted => complete reload return { pipelineId: nextProps.pipeline.id, version: nextProps.pipeline.version, boxes: nextProps.pipeline.pipeline.boxes, variables: nextProps.pipeline.pipeline.variables, + boxTypes: nextProps.boxTypes, + errors: validatePipeline( + nextProps.pipeline.pipeline.boxes, + nextProps.pipeline.pipeline.variables, + nextProps.boxTypes + ), history: [], future: [], ...STATE_DEFAULTS, }; } + if (prevState.boxTypes !== nextProps.boxTypes) { + // boxTypes changed (probably get loaded) -> revalidate + return { + boxTypes: nextProps.boxTypes, + errors: validatePipeline(prevState.boxes, prevState.variables, nextProps.boxTypes), + }; + } + if (prevState.version < nextProps.pipeline.version) { // TODO -- deal with mergin issues return { version: nextProps.pipeline.version }; @@ -126,6 +148,11 @@ class PipelineEditContainer extends Component { this.setState({ ...restore, + errors: validatePipeline( + restore.boxes || this.state.boxes, + restore.variables || this.state.variables, + this.props.boxTypes + ), history, future: [snapshot, ...this.state.future], selectedBoxVariables: _getSelectedBoxVariables(this.state.selectedBox, restore.boxes || this.state.boxes), @@ -143,6 +170,11 @@ class PipelineEditContainer extends Component { this.setState({ ...restore, + errors: validatePipeline( + restore.boxes || this.state.boxes, + restore.variables || this.state.variables, + this.props.boxTypes + ), history: [snapshot, ...this.state.history], future, selectedBoxVariables: _getSelectedBoxVariables(this.state.selectedBox, restore.boxes || this.state.boxes), @@ -163,6 +195,11 @@ class PipelineEditContainer extends Component { version: this.props.pipeline.version, boxes: this.props.pipeline.pipeline.boxes, variables: this.props.pipeline.pipeline.variables, + errors: validatePipeline( + this.props.pipeline.pipeline.boxes, + this.props.pipeline.pipeline.variables, + this.props.boxTypes + ), history: [], future, ...STATE_DEFAULTS, @@ -284,6 +321,11 @@ class PipelineEditContainer extends Component { if (pipelineChanged) { stateUpdate.history = [snapshot, ...this.state.history]; stateUpdate.future = []; + stateUpdate.errors = validatePipeline( + stateUpdate.boxes || this.state.boxes, + stateUpdate.variables || this.state.variables, + this.props.boxTypes + ); } // update (recompute) selections if necessary @@ -461,6 +503,7 @@ class PipelineEditContainer extends Component { } unlimitedHeight + type={this.state.errors && this.state.errors.length > 0 ? 'danger' : 'light'} footer={
@@ -490,7 +533,7 @@ class PipelineEditContainer extends Component { - @@ -537,6 +580,27 @@ class PipelineEditContainer extends Component { )} + + {this.state.errors && this.state.errors.length > 0 && ( + + + +
+ + : +
+
    + {this.state.errors.map(error => ( +
  • {error}
    • + ))} +
+ + + + )} { + state => { return { boxTypes: getBoxTypes(state), }; }, (dispatch, { pipeline }) => ({ - loadFiles: () => dispatch(fetchSupplementaryFilesForPipeline(pipeline.id)), + // }) )(PipelineEditContainer); diff --git a/src/helpers/pipelines.js b/src/helpers/pipelines.js index 797245573..cfaa70926 100644 --- a/src/helpers/pipelines.js +++ b/src/helpers/pipelines.js @@ -1,9 +1,14 @@ /* * Functions related to pipeline editing and visualization. */ - +import React from 'react'; +import { FormattedMessage } from 'react-intl'; import { defaultMemoize } from 'reselect'; -import { arrayToObject } from './common'; +import { arrayToObject, identity } from './common'; + +export const KNOWN_DATA_TYPES = ['file', 'remote-file', 'string'] + .reduce((acc, type) => [...acc, type, type + '[]'], []) + .sort(); /** * Check whether given data type is a list of values. @@ -100,3 +105,345 @@ export const getVariablesTypes = defaultMemoize(variables => ({ type }) => type ) ); + +/* + * Validation + */ + +/** + * Make sure that names of boxes and variables are properly set (not empty). + * @param {Array} boxes + * @param {Array} variables + * @param {Array} errors output collector + */ +const validateNamesAreSet = (boxes, variables, errors) => { + const boxNames = boxes.map(({ name }) => name).filter(name => name && name.trim() !== ''); + if (boxes.length !== boxNames.length) { + errors.push( + + ); + } + if (new Set(boxNames).size !== boxNames.length) { + errors.push( + + ); + } + + const varNames = variables.map(({ name }) => name).filter(name => name && name.trim() !== ''); + if (variables.length !== varNames.length) { + errors.push( + + ); + } + if (new Set(varNames).size !== varNames.length) { + errors.push( + + ); + } +}; + +/** + * Verify that associated variable exist and match the port type. + * @param {string} boxName + * @param {string} ports + * @param {string} portName + * @param {Object} port + * @param {Object} variablesIndex + * @param {Array} errors output collector + * @returns + */ +const validatePortVariable = (boxName, ports, portName, port, variablesIndex, errors) => { + const variable = port.value; + if (!variable) { + return; // no variable associated, nothing to check + } + + if (!variablesIndex[variable]) { + // the variable does not exit + errors.push( + {content}, + }} + /> + ); + } else if (variablesIndex[variable].type !== port.type) { + // the type of the variable does not match + errors.push( + {content}, + }} + /> + ); + } +}; + +/** + * Validate ports of a given box whether its type match box type definition, + * and and associated variables exist and match their port types. + * @param {Object} box + * @param {Object} boxType + * @param {string} ports portsIn or portsOut + * @param {Object} variablesIndex build from variables array using name as key + * @param {Array} errors output collector + */ +const validateBoxPorts = (box, boxType, ports, variablesIndex, errors) => { + Object.keys(boxType[ports]).forEach(portName => { + const portDef = boxType[ports][portName]; + const port = box[ports] && box[ports][portName]; + if (!port) { + // port missing + errors.push( + {content}, + }} + /> + ); + } else { + // port type mismatch + if (port.type !== portDef.type) { + errors.push( + {content}, + }} + /> + ); + } + + validatePortVariable(box.name, ports, portName, port, variablesIndex, errors); + } + }); +}; + +/** + * Complete validation of boxes. + * - Boxes have known type, + * - structure of ports is matching that type, + * - and associated variables exist and match their port types. + * @param {Array} boxes + * @param {Array} variables + * @param {Object} boxTypes + * @param {Array} errors output collector + */ +const validateBoxes = (boxes, variables, boxTypes, errors) => { + const variablesIndex = arrayToObject(variables, ({ name }) => name); + boxes.forEach(box => { + if (!boxTypes[box.type]) { + errors.push( + {content}, + }} + /> + ); + } else { + const boxType = boxTypes[box.type]; + ['portsIn', 'portsOut'].forEach(ports => validateBoxPorts(box, boxType, ports, variablesIndex, errors)); + } + }); +}; + +/** + * Validate that variables have known types, value is matching that type, + * and no variable is attached to more than one output port. + * @param {Array} variables + * @param {Array} boxes + * @param {Array} errors output collector + */ +const validateVariables = (variables, boxes, errors) => { + const dataTypes = new Set(KNOWN_DATA_TYPES); + const utilization = getVariablesUtilization(boxes); + variables.forEach(variable => { + if (!dataTypes.has(variable.type)) { + errors.push( + {content}, + }} + /> + ); + } else if (!isVariableValueValid(variable)) { + errors.push( + {content}, + }} + /> + ); + } + + if (utilization[variable.name] && utilization[variable.name].portsOut.length > 1) { + errors.push( + {content}, + }} + /> + ); + } + }); +}; + +/** + * Build a oriented-graph representation of the pipeline (boxes are vertices, variables define edges). + * @param {Array} boxes + * @returns {Object} keys are box names, values are Set objects holding box names + */ +const buildGraph = boxes => { + const variablesToBoxes = {}; + boxes.forEach(box => + Object.values(box.portsIn) + .map(({ value }) => value) + .filter(identity) + .forEach(variable => { + if (!variablesToBoxes[variable]) { + variablesToBoxes[variable] = []; + } + variablesToBoxes[variable].push(box.name); + }) + ); + + return arrayToObject( + boxes, + ({ name }) => name, + box => + new Set( + Object.values(box.portsOut) + .map(({ value }) => value && variablesToBoxes[value]) + .filter(dests => dests && dests.length > 0) + .reduce((acc, dests) => [...acc, ...dests], []) + ) + ); +}; + +/** + * Internal function that performs DFS on the graph to detect loops. + * @param {Object} graph + * @param {string} vertex + * @param {Object} visited + * @param {Object} open + * @returns {boolean} true if a loop was detected + */ +const _visitGraphVertex = (graph, vertex, visited, open) => { + if (!vertex || !graph[vertex] || visited[vertex]) { + return false; + } + + if (open[vertex]) { + return true; + } + + open[vertex] = true; + const res = [...graph[vertex]].some(dest => _visitGraphVertex(graph, dest, visited, open)); + visited[vertex] = true; + return res; +}; + +/** + * Detect whether the graph has loop + * @param {Array} boxes + * @returns {boolean} true if the graph has an oriented loop + */ +const pipelineGraphHasLoops = boxes => { + const graph = buildGraph(boxes); + const open = {}; // vertices which have entered the processing + const visited = {}; // vertices which are done being processed + + let box = null; + let res = false; + while (!res && (box = Object.keys(graph).find(name => !visited[name]))) { + res = _visitGraphVertex(graph, box, visited, open); + } + return res; +}; + +/** + * Perform standard valiadtion of a pipeline. + * - All box names and variable names are properly filled. + * - All boxes, ports, and variables have valid types. + * - Variables assigned to ports match the port type. + * - No variable is used in two output ports. + * - There are no loops in the graph. + * @param {Array} boxes + * @param {Array} variables + * @return {Array|null} list of errors or null, if everything is ok + */ +export const validatePipeline = (boxes, variables, boxTypes) => { + const errors = []; + + validateNamesAreSet(boxes, variables, errors); + if (errors.length > 0) { + // if names are corrupted, no point in checking further... + return errors; + } + + validateBoxes(boxes, variables, boxTypes, errors); + validateVariables(variables, boxes, errors); + + if (errors.length === 0) { + // graph structure is only worth checking if everyting else is correct + if (pipelineGraphHasLoops(boxes)) { + errors.push( + + ); + } + } + + return errors.length > 0 ? errors : null; +}; diff --git a/src/locales/cs.json b/src/locales/cs.json index aad032f6e..078e66233 100644 --- a/src/locales/cs.json +++ b/src/locales/cs.json @@ -1115,6 +1115,7 @@ "app.pipelineEditContainer.addBoxButton": "Přidat krabičku", "app.pipelineEditContainer.addVariableButton": "Přidat proměnnou", "app.pipelineEditContainer.boxesTitle": "Krabičky", + "app.pipelineEditContainer.errorsCalloutTitle": "V pipeline byly nalezeny následující chyby", "app.pipelineEditContainer.title": "Struktura pipeline", "app.pipelineEditContainer.variablesTitle": "Proměnné", "app.pipelineEditor.AddBoxForm.title": "Přidat krabičku", @@ -1154,6 +1155,19 @@ "app.pipelines.createNew": "Vytvořit novou pipeline", "app.pipelines.listTitle": "Pipeline", "app.pipelines.title": "Seznam všech pipeline", + "app.pipelines.validation.invalidBoxType": "Neznámý typ {type} krabičky {name}.", + "app.pipelines.validation.invalidPortType": "Port {boxName}.{ports}.{portName} je typu {type}, ale {typeDef} je předepsán typem krabičky.", + "app.pipelines.validation.invalidVariableType": "Neznámý datový typ {type} u proměnné {name}.", + "app.pipelines.validation.loopDetected": "Pipeline obsahuje cyklus.", + "app.pipelines.validation.portMissing": "Port {boxName}.{ports}.{portName} je očekáván v krabičce daného typu, ale chybí.", + "app.pipelines.validation.portVariableDoesNotExist": "Port {boxName}.{ports}.{portName} má přiřazenou proměnnou {variable}, která neexistuje.", + "app.pipelines.validation.portVariableTypeMismatch": "Port {boxName}.{ports}.{portName} má přiřazenou proměnnou {variable}, ale její typ neodpovídá typu portu ({portType} != varType).", + "app.pipelines.validation.someBoxesHaveDuplicitNames": "Některé krabičky mají duplicitní jména.", + "app.pipelines.validation.someBoxesWithoutName": "Některé krabičky nemají vyplněné jméno.", + "app.pipelines.validation.someVariablesHaveDuplicitNames": "Některé proměnné mají duplicitní jména.", + "app.pipelines.validation.someVariablesWithoutName": "Některé proměnné nemají vyplněné jméno.", + "app.pipelines.validation.variableAttachedToMultiple": "Proměnná {name} je přiřazená více výstupním portům.", + "app.pipelines.validation.variableValueNotValid": "Hodnota proměnné {name} neodpovídá deklarovanému datovému typu {type}.", "app.pipelines.variableForm.duplicitName": "Toto jméno již bylo přiděleno pro jinou proměnnou.", "app.pipelines.variableForm.emptyName": "Jméno proměnné nemůže zůstat prázdné.", "app.pipelines.variableForm.external": "Reference na externí proměnnou ($)", @@ -1369,14 +1383,12 @@ "app.scoreConfigExpression.optimize.removeConstantsButton": "Nahrazení konstatních podvýrazů", "app.scoreConfigExpression.optimize.removeConstantsInfo": "Všechny konstantní podvýrazy (podstromy) budou vyhodnoceny a nahrazeny číselnými literály.", "app.scoreConfigExpression.optimize.title": "Optimalizovat výraz", - "generic.redo": "Znovu", "app.scoreConfigExpression.removeNode": "Odstranit tento uzel", "app.scoreConfigExpression.removeOnlyNode": "Odstranit pouze tento uzel (vnořené uzly se přesunou nahoru)", "app.scoreConfigExpression.sub.description": "Odečte hodnotu druhého vloženého uzlu od hodnoty prvního uzlu", "app.scoreConfigExpression.sum.description": "Sečte hodnoty všech vložených uzlů", "app.scoreConfigExpression.swapNodes": "Prohodit s vybraným uzlem (včetně podstromů)", "app.scoreConfigExpression.test-result.description": "Vstupní uzel obsahující hodnotu výsledku testu přiřazenou sudím (hodnota je v intervalu [0,1])", - "generic.undo": "Zpět", "app.scoreConfigExpression.value.description": "Číselný literál", "app.scoreConfigInfo.calculator": "Algoritmus", "app.scoreConfigInfo.createdAt": "Konfigurace uložena", @@ -1709,6 +1721,7 @@ "generic.noRecordsInTable": "V tabulce nejsou žádné záznamy.", "generic.operationFailed": "Operace se nezdařila. Prosíme, opakujte akci později.", "generic.reason": "Důvod", + "generic.redo": "Znovu", "generic.reevaluatedBy": "Znovu vyhodnotil(a)", "generic.refresh": "Občerstvit", "generic.remove": "Odebrat", @@ -1733,6 +1746,7 @@ "generic.submitted": "Odevzdáno", "generic.submitting": "Odevzdávání...", "generic.tags": "Nálepky", + "generic.undo": "Zpět", "generic.update": "Upravit", "generic.updated": "Upraveno", "generic.uploadedAt": "Nahráno", diff --git a/src/locales/en.json b/src/locales/en.json index 82ea26d17..8c21c4005 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -1115,6 +1115,7 @@ "app.pipelineEditContainer.addBoxButton": "Add Box", "app.pipelineEditContainer.addVariableButton": "Add Variable", "app.pipelineEditContainer.boxesTitle": "Boxes", + "app.pipelineEditContainer.errorsCalloutTitle": "The following errors were found in the pipeline", "app.pipelineEditContainer.title": "Edit Pipeline Structure", "app.pipelineEditContainer.variablesTitle": "Variables", "app.pipelineEditor.AddBoxForm.title": "Add a box", @@ -1154,6 +1155,19 @@ "app.pipelines.createNew": "Create New Pipeline", "app.pipelines.listTitle": "Pipelines", "app.pipelines.title": "List of All Pipelines", + "app.pipelines.validation.invalidBoxType": "Unknown box type {type} of box {name}.", + "app.pipelines.validation.invalidPortType": "Port {boxName}.{ports}.{portName} is of type {type}, but {typeDef} type is prescribed by the box type.", + "app.pipelines.validation.invalidVariableType": "Unknown data type {type} of variable {name}.", + "app.pipelines.validation.loopDetected": "Loop detected in the pipeline.", + "app.pipelines.validation.portMissing": "Port {boxName}.{ports}.{portName} is prescribed by the box type but missing.", + "app.pipelines.validation.portVariableDoesNotExist": "Port {boxName}.{ports}.{portName} has attached variable {variable} which does not exist.", + "app.pipelines.validation.portVariableTypeMismatch": "Port {boxName}.{ports}.{portName} has attached variable {variable}, but their types do not match ({portType} != varType).", + "app.pipelines.validation.someBoxesHaveDuplicitNames": "Some boxes have duplicit names.", + "app.pipelines.validation.someBoxesWithoutName": "At least one box does not have properly filled name.", + "app.pipelines.validation.someVariablesHaveDuplicitNames": "Some variables have duplicit names.", + "app.pipelines.validation.someVariablesWithoutName": "At least one variables does not have properly filled name.", + "app.pipelines.validation.variableAttachedToMultiple": "Variable {name} is attached to more than one output port.", + "app.pipelines.validation.variableValueNotValid": "Value of variable {name} does not match its declared data type {type}.", "app.pipelines.variableForm.duplicitName": "This name is already taken by another variable.", "app.pipelines.variableForm.emptyName": "Variable name cannot be empty.", "app.pipelines.variableForm.external": "External variable reference ($)", @@ -1369,14 +1383,12 @@ "app.scoreConfigExpression.optimize.removeConstantsButton": "Replace Constant Sub-expressions", "app.scoreConfigExpression.optimize.removeConstantsInfo": "All constant sub-expressions (sub-trees) are evaluated and replaced with numeric literal nodes.", "app.scoreConfigExpression.optimize.title": "Optimize The Expression", - "generic.redo": "Redo", "app.scoreConfigExpression.removeNode": "Remove this node", "app.scoreConfigExpression.removeOnlyNode": "Remove only this node (children are moved upwards)", "app.scoreConfigExpression.sub.description": "Subtracts the value of the second node from the value of the first node", "app.scoreConfigExpression.sum.description": "Computes a sum of all nested nodes", "app.scoreConfigExpression.swapNodes": "Swap with selected node (including sub-trees)", "app.scoreConfigExpression.test-result.description": "Input node that takes the result assigned by a judge to a particular test (as value in [0,1] range)", - "generic.undo": "Undo", "app.scoreConfigExpression.value.description": "Numeric literal", "app.scoreConfigInfo.calculator": "Algorithm", "app.scoreConfigInfo.createdAt": "Configured at", @@ -1709,6 +1721,7 @@ "generic.noRecordsInTable": "There are no records in the table.", "generic.operationFailed": "Operation failed. Please try again later.", "generic.reason": "Reason", + "generic.redo": "Redo", "generic.reevaluatedBy": "Re-evaluated by", "generic.refresh": "Refresh", "generic.remove": "Remove", @@ -1733,6 +1746,7 @@ "generic.submitted": "Submitted", "generic.submitting": "Submitting...", "generic.tags": "Tags", + "generic.undo": "Undo", "generic.update": "Update", "generic.updated": "Updated", "generic.uploadedAt": "Uploaded at",