From 2c3283fd343d4dc4d5e34e9ed4a0d7ffcb112398 Mon Sep 17 00:00:00 2001 From: Tuomo Ala-Vannesluoma Date: Thu, 12 Sep 2019 13:36:56 +0300 Subject: [PATCH 01/15] Add test for search method binding matching --- .../method/SearchMethodBindingTest.java | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java new file mode 100644 index 00000000000..4a84aa56c52 --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java @@ -0,0 +1,111 @@ +package ca.uhn.fhir.rest.server.method; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.rest.annotation.OptionalParam; +import ca.uhn.fhir.rest.annotation.RequiredParam; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.api.RequestTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import com.google.common.collect.ImmutableMap; +import org.hamcrest.Matchers; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.util.Map; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SearchMethodBindingTest { + + private static final TestResourceProvider TEST_RESOURCE_PROVIDER = new TestResourceProvider(); + + private FhirContext fhirContext; + + @Before + public void setUp() { + fhirContext = mock(FhirContext.class); + RuntimeResourceDefinition definition = mock(RuntimeResourceDefinition.class); + when(definition.isBundle()).thenReturn(false); + when(fhirContext.getResourceDefinition(any(Class.class))).thenReturn(definition); + } + + @Test // fails + public void methodShouldNotMatchWhenExtraUnderscoreQueryParameter() throws NoSuchMethodException { + Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))), + Matchers.is(false)); + Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))), + Matchers.is(false)); + Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))), + Matchers.is(false)); + } + + @Test + public void methodShouldNotMatchWhenExtraQueryParameter() throws NoSuchMethodException { + Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "extra", new String[]{"test"}))), + Matchers.is(false)); + Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "extra", new String[]{"test"}))), + Matchers.is(false)); + Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "extra", new String[]{"test"}))), + Matchers.is(false)); + } + + @Test + public void methodMatchesOwnParams() throws NoSuchMethodException { + Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}))), + Matchers.is(true)); + Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "test", new String[]{"test"}))), + Matchers.is(true)); + Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_test", new String[]{"test"}))), + Matchers.is(true)); + } + + private SearchMethodBinding getBinding(String name, Class... parameters) throws NoSuchMethodException { + return new SearchMethodBinding(IBaseResource.class, + IBaseResource.class, + TestResourceProvider.class.getMethod(name, parameters), + fhirContext, + TEST_RESOURCE_PROVIDER); + } + + private RequestDetails mockSearchRequest(Map params) { + RequestDetails requestDetails = mock(RequestDetails.class); + when(requestDetails.getOperation()).thenReturn("_search"); + when(requestDetails.getRequestType()).thenReturn(RequestTypeEnum.GET); + when(requestDetails.getParameters()).thenReturn(params); + return requestDetails; + } + + private static class TestResourceProvider { + + @Search + public IBaseResource param(@RequiredParam(name = "param") String param) { + return null; + } + + @Search + public IBaseResource paramAndTest(@RequiredParam(name = "param") String param, @OptionalParam(name = "test") String test) { + return null; + } + + @Search + public IBaseResource paramAndUnderscoreTest(@RequiredParam(name = "param") String param, @OptionalParam(name = "_test") String test) { + return null; + } + + } + +} From 945cc6419fa3d2af621e3637a6c874085e8506bf Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Tue, 24 Sep 2019 18:13:53 -0400 Subject: [PATCH 02/15] fixed npe --- .../jpa/migrate/taskdef/ModifyColumnTask.java | 2 +- .../jpa/migrate/taskdef/ModifyColumnTest.java | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java index 7e222905ce1..f0683fe496d 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java @@ -52,7 +52,7 @@ public class ModifyColumnTask extends BaseTableColumnTypeTask throw new InternalErrorException(e); } - if (isNoColumnShrink()) { + if (getColumnLength() != null && isNoColumnShrink()) { long existingLength = existingType.getLength() != null ? existingType.getLength() : 0; if (existingLength > getColumnLength()) { setColumnLength(existingLength); diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTest.java index 6a2cb7c63d3..470870226f0 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTest.java @@ -98,6 +98,45 @@ public class ModifyColumnTest extends BaseTest { } + @Test + public void testNoShrink_ColumnMakeDateNullable() throws SQLException { + executeSql("create table SOMETABLE (PID bigint not null, DATECOL timestamp not null)"); + assertFalse(JdbcUtils.isColumnNullable(getConnectionProperties(), "SOMETABLE", "PID")); + assertFalse(JdbcUtils.isColumnNullable(getConnectionProperties(), "SOMETABLE", "DATECOL")); + assertEquals(new JdbcUtils.ColumnType(BaseTableColumnTypeTask.ColumnTypeEnum.LONG, 19), JdbcUtils.getColumnType(getConnectionProperties(), "SOMETABLE", "PID")); + assertEquals(new JdbcUtils.ColumnType(BaseTableColumnTypeTask.ColumnTypeEnum.DATE_TIMESTAMP, 26), JdbcUtils.getColumnType(getConnectionProperties(), "SOMETABLE", "DATECOL")); + + getMigrator().setNoColumnShrink(true); + + // PID + ModifyColumnTask task = new ModifyColumnTask(); + task.setTableName("SOMETABLE"); + task.setColumnName("PID"); + task.setColumnType(AddColumnTask.ColumnTypeEnum.LONG); + task.setNullable(true); + getMigrator().addTask(task); + + // STRING + task = new ModifyColumnTask(); + task.setTableName("SOMETABLE"); + task.setColumnName("DATECOL"); + task.setColumnType(AddColumnTask.ColumnTypeEnum.DATE_TIMESTAMP); + task.setNullable(true); + getMigrator().addTask(task); + + // Do migration + getMigrator().migrate(); + + assertTrue(JdbcUtils.isColumnNullable(getConnectionProperties(), "SOMETABLE", "PID")); + assertTrue(JdbcUtils.isColumnNullable(getConnectionProperties(), "SOMETABLE", "DATECOL")); + assertEquals(new JdbcUtils.ColumnType(BaseTableColumnTypeTask.ColumnTypeEnum.LONG, 19), JdbcUtils.getColumnType(getConnectionProperties(), "SOMETABLE", "PID")); + assertEquals(new JdbcUtils.ColumnType(BaseTableColumnTypeTask.ColumnTypeEnum.DATE_TIMESTAMP, 26), JdbcUtils.getColumnType(getConnectionProperties(), "SOMETABLE", "DATECOL")); + + // Make sure additional migrations don't crash + getMigrator().migrate(); + getMigrator().migrate(); + } + @Test public void testColumnMakeNotNullable() throws SQLException { executeSql("create table SOMETABLE (PID bigint, TEXTCOL varchar(255))"); From 4c064186a99c17e2d333685d591173a63646fe97 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 25 Sep 2019 05:39:00 -0400 Subject: [PATCH 03/15] Try to fix intermittent testfailure --- .../ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java index a27ce5db65f..167e9bd12aa 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java @@ -98,6 +98,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { assertEquals(myObservationIds.subList(0, 20), interceptedResourceIds); // Fetch the next 30 (do cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); resources = outcome.getResources(10, 40); returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(myObservationIds.subList(10, 40), returnedIdValues); @@ -129,6 +130,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { assertEquals(myObservationIds.subList(0, 20), interceptedResourceIds); // Fetch the next 30 (do cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); resources = outcome.getResources(10, 40); returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(myObservationIdsEvenOnly.subList(10, 25), returnedIdValues); @@ -160,6 +162,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { assertEquals(myObservationIds, interceptedResourceIds); // Fetch the next 30 (do cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); resources = outcome.getResources(10, 40); returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(myObservationIdsEvenOnly.subList(10, 25), returnedIdValues); @@ -184,6 +187,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -209,6 +213,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -232,6 +237,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -257,6 +263,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIds, myObservationIds), sort(returnedIdValues)); @@ -281,6 +288,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -304,6 +312,7 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) + outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 10); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); ourLog.info("Returned values: {}", returnedIdValues); From 2efef135285e2a4e707d5fa9626eb8f999581ef5 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 25 Sep 2019 05:43:58 -0400 Subject: [PATCH 04/15] Another test fix --- .../java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java index 167e9bd12aa..e37c4a49c9e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java @@ -162,7 +162,6 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { assertEquals(myObservationIds, interceptedResourceIds); // Fetch the next 30 (do cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); resources = outcome.getResources(10, 40); returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(myObservationIdsEvenOnly.subList(10, 25), returnedIdValues); @@ -187,7 +186,6 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -213,7 +211,6 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -237,7 +234,6 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); @@ -263,7 +259,6 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIds, myObservationIds), sort(returnedIdValues)); @@ -288,7 +283,6 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { ourLog.info("Search UUID: {}", outcome.getUuid()); // Fetch the first 10 (don't cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); List resources = outcome.getResources(0, 100); List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(sort(myPatientIdsEvenOnly, myObservationIdsEvenOnly), sort(returnedIdValues)); From 85f039caae6983f46fefc36390d1adbe9bbb1823 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 09:19:44 -0400 Subject: [PATCH 05/15] Remove lenient method match feature, as I am unable to find any documentation explaining the behaviour or any way of triggering its use. --- .../server/method/SearchMethodBinding.java | 26 ++++--------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java index e449f84008d..8f852762b09 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java @@ -129,22 +129,8 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { @Override public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { - String clientPreference = theRequest.getHeader(Constants.HEADER_PREFER); - boolean lenientHandling = false; - if(clientPreference != null) - { - String[] preferences = clientPreference.split(";"); - for( String p : preferences){ - if("handling:lenient".equalsIgnoreCase(p)) - { - lenientHandling = true; - break; - } - } - } - if (theRequest.getId() != null && myIdParamIndex == null) { - ourLog.trace("Method {} doesn't match because ID is not null: {}", theRequest.getId()); + ourLog.trace("Method {} doesn't match because ID is not null: {}", getMethod(), theRequest.getId()); return false; } if (theRequest.getRequestType() == RequestTypeEnum.GET && theRequest.getOperation() != null && !Constants.PARAM_SEARCH.equals(theRequest.getOperation())) { @@ -156,16 +142,16 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { return false; } if (theRequest.getRequestType() != RequestTypeEnum.GET && theRequest.getRequestType() != RequestTypeEnum.POST) { - ourLog.trace("Method {} doesn't match because request type is {}", getMethod()); + ourLog.trace("Method {} doesn't match because request type is {}", getMethod(), theRequest.getRequestType()); return false; } if (!StringUtils.equals(myCompartmentName, theRequest.getCompartmentName())) { - ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", new Object[] { getMethod(), myCompartmentName, theRequest.getCompartmentName() }); + ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", getMethod(), myCompartmentName, theRequest.getCompartmentName()); return false; } // This is used to track all the parameters so we can reject queries that // have additional params we don't understand - Set methodParamsTemp = new HashSet(); + Set methodParamsTemp = new HashSet<>(); Set unqualifiedNames = theRequest.getUnqualifiedToQualifiedNames().keySet(); Set qualifiedParamNames = theRequest.getParameters().keySet(); @@ -237,8 +223,6 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } } Set keySet = theRequest.getParameters().keySet(); - if(lenientHandling == true) - return true; if (myAllowUnknownParams == false) { for (String next : keySet) { @@ -272,7 +256,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { if (theQualifierWhitelist == null && theQualifierBlacklist == null) { return theQualifiedNames; } - ArrayList retVal = new ArrayList(theQualifiedNames.size()); + ArrayList retVal = new ArrayList<>(theQualifiedNames.size()); for (String next : theQualifiedNames) { QualifierDetails qualifiers = extractQualifiersFromParameterName(next); if (!qualifiers.passes(theQualifierWhitelist, theQualifierBlacklist)) { From 57377f5557c7f25c938a06a445113a0032e89682 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 10:54:37 -0400 Subject: [PATCH 06/15] Fix #1483 - Don't let RP methods handle a request with includes if the method doesn't understand them --- .../server/method/SearchMethodBinding.java | 102 ++++------ .../method/SearchMethodBindingTest.java | 8 +- .../server/ServerMethodSelectionR4Test.java | 190 ++++++++++++++++++ src/changes/changes.xml | 5 + 4 files changed, 243 insertions(+), 62 deletions(-) create mode 100644 hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerMethodSelectionR4Test.java diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java index e449f84008d..066a7f751a7 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java @@ -19,15 +19,6 @@ package ca.uhn.fhir.rest.server.method; * limitations under the License. * #L% */ -import static org.apache.commons.lang3.StringUtils.isBlank; -import static org.apache.commons.lang3.StringUtils.isNotBlank; - -import java.lang.reflect.Method; -import java.util.*; - -import org.apache.commons.lang3.StringUtils; -import org.hl7.fhir.instance.model.api.IAnyResource; -import org.hl7.fhir.instance.model.api.IBaseResource; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; @@ -44,27 +35,38 @@ import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.param.QualifierDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import org.apache.commons.lang3.StringUtils; +import org.hl7.fhir.instance.model.api.IAnyResource; +import org.hl7.fhir.instance.model.api.IBaseResource; import javax.annotation.Nonnull; +import java.lang.reflect.Method; +import java.util.*; + +import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.apache.commons.lang3.StringUtils.isNotBlank; public class SearchMethodBinding extends BaseResourceReturningMethodBinding { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchMethodBinding.class); private static final Set SPECIAL_SEARCH_PARAMS; - private String myCompartmentName; - private String myDescription; - private Integer myIdParamIndex; - private String myQueryName; - private boolean myAllowUnknownParams; - private final String myResourceProviderResourceName; static { HashSet specialSearchParams = new HashSet<>(); specialSearchParams.add(IAnyResource.SP_RES_ID); specialSearchParams.add(IAnyResource.SP_RES_LANGUAGE); + specialSearchParams.add(Constants.PARAM_INCLUDE); + specialSearchParams.add(Constants.PARAM_REVINCLUDE); SPECIAL_SEARCH_PARAMS = Collections.unmodifiableSet(specialSearchParams); } + private final String myResourceProviderResourceName; + private String myCompartmentName; + private String myDescription; + private Integer myIdParamIndex; + private String myQueryName; + private boolean myAllowUnknownParams; + public SearchMethodBinding(Class theReturnResourceType, Class theResourceProviderResourceType, Method theMethod, FhirContext theContext, Object theProvider) { super(theReturnResourceType, theMethod, theContext, theProvider); Search search = theMethod.getAnnotation(Search.class); @@ -90,11 +92,11 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { throw new ConfigurationException(msg); } - if (theResourceProviderResourceType != null) { - this.myResourceProviderResourceName = theContext.getResourceDefinition(theResourceProviderResourceType).getName(); - } else { - this.myResourceProviderResourceName = null; - } + if (theResourceProviderResourceType != null) { + this.myResourceProviderResourceName = theContext.getResourceDefinition(theResourceProviderResourceType).getName(); + } else { + this.myResourceProviderResourceName = null; + } } @@ -106,8 +108,8 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { return myQueryName; } - public String getResourceProviderResourceName() { - return myResourceProviderResourceName; + public String getResourceProviderResourceName() { + return myResourceProviderResourceName; } @Nonnull @@ -123,28 +125,14 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { @Override public ReturnTypeEnum getReturnType() { - return ReturnTypeEnum.BUNDLE; + return ReturnTypeEnum.BUNDLE; } @Override public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { - - String clientPreference = theRequest.getHeader(Constants.HEADER_PREFER); - boolean lenientHandling = false; - if(clientPreference != null) - { - String[] preferences = clientPreference.split(";"); - for( String p : preferences){ - if("handling:lenient".equalsIgnoreCase(p)) - { - lenientHandling = true; - break; - } - } - } - + if (theRequest.getId() != null && myIdParamIndex == null) { - ourLog.trace("Method {} doesn't match because ID is not null: {}", theRequest.getId()); + ourLog.trace("Method {} doesn't match because ID is not null: {}", getMethod(), theRequest.getId()); return false; } if (theRequest.getRequestType() == RequestTypeEnum.GET && theRequest.getOperation() != null && !Constants.PARAM_SEARCH.equals(theRequest.getOperation())) { @@ -156,40 +144,39 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { return false; } if (theRequest.getRequestType() != RequestTypeEnum.GET && theRequest.getRequestType() != RequestTypeEnum.POST) { - ourLog.trace("Method {} doesn't match because request type is {}", getMethod()); + ourLog.trace("Method {} doesn't match because request type is {}", getMethod(), theRequest.getRequestType()); return false; } if (!StringUtils.equals(myCompartmentName, theRequest.getCompartmentName())) { - ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", new Object[] { getMethod(), myCompartmentName, theRequest.getCompartmentName() }); + ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", getMethod(), myCompartmentName, theRequest.getCompartmentName()); return false; } // This is used to track all the parameters so we can reject queries that // have additional params we don't understand - Set methodParamsTemp = new HashSet(); + Set methodParamsTemp = new HashSet<>(); Set unqualifiedNames = theRequest.getUnqualifiedToQualifiedNames().keySet(); Set qualifiedParamNames = theRequest.getParameters().keySet(); - for (int i = 0; i < this.getParameters().size(); i++) { - if (!(getParameters().get(i) instanceof BaseQueryParameter)) { + for (IParameter nextParameter : getParameters()) { + if (!(nextParameter instanceof BaseQueryParameter)) { continue; } - BaseQueryParameter temp = (BaseQueryParameter) getParameters().get(i); - String name = temp.getName(); - if (temp.isRequired()) { + BaseQueryParameter nextQueryParameter = (BaseQueryParameter) nextParameter; + String name = nextQueryParameter.getName(); + if (nextQueryParameter.isRequired()) { if (qualifiedParamNames.contains(name)) { QualifierDetails qualifiers = extractQualifiersFromParameterName(name); - if (qualifiers.passes(temp.getQualifierWhitelist(), temp.getQualifierBlacklist())) { + if (qualifiers.passes(nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist())) { methodParamsTemp.add(name); } } if (unqualifiedNames.contains(name)) { List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name); - qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, temp.getQualifierWhitelist(), temp.getQualifierBlacklist()); + qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist()); methodParamsTemp.addAll(qualifiedNames); } - if (!qualifiedParamNames.contains(name) && !unqualifiedNames.contains(name)) - { + if (!qualifiedParamNames.contains(name) && !unqualifiedNames.contains(name)) { ourLog.trace("Method {} doesn't match param '{}' is not present", getMethod().getName(), name); return false; } @@ -197,16 +184,16 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } else { if (qualifiedParamNames.contains(name)) { QualifierDetails qualifiers = extractQualifiersFromParameterName(name); - if (qualifiers.passes(temp.getQualifierWhitelist(), temp.getQualifierBlacklist())) { + if (qualifiers.passes(nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist())) { methodParamsTemp.add(name); } - } + } if (unqualifiedNames.contains(name)) { List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name); - qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, temp.getQualifierWhitelist(), temp.getQualifierBlacklist()); + qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist()); methodParamsTemp.addAll(qualifiedNames); } - if (!qualifiedParamNames.contains(name)) { + if (!qualifiedParamNames.contains(name)) { methodParamsTemp.add(name); } } @@ -237,8 +224,6 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } } Set keySet = theRequest.getParameters().keySet(); - if(lenientHandling == true) - return true; if (myAllowUnknownParams == false) { for (String next : keySet) { @@ -272,7 +257,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { if (theQualifierWhitelist == null && theQualifierBlacklist == null) { return theQualifiedNames; } - ArrayList retVal = new ArrayList(theQualifiedNames.size()); + ArrayList retVal = new ArrayList<>(theQualifiedNames.size()); for (String next : theQualifiedNames) { QualifierDetails qualifiers = extractQualifiersFromParameterName(next); if (!qualifiers.passes(theQualifierWhitelist, theQualifierBlacklist)) { @@ -287,6 +272,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { public String toString() { return getMethod().toString(); } + public static QualifierDetails extractQualifiersFromParameterName(String theParamName) { QualifierDetails retVal = new QualifierDetails(); if (theParamName == null || theParamName.length() == 0) { diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java index 4a84aa56c52..11bc0a42426 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java @@ -35,15 +35,15 @@ public class SearchMethodBindingTest { } @Test // fails - public void methodShouldNotMatchWhenExtraUnderscoreQueryParameter() throws NoSuchMethodException { + public void methodShouldNotMatchWhenUnderscoreQueryParameter() throws NoSuchMethodException { Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( - mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))), + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))), Matchers.is(false)); Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( - mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))), + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))), Matchers.is(false)); Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( - mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))), + mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))), Matchers.is(false)); } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerMethodSelectionR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerMethodSelectionR4Test.java new file mode 100644 index 00000000000..14fa05826ef --- /dev/null +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerMethodSelectionR4Test.java @@ -0,0 +1,190 @@ +package ca.uhn.fhir.rest.server; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.api.BundleInclusionRule; +import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.rest.annotation.IncludeParam; +import ca.uhn.fhir.rest.annotation.OptionalParam; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.api.EncodingEnum; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.test.utilities.JettyUtil; +import com.google.common.collect.Lists; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.StringType; +import org.junit.After; +import org.junit.Test; + +import java.util.List; +import java.util.Set; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.*; + +public class ServerMethodSelectionR4Test { + + + private FhirContext myCtx = FhirContext.forR4(); + private Server myServer; + private IGenericClient myClient; + + @After + public void after() throws Exception { + JettyUtil.closeServer(myServer); + } + + /** + * Server method with no _include + * Client request with _include + *

+ * See #1421 + */ + @Test + public void testRejectIncludeIfNotProvided() throws Exception { + + class MyProvider extends MyBaseProvider { + @Search + public List search(@OptionalParam(name = "name") StringType theName) { + return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123")); + } + } + MyProvider provider = new MyProvider(); + + startServer(provider); + + try { + myClient + .search() + .forResource(Patient.class) + .where(Patient.NAME.matches().value("foo")) + .include(Patient.INCLUDE_ORGANIZATION) + .execute(); + fail(); + } catch (InvalidRequestException e) { + assertThat(e.getMessage(), containsString("this server does not know how to handle GET operation[Patient] with parameters [[_include, name]]")); + } + } + + /** + * Server method with no _include + * Client request with _include + *

+ * See #1421 + */ + @Test + public void testAllowIncludeIfProvided() throws Exception { + + class MyProvider extends MyBaseProvider { + @Search + public List search(@OptionalParam(name = "name") StringType theName, @IncludeParam Set theIncludes) { + return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123")); + } + } + MyProvider provider = new MyProvider(); + + startServer(provider); + + Bundle results = myClient + .search() + .forResource(Patient.class) + .where(Patient.NAME.matches().value("foo")) + .include(Patient.INCLUDE_ORGANIZATION) + .returnBundle(Bundle.class) + .execute(); + assertEquals(1, results.getEntry().size()); + } + + /** + * Server method with no _revinclude + * Client request with _revinclude + *

+ * See #1421 + */ + @Test + public void testRejectRevIncludeIfNotProvided() throws Exception { + + class MyProvider extends MyBaseProvider { + @Search + public List search(@OptionalParam(name = "name") StringType theName) { + return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123")); + } + } + MyProvider provider = new MyProvider(); + + startServer(provider); + + try { + myClient + .search() + .forResource(Patient.class) + .where(Patient.NAME.matches().value("foo")) + .revInclude(Patient.INCLUDE_ORGANIZATION) + .execute(); + fail(); + } catch (InvalidRequestException e) { + assertThat(e.getMessage(), containsString("this server does not know how to handle GET operation[Patient] with parameters [[_revinclude, name]]")); + } + } + + /** + * Server method with no _revInclude + * Client request with _revInclude + *

+ * See #1421 + */ + @Test + public void testAllowRevIncludeIfProvided() throws Exception { + + class MyProvider extends MyBaseProvider { + @Search + public List search(@OptionalParam(name = "name") StringType theName, @IncludeParam(reverse = true) Set theRevIncludes) { + return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123")); + } + } + MyProvider provider = new MyProvider(); + + startServer(provider); + + Bundle results = myClient + .search() + .forResource(Patient.class) + .where(Patient.NAME.matches().value("foo")) + .revInclude(Patient.INCLUDE_ORGANIZATION) + .returnBundle(Bundle.class) + .execute(); + assertEquals(1, results.getEntry().size()); + } + + private void startServer(Object theProvider) throws Exception { + RestfulServer servlet = new RestfulServer(myCtx); + servlet.registerProvider(theProvider); + ServletHandler proxyHandler = new ServletHandler(); + servlet.setDefaultResponseEncoding(EncodingEnum.XML); + servlet.setBundleInclusionRule(BundleInclusionRule.BASED_ON_RESOURCE_PRESENCE); + ServletHolder servletHolder = new ServletHolder(servlet); + proxyHandler.addServletWithMapping(servletHolder, "/*"); + + myServer = new Server(0); + myServer.setHandler(proxyHandler); + JettyUtil.startServer(myServer); + int port = JettyUtil.getPortForStartedServer(myServer); + + myClient = myCtx.newRestfulGenericClient("http://localhost:" + port); + } + + + public static class MyBaseProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + } + +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 293b3260a9a..0e1bb4b885a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -212,6 +212,11 @@ with the new request id, resulting in an ever growing source.meta value. E.g. after the first update, it looks like "#9f0a901387128111#5f37835ee38a89e2" when it should only be "#5f37835ee38a89e2". This has been corrected. + + The Plain Server method selector was incorrectly allowing client requests with _include statements to be + handled by method implementations that did not have any @IncludeParam]]> defined. This + is now corrected. Thanks to Tuomo Ala-Vannesluoma for reporting and providing a test case! + From b2c13f30185b311f50ba553f127d50c54d88f194 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 25 Sep 2019 13:25:59 -0400 Subject: [PATCH 07/15] Work on codecov integration --- hapi-fhir-jacoco/pom.xml | 14 +++++++++++++- hapi-fhir-jpaserver-base/pom.xml | 19 +++++++++++-------- ...FhirResourceDaoR4SearchPageExpiryTest.java | 2 +- pom.xml | 2 +- src/changes/changes.xml | 1 + 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/hapi-fhir-jacoco/pom.xml b/hapi-fhir-jacoco/pom.xml index 393098dad09..24532059c55 100644 --- a/hapi-fhir-jacoco/pom.xml +++ b/hapi-fhir-jacoco/pom.xml @@ -318,6 +318,7 @@ ${basedir}/.. + hapi-fhir-jpaserver-model/target/jacoco.exec hapi-fhir-jpaserver-searchparam/target/jacoco.exec hapi-fhir-jpaserver-subscription/target/jacoco.exec hapi-fhir-jpaserver-base/target/jacoco.exec + @@ -351,6 +355,7 @@ + org.codehaus.mojo build-helper-maven-plugin @@ -394,9 +400,11 @@ + ../hapi-fhir-jpaserver-model/src/main/java ../hapi-fhir-jpaserver-searchparam/src/main/java ../hapi-fhir-jpaserver-subscription/src/main/java @@ -424,9 +432,11 @@ + ../hapi-fhir-jpaserver-model/src/test/resources @@ -439,6 +449,7 @@ ../hapi-fhir-jpaserver-base/src/test/resources + diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index dad80f5523a..7f717807d8d 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -660,14 +660,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}../hapi-fhir-base/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - true @@ -677,6 +669,17 @@ prepare-agent + + post-integration-test + install + + report + + + ${project.build.directory}/jacoco.exec + ${project.reporting.outputDirectory}/jacoco-report + + diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java index c14e457e9d6..9f0435102e4 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java @@ -324,7 +324,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 2100); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { diff --git a/pom.xml b/pom.xml index ebb542bc664..55b1dd4ae39 100755 --- a/pom.xml +++ b/pom.xml @@ -607,7 +607,7 @@ 4.4.11 4.5.9 2.9.9 - 2.9.9.1 + 2.9.10 3.1.0 1.8 4.0.0.Beta3 diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 0e1bb4b885a..bb0164e3909 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -13,6 +13,7 @@

  • Hibernate Core (Core): 5.4.2.Final -> 5.4.4.Final
    • +
  • Jackson Databind (JPA): 2.9.9 -> 2.9.10 (CVE-2019-16335, CVE-2019-14540)
    • ]]> From b027963c7a692e1223a7aa57a03ac31f1c982c7c Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 13:39:51 -0400 Subject: [PATCH 08/15] Add some test logging --- .../src/test/java/ca/uhn/fhir/ctx/FhirContextDstu2Test.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/ctx/FhirContextDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/ctx/FhirContextDstu2Test.java index 06312de9a10..97a17c3770c 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/ctx/FhirContextDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/ctx/FhirContextDstu2Test.java @@ -148,6 +148,7 @@ public class FhirContextDstu2Test { afterInitBlocker.await(); submittedTestRunnable.run(); } catch (final Throwable e) { + ourLog.error("Exception", e); exceptions.add(e); } finally { allDone.countDown(); From dfe07e7a2b633350123de8a61ef99f922918f3fe Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 13:45:02 -0400 Subject: [PATCH 09/15] Avoid concurrency issue introduced by #1489 --- .../java/ca/uhn/fhir/parser/BaseParser.java | 108 +++++++++--------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java index 92b6ca02688..5603d5faa77 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java @@ -27,7 +27,6 @@ import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.UrlUtil; - import com.google.common.base.Charsets; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; @@ -68,47 +67,6 @@ public abstract class BaseParser implements IParser { private boolean mySuppressNarratives; private Set myDontStripVersionsFromReferencesAtPaths; - private Map> compositeChildrenCache = new HashMap<>(); - - private static class Key { - private final BaseRuntimeElementCompositeDefinition resDef; - private final boolean theContainedResource; - private final CompositeChildElement theParent; - private final EncodeContext theEncodeContext; - - public Key(BaseRuntimeElementCompositeDefinition resDef, final boolean theContainedResource, final CompositeChildElement theParent, EncodeContext theEncodeContext) { - this.resDef = resDef; - this.theContainedResource = theContainedResource; - this.theParent = theParent; - this.theEncodeContext = theEncodeContext; - } - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((resDef == null) ? 0 : resDef.hashCode()); - result = prime * result + (theContainedResource ? 1231 : 1237); - result = prime * result + ((theParent == null) ? 0 : theParent.hashCode()); - result = prime * result + ((theEncodeContext == null) ? 0 : theEncodeContext.hashCode()); - return result; - } - - @Override - public boolean equals(final Object obj) { - if (this == obj) { - return true; - } - if (obj instanceof Key) { - final Key that = (Key) obj; - return Objects.equals(this.resDef, that.resDef) && - this.theContainedResource == that.theContainedResource && - Objects.equals(this.theParent, that.theParent) && - Objects.equals(this.theEncodeContext, that.theEncodeContext); - } - return false; - } - } - /** * Constructor */ @@ -171,12 +129,12 @@ public abstract class BaseParser implements IParser { protected Iterable compositeChildIterator(IBase theCompositeElement, final boolean theContainedResource, final CompositeChildElement theParent, EncodeContext theEncodeContext) { BaseRuntimeElementCompositeDefinition elementDef = (BaseRuntimeElementCompositeDefinition) myContext.getElementDefinition(theCompositeElement.getClass()); - return compositeChildrenCache.computeIfAbsent(new Key(elementDef, theContainedResource, theParent, theEncodeContext), (k) -> { - + return theEncodeContext.getCompositeChildrenCache().computeIfAbsent(new Key(elementDef, theContainedResource, theParent, theEncodeContext), (k) -> { + final List children = elementDef.getChildrenAndExtension(); final List result = new ArrayList<>(children.size()); - for(final BaseRuntimeChildDefinition child: children) { + for (final BaseRuntimeChildDefinition child : children) { CompositeChildElement myNext = new CompositeChildElement(theParent, child, theEncodeContext); /* @@ -274,7 +232,7 @@ public abstract class BaseParser implements IParser { } - protected List getAllBaseReferences(IBaseResource theResource){ + protected List getAllBaseReferences(IBaseResource theResource) { final ArrayList retVal = new ArrayList(); findBaseReferences(retVal, theResource, myContext.getResourceDefinition(theResource)); return retVal; @@ -288,7 +246,7 @@ public abstract class BaseParser implements IParser { if (theElement instanceof IBaseReference) { allElements.add((IBaseReference) theElement); } - + BaseRuntimeElementDefinition def = theDefinition; if (def.getChildType() == ChildTypeEnum.CONTAINED_RESOURCE_LIST) { def = myContext.getElementDefinition(theElement.getClass()); @@ -353,7 +311,7 @@ public abstract class BaseParser implements IParser { } } } - + private String determineReferenceText(IBaseReference theRef, CompositeChildElement theCompositeChildElement) { IIdType ref = theRef.getReferenceElement(); if (isBlank(ref.getIdPart())) { @@ -1287,10 +1245,10 @@ public abstract class BaseParser implements IParser { if (obj instanceof CompositeChildElement) { final CompositeChildElement that = (CompositeChildElement) obj; return Objects.equals(this.getEnclosingInstance(), that.getEnclosingInstance()) && - Objects.equals(this.myDef, that.myDef) && - Objects.equals(this.myParent, that.myParent) && - Objects.equals(this.myResDef, that.myResDef) && - Objects.equals(this.myEncodeContext, that.myEncodeContext); + Objects.equals(this.myDef, that.myDef) && + Objects.equals(this.myParent, that.myParent) && + Objects.equals(this.myResDef, that.myResDef) && + Objects.equals(this.myEncodeContext, that.myEncodeContext); } return false; } @@ -1369,13 +1327,17 @@ public abstract class BaseParser implements IParser { } } - /** * EncodeContext is a shared state object that is passed around the * encode process */ protected class EncodeContext extends EncodeContextPath { private final ArrayList myResourcePath = new ArrayList<>(10); + private final Map> myCompositeChildrenCache = new HashMap<>(); + + public Map> getCompositeChildrenCache() { + return myCompositeChildrenCache; + } protected ArrayList getResourcePath() { return myResourcePath; @@ -1508,6 +1470,46 @@ public abstract class BaseParser implements IParser { } } + private static class Key { + private final BaseRuntimeElementCompositeDefinition resDef; + private final boolean theContainedResource; + private final CompositeChildElement theParent; + private final EncodeContext theEncodeContext; + + public Key(BaseRuntimeElementCompositeDefinition resDef, final boolean theContainedResource, final CompositeChildElement theParent, EncodeContext theEncodeContext) { + this.resDef = resDef; + this.theContainedResource = theContainedResource; + this.theParent = theParent; + this.theEncodeContext = theEncodeContext; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((resDef == null) ? 0 : resDef.hashCode()); + result = prime * result + (theContainedResource ? 1231 : 1237); + result = prime * result + ((theParent == null) ? 0 : theParent.hashCode()); + result = prime * result + ((theEncodeContext == null) ? 0 : theEncodeContext.hashCode()); + return result; + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) { + return true; + } + if (obj instanceof Key) { + final Key that = (Key) obj; + return Objects.equals(this.resDef, that.resDef) && + this.theContainedResource == that.theContainedResource && + Objects.equals(this.theParent, that.theParent) && + Objects.equals(this.theEncodeContext, that.theEncodeContext); + } + return false; + } + } + static class ContainedResources { private long myNextContainedId = 1; From 473fa3456efe08a194b4d8ea982ee4628ab1a2ac Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 25 Sep 2019 14:12:42 -0400 Subject: [PATCH 10/15] Work on adding codecov --- hapi-fhir-base/pom.xml | 11 +++++++++++ hapi-fhir-jpaserver-model/pom.xml | 11 +++++++++++ hapi-fhir-jpaserver-searchparam/pom.xml | 11 +++++++++++ hapi-fhir-jpaserver-subscription/pom.xml | 11 +++++++++++ 4 files changed, 44 insertions(+) diff --git a/hapi-fhir-base/pom.xml b/hapi-fhir-base/pom.xml index d3bfafcee39..37b49886228 100644 --- a/hapi-fhir-base/pom.xml +++ b/hapi-fhir-base/pom.xml @@ -144,6 +144,17 @@ prepare-agent + + post-integration-test + install + + report + + + ${project.build.directory}/jacoco.exec + ${project.reporting.outputDirectory}/jacoco-report + + diff --git a/hapi-fhir-jpaserver-model/pom.xml b/hapi-fhir-jpaserver-model/pom.xml index d05f0d7e70f..1d16261c260 100644 --- a/hapi-fhir-jpaserver-model/pom.xml +++ b/hapi-fhir-jpaserver-model/pom.xml @@ -159,6 +159,17 @@ prepare-agent + + post-integration-test + install + + report + + + ${project.build.directory}/jacoco.exec + ${project.reporting.outputDirectory}/jacoco-report + + diff --git a/hapi-fhir-jpaserver-searchparam/pom.xml b/hapi-fhir-jpaserver-searchparam/pom.xml index bbe90fcb671..9645baae335 100755 --- a/hapi-fhir-jpaserver-searchparam/pom.xml +++ b/hapi-fhir-jpaserver-searchparam/pom.xml @@ -148,6 +148,17 @@ prepare-agent + + post-integration-test + install + + report + + + ${project.build.directory}/jacoco.exec + ${project.reporting.outputDirectory}/jacoco-report + + diff --git a/hapi-fhir-jpaserver-subscription/pom.xml b/hapi-fhir-jpaserver-subscription/pom.xml index 356a896078c..952f5254128 100644 --- a/hapi-fhir-jpaserver-subscription/pom.xml +++ b/hapi-fhir-jpaserver-subscription/pom.xml @@ -142,6 +142,17 @@ prepare-agent + + post-integration-test + install + + report + + + ${project.build.directory}/jacoco.exec + ${project.reporting.outputDirectory}/jacoco-report + + From cddaf058a25c35c1511718f3b738565273606171 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 15:27:17 -0400 Subject: [PATCH 11/15] Add some extra test logging --- .../jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java index ccc934154e0..4579bda8fba 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.entity.SearchResult; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; +import org.apache.commons.lang3.Validate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -57,6 +58,8 @@ public class DatabaseSearchResultCacheSvcImpl implements ISearchResultCacheSvc { ourLog.trace("fetchResultPids for range {}-{} returned {} pids", theFrom, theTo, retVal.size()); + Validate.isTrue(theSearch.getNumFound() < theTo || retVal.size() == (theTo - theFrom), "Failed to find results in cache, requested %d - %d and git %d with numfound= %d", theFrom, theTo, retVal.size(), theSearch.getNumFound()); + return new ArrayList<>(retVal); } From 4570e50c03a089606c32b3b62d29474631dc2935 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 15:31:26 -0400 Subject: [PATCH 12/15] Fix nullability on one column --- .../main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java index 85874bac10c..f0effc0e5c6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java @@ -53,7 +53,7 @@ public class TermCodeSystemVersion implements Serializable { @JoinColumn(name = "RES_ID", referencedColumnName = "RES_ID", nullable = false, updatable = false, foreignKey = @ForeignKey(name = "FK_CODESYSVER_RES_ID")) private ResourceTable myResource; - @Column(name = "RES_ID", insertable = false, updatable = false) + @Column(name = "RES_ID", nullable = false, insertable = false, updatable = false) private Long myResourcePid; @Column(name = "CS_VERSION_ID", nullable = true, updatable = false, length = MAX_VERSION_LENGTH) From 8657afb01e817ebcae1609721dc2085a356c175d Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 20:15:21 -0400 Subject: [PATCH 13/15] Fix intermittent test failures regarding search (#1509) * Add some logging * Add more test logging * Work on some test logging * Fix compile error * Work on codecov * Work on codecov * Remove test debug messages --- azure-pipelines.yml | 5 +- hapi-fhir-base/pom.xml | 31 -- hapi-fhir-client-okhttp/pom.xml | 8 - hapi-fhir-client/pom.xml | 8 - hapi-fhir-jacoco/pom.xml | 373 +----------------- hapi-fhir-jpaserver-base/pom.xml | 11 - .../java/ca/uhn/fhir/jpa/entity/Search.java | 18 + .../jpa/search/SearchCoordinatorSvcImpl.java | 14 +- .../DatabaseSearchResultCacheSvcImpl.java | 5 +- .../FhirResourceDaoR4SearchOptimizedTest.java | 4 +- .../tasks/HapiFhirJpaMigrationTasks.java | 1 + hapi-fhir-jpaserver-model/pom.xml | 11 - hapi-fhir-jpaserver-searchparam/pom.xml | 11 - hapi-fhir-jpaserver-subscription/pom.xml | 11 - hapi-fhir-narrativegenerator/pom.xml | 8 - hapi-fhir-server/pom.xml | 8 - hapi-fhir-structures-dstu/pom.xml | 8 - hapi-fhir-structures-dstu2.1/pom.xml | 12 - hapi-fhir-structures-dstu2/pom.xml | 12 - hapi-fhir-structures-hl7org-dstu2/pom.xml | 12 - hapi-fhir-structures-r4/pom.xml | 12 - hapi-fhir-structures-r5/pom.xml | 12 - hapi-fhir-validation/pom.xml | 12 - 23 files changed, 44 insertions(+), 563 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 6685d4e41c5..011cfb6cc25 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -28,7 +28,7 @@ jobs: inputs: #mavenPomFile: 'pom.xml' goals: 'clean install' # Optional - options: '' + options: '-P ALLMODULES,JACOCO' #publishJUnitResults: true #testResultsFiles: '**/surefire-reports/TEST-*.xml' # Required when publishJUnitResults == True #testRunTitle: # Optional @@ -53,4 +53,7 @@ jobs: #pmdRunAnalysis: false # Optional #findBugsRunAnalysis: false # Optional + - script: bash <(curl https://codecov.io/bash) -t $(CODECOV_TOKEN) + displayName: 'codecov' + diff --git a/hapi-fhir-base/pom.xml b/hapi-fhir-base/pom.xml index 37b49886228..b7209b8d612 100644 --- a/hapi-fhir-base/pom.xml +++ b/hapi-fhir-base/pom.xml @@ -129,12 +129,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - - - ${basedir}/src/main/java - true @@ -144,17 +138,6 @@ prepare-agent - - post-integration-test - install - - report - - - ${project.build.directory}/jacoco.exec - ${project.reporting.outputDirectory}/jacoco-report - - @@ -164,20 +147,6 @@ @{argLine} -Dfile.encoding=UTF-8 -Xmx712m - org.apache.felix maven-bundle-plugin diff --git a/hapi-fhir-client-okhttp/pom.xml b/hapi-fhir-client-okhttp/pom.xml index e0cf2b8be7c..23694c35b0f 100644 --- a/hapi-fhir-client-okhttp/pom.xml +++ b/hapi-fhir-client-okhttp/pom.xml @@ -104,14 +104,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - true diff --git a/hapi-fhir-client/pom.xml b/hapi-fhir-client/pom.xml index dc8d67fb33c..1481f61eb43 100644 --- a/hapi-fhir-client/pom.xml +++ b/hapi-fhir-client/pom.xml @@ -43,14 +43,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - true diff --git a/hapi-fhir-jacoco/pom.xml b/hapi-fhir-jacoco/pom.xml index 24532059c55..a9d1b003e9a 100644 --- a/hapi-fhir-jacoco/pom.xml +++ b/hapi-fhir-jacoco/pom.xml @@ -51,6 +51,11 @@ hapi-fhir-structures-r4 ${project.version} + + ca.uhn.hapi.fhir + hapi-fhir-structures-r5 + ${project.version} + ca.uhn.hapi.fhir hapi-fhir-structures-hl7org-dstu2 @@ -91,393 +96,29 @@ hapi-fhir-jpaserver-model ${project.version} - - - javax.mail - javax.mail-api - - - com.sun.mail - javax.mail - - - javax.activation - activation - - - - - - com.helger - ph-schematron - - - Saxon-HE - net.sf.saxon - - - - - com.helger - ph-commons - - - org.thymeleaf - thymeleaf - - - - - org.slf4j - slf4j-api - - - ch.qos.logback - logback-classic - - - - - org.apache.derby - derby - test - - - org.apache.commons - commons-dbcp2 - test - - - - - javax.servlet - javax.servlet-api - provided - - - - org.eclipse.jetty - jetty-servlets - - - org.eclipse.jetty - jetty-servlet - - - org.eclipse.jetty - jetty-server - - - org.eclipse.jetty - jetty-util - - - - net.sf.json-lib - json-lib - jdk15 - test - - - commons-logging - commons-logging - - - commons-lang - commons-lang - - - ezmorph - net.sf.ezmorph - - - - - net.sf.json-lib - json-lib - jdk15-sources - test - - - commons-logging - commons-logging - - - commons-lang - commons-lang - - - ezmorph - net.sf.ezmorph - - - - - directory-naming - naming-java - test - - - commons-logging - commons-logging - - - - - com.google.guava - guava - - - org.xmlunit - xmlunit-core - test - - - org.springframework - spring-test - test - - - org.eclipse.jetty.websocket - websocket-api - test - - - org.eclipse.jetty.websocket - websocket-client - test - - - org.eclipse.jetty.websocket - websocket-server - test - - - org.springframework - spring-web - - - - - javax.interceptor - javax.interceptor-api - provided - - - - - - - org.apache.maven.plugins - maven-site-plugin - - true - - - - - - org.basepom.maven - duplicate-finder-maven-plugin - - true - - org.jacoco jacoco-maven-plugin - - jacoco-merge - - merge - - install - - - - ${basedir}/.. - - - hapi-fhir-jpaserver-model/target/jacoco.exec - hapi-fhir-jpaserver-searchparam/target/jacoco.exec - hapi-fhir-jpaserver-subscription/target/jacoco.exec - hapi-fhir-jpaserver-base/target/jacoco.exec - - - - - - post-integration-test - install + verify - report + report-aggregate - ${project.build.directory}/jacoco.exec ${project.reporting.outputDirectory}/jacoco-report - - - org.codehaus.mojo - build-helper-maven-plugin - - - add-source - generate-sources - - add-source - - - - - ../hapi-fhir-jpaserver-model/src/main/java - ../hapi-fhir-jpaserver-searchparam/src/main/java - ../hapi-fhir-jpaserver-subscription/src/main/java - ../hapi-fhir-jpaserver-base/src/main/java - - - - - - - org.apache.maven.plugins - maven-install-plugin - - true - - - - org.apache.maven.plugins - maven-deploy-plugin - - true - - - - - - - - ../hapi-fhir-jpaserver-model/src/test/resources - - - ../hapi-fhir-jpaserver-searchparam/src/test/resources - - - ../hapi-fhir-jpaserver-subscription/src/test/resources - - - ../hapi-fhir-jpaserver-base/src/test/resources - - - - - - - org.apache.maven.plugins - maven-project-info-reports-plugin - - true - - - - - diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 7f717807d8d..69f60311de9 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -669,17 +669,6 @@ prepare-agent - - post-integration-test - install - - report - - - ${project.build.directory}/jacoco.exec - ${project.reporting.outputDirectory}/jacoco-report - - diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java index 4649d8b8a7e..448e1a542bd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java @@ -7,6 +7,8 @@ import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.server.util.ICachedSearchDetails; import org.apache.commons.lang3.SerializationUtils; import org.hibernate.annotations.OptimisticLock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.persistence.*; import javax.validation.constraints.NotNull; @@ -49,6 +51,8 @@ public class Search implements ICachedSearchDetails, Serializable { private static final int MAX_SEARCH_QUERY_STRING = 10000; private static final int FAILURE_MESSAGE_LENGTH = 500; private static final long serialVersionUID = 1L; + private static final Logger ourLog = LoggerFactory.getLogger(Search.class); + @Temporal(TemporalType.TIMESTAMP) @Column(name = "CREATED", nullable = false, updatable = false) private Date myCreated; @@ -77,6 +81,8 @@ public class Search implements ICachedSearchDetails, Serializable { private Date myLastUpdatedLow; @Column(name = "NUM_FOUND", nullable = false) private int myNumFound; + @Column(name = "NUM_BLOCKED", nullable = true) + private Integer myNumBlocked; @Column(name = "PREFERRED_PAGE_SIZE", nullable = true) private Integer myPreferredPageSize; @Column(name = "RESOURCE_ID", nullable = true) @@ -118,6 +124,14 @@ public class Search implements ICachedSearchDetails, Serializable { super(); } + public int getNumBlocked() { + return myNumBlocked != null ? myNumBlocked : 0; + } + + public void setNumBlocked(int theNumBlocked) { + myNumBlocked = theNumBlocked; + } + public Date getExpiryOrNull() { return myExpiryOrNull; } @@ -196,10 +210,12 @@ public class Search implements ICachedSearchDetails, Serializable { } public int getNumFound() { + ourLog.trace("getNumFound {}", myNumFound); return myNumFound; } public void setNumFound(int theNumFound) { + ourLog.trace("setNumFound {}", theNumFound); myNumFound = theNumFound; } @@ -260,10 +276,12 @@ public class Search implements ICachedSearchDetails, Serializable { } public SearchStatusEnum getStatus() { + ourLog.trace("getStatus {}", myStatus); return myStatus; } public void setStatus(SearchStatusEnum theStatus) { + ourLog.trace("setStatus {}", theStatus); myStatus = theStatus; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index 763e1f18251..f5b56b83943 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -232,7 +232,11 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } - return mySearchResultCacheSvc.fetchResultPids(search, theFrom, theTo); + List pids = mySearchResultCacheSvc.fetchResultPids(search, theFrom, theTo); + + + + return pids; } @@ -652,6 +656,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } ArrayList unsyncedPids = myUnsyncedPids; + int countBlocked = 0; // Interceptor call: STORAGE_PREACCESS_RESOURCES // This can be used to remove results from the search result details before @@ -669,6 +674,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { unsyncedPids.remove(i); myCountBlockedThisPass++; myCountSavedTotal++; + countBlocked++; } } } @@ -685,7 +691,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { unsyncedPids.clear(); if (theResultIter.hasNext() == false) { - mySearch.setNumFound(myCountSavedTotal); int skippedCount = theResultIter.getSkippedCount(); int totalFetched = skippedCount + myCountSavedThisPass + myCountBlockedThisPass; ourLog.trace("MaxToFetch[{}] SkippedCount[{}] CountSavedThisPass[{}] CountSavedThisTotal[{}] AdditionalPrefetchRemaining[{}]", myMaxResultsToFetch, skippedCount, myCountSavedThisPass, myCountSavedTotal, myAdditionalPrefetchThresholdsRemaining); @@ -707,6 +712,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } mySearch.setNumFound(myCountSavedTotal); + mySearch.setNumBlocked(mySearch.getNumBlocked() + countBlocked); int numSynced; synchronized (mySyncedPids) { @@ -1033,10 +1039,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { return super.call(); } - @Override - public List getResourcePids(int theFromIndex, int theToIndex) { - return super.getResourcePids(theFromIndex, theToIndex); - } } /** diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java index 4579bda8fba..d3b371cb30a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java @@ -56,9 +56,10 @@ public class DatabaseSearchResultCacheSvcImpl implements ISearchResultCacheSvc { .findWithSearchPid(theSearch.getId(), page) .getContent(); - ourLog.trace("fetchResultPids for range {}-{} returned {} pids", theFrom, theTo, retVal.size()); + ourLog.debug("fetchResultPids for range {}-{} returned {} pids", theFrom, theTo, retVal.size()); - Validate.isTrue(theSearch.getNumFound() < theTo || retVal.size() == (theTo - theFrom), "Failed to find results in cache, requested %d - %d and git %d with numfound= %d", theFrom, theTo, retVal.size(), theSearch.getNumFound()); + // FIXME: should we remove the blocked number from this message? + Validate.isTrue((theSearch.getNumFound() - theSearch.getNumBlocked()) < theTo || retVal.size() == (theTo - theFrom), "Failed to find results in cache, requested %d - %d and got %d with total found=%d and blocked %s", theFrom, theTo, retVal.size(), theSearch.getNumFound(), theSearch.getNumBlocked()); return new ArrayList<>(retVal); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java index 79d3929a8e1..5a04bac0169 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java @@ -6,6 +6,7 @@ import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; +import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.SearchTotalModeEnum; @@ -142,8 +143,9 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { assertEquals(201, results.size().intValue()); ids = toUnqualifiedVersionlessIdValues(results, 0, 10, true); assertThat(ids, hasSize(10)); - IBundleProvider bundleProvider = myDatabaseBackedPagingProvider.retrieveResultList(null, uuid); + PersistedJpaBundleProvider bundleProvider = (PersistedJpaBundleProvider) myDatabaseBackedPagingProvider.retrieveResultList(null, uuid); Integer bundleSize = bundleProvider.size(); + assertNotNull("Null size from provider of type " + bundleProvider.getClass() + " - Cache hit: " + bundleProvider.isCacheHit(), bundleSize); assertEquals(201, bundleSize.intValue()); // Search with count only diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index bafaebf0b1e..3817e196b70 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -63,6 +63,7 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { // HFJ_SEARCH version.onTable("HFJ_SEARCH").addColumn("EXPIRY_OR_NULL").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.DATE_TIMESTAMP); + version.onTable("HFJ_SEARCH").addColumn("NUM_BLOCKED").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.INT); // HFJ_BLK_EXPORT_JOB version.addIdGenerator("SEQ_BLKEXJOB_PID"); diff --git a/hapi-fhir-jpaserver-model/pom.xml b/hapi-fhir-jpaserver-model/pom.xml index 1d16261c260..d05f0d7e70f 100644 --- a/hapi-fhir-jpaserver-model/pom.xml +++ b/hapi-fhir-jpaserver-model/pom.xml @@ -159,17 +159,6 @@ prepare-agent - - post-integration-test - install - - report - - - ${project.build.directory}/jacoco.exec - ${project.reporting.outputDirectory}/jacoco-report - - diff --git a/hapi-fhir-jpaserver-searchparam/pom.xml b/hapi-fhir-jpaserver-searchparam/pom.xml index 9645baae335..bbe90fcb671 100755 --- a/hapi-fhir-jpaserver-searchparam/pom.xml +++ b/hapi-fhir-jpaserver-searchparam/pom.xml @@ -148,17 +148,6 @@ prepare-agent - - post-integration-test - install - - report - - - ${project.build.directory}/jacoco.exec - ${project.reporting.outputDirectory}/jacoco-report - - diff --git a/hapi-fhir-jpaserver-subscription/pom.xml b/hapi-fhir-jpaserver-subscription/pom.xml index 952f5254128..356a896078c 100644 --- a/hapi-fhir-jpaserver-subscription/pom.xml +++ b/hapi-fhir-jpaserver-subscription/pom.xml @@ -142,17 +142,6 @@ prepare-agent - - post-integration-test - install - - report - - - ${project.build.directory}/jacoco.exec - ${project.reporting.outputDirectory}/jacoco-report - - diff --git a/hapi-fhir-narrativegenerator/pom.xml b/hapi-fhir-narrativegenerator/pom.xml index e113c8a434a..0b69de1e127 100644 --- a/hapi-fhir-narrativegenerator/pom.xml +++ b/hapi-fhir-narrativegenerator/pom.xml @@ -56,14 +56,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - true diff --git a/hapi-fhir-server/pom.xml b/hapi-fhir-server/pom.xml index 9ba8f3f4a18..377657b5ef9 100644 --- a/hapi-fhir-server/pom.xml +++ b/hapi-fhir-server/pom.xml @@ -77,14 +77,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - true diff --git a/hapi-fhir-structures-dstu/pom.xml b/hapi-fhir-structures-dstu/pom.xml index ff618c062bc..c280284e4cf 100644 --- a/hapi-fhir-structures-dstu/pom.xml +++ b/hapi-fhir-structures-dstu/pom.xml @@ -171,14 +171,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - true diff --git a/hapi-fhir-structures-dstu2.1/pom.xml b/hapi-fhir-structures-dstu2.1/pom.xml index 1cc0bd65606..59d93e1f5f3 100644 --- a/hapi-fhir-structures-dstu2.1/pom.xml +++ b/hapi-fhir-structures-dstu2.1/pom.xml @@ -255,18 +255,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - ${basedir}/../hapi-fhir-client/target/classes - ${basedir}/../hapi-fhir-server/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - ${basedir}/../hapi-fhir-client/src/main/java - ${basedir}/../hapi-fhir-server/src/main/java - true diff --git a/hapi-fhir-structures-dstu2/pom.xml b/hapi-fhir-structures-dstu2/pom.xml index b16cd0536cd..fbc0f3ab718 100644 --- a/hapi-fhir-structures-dstu2/pom.xml +++ b/hapi-fhir-structures-dstu2/pom.xml @@ -229,18 +229,6 @@ org.jacoco jacoco-maven-plugin - - - - - - - - - - - - true diff --git a/hapi-fhir-structures-hl7org-dstu2/pom.xml b/hapi-fhir-structures-hl7org-dstu2/pom.xml index 3cc9a2a12f8..a904b0f4567 100644 --- a/hapi-fhir-structures-hl7org-dstu2/pom.xml +++ b/hapi-fhir-structures-hl7org-dstu2/pom.xml @@ -254,18 +254,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}../hapi-fhir-base/target/classes - ${basedir}/../hapi-fhir-client/target/classes - ${basedir}/../hapi-fhir-server/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - ${basedir}/../hapi-fhir-client/src/main/java - ${basedir}/../hapi-fhir-server/src/main/java - true diff --git a/hapi-fhir-structures-r4/pom.xml b/hapi-fhir-structures-r4/pom.xml index b90d8506a45..f30d080a929 100644 --- a/hapi-fhir-structures-r4/pom.xml +++ b/hapi-fhir-structures-r4/pom.xml @@ -296,18 +296,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - ${basedir}/../hapi-fhir-client/target/classes - ${basedir}/../hapi-fhir-server/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - ${basedir}/../hapi-fhir-client/src/main/java - ${basedir}/../hapi-fhir-server/src/main/java - true diff --git a/hapi-fhir-structures-r5/pom.xml b/hapi-fhir-structures-r5/pom.xml index d4d159e0a1b..43becf5f7ee 100644 --- a/hapi-fhir-structures-r5/pom.xml +++ b/hapi-fhir-structures-r5/pom.xml @@ -289,18 +289,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - ${basedir}/../hapi-fhir-client/target/classes - ${basedir}/../hapi-fhir-server/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - ${basedir}/../hapi-fhir-client/src/main/java - ${basedir}/../hapi-fhir-server/src/main/java - true diff --git a/hapi-fhir-validation/pom.xml b/hapi-fhir-validation/pom.xml index d683e053690..d02d942675b 100644 --- a/hapi-fhir-validation/pom.xml +++ b/hapi-fhir-validation/pom.xml @@ -254,18 +254,6 @@ org.jacoco jacoco-maven-plugin - - ${basedir}/target/classes - ${basedir}/../hapi-fhir-base/target/classes - ${basedir}/../hapi-fhir-client/target/classes - ${basedir}/../hapi-fhir-server/target/classes - - - ${basedir}/src/main/java - ${basedir}/../hapi-fhir-base/src/main/java - ${basedir}/../hapi-fhir-client/src/main/java - ${basedir}/../hapi-fhir-server/src/main/java - true From 99787d14f1b34f95daa8b3113cfcd572777032b2 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 21:20:36 -0400 Subject: [PATCH 14/15] Add some test logging --- README.md | 4 ++-- .../uhn/fhir/jpa/dao/data/ISearchResultDao.java | 4 ++++ .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 4 ++++ .../fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java | 16 +++++++++++++--- .../dao/r4/FhirResourceDaoR4SearchNoFtTest.java | 13 ++++++++++--- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 4d31ea76496..84afd8716b6 100644 --- a/README.md +++ b/README.md @@ -3,10 +3,10 @@ HAPI FHIR HAPI FHIR - Java API for HL7 FHIR Clients and Servers -[![Coverage Status](https://coveralls.io/repos/jamesagnew/hapi-fhir/badge.svg?branch=master&service=github)](https://coveralls.io/github/jamesagnew/hapi-fhir?branch=master) +[![Build Status](https://dev.azure.com/jamesagnew214/jamesagnew214/_apis/build/status/jamesagnew.hapi-fhir?branchName=master)](https://dev.azure.com/jamesagnew214/jamesagnew214/_build/latest?definitionId=1&branchName=master) +[![codecov](https://codecov.io/gh/jamesagnew/hapi-fhir/branch/master/graph/badge.svg)](https://codecov.io/gh/jamesagnew/hapi-fhir) [![Maven Central](https://maven-badges.herokuapp.com/maven-central/ca.uhn.hapi.fhir/hapi-fhir-base/badge.svg)](http://search.maven.org/#search|ga|1|ca.uhn.hapi.fhir) [![License](https://img.shields.io/badge/license-apache%202.0-60C060.svg)](http://jamesagnew.github.io/hapi-fhir/license.html) -[![Build Status](https://dev.azure.com/jamesagnew214/jamesagnew214/_apis/build/status/jamesagnew.hapi-fhir?branchName=master)](https://dev.azure.com/jamesagnew214/jamesagnew214/_build/latest?definitionId=1&branchName=master) Complete project documentation is available here: http://hapifhir.io diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java index 107537ed3c9..94c671a0908 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao.data; +import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.entity.SearchResult; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; @@ -44,4 +45,7 @@ public interface ISearchResultDao extends JpaRepository { @Modifying @Query("DELETE FROM SearchResult s WHERE s.myId IN :ids") void deleteByIds(@Param("ids") List theContent); + + @Query("SELECT count(r) FROM SearchResult r WHERE r.mySearchPid = :search") + int countForSearch(@Param("search") Long theSearchPid); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index e78572f417b..11be87537f9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -77,6 +77,10 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { private static JpaValidationSupportChainR4 ourJpaValidationSupportChainR4; private static IFhirResourceDaoValueSet ourValueSetDao; + @Autowired + protected ISearchDao mySearchEntityDao; + @Autowired + protected ISearchResultDao mySearchResultDao; @Autowired @Qualifier("myResourceCountsCache") protected ResourceCountCache myResourceCountsCache; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java index e37c4a49c9e..550d959b96d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ConsentEventsDaoR4Test.java @@ -6,6 +6,7 @@ import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.interceptor.executor.InterceptorService; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; @@ -127,13 +128,22 @@ public class ConsentEventsDaoR4Test extends BaseJpaR4SystemTest { List returnedIdValues = toUnqualifiedVersionlessIdValues(resources); assertEquals(myObservationIdsEvenOnly.subList(0, 10), returnedIdValues); assertEquals(1, hitCount.get()); - assertEquals(myObservationIds.subList(0, 20), interceptedResourceIds); + assertEquals("Wrong response from " + outcome.getClass(), myObservationIds.subList(0, 20), interceptedResourceIds); // Fetch the next 30 (do cross a fetch boundary) - outcome = myPagingProvider.retrieveResultList(mySrd, outcome.getUuid()); + String searchId = outcome.getUuid(); + outcome = myPagingProvider.retrieveResultList(mySrd, searchId); resources = outcome.getResources(10, 40); returnedIdValues = toUnqualifiedVersionlessIdValues(resources); - assertEquals(myObservationIdsEvenOnly.subList(10, 25), returnedIdValues); + if (!myObservationIdsEvenOnly.subList(10,25).equals(returnedIdValues)) { + if (resources.size() != 1) { + runInTransaction(() -> { + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(searchId).get(); + fail("Failed to load - " + mySearchResultDao.countForSearch(search.getId()) + " results in " + search); + }); + } + } + assertEquals("Wrong response from " + outcome.getClass(), myObservationIdsEvenOnly.subList(10, 25), returnedIdValues); assertEquals(2, hitCount.get()); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 46c25066285..11f460d3154 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -3,6 +3,8 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.data.ISearchDao; +import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; +import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; @@ -56,8 +58,6 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { @Autowired MatchUrlService myMatchUrlService; - @Autowired - private ISearchDao mySearchEntityDao; @After public void afterResetSearchSize() { @@ -1325,7 +1325,6 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } - @Test public void testSearchDateWrongParam() { Patient p1 = new Patient(); @@ -1371,6 +1370,14 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { String searchId = found.getUuid(); for (int i = 0; i < 9; i++) { List resources = found.getResources(i, i + 1); + if (resources.size() != 1) { + int finalI = i; + int finalI1 = i; + runInTransaction(() -> { + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(searchId).get(); + fail("Failed to load range " + finalI + " - " + (finalI1 + 1) + " - " + mySearchResultDao.countForSearch(search.getId()) + " results in " + search); + }); + } assertThat("Failed to load range " + i + " - " + (i + 1) + " - from provider of type: " + found.getClass(), resources, hasSize(1)); Patient nextResource = (Patient) resources.get(0); dates.add(nextResource.getBirthDateElement().getValueAsString()); From ddb5605830c1c4fb6faea6cbd91d808cb4e8cd33 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 25 Sep 2019 22:17:47 -0400 Subject: [PATCH 15/15] Improve test logging --- azure-pipelines.yml | 2 +- .../main/java/ca/uhn/fhir/jpa/entity/Search.java | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 011cfb6cc25..386f1f3ae08 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -44,7 +44,7 @@ jobs: #mavenVersionOption: 'Default' # Options: default, path #mavenDirectory: # Required when mavenVersionOption == Path #mavenSetM2Home: false # Required when mavenVersionOption == Path - mavenOptions: '-Xmx2048m $(MAVEN_OPTS)' # Optional + mavenOptions: '-Xmx2048m $(MAVEN_OPTS) -Dorg.slf4j.simpleLogger.showDateTime=true -Dorg.slf4j.simpleLogger.dateTimeFormat=HH:mm:ss,SSS' # Optional #mavenAuthenticateFeed: false #effectivePomSkip: false #sonarQubeRunAnalysis: false diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java index 448e1a542bd..add97e858a8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java @@ -6,6 +6,7 @@ import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.server.util.ICachedSearchDetails; import org.apache.commons.lang3.SerializationUtils; +import org.apache.commons.lang3.builder.ToStringBuilder; import org.hibernate.annotations.OptimisticLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,6 +54,20 @@ public class Search implements ICachedSearchDetails, Serializable { private static final long serialVersionUID = 1L; private static final Logger ourLog = LoggerFactory.getLogger(Search.class); + @Override + public String toString() { + return new ToStringBuilder(this) + .append("myLastUpdatedHigh", myLastUpdatedHigh) + .append("myLastUpdatedLow", myLastUpdatedLow) + .append("myNumFound", myNumFound) + .append("myNumBlocked", myNumBlocked) + .append("myStatus", myStatus) + .append("myTotalCount", myTotalCount) + .append("myUuid", myUuid) + .append("myVersion", myVersion) + .toString(); + } + @Temporal(TemporalType.TIMESTAMP) @Column(name = "CREATED", nullable = false, updatable = false) private Date myCreated;