From f934f76c9606f642d7af4b15552def209feb94f3 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 11 Mar 2019 15:49:34 -0400 Subject: [PATCH] Several JPA search fixes (#1231) * Search fixes * Add some tests * CHangelog * Some cleanup of the query tracker * FIx XML issue in changelog * Test fixes * SOme test fixes * Address review comments * Fix test breakage --- .../fhir/rest/param/StringAndListParam.java | 6 +- .../fhir/rest/param/TokenAndListParam.java | 8 +- hapi-fhir-jpaserver-base/pom.xml | 20 +- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 78 +++---- .../jpa/util/BaseCaptureQueriesListener.java | 95 +++++++++ .../CircularQueueCaptureQueriesListener.java | 103 ++++++++++ .../jpa/config/CaptureQueriesListener.java | 92 --------- .../uhn/fhir/jpa/config/TestDstu2Config.java | 11 +- .../uhn/fhir/jpa/config/TestDstu3Config.java | 7 + .../ca/uhn/fhir/jpa/config/TestR4Config.java | 8 +- .../java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java | 9 +- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 3 + .../dao/r4/FhirResourceDaoR4SearchFtTest.java | 1 + .../r4/FhirResourceDaoR4SearchNoFtTest.java | 192 +++++++++++++----- .../FhirResourceDaoR4SearchNoHashesTest.java | 23 +-- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 74 ------- pom.xml | 2 +- src/changes/changes.xml | 5 + 18 files changed, 443 insertions(+), 294 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/BaseCaptureQueriesListener.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringAndListParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringAndListParam.java index f09d4fe3152..d6d97c97c7b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringAndListParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringAndListParam.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.rest.param; +import ca.uhn.fhir.model.api.IQueryParameterOr; import ca.uhn.fhir.util.CoverageIgnore; /* @@ -30,11 +31,14 @@ public class StringAndListParam extends BaseAndListParam { return new StringOrListParam(); } - @CoverageIgnore @Override public StringAndListParam addAnd(StringOrListParam theValue) { addValue(theValue); return this; } + public StringAndListParam addAnd(StringParam theValue) { + addValue(new StringOrListParam().addOr(theValue)); + return this; + } } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenAndListParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenAndListParam.java index 98b60e55b4a..bb88b131ab2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenAndListParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenAndListParam.java @@ -1,6 +1,6 @@ package ca.uhn.fhir.rest.param; -import ca.uhn.fhir.util.CoverageIgnore; +import org.apache.commons.lang3.Validate; /* * #%L @@ -30,11 +30,15 @@ public class TokenAndListParam extends BaseAndListParam { return new TokenOrListParam(); } - @CoverageIgnore @Override public TokenAndListParam addAnd(TokenOrListParam theValue) { addValue(theValue); return this; } + public TokenAndListParam addAnd(TokenParam theValue) { + Validate.notNull(theValue, "theValue must not be null"); + addValue(new TokenOrListParam().add(theValue)); + return this; + } } diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 7cb3f25e1e7..a31a9b236e5 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -126,23 +126,16 @@ ${project.version} - ch.qos.logback logback-classic test - - net.ttddyy - datasource-proxy - test - org.javassist @@ -460,6 +453,10 @@ com.google.guava guava + + org.apache.commons + commons-collections4 + org.eclipse.jetty @@ -523,11 +520,6 @@ greenmail-spring test - - org.apache.commons - commons-collections4 - test - com.github.ben-manes.caffeine diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 2d46a1f7e4f..0bf8a3a0aaf 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -91,6 +91,7 @@ import java.math.BigDecimal; import java.math.MathContext; import java.util.*; import java.util.Map.Entry; +import java.util.function.Function; import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.*; @@ -193,7 +194,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateDate(String theResourceName, String theParamName, List theList) { - Join join = createOrReuseJoin(JoinEnum.DATE, theParamName); + Join join = createJoin(JoinEnum.DATE, theParamName); if (theList.get(0).getMissing() != null) { Boolean missing = theList.get(0).getMissing(); @@ -301,7 +302,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateNumber(String theResourceName, String theParamName, List theList) { - Join join = createOrReuseJoin(JoinEnum.NUMBER, theParamName); + Join join = createJoin(JoinEnum.NUMBER, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -361,7 +362,7 @@ public class SearchBuilder implements ISearchBuilder { } private void addPredicateQuantity(String theResourceName, String theParamName, List theList) { - Join join = createOrReuseJoin(JoinEnum.QUANTITY, theParamName); + Join join = createJoin(JoinEnum.QUANTITY, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -389,7 +390,7 @@ public class SearchBuilder implements ISearchBuilder { return; } - Join join = createOrReuseJoin(JoinEnum.REFERENCE, theParamName); + Join join = createJoin(JoinEnum.REFERENCE, theParamName); List codePredicates = new ArrayList<>(); @@ -683,7 +684,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateString(String theResourceName, String theParamName, List theList) { - Join join = createOrReuseJoin(JoinEnum.STRING, theParamName); + Join join = createJoin(JoinEnum.STRING, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -831,13 +832,12 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateToken(String theResourceName, String theParamName, List theList) { if (theList.get(0).getMissing() != null) { - Join join = createOrReuseJoin(JoinEnum.TOKEN, theParamName); + Join join = createJoin(JoinEnum.TOKEN, theParamName); addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); return; } List codePredicates = new ArrayList<>(); - Join join = null; List tokens = new ArrayList<>(); for (IQueryParameterType nextOr : theList) { @@ -849,10 +849,6 @@ public class SearchBuilder implements ISearchBuilder { } } - if (join == null) { - join = createOrReuseJoin(JoinEnum.TOKEN, theParamName); - } - tokens.add(nextOr); } @@ -860,6 +856,7 @@ public class SearchBuilder implements ISearchBuilder { return; } + Join join = createJoin(JoinEnum.TOKEN, theParamName); List singleCode = createPredicateToken(tokens, theResourceName, theParamName, myBuilder, join); codePredicates.addAll(singleCode); @@ -869,7 +866,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateUri(String theResourceName, String theParamName, List theList) { - Join join = createOrReuseJoin(JoinEnum.URI, theParamName); + Join join = createJoin(JoinEnum.URI, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -1018,33 +1015,36 @@ public class SearchBuilder implements ISearchBuilder { @SuppressWarnings("unchecked") private Join createOrReuseJoin(JoinEnum theType, String theSearchParameterName) { JoinKey key = new JoinKey(theSearchParameterName, theType); - return (Join) myIndexJoins.computeIfAbsent(key, k -> { - Join join = null; - switch (theType) { - case DATE: - join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); - break; - case NUMBER: - join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); - break; - case QUANTITY: - join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); - break; - case REFERENCE: - join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); - break; - case STRING: - join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); - break; - case URI: - join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); - break; - case TOKEN: - join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); - break; - } - return join; - }); + return (Join) myIndexJoins.computeIfAbsent(key, k -> createJoin(theType, theSearchParameterName)); + } + + @SuppressWarnings("unchecked") + private Join createJoin(JoinEnum theType, String theSearchParameterName) { + Join join = null; + switch (theType) { + case DATE: + join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); + break; + case NUMBER: + join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); + break; + case QUANTITY: + join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); + break; + case REFERENCE: + join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); + break; + case STRING: + join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); + break; + case URI: + join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); + break; + case TOKEN: + join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); + break; + } + return (Join) join; } private Predicate createPredicateDate(IQueryParameterType theParam, String theResourceName, String theParamName, CriteriaBuilder theBuilder, From theFrom) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/BaseCaptureQueriesListener.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/BaseCaptureQueriesListener.java new file mode 100644 index 00000000000..4956cb229ca --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/BaseCaptureQueriesListener.java @@ -0,0 +1,95 @@ +package ca.uhn.fhir.jpa.util; + +import net.ttddyy.dsproxy.ExecutionInfo; +import net.ttddyy.dsproxy.QueryInfo; +import net.ttddyy.dsproxy.proxy.ParameterSetOperation; +import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; +import org.apache.commons.lang3.StringUtils; +import org.hibernate.engine.jdbc.internal.BasicFormatterImpl; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Queue; +import java.util.stream.Collectors; + +public abstract class BaseCaptureQueriesListener implements ProxyDataSourceBuilder.SingleQueryExecution { + + @Override + public void execute(ExecutionInfo theExecutionInfo, List theQueryInfoList) { + final Queue queryList = provideQueryList(); + for (QueryInfo next : theQueryInfoList) { + String sql = StringUtils.trim(next.getQuery()); + List params; + if (next.getParametersList().size() > 0 && next.getParametersList().get(0).size() > 0) { + List values = next + .getParametersList() + .get(0); + params = values.stream() + .map(t -> t.getArgs()[1]) + .map(t -> t != null ? t.toString() : "NULL") + .collect(Collectors.toList()); + } else { + params = Collections.emptyList(); + } + + long elapsedTime = theExecutionInfo.getElapsedTime(); + long startTime = System.currentTimeMillis() - elapsedTime; + queryList.add(new Query(sql, params, startTime, elapsedTime)); + } + } + + protected abstract Queue provideQueryList(); + + public static class Query { + private final String myThreadName = Thread.currentThread().getName(); + private final String mySql; + private final List myParams; + private final long myQueryTimestamp; + private final long myElapsedTime; + + Query(String theSql, List theParams, long theQueryTimestamp, long theElapsedTime) { + mySql = theSql; + myParams = Collections.unmodifiableList(theParams); + myQueryTimestamp = theQueryTimestamp; + myElapsedTime = theElapsedTime; + } + + public long getQueryTimestamp() { + return myQueryTimestamp; + } + + public long getElapsedTime() { + return myElapsedTime; + } + + public String getThreadName() { + return myThreadName; + } + + public String getSql(boolean theInlineParams, boolean theFormat) { + String retVal = mySql; + if (theFormat) { + retVal = new BasicFormatterImpl().format(retVal); + + // BasicFormatterImpl annoyingly adds a newline at the very start of its output + while (retVal.startsWith("\n")) { + retVal = retVal.substring(1); + } + } + + if (theInlineParams) { + List nextParams = new ArrayList<>(myParams); + while (retVal.contains("?") && nextParams.size() > 0) { + int idx = retVal.indexOf("?"); + retVal = retVal.substring(0, idx) + "'" + nextParams.remove(0) + "'" + retVal.substring(idx + 1); + } + } + + return retVal; + + } + + } + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java new file mode 100644 index 00000000000..0acc60b3636 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java @@ -0,0 +1,103 @@ +package ca.uhn.fhir.jpa.util; + +import ca.uhn.fhir.util.StopWatch; +import com.google.common.collect.Queues; +import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; +import org.apache.commons.collections4.queue.CircularFifoQueue; +import org.hl7.fhir.dstu3.model.InstantType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.*; +import java.util.stream.Collectors; + +/** + * This is a query listener designed to be plugged into a {@link ProxyDataSourceBuilder proxy DataSource}. + * This listener keeps the last 1000 queries across all threads in a {@link CircularFifoQueue}, dropping queries off the + * end of the list as new ones are added. + *

+ * Note that this class is really only designed for use in testing - It adds a non-trivial overhead + * to each query. + *

+ */ +public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListener { + + private static final int CAPACITY = 1000; + private static final Logger ourLog = LoggerFactory.getLogger(CircularQueueCaptureQueriesListener.class); + private final Queue myQueries = Queues.synchronizedQueue(new CircularFifoQueue<>(CAPACITY)); + + @Override + protected Queue provideQueryList() { + return myQueries; + } + + /** + * Clear all stored queries + */ + public void clear() { + myQueries.clear(); + } + + /** + * Index 0 is oldest + */ + @SuppressWarnings("UseBulkOperation") + public List getCapturedQueries() { + // Make a copy so that we aren't affected by changes to the list outside of the + // synchronized block + ArrayList retVal = new ArrayList<>(CAPACITY); + myQueries.forEach(retVal::add); + return Collections.unmodifiableList(retVal); + } + + /** + * Returns all SELECT queries executed on the current thread - Index 0 is oldest + */ + public List getSelectQueriesForCurrentThread() { + String currentThreadName = Thread.currentThread().getName(); + return getCapturedQueries() + .stream() + .filter(t -> t.getThreadName().equals(currentThreadName)) + .filter(t -> t.getSql(false, false).toLowerCase().contains("select")) + .collect(Collectors.toList()); + } + + /** + * Returns all INSERT queries executed on the current thread - Index 0 is oldest + */ + public List getInsertQueriesForCurrentThread() { + return getCapturedQueries() + .stream() + .filter(t -> t.getThreadName().equals(Thread.currentThread().getName())) + .filter(t -> t.getSql(false, false).toLowerCase().contains("insert")) + .collect(Collectors.toList()); + } + + /** + * Log all captured SELECT queries + */ + public void logSelectQueriesForCurrentThread() { + List queries = getSelectQueriesForCurrentThread() + .stream() + .map(CircularQueueCaptureQueriesListener::formatQueryAsSql) + .collect(Collectors.toList()); + ourLog.info("Select Queries:\n{}", String.join("\n", queries)); + } + + /** + * Log all captured INSERT queries + */ + public void logInsertQueriesForCurrentThread() { + List queries = getInsertQueriesForCurrentThread() + .stream() + .map(CircularQueueCaptureQueriesListener::formatQueryAsSql) + .collect(Collectors.toList()); + ourLog.info("Insert Queries:\n{}", String.join("\n", queries)); + } + + private static String formatQueryAsSql(Query theQuery) { + String formattedSql = theQuery.getSql(true, true); + return "Query at " + new InstantType(new Date(theQuery.getQueryTimestamp())).getValueAsString() + " took " + StopWatch.formatMillis(theQuery.getElapsedTime()) + " on Thread: " + theQuery.getThreadName() + "\nSQL:\n" + formattedSql; + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java deleted file mode 100644 index 9c4e84c32fe..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java +++ /dev/null @@ -1,92 +0,0 @@ -package ca.uhn.fhir.jpa.config; - -import net.ttddyy.dsproxy.ExecutionInfo; -import net.ttddyy.dsproxy.QueryInfo; -import net.ttddyy.dsproxy.proxy.ParameterSetOperation; -import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; -import org.hibernate.engine.jdbc.internal.BasicFormatterImpl; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedList; -import java.util.List; -import java.util.stream.Collectors; - -public class CaptureQueriesListener implements ProxyDataSourceBuilder.SingleQueryExecution { - - private static final LinkedList LAST_N_QUERIES = new LinkedList<>(); - - @Override - public void execute(ExecutionInfo execInfo, List queryInfoList) { - synchronized (LAST_N_QUERIES) { - for (QueryInfo next : queryInfoList) { - String sql = next.getQuery(); - List params; - if (next.getParametersList().size() > 0 && next.getParametersList().get(0).size() > 0) { - List values = next - .getParametersList() - .get(0); - params = values.stream() - .map(t -> t.getArgs()[1]) - .map(t -> t != null ? t.toString() : "NULL") - .collect(Collectors.toList()); - } else { - params = new ArrayList<>(); - } - LAST_N_QUERIES.add(0, new Query(sql, params)); - } - while (LAST_N_QUERIES.size() > 100) { - LAST_N_QUERIES.removeLast(); - } - } - } - - public static class Query { - private final String myThreadName = Thread.currentThread().getName(); - private final String mySql; - private final List myParams; - - Query(String theSql, List theParams) { - mySql = theSql; - myParams = Collections.unmodifiableList(theParams); - } - - public String getThreadName() { - return myThreadName; - } - - public String getSql(boolean theInlineParams, boolean theFormat) { - String retVal = mySql; - if (theFormat) { - retVal = new BasicFormatterImpl().format(retVal); - } - - if (theInlineParams) { - List nextParams = new ArrayList<>(myParams); - while (retVal.contains("?") && nextParams.size() > 0) { - int idx = retVal.indexOf("?"); - retVal = retVal.substring(0, idx) + "'" + nextParams.remove(0) + "'" + retVal.substring(idx + 1); - } - } - - return retVal; - - } - - } - - public static void clear() { - synchronized (LAST_N_QUERIES) { - LAST_N_QUERIES.clear(); - } - } - - /** - * Index 0 is newest! - */ - public static ArrayList getLastNQueries() { - synchronized (LAST_N_QUERIES) { - return new ArrayList<>(LAST_N_QUERIES); - } - } -} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu2Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu2Config.java index 422c7ecf663..597bbd13da6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu2Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu2Config.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.config; -import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.search.LuceneSearchMappingFactory; +import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.validation.ResultSeverityEnum; import net.ttddyy.dsproxy.listener.ThreadQueryCountHolder; @@ -13,12 +13,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Lazy; -import org.springframework.core.env.Environment; -import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; import org.springframework.transaction.annotation.EnableTransactionManagement; -import javax.persistence.EntityManagerFactory; import javax.sql.DataSource; import java.sql.Connection; import java.sql.SQLException; @@ -46,6 +43,11 @@ public class TestDstu2Config extends BaseJavaConfigDstu2 { private Exception myLastStackTrace; private String myLastStackTraceThreadName; + @Bean + public CircularQueueCaptureQueriesListener captureQueriesListener() { + return new CircularQueueCaptureQueriesListener(); + } + @Bean public DataSource dataSource() { BasicDataSource retVal = new BasicDataSource() { @@ -108,6 +110,7 @@ public class TestDstu2Config extends BaseJavaConfigDstu2 { .create(retVal) // .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) + .afterQuery(captureQueriesListener()) .countQuery(new ThreadQueryCountHolder()) .build(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java index ed7243b9b9e..178d2de710b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.config; import ca.uhn.fhir.jpa.search.LuceneSearchMappingFactory; import ca.uhn.fhir.jpa.subscription.module.subscriber.email.IEmailSender; import ca.uhn.fhir.jpa.subscription.module.subscriber.email.JavaMailEmailSender; +import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.validation.ResultSeverityEnum; import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; @@ -27,6 +28,11 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 { static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(TestDstu3Config.class); private Exception myLastStackTrace; + @Bean + public CircularQueueCaptureQueriesListener captureQueriesListener() { + return new CircularQueueCaptureQueriesListener(); + } + @Bean public BasicDataSource basicDataSource() { BasicDataSource retVal = new BasicDataSource() { @@ -100,6 +106,7 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 { .create(basicDataSource()) // .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") .logSlowQueryBySlf4j(1000, TimeUnit.MILLISECONDS) + .afterQuery(captureQueriesListener()) .countQuery() .build(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java index ac97e20cbdf..42a84d3862b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.config; +import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.validation.ResultSeverityEnum; import net.ttddyy.dsproxy.listener.SingleQueryCountHolder; @@ -40,6 +41,11 @@ public class TestR4Config extends BaseJavaConfigR4 { private Exception myLastStackTrace; + @Bean + public CircularQueueCaptureQueriesListener captureQueriesListener() { + return new CircularQueueCaptureQueriesListener(); + } + @Bean public DataSource dataSource() { BasicDataSource retVal = new BasicDataSource() { @@ -100,7 +106,7 @@ public class TestR4Config extends BaseJavaConfigR4 { // .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) // .countQuery(new ThreadQueryCountHolder()) .beforeQuery(new BlockLargeNumbersOfParamsListener()) - .afterQuery(new CaptureQueriesListener()) + .afterQuery(captureQueriesListener()) .countQuery(singleQueryCountHolder()) .build(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java index 3881d3fec9e..89e2d1998bf 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.jpa.config.CaptureQueriesListener; +import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.jpa.entity.TermConcept; import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; @@ -11,7 +11,6 @@ import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry; -import ca.uhn.fhir.jpa.subscription.dbmatcher.DaoSubscriptionMatcher; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.util.ExpungeOptions; import ca.uhn.fhir.jpa.util.JpaConstants; @@ -92,11 +91,15 @@ public abstract class BaseJpaTest { protected DatabaseBackedPagingProvider myDatabaseBackedPagingProvider; @Autowired protected IInterceptorRegistry myInterceptorRegistry; + @Autowired + protected CircularQueueCaptureQueriesListener myCaptureQueriesListener; @After public void afterPerformCleanup() { BaseHapiFhirResourceDao.setDisableIncrementOnUpdateForUnitTest(false); - CaptureQueriesListener.clear(); + if (myCaptureQueriesListener != null) { + myCaptureQueriesListener.clear(); + } } @After diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 12aa647ef87..f46ea757b26 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -100,6 +100,9 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { @Qualifier("myCommunicationDaoR4") protected IFhirResourceDao myCommunicationDao; @Autowired + @Qualifier("myCommunicationRequestDaoR4") + protected IFhirResourceDao myCommunicationRequestDao; + @Autowired @Qualifier("myCarePlanDaoR4") protected IFhirResourceDao myCarePlanDao; @Autowired diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java index f6c35f94633..680ec2b63c7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java @@ -10,6 +10,7 @@ import javax.servlet.http.HttpServletRequest; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao; +import ca.uhn.fhir.rest.api.server.IBundleProvider; import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.Observation.ObservationStatus; import org.hl7.fhir.instance.model.api.IIdType; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index bc18cd01ed3..705c1abdf4b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -1,6 +1,6 @@ package ca.uhn.fhir.jpa.dao.r4; -import ca.uhn.fhir.jpa.config.CaptureQueriesListener; +import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; @@ -62,6 +62,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { myDaoConfig.setFetchSizeDefaultMaximum(new DaoConfig().getFetchSizeDefaultMaximum()); myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches()); myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); + myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields()); } @Before @@ -489,7 +490,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { Bundle inputBundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, inputString); inputBundle.setType(BundleType.TRANSACTION); - Set allIds = new TreeSet(); + Set allIds = new TreeSet<>(); for (BundleEntryComponent nextEntry : inputBundle.getEntry()) { nextEntry.getRequest().setMethod(HTTPVerb.PUT); nextEntry.getRequest().setUrl(nextEntry.getResource().getId()); @@ -503,7 +504,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { IPrimitiveType count = new IntegerType(1000); IBundleProvider everything = myPatientDao.patientInstanceEverything(mySrd.getServletRequest(), new IdType("Patient/A161443"), count, null, null, null, null, mySrd); - TreeSet ids = new TreeSet(toUnqualifiedVersionlessIdValues(everything)); + TreeSet ids = new TreeSet<>(toUnqualifiedVersionlessIdValues(everything)); assertThat(ids, hasItem("List/A161444")); assertThat(ids, hasItem("List/A161468")); assertThat(ids, hasItem("List/A161500")); @@ -512,7 +513,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { ourLog.info("Actual {} - {}", ids.size(), ids); assertEquals(allIds, ids); - ids = new TreeSet(); + ids = new TreeSet<>(); for (int i = 0; i < everything.size(); i++) { for (IBaseResource next : everything.getResources(i, i + 1)) { ids.add(next.getIdElement().toUnqualifiedVersionless().getValue()); @@ -2215,8 +2216,8 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { map.setLoadSynchronous(true); myPatientDao.search(map); - List queries = CaptureQueriesListener - .getLastNQueries() + List queries = myCaptureQueriesListener + .getCapturedQueries() .stream() .map(t -> t.getSql(true, true)) .filter(t -> t.contains("select")) @@ -2238,31 +2239,22 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { p = new Patient(); p.addIdentifier().setSystem("SYS").setValue("BAZ"); myPatientDao.create(p); - CaptureQueriesListener.clear(); + myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_IDENTIFIER, new TokenOrListParam().addOr(new TokenParam("FOO")).addOr(new TokenParam("BAR"))); map.setLoadSynchronous(true); IBundleProvider search = myPatientDao.search(map); - List queries = CaptureQueriesListener - .getLastNQueries() - .stream() - .map(t -> t.getSql(true, true)) - .filter(t -> t.contains("select")) - .collect(Collectors.toList()); - String resultingQueryFormatted = queries.get(queries.size() - 1); - ourLog.info("Resulting query formatted:\n{}", resultingQueryFormatted); - - queries = CaptureQueriesListener - .getLastNQueries() + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + List queries = myCaptureQueriesListener + .getSelectQueriesForCurrentThread() .stream() .map(t -> t.getSql(true, false)) - .filter(t -> t.contains("select")) .collect(Collectors.toList()); - String resultingQueryNotFormatted = queries.get(queries.size() - 1); + String resultingQueryNotFormatted = queries.get(0); - assertEquals(resultingQueryFormatted, 1, StringUtils.countMatches(resultingQueryNotFormatted, "HASH_VALUE")); + assertEquals(resultingQueryNotFormatted, 1, StringUtils.countMatches(resultingQueryNotFormatted, "HASH_VALUE")); assertThat(resultingQueryNotFormatted, containsString("HASH_VALUE in ('3140583648400062149' , '4929264259256651518')")); // Ensure that the search actually worked @@ -2283,31 +2275,22 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { p = new Patient(); p.addIdentifier().setSystem("SAS").setValue("BAZ"); myPatientDao.create(p); - CaptureQueriesListener.clear(); + myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_IDENTIFIER, new TokenOrListParam().addOr(new TokenParam("SAS", null)).addOr(new TokenParam("FOO")).addOr(new TokenParam("BAR"))); map.setLoadSynchronous(true); IBundleProvider search = myPatientDao.search(map); - List queries = CaptureQueriesListener - .getLastNQueries() - .stream() - .map(t -> t.getSql(true, true)) - .filter(t -> t.contains("select")) - .collect(Collectors.toList()); - String resultingQueryFormatted = queries.get(queries.size() - 1); - ourLog.info("Resulting query formatted:\n{}", resultingQueryFormatted); - - queries = CaptureQueriesListener - .getLastNQueries() + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + List queries = myCaptureQueriesListener + .getSelectQueriesForCurrentThread() .stream() .map(t -> t.getSql(true, false)) - .filter(t -> t.contains("select")) .collect(Collectors.toList()); - String resultingQueryNotFormatted = queries.get(queries.size() - 1); + String resultingQueryNotFormatted = queries.get(0); - assertEquals(resultingQueryFormatted, 1, StringUtils.countMatches(resultingQueryNotFormatted, "HASH_VALUE")); + assertEquals(resultingQueryNotFormatted, 1, StringUtils.countMatches(resultingQueryNotFormatted, "HASH_VALUE")); assertThat(resultingQueryNotFormatted, containsString("HASH_VALUE in ('3140583648400062149' , '4929264259256651518')")); // Ensure that the search actually worked @@ -2509,20 +2492,17 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { ReferenceParam param3 = new ReferenceParam("valuec").setChain("code:text"); sp.add("medication", new ReferenceOrListParam().addOr(param1).addOr(param2).addOr(param3)); + myCaptureQueriesListener.clear(); IBundleProvider retrieved = myMedicationRequestDao.search(sp); assertEquals(1, retrieved.size().intValue()); - List queries = CaptureQueriesListener - .getLastNQueries() + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + List queries = myCaptureQueriesListener + .getSelectQueriesForCurrentThread() .stream() - .filter(t -> t.getThreadName().equals("main")) - .filter(t -> t.getSql(false, false).toLowerCase().contains("select")) - .filter(t -> t.getSql(false, false).toLowerCase().contains("token")) .map(t -> t.getSql(true, true)) .collect(Collectors.toList()); - ourLog.info("Queries:\n {}", queries.stream().findFirst()); - String searchQuery = queries.get(0); assertEquals(searchQuery, 3, StringUtils.countMatches(searchQuery.toUpperCase(), "HFJ_SPIDX_TOKEN")); assertEquals(searchQuery, 5, StringUtils.countMatches(searchQuery.toUpperCase(), "LEFT OUTER JOIN")); @@ -2531,6 +2511,8 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { @Test public void testSearchWithDateRange() { + + myCaptureQueriesListener.clear(); SearchParameterMap sp = new SearchParameterMap(); sp.setLoadSynchronous(true); sp.add(MedicationRequest.SP_INTENT, new TokenParam("FOO", "BAR")); @@ -2539,14 +2521,13 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { .setLowerBound(new DateParam("ge2019-02-22T13:50:00"))); IBundleProvider retrieved = myMedicationRequestDao.search(sp); - List queries = CaptureQueriesListener - .getLastNQueries() + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + List queries = myCaptureQueriesListener + .getSelectQueriesForCurrentThread() .stream() .map(t -> t.getSql(true, true)) .collect(Collectors.toList()); - ourLog.info("Queries:\n {}", queries.stream().findFirst()); - String searchQuery = queries.get(0); assertEquals(searchQuery, 1, StringUtils.countMatches(searchQuery.toUpperCase(), "HFJ_SPIDX_TOKEN")); assertEquals(searchQuery, 1, StringUtils.countMatches(searchQuery.toUpperCase(), "LEFT OUTER JOIN")); @@ -2628,6 +2609,81 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } } + + @Test + public void testSearchDoubleToken() { + Patient patient = new Patient(); + patient.addIdentifier().setSystem("urn:system").setValue("TOKENA"); + patient.addIdentifier().setSystem("urn:system").setValue("TOKENB"); + String idBoth = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless().getValue(); + + patient = new Patient(); + patient.addIdentifier().setSystem("urn:system").setValue("TOKENA"); + String idA = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless().getValue(); + + patient = new Patient(); + patient.addIdentifier().setSystem("urn:system").setValue("TOKENB"); + myPatientDao.create(patient, mySrd); + + + { + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Patient.SP_IDENTIFIER, new TokenAndListParam() + .addAnd(new TokenParam("urn:system", "TOKENA")) + .addAnd(new TokenParam("urn:system", "TOKENB")) + ); + IBundleProvider retrieved = myPatientDao.search(map); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(retrieved), containsInAnyOrder(idBoth)); + } + { + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", "TOKENA")); + IBundleProvider retrieved = myPatientDao.search(map); + assertThat(toUnqualifiedVersionlessIdValues(retrieved), containsInAnyOrder(idA, idBoth)); + } + } + + + @Test + public void testSearchDoubleString() { + Patient patient = new Patient(); + patient.addName().setFamily("STRINGA"); + patient.addName().setFamily("STRINGB"); + String idBoth = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless().getValue(); + + patient = new Patient(); + patient.addName().setFamily("STRINGA"); + String idA = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless().getValue(); + + patient = new Patient(); + patient.addName().setFamily("STRINGB"); + myPatientDao.create(patient, mySrd); + + + { + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Patient.SP_FAMILY, new StringAndListParam() + .addAnd(new StringParam("STRINGA")) + .addAnd(new StringParam( "STRINGB")) + ); + myCaptureQueriesListener.clear(); + IBundleProvider retrieved = myPatientDao.search(map); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(retrieved), containsInAnyOrder(idBoth)); + } + { + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Patient.SP_FAMILY, new StringParam("STRINGA")); + IBundleProvider retrieved = myPatientDao.search(map); + assertThat(toUnqualifiedVersionlessIdValues(retrieved), containsInAnyOrder(idA, idBoth)); + } + } + @Test public void testSearchTokenParamNoValue() { Patient patient = new Patient(); @@ -3757,6 +3813,48 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } + /** + * CommunicationRequest:occurrence only indexes DateTime, not Period + */ + @Test + public void testSearchOnPeriod() { + myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED); + + // Matching period + myCaptureQueriesListener.clear(); + CommunicationRequest cr = new CommunicationRequest(); + Period occurrence = new Period(); + occurrence.setStartElement(new DateTimeType("2016-08-10T11:33:00-04:00")); + occurrence.setEndElement(new DateTimeType("2016-08-10T11:33:00-04:00")); + cr.setOccurrence(occurrence); + myCommunicationRequestDao.create(cr).getId().toUnqualifiedVersionless().getValue(); + myCaptureQueriesListener.logInsertQueriesForCurrentThread(); + + // Matching dateTime + myCaptureQueriesListener.clear(); + cr = new CommunicationRequest(); + cr.setOccurrence(new DateTimeType("2016-08-10T11:33:00-04:00")); + String crId = myCommunicationRequestDao.create(cr).getId().toUnqualifiedVersionless().getValue(); + myCaptureQueriesListener.logInsertQueriesForCurrentThread(); + + // Non matching period + cr = new CommunicationRequest(); + occurrence = new Period(); + occurrence.setStartElement(new DateTimeType("2001-08-10T11:33:00-04:00")); + occurrence.setEndElement(new DateTimeType("2001-08-10T11:33:00-04:00")); + cr.setOccurrence(occurrence); + myCommunicationRequestDao.create(cr).getId().toUnqualifiedVersionless().getValue(); + + myCaptureQueriesListener.clear(); + SearchParameterMap params = new SearchParameterMap(); + params.setLoadSynchronous(true); + params.add(CommunicationRequest.SP_OCCURRENCE, new DateParam(ParamPrefixEnum.GREATERTHAN_OR_EQUALS, "2015-08-10T11:33:00-04:00")); + IBundleProvider outcome = myCommunicationRequestDao.search(params); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(outcome), contains(crId)); + } + + private String toStringMultiline(List theResults) { StringBuilder b = new StringBuilder(); for (Object next : theResults) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java index f76efacedc6..3e7548260fa 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java @@ -1,6 +1,6 @@ package ca.uhn.fhir.jpa.dao.r4; -import ca.uhn.fhir.jpa.config.CaptureQueriesListener; +import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; @@ -400,31 +400,22 @@ public class FhirResourceDaoR4SearchNoHashesTest extends BaseJpaR4Test { p = new Patient(); p.addIdentifier().setSystem("SYS").setValue("BAZ"); myPatientDao.create(p); - CaptureQueriesListener.clear(); + myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_IDENTIFIER, new TokenOrListParam().addOr(new TokenParam("FOO")).addOr(new TokenParam("BAR"))); map.setLoadSynchronous(true); IBundleProvider search = myPatientDao.search(map); - List queries = CaptureQueriesListener - .getLastNQueries() - .stream() - .map(t -> t.getSql(true, true)) - .filter(t -> t.contains("select")) - .collect(Collectors.toList()); - String resultingQueryFormatted = queries.get(queries.size() - 1); - ourLog.info("Resulting query formatted:\n{}", resultingQueryFormatted); - - queries = CaptureQueriesListener - .getLastNQueries() + myCaptureQueriesListener.logInsertQueriesForCurrentThread(); + List queries = myCaptureQueriesListener + .getSelectQueriesForCurrentThread() .stream() .map(t -> t.getSql(true, false)) - .filter(t -> t.contains("select")) .collect(Collectors.toList()); - String resultingQueryNotFormatted = queries.get(queries.size() - 1); + String resultingQueryNotFormatted = queries.get(0); - assertEquals(resultingQueryFormatted, 1, StringUtils.countMatches(resultingQueryNotFormatted, "SP_VALUE")); + assertEquals(resultingQueryNotFormatted, 1, StringUtils.countMatches(resultingQueryNotFormatted, "SP_VALUE")); assertThat(resultingQueryNotFormatted, containsString("SP_VALUE in ('BAR' , 'FOO')")); // Ensure that the search actually worked diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index 9d456f4816e..a66f692f6a3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -937,80 +937,6 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } - @Test - public void testTransactionUpdatingManuallyDeletedResource() { - - // Create an observation - Observation obs = new Observation(); - obs.addIdentifier().setSystem("urn:system").setValue("foo"); - IIdType obId = myObservationDao.create(obs).getId(); - - // Manually mark it a deleted - runInTransaction(()->{ - myEntityManager.createNativeQuery("UPDATE HFJ_RESOURCE SET RES_DELETED_AT = CURRENT_TIMESTAMP").executeUpdate(); - }); - - runInTransaction(()->{ - ResourceTable obsTable = myResourceTableDao.findById(obId.getIdPartAsLong()).get(); - assertNotNull(obsTable.getDeleted()); - assertEquals(1L, obsTable.getVersion()); - }); - - // Now create a transaction - - obs = new Observation(); - obs.setId(IdType.newRandomUuid()); - obs.addIdentifier().setSystem("urn:system").setValue("foo"); - - DiagnosticReport dr = new DiagnosticReport(); - dr.setId(IdType.newRandomUuid()); - dr.addIdentifier().setSystem("urn:system").setValue("bar"); - dr.addResult().setReference(obs.getId()); - - Bundle bundle = new Bundle(); - bundle.setType(BundleType.TRANSACTION); - bundle.addEntry() - .setResource(obs) - .setFullUrl(obs.getId()) - .getRequest() - .setMethod(HTTPVerb.PUT) - .setUrl("Observation?identifier=urn:system|foo"); - bundle.addEntry() - .setResource(dr) - .setFullUrl(dr.getId()) - .getRequest() - .setMethod(HTTPVerb.PUT) - .setUrl("DiagnosticReport?identifier=urn:system|bar"); - - Bundle resp = mySystemDao.transaction(mySrd, bundle); - assertEquals(2, resp.getEntry().size()); - - BundleEntryComponent respEntry = resp.getEntry().get(0); - assertEquals(Constants.STATUS_HTTP_200_OK + " OK", respEntry.getResponse().getStatus()); - assertThat(respEntry.getResponse().getLocation(), containsString("Observation/" + obId.getIdPart())); - assertThat(respEntry.getResponse().getLocation(), endsWith("/_history/3")); - assertEquals("3", respEntry.getResponse().getEtag()); - - respEntry = resp.getEntry().get(1); - assertEquals(Constants.STATUS_HTTP_201_CREATED + " Created", respEntry.getResponse().getStatus()); - assertThat(respEntry.getResponse().getLocation(), containsString("DiagnosticReport/")); - assertThat(respEntry.getResponse().getLocation(), endsWith("/_history/1")); - IdType drId = new IdType(respEntry.getResponse().getLocation()); - assertEquals("1", respEntry.getResponse().getEtag()); - - runInTransaction(()->{ - ResourceTable obsTable = myResourceTableDao.findById(obId.getIdPartAsLong()).get(); - assertNull(obsTable.getDeleted()); - assertEquals(3L, obsTable.getVersion()); - }); - - runInTransaction(()->{ - DiagnosticReport savedDr = myDiagnosticReportDao.read(drId); - assertEquals(obId.toUnqualifiedVersionless().getValue(), savedDr.getResult().get(0).getReference()); - }); - - } - @Test public void testTransactionCreateInlineMatchUrlWithOneMatchLastUpdated() { Bundle request = new Bundle(); diff --git a/pom.xml b/pom.xml index a0ab12ac3a5..265579cc660 100755 --- a/pom.xml +++ b/pom.xml @@ -872,7 +872,7 @@ net.ttddyy datasource-proxy - 1.4.9 + 1.5 org.antlr diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 5c156c01228..2801f4c2697 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -68,6 +68,11 @@ collapse these into a single operation. This is done as a convenience, since many conversion algorithms can accidentally generate such duplicates. + + Searching the JPA server with multiple instances of the same token search parameter + (e.g. "Patient?identifier=&identifier=b" returned no results even if resources + should have matched. Thanks to @mingdatacom for reporting! +