[BI-2925] Swap externalIds with real Ids for BrAPI endpoints for observationVariables

src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationVariableController.java

@@ -150,23 +150,25 @@ public HttpResponse<BrAPIObservationVariableListResponse> variablesGet(@PathVari
     public HttpResponse<BrAPIObservationVariableSingleResponse> variablesObservationVariableDbIdGet(@PathVariable("programId") UUID programId,
                                                             @PathVariable("observationVariableDbId") String observationVariableDbId) {
         log.debug("fetching variable: " + observationVariableDbId);
-        UUID traitId;
         try {
-            traitId = UUID.fromString(observationVariableDbId);
+            // Check if submitted ID is valid UUID
+            UUID.fromString(observationVariableDbId);
         } catch (IllegalArgumentException e) {
             return HttpResponse.notFound();
         }
 
         try {
-            Optional<Trait> trait = traitService.getById(programId, traitId);
+            List<Trait> programTraits = traitService.getByProgramId(programId, true);
 
-            if(trait.isEmpty()) {
+            if(programTraits.isEmpty()) {
                 return HttpResponse.notFound();
             }
 
+            BrAPIObservationVariable observationVariable = observationVariableService.filterVariableById(programTraits, observationVariableDbId);
+
             BrAPIObservationVariableSingleResponse response = new BrAPIObservationVariableSingleResponse()
                     .metadata(new BrAPIMetadata())
-                    .result(observationVariableService.convertToBrAPI(trait.get()));
+                    .result(observationVariable);
 
             return HttpResponse.ok(response);
         } catch (DoesNotExistException e) {

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java

@@ -98,15 +98,6 @@ private void processObservations(String programKey, List<BrAPIObservation> obser
         }
     }
 
-    private List<BrAPIObservation> getProgramObservations(UUID programId) throws ApiException {
-        Program program = programDAO.get(programId)
-                .stream()
-                .findFirst()
-                .orElseThrow();
-
-        return getBrAPIObservationsUsingBrAPIProgramId(program);
-    }
-
     public List<BrAPIObservation> getObservationsByStudyName(List<String> studyNames, Program program) throws ApiException {
         if(studyNames.isEmpty()) {
             return Collections.emptyList();
@@ -220,15 +211,6 @@ public BrAPIObservationListResponse getObservationsByFilters(Program program, St
         List<BrAPIObservation> observations = brAPIDAOUtil.getListResult(brAPIResponse);
         processObservations(program.getKey(), observations);
 
-        // TODO: Investigate if this resetting of the observationVariableDbId can be undone when trait exrefs are examined [BI-2925]
-        HashMap<String, String> traitIdsByObservationVariableDbId = traitService.getIdsByObservationVariableDbIds(
-                program.getId(), observations.stream()
-                        .map(BrAPIObservation::getObservationVariableDbId).collect(Collectors.toList()));
-
-        observations.forEach(observation -> observation
-                .setObservationVariableDbId(traitIdsByObservationVariableDbId.get(observation.getObservationVariableDbId())));
-        brAPIResponse.getResult().setData(observations);
-
         return brAPIResponse;
     }
 

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationVariableDAO.java

@@ -16,13 +16,19 @@
  */
 package org.breedinginsight.brapi.v2.dao;
 
+import io.micronaut.http.server.exceptions.InternalServerException;
+import org.brapi.client.v2.ApiResponse;
 import org.brapi.client.v2.model.exceptions.ApiException;
 import org.brapi.client.v2.modules.phenotype.ObservationVariablesApi;
 import org.brapi.v2.model.pheno.BrAPIObservationVariable;
 import org.brapi.v2.model.pheno.request.BrAPIObservationVariableSearchRequest;
+import org.brapi.v2.model.pheno.response.BrAPIObservationVariableSingleResponse;
 import org.breedinginsight.daos.ProgramDAO;
+import org.breedinginsight.model.Program;
 import org.breedinginsight.services.brapi.BrAPIEndpointProvider;
+import org.breedinginsight.services.exceptions.DoesNotExistException;
 import org.breedinginsight.utilities.BrAPIDAOUtil;
+import org.breedinginsight.utilities.Utilities;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;

src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java

@@ -17,6 +17,7 @@
 
 package org.breedinginsight.brapi.v2.services;
 
+import io.micronaut.http.server.exceptions.InternalServerException;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
@@ -25,9 +26,12 @@
 import org.brapi.v2.model.core.BrAPIStudy;
 import org.brapi.v2.model.core.BrAPITrial;
 import org.brapi.v2.model.pheno.*;
+import org.brapi.v2.model.pheno.response.BrAPIObservationVariableSingleResponse;
 import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields;
+import org.breedinginsight.brapi.v2.dao.BrAPIObservationVariableDAO;
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.Trait;
+import org.breedinginsight.services.OntologyService;
 import org.breedinginsight.services.ProgramService;
 import org.breedinginsight.services.exceptions.DoesNotExistException;
 import org.breedinginsight.utilities.DatasetUtil;
@@ -43,12 +47,16 @@
 public class BrAPIObservationVariableService {
     private final ProgramService programService;
     private final BrAPITrialService trialService;
+    private final BrAPIObservationVariableDAO observationVariableDAO;
+    private final OntologyService ontologyService;
 
     @Inject
     public BrAPIObservationVariableService(
-            ProgramService programService, BrAPITrialService trialService){
+            ProgramService programService, BrAPITrialService trialService, BrAPIObservationVariableDAO observationVariableDAO, OntologyService ontologyService){
         this.programService = programService;
         this.trialService = trialService;
+        this.observationVariableDAO = observationVariableDAO;
+        this.ontologyService = ontologyService;
     }
 
     // TODO: support sub-entity datasets.
@@ -84,6 +92,31 @@ public List<Trait> getBrAPIObservationVariablesForExperiment(
         return new ArrayList<>();
     }
 
+    @NotNull
+    public BrAPIObservationVariable filterVariableById(List<Trait> programTraits,
+                                                       String observationVariableDbId) throws DoesNotExistException {
+        List<BrAPIObservationVariable> filterResult = filterVariables(programTraits,
+                Optional.of(observationVariableDbId),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty()
+                );
+
+        if (filterResult.isEmpty()) {
+            throw new DoesNotExistException(String.format("No observation variables found for brapi observation variable dbId [%s]", observationVariableDbId));
+        } else if  (filterResult.size() > 1) {
+            throw new InternalServerException(String.format("Multiple observation variables found for brapi observation variable db id [%s].  This should not be possible considering this id is unique.", observationVariableDbId));
+        }
+
+        return filterResult.get(0);
+    }
+
     @NotNull
     public List<BrAPIObservationVariable> filterVariables(List<Trait> programTraits,
                                                            Optional<String> observationVariableDbId,
@@ -115,31 +148,27 @@ public List<BrAPIObservationVariable> filterVariables(List<Trait> programTraits,
                     Map<String, Pair<Optional<String>, String>> filterParams = new HashMap<>();
                     filterParams.put("observationVariableDbId",
                             Pair.of(observationVariableDbId,
-                                    trait.getId()
-                                            .toString()));
+                                    trait.getObservationVariableDbId()));
                     filterParams.put("observationVariableName", Pair.of(observationVariableName, trait.getObservationVariableName()));
                     filterParams.put("traitClass", Pair.of(traitClass, trait.getTraitClass()));
                     filterParams.put("methodDbId",
                             Pair.of(methodDbId,
-                                    trait.getMethodId()
-                                            .toString()));
+                                    trait.getMethod().getBrapiMethodDbId()));
                     filterParams.put("methodName",
                             Pair.of(methodName,
                                     trait.getMethod()
                                             .getDescription()));
                     filterParams.put("scaleDbId",
                             Pair.of(scaleDbId,
                                     trait.getScale()
-                                            .getId()
-                                            .toString()));
+                                            .getBrapiScaleDbId()));
                     filterParams.put("scaleName",
                             Pair.of(scaleName,
                                     trait.getScale()
                                             .getScaleName()));
                     filterParams.put("traitDbId",
                             Pair.of(traitDbId,
-                                    trait.getId()
-                                            .toString()));
+                                    trait.getBrapiTraitDbId()));
                     filterParams.put("traitName", Pair.of(traitName, trait.getObservationVariableName()));
                     filterParams.put("ontologyDbId",
                             Pair.of(ontologyDbId,
@@ -167,29 +196,31 @@ public List<BrAPIObservationVariable> filterVariables(List<Trait> programTraits,
     }
 
     public BrAPIObservationVariable convertToBrAPI(Trait trait) {
+        // This data is not stored in BrAPI by DeltaBreed, which is why it is being set with the DeltaBreed dbIds instead.
+        // They can be filtered upon in BrAPIObservationVariableController.variablesGet()
         BrAPIOntologyReference brAPIOntologyReference = new BrAPIOntologyReference().ontologyDbId(trait.getProgramOntologyId()
                 .toString());
         String status = trait.getActive() ? "active" : "inactive";
         List<String> synonyms = prepSynonyms(trait);
-        return new BrAPIObservationVariable().observationVariableDbId(trait.getId().toString())
+        return new BrAPIObservationVariable().observationVariableDbId(trait.getObservationVariableDbId())
                 .observationVariableName(trait.getObservationVariableName())
                 .defaultValue(trait.getDefaultValue())
                 .status(status)
                 .synonyms(synonyms)
                 .trait(new BrAPITrait().ontologyReference(brAPIOntologyReference)
                         .traitName(trait.getObservationVariableName())
-                        .traitDbId(trait.getId().toString())
+                        .traitDbId(trait.getBrapiTraitDbId())
                         .entity(trait.getEntity())
                         .attribute(trait.getAttribute())
                         .status(status)
                         .synonyms(synonyms))
                 .method(new BrAPIMethod().ontologyReference(brAPIOntologyReference)
-                        .methodDbId(trait.getMethod().getId().toString())
+                        .methodDbId(trait.getMethod().getBrapiMethodDbId())
                         .methodClass(trait.getMethod().getMethodClass())
                         .description(trait.getMethod().getDescription())
                         .formula(trait.getMethod().getFormula()))
                 .scale(new BrAPIScale().ontologyReference(brAPIOntologyReference)
-                        .scaleDbId(trait.getScale().getId().toString())
+                        .scaleDbId(trait.getScale().getBrapiScaleDbId())
                         .scaleName(trait.getScale().getScaleName())
                         .dataType(BrAPITraitDataType.fromValue(trait.getScale().getDataType().getLiteral()))
                         .decimalPlaces(trait.getScale().getDecimalPlaces())

src/main/java/org/breedinginsight/model/Method.java

@@ -17,6 +17,7 @@
 
 package org.breedinginsight.model;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import io.micronaut.core.annotation.Introspected;
 import lombok.Getter;
@@ -48,6 +49,9 @@ public class Method extends MethodEntity {
     private ProgramOntology programOntology;
 
     // BrAPI Properties
+    @JsonIgnore
+    // brapiMethodDbId is only used to convert the bi method id to brapiMethodDbId for the BrAPIObservationVariableController GET endpoints
+    private String brapiMethodDbId;
     private String methodClass;
     private String description;
     private String formula;
@@ -76,5 +80,6 @@ public void setBrAPIProperties(BrAPIMethod brApiMethod){
         this.setMethodClass(brApiMethod.getMethodClass());
         this.setDescription(brApiMethod.getDescription());
         this.setFormula(brApiMethod.getFormula());
+        this.setBrapiMethodDbId(brApiMethod.getMethodDbId());
     }
 }

src/main/java/org/breedinginsight/model/Scale.java

@@ -17,6 +17,7 @@
 
 package org.breedinginsight.model;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import io.micronaut.core.annotation.Introspected;
 import lombok.Getter;
@@ -50,6 +51,9 @@ public class Scale extends ScaleEntity {
     private ProgramOntology programOntology;
 
     // BrAPI properties
+    @JsonIgnore
+    // brapiScaleDbId is only used to convert the bi scale id to brapiScaleDbId for the BrAPIObservationVariableController GET endpoints
+    private String brapiScaleDbId;
     private Integer validValueMax;
     private Integer validValueMin;
     private Integer decimalPlaces;
@@ -89,6 +93,7 @@ public void setBrAPIProperties(BrAPIScale brApiScale) {
             this.setValidValueMin(validValues.getMin());
             this.setCategories(validValues.getCategories());
         }
+        this.setBrapiScaleDbId(brApiScale.getScaleDbId());
     }
 
 }

src/main/java/org/breedinginsight/model/Trait.java

@@ -29,12 +29,10 @@
 import lombok.experimental.SuperBuilder;
 import lombok.extern.jackson.Jacksonized;
 import org.brapi.v2.model.pheno.BrAPIObservationVariable;
-import org.breedinginsight.api.deserializer.ArrayOfStringDeserializer;
 import org.breedinginsight.api.deserializer.ListOfStringDeserializer;
 import org.breedinginsight.dao.db.tables.pojos.TraitEntity;
 import org.jooq.Record;
 
-import javax.validation.Valid;
 import java.util.List;
 
 import static org.breedinginsight.dao.db.Tables.TRAIT;
@@ -64,6 +62,9 @@ public class Trait extends TraitEntity {
 
     // Properties from brapi
     private String observationVariableDbId;
+    // brapiTraitDbId is only used to convert the bi trait id to brapiTraitDbId for the BrAPIObservationVariableController GET endpoints
+    @JsonIgnore
+    private String brapiTraitDbId;
     private String traitClass;
     private String traitDescription;
     private String attribute;
@@ -122,6 +123,7 @@ public void setBrAPIProperties(BrAPIObservationVariable brApiVariable) {
             this.setEntity(brApiVariable.getTrait().getEntity());
             this.setMainAbbreviation(brApiVariable.getTrait().getMainAbbreviation());
             this.setSynonyms(brApiVariable.getTrait().getSynonyms());
+            this.setBrapiTraitDbId(brApiVariable.getTrait().getTraitDbId());
         }
         this.setObservationVariableDbId(brApiVariable.getObservationVariableDbId());
         this.setDefaultValue(brApiVariable.getDefaultValue());

src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java

@@ -161,12 +161,17 @@ void setup() throws Exception {
         traits = createTraits(4);
         AuthenticatedUser user = new AuthenticatedUser(testUser.getName(), new ArrayList<>(), testUser.getId(), new ArrayList<>());
         try {
-            ontologyService.createTraits(program.getId(), traits, user, false);
+            // Re-assign to get obtain all brapi related data as well
+            traits = ontologyService.createTraits(program.getId(), traits, user, false);
         } catch (ValidatorException e) {
             System.err.println(e.getErrors());
             throw e;
         }
 
+        // Re-assignment returns the data in an indeterminate order.  Sort on variable name, which should be determinate.
+        // Tests below depend on the order being correct.
+        traits.sort(Comparator.comparing(Trait::getObservationVariableName));
+
         // Add germplasm to program
         List<BrAPIGermplasm> germplasm = createGermplasm(1, brapiProgramDbId);
         BrAPIExternalReference newReference = new BrAPIExternalReference();
@@ -305,7 +310,7 @@ public void testGetVariablesByExpId() {
     @Test
     public void testGetVariableById() {
         Flowable<HttpResponse<String>> call = client.exchange(
-                GET(String.format("/programs/%s/brapi/v2/variables/%s", program.getId(), traits.get(0).getId()))
+                GET(String.format("/programs/%s/brapi/v2/variables/%s", program.getId(), traits.get(0).getObservationVariableDbId()))
                         .bearerAuth("test-registered-user"),
                 String.class
         );
@@ -316,7 +321,7 @@ public void testGetVariableById() {
         JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class);
         JsonObject variable = responseObj.getAsJsonObject("result");
         assertNotNull(variable);
-        assertEquals(traits.get(0).getId().toString(), variable.get("observationVariableDbId").getAsString());
+        assertEquals(traits.get(0).getObservationVariableDbId(), variable.get("observationVariableDbId").getAsString());
         assertEquals(traits.get(0).getObservationVariableName(), variable.get("observationVariableName").getAsString());
 
         JsonObject trait = variable.getAsJsonObject("trait");
@@ -404,6 +409,7 @@ private List<Trait> createTraits(int numToCreate) {
             String varName = "tt_test_" + (i + 1);
             traits.add(Trait.builder()
                     .observationVariableName(varName)
+                    .observationVariableDbId(UUID.randomUUID().toString())
                     .fullName(varName)
                     .entity("Plant " + i)
                     .attribute("height " + i)

src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java

@@ -269,6 +269,6 @@ void getObservationsByFiltersUsesBrAPISearchPagination() {
         assertEquals(List.of("study-1"), request.getStudyDbIds());
         assertEquals(2, request.getPage());
         assertEquals(25, request.getPageSize());
-        assertEquals(traitId, result.getResult().getData().get(0).getObservationVariableDbId());
+        assertEquals(brapiVariableDbId, result.getResult().getData().get(0).getObservationVariableDbId());
     }
 }