Untangle/Hack Pedigree UUID scheme so links work on fe - #553
Conversation
…plement stopgap hack Unit tests made me aware of the use case where rows in a gemrplasm import can reference other rows in the import as parents, and when this happens, no germplasmDbId will be created until the germplasm is fully imported because of the way the import is organized. Until we manage to change this, I've created a stop-gap which will mutate this additional info on the fly from the bi-generated exrefs to the brapiGermplasmDbIds in the BrAPIGermplasmDAO.processGermplasmForDisplay method. This will lookup the germplasmDbIds of the pedigree nodes using the exrefs, and will mutate the additional info records for male/female IDs so that the links still work as expected in the frontend UI
jloux-brapi
changed the title
| private Map<String, String> getPedigreeGermplasmDbIdByBICreatedExRef(List<BrAPIGermplasm> brAPIGermplasm, Program program) throws ApiException { | ||
| Map<String, String> germplasmDbIdByGeneratedExRef = new HashMap<>(); | ||
|
|
||
| for (BrAPIGermplasm germplasm : brAPIGermplasm) { |
There was a problem hiding this comment.
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?
| } | ||
| } | ||
|
|
||
| List<String> exRefIds = new ArrayList<>(germplasmDbIdByGeneratedExRef.keySet()); |
There was a problem hiding this comment.
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?
| 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()) : ""; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), GermplasmApi.class); | ||
|
|
||
|
|
||
| List<BrAPIGermplasm> germplasmSearchedWithExRef = brAPIDAOUtil.searchNoPaging( |
There was a problem hiding this comment.
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?
2 participants
Description
Story: BI-2904
germplasm.additionalInfo.male/femaleGermplasmUUIDto convert bi-generated ids to the eventually saved brapi germplasmDbIds. This issue was causing links of female/parent Germplasm data table rows to throw and error when clicked, because these IDs are being looked up as brapi germplasmDbIds with the set of changes fromfeature/BI-2902. This is a rather messy/hacky fix I am using as a stop-gap until we decide to import pedigree after germplasm records are created. A comment explaining this situation has been provided in the code.Dependencies
epic/BI-2862branch ofbrapi-java-prodserverfeature/BI-2905branch ofbi-webTesting
Full regression for most germplasm features, excluding sample importing, which will be dealt with once
developbi-api code is merged and this ticketThis features include:
Germplasmtab, and clicking all associated links in the rows including the GID, both parent links, and Show DetailsChecklist: