Skip to content
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

Closed
wants to merge 3 commits into from

Conversation

ivarne
Copy link
Member

@ivarne ivarne commented Jun 29, 2024

The previous three validation interfaces ITaskValidator, IDataElementValidator and IFormDataValidator where only the formData variant supported incremental validation HasRelevantChanges 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 a ITaskValidator 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 a IInstanceDataAccessor 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 a PATCH request.

This caused the addition of a IValidator interface with compatibility implementations for the older interfaces

PS In addition I marked ValidationIssue.Source obsolete, and created ValidationIssueWithSource instead. The source parameter is set by the ValidationService 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

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

_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

This log entry depends on a
user-provided value
.
This log entry depends on a
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

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
.
"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

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
.
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

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
.
"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

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
.
"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

This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
Comment on lines +514 to +519
CachedInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor(
instance,
_dataClient,
_appMetadata,
_appModel
);

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
dataAccessor
is useless, since its value is never read.
* new interface IValidator to rule them all
* Incremental validation is now a separate interface
* Try to add compatibility
@ivarne ivarne force-pushed the feat/ivarne/full-incrmental-validation branch from 75b75ce to 657efd6 Compare August 15, 2024 19:28
Copy link

sonarcloud bot commented Aug 15, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant