BI-2801 - Connection Pool Error while trying to download sub-entity file

AGENTS.md

@@ -0,0 +1,75 @@
+# AGENTS.md
+
+## Scope
+- These instructions apply to the whole repository.
+- If a subdirectory contains its own `AGENTS.md`, the more specific file wins for that subtree.
+
+## Repository Overview
+- Java 13 Maven backend using Micronaut.
+- Main application code: `src/main/java/org/breedinginsight`
+- Tests: `src/test/java/org/breedinginsight`
+- DB migrations: `src/main/resources/db/migration`
+- API docs and related materials: `docs`
+- Build config: `pom.xml`
+- Docker and local setup: `README.md`, `docker-compose.yml`
+
+## Working Rules
+- Keep changes narrow and task-focused.
+- Prefer fixing root causes over adding one-off workarounds.
+- Do not edit `.env`, `.env.test`, `.env.template`, or other secret-bearing config files unless explicitly asked.
+- Do not modify `target/`, `.idea/`, or the large SQL dump files unless the task is specifically about them.
+- Scope searches to relevant directories instead of scanning the entire repository when possible.
+- Do not edit historical Flyway migrations unless explicitly requested. Add a new migration instead.
+
+## Files To Ignore By Default
+- `target/`
+- `.idea/`
+
+## Validation
+- Use Maven for validation.
+- Source envs from `.env` when running `mvn` commands by using `set -a`, `source .env`, and `set +a`.
+- Preferred targeted test command:
+  ```sh
+  set -a
+  source .env
+  set +a
+  mvn -Dtest=ClassName test --settings settings.xml
+  ```
+- Full test command:
+  ```sh
+  set -a
+  source .env
+  set +a
+  mvn test --settings settings.xml
+  ```
+- Full build without tests:
+  ```sh
+  set -a
+  source .env
+  set +a
+  mvn clean validate install -D maven.test.skip=true --settings settings.xml
+  ```
+- Tests may require Docker, Testcontainers, and local services. If validation cannot run, say exactly what blocked it.
+
+## API Change Rules
+- If endpoint behavior changes, do not update the relevant API docs or spec files in `docs`, they are no longer being maintained.
+- If API code changes, add or update endpoint or integration tests.
+
+## Database Rules
+- Schema changes belong in a new Flyway migration under `src/main/resources/db/migration`.
+- If schema changes affect generated jOOQ code, run the documented generation flow.
+
+## Testing Guidance
+- Prefer targeted tests for the changed area before suggesting a full test run.
+- Prefer high-signal tests over broad or repetitive test coverage.
+- Do not add unit tests by default for straightforward changes.
+- Add unit tests selectively for hard-to-reproduce bugs, complex logic regressions, or behavior that is difficult or expensive to validate through integration tests.
+- Prefer integration or endpoint tests for user-facing and API behavior changes.
+- Avoid tests that mainly lock in implementation details or create disproportionate maintenance burden.
+- When adding regression coverage, prefer the smallest number of tests that gives confidence in the fix.
+- For controller or API changes, look first under `src/test/java/org/breedinginsight/api` and `src/test/java/org/breedinginsight/brapi`.
+- For importer work, check `src/test/java/org/breedinginsight/brapps/importer`.
+
+## Response Expectations
+- Summarize changed files, validation performed, and any remaining risks.
+- If tests or docs updates were skipped, explain why.

src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java

@@ -176,6 +176,7 @@ public DownloadFile exportObservations(
         List<Trait> obsVars = new ArrayList<>();
         Map<String, Map<String, Object>> rowByOUId = new HashMap<>();
         Map<String, BrAPIStudy> studyByDbId = new HashMap<>();
+        Map<String, Integer> yearByStudyDbId = new HashMap<>();
         Map<String, String> studyDbIdByOUId = new HashMap<>();
         List<String> requestedEnvIds = StringUtils.isNotBlank(params.getEnvironments()) ?
                 new ArrayList<>(Arrays.asList(params.getEnvironments().split(","))) : new ArrayList<>();
@@ -211,6 +212,7 @@ public DownloadFile exportObservations(
             log.error(logHash + ": Error fetching observation units for a study by its DbId" +
                     Utilities.generateApiExceptionLogMessage(err), err);
         }
+        yearByStudyDbId.putAll(getYearByStudyDbId(expStudies, program.getId()));
 
         boolean isSubObs = isSubEntityDataset(ous);
 
@@ -264,6 +266,7 @@ public DownloadFile exportObservations(
                 obsVars,
                 studyDbIdByOUId,
                 programGermplasmByDbId,
+                yearByStudyDbId,
                 isSubObs
         );
 
@@ -274,7 +277,7 @@ public DownloadFile exportObservations(
                 // Map Observation Unit to the Study it belongs to.
                 studyDbIdByOUId.put(ouId, ou.getStudyDbId());
                 if (!rowByOUId.containsKey(ouId)) {
-                    rowByOUId.put(ouId, createExportRow(experiment, program, ou, studyByDbId, programGermplasmByDbId, isSubObs));
+                    rowByOUId.put(ouId, createExportRow(experiment, program, ou, studyByDbId, programGermplasmByDbId, yearByStudyDbId, isSubObs));
                 }
             }
         }
@@ -369,30 +372,12 @@ public Dataset getDatasetData(Program program, UUID experimentId, UUID datasetId
         log.debug("fetching observationUnits for dataset: " + datasetId);
         List<BrAPIObservationUnit> datasetOUs = ouDAO.getObservationUnitsForDataset(datasetId.toString(), program);
 
-        //Add years to the addition_info elements
-        //TODO yearByStudyDbId will no longer be needed, and should be removed, once the seasonDAO uses the redis cache (BI-2261).
-        Map<String, Integer> yearByStudyDbId = new HashMap<>();  // used to prevent the same season from being fetched repeatedly.
-        for ( BrAPIObservationUnit ou: datasetOUs ) {
-            String environmentId = Utilities.getExternalReference(ou.getExternalReferences(), this.referenceSource, ExternalReferenceSource.STUDIES)
-                    .orElseThrow( ()-> new DoesNotExistException("No BI external reference for STUDIES was found"))
-                    .getReferenceId();
-            if( !yearByStudyDbId.containsKey( environmentId ))  {
-                // Get the Study and extract the year from its Season
-                BrAPIStudy study = studyDAO.getStudyByEnvironmentId(UUID.fromString(environmentId), program).orElseThrow( () -> new DoesNotExistException(String.format("Study Id '%s' not found.", environmentId)) );
-                if(study.getSeasons().isEmpty()){
-                    throw new DoesNotExistException(String.format("No Seasons found in Study Id = '%s'.", environmentId));
-                }
-                String seasonId = study.getSeasons().get(0);
-                BrAPISeason season = seasonDAO.getSeasonById(seasonId, program.getId());
-                if(season==null){
-                    throw new DoesNotExistException(String.format("Seasons not found for Id = '%s'.", seasonId));
-                }
-                Integer year = season.getYear();
-                yearByStudyDbId.put(environmentId, year);
-            }
-
-            ou.putAdditionalInfoItem(BrAPIAdditionalInfoFields.ENV_YEAR, yearByStudyDbId.get(environmentId));
-        }
+        Map<String, Integer> yearByStudyDbId = getYearByStudyDbIds(
+                datasetOUs.stream()
+                        .map(BrAPIObservationUnit::getStudyDbId)
+                        .collect(Collectors.toSet()),
+                program);
+        addEnvYearToObservationUnits(datasetOUs, yearByStudyDbId);
 
         log.debug("fetching dataset variables dataset: " + datasetId);
         List<Trait> datasetObsVars = getDatasetObsVars(datasetId.toString(), program);
@@ -699,6 +684,7 @@ private void addBrAPIObsToRecords(
             List<Trait> obsVars,
             Map<String, String> studyDbIdByOUId,
             Map<String, BrAPIGermplasm> programGermplasmByDbId,
+            Map<String, Integer> yearByStudyDbId,
             boolean isSubObs) throws ApiException, DoesNotExistException {
         Map<String, Trait> varByDbId = new HashMap<>();
         obsVars.forEach(var -> varByDbId.put(var.getObservationVariableDbId(), var));
@@ -717,7 +703,7 @@ private void addBrAPIObsToRecords(
             } else {
 
                 // otherwise make a new row
-                Map<String, Object> row = createExportRow(experiment, program, ou, studyByDbId, programGermplasmByDbId, isSubObs);
+                Map<String, Object> row = createExportRow(experiment, program, ou, studyByDbId, programGermplasmByDbId, yearByStudyDbId, isSubObs);
                 addObsVarDataToRow(row, obs, includeTimestamp, var, program);
                 rowByOUId.put(ouId, row);
             }
@@ -830,6 +816,12 @@ public int deleteExperiment(Program program, UUID experimentId, boolean hard) th
         return existingObservations.size();
     }
 
+    /**
+     * Builds the static export columns for a single observation unit.
+     *
+     * Env Year is resolved ahead of time and passed in so export generation and
+     * dataset retrieval share the same study/season lookup path
+     */
     private List<String> getProgramObservationLevelNames(Program program) {
         String programDbId = program.getBrapiProgram() != null ? program.getBrapiProgram().getProgramDbId() : null;
         List<BrAPIObservationUnitHierarchyLevel> levelNames = observationLevelService.getProgrammaticLevelNames(program, programDbId);
@@ -842,6 +834,7 @@ private Map<String, Object> createExportRow(
             BrAPIObservationUnit ou,
             Map<String, BrAPIStudy> studyByDbId,
             Map<String, BrAPIGermplasm> programGermplasmByDbId,
+            Map<String, Integer> yearByStudyDbId,
             boolean isSubEntity) throws ApiException, DoesNotExistException {
         HashMap<String, Object> row = new HashMap<>();
 
@@ -853,7 +846,8 @@ private Map<String, Object> createExportRow(
         String ouId = ouXref.getReferenceID();
         BrAPIGermplasm germplasm = Optional.ofNullable(programGermplasmByDbId.get(ou.getGermplasmDbId()))
                 .orElseThrow(() -> new DoesNotExistException("Germplasm not returned from BrAPI service"));
-        BrAPIStudy study = studyByDbId.get(ou.getStudyDbId());
+        BrAPIStudy study = Optional.ofNullable(studyByDbId.get(ou.getStudyDbId()))
+                .orElseThrow(() -> new DoesNotExistException(String.format("Study DbId '%s' not found.", ou.getStudyDbId())));
 
         // make export row from BrAPI objects
         row.put(ExperimentObservation.Columns.GERMPLASM_NAME, Utilities.removeProgramKey(ou.getGermplasmName(), program.getKey(), germplasm.getAccessionNumber()));
@@ -889,8 +883,11 @@ private Map<String, Object> createExportRow(
                         :   additionalInfo.get(BrAPIAdditionalInfoFields.RTK).getAsString();
         row.put(ExperimentObservation.Columns.RTK, rtk);
 
-        BrAPISeason season = seasonDAO.getSeasonById(study.getSeasons().get(0), program.getId());
-        row.put(ExperimentObservation.Columns.ENV_YEAR, season.getYear());
+        // Treat a null season year as missing data. Experiment import requires Env Year,
+        // so a missing year here indicates unsupported upstream BrAPI data.
+        Integer year = Optional.ofNullable(yearByStudyDbId.get(study.getStudyDbId()))
+                .orElseThrow(() -> new DoesNotExistException(String.format("Env Year not found for Study DbId = '%s'.", study.getStudyDbId())));
+        row.put(ExperimentObservation.Columns.ENV_YEAR, year);
 
         // get replicate number
         Optional<BrAPIObservationUnitLevelRelationship> repLevel = ou.getObservationUnitPosition()
@@ -947,6 +944,86 @@ private Map<String, Object> createExportRow(
         return row;
     }
 
+    /**
+     * Resolves Env Year once per study while caching repeated season lookups by season DbId.
+     *
+     * This keeps export and dataset retrieval on the same code path and avoids refetching
+     * a season for every observation unit in the same study.
+     */
+    private Map<String, Integer> getYearByStudyDbId(Collection<BrAPIStudy> studies, UUID programId) throws ApiException, DoesNotExistException {
+        Map<String, Integer> yearByStudyDbId = new HashMap<>();
+        Map<String, Integer> yearBySeasonDbId = new HashMap<>();
+        for (BrAPIStudy study : studies) {
+            yearByStudyDbId.put(study.getStudyDbId(), getYearForStudy(study, programId, yearBySeasonDbId));
+        }
+
+        return yearByStudyDbId;
+    }
+
+    /**
+     * Bulk-loads studies by studyDbId so dataset retrieval can avoid per-observation-unit
+     * environment lookups and reuse the shared year resolution flow.
+     *
+     * Missing studies are rejected here so later Env Year writes can reserve their
+     * failures for actual year resolution problems.
+     */
+    private Map<String, Integer> getYearByStudyDbIds(Collection<String> studyDbIds, Program program) throws ApiException, DoesNotExistException {
+        List<BrAPIStudy> studies = studyDAO.getStudiesByStudyDbId(studyDbIds, program);
+        Set<String> resolvedStudyDbIds = studies.stream()
+                .map(BrAPIStudy::getStudyDbId)
+                .collect(Collectors.toSet());
+        for (String studyDbId : studyDbIds) {
+            if (!resolvedStudyDbIds.contains(studyDbId)) {
+                throw new DoesNotExistException(String.format("Study DbId '%s' not found.", studyDbId));
+            }
+        }
+
+        return getYearByStudyDbId(studies, program.getId());
+    }
+
+    /**
+     * Adds the resolved Env Year to each observation unit for the dataset response.
+     */
+    private void addEnvYearToObservationUnits(Collection<BrAPIObservationUnit> observationUnits, Map<String, Integer> yearByStudyDbId) throws DoesNotExistException {
+        // additionalinfo.envYear is for the frontend dataset view
+        for (BrAPIObservationUnit observationUnit : observationUnits) {
+            Integer year = yearByStudyDbId.get(observationUnit.getStudyDbId());
+            if (year == null) {
+                throw new DoesNotExistException(String.format("Env Year not found for Study DbId = '%s'.", observationUnit.getStudyDbId()));
+            }
+            observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.ENV_YEAR, year);
+        }
+    }
+
+    /**
+     * Resolves the year for the study's first season reference.
+     *
+     * A season that exists but does not expose a numeric year is treated as invalid data.
+     * The experiment import workflow requires Env Year for created experiments/environments,
+     * so the stricter failure here is intentional
+     */
+    private Integer getYearForStudy(BrAPIStudy study, UUID programId, Map<String, Integer> yearBySeasonDbId) throws ApiException, DoesNotExistException {
+        if (study.getSeasons() == null || study.getSeasons().isEmpty()) {
+            throw new DoesNotExistException(String.format("No Seasons found in Study DbId = '%s'.", study.getStudyDbId()));
+        }
+
+        String seasonId = study.getSeasons().get(0);
+        Integer year = yearBySeasonDbId.get(seasonId);
+        if (year == null) {
+            BrAPISeason season = seasonDAO.getSeasonById(seasonId, programId);
+            if (season == null) {
+                throw new DoesNotExistException(String.format("Seasons not found for Id = '%s'.", seasonId));
+            }
+            year = season.getYear();
+            if (year == null) {
+                throw new DoesNotExistException(String.format("Env Year not found for Study DbId = '%s'.", study.getStudyDbId()));
+            }
+            yearBySeasonDbId.put(seasonId, year);
+        }
+
+        return year;
+    }
+
     private String doubleToString(double val){
         return Double.isNaN(val) ? null : String.valueOf( val );
     }