From 0ab8a9a0ba521f67016b316a674d050db99ee070 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 5 Oct 2017 13:41:22 -0400 Subject: [PATCH] Fix #744 - Long search URLs on Derby shouldn't cause a failure --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 2 +- .../java/ca/uhn/fhir/jpa/entity/Search.java | 11 ++- .../jpa/search/SearchCoordinatorSvcImpl.java | 7 -- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 83 ++++++++++++++++++- src/changes/changes.xml | 5 ++ 5 files changed, 96 insertions(+), 12 deletions(-) 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 0a077f9f04b..23426709440 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 @@ -1259,7 +1259,7 @@ public class SearchBuilder implements ISearchBuilder { * Check if there is a unique key associated with the set * of parameters passed in */ - ourLog.info("Checking for unique index for query: {}", theParams.toNormalizedQueryString(myContext)); + ourLog.debug("Checking for unique index for query: {}", theParams.toNormalizedQueryString(myContext)); if (myCallingDao.getConfig().isUniqueIndexesEnabled()) { if (myParams.getIncludes().isEmpty()) { if (myParams.getRevIncludes().isEmpty()) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java index dfbc745c0b9..ff2086f1cbf 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java @@ -50,6 +50,7 @@ public class Search implements Serializable { private static final int FAILURE_MESSAGE_LENGTH = 500; private static final long serialVersionUID = 1L; + public static final int MAX_SEARCH_QUERY_STRING = 10000; @Temporal(TemporalType.TIMESTAMP) @Column(name="CREATED", nullable=false, updatable=false) @@ -100,8 +101,8 @@ public class Search implements Serializable { private Date mySearchLastReturned; @Lob() - @Basic(fetch=FetchType.LAZY) - @Column(name="SEARCH_QUERY_STRING", nullable=true, updatable=false) + @Basic(fetch=FetchType.LAZY) + @Column(name="SEARCH_QUERY_STRING", nullable=true, updatable=false, length = MAX_SEARCH_QUERY_STRING) private String mySearchQueryString; @Column(name="SEARCH_QUERY_STRING_HASH", nullable=true, updatable=false) @@ -256,7 +257,11 @@ public class Search implements Serializable { } public void setSearchQueryString(String theSearchQueryString) { - mySearchQueryString = theSearchQueryString; + if (theSearchQueryString != null && theSearchQueryString.length() > MAX_SEARCH_QUERY_STRING) { + mySearchQueryString = null; + } else { + mySearchQueryString = theSearchQueryString; + } } public void setSearchQueryStringHash(Integer theSearchQueryStringHash) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index e2e9cb30da6..7432222592f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -78,13 +78,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { private int mySyncSize = DEFAULT_SYNC_SIZE; - // @Autowired - // private DataSource myDataSource; - // @PostConstruct - // public void start() { - // JpaTransactionManager txManager = (JpaTransactionManager) myManagedTxManager; - // } - /** * Constructor */ 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 42819bb0345..24d1ce80b8a 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 @@ -44,11 +44,92 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4SearchNoFtTest.class); @Before - public void beforeDisableResultReuse() { + public void beforeDidsableCacheReuse() { myDaoConfig.setReuseCachedSearchResultsForMillis(null); + } + + @After + public void afterResetSearchSize() { + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); myDaoConfig.setFetchSizeDefaultMaximum(new DaoConfig().getFetchSizeDefaultMaximum()); } + /** + * See #744 + */ + @Test + public void testSearchWithVeryLongUrlShorter() { + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + + Patient p = new Patient(); + p.addName().setFamily("A1"); + myPatientDao.create(p); + + assertEquals(0, mySearchEntityDao.count()); + + SearchParameterMap map = new SearchParameterMap(); + StringOrListParam or = new StringOrListParam(); + or.addOr(new StringParam("A1")); + or.addOr(new StringParam(StringUtils.leftPad("", 200, 'A'))); + or.addOr(new StringParam(StringUtils.leftPad("", 200, 'B'))); + or.addOr(new StringParam(StringUtils.leftPad("", 200, 'C'))); + map.add(Patient.SP_NAME, or); + IBundleProvider results = myPatientDao.search(map); + assertEquals(1, results.getResources(0, 10).size()); + assertEquals(1, mySearchEntityDao.count()); + + map = new SearchParameterMap(); + or = new StringOrListParam(); + or.addOr(new StringParam("A1")); + or.addOr(new StringParam(StringUtils.leftPad("", 200, 'A'))); + or.addOr(new StringParam(StringUtils.leftPad("", 200, 'B'))); + or.addOr(new StringParam(StringUtils.leftPad("", 200, 'C'))); + map.add(Patient.SP_NAME, or); + results = myPatientDao.search(map); + assertEquals(1, results.getResources(0, 10).size()); + assertEquals(1, mySearchEntityDao.count()); + + } + + /** + * See #744 + */ + @Test + public void testSearchWithVeryLongUrlLonger() { + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + + Patient p = new Patient(); + p.addName().setFamily("A1"); + myPatientDao.create(p); + + assertEquals(0, mySearchEntityDao.count()); + + SearchParameterMap map = new SearchParameterMap(); + StringOrListParam or = new StringOrListParam(); + or.addOr(new StringParam("A1")); + for (int i = 0; i < 50; i++) { + or.addOr(new StringParam(StringUtils.leftPad("", 200, (char)('A' + i)))); + } + map.add(Patient.SP_NAME, or); + IBundleProvider results = myPatientDao.search(map); + assertEquals(1, results.getResources(0, 10).size()); + assertEquals(1, mySearchEntityDao.count()); + + map = new SearchParameterMap(); + or = new StringOrListParam(); + or.addOr(new StringParam("A1")); + or.addOr(new StringParam("A1")); + for (int i = 0; i < 50; i++) { + or.addOr(new StringParam(StringUtils.leftPad("", 200, (char)('A' + i)))); + } + map.add(Patient.SP_NAME, or); + results = myPatientDao.search(map); + assertEquals(1, results.getResources(0, 10).size()); + // We expect a new one because we don't cache the search URL for very long search URLs + assertEquals(2, mySearchEntityDao.count()); + + } + /** * See #441 */ diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4cca7e235fc..22d7acc1fad 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -27,6 +27,11 @@ has been changed to only accept "url". Thanks to Avinash Shanbhag for reporting! + + Fix an error in JPA server when using Derby Database, where search queries with + a search URL longer than 255 characters caused a mysterious failure. Thanks to + Chris Schuler and Bryn Rhodes for all of their help in reproducing this issue. +