[BI-2806] Remove trials cache by jloux-brapi · Pull Request #516 · Breeding-Insight/bi-api

jloux-brapi · 2026-05-06T20:27:36Z

Description

Story: BI-2806

The body of changes in this MR effectively removes the cache implementation for BrAPITrials in bi-api.

There's quite a lot in here, so I'll try to sum up the important areas:

BrAPITrialDAOImpl was edited to

Retrieve trials by ID via brapi trialDbId
getTrialMethods updated utilize above method
createTrials uses brapi post with no update to the cache
updateTrial uses brapi put with no update to the cache
deleteTrial uses brapi delete with no update to the cache
repopulateCache method and usages deleted
fetch method deleted which grabbed all trials for a program from brapi whenever post, put, delete was made
Trial cache removed.

Various entities in need of relation to a trial were updated to use the bi-api generated trial ex ref id from the trial itself like Study, List. These can be changed after the cache is removed

Fixed some instances where certain controllers automatically updated the BrAPITrialDbId in the entity to the bi generated id, which was breaking compatability.

Unit tests were updated to support the changes. Many of these unit tests depended on the bi-api generated uuid from the workflow response to pass through to methods which validated everything. This doesn't really work with the cache gone for trials because now it needs to validate on the trialDbId for the created record. Tests have been updated to grab these IDs from the brapiDb once the workflows complete.

Dependencies

bi-web

Testing

Regression on just about anything Experiment related, including:

Experiment creation
Experiment deletion
Experiment updates
Experiment Collaborator checks
Experiment viewing
Dataset viewing
Dataset creation
Dataset appending

Verify there are no log messages related to caching trials for a program by looking at logs once above tests have been completed.

Checklist:

I have performed a self-review of my own code
I have tested my code and ensured it meets the acceptance criteria of the story
I have create/modified unit and/or integration tests to cover this change or tests are not applicable
I have commented my code, particularly in hard-to-understand areas
I have either updated the source of truth or arranged for update with product owner if needed https://breedinginsight.atlassian.net/wiki/spaces/BI/pages/1559953409/Source+of+Truth

jloux-brapi · 2026-05-06T20:43:03Z

+        return getTrialsFromBrAPI(program, trialQueryParams);
    }

-    private Map<String, BrAPITrial> fetchProgramExperiments(UUID programId) throws ApiException {


Don't really love the way github is presenting this.

fetchProgramExperiments() is essentially what was called by the trialCache every time it was updated.

This method was removed completely along with the cache.

The "change" to this method is not really a change at all, this is a brand new method specifically for grabbing trials from BrAPI.

jloux-brapi · 2026-05-06T20:44:17Z

-        return trials;
-    }
-
-    private List<BrAPITrial> getTrialsByExRef(String referenceSource, String referenceId, Program program) throws ApiException {


This method was unused and removed.

jloux-brapi · 2026-05-06T21:09:05Z

    }

    public List<BrAPITrial> getExperiments(UUID programId) throws ApiException, DoesNotExistException {
+        // TODO: Edit this to filter/paginate trials


This will be important for BI-2861

jloux-brapi · 2026-05-06T21:13:42Z

        // get requested environments for the experiment
        log.debug(logHash + ": fetching environments for export");
-        List<BrAPIStudy> expStudies = studyDAO.getStudiesByExperimentID(experimentId, program);
+        List<BrAPIStudy> expStudies = studyDAO.getStudiesByExperimentID(UUID.fromString(expExRefId), program);


Study lookup will still occur on BI generated ID stored in exrefs. We can change this in this epic after we finish removing the cache for other entities. We will run into similar changes updating other entities in the process of removing the cache I'm sure.

Link to epic just points to this PR

jloux-brapi · 2026-05-06T21:14:26Z

            // Make request to delete experiment.
            trialDAO.deleteBrAPITrial(program, trial, hard);
            // Get all lists for the trial.
+            // TODO: Get lists by trialDbId if trials get decoupled from datasets.


Lists still require the BI-API generated ID as well.

jloux-brapi · 2026-05-06T21:15:37Z

                                 .orElseThrow(() -> new IllegalStateException("No BI external reference found"))
                                 .getReferenceID());

-        study.trialDbId(Utilities.getExternalReference(study.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))


This is kind of a get ahead of the curve change. It might be able to remain as is, but might be tough to find later on when we use trialDbIds directly for studies.

Ya probably ok for poc but could cause confusing issues mixing deltabreed UUIDs and BrAPI UUIDs.

jloux-brapi · 2026-05-06T21:16:24Z


+    // TODO: Remove for trialDbId once cache is removed for that entity
    private void setDbIds(BrAPITrial trial) {
-        trial.trialDbId(Utilities.getExternalReference(trial.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))


This was preventing the frontend effectively from being able to use brapiTrialDbIds in the router. Bit of a nasty bugger. Expecting more changes like this in the future for other entities.

Another thing I think we'll need to resolve is having inconsistent UUIDs like using the BrAPI trialDbId here but then the DeltaBreed program UUID below.

jloux-brapi · 2026-05-06T21:17:49Z

                        .getBrAPIObject()
                        .setTrialDbId(createdTrial.getTrialDbId());
+                // TODO: May need to set the main ID for pending objects with the BrAPI ID for downstream usage once cache is removed.
+                trialByNameNoScope.get(createdTrialName)


I'm pretty sure we can live without this change. Originally I made this change to see if it would help the unit tests pick up the dbId in the workflow responses but to no avail.

jloux-brapi · 2026-05-06T21:21:07Z

-                        String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName()))
-                .orElseThrow(() -> new InternalServerException("An Experiment ID was not found in any of the external references"));
-        UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId());
+        UUID experimentId = UUID.fromString(existingTrial.getTrialDbId());


Originating caller of this method is never used. We might want to do away with this whole call stack at this point

jloux-brapi · 2026-05-06T21:36:31Z

I've tagged all developers to review this code since it's essentially the basis of changes that will need to be made throughout 1.5. This will be good for exposure to get familiar with this kind of code as we make changes to it.

davedrp

Nicely documented. Thanks

jloux-brapi · 2026-05-15T15:58:14Z

I've added the on hold label to this MR. This should not be merged until this label is removed.

HMS17

Looks good and seconding the nicely commented! Approved contingent on the todos getting cards associated with them before merging.

humsika

Everything looks good to me.

jloux-brapi · 2026-06-10T16:22:13Z

@HMS17 I updated relevant TODO comments in this PR with ticket numbers in the other one which will be merged into this one eventually: #521

nickpalladino

Nice work, few comments, more thoughts than anything actionable other than possible future work. Do we already have cards to address the bi-api brapi endpoints like /observationunits?trialDbId. Currently that will use the passed in trialDbId for an exref filter in the brapi search but the /trials endpoint will return the trialDbId now instead of the deltabreed UUID so if that is used to retrieve the id the id won't match.

nickpalladino · 2026-05-29T19:14:17Z


+    // TODO: Remove for trialDbId once cache is removed for that entity
    private void setDbIds(BrAPITrial trial) {
-        trial.trialDbId(Utilities.getExternalReference(trial.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))


Another thing I think we'll need to resolve is having inconsistent UUIDs like using the BrAPI trialDbId here but then the DeltaBreed program UUID below.

nickpalladino · 2026-05-29T19:18:35Z

                                 .orElseThrow(() -> new IllegalStateException("No BI external reference found"))
                                 .getReferenceID());

-        study.trialDbId(Utilities.getExternalReference(study.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))


Ya probably ok for poc but could cause confusing issues mixing deltabreed UUIDs and BrAPI UUIDs.

nickpalladino · 2026-06-17T18:22:08Z

+            brapiProgramDbId = programDAO.getProgramBrAPI(program).getProgramDbId();
        }
+
+        // TODO: Configurable max amount of trials per program, or paginate.


I see this is being handled in BI-2932

nickpalladino · 2026-06-17T18:26:52Z

    }

    @Override
    public List<BrAPITrial> getTrialsByName(List<String> trialNames, Program program) throws ApiException {


We could probably add a card to update this to use a brapi search instead as a low priority

This was done via BI-2861

nickpalladino · 2026-06-17T18:31:21Z

    @Inject
-    public BrAPITrialDAOImpl(ProgramCacheProvider programCacheProvider,
-                             ProgramDAO programDAO,
+    public BrAPITrialDAOImpl(ProgramDAO programDAO,


Was kind of thinking we might be able to create a non-cache implementation of the DAO interface but see the interface changed and will have different assumptions about ids so probably not.

I didn't feel the need to do that because there didn't seem a need to keep the cache implementation version around

jloux-brapi · 2026-06-22T21:43:20Z

@nickpalladino

Do we already have cards to address the bi-api brapi endpoints like /observationunits?trialDbId.
Regarding this, no, there's no direct card created. We an make one, but I figured it might be covered as part of a spike in the observationunits cache removal.

Because it's related to the trial cache it might make more sense to make a bugfix for it.

jloux-brapi added 13 commits April 22, 2026 16:13

Add todos

a04b36e

Merge branch 'develop' into feature/BI-2806

b8444e9

Remove unused method from BrAPITrialDAO

df7affd

Fix first get method

d1ae3d9

Update getTrials(UUID) method to use brapi directly

c1b0174

Udpate remaining getMethods, fix enough exRef things to pass unit tests

6b6a695

Remove code setting brapiId to exRef

bb30594

Merge branch 'develop' into fix_exrefs

a36229b

Changes to support ExperimentControllerIntegrationTest

35cde9c

Fix remaining test failures and alter some code to support dbIds

af7ceda

Remove cache from last get method

6854606

Remove cache from trial update

1f18377

Remove cache from post method and remove entirely for trials

c2083d8

github-actions Bot added the feature label May 6, 2026

jloux-brapi commented May 6, 2026

View reviewed changes

jloux-brapi mentioned this pull request May 6, 2026

[BI-2806] Route Experiments using trialDbId, use trialDbId to get metadata Breeding-Insight/bi-web#466

Merged

5 tasks

jloux-brapi requested a review from nickpalladino May 6, 2026 21:33

jloux-brapi marked this pull request as ready for review May 6, 2026 21:33

jloux-brapi requested review from HMS17, davedrp and humsika May 6, 2026 21:34

davedrp approved these changes May 14, 2026

View reviewed changes

jloux-brapi added the on hold Do not merge until this label is removed label May 15, 2026

HMS17 approved these changes May 19, 2026

View reviewed changes

humsika approved these changes Jun 1, 2026

View reviewed changes

nickpalladino reviewed Jun 17, 2026

View reviewed changes

jloux-brapi removed the on hold Do not merge until this label is removed label Jun 24, 2026

Uh oh!

Conversation

jloux-brapi commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Testing

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jloux-brapi May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jloux-brapi commented May 6, 2026

Uh oh!

davedrp left a comment

Choose a reason for hiding this comment

Uh oh!

jloux-brapi commented May 15, 2026

Uh oh!

HMS17 left a comment

Choose a reason for hiding this comment

Uh oh!

humsika left a comment

Choose a reason for hiding this comment

Uh oh!

jloux-brapi commented Jun 10, 2026

Uh oh!

nickpalladino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jloux-brapi commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jloux-brapi commented May 6, 2026 •

edited

Loading

jloux-brapi May 6, 2026 •

edited

Loading