Merge pull request #2889 from hapifhir/issue-2887-contained-cache-key
Add _contained parameter to SearchParameterMap query string normalizer
This commit is contained in:
commit
d7d2919146
|
@ -51,7 +51,7 @@ public enum SearchContainedModeEnum {
|
||||||
myCode = theCode;
|
myCode = theCode;
|
||||||
}
|
}
|
||||||
|
|
||||||
private String getCode() {
|
public String getCode() {
|
||||||
return myCode;
|
return myCode;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
type: fix
|
||||||
|
issue: 2887
|
||||||
|
jira: SMILE-2896
|
||||||
|
title: "Fixed a bug where the search results cache was ignoring the value of `_contained` parameter when assigning a cache key.
|
||||||
|
This was causing queries run in a short period of time to return wrong cached results if one query used `_contained=true` and the other did not."
|
|
@ -10,10 +10,8 @@ import ca.uhn.fhir.jpa.entity.SearchTypeEnum;
|
||||||
import ca.uhn.fhir.jpa.model.search.SearchStatusEnum;
|
import ca.uhn.fhir.jpa.model.search.SearchStatusEnum;
|
||||||
import ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl;
|
import ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl;
|
||||||
import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc;
|
import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc;
|
||||||
import ca.uhn.fhir.util.TestUtil;
|
|
||||||
import org.apache.commons.lang3.time.DateUtils;
|
import org.apache.commons.lang3.time.DateUtils;
|
||||||
import org.junit.jupiter.api.AfterEach;
|
import org.junit.jupiter.api.AfterEach;
|
||||||
import org.junit.jupiter.api.AfterAll;
|
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
|
|
||||||
|
@ -40,7 +38,7 @@ public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
|
||||||
private ISearchCoordinatorSvc mySearchCoordinator;
|
private ISearchCoordinatorSvc mySearchCoordinator;
|
||||||
|
|
||||||
@Autowired
|
@Autowired
|
||||||
private ISearchCacheSvc myDataaseCacheSvc;
|
private ISearchCacheSvc myDatabaseCacheSvc;
|
||||||
|
|
||||||
@AfterEach
|
@AfterEach
|
||||||
public void after() {
|
public void after() {
|
||||||
|
@ -92,7 +90,7 @@ public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
|
||||||
assertEquals(30, mySearchResultDao.count());
|
assertEquals(30, mySearchResultDao.count());
|
||||||
});
|
});
|
||||||
|
|
||||||
myDataaseCacheSvc.pollForStaleSearchesAndDeleteThem();
|
myDatabaseCacheSvc.pollForStaleSearchesAndDeleteThem();
|
||||||
runInTransaction(()->{
|
runInTransaction(()->{
|
||||||
// We should delete up to 10, but 3 don't get deleted since they have too many results to delete in one pass
|
// We should delete up to 10, but 3 don't get deleted since they have too many results to delete in one pass
|
||||||
assertEquals(13, mySearchDao.count());
|
assertEquals(13, mySearchDao.count());
|
||||||
|
@ -101,7 +99,7 @@ public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
|
||||||
assertEquals(15, mySearchResultDao.count());
|
assertEquals(15, mySearchResultDao.count());
|
||||||
});
|
});
|
||||||
|
|
||||||
myDataaseCacheSvc.pollForStaleSearchesAndDeleteThem();
|
myDatabaseCacheSvc.pollForStaleSearchesAndDeleteThem();
|
||||||
runInTransaction(()->{
|
runInTransaction(()->{
|
||||||
// Once again we attempt to delete 10, but the first 3 don't get deleted and still remain
|
// Once again we attempt to delete 10, but the first 3 don't get deleted and still remain
|
||||||
// (total is 6 because 3 weren't deleted, and they blocked another 3 that might have been)
|
// (total is 6 because 3 weren't deleted, and they blocked another 3 that might have been)
|
||||||
|
@ -110,7 +108,7 @@ public class SearchCoordinatorSvcImplTest extends BaseJpaR4Test {
|
||||||
assertEquals(0, mySearchResultDao.count());
|
assertEquals(0, mySearchResultDao.count());
|
||||||
});
|
});
|
||||||
|
|
||||||
myDataaseCacheSvc.pollForStaleSearchesAndDeleteThem();
|
myDatabaseCacheSvc.pollForStaleSearchesAndDeleteThem();
|
||||||
runInTransaction(()->{
|
runInTransaction(()->{
|
||||||
assertEquals(0, mySearchDao.count());
|
assertEquals(0, mySearchDao.count());
|
||||||
assertEquals(0, mySearchDao.countDeleted());
|
assertEquals(0, mySearchDao.countDeleted());
|
||||||
|
|
|
@ -1,8 +1,10 @@
|
||||||
package ca.uhn.fhir.jpa.provider;
|
package ca.uhn.fhir.jpa.provider;
|
||||||
|
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNotEquals;
|
||||||
|
|
||||||
import ca.uhn.fhir.context.FhirVersionEnum;
|
import ca.uhn.fhir.context.FhirVersionEnum;
|
||||||
|
import ca.uhn.fhir.rest.api.SearchContainedModeEnum;
|
||||||
import org.junit.jupiter.api.AfterAll;
|
import org.junit.jupiter.api.AfterAll;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
@ -41,6 +43,24 @@ public class SearchParameterMapTest {
|
||||||
assertEquals("?birthdate=ge2001&birthdate=lt2002&name=bouvier,simpson&name=homer,jay&name:exact=ZZZ?", UrlUtil.unescape(queryString));
|
assertEquals("?birthdate=ge2001&birthdate=lt2002&name=bouvier,simpson&name=homer,jay&name:exact=ZZZ?", UrlUtil.unescape(queryString));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testContainedParameterIsIncludedInNormalizedString() {
|
||||||
|
SearchParameterMap map = new SearchParameterMap();
|
||||||
|
map.add("name", new StringParam("Smith"));
|
||||||
|
map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
|
||||||
|
String containedQueryString = map.toNormalizedQueryString(ourCtx);
|
||||||
|
|
||||||
|
SearchParameterMap uncontainedMap = new SearchParameterMap();
|
||||||
|
uncontainedMap.add("name", new StringParam("Smith"));
|
||||||
|
uncontainedMap.setSearchContainedMode(SearchContainedModeEnum.FALSE);
|
||||||
|
String uncontainedQueryString = uncontainedMap.toNormalizedQueryString(ourCtx);
|
||||||
|
|
||||||
|
ourLog.info(containedQueryString);
|
||||||
|
ourLog.info(uncontainedQueryString);
|
||||||
|
assertNotEquals(containedQueryString, uncontainedQueryString);
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testToQueryStringEmpty() {
|
public void testToQueryStringEmpty() {
|
||||||
SearchParameterMap map = new SearchParameterMap();
|
SearchParameterMap map = new SearchParameterMap();
|
||||||
|
|
|
@ -284,7 +284,6 @@ public class ResourceProviderR4SearchContainedTest extends BaseResourceProviderR
|
||||||
assertEquals(0L, oids.size());
|
assertEquals(0L, oids.size());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testContainedSearchByNumber() throws Exception {
|
public void testContainedSearchByNumber() throws Exception {
|
||||||
|
|
||||||
|
@ -975,6 +974,11 @@ public class ResourceProviderR4SearchContainedTest extends BaseResourceProviderR
|
||||||
|
|
||||||
assertEquals(1L, oids.size());
|
assertEquals(1L, oids.size());
|
||||||
assertThat(oids, contains(oid1.getValue()));
|
assertThat(oids, contains(oid1.getValue()));
|
||||||
|
}
|
||||||
|
|
||||||
|
//See https://github.com/hapifhir/hapi-fhir/issues/2887
|
||||||
|
@Test
|
||||||
|
public void testContainedResourceParameterIsUsedInCache() {
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -511,6 +511,15 @@ public class SearchParameterMap implements Serializable {
|
||||||
b.append(getSearchTotalMode().getCode());
|
b.append(getSearchTotalMode().getCode());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
//Contained mode
|
||||||
|
//For some reason, instead of null here, we default to false. That said, ommitting it is identical to setting it to false.
|
||||||
|
if (getSearchContainedMode() != SearchContainedModeEnum.FALSE) {
|
||||||
|
addUrlParamSeparator(b);
|
||||||
|
b.append(Constants.PARAM_CONTAINED);
|
||||||
|
b.append("=");
|
||||||
|
b.append(getSearchContainedMode().getCode());
|
||||||
|
}
|
||||||
|
|
||||||
if (b.length() == 0) {
|
if (b.length() == 0) {
|
||||||
b.append('?');
|
b.append('?');
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue