Fix offset error when returning multiple pages in JPA search

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

@@ -669,7 +669,9 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 				next.processingCompletedNormally(requestDetails);
 			}
 
-			outputStreamOrWriter.close();
+			if (outputStreamOrWriter != null) {
+				outputStreamOrWriter.close();
+			}
 			
 		} catch (NotModifiedException e) {
 

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

@@ -256,7 +256,7 @@ public final class PersistedJpaBundleProvider implements IBundleProvider {
 		return mySearchEntity.getTotalCount();
 	}
 
-	private Pageable toPage(int theFromIndex, int theToIndex) {
+	public static Pageable toPage(final int theFromIndex, int theToIndex) {
 		int pageSize = theToIndex - theFromIndex;
 		if (pageSize < 1) {
 			return null;
@@ -264,7 +264,14 @@ public final class PersistedJpaBundleProvider implements IBundleProvider {
 
 		int pageIndex = theFromIndex / pageSize;
 
-		Pageable page = new PageRequest(pageIndex, pageSize);
+		Pageable page = new PageRequest(pageIndex, pageSize) {
+			private static final long serialVersionUID = 1L;
+
+			@Override
+			public int getOffset() {
+				return theFromIndex;
+			}};
+		
 		return page;
 	}
 }

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java

@@ -42,10 +42,7 @@ import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicNameValuePair;
 import org.hl7.fhir.dstu3.model.*;
-import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
+import org.hl7.fhir.dstu3.model.Bundle.*;
-import org.hl7.fhir.dstu3.model.Bundle.BundleType;
-import org.hl7.fhir.dstu3.model.Bundle.HTTPVerb;
-import org.hl7.fhir.dstu3.model.Bundle.SearchEntryMode;
 import org.hl7.fhir.dstu3.model.Encounter.EncounterLocationComponent;
 import org.hl7.fhir.dstu3.model.Encounter.EncounterStatus;
 import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender;
@@ -74,6 +71,7 @@ import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.api.SummaryEnum;
 import ca.uhn.fhir.rest.api.ValidationModeEnum;
 import ca.uhn.fhir.rest.client.IGenericClient;
+import ca.uhn.fhir.rest.gclient.StringClientParam;
 import ca.uhn.fhir.rest.param.*;
 import ca.uhn.fhir.rest.server.Constants;
 import ca.uhn.fhir.rest.server.IBundleProvider;
@@ -101,13 +99,13 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 		obs1.setValue(new Quantity(123));
 		obs1.setComment("obs1");
 		IIdType id1 = myObservationDao.create(obs1, mySrd).getId().toUnqualifiedVersionless();
-		
+
 		Observation obs2 = new Observation();
 		obs2.getCode().setText("Diastolic Blood Pressure");
 		obs2.setStatus(ObservationStatus.FINAL);
 		obs2.setValue(new Quantity(81));
 		IIdType id2 = myObservationDao.create(obs2, mySrd).getId().toUnqualifiedVersionless();
-			
+
 		HttpGet get = new HttpGet(ourServerBase + "/Observation?_content=systolic&_pretty=true");
 		CloseableHttpResponse response = ourHttpClient.execute(get);
 		try {
@@ -119,7 +117,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 			response.close();
 		}
 	}
-	
+
 	/**
 	 * See #441
 	 */
@@ -128,7 +126,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 		Medication medication = new Medication();
 		medication.getCode().addCoding().setSystem("SYSTEM").setCode("04823543");
 		IIdType medId = myMedicationDao.create(medication).getId().toUnqualifiedVersionless();
-		
+
 		MedicationAdministration ma = new MedicationAdministration();
 		ma.setMedication(new Reference(medId));
 		IIdType moId = myMedicationAdministrationDao.create(ma).getId().toUnqualifiedVersionless();
@@ -146,8 +144,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 
 	}
 
-
-	
 	/**
 	 * Per message from David Hay on Skype
 	 */
@@ -156,32 +152,26 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 		String inputString = IOUtils.toString(getClass().getResourceAsStream("/david_big_bundle.json"), StandardCharsets.UTF_8);
 		Bundle inputBundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, inputString);
 		inputBundle.setType(BundleType.TRANSACTION);
-		
+
 		Set<String> allIds = new TreeSet<String>();
 		for (BundleEntryComponent nextEntry : inputBundle.getEntry()) {
 			nextEntry.getRequest().setMethod(HTTPVerb.PUT);
 			nextEntry.getRequest().setUrl(nextEntry.getResource().getId());
 			allIds.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue());
 		}
