Skip to content

Commit

Permalink
For checkActionIdsExist() checks conditions, explicitly check the res…
Browse files Browse the repository at this point in the history
…olved references are objects rather than relying on the behaviour of idExists().

Improved the context information provided by all errors to include index information for the position in the config file arrays.

Added mehtods to ConfigValidationError so that context can be added without having to pass excessive contextual information around.
  • Loading branch information
barnettwilliam committed Nov 28, 2023
1 parent dcc62b0 commit 40616a5
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 25 deletions.
56 changes: 32 additions & 24 deletions platform/src/ActivityValidator.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ConfigValidationError } from './ConfigValidationError';
import { ConfigValidationError, CONTEXT_SEPARATOR } from './ConfigValidationError';

const ERROR_CATEGORY = "ERROR_CONFIG_DEF"

Expand Down Expand Up @@ -29,7 +29,7 @@ class ActivityValidator {
if (!this.idExists(activity.panels, id)){
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A panel does not exist for the given id.",
`${activity.id} -> layout -> ${id}`, errorFileType.ACTIVITY)
`activity.id: ${activity.id}, layout ${CONTEXT_SEPARATOR} area ${CONTEXT_SEPARATOR} ${id}`, errorFileType.ACTIVITY)
);
}
});
Expand All @@ -39,46 +39,51 @@ class ActivityValidator {

/**
* Check the actions reference ids
* @param {*} activity - An activity with all references resolved.
* @param {Object} activity - An activity with all references resolved.
* @returns {ConfigValidationError[]} Array of configuration errors, empty if there are none.
*/
static checkActions(activity){
let errors = [];

activity.actions.forEach( (act) => {
errors = errors.concat( this.checkActionIdsExist(act, activity.panels) );
activity.actions.forEach( (act, index) => {
let newErrors = this.checkActionIdsExist(act, activity.panels);

ConfigValidationError.addContextToErrors(newErrors, `activity.id: ${activity.id}, actions[${index}]`);
errors = errors.concat(newErrors);
});



return errors;
}

/**
* Check an action panel ids exist
* @param {*} action - The action to check
* @param {*} panels - The panels
* @param {Object} action - The action to check.
* @param {Object[]} panels - The panels.
* @returns {ConfigValidationError[]} Array of configuration errors, empty if there are none.
*/
static checkActionIdsExist(action, panels){
let errors = [];

if (!this.idExists( panels, action.source.id)){ // action ids are resolved so get id from object
if ( !(action.source instanceof Object) && !this.idExists(panels, action.source.id) ){ // action ids are resolved so get id from object
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A panel does not exist for the given id.",
`actions -> source: ${action.source}`, errorFileType.ACTIVITY)
`source: ${action.source}`, errorFileType.ACTIVITY)
);
}

if (!this.idExists( panels, action.output.id)){ // action ids are resolved so get id from object
if ( !(action.output instanceof Object) && !this.idExists(panels, action.output.id) ){ // action ids are resolved so get id from object
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A panel does not exist for the given id.",
`actions -> output: ${action.output}`, errorFileType.ACTIVITY)
`output: ${action.output}`, errorFileType.ACTIVITY)
);
}

if ( (action.outputConsole !=null) && !this.idExists( panels, action.outputConsole.id)){ // action ids are resolved so get id from object
if ( action.outputConsole != null && !(action.outputConsole instanceof Object) && !this.idExists( panels, action.outputConsole.id) ){ // action ids are resolved so get id from object
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A panel does not exist for the given id.",
`actions -> outputConsole: ${action.outputConsole}`, errorFileType.ACTIVITY)
`outputConsole: ${action.outputConsole}`, errorFileType.ACTIVITY)
);
}

Expand All @@ -87,17 +92,17 @@ class ActivityValidator {

/**
* Checks the given panels references exist.
* @param {*} panels - The panels with references resolved.
* @param {Object[]} activity - The activity with references resolved.
* @returns {ConfigValidationError[]} Array of configuration errors, empty if there are none.
*/
static checkPanelRefs(panels){
static checkPanelRefs(activity){
let errors = [];

panels.forEach( (pn) => {
activity.panels.forEach( (pn, index) => {
if (!(pn.ref instanceof Object)){ // Un-resolved panels remain as ids
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A referenced panel definition does not exist with the given id.",
`panels -> id: ${pn.id}, ref: ${pn.ref}`, errorFileType.ACTIVITY)
`activity.id: ${activity.id}, panels[${index}] ${CONTEXT_SEPARATOR} id: ${pn.id}, ref: ${pn.ref}`, errorFileType.ACTIVITY)
);
}
});
Expand All @@ -114,17 +119,20 @@ class ActivityValidator {
static checkPanelDefs(tool){
let errors = [];

tool.panelDefs.forEach( (pDef) => {
errors = errors.concat( this.checkPanelButtonsFunctionIdsExist(pDef, tool.functions) );
tool.panelDefs.forEach( (pDef, index) => {
let newErrors = this.checkPanelButtonsFunctionIdsExist(pDef, tool.functions);

ConfigValidationError.addContextToErrors(newErrors, `tool.id: ${tool.id}, panelDefs[${index}]:`);
errors = errors.concat(newErrors);
});

return errors;
}

/**
* Check buttons function ids exist for a given panel.
* @param {*} panel - Panel to check.
* @param {*} functions - The available functions.
* @param {Obejct} panel - Panel to check.
* @param {Object[]} functions - The available functions.
* @returns {ConfigValidationError[]} Array of configuration errors, empty if there are none.
*/
static checkPanelButtonsFunctionIdsExist(panel, functions) {
Expand All @@ -136,12 +144,12 @@ class ActivityValidator {
if ( ("actionfunction" in btn) && !this.idExists(functions, btn.actionfunction) ) {
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A function does not exist for the given id.",
`panel.id: ${panel.id} -> buttton.id: ${btn.id}, actionfunction: ${btn.actionfunction}`, errorFileType.TOOL)
`id: ${panel.id} ${CONTEXT_SEPARATOR} buttton.id: ${btn.id}, actionfunction: ${btn.actionfunction}`, errorFileType.TOOL)
);
} else if (("renderfunction" in btn) && !this.idExists(functions, btn.renderfunction) ){
errors.push(
new ConfigValidationError(ERROR_CATEGORY, "A function does not exist for the given id.",
`panel.id: ${panel.id} -> buttton.id: ${btn.id}, renderfunction: ${btn.renderfunction}`, errorFileType.TOOL)
`id: ${panel.id} ${CONTEXT_SEPARATOR} buttton.id: ${btn.id}, renderfunction: ${btn.renderfunction}`, errorFileType.TOOL)
);
}
});
Expand Down Expand Up @@ -198,7 +206,7 @@ class ActivityValidator {

errors = errors.concat( this.checkActions(activity) );

errors = errors.concat( this.checkPanelRefs(activity.panels) );
errors = errors.concat( this.checkPanelRefs(activity) );

return errors;
}
Expand Down
23 changes: 22 additions & 1 deletion platform/src/ConfigValidationError.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {EducationPlatformError} from "./EducationPlatformError.js"

const CONTEXT_SEPARATOR = "->";

class ConfigValidationError extends EducationPlatformError {

fileType;
Expand All @@ -15,6 +17,25 @@ class ConfigValidationError extends EducationPlatformError {
this.fileType= fileType;
}


/**
* Prepends the addtional context to errors location.
* @param {String} context - The context to prepend.
* @param {String} separator - The separtor to insert.
*/
addContext(context, separator=CONTEXT_SEPARATOR){
this.location = context + " " + separator + " " + this.location;
}

/**
* Prepend additional context to multiple errors.
* @param {ConfigValidationError[]} errors
* @param {String} context - The context to prepend.
* @param {String} separator - The separtor to insert.
*/
static addContextToErrors(errors, context, separator=CONTEXT_SEPARATOR){
errors.forEach( (e) => e.addContext(context, separator) );
}
}

export {ConfigValidationError}
export {ConfigValidationError, CONTEXT_SEPARATOR}

0 comments on commit 40616a5

Please sign in to comment.