From 0a6c61372c5015dc327e4696effb4bf17d4dda20 Mon Sep 17 00:00:00 2001 From: michelgleeson <65299517+michelgleeson@users.noreply.github.com> Date: Tue, 16 Mar 2021 16:26:19 -0400 Subject: [PATCH] Parameterize term valueset concept view dao (#2477) * Generic the list type to accomidate different substring queries * special subquery for Oracle * change the conceptview dao depending on dialect * address review comments * remove generics * no longer inherits * copy paste from non Oracle version * dang copy pasters * changelog Co-authored-by: Mike G --- ...e-in-failing-with-ora-00904-concat_ws.yaml | 4 + .../ITermValueSetConceptViewOracleDao.java | 17 +++ .../entity/TermValueSetConceptViewOracle.java | 142 ++++++++++++++++++ .../fhir/jpa/term/BaseTermReadSvcImpl.java | 122 ++++++++++++++- 4 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/1644-fhir-code-in-failing-with-ora-00904-concat_ws.yaml create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptViewOracleDao.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptViewOracle.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/1644-fhir-code-in-failing-with-ora-00904-concat_ws.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/1644-fhir-code-in-failing-with-ora-00904-concat_ws.yaml new file mode 100644 index 00000000000..75340c1fcc1 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/1644-fhir-code-in-failing-with-ora-00904-concat_ws.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 1644 +title: "support Oracle specific syntax when using a TermValueSetConceptView" diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptViewOracleDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptViewOracleDao.java new file mode 100644 index 00000000000..67fd8f5d2bf --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptViewOracleDao.java @@ -0,0 +1,17 @@ +package ca.uhn.fhir.jpa.dao.data; + +import ca.uhn.fhir.jpa.entity.TermValueSetConceptViewOracle; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; + +import java.io.Serializable; +import java.util.List; + +public interface ITermValueSetConceptViewOracleDao extends JpaRepository { + @Query("SELECT v FROM TermValueSetConceptView v WHERE v.myConceptValueSetPid = :pid AND v.myConceptOrder >= :from AND v.myConceptOrder < :to ORDER BY v.myConceptOrder") + List findByTermValueSetId(@Param("from") int theFrom, @Param("to") int theTo, @Param("pid") Long theValueSetId); + + @Query("SELECT v FROM TermValueSetConceptView v WHERE v.myConceptValueSetPid = :pid AND LOWER(v.myConceptDisplay) LIKE :display ORDER BY v.myConceptOrder") + List findByTermValueSetId(@Param("pid") Long theValueSetId, @Param("display") String theDisplay); +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptViewOracle.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptViewOracle.java new file mode 100644 index 00000000000..7f6121e50ce --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptViewOracle.java @@ -0,0 +1,142 @@ +package ca.uhn.fhir.jpa.entity; + +/* + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import org.hibernate.annotations.Immutable; +import org.hibernate.annotations.Subselect; + +import javax.persistence.Column; +import javax.persistence.DiscriminatorValue; +import javax.persistence.EmbeddedId; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import java.io.Serializable; + +@Entity +@Immutable +@Subselect( + /* + * Note about the CONCAT function below- We need a primary key (an @Id) column + * because hibernate won't allow the view the function without it, but + */ + "SELECT CONCAT(vsc.PID, CONCAT(' ', vscd.PID)) AS PID, " + + " vsc.PID AS CONCEPT_PID, " + + " vsc.VALUESET_PID AS CONCEPT_VALUESET_PID, " + + " vsc.VALUESET_ORDER AS CONCEPT_VALUESET_ORDER, " + + " vsc.SYSTEM_URL AS CONCEPT_SYSTEM_URL, " + + " vsc.CODEVAL AS CONCEPT_CODEVAL, " + + " vsc.DISPLAY AS CONCEPT_DISPLAY, " + + " vscd.PID AS DESIGNATION_PID, " + + " vscd.LANG AS DESIGNATION_LANG, " + + " vscd.USE_SYSTEM AS DESIGNATION_USE_SYSTEM, " + + " vscd.USE_CODE AS DESIGNATION_USE_CODE, " + + " vscd.USE_DISPLAY AS DESIGNATION_USE_DISPLAY, " + + " vscd.VAL AS DESIGNATION_VAL " + + "FROM TRM_VALUESET_CONCEPT vsc " + + "LEFT OUTER JOIN TRM_VALUESET_C_DESIGNATION vscd ON vsc.PID = vscd.VALUESET_CONCEPT_PID" +) +public class TermValueSetConceptViewOracle implements Serializable { + private static final long serialVersionUID = 1L; + + @Id + @Column(name="PID", length = 1000 /* length only needed to satisfy JpaEntityTest, it's not used*/) + private String id; // still set automatically + + @Column(name = "CONCEPT_PID") + private Long myConceptPid; + + @Column(name = "CONCEPT_VALUESET_PID") + private Long myConceptValueSetPid; + + @Column(name = "CONCEPT_VALUESET_ORDER") + private int myConceptOrder; + + @Column(name = "CONCEPT_SYSTEM_URL", length = TermCodeSystem.MAX_URL_LENGTH) + private String myConceptSystemUrl; + + @Column(name = "CONCEPT_CODEVAL", length = TermConcept.MAX_CODE_LENGTH) + private String myConceptCode; + + @Column(name = "CONCEPT_DISPLAY", length = TermConcept.MAX_DESC_LENGTH) + private String myConceptDisplay; + + @Column(name = "DESIGNATION_PID") + private Long myDesignationPid; + + @Column(name = "DESIGNATION_LANG", length = TermConceptDesignation.MAX_LENGTH) + private String myDesignationLang; + + @Column(name = "DESIGNATION_USE_SYSTEM", length = TermConceptDesignation.MAX_LENGTH) + private String myDesignationUseSystem; + + @Column(name = "DESIGNATION_USE_CODE", length = TermConceptDesignation.MAX_LENGTH) + private String myDesignationUseCode; + + @Column(name = "DESIGNATION_USE_DISPLAY", length = TermConceptDesignation.MAX_LENGTH) + private String myDesignationUseDisplay; + + @Column(name = "DESIGNATION_VAL", length = TermConceptDesignation.MAX_VAL_LENGTH) + private String myDesignationVal; + + + public Long getConceptPid() { + return myConceptPid; + } + + public String getConceptSystemUrl() { + return myConceptSystemUrl; + } + + public String getConceptCode() { + return myConceptCode; + } + + public String getConceptDisplay() { + return myConceptDisplay; + } + + public Long getDesignationPid() { + return myDesignationPid; + } + + public String getDesignationLang() { + return myDesignationLang; + } + + public String getDesignationUseSystem() { + return myDesignationUseSystem; + } + + public String getDesignationUseCode() { + return myDesignationUseCode; + } + + public String getDesignationUseDisplay() { + return myDesignationUseDisplay; + } + + public String getDesignationVal() { + return myDesignationVal; + } +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java index 0a8d646a9bb..30526d363e0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java @@ -45,6 +45,7 @@ import ca.uhn.fhir.jpa.dao.data.ITermConceptPropertyDao; import ca.uhn.fhir.jpa.dao.data.ITermValueSetConceptDao; import ca.uhn.fhir.jpa.dao.data.ITermValueSetConceptDesignationDao; import ca.uhn.fhir.jpa.dao.data.ITermValueSetConceptViewDao; +import ca.uhn.fhir.jpa.dao.data.ITermValueSetConceptViewOracleDao; import ca.uhn.fhir.jpa.dao.data.ITermValueSetDao; import ca.uhn.fhir.jpa.entity.TermCodeSystem; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; @@ -62,6 +63,7 @@ import ca.uhn.fhir.jpa.entity.TermConceptPropertyTypeEnum; import ca.uhn.fhir.jpa.entity.TermValueSet; import ca.uhn.fhir.jpa.entity.TermValueSetConcept; import ca.uhn.fhir.jpa.entity.TermValueSetConceptView; +import ca.uhn.fhir.jpa.entity.TermValueSetConceptViewOracle; import ca.uhn.fhir.jpa.entity.TermValueSetPreExpansionStatusEnum; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.sched.HapiJob; @@ -246,6 +248,8 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { @Autowired private ITermValueSetConceptViewDao myTermValueSetConceptViewDao; @Autowired + private ITermValueSetConceptViewOracleDao myTermValueSetConceptViewOracleDao; + @Autowired private ISchedulerService mySchedulerService; @Autowired(required = false) private ITermDeferredStorageSvc myDeferredStorageSvc; @@ -527,11 +531,126 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { */ String msg = myContext.getLocalizer().getMessage(BaseTermReadSvcImpl.class, "valueSetExpandedUsingPreExpansion"); theAccumulator.addMessage(msg); - expandConcepts(theAccumulator, termValueSet, theFilter, theAdd); + if (isOracleDialect()) { + expandConceptsOracle(theAccumulator, termValueSet, theFilter, theAdd); + } + else { + expandConcepts(theAccumulator, termValueSet, theFilter, theAdd); + } } + private boolean isOracleDialect(){ + return myHibernatePropertiesProvider.getDialect() instanceof org.hibernate.dialect.Oracle12cDialect; + } + + private void expandConceptsOracle(IValueSetConceptAccumulator theAccumulator, TermValueSet theTermValueSet, ExpansionFilter theFilter, boolean theAdd) { + // Literal copy paste from expandConcepts but tailored for Oracle since we can't reliably extend the DAO and hibernate classes + Integer offset = theAccumulator.getSkipCountRemaining(); + offset = ObjectUtils.defaultIfNull(offset, 0); + offset = Math.min(offset, theTermValueSet.getTotalConcepts().intValue()); + + Integer count = theAccumulator.getCapacityRemaining(); + count = defaultIfNull(count, myDaoConfig.getMaximumExpansionSize()); + + int conceptsExpanded = 0; + int designationsExpanded = 0; + int toIndex = offset + count; + + Collection conceptViews; + boolean wasFilteredResult = false; + String filterDisplayValue = null; + if (!theFilter.getFilters().isEmpty() && JpaConstants.VALUESET_FILTER_DISPLAY.equals(theFilter.getFilters().get(0).getProperty()) && theFilter.getFilters().get(0).getOp() == ValueSet.FilterOperator.EQUAL) { + filterDisplayValue = lowerCase(theFilter.getFilters().get(0).getValue().replace("%", "[%]")); + String displayValue = "%" + lowerCase(filterDisplayValue) + "%"; + conceptViews = myTermValueSetConceptViewOracleDao.findByTermValueSetId(theTermValueSet.getId(), displayValue); + wasFilteredResult = true; + } else { + // TODO JA HS: I'm pretty sure we are overfetching here. test says offset 3, count 4, but we are fetching index 3 -> 10 here, grabbing 7 concepts. + //Specifically this test testExpandInline_IncludePreExpandedValueSetByUri_FilterOnDisplay_LeftMatch_SelectRange + conceptViews = myTermValueSetConceptViewOracleDao.findByTermValueSetId(offset, toIndex, theTermValueSet.getId()); + theAccumulator.consumeSkipCount(offset); + if (theAdd) { + theAccumulator.incrementOrDecrementTotalConcepts(true, theTermValueSet.getTotalConcepts().intValue()); + } + } + + if (conceptViews.isEmpty()) { + logConceptsExpanded("No concepts to expand. ", theTermValueSet, conceptsExpanded); + return; + } + + Map pidToConcept = new LinkedHashMap<>(); + ArrayListMultimap pidToDesignations = ArrayListMultimap.create(); + + for (TermValueSetConceptViewOracle conceptView : conceptViews) { + + String system = conceptView.getConceptSystemUrl(); + String code = conceptView.getConceptCode(); + String display = conceptView.getConceptDisplay(); + + //-- this is quick solution, may need to revisit + if (!applyFilter(display, filterDisplayValue)) + continue; + + Long conceptPid = conceptView.getConceptPid(); + if (!pidToConcept.containsKey(conceptPid)) { + FhirVersionIndependentConcept concept = new FhirVersionIndependentConcept(system, code, display); + pidToConcept.put(conceptPid, concept); + } + + // TODO: DM 2019-08-17 - Implement includeDesignations parameter for $expand operation to designations optional. + if (conceptView.getDesignationPid() != null) { + TermConceptDesignation designation = new TermConceptDesignation(); + designation.setUseSystem(conceptView.getDesignationUseSystem()); + designation.setUseCode(conceptView.getDesignationUseCode()); + designation.setUseDisplay(conceptView.getDesignationUseDisplay()); + designation.setValue(conceptView.getDesignationVal()); + designation.setLanguage(conceptView.getDesignationLang()); + pidToDesignations.put(conceptPid, designation); + + if (++designationsExpanded % 250 == 0) { + logDesignationsExpanded("Expansion of designations in progress. ", theTermValueSet, designationsExpanded); + } + } + + if (++conceptsExpanded % 250 == 0) { + logConceptsExpanded("Expansion of concepts in progress. ", theTermValueSet, conceptsExpanded); + } + } + + for (Long nextPid : pidToConcept.keySet()) { + FhirVersionIndependentConcept concept = pidToConcept.get(nextPid); + List designations = pidToDesignations.get(nextPid); + String system = concept.getSystem(); + String code = concept.getCode(); + String display = concept.getDisplay(); + + if (theAdd) { + if (theAccumulator.getCapacityRemaining() != null) { + if (theAccumulator.getCapacityRemaining() == 0) { + break; + } + } + + theAccumulator.includeConceptWithDesignations(system, code, display, designations); + } else { + boolean removed = theAccumulator.excludeConcept(system, code); + if (removed) { + theAccumulator.incrementOrDecrementTotalConcepts(false, 1); + } + } + } + + if (wasFilteredResult && theAdd) { + theAccumulator.incrementOrDecrementTotalConcepts(true, pidToConcept.size()); + } + + logDesignationsExpanded("Finished expanding designations. ", theTermValueSet, designationsExpanded); + logConceptsExpanded("Finished expanding concepts. ", theTermValueSet, conceptsExpanded); + } private void expandConcepts(IValueSetConceptAccumulator theAccumulator, TermValueSet theTermValueSet, ExpansionFilter theFilter, boolean theAdd) { + // NOTE: if you modifiy the logic here, look to `expandConceptsOracle` and see if your new code applies to its copy pasted sibling Integer offset = theAccumulator.getSkipCountRemaining(); offset = ObjectUtils.defaultIfNull(offset, 0); offset = Math.min(offset, theTermValueSet.getTotalConcepts().intValue()); @@ -544,6 +663,7 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { int toIndex = offset + count; Collection conceptViews; + Collection conceptViewsOracle; boolean wasFilteredResult = false; String filterDisplayValue = null; if (!theFilter.getFilters().isEmpty() && JpaConstants.VALUESET_FILTER_DISPLAY.equals(theFilter.getFilters().get(0).getProperty()) && theFilter.getFilters().get(0).getOp() == ValueSet.FilterOperator.EQUAL) {