-		
+
 		mySystemDao.transaction(mySrd, inputBundle);
 
-		Bundle responseBundle = ourClient
+		Bundle responseBundle = ourClient.operation().onInstance(new IdType("Patient/A161443")).named("everything").withParameter(Parameters.class, "_count", new IntegerType(50)).useHttpGet()
-			.operation()
+				.returnResourceType(Bundle.class).execute();
-			.onInstance(new IdType("Patient/A161443"))
+
-			.named("everything")
-			.withParameter(Parameters.class, "_count", new IntegerType(50))
-			.useHttpGet()
-			.returnResourceType(Bundle.class)
-			.execute();
-		
 		TreeSet<String> ids = new TreeSet<String>();
 		for (int i = 0; i < responseBundle.getEntry().size(); i++) {
 			for (BundleEntryComponent nextEntry : responseBundle.getEntry()) {
 				ids.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue());
 			}
 		}
-		
+
 		String nextUrl = responseBundle.getLink("next").getUrl();
 		responseBundle = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl);
 		for (int i = 0; i < responseBundle.getEntry().size(); i++) {
@@ -189,20 +179,84 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 				ids.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue());
 			}
 		}
-		
+
 		assertThat(ids, hasItem("List/A161444"));
 		assertThat(ids, hasItem("List/A161468"));
 		assertThat(ids, hasItem("List/A161500"));
 
 		assertEquals(null, responseBundle.getLink("next"));
-		
+
 		ourLog.info("Expected {} - {}", allIds.size(), allIds);
 		ourLog.info("Actual   {} - {}", ids.size(), ids);
 		assertEquals(allIds, ids);
+
+	}
+
+	/**
+	 * Per message from David Hay on Skype
+	 */
+	@Test
+	public void testEverythingWithLargeSet2() throws Exception {
+		Patient p = new Patient();
+		p.setActive(true);
+		IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless();
+
+		for (int i = 1; i < 77; i++) {
+			Observation obs = new Observation();
+			obs.setId("A" + StringUtils.leftPad(Integer.toString(i), 2, '0'));
+			obs.setSubject(new Reference(id));
+			ourClient.update().resource(obs).execute();
+		}
+
+		Bundle responseBundle = ourClient.operation().onInstance(id).named("everything").withParameter(Parameters.class, "_count", new IntegerType(50)).useHttpGet().returnResourceType(Bundle.class)
+				.execute();
+
+		TreeSet<String> ids = new TreeSet<String>();
+		for (int i = 0; i < responseBundle.getEntry().size(); i++) {
+			for (BundleEntryComponent nextEntry : responseBundle.getEntry()) {
+				ids.add(nextEntry.getResource().getIdElement().getIdPart());
+			}
+		}
+
+		ourLog.info("Have {} IDs: {}", ids.size(), ids);
+
+		BundleLinkComponent nextLink = responseBundle.getLink("next");
+		while (nextLink != null) {
+			String nextUrl = nextLink.getUrl();
+			responseBundle = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl);
+			for (int i = 0; i < responseBundle.getEntry().size(); i++) {
+				for (BundleEntryComponent nextEntry : responseBundle.getEntry()) {
+					ids.add(nextEntry.getResource().getIdElement().getIdPart());
+				}
+			}
+
+			ourLog.info("Have {} IDs: {}", ids.size(), ids);
+			nextLink = responseBundle.getLink("next");
+		}
+
+		assertThat(ids, hasItem(id.getIdPart()));
+		for (int i = 1; i < 77; i++) {
+			assertThat(ids, hasItem("A" + StringUtils.leftPad(Integer.toString(i), 2, '0')));
+		}
+		assertEquals(77, ids.size());
+	}
+
+	@Test
+	public void testEmptySearch() throws Exception {
+		Bundle responseBundle;
+		
+		responseBundle = ourClient.search().forResource(Patient.class).returnBundle(Bundle.class).execute();
+		assertEquals(0, responseBundle.getTotal());
+		
+		responseBundle = ourClient.search().forResource(Patient.class).where(Patient.NAME.matches().value("AAA")).returnBundle(Bundle.class).execute();
+		assertEquals(0, responseBundle.getTotal());
+		
+		responseBundle = ourClient.search().forResource(Patient.class).where(new StringClientParam("_content").matches().value("AAA")).returnBundle(Bundle.class).execute();
+		assertEquals(0, responseBundle.getTotal());
+		
 		
 	}
 
-	
 	/**
 	 * See #411
 	 * 
@@ -214,15 +268,15 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 		patient.addIdentifier().setSystem("urn:system").setValue("0");
 		patient.addName().addFamily("testSearchWithMixedParams").addGiven("Joe");
 		myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless();
-		
+
-		HttpPost httpPost = new HttpPost(ourServerBase +  "/Patient/_search?_format=application/xml");
+		HttpPost httpPost = new HttpPost(ourServerBase + "/Patient/_search?_format=application/xml");
-		httpPost.addHeader("Cache-Control","no-cache");
+		httpPost.addHeader("Cache-Control", "no-cache");
 		List<NameValuePair> parameters = Lists.newArrayList();
 		parameters.add(new BasicNameValuePair("name", "Smith"));
 		httpPost.setEntity(new UrlEncodedFormEntity(parameters));
 
 		ourLog.info("Outgoing post: {}", httpPost);
-		
+
 		CloseableHttpResponse status = ourHttpClient.execute(httpPost);
 		try {
 			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
@@ -234,7 +288,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 
 	}
 
-	
 	@Test
 	public void testSearchPagingKeepsOldSearches() throws Exception {
 		String methodName = "testSearchPagingKeepsOldSearches";
@@ -1875,7 +1928,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 
 		assertEquals(id.withVersion("1").getValue(), history.getEntry().get(2).getResource().getId());
 		assertEquals(1, ((Patient) history.getEntry().get(2).getResource()).getName().size());
-		
+
 		try {
 			myBundleDao.validate(history, null, null, null, null, null, mySrd);
 		} catch (PreconditionFailedException e) {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProviderTest.java

@@ -0,0 +1,17 @@
+package ca.uhn.fhir.jpa.search;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+import org.springframework.data.domain.Pageable;
+
+public class PersistedJpaBundleProviderTest {
+
+	@Test
+	public void testGetPage() {
+		Pageable page = PersistedJpaBundleProvider.toPage(50, 73);
+		assertEquals(50, page.getOffset());
+//		assertEquals(50, page.get);
+	}
+	
+}

hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml

@@ -10,11 +10,11 @@
 		</encoder>
 	</appender>
 
-	<logger name="org.springframework.web.socket.handler.ExceptionWebSocketHandlerDecorator" additivity="false" level="debug">
+	<logger name="org.springframework.web.socket.handler.ExceptionWebSocketHandlerDecorator" additivity="false" level="info">
 		<appender-ref ref="STDOUT" />
 	</logger>
 
-	<logger name="ca.uhn.fhir.jpa.dao.FhirResourceDaoSubscriptionDstu2" additivity="false" level="debug">
+	<logger name="ca.uhn.fhir.jpa.dao.FhirResourceDaoSubscriptionDstu2" additivity="false" level="info">
 		<appender-ref ref="STDOUT" />
 	</logger>
 
@@ -34,7 +34,7 @@
 	</logger>
 	
 	<!-- Set to 'trace' to enable SQL logging -->
-	<logger name="org.hibernate.SQL" additivity="false" level="trace">
+	<logger name="org.hibernate.SQL" additivity="false" level="info">
 		<appender-ref ref="STDOUT" />
 	</logger>
 	<!-- Set to 'trace' to enable SQL Value logging -->

hapi-fhir-jpaserver-uhnfhirtest/src/test/java/ca/uhn/fhirtest/UhnFhirTestApp.java

@@ -28,7 +28,7 @@ public class UhnFhirTestApp {
 		System.setProperty("fhir.db.location", "./target/testdb");
 		System.setProperty("fhir.db.location.dstu2", "./target/testdb_dstu2");
 		System.setProperty("fhir.lucene.location.dstu2", "./target/testlucene_dstu2");
-		System.setProperty("fhir.db.location.dstu3", "./target/testdb_dstu3");
+		System.setProperty("fhir.db.location.dstu3", "./target/fhirtest_dstu3");
 		System.setProperty("fhir.lucene.location.dstu3", "./target/testlucene_dstu3");
 		System.setProperty("fhir.db.location.tdl2", "./target/testdb_tdl2");
 		System.setProperty("fhir.lucene.location.tdl2", "./target/testlucene_tdl2");

src/changes/changes.xml

@@ -16,6 +16,11 @@
 					</ul>
 				]]>
 			</action>
+			<action type="fix">
+				Fix a fairly significant issue in JPA Server when using the <![CDATA[<code>DatabaseBackedPagingProvider</code>]]>: When paging over the results
+				of a search / $everything operation, under certain circumstances resources may be missing from the last page of results
+				that is returned. Thanks to David Hay for reporting!
+			</action>
 			<action type="add">
 				Client, Server, and JPA server now support experimental support
 				for