-
Sub-task
-
Resolution: Done
-
Critical
-
False
-
False
-
Undefined
-
At the moment validation is performed in Reducers as the final stage of the immer "recipe" that updates model state. This woks great when an Action only needs to operate on and validate a single sub-tree element. However more sophisticated validations need access to multiple sub-trees; such as updating an Attribute that needs to possibly invalidate predicate in the Characteristics by checking the ModelFields sub-tree.
Problems
Fractured code
The validation needs to be performed in a Reducer "higher" up the model tree where access to both Characteristics and ModelFields. This is not keeping validation "close to" the state that is changing. It is also fragmenting different types of validation for a particular sub-tree into multiple Reducers.
Pre-application of state changes
When validation is performed "higher" up the model tree the state change for the child sub-tree has not been applied and hence the validation code has to "pre-apply" the pending change to the model in order to correctly validate the change (that "is coming" when applied by the child reducer). For example, when deleting a ModelField; validation of the Characteristics Attribute}}s predicates in the {{ScorecardReducer needs to remove the ModelField from the model before the child ModelFieldReducer has updated the state. This is because the ScorecardReducer executes before the child ModelFieldReducer has executed.
Proposal
Validate after the whole model has been updated
At the moment validation is performed in the immer recipe. The proposal is to move validation into another function that is registered as an optional parameter on the HistoryService.batch(..) function. When HistoryService.commit(..) is called and the whole model updated the registered validation methods are invoked with the top level PMML object. Each validator function then has full access to the whole updated model.
interface BatchEntry<M> { state: M; path: string | null; recipe: (draft: WritableDraft<M>) => void; validate?: (pmml: PMML) => void; } public batch = <M>( state: M, path: string | null, recipe: (draft: WritableDraft<M>) => void, validate?: (pmml: PMML) => void ): void => { this.pending.push({ state: state, path: path, recipe: recipe, validate: validate }); }; public commit = (state: PMML | undefined): PMML | undefined => { if (state === undefined) { return; } // Apply state changes const newState = this.mutate(state, null, draft => { this.pending.forEach(be => { const segment = be.path === null ? draft : get(draft, be.path as string); be.recipe(segment as WritableDraft<any>); }); }); //Validate the changes this.pending.forEach(be => { if (be.validate !== undefined) { be.validate(state); } }); this.pending = new Array<BatchEntry<any>>(); return newState; }; ... case Actions.DeleteMiningSchemaField: service.batch( state, `models[${action.payload.modelIndex}].MiningSchema`, draft => { const miningSchemaIndex = action.payload.miningSchemaIndex; if (miningSchemaIndex >= 0 && miningSchemaIndex < draft.MiningField.length) { draft.MiningField.splice(miningSchemaIndex, 1); } }, pmml => { validation.clear(`models[${action.payload.modelIndex}].MiningSchema`); validateMiningFields(action.payload.modelIndex, pmml, validation); } );