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

Add External Validation Rules for Analysis Properties Based on Dynamic Schema #877

Open
wants to merge 6 commits into
base: feat/refactor-analyis-data-model
Choose a base branch
from

Conversation

Azher2Ali
Copy link
Contributor

@Azher2Ali Azher2Ali commented Oct 31, 2024

This PR introduces external validation capabilities for analysis properties, allowing for dynamic schemas to specify validation rules that are stored in the database. Each rule specifies a jsonPath to identify the property for validation and a URL to an external service for validation logic. During analysis submission, the system will validate specific property values by querying the external service, confirming that the values (e.g., donor IDs) meet the criteria defined by the external registry or ontology.

  • Adds support for configurable external validation rules per analysis type.

  • Enables database storage of validation rules tied to specific analysis types.

  • Implements web request calls to validate property values according to the rules during submission.

This update also introduces external validation checks for analyses during creation and update in SONG. These checks are applied after JSON schema validation and file type validation. For each configured external validation rule on an analysis type, the system will:

Check Analysis Properties: Determine if the property at the specified path has a value. If no value exists, the validation is considered passed.

Validate Using External Source: If a value is present, SONG constructs a GET request to the external validation URL. The URL includes query parameters:

  • studyId: the study identifier the analysis is associated with.

  • value: the value of the property being validated.

Process the External Validation Response:

  • If the value is valid, the external service returns an HTTP 200 status with no response body.

  • If invalid, the external service returns an HTTP 400 status and optionally includes a JSON error message explaining why the value is invalid. This message is provided to the submitter for clarity.

  • Other HTTP errors indicate a validation failure, and a message is returned to the submitter stating that external validation was unsuccessful, preventing the analysis submission/update.

This update sets the groundwork for calling external validation services, but the actual call to the external service is pending final implementation .We added this feature with an exception that the actual call to the external service is skipped as the service is not up and running and if we start invoking it, this will fail all the validations #872 #875

@Azher2Ali Azher2Ali requested review from joneubank, leoraba and justincorrigible and removed request for joneubank October 31, 2024 18:37
Copy link
Contributor

@leoraba leoraba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far so good! Just left some comments to consider 📝

@Azher2Ali Azher2Ali requested a review from leoraba November 6, 2024 17:33
Comment on lines 143 to 150
URIBuilder uriBuilder = new URIBuilder(url);
uriBuilder.addParameter("studyId", studyId);
uriBuilder.addParameter("value",value);
// Make the HTTP GET request
ResponseEntity<Void> response = restTemplate.getForEntity(uriBuilder.build().toURL().toString(), Void.class);

// Return true if the response status is 200 OK
return response.getStatusCode().is2xxSuccessful();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -100,6 +119,7 @@ public Optional<String> validate(@NonNull JsonNode payload) {
validateFileType(fileTypes, payload);
}

// log.info("SCHEMA :- " + analysisType.getSchema());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented code.

public boolean invokeExternalUrl(String studyId, String url, String value) {
try {

Pattern pattern = Pattern.compile(LYRIC_URL_REGEX);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about external validation URLs when they are validating against a service that isn't Lyric? We need to define a schema for writing a URL that isn't specific to Lyric. This is about defining string template rules with variables we will replace with the studyId and entity name.

Perhaps ${studyId}, ${entityName}, and ${entityId}. So an external URL value could be https://mydomain.example.org/whatever/path/to/validate/${studyId}/${entityName}/${entityId}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code will take any URL apart from lyric, but for the path and path params should be well defined in order to have a common logic

Comment on lines 3 to 4
ALTER TABLE public.analysis_schema
DROP COLUMN file_types;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we published this version with file types yet? We would need to migrate all the file_types that have already been set into the new analysis schema format.

Did you consider storign analysis_schema in a separate column instead of storing options as a JSON?

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.

3 participants