Repository navigation
[BI-2924] Remove cache.get() dependencies in TraitDAOImpl - #572
jloux-brapi wants to merge 2 commits into
Conversation
For one use case, I implemented a search for a single trait via exref
| // Get our db trait (equivalent to brapi variables) | ||
| Optional<Trait> biTraitOpt = getTrait(programId, traitId); | ||
| if (biTraitOpt.isEmpty()) { | ||
| throw new InternalServerException(String.format("Unable to find BI trait with id [%s]", traitId)); |
There was a problem hiding this comment.
This turns a not-found case into a 500. Before, getTraitFull returned Optional.empty() for an unknown trait, and TraitController.getTrait and BrAPIObservationVariableController returned 404.
Now an unknown or other-program traitId throws InternalServerException, and getTraitFull wraps the result in Optional.of (line 302), so those notFound() branches can’t run.
Could this method return an empty Optional when getTrait is empty, and getTraitFull pass it through? A test for GET /programs/{programId}/traits/{nonexistentId} expecting 404 would cover this.
| } catch (ApiException e) { | ||
| log.warn(Utilities.generateApiExceptionLogMessage(e)); | ||
| throw new InternalServerException("Error making BrAPI call", e); | ||
| } finally { |
There was a problem hiding this comment.
The extraction is in finally, so it also runs when variablesGet throws. brApiVariables is still null then, so .orElseThrow() throws NoSuchElementException. That replaces the InternalServerException("Error making BrAPI call", e) from the catch, and the original cause is lost.
Can you move the extraction after the try/catch? The existing getBrAPIVariable(ObservationVariablesApi, UUID) already does this lookup correctly and could be reused.
| .map(BrAPIObservationVariableListResponse::getResult) | ||
| .map(BrAPIObservationVariableListResponseResult::getData) | ||
| .orElseThrow() | ||
| .get(0); |
There was a problem hiding this comment.
If the BrAPI server returns an empty data list (the DB trait exists but has no matching variable), .get(0) throws an unhandled IndexOutOfBoundsException. In TraitService.setTraitActiveStatus this happens after the DB update is already committed.
Can we check the result size and throw a clear InternalServerException, as getBrAPIVariable already does?
Description
Story: BI-2924
TraitController.Dependencies
Testing
Ontology PageShow Detailsand then clickingEdit, where if they are not already used you can edit some elements of the term. Activating and Deactivating also updates ontology. Simplify verify the changes are reflected once they are made.Checklist: