-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New validation interfaces to support incremental validation on multiple data models #701
Conversation
_logger.LogError( | ||
"Could not determine if {dataType} requires app logic for application {org}/{app}", | ||
dataType, | ||
org, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
e, | ||
"Error while running validator {validatorName} for task {taskId} on instance {instanceId}", | ||
v.ValidationSource, | ||
taskId, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
"Error while running validator {validatorName} for task {taskId} on instance {instanceId}", | ||
v.ValidationSource, | ||
taskId, | ||
instance.Id |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
e, | ||
"Error while running validator {validatorName} on task {taskId} in instance {instanceId}", | ||
validator.GetType().Name, | ||
taskId, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
"Error while running validator {validatorName} on task {taskId} in instance {instanceId}", | ||
validator.GetType().Name, | ||
taskId, | ||
instance.Id |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
"Could not determine if {dataType} requires app logic for application {org}/{app}", | ||
dataType, | ||
org, | ||
app |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs
Dismissed
Show dismissed
Hide dismissed
src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs
Fixed
Show fixed
Hide fixed
src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs
Fixed
Show fixed
Hide fixed
CachedInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor( | ||
instance, | ||
_dataClient, | ||
_appMetadata, | ||
_appModel | ||
); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
dataAccessor
* new interface IValidator to rule them all * Incremental validation is now a separate interface * Try to add compatibility
75b75ce
to
657efd6
Compare
src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs
Dismissed
Show dismissed
Hide dismissed
src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs
Dismissed
Show dismissed
Hide dismissed
Quality Gate passedIssues Measures |
The previous three validation interfaces
ITaskValidator
,IDataElementValidator
andIFormDataValidator
where only the formData variant supported incremental validationHasRelevantChanges
does not work properly in a world where validation depends on the data from multiple data models. The hardest issue to solve was that our standard validators would run multiple times for different data elements, and the only way to fix that was to write aITaskValidator
but that would not support incremental validation.The proposed solution here is to abandon the concepts of validating single data elements individually, but only ever validate everything on a single task (possibly incrementally by only running a subset of the validators selected by
HasRelevantChanges
). I also add aIInstanceDataAccessor
to allow validators to access the content of data elements in a way so that fetches are shared between validators and we ensure that the updated data is used in aPATCH
request.This caused the addition of a
IValidator
interface with compatibility implementations for the older interfacesPS In addition I marked
ValidationIssue.Source
obsolete, and createdValidationIssueWithSource
instead. The source parameter is set by theValidationService
anyway, so it makes sense to hide it for app developers so that they don't think about what value to give.Related Issue(s)
Likely remaining work
Altinn.App.Core.Features.Validation.Default
to implementIValidator
instead of relying on the compatibility layersVerification
Documentation