diff --git a/.env.template b/.env.template index 0e768adcd..73ff2d89e 100644 --- a/.env.template +++ b/.env.template @@ -79,3 +79,5 @@ TRIAL_START_DELAY=15s TRAIT_START_DELAY=20s OBSERVATION_START_DELAY=25s OBSERVATION_UNIT_START_DELAY=30s + +DATA_TABLE_MAX_SIZE=200 \ No newline at end of file diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java index 2be39af43..625816256 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java @@ -226,6 +226,7 @@ private void setDbIds(BrAPIObservationUnit ou) { ou.programDbId(Utilities.getExternalReference(ou.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.PROGRAMS)) .orElseThrow(() -> new IllegalStateException("No BI external reference found")) .getReferenceID()); + // TODO: Remove this as part of [BI-3006] if (ou.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_UUID)) { ou.setGermplasmDbId(ou.getAdditionalInfo() .get(BrAPIAdditionalInfoFields.GERMPLASM_UUID) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java index ca896441e..59e4776c9 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -20,6 +20,7 @@ import com.google.gson.JsonObject; import io.micronaut.context.annotation.Context; import io.micronaut.context.annotation.Property; +import io.micronaut.http.HttpStatus; import io.micronaut.http.server.exceptions.InternalServerException; import io.micronaut.scheduling.annotation.Scheduled; import lombok.extern.slf4j.Slf4j; @@ -74,6 +75,9 @@ public class BrAPIGermplasmDAO { @Property(name = "brapi.paginate.germplasm") private boolean paginateGermplasm; + @Property(name = "data-table.max-size") + private int dataTableMaxSize; + private final ProgramCache programGermplasmCache; private final BrAPIEndpointProvider brAPIEndpointProvider; @@ -188,28 +192,28 @@ private Map fetchProgramGermplasm(UUID programId) throws api::searchGermplasmPost, api::searchGermplasmSearchResultsDbIdGet, germplasmSearch), - program.getKey()); + program); } else { log.debug("Fetching germplasm without pagination to BrAPI"); return processGermplasmForDisplay(brAPIDAOUtil.searchNoPaging( api::searchGermplasmPost, api::searchGermplasmSearchResultsDbIdGet, germplasmSearch), - program.getKey()); + program); } } public void repopulateGermplasmCacheForProgram(UUID programId) { programGermplasmCache.populate(programId); } - /** * Process germplasm into a format for display * @param programGermplasm * @return Map * @throws ApiException */ - private Map processGermplasmForDisplay(List programGermplasm, String programKey) { + private Map processGermplasmForDisplay(List programGermplasm, + Program program) throws ApiException { // Process the germplasm Map programGermplasmMap = new HashMap<>(); log.trace("processing germ for display: " + programGermplasm); @@ -227,12 +231,32 @@ private Map processGermplasmForDisplay(Listmale/femaleParentUUID. (See GermplasmProcessor.constructPedigreeString() for assignment of this data) + // To avoid completely re-working the importer to create pedigree nodes and associated germplasm referenced first, this "hack" relates the bi-generated exrefs + // to the brapi-generated germplasmDbId via an extra lookup and processing to the database. + // This is necessary because we need to overwrite bi-generated ids in the pedigree string to use the brapi germplasmDbIds so that front end reference links work properly. + // This should always work because processGermplasmForDisplay is only ever called once Germplasm data has been created in the database, and provided we improve lookups to not fetch + // all program germplasm at once, the performance hit should be negligible. + // TODO: This hack can be removed once/if we implement [BI-2588/BI-2452] + Map pedigreeBrAPIGermplasmDbIdByBICreatedExRef = null; + + // This should ease concerns for extra time pulling this data. The intended usage is for the Germplasm data table, + // but any usage under 200 germs (current data table max size) should not be too large of a hit to the server or database. + boolean pedigreeExRefMutation = programGermplasm.size() <= dataTableMaxSize; + + if (pedigreeExRefMutation) { + // Only perform this operation for callers that require it + pedigreeBrAPIGermplasmDbIdByBICreatedExRef = getPedigreeGermplasmDbIdByBICreatedExRef(programGermplasm, program); + } + // Update pedigree string for (BrAPIGermplasm germplasm: programGermplasm) { JsonObject additionalInfo = germplasm.getAdditionalInfo(); @@ -260,7 +284,7 @@ private Map processGermplasmForDisplay(List processGermplasmForDisplay(List processGermplasmForDisplay(List processGermplasmForDisplay(List getPedigreeGermplasmDbIdByBICreatedExRef(List brAPIGermplasm, Program program) throws ApiException { + Map germplasmDbIdByGeneratedExRef = new HashMap<>(); + + // First, utilize initialized map to store all related parent bi-generated exref IDs. + for (BrAPIGermplasm germplasm : brAPIGermplasm) { + if (germplasm.getAdditionalInfo() == null) { + continue; + } + if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID)) { + try { + UUID.fromString(germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID).getAsString()); + } catch (IllegalArgumentException e) { + throw new ApiException(HttpStatus.INTERNAL_SERVER_ERROR.getCode(), String.format("Germplasm with BrAPI dbId [%s] has an additionalInfo.femaleParentUUID that is not a UUID", germplasm.getGermplasmDbId())); + } + germplasmDbIdByGeneratedExRef.put(germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID).getAsString(), null); + } + if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID)) { + try { + UUID.fromString(germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID).getAsString()); + } catch (IllegalArgumentException e) { + throw new ApiException(HttpStatus.INTERNAL_SERVER_ERROR.getCode(), String.format("Germplasm with BrAPI dbId [%s] has an additionalInfo.maleParentUUID that is not a UUID", germplasm.getGermplasmDbId())); + } + germplasmDbIdByGeneratedExRef.put(germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID).getAsString(), null); + } + } + + if (germplasmDbIdByGeneratedExRef.isEmpty()) { + // If there are none, no reason to proceed with a BrAPI search. + return germplasmDbIdByGeneratedExRef; + } + + // Now grab all male/female exrefs and search for them to get their germplasmDbIds + List exRefIds = new ArrayList<>(germplasmDbIdByGeneratedExRef.keySet()); + + BrAPIGermplasmSearchRequest searchRequest = new BrAPIGermplasmSearchRequest(); + searchRequest.setExternalReferenceIds(exRefIds); + searchRequest.setExternalReferenceSources(List.of(referenceSource)); + + // Reuse some code to properly set searchRequest with brapiProgramDbId, and proper paging + searchRequest = buildSearchRequest(program, null, null, searchRequest); + + GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), GermplasmApi.class); + + + // This will use CACHE_BRAPI_FETCH_PAGE_SIZE env variable to lookup at maximum 65k pedigree germplasm at once. + // This should not be normal though, as we should try not to exceed 1k lookups at once using this code. + // If there is a need to look up a large amount of germplasm records at once, alternative methods should be used. + List germplasmSearchedWithExRef = brAPIDAOUtil.searchNoPaging( + api::searchGermplasmPost, + api::searchGermplasmSearchResultsDbIdGet, + searchRequest); + + for (BrAPIGermplasm germplasm : germplasmSearchedWithExRef) { + Optional exRef = Utilities.getExternalReference(germplasm.getExternalReferences(), referenceSource); + + if (exRef.isPresent()) { + germplasmDbIdByGeneratedExRef.put(exRef.get().getReferenceId(), germplasm.getGermplasmDbId()); + } else { + throw new IllegalStateException("External references wasn't found for germplasm (dbid): " + germplasm.getGermplasmDbId()); + } + } + + return germplasmDbIdByGeneratedExRef; + } + /** * This method requires a BI-API program. If the BrAPIProgram inside this data model is not set, * this method will retrieve it. @@ -336,7 +437,7 @@ private List getBrAPIGermplasmUsingBrAPIProgramId(GermplasmQuery List result = brAPIDAOUtil.get(api::germplasmGet, germplasmQueryParams); // TODO: Once cache is removed for this class, fix processGermplasmForDisplay to return List [BI-2906] - return new ArrayList<>(processGermplasmForDisplay(result, program.getKey()).values()); + return new ArrayList<>(processGermplasmForDisplay(result, program).values()); } // TODO: hack for now, probably should update breedbase @@ -369,11 +470,11 @@ private String processBreedbasePedigree(String pedigree) { public List createBrAPIGermplasm(List postBrAPIGermplasmList, UUID programId, ImportUpload upload) { GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), GermplasmApi.class); - var program = programDAO.fetchOneById(programId); + var program = new Program(programDAO.fetchOneById(programId)); try { if (!postBrAPIGermplasmList.isEmpty()) { List postResponse = brAPIDAOUtil.post(postBrAPIGermplasmList, upload, api::germplasmPost, importDAO::update); - return new ArrayList<>(processGermplasmForDisplay(postResponse, program.getKey()).values()); + return new ArrayList<>(processGermplasmForDisplay(postResponse, program).values()); } return new ArrayList<>(); } catch (Exception e) { @@ -383,12 +484,12 @@ public List createBrAPIGermplasm(List postBrAPIG public List updateBrAPIGermplasm(List putBrAPIGermplasmList, UUID programId, ImportUpload upload) { GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), GermplasmApi.class); - var program = programDAO.fetchOneById(programId); + var program = new Program(programDAO.fetchOneById(programId)); try { if (!putBrAPIGermplasmList.isEmpty()) { Callable> postFunction = () -> { List putResponse = putGermplasm(putBrAPIGermplasmList, api); - return processGermplasmForDisplay(putResponse, program.getKey()); + return processGermplasmForDisplay(putResponse, program); }; return programGermplasmCache.post(programId, postFunction); } @@ -428,15 +529,25 @@ public BrAPIGermplasmListResponse brapiGermplasmSearchReturnResponse(Program pro // TODO: Once cache is removed for this class, fix processGermplasmForDisplay to return List [BI-2906] List processedGermplasm = - new ArrayList<>(processGermplasmForDisplay(brAPIDAOUtil.getListResult(brAPIResponse), program.getKey()).values()); + new ArrayList<>(processGermplasmForDisplay(brAPIDAOUtil.getListResult(brAPIResponse), program).values()); brAPIResponse.getResult().setData(processedGermplasm); return brAPIResponse; } - private BrAPIGermplasmSearchRequest buildSearchRequest(Program program, List brapiGermplasmIds, GermplasmQuery germplasmQuery) throws ApiException { - BrAPIGermplasmSearchRequest searchRequest = new BrAPIGermplasmSearchRequest(); + private BrAPIGermplasmSearchRequest buildSearchRequest(Program program, List brapiGermplasmIds, GermplasmQuery query) throws ApiException { + return buildSearchRequest(program, brapiGermplasmIds, query, null); + } + + private BrAPIGermplasmSearchRequest buildSearchRequest(Program program, List brapiGermplasmIds, GermplasmQuery germplasmQuery, BrAPIGermplasmSearchRequest searchRequestPassThru) throws ApiException { + BrAPIGermplasmSearchRequest searchRequest; + + if (searchRequestPassThru == null) { + searchRequest = new BrAPIGermplasmSearchRequest(); + } else { + searchRequest = searchRequestPassThru; + } searchRequest.programDbIds(List.of(brAPIDAOUtil.getBrAPIProgramDbId(program.getId()))); diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java index 550dc9b56..5406ea134 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java @@ -316,7 +316,7 @@ public List getObservationUnits(Program program, .orElse(true); //adding filter for germplasmDbId because we can't easily search that in the stored data object - // TODO: Add search on accessionNumber once it's been added to prod server and brapi client [BI-2978] + // TODO: Add search on germplasmDbId directly in search request [BI-3006] return matches && germplasmId.map(id -> id.equals(ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_UUID).getAsString())).orElse(true); }).collect(Collectors.toList()); } @@ -382,7 +382,7 @@ private void processObservationUnits(Program program, List HashMap germplasmByDbId = new HashMap<>(); if( withGID ){ - // TODO: Optimize this to use germplasm information directly in BrAPIObservationUnit by adding accession num/GID there via the prodserver/client [BI-2978] + // TODO: Optimize this to use germplasm information directly in BrAPIObservationUnit by searching on ou.germplasmDbIds in a GermplasmSearchRequest [BI-3006] this.germplasmService.getGermplasm(program.getId()).forEach((germplasm -> germplasmByDbId.put(germplasm.getGermplasmDbId(), germplasm))); } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIPedigreeDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIPedigreeDAO.java index 3a164c07d..b89a28e7e 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIPedigreeDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIPedigreeDAO.java @@ -85,7 +85,7 @@ public List getPedigree( PedigreeQueryParams pedigreeRequest = new PedigreeQueryParams(); - // TODO: Issue with BrAPI server programDbId filtering, think germplasm are linked to program through observation + // TODO: Issue with BrAPI server programDbId filtering, think germplasm are linked to program through observation [BI- // units and doesn't work if don't have any loaded // use external refs instead for now //pedigreeSearchRequest.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index d58a8d8d2..7114763f0 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -237,3 +237,5 @@ startup: trait: ${TRAIT_START_DELAY:2s} observation: ${OBSERVATION_START_DELAY:3s} observation_unit: ${OBSERVATION_UNIT_START_DELAY:3s} +data-table: + max-size: ${DATA_TABLE_MAX_SIZE:200} diff --git a/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java b/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java index a47a3eebb..127feb94c 100644 --- a/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java +++ b/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java @@ -155,6 +155,7 @@ public void getGermplasmListExport() { when(programDAO.getProgramBrAPI(any())).thenReturn(brapiProgram); when(brAPIDAOUtil.get(any(Function.class), any(GermplasmQueryParams.class))).thenReturn(germplasm); + when(brAPIDAOUtil.getBrAPIProgramDbId(any())).thenReturn(brapiProgramDbId); //Create germplasm cache of stub data Method setupMethod = BrAPIGermplasmDAO.class.getDeclaredMethod("setup");