Make sure that direct resource load path is only executed for synchro… (#3833)
* Make sure that direct resource load path is only executed for synchronous searches * Remove unneeded mockito extension * Only allow for resource loading from fulltext index when advanced search (indexed parameters) is also enabled Co-authored-by: juan.marchionatto <juan.marchionatto@smilecdr.com>
This commit is contained in:
parent
9324c6a051
commit
2e1f4c25f5
|
@ -361,13 +361,13 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
|
|||
// SearchStrategyFactory.ISearchStrategy searchStrategy = mySearchStrategyFactory.pickStrategy(theResourceType, theParams, theRequestDetails);
|
||||
// return searchStrategy.get();
|
||||
|
||||
if (mySearchStrategyFactory.isSupportsHSearchDirect(theResourceType, theParams, theRequestDetails)) {
|
||||
ourLog.info("Search {} is using direct load strategy", searchUuid);
|
||||
SearchStrategyFactory.ISearchStrategy direct = mySearchStrategyFactory.makeDirectStrategy(searchUuid, theResourceType, theParams, theRequestDetails);
|
||||
return direct.get();
|
||||
}
|
||||
|
||||
if (theParams.isLoadSynchronous() || loadSynchronousUpTo != null || isOffsetQuery) {
|
||||
if (mySearchStrategyFactory.isSupportsHSearchDirect(theResourceType, theParams, theRequestDetails)) {
|
||||
ourLog.info("Search {} is using direct load strategy", searchUuid);
|
||||
SearchStrategyFactory.ISearchStrategy direct = mySearchStrategyFactory.makeDirectStrategy(searchUuid, theResourceType, theParams, theRequestDetails);
|
||||
return direct.get();
|
||||
}
|
||||
|
||||
ourLog.debug("Search {} is loading in synchronous mode", searchUuid);
|
||||
return mySynchronousSearchSvc.executeQuery(theParams, theRequestDetails, searchUuid, sb, loadSynchronousUpTo, theRequestPartitionId);
|
||||
}
|
||||
|
|
|
@ -969,9 +969,9 @@ public class SearchBuilder implements ISearchBuilder {
|
|||
* @return can we fetch from Hibernate Search?
|
||||
*/
|
||||
private boolean isLoadingFromElasticSearchSupported(Collection<ResourcePersistentId> thePids) {
|
||||
|
||||
// is storage enabled?
|
||||
return myDaoConfig.isStoreResourceInHSearchIndex() &&
|
||||
myDaoConfig.isAdvancedHSearchIndexing() &&
|
||||
// we don't support history
|
||||
thePids.stream().noneMatch(p->p.getVersion()!=null) &&
|
||||
// skip the complexity for metadata in dstu2
|
||||
|
|
|
@ -35,12 +35,14 @@ import ca.uhn.fhir.rest.api.Constants;
|
|||
import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
|
||||
import ca.uhn.fhir.rest.api.SortSpec;
|
||||
import ca.uhn.fhir.rest.api.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.api.server.RequestDetails;
|
||||
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
|
||||
import ca.uhn.fhir.rest.param.ReferenceParam;
|
||||
import ca.uhn.fhir.rest.param.StringOrListParam;
|
||||
import ca.uhn.fhir.rest.param.StringParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParamModifier;
|
||||
import ca.uhn.fhir.rest.server.IPagingProvider;
|
||||
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
|
||||
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
|
||||
import ca.uhn.fhir.storage.test.BaseDateSearchDaoTests;
|
||||
|
@ -127,6 +129,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
|
|||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.spy;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
@ExtendWith(SpringExtension.class)
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
|
@ -1053,6 +1058,43 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
|
|||
// assertThat(newMeta.getTag(), hasSize(2));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void noDirectSearchWhenNotSynchronousOrOffsetQuery() {
|
||||
myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-1")));
|
||||
myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-2")));
|
||||
myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-3")));
|
||||
myCaptureQueriesListener.clear();
|
||||
myHSearchEventDispatcher.register(mySearchEventListener);
|
||||
|
||||
myTestDaoSearch.searchForBundleProvider("Observation?code=code-1,code-2,code-3", false);
|
||||
|
||||
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
|
||||
|
||||
assertEquals(2, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "we build the bundle with no sql");
|
||||
|
||||
// index only queried once for count
|
||||
Mockito.verify(mySearchEventListener, Mockito.times(1)).hsearchEvent(IHSearchEventListener.HSearchEventType.SEARCH);
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void directSearchForOffsetQuery() {
|
||||
myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-1")));
|
||||
IIdType idCode2 = myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-2")));
|
||||
IIdType idCode3 = myTestDataBuilder.createObservation(asArray(myTestDataBuilder.withObservationCode("http://example.com/", "code-3")));
|
||||
myCaptureQueriesListener.clear();
|
||||
myHSearchEventDispatcher.register(mySearchEventListener);
|
||||
|
||||
List<String> resultIds = myTestDaoSearch.searchForIds("Observation?code=code-1,code-2,code-3&_offset=1");
|
||||
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
|
||||
|
||||
assertThat(resultIds, containsInAnyOrder(idCode2.getIdPart(), idCode3.getIdPart()));
|
||||
assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "we build the bundle with no sql");
|
||||
|
||||
// only one hibernate search took place
|
||||
Mockito.verify(mySearchEventListener, Mockito.times(1)).hsearchEvent(IHSearchEventListener.HSearchEventType.SEARCH);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
||||
|
@ -2622,4 +2664,5 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
|
|||
}
|
||||
|
||||
|
||||
|
||||
}
|
||||
|
|
|
@ -277,7 +277,7 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry, IResourceC
|
|||
*/
|
||||
@PostConstruct
|
||||
public void registerListener() {
|
||||
myResourceChangeListenerCache = myResourceChangeListenerRegistry.registerResourceResourceChangeListener("SearchParameter", SearchParameterMap.newSynchronous(), this, REFRESH_INTERVAL);
|
||||
myResourceChangeListenerCache = myResourceChangeListenerRegistry.registerResourceResourceChangeListener("SearchParameter", SearchParameterMap.newSynchronous(), this, REFRESH_INTERVAL);
|
||||
}
|
||||
|
||||
@PreDestroy
|
||||
|
|
|
@ -29,6 +29,8 @@ import ca.uhn.fhir.jpa.searchparam.ResourceSearch;
|
|||
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
||||
import ca.uhn.fhir.rest.api.SortSpec;
|
||||
import ca.uhn.fhir.rest.api.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.server.IPagingProvider;
|
||||
import ca.uhn.fhir.rest.server.IRestfulServerDefaults;
|
||||
import ca.uhn.fhir.rest.server.method.SortParameter;
|
||||
import org.hamcrest.Matcher;
|
||||
import org.hamcrest.MatcherAssert;
|
||||
|
@ -48,6 +50,9 @@ import static org.hamcrest.Matchers.everyItem;
|
|||
import static org.hamcrest.Matchers.hasItems;
|
||||
import static org.hamcrest.Matchers.in;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
import static org.mockito.Mockito.doReturn;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.spy;
|
||||
|
||||
/**
|
||||
* Simplistic implementation of FHIR queries.
|
||||
|
@ -138,24 +143,28 @@ public class TestDaoSearch {
|
|||
IBundleProvider result = searchForBundleProvider(theQueryUrl);
|
||||
|
||||
// getAllResources is not safe as size is not always set
|
||||
List<String> resourceIds = result.getResources(0, Integer.MAX_VALUE)
|
||||
return result.getResources(0, Integer.MAX_VALUE)
|
||||
.stream().map(resource -> resource.getIdElement().getIdPart()).collect(Collectors.toList());
|
||||
return resourceIds;
|
||||
}
|
||||
|
||||
public IBundleProvider searchForBundleProvider(String theQueryUrl) {
|
||||
public IBundleProvider searchForBundleProvider(String theQueryUrl, boolean theSynchronousMode) {
|
||||
ResourceSearch search = myMatchUrlService.getResourceSearch(theQueryUrl);
|
||||
IFhirResourceDao<?> dao = myDaoRegistry.getResourceDao(search.getResourceName());
|
||||
|
||||
SearchParameterMap map = search.getSearchParameterMap();
|
||||
map.setLoadSynchronous(true);
|
||||
map.setLoadSynchronous(theSynchronousMode);
|
||||
SortSpec sort = (SortSpec) new SortParameter(myFhirCtx).translateQueryParametersIntoServerArgument(fakeRequestDetailsFromUrl(theQueryUrl), null);
|
||||
if (sort != null) {
|
||||
map.setSort(sort);
|
||||
}
|
||||
|
||||
IBundleProvider result = dao.search(map, fakeRequestDetailsFromUrl(theQueryUrl));
|
||||
return result;
|
||||
// for asynchronous mode, we also need to make the request paginated ar synchronous is forced
|
||||
SystemRequestDetails reqDetails = theSynchronousMode ? fakeRequestDetailsFromUrl(theQueryUrl) : fakePaginatedRequestDetailsFromUrl(theQueryUrl);
|
||||
return dao.search(map, reqDetails);
|
||||
}
|
||||
|
||||
public IBundleProvider searchForBundleProvider(String theQueryUrl) {
|
||||
return searchForBundleProvider(theQueryUrl, true);
|
||||
}
|
||||
|
||||
public SearchParameterMap toSearchParameters(String theQueryUrl) {
|
||||
|
@ -178,4 +187,20 @@ public class TestDaoSearch {
|
|||
.forEach((key, value) -> request.addParameter(key, value.toArray(new String[0])));
|
||||
return request;
|
||||
}
|
||||
|
||||
@Nonnull
|
||||
private SystemRequestDetails fakePaginatedRequestDetailsFromUrl(String theQueryUrl) {
|
||||
SystemRequestDetails spiedReqDetails = spy(SystemRequestDetails.class);
|
||||
UriComponents uriComponents = UriComponentsBuilder.fromUriString(theQueryUrl).build();
|
||||
uriComponents.getQueryParams()
|
||||
.forEach((key, value) -> spiedReqDetails.addParameter(key, value.toArray(new String[0])));
|
||||
|
||||
IPagingProvider mockPagingProvider = mock(IPagingProvider.class);
|
||||
IRestfulServerDefaults mockServerDfts = mock(IRestfulServerDefaults.class);
|
||||
doReturn(mockServerDfts).when(spiedReqDetails).getServer();
|
||||
doReturn(mockPagingProvider).when(mockServerDfts).getPagingProvider();
|
||||
return spiedReqDetails;
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue