From 5d469a686d0c856ac398b3a1677aabbd5d84d523 Mon Sep 17 00:00:00 2001 From: nickpalladino Date: Tue, 7 Apr 2026 11:07:07 -0400 Subject: [PATCH 1/4] Refinement and test simplification --- AGENTS.md | 54 +++ .../brapi/v2/services/BrAPITrialService.java | 121 +++-- .../services/BrAPITrialServiceUnitTest.java | 420 ++++++++++++++++++ 3 files changed, 566 insertions(+), 29 deletions(-) create mode 100644 AGENTS.md create mode 100644 src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..6a7b49b5a --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,54 @@ +# 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 +- Preferred targeted test command: + `mvn -Dtest=ClassName test --settings settings.xml` +- Full test command: + `mvn test --settings settings.xml` +- Full build without tests: + `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. +- 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. diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java index 92a7ac2db..70c79afa8 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -170,6 +170,7 @@ public DownloadFile exportObservations( List obsVars = new ArrayList<>(); Map> rowByOUId = new HashMap<>(); Map studyByDbId = new HashMap<>(); + Map yearByStudyDbId = new HashMap<>(); Map studyDbIdByOUId = new HashMap<>(); List requestedEnvIds = StringUtils.isNotBlank(params.getEnvironments()) ? new ArrayList<>(Arrays.asList(params.getEnvironments().split(","))) : new ArrayList<>(); @@ -205,6 +206,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); @@ -258,6 +260,7 @@ public DownloadFile exportObservations( obsVars, studyDbIdByOUId, programGermplasmByDbId, + yearByStudyDbId, isSubObs ); @@ -268,7 +271,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)); } } } @@ -363,30 +366,12 @@ public Dataset getDatasetData(Program program, UUID experimentId, UUID datasetId log.debug("fetching observationUnits for dataset: " + datasetId); List 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 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 yearByStudyDbId = getYearByStudyDbIds( + datasetOUs.stream() + .map(BrAPIObservationUnit::getStudyDbId) + .collect(Collectors.toSet()), + program); + addEnvYearToObservationUnits(datasetOUs, yearByStudyDbId); log.debug("fetching dataset variables dataset: " + datasetId); List datasetObsVars = getDatasetObsVars(datasetId.toString(), program); @@ -670,6 +655,7 @@ private void addBrAPIObsToRecords( List obsVars, Map studyDbIdByOUId, Map programGermplasmByDbId, + Map yearByStudyDbId, boolean isSubObs) throws ApiException, DoesNotExistException { Map varByDbId = new HashMap<>(); obsVars.forEach(var -> varByDbId.put(var.getObservationVariableDbId(), var)); @@ -688,7 +674,7 @@ private void addBrAPIObsToRecords( } else { // otherwise make a new row - Map row = createExportRow(experiment, program, ou, studyByDbId, programGermplasmByDbId, isSubObs); + Map row = createExportRow(experiment, program, ou, studyByDbId, programGermplasmByDbId, yearByStudyDbId, isSubObs); addObsVarDataToRow(row, obs, includeTimestamp, var, program); rowByOUId.put(ouId, row); } @@ -801,12 +787,19 @@ 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 Map createExportRow( BrAPITrial experiment, Program program, BrAPIObservationUnit ou, Map studyByDbId, Map programGermplasmByDbId, + Map yearByStudyDbId, boolean isSubEntity) throws ApiException, DoesNotExistException { HashMap row = new HashMap<>(); @@ -818,7 +811,8 @@ private Map 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())); @@ -854,8 +848,11 @@ private Map 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 repLevel = ou.getObservationUnitPosition() @@ -912,6 +909,72 @@ private Map 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 getYearByStudyDbId(Collection studies, UUID programId) throws ApiException, DoesNotExistException { + Map yearByStudyDbId = new HashMap<>(); + Map 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. + */ + private Map getYearByStudyDbIds(Collection studyDbIds, Program program) throws ApiException, DoesNotExistException { + return getYearByStudyDbId( + studyDAO.getStudiesByStudyDbId(studyDbIds, program), + program.getId()); + } + + /** + * Adds the resolved Env Year to each observation unit for the dataset response. + */ + private void addEnvYearToObservationUnits(Collection observationUnits, Map 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("Study DbId '%s' not found.", 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 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(); + yearBySeasonDbId.put(seasonId, year); + } + + return year; + } + private String doubleToString(double val){ return Double.isNaN(val) ? null : String.valueOf( val ); } diff --git a/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java new file mode 100644 index 000000000..0554319ec --- /dev/null +++ b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java @@ -0,0 +1,420 @@ +package org.breedinginsight.brapi.v2.services; + +import com.google.gson.JsonObject; +import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.core.BrAPIListSummary; +import org.brapi.v2.model.core.BrAPISeason; +import org.brapi.v2.model.core.BrAPIStudy; +import org.brapi.v2.model.core.BrAPITrial; +import org.brapi.v2.model.germ.BrAPIGermplasm; +import org.brapi.v2.model.pheno.BrAPIObservation; +import org.brapi.v2.model.pheno.BrAPIObservationUnit; +import org.brapi.v2.model.pheno.BrAPIObservationUnitLevelRelationship; +import org.brapi.v2.model.pheno.BrAPIObservationUnitPosition; +import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; +import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; +import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; +import org.breedinginsight.brapi.v2.dao.BrAPIObservationDAO; +import org.breedinginsight.brapi.v2.dao.BrAPIObservationLevelDAO; +import org.breedinginsight.brapi.v2.dao.BrAPIObservationUnitDAO; +import org.breedinginsight.brapi.v2.dao.BrAPISeasonDAO; +import org.breedinginsight.brapi.v2.dao.BrAPIStudyDAO; +import org.breedinginsight.brapi.v2.dao.BrAPITrialDAO; +import org.breedinginsight.brapi.v2.model.request.query.ExperimentExportQuery; +import org.breedinginsight.brapps.importer.model.exports.FileType; +import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation.Columns; +import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; +import org.breedinginsight.brapps.importer.services.FileMappingUtil; +import org.breedinginsight.brapps.importer.services.processors.experiment.service.DatasetService; +import org.breedinginsight.model.BrAPIConstants; +import org.breedinginsight.model.Dataset; +import org.breedinginsight.model.DownloadFile; +import org.breedinginsight.model.Program; +import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.services.TraitService; +import org.breedinginsight.services.lock.DistributedLockService; +import org.breedinginsight.utilities.FileUtil; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import tech.tablesaw.api.Table; + +import java.io.ByteArrayInputStream; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class BrAPITrialServiceUnitTest { + private static final String REFERENCE_SOURCE = "breedinginsight.org"; + private static final String EXPORT_DATASET_ID = "33333333-3333-3333-3333-333333333333"; + private static final String DATASET_ID = "22222222-2222-2222-2222-222222222222"; + private static final String ENVIRONMENT_ID = "44444444-4444-4444-4444-444444444444"; + private static final String SECOND_ENVIRONMENT_ID = "55555555-5555-5555-5555-555555555555"; + + private final BrAPITrialDAO trialDAO = mock(BrAPITrialDAO.class); + private final BrAPIObservationDAO observationDAO = mock(BrAPIObservationDAO.class); + private final BrAPIObservationUnitDAO observationUnitDAO = mock(BrAPIObservationUnitDAO.class); + private final BrAPIListDAO listDAO = mock(BrAPIListDAO.class); + private final TraitService traitService = mock(TraitService.class); + private final BrAPIStudyDAO studyDAO = mock(BrAPIStudyDAO.class); + private final BrAPISeasonDAO seasonDAO = mock(BrAPISeasonDAO.class); + private final BrAPIObservationLevelDAO observationLevelDAO = mock(BrAPIObservationLevelDAO.class); + private final BrAPIGermplasmDAO germplasmDAO = mock(BrAPIGermplasmDAO.class); + private final FileMappingUtil fileMappingUtil = mock(FileMappingUtil.class); + private final DistributedLockService lockService = mock(DistributedLockService.class); + private final DatasetService datasetService = mock(DatasetService.class); + + private BrAPITrialService service; + private Program program; + private BrAPITrial experiment; + private BrAPIStudy study; + private BrAPIGermplasm germplasm; + private BrAPISeason season; + + @BeforeEach + void setup() { + service = new BrAPITrialService( + REFERENCE_SOURCE, + trialDAO, + observationDAO, + observationUnitDAO, + listDAO, + traitService, + studyDAO, + seasonDAO, + observationUnitDAO, + observationLevelDAO, + germplasmDAO, + fileMappingUtil, + lockService, + datasetService + ); + + program = new Program(); + program.setId(UUID.randomUUID()); + program.setKey("TEST"); + + experiment = new BrAPITrial(); + experiment.setTrialDbId("trial-db-id"); + experiment.setTrialName("Unit Test Experiment"); + experiment.setTrialDescription("Trial description"); + JsonObject experimentInfo = new JsonObject(); + experimentInfo.addProperty(BrAPIAdditionalInfoFields.EXPERIMENT_TYPE, "Phenotyping"); + experiment.setAdditionalInfo(experimentInfo); + + study = new BrAPIStudy(); + study.setStudyDbId("study-1"); + study.setStudyName("Environment 1"); + study.setLocationName("Location 1"); + study.setSeasons(List.of("season-1")); + study.setExternalReferences(List.of(createExternalReference( + String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName()), + ENVIRONMENT_ID + ))); + + germplasm = new BrAPIGermplasm(); + germplasm.setGermplasmDbId("germ-1"); + germplasm.setAccessionNumber("G-1"); + + season = new BrAPISeason(); + season.setSeasonDbId("season-1"); + season.setYear(2023); + } + + @Test + void exportObservationsFetchesSeasonOncePerSeasonAndWritesEnvYear() throws Exception { + ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); + List observationUnits = new ArrayList<>(List.of( + createObservationUnit("ou-db-1", "plot-1"), + createObservationUnit("ou-db-2", "plot-2"), + createObservationUnit("ou-db-3", "plot-3") + )); + + when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) + .thenReturn(List.of(experiment)); + when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) + .thenReturn(List.of(study)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + when(observationUnitDAO.getObservationUnitsForDataset(EXPORT_DATASET_ID, program)).thenReturn(observationUnits); + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); + when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); + when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); + + DownloadFile downloadFile = service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params); + + Table exportTable = FileUtil.parseTableFromCsv(new ByteArrayInputStream(downloadFile.getStreamedFile().getInputStream().readAllBytes())); + assertEquals(3, exportTable.rowCount()); + assertEquals(List.of(2023, 2023, 2023), exportTable.intColumn(Columns.ENV_YEAR).asList()); + verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); + } + + @Test + void exportObservationsFetchesYearsOnlyForRequestedEnvironments() throws Exception { + ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); + setField(params, "environments", ENVIRONMENT_ID); + List observationUnits = List.of(createObservationUnit("ou-db-1", "plot-1")); + BrAPIStudy unrelatedStudy = new BrAPIStudy(); + unrelatedStudy.setStudyDbId("study-2"); + unrelatedStudy.setStudyName("Environment 2"); + unrelatedStudy.setLocationName("Location 2"); + unrelatedStudy.setSeasons(Collections.emptyList()); + unrelatedStudy.setExternalReferences(List.of(createExternalReference( + String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName()), + SECOND_ENVIRONMENT_ID + ))); + + when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) + .thenReturn(List.of(experiment)); + when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) + .thenReturn(List.of(study, unrelatedStudy)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + when(observationUnitDAO.getObservationUnitsForDatasetAndEnvs(EXPORT_DATASET_ID, List.of(ENVIRONMENT_ID), program)).thenReturn(observationUnits); + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); + when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); + when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); + + DownloadFile downloadFile = service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params); + + Table exportTable = FileUtil.parseTableFromCsv(new ByteArrayInputStream(downloadFile.getStreamedFile().getInputStream().readAllBytes())); + assertEquals(1, exportTable.rowCount()); + assertEquals(List.of(2023), exportTable.intColumn(Columns.ENV_YEAR).asList()); + verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); + } + + @Test + void exportObservationsWritesDistinctYearsForMultipleEnvironments() throws Exception { + ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); + BrAPIStudy secondStudy = new BrAPIStudy(); + secondStudy.setStudyDbId("study-2"); + secondStudy.setStudyName("Environment 2"); + secondStudy.setLocationName("Location 2"); + secondStudy.setSeasons(List.of("season-2")); + secondStudy.setExternalReferences(List.of(createExternalReference( + String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName()), + SECOND_ENVIRONMENT_ID + ))); + + BrAPISeason secondSeason = new BrAPISeason(); + secondSeason.setSeasonDbId("season-2"); + secondSeason.setYear(2024); + + List observationUnits = new ArrayList<>(List.of( + createObservationUnit("ou-db-1", "plot-1"), + createObservationUnit("ou-db-2", "plot-2", "study-2", "Environment 2", SECOND_ENVIRONMENT_ID) + )); + + when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) + .thenReturn(List.of(experiment)); + when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) + .thenReturn(List.of(study, secondStudy)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + when(seasonDAO.getSeasonById("season-2", program.getId())).thenReturn(secondSeason); + when(observationUnitDAO.getObservationUnitsForDataset(EXPORT_DATASET_ID, program)).thenReturn(observationUnits); + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); + when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); + when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); + + DownloadFile downloadFile = service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params); + + Table exportTable = FileUtil.parseTableFromCsv(new ByteArrayInputStream(downloadFile.getStreamedFile().getInputStream().readAllBytes())); + assertEquals(2, exportTable.rowCount()); + assertEquals(List.of("Environment 1", "Environment 2"), exportTable.stringColumn(Columns.ENV).asList()); + assertEquals(List.of(2023, 2024), exportTable.intColumn(Columns.ENV_YEAR).asList()); + verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); + verify(seasonDAO, times(1)).getSeasonById("season-2", program.getId()); + } + + @Test + void exportObservationsThrowsWhenSeasonYearIsNull() throws Exception { + // This PR intentionally hardens null season years because experiment import + // requires Env Year when creating experiments/environments. + ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); + List observationUnits = List.of(createObservationUnit("ou-db-1", "plot-1")); + season.setYear(null); + + when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) + .thenReturn(List.of(experiment)); + when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) + .thenReturn(List.of(study)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + when(observationUnitDAO.getObservationUnitsForDataset(EXPORT_DATASET_ID, program)).thenReturn(observationUnits); + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); + when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); + when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); + + DoesNotExistException exception = assertThrows(DoesNotExistException.class, + () -> service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params)); + + assertEquals("Env Year not found for Study DbId = 'study-1'.", exception.getMessage()); + } + + @Test + void getDatasetDataUsesStudyDbIdsAndCachesSeasonByStudy() throws Exception { + List observationUnits = new ArrayList<>(List.of( + createObservationUnit("ou-db-1", "plot-1"), + createObservationUnit("ou-db-2", "plot-2") + )); + + when(observationUnitDAO.getObservationUnitsForDataset(DATASET_ID, program)).thenReturn(observationUnits); + when(studyDAO.getStudiesByStudyDbId(eq(Set.of("study-1")), eq(program))).thenReturn(List.of(study)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); + when(observationDAO.getObservationsByObservationUnitsAndVariables(anyList(), eq(Collections.emptyList()), eq(program))) + .thenReturn(Collections.emptyList()); + + Dataset dataset = service.getDatasetData(program, UUID.randomUUID(), UUID.fromString(DATASET_ID), false); + + assertEquals(2, dataset.observationUnits.size()); + dataset.observationUnits.forEach(ou -> + assertEquals(2023, ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.ENV_YEAR).getAsInt())); + verify(studyDAO, times(1)).getStudiesByStudyDbId(eq(Set.of("study-1")), eq(program)); + verify(studyDAO, never()).getStudyByEnvironmentId(eq(UUID.fromString(ENVIRONMENT_ID)), eq(program)); + verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); + } + + @Test + void getDatasetDataWritesDistinctYearsForMultipleStudies() throws Exception { + BrAPIStudy secondStudy = new BrAPIStudy(); + secondStudy.setStudyDbId("study-2"); + secondStudy.setStudyName("Environment 2"); + secondStudy.setLocationName("Location 2"); + secondStudy.setSeasons(List.of("season-2")); + secondStudy.setExternalReferences(List.of(createExternalReference( + String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName()), + SECOND_ENVIRONMENT_ID + ))); + + BrAPISeason secondSeason = new BrAPISeason(); + secondSeason.setSeasonDbId("season-2"); + secondSeason.setYear(2024); + + List observationUnits = new ArrayList<>(List.of( + createObservationUnit("ou-db-1", "plot-1"), + createObservationUnit("ou-db-2", "plot-2", "study-2", "Environment 2", SECOND_ENVIRONMENT_ID) + )); + + when(observationUnitDAO.getObservationUnitsForDataset(DATASET_ID, program)).thenReturn(observationUnits); + when(studyDAO.getStudiesByStudyDbId(eq(Set.of("study-1", "study-2")), eq(program))).thenReturn(List.of(study, secondStudy)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + when(seasonDAO.getSeasonById("season-2", program.getId())).thenReturn(secondSeason); + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); + when(observationDAO.getObservationsByObservationUnitsAndVariables(anyList(), eq(Collections.emptyList()), eq(program))) + .thenReturn(Collections.emptyList()); + + Dataset dataset = service.getDatasetData(program, UUID.randomUUID(), UUID.fromString(DATASET_ID), false); + + assertEquals(2, dataset.observationUnits.size()); + assertEquals(List.of(2023, 2024), dataset.observationUnits.stream() + .map(ou -> ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.ENV_YEAR).getAsInt()) + .collect(Collectors.toList())); + verify(studyDAO, times(1)).getStudiesByStudyDbId(eq(Set.of("study-1", "study-2")), eq(program)); + verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); + verify(seasonDAO, times(1)).getSeasonById("season-2", program.getId()); + } + + @Test + void getDatasetDataThrowsWhenSeasonYearIsNull() throws Exception { + // Dataset retrieval now shares the same year resolution path as export, so + // unsupported null season years fail consistently across both entry points. + List observationUnits = List.of(createObservationUnit("ou-db-1", "plot-1")); + season.setYear(null); + + when(observationUnitDAO.getObservationUnitsForDataset(DATASET_ID, program)).thenReturn(observationUnits); + when(studyDAO.getStudiesByStudyDbId(eq(Set.of("study-1")), eq(program))).thenReturn(List.of(study)); + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + + DoesNotExistException exception = assertThrows(DoesNotExistException.class, + () -> service.getDatasetData(program, UUID.randomUUID(), UUID.fromString(DATASET_ID), false)); + + assertEquals("Study DbId 'study-1' not found.", exception.getMessage()); + } + + private ExperimentExportQuery exportQuery(String datasetId) throws Exception { + ExperimentExportQuery params = new ExperimentExportQuery(); + setField(params, "datasetId", datasetId); + setField(params, "fileExtension", FileType.CSV); + setField(params, "includeTimestamps", false); + return params; + } + + private BrAPIObservationUnit createObservationUnit(String observationUnitDbId, String observationUnitName) { + return createObservationUnit(observationUnitDbId, observationUnitName, "study-1", "Environment 1", ENVIRONMENT_ID); + } + + private BrAPIObservationUnit createObservationUnit( + String observationUnitDbId, + String observationUnitName, + String studyDbId, + String studyName, + String environmentId) { + BrAPIObservationUnit observationUnit = new BrAPIObservationUnit(); + observationUnit.setObservationUnitDbId(observationUnitDbId); + observationUnit.setObservationUnitName(observationUnitName); + observationUnit.setStudyDbId(studyDbId); + observationUnit.setStudyName(studyName); + observationUnit.setGermplasmDbId("germ-1"); + observationUnit.setGermplasmName("Germplasm 1"); + observationUnit.setExternalReferences(List.of( + createExternalReference( + String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()), + observationUnitDbId + ), + createExternalReference( + String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName()), + environmentId + ))); + observationUnit.setObservationUnitPosition(createObservationUnitPosition()); + return observationUnit; + } + + private BrAPIObservationUnitPosition createObservationUnitPosition() { + BrAPIObservationUnitPosition position = new BrAPIObservationUnitPosition(); + BrAPIObservationUnitLevelRelationship observationLevel = new BrAPIObservationUnitLevelRelationship(); + observationLevel.setLevelName("plot"); + observationLevel.setLevelCode("plot"); + observationLevel.setLevelOrder(1); + position.setObservationLevel(observationLevel); + + BrAPIObservationUnitLevelRelationship repLevel = new BrAPIObservationUnitLevelRelationship(); + repLevel.setLevelName(BrAPIConstants.REPLICATE.getValue()); + repLevel.setLevelCode("1"); + repLevel.setLevelOrder(2); + + BrAPIObservationUnitLevelRelationship blockLevel = new BrAPIObservationUnitLevelRelationship(); + blockLevel.setLevelName(BrAPIConstants.BLOCK.getValue()); + blockLevel.setLevelCode("1"); + blockLevel.setLevelOrder(3); + + position.setObservationLevelRelationships(new ArrayList<>(List.of(repLevel, blockLevel))); + return position; + } + + private BrAPIExternalReference createExternalReference(String source, String id) { + BrAPIExternalReference externalReference = new BrAPIExternalReference(); + externalReference.setReferenceSource(source); + externalReference.setReferenceId(id); + externalReference.setReferenceID(id); + return externalReference; + } + + private void setField(Object target, String fieldName, Object value) throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } +} From 893394a022703998aafe410c39a82274883fe0d6 Mon Sep 17 00:00:00 2001 From: nickpalladino Date: Tue, 21 Apr 2026 11:48:17 -0400 Subject: [PATCH 2/4] Fix test setup and update mvn test command instructions --- AGENTS.md | 23 +++++++++++++++---- .../services/BrAPITrialServiceUnitTest.java | 7 +++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6a7b49b5a..ea7a5070e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,13 +27,28 @@ ## Validation - Use Maven for validation. -- Source envs from .env when running mvn commands +- Source envs from `.env` when running `mvn` commands by using `set -a`, `source .env`, and `set +a`. - Preferred targeted test command: - `mvn -Dtest=ClassName test --settings settings.xml` + ```sh + set -a + source .env + set +a + mvn -Dtest=ClassName test --settings settings.xml + ``` - Full test command: - `mvn test --settings settings.xml` + ```sh + set -a + source .env + set +a + mvn test --settings settings.xml + ``` - Full build without tests: - `mvn clean validate install -D maven.test.skip=true --settings settings.xml` + ```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 diff --git a/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java index 0554319ec..0f6041874 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java @@ -30,6 +30,7 @@ import org.breedinginsight.model.Dataset; import org.breedinginsight.model.DownloadFile; import org.breedinginsight.model.Program; +import org.breedinginsight.model.delta.DeltaEntityFactory; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.services.TraitService; import org.breedinginsight.services.lock.DistributedLockService; @@ -78,6 +79,8 @@ class BrAPITrialServiceUnitTest { private final FileMappingUtil fileMappingUtil = mock(FileMappingUtil.class); private final DistributedLockService lockService = mock(DistributedLockService.class); private final DatasetService datasetService = mock(DatasetService.class); + private final DeltaEntityFactory deltaEntityFactory = mock(DeltaEntityFactory.class); + private final BrAPIObservationLevelService observationLevelService = mock(BrAPIObservationLevelService.class); private BrAPITrialService service; private Program program; @@ -102,7 +105,9 @@ void setup() { germplasmDAO, fileMappingUtil, lockService, - datasetService + datasetService, + deltaEntityFactory, + observationLevelService ); program = new Program(); From 5acfaa66e607c44c2962063ad89f35b16591ec6a Mon Sep 17 00:00:00 2001 From: nickpalladino Date: Fri, 24 Apr 2026 17:09:29 -0400 Subject: [PATCH 3/4] Added more detailed testing guidance --- AGENTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index ea7a5070e..aefcee485 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,12 @@ ## 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`. From 5383d0481de8d1718366c4d7f39a76bd6c29c1bc Mon Sep 17 00:00:00 2001 From: nickpalladino Date: Mon, 27 Apr 2026 16:09:03 -0400 Subject: [PATCH 4/4] Clean up tests --- .../brapi/v2/services/BrAPITrialService.java | 22 +++- .../services/BrAPITrialServiceUnitTest.java | 115 +----------------- 2 files changed, 21 insertions(+), 116 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java index f991b0e0d..bc1ce1ce0 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -963,11 +963,22 @@ private Map getYearByStudyDbId(Collection studies, /** * 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 getYearByStudyDbIds(Collection studyDbIds, Program program) throws ApiException, DoesNotExistException { - return getYearByStudyDbId( - studyDAO.getStudiesByStudyDbId(studyDbIds, program), - program.getId()); + List studies = studyDAO.getStudiesByStudyDbId(studyDbIds, program); + Set 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()); } /** @@ -978,7 +989,7 @@ private void addEnvYearToObservationUnits(Collection obser for (BrAPIObservationUnit observationUnit : observationUnits) { Integer year = yearByStudyDbId.get(observationUnit.getStudyDbId()); if (year == null) { - throw new DoesNotExistException(String.format("Study DbId '%s' not found.", observationUnit.getStudyDbId())); + throw new DoesNotExistException(String.format("Env Year not found for Study DbId = '%s'.", observationUnit.getStudyDbId())); } observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.ENV_YEAR, year); } @@ -1004,6 +1015,9 @@ private Integer getYearForStudy(BrAPIStudy study, UUID programId, Map observationUnits = new ArrayList<>(List.of( - createObservationUnit("ou-db-1", "plot-1"), - createObservationUnit("ou-db-2", "plot-2"), - createObservationUnit("ou-db-3", "plot-3") - )); - - when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) - .thenReturn(List.of(experiment)); - when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) - .thenReturn(List.of(study)); - when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); - when(observationUnitDAO.getObservationUnitsForDataset(EXPORT_DATASET_ID, program)).thenReturn(observationUnits); - when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); - when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); - when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); - - DownloadFile downloadFile = service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params); - - Table exportTable = FileUtil.parseTableFromCsv(new ByteArrayInputStream(downloadFile.getStreamedFile().getInputStream().readAllBytes())); - assertEquals(3, exportTable.rowCount()); - assertEquals(List.of(2023, 2023, 2023), exportTable.intColumn(Columns.ENV_YEAR).asList()); - verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); - } - - @Test - void exportObservationsFetchesYearsOnlyForRequestedEnvironments() throws Exception { - ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); - setField(params, "environments", ENVIRONMENT_ID); - List observationUnits = List.of(createObservationUnit("ou-db-1", "plot-1")); - BrAPIStudy unrelatedStudy = new BrAPIStudy(); - unrelatedStudy.setStudyDbId("study-2"); - unrelatedStudy.setStudyName("Environment 2"); - unrelatedStudy.setLocationName("Location 2"); - unrelatedStudy.setSeasons(Collections.emptyList()); - unrelatedStudy.setExternalReferences(List.of(createExternalReference( - String.format("%s/%s", REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName()), - SECOND_ENVIRONMENT_ID - ))); - - when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) - .thenReturn(List.of(experiment)); - when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) - .thenReturn(List.of(study, unrelatedStudy)); - when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); - when(observationUnitDAO.getObservationUnitsForDatasetAndEnvs(EXPORT_DATASET_ID, List.of(ENVIRONMENT_ID), program)).thenReturn(observationUnits); - when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); - when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); - when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); - - DownloadFile downloadFile = service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params); - - Table exportTable = FileUtil.parseTableFromCsv(new ByteArrayInputStream(downloadFile.getStreamedFile().getInputStream().readAllBytes())); - assertEquals(1, exportTable.rowCount()); - assertEquals(List.of(2023), exportTable.intColumn(Columns.ENV_YEAR).asList()); - verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); - } - - @Test - void exportObservationsWritesDistinctYearsForMultipleEnvironments() throws Exception { + void exportObservationsFetchesSeasonOncePerDistinctSeasonAndWritesEnvYears() throws Exception { ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); BrAPIStudy secondStudy = new BrAPIStudy(); secondStudy.setStudyDbId("study-2"); @@ -245,55 +184,7 @@ void exportObservationsWritesDistinctYearsForMultipleEnvironments() throws Excep } @Test - void exportObservationsThrowsWhenSeasonYearIsNull() throws Exception { - // This PR intentionally hardens null season years because experiment import - // requires Env Year when creating experiments/environments. - ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); - List observationUnits = List.of(createObservationUnit("ou-db-1", "plot-1")); - season.setYear(null); - - when(trialDAO.getTrialsByExperimentIds(eq(List.of(UUID.fromString("11111111-1111-1111-1111-111111111111"))), eq(program))) - .thenReturn(List.of(experiment)); - when(studyDAO.getStudiesByExperimentID(eq(UUID.fromString("11111111-1111-1111-1111-111111111111")), eq(program))) - .thenReturn(List.of(study)); - when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); - when(observationUnitDAO.getObservationUnitsForDataset(EXPORT_DATASET_ID, program)).thenReturn(observationUnits); - when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); - when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))).thenReturn(Collections.emptyList()); - when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))).thenReturn(List.of(germplasm)); - - DoesNotExistException exception = assertThrows(DoesNotExistException.class, - () -> service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params)); - - assertEquals("Env Year not found for Study DbId = 'study-1'.", exception.getMessage()); - } - - @Test - void getDatasetDataUsesStudyDbIdsAndCachesSeasonByStudy() throws Exception { - List observationUnits = new ArrayList<>(List.of( - createObservationUnit("ou-db-1", "plot-1"), - createObservationUnit("ou-db-2", "plot-2") - )); - - when(observationUnitDAO.getObservationUnitsForDataset(DATASET_ID, program)).thenReturn(observationUnits); - when(studyDAO.getStudiesByStudyDbId(eq(Set.of("study-1")), eq(program))).thenReturn(List.of(study)); - when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); - when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())).thenReturn(Collections.emptyList()); - when(observationDAO.getObservationsByObservationUnitsAndVariables(anyList(), eq(Collections.emptyList()), eq(program))) - .thenReturn(Collections.emptyList()); - - Dataset dataset = service.getDatasetData(program, UUID.randomUUID(), UUID.fromString(DATASET_ID), false); - - assertEquals(2, dataset.observationUnits.size()); - dataset.observationUnits.forEach(ou -> - assertEquals(2023, ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.ENV_YEAR).getAsInt())); - verify(studyDAO, times(1)).getStudiesByStudyDbId(eq(Set.of("study-1")), eq(program)); - verify(studyDAO, never()).getStudyByEnvironmentId(eq(UUID.fromString(ENVIRONMENT_ID)), eq(program)); - verify(seasonDAO, times(1)).getSeasonById("season-1", program.getId()); - } - - @Test - void getDatasetDataWritesDistinctYearsForMultipleStudies() throws Exception { + void getDatasetDataFetchesSeasonOncePerDistinctSeasonAndWritesEnvYears() throws Exception { BrAPIStudy secondStudy = new BrAPIStudy(); secondStudy.setStudyDbId("study-2"); secondStudy.setStudyName("Environment 2"); @@ -346,7 +237,7 @@ void getDatasetDataThrowsWhenSeasonYearIsNull() throws Exception { DoesNotExistException exception = assertThrows(DoesNotExistException.class, () -> service.getDatasetData(program, UUID.randomUUID(), UUID.fromString(DATASET_ID), false)); - assertEquals("Study DbId 'study-1' not found.", exception.getMessage()); + assertEquals("Env Year not found for Study DbId = 'study-1'.", exception.getMessage()); } private ExperimentExportQuery exportQuery(String datasetId) throws Exception {