Ensure that JPA PRESHOW interceptor broadcasts don't include expunged

resources

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java

@@ -31,6 +31,7 @@ import ca.uhn.fhir.jpa.entity.SearchTypeEnum;
 import ca.uhn.fhir.jpa.model.entity.BaseHasResource;
 import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
 import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc;
+import ca.uhn.fhir.jpa.util.InterceptorUtil;
 import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster;
 import ca.uhn.fhir.model.primitive.InstantDt;
 import ca.uhn.fhir.rest.api.server.*;
@@ -52,7 +53,6 @@ import javax.persistence.criteria.CriteriaQuery;
 import javax.persistence.criteria.Predicate;
 import javax.persistence.criteria.Root;
 import java.util.*;
-import java.util.stream.Collectors;
 
 public class PersistedJpaBundleProvider implements IBundleProvider {
 
@@ -319,24 +319,8 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 		// Execute the query and make sure we return distinct results
 		List<IBaseResource> resources = new ArrayList<>();
 		theSearchBuilder.loadResourcesByPid(thePids, includedPidList, resources, false, myRequest);
-//		resources.removeIf(t->t == null); aaaaaaaaabbbbbbbb
 
-		// Interceptor call: STORAGE_PRESHOW_RESOURCE
+		InterceptorUtil.fireStoragePreshowResource(resources, myRequest, myInterceptorBroadcaster);
-		// This can be used to remove results from the search result details before
-		// the user has a chance to know that they were in the results
-		if (resources.size() > 0) {
-			SimplePreResourceShowDetails accessDetails = new SimplePreResourceShowDetails(resources);
-			HookParams params = new HookParams()
-				.add(IPreResourceShowDetails.class, accessDetails)
-				.add(RequestDetails.class, myRequest)
-				.addIfMatchesType(ServletRequestDetails.class, myRequest);
-			JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, myRequest, Pointcut.STORAGE_PRESHOW_RESOURCES, params);
-
-			resources = resources
-				.stream()
-				.filter(t -> t != null)
-				.collect(Collectors.toList());
-		}
 
 		return resources;
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java

@@ -34,6 +34,7 @@ import ca.uhn.fhir.jpa.model.search.SearchStatusEnum;
 import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc;
 import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
+import ca.uhn.fhir.jpa.util.InterceptorUtil;
 import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster;
 import ca.uhn.fhir.model.api.Include;
 import ca.uhn.fhir.rest.api.CacheControlDirective;
@@ -485,6 +486,10 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
 
 			List<IBaseResource> resources = new ArrayList<>();
 			theSb.loadResourcesByPid(pids, includedPidsList, resources, false, theRequestDetails);
+
+			// Hook: STORAGE_PRESHOW_RESOURCES
+			InterceptorUtil.fireStoragePreshowResource(resources, theRequestDetails, myInterceptorBroadcaster);
+
 			return new SimpleBundleProvider(resources);
 		});
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/InterceptorUtil.java

@@ -0,0 +1,39 @@
+package ca.uhn.fhir.jpa.util;
+
+import ca.uhn.fhir.interceptor.api.HookParams;
+import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
+import ca.uhn.fhir.interceptor.api.Pointcut;
+import ca.uhn.fhir.rest.api.server.IPreResourceShowDetails;
+import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.api.server.SimplePreResourceShowDetails;
+import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+
+import java.util.List;
+
+public class InterceptorUtil {
+
+	/**
+	 * Fires {@link Pointcut#STORAGE_PRESHOW_RESOURCES} interceptor hook, and potentially remove resources
+	 * from the resource list
+	 */
+	public static void fireStoragePreshowResource(List<IBaseResource> theResources, RequestDetails theRequest, IInterceptorBroadcaster theInterceptorBroadcaster) {
+		theResources.removeIf(t -> t == null);
+
+		// Interceptor call: STORAGE_PRESHOW_RESOURCE
+		// This can be used to remove results from the search result details before
+		// the user has a chance to know that they were in the results
+		if (theResources.size() > 0) {
+			SimplePreResourceShowDetails accessDetails = new SimplePreResourceShowDetails(theResources);
+			HookParams params = new HookParams()
+				.add(IPreResourceShowDetails.class, accessDetails)
+				.add(RequestDetails.class, theRequest)
+				.addIfMatchesType(ServletRequestDetails.class, theRequest);
+			JpaInterceptorBroadcaster.doCallHooks(theInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PRESHOW_RESOURCES, params);
+
+			theResources.removeIf(t -> t == null);
+		}
+
+	}
+
+}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r5/StorageInterceptorEventsR5Test.java

@@ -0,0 +1,134 @@
+package ca.uhn.fhir.jpa.dao.r5;
+
+import ca.uhn.fhir.interceptor.api.Pointcut;
+import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
+import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider;
+import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
+import ca.uhn.fhir.jpa.util.ExpungeOptions;
+import ca.uhn.fhir.jpa.util.TestUtil;
+import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.api.server.IPreResourceShowDetails;
+import ca.uhn.fhir.rest.server.SimpleBundleProvider;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.r5.model.IdType;
+import org.hl7.fhir.r5.model.Patient;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
+
+@SuppressWarnings({"unchecked", "Duplicates"})
+public class StorageInterceptorEventsR5Test extends BaseJpaR5Test {
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(StorageInterceptorEventsR5Test.class);
+
+	@Test
+	public void before() {
+	}
+
+	@Test
+	public void testPreShowEventsDontIncludeExpungedResources_AsyncSearch() {
+		when(mySrd.getServer().getPagingProvider()).thenReturn(new DatabaseBackedPagingProvider());
+
+		Patient p0 = new Patient();
+		p0.setId("P0");
+		p0.addIdentifier().setValue("P0");
+		myPatientDao.update(p0);
+
+		Patient p1 = new Patient();
+		p1.setId("P1");
+		p1.addIdentifier().setValue("P1");
+		myPatientDao.update(p1);
+
+		Patient p2 = new Patient();
+		p2.setId("P2");
+		p2.addIdentifier().setValue("P2");
+		myPatientDao.update(p2);
+
+		AtomicInteger showedCounter = new AtomicInteger(0);
+		myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESHOW_RESOURCES, (thePointcut, theArgs) -> {
+			int showedCountThisPass = theArgs.get(IPreResourceShowDetails.class).size();
+			showedCounter.addAndGet(showedCountThisPass);
+		});
+
+		// Initial search returns all
+		SearchParameterMap params = new SearchParameterMap();
+		IBundleProvider search = myPatientDao.search(params, mySrd);
+		assertTrue(search.getClass().toString(), search instanceof PersistedJpaBundleProvider);
+		List<IBaseResource> found = search.getResources(0, 100);
+		assertEquals(3, found.size());
+		assertEquals(3, showedCounter.get());
+
+		// Delete and expunge one
+		myPatientDao.delete(new IdType("Patient/P1"));
+		mySystemDao.expunge(new ExpungeOptions().setExpungeDeletedResources(true), mySrd);
+		showedCounter.set(0);
+
+		// Next search should return only the non-expunged ones
+		params = new SearchParameterMap();
+		found = myPatientDao.search(params, mySrd).getResources(0, 100);
+		assertEquals(2, found.size());
+		assertEquals(2, showedCounter.get());
+	}
+
+	@Test
+	public void testPreShowEventsDontIncludeExpungedResources_SyncSearch() {
+		Patient p0 = new Patient();
+		p0.setId("P0");
+		p0.addIdentifier().setValue("P0");
+		myPatientDao.update(p0);
+
+		Patient p1 = new Patient();
+		p1.setId("P1");
+		p1.addIdentifier().setValue("P1");
+		myPatientDao.update(p1);
+
+		Patient p2 = new Patient();
+		p2.setId("P2");
+		p2.addIdentifier().setValue("P2");
+		myPatientDao.update(p2);
+
+		AtomicInteger showedCounter = new AtomicInteger(0);
+		myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESHOW_RESOURCES, (thePointcut, theArgs) -> {
+			int showedCountThisPass = theArgs.get(IPreResourceShowDetails.class).size();
+			showedCounter.addAndGet(showedCountThisPass);
+		});
+
+		// Initial search returns all
+		SearchParameterMap params = new SearchParameterMap();
+		params.setLoadSynchronous(true);
+		IBundleProvider search = myPatientDao.search(params, mySrd);
+		assertTrue(search.getClass().toString(), search instanceof SimpleBundleProvider);
+		List<IBaseResource> found = search.getResources(0, 100);
+		assertEquals(3, found.size());
+		assertEquals(3, showedCounter.get());
+
+		// Delete and expunge one
+		myPatientDao.delete(new IdType("Patient/P1"));
+		mySystemDao.expunge(new ExpungeOptions().setExpungeDeletedResources(true), mySrd);
+		showedCounter.set(0);
+
+		// Next search should return only the non-expunged ones
+		params = new SearchParameterMap();
+		found = myPatientDao.search(params, mySrd).getResources(0, 100);
+		assertEquals(2, found.size());
+		assertEquals(2, showedCounter.get());
+	}
+
+	@After
+	public void after() {
+		myInterceptorRegistry.unregisterAllInterceptors();
+	}
+
+
+	@AfterClass
+	public static void afterClassClearContext() {
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+}

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java

@@ -59,7 +59,6 @@ public class SearchParameterMap implements Serializable {
 	private SortSpec mySort;
 	private SummaryEnum mySummaryMode;
 	private SearchTotalModeEnum mySearchTotalMode;
-	private Long myQueryCacheExpiryTime;
 
 	/**
 	 * Constructor
@@ -75,14 +74,6 @@ public class SearchParameterMap implements Serializable {
 		add(theName, theParam);
 	}
 
-    public Long getQueryCacheExpiryTime() {
-        return myQueryCacheExpiryTime;
-    }
-    
-    public void setQueryCacheExpiryTime(Long theQueryCacheExpiryTime) {
-        myQueryCacheExpiryTime = theQueryCacheExpiryTime;
-    }
-    
 	public SummaryEnum getSummaryMode() {
 		return mySummaryMode;
 	}

src/changes/changes.xml

@@ -322,6 +322,10 @@
 				The subscription triggering operation was not able to handle commas within search URLs being
 				used to trigger resources for subscription checking. This has been corrected.
 			</action>
+			<action type="fix">
+				In some cases where resources were recently expunged, null entries could be passed to JPA interceptors registered
+				against the STORAGE_PRESHOW_RESOURCES hook.
+			</action>
 		</release>
 		<release version="4.0.3" date="2019-09-03" description="Igloo (Point Release)">
 			<action type="fix">