From dbc9427a05a3fed25ebd98a20aceb8eeb7635de7 Mon Sep 17 00:00:00 2001 From: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com> Date: Thu, 24 Feb 2022 16:32:11 -0500 Subject: [PATCH] Issue 3391 2588 expand valueset regex include not working (#3410) * Refactor ValueSet expansion code to simplify property filtering by using new Hibernate search V6 nested classes capabilities; use generic hibernate search queries where possible (mostly everywhere but regex queries) and add R4 integration tests for both elastic and lucene. * Refactor ValueSet expansion code to simplify property filtering by using new Hibernate search V6 nested classes capabilities; use generic hibernate search queries where possible (mostly everywhere but regex queries) and add R4 integration tests for both elastic and lucene. * Add comment for trick * Add codes to exception messages * Add test specific for the fix * Move changelog to current release. Add upgrade notice. * Improve fail-fast method * Update upgrade instructions * Try luck with new exception codes * Update hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtilTest.java Add missed quote Co-authored-by: michaelabuckley Co-authored-by: juan.marchionatto Co-authored-by: michaelabuckley --- .../src/main/java/ca/uhn/fhir/i18n/Msg.java | 2 +- ...nd-valueset-regex-include-not-working.yaml | 4 + .../uhn/hapi/fhir/changelog/6_0_0/upgrade.md | 28 + .../fhir/jpa/dao/data/ITermConceptDao.java | 4 +- .../ca/uhn/fhir/jpa/entity/TermConcept.java | 38 +- .../fhir/jpa/entity/TermConceptProperty.java | 17 + .../jpa/entity/TermConceptPropertyBinder.java | 80 - .../fhir/jpa/term/BaseTermReadSvcImpl.java | 306 ++- .../uhn/fhir/jpa/term/api/ITermReadSvc.java | 2 + .../TestHibernateSearchAddInConfig.java | 50 +- ...bstractValeSetFreeTextExpansionR4Test.java | 1913 +++++++++++++++++ .../jpa/term/TerminologySvcImplDstu3Test.java | 4 +- .../ValeSetFreeTextExpansionR4ElasticIT.java | 16 + .../ValeSetFreeTextExpansionR4LuceneIT.java | 17 + .../jpa/term/ValueSetExpansionR4Test.java | 53 - .../fhir/jpa/term/api/ITermReadSvcTest.java | 53 + .../ElasticsearchNestedQueryBuilderUtil.java | 74 + ...asticsearchNestedQueryBuilderUtilTest.java | 87 + 18 files changed, 2497 insertions(+), 251 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/2588-expand-valueset-regex-include-not-working.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/upgrade.md delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptPropertyBinder.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/AbstractValeSetFreeTextExpansionR4Test.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4ElasticIT.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4LuceneIT.java create mode 100644 hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtil.java create mode 100644 hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtilTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java index 7b0db303275..e2a0aa1dffc 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java @@ -25,7 +25,7 @@ public final class Msg { /** * IMPORTANT: Please update the following comment after you add a new code - * Last code value: 2036 + * Last code value: 2038 */ private Msg() {} diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/2588-expand-valueset-regex-include-not-working.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/2588-expand-valueset-regex-include-not-working.yaml new file mode 100644 index 00000000000..71db2b2d869 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/2588-expand-valueset-regex-include-not-working.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 2588 +title: "Include property regex operation was not working when expanding ValueSet. This is now fixed" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/upgrade.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/upgrade.md new file mode 100644 index 00000000000..05fa94347f2 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/upgrade.md @@ -0,0 +1,28 @@ +This release has some breaking changes that are worth knowing about. +* Hibernate Search mappings for Terminology entities have been upgraded, which causes a reindex of ValueSet resources to be required. + +## Hibernate Search Mappings Upgrade + +Some freetext mappings (definitions about how resources are freetext indexed) were changed for Terminology resources. +This change requires a full reindexing for any Smile CDR installations which make use of the following features: + +* Fulltext/Content search +* Terminology Expansion + +To reindex all resources call: + +```http +POST /$reindex +Content-Type: application/fhir+json + +{ + "resourceType": "Parameters", + "parameter": [ { + "name": "everything", + "valueBoolean": "true" + }, { + "name": "batchSize", + "valueDecimal": 1000 + } ] +} +``` diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptDao.java index bedfe146ae1..4c090284173 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptDao.java @@ -9,7 +9,6 @@ import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; -import java.util.Collection; import java.util.List; import java.util.Optional; @@ -51,6 +50,9 @@ public interface ITermConceptDao extends JpaRepository, IHapi @Query("SELECT c FROM TermConcept c WHERE c.myCodeSystem = :code_system AND c.myCode = :code") Optional findByCodeSystemAndCode(@Param("code_system") TermCodeSystemVersion theCodeSystem, @Param("code") String theCode); + @Query("FROM TermConcept WHERE myCodeSystem = :code_system AND myCode in (:codeList)") + List findByCodeSystemAndCodeList(@Param("code_system") TermCodeSystemVersion theCodeSystem, @Param("codeList") List theCodeList); + @Modifying @Query("DELETE FROM TermConcept WHERE myCodeSystem.myId = :cs_pid") int deleteByCodeSystemVersion(@Param("cs_pid") Long thePid); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java index 8fa1217095c..663831a6a6d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java @@ -20,8 +20,8 @@ package ca.uhn.fhir.jpa.entity; * #L% */ -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.support.IValidationSupport; +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; import ca.uhn.fhir.jpa.search.DeferConceptIndexingRoutingBinder; import ca.uhn.fhir.util.ValidateUtil; @@ -30,21 +30,43 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import org.hibernate.search.engine.backend.types.ObjectStructure; import org.hibernate.search.engine.backend.types.Projectable; import org.hibernate.search.engine.backend.types.Searchable; -import org.hibernate.search.mapper.pojo.bridge.mapping.annotation.PropertyBinderRef; import org.hibernate.search.mapper.pojo.bridge.mapping.annotation.RoutingBinderRef; import org.hibernate.search.mapper.pojo.mapping.definition.annotation.FullTextField; import org.hibernate.search.mapper.pojo.mapping.definition.annotation.GenericField; import org.hibernate.search.mapper.pojo.mapping.definition.annotation.Indexed; -import org.hibernate.search.mapper.pojo.mapping.definition.annotation.PropertyBinding; +import org.hibernate.search.mapper.pojo.mapping.definition.annotation.IndexedEmbedded; import org.hl7.fhir.r4.model.Coding; import javax.annotation.Nonnull; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.ForeignKey; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; import javax.persistence.Index; -import javax.persistence.*; +import javax.persistence.JoinColumn; +import javax.persistence.Lob; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; +import javax.persistence.PrePersist; +import javax.persistence.PreUpdate; +import javax.persistence.SequenceGenerator; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.UniqueConstraint; import java.io.Serializable; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Date; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.left; @@ -92,8 +114,12 @@ public class TermConcept implements Serializable { @FullTextField(name = "myDisplayPhonetic", searchable= Searchable.YES, projectable= Projectable.NO, analyzer = "autocompletePhoneticAnalyzer") private String myDisplay; + /** + * IndexedEmbedded uses ObjectStructure.NESTED to be able to use hibernate search nested predicate queries. + * @see "https://docs.jboss.org/hibernate/search/6.0/reference/en-US/html_single/#search-dsl-predicate-nested" + */ @OneToMany(mappedBy = "myConcept", orphanRemoval = false, fetch = FetchType.LAZY) - @PropertyBinding(binder = @PropertyBinderRef(type = TermConceptPropertyBinder.class)) + @IndexedEmbedded(structure = ObjectStructure.NESTED) private Collection myProperties; @OneToMany(mappedBy = "myConcept", orphanRemoval = false, fetch = FetchType.LAZY) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptProperty.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptProperty.java index 5d9b79cd470..82f39c96100 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptProperty.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptProperty.java @@ -26,6 +26,10 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import org.hibernate.search.engine.backend.types.Projectable; +import org.hibernate.search.engine.backend.types.Searchable; +import org.hibernate.search.mapper.pojo.mapping.definition.annotation.FullTextField; +import org.hibernate.search.mapper.pojo.mapping.definition.annotation.GenericField; import org.hibernate.validator.constraints.NotBlank; import javax.annotation.Nonnull; @@ -57,9 +61,11 @@ public class TermConceptProperty implements Serializable { public static final int MAX_PROPTYPE_ENUM_LENGTH = 6; private static final long serialVersionUID = 1L; private static final int MAX_LENGTH = 500; + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "CONCEPT_PID", referencedColumnName = "PID", foreignKey = @ForeignKey(name = "FK_CONCEPTPROP_CONCEPT")) private TermConcept myConcept; + /** * TODO: Make this non-null * @@ -68,30 +74,41 @@ public class TermConceptProperty implements Serializable { @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "CS_VER_PID", nullable = true, referencedColumnName = "PID", foreignKey = @ForeignKey(name = "FK_CONCEPTPROP_CSV")) private TermCodeSystemVersion myCodeSystemVersion; + @Id() @SequenceGenerator(name = "SEQ_CONCEPT_PROP_PID", sequenceName = "SEQ_CONCEPT_PROP_PID") @GeneratedValue(strategy = GenerationType.AUTO, generator = "SEQ_CONCEPT_PROP_PID") @Column(name = "PID") private Long myId; + @Column(name = "PROP_KEY", nullable = false, length = MAX_LENGTH) @NotBlank + @GenericField(searchable = Searchable.YES) private String myKey; + @Column(name = "PROP_VAL", nullable = true, length = MAX_LENGTH) + @FullTextField(searchable = Searchable.YES, projectable = Projectable.YES, analyzer = "standardAnalyzer") + @GenericField(name = "myValueString", searchable = Searchable.YES) private String myValue; + @Column(name = "PROP_VAL_LOB") @Lob() private byte[] myValueLob; + @Column(name = "PROP_TYPE", nullable = false, length = MAX_PROPTYPE_ENUM_LENGTH) private TermConceptPropertyTypeEnum myType; + /** * Relevant only for properties of type {@link TermConceptPropertyTypeEnum#CODING} */ @Column(name = "PROP_CODESYSTEM", length = MAX_LENGTH, nullable = true) private String myCodeSystem; + /** * Relevant only for properties of type {@link TermConceptPropertyTypeEnum#CODING} */ @Column(name = "PROP_DISPLAY", length = MAX_LENGTH, nullable = true) + @GenericField(name = "myDisplayString", searchable = Searchable.YES) private String myDisplay; /** diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptPropertyBinder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptPropertyBinder.java deleted file mode 100644 index 253b4695ff2..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptPropertyBinder.java +++ /dev/null @@ -1,80 +0,0 @@ -package ca.uhn.fhir.jpa.entity; - -/*- - * #%L - * HAPI FHIR JPA Server - * %% - * Copyright (C) 2014 - 2022 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.search.engine.backend.document.DocumentElement; -import org.hibernate.search.engine.backend.document.model.dsl.IndexSchemaElement; -import org.hibernate.search.mapper.pojo.bridge.PropertyBridge; -import org.hibernate.search.mapper.pojo.bridge.binding.PropertyBindingContext; -import org.hibernate.search.mapper.pojo.bridge.mapping.programmatic.PropertyBinder; -import org.hibernate.search.mapper.pojo.bridge.runtime.PropertyBridgeWriteContext; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.Collection; - -import static org.apache.commons.lang3.StringUtils.isNotBlank; - -/** - * Allows hibernate search to index individual concepts' properties - */ -public class TermConceptPropertyBinder implements PropertyBinder { - - - public static final String CONCEPT_FIELD_PROPERTY_PREFIX = "PROP"; - private static final Logger ourLog = LoggerFactory.getLogger(TermConceptPropertyBinder.class); - - @Override - public void bind(PropertyBindingContext thePropertyBindingContext) { - thePropertyBindingContext.dependencies().use("myKey").use("myValue"); - IndexSchemaElement indexSchemaElement = thePropertyBindingContext.indexSchemaElement(); - - //In order to support dynamic fields, we have to use field templates. We _must_ define the template at bootstrap time and cannot - //create them adhoc. https://docs.jboss.org/hibernate/search/6.0/reference/en-US/html_single/#mapper-orm-bridge-index-field-dsl-dynamic - //I _think_ im doing the right thing here by indicating that everything matching this template uses this analyzer. - indexSchemaElement.fieldTemplate("propTemplate", f -> f.asString().analyzer("termConceptPropertyAnalyzer")) - .matchingPathGlob(CONCEPT_FIELD_PROPERTY_PREFIX + "*") - .multiValued(); - - - thePropertyBindingContext.bridge(new TermConceptPropertyBridge()); - } - - private class TermConceptPropertyBridge implements PropertyBridge { - - @Override - public void write(DocumentElement theDocument, Object theObject, PropertyBridgeWriteContext thePropertyBridgeWriteContext) { - - Collection properties = (Collection) theObject; - - if (properties != null) { - for (TermConceptProperty next : properties) { - theDocument.addValue(CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey(), next.getValue()); - ourLog.trace("Adding Prop: {}{} -- {}", CONCEPT_FIELD_PROPERTY_PREFIX, next.getKey(), next.getValue()); - if (next.getType() == TermConceptPropertyTypeEnum.CODING && isNotBlank(next.getDisplay())) { - theDocument.addValue(CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey(), next.getDisplay()); - ourLog.trace("Adding multivalue Prop: {}{} -- {}", CONCEPT_FIELD_PROPERTY_PREFIX, next.getKey(), next.getDisplay()); - } - } - } - } - } -} 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 6d5f5949bb5..a0624d7fefd 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 @@ -51,7 +51,6 @@ import ca.uhn.fhir.jpa.entity.TermConceptDesignation; import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink; import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; import ca.uhn.fhir.jpa.entity.TermConceptProperty; -import ca.uhn.fhir.jpa.entity.TermConceptPropertyBinder; import ca.uhn.fhir.jpa.entity.TermConceptPropertyTypeEnum; import ca.uhn.fhir.jpa.entity.TermValueSet; import ca.uhn.fhir.jpa.entity.TermValueSetConcept; @@ -62,6 +61,7 @@ import ca.uhn.fhir.jpa.model.sched.HapiJob; import ca.uhn.fhir.jpa.model.sched.ISchedulerService; import ca.uhn.fhir.jpa.model.sched.ScheduledJobDefinition; import ca.uhn.fhir.jpa.model.util.JpaConstants; +import ca.uhn.fhir.jpa.search.ElasticsearchNestedQueryBuilderUtil; import ca.uhn.fhir.jpa.search.builder.SearchBuilder; import ca.uhn.fhir.jpa.term.api.ITermCodeSystemStorageSvc; import ca.uhn.fhir.jpa.term.api.ITermConceptMappingSvc; @@ -87,12 +87,15 @@ import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; import com.google.common.collect.ArrayListMultimap; +import com.google.gson.JsonObject; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.time.DateUtils; import org.apache.lucene.index.Term; +import org.apache.lucene.search.Query; import org.apache.lucene.search.RegexpQuery; +import org.apache.lucene.search.TermQuery; import org.hibernate.search.backend.elasticsearch.ElasticsearchExtension; import org.hibernate.search.backend.lucene.LuceneExtension; import org.hibernate.search.engine.search.predicate.dsl.BooleanPredicateClausesStep; @@ -159,6 +162,7 @@ import javax.persistence.criteria.JoinType; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; @@ -180,6 +184,10 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import static ca.uhn.fhir.jpa.term.api.ITermLoaderSvc.LOINC_URI; +import static java.lang.String.join; +import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; import static org.apache.commons.lang3.StringUtils.defaultString; import static org.apache.commons.lang3.StringUtils.isBlank; @@ -199,6 +207,11 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { private static Runnable myInvokeOnNextCallForUnitTest; private static boolean ourForceDisableHibernateSearchForUnitTest; + private static final String IDX_PROPERTIES = "myProperties"; + private static final String IDX_PROP_KEY = IDX_PROPERTIES + ".myKey"; + private static final String IDX_PROP_VALUE_STRING = IDX_PROPERTIES + ".myValueString"; + private static final String IDX_PROP_DISPLAY_STRING = IDX_PROPERTIES + ".myDisplayString"; + private final Cache myCodeSystemCurrentVersionCache = Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.MINUTES).build(); @Autowired protected DaoRegistry myDaoRegistry; @@ -278,7 +291,7 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { .getParents() .stream() .map(t -> t.getParent().getId().toString()) - .collect(Collectors.joining(" ")); + .collect(joining(" ")); } Collection designations = theConcept.getDesignations(); @@ -1100,7 +1113,7 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { break; case "ancestor": isCodeSystemLoincOrThrowInvalidRequestException(theCodeSystemIdentifier, theFilter.getProperty()); - handleFilterLoincAncestor2(theCodeSystemIdentifier, theF, theB, theFilter); + handleFilterLoincAncestor(theCodeSystemIdentifier, theF, theB, theFilter); break; case "descendant": isCodeSystemLoincOrThrowInvalidRequestException(theCodeSystemIdentifier, theFilter.getProperty()); @@ -1111,50 +1124,66 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { handleFilterLoincCopyright(theF, theB, theFilter); break; default: - handleFilterRegex(theF, theB, theFilter); + if (theFilter.getOp() == ValueSet.FilterOperator.REGEX) { + handleFilterRegex(theF, theB, theFilter); + } else { + handleFilterPropertyDefault(theF, theB, theFilter); + } break; } } + private void handleFilterPropertyDefault(SearchPredicateFactory theF, + BooleanPredicateClausesStep theB, ValueSet.ConceptSetFilterComponent theFilter) { + + theB.must(getPropertyNameValueNestedPredicate(theF, theFilter.getProperty(), theFilter.getValue())); + } + + private void handleFilterRegex(SearchPredicateFactory theF, BooleanPredicateClausesStep theB, ValueSet.ConceptSetFilterComponent theFilter) { - if (theFilter.getOp() == ValueSet.FilterOperator.REGEX) { + /* + * We treat the regex filter as a match on the regex + * anywhere in the property string. The spec does not + * say whether or not this is the right behaviour, but + * there are examples that seem to suggest that it is. + */ + String value = theFilter.getValue(); + if (value.endsWith("$")) { + value = value.substring(0, value.length() - 1); + } else if (!value.endsWith(".*")) { + value = value + ".*"; + } + if (!value.startsWith("^") && !value.startsWith(".*")) { + value = ".*" + value; + } else if (value.startsWith("^")) { + value = value.substring(1); + } - /* - * We treat the regex filter as a match on the regex - * anywhere in the property string. The spec does not - * say whether or not this is the right behaviour, but - * there are examples that seem to suggest that it is. - */ - String value = theFilter.getValue(); - if (value.endsWith("$")) { - value = value.substring(0, value.length() - 1); - } else if (!value.endsWith(".*")) { - value = value + ".*"; - } - if (!value.startsWith("^") && !value.startsWith(".*")) { - value = ".*" + value; - } else if (value.startsWith("^")) { - value = value.substring(1); - } + if (isFullTextSetToUseElastic()) { + ElasticsearchNestedQueryBuilderUtil nestedQueryBuildUtil = new ElasticsearchNestedQueryBuilderUtil( + "myProperties", "myKey", theFilter.getProperty(), + "myValueString", value); - Term term = new Term(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + theFilter.getProperty(), value); + JsonObject nestedQueryJO = nestedQueryBuildUtil.toGson(); - if (isFullTextSetToUseElastic()) { - String regexpQuery = "{'regexp':{'" + term.field() + "':{'value':'" + term.text() + "'}}}"; - ourLog.debug("Build Elasticsearch Regexp Query: {}", regexpQuery); - theB.must(theF.extension(ElasticsearchExtension.get()).fromJson(regexpQuery)); - } else { - RegexpQuery query = new RegexpQuery(term); - theB.must(theF.extension(LuceneExtension.get()).fromLuceneQuery(query)); - } - } else { - String value = theFilter.getValue(); - Term term = new Term(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + theFilter.getProperty(), value); - theB.must(theF.match().field(term.field()).matching(term.text())); + ourLog.debug("Build nested Elasticsearch query: {}", nestedQueryJO); + theB.must(theF.extension(ElasticsearchExtension.get()).fromJson(nestedQueryJO)); + return; } + + // native Lucene configured + Query termPropKeyQuery = new TermQuery(new Term(IDX_PROP_KEY, theFilter.getProperty())); + Query regexpValueQuery = new RegexpQuery(new Term(IDX_PROP_VALUE_STRING, value)); + + theB.must(theF.nested().objectField("myProperties").nest( + theF.bool() + .must(theF.extension(LuceneExtension.get()).fromLuceneQuery(termPropKeyQuery)) + .must(theF.extension(LuceneExtension.get()).fromLuceneQuery(regexpValueQuery)) + )); } + private void handleFilterLoincCopyright(SearchPredicateFactory theF, BooleanPredicateClausesStep theB, ValueSet.ConceptSetFilterComponent theFilter) { if (theFilter.getOp() == ValueSet.FilterOperator.EQUAL) { @@ -1162,11 +1191,11 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { switch (copyrightFilterValue) { case "3rdparty": logFilteringValueOnProperty(theFilter.getValue(), theFilter.getProperty()); - addFilterLoincCopyright3rdParty(theF, theB); + addFilterLoincCopyright3rdParty(theF, theB, theFilter); break; case "loinc": logFilteringValueOnProperty(theFilter.getValue(), theFilter.getProperty()); - addFilterLoincCopyrightLoinc(theF, theB); + addFilterLoincCopyrightLoinc(theF, theB, theFilter); break; default: throwInvalidRequestForValueOnProperty(theFilter.getValue(), theFilter.getProperty()); @@ -1177,19 +1206,25 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { } } - private void addFilterLoincCopyrightLoinc(SearchPredicateFactory thePredicateFactory, BooleanPredicateClausesStep theBooleanClause) { - theBooleanClause.mustNot(thePredicateFactory.exists().field(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + "EXTERNAL_COPYRIGHT_NOTICE")); + private void addFilterLoincCopyrightLoinc(SearchPredicateFactory theF, + BooleanPredicateClausesStep theB, ValueSet.ConceptSetFilterComponent theFilter) { + + theB.mustNot(theF.match().field(IDX_PROP_KEY).matching("EXTERNAL_COPYRIGHT_NOTICE")); } - private void addFilterLoincCopyright3rdParty(SearchPredicateFactory thePredicateFactory, BooleanPredicateClausesStep theBooleanClause) { + + private void addFilterLoincCopyright3rdParty(SearchPredicateFactory thePredicateFactory, + BooleanPredicateClausesStep theBooleanClause, ValueSet.ConceptSetFilterComponent theFilter) { //TODO GGG HS These used to be Term term = new Term(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + "EXTERNAL_COPYRIGHT_NOTICE", ".*");, which was lucene-specific. //TODO GGG HS ask diederik if this is equivalent. //This old .* regex is the same as an existence check on a field, which I've implemented here. - theBooleanClause.must(thePredicateFactory.exists().field(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + "EXTERNAL_COPYRIGHT_NOTICE")); +// theBooleanClause.must(thePredicateFactory.exists().field("EXTERNAL_COPYRIGHT_NOTICE")); + + theBooleanClause.must(thePredicateFactory.match().field(IDX_PROP_KEY).matching("EXTERNAL_COPYRIGHT_NOTICE")); } @SuppressWarnings("EnumSwitchStatementWhichMissesCases") - private void handleFilterLoincAncestor2(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { + private void handleFilterLoincAncestor(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { switch (theFilter.getOp()) { case EQUAL: addLoincFilterAncestorEqual(theSystem, f, b, theFilter); @@ -1204,24 +1239,24 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { } private void addLoincFilterAncestorEqual(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { - addLoincFilterAncestorEqual(theSystem, f, b, theFilter.getProperty(), theFilter.getValue()); + long parentPid = getAncestorCodePid(theSystem, theFilter.getProperty(), theFilter.getValue()); + b.must(f.match().field("myParentPids").matching(String.valueOf(parentPid))); } - private void addLoincFilterAncestorEqual(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, String theProperty, String theValue) { - List terms = getAncestorTerms(theSystem, theProperty, theValue); - b.must(f.bool(innerB -> terms.forEach(term -> innerB.should(f.match().field(term.field()).matching(term.text()))))); - } private void addLoincFilterAncestorIn(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { String[] values = theFilter.getValue().split(","); - List terms = new ArrayList<>(); + List ancestorCodePidList = new ArrayList<>(); for (String value : values) { - terms.addAll(getAncestorTerms(theSystem, theFilter.getProperty(), value)); + ancestorCodePidList.add(getAncestorCodePid(theSystem, theFilter.getProperty(), value)); } - b.must(f.bool(innerB -> terms.forEach(term -> innerB.should(f.match().field(term.field()).matching(term.text()))))); + b.must(f.bool(innerB -> ancestorCodePidList.forEach( + ancestorPid -> innerB.should(f.match().field("myParentPids").matching(String.valueOf(ancestorPid))) + ))); } + @SuppressWarnings("EnumSwitchStatementWhichMissesCases") private void handleFilterLoincParentChild(SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { switch (theFilter.getOp()) { @@ -1238,31 +1273,33 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { private void addLoincFilterParentChildIn(SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { String[] values = theFilter.getValue().split(","); - List terms = new ArrayList<>(); + b.minimumShouldMatchNumber(1); for (String value : values) { logFilteringValueOnProperty(value, theFilter.getProperty()); - terms.add(getPropertyTerm(theFilter.getProperty(), value)); + b.should(getPropertyNameValueNestedPredicate(f, theFilter.getProperty(), value)); } - - //TODO GGG HS: Not sure if this is the right equivalent...seems to be no equivalent to `TermsQuery` in HS6. - //Far as I'm aware, this is a single element of a MUST portion of a bool, which itself should contain a list of OR'ed options, e.g. - // shape == round && color == (green || blue) - b.must(f.bool(innerB -> terms.forEach(term -> innerB.should(f.match().field(term.field()).matching(term.text()))))); } private void addLoincFilterParentChildEqual(SearchPredicateFactory f, BooleanPredicateClausesStep b, String theProperty, String theValue) { logFilteringValueOnProperty(theValue, theProperty); - //TODO GGG HS: Not sure if this is the right equivalent...seems to be no equivalent to `TermsQuery` in HS6. - //b.must(new TermsQuery(getPropertyTerm(theProperty, theValue))); - //According to the DSL migration reference (https://docs.jboss.org/hibernate/search/6.0/migration/html_single/#queries-reference), - //Since this property is handled with a specific analyzer, I'm not sure a terms match here is actually correct. The analyzer is literally just a whitespace tokenizer here. - - b.must(f.match().field(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + theProperty).matching(theValue)); + b.must(getPropertyNameValueNestedPredicate(f, theProperty, theValue)); } + /** + * A nested predicate is required for both predicates to be applied to same property, otherwise if properties + * propAA:valueAA and propBB:valueBB are defined, a search for propAA:valueBB would be a match + * @see "https://docs.jboss.org/hibernate/search/6.0/reference/en-US/html_single/#search-dsl-predicate-nested" + */ + private PredicateFinalStep getPropertyNameValueNestedPredicate(SearchPredicateFactory f, String theProperty, String theValue) { + return f.nested().objectField(IDX_PROPERTIES).nest( + f.bool() + .must(f.match().field(IDX_PROP_KEY).matching(theProperty)) + .must(f.match().field(IDX_PROP_VALUE_STRING).field(IDX_PROP_DISPLAY_STRING).matching(theValue)) + ); + } + + private void handleFilterConceptAndCode(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { - - TermConcept code = findCode(theSystem, theFilter.getValue()) .orElseThrow(() -> new InvalidRequestException("Invalid filter criteria - code does not exist: {" + Constants.codeSystemWithDefaultDescription(theSystem) + "}" + theFilter.getValue())); @@ -1312,20 +1349,12 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { ); } - private Term getPropertyTerm(String theProperty, String theValue) { - return new Term(TermConceptPropertyBinder.CONCEPT_FIELD_PROPERTY_PREFIX + theProperty, theValue); - } - - private List getAncestorTerms(String theSystem, String theProperty, String theValue) { - List retVal = new ArrayList<>(); - + private long getAncestorCodePid(String theSystem, String theProperty, String theValue) { TermConcept code = findCode(theSystem, theValue) .orElseThrow(() -> new InvalidRequestException("Invalid filter criteria - code does not exist: {" + Constants.codeSystemWithDefaultDescription(theSystem) + "}" + theValue)); - retVal.add(new Term("myParentPids", "" + code.getId())); logFilteringValueOnProperty(theValue, theProperty); - - return retVal; + return code.getId(); } @SuppressWarnings("EnumSwitchStatementWhichMissesCases") @@ -1342,56 +1371,117 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { } } - private void addLoincFilterDescendantEqual(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { - addLoincFilterDescendantEqual(theSystem, f, b, theFilter.getProperty(), theFilter.getValue()); - } - private void addLoincFilterDescendantIn(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { - String[] values = theFilter.getValue().split(","); - List terms = new ArrayList<>(); - for (String value : values) { - terms.addAll(getDescendantTerms(theSystem, theFilter.getProperty(), value)); + private void addLoincFilterDescendantEqual(String theSystem, SearchPredicateFactory f, + BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { + + List parentPids = getCodeParentPids(theSystem, theFilter.getProperty(), theFilter.getValue()); + if (parentPids.isEmpty()) { + // Can't return empty must, because it wil match according to other predicates. + // Some day there will be a 'matchNone' predicate (https://discourse.hibernate.org/t/fail-fast-predicate/6062) + b.mustNot( f.matchAll() ); + return; } - searchByParentPids(f, b, terms); - } - private void addLoincFilterDescendantEqual(String theSystem, SearchPredicateFactory f, BooleanPredicateClausesStep b, String theProperty, String theValue) { - List terms = getDescendantTerms(theSystem, theProperty, theValue); - searchByParentPids(f, b, terms); - } - - private void searchByParentPids(SearchPredicateFactory f, BooleanPredicateClausesStep b, List theTerms) { - List parentPids = convertTermsToParentPids(theTerms); b.must(f.bool(innerB -> { - parentPids.forEach(pid -> innerB.should(f.match().field(theTerms.get(0).field()).matching(pid))); + innerB.minimumShouldMatchNumber(1); + parentPids.forEach(pid -> innerB.should(f.match().field("myId").matching(pid))); })); + } - private List convertTermsToParentPids(List theTerms) { - return theTerms.stream().map(Term::text).map(Long::valueOf).collect(Collectors.toList()); + /** + * We are looking for codes which have codes indicated in theFilter.getValue() as descendants. + * Strategy is to find codes which have their pId(s) in the list of the parentId(s) of all the TermConcept(s) + * representing the codes in theFilter.getValue() + */ + private void addLoincFilterDescendantIn(String theSystem, SearchPredicateFactory f, + BooleanPredicateClausesStep b, ValueSet.ConceptSetFilterComponent theFilter) { + + String[] values = theFilter.getValue().split(","); + if (values.length == 0) { + throw new InvalidRequestException(Msg.code(2037) + "Invalid filter criteria - no codes specified"); + } + + List descendantCodePidList = getMultipleCodeParentPids(theSystem, theFilter.getProperty(), values); + + b.must(f.bool(innerB -> descendantCodePidList.forEach( + pId -> innerB.should(f.match().field("myId").matching(pId)) + ))); } - private List getDescendantTerms(String theSystem, String theProperty, String theValue) { - List retVal = new ArrayList<>(); + /** + * Returns the list of parentId(s) of the TermConcept representing theValue as a code + */ + private List getCodeParentPids(String theSystem, String theProperty, String theValue) { TermConcept code = findCode(theSystem, theValue) - .orElseThrow(() -> new InvalidRequestException("Invalid filter criteria - code does not exist: {" + Constants.codeSystemWithDefaultDescription(theSystem) + "}" + theValue)); + .orElseThrow(() -> new InvalidRequestException("Invalid filter criteria - code does not exist: {" + + Constants.codeSystemWithDefaultDescription(theSystem) + "}" + theValue)); String[] parentPids = code.getParentPidsAsString().split(" "); - for (String parentPid : parentPids) { - if (!StringUtils.equals(parentPid, "NONE")) { - retVal.add(new Term("myId", parentPid)); - } - } + List retVal = Arrays.stream(parentPids) + .filter( pid -> !StringUtils.equals(pid, "NONE") ) + .map(Long::parseLong) + .collect(Collectors.toList()); logFilteringValueOnProperty(theValue, theProperty); + return retVal; + } + + + /** + * Returns the list of parentId(s) of the TermConcept representing theValue as a code + */ + private List getMultipleCodeParentPids(String theSystem, String theProperty, String[] theValues) { + List valuesList = Arrays.asList(theValues); + List termConcepts = findCodes(theSystem, valuesList); + if (valuesList.size() != termConcepts.size()) { + String exMsg = getTermConceptsFetchExceptionMsg(termConcepts, valuesList); + throw new InvalidRequestException(Msg.code(2038) + "Invalid filter criteria - {" + + Constants.codeSystemWithDefaultDescription(theSystem) + "}: " + exMsg); + } + + List retVal = termConcepts.stream() + .flatMap(tc -> Arrays.stream(tc.getParentPidsAsString().split(" "))) + .filter( pid -> !StringUtils.equals(pid, "NONE") ) + .map(Long::parseLong) + .collect(Collectors.toList()); + + logFilteringValueOnProperties(valuesList, theProperty); return retVal; } + /** + * Generate message indicating for which of theValues a TermConcept was not found + */ + private String getTermConceptsFetchExceptionMsg(List theTermConcepts, List theValues) { + // case: more TermConcept(s) retrieved than codes queried + if (theTermConcepts.size() > theValues.size()) { + return "Invalid filter criteria - More TermConcepts were found than indicated codes. Queried codes: [" + + join(",", theValues + "]; Obtained TermConcept IDs, codes: [" + + theTermConcepts.stream().map(tc -> tc.getId() + ", " + tc.getCode()) + .collect(joining("; "))+ "]"); + } + + // case: less TermConcept(s) retrieved than codes queried + Set matchedCodes = theTermConcepts.stream().map(TermConcept::getCode).collect(toSet()); + List notMatchedValues = theValues.stream() + .filter(v -> ! matchedCodes.contains (v)) .collect(toList()); + + return "Invalid filter criteria - No TermConcept(s) were found for the requested codes: [" + + join(",", notMatchedValues + "]"); + } + + private void logFilteringValueOnProperty(String theValue, String theProperty) { ourLog.debug(" * Filtering with value={} on property {}", theValue, theProperty); } + private void logFilteringValueOnProperties(List theValues, String theProperty) { + ourLog.debug(" * Filtering with values={} on property {}", String.join(", ", theValues), theProperty); + } + private void throwInvalidRequestForOpOnProperty(ValueSet.FilterOperator theOp, String theProperty) { throw new InvalidRequestException(Msg.code(897) + "Don't know how to handle op=" + theOp + " on property " + theProperty); } @@ -1635,6 +1725,16 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { }); } + + @Transactional(propagation = Propagation.MANDATORY) + public List findCodes(String theCodeSystem, List theCodeList) { + TermCodeSystemVersion csv = getCurrentCodeSystemVersion(theCodeSystem); + if (csv == null) { return Collections.emptyList(); } + + return myConceptDao.findByCodeSystemAndCodeList(csv, theCodeList); + } + + @Nullable private TermCodeSystemVersion getCurrentCodeSystemVersion(String theCodeSystemIdentifier) { String version = getVersionFromIdentifier(theCodeSystemIdentifier); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java index dd98372f865..bea2271b025 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java @@ -74,6 +74,8 @@ public interface ITermReadSvc extends IValidationSupport { Optional findCode(String theCodeSystem, String theCode); + List findCodes(String theCodeSystem, List theCodes); + Set findCodesAbove(Long theCodeSystemResourcePid, Long theCodeSystemResourceVersionPid, String theCode); List findCodesAbove(String theSystem, String theCode); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestHibernateSearchAddInConfig.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestHibernateSearchAddInConfig.java index f6b7319f603..034e26b66fe 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestHibernateSearchAddInConfig.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestHibernateSearchAddInConfig.java @@ -23,13 +23,15 @@ import org.testcontainers.elasticsearch.ElasticsearchContainer; import javax.annotation.PreDestroy; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import java.util.Properties; /** - * Configurations for Hibernate Search: off, lucene, or elastic. - * + * Configurations for Hibernate Search: off, lucene in-memory, lucene on file system or elastic. + * * We use {@link DefaultLuceneHeap} by default in our JPA test configs. * Turn off by adding {@link NoFT} to the test Contexts. * Use Elasticsearch instead via docker by adding {@link Elasticsearch} to the test Contexts; @@ -44,10 +46,48 @@ public class TestHibernateSearchAddInConfig { void apply(Properties theJPAProperties); } + /** + * Lucene on file system. Useful for debugging + * Uses temporary directory by default. Replace by permanent directory for debugging + */ + @Configuration + public static class LuceneFilesystem { + + @Bean + @Primary + IHibernateSearchConfigurer hibernateSearchConfigurer() throws IOException { + ourLog.warn("Hibernate Search: using lucene - filesystem"); + + // replace by existing directory for debugging purposes + Path tempDirPath = Files.createTempDirectory(null); + String dirPath = tempDirPath.toString(); + + Map luceneProperties = new HashMap<>(); + luceneProperties.put(BackendSettings.backendKey(BackendSettings.TYPE), "lucene"); + luceneProperties.put(BackendSettings.backendKey(LuceneBackendSettings.ANALYSIS_CONFIGURER), HapiLuceneAnalysisConfigurer.class.getName()); + luceneProperties.put(BackendSettings.backendKey(LuceneIndexSettings.DIRECTORY_TYPE), "local-filesystem"); + luceneProperties.put(BackendSettings.backendKey(LuceneIndexSettings.DIRECTORY_ROOT), dirPath); + ourLog.info("Using lucene root dir: {}", dirPath); + luceneProperties.put(BackendSettings.backendKey(LuceneBackendSettings.LUCENE_VERSION), "LUCENE_CURRENT"); + // for lucene trace logging + luceneProperties.put(BackendSettings.backendKey(LuceneIndexSettings.IO_WRITER_INFOSTREAM), "true"); + luceneProperties.put(HibernateOrmMapperSettings.ENABLED, "true"); + + return (theProperties) -> + theProperties.putAll(luceneProperties); + } + + + @Bean(name={"searchDao", "searchDaoDstu2", "searchDaoDstu3", "searchDaoR4", "searchDaoR5"}) + public IFulltextSearchSvc searchDao() { + ourLog.info("Hibernate Search: FulltextSearchSvcImpl present"); + return new FulltextSearchSvcImpl(); + } + } + + /** * Our default config - Lucene in-memory. - * - * Override by adding {@link NoFT} or {@link Elasticsearch} to your test class contexts. */ @Configuration public static class DefaultLuceneHeap { @@ -62,7 +102,7 @@ public class TestHibernateSearchAddInConfig { luceneHeapProperties.put(BackendSettings.backendKey(LuceneIndexSettings.DIRECTORY_TYPE), "local-heap"); luceneHeapProperties.put(BackendSettings.backendKey(LuceneBackendSettings.LUCENE_VERSION), "LUCENE_CURRENT"); luceneHeapProperties.put(HibernateOrmMapperSettings.ENABLED, "true"); - + return (theProperties) -> theProperties.putAll(luceneHeapProperties); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/AbstractValeSetFreeTextExpansionR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/AbstractValeSetFreeTextExpansionR4Test.java new file mode 100644 index 00000000000..67af4004371 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/AbstractValeSetFreeTextExpansionR4Test.java @@ -0,0 +1,1913 @@ +package ca.uhn.fhir.jpa.term; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoCodeSystem; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoValueSet; +import ca.uhn.fhir.jpa.api.dao.IFhirSystemDao; +import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc; +import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportSvc; +import ca.uhn.fhir.jpa.dao.BaseJpaTest; +import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; +import ca.uhn.fhir.jpa.dao.data.ITermCodeSystemDao; +import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; +import ca.uhn.fhir.jpa.entity.TermConcept; +import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; +import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; +import ca.uhn.fhir.jpa.term.api.ITermCodeSystemStorageSvc; +import ca.uhn.fhir.jpa.term.api.ITermDeferredStorageSvc; +import ca.uhn.fhir.jpa.term.api.ITermReadSvcR4; +import ca.uhn.fhir.jpa.term.custom.CustomTerminologySet; +import ca.uhn.fhir.parser.StrictErrorHandler; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.CodeableConcept; +import org.hl7.fhir.r4.model.Coding; +import org.hl7.fhir.r4.model.ValueSet; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.mockito.Answers; +import org.mockito.Mock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.test.util.AopTestUtils; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Transactional; + +import javax.persistence.EntityManager; +import java.util.ArrayList; +import java.util.List; + +import static ca.uhn.fhir.jpa.term.api.ITermCodeSystemStorageSvc.MAKE_LOADING_VERSION_CURRENT; +import static ca.uhn.fhir.jpa.term.api.ITermLoaderSvc.LOINC_URI; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_LOW; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.anyCollection; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * These tests are executed from child classes with different configurations + * In case you need to run a specific test, uncomment the @ExtendWith and one of the following configurations + * and remove the abstract qualifier + */ +//@ExtendWith(SpringExtension.class) +//@ContextConfiguration(classes = {TestR4Config.class, TestHibernateSearchAddInConfig.DefaultLuceneHeap.class}) +//@ContextConfiguration(classes = {TestR4Config.class, TestHibernateSearchAddInConfig.DefaultLuceneHeap.class}) +public abstract class AbstractValeSetFreeTextExpansionR4Test extends BaseJpaTest { + private static final Logger ourLog = LoggerFactory.getLogger(AbstractValeSetFreeTextExpansionR4Test.class); + + private static final String CS_URL = "http://example.com/my_code_system"; + private static final String CS_URL_2 = "http://example.com/my_code_system2"; + private static final String CS_URL_3 = "http://example.com/my_code_system3"; + + @Autowired FhirContext myFhirContext; + @Autowired PlatformTransactionManager myTxManager; + + @Autowired + private EntityManager myEntityManager; + + @Autowired + protected ITermCodeSystemDao myTermCodeSystemDao; + + @Autowired + protected DaoConfig myDaoConfig; + + @Autowired + @Qualifier("myCodeSystemDaoR4") + protected IFhirResourceDaoCodeSystem myCodeSystemDao; + + @Autowired + protected IResourceTableDao myResourceTableDao; + + @Autowired + protected ITermCodeSystemStorageSvc myTermCodeSystemStorageSvc; + + @Autowired + @Qualifier("myValueSetDaoR4") + protected IFhirResourceDaoValueSet myValueSetDao; + + @Autowired + protected ITermReadSvcR4 myTermSvc; + + @Autowired + protected ITermDeferredStorageSvc myTerminologyDeferredStorageSvc; + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + protected ServletRequestDetails mySrd; + + @Autowired + private IFhirSystemDao mySystemDao; + + @Autowired + private IResourceReindexingSvc myResourceReindexingSvc; + + @Autowired + private ISearchCoordinatorSvc mySearchCoordinatorSvc; + + @Autowired + private ISearchParamRegistry mySearchParamRegistry; + + @Autowired + private IBulkDataExportSvc myBulkDataExportSvc; + + @Mock + private IValueSetConceptAccumulator myValueSetCodeAccumulator; + + + @BeforeEach + public void beforeEach() { + when(mySrd.getUserData().getOrDefault(MAKE_LOADING_VERSION_CURRENT, Boolean.TRUE)).thenReturn(Boolean.TRUE); + myFhirContext.setParserErrorHandler(new StrictErrorHandler()); + + purgeHibernateSearch(myEntityManager); + + myDaoConfig.setSchedulingDisabled(true); + myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.ENABLED); + } + + @BeforeEach + @Transactional() + public void beforePurgeDatabase() { + purgeDatabase(myDaoConfig, mySystemDao, myResourceReindexingSvc, mySearchCoordinatorSvc, mySearchParamRegistry, myBulkDataExportSvc); + } + + @AfterEach + public void after() { + myDaoConfig.setDeferIndexingForCodesystemsOfSize(new DaoConfig().getDeferIndexingForCodesystemsOfSize()); + TermReindexingSvcImpl.setForceSaveDeferredAlwaysForUnitTest(false); + myDaoConfig.setMaximumExpansionSize(DaoConfig.DEFAULT_MAX_EXPANSION_SIZE); + purgeDatabase(myDaoConfig, mySystemDao, myResourceReindexingSvc, mySearchCoordinatorSvc, mySearchParamRegistry, myBulkDataExportSvc); + } + + @AfterEach() + public void afterCleanupDao() { + myDaoConfig.setExpireSearchResults(new DaoConfig().isExpireSearchResults()); + myDaoConfig.setExpireSearchResultsAfterMillis(new DaoConfig().getExpireSearchResultsAfterMillis()); + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + myDaoConfig.setSuppressUpdatesWithNoChange(new DaoConfig().isSuppressUpdatesWithNoChange()); + } + + @AfterEach + public void afterResetInterceptors() { + myInterceptorRegistry.unregisterAllInterceptors(); + } + + @AfterEach + public void afterClearTerminologyCaches() { + BaseTermReadSvcImpl baseHapiTerminologySvc = AopTestUtils.getTargetObject(myTermSvc); + baseHapiTerminologySvc.clearCaches(); + TermConceptMappingSvcImpl.clearOurLastResultsFromTranslationCache(); + TermConceptMappingSvcImpl.clearOurLastResultsFromTranslationWithReverseCache(); + TermDeferredStorageSvcImpl deferredSvc = AopTestUtils.getTargetObject(myTerminologyDeferredStorageSvc); + deferredSvc.clearDeferred(); + } + + + @Nested + public class TestExpandLoincValueSetFilter { + + @Test + public void testCopyrightWithExclude3rdParty() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("3rdParty"); // mixed case + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("3rdparty"); // lowercase + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4")); + } + + @Test + public void testCopyrightWithExcludeLoinc() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("LOINC"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue(LOINC_LOW); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("47239-9")); + } + + @Test + public void testCopyrightWithInclude3rdParty() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("3rdParty"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("47239-9")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("3rdparty"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("47239-9")); + } + + @Test + public void testCopyrightWithIncludeLoinc() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("LOINC"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue(LOINC_LOW); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4")); + } + + @Test + public void testCopyrightWithUnsupportedOp() { + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.ISA) + .setValue("LOINC"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(897) + "Don't know how to handle op=ISA on property copyright", e.getMessage()); + } + } + + @Test + public void testCopyrightWithUnsupportedSystem() { + createCodeSystem(); + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("LOINC"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(895) + "Invalid filter, property copyright is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage()); + } + + } + + @Test + public void testCopyrightWithUnsupportedValue() { + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("copyright") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("bogus"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(898) + "Don't know how to handle value=bogus on property copyright", e.getMessage()); + } + + } + + @Test + public void testAncestorWithExcludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4", "47239-9")); + } + + @Test + public void testAncestorWithExcludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + } + + @Test + public void testAncestorWithIncludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(0, outcome.getExpansion().getContains().size()); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(0, outcome.getExpansion().getContains().size()); + } + + @Test + public void testAncestorWithIncludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9")); + } + + @Test + public void testAncestorWithUnsupportedOp() { + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.ISA) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(892) + "Don't know how to handle op=ISA on property ancestor", e.getMessage()); + } + + } + + @Test + public void testAncestorWithUnsupportedSystem() { + createCodeSystem(); + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("ancestor") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(895) + "Invalid filter, property ancestor is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage()); + } + + } + + @Test + public void testChildWithExcludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-4", "47239-9")); + } + + @Test + public void testChildWithExcludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-4", "47239-9")); + } + + @Test + public void testChildWithIncludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(0, outcome.getExpansion().getContains().size()); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3")); + } + + @Test + public void testChildWithIncludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3")); + } + + @Test + public void testChildWithUnsupportedOp() { + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.ISA) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(893) + "Don't know how to handle op=ISA on property child", e.getMessage()); + } + + } + + @Test + public void testChildWithUnsupportedSystem() { + createCodeSystem(); + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("child") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(895) + "Invalid filter, property child is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage()); + } + + } + + @Test + public void testDescendantWithExcludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-4", "47239-9")); + } + + @Test + public void testDescendantWithExcludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + + assertThat(codes.toString(), codes, containsInAnyOrder("43343-4", "47239-9")); + } + + @Test + public void testDescendantWithIncludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(0, outcome.getExpansion().getContains().size()); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3")); + } + + @Test + public void testDescendantWithIncludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3")); + } + + @Test + public void testDescendantWithUnsupportedOp() { + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.ISA) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(896) + "Don't know how to handle op=ISA on property descendant", e.getMessage()); + } + + } + + @Test + public void testDescendantWithUnsupportedSystem() { + createCodeSystem(); + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("descendant") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(895) + "Invalid filter, property descendant is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage()); + } + + } + + @Test + public void testParentWithExcludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4", "47239-9")); + } + + @Test + public void testParentWithExcludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + // Exclude + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + } + + @Test + public void testParentWithIncludeAndEqual() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-3"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-4", "47239-9")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("43343-4"); + outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(0, outcome.getExpansion().getContains().size()); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(0, outcome.getExpansion().getContains().size()); + } + + @Test + public void testParentWithIncludeAndIn() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.IN) + .setValue("50015-7,43343-3,43343-4,47239-9"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9")); + } + + @Test + public void testParentWithUnsupportedOp() { + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.ISA) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(893) + "Don't know how to handle op=ISA on property parent", e.getMessage()); + } + + } + + @Test + public void testParentWithUnsupportedSystem() { + createCodeSystem(); + createLoincSystemWithSomeCodes(); + + ValueSet vs; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("parent") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("50015-7"); + + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InvalidRequestException e) { + assertEquals(Msg.code(895) + "Invalid filter, property parent is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage()); + } + + } + + + @Test + public void testExpandValueSetInMemoryRespectsMaxSize() { + createCodeSystem(); + + // Add lots more codes + CustomTerminologySet additions = new CustomTerminologySet(); + for (int i = 0; i < 100; i++) { + additions.addRootConcept("CODE" + i, "Display " + i); + } + myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd(CS_URL, additions); + + + // Codes available exceeds the max + myDaoConfig.setMaximumExpansionSize(50); + ValueSet vs = new ValueSet(); + ValueSet.ConceptSetComponent include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + try { + myTermSvc.expandValueSet(null, vs); + fail(); + } catch (InternalErrorException e) { + assertEquals(Msg.code(832) + "Expansion of ValueSet produced too many codes (maximum 50) - Operation aborted!", e.getMessage()); + } + + // Increase the max so it won't exceed + myDaoConfig.setMaximumExpansionSize(150); + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + ValueSet outcome = myTermSvc.expandValueSet(null, vs); + assertEquals(109, outcome.getExpansion().getContains().size()); + + } + + @Test + public void testExpandValueSetWithValueSetCodeAccumulator() { + createCodeSystem(); + + when(myValueSetCodeAccumulator.getCapacityRemaining()).thenReturn(100); + + ValueSet vs = new ValueSet(); + ValueSet.ConceptSetComponent include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + + myTermSvc.expandValueSet(null, vs, myValueSetCodeAccumulator); + verify(myValueSetCodeAccumulator, times(9)).includeConceptWithDesignations(anyString(), anyString(), nullable(String.class), anyCollection(), nullable(Long.class), nullable(String.class), nullable(String.class)); + } + + + + } + + + @Nested + public class TestExpandValueSetProperty { + @Test + public void testSearch() { + createCodeSystem(); + createCodeSystem2(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Property matches one code + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("propA") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("valueAAA"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("childAAA")); + + // Property matches several codes + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("propB") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("foo"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + + // Property matches no codes + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL_2); + include + .addFilter() + .setProperty("propA") + .setOp(ValueSet.FilterOperator.EQUAL) + .setValue("valueAAA"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, empty()); + } + + + @Test + public void testSearchWithRegexExclude() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue(".*\\^Donor$"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9")); + } + + @Test + public void testSearchWithRegexExcludeUsingOr() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent exclude; + + // Include + vs = new ValueSet(); + vs.getCompose() + .addInclude() + .setSystem(LOINC_URI); + + exclude = vs.getCompose().addExclude(); + exclude.setSystem(LOINC_URI); + exclude + .addFilter() + .setProperty("HELLO") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("12345-1|12345-2"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7", "47239-9")); + } + + @Test + public void testSearchWithRegexInclude() { + createLoincSystemWithSomeCodes(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue(".*\\^Donor$"); // <------ block diff is here + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("\\^Donor$"); // <------ block diff is here + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("\\^Dono$"); // <------ block diff is here + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, empty()); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("^Donor$"); // <------ block diff is here + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, empty()); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("\\^Dono"); // <------ block diff is here + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("50015-7")); + + // Include + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(LOINC_URI); + include + .addFilter() + .setProperty("SYSTEM") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("^Ser$"); // <------ block diff is here + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("43343-3", "43343-4")); + + } + + /** + * Test for fix to issue-2588 + */ + @Test + public void testRegexMatchesPropertyNameAndValue() { + createCodeSystem3(); + + List codes; + ValueSet vs; + ValueSet outcome; + ValueSet.ConceptSetComponent include; + + vs = new ValueSet(); + include = vs.getCompose().addInclude(); + include.setSystem(CS_URL); + include + .addFilter() + .setProperty("propB") + .setOp(ValueSet.FilterOperator.REGEX) + .setValue("^[No ]*IG exists$"); + outcome = myTermSvc.expandValueSet(null, vs); + codes = toCodesContains(outcome.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("childAAC", "childAAD")); + + } + + } + + + + + + + private IIdType createCodeSystem() { + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl(CS_URL); + codeSystem.setContent(CodeSystem.CodeSystemContentMode.NOTPRESENT); + codeSystem.setName("SYSTEM NAME"); + IIdType id = myCodeSystemDao.create(codeSystem, mySrd).getId().toUnqualified(); + + ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(IllegalArgumentException::new); + + TermCodeSystemVersion cs = new TermCodeSystemVersion(); + cs.setResource(table); + + TermConcept parent; + parent = new TermConcept(cs, "ParentWithNoChildrenA"); + cs.getConcepts().add(parent); + parent = new TermConcept(cs, "ParentWithNoChildrenB"); + cs.getConcepts().add(parent); + parent = new TermConcept(cs, "ParentWithNoChildrenC"); + cs.getConcepts().add(parent); + + TermConcept parentA = new TermConcept(cs, "ParentA"); + cs.getConcepts().add(parentA); + + TermConcept childAA = new TermConcept(cs, "childAA"); + parentA.addChild(childAA, RelationshipTypeEnum.ISA); + + TermConcept childAAA = new TermConcept(cs, "childAAA"); + childAAA.addPropertyString("propA", "valueAAA"); + childAAA.addPropertyString("propB", "foo"); + childAA.addChild(childAAA, RelationshipTypeEnum.ISA); + + TermConcept childAAB = new TermConcept(cs, "childAAB"); + childAAB.addPropertyString("propA", "valueAAB"); + childAAB.addPropertyString("propB", "foo"); + childAAB.addDesignation() + .setLanguage("D1L") + .setUseSystem("D1S") + .setUseCode("D1C") + .setUseDisplay("D1D") + .setValue("D1V"); + childAA.addChild(childAAB, RelationshipTypeEnum.ISA); + + TermConcept childAB = new TermConcept(cs, "childAB"); + parentA.addChild(childAB, RelationshipTypeEnum.ISA); + + TermConcept parentB = new TermConcept(cs, "ParentB"); + cs.getConcepts().add(parentB); + + myTermCodeSystemStorageSvc.storeNewCodeSystemVersion(new ResourcePersistentId(table.getId()), CS_URL, "SYSTEM NAME", null, cs, table); + + myTerminologyDeferredStorageSvc.saveAllDeferred(); + + return id; + } + + private void createCodeSystem2() { + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl(CS_URL_2); + codeSystem.setVersion("SYSTEM VERSION"); + codeSystem.setContent(CodeSystem.CodeSystemContentMode.NOTPRESENT); + IIdType id = myCodeSystemDao.create(codeSystem, mySrd).getId().toUnqualified(); + + ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(IllegalArgumentException::new); + + TermCodeSystemVersion cs = new TermCodeSystemVersion(); + cs.setResource(table); + + TermConcept parentA = new TermConcept(cs, "CS2"); + cs.getConcepts().add(parentA); + + myTermCodeSystemStorageSvc.storeNewCodeSystemVersion(new ResourcePersistentId(table.getId()), CS_URL_2, "SYSTEM NAME", "SYSTEM VERSION", cs, table); + + } + + private IIdType createCodeSystem3() { + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl(CS_URL_3); + codeSystem.setContent(CodeSystem.CodeSystemContentMode.NOTPRESENT); + codeSystem.setName("SYSTEM NAME 3"); + IIdType id = myCodeSystemDao.create(codeSystem, mySrd).getId().toUnqualified(); + + ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(IllegalArgumentException::new); + + TermCodeSystemVersion cs = new TermCodeSystemVersion(); + cs.setResource(table); + + TermConcept parent; + parent = new TermConcept(cs, "ParentWithNoChildrenA"); + cs.getConcepts().add(parent); + parent = new TermConcept(cs, "ParentWithNoChildrenB"); + cs.getConcepts().add(parent); + parent = new TermConcept(cs, "ParentWithNoChildrenC"); + cs.getConcepts().add(parent); + + TermConcept parentA = new TermConcept(cs, "ParentA"); + cs.getConcepts().add(parentA); + + TermConcept childAA = new TermConcept(cs, "childAA"); + parentA.addChild(childAA, RelationshipTypeEnum.ISA); + + TermConcept childAAA = new TermConcept(cs, "childAAA"); + childAAA.addPropertyString("propA", "valueAAA"); + childAAA.addPropertyString("propB", "foo"); + childAA.addChild(childAAA, RelationshipTypeEnum.ISA); + + TermConcept childAAB = new TermConcept(cs, "childAAB"); + childAAB.addPropertyString("propA", "valueAAB"); + childAAB.addPropertyString("propB", "foo"); + childAAB.addDesignation() + .setLanguage("D1L") + .setUseSystem("D1S") + .setUseCode("D1C") + .setUseDisplay("D1D") + .setValue("D1V"); + childAA.addChild(childAAB, RelationshipTypeEnum.ISA); + + TermConcept childAAC = new TermConcept(cs, "childAAC"); + childAAC.addPropertyString("propA", "valueAAC"); + childAAC.addPropertyString("propB", "No IG exists"); + childAA.addChild(childAAC, RelationshipTypeEnum.ISA); + + TermConcept childAAD = new TermConcept(cs, "childAAD"); + childAAD.addPropertyString("propA", "valueAAD"); + childAAD.addPropertyString("propB", "IG exists"); + childAA.addChild(childAAD, RelationshipTypeEnum.ISA); + + // this one shouldn't come up in search result because searched argument is not in searched property (propB) but in propA + TermConcept childAAE = new TermConcept(cs, "childAAE"); + childAAE.addPropertyString("propA", "IG exists"); + childAAE.addPropertyString("propB", "valueAAE"); + childAA.addChild(childAAE, RelationshipTypeEnum.ISA); + + TermConcept childAB = new TermConcept(cs, "childAB"); + parentA.addChild(childAB, RelationshipTypeEnum.ISA); + + TermConcept parentB = new TermConcept(cs, "ParentB"); + cs.getConcepts().add(parentB); + + myTermCodeSystemStorageSvc.storeNewCodeSystemVersion(new ResourcePersistentId(table.getId()), CS_URL, "SYSTEM NAME", null, cs, table); + + myTerminologyDeferredStorageSvc.saveAllDeferred(); + + return id; + } + + + public void createLoincSystemWithSomeCodes() { + runInTransaction(() -> { + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl(LOINC_URI); + codeSystem.setId("test-loinc"); + codeSystem.setVersion("SYSTEM VERSION"); + codeSystem.setContent(CodeSystem.CodeSystemContentMode.NOTPRESENT); + IIdType id = myCodeSystemDao.create(codeSystem).getId().toUnqualified(); + + ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(IllegalArgumentException::new); + + TermCodeSystemVersion cs = new TermCodeSystemVersion(); + cs.setResource(table); + + TermConcept code1 = new TermConcept(cs, "50015-7"); // has -3 as a child + TermConcept code2 = new TermConcept(cs, "43343-3"); // has -4 as a child + TermConcept code3 = new TermConcept(cs, "43343-4"); //has no children + TermConcept code4 = new TermConcept(cs, "47239-9"); //has no children + + code1.addPropertyString("SYSTEM", "Bld/Bone mar^Donor"); + code1.addPropertyCoding( + "child", + LOINC_URI, + code2.getCode(), + code2.getDisplay()); + code1.addChild(code2, RelationshipTypeEnum.ISA); + cs.getConcepts().add(code1); + + code2.addPropertyString("SYSTEM", "Ser"); + code2.addPropertyString("HELLO", "12345-1"); + code2.addPropertyCoding( + "parent", + LOINC_URI, + code1.getCode(), + code1.getDisplay()); + code2.addPropertyCoding( + "child", + LOINC_URI, + code3.getCode(), + code3.getDisplay()); + code2.addChild(code3, RelationshipTypeEnum.ISA); + code2.addPropertyCoding( + "child", + LOINC_URI, + code4.getCode(), + code4.getDisplay()); + code2.addChild(code4, RelationshipTypeEnum.ISA); + cs.getConcepts().add(code2); + + code3.addPropertyString("SYSTEM", "Ser"); + code3.addPropertyString("HELLO", "12345-2"); + code3.addPropertyCoding( + "parent", + LOINC_URI, + code2.getCode(), + code2.getDisplay()); + cs.getConcepts().add(code3); + + code4.addPropertyString("SYSTEM", "^Patient"); + code4.addPropertyString("EXTERNAL_COPYRIGHT_NOTICE", "Copyright © 2006 World Health Organization..."); + code4.addPropertyCoding( + "parent", + LOINC_URI, + code2.getCode(), + code2.getDisplay()); + cs.getConcepts().add(code4); + + myTermCodeSystemStorageSvc.storeNewCodeSystemVersion(new ResourcePersistentId(table.getId()), LOINC_URI, "SYSTEM NAME", "SYSTEM VERSION", cs, table); + }); + } + + private List toCodesContains(List theContains) { + List retVal = new ArrayList<>(); + + for (ValueSet.ValueSetExpansionContainsComponent next : theContains) { + retVal.add(next.getCode()); + } + + return retVal; + } + + + @Override + protected FhirContext getFhirContext() { + return myFhirContext; + } + + @Override + protected PlatformTransactionManager getTxManager() { + return myTxManager; + } +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplDstu3Test.java index ea0d2d94d94..803804b91f3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplDstu3Test.java @@ -347,7 +347,7 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test { .addFilter() .setProperty("copyright") .setOp(ValueSet.FilterOperator.EQUAL) - .setValue("3rdParty"); + .setValue("3rdParty"); // mixed case outcome = myTermSvc.expandValueSet(null, vs); codes = toCodesContains(outcome.getExpansion().getContains()); assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4")); @@ -364,7 +364,7 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test { .addFilter() .setProperty("copyright") .setOp(ValueSet.FilterOperator.EQUAL) - .setValue("3rdparty"); + .setValue("3rdparty"); // lowercase outcome = myTermSvc.expandValueSet(null, vs); codes = toCodesContains(outcome.getExpansion().getContains()); assertThat(codes, containsInAnyOrder("50015-7", "43343-3", "43343-4")); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4ElasticIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4ElasticIT.java new file mode 100644 index 00000000000..6fa70b7b30c --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4ElasticIT.java @@ -0,0 +1,16 @@ +package ca.uhn.fhir.jpa.term; + +import ca.uhn.fhir.jpa.config.TestHibernateSearchAddInConfig; +import ca.uhn.fhir.jpa.config.TestR4Config; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +/** + * This class runs all parent class tests using Elasticsearch configuration + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {TestR4Config.class, TestHibernateSearchAddInConfig.Elasticsearch.class}) +public class ValeSetFreeTextExpansionR4ElasticIT extends AbstractValeSetFreeTextExpansionR4Test { + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4LuceneIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4LuceneIT.java new file mode 100644 index 00000000000..4dfefb73910 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValeSetFreeTextExpansionR4LuceneIT.java @@ -0,0 +1,17 @@ +package ca.uhn.fhir.jpa.term; + +import ca.uhn.fhir.jpa.config.TestHibernateSearchAddInConfig; +import ca.uhn.fhir.jpa.config.TestR4Config; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +/** + * This class runs all parent class tests using Lucene configuration + * There is also a LuceneFilesystem configuration available, for debugging purposes + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {TestR4Config.class, TestHibernateSearchAddInConfig.DefaultLuceneHeap.class}) +public class ValeSetFreeTextExpansionR4LuceneIT extends AbstractValeSetFreeTextExpansionR4Test { + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionR4Test.java index 704bc802d47..f3151fe33c9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionR4Test.java @@ -61,12 +61,6 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.ArgumentMatchers.anyCollection; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class ValueSetExpansionR4Test extends BaseTermR4Test { private static final Logger ourLog = LoggerFactory.getLogger(ValueSetExpansionR4Test.class); @@ -978,53 +972,6 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test { assertThat(ValueSetTestUtil.toCodes(expandedValueSet), is(equalTo(expandedConceptCodes.subList(1, 23)))); } - @Test - public void testExpandValueSetInMemoryRespectsMaxSize() { - createCodeSystem(); - - // Add lots more codes - CustomTerminologySet additions = new CustomTerminologySet(); - for (int i = 0; i < 100; i++) { - additions.addRootConcept("CODE" + i, "Display " + i); - } - myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd(CS_URL, additions); - - - // Codes available exceeds the max - myDaoConfig.setMaximumExpansionSize(50); - ValueSet vs = new ValueSet(); - ValueSet.ConceptSetComponent include = vs.getCompose().addInclude(); - include.setSystem(CS_URL); - try { - ValueSet expansion = myTermSvc.expandValueSet(null, vs); - fail("Expanded to " + expansion.getExpansion().getContains().size() + " but max was " + myDaoConfig.getMaximumExpansionSize()); - } catch (InternalErrorException e) { - assertEquals(Msg.code(832) + "Expansion of ValueSet produced too many codes (maximum 50) - Operation aborted!", e.getMessage()); - } - - // Increase the max so it won't exceed - myDaoConfig.setMaximumExpansionSize(150); - vs = new ValueSet(); - include = vs.getCompose().addInclude(); - include.setSystem(CS_URL); - ValueSet outcome = myTermSvc.expandValueSet(null, vs); - assertEquals(109, outcome.getExpansion().getContains().size()); - - } - - @Test - public void testExpandValueSetWithValueSetCodeAccumulator() { - createCodeSystem(); - - when(myValueSetCodeAccumulator.getCapacityRemaining()).thenReturn(100); - - ValueSet vs = new ValueSet(); - ValueSet.ConceptSetComponent include = vs.getCompose().addInclude(); - include.setSystem(CS_URL); - - myTermSvc.expandValueSet(null, vs, myValueSetCodeAccumulator); - verify(myValueSetCodeAccumulator, times(9)).includeConceptWithDesignations(anyString(), anyString(), nullable(String.class), anyCollection(), nullable(Long.class), nullable(String.class), nullable(String.class)); - } @Test public void testExpandValueSetPreservesExplicitOrder() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java index c86f265ea99..f279ca15267 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java @@ -23,6 +23,7 @@ package ca.uhn.fhir.jpa.term.api; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.data.ITermValueSetDao; +import ca.uhn.fhir.jpa.entity.TermConcept; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.term.TermReadSvcR4; import ca.uhn.fhir.jpa.term.TermReadSvcUtil; @@ -41,7 +42,9 @@ import org.springframework.test.util.ReflectionTestUtils; import javax.persistence.EntityManager; import javax.persistence.NonUniqueResultException; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -232,4 +235,54 @@ class ITermReadSvcTest { } + + @Nested + public class TestGetMultipleCodeParentPids { + + public static final String CODE_1 = "code-1"; + public static final String CODE_2 = "code-2"; + public static final String CODE_3 = "code-3"; + public static final String CODE_4 = "code-4"; + public static final String CODE_5 = "code-5"; + + @Mock TermConcept termConceptCode1; + @Mock TermConcept termConceptCode3; + @Mock TermConcept termConceptCode4; + + @Test + public void morePropertiesThanValues() { + when(termConceptCode1.getCode()).thenReturn(CODE_1); + when(termConceptCode3.getCode()).thenReturn(CODE_3); + when(termConceptCode4.getCode()).thenReturn(CODE_4); + + List termConcepts = Lists.newArrayList(termConceptCode1, termConceptCode3, termConceptCode4); + List values = Arrays.asList(CODE_1, CODE_2, CODE_3, CODE_4, CODE_5); + String msg = (String) ReflectionTestUtils.invokeMethod( + testedClass, "getTermConceptsFetchExceptionMsg", termConcepts, values); + + assertTrue(msg.contains("No TermConcept(s) were found")); + assertFalse(msg.contains(CODE_1)); + assertTrue(msg.contains(CODE_2)); + assertFalse(msg.contains(CODE_3)); + assertFalse(msg.contains(CODE_4)); + assertTrue(msg.contains(CODE_5)); + } + + @Test + void moreValuesThanProperties() { + when(termConceptCode1.getCode()).thenReturn(CODE_1); + when(termConceptCode1.getId()).thenReturn(1L); + when(termConceptCode3.getCode()).thenReturn(CODE_3); + when(termConceptCode3.getId()).thenReturn(3L); + + List termConcepts = Lists.newArrayList(termConceptCode1, termConceptCode3); + List values = Arrays.asList(CODE_3); + String msg = (String) ReflectionTestUtils.invokeMethod( + testedClass, "getTermConceptsFetchExceptionMsg", termConcepts, values); + + assertTrue(msg.contains("More TermConcepts were found than indicated codes")); + assertFalse(msg.contains("Queried codes: [" + CODE_3 + "]")); + assertTrue(msg.contains("Obtained TermConcept IDs, codes: [1, code-1; 3, code-3]")); + } + } } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtil.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtil.java new file mode 100644 index 00000000000..88728ff4855 --- /dev/null +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtil.java @@ -0,0 +1,74 @@ +package ca.uhn.fhir.jpa.search; + +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; + +/** + * The whole purpose of his class is to ease construction of a non-trivial gson.JsonObject, + * which can't be done the easy way in this case (using a JSON string), because there are + * valid regex strings which break gson, as this: ".*\\^Donor$" + */ +public class ElasticsearchNestedQueryBuilderUtil { + + private final String myNestedObjName; + private final String myNestedKeyPropName; + private final String myNestedKeyPropValue; + private final String myNestedValuePropName; + private final String myNestedValuePropValue; + + private final String myNestedPropertyKeyPath; + private final String myNestedPropertyValuePath; + + private JsonObject builtJsonObj = new JsonObject(); + + public ElasticsearchNestedQueryBuilderUtil(String theNestedObjName, String theNestedKeyPropName, + String theNestedKeyPropValue, String theNestedValuePropName, String theNestedValuePropValue) { + + myNestedObjName = theNestedObjName ; + myNestedKeyPropName = theNestedKeyPropName; + myNestedKeyPropValue = theNestedKeyPropValue; + myNestedValuePropName = theNestedValuePropName; + myNestedValuePropValue = theNestedValuePropValue; + + myNestedPropertyKeyPath = myNestedObjName + "." + myNestedKeyPropName; + myNestedPropertyValuePath = myNestedObjName + "." + myNestedValuePropName; + + buildJsonObj(); + } + + + private void buildJsonObj() { + JsonObject matchPropJO = new JsonObject(); + matchPropJO.addProperty(myNestedObjName + "." + myNestedKeyPropName, myNestedKeyPropValue); + JsonObject matchJO = new JsonObject(); + matchJO.add("match", matchPropJO); + + JsonObject regexpPropJO = new JsonObject(); + regexpPropJO.addProperty(myNestedObjName + "." + myNestedValuePropName, myNestedValuePropValue); + JsonObject regexpJO = new JsonObject(); + regexpJO.add("regexp", regexpPropJO); + + JsonArray mustPropJA = new JsonArray(); + mustPropJA.add(matchJO); + mustPropJA.add(regexpJO); + + JsonObject mustPropJO = new JsonObject(); + mustPropJO.add("must", mustPropJA); + + JsonObject boolJO = new JsonObject(); + boolJO.add("bool", mustPropJO); + + JsonObject nestedJO = new JsonObject(); + nestedJO.addProperty("path", myNestedObjName); + nestedJO.add("query", boolJO); + + builtJsonObj.add("nested", nestedJO); + } + + public JsonObject toGson() { return builtJsonObj; } + + public String getNestedPropertyKeyPath() { return myNestedPropertyKeyPath; } + + public String getNestedPropertyValuePath() { return myNestedPropertyValuePath; } + +} diff --git a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtilTest.java b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtilTest.java new file mode 100644 index 00000000000..d95ef6f099b --- /dev/null +++ b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/search/ElasticsearchNestedQueryBuilderUtilTest.java @@ -0,0 +1,87 @@ +package ca.uhn.fhir.jpa.search; + +import com.google.gson.Gson; +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import com.google.gson.JsonSyntaxException; +import com.google.gson.stream.MalformedJsonException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ElasticsearchNestedQueryBuilderUtilTest { + + private static final String NESTED_KEY_PROP_NAME = "myKey"; + private static final String NESTED_VALUE_PROP_NAME = "myValueString"; + private static final String NESTED_OBJECT_NAME = "myProperties"; + + // a valid regex string which breaks gson + private static final String GSON_FAILING_REGEX = ".*\\^Donor$"; + + + private ElasticsearchNestedQueryBuilderUtil testedUtil; + + + @Test + void testBuildUsingTestedClass() { + String propertyValue = "propAAA"; + + testedUtil = new ElasticsearchNestedQueryBuilderUtil( + NESTED_OBJECT_NAME, NESTED_KEY_PROP_NAME, propertyValue, NESTED_VALUE_PROP_NAME, GSON_FAILING_REGEX); + + assertFalse(testedUtil.toGson().isJsonNull()); + + JsonObject generatedJson = testedUtil.toGson(); + JsonObject nestedJO = generatedJson.getAsJsonObject("nested"); + assertEquals(NESTED_OBJECT_NAME, nestedJO.get("path").getAsString()); + + JsonObject queryJO = nestedJO.getAsJsonObject("query"); + JsonObject boolJO = queryJO.getAsJsonObject("bool"); + JsonArray mustJA = boolJO.getAsJsonArray("must"); + + JsonObject matchPropKeyJE = (JsonObject) mustJA.get(0); + JsonObject propKeyJO = matchPropKeyJE.getAsJsonObject("match"); + assertEquals(propertyValue, propKeyJO.get(testedUtil.getNestedPropertyKeyPath()).getAsString()); + + JsonObject regexpPropValueJE = (JsonObject) mustJA.get(1); + JsonObject propValueJO = regexpPropValueJE.getAsJsonObject("regexp"); + assertEquals(GSON_FAILING_REGEX, propValueJO.get(testedUtil.getNestedPropertyValuePath()).getAsString()); + } + + + /** + * This test demonstrates that valid regex strings break gson library. + * If one day this test fails would mean the library added support for this kind of strings + * so the tested class could be removed and the code using it just build a gson.JsonObject from a JSON string + */ + @Test + void testShowGsonLibFailing() { + String workingRegex = ".*[abc]?jk"; + String workingNestedQuery = + "{'nested': { 'path': 'myProperties', 'query': { 'bool': { 'must': [" + + "{'match': {'myProperties.myKey': 'propAAA' }}," + + "{'regexp': {'myProperties.myValueString': '" + workingRegex + "'}}" + + "]}}}}"; + + JsonObject jsonObj = new Gson().fromJson(workingNestedQuery, JsonObject.class); + assertFalse(jsonObj.isJsonNull()); + + // same json structure fails with some valid regex strings + String nestedQuery = + "{'nested': { 'path': 'myProperties', 'query': { 'bool': { 'must': [" + + "{'match': {'myProperties.myKey': 'propAAA' }}," + + "{'regexp': {'myProperties.myValueString': '" + GSON_FAILING_REGEX + "'}}" + + "]}}}}"; + +// MalformedJsonException thrown = assertThrows( + JsonSyntaxException thrown = assertThrows( + JsonSyntaxException.class, + () -> new Gson().fromJson(nestedQuery, JsonObject.class)); + + assertTrue(thrown.getCause() instanceof MalformedJsonException); + } + +}