diff --git a/.env.template b/.env.template index abe859391..1a481aae6 100644 --- a/.env.template +++ b/.env.template @@ -31,6 +31,10 @@ BRAPI_READ_TIMEOUT=60m # Max number of records to POST to the BrAPI service per request POST_CHUNK_SIZE=1000 +# Request cache records paginating through available records per program by CACHE_BRAPI_FETCH_PAGE_SIZE +CACHE_PAGINATE_GERMPLASM=false +CACHE_BRAPI_FETCH_PAGE_SIZE=65000 + # BrAPI Server Variables BRAPI_SERVER_PORT=8083 diff --git a/docker-compose.yml b/docker-compose.yml index bb30e858e..efebf5fb6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -71,7 +71,7 @@ services: networks: - backend bidb: - image: postgres:11.4 + image: postgres:17.5 container_name: bidb environment: - POSTGRES_DB=${DB_NAME} diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java index 84a6a17f0..850ed8138 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java @@ -222,6 +222,26 @@ public HttpResponse>>> getGermplasm( } } + @Get("/programs/{programId}/germplasm/lists/{listDbId}/export{?fileExtension}") + @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") + @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) + public HttpResponse germplasmListExport( + @PathVariable("programId") UUID programId, @PathVariable("listDbId") String listDbId, @QueryValue(defaultValue = "XLSX") String fileExtension) { + String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu."; + try { + FileType extension = Enum.valueOf(FileType.class, fileExtension); + DownloadFile germplasmListFile = germplasmService.exportGermplasmList(programId, listDbId, extension); + HttpResponse germplasmListExport = HttpResponse.ok(germplasmListFile.getStreamedFile()).header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename="+germplasmListFile.getFileName()+extension.getExtension()); + return germplasmListExport; + } + catch (Exception e) { + log.info(e.getMessage(), e); + e.printStackTrace(); + HttpResponse response = HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, downloadErrorMessage).contentType(MediaType.TEXT_PLAIN).body(downloadErrorMessage); + return response; + } + } + @Get("/programs/{programId}/germplasm/export{?fileExtension,list}") @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) 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 75226babf..478c99bcf 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -65,6 +65,9 @@ public class BrAPIGermplasmDAO { @Property(name = "micronaut.bi.api.run-scheduled-tasks") private boolean runScheduledTasks; + @Property(name = "brapi.paginate.germplasm") + private boolean paginateGermplasm; + private final ProgramCache programGermplasmCache; private final BrAPIEndpointProvider brAPIEndpointProvider; @@ -141,11 +144,26 @@ private Map fetchProgramGermplasm(UUID programId) throws BrAPIGermplasmSearchRequest germplasmSearch = new BrAPIGermplasmSearchRequest(); germplasmSearch.externalReferenceIDs(List.of(programId.toString())); germplasmSearch.externalReferenceSources(List.of(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.PROGRAMS))); - return processGermplasmForDisplay(brAPIDAOUtil.search( - api::searchGermplasmPost, - api::searchGermplasmSearchResultsDbIdGet, - germplasmSearch - ), program.getKey()); + + if (paginateGermplasm) { + log.debug("Fetching germplasm with pagination to BrAPI"); + return processGermplasmForDisplay(brAPIDAOUtil.search( + api::searchGermplasmPost, + api::searchGermplasmSearchResultsDbIdGet, + germplasmSearch), + program.getKey()); + } else { + log.debug("Fetching germplasm without pagination to BrAPI"); + return processGermplasmForDisplay(brAPIDAOUtil.searchNoPaging( + api::searchGermplasmPost, + api::searchGermplasmSearchResultsDbIdGet, + germplasmSearch), + program.getKey()); + } + } + + public void repopulateGermplasmCacheForProgram(UUID programId) { + programGermplasmCache.populate(programId); } /** @@ -296,11 +314,8 @@ public List createBrAPIGermplasm(List postBrAPIG var program = programDAO.fetchOneById(programId); try { if (!postBrAPIGermplasmList.isEmpty()) { - Callable> postFunction = () -> { List postResponse = brAPIDAOUtil.post(postBrAPIGermplasmList, upload, api::germplasmPost, importDAO::update); - return processGermplasmForDisplay(postResponse, program.getKey()); - }; - return programGermplasmCache.post(programId, postFunction); + return new ArrayList<>(processGermplasmForDisplay(postResponse, program.getKey()).values()); } return new ArrayList<>(); } catch (Exception e) { diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java b/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java index 00d89e4db..de1cf19f1 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java @@ -155,7 +155,19 @@ public static String constructGermplasmListName(String listName, Program program return String.format("%s [%s-germplasm]", listName, program.getKey()); } - public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID listId, boolean commit, boolean updatePedigree) { + /** + * Will mutate synonym and pedigree fields if changed and meet change criteria + * + * @param germplasm germplasm object + * @param program program + * @param listId list id + * @param commit flag indicating if commit changes should be made + * @param updatePedigree flag indicating if pedigree should be updated + * @return mutated indicator + */ + public boolean updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID listId, boolean commit, boolean updatePedigree) { + + boolean mutated = false; if (updatePedigree) { if (!StringUtils.isBlank(getFemaleParentAccessionNumber())) { @@ -170,6 +182,7 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID if (!StringUtils.isBlank(getMaleParentEntryNo())) { germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_ENTRY_NO, getMaleParentEntryNo()); } + mutated = true; } // Append synonyms to germplasm that don't already exist @@ -181,6 +194,7 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID brapiSynonym.setSynonym(synonym); if (!existingSynonyms.contains(brapiSynonym)) { germplasm.addSynonymsItem(brapiSynonym); + mutated = true; } } } @@ -193,6 +207,8 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID if (commit) { setUpdateCommitFields(germplasm, program.getKey()); } + + return mutated; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java index 54a1afe06..f3ce7e530 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java @@ -135,7 +135,7 @@ public PendingImportObject constructPendingObservation() { original = observation.getValue(); } - if (!isTimestampMatched()) { + if (!isTimestampMatched() && StringUtils.isNotBlank(timestamp)) { // Update the timestamp DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; String formattedTimeStampValue = formatter.format(observationService.parseDateTime(timestamp)); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java index 770576b67..f2e74982e 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java @@ -284,7 +284,7 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext BrAPIObservation observation = gson.fromJson(gson.toJson(observationByObsHash.get(observationHash)), BrAPIObservation.class); // Is there a change to the prior data? - if (isChanged(cellData, observation, cell.timestamp)) { + if (isChanged(cellData, observation, cell.timestamp, tsColByPheno.containsKey(phenoColumnName))) { // Is prior data protected? /** @@ -394,14 +394,15 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext } } - private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp) { + private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp, boolean timestampColumnPresent) { if (!cellData.isBlank() && !cellData.equals(observation.getValue())){ return true; } - if (StringUtils.isBlank(newTimestamp)) { - return (observation.getObservationTimeStamp()!=null); + // Only check timestamp if the TS: column was present in the uploaded file and there's a valid timestamp. + if (timestampColumnPresent && !StringUtils.isBlank(newTimestamp)) { + return !observationService.parseDateTime(newTimestamp).equals(observation.getObservationTimeStamp()); } - return !observationService.parseDateTime(newTimestamp).equals(observation.getObservationTimeStamp()); + return false; } /** diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java index a6ecac2c0..de98202fa 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java @@ -17,6 +17,7 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.service; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.StringUtils; import org.brapi.v2.model.core.BrAPISeason; @@ -42,6 +43,7 @@ import java.util.*; import java.util.stream.Collectors; +@Slf4j @Singleton public class ObservationService { private final ExperimentUtilities experimentUtilities; @@ -115,6 +117,7 @@ public OffsetDateTime parseDateTime(String dateString) { LocalDate localDate = LocalDate.parse(dateString, formatter); return localDate.atStartOfDay().atOffset(ZoneOffset.UTC); } catch (DateTimeParseException ex) { + log.error("Failed to parse timestamp: \"{}\".", dateString); // If both parsing attempts fail, return null return null; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java index d4d2389a8..491706493 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java @@ -87,6 +87,8 @@ public class GermplasmProcessor implements Processor { List> postOrder = new ArrayList<>(); BrAPIListNewRequest importList = new BrAPIListNewRequest(); + private int numNewPedigreeConnections = 0; + public static String missingGIDsMsg = "The following GIDs were not found in the database: %s"; public static String missingParentalGIDsMsg = "The following parental GIDs were not found in the database: %s"; public static String missingParentalEntryNoMsg = "The following parental entry numbers were not found in the database: %s"; @@ -332,7 +334,7 @@ public Map process(ImportUpload upload, List
breedingMethods, @@ -359,6 +361,10 @@ private void processNewGermplasm(Germplasm germplasm, ValidationErrors validatio validatePedigree(germplasm, i + 2, validationErrors); + if (germplasm.pedigreeExists()) { + numNewPedigreeConnections++; + } + BrAPIGermplasm newGermplasm = germplasm.constructBrAPIGermplasm(program, breedingMethod, user, commit, BRAPI_REFERENCE_SOURCE, nextVal, importListId); newGermplasmList.add(newGermplasm); @@ -383,6 +389,9 @@ private Germplasm removeBreedingMethodBlanks(Germplasm germplasm) { private boolean processExistingGermplasm(Germplasm germplasm, ValidationErrors validationErrors, List importRows, Program program, UUID importListId, boolean commit, PendingImport mappedImportRow, int rowIndex) { BrAPIGermplasm existingGermplasm; String gid = germplasm.getAccessionNumber(); + boolean mutated = false; + boolean updatePedigree = false; + if (germplasmByAccessionNumber.containsKey(gid)) { existingGermplasm = germplasmByAccessionNumber.get(gid).getBrAPIObject(); // Serialize and deserialize to deep copy @@ -408,17 +417,26 @@ private boolean processExistingGermplasm(Germplasm germplasm, ValidationErrors v } } - if(germplasm.pedigreeExists()) { + // if no existing pedigree and file has pedigree then validate and update + if(germplasm.pedigreeExists() && !hasPedigree(existingGermplasm)) { validatePedigree(germplasm, rowIndex + 2, validationErrors); + updatePedigree = true; } - germplasm.updateBrAPIGermplasm(existingGermplasm, program, importListId, commit, true); - - updatedGermplasmList.add(existingGermplasm); - mappedImportRow.setGermplasm(new PendingImportObject<>(ImportObjectState.MUTATED, existingGermplasm)); - importList.addDataItem(existingGermplasm.getGermplasmName()); + mutated = germplasm.updateBrAPIGermplasm(existingGermplasm, program, importListId, commit, updatePedigree); + if (mutated) { + updatedGermplasmList.add(existingGermplasm); + mappedImportRow.setGermplasm(new PendingImportObject<>(ImportObjectState.MUTATED, existingGermplasm)); + if (updatePedigree) { + numNewPedigreeConnections++; + } + } else { + mappedImportRow.setGermplasm(new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm)); + } + // add to list regardless of mutated or not + importList.addDataItem(existingGermplasm.getGermplasmName()); return true; } @@ -521,20 +539,17 @@ private boolean canUpdatePedigreeNoEqualsCheck(BrAPIGermplasm existingGermplasm, germplasm.pedigreeExists(); } - private Map getStatisticsMap(List importRows) { + private Map getStatisticsMap() { ImportPreviewStatistics germplasmStats = ImportPreviewStatistics.builder() .newObjectCount(newGermplasmList.size()) .ignoredObjectCount(germplasmByAccessionNumber.size()) .build(); - //Modified logic here to check for female parent accession number or entry no, removed check for male due to assumption that shouldn't have only male parent - int newObjectCount = newGermplasmList.stream().filter(newGermplasm -> newGermplasm != null).collect(Collectors.toList()).size(); + // TODO: numNewPedigreeConnections is global modified in existing and new flows, refactor at some point ImportPreviewStatistics pedigreeConnectStats = ImportPreviewStatistics.builder() - .newObjectCount(importRows.stream().filter(germplasmImport -> - germplasmImport.getGermplasm() != null && - (germplasmImport.getGermplasm().getFemaleParentAccessionNumber() != null || germplasmImport.getGermplasm().getFemaleParentEntryNo() != null) - ).collect(Collectors.toList()).size()).build(); + .newObjectCount(numNewPedigreeConnections) + .build(); return Map.of( "Germplasm", germplasmStats, @@ -631,10 +646,13 @@ public void postBrapiData(Map mappedBrAPIImport, Program } // Create list - if (!newGermplasmList.isEmpty() || !updatedGermplasmList.isEmpty()) { + // create & update flows both unconditionally add germplasm names to importList so use that for check + if (!importList.getData().isEmpty()) { try { // Create germplasm list brAPIListDAO.createBrAPILists(List.of(importList), program.getId(), upload); + // Now that we have finished uploading, fetch all the data posted to BrAPI to the cache so it is up-to-date. + brAPIGermplasmDAO.repopulateGermplasmCacheForProgram(program.getId()); } catch (ApiException e) { throw new InternalServerException(e.toString(), e); } diff --git a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java index b730660e3..cab44d951 100644 --- a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java +++ b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java @@ -60,6 +60,7 @@ public class BrAPIDAOUtil { private final Duration searchTimeout; private final int pageSize; private final int postGroupSize; + private final int brapiFetchPageSize; private final ProgramService programService; @Inject @@ -67,11 +68,13 @@ public BrAPIDAOUtil(@Property(name = "brapi.search.wait-time") int searchWaitTim @Property(name = "brapi.read-timeout") Duration searchTimeout, @Property(name = "brapi.page-size") int pageSize, @Property(name = "brapi.post-group-size") int postGroupSize, + @Property(name = "brapi.cache.fetch-page-size") int brapiFetchPageSize, ProgramService programService) { this.searchWaitTime = searchWaitTime; this.searchTimeout = searchTimeout; this.pageSize = pageSize; this.postGroupSize = postGroupSize; + this.brapiFetchPageSize = brapiFetchPageSize; this.programService = programService; } @@ -79,26 +82,37 @@ public List search(Funct Function3, Optional>>> searchGetMethod, U searchBody ) throws ApiException { - return searchInternal(searchMethod, searchGetMethod, null, searchBody); + return searchInternal(searchMethod, searchGetMethod, null, searchBody, true); } public List search(Function, Optional>>> searchMethod, Function4, Optional>>> searchGetMethod, U searchBody ) throws ApiException { - return searchInternal(searchMethod, null, searchGetMethod, searchBody); + return searchInternal(searchMethod, null, searchGetMethod, searchBody, true); + } + + public List searchNoPaging(Function, Optional>>> searchMethod, + Function3, Optional>>> searchGetMethod, + U searchBody + ) throws ApiException { + return searchInternal(searchMethod, searchGetMethod, null, searchBody, false); } private List searchInternal(Function, Optional>>> searchMethod, Function3, Optional>>> searchGetMethod, Function4, Optional>>> searchGetMethodWithMimeType, - U searchBody) throws ApiException { + U searchBody, boolean sendPaging) throws ApiException { try { List listResult = new ArrayList<>(); - //NOTE: Because of the way Breedbase implements BrAPI searches, the page size is initially set to an - //arbitrary, large value to ensure that in the event that a 202 response is returned, the searchDbId - //stored will refer to all records of the BrAPI variable. - searchBody.pageSize(10000000); + + if (sendPaging) { + // This should be set to whatever the maximum allowable value is configured in the brapi test server, + // perhaps it should be configurable on bi side as well. + // For reference, that prop name is paging.page-size.max-allowed + searchBody.pageSize(brapiFetchPageSize); + } + ApiResponse, Optional>> response = searchMethod.apply(searchBody); if (response.getBody().getLeft().isPresent()) { BrAPIResponse listResponse = (BrAPIResponse) response.getBody().getLeft().get(); @@ -108,7 +122,7 @@ private List searchInter pagination params are handled for POST search endpoints or the corresponding endpoints in Breedbase are changed or updated */ - if(hasMorePages(listResponse)) { + if(sendPaging && hasMorePages(listResponse)) { int currentPage = listResponse.getMetadata().getPagination().getCurrentPage() + 1; int totalPages = listResponse.getMetadata().getPagination().getTotalPages(); @@ -137,7 +151,7 @@ private List searchInter BrAPIResponse listResponse = (BrAPIResponse) searchGetResponse.getBody().getLeft().get(); listResult = getListResult(searchGetResponse); - if(hasMorePages(listResponse)) { + if(sendPaging && hasMorePages(listResponse)) { currentPage++; int totalPages = listResponse.getMetadata() .getPagination() diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 3a98c3fb6..313540dbd 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -145,7 +145,7 @@ web: logout: url: ${web.base-url} signup: - url-timeout: 60m + url-timeout: 24h signup: url: ${web.base-url}/signup success: @@ -173,6 +173,19 @@ brapi: search: wait-time: 1000 post-group-size: ${POST_CHUNK_SIZE:1000} + paginate: + # These props are used as flippers for specific cache entities to be fetched all at once or paginated. + # If the props are set to true, there's a possibility the BrAPI server could run out of memory grabbing all the + # data and assigning it to entities in memory given a program that has a large amount of those types of entities. + # At that point these flippers should be set to true. + germplasm: ${CACHE_PAGINATE_GERMPLASM:false} + cache: + # This prop sets how many program cache entity records can be fetched at a time from BrAPI. Today, if over 65000 is used, + # a SQL error can happen for programs that have more than this page size, because to fetch these certain entities + # the same amount of IDs must be passed for any collections that need to be fetched in the entity, and SQL has a max of ~65355 params. + # The ProgramCache will iterate through all pages of an entity to retrieve all the data from BrAPI. + fetch-page-size: ${CACHE_BRAPI_FETCH_PAGE_SIZE:65000} + email: relay-server: diff --git a/src/main/resources/brapi/properties/application.properties b/src/main/resources/brapi/properties/application.properties index 28a097f3a..5ee3656c5 100644 --- a/src/main/resources/brapi/properties/application.properties +++ b/src/main/resources/brapi/properties/application.properties @@ -25,14 +25,36 @@ spring.datasource.password=${BRAPI_DB_PASSWORD} spring.datasource.driver-class-name=org.postgresql.Driver -spring.flyway.locations=classpath:db/migration,classpath:db/sql,classpath:org/brapi/test/BrAPITestServer/db/migration -spring.flyway.schemas=public -spring.flyway.baselineOnMigrate=true +# This property when set to true makes it so that a DB transaction is open through the body of a request, nullifying the use of @Transactional. +# It is generally recommended that this be set to false, and methods are properly annotated and release the transactions when complete. +# However, many of the endpoints already rely on this kind of connection infrastructure and changing is more trouble than it's worth. +spring.jpa.open-in-view=true +spring.jpa.properties.hibernate.jdbc.batch_size=50 +spring.jpa.properties.hibernate.order_inserts=true +spring.jpa.properties.hibernate.order_updates=true spring.jpa.hibernate.ddl-auto=validate + +# Use these to help debug queries. +# The stats will tell you how long hibernate transactions are taking, how many queries occur, how many entities are being flushed/accessed, etc. spring.jpa.show-sql=false +spring.jpa.properties.hibernate.generate_statistics=false + +spring.flyway.locations=classpath:db/migration,classpath:db/sql,classpath:org/brapi/test/BrAPITestServer/db/migration +spring.flyway.schemas=public +spring.flyway.baselineOnMigrate=true spring.mvc.dispatch-options-request=true security.oidc_discovery_url=https://example.com/auth/.well-known/openid-configuration security.enabled=false +security.issuer_url=http://example.com/issuerurl + +# This should either be set in accordance with a maximum number of SQL parameters (on JOIN FETCHES of collections, +# if there is more than one collection the IDs of each entity need to be passed through as parameters, and there is a SQL +# maximum of 65535. See GermplasmService.findGermplasmEntities()), +# whatever returns in a reasonable amount of time, +# or if you want to limit for the sake of server efficiency. +paging.page-size.max-allowed=65000 + +paging.page-size.default=1000 diff --git a/src/main/resources/brapi/sql/R__species.sql b/src/main/resources/brapi/sql/R__species.sql index 00c1b7042..ec6ae4c6c 100644 --- a/src/main/resources/brapi/sql/R__species.sql +++ b/src/main/resources/brapi/sql/R__species.sql @@ -13,32 +13,38 @@ -- See the License for the specific language governing permissions and -- limitations under the License. -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '4', 'Blueberry') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '5', 'Salmon') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '6', 'Grape') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '7', 'Alfalfa') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '8', 'Sweet Potato') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '9', 'Trout') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '10', 'Soybean') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '11', 'Cranberry') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '12', 'Cucumber') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '13', 'Oat') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '14', 'Citrus') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '15', 'Sugar Cane') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '16', 'Strawberry') ON CONFLICT DO NOTHING; --- for the Honey Bee case, want to overwrite name, not preserve existing -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '17', 'Honey Bee') ON CONFLICT (id) DO UPDATE SET crop_name = EXCLUDED.crop_name; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '18', 'Pecan') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '19', 'Lettuce') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '20', 'Cotton') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '21', 'Sorghum') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '22', 'Hemp') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '23', 'Hop') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '24', 'Hydrangea') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '25', 'Red Clover') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '26', 'Potato') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '27', 'Blackberry') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '28', 'Raspberry') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '29', 'Sugar Beet') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '30', 'Strawberry') ON CONFLICT DO NOTHING; -INSERT INTO crop (auth_user_id, id, crop_name) VALUES ('anonymousUser', '31', 'Coffee') ON CONFLICT DO NOTHING; \ No newline at end of file +CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; + +DO $$ +DECLARE +v_auth_id CONSTANT uuid := 'AAAAAAAA-AAAA-AAAA-AAAA-AAAAAAAAAAAA'; +BEGIN + /* ------------------------------------------------------------------------------------------ + • uuid_generate_v5(namespace, crop_name) → deterministic UUID can be used for idempotency + • Do it this way so no schema changes are required + • Removed the Honey Bee special case because all systems will be starting fresh + ------------------------------------------------------------------------------------------ */ +INSERT INTO crop (id, auth_user_id, crop_name) +SELECT + uuid_generate_v5('9a4deca9-4068-46a3-9efe-db0c181f491a'::uuid, + -- 1) lower‑case + -- 2) trim leading/trailing space + -- 3) REMOVE every space or tab + regexp_replace(lower(trim(crop_name)), '\s', '', 'g')), + v_auth_id, + crop_name +FROM (VALUES + ('Blueberry'), ('Salmon'), ('Grape'), ('Alfalfa'), + ('Sweet Potato'), ('Trout'), ('Soybean'), ('Cranberry'), + ('Cucumber'), ('Oat'), ('Citrus'), ('Sugar Cane'), + ('Strawberry'), ('Honey Bee'), ('Pecan'), ('Lettuce'), + ('Cotton'), ('Sorghum'), ('Hemp'), ('Hop'), + ('Hydrangea'), ('Red Clover'), ('Potato'), ('Blackberry'), + ('Raspberry'), ('Sugar Beet'), ('Coffee') + ) AS src(crop_name) + ON CONFLICT (id) DO +-- want case changes or space changes to overwrite existing +-- Only rewrite the row if name changed +UPDATE SET crop_name = EXCLUDED.crop_name +WHERE crop.crop_name IS DISTINCT FROM EXCLUDED.crop_name; +END $$; \ No newline at end of file diff --git a/src/main/resources/version.properties b/src/main/resources/version.properties index 6d9227721..bb93e957a 100644 --- a/src/main/resources/version.properties +++ b/src/main/resources/version.properties @@ -15,6 +15,5 @@ # - -version=v1.1.0 -versionInfo=https://github.com/Breeding-Insight/bi-api/releases/tag/v1.1.0 +version=v1.2.0+1031 +versionInfo=https://github.com/Breeding-Insight/bi-api/commit/4497ad61d94168c8c9e44e23698c01f91002599f diff --git a/src/test/java/org/breedinginsight/BrAPITest.java b/src/test/java/org/breedinginsight/BrAPITest.java index e6e6072d6..bdf362979 100644 --- a/src/test/java/org/breedinginsight/BrAPITest.java +++ b/src/test/java/org/breedinginsight/BrAPITest.java @@ -49,7 +49,7 @@ public class BrAPITest extends DatabaseTest { public BrAPITest() { super(); - brapiContainer = new GenericContainer<>("breedinginsight/brapi-java-server:rc") + brapiContainer = new GenericContainer<>("breedinginsight/brapi-java-server:develop") .withNetwork(super.getNetwork()) .withImagePullPolicy(PullPolicy.ageBased(Duration.ofMinutes(60))) .withExposedPorts(8080) @@ -60,6 +60,7 @@ public BrAPITest() { .withEnv("BRAPI_DB", "postgres") .withEnv("BRAPI_DB_USER", "postgres") .withEnv("BRAPI_DB_PASSWORD", "postgres") + .withEnv("SECURITY_ISSUER_URL", "http://example.com/issuerurl") .withClasspathResourceMapping("sql/brapi/mount", "/home/brapi/db/sql", BindMode.READ_WRITE) // HACK - READ_WRITE forces testcontainers to use a bind mount (which overwrites) instead of copying files. .withClasspathResourceMapping("brapi/properties/application.properties", "/home/brapi/properties/application.properties", BindMode.READ_ONLY) .waitingFor(Wait.forLogMessage(".*Started BrapiTestServer in \\d*.\\d* seconds.*", 1).withStartupTimeout(Duration.ofMinutes(1))); diff --git a/src/test/java/org/breedinginsight/DatabaseTest.java b/src/test/java/org/breedinginsight/DatabaseTest.java index b1213ee33..5a1279e7c 100644 --- a/src/test/java/org/breedinginsight/DatabaseTest.java +++ b/src/test/java/org/breedinginsight/DatabaseTest.java @@ -62,7 +62,7 @@ public DatabaseTest() { network = Network.newNetwork(); } if(dbContainer == null) { - dbContainer = new GenericContainer<>("postgres:11.4") + dbContainer = new GenericContainer<>("postgres:17.5") .withNetwork(network) .withNetworkAliases("testdb") .withImagePullPolicy(PullPolicy.defaultPolicy()) diff --git a/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java index 32781dd8c..760974150 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java @@ -25,16 +25,22 @@ import org.breedinginsight.model.Program; import org.breedinginsight.model.Species; import org.breedinginsight.services.SpeciesService; +import org.breedinginsight.utilities.FileUtil; import org.breedinginsight.utilities.response.mappers.GermplasmQueryMapper; import org.jooq.DSLContext; import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import javax.inject.Inject; +import java.io.ByteArrayInputStream; import java.io.File; import java.time.OffsetDateTime; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; +import tech.tablesaw.api.Table; import static io.micronaut.http.HttpRequest.GET; import static io.micronaut.http.HttpRequest.POST; @@ -238,6 +244,38 @@ public void getAllGermplasmListsSuccess() { } } + @ParameterizedTest + @CsvSource(value = {"CSV", "XLSX", "XLS"}) + @SneakyThrows + public void germplasmListExport(String extension) { + String programId = validProgram.getId().toString(); + String germplasmListDbId = fetchGermplasmListDbId(programId); + + // Build the endpoint to get germplasm by germplasm list. + String endpoint = String.format("/programs/%s/germplasm/lists/%s/export?fileExtension=%s", programId, germplasmListDbId, extension); + + // Get germplasm by list. + Flowable> call = client.exchange( + GET(endpoint).cookie(new NettyCookie("phylo-token", "test-registered-user")), byte[].class + ); + + HttpResponse response = call.blockingFirst(); + + assertEquals(HttpStatus.OK, response.getStatus()); + + + ByteArrayInputStream bodyStream = new ByteArrayInputStream(Objects.requireNonNull(response.body())); + + Table download = Table.create(); + if (extension.equals("CSV")) { + download = FileUtil.parseTableFromCsv(bodyStream); + } + if (extension.equals("XLS") || extension.equals("XLSX")) { + download = FileUtil.parseTableFromExcel(bodyStream, 0); + } + int dataSize = download.rowCount(); + assertEquals(3, dataSize, "Wrong number of germplasm were returned"); + } @Test @SneakyThrows public void getAllGermplasmByListSuccess() { diff --git a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java index 2d421d306..108eb2fea 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java @@ -231,7 +231,7 @@ public void deleteListSuccess() { // A DELETE request to the brapi/v2/lists/ endpoint with invalid dbId. Flowable> invalidDeleteCall = client.exchange( - DELETE(String.format("/programs/%s/brapi/v2/lists/%s", program.getId().toString(), "NOT-VALID-DBID")) + DELETE(String.format("/programs/%s/brapi/v2/lists/%s", program.getId().toString(), "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa")) .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class ); diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 975c2b083..7a984072c 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -1172,6 +1172,82 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { } } + /* + Scenario: + - an experiment was created with observations and timestamps + - do a second upload with additional observations for the experiment, but without the timestamp column + - verify the second set of observations get uploaded successfully + */ + @Test + @SneakyThrows + public void importNewObsAfterFirstExpWithObsAndTimestamps() { + log.debug("importNewObsAfterFirstExpWithObsAndTimestamps"); + List traits = importTestUtils.createTraits(2); + Program program = createProgram("Exp with TS and additional Uploads ", "EXTSAU", "EXTSAU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Map newExp = new HashMap<>(); + newExp.put(Columns.GERMPLASM_GID, "1"); + newExp.put(Columns.TEST_CHECK, "T"); + newExp.put(Columns.EXP_TITLE, "Test Exp"); + newExp.put(Columns.EXP_UNIT, "Plot"); + newExp.put(Columns.EXP_TYPE, "Phenotyping"); + newExp.put(Columns.ENV, "New Env"); + newExp.put(Columns.ENV_LOCATION, "Location A"); + newExp.put(Columns.ENV_YEAR, "2025"); + newExp.put(Columns.EXP_UNIT_ID, "a-1"); + newExp.put(Columns.REP_NUM, "1"); + newExp.put(Columns.BLOCK_NUM, "1"); + newExp.put(Columns.ROW, "1"); + newExp.put(Columns.COLUMN, "1"); + newExp.put(traits.get(0).getObservationVariableName(), "1"); + newExp.put("TS:" + traits.get(0).getObservationVariableName(), "2019-12-19T12:14:50Z"); + + // In the first upload, only 1 trait should be present. + List initialTraits = List.of(traits.get(0)); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), initialTraits), null, true, client, program, mappingId, newExperimentWorkflowId); + + BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); + Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); + assertTrue(trialIdXref.isPresent()); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); + + BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); + Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); + assertTrue(ouIdXref.isPresent()); + + Map newObservation = new HashMap<>(); + newObservation.put(Columns.GERMPLASM_GID, "1"); + newObservation.put(Columns.TEST_CHECK, "T"); + newObservation.put(Columns.EXP_TITLE, "Test Exp"); + newObservation.put(Columns.EXP_UNIT, "Plot"); + newObservation.put(Columns.EXP_TYPE, "Phenotyping"); + newObservation.put(Columns.ENV, "New Env"); + newObservation.put(Columns.ENV_LOCATION, "Location A"); + newObservation.put(Columns.ENV_YEAR, "2025"); + newObservation.put(Columns.EXP_UNIT_ID, "a-1"); + newObservation.put(Columns.REP_NUM, "1"); + newObservation.put(Columns.BLOCK_NUM, "1"); + newObservation.put(Columns.ROW, "1"); + newObservation.put(Columns.COLUMN, "1"); + newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); + newObservation.put(traits.get(0).getObservationVariableName(), "1"); + newObservation.put(traits.get(1).getObservationVariableName(), "1"); + + // Send overwrite parameters in request body to allow the append workflow to work normally. + Map userData = Map.of("overwrite", "true", "overwriteReason", "testing"); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), userData, true, client, program, mappingId, appendOverwriteWorkflowId); + + JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); + assertEquals(1, previewRows.size()); + JsonObject row = previewRows.get(0).getAsJsonObject(); + + assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); + assertRowSaved(newObservation, program, traits); + + } + /* Scenario: - Create an experiment with valid observations. diff --git a/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java b/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java index 427ed6d88..d77971817 100644 --- a/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java +++ b/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java @@ -70,7 +70,7 @@ public ApiResponse, Optional