Improve paging in JPA server for _history operation

.editorconfig

@@ -0,0 +1,9 @@
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+
+[*.java]
+charset = utf-8
+indent_style = tab

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/HistoryMethodBinding.java

@@ -201,7 +201,12 @@ public class HistoryMethodBinding extends BaseResourceReturningMethodBinding {
 
 			@Override
 			public Integer preferredPageSize() {
-				return null;
+				return resources.preferredPageSize();
+			}
+
+			@Override
+			public String getUuid() {
+				return resources.getUuid();
 			}
 		};
 	}

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/BasePagingProvider.java

@@ -0,0 +1,36 @@
+package ca.uhn.fhir.rest.server;
+
+import org.apache.commons.lang3.Validate;
+
+public abstract class BasePagingProvider implements IPagingProvider {
+
+	private int myDefaultPageSize = 10;
+	private int myMaximumPageSize = 50;
+
+	public BasePagingProvider() {
+		super();
+	}
+
+	@Override
+	public int getDefaultPageSize() {
+		return myDefaultPageSize;
+	}
+
+	@Override
+	public int getMaximumPageSize() {
+		return myMaximumPageSize;
+	}
+
+	public BasePagingProvider setDefaultPageSize(int theDefaultPageSize) {
+		Validate.isTrue(theDefaultPageSize > 0, "size must be greater than 0");
+		myDefaultPageSize = theDefaultPageSize;
+		return this;
+	}
+
+	public BasePagingProvider setMaximumPageSize(int theMaximumPageSize) {
+		Validate.isTrue(theMaximumPageSize > 0, "size must be greater than 0");
+		myMaximumPageSize = theMaximumPageSize;
+		return this;
+	}
+
+}

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/BundleProviders.java

@@ -64,6 +64,11 @@ public class BundleProviders {
 			public Integer preferredPageSize() {
 				return null;
 			}
+
+			@Override
+			public String getUuid() {
+				return null;
+			}
 		};
 	}
 

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/FifoMemoryPagingProvider.java

@@ -10,7 +10,7 @@ package ca.uhn.fhir.rest.server;
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  * 
- *      http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  * 
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -25,11 +25,9 @@ import java.util.UUID;
 
 import org.apache.commons.lang3.Validate;
 
-public class FifoMemoryPagingProvider implements IPagingProvider {
+public class FifoMemoryPagingProvider extends BasePagingProvider implements IPagingProvider {
 
 	private LinkedHashMap<String, IBundleProvider> myBundleProviders;
-	private int myDefaultPageSize=10;
-	private int myMaximumPageSize=50;
 	private int mySize;
 
 	public FifoMemoryPagingProvider(int theSize) {
@@ -39,33 +37,11 @@ public class FifoMemoryPagingProvider implements IPagingProvider {
 		myBundleProviders = new LinkedHashMap<String, IBundleProvider>(mySize);
 	}
 
-	@Override
-	public int getDefaultPageSize() {
-		return myDefaultPageSize;
-	}
-
-	@Override
-	public int getMaximumPageSize() {
-		return myMaximumPageSize;
-	}
-
 	@Override
 	public synchronized IBundleProvider retrieveResultList(String theId) {
 		return myBundleProviders.get(theId);
 	}
 
-	public FifoMemoryPagingProvider setDefaultPageSize(int theDefaultPageSize) {
-		Validate.isTrue(theDefaultPageSize > 0, "size must be greater than 0");
-		myDefaultPageSize = theDefaultPageSize;
-		return this;
-	}
-
-	public FifoMemoryPagingProvider setMaximumPageSize(int theMaximumPageSize) {
-		Validate.isTrue(theMaximumPageSize > 0, "size must be greater than 0");
-		myMaximumPageSize = theMaximumPageSize;
-		return this;
-	}
-
 	@Override
 	public synchronized String storeResultList(IBundleProvider theList) {
 		while (myBundleProviders.size() > mySize) {

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IBundleProvider.java

@@ -62,4 +62,15 @@ public interface IBundleProvider {
 	 */
 	InstantDt getPublished();
 
+	/**
+	 * Returns the UUID associated with this search. Note that this
+	 * does not need to return a non-null value unless it a
+	 * {@link IPagingProvider} is being used that requires UUIDs
+	 * being returned.
+	 * <p>
+	 * Otherwise you may simply return {@code null}
+	 * </p>
+	 */
+	public String getUuid();
+
 }

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IResourceProvider.java

@@ -10,7 +10,7 @@ package ca.uhn.fhir.rest.server;
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  * 
- *      http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  * 
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/SimpleBundleProvider.java

@@ -65,5 +65,10 @@ public class SimpleBundleProvider implements IBundleProvider {
 	public Integer preferredPageSize() {
 		return null;
 	}
+
+	@Override
+	public String getUuid() {
+		return null;
+	}
 	
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseSearchParamRegistry.java

@@ -10,7 +10,7 @@ package ca.uhn.fhir.jpa.dao;
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  * 
- *      http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  * 
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -56,27 +56,41 @@ public abstract class BaseSearchParamRegistry implements ISearchParamRegistry {
 		// nothing by default
 	}
 
-	public Map<String, Map<String, RuntimeSearchParam>> getBuiltInSearchParams() {
+	@Override
+	public Map<String, Map<String, RuntimeSearchParam>> getActiveSearchParams() {
 		return myBuiltInSearchParams;
 	}
 
 	@Override
-	public Map<String,RuntimeSearchParam> getActiveSearchParams(String theResourceName) {
+	public Map<String, RuntimeSearchParam> getActiveSearchParams(String theResourceName) {
 		Validate.notBlank(theResourceName, "theResourceName must not be blank or null");
 
 		return myBuiltInSearchParams.get(theResourceName);
 	}
 
+	@Override
+	public Collection<RuntimeSearchParam> getAllSearchParams(String theResourceName) {
+		Validate.notBlank(theResourceName, "theResourceName must not be null or blank");
+
+		Map<String, RuntimeSearchParam> map = myBuiltInSearchParams.get(theResourceName);
+		map = ObjectUtils.defaultIfNull(map, EMPTY_SP_MAP);
+		return Collections.unmodifiableCollection(map.values());
+	}
+
+	public Map<String, Map<String, RuntimeSearchParam>> getBuiltInSearchParams() {
+		return myBuiltInSearchParams;
+	}
+
 	@PostConstruct
 	public void postConstruct() {
-		Map<String, Map<String, RuntimeSearchParam>> resourceNameToSearchParams = new HashMap<String, Map<String,RuntimeSearchParam>>();
+		Map<String, Map<String, RuntimeSearchParam>> resourceNameToSearchParams = new HashMap<String, Map<String, RuntimeSearchParam>>();
 
 		for (IFhirResourceDao<?> nextDao : myDaos) {
 			RuntimeResourceDefinition nextResDef = myCtx.getResourceDefinition(nextDao.getResourceType());
 			String nextResourceName = nextResDef.getName();
 			HashMap<String, RuntimeSearchParam> nameToParam = new HashMap<String, RuntimeSearchParam>();
 			resourceNameToSearchParams.put(nextResourceName, nameToParam);
-			
+
 			for (RuntimeSearchParam nextSp : nextResDef.getSearchParams()) {
 				nameToParam.put(nextSp.getName(), nextSp);
 			}
@@ -85,13 +99,4 @@ public abstract class BaseSearchParamRegistry implements ISearchParamRegistry {
 		myBuiltInSearchParams = Collections.unmodifiableMap(resourceNameToSearchParams);
 	}
 
-	@Override
-	public Collection<RuntimeSearchParam> getAllSearchParams(String theResourceName) {
-		Validate.notBlank(theResourceName, "theResourceName must not be null or blank");
-		
-		Map<String, RuntimeSearchParam> map = myBuiltInSearchParams.get(theResourceName);
-		map = ObjectUtils.defaultIfNull(map, EMPTY_SP_MAP);
-		return Collections.unmodifiableCollection(map.values());
-	}
-
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/ISearchParamRegistry.java

@@ -27,10 +27,12 @@ import ca.uhn.fhir.context.RuntimeSearchParam;
 
 public interface ISearchParamRegistry {
 
+	void forceRefresh();
+
+	Map<String, Map<String, RuntimeSearchParam>> getActiveSearchParams();
+
 	Map<String,RuntimeSearchParam> getActiveSearchParams(String theResourceName);
 
 	Collection<RuntimeSearchParam> getAllSearchParams(String theResourceName);
 
-	void forceRefresh();
-
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java

@@ -2309,6 +2309,11 @@ public class SearchBuilder {
 		public int size() {
 			return myPids.size();
 		}
+
+		@Override
+		public String getUuid() {
+			return null;
+		}
 	}
 
 }

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

@@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.search;
 
 import javax.persistence.EntityManager;
 
+import org.apache.commons.lang3.Validate;
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.PlatformTransactionManager;
@@ -30,10 +31,12 @@ import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jpa.dao.IDao;
 import ca.uhn.fhir.jpa.dao.IFhirSystemDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchResultDao;
+import ca.uhn.fhir.rest.server.BasePagingProvider;
 import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
 import ca.uhn.fhir.rest.server.IBundleProvider;
+import ca.uhn.fhir.rest.server.IPagingProvider;
 
-public class DatabaseBackedPagingProvider extends FifoMemoryPagingProvider {
+public class DatabaseBackedPagingProvider extends BasePagingProvider implements IPagingProvider {
 
 	@Autowired
 	private PlatformTransactionManager thePlatformTransactionManager;
@@ -46,29 +49,36 @@ public class DatabaseBackedPagingProvider extends FifoMemoryPagingProvider {
 	@Autowired
 	private IFhirSystemDao<?, ?> theDao;
 
+	/**
+	 * Constructor
+	 * @deprecated Use {@link DatabaseBackedPagingProvider} as this constructor has no purpose
+	 */
+	@Deprecated
 	public DatabaseBackedPagingProvider(int theSize) {
-		super(theSize);
+		this();
+	}
+
+	/**
+	 * Constructor
+	 */
+	public DatabaseBackedPagingProvider() {
+		super();
 	}
 
 	@Override
 	public synchronized IBundleProvider retrieveResultList(String theId) {
-		IBundleProvider retVal = super.retrieveResultList(theId);
-		if (retVal == null) {
-			PersistedJpaBundleProvider provider = new PersistedJpaBundleProvider(theId, theDao);
-			if (!provider.ensureSearchEntityLoaded()) {
-				return null;
-			}
-			retVal = provider;
+		PersistedJpaBundleProvider provider = new PersistedJpaBundleProvider(theId, theDao);
+		if (!provider.ensureSearchEntityLoaded()) {
+			return null;
 		}
-		return retVal;
+		return provider;
 	}
 
 	@Override
 	public synchronized String storeResultList(IBundleProvider theList) {
-		if (theList instanceof PersistedJpaBundleProvider) {
-			return ((PersistedJpaBundleProvider)theList).getSearchUuid();
-		}
-		return super.storeResultList(theList);
+		String uuid = theList.getUuid();
+		Validate.notNull(uuid);
+		return uuid;
 	}
 
 }

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

@@ -19,13 +19,7 @@ package ca.uhn.fhir.jpa.search;
  * limitations under the License.
  * #L%
  */
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 
 import javax.persistence.EntityManager;
 import javax.persistence.NoResultException;
@@ -49,11 +43,7 @@ import ca.uhn.fhir.jpa.dao.IDao;
 import ca.uhn.fhir.jpa.dao.SearchBuilder;
 import ca.uhn.fhir.jpa.dao.data.ISearchDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchResultDao;
-import ca.uhn.fhir.jpa.entity.BaseHasResource;
-import ca.uhn.fhir.jpa.entity.ResourceHistoryTable;
-import ca.uhn.fhir.jpa.entity.Search;
-import ca.uhn.fhir.jpa.entity.SearchResult;
-import ca.uhn.fhir.jpa.entity.SearchTypeEnum;
+import ca.uhn.fhir.jpa.entity.*;
 import ca.uhn.fhir.model.primitive.InstantDt;
 import ca.uhn.fhir.rest.server.IBundleProvider;
 
@@ -220,7 +210,7 @@ public final class PersistedJpaBundleProvider implements IBundleProvider {
 		});
 	}
 
-	public String getSearchUuid() {
+	public String getUuid() {
 		return myUuid;
 	}
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/SearchParamExtractorDstu3Test.java

@@ -63,6 +63,11 @@ public class SearchParamExtractorDstu3Test {
 			public Collection<RuntimeSearchParam> getAllSearchParams(String theResourceName) {
 				throw new UnsupportedOperationException();
 			}
+
+			@Override
+			public Map<String, Map<String, RuntimeSearchParam>> getActiveSearchParams() {
+				throw new UnsupportedOperationException();
+			}
 		};
 		
 		SearchParamExtractorDstu3 extractor = new SearchParamExtractorDstu3(ourCtx, ourValidationSupport, searchParamRegistry);

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchDstu2Test.java

@@ -575,6 +575,11 @@ public class SearchDstu2Test {
 				public int size() {
 					return 0;
 				}
+
+				@Override
+				public String getUuid() {
+					return null;
+				}
 			};
 		}
 

src/changes/changes.xml

@@ -138,6 +138,13 @@
 				JPA server did not return an OperationOutcome in the response for
 				a normal delete operation.
 			</action>
+			<action type="fix">
+				Fix an issue in JPA server where _history results were kept in memory instead
+				of being spooled to the database as they should be. Note that as a part of this fix
+				a new method was added to 
+				<![CDATA[<code>IBundleProvider</code> called <code>getUuid()</code>]]>. This
+				method may return <![CDATA[<code>null</code>]]> in any current cases.
+			</action>
 		</release>
 		<release version="2.2" date="2016-12-20">
 			<action type="add">