From 2ff1b765653d12d06224c575d395858d4120a086 Mon Sep 17 00:00:00 2001 From: Nick <53413353+nickpalladino@users.noreply.github.com> Date: Tue, 18 Jul 2023 11:58:58 -0400 Subject: [PATCH 1/5] Store treatments in additional info and populate if missing in treatments --- .../constants/BrAPIAdditionalInfoFields.java | 2 + .../daos/BrAPIObservationUnitDAO.java | 69 ++++++++++++++----- .../ExperimentObservation.java | 3 + 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java b/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java index 9e0289a51..39fd856a2 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java +++ b/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java @@ -43,4 +43,6 @@ public final class BrAPIAdditionalInfoFields { public static final String OBSERVATION_DATASET_ID = "observationDatasetId"; public static final String FEMALE_PARENT_UNKNOWN = "femaleParentUnknown"; public static final String MALE_PARENT_UNKNOWN = "maleParentUnknown"; + + public static final String TREATMENTS = "treatments"; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java index 6401c031e..5fb019201 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java @@ -17,11 +17,19 @@ package org.breedinginsight.brapps.importer.daos; +import com.google.gson.Gson; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.reflect.TypeToken; import io.micronaut.context.annotation.Property; +import org.brapi.client.v2.JSON; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.phenotype.ObservationUnitsApi; +import org.brapi.v2.model.pheno.BrAPIObservation; +import org.brapi.v2.model.pheno.BrAPIObservationTreatment; import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.brapi.v2.model.pheno.request.BrAPIObservationUnitSearchRequest; +import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.daos.ProgramDAO; @@ -34,7 +42,9 @@ import javax.inject.Inject; import javax.inject.Singleton; import javax.validation.constraints.NotNull; +import java.lang.reflect.Type; import java.util.*; +import java.util.stream.Collectors; @Singleton public class BrAPIObservationUnitDAO { @@ -46,6 +56,9 @@ public class BrAPIObservationUnitDAO { private final String referenceSource; + private Gson gson = new JSON().getGson(); + private Type treatmentlistType = new TypeToken>(){}.getType(); + @Inject public BrAPIObservationUnitDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, ProgramService programService, @Property(name = "brapi.server.reference-source") String referenceSource) { this.programDAO = programDAO; @@ -64,17 +77,15 @@ public List getObservationUnitByName(List observat BrAPIObservationUnitSearchRequest observationUnitSearchRequest = new BrAPIObservationUnitSearchRequest(); observationUnitSearchRequest.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); observationUnitSearchRequest.observationUnitNames(observationUnitNames); - ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationUnitsApi.class); - return brAPIDAOUtil.search( - api::searchObservationunitsPost, - api::searchObservationunitsSearchResultsDbIdGet, - observationUnitSearchRequest - ); + + return searchObservationUnitsAndProcess(observationUnitSearchRequest, program.getId()); } public List createBrAPIObservationUnits(List brAPIObservationUnitList, UUID programId, ImportUpload upload) throws ApiException { ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ObservationUnitsApi.class); - return brAPIDAOUtil.post(brAPIObservationUnitList, upload, api::observationunitsPost, importDAO::update); + List ous = brAPIDAOUtil.post(brAPIObservationUnitList, upload, api::observationunitsPost, importDAO::update); + processObservationUnits(ous); + return ous; } public List getObservationUnitsById(Collection observationUnitExternalIds, Program program) throws ApiException { @@ -88,10 +99,7 @@ public List getObservationUnitsById(Collection obs observationUnitSearchRequest.externalReferenceIDs(new ArrayList<>(observationUnitExternalIds)); observationUnitSearchRequest.externalReferenceSources(List.of(String.format("%s/%s", referenceSource, ExternalReferenceSource.OBSERVATION_UNITS.getName()))); - ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationUnitsApi.class); - return brAPIDAOUtil.search(api::searchObservationunitsPost, - api::searchObservationunitsSearchResultsDbIdGet, - observationUnitSearchRequest); + return searchObservationUnitsAndProcess(observationUnitSearchRequest, program.getId()); } public List getObservationUnitsForStudyDbId(@NotNull String studyDbId, Program program) throws ApiException { @@ -100,10 +108,7 @@ public List getObservationUnitsForStudyDbId(@NotNull Strin .getProgramDbId())); observationUnitSearchRequest.studyDbIds(List.of(studyDbId)); - ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationUnitsApi.class); - return brAPIDAOUtil.search(api::searchObservationunitsPost, - api::searchObservationunitsSearchResultsDbIdGet, - observationUnitSearchRequest); + return searchObservationUnitsAndProcess(observationUnitSearchRequest, program.getId()); } public List getObservationUnitsForTrialDbId(@NotNull UUID programId, @NotNull String trialDbId) throws ApiException, DoesNotExistException { Program program = programService.getById(programId).orElseThrow(() -> new DoesNotExistException("Program id does not exist")); @@ -113,9 +118,37 @@ public List getObservationUnitsForTrialDbId(@NotNull UUID .getProgramDbId())); observationUnitSearchRequest.trialDbIds(List.of(trialDbId)); - ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationUnitsApi.class); - return brAPIDAOUtil.search(api::searchObservationunitsPost, + return searchObservationUnitsAndProcess(observationUnitSearchRequest, programId); + } + + + /** + * Perform observation unit search and process returned observation units to handle any modifications to the data + * to be returned by bi-api + */ + private List searchObservationUnitsAndProcess(BrAPIObservationUnitSearchRequest request, UUID programId) throws ApiException { + + ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ObservationUnitsApi.class); + List brapiObservationUnits = brAPIDAOUtil.search(api::searchObservationunitsPost, api::searchObservationunitsSearchResultsDbIdGet, - observationUnitSearchRequest); + request); + + processObservationUnits(brapiObservationUnits); + return brapiObservationUnits; + } + + private void processObservationUnits(List brapiObservationUnits) { + + // if has treatments in additionalInfo but not treatments property, copy to treatments property + for (BrAPIObservationUnit ou : brapiObservationUnits) { + JsonObject additionalInfo = ou.getAdditionalInfo(); + if (additionalInfo != null) { + JsonElement treatmentsElement = additionalInfo.get(BrAPIAdditionalInfoFields.TREATMENTS); + if (treatmentsElement != null) { + List treatments = gson.fromJson(treatmentsElement, treatmentlistType); + ou.setTreatments(treatments); + } + } + } } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java index 778ee85b1..b642974d8 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java @@ -290,6 +290,9 @@ public BrAPIObservationUnit constructBrAPIObservationUnit( BrAPIObservationTreatment treatment = new BrAPIObservationTreatment(); treatment.setFactor(getTreatmentFactors()); observationUnit.setTreatments(List.of(treatment)); + + // Store treatments in additionalinfo as well for temporary BreedBase workaround + observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.TREATMENTS, List.of(treatment)); } if (getObsUnitID() != null) { From dfd2fc0c7dc841924f451c8ca2d6234f44d21e23 Mon Sep 17 00:00:00 2001 From: Nick <53413353+nickpalladino@users.noreply.github.com> Date: Tue, 18 Jul 2023 14:53:38 -0400 Subject: [PATCH 2/5] Add tests --- .../daos/BrAPIObservationUnitDAOTest.java | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 src/test/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAOTest.java diff --git a/src/test/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAOTest.java b/src/test/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAOTest.java new file mode 100644 index 000000000..ce742dc7c --- /dev/null +++ b/src/test/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAOTest.java @@ -0,0 +1,144 @@ +package org.breedinginsight.brapps.importer.daos; + +import com.google.gson.Gson; +import io.kowalski.fannypack.FannyPack; +import io.micronaut.http.client.RxHttpClient; +import io.micronaut.http.client.annotation.Client; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import lombok.SneakyThrows; +import org.brapi.client.v2.JSON; +import org.brapi.v2.model.pheno.BrAPIObservationTreatment; +import org.brapi.v2.model.pheno.BrAPIObservationUnit; +import org.breedinginsight.BrAPITest; +import org.breedinginsight.api.model.v1.request.ProgramRequest; +import org.breedinginsight.api.model.v1.request.SpeciesRequest; +import org.breedinginsight.api.v1.controller.TestTokenValidator; +import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; +import org.breedinginsight.brapps.importer.model.ImportProgress; +import org.breedinginsight.brapps.importer.model.ImportUpload; +import org.breedinginsight.dao.db.tables.pojos.SpeciesEntity; +import org.breedinginsight.daos.SpeciesDAO; +import org.breedinginsight.daos.UserDAO; +import org.breedinginsight.model.Program; +import org.breedinginsight.model.User; +import org.breedinginsight.services.ProgramService; +import org.jooq.DSLContext; +import org.junit.jupiter.api.*; + +import javax.inject.Inject; +import java.util.List; + +import static org.breedinginsight.TestUtils.insertAndFetchTestProgram; +import static org.junit.jupiter.api.Assertions.assertEquals; + +@MicronautTest +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class BrAPIObservationUnitDAOTest extends BrAPITest { + + private FannyPack fp; + + @Inject + private DSLContext dsl; + + @Inject + private SpeciesDAO speciesDAO; + + @Inject + private UserDAO userDAO; + + @Inject + private BrAPIObservationUnitDAO obsUnitDAO; + + @Inject + private ProgramService programService; + + private Program validProgram; + + private Gson gson; + private ImportUpload upload; + + private BrAPIObservationTreatment testTreatment; + + @Inject + @Client("/${micronaut.bi.api.version}") + RxHttpClient biClient; + + @BeforeAll + @SneakyThrows + public void setup() { + + ImportProgress progress = ImportProgress.builder().build(); + + upload = ImportUpload.uploadBuilder() + .progress(progress) + .build(); + + gson = new JSON().getGson(); + + // Add species needed to create program + fp = FannyPack.fill("src/test/resources/sql/brapi/species.sql"); + super.getBrapiDsl().execute(fp.get("InsertSpecies")); + SpeciesEntity validSpecies = speciesDAO.findAll().get(0); + + // Insert system admin role so can create program + FannyPack securityFp = FannyPack.fill("src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql"); + User testUser = userDAO.getUserByOrcId(TestTokenValidator.TEST_USER_ORCID).get(); + dsl.execute(securityFp.get("InsertSystemRoleAdmin"), testUser.getId().toString()); + + SpeciesRequest speciesRequest = SpeciesRequest.builder() + .commonName(validSpecies.getCommonName()) + .id(validSpecies.getId()) + .build(); + + ProgramRequest program = ProgramRequest.builder() + .name("Test Program") + .species(speciesRequest) + .key("TEST") + .build(); + + // create test program + validProgram = insertAndFetchTestProgram(gson, biClient, program); + // updated with brapi db id + validProgram = programService.getById(validProgram.getId()).get(); + + testTreatment = new BrAPIObservationTreatment(); + testTreatment.setFactor("ou1 treatment"); + } + + @Test + @SneakyThrows + @Order(1) + public void testCreateObservationUnitAdditionalInfoSingleTreatmentFactor() { + // create observation unit with treatments only in additional info to simulate breedbase not populating + // treatments field + BrAPIObservationUnit ou1 = new BrAPIObservationUnit(); + ou1.setObservationUnitName("test1"); + ou1.putAdditionalInfoItem(BrAPIAdditionalInfoFields.TREATMENTS, List.of(testTreatment)); + ou1.setProgramDbId(validProgram.getBrapiProgram().getProgramDbId()); + + List ous = List.of(ou1); + List createdOus = obsUnitDAO.createBrAPIObservationUnits(ous, validProgram.getId(), upload); + singleTreatmentAsserts(createdOus, testTreatment); + } + + @Test + @SneakyThrows + @Order(2) + public void testGetObservationUnitAdditionalInfoSingleTreatmentFactor() { + List createdOus = obsUnitDAO.getObservationUnitByName(List.of("test1"), validProgram); + singleTreatmentAsserts(createdOus, testTreatment); + } + + private void singleTreatmentAsserts(List obsUnits, BrAPIObservationTreatment expectedTreatment) { + assertEquals(1, obsUnits.size(), "Expected 1 observation unit"); + + BrAPIObservationUnit ou = obsUnits.get(0); + List treatments = ou.getTreatments(); + assertEquals(1, treatments.size(), "Expected treatments property"); + + BrAPIObservationTreatment treatment = treatments.get(0); + assertEquals(expectedTreatment, treatment, "Expected treatments to be same"); + } + +} From 890051e80e6291cf9aed3374d45d74e98e5df919 Mon Sep 17 00:00:00 2001 From: Nick <53413353+nickpalladino@users.noreply.github.com> Date: Wed, 19 Jul 2023 16:16:35 -0400 Subject: [PATCH 3/5] Moved adding treatment to additional info into dao --- .../importer/daos/BrAPIObservationUnitDAO.java | 16 ++++++++++++++-- .../ExperimentObservation.java | 3 --- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java index 5fb019201..0096c3599 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java @@ -25,7 +25,6 @@ import org.brapi.client.v2.JSON; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.phenotype.ObservationUnitsApi; -import org.brapi.v2.model.pheno.BrAPIObservation; import org.brapi.v2.model.pheno.BrAPIObservationTreatment; import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.brapi.v2.model.pheno.request.BrAPIObservationUnitSearchRequest; @@ -44,7 +43,6 @@ import javax.validation.constraints.NotNull; import java.lang.reflect.Type; import java.util.*; -import java.util.stream.Collectors; @Singleton public class BrAPIObservationUnitDAO { @@ -81,8 +79,12 @@ public List getObservationUnitByName(List observat return searchObservationUnitsAndProcess(observationUnitSearchRequest, program.getId()); } + /** + * Create observation units, mutates brAPIObservationUnitList + */ public List createBrAPIObservationUnits(List brAPIObservationUnitList, UUID programId, ImportUpload upload) throws ApiException { ObservationUnitsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ObservationUnitsApi.class); + preprocessObservationUnits(brAPIObservationUnitList); List ous = brAPIDAOUtil.post(brAPIObservationUnitList, upload, api::observationunitsPost, importDAO::update); processObservationUnits(ous); return ous; @@ -151,4 +153,14 @@ private void processObservationUnits(List brapiObservation } } } + + private void preprocessObservationUnits(List brapiObservationUnits) { + // add treatments to additional info + for (BrAPIObservationUnit obsUnit : brapiObservationUnits) { + List treatments = obsUnit.getTreatments(); + if (treatments != null) { + obsUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.TREATMENTS, treatments); + } + } + } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java index b642974d8..778ee85b1 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java @@ -290,9 +290,6 @@ public BrAPIObservationUnit constructBrAPIObservationUnit( BrAPIObservationTreatment treatment = new BrAPIObservationTreatment(); treatment.setFactor(getTreatmentFactors()); observationUnit.setTreatments(List.of(treatment)); - - // Store treatments in additionalinfo as well for temporary BreedBase workaround - observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.TREATMENTS, List.of(treatment)); } if (getObsUnitID() != null) { From 5cf2c2d8a6ece2cb777102b9708c29be3a000e83 Mon Sep 17 00:00:00 2001 From: Nick <53413353+nickpalladino@users.noreply.github.com> Date: Mon, 24 Jul 2023 10:06:05 -0400 Subject: [PATCH 4/5] Made json processing variables final --- .../brapps/importer/daos/BrAPIObservationUnitDAO.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java index 0096c3599..5554e4227 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java @@ -54,8 +54,8 @@ public class BrAPIObservationUnitDAO { private final String referenceSource; - private Gson gson = new JSON().getGson(); - private Type treatmentlistType = new TypeToken>(){}.getType(); + private final Gson gson = new JSON().getGson(); + private final Type treatmentlistType = new TypeToken>(){}.getType(); @Inject public BrAPIObservationUnitDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, ProgramService programService, @Property(name = "brapi.server.reference-source") String referenceSource) { From e72f52bd2607522c228d7606ba2b4cd94c670f53 Mon Sep 17 00:00:00 2001 From: Nick <53413353+nickpalladino@users.noreply.github.com> Date: Tue, 25 Jul 2023 14:53:23 -0400 Subject: [PATCH 5/5] Updated comment --- .../brapps/importer/daos/BrAPIObservationUnitDAO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java index 5554e4227..66465a7a4 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationUnitDAO.java @@ -141,7 +141,7 @@ private List searchObservationUnitsAndProcess(BrAPIObserva private void processObservationUnits(List brapiObservationUnits) { - // if has treatments in additionalInfo but not treatments property, copy to treatments property + // if has treatments in additionalInfo, copy to treatments property for (BrAPIObservationUnit ou : brapiObservationUnits) { JsonObject additionalInfo = ou.getAdditionalInfo(); if (additionalInfo != null) {