From fa644ecf955a20723b889ebd196c381067b30137 Mon Sep 17 00:00:00 2001 From: Azher2Ali <121898125+Azher2Ali@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:36:14 -0400 Subject: [PATCH 1/9] Add External Validation Rules for Analysis Properties Based on Dynamic Schemas --- .../server/service/AnalysisTypeService.java | 35 +++++++++++-- .../song/server/service/SubmitService.java | 11 ++-- .../server/service/ValidationService.java | 50 ++++++++++++++++++- ..._21__remove_file_types_and_add_options.sql | 8 +++ .../song/server/service/MockedSubmitTest.java | 16 +++--- .../validation/ValidationServiceTest.java | 18 +++---- 6 files changed, 110 insertions(+), 28 deletions(-) create mode 100644 song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index 539053bea..780c9c4c2 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -155,9 +155,11 @@ public AnalysisType getAnalysisType( val resolvedSchemaJson = resolveSchemaJsonView(analysisSchema.getSchema(), unrenderedOnly, false); + AnalysisTypeOptions options = analysisSchema.getOptions(); + List fileTypes = - (analysisSchema.getFileTypes() != null && !analysisSchema.getFileTypes().isEmpty()) - ? analysisSchema.getFileTypes() + (options.getFileTypes() != null && !options.getFileTypes().isEmpty()) + ? options.getFileTypes() : new ArrayList<>(); return AnalysisType.builder() .name(analysisTypeId.getName()) @@ -265,11 +267,33 @@ private AnalysisType commitAnalysisType( if (options != null && CollectionUtils.isNotEmpty(options.getFileTypes())) { fileTypes = options.getFileTypes(); } + + // checking if file types is empty + // if the version is new version of the schema , we are checking the previous version allowed + // file types + // if it is new then it is empty + if (fileTypes.isEmpty()) { + List analysisSchemaList = + analysisSchemaRepository.findAllByName(analysisTypeName); + + if (!analysisSchemaList.isEmpty()) { + Optional latestSchemaOptional = + analysisSchemaList.stream() + .filter(schema -> schema.getVersion() != null) + .max(Comparator.comparingInt(AnalysisSchema::getVersion)); + + if (latestSchemaOptional.isPresent()) { + AnalysisTypeOptions optionsFromDb = latestSchemaOptional.get().getOptions(); + fileTypes = optionsFromDb.getFileTypes(); + } + } + } + val analysisSchema = AnalysisSchema.builder() .name(analysisTypeName) .schema(analysisTypeSchema) - .fileTypes(fileTypes) + .options(options) .build(); log.debug("Creating analysisSchema with file types: {} " + fileTypes); @@ -319,7 +343,10 @@ public static AnalysisTypeId resolveAnalysisTypeId(@NonNull AnalysisType analysi private AnalysisType convertToAnalysisType( AnalysisSchema analysisSchema, boolean hideSchema, boolean unrenderedOnly) { AnalysisTypeOptions options = new AnalysisTypeOptions(); - options.setFileTypes(analysisSchema.getFileTypes()); + if (analysisSchema.getOptions() != null) { + options.setFileTypes(analysisSchema.getOptions().getFileTypes()); + options.setExternalValidation(analysisSchema.getOptions().getExternalValidation()); + } return AnalysisType.builder() .name(analysisSchema.getName()) .version(analysisSchema.getVersion()) diff --git a/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java b/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java index 93d4dbd8a..9116dad57 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/SubmitService.java @@ -58,7 +58,8 @@ public SubmitService( this.studyService = studyService; } - public SubmitResponse submit(@NonNull String studyId, String payloadString, boolean allowDuplicates) { + public SubmitResponse submit( + @NonNull String studyId, String payloadString, boolean allowDuplicates) { // Check study exists studyService.checkStudyExist(studyId); @@ -66,7 +67,7 @@ public SubmitResponse submit(@NonNull String studyId, String payloadString, bool val payloadJson = parsePayload(payloadString); // Validate JSON Payload - validatePayload(payloadJson); + validatePayload(payloadJson, studyId); // Deserialize JSON payload to Payload DTO val payload = fromJson(payloadJson, Payload.class); @@ -75,7 +76,7 @@ public SubmitResponse submit(@NonNull String studyId, String payloadString, bool checkStudyInPayload(studyId, payload); // check duplicate analysis - if(!allowDuplicates){ + if (!allowDuplicates) { analysisService.checkDuplicateAnalysis(payload); } @@ -119,9 +120,9 @@ private AnalysisTypeId parseAnalysisTypeId(@NonNull JsonNode payloadJson) { return fromJson(analysisTypePath, AnalysisTypeId.class); } - private void validatePayload(JsonNode payloadJson) { + private void validatePayload(JsonNode payloadJson, String studyId) { // Validate payload format and content - val error = validator.validate(payloadJson); + val error = validator.validate(payloadJson, studyId); if (error.isPresent()) { val message = error.get(); throw buildServerException(getClass(), SCHEMA_VIOLATION, message); diff --git a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java index 7b35f77ee..407a58b35 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java @@ -29,14 +29,17 @@ import static org.apache.commons.lang.StringUtils.isBlank; import bio.overture.song.core.model.AnalysisTypeId; +import bio.overture.song.core.model.ExternalValidation; import bio.overture.song.core.model.FileData; import bio.overture.song.server.model.enums.UploadStates; import bio.overture.song.server.repository.UploadRepository; import bio.overture.song.server.validation.SchemaValidator; import bio.overture.song.server.validation.ValidationResponse; import com.fasterxml.jackson.databind.JsonNode; +import com.jayway.jsonpath.JsonPath; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; import lombok.NonNull; @@ -44,11 +47,14 @@ import lombok.extern.slf4j.Slf4j; import lombok.val; import org.apache.commons.collections.CollectionUtils; +import org.apache.http.client.utils.URIBuilder; import org.everit.json.schema.Schema; import org.everit.json.schema.ValidationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; +import org.springframework.web.client.RestTemplate; @Slf4j @Service @@ -62,6 +68,7 @@ public class ValidationService { private final UploadRepository uploadRepository; private final boolean enforceLatest; private final Schema analysisTypeIdSchema; + private final RestTemplate restTemplate; @Autowired public ValidationService( @@ -69,16 +76,18 @@ public ValidationService( @NonNull SchemaValidator validator, @NonNull AnalysisTypeService analysisTypeService, @NonNull Supplier analysisTypeIdSchemaSupplier, - @NonNull UploadRepository uploadRepository) { + @NonNull UploadRepository uploadRepository, + @NonNull RestTemplate restTemplate) { this.validator = validator; this.analysisTypeService = analysisTypeService; this.uploadRepository = uploadRepository; this.enforceLatest = enforceLatest; this.analysisTypeIdSchema = analysisTypeIdSchemaSupplier.get(); + this.restTemplate = restTemplate; } @SneakyThrows - public Optional validate(@NonNull JsonNode payload) { + public Optional validate(@NonNull JsonNode payload, @NonNull String studyId) { String errors = null; try { validateWithSchema(analysisTypeIdSchema, payload); @@ -109,6 +118,43 @@ public Optional validate(@NonNull JsonNode payload) { return Optional.ofNullable(errors); } + private void externalValidations( + List externalValidations, @NonNull JsonNode payload) { + + for (ExternalValidation externalValidation : externalValidations) { + String jsonPath = externalValidation.getJsonPath(); + String externalUrl = externalValidation.getUrl(); + + // Extract the value from the uploaded JSON using the jsonPath + String value = JsonPath.read(payload, "$." + jsonPath); + + if(Objects.isNull(value) || value.isEmpty()) + return; + + // call the external url using restTemplate + } + } + + public boolean invokeExternalUrl(String studyId, String url, String value) { + // will change the logic to make it parameterised + try { + + // create URL + URIBuilder uriBuilder = new URIBuilder(url); + uriBuilder.addParameter("studyId", studyId); + uriBuilder.addParameter("value",value); + // Make the HTTP GET request + ResponseEntity response = restTemplate.getForEntity(uriBuilder.build().toURL().toString(), Void.class); + + // Return true if the response status is 200 OK + return response.getStatusCode().is2xxSuccessful(); + } catch (Exception e) { + // Handle exceptions (e.g., log them) + System.err.println("Validation failed " + e.getMessage()); + return false; + } + } + private void validateFileType(List fileTypes, @NonNull JsonNode payload) { if (CollectionUtils.isNotEmpty(fileTypes)) { diff --git a/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql b/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql new file mode 100644 index 000000000..0cb44b92f --- /dev/null +++ b/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql @@ -0,0 +1,8 @@ + +-- Remove the file_types column +ALTER TABLE public.analysis_schema +DROP COLUMN file_types; + +-- Add the options column as jsonb (nullable by default) +ALTER TABLE public.analysis_schema +ADD COLUMN options jsonb; \ No newline at end of file diff --git a/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java b/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java index 8c7b42966..854a754e2 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java @@ -101,7 +101,7 @@ public void submit_StudyDne_StudyIdDoesNotExist() { // Verify assertSongError( () -> submitService.submit("anyStudy", "anyAnalysisId", false), STUDY_ID_DOES_NOT_EXIST); - verify(validationService, never()).validate(isA(JsonNode.class)); + verify(validationService, never()).validate(isA(JsonNode.class),"anyStudy"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -113,7 +113,7 @@ public void submit_malformedPayload_PayloadParsingError() { // Verify assertSongError( () -> submitService.submit("anyStudy", "non json format", false), PAYLOAD_PARSING); - verify(validationService, never()).validate(isA(JsonNode.class)); + verify(validationService, never()).validate(isA(JsonNode.class),"anyStudy"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -122,7 +122,7 @@ public void submit_invalidPayload_SchemaViolation() { // Setup val studyId = "anyStudy"; doNothing().when(studyService).checkStudyExist(anyString()); - when(validationService.validate(isA(JsonNode.class))) + when(validationService.validate(isA(JsonNode.class), "anyStudy")) .thenReturn(Optional.of("there was an error")); // Create an invalid payload and not a malformed one @@ -136,7 +136,7 @@ public void submit_invalidPayload_SchemaViolation() { // Verify assertSongError(() -> submitService.submit(studyId, invalidPayload, false), SCHEMA_VIOLATION); - verify(validationService, times(1)).validate(isA(JsonNode.class)); + verify(validationService, times(1)).validate(isA(JsonNode.class),"anyStudy"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -146,7 +146,7 @@ public void submit_mismatchingStudies_StudyIdMismatch() { val study1 = "study1"; val study2 = "study2"; doNothing().when(studyService).checkStudyExist(anyString()); - when(validationService.validate(isA(JsonNode.class))).thenReturn(Optional.empty()); + when(validationService.validate(isA(JsonNode.class), "anyStudy")).thenReturn(Optional.empty()); val payloadString = toJson( Payload.builder() @@ -158,7 +158,7 @@ public void submit_mismatchingStudies_StudyIdMismatch() { // Verify assertNotEquals(study1, study2); assertSongError(() -> submitService.submit(study2, payloadString, false), STUDY_ID_MISMATCH); - verify(validationService, times(1)).validate(isA(JsonNode.class)); + verify(validationService, times(1)).validate(isA(JsonNode.class), "anyStudy"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -174,7 +174,7 @@ public void submit_validPayload_Success() { .build(); doNothing().when(studyService).checkStudyExist(anyString()); - when(validationService.validate(isA(JsonNode.class))).thenReturn(Optional.empty()); + when(validationService.validate(isA(JsonNode.class), "anyStudy")).thenReturn(Optional.empty()); val payloadString = toJson(payload); when(analysisService.create(study, payload)) @@ -184,7 +184,7 @@ public void submit_validPayload_Success() { // Verify val actualSubmitResponse = submitService.submit(study, payloadString, false); assertEquals(expectedSubmitResponse, actualSubmitResponse); - verify(validationService, times(1)).validate(isA(JsonNode.class)); + verify(validationService, times(1)).validate(isA(JsonNode.class), "anyStudy"); verify(analysisService, times(1)).create(anyString(), isA(Payload.class)); } } diff --git a/song-server/src/test/java/bio/overture/song/server/validation/ValidationServiceTest.java b/song-server/src/test/java/bio/overture/song/server/validation/ValidationServiceTest.java index b3e59c9e8..d07a87d71 100644 --- a/song-server/src/test/java/bio/overture/song/server/validation/ValidationServiceTest.java +++ b/song-server/src/test/java/bio/overture/song/server/validation/ValidationServiceTest.java @@ -64,21 +64,21 @@ public class ValidationServiceTest { @Test public void testValidateValidSequencingRead() { val payload = getJsonFile("sequencingRead.json"); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertFalse(results.isPresent()); } @Test public void testValidateValidSequencingReadWithArchive() { val payload = getJsonFile("sequencingReadWithArchive.json"); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertFalse(results.isPresent()); } @Test public void testValidateValidVariantCall() { val payload = getJsonFile("variantCall.json"); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertFalse(results.isPresent()); } @@ -86,7 +86,7 @@ public void testValidateValidVariantCall() { public void testValidateVariantCallNullAnalysisType() { val payload = getJsonFile("variantCall.json"); ((ObjectNode) payload).put("analysisType", (String) null); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertTrue(results.isPresent()); assertTrue(results.get().contains("#/analysisType: expected type: JSONObject, found:")); } @@ -95,7 +95,7 @@ public void testValidateVariantCallNullAnalysisType() { public void testValidateVariantCallMissingAnalysisType() { val payload = getJsonFile("variantCall.json"); ((ObjectNode) payload).remove("analysisType"); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertTrue(results.isPresent()); assertTrue(results.get().contains("#: required key [analysisType] not found")); } @@ -104,7 +104,7 @@ public void testValidateVariantCallMissingAnalysisType() { public void testValidateSequencingReadNullAnalysisType() { val payload = getJsonFile("sequencingRead.json"); ((ObjectNode) payload).put("analysisType", (String) null); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertTrue(results.isPresent()); assertTrue(results.get().contains("#/analysisType: expected type: JSONObject, found:")); } @@ -113,7 +113,7 @@ public void testValidateSequencingReadNullAnalysisType() { public void testValidateSequencingReadMissingAnalysisType() { val payload = getJsonFile("sequencingRead.json"); ((ObjectNode) payload).remove("analysisType"); - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertTrue(results.isPresent()); assertTrue(results.get().contains("#: required key [analysisType] not found")); } @@ -139,7 +139,7 @@ public void testFileMd5Validation() { "fileMd5sum", "q0123456789012345678901234567890123456789"); // more than 32 and non-hex number - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); assertTrue(results.isPresent()); @@ -176,7 +176,7 @@ private void runFileMd5sumValidationTest(String md5, String schemaType, boolean ((ObjectNode) fileNode).put("fileMd5sum", md5); } - val results = service.validate(payload); + val results = service.validate(payload, "anyStudy"); if (shouldBeError) { assertTrue(results.isPresent()); From f4f376fd6e76967639898fb17958ead033660c3d Mon Sep 17 00:00:00 2001 From: Azher2Ali <121898125+Azher2Ali@users.noreply.github.com> Date: Mon, 4 Nov 2024 12:30:39 -0500 Subject: [PATCH 2/9] Resolving the build issues and updating the PR with missing files --- .../song/core/model/AnalysisTypeOptions.java | 1 + .../song/core/model/ExternalValidation.java | 15 +++++++++++++++ .../song/server/config/RestTemplateConfig.java | 13 +++++++++++++ .../server/model/entity/AnalysisSchema.java | 17 ++++++----------- .../server/model/enums/TableAttributeNames.java | 1 + 5 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 song-core/src/main/java/bio/overture/song/core/model/ExternalValidation.java create mode 100644 song-server/src/main/java/bio/overture/song/server/config/RestTemplateConfig.java diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisTypeOptions.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisTypeOptions.java index 4aca09844..08f104f48 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/AnalysisTypeOptions.java +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisTypeOptions.java @@ -9,4 +9,5 @@ @AllArgsConstructor public class AnalysisTypeOptions { private List fileTypes; + private List externalValidation; } diff --git a/song-core/src/main/java/bio/overture/song/core/model/ExternalValidation.java b/song-core/src/main/java/bio/overture/song/core/model/ExternalValidation.java new file mode 100644 index 000000000..4682db1cf --- /dev/null +++ b/song-core/src/main/java/bio/overture/song/core/model/ExternalValidation.java @@ -0,0 +1,15 @@ +package bio.overture.song.core.model; + +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class ExternalValidation { + private String jsonPath; + private String url; +} diff --git a/song-server/src/main/java/bio/overture/song/server/config/RestTemplateConfig.java b/song-server/src/main/java/bio/overture/song/server/config/RestTemplateConfig.java new file mode 100644 index 000000000..433495ef7 --- /dev/null +++ b/song-server/src/main/java/bio/overture/song/server/config/RestTemplateConfig.java @@ -0,0 +1,13 @@ +package bio.overture.song.server.config; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.client.RestTemplate; + +@Configuration +public class RestTemplateConfig { + @Bean + public RestTemplate restTemplate() { + return new RestTemplate(); + } +} diff --git a/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java b/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java index f5595353d..75c509479 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java +++ b/song-server/src/main/java/bio/overture/song/server/model/entity/AnalysisSchema.java @@ -4,24 +4,18 @@ import static bio.overture.song.server.repository.CustomJsonType.CUSTOM_JSON_TYPE_PKG_PATH; import static com.google.common.collect.Sets.newHashSet; +import bio.overture.song.core.model.AnalysisTypeOptions; import bio.overture.song.server.model.analysis.Analysis; import bio.overture.song.server.model.enums.ModelAttributeNames; import bio.overture.song.server.model.enums.TableAttributeNames; import bio.overture.song.server.model.enums.TableNames; -import bio.overture.song.server.utils.StringListConverter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.JsonNode; import java.time.LocalDateTime; -import java.util.List; import java.util.Set; import javax.persistence.*; import javax.validation.constraints.NotNull; -import lombok.AllArgsConstructor; -import lombok.Builder; -import lombok.Data; -import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; -import lombok.ToString; +import lombok.*; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.annotations.Type; @@ -54,9 +48,10 @@ public class AnalysisSchema { @Type(type = CUSTOM_JSON_TYPE_PKG_PATH) private JsonNode schema; - @Column(name = FILE_TYPES, columnDefinition = "text[]") - @Convert(converter = StringListConverter.class) - private List fileTypes; + @NotNull + @Column(name = OPTIONS) + @Type(type = CUSTOM_JSON_TYPE_PKG_PATH) + private AnalysisTypeOptions options; @JsonIgnore @Builder.Default diff --git a/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java b/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java index 29e847ef1..156466e3b 100644 --- a/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java +++ b/song-server/src/main/java/bio/overture/song/server/model/enums/TableAttributeNames.java @@ -69,6 +69,7 @@ public class TableAttributeNames { public static final String MATCHED_NORMAL_SUBMITTER_SAMPLE_ID = "matched_normal_submitter_sample_id"; public static final String SCHEMA = "schema"; + public static final String OPTIONS = "options"; public static final String VERSION = "version"; public static final String ANALYSIS_SCHEMA_ID = "analysis_schema_id"; public static final String DATA = "data"; From ffde783d0d14b3529b9c87ac90065f1c855b0a7b Mon Sep 17 00:00:00 2001 From: Azher2Ali <121898125+Azher2Ali@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:32:21 -0500 Subject: [PATCH 3/9] Changes related to feedback --- .../server/service/AnalysisTypeService.java | 11 ++++---- .../song/server/service/MockedSubmitTest.java | 28 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index 780c9c4c2..eadaa8d39 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -289,6 +289,9 @@ private AnalysisType commitAnalysisType( } } + if(options!=null) + options.setFileTypes(fileTypes); + val analysisSchema = AnalysisSchema.builder() .name(analysisTypeName) @@ -342,17 +345,13 @@ public static AnalysisTypeId resolveAnalysisTypeId(@NonNull AnalysisType analysi private AnalysisType convertToAnalysisType( AnalysisSchema analysisSchema, boolean hideSchema, boolean unrenderedOnly) { - AnalysisTypeOptions options = new AnalysisTypeOptions(); - if (analysisSchema.getOptions() != null) { - options.setFileTypes(analysisSchema.getOptions().getFileTypes()); - options.setExternalValidation(analysisSchema.getOptions().getExternalValidation()); - } + return AnalysisType.builder() .name(analysisSchema.getName()) .version(analysisSchema.getVersion()) .createdAt(analysisSchema.getCreatedAt()) .schema(resolveSchemaJsonView(analysisSchema.getSchema(), unrenderedOnly, hideSchema)) - .options(options) + .options(analysisSchema.getOptions()) .build(); } diff --git a/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java b/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java index 854a754e2..c0ff40c36 100644 --- a/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java +++ b/song-server/src/test/java/bio/overture/song/server/service/MockedSubmitTest.java @@ -70,7 +70,7 @@ public void submit_MissingAnalysisType_MalformedParameter() { doNothing().when(studyService).checkStudyExist(anyString()); // Verify - assertSongError(() -> submitService.submit("anyStudy", "{}", false), MALFORMED_PARAMETER); + assertSongError(() -> submitService.submit("anyStudyId", "{}", false), MALFORMED_PARAMETER); } @Test @@ -83,12 +83,12 @@ public void submit_MissingAnalysisTypeName_MalformedParameter() { val payload = JsonUtils.toJson( Payload.builder() - .studyId("anyStudy") + .studyId("anyStudyId") .analysisType(AnalysisTypeId.builder().build()) .build()); // Verify - assertSongError(() -> submitService.submit("anyStudy", payload, false), MALFORMED_PARAMETER); + assertSongError(() -> submitService.submit("anyStudyId", payload, false), MALFORMED_PARAMETER); } @Test @@ -100,8 +100,8 @@ public void submit_StudyDne_StudyIdDoesNotExist() { // Verify assertSongError( - () -> submitService.submit("anyStudy", "anyAnalysisId", false), STUDY_ID_DOES_NOT_EXIST); - verify(validationService, never()).validate(isA(JsonNode.class),"anyStudy"); + () -> submitService.submit("anyStudyId", "anyAnalysisId", false), STUDY_ID_DOES_NOT_EXIST); + verify(validationService, never()).validate(isA(JsonNode.class),"anyStudyId"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -112,17 +112,17 @@ public void submit_malformedPayload_PayloadParsingError() { // Verify assertSongError( - () -> submitService.submit("anyStudy", "non json format", false), PAYLOAD_PARSING); - verify(validationService, never()).validate(isA(JsonNode.class),"anyStudy"); + () -> submitService.submit("anyStudyId", "non json format", false), PAYLOAD_PARSING); + verify(validationService, never()).validate(isA(JsonNode.class),"anyStudyId"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @Test public void submit_invalidPayload_SchemaViolation() { // Setup - val studyId = "anyStudy"; + val studyId = "anyStudyId"; doNothing().when(studyService).checkStudyExist(anyString()); - when(validationService.validate(isA(JsonNode.class), "anyStudy")) + when(validationService.validate(isA(JsonNode.class), "anyStudyId")) .thenReturn(Optional.of("there was an error")); // Create an invalid payload and not a malformed one @@ -136,7 +136,7 @@ public void submit_invalidPayload_SchemaViolation() { // Verify assertSongError(() -> submitService.submit(studyId, invalidPayload, false), SCHEMA_VIOLATION); - verify(validationService, times(1)).validate(isA(JsonNode.class),"anyStudy"); + verify(validationService, times(1)).validate(isA(JsonNode.class),"anyStudyId"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -146,7 +146,7 @@ public void submit_mismatchingStudies_StudyIdMismatch() { val study1 = "study1"; val study2 = "study2"; doNothing().when(studyService).checkStudyExist(anyString()); - when(validationService.validate(isA(JsonNode.class), "anyStudy")).thenReturn(Optional.empty()); + when(validationService.validate(isA(JsonNode.class), "anyStudyId")).thenReturn(Optional.empty()); val payloadString = toJson( Payload.builder() @@ -158,7 +158,7 @@ public void submit_mismatchingStudies_StudyIdMismatch() { // Verify assertNotEquals(study1, study2); assertSongError(() -> submitService.submit(study2, payloadString, false), STUDY_ID_MISMATCH); - verify(validationService, times(1)).validate(isA(JsonNode.class), "anyStudy"); + verify(validationService, times(1)).validate(isA(JsonNode.class), "anyStudyId"); verify(analysisService, never()).create(anyString(), isA(Payload.class)); } @@ -174,7 +174,7 @@ public void submit_validPayload_Success() { .build(); doNothing().when(studyService).checkStudyExist(anyString()); - when(validationService.validate(isA(JsonNode.class), "anyStudy")).thenReturn(Optional.empty()); + when(validationService.validate(isA(JsonNode.class), "anyStudyId")).thenReturn(Optional.empty()); val payloadString = toJson(payload); when(analysisService.create(study, payload)) @@ -184,7 +184,7 @@ public void submit_validPayload_Success() { // Verify val actualSubmitResponse = submitService.submit(study, payloadString, false); assertEquals(expectedSubmitResponse, actualSubmitResponse); - verify(validationService, times(1)).validate(isA(JsonNode.class), "anyStudy"); + verify(validationService, times(1)).validate(isA(JsonNode.class), "anyStudyId"); verify(analysisService, times(1)).create(anyString(), isA(Payload.class)); } } From fe20ec7f42e64f0beeaea8bf124d7868b2a4510e Mon Sep 17 00:00:00 2001 From: Azher2Ali <121898125+Azher2Ali@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:30:25 -0500 Subject: [PATCH 4/9] Changes to ValidationService related to externalValiation --- .../server/service/ValidationService.java | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java index 407a58b35..8f432c7b8 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java @@ -42,6 +42,8 @@ import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import lombok.NonNull; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; @@ -62,7 +64,7 @@ public class ValidationService { private static final String FILE_DATA_SCHEMA_ID = "fileData"; private static final String STORAGE_DOWNLOAD_RESPONSE_SCHEMA_ID = "storageDownloadResponse"; - + private static final String LYRIC_URL_REGEX = "^http://[^/]+/validator/([^/]+)/entity/([^/]+)$"; private final SchemaValidator validator; private final AnalysisTypeService analysisTypeService; private final UploadRepository uploadRepository; @@ -93,12 +95,20 @@ public Optional validate(@NonNull JsonNode payload, @NonNull String stud validateWithSchema(analysisTypeIdSchema, payload); val analysisTypeResult = extractAnalysisTypeFromPayload(payload); val analysisTypeId = fromJson(analysisTypeResult.get(), AnalysisTypeId.class); - val analysisType = analysisTypeService.getAnalysisType(analysisTypeId, false); + val analysisType = analysisTypeService.getAnalysisType(analysisTypeId, true); + log.info( format( "Found Analysis type: name=%s version=%s", analysisType.getName(), analysisType.getVersion())); + if (analysisType.getOptions() != null + && analysisType.getOptions().getExternalValidation() != null + && !externalValidations( + analysisType.getOptions().getExternalValidation(), payload, studyId)) { + throw new ValidationException("External Validations failed"); + } + List fileTypes = new ArrayList<>(); if (analysisType.getOptions() != null && analysisType.getOptions().getFileTypes() != null) { @@ -109,6 +119,7 @@ public Optional validate(@NonNull JsonNode payload, @NonNull String stud validateFileType(fileTypes, payload); } + // log.info("SCHEMA :- " + analysisType.getSchema()); val schema = buildSchema(analysisType.getSchema()); validateWithSchema(schema, payload); } catch (ValidationException e) { @@ -118,33 +129,55 @@ public Optional validate(@NonNull JsonNode payload, @NonNull String stud return Optional.ofNullable(errors); } - private void externalValidations( - List externalValidations, @NonNull JsonNode payload) { + private boolean externalValidations( + List externalValidations, @NonNull JsonNode payload, String studyId) { + boolean validated = true; for (ExternalValidation externalValidation : externalValidations) { String jsonPath = externalValidation.getJsonPath(); String externalUrl = externalValidation.getUrl(); // Extract the value from the uploaded JSON using the jsonPath - String value = JsonPath.read(payload, "$." + jsonPath); + String value = null; + try { + value = JsonPath.read(payload.toString(), "$." + jsonPath); + log.info("value of {} is {}", jsonPath, value); + } catch (Exception e) { + log.info("path {} not found ", jsonPath); + } - if(Objects.isNull(value) || value.isEmpty()) - return; + if (Objects.isNull(value) || value.isEmpty()) return validated; // call the external url using restTemplate + validated = invokeExternalUrl(studyId, externalUrl, value); } + return validated; } public boolean invokeExternalUrl(String studyId, String url, String value) { - // will change the logic to make it parameterised try { + Pattern pattern = Pattern.compile(LYRIC_URL_REGEX); + Matcher matcher = pattern.matcher(url); + + if (!matcher.matches()) { + throw new IllegalArgumentException("Invalid URL format or missing parameters."); + } + + String categoryId = matcher.group(1); + String entityName = matcher.group(2); + // create URL URIBuilder uriBuilder = new URIBuilder(url); uriBuilder.addParameter("studyId", studyId); - uriBuilder.addParameter("value",value); + uriBuilder.addParameter("value", value); + String newPath = + uriBuilder.getPath().replace("categoryId", categoryId).replace("entityName", entityName); + uriBuilder.setPath(newPath); + log.info("invoking external url {}", uriBuilder.getPath()); // Make the HTTP GET request - ResponseEntity response = restTemplate.getForEntity(uriBuilder.build().toURL().toString(), Void.class); + ResponseEntity response = + restTemplate.getForEntity(uriBuilder.build().toURL().toString(), Void.class); // Return true if the response status is 200 OK return response.getStatusCode().is2xxSuccessful(); From fe0883f512c02a6021c30010b172805fa8d6cbb2 Mon Sep 17 00:00:00 2001 From: Azher2Ali <121898125+Azher2Ali@users.noreply.github.com> Date: Mon, 6 Jan 2025 09:16:31 -0500 Subject: [PATCH 5/9] Changes related to the feedback comments --- .../song/server/service/AnalysisTypeService.java | 8 +++----- .../song/server/service/ValidationService.java | 13 ++++++------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index eadaa8d39..d8f381497 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -269,9 +269,8 @@ private AnalysisType commitAnalysisType( } // checking if file types is empty - // if the version is new version of the schema , we are checking the previous version allowed - // file types - // if it is new then it is empty + // if the analysisSchemaVersion is new version of the schema and fileTypes is empty then, + // we are checking the previous version to map the fileTypes allowed to the latestVersion if (fileTypes.isEmpty()) { List analysisSchemaList = analysisSchemaRepository.findAllByName(analysisTypeName); @@ -289,8 +288,7 @@ private AnalysisType commitAnalysisType( } } - if(options!=null) - options.setFileTypes(fileTypes); + if (options != null) options.setFileTypes(fileTypes); val analysisSchema = AnalysisSchema.builder() diff --git a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java index 8f432c7b8..dcc5c64a5 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java @@ -118,8 +118,7 @@ public Optional validate(@NonNull JsonNode payload, @NonNull String stud if (!fileTypes.isEmpty()) { validateFileType(fileTypes, payload); } - - // log.info("SCHEMA :- " + analysisType.getSchema()); + val schema = buildSchema(analysisType.getSchema()); validateWithSchema(schema, payload); } catch (ValidationException e) { @@ -157,15 +156,15 @@ private boolean externalValidations( public boolean invokeExternalUrl(String studyId, String url, String value) { try { - Pattern pattern = Pattern.compile(LYRIC_URL_REGEX); - Matcher matcher = pattern.matcher(url); + Pattern lyricUrlPattern = Pattern.compile(LYRIC_URL_REGEX); + Matcher lyricUrlMatcher = lyricUrlPattern.matcher(url); - if (!matcher.matches()) { + if (!lyricUrlMatcher.matches()) { throw new IllegalArgumentException("Invalid URL format or missing parameters."); } - String categoryId = matcher.group(1); - String entityName = matcher.group(2); + String categoryId = lyricUrlMatcher.group(1); + String entityName = lyricUrlMatcher.group(2); // create URL URIBuilder uriBuilder = new URIBuilder(url); From 20652050ae4e7c7cbb48cd9c3b3f49c043f458f4 Mon Sep 17 00:00:00 2001 From: Azher2Ali <121898125+Azher2Ali@users.noreply.github.com> Date: Fri, 17 Jan 2025 17:06:32 -0500 Subject: [PATCH 6/9] Migrate file_types to options column in analysis_schema --- .../V1_21__remove_file_types_and_add_options.sql | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql b/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql index 0cb44b92f..5b16c7518 100644 --- a/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql +++ b/song-server/src/main/resources/db/migration/V1_21__remove_file_types_and_add_options.sql @@ -1,8 +1,14 @@ -- Remove the file_types column ALTER TABLE public.analysis_schema -DROP COLUMN file_types; +ADD COLUMN options jsonb; --- Add the options column as jsonb (nullable by default) +UPDATE public.analysis_schema +SET options = jsonb_build_object( + 'fileTypes', file_types, + 'externalValidation', NULL +); +-- Remove the file_types column ALTER TABLE public.analysis_schema -ADD COLUMN options jsonb; \ No newline at end of file +DROP COLUMN file_types; + From 57e83afa7b10f93959b7ed73c933ea5c00a4b968 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 27 Jan 2025 00:25:59 -0500 Subject: [PATCH 7/9] External valdiation URL template for study and value tokens --- docs/custom-schemas.md | 97 ++++ .../song/core/model/AnalysisType.java | 22 + .../server/service/ValidationService.java | 425 +++++++++--------- 3 files changed, 328 insertions(+), 216 deletions(-) create mode 100644 docs/custom-schemas.md diff --git a/docs/custom-schemas.md b/docs/custom-schemas.md new file mode 100644 index 000000000..59a381c2e --- /dev/null +++ b/docs/custom-schemas.md @@ -0,0 +1,97 @@ +# Custom Analysis Schemas + +## Minimal Example + +```json +{ + "name": "minimalExample", + "options":{}, + "schema":{ + "type": "object", + "required":[ + "experiment" + ], + "properties":{ + "experiment":{} + } + } +} +``` + +## Options + +All properties in options are optional. If no value is provided, a default configuration will be used for the analysis. If this is an update to an existing analysis type, you can omit any option and the value will be maintained from the previous version. + +```json +{ + "fileTypes":["bam", "cram"], + "externalValidation":[ + { + "url": "http://localhost:8099/", + "jsonPath": "experiment.someId" + } + ] +}, +``` + +### File Types + +`options.fileTypes` can be provided an array of strings. These represent the file types (file extensions) that are allowed to be uploaded for this type of analysis. + +If an empty array is provided, then any file type will be allowed. If an array of file types is provided, then an analysis will be invalid if it contains files of a type not listed. + +```json +{ + "options": { + "fileTypes": ["bam","cram"] + } +} +``` + +### External Validation + +External validations configure Song to check a value in the analysis against an external service by sending an HTTP GET request to a configurable URL. The URL needs to return 2XX status message to indicate that if the provided value "is valid", typically meaning that the value for this property is known by that service. + +As an example, if the project clinical data is being managed in a separate service, we can add an external validation to the donor id field of our custom scheme. This will send the donor id to the external service which can confirm that we have previously registered that donor. + +This might look like the following: + +```json +{ + "url": "http://example.com/{study}/donor/{value}", + "jsonPath": "experiment.donorId" +} +``` + +The URL provided is a template, with two variables that will be replaced during validation. Song will replace the token `{value}` with the value read from the analysis at the property as defined in the `jsonPath`. Song will also replace the token `{study}` with the study ID for the Analysis. + +Continuing the above example, if the following analysis was submitted: + +```json +{ + "studyId": "ABC123", + "analysisType": { + "name": "minimalExample" + }, + "files": [ + { + "dataType": "text", + "fileName": "file1.txt", + "fileSize": 123, + "fileType": "txt", + "fileAccess": "open", + "fileMd5sum": "595f44fec1e92a71d3e9e77456ba80d1" + } + ], + "experiment": { + "donorId": "id01" + } +} +``` + +Song would attempt to validate the donorId by sending a URL to `http://example.com/ABC123/donor/id01`. + +It is allowable to use either the `{study}` or `{value}` multiple times in the URL, each instance will be replaced by the corresponding value. + +> [!Warning] +> The URL may cause errors in song if it contains any tokens matching the `{word}` format other than `{study}` and `{value}` \ No newline at end of file diff --git a/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java b/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java index f7464eb3d..adc2503f2 100644 --- a/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java +++ b/song-core/src/main/java/bio/overture/song/core/model/AnalysisType.java @@ -3,6 +3,8 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.List; import javax.validation.constraints.NotNull; import lombok.AllArgsConstructor; import lombok.Builder; @@ -22,4 +24,24 @@ public class AnalysisType { @JsonInclude(JsonInclude.Include.NON_NULL) private JsonNode schema; + + public List getFileTypes() { + if (this.options == null) { + return new ArrayList(); + } + if (this.options.getFileTypes() == null) { + return new ArrayList(); + } + return this.options.getFileTypes(); + } + + public List getExternalValidations() { + if (this.options == null) { + return new ArrayList(); + } + if (this.options.getExternalValidation() == null) { + return new ArrayList(); + } + return this.options.getExternalValidation(); + } } diff --git a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java index dcc5c64a5..b8ffa8376 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/ValidationService.java @@ -16,18 +16,6 @@ */ package bio.overture.song.server.service; -import static bio.overture.song.core.exceptions.ServerErrors.MALFORMED_PARAMETER; -import static bio.overture.song.core.exceptions.ServerException.checkServer; -import static bio.overture.song.core.utils.JsonUtils.fromJson; -import static bio.overture.song.core.utils.JsonUtils.mapper; -import static bio.overture.song.core.utils.Separators.COMMA; -import static bio.overture.song.server.utils.JsonParser.extractAnalysisTypeFromPayload; -import static bio.overture.song.server.utils.JsonSchemas.buildSchema; -import static bio.overture.song.server.utils.JsonSchemas.validateWithSchema; -import static java.lang.String.format; -import static java.util.Objects.isNull; -import static org.apache.commons.lang.StringUtils.isBlank; - import bio.overture.song.core.model.AnalysisTypeId; import bio.overture.song.core.model.ExternalValidation; import bio.overture.song.core.model.FileData; @@ -37,246 +25,251 @@ import bio.overture.song.server.validation.ValidationResponse; import com.fasterxml.jackson.databind.JsonNode; import com.jayway.jsonpath.JsonPath; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.function.Supplier; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import lombok.NonNull; -import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import lombok.val; import org.apache.commons.collections.CollectionUtils; -import org.apache.http.client.utils.URIBuilder; import org.everit.json.schema.Schema; import org.everit.json.schema.ValidationException; +import org.json.JSONException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.servlet.View; + +import java.util.List; +import java.util.Optional; +import java.util.function.Supplier; + +import static bio.overture.song.core.exceptions.ServerErrors.MALFORMED_PARAMETER; +import static bio.overture.song.core.exceptions.ServerException.checkServer; +import static bio.overture.song.core.utils.JsonUtils.fromJson; +import static bio.overture.song.core.utils.JsonUtils.mapper; +import static bio.overture.song.core.utils.Separators.COMMA; +import static bio.overture.song.server.utils.JsonParser.extractAnalysisTypeFromPayload; +import static bio.overture.song.server.utils.JsonSchemas.buildSchema; +import static bio.overture.song.server.utils.JsonSchemas.validateWithSchema; +import static java.lang.String.format; +import static java.util.Objects.isNull; +import static org.apache.commons.lang.StringUtils.isBlank; @Slf4j @Service public class ValidationService { - private static final String FILE_DATA_SCHEMA_ID = "fileData"; - private static final String STORAGE_DOWNLOAD_RESPONSE_SCHEMA_ID = "storageDownloadResponse"; - private static final String LYRIC_URL_REGEX = "^http://[^/]+/validator/([^/]+)/entity/([^/]+)$"; - private final SchemaValidator validator; - private final AnalysisTypeService analysisTypeService; - private final UploadRepository uploadRepository; - private final boolean enforceLatest; - private final Schema analysisTypeIdSchema; - private final RestTemplate restTemplate; - - @Autowired - public ValidationService( - @Value("${schemas.enforceLatest}") boolean enforceLatest, - @NonNull SchemaValidator validator, - @NonNull AnalysisTypeService analysisTypeService, - @NonNull Supplier analysisTypeIdSchemaSupplier, - @NonNull UploadRepository uploadRepository, - @NonNull RestTemplate restTemplate) { - this.validator = validator; - this.analysisTypeService = analysisTypeService; - this.uploadRepository = uploadRepository; - this.enforceLatest = enforceLatest; - this.analysisTypeIdSchema = analysisTypeIdSchemaSupplier.get(); - this.restTemplate = restTemplate; - } - - @SneakyThrows - public Optional validate(@NonNull JsonNode payload, @NonNull String studyId) { - String errors = null; - try { - validateWithSchema(analysisTypeIdSchema, payload); - val analysisTypeResult = extractAnalysisTypeFromPayload(payload); - val analysisTypeId = fromJson(analysisTypeResult.get(), AnalysisTypeId.class); - val analysisType = analysisTypeService.getAnalysisType(analysisTypeId, true); - - log.info( - format( - "Found Analysis type: name=%s version=%s", - analysisType.getName(), analysisType.getVersion())); - - if (analysisType.getOptions() != null - && analysisType.getOptions().getExternalValidation() != null - && !externalValidations( - analysisType.getOptions().getExternalValidation(), payload, studyId)) { - throw new ValidationException("External Validations failed"); - } - - List fileTypes = new ArrayList<>(); - - if (analysisType.getOptions() != null && analysisType.getOptions().getFileTypes() != null) { - fileTypes = analysisType.getOptions().getFileTypes(); - } - - if (!fileTypes.isEmpty()) { - validateFileType(fileTypes, payload); - } - - val schema = buildSchema(analysisType.getSchema()); - validateWithSchema(schema, payload); - } catch (ValidationException e) { - errors = COMMA.join(e.getAllMessages()); - log.error(errors); + private static final String FILE_DATA_SCHEMA_ID = "fileData"; + private static final String STORAGE_DOWNLOAD_RESPONSE_SCHEMA_ID = "storageDownloadResponse"; + private static final String EXTERNAL_URL_TEMPLATE_PATTERN_STUDYID = "\\{study\\}"; + private static final String EXTERNAL_URL_TEMPLATE_PATTERN_DATAVALUE = "\\{value\\}"; + private final SchemaValidator validator; + private final AnalysisTypeService analysisTypeService; + private final UploadRepository uploadRepository; + private final boolean enforceLatest; + private final Schema analysisTypeIdSchema; + private final RestTemplate restTemplate; + private final View error; + + @Autowired + public ValidationService( + @Value("${schemas.enforceLatest}") boolean enforceLatest, + @NonNull SchemaValidator validator, + @NonNull AnalysisTypeService analysisTypeService, + @NonNull Supplier analysisTypeIdSchemaSupplier, + @NonNull UploadRepository uploadRepository, + @NonNull RestTemplate restTemplate, + View error) { + this.validator = validator; + this.analysisTypeService = analysisTypeService; + this.uploadRepository = uploadRepository; + this.enforceLatest = enforceLatest; + this.analysisTypeIdSchema = analysisTypeIdSchemaSupplier.get(); + this.restTemplate = restTemplate; + this.error = error; } - return Optional.ofNullable(errors); - } - private boolean externalValidations( - List externalValidations, @NonNull JsonNode payload, String studyId) { - - boolean validated = true; - for (ExternalValidation externalValidation : externalValidations) { - String jsonPath = externalValidation.getJsonPath(); - String externalUrl = externalValidation.getUrl(); - - // Extract the value from the uploaded JSON using the jsonPath - String value = null; - try { - value = JsonPath.read(payload.toString(), "$." + jsonPath); - log.info("value of {} is {}", jsonPath, value); - } catch (Exception e) { - log.info("path {} not found ", jsonPath); - } - - if (Objects.isNull(value) || value.isEmpty()) return validated; + public Optional validate(@NonNull JsonNode payload, @NonNull String studyId) { + String errors = null; + + try { + validateWithSchema(analysisTypeIdSchema, payload); + val analysisTypeResult = extractAnalysisTypeFromPayload(payload); + if (analysisTypeResult.isEmpty()) { + throw new ValidationException("Analysis type not found"); + } + val analysisTypeId = fromJson(analysisTypeResult.get(), AnalysisTypeId.class); + val analysisType = analysisTypeService.getAnalysisType(analysisTypeId, true); + log.debug( + format( + "Validation Analysis with schema: name=%s version=%s", + analysisType.getName(), analysisType.getVersion())); + + val schema = buildSchema(analysisType.getSchema()); + validateWithSchema(schema, payload); + + val fileTypes = analysisType.getFileTypes(); + if (!fileTypes.isEmpty()) { + validateFileType(fileTypes, payload); + } + + val externalValidations = analysisType.getExternalValidations(); + if (!externalValidations.isEmpty()) { + externalValidations(analysisType.getOptions().getExternalValidation(), payload, studyId); + } + } catch (ValidationException e) { + errors = COMMA.join(e.getAllMessages()); + log.error(errors); + } catch (JSONException e) { + errors = e.getMessage(); + log.error(errors); + } - // call the external url using restTemplate - validated = invokeExternalUrl(studyId, externalUrl, value); + return Optional.ofNullable(errors); } - return validated; - } - - public boolean invokeExternalUrl(String studyId, String url, String value) { - try { - - Pattern lyricUrlPattern = Pattern.compile(LYRIC_URL_REGEX); - Matcher lyricUrlMatcher = lyricUrlPattern.matcher(url); - - if (!lyricUrlMatcher.matches()) { - throw new IllegalArgumentException("Invalid URL format or missing parameters."); - } - String categoryId = lyricUrlMatcher.group(1); - String entityName = lyricUrlMatcher.group(2); - - // create URL - URIBuilder uriBuilder = new URIBuilder(url); - uriBuilder.addParameter("studyId", studyId); - uriBuilder.addParameter("value", value); - String newPath = - uriBuilder.getPath().replace("categoryId", categoryId).replace("entityName", entityName); - uriBuilder.setPath(newPath); - log.info("invoking external url {}", uriBuilder.getPath()); - // Make the HTTP GET request - ResponseEntity response = - restTemplate.getForEntity(uriBuilder.build().toURL().toString(), Void.class); + private Optional getValueAtJsonPath(@NonNull JsonNode payload, @NonNull String jsonPath) { + try { + String value = JsonPath.read(payload.toString(), "$." + jsonPath); + return Optional.of(value); + } catch (Exception e) { + log.debug( + String.format("Error reading value for external validation. Reason: %s", e.getMessage())); + return Optional.empty(); + } + } - // Return true if the response status is 200 OK - return response.getStatusCode().is2xxSuccessful(); - } catch (Exception e) { - // Handle exceptions (e.g., log them) - System.err.println("Validation failed " + e.getMessage()); - return false; + private String buildExternalUrlFromTemplate( + @NonNull String urlTemplate, @NonNull String studyId, @NonNull String value) { + return urlTemplate + .replaceAll(EXTERNAL_URL_TEMPLATE_PATTERN_STUDYID, studyId) + .replaceAll(EXTERNAL_URL_TEMPLATE_PATTERN_DATAVALUE, value); } - } - private void validateFileType(List fileTypes, @NonNull JsonNode payload) { + private void externalValidations( + List externalValidations, @NonNull JsonNode payload, String studyId) + throws ValidationException { + + for (ExternalValidation externalValidation : externalValidations) { + + val value = getValueAtJsonPath(payload, externalValidation.getJsonPath()); + if (value.isPresent()) { + // Only validate vs external source if the value is present in the analysis payload + val formattedExternalUrl = + buildExternalUrlFromTemplate(externalValidation.getUrl(), studyId, value.get()); + try { + val response = restTemplate.getForEntity(formattedExternalUrl, Void.class); + if (response.getStatusCode().isError()) { + val errorMessage = + String.format( + "Value '%s' from path '%s' is not permitted as it failed to validate with external validation source.", + value.get(), externalValidation.getJsonPath(), formattedExternalUrl); + log.debug(errorMessage); + throw new ValidationException(errorMessage); + } + } catch (RestClientException e) { + val errorMessage = + String.format( + "Value '%s' from path '%s' is not permitted as it failed to validate with external validation source.", + value.get(), externalValidation.getJsonPath()); + log.info(String.format("Error occurred while executing external validation against url '%s'.", formattedExternalUrl)); + throw new ValidationException(errorMessage); + } + } + } + } - if (CollectionUtils.isNotEmpty(fileTypes)) { - JsonNode files = payload.get("files"); - if (files.isArray()) { - for (JsonNode file : files) { - log.info("file is " + file); - String fileType = file.get("fileType").asText(); - String fileName = file.get("fileName").asText(); - if (!fileTypes.contains(fileType)) { - throw new ValidationException( - String.format( - "%s name is not supported, supported formats are %s", - fileName, String.join(", ", fileTypes))); - } + private void validateFileType(List fileTypes, @NonNull JsonNode payload) + throws ValidationException { + + if (CollectionUtils.isNotEmpty(fileTypes)) { + JsonNode files = payload.get("files"); + if (files.isArray()) { + for (JsonNode file : files) { + log.info("file is " + file); + String fileType = file.get("fileType").asText(); + String fileName = file.get("fileName").asText(); + if (!fileTypes.contains(fileType)) { + throw new ValidationException( + String.format( + "%s name is not supported, supported formats are %s", + fileName, String.join(", ", fileTypes))); + } + } + } } - } } - } - public void update(@NonNull String uploadId, String errorMessages) { - if (isNull(errorMessages)) { - updateAsValid(uploadId); - } else { - updateAsInvalid(uploadId, errorMessages); + public void update(@NonNull String uploadId, String errorMessages) { + if (isNull(errorMessages)) { + updateAsValid(uploadId); + } else { + updateAsInvalid(uploadId, errorMessages); + } } - } - // TODO: transition to everit json schema library - public Optional validate(FileData fileData) { - val json = mapper().valueToTree(fileData); - val resp = validator.validate(FILE_DATA_SCHEMA_ID, json); - return processResponse(resp); - } + // TODO: transition to everit json schema library + public Optional validate(FileData fileData) { + val json = mapper().valueToTree(fileData); + val resp = validator.validate(FILE_DATA_SCHEMA_ID, json); + return processResponse(resp); + } - // TODO: transition to everit json schema library - public Optional validateStorageDownloadResponse(JsonNode response) { - return processResponse(validator.validate(STORAGE_DOWNLOAD_RESPONSE_SCHEMA_ID, response)); - } + // TODO: transition to everit json schema library + public Optional validateStorageDownloadResponse(JsonNode response) { + return processResponse(validator.validate(STORAGE_DOWNLOAD_RESPONSE_SCHEMA_ID, response)); + } - public String validateAnalysisTypeVersion(AnalysisTypeId a) { - checkServer( - !isBlank(a.getName()), - getClass(), - MALFORMED_PARAMETER, - "The analysisType name cannot be null"); - return validateAnalysisTypeVersion(a.getName(), a.getVersion()); - } + public String validateAnalysisTypeVersion(AnalysisTypeId a) { + checkServer( + !isBlank(a.getName()), + getClass(), + MALFORMED_PARAMETER, + "The analysisType name cannot be null"); + return validateAnalysisTypeVersion(a.getName(), a.getVersion()); + } - public String validateAnalysisTypeVersion(@NonNull String name, Integer version) { - if (enforceLatest && !isNull(version)) { - val latestVersion = analysisTypeService.getLatestVersionNumber(name); - if (!version.equals(latestVersion)) { - val message = - format( - "Must use the latest version '%s' while enforceLatest=true, but using version '%s' of analysisType '%s' instead", - latestVersion, version, name); - log.error(message); - return message; - } + public String validateAnalysisTypeVersion(@NonNull String name, Integer version) { + if (enforceLatest && !isNull(version)) { + val latestVersion = analysisTypeService.getLatestVersionNumber(name); + if (!version.equals(latestVersion)) { + val message = + format( + "Must use the latest version '%s' while enforceLatest=true, but using version '%s' of analysisType '%s' instead", + latestVersion, version, name); + log.error(message); + return message; + } + } + return null; } - return null; - } - private void updateState( - @NonNull String uploadId, @NonNull UploadStates state, @NonNull String errors) { - uploadRepository - .findById(uploadId) - .map( - x -> { - x.setState(state); - x.setErrors(errors); - return x; - }) - .ifPresent(uploadRepository::save); - } + private void updateState( + @NonNull String uploadId, @NonNull UploadStates state, @NonNull String errors) { + uploadRepository + .findById(uploadId) + .map( + x -> { + x.setState(state); + x.setErrors(errors); + return x; + }) + .ifPresent(uploadRepository::save); + } - private void updateAsValid(@NonNull String uploadId) { - updateState(uploadId, UploadStates.VALIDATED, ""); - } + private void updateAsValid(@NonNull String uploadId) { + updateState(uploadId, UploadStates.VALIDATED, ""); + } - public void updateAsInvalid(@NonNull String uploadId, @NonNull String errorMessages) { - updateState(uploadId, UploadStates.VALIDATION_ERROR, errorMessages); - } + public void updateAsInvalid(@NonNull String uploadId, @NonNull String errorMessages) { + updateState(uploadId, UploadStates.VALIDATION_ERROR, errorMessages); + } - private static Optional processResponse(ValidationResponse response) { - if (response.isValid()) { - return Optional.empty(); - } else { - return Optional.of(response.getValidationErrors()); + private static Optional processResponse(ValidationResponse response) { + if (response.isValid()) { + return Optional.empty(); + } else { + return Optional.of(response.getValidationErrors()); + } } - } } From a09c88e786df201433b46041daaf71d74f772c8e Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 27 Jan 2025 00:30:29 -0500 Subject: [PATCH 8/9] Retrieve all options data with analysis type --- .../server/service/AnalysisTypeService.java | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java index d8f381497..45465d084 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java +++ b/song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java @@ -17,11 +17,7 @@ package bio.overture.song.server.service; -import static bio.overture.song.core.exceptions.ServerErrors.ANALYSIS_TYPE_NOT_FOUND; -import static bio.overture.song.core.exceptions.ServerErrors.ILLEGAL_ANALYSIS_TYPE_NAME; -import static bio.overture.song.core.exceptions.ServerErrors.MALFORMED_JSON_SCHEMA; -import static bio.overture.song.core.exceptions.ServerErrors.MALFORMED_PARAMETER; -import static bio.overture.song.core.exceptions.ServerErrors.SCHEMA_VIOLATION; +import static bio.overture.song.core.exceptions.ServerErrors.*; import static bio.overture.song.core.exceptions.ServerException.buildServerException; import static bio.overture.song.core.exceptions.ServerException.checkServer; import static bio.overture.song.core.utils.CollectionUtils.isCollectionBlank; @@ -29,10 +25,7 @@ import static bio.overture.song.core.utils.Separators.COMMA; import static bio.overture.song.server.controller.analysisType.AnalysisTypeController.REGISTRATION; import static bio.overture.song.server.repository.specification.AnalysisSchemaSpecification.buildListQuery; -import static bio.overture.song.server.utils.JsonSchemas.PROPERTIES; -import static bio.overture.song.server.utils.JsonSchemas.REQUIRED; -import static bio.overture.song.server.utils.JsonSchemas.buildSchema; -import static bio.overture.song.server.utils.JsonSchemas.validateWithSchema; +import static bio.overture.song.server.utils.JsonSchemas.*; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static java.util.Objects.isNull; @@ -156,17 +149,12 @@ public AnalysisType getAnalysisType( resolveSchemaJsonView(analysisSchema.getSchema(), unrenderedOnly, false); AnalysisTypeOptions options = analysisSchema.getOptions(); - - List fileTypes = - (options.getFileTypes() != null && !options.getFileTypes().isEmpty()) - ? options.getFileTypes() - : new ArrayList<>(); return AnalysisType.builder() .name(analysisTypeId.getName()) .version(analysisTypeId.getVersion()) .createdAt(analysisSchema.getCreatedAt()) .schema(resolvedSchemaJson) - .options(AnalysisTypeOptions.builder().fileTypes(fileTypes).build()) + .options(options) .build(); } From 6fd36b163bc24c7e20c53c12216e0eb6f72d9040 Mon Sep 17 00:00:00 2001 From: Jon Eubank Date: Mon, 27 Jan 2025 00:31:18 -0500 Subject: [PATCH 9/9] Remove sneaky throws from buildSchema with JSON parsing failure case --- .../service/analysis/AnalysisServiceImpl.java | 18 ++++++++++++------ .../song/server/utils/JsonSchemas.java | 14 +++++++------- .../V1_2__dynamic_schema_integration.java | 17 +++++++++++------ 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java index 1e7356f05..833e89520 100644 --- a/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java +++ b/song-server/src/main/java/bio/overture/song/server/service/analysis/AnalysisServiceImpl.java @@ -62,6 +62,7 @@ import lombok.extern.slf4j.Slf4j; import lombok.val; import org.everit.json.schema.ValidationException; +import org.json.JSONException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -137,7 +138,8 @@ public Analysis updateAnalysis( // Validate the updateAnalysisRequest against the scheme validateUpdateRequest(updateAnalysisRequest, newAnalysisSchema); - // Now that the request is validated, is safe to fetch the old analysis with all files, samples and info + // Now that the request is validated, is safe to fetch the old analysis with all files, samples + // and info val analysis = unsecuredDeepRead(analysisId); // Update the association between the old schema and new schema entities for the requested @@ -156,16 +158,17 @@ public Analysis updateAnalysis( @Override @Transactional public Analysis patchUpdateAnalysis( - @NonNull String studyId, - @NonNull String analysisId, - @NonNull JsonNode patchUpdateAnalysisRequest) { + @NonNull String studyId, + @NonNull String analysisId, + @NonNull JsonNode patchUpdateAnalysisRequest) { // Securely read analysis with all files, samples and info val analysis = securedDeepRead(studyId, analysisId); log.debug("analysis found:" + analysis); val originalData = analysis.getData(); - originalData.put("analysisType", analysis.getAnalysisType()); // we need this to validate against schema. + originalData.put( + "analysisType", analysis.getAnalysisType()); // we need this to validate against schema. val updatedAnalysis = mergePatchRequest(toJsonNode(originalData), patchUpdateAnalysisRequest); @@ -647,11 +650,14 @@ private Analysis shallowRead(String id) { private void validateUpdateRequest(JsonNode request, AnalysisSchema analysisSchema) { checkAnalysisTypeVersion(analysisSchema); val renderedUpdateJsonSchema = renderUpdateJsonSchema(analysisSchema); - val schema = buildSchema(renderedUpdateJsonSchema); try { + val schema = buildSchema(renderedUpdateJsonSchema); validateWithSchema(schema, request); } catch (ValidationException e) { throw buildServerException(getClass(), SCHEMA_VIOLATION, COMMA.join(e.getAllMessages())); + } catch (JSONException jsonException) { + throw buildServerException( + getClass(), SCHEMA_VIOLATION, COMMA.join(jsonException.getMessage())); } } diff --git a/song-server/src/main/java/bio/overture/song/server/utils/JsonSchemas.java b/song-server/src/main/java/bio/overture/song/server/utils/JsonSchemas.java index f21973f24..675fe37d3 100644 --- a/song-server/src/main/java/bio/overture/song/server/utils/JsonSchemas.java +++ b/song-server/src/main/java/bio/overture/song/server/utils/JsonSchemas.java @@ -4,14 +4,15 @@ import static lombok.AccessLevel.PRIVATE; import com.fasterxml.jackson.databind.JsonNode; +import java.io.IOException; import java.nio.file.Path; import lombok.NoArgsConstructor; import lombok.NonNull; -import lombok.SneakyThrows; import lombok.val; import org.everit.json.schema.Schema; import org.everit.json.schema.loader.SchemaClient; import org.everit.json.schema.loader.SchemaLoader; +import org.json.JSONException; @NoArgsConstructor(access = PRIVATE) public class JsonSchemas { @@ -21,8 +22,8 @@ public class JsonSchemas { public static final String TYPE = "type"; public static final String STRING = "string"; - @SneakyThrows - public static Schema buildSchema(@NonNull Path schemaDir, @NonNull String filePathname) { + public static Schema buildSchema(@NonNull Path schemaDir, @NonNull String filePathname) + throws JSONException, IOException { val jsonObject = convertToJSONObject(schemaDir.resolve(filePathname)); return SchemaLoader.builder() .schemaClient(SchemaClient.classPathAwareClient()) @@ -34,8 +35,7 @@ public static Schema buildSchema(@NonNull Path schemaDir, @NonNull String filePa .build(); } - @SneakyThrows - public static Schema buildSchema(JsonNode json) { + public static Schema buildSchema(JsonNode json) throws JSONException { val jsonObject = convertToJSONObject(json); return SchemaLoader.builder() .schemaClient(SchemaClient.classPathAwareClient()) @@ -46,8 +46,8 @@ public static Schema buildSchema(JsonNode json) { .build(); } - @SneakyThrows - public static void validateWithSchema(@NonNull Schema schema, @NonNull JsonNode j) { + public static void validateWithSchema(@NonNull Schema schema, @NonNull JsonNode j) + throws JSONException { val jsonObject = convertToJSONObject(j); schema.validate(jsonObject); } diff --git a/song-server/src/main/java/db/migration/V1_2__dynamic_schema_integration.java b/song-server/src/main/java/db/migration/V1_2__dynamic_schema_integration.java index 9a1be9b9d..426e2692f 100644 --- a/song-server/src/main/java/db/migration/V1_2__dynamic_schema_integration.java +++ b/song-server/src/main/java/db/migration/V1_2__dynamic_schema_integration.java @@ -14,11 +14,9 @@ import bio.overture.song.server.model.enums.TableAttributeNames; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; -import lombok.AccessLevel; -import lombok.NonNull; -import lombok.RequiredArgsConstructor; +import java.nio.file.Path; +import lombok.*; import lombok.extern.slf4j.Slf4j; -import lombok.val; import org.everit.json.schema.Schema; import org.everit.json.schema.ValidationException; import org.flywaydb.core.api.migration.spring.SpringJdbcMigration; @@ -31,9 +29,14 @@ public class V1_2__dynamic_schema_integration implements SpringJdbcMigration { private static final String VARIANT_CALL_LEGACY_R_PATH = "legacy/variantCall.json"; private static final ObjectMapper OBJECT_MAPPER = mapper(); private static final Schema LEGACY_VARIANT_CALL_SCHEMA = - buildSchema(SCHEMA_ANALYSIS_PATH, VARIANT_CALL_LEGACY_R_PATH); + sneakyBuildSchema(SCHEMA_ANALYSIS_PATH, VARIANT_CALL_LEGACY_R_PATH); private static final Schema LEGACY_SEQUENCING_READ_SCHEMA = - buildSchema(SCHEMA_ANALYSIS_PATH, SEQUENCING_READ_LEGACY_R_PATH); + sneakyBuildSchema(SCHEMA_ANALYSIS_PATH, SEQUENCING_READ_LEGACY_R_PATH); + + @SneakyThrows + private static Schema sneakyBuildSchema(@NonNull Path schemaDir, @NonNull String filePathname) { + return buildSchema(schemaDir, filePathname); + } @Override public void migrate(JdbcTemplate jdbcTemplate) throws Exception { @@ -124,6 +127,7 @@ public void migrate(JdbcTemplate jdbcTemplate) throws Exception { private void createTestData(JdbcTemplate jdbcTemplate) {} + @SneakyThrows private void migrateVariantCall(JdbcTemplate jdbcTemplate) { log.info("Starting VariantCall migration"); val variantCalls = jdbcTemplate.queryForList("SELECT * FROM variantcall"); @@ -148,6 +152,7 @@ private void migrateVariantCall(JdbcTemplate jdbcTemplate) { log.info("Finished VariantCall migration"); } + @SneakyThrows private void migrateSequencingRead(JdbcTemplate jdbcTemplate) { log.info("Starting SequencingRead migration"); val sequencingReads = jdbcTemplate.queryForList("SELECT * FROM sequencingread");