From a761163cfd9f9490c679eb43276e93e7911a5d0c Mon Sep 17 00:00:00 2001 From: David Randolph Phillips Date: Wed, 9 Oct 2024 11:53:32 -0400 Subject: [PATCH 1/4] [BI-2328] Added new validation for periods in a trait name --- .../validators/TraitFileValidatorError.java | 5 ++++ .../validators/TraitValidatorError.java | 4 ++++ .../TraitValidatorErrorInterface.java | 1 + .../validators/TraitValidatorService.java | 21 ++++++++++++++++- .../delta/DeltaEntityFactoryUnitTest.java | 1 - .../validators/TraitValidatorUnitTest.java | 23 +++++++++++++++++++ 6 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/services/validators/TraitFileValidatorError.java b/src/main/java/org/breedinginsight/services/validators/TraitFileValidatorError.java index 4ba0f99d7..40807cf03 100644 --- a/src/main/java/org/breedinginsight/services/validators/TraitFileValidatorError.java +++ b/src/main/java/org/breedinginsight/services/validators/TraitFileValidatorError.java @@ -65,6 +65,11 @@ public ValidationError getMissingScaleDataTypeMsg() { return new ValidationError("Scale Class", "Missing scale class", HttpStatus.UNPROCESSABLE_ENTITY); } + @Override + public ValidationError getPeriodObsVarNameMsg() { + return new ValidationError("Name", "Period is invalid", HttpStatus.UNPROCESSABLE_ENTITY); + } + @Override public ValidationError getMissingObsVarNameMsg() { return new ValidationError("Name", "Missing name", HttpStatus.UNPROCESSABLE_ENTITY); diff --git a/src/main/java/org/breedinginsight/services/validators/TraitValidatorError.java b/src/main/java/org/breedinginsight/services/validators/TraitValidatorError.java index b2db156f6..c9765ed82 100644 --- a/src/main/java/org/breedinginsight/services/validators/TraitValidatorError.java +++ b/src/main/java/org/breedinginsight/services/validators/TraitValidatorError.java @@ -66,6 +66,10 @@ public ValidationError getMissingScaleDataTypeMsg() { } @Override + public ValidationError getPeriodObsVarNameMsg() { + return new ValidationError("observationVariableName", "Period in name is invalid", HttpStatus.BAD_REQUEST); + } + public ValidationError getMissingObsVarNameMsg() { return new ValidationError("observationVariableName", "Missing Name", HttpStatus.BAD_REQUEST); } diff --git a/src/main/java/org/breedinginsight/services/validators/TraitValidatorErrorInterface.java b/src/main/java/org/breedinginsight/services/validators/TraitValidatorErrorInterface.java index 49ef88da5..27400e2c5 100644 --- a/src/main/java/org/breedinginsight/services/validators/TraitValidatorErrorInterface.java +++ b/src/main/java/org/breedinginsight/services/validators/TraitValidatorErrorInterface.java @@ -31,6 +31,7 @@ public interface TraitValidatorErrorInterface { ValidationError getMissingScaleUnitMsg(); ValidationError getMissingScaleDataTypeMsg(); ValidationError getMissingObsVarNameMsg(); + ValidationError getPeriodObsVarNameMsg(); ValidationError getMissingTraitEntityMsg(); ValidationError getMissingTraitAttributeMsg(); ValidationError getMissingTraitDescriptionMsg(); diff --git a/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java b/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java index e778898bd..de88755d2 100644 --- a/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java +++ b/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java @@ -16,6 +16,7 @@ */ package org.breedinginsight.services.validators; +import io.micronaut.http.HttpStatus; import org.breedinginsight.api.model.v1.response.ValidationError; import org.breedinginsight.api.model.v1.response.ValidationErrors; import org.breedinginsight.dao.db.enums.DataType; @@ -207,6 +208,23 @@ public ValidationErrors checkTraitFieldsLength(List traits, TraitValidato return errors; } + public ValidationErrors checkTraitFieldsFormat(List traits, TraitValidatorErrorInterface traitValidatorErrors) { + + ValidationErrors errors = new ValidationErrors(); + + for (int i = 0; i < traits.size(); i++) { + + Trait trait = traits.get(i); + String name = trait.getObservationVariableName(); + + if (name != null && name.contains(".")){ + ValidationError error = traitValidatorErrors.getPeriodObsVarNameMsg(); + errors.addError(traitValidatorErrors.getRowNumber(i), error); + } + } + return errors; + } + public List checkDuplicateTraitsExistingByName(UUID programId, List traits){ List duplicates = new ArrayList<>(); @@ -273,7 +291,8 @@ public Optional checkAllTraitValidations(List traits, T ValidationErrors dataConsistencyErrors = checkTraitDataConsistency(traits, traitValidatorError); ValidationErrors duplicateTraitsInFile = checkDuplicateTraitsInFile(traits, traitValidatorError); ValidationErrors fieldLengthError = checkTraitFieldsLength(traits, traitValidatorError); - validationErrors.mergeAll(requiredFieldErrors, dataConsistencyErrors, duplicateTraitsInFile, fieldLengthError); + ValidationErrors fieldFormatErrors = checkTraitFieldsFormat(traits, traitValidatorError); + validationErrors.mergeAll(requiredFieldErrors, dataConsistencyErrors, duplicateTraitsInFile, fieldLengthError, fieldFormatErrors); if (validationErrors.hasErrors()){ return Optional.of(validationErrors); diff --git a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java index 809b5df86..48066c923 100644 --- a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java +++ b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java @@ -426,7 +426,6 @@ private BrAPIObservation createBrAPIObservation() { .additionalInfo(toJsonObject(additionalInfoString)) .collector("intern") .externalReferences(createExternalReferences()) - .geoCoordinates(BrApiGeoJSON.builder().geometry(null).type("Feature").build()) .germplasmDbId("2bb19ef2-fcc5-406d-b9c3-4517c665b699") .germplasmName("lucky") .observationTimeStamp(OffsetDateTime.now()) diff --git a/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java b/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java index 184e7308e..22bd5dfa8 100644 --- a/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java +++ b/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java @@ -348,5 +348,28 @@ public void charLimitExceeded() { } } + @Test + @SneakyThrows + public void periodInName() { + + Trait trait = new Trait(); + trait.setObservationVariableName("Period.1"); + + ValidationErrors validationErrors = traitValidatorService.checkTraitFieldsFormat(List.of(trait), new TraitValidatorError()); + + assertEquals(1, validationErrors.getRowErrors().size(), "Wrong number of row errors returned"); + RowValidationErrors rowValidationErrors = validationErrors.getRowErrors().get(0); + assertEquals(1, rowValidationErrors.getErrors().size(), "Wrong number of errors for row"); + assertEquals(400, rowValidationErrors.getErrors().get(0).getHttpStatusCode(), "Wrong error code"); + assertEquals("Name", rowValidationErrors.getErrors().get(0).getField(), "Wrong error column"); + + //There should be no errors + Trait noPeriodTrait = new Trait(); + noPeriodTrait.setObservationVariableName("NoPeriod"); + validationErrors = traitValidatorService.checkTraitFieldsFormat(List.of(noPeriodTrait), new TraitValidatorError()); + assertEquals(0, validationErrors.getRowErrors().size(), "Wrong number of row errors returned"); + } + + } From f437b970b5e5ab5ad78ed4b8a8ae2658235642c3 Mon Sep 17 00:00:00 2001 From: David Randolph Phillips Date: Fri, 11 Oct 2024 09:22:27 -0400 Subject: [PATCH 2/4] [BI-2328] fixed error I created in previous commit --- .../breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java index 48066c923..809b5df86 100644 --- a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java +++ b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java @@ -426,6 +426,7 @@ private BrAPIObservation createBrAPIObservation() { .additionalInfo(toJsonObject(additionalInfoString)) .collector("intern") .externalReferences(createExternalReferences()) + .geoCoordinates(BrApiGeoJSON.builder().geometry(null).type("Feature").build()) .germplasmDbId("2bb19ef2-fcc5-406d-b9c3-4517c665b699") .germplasmName("lucky") .observationTimeStamp(OffsetDateTime.now()) From beaee3ad4a805eeb1ce291c9e979cb0f6f7b7682 Mon Sep 17 00:00:00 2001 From: David Randolph Phillips Date: Wed, 23 Oct 2024 11:52:29 -0400 Subject: [PATCH 3/4] [BI-2328] fixed test, future proofed some code --- .../services/validators/TraitValidatorService.java | 8 +++++++- .../model/delta/DeltaEntityFactoryUnitTest.java | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java b/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java index de88755d2..d8bd6bf7e 100644 --- a/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java +++ b/src/main/java/org/breedinginsight/services/validators/TraitValidatorService.java @@ -27,6 +27,8 @@ import javax.inject.Inject; import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isBlank; @@ -217,7 +219,11 @@ public ValidationErrors checkTraitFieldsFormat(List traits, TraitValidato Trait trait = traits.get(i); String name = trait.getObservationVariableName(); - if (name != null && name.contains(".")){ + Pattern pattern = Pattern.compile("\\."); + Matcher matcher = pattern.matcher(name); + boolean containsInvalidCharacter = matcher.find(); + + if (name != null && containsInvalidCharacter){ ValidationError error = traitValidatorErrors.getPeriodObsVarNameMsg(); errors.addError(traitValidatorErrors.getRowNumber(i), error); } diff --git a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java index 809b5df86..cd8f71028 100644 --- a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java +++ b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java @@ -426,7 +426,7 @@ private BrAPIObservation createBrAPIObservation() { .additionalInfo(toJsonObject(additionalInfoString)) .collector("intern") .externalReferences(createExternalReferences()) - .geoCoordinates(BrApiGeoJSON.builder().geometry(null).type("Feature").build()) +// .geoCoordinates(BrApiGeoJSON.builder().geometry(null).type("Feature").build()) .germplasmDbId("2bb19ef2-fcc5-406d-b9c3-4517c665b699") .germplasmName("lucky") .observationTimeStamp(OffsetDateTime.now()) From e4177a026ae86674917ab48c5505bf729748c085 Mon Sep 17 00:00:00 2001 From: David Randolph Phillips Date: Thu, 24 Oct 2024 09:17:13 -0400 Subject: [PATCH 4/4] [BI-2328] fixed bug and unit test --- .../breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java | 2 +- .../services/validators/TraitValidatorUnitTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java index cd8f71028..809b5df86 100644 --- a/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java +++ b/src/test/java/org/breedinginsight/model/delta/DeltaEntityFactoryUnitTest.java @@ -426,7 +426,7 @@ private BrAPIObservation createBrAPIObservation() { .additionalInfo(toJsonObject(additionalInfoString)) .collector("intern") .externalReferences(createExternalReferences()) -// .geoCoordinates(BrApiGeoJSON.builder().geometry(null).type("Feature").build()) + .geoCoordinates(BrApiGeoJSON.builder().geometry(null).type("Feature").build()) .germplasmDbId("2bb19ef2-fcc5-406d-b9c3-4517c665b699") .germplasmName("lucky") .observationTimeStamp(OffsetDateTime.now()) diff --git a/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java b/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java index 22bd5dfa8..ea60ac285 100644 --- a/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java +++ b/src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java @@ -361,7 +361,7 @@ public void periodInName() { RowValidationErrors rowValidationErrors = validationErrors.getRowErrors().get(0); assertEquals(1, rowValidationErrors.getErrors().size(), "Wrong number of errors for row"); assertEquals(400, rowValidationErrors.getErrors().get(0).getHttpStatusCode(), "Wrong error code"); - assertEquals("Name", rowValidationErrors.getErrors().get(0).getField(), "Wrong error column"); + assertEquals("observationVariableName", rowValidationErrors.getErrors().get(0).getField(), "Wrong error column"); //There should be no errors Trait noPeriodTrait = new Trait();