Skip to content

Untangle/Hack Pedigree UUID scheme so links work on fe #553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jloux-brapi wants to merge 4 commits into
base: feature/BI-2902
Choose a base branch
from
Open

Untangle/Hack Pedigree UUID scheme so links work on fe #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40c5f71
Add comments to fix during BrAPI endpoint fixing
jloux-brapi Sep 1, 2026
54ada76
Untagnle pedigree male/female UUID setting, use brapiGermplasmDbIds f…
jloux-brapi Sep 2, 2026
0196eb7
Reverse changes to setting male/femaleParentUUID to germplasmDbId, im…
jloux-brapi Sep 4, 2026
2262274
Fix unit test
jloux-brapi Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ private void setDbIds(BrAPIObservationUnit ou) {
ou.programDbId(Utilities.getExternalReference(ou.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.PROGRAMS))
.orElseThrow(() -> new IllegalStateException("No BI external reference found"))
.getReferenceID());
// TODO: Remove this as part of [BI-3006]
if (ou.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_UUID)) {
ou.setGermplasmDbId(ou.getAdditionalInfo()
.get(BrAPIAdditionalInfoFields.GERMPLASM_UUID)
Expand Down
96 changes: 81 additions & 15 deletions src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,14 @@ private Map<String, BrAPIGermplasm> fetchProgramGermplasm(UUID programId) throws
api::searchGermplasmPost,
api::searchGermplasmSearchResultsDbIdGet,
germplasmSearch),
program.getKey());
program);
} else {
log.debug("Fetching germplasm without pagination to BrAPI");
return processGermplasmForDisplay(brAPIDAOUtil.searchNoPaging(
api::searchGermplasmPost,
api::searchGermplasmSearchResultsDbIdGet,
germplasmSearch),
program.getKey());
program);
}
}

Expand All @@ -209,7 +209,8 @@ public void repopulateGermplasmCacheForProgram(UUID programId) {
* @return Map<Key = string representing germplasm UUID, value = formatted BrAPIGermplasm>
* @throws ApiException
*/
private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplasm> programGermplasm, String programKey) {
private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplasm> programGermplasm,
Program program) throws ApiException {
// Process the germplasm
Map<String, BrAPIGermplasm> programGermplasmMap = new HashMap<>();
log.trace("processing germ for display: " + programGermplasm);
Expand All @@ -227,12 +228,23 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
// Remove program key
if (germplasm.getSynonyms() != null && !germplasm.getSynonyms().isEmpty()) {
for (BrAPIGermplasmSynonyms synonym: germplasm.getSynonyms()) {
String newSynonym = Utilities.removeProgramKey(synonym.getSynonym(), programKey, germplasm.getAccessionNumber());
String newSynonym = Utilities.removeProgramKey(synonym.getSynonym(), program.getKey(), germplasm.getAccessionNumber());
synonym.setSynonym(newSynonym);
}
}
}

// With the programCache removal, any and all outward (fe, API endpoints) facing germplasm should be prepared to use the brapi-generated germplasmDbId instead of the bi-generated exref.
// This notion works fine for the higher level Germplasm entity, but our existing Pedigree implementation is predicated on the usage of bi-generated exref IDs
// stored in germplasm.additionalInfo->male/femaleParentUUID. (See GermplasmProcessor.constructPedigreeString() for assignment of this data)
// To avoid completely re-working the importer to create pedigree nodes and associated germplasm referenced first, this "hack" relates the bi-generated exrefs
// to the brapi-generated germplasmDbId via an extra lookup and processing to the database.
// This is necessary because we need to overwrite bi-generated ids in the pedigree string to use the brapi germplasmDbIds so that front end reference links work properly.
// This should always work because processGermplasmForDisplay is only ever called once Germplasm data has been created in the database, and provided we improve lookups to not fetch
// all program germplasm at once, the performance hit should be negligible.
// TODO: This hack can be removed once/if we implement [BI-2588/BI-2452]
Map<String, String> pedigreeBrAPIGermplasmDbIdByBICreatedExRef = getPedigreeGermplasmDbIdByBICreatedExRef(programGermplasm, program);

// Update pedigree string
for (BrAPIGermplasm germplasm: programGermplasm) {
JsonObject additionalInfo = germplasm.getAdditionalInfo();
Expand Down Expand Up @@ -260,7 +272,7 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
if (!name.isEmpty())
{
// Strip program key.
name = Utilities.removeProgramKeyAndUnknownAdditionalData(name, programKey);
name = Utilities.removeProgramKeyAndUnknownAdditionalData(name, program.getKey());
}
parentNames.add(name);
}
Expand All @@ -271,7 +283,8 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
{
gidPedigreeString = additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID) ? additionalInfo.get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID).getAsString() : "";
namePedigreeString = parentNames.get(0);
uuidPedigreeString = additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID) ? additionalInfo.get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID).getAsString() : "";
// Convert the bi-generated additionalInfo parent ID to the brapi germplasmDbId using map from previous step.
uuidPedigreeString = additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID) ? pedigreeBrAPIGermplasmDbIdByBICreatedExRef.get(additionalInfo.get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID).getAsString()) : "";

@humsika humsika Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the BrAPI search does not return one of the requested parent external references, Map.get() returns null. This can serialize pedigreeByUUID as JSON null or produce values such as null/, which BI-Web cannot safely use as a germplasm link. The old implementation did not have this lookup failure mode. Could we validate that every requested parent was resolved, or intentionally use an empty value/GID fallback instead of returning null?

@jloux-brapi jloux-brapi Sep 8, 2026

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.

Ultimately we have a huge problem if the search returns empty for a specific exref. In that case, we should probably just throw an error that prevents the page from loading.

// Throw a descriptive error if femaleParentUUID is absent.
if (!additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID)) {
String programId = "unknown program";
Expand All @@ -290,7 +303,8 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
{
gidPedigreeString += "/" + (additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_GID) ? additionalInfo.get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_GID).getAsString() : "");
namePedigreeString += "/" + parentNames.get(1);
uuidPedigreeString += "/" + (additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID) ? additionalInfo.get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID).getAsString() : "");
// Convert the bi-generated additionalInfo parent ID to the brapi germplasmDbId using map from previous step.
uuidPedigreeString += "/" + (additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID) ? pedigreeBrAPIGermplasmDbIdByBICreatedExRef.get(additionalInfo.get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID).getAsString()) : "");
// Throw a descriptive error if maleParentUUID is absent.
if (!additionalInfo.has(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID)) {
String programId = "unknown program";
Expand Down Expand Up @@ -318,6 +332,48 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
return programGermplasmMap;
}

private Map<String, String> getPedigreeGermplasmDbIdByBICreatedExRef(List<BrAPIGermplasm> brAPIGermplasm, Program program) throws ApiException {
Map<String, String> germplasmDbIdByGeneratedExRef = new HashMap<>();

for (BrAPIGermplasm germplasm : brAPIGermplasm) {

@humsika humsika Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

additionalInfo appears to be nullable, and the existing processing loop below explicitly handles that case. This new helper dereferences it before that normalization occurs, so one germplasm without additionalInfo could fail the entire response. Could we skip parent extraction when additionalInfo is null and also guard against JSON-null parent UUID values?

if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID)) {
germplasmDbIdByGeneratedExRef.put(germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_UUID).getAsString(), null);
}
if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID)) {
germplasmDbIdByGeneratedExRef.put(germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_UUID).getAsString(), null);
}
}

List<String> exRefIds = new ArrayList<>(germplasmDbIdByGeneratedExRef.keySet());

@humsika humsika Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When no parent UUIDs are present, exRefIds is empty, but this method still sends a BrAPI search containing the program and external-reference source filters. Because the empty ID list does not restrict the search, this may retrieve all matching germplasm for the program even though there is nothing to resolve. Could we return an empty map before making the request?


BrAPIGermplasmSearchRequest searchRequest = new BrAPIGermplasmSearchRequest();
searchRequest.setExternalReferenceIds(exRefIds);
searchRequest.setExternalReferenceSources(List.of(referenceSource));

// Reuse some code to properly set searchRequest with brapiProgramDbId, and proper paging
searchRequest = buildSearchRequest(program, null, null, searchRequest);

GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), GermplasmApi.class);


List<BrAPIGermplasm> germplasmSearchedWithExRef = brAPIDAOUtil.searchNoPaging(

@humsika humsika Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

searchNoPaging() does not iterate additional pages for an asynchronous search result, and the result GET uses the configured 1,000-record page size. If this lookup can return more than 1,000 distinct parents, later mappings may remain unresolved. Can we use the paging-aware search method, or confirm and enforce that this request can never exceed one result page?

api::searchGermplasmPost,
api::searchGermplasmSearchResultsDbIdGet,
searchRequest);

for (BrAPIGermplasm germplasm : germplasmSearchedWithExRef) {
Optional<BrAPIExternalReference> exRef = Utilities.getExternalReference(germplasm.getExternalReferences(), referenceSource);

if (exRef.isPresent()) {
germplasmDbIdByGeneratedExRef.put(exRef.get().getReferenceId(), germplasm.getGermplasmDbId());
} else {
throw new IllegalStateException("External references wasn't found for germplasm (dbid): " + germplasm.getGermplasmDbId());
}
}

return germplasmDbIdByGeneratedExRef;
}

/**
* This method requires a BI-API program. If the BrAPIProgram inside this data model is not set,
* this method will retrieve it.
Expand All @@ -336,7 +392,7 @@ private List<BrAPIGermplasm> getBrAPIGermplasmUsingBrAPIProgramId(GermplasmQuery
List<BrAPIGermplasm> result = brAPIDAOUtil.get(api::germplasmGet, germplasmQueryParams);

// TODO: Once cache is removed for this class, fix processGermplasmForDisplay to return List<BrAPIGermplasm> [BI-2906]
return new ArrayList<>(processGermplasmForDisplay(result, program.getKey()).values());
return new ArrayList<>(processGermplasmForDisplay(result, program).values());
}

// TODO: hack for now, probably should update breedbase
Expand Down Expand Up @@ -369,11 +425,11 @@ private String processBreedbasePedigree(String pedigree) {

public List<BrAPIGermplasm> createBrAPIGermplasm(List<BrAPIGermplasm> postBrAPIGermplasmList, UUID programId, ImportUpload upload) {
GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), GermplasmApi.class);
var program = programDAO.fetchOneById(programId);
var program = new Program(programDAO.fetchOneById(programId));
try {
if (!postBrAPIGermplasmList.isEmpty()) {
List<BrAPIGermplasm> postResponse = brAPIDAOUtil.post(postBrAPIGermplasmList, upload, api::germplasmPost, importDAO::update);
return new ArrayList<>(processGermplasmForDisplay(postResponse, program.getKey()).values());
return new ArrayList<>(processGermplasmForDisplay(postResponse, program).values());
}
return new ArrayList<>();
} catch (Exception e) {
Expand All @@ -383,12 +439,12 @@ public List<BrAPIGermplasm> createBrAPIGermplasm(List<BrAPIGermplasm> postBrAPIG

public List<BrAPIGermplasm> updateBrAPIGermplasm(List<BrAPIGermplasm> putBrAPIGermplasmList, UUID programId, ImportUpload upload) {
GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), GermplasmApi.class);
var program = programDAO.fetchOneById(programId);
var program = new Program(programDAO.fetchOneById(programId));
try {
if (!putBrAPIGermplasmList.isEmpty()) {
Callable<Map<String, BrAPIGermplasm>> postFunction = () -> {
List<BrAPIGermplasm> putResponse = putGermplasm(putBrAPIGermplasmList, api);
return processGermplasmForDisplay(putResponse, program.getKey());
return processGermplasmForDisplay(putResponse, program);
};
return programGermplasmCache.post(programId, postFunction);
}
Expand Down Expand Up @@ -428,15 +484,25 @@ public BrAPIGermplasmListResponse brapiGermplasmSearchReturnResponse(Program pro

// TODO: Once cache is removed for this class, fix processGermplasmForDisplay to return List<BrAPIGermplasm> [BI-2906]
List<BrAPIGermplasm> processedGermplasm =
new ArrayList<>(processGermplasmForDisplay(brAPIDAOUtil.getListResult(brAPIResponse), program.getKey()).values());
new ArrayList<>(processGermplasmForDisplay(brAPIDAOUtil.getListResult(brAPIResponse), program).values());

brAPIResponse.getResult().setData(processedGermplasm);

return brAPIResponse;
}

private BrAPIGermplasmSearchRequest buildSearchRequest(Program program, List<String> brapiGermplasmIds, GermplasmQuery germplasmQuery) throws ApiException {
BrAPIGermplasmSearchRequest searchRequest = new BrAPIGermplasmSearchRequest();
private BrAPIGermplasmSearchRequest buildSearchRequest(Program program, List<String> brapiGermplasmIds, GermplasmQuery query) throws ApiException {
return buildSearchRequest(program, brapiGermplasmIds, query, null);
}

private BrAPIGermplasmSearchRequest buildSearchRequest(Program program, List<String> brapiGermplasmIds, GermplasmQuery germplasmQuery, BrAPIGermplasmSearchRequest searchRequestPassThru) throws ApiException {
BrAPIGermplasmSearchRequest searchRequest;

if (searchRequestPassThru == null) {
searchRequest = new BrAPIGermplasmSearchRequest();
} else {
searchRequest = searchRequestPassThru;
}

searchRequest.programDbIds(List.of(brAPIDAOUtil.getBrAPIProgramDbId(program.getId())));

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ public List<BrAPIObservationUnit> getObservationUnits(Program program,
.orElse(true);

//adding filter for germplasmDbId because we can't easily search that in the stored data object
// TODO: Add search on accessionNumber once it's been added to prod server and brapi client [BI-2978]
// TODO: Add search on germplasmDbId directly in search request [BI-3006]
return matches && germplasmId.map(id -> id.equals(ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_UUID).getAsString())).orElse(true);
}).collect(Collectors.toList());
}
Expand Down Expand Up @@ -382,7 +382,7 @@ private void processObservationUnits(Program program, List<BrAPIObservationUnit>

HashMap<String, BrAPIGermplasm> germplasmByDbId = new HashMap<>();
if( withGID ){
// TODO: Optimize this to use germplasm information directly in BrAPIObservationUnit by adding accession num/GID there via the prodserver/client [BI-2978]
// TODO: Optimize this to use germplasm information directly in BrAPIObservationUnit by searching on ou.germplasmDbIds in a GermplasmSearchRequest [BI-3006]
this.germplasmService.getGermplasm(program.getId()).forEach((germplasm -> germplasmByDbId.put(germplasm.getGermplasmDbId(), germplasm)));
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIPedigreeDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public List<BrAPIPedigreeNode> getPedigree(

PedigreeQueryParams pedigreeRequest = new PedigreeQueryParams();

// TODO: Issue with BrAPI server programDbId filtering, think germplasm are linked to program through observation
// TODO: Issue with BrAPI server programDbId filtering, think germplasm are linked to program through observation [BI-
// units and doesn't work if don't have any loaded
// use external refs instead for now
//pedigreeSearchRequest.programDbIds(List.of(program.getBrapiProgram().getProgramDbId()));
Expand Down
1 change: 1 addition & 0 deletions src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ public void getGermplasmListExport() {
when(programDAO.getProgramBrAPI(any())).thenReturn(brapiProgram);
when(brAPIDAOUtil.get(any(Function.class),
any(GermplasmQueryParams.class))).thenReturn(germplasm);
when(brAPIDAOUtil.getBrAPIProgramDbId(any())).thenReturn(brapiProgramDbId);

//Create germplasm cache of stub data
Method setupMethod = BrAPIGermplasmDAO.class.getDeclaredMethod("setup");
Expand Down
Loading