Skip to content

[BI-2806] Remove trials cache#516

Open
jloux-brapi wants to merge 13 commits into
epic/BI-2862from
feature/BI-2806
Open

[BI-2806] Remove trials cache#516
jloux-brapi wants to merge 13 commits into
epic/BI-2862from
feature/BI-2806

Conversation

@jloux-brapi
Copy link
Copy Markdown
Collaborator

@jloux-brapi jloux-brapi commented May 6, 2026

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

return getTrialsFromBrAPI(program, trialQueryParams);
}

private Map<String, BrAPITrial> fetchProgramExperiments(UUID programId) throws ApiException {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return trials;
}

private List<BrAPITrial> getTrialsByExRef(String referenceSource, String referenceId, Program program) throws ApiException {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method was unused and removed.

}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be important for BI-2861

// 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);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// 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.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

study.trialDbId(Utilities.getExternalReference(study.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// 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))
Copy link
Copy Markdown
Collaborator Author

@jloux-brapi jloux-brapi May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.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)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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());
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jloux-brapi
Copy link
Copy Markdown
Collaborator Author

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.

Copy link
Copy Markdown
Contributor

@davedrp davedrp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely documented. Thanks

@jloux-brapi jloux-brapi added the on hold Do not merge until this label is removed label May 15, 2026
@jloux-brapi
Copy link
Copy Markdown
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature on hold Do not merge until this label is removed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants