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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that in the long term it would be best to store all options as their own dedicated columns in postgres. For the scope of this PR, we'll just leave it as a column for options since this is working today.

@joneubank joneubank merged commit dd4c9f8 into feat/refactor-analyis-data-model Jan 27, 2025
2 checks passed
@joneubank joneubank deleted the feature/external-validation-rules-for-analysis branch January 27, 2025 20:17
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