Handle null returns from search cache methods
This commit is contained in:
parent
9c23959b38
commit
bf476b9c32
|
@ -209,11 +209,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
|
||||||
|
|
||||||
search = mySearchCacheSvc
|
search = mySearchCacheSvc
|
||||||
.fetchByUuid(theUuid)
|
.fetchByUuid(theUuid)
|
||||||
.orElseThrow(() -> {
|
.orElseThrow(() -> newResourceGoneException(theUuid));
|
||||||
ourLog.trace("Client requested unknown paging ID[{}]", theUuid);
|
|
||||||
String msg = myContext.getLocalizer().getMessage(PageMethodBinding.class, "unknownSearchId", theUuid);
|
|
||||||
return new ResourceGoneException(msg);
|
|
||||||
});
|
|
||||||
|
|
||||||
verifySearchHasntFailedOrThrowInternalErrorException(search);
|
verifySearchHasntFailedOrThrowInternalErrorException(search);
|
||||||
if (search.getStatus() == SearchStatusEnum.FINISHED) {
|
if (search.getStatus() == SearchStatusEnum.FINISHED) {
|
||||||
|
@ -252,12 +248,22 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
|
||||||
ourLog.trace("Finished looping");
|
ourLog.trace("Finished looping");
|
||||||
|
|
||||||
List<Long> pids = mySearchResultCacheSvc.fetchResultPids(search, theFrom, theTo);
|
List<Long> pids = mySearchResultCacheSvc.fetchResultPids(search, theFrom, theTo);
|
||||||
|
if (pids == null) {
|
||||||
|
throw newResourceGoneException(theUuid);
|
||||||
|
}
|
||||||
|
|
||||||
ourLog.trace("Fetched {} results", pids.size());
|
ourLog.trace("Fetched {} results", pids.size());
|
||||||
|
|
||||||
return pids;
|
return pids;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Nonnull
|
||||||
|
private ResourceGoneException newResourceGoneException(String theUuid) {
|
||||||
|
ourLog.trace("Client requested unknown paging ID[{}]", theUuid);
|
||||||
|
String msg = myContext.getLocalizer().getMessage(PageMethodBinding.class, "unknownSearchId", theUuid);
|
||||||
|
return new ResourceGoneException(msg);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
private void populateBundleProvider(PersistedJpaBundleProvider theRetVal) {
|
private void populateBundleProvider(PersistedJpaBundleProvider theRetVal) {
|
||||||
theRetVal.setContext(myContext);
|
theRetVal.setContext(myContext);
|
||||||
|
@ -1101,14 +1107,21 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
|
||||||
txTemplate.afterPropertiesSet();
|
txTemplate.afterPropertiesSet();
|
||||||
txTemplate.execute(t -> {
|
txTemplate.execute(t -> {
|
||||||
List<Long> previouslyAddedResourcePids = mySearchResultCacheSvc.fetchAllResultPids(getSearch());
|
List<Long> previouslyAddedResourcePids = mySearchResultCacheSvc.fetchAllResultPids(getSearch());
|
||||||
|
if (previouslyAddedResourcePids == null) {
|
||||||
|
throw newResourceGoneException(getSearch().getUuid());
|
||||||
|
}
|
||||||
|
|
||||||
ourLog.debug("Have {} previously added IDs in search: {}", previouslyAddedResourcePids.size(), getSearch().getUuid());
|
ourLog.debug("Have {} previously added IDs in search: {}", previouslyAddedResourcePids.size(), getSearch().getUuid());
|
||||||
setPreviouslyAddedResourcePids(previouslyAddedResourcePids);
|
setPreviouslyAddedResourcePids(previouslyAddedResourcePids);
|
||||||
return null;
|
return null;
|
||||||
});
|
});
|
||||||
} catch (Throwable e) {
|
} catch (Throwable e) {
|
||||||
ourLog.error("Failure processing search", e);
|
ourLog.error("Failure processing search", e);
|
||||||
getSearch().setFailureMessage(e.toString());
|
getSearch().setFailureMessage(e.getMessage());
|
||||||
getSearch().setStatus(SearchStatusEnum.FAILED);
|
getSearch().setStatus(SearchStatusEnum.FAILED);
|
||||||
|
if (e instanceof BaseServerResponseException) {
|
||||||
|
getSearch().setFailureCode(((BaseServerResponseException) e).getStatusCode());
|
||||||
|
}
|
||||||
|
|
||||||
saveSearch();
|
saveSearch();
|
||||||
return null;
|
return null;
|
||||||
|
|
|
@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.search.cache;
|
||||||
|
|
||||||
import ca.uhn.fhir.jpa.entity.Search;
|
import ca.uhn.fhir.jpa.entity.Search;
|
||||||
|
|
||||||
|
import javax.annotation.Nullable;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
public interface ISearchResultCacheSvc {
|
public interface ISearchResultCacheSvc {
|
||||||
|
@ -38,14 +39,20 @@ public interface ISearchResultCacheSvc {
|
||||||
* @param theSearch The search to fetch IDs for
|
* @param theSearch The search to fetch IDs for
|
||||||
* @param theFrom The starting index (inclusive)
|
* @param theFrom The starting index (inclusive)
|
||||||
* @param theTo The ending index (exclusive)
|
* @param theTo The ending index (exclusive)
|
||||||
* @return A list of resource PIDs
|
* @return A list of resource PIDs, or <code>null</code> if the results no longer exist (this should only happen if the results
|
||||||
|
* have been removed from the cache for some reason, such as expiry or manual purge)
|
||||||
*/
|
*/
|
||||||
|
@Nullable
|
||||||
List<Long> fetchResultPids(Search theSearch, int theFrom, int theTo);
|
List<Long> fetchResultPids(Search theSearch, int theFrom, int theTo);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fetch all result PIDs for a given search with no particular order required
|
* Fetch all result PIDs for a given search with no particular order required
|
||||||
* @param theSearch
|
*
|
||||||
* @return
|
* @param theSearch The search object
|
||||||
|
* @return A list of resource PIDs, or <code>null</code> if the results no longer exist (this should only happen if the results
|
||||||
|
* have been removed from the cache for some reason, such as expiry or manual purge)
|
||||||
*/
|
*/
|
||||||
|
@Nullable
|
||||||
List<Long> fetchAllResultPids(Search theSearch);
|
List<Long> fetchAllResultPids(Search theSearch);
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -469,6 +469,62 @@ public class SearchCoordinatorSvcImplTest {
|
||||||
assertEquals("109", resources.get(99).getIdElement().getValueAsString());
|
assertEquals("109", resources.get(99).getIdElement().getValueAsString());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Simulate results being removed from the search result cache but not the search cache
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
public void testFetchResultsReturnsNull() {
|
||||||
|
|
||||||
|
Search search = new Search();
|
||||||
|
search.setStatus(SearchStatusEnum.FINISHED);
|
||||||
|
search.setNumFound(100);
|
||||||
|
search.setTotalCount(100);
|
||||||
|
when(mySearchCacheSvc.fetchByUuid(eq("0000-1111"))).thenReturn(Optional.of(search));
|
||||||
|
|
||||||
|
when(mySearchResultCacheSvc.fetchResultPids(any(), anyInt(), anyInt())).thenReturn(null);
|
||||||
|
|
||||||
|
try {
|
||||||
|
mySvc.getResources("0000-1111", 0, 10, null);
|
||||||
|
fail();
|
||||||
|
} catch (ResourceGoneException e) {
|
||||||
|
assertEquals("Search ID \"0000-1111\" does not exist and may have expired", e.getMessage());
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Simulate results being removed from the search result cache but not the search cache
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
public void testFetchAllResultsReturnsNull() {
|
||||||
|
|
||||||
|
when(myDaoRegistry.getResourceDao(anyString())).thenReturn(myCallingDao);
|
||||||
|
when(myCallingDao.getContext()).thenReturn(ourCtx);
|
||||||
|
|
||||||
|
Search search = new Search();
|
||||||
|
search.setUuid("0000-1111");
|
||||||
|
search.setResourceType("Patient");
|
||||||
|
search.setStatus(SearchStatusEnum.PASSCMPLET);
|
||||||
|
search.setNumFound(5);
|
||||||
|
search.setSearchParameterMap(new SearchParameterMap());
|
||||||
|
when(mySearchCacheSvc.fetchByUuid(eq("0000-1111"))).thenReturn(Optional.of(search));
|
||||||
|
|
||||||
|
when(mySearchCacheSvc.tryToMarkSearchAsInProgress(any())).thenAnswer(t->{
|
||||||
|
search.setStatus(SearchStatusEnum.LOADING);
|
||||||
|
return Optional.of(search);
|
||||||
|
});
|
||||||
|
|
||||||
|
when(mySearchResultCacheSvc.fetchAllResultPids(any())).thenReturn(null);
|
||||||
|
|
||||||
|
try {
|
||||||
|
mySvc.getResources("0000-1111", 0, 10, null);
|
||||||
|
fail();
|
||||||
|
} catch (ResourceGoneException e) {
|
||||||
|
assertEquals("Search ID \"0000-1111\" does not exist and may have expired", e.getMessage());
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
public static class FailAfterNIterator extends BaseIterator<Long> implements IResultIterator {
|
public static class FailAfterNIterator extends BaseIterator<Long> implements IResultIterator {
|
||||||
|
|
||||||
private int myCount;
|
private int myCount;
|
||||||
|
|
|
@ -39,6 +39,10 @@ public enum SearchStatusEnum {
|
||||||
/**
|
/**
|
||||||
* The search failed and will not continue
|
* The search failed and will not continue
|
||||||
*/
|
*/
|
||||||
FAILED
|
FAILED,
|
||||||
|
/**
|
||||||
|
* The search has been expired and will be expunged shortly
|
||||||
|
*/
|
||||||
|
GONE
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue