From 3ed12ce944f652c67e94b37a835ee4e74dbfc8bb Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 28 Jun 2017 18:13:44 -0400 Subject: [PATCH 01/14] Improve performance of JPA searches by using Hibernate ScrollableResults --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 143 ++++++++---------- src/changes/changes.xml | 4 + 2 files changed, 63 insertions(+), 84 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 36f99d9368e..f39edd259a8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -27,86 +27,34 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; import java.math.BigDecimal; import java.math.MathContext; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Set; import javax.persistence.EntityManager; import javax.persistence.TypedQuery; -import javax.persistence.criteria.AbstractQuery; -import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.*; import javax.persistence.criteria.CriteriaBuilder.In; -import javax.persistence.criteria.CriteriaQuery; -import javax.persistence.criteria.Expression; -import javax.persistence.criteria.From; -import javax.persistence.criteria.Join; -import javax.persistence.criteria.JoinType; -import javax.persistence.criteria.Order; -import javax.persistence.criteria.Path; -import javax.persistence.criteria.Predicate; -import javax.persistence.criteria.Root; -import javax.persistence.criteria.Subquery; -import org.apache.commons.lang3.ObjectUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.*; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.tuple.Pair; -import org.hl7.fhir.instance.model.api.IAnyResource; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; +import org.hibernate.ScrollMode; +import org.hibernate.ScrollableResults; +import org.hibernate.query.Query; +import org.hl7.fhir.instance.model.api.*; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; +import com.google.common.collect.*; -import ca.uhn.fhir.context.BaseRuntimeChildDefinition; -import ca.uhn.fhir.context.BaseRuntimeDeclaredChildDefinition; -import ca.uhn.fhir.context.BaseRuntimeElementDefinition; -import ca.uhn.fhir.context.ConfigurationException; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.FhirVersionEnum; -import ca.uhn.fhir.context.RuntimeChildChoiceDefinition; -import ca.uhn.fhir.context.RuntimeChildResourceDefinition; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.context.*; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; -import ca.uhn.fhir.jpa.entity.BaseHasResource; -import ca.uhn.fhir.jpa.entity.BaseResourceIndexedSearchParam; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamDate; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamNumber; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamQuantity; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamString; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamToken; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamUri; -import ca.uhn.fhir.jpa.entity.ResourceLink; -import ca.uhn.fhir.jpa.entity.ResourceTable; -import ca.uhn.fhir.jpa.entity.ResourceTag; -import ca.uhn.fhir.jpa.entity.SearchParam; -import ca.uhn.fhir.jpa.entity.SearchParamPresent; -import ca.uhn.fhir.jpa.entity.TagDefinition; -import ca.uhn.fhir.jpa.entity.TagTypeEnum; +import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.util.StopWatch; -import ca.uhn.fhir.model.api.IPrimitiveDatatype; -import ca.uhn.fhir.model.api.IQueryParameterType; -import ca.uhn.fhir.model.api.IResource; -import ca.uhn.fhir.model.api.Include; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; -import ca.uhn.fhir.model.base.composite.BaseCodingDt; -import ca.uhn.fhir.model.base.composite.BaseIdentifierDt; -import ca.uhn.fhir.model.base.composite.BaseQuantityDt; +import ca.uhn.fhir.model.api.*; +import ca.uhn.fhir.model.base.composite.*; import ca.uhn.fhir.model.dstu.resource.BaseResource; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.InstantDt; @@ -115,23 +63,9 @@ import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.method.RestSearchParameterTypeEnum; -import ca.uhn.fhir.rest.param.CompositeParam; -import ca.uhn.fhir.rest.param.DateParam; -import ca.uhn.fhir.rest.param.DateRangeParam; -import ca.uhn.fhir.rest.param.HasParam; -import ca.uhn.fhir.rest.param.NumberParam; -import ca.uhn.fhir.rest.param.ParamPrefixEnum; -import ca.uhn.fhir.rest.param.QuantityParam; -import ca.uhn.fhir.rest.param.ReferenceParam; -import ca.uhn.fhir.rest.param.StringParam; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.rest.param.TokenParamModifier; -import ca.uhn.fhir.rest.param.UriParam; -import ca.uhn.fhir.rest.param.UriParamQualifierEnum; +import ca.uhn.fhir.rest.param.*; import ca.uhn.fhir.rest.server.Constants; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.util.UrlUtil; /** @@ -139,8 +73,8 @@ import ca.uhn.fhir.util.UrlUtil; * searchs for resources */ public class SearchBuilder implements ISearchBuilder { + private static final List EMPTY_LONG_LIST = Collections.unmodifiableList(new ArrayList()); - private static Long NO_MORE = Long.valueOf(-1); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchBuilder.class); private List myAlsoIncludePids; @@ -161,7 +95,7 @@ public class SearchBuilder implements ISearchBuilder { private ISearchParamRegistry mySearchParamRegistry; private String mySearchUuid; private IHapiTerminologySvc myTerminologySvc; - + /** * Constructor */ @@ -2037,6 +1971,7 @@ public class SearchBuilder implements ISearchBuilder { static Predicate[] toArray(List thePredicates) { return thePredicates.toArray(new Predicate[thePredicates.size()]); } + public class IncludesIterator implements Iterator{ private Iterator myCurrentIterator; @@ -2094,7 +2029,6 @@ public class SearchBuilder implements ISearchBuilder { } } - private enum JoinEnum { DATE, NUMBER, QUANTITY, REFERENCE, STRING, TOKEN, URI @@ -2160,7 +2094,12 @@ public class SearchBuilder implements ISearchBuilder { Integer maximumResults = myCallingDao.getConfig().getFetchSizeDefaultMaximum(); final TypedQuery query = createQuery(mySort, maximumResults); - myResultsIterator = query.getResultList().iterator(); + + Query hibernateQuery = (Query) query; + ScrollableResults scroll = hibernateQuery.scroll(ScrollMode.FORWARD_ONLY); + myResultsIterator = new ScrollableResultsIterator(scroll); + +// myResultsIterator = query.getResultList().iterator(); // If the query resulted in extra results being requested if (myAlsoIncludePids != null) { @@ -2245,4 +2184,40 @@ public class SearchBuilder implements ISearchBuilder { } } + public class ScrollableResultsIterator implements Iterator { + + private Long myNext; + private ScrollableResults myScroll; + + public ScrollableResultsIterator(ScrollableResults theScroll) { + myScroll = theScroll; + } + + private void ensureHaveNext() { + if (myNext == null) { + if (myScroll.next()) { + myNext = (Long) myScroll.get(0); + } else { + myNext = NO_MORE; + } + } + } + + @Override + public boolean hasNext() { + ensureHaveNext(); + return myNext != NO_MORE; + } + + @Override + public Long next() { + ensureHaveNext(); + Validate.isTrue(myNext != NO_MORE); + Long next = myNext; + myNext = null; + return next; + } + + } + } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index ec61790271a..c33817e9798 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -62,6 +62,10 @@ actually notify listeners of the first 10 subscriptions. Thanks to Jeff Chung for the pull request! + + JPA search now uses hibernate ScrollableResults instead of plain JPA List. This + should improve performance over large search results. + From 10ff2dd16c588b74b83975f1ada8f4c8ca2dc224 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 28 Jun 2017 19:54:24 -0400 Subject: [PATCH 02/14] Add test --- .../java/ca/uhn/fhir/rest/param/ReferenceParam.java | 6 ++++-- .../dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java index cd29240777e..d27c8dcb4dd 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java @@ -173,12 +173,14 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/ return true; } - public void setChain(String theChain) { + public ReferenceParam setChain(String theChain) { myChain = theChain; + return this; } - public void setValue(String theValue) { + public ReferenceParam setValue(String theValue) { myId.setValue(theValue); + return this; } /** diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 748901984e5..109984e1f9b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -153,6 +153,17 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { assertThat(ids, contains(moId.getValue())); } + @Test + public void testEmptyChain() { + + SearchParameterMap map = new SearchParameterMap(); + map.add(Encounter.SP_SUBJECT, new ReferenceAndListParam().addAnd(new ReferenceOrListParam().add((ReferenceParam)new ReferenceParam("subject", "04823543").setChain("identifier")))); + IBundleProvider results = myMedicationAdministrationDao.search(map); + List ids = toUnqualifiedIdValues(results); + + assertThat(ids, empty()); + } + @Test public void testEverythingTimings() throws Exception { String methodName = "testEverythingTimings"; From 674424a30b8798897400a261af8271b2c98dddcf Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 29 Jun 2017 21:34:56 -0400 Subject: [PATCH 03/14] Add tests --- .../ca/uhn/fhir/rest/param/NumberParam.java | 29 +- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 2 - .../jpa/search/SearchCoordinatorSvcImpl.java | 56 +-- .../provider/ResourceProviderDstu2Test.java | 82 +++- .../dstu3/ResourceProviderDstu3Test.java | 433 ++++++++++-------- .../PagingMultinodeProviderDstu3Test.java | 2 - 6 files changed, 356 insertions(+), 248 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java index 7dc32ef8b5b..1e9e113a374 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java @@ -32,6 +32,7 @@ import ca.uhn.fhir.model.api.IQueryParameterType; public class NumberParam extends BaseParamWithPrefix implements IQueryParameterType { + private static final long serialVersionUID = 1L; private BigDecimal myQuantity; /** @@ -41,6 +42,16 @@ public class NumberParam extends BaseParamWithPrefix implements IQu super(); } + /** + * Constructor + * + * @param theValue + * A value, e.g. "10" + */ + public NumberParam(int theValue) { + setValue(new BigDecimal(theValue)); + } + /** * Constructor * @@ -79,6 +90,15 @@ public class NumberParam extends BaseParamWithPrefix implements IQu } + public BigDecimal getValue() { + return myQuantity; + } + + public NumberParam setValue(BigDecimal theValue) { + myQuantity = theValue; + return this; + } + @Override public String toString() { ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SIMPLE_STYLE); @@ -87,13 +107,4 @@ public class NumberParam extends BaseParamWithPrefix implements IQu return b.build(); } - public BigDecimal getValue() { - return myQuantity; - } - - public NumberParam setValue(BigDecimal theValue) { - myQuantity = theValue; - return this; - } - } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index f39edd259a8..a593810fe67 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -2099,8 +2099,6 @@ public class SearchBuilder implements ISearchBuilder { ScrollableResults scroll = hibernateQuery.scroll(ScrollMode.FORWARD_ONLY); myResultsIterator = new ScrollableResultsIterator(scroll); -// myResultsIterator = query.getResultList().iterator(); - // If the query resulted in extra results being requested if (myAlsoIncludePids != null) { myPreResultsIterator = myAlsoIncludePids.iterator(); 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 77b0097533c..8eb3e8885be 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 @@ -19,24 +19,9 @@ package ca.uhn.fhir.jpa.search; * limitations under the License. * #L% */ +import java.util.*; +import java.util.concurrent.*; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Set; -import java.util.UUID; -import java.util.concurrent.Callable; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; - -import javax.annotation.PostConstruct; import javax.persistence.EntityManager; import javax.transaction.Transactional; import javax.transaction.Transactional.TxType; @@ -47,34 +32,18 @@ import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.commons.lang3.time.DateUtils; import org.hl7.fhir.instance.model.api.IBaseResource; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.data.domain.Page; -import org.springframework.data.domain.PageRequest; -import org.springframework.data.domain.Pageable; -import org.springframework.orm.jpa.JpaTransactionManager; +import org.springframework.data.domain.*; import org.springframework.scheduling.concurrent.CustomizableThreadFactory; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.TransactionStatus; -import org.springframework.transaction.support.TransactionCallback; -import org.springframework.transaction.support.TransactionCallbackWithoutResult; -import org.springframework.transaction.support.TransactionTemplate; +import org.springframework.transaction.*; +import org.springframework.transaction.support.*; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.jpa.dao.IDao; -import ca.uhn.fhir.jpa.dao.ISearchBuilder; -import ca.uhn.fhir.jpa.dao.SearchParameterMap; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; -import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao; -import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; -import ca.uhn.fhir.jpa.entity.Search; -import ca.uhn.fhir.jpa.entity.SearchInclude; -import ca.uhn.fhir.jpa.entity.SearchResult; -import ca.uhn.fhir.jpa.entity.SearchStatusEnum; -import ca.uhn.fhir.jpa.entity.SearchTypeEnum; +import ca.uhn.fhir.jpa.dao.*; +import ca.uhn.fhir.jpa.dao.data.*; +import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.util.StopWatch; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.rest.method.PageMethodBinding; @@ -83,7 +52,7 @@ import ca.uhn.fhir.rest.server.SimpleBundleProvider; import ca.uhn.fhir.rest.server.exceptions.*; public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { - static final int DEFAULT_SYNC_SIZE = 250; + public static final int DEFAULT_SYNC_SIZE = 250; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchCoordinatorSvcImpl.class); @@ -360,7 +329,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } @VisibleForTesting - void setLoadingThrottleForUnitTests(Integer theLoadingThrottleForUnitTests) { + public void setLoadingThrottleForUnitTests(Integer theLoadingThrottleForUnitTests) { myLoadingThrottleForUnitTests = theLoadingThrottleForUnitTests; } @@ -369,7 +338,8 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { myMaxMillisToWaitForRemoteResults = theMaxMillisToWaitForRemoteResults; } - void setNeverUseLocalSearchForUnitTests(boolean theNeverUseLocalSearchForUnitTests) { + @VisibleForTesting + public void setNeverUseLocalSearchForUnitTests(boolean theNeverUseLocalSearchForUnitTests) { myNeverUseLocalSearchForUnitTests = theNeverUseLocalSearchForUnitTests; } @@ -389,7 +359,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } @VisibleForTesting - void setSyncSizeForUnitTests(int theSyncSize) { + public void setSyncSizeForUnitTests(int theSyncSize) { mySyncSize = theSyncSize; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java index b3d3c83526b..41b5587f423 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java @@ -5,6 +5,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsInRelativeOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.not; @@ -47,11 +48,12 @@ import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.Ignore; +import org.junit.*; import org.junit.Test; +import org.springframework.test.util.AopTestUtils; +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.model.api.*; import ca.uhn.fhir.model.api.Bundle; import ca.uhn.fhir.model.api.BundleEntry; @@ -85,10 +87,7 @@ import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.SummaryEnum; import ca.uhn.fhir.rest.client.IGenericClient; -import ca.uhn.fhir.rest.param.DateRangeParam; -import ca.uhn.fhir.rest.param.StringAndListParam; -import ca.uhn.fhir.rest.param.StringOrListParam; -import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.param.*; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; @@ -100,10 +99,27 @@ import ca.uhn.fhir.util.UrlUtil; public class ResourceProviderDstu2Test extends BaseResourceProviderDstu2Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderDstu2Test.class); + private SearchCoordinatorSvcImpl mySearchCoordinatorSvcRaw; + + @Override + @After + public void after() throws Exception { + super.after(); + + myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); + myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences()); + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(null); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(SearchCoordinatorSvcImpl.DEFAULT_SYNC_SIZE); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(false); + + } @Before public void beforeDisableResultReuse() { myDaoConfig.setReuseCachedSearchResultsForMillis(null); + mySearchCoordinatorSvcRaw = AopTestUtils.getTargetObject(mySearchCoordinatorSvc); } @AfterClass @@ -118,6 +134,58 @@ public class ResourceProviderDstu2Test extends BaseResourceProviderDstu2Test { myDaoConfig.setAllowMultipleDelete(true); } + + @Test + public void testPagingOverEverythingSet() throws InterruptedException { + Patient p = new Patient(); + p.setActive(true); + String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + for (int i = 0; i < 20; i++) { + Observation o = new Observation(); + o.getSubject().setReference(pid); + o.addIdentifier().setSystem("foo").setValue(Integer.toString(i)); + myObservationDao.create(o); + } + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true); + + ca.uhn.fhir.model.dstu2.resource.Bundle response = ourClient + .operation() + .onInstance(new IdDt(pid)) + .named("everything") + .withSearchParameter(Parameters.class, "_count", new NumberParam(10)) + .returnResourceType(ca.uhn.fhir.model.dstu2.resource.Bundle.class) + .useHttpGet() + .execute(); + + assertEquals(10, response.getEntry().size()); + assertEquals(null, response.getTotalElement().getValueAsString()); + assertThat(response.getLink("next").getUrl(), not(emptyString())); + + // Load page 2 + + String nextUrl = response.getLink("next").getUrl(); + response = ourClient.fetchResourceFromUrl(ca.uhn.fhir.model.dstu2.resource.Bundle.class, nextUrl); + + assertEquals(10, response.getEntry().size()); + assertThat(response.getLink("next").getUrl(), not(emptyString())); + + // Load page 3 + Thread.sleep(2000); + + nextUrl = response.getLink("next").getUrl(); + response = ourClient.fetchResourceFromUrl(ca.uhn.fhir.model.dstu2.resource.Bundle.class, nextUrl); + + assertEquals(1, response.getEntry().size()); + assertEquals(21, response.getTotal().intValue()); + assertEquals(null, response.getLink("next")); + + } + + private void checkParamMissing(String paramName) throws IOException, ClientProtocolException { HttpGet get = new HttpGet(ourServerBase + "/Observation?" + paramName + ":missing=false"); CloseableHttpResponse resp = ourHttpClient.execute(get); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index c4a74a2c204..093cfc08356 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -6,6 +6,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsInRelativeOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; @@ -50,13 +51,16 @@ import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; import org.hl7.fhir.instance.model.Encounter.EncounterState; import org.hl7.fhir.instance.model.api.*; import org.junit.*; +import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; import com.google.common.collect.Lists; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.SearchBuilder; import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.UriDt; @@ -76,11 +80,7 @@ import ca.uhn.fhir.util.UrlUtil; public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderDstu3Test.class); - - @Before - public void beforeDisableResultReuse() { - myDaoConfig.setReuseCachedSearchResultsForMillis(null); - } + private SearchCoordinatorSvcImpl mySearchCoordinatorSvcRaw; @Override @After @@ -90,6 +90,11 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences()); myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(null); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(SearchCoordinatorSvcImpl.DEFAULT_SYNC_SIZE); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(false); + } @Override @@ -100,95 +105,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { myDaoConfig.setAllowMultipleDelete(true); } - @Test - public void testSearchWithEmptyParameter() throws Exception { - Observation obs= new Observation(); - obs.setStatus(ObservationStatus.FINAL); - obs.getCode().addCoding().setSystem("foo").setCode("bar"); - ourClient.create().resource(obs).execute(); - - testSearchWithEmptyParameter("/Observation?value-quantity="); - testSearchWithEmptyParameter("/Observation?code=bar&value-quantity="); - testSearchWithEmptyParameter("/Observation?value-date="); - testSearchWithEmptyParameter("/Observation?code=bar&value-date="); - testSearchWithEmptyParameter("/Observation?value-concept="); - testSearchWithEmptyParameter("/Observation?code=bar&value-concept="); - } - - private void testSearchWithEmptyParameter(String url) throws IOException, ClientProtocolException { - HttpGet get = new HttpGet(ourServerBase + url); - CloseableHttpResponse resp = ourHttpClient.execute(get); - try { - assertEquals(200, resp.getStatusLine().getStatusCode()); - String respString = IOUtils.toString(resp.getEntity().getContent(), Constants.CHARSET_UTF8); - Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, respString); - assertEquals(1, bundle.getEntry().size()); - } finally { - IOUtils.closeQuietly(resp.getEntity().getContent()); - } - } - - @Test - public void testSearchWithMissingDate2() throws Exception { - MedicationRequest mr1 = new MedicationRequest(); - mr1.getCategory().addCoding().setSystem("urn:medicationroute").setCode("oral"); - mr1.addDosageInstruction().getTiming().addEventElement().setValueAsString("2017-01-01"); - IIdType id1 = myMedicationRequestDao.create(mr1).getId().toUnqualifiedVersionless(); - - MedicationRequest mr2 = new MedicationRequest(); - mr2.getCategory().addCoding().setSystem("urn:medicationroute").setCode("oral"); - IIdType id2 = myMedicationRequestDao.create(mr2).getId().toUnqualifiedVersionless(); - - HttpGet get = new HttpGet(ourServerBase + "/MedicationRequest?date:missing=false"); - CloseableHttpResponse resp = ourHttpClient.execute(get); - try { - assertEquals(200, resp.getStatusLine().getStatusCode()); - Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, IOUtils.toString(resp.getEntity().getContent(), Constants.CHARSET_UTF8)); - - List ids = toUnqualifiedVersionlessIdValues(bundle); - assertThat(ids, contains(id1.getValue())); - } finally { - IOUtils.closeQuietly(resp); - } - - } - - @Test - public void testEverythingWithOnlyPatient() { - Patient p = new Patient(); - p.setActive(true); - IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); - - myFhirCtx.getRestfulClientFactory().setSocketTimeout(300 * 1000); - - Bundle response = ourClient - .operation() - .onInstance(id) - .named("everything") - .withNoParameters(Parameters.class) - .returnResourceType(Bundle.class) - .execute(); - - assertEquals(1, response.getEntry().size()); - } - - @Test - public void testSaveAndRetrieveResourceWithExtension() { - Patient nextPatient = new Patient(); - nextPatient.setId("Patient/B"); - nextPatient - .addExtension() - .setUrl("http://foo") - .setValue(new Reference("Practitioner/A")); - - ourClient.update().resource(nextPatient).execute(); - - Patient p = ourClient.read().resource(Patient.class).withId("B").execute(); - - String encoded = myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(p); - ourLog.info(encoded); - - assertThat(encoded, containsString("http://foo")); + @Before + public void beforeDisableResultReuse() { + myDaoConfig.setReuseCachedSearchResultsForMillis(null); + mySearchCoordinatorSvcRaw = AopTestUtils.getTargetObject(mySearchCoordinatorSvc); } private void checkParamMissing(String paramName) throws IOException, ClientProtocolException { @@ -197,7 +117,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { IOUtils.closeQuietly(resp.getEntity().getContent()); assertEquals(200, resp.getStatusLine().getStatusCode()); } - + + private ArrayList genResourcesOfType(Bundle theRes, Class theClass) { ArrayList retVal = new ArrayList(); for (BundleEntryComponent next : theRes.getEntry()) { @@ -354,8 +275,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } - - /** * See #438 */ @@ -429,7 +348,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { Validate.notNull(input); ourClient.create().resource(input).execute().getResource(); } - @Test public void testCreateConditional() { @@ -552,6 +470,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + + @Test public void testCreateResourceConditionalComplex() throws IOException { Patient pt = new Patient(); @@ -614,6 +534,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { response.close(); } } + @Test public void testCreateResourceWithNumericId() throws IOException { @@ -1300,31 +1221,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } - // private void delete(String theResourceType, String theParamName, String theParamValue) { - // Bundle resources; - // do { - // IQuery forResource = ourClient.search().forResource(theResourceType); - // if (theParamName != null) { - // forResource = forResource.where(new StringClientParam(theParamName).matches().value(theParamValue)); - // } - // resources = forResource.execute(); - // for (IResource next : resources.toListOfResources()) { - // ourLog.info("Deleting resource: {}", next.getId()); - // ourClient.delete().resource(next).execute(); - // } - // } while (resources.size() > 0); - // } - // - // private void deleteToken(String theResourceType, String theParamName, String theParamSystem, String theParamValue) - // { - // Bundle resources = ourClient.search().forResource(theResourceType).where(new - // TokenClientParam(theParamName).exactly().systemAndCode(theParamSystem, theParamValue)).execute(); - // for (IResource next : resources.toListOfResources()) { - // ourLog.info("Deleting resource: {}", next.getId()); - // ourClient.delete().resource(next).execute(); - // } - // } - @Test public void testEverythingPatientOperation() throws Exception { String methodName = "testEverythingOperation"; @@ -1370,53 +1266,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info(ids.toString()); } - @Test - public void testSearchByLastUpdated() throws Exception { - String methodName = "testSearchByLastUpdated"; - - Patient p = new Patient(); - p.addName().setFamily(methodName+"1"); - IIdType pid1 = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); - - Thread.sleep(10); - long time1 = System.currentTimeMillis(); - Thread.sleep(10); - - Patient p2 = new Patient(); - p2.addName().setFamily(methodName+"2"); - IIdType pid2 = ourClient.create().resource(p2).execute().getId().toUnqualifiedVersionless(); - - HttpGet get = new HttpGet(ourServerBase + "/Patient?_lastUpdated=lt" + new InstantType(new Date(time1)).getValueAsString()); - CloseableHttpResponse response = ourHttpClient.execute(get); - try { - assertEquals(200, response.getStatusLine().getStatusCode()); - String output = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); - IOUtils.closeQuietly(response.getEntity().getContent()); - ourLog.info(output); - List ids = toUnqualifiedVersionlessIds(myFhirCtx.newXmlParser().parseResource(Bundle.class, output)); - ourLog.info(ids.toString()); - assertThat(ids, containsInAnyOrder(pid1)); - } finally { - response.close(); - } - - get = new HttpGet(ourServerBase + "/Patient?_lastUpdated=gt" + new InstantType(new Date(time1)).getValueAsString()); - response = ourHttpClient.execute(get); - try { - assertEquals(200, response.getStatusLine().getStatusCode()); - String output = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); - IOUtils.closeQuietly(response.getEntity().getContent()); - ourLog.info(output); - List ids = toUnqualifiedVersionlessIds(myFhirCtx.newXmlParser().parseResource(Bundle.class, output)); - ourLog.info(ids.toString()); - assertThat(ids, containsInAnyOrder(pid2)); - } finally { - response.close(); - } - - } - - @Test public void testEverythingPatientType() throws Exception { String methodName = "testEverythingPatientType"; @@ -1679,6 +1528,50 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { assertEquals(77, ids.size()); } + @Test + public void testEverythingWithOnlyPatient() { + Patient p = new Patient(); + p.setActive(true); + IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + myFhirCtx.getRestfulClientFactory().setSocketTimeout(300 * 1000); + + Bundle response = ourClient + .operation() + .onInstance(id) + .named("everything") + .withNoParameters(Parameters.class) + .returnResourceType(Bundle.class) + .execute(); + + assertEquals(1, response.getEntry().size()); + } + + // private void delete(String theResourceType, String theParamName, String theParamValue) { + // Bundle resources; + // do { + // IQuery forResource = ourClient.search().forResource(theResourceType); + // if (theParamName != null) { + // forResource = forResource.where(new StringClientParam(theParamName).matches().value(theParamValue)); + // } + // resources = forResource.execute(); + // for (IResource next : resources.toListOfResources()) { + // ourLog.info("Deleting resource: {}", next.getId()); + // ourClient.delete().resource(next).execute(); + // } + // } while (resources.size() > 0); + // } + // + // private void deleteToken(String theResourceType, String theParamName, String theParamSystem, String theParamValue) + // { + // Bundle resources = ourClient.search().forResource(theResourceType).where(new + // TokenClientParam(theParamName).exactly().systemAndCode(theParamSystem, theParamValue)).execute(); + // for (IResource next : resources.toListOfResources()) { + // ourLog.info("Deleting resource: {}", next.getId()); + // ourClient.delete().resource(next).execute(); + // } + // } + @SuppressWarnings("unused") @Test public void testFullTextSearch() throws RuntimeException, Exception { @@ -1727,7 +1620,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { response.close(); } } - + + @Test public void testHasParameter() throws Exception { IIdType pid0; @@ -2074,6 +1968,57 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + @Test + public void testPagingOverEverythingSet() throws InterruptedException { + Patient p = new Patient(); + p.setActive(true); + String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + for (int i = 0; i < 20; i++) { + Observation o = new Observation(); + o.getSubject().setReference(pid); + o.addIdentifier().setSystem("foo").setValue(Integer.toString(i)); + myObservationDao.create(o); + } + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true); + + Bundle response = ourClient + .operation() + .onInstance(new IdType(pid)) + .named("everything") + .withSearchParameter(Parameters.class, "_count", new NumberParam(10)) + .returnResourceType(Bundle.class) + .useHttpGet() + .execute(); + + assertEquals(10, response.getEntry().size()); + assertEquals(null, response.getTotalElement().getValueAsString()); + assertThat(response.getLink("next").getUrl(), not(emptyString())); + + // Load page 2 + + String nextUrl = response.getLink("next").getUrl(); + response = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl); + + assertEquals(10, response.getEntry().size()); + assertEquals(null, response.getTotalElement().getValueAsString()); + assertThat(response.getLink("next").getUrl(), not(emptyString())); + + // Load page 3 + Thread.sleep(2000); + + nextUrl = response.getLink("next").getUrl(); + response = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl); + + assertEquals(1, response.getEntry().size()); + assertEquals(21, response.getTotal()); + assertEquals(null, response.getLink("next")); + + } + @Test public void testPatchUsingJsonPatch() throws Exception { String methodName = "testPatchUsingJsonPatch"; @@ -2297,6 +2242,25 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { assertEquals("
HELLO WORLD
", actual.getText().getDiv().getValueAsString()); } + @Test + public void testSaveAndRetrieveResourceWithExtension() { + Patient nextPatient = new Patient(); + nextPatient.setId("Patient/B"); + nextPatient + .addExtension() + .setUrl("http://foo") + .setValue(new Reference("Practitioner/A")); + + ourClient.update().resource(nextPatient).execute(); + + Patient p = ourClient.read().resource(Patient.class).withId("B").execute(); + + String encoded = myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(p); + ourLog.info(encoded); + + assertThat(encoded, containsString("http://foo")); + } + @Test public void testSaveAndRetrieveWithContained() { Patient p1 = new Patient(); @@ -2517,6 +2481,52 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + @Test + public void testSearchByLastUpdated() throws Exception { + String methodName = "testSearchByLastUpdated"; + + Patient p = new Patient(); + p.addName().setFamily(methodName+"1"); + IIdType pid1 = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + Thread.sleep(10); + long time1 = System.currentTimeMillis(); + Thread.sleep(10); + + Patient p2 = new Patient(); + p2.addName().setFamily(methodName+"2"); + IIdType pid2 = ourClient.create().resource(p2).execute().getId().toUnqualifiedVersionless(); + + HttpGet get = new HttpGet(ourServerBase + "/Patient?_lastUpdated=lt" + new InstantType(new Date(time1)).getValueAsString()); + CloseableHttpResponse response = ourHttpClient.execute(get); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String output = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + IOUtils.closeQuietly(response.getEntity().getContent()); + ourLog.info(output); + List ids = toUnqualifiedVersionlessIds(myFhirCtx.newXmlParser().parseResource(Bundle.class, output)); + ourLog.info(ids.toString()); + assertThat(ids, containsInAnyOrder(pid1)); + } finally { + response.close(); + } + + get = new HttpGet(ourServerBase + "/Patient?_lastUpdated=gt" + new InstantType(new Date(time1)).getValueAsString()); + response = ourHttpClient.execute(get); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String output = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + IOUtils.closeQuietly(response.getEntity().getContent()); + ourLog.info(output); + List ids = toUnqualifiedVersionlessIds(myFhirCtx.newXmlParser().parseResource(Bundle.class, output)); + ourLog.info(ids.toString()); + assertThat(ids, containsInAnyOrder(pid2)); + } finally { + response.close(); + } + + } + @Test public void testSearchByReferenceIds() { Organization o1 = new Organization(); @@ -2587,6 +2597,28 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + @Test + public void testSearchInvalidParam() throws Exception { + Patient patient = new Patient(); + patient.addIdentifier().setSystem("urn:system").setValue("0"); + patient.addName().setFamily("testSearchWithMixedParams").addGiven("Joe"); + myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + + // should be subject._id + HttpGet httpPost = new HttpGet(ourServerBase + "/Observation?subject.id=FOO"); + + CloseableHttpResponse resp = ourHttpClient.execute(httpPost); + try { + String respString = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(respString); + assertThat(respString, containsString("Invalid parameter chain: subject.id")); + assertEquals(400, resp.getStatusLine().getStatusCode()); + } finally { + IOUtils.closeQuietly(resp.getEntity().getContent()); + } + ourLog.info("Outgoing post: {}", httpPost); + } + @Test public void testSearchLastUpdatedParamRp() throws InterruptedException { String methodName = "testSearchLastUpdatedParamRp"; @@ -3047,6 +3079,34 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { assertEquals(2, response.getEntry().size()); } + @Test + public void testSearchWithEmptyParameter() throws Exception { + Observation obs= new Observation(); + obs.setStatus(ObservationStatus.FINAL); + obs.getCode().addCoding().setSystem("foo").setCode("bar"); + ourClient.create().resource(obs).execute(); + + testSearchWithEmptyParameter("/Observation?value-quantity="); + testSearchWithEmptyParameter("/Observation?code=bar&value-quantity="); + testSearchWithEmptyParameter("/Observation?value-date="); + testSearchWithEmptyParameter("/Observation?code=bar&value-date="); + testSearchWithEmptyParameter("/Observation?value-concept="); + testSearchWithEmptyParameter("/Observation?code=bar&value-concept="); + } + + private void testSearchWithEmptyParameter(String url) throws IOException, ClientProtocolException { + HttpGet get = new HttpGet(ourServerBase + url); + CloseableHttpResponse resp = ourHttpClient.execute(get); + try { + assertEquals(200, resp.getStatusLine().getStatusCode()); + String respString = IOUtils.toString(resp.getEntity().getContent(), Constants.CHARSET_UTF8); + Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, respString); + assertEquals(1, bundle.getEntry().size()); + } finally { + IOUtils.closeQuietly(resp.getEntity().getContent()); + } + } + @Test public void testSearchWithInclude() throws Exception { Organization org = new Organization(); @@ -3211,25 +3271,28 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } @Test - public void testSearchInvalidParam() throws Exception { - Patient patient = new Patient(); - patient.addIdentifier().setSystem("urn:system").setValue("0"); - patient.addName().setFamily("testSearchWithMixedParams").addGiven("Joe"); - myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + public void testSearchWithMissingDate2() throws Exception { + MedicationRequest mr1 = new MedicationRequest(); + mr1.getCategory().addCoding().setSystem("urn:medicationroute").setCode("oral"); + mr1.addDosageInstruction().getTiming().addEventElement().setValueAsString("2017-01-01"); + IIdType id1 = myMedicationRequestDao.create(mr1).getId().toUnqualifiedVersionless(); - // should be subject._id - HttpGet httpPost = new HttpGet(ourServerBase + "/Observation?subject.id=FOO"); + MedicationRequest mr2 = new MedicationRequest(); + mr2.getCategory().addCoding().setSystem("urn:medicationroute").setCode("oral"); + IIdType id2 = myMedicationRequestDao.create(mr2).getId().toUnqualifiedVersionless(); - CloseableHttpResponse resp = ourHttpClient.execute(httpPost); + HttpGet get = new HttpGet(ourServerBase + "/MedicationRequest?date:missing=false"); + CloseableHttpResponse resp = ourHttpClient.execute(get); try { - String respString = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8); - ourLog.info(respString); - assertThat(respString, containsString("Invalid parameter chain: subject.id")); - assertEquals(400, resp.getStatusLine().getStatusCode()); + assertEquals(200, resp.getStatusLine().getStatusCode()); + Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, IOUtils.toString(resp.getEntity().getContent(), Constants.CHARSET_UTF8)); + + List ids = toUnqualifiedVersionlessIdValues(bundle); + assertThat(ids, contains(id1.getValue())); } finally { - IOUtils.closeQuietly(resp.getEntity().getContent()); + IOUtils.closeQuietly(resp); } - ourLog.info("Outgoing post: {}", httpPost); + } /** diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/PagingMultinodeProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/PagingMultinodeProviderDstu3Test.java index e23311281b3..531205ab31c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/PagingMultinodeProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/PagingMultinodeProviderDstu3Test.java @@ -42,8 +42,6 @@ public class PagingMultinodeProviderDstu3Test extends BaseResourceProviderDstu3T myDaoConfig.setAllowMultipleDelete(true); mySearchCoordinatorSvcRaw = AopTestUtils.getTargetObject(mySearchCoordinatorSvc); -// mySearchCoordinatorSvcRaw = (SearchCoordinatorSvcImpl)mySearchCoordinatorSvc; - } @Test From 7834bb262596e611e2845f4e5da32ff053fbdf5d Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 29 Jun 2017 22:15:23 -0400 Subject: [PATCH 04/14] Do not load results into the database in JPA if there is no paging provider --- .../rest/server/IRestfulServerDefaults.java | 5 +++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 25 +++++++++---- .../jpa/dao/FhirResourceDaoPatientDstu2.java | 10 +++-- .../dstu3/FhirResourceDaoPatientDstu3.java | 10 +++-- .../BaseResourceProviderDstu2Test.java | 17 +++++---- .../provider/ResourceProviderDstu2Test.java | 34 +++++++++++++++++ .../dstu3/BaseResourceProviderDstu3Test.java | 5 ++- .../dstu3/ResourceProviderDstu3Test.java | 37 +++++++++++++++++-- src/changes/changes.xml | 9 +++++ 9 files changed, 127 insertions(+), 25 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java index 8ab3697d1c9..4061ca7effb 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java @@ -79,4 +79,9 @@ public interface IRestfulServerDefaults { */ boolean isUseBrowserFriendlyContentTypes(); + /** + * Returns the paging provider for this server + */ + IPagingProvider getPagingProvider(); + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 56400d4f9e5..26ffe8fa261 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -48,6 +48,7 @@ import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.interceptor.IJpaServerInterceptor; +import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.jpa.util.StopWatch; @@ -562,6 +563,16 @@ public abstract class BaseHapiFhirResourceDao extends B theSavedEntity.setVersion(newVersionLong); } + protected boolean isPagingProviderDatabaseBacked(RequestDetails theRequestDetails) { + if (theRequestDetails == null) { + return false; + } + if (theRequestDetails.getServer().getPagingProvider() instanceof DatabaseBackedPagingProvider) { + return true; + } + return false; + } + @Override public MT metaAddOperation(IIdType theResourceId, MT theMetaAdd, RequestDetails theRequestDetails) { // Notify interceptors @@ -594,6 +605,7 @@ public abstract class BaseHapiFhirResourceDao extends B return retVal; } + @Override public MT metaDeleteOperation(IIdType theResourceId, MT theMetaDel, RequestDetails theRequestDetails) { // Notify interceptors @@ -628,7 +640,6 @@ public abstract class BaseHapiFhirResourceDao extends B return retVal; } - @Override public MT metaGetOperation(Class theType, IIdType theId, RequestDetails theRequestDetails) { // Notify interceptors @@ -860,12 +871,12 @@ public abstract class BaseHapiFhirResourceDao extends B ourLog.debug("Indexing resource {} - PID {}", theResource.getIdElement().getValue(), theEntity.getId()); updateEntity(theResource, theEntity, null, true, false, theEntity.getUpdatedDate(), true, false); } - + @Override public void removeTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm) { removeTag(theId, theTagType, theScheme, theTerm, null); } - + @Override public void removeTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm, RequestDetails theRequestDetails) { // Notify interceptors @@ -919,15 +930,15 @@ public abstract class BaseHapiFhirResourceDao extends B int max = myDaoConfig.getMaximumSearchResultCountInTransaction(); theParams.setLoadSynchronousUpTo(myDaoConfig.getMaximumSearchResultCountInTransaction()); } - + + if (!isPagingProviderDatabaseBacked(theRequestDetails)) { + theParams.setLoadSynchronous(true); + } } return mySearchCoordinatorSvc.registerSearch(this, theParams, getResourceName()); } - - - @Override public Set searchForIds(SearchParameterMap theParams) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java index c9c48450959..f2681e1a1ea 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java @@ -46,7 +46,7 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2im super(); } - private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative) { + private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) { SearchParameterMap paramMap = new SearchParameterMap(); if (theCount != null) { paramMap.setCount(theCount.getValue()); @@ -65,6 +65,10 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2im paramMap.add("_id", new StringParam(theId.getIdPart())); } + if (!isPagingProviderDatabaseBacked(theRequestDetails)) { + paramMap.setLoadSynchronous(true); + } + return mySearchCoordinatorSvc.registerSearch(this, paramMap, getResourceName()); } @@ -74,7 +78,7 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2im ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getResourceName(), null); notifyInterceptors(RestOperationTypeEnum.EXTENDED_OPERATION_INSTANCE, requestDetails); - return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative); + return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails); } @Override @@ -83,7 +87,7 @@ public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2im ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getResourceName(), null); notifyInterceptors(RestOperationTypeEnum.EXTENDED_OPERATION_TYPE, requestDetails); - return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative); + return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoPatientDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoPatientDstu3.java index cfbde6e12da..eed04b60297 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoPatientDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoPatientDstu3.java @@ -50,7 +50,7 @@ public class FhirResourceDaoPatientDstu3 extends FhirResourceDaoDstu3im @Autowired private ISearchParamRegistry mySerarchParamRegistry; - private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative) { + private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) { SearchParameterMap paramMap = new SearchParameterMap(); if (theCount != null) { paramMap.setCount(theCount.getValue()); @@ -69,17 +69,21 @@ public class FhirResourceDaoPatientDstu3 extends FhirResourceDaoDstu3im paramMap.add("_id", new StringParam(theId.getIdPart())); } + if (!isPagingProviderDatabaseBacked(theRequestDetails)) { + paramMap.setLoadSynchronous(true); + } + return mySearchCoordinatorSvc.registerSearch(this, paramMap, getResourceName()); } @Override public IBundleProvider patientInstanceEverything(HttpServletRequest theServletRequest, IIdType theId, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) { - return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative); + return doEverythingOperation(theId, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails); } @Override public IBundleProvider patientTypeEverything(HttpServletRequest theServletRequest, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, RequestDetails theRequestDetails) { - return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative); + return doEverythingOperation(null, theCount, theLastUpdated, theSort, theContent, theNarrative, theRequestDetails); } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java index 96673f89ef2..a93551fc9cf 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java @@ -12,9 +12,7 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; +import org.junit.*; import org.springframework.web.context.ContextLoader; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; @@ -25,17 +23,16 @@ import ca.uhn.fhir.jpa.config.WebsocketDstu2Config; import ca.uhn.fhir.jpa.config.WebsocketDstu2DispatcherConfig; import ca.uhn.fhir.jpa.dao.dstu2.BaseJpaDstu2Test; import ca.uhn.fhir.jpa.interceptor.RestHookSubscriptionDstu2Interceptor; +import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider; import ca.uhn.fhir.model.api.Bundle; import ca.uhn.fhir.model.api.BundleEntry; import ca.uhn.fhir.model.dstu2.resource.Patient; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; -import ca.uhn.fhir.parser.LenientErrorHandler; import ca.uhn.fhir.rest.client.IGenericClient; import ca.uhn.fhir.rest.client.ServerValidationModeEnum; import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; -import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.util.TestUtil; @@ -45,10 +42,11 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { protected static CloseableHttpClient ourHttpClient; protected static int ourPort; protected static RestfulServer ourRestServer; - private static Server ourServer; + protected static Server ourServer; protected static String ourServerBase; - private static GenericWebApplicationContext ourWebApplicationContext; + protected static GenericWebApplicationContext ourWebApplicationContext; protected static RestHookSubscriptionDstu2Interceptor ourRestHookSubscriptionInterceptor; + protected static DatabaseBackedPagingProvider ourPagingProvider; public BaseResourceProviderDstu2Test() { super(); @@ -83,7 +81,8 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { confProvider.setImplementationDescription("THIS IS THE DESC"); ourRestServer.setServerConformanceProvider(confProvider); - ourRestServer.setPagingProvider(new FifoMemoryPagingProvider(10)); + ourPagingProvider = myAppCtx.getBean(DatabaseBackedPagingProvider.class); + ourRestServer.setPagingProvider(ourPagingProvider); Server server = new Server(ourPort); @@ -126,6 +125,8 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { ourServer = server; } + + ourRestServer.setPagingProvider(ourPagingProvider); } protected List toIdListUnqualifiedVersionless(Bundle found) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java index 41b5587f423..89808556cad 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java @@ -184,6 +184,40 @@ public class ResourceProviderDstu2Test extends BaseResourceProviderDstu2Test { assertEquals(null, response.getLink("next")); } + + @Test + public void testPagingOverEverythingSetWithNoPagingProvider() throws InterruptedException { + ourRestServer.setPagingProvider(null); + + Patient p = new Patient(); + p.setActive(true); + String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + for (int i = 0; i < 20; i++) { + Observation o = new Observation(); + o.getSubject().setReference(pid); + o.addIdentifier().setSystem("foo").setValue(Integer.toString(i)); + myObservationDao.create(o); + } + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true); + + ca.uhn.fhir.model.dstu2.resource.Bundle response = ourClient + .operation() + .onInstance(new IdDt(pid)) + .named("everything") + .withSearchParameter(Parameters.class, "_count", new NumberParam(10)) + .returnResourceType(ca.uhn.fhir.model.dstu2.resource.Bundle.class) + .useHttpGet() + .execute(); + + assertEquals(21, response.getEntry().size()); + assertEquals(21, response.getTotalElement().getValue().intValue()); + assertEquals(null, response.getLink("next")); + + } private void checkParamMissing(String paramName) throws IOException, ClientProtocolException { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java index ebd7c5c34fd..a5914cf0cf7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java @@ -56,6 +56,7 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test { protected static String ourServerBase; private static GenericWebApplicationContext ourWebApplicationContext; private TerminologyUploaderProviderDstu3 myTerminologyUploaderProvider; + protected static DatabaseBackedPagingProvider ourPagingProvider; protected static RestHookSubscriptionDstu3Interceptor ourRestHookSubscriptionInterceptor; protected static ISearchDao mySearchEntityDao; protected static ISearchCoordinatorSvc mySearchCoordinatorSvc; @@ -95,7 +96,7 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test { confProvider.setImplementationDescription("THIS IS THE DESC"); ourRestServer.setServerConformanceProvider(confProvider); - ourRestServer.setPagingProvider(myAppCtx.getBean(DatabaseBackedPagingProvider.class)); + ourPagingProvider = myAppCtx.getBean(DatabaseBackedPagingProvider.class); Server server = new Server(ourPort); @@ -162,6 +163,8 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test { ourServer = server; } + + ourRestServer.setPagingProvider(ourPagingProvider); } protected boolean shouldLogClient() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 093cfc08356..a6dc8ddfb7f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -11,7 +11,6 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; @@ -48,7 +47,6 @@ import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; import org.hl7.fhir.dstu3.model.Questionnaire.QuestionnaireItemType; import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType; import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; -import org.hl7.fhir.instance.model.Encounter.EncounterState; import org.hl7.fhir.instance.model.api.*; import org.junit.*; import org.springframework.test.util.AopTestUtils; @@ -58,7 +56,6 @@ import org.springframework.transaction.support.TransactionCallback; import com.google.common.collect.Lists; import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.jpa.dao.SearchBuilder; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; @@ -2019,6 +2016,40 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + @Test + public void testPagingOverEverythingSetWithNoPagingProvider() throws InterruptedException { + ourRestServer.setPagingProvider(null); + + Patient p = new Patient(); + p.setActive(true); + String pid = myPatientDao.create(p).getId().toUnqualifiedVersionless().getValue(); + + for (int i = 0; i < 20; i++) { + Observation o = new Observation(); + o.getSubject().setReference(pid); + o.addIdentifier().setSystem("foo").setValue(Integer.toString(i)); + myObservationDao.create(o); + } + + mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(50); + mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(10); + mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(true); + + Bundle response = ourClient + .operation() + .onInstance(new IdType(pid)) + .named("everything") + .withSearchParameter(Parameters.class, "_count", new NumberParam(10)) + .returnResourceType(Bundle.class) + .useHttpGet() + .execute(); + + assertEquals(21, response.getEntry().size()); + assertEquals(21, response.getTotalElement().getValue().intValue()); + assertEquals(null, response.getLink("next")); + + } + @Test public void testPatchUsingJsonPatch() throws Exception { String methodName = "testPatchUsingJsonPatch"; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index c33817e9798..4410d6496d0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -66,6 +66,15 @@ JPA search now uses hibernate ScrollableResults instead of plain JPA List. This should improve performance over large search results. + + JPA servers with no paging provider configured, or with a paging provider other than + DatabaseBackedPagingProvider will load all results in a single pass and keep them + in memory. Using this setup is not a good idea unless you know for sure that you + will never have very large queries since it means that all results will be loaded into + memory, but there are valid reasons to need this and it will perform better than + paging to the database in that case. This fix also resolves a NullPointerException + when performing an $everything search. Thanks to Kamal Othman for reporting! + From dfd37c69d8bd2ea23ffcd1be7be5d0f65d57e596 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 29 Jun 2017 22:16:39 -0400 Subject: [PATCH 05/14] Add tests --- .../uhn/fhir/jpa/config/TestDstu3Config.java | 1 + .../fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java | 79 ++++++++++--------- .../FhirResourceDaoDstu3SearchNoFtTest.java | 55 ++++++++++++- src/changes/changes.xml | 2 +- 4 files changed, 99 insertions(+), 38 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java index 578edef274d..4b418e87de0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java @@ -40,6 +40,7 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 { DataSource dataSource = ProxyDataSourceBuilder .create(retVal) + .multiline() .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) .countQuery() diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java index 1f9dbd013b3..38d94f9af7f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java @@ -27,6 +27,7 @@ import org.hl7.fhir.dstu3.model.Condition; import org.hl7.fhir.dstu3.model.Device; import org.hl7.fhir.dstu3.model.DiagnosticReport; import org.hl7.fhir.dstu3.model.Encounter; +import org.hl7.fhir.dstu3.model.Group; import org.hl7.fhir.dstu3.model.Immunization; import org.hl7.fhir.dstu3.model.ImmunizationRecommendation; import org.hl7.fhir.dstu3.model.Location; @@ -77,7 +78,10 @@ import ca.uhn.fhir.jpa.dao.IFhirResourceDaoValueSet; import ca.uhn.fhir.jpa.dao.IFhirSystemDao; import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; import ca.uhn.fhir.jpa.dao.ISearchParamRegistry; -import ca.uhn.fhir.jpa.dao.data.*; +import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; +import ca.uhn.fhir.jpa.dao.data.IResourceTagDao; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; +import ca.uhn.fhir.jpa.dao.data.ITagDefinitionDao; import ca.uhn.fhir.jpa.dao.dstu2.FhirResourceDaoDstu2SearchNoFtTest; import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.entity.ResourceTable; @@ -104,24 +108,13 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { private static JpaValidationSupportChainDstu3 ourJpaValidationSupportChainDstu3; private static IFhirResourceDaoValueSet ourValueSetDao; - @Autowired - protected ITagDefinitionDao myTagDefinitionDao; - @Autowired - protected IResourceTagDao myResourceTagDao; - @Autowired - protected ISearchDao mySearchEntityDao; - @Autowired - @Qualifier("mySearchParameterDaoDstu3") - protected IFhirResourceDao mySearchParameterDao; - @Autowired - protected ISearchParamRegistry mySearchParamRegsitry; -// @Autowired + // @Autowired // protected HapiWorkerContext myHapiWorkerContext; @Autowired @Qualifier("myAllergyIntoleranceDaoDstu3") protected IFhirResourceDao myAllergyIntoleranceDao; @Autowired - protected ApplicationContext myAppCtx; + protected ApplicationContext myAppCtx; @Autowired @Qualifier("myAppointmentDaoDstu3") protected IFhirResourceDao myAppointmentDao; @@ -131,13 +124,10 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired @Qualifier("myBundleDaoDstu3") protected IFhirResourceDao myBundleDao; - @Autowired +@Autowired @Qualifier("myCarePlanDaoDstu3") protected IFhirResourceDao myCarePlanDao; @Autowired - @Qualifier("myProcedureRequestDaoDstu3") - protected IFhirResourceDao myProcedureRequestDao; - @Autowired @Qualifier("myCodeSystemDaoDstu3") protected IFhirResourceDaoCodeSystem myCodeSystemDao; @Autowired @@ -166,6 +156,9 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired protected FhirContext myFhirCtx; @Autowired + @Qualifier("myGroupDaoDstu3") + protected IFhirResourceDao myGroupDao; + @Autowired @Qualifier("myImmunizationDaoDstu3") protected IFhirResourceDao myImmunizationDao; @Autowired @@ -181,12 +174,12 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Qualifier("myMediaDaoDstu3") protected IFhirResourceDao myMediaDao; @Autowired - @Qualifier("myMedicationDaoDstu3") - protected IFhirResourceDao myMedicationDao; - @Autowired @Qualifier("myMedicationAdministrationDaoDstu3") protected IFhirResourceDao myMedicationAdministrationDao; @Autowired + @Qualifier("myMedicationDaoDstu3") + protected IFhirResourceDao myMedicationDao; + @Autowired @Qualifier("myMedicationRequestDaoDstu3") protected IFhirResourceDao myMedicationRequestDao; @Autowired @@ -202,15 +195,15 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Qualifier("myOrganizationDaoDstu3") protected IFhirResourceDao myOrganizationDao; @Autowired - @Qualifier("myTaskDaoDstu3") - protected IFhirResourceDao myTaskDao; - @Autowired @Qualifier("myPatientDaoDstu3") protected IFhirResourceDaoPatient myPatientDao; @Autowired @Qualifier("myPractitionerDaoDstu3") protected IFhirResourceDao myPractitionerDao; @Autowired + @Qualifier("myProcedureRequestDaoDstu3") + protected IFhirResourceDao myProcedureRequestDao; + @Autowired @Qualifier("myQuestionnaireDaoDstu3") protected IFhirResourceDao myQuestionnaireDao; @Autowired @@ -222,8 +215,21 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired protected IResourceTableDao myResourceTableDao; @Autowired + protected IResourceTagDao myResourceTagDao; + @Autowired + protected ISearchCoordinatorSvc mySearchCoordinatorSvc; + @Autowired protected IFulltextSearchSvc mySearchDao; @Autowired + protected ISearchDao mySearchEntityDao; + @Autowired + @Qualifier("mySearchParameterDaoDstu3") + protected IFhirResourceDao mySearchParameterDao; + @Autowired + protected ISearchParamPresenceSvc mySearchParamPresenceSvc; + @Autowired + protected ISearchParamRegistry mySearchParamRegsitry; + @Autowired protected IStaleSearchDeletingSvc myStaleSearchDeletingSvc; @Autowired @Qualifier("myStructureDefinitionDaoDstu3") @@ -241,8 +247,15 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Qualifier("mySystemProviderDstu3") protected JpaSystemProviderDstu3 mySystemProvider; @Autowired + protected ITagDefinitionDao myTagDefinitionDao; + @Autowired + @Qualifier("myTaskDaoDstu3") + protected IFhirResourceDao myTaskDao; + @Autowired protected IHapiTerminologySvc myTermSvc; @Autowired + protected PlatformTransactionManager myTransactionMgr; + @Autowired protected PlatformTransactionManager myTxManager; @Autowired @Qualifier("myJpaValidationSupportChainDstu3") @@ -250,18 +263,6 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired @Qualifier("myValueSetDaoDstu3") protected IFhirResourceDaoValueSet myValueSetDao; - @Autowired - protected PlatformTransactionManager myTransactionMgr; - @Autowired - protected ISearchParamPresenceSvc mySearchParamPresenceSvc; - @Autowired - protected ISearchCoordinatorSvc mySearchCoordinatorSvc; - - @After() - public void afterGrabCaches() { - ourValueSetDao = myValueSetDao; - ourJpaValidationSupportChainDstu3 = myJpaValidationSupportChainDstu3; - } @After() public void afterCleanupDao() { @@ -271,6 +272,12 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { myDaoConfig.setSuppressUpdatesWithNoChange(new DaoConfig().isSuppressUpdatesWithNoChange()); } + @After() + public void afterGrabCaches() { + ourValueSetDao = myValueSetDao; + ourJpaValidationSupportChainDstu3 = myJpaValidationSupportChainDstu3; + } + @Before public void beforeCreateInterceptor() { myInterceptor = mock(IServerInterceptor.class); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 109984e1f9b..ee43c8f5e0d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -43,6 +43,7 @@ import org.hl7.fhir.dstu3.model.Device; import org.hl7.fhir.dstu3.model.DiagnosticReport; import org.hl7.fhir.dstu3.model.Encounter; import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; +import org.hl7.fhir.dstu3.model.Group; import org.hl7.fhir.dstu3.model.IdType; import org.hl7.fhir.dstu3.model.Immunization; import org.hl7.fhir.dstu3.model.ImmunizationRecommendation; @@ -157,13 +158,65 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { public void testEmptyChain() { SearchParameterMap map = new SearchParameterMap(); - map.add(Encounter.SP_SUBJECT, new ReferenceAndListParam().addAnd(new ReferenceOrListParam().add((ReferenceParam)new ReferenceParam("subject", "04823543").setChain("identifier")))); + map.add(Encounter.SP_SUBJECT, new ReferenceAndListParam().addAnd(new ReferenceOrListParam().add(new ReferenceParam("subject", "04823543").setChain("identifier")))); IBundleProvider results = myMedicationAdministrationDao.search(map); List ids = toUnqualifiedIdValues(results); assertThat(ids, empty()); } + @Test + public void testChainWithMultipleTypePossibilities() { + + Patient sub1 = new Patient(); + sub1.setActive(true); + sub1.addIdentifier().setSystem("foo").setValue("bar"); + String sub1Id = myPatientDao.create(sub1).getId().toUnqualifiedVersionless().getValue(); + + Group sub2 = new Group(); + sub2.setActive(true); + sub2.addIdentifier().setSystem("foo").setValue("bar"); + String sub2Id = myGroupDao.create(sub2).getId().toUnqualifiedVersionless().getValue(); + + Encounter enc1 = new Encounter(); + enc1.getSubject().setReference(sub1Id); + String enc1Id = myEncounterDao.create(enc1).getId().toUnqualifiedVersionless().getValue(); + + Encounter enc2 = new Encounter(); + enc2.getSubject().setReference(sub2Id); + String enc2Id = myEncounterDao.create(enc2).getId().toUnqualifiedVersionless().getValue(); + + List ids; + SearchParameterMap map; + IBundleProvider results; + + map = new SearchParameterMap(); + map.add(Encounter.SP_SUBJECT, new ReferenceParam("subject", "foo|bar").setChain("identifier")); + results = myEncounterDao.search(map); + ids = toUnqualifiedVersionlessIdValues(results); + assertThat(ids, hasItems(enc1Id, enc2Id)); + + System.exit(0); + + map = new SearchParameterMap(); + map.add(Encounter.SP_SUBJECT, new ReferenceParam("subject:Patient", "foo|bar").setChain("identifier")); + results = myEncounterDao.search(map); + ids = toUnqualifiedVersionlessIdValues(results); + assertThat(ids, hasItems(enc1Id)); + + map = new SearchParameterMap(); + map.add(Encounter.SP_SUBJECT, new ReferenceParam("subject:Group", "foo|bar").setChain("identifier")); + results = myEncounterDao.search(map); + ids = toUnqualifiedVersionlessIdValues(results); + assertThat(ids, hasItems(enc2Id)); + + map = new SearchParameterMap(); + map.add(Encounter.SP_SUBJECT, new ReferenceParam("subject", "04823543").setChain("identifier")); + results = myEncounterDao.search(map); + ids = toUnqualifiedVersionlessIdValues(results); + assertThat(ids, empty()); + } + @Test public void testEverythingTimings() throws Exception { String methodName = "testEverythingTimings"; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index c33817e9798..c3e90878508 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -45,7 +45,7 @@ Add configuration to JPA server DaoConfig that allows a maximum number of search results to be specified. Queries will never return more than this number, which can be good for avoiding accidental - performance problems in situations where lare queries should not be + performance problems in situations where large queries should not be needed From b44bdeec886cf3ff736d836f2bde1195fb2f7718 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 30 Jun 2017 07:24:33 -0400 Subject: [PATCH 06/14] Clean up rest hook interceptor a bit --- .../rest/server/IRestfulServerDefaults.java | 70 +-- .../jaxrs/server/AbstractJaxRsProvider.java | 407 ++++++++++-------- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 2 +- .../BaseRestHookSubscriptionInterceptor.java | 71 +++ .../RestHookSubscriptionDstu2Interceptor.java | 114 ++--- .../RestHookSubscriptionDstu3Interceptor.java | 123 ++---- .../FhirResourceDaoDstu3SearchNoFtTest.java | 2 - .../subscription/RestHookTestDstu2Test.java | 35 +- .../subscription/RestHookTestDstu3Test.java | 16 + 9 files changed, 426 insertions(+), 414 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java index 4061ca7effb..3288d67d153 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/IRestfulServerDefaults.java @@ -26,37 +26,6 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; public interface IRestfulServerDefaults { - - /** - * Returns the list of interceptors registered against this server - */ - List getInterceptors(); - - /** - * Gets the {@link FhirContext} associated with this server. For efficient processing, resource providers and plain - * providers should generally use this context if one is needed, as opposed to - * creating their own. - */ - FhirContext getFhirContext(); - - /** - * Should the server "pretty print" responses by default (requesting clients can always override this default by - * supplying an Accept header in the request, or a _pretty - * parameter in the request URL. - *

- * The default is false - *

- * - * @return Returns the default pretty print setting - */ - boolean isDefaultPrettyPrint(); - - /** - * @return Returns the server support for ETags (will not be null). Default is - * {@link RestfulServer#DEFAULT_ETAG_SUPPORT} - */ - ETagSupportEnum getETagSupport(); - /** * @return Returns the setting for automatically adding profile tags * @deprecated As of HAPI FHIR 1.5, this property has been moved to @@ -73,15 +42,46 @@ public interface IRestfulServerDefaults { EncodingEnum getDefaultResponseEncoding(); /** - * @return If true the server will use browser friendly content-types (instead of standard FHIR ones) - * when it detects that the request is coming from a browser - * instead of a FHIR + * @return Returns the server support for ETags (will not be null). Default is + * {@link RestfulServer#DEFAULT_ETAG_SUPPORT} */ - boolean isUseBrowserFriendlyContentTypes(); + ETagSupportEnum getETagSupport(); + + /** + * Gets the {@link FhirContext} associated with this server. For efficient processing, resource providers and plain + * providers should generally use this context if one is needed, as opposed to + * creating their own. + */ + FhirContext getFhirContext(); + + /** + * Returns the list of interceptors registered against this server + */ + List getInterceptors(); /** * Returns the paging provider for this server */ IPagingProvider getPagingProvider(); + /** + * Should the server "pretty print" responses by default (requesting clients can always override this default by + * supplying an Accept header in the request, or a _pretty + * parameter in the request URL. + *

+ * The default is false + *

+ * + * @return Returns the default pretty print setting + */ + boolean isDefaultPrettyPrint(); + + /** + * @return If true the server will use browser friendly content-types (instead of standard FHIR ones) + * when it detects that the request is coming from a browser + * instead of a FHIR + */ + boolean isUseBrowserFriendlyContentTypes(); + + } diff --git a/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsProvider.java b/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsProvider.java index 368e2359324..0962b25db41 100644 --- a/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsProvider.java +++ b/hapi-fhir-jaxrsserver-base/src/main/java/ca/uhn/fhir/jaxrs/server/AbstractJaxRsProvider.java @@ -48,6 +48,7 @@ import ca.uhn.fhir.rest.server.AddProfileTagEnum; import ca.uhn.fhir.rest.server.ETagSupportEnum; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.rest.server.HardcodedServerAddressStrategy; +import ca.uhn.fhir.rest.server.IPagingProvider; import ca.uhn.fhir.rest.server.IRestfulServerDefaults; import ca.uhn.fhir.rest.server.IServerAddressStrategy; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -62,216 +63,246 @@ import ca.uhn.fhir.util.OperationOutcomeUtil; */ public abstract class AbstractJaxRsProvider implements IRestfulServerDefaults { - private final FhirContext CTX; + private static final String ERROR = "error"; - private static final String PROCESSING = "processing"; - private static final String ERROR = "error"; + private static final String PROCESSING = "processing"; - /** the uri info */ - @Context - private UriInfo theUriInfo; - /** the http headers */ - @Context - private HttpHeaders theHeaders; + private final FhirContext CTX; + /** the http headers */ + @Context + private HttpHeaders theHeaders; - @Override - public FhirContext getFhirContext() { - return CTX; - } + /** the uri info */ + @Context + private UriInfo theUriInfo; - /** - * Default is DSTU2. Use {@link AbstractJaxRsProvider#AbstractJaxRsProvider(FhirContext)} to specify a DSTU3 context. - */ - protected AbstractJaxRsProvider() { - CTX = FhirContext.forDstu2(); - } + /** + * Default is DSTU2. Use {@link AbstractJaxRsProvider#AbstractJaxRsProvider(FhirContext)} to specify a DSTU3 context. + */ + protected AbstractJaxRsProvider() { + CTX = FhirContext.forDstu2(); + } - /** - * - * @param ctx the {@link FhirContext} to support. - */ - protected AbstractJaxRsProvider(final FhirContext ctx) { - CTX = ctx; - } + /** + * + * @param ctx + * the {@link FhirContext} to support. + */ + protected AbstractJaxRsProvider(final FhirContext ctx) { + CTX = ctx; + } - /** - * This method returns the query parameters - * @return the query parameters - */ - public Map getParameters() { - final MultivaluedMap queryParameters = getUriInfo().getQueryParameters(); - final HashMap params = new HashMap(); - for (final Entry> paramEntry : queryParameters.entrySet()) { - params.put(paramEntry.getKey(), paramEntry.getValue().toArray(new String[paramEntry.getValue().size()])); - } - return params; - } + private IBaseOperationOutcome createOutcome(final DataFormatException theException) { + final IBaseOperationOutcome oo = OperationOutcomeUtil.newInstance(getFhirContext()); + final String detailsValue = theException.getMessage() + "\n\n" + ExceptionUtils.getStackTrace(theException); + OperationOutcomeUtil.addIssue(getFhirContext(), oo, ERROR, detailsValue, null, PROCESSING); + return oo; + } - /** - * This method returns the default server address strategy. The default strategy return the - * base uri for the request {@link AbstractJaxRsProvider#getBaseForRequest() getBaseForRequest()} - * @return - */ - public IServerAddressStrategy getServerAddressStrategy() { - final HardcodedServerAddressStrategy addressStrategy = new HardcodedServerAddressStrategy(); - addressStrategy.setValue(getBaseForRequest()); - return addressStrategy; - } + /** + * DEFAULT = AddProfileTagEnum.NEVER + */ + @Override + public AddProfileTagEnum getAddProfileTag() { + return AddProfileTagEnum.NEVER; + } - /** - * This method returns the server base, independent of the request or resource. - * @see javax.ws.rs.core.UriInfo#getBaseUri() - * @return the ascii string for the server base - */ - public String getBaseForServer() { - final String url = getUriInfo().getBaseUri().toASCIIString(); - return StringUtils.isNotBlank(url) && url.endsWith("/") ? url.substring(0, url.length() - 1) : url; - } + /** + * This method returns the server base, including the resource path. + * {@link javax.ws.rs.core.UriInfo#getBaseUri() UriInfo#getBaseUri()} + * + * @return the ascii string for the base resource provider path + */ + public String getBaseForRequest() { + return getBaseForServer(); + } - /** - * This method returns the server base, including the resource path. - * {@link javax.ws.rs.core.UriInfo#getBaseUri() UriInfo#getBaseUri()} - * @return the ascii string for the base resource provider path - */ - public String getBaseForRequest() { - return getBaseForServer(); - } + /** + * This method returns the server base, independent of the request or resource. + * + * @see javax.ws.rs.core.UriInfo#getBaseUri() + * @return the ascii string for the server base + */ + public String getBaseForServer() { + final String url = getUriInfo().getBaseUri().toASCIIString(); + return StringUtils.isNotBlank(url) && url.endsWith("/") ? url.substring(0, url.length() - 1) : url; + } - /** - * Default: an empty list of interceptors (Interceptors are not yet supported - * in the JAX-RS server). Please get in touch if you'd like to help! - * - * @see ca.uhn.fhir.rest.server.IRestfulServer#getInterceptors() - */ - @Override - public List getInterceptors() { - return Collections.emptyList(); - } + /** + * DEFAULT = EncodingEnum.JSON + */ + @Override + public EncodingEnum getDefaultResponseEncoding() { + return EncodingEnum.JSON; + } - /** - * Get the uriInfo - * @return the uri info - */ - public UriInfo getUriInfo() { - return this.theUriInfo; - } + /** + * DEFAULT = ETagSupportEnum.DISABLED + */ + @Override + public ETagSupportEnum getETagSupport() { + return ETagSupportEnum.DISABLED; + } - /** - * Set the Uri Info - * @param uriInfo the uri info - */ - public void setUriInfo(final UriInfo uriInfo) { - this.theUriInfo = uriInfo; - } + @Override + public FhirContext getFhirContext() { + return CTX; + } - /** - * Get the headers - * @return the headers - */ - public HttpHeaders getHeaders() { - return this.theHeaders; - } + /** + * Get the headers + * + * @return the headers + */ + public HttpHeaders getHeaders() { + return this.theHeaders; + } - /** - * Set the headers - * @param headers the headers to set - */ - public void setHeaders(final HttpHeaders headers) { - this.theHeaders = headers; - } + /** + * Default: an empty list of interceptors (Interceptors are not yet supported + * in the JAX-RS server). Please get in touch if you'd like to help! + * + * @see ca.uhn.fhir.rest.server.IRestfulServer#getInterceptors() + */ + @Override + public List getInterceptors() { + return Collections.emptyList(); + } - /** - * Return the requestbuilder for the server - * @param requestType the type of the request - * @param restOperation the rest operation type - * @param theResourceName the resource name - * @return the requestbuilder - */ - public Builder getRequest(final RequestTypeEnum requestType, final RestOperationTypeEnum restOperation, final String theResourceName) { - return new JaxRsRequest.Builder(this, requestType, restOperation, theUriInfo.getRequestUri().toString(), theResourceName); - } + /** + * By default, no paging provider is used + */ + @Override + public IPagingProvider getPagingProvider() { + return null; + } - /** - * Return the requestbuilder for the server - * @param requestType the type of the request - * @param restOperation the rest operation type - * @return the requestbuilder - */ - public Builder getRequest(final RequestTypeEnum requestType, final RestOperationTypeEnum restOperation) { - return getRequest(requestType, restOperation, null); - } + /** + * This method returns the query parameters + * + * @return the query parameters + */ + public Map getParameters() { + final MultivaluedMap queryParameters = getUriInfo().getQueryParameters(); + final HashMap params = new HashMap(); + for (final Entry> paramEntry : queryParameters.entrySet()) { + params.put(paramEntry.getKey(), paramEntry.getValue().toArray(new String[paramEntry.getValue().size()])); + } + return params; + } - /** - * DEFAULT = EncodingEnum.JSON - */ - @Override - public EncodingEnum getDefaultResponseEncoding() { - return EncodingEnum.JSON; - } + /** + * Return the requestbuilder for the server + * + * @param requestType + * the type of the request + * @param restOperation + * the rest operation type + * @return the requestbuilder + */ + public Builder getRequest(final RequestTypeEnum requestType, final RestOperationTypeEnum restOperation) { + return getRequest(requestType, restOperation, null); + } - /** - * DEFAULT = true - */ - @Override - public boolean isDefaultPrettyPrint() { - return true; - } + /** + * Return the requestbuilder for the server + * + * @param requestType + * the type of the request + * @param restOperation + * the rest operation type + * @param theResourceName + * the resource name + * @return the requestbuilder + */ + public Builder getRequest(final RequestTypeEnum requestType, final RestOperationTypeEnum restOperation, final String theResourceName) { + return new JaxRsRequest.Builder(this, requestType, restOperation, theUriInfo.getRequestUri().toString(), theResourceName); + } - /** - * DEFAULT = ETagSupportEnum.DISABLED - */ - @Override - public ETagSupportEnum getETagSupport() { - return ETagSupportEnum.DISABLED; - } + /** + * This method returns the default server address strategy. The default strategy return the + * base uri for the request {@link AbstractJaxRsProvider#getBaseForRequest() getBaseForRequest()} + * + * @return + */ + public IServerAddressStrategy getServerAddressStrategy() { + final HardcodedServerAddressStrategy addressStrategy = new HardcodedServerAddressStrategy(); + addressStrategy.setValue(getBaseForRequest()); + return addressStrategy; + } - /** - * DEFAULT = AddProfileTagEnum.NEVER - */ - @Override - public AddProfileTagEnum getAddProfileTag() { - return AddProfileTagEnum.NEVER; - } + /** + * Get the uriInfo + * + * @return the uri info + */ + public UriInfo getUriInfo() { + return this.theUriInfo; + } - /** - * DEFAULT = false - */ - @Override - public boolean isUseBrowserFriendlyContentTypes() { - return true; - } + /** + * Convert an exception to a response + * + * @param theRequest + * the incoming request + * @param theException + * the exception to convert + * @return response + * @throws IOException + */ + public Response handleException(final JaxRsRequest theRequest, final Throwable theException) + throws IOException { + if (theException instanceof JaxRsResponseException) { + return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, (JaxRsResponseException) theException); + } else if (theException instanceof DataFormatException) { + return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, new JaxRsResponseException( + new InvalidRequestException(theException.getMessage(), createOutcome((DataFormatException) theException)))); + } else { + return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, + new JaxRsExceptionInterceptor().convertException(this, theException)); + } + } - /** - * DEFAULT = false - */ - public boolean withStackTrace() { - return false; - } + /** + * DEFAULT = true + */ + @Override + public boolean isDefaultPrettyPrint() { + return true; + } - /** - * Convert an exception to a response - * @param theRequest the incoming request - * @param theException the exception to convert - * @return response - * @throws IOException - */ - public Response handleException(final JaxRsRequest theRequest, final Throwable theException) - throws IOException { - if (theException instanceof JaxRsResponseException) { - return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, (JaxRsResponseException) theException); - } else if (theException instanceof DataFormatException) { - return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, new JaxRsResponseException( - new InvalidRequestException(theException.getMessage(), createOutcome((DataFormatException) theException)))); - } else { - return new JaxRsExceptionInterceptor().convertExceptionIntoResponse(theRequest, - new JaxRsExceptionInterceptor().convertException(this, theException)); - } - } + /** + * DEFAULT = false + */ + @Override + public boolean isUseBrowserFriendlyContentTypes() { + return true; + } - private IBaseOperationOutcome createOutcome(final DataFormatException theException) { - final IBaseOperationOutcome oo = OperationOutcomeUtil.newInstance(getFhirContext()); - final String detailsValue = theException.getMessage() + "\n\n" + ExceptionUtils.getStackTrace(theException); - OperationOutcomeUtil.addIssue(getFhirContext(), oo, ERROR, detailsValue, null, PROCESSING); - return oo; - } + /** + * Set the headers + * + * @param headers + * the headers to set + */ + public void setHeaders(final HttpHeaders headers) { + this.theHeaders = headers; + } + + /** + * Set the Uri Info + * + * @param uriInfo + * the uri info + */ + public void setUriInfo(final UriInfo uriInfo) { + this.theUriInfo = uriInfo; + } + + /** + * DEFAULT = false + */ + public boolean withStackTrace() { + return false; + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 26ffe8fa261..42033bd85cf 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -564,7 +564,7 @@ public abstract class BaseHapiFhirResourceDao extends B } protected boolean isPagingProviderDatabaseBacked(RequestDetails theRequestDetails) { - if (theRequestDetails == null) { + if (theRequestDetails == null || theRequestDetails.getServer() == null) { return false; } if (theRequestDetails.getServer().getPagingProvider() instanceof DatabaseBackedPagingProvider) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java new file mode 100644 index 00000000000..b67564aca1e --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java @@ -0,0 +1,71 @@ +package ca.uhn.fhir.jpa.interceptor; + +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; +import ca.uhn.fhir.jpa.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.dao.SearchParameterMap; +import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails; +import ca.uhn.fhir.rest.method.RequestDetails; +import ca.uhn.fhir.rest.server.IBundleProvider; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.interceptor.ServerOperationInterceptorAdapter; + +public abstract class BaseRestHookSubscriptionInterceptor extends ServerOperationInterceptorAdapter { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseRestHookSubscriptionInterceptor.class); + + protected static final Integer MAX_SUBSCRIPTION_RESULTS = 10000; + + protected abstract IFhirResourceDao getSubscriptionDao(); + + protected void checkSubscriptionCriterias(String theCriteria) { + try { + IBundleProvider results = executeSubscriptionCriteria(theCriteria, null); + } catch (Exception e) { + ourLog.warn("Invalid criteria when creating subscription", e); + throw new InvalidRequestException("Invalid criteria: " + e.getMessage()); + } + } + + private IBundleProvider executeSubscriptionCriteria(String theCriteria, IIdType idType) { + String criteria = theCriteria; + + /* + * Run the subscriptions query and look for matches, add the id as part of the criteria + * to avoid getting matches of previous resources rather than the recent resource + */ + if (idType != null) { + criteria += "&_id=" + idType.getResourceType() + "/" + idType.getIdPart(); + } + + IBundleProvider results = getBundleProvider(criteria, true); + return results; + } + + /** + * Search based on a query criteria + * + * @param theCheckOnly Is this just a test that the search works + */ + protected IBundleProvider getBundleProvider(String theCriteria, boolean theCheckOnly) { + RuntimeResourceDefinition responseResourceDef = getSubscriptionDao().validateCriteriaAndReturnResourceDefinition(theCriteria); + SearchParameterMap responseCriteriaUrl = BaseHapiFhirDao.translateMatchUrl(getSubscriptionDao(), getSubscriptionDao().getContext(), theCriteria, responseResourceDef); + + RequestDetails req = new ServletSubRequestDetails(); + req.setSubRequest(true); + + IFhirResourceDao responseDao = getSubscriptionDao().getDao(responseResourceDef.getImplementingClass()); + + if (theCheckOnly) { + responseCriteriaUrl.setLoadSynchronousUpTo(1); + } else { + responseCriteriaUrl.setLoadSynchronousUpTo(MAX_SUBSCRIPTION_RESULTS); + } + + IBundleProvider responseResults = responseDao.search(responseCriteriaUrl, req); + return responseResults; + } + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu2Interceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu2Interceptor.java index cf33f7d0a11..c74fb12602f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu2Interceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu2Interceptor.java @@ -29,12 +29,9 @@ import javax.annotation.PostConstruct; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import org.apache.http.NameValuePair; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -45,8 +42,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.SearchParameterMap; import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails; @@ -62,11 +57,9 @@ import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.rest.server.IBundleProvider; -import ca.uhn.fhir.rest.server.interceptor.*; -public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterceptorAdapter { +public class RestHookSubscriptionDstu2Interceptor extends BaseRestHookSubscriptionInterceptor { - private static final Integer MAX_SUBSCRIPTION_RESULTS = 10000; private static volatile ExecutorService executor; private final static int MAX_THREADS = 1; @@ -75,13 +68,13 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce @Autowired private FhirContext myFhirContext; + private boolean myNotifyOnDelete = false; + + private final List myRestHookSubscriptions = new ArrayList(); @Autowired @Qualifier("mySubscriptionDaoDstu2") private IFhirResourceDao mySubscriptionDao; - private boolean myNotifyOnDelete = false; - private final List myRestHookSubscriptions = new ArrayList(); - /** * Check subscriptions and send notifications or payload * @@ -101,7 +94,7 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce } if (resourceType != null && subscription.getCriteria() != null && !criteriaResource.equals(resourceType)) { - ourLog.info("Skipping subscription search for {} because it does not match the criteria {}", resourceType , subscription.getCriteria()); + ourLog.info("Skipping subscription search for {} because it does not match the criteria {}", resourceType, subscription.getCriteria()); continue; } @@ -110,7 +103,7 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce criteria += "&_id=" + idType.getResourceType() + "/" + idType.getIdPart(); criteria = massageCriteria(criteria); - IBundleProvider results = getBundleProvider(criteria); + IBundleProvider results = getBundleProvider(criteria, false); if (results.size() == 0) { continue; @@ -195,25 +188,6 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce return request; } - /** - * Search based on a query criteria - * - * @param criteria - * @return - */ - private IBundleProvider getBundleProvider(String criteria) { - RuntimeResourceDefinition responseResourceDef = mySubscriptionDao.validateCriteriaAndReturnResourceDefinition(criteria); - SearchParameterMap responseCriteriaUrl = BaseHapiFhirDao.translateMatchUrl(mySubscriptionDao, mySubscriptionDao.getContext(), criteria, responseResourceDef); - - RequestDetails req = new ServletSubRequestDetails(); - req.setSubRequest(true); - - IFhirResourceDao responseDao = mySubscriptionDao.getDao(responseResourceDef.getImplementingClass()); - responseCriteriaUrl.setCount(MAX_SUBSCRIPTION_RESULTS); - IBundleProvider responseResults = responseDao.search(responseCriteriaUrl, req); - return responseResults; - } - /** * Get subscription from cache * @@ -259,6 +233,27 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce return entity; } + @Override + protected IFhirResourceDao getSubscriptionDao() { + return mySubscriptionDao; + } + + @Override + public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theDetails) { + // check the subscription criteria to see if its valid before creating or updating a subscription + if (RestOperationTypeEnum.CREATE.equals(theOperation) || RestOperationTypeEnum.UPDATE.equals(theOperation)) { + String resourceType = theDetails.getResourceType(); + ourLog.info("prehandled resource type: " + resourceType); + if (resourceType != null && resourceType.equals(Subscription.class.getSimpleName())) { + Subscription subscription = (Subscription) theDetails.getResource(); + if (subscription != null) { + checkSubscriptionCriterias(subscription.getCriteria()); + } + } + } + super.incomingRequestPreHandled(theOperation, theDetails); + } + /** * Read the existing subscriptions from the database */ @@ -273,7 +268,7 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce map.setCount(MAX_SUBSCRIPTION_RESULTS); IBundleProvider subscriptionBundleList = mySubscriptionDao.search(map, req); if (subscriptionBundleList.size() >= MAX_SUBSCRIPTION_RESULTS) { - ourLog.error("Currently over "+MAX_SUBSCRIPTION_RESULTS+" subscriptions. Some subscriptions have not been loaded."); + ourLog.error("Currently over " + MAX_SUBSCRIPTION_RESULTS + " subscriptions. Some subscriptions have not been loaded."); } List resourceList = subscriptionBundleList.getResources(0, subscriptionBundleList.size()); @@ -409,57 +404,4 @@ public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterce mySubscriptionDao = theSubscriptionDao; } - @Override - public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theDetails) { - //check the subscription criteria to see if its valid before creating or updating a subscription - if (RestOperationTypeEnum.CREATE.equals(theOperation) || RestOperationTypeEnum.UPDATE.equals(theOperation)) { - String resourceType = theDetails.getResourceType(); - ourLog.info("prehandled resource type: " + resourceType); - if (resourceType != null && resourceType.equals(Subscription.class.getSimpleName())) { - Subscription subscription = (Subscription) theDetails.getResource(); - if (subscription != null) { - checkSubscriptionCriterias(subscription); - } - } - } - super.incomingRequestPreHandled(theOperation, theDetails); - } - - private void checkSubscriptionCriterias(Subscription subscription){ - try { - IBundleProvider results = executeSubscriptionCriteria(subscription, null); - }catch (Exception e){ - throw new InvalidRequestException("Invalid criteria"); - } - } - - private IBundleProvider executeSubscriptionCriteria(Subscription subscription, IIdType idType){ - //run the subscriptions query and look for matches, add the id as part of the criteria to avoid getting matches of previous resources rather than the recent resource - String criteria = subscription.getCriteria(); - if(idType != null) { - criteria += "&_id=" + idType.getResourceType() + "/" + idType.getIdPart(); - } - - IBundleProvider results = getBundleProvider(criteria); - return results; - } - - /** - * Get the encoding from the criteria or return JSON encoding if its not found - * - * @param criteria - * @return - */ - private EncodingEnum getEncoding(String criteria) { - //check criteria - String params = criteria.substring(criteria.indexOf('?') + 1); - List paramValues = URLEncodedUtils.parse(params, Constants.CHARSET_UTF8, '&'); - for (NameValuePair nameValuePair : paramValues) { - if (Constants.PARAM_FORMAT.equals(nameValuePair.getName())) { - return EncodingEnum.forContentType(nameValuePair.getValue()); - } - } - return EncodingEnum.JSON; - } - } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu3Interceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu3Interceptor.java index 757411f4125..0eda2bd28a2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu3Interceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/RestHookSubscriptionDstu3Interceptor.java @@ -30,12 +30,9 @@ import javax.annotation.PostConstruct; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import org.apache.http.NameValuePair; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.hl7.fhir.dstu3.model.Subscription; @@ -48,8 +45,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.SearchParameterMap; import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails; @@ -60,32 +55,31 @@ import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.rest.server.IBundleProvider; -import ca.uhn.fhir.rest.server.interceptor.*; -public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterceptorAdapter { +public class RestHookSubscriptionDstu3Interceptor extends BaseRestHookSubscriptionInterceptor { - private static final Integer MAX_SUBSCRIPTION_RESULTS = 10000; private static volatile ExecutorService executor; - private final static int MAX_THREADS = 1; + private final static int MAX_THREADS = 1; private static final Logger ourLog = LoggerFactory.getLogger(RestHookSubscriptionDstu3Interceptor.class); @Autowired private FhirContext myFhirContext; + private final List myRestHookSubscriptions = new ArrayList(); + @Autowired @Qualifier("mySubscriptionDaoDstu3") private IFhirResourceDao mySubscriptionDao; - + private boolean notifyOnDelete = false; - private final List myRestHookSubscriptions = new ArrayList(); /** * Check subscriptions and send notifications or payload * * @param idType * @param resourceType - * @param theOperation + * @param theOperation */ private void checkSubscriptions(IIdType idType, String resourceType, RestOperationTypeEnum theOperation) { for (Subscription subscription : myRestHookSubscriptions) { @@ -99,7 +93,7 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce } if (resourceType != null && subscription.getCriteria() != null && !criteriaResource.equals(resourceType)) { - ourLog.info("Skipping subscription search for {} because it does not match the criteria {}", resourceType , subscription.getCriteria()); + ourLog.info("Skipping subscription search for {} because it does not match the criteria {}", resourceType, subscription.getCriteria()); continue; } @@ -108,7 +102,7 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce criteria += "&_id=" + idType.getResourceType() + "/" + idType.getIdPart(); criteria = massageCriteria(criteria); - IBundleProvider results = getBundleProvider(criteria); + IBundleProvider results = getBundleProvider(criteria, false); if (results.size() == 0) { continue; @@ -134,13 +128,13 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce while (url.endsWith("/")) { url = url.substring(0, url.length() - 1); } - + HttpUriRequest request = null; - String resourceName = myFhirContext.getResourceDefinition(theResource).getName(); + String resourceName = myFhirContext.getResourceDefinition(theResource).getName(); String payload = theSubscription.getChannel().getPayload(); String resourceId = theResource.getIdElement().getIdPart(); - + // HTTP put if (theOperation == RestOperationTypeEnum.UPDATE && EncodingEnum.XML.equals(EncodingEnum.forContentType(payload))) { ourLog.info("XML payload found"); @@ -186,25 +180,6 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce return request; } - /** - * Search based on a query criteria - * - * @param criteria - * @return - */ - private IBundleProvider getBundleProvider(String criteria) { - RuntimeResourceDefinition responseResourceDef = mySubscriptionDao.validateCriteriaAndReturnResourceDefinition(criteria); - SearchParameterMap responseCriteriaUrl = BaseHapiFhirDao.translateMatchUrl(mySubscriptionDao, mySubscriptionDao.getContext(), criteria, responseResourceDef); - - RequestDetails req = new ServletSubRequestDetails(); - req.setSubRequest(true); - - IFhirResourceDao responseDao = mySubscriptionDao.getDao(responseResourceDef.getImplementingClass()); - responseCriteriaUrl.setCount(MAX_SUBSCRIPTION_RESULTS); - IBundleProvider responseResults = responseDao.search(responseCriteriaUrl, req); - return responseResults; - } - /** * Get subscription from cache * @@ -250,6 +225,27 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce return entity; } + @Override + protected IFhirResourceDao getSubscriptionDao() { + return mySubscriptionDao; + } + + @Override + public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theDetails) { + // check the subscription criteria to see if its valid before creating or updating a subscription + if (RestOperationTypeEnum.CREATE.equals(theOperation) || RestOperationTypeEnum.UPDATE.equals(theOperation)) { + String resourceType = theDetails.getResourceType(); + ourLog.info("prehandled resource type: " + resourceType); + if (resourceType != null && resourceType.equals(Subscription.class.getSimpleName())) { + Subscription subscription = (Subscription) theDetails.getResource(); + if (subscription != null) { + checkSubscriptionCriterias(subscription.getCriteria()); + } + } + } + super.incomingRequestPreHandled(theOperation, theDetails); + } + /** * Read the existing subscriptions from the database */ @@ -264,7 +260,7 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce map.setCount(MAX_SUBSCRIPTION_RESULTS); IBundleProvider subscriptionBundleList = mySubscriptionDao.search(map, req); if (subscriptionBundleList.size() >= MAX_SUBSCRIPTION_RESULTS) { - ourLog.error("Currently over "+MAX_SUBSCRIPTION_RESULTS+" subscriptions. Some subscriptions have not been loaded."); + ourLog.error("Currently over " + MAX_SUBSCRIPTION_RESULTS + " subscriptions. Some subscriptions have not been loaded."); } List resourceList = subscriptionBundleList.getResources(0, subscriptionBundleList.size()); @@ -400,57 +396,4 @@ public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterce mySubscriptionDao = theSubscriptionDao; } - @Override - public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theDetails) { - //check the subscription criteria to see if its valid before creating or updating a subscription - if (RestOperationTypeEnum.CREATE.equals(theOperation) || RestOperationTypeEnum.UPDATE.equals(theOperation)) { - String resourceType = theDetails.getResourceType(); - ourLog.info("prehandled resource type: " + resourceType); - if (resourceType != null && resourceType.equals(Subscription.class.getSimpleName())) { - Subscription subscription = (Subscription) theDetails.getResource(); - if (subscription != null) { - checkSubscriptionCriterias(subscription); - } - } - } - super.incomingRequestPreHandled(theOperation, theDetails); - } - - private void checkSubscriptionCriterias(Subscription subscription){ - try { - IBundleProvider results = executeSubscriptionCriteria(subscription, null); - }catch (Exception e){ - throw new InvalidRequestException("Invalid criteria"); - } - } - - private IBundleProvider executeSubscriptionCriteria(Subscription subscription, IIdType idType){ - //run the subscriptions query and look for matches, add the id as part of the criteria to avoid getting matches of previous resources rather than the recent resource - String criteria = subscription.getCriteria(); - if(idType != null) { - criteria += "&_id=" + idType.getResourceType() + "/" + idType.getIdPart(); - } - - IBundleProvider results = getBundleProvider(criteria); - return results; - } - - /** - * Get the encoding from the criteria or return JSON encoding if its not found - * - * @param criteria - * @return - */ - private EncodingEnum getEncoding(String criteria) { - //check criteria - String params = criteria.substring(criteria.indexOf('?') + 1); - List paramValues = URLEncodedUtils.parse(params, Constants.CHARSET_UTF8, '&'); - for (NameValuePair nameValuePair : paramValues) { - if (Constants.PARAM_FORMAT.equals(nameValuePair.getName())) { - return EncodingEnum.forContentType(nameValuePair.getValue()); - } - } - return EncodingEnum.JSON; - } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index ee43c8f5e0d..ba1619239d1 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -196,8 +196,6 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { ids = toUnqualifiedVersionlessIdValues(results); assertThat(ids, hasItems(enc1Id, enc2Id)); - System.exit(0); - map = new SearchParameterMap(); map.add(Encounter.SP_SUBJECT, new ReferenceParam("subject:Patient", "foo|bar").setChain("identifier")); results = myEncounterDao.search(map); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu2Test.java index 3d7b5b18bac..b8a5fc32084 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu2Test.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.subscription; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; import java.util.List; @@ -36,6 +36,7 @@ import ca.uhn.fhir.rest.annotation.Update; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.PortUtil; /** @@ -58,13 +59,12 @@ public class RestHookTestDstu2Test extends BaseResourceProviderDstu2Test { ourClient.delete().resourceConditionalByUrl("Subscription?status=active").execute(); ourLog.info("Done deleting all subscriptions"); myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); - + ourRestServer.unregisterInterceptor(ourRestHookSubscriptionInterceptor); } @Before public void beforeRegisterRestHookListener() { -// ourRestHookSubscriptionInterceptor.set ourRestServer.registerInterceptor(ourRestHookSubscriptionInterceptor); } @@ -127,22 +127,21 @@ public class RestHookTestDstu2Test extends BaseResourceProviderDstu2Test { Thread.sleep(500); assertEquals(1, ourCreatedObservations.size()); assertEquals(0, ourUpdatedObservations.size()); - + Subscription subscriptionTemp = ourClient.read(Subscription.class, subscription2.getId()); Assert.assertNotNull(subscriptionTemp); subscriptionTemp.setCriteria(criteria1); ourClient.update().resource(subscriptionTemp).withId(subscriptionTemp.getIdElement()).execute(); - Observation observation2 = sendObservation(code, "SNOMED-CT"); // Should see two subscription notifications Thread.sleep(500); assertEquals(3, ourCreatedObservations.size()); assertEquals(0, ourUpdatedObservations.size()); - - ourClient.delete().resourceById(new IdDt("Subscription/"+ subscription2.getId())).execute(); + + ourClient.delete().resourceById(new IdDt("Subscription/" + subscription2.getId())).execute(); Observation observationTemp3 = sendObservation(code, "SNOMED-CT"); @@ -183,6 +182,20 @@ public class RestHookTestDstu2Test extends BaseResourceProviderDstu2Test { Assert.assertFalse(observation2.getId().isEmpty()); } + @Test + public void testRestHookSubscriptionInvalidCriteria() throws Exception { + String payload = "application/xml"; + + String criteria1 = "Observation?codeeeee=SNOMED-CT"; + + try { + createSubscription(criteria1, payload, ourListenerServerBase); + fail(); + } catch (InvalidRequestException e) { + assertEquals("HTTP 400 Bad Request: Invalid criteria: Failed to parse match URL[Observation?codeeeee=SNOMED-CT] - Resource type Observation does not have a parameter with name: codeeeee", e.getMessage()); + } + } + @Test public void testRestHookSubscriptionXml() throws Exception { String payload = "application/xml"; @@ -200,22 +213,21 @@ public class RestHookTestDstu2Test extends BaseResourceProviderDstu2Test { Thread.sleep(500); assertEquals(1, ourCreatedObservations.size()); assertEquals(0, ourUpdatedObservations.size()); - + Subscription subscriptionTemp = ourClient.read(Subscription.class, subscription2.getId()); Assert.assertNotNull(subscriptionTemp); subscriptionTemp.setCriteria(criteria1); ourClient.update().resource(subscriptionTemp).withId(subscriptionTemp.getIdElement()).execute(); - Observation observation2 = sendObservation(code, "SNOMED-CT"); // Should see two subscription notifications Thread.sleep(500); assertEquals(3, ourCreatedObservations.size()); assertEquals(0, ourUpdatedObservations.size()); - - ourClient.delete().resourceById(new IdDt("Subscription/"+ subscription2.getId())).execute(); + + ourClient.delete().resourceById(new IdDt("Subscription/" + subscription2.getId())).execute(); Observation observationTemp3 = sendObservation(code, "SNOMED-CT"); @@ -256,7 +268,6 @@ public class RestHookTestDstu2Test extends BaseResourceProviderDstu2Test { Assert.assertFalse(observation2.getId().isEmpty()); } - @BeforeClass public static void startListenerServer() throws Exception { ourListenerPort = PortUtil.findFreePort(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu3Test.java index d78a716f4d6..b53003fe470 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestDstu3Test.java @@ -27,6 +27,7 @@ import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; /** * Test the rest-hook subscriptions @@ -65,6 +66,21 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test { ourUpdatedObservations.clear(); ourContentTypes.clear(); } + + @Test + public void testRestHookSubscriptionInvalidCriteria() throws Exception { + String payload = "application/xml"; + + String criteria1 = "Observation?codeeeee=SNOMED-CT"; + + try { + createSubscription(criteria1, payload, ourListenerServerBase); + fail(); + } catch (InvalidRequestException e) { + assertEquals("HTTP 400 Bad Request: Invalid criteria: Failed to parse match URL[Observation?codeeeee=SNOMED-CT] - Resource type Observation does not have a parameter with name: codeeeee", e.getMessage()); + } + } + private Subscription createSubscription(String theCriteria, String thePayload, String theEndpoint) { Subscription subscription = new Subscription(); From d626c58067ff487ba8ecd31557a56175ca72fc2f Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 30 Jun 2017 08:42:11 -0400 Subject: [PATCH 07/14] Dont fail on cleanup --- .../java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java | 4 ++-- .../fhir/jpa/search/StaleSearchDeletingSvcImpl.java | 13 +++++-------- .../provider/dstu3/ResourceProviderDstu3Test.java | 1 + src/changes/changes.xml | 5 +++++ 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java index 61f071f748f..3d334deeda4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java @@ -35,8 +35,8 @@ public interface ISearchDao extends JpaRepository { @Query("SELECT s FROM Search s WHERE s.myUuid = :uuid") public Search findByUuid(@Param("uuid") String theUuid); - @Query("SELECT s FROM Search s WHERE s.mySearchLastReturned < :cutoff") - public Collection findWhereLastReturnedBefore(@Param("cutoff") Date theCutoff); + @Query("SELECT s.myId FROM Search s WHERE s.mySearchLastReturned < :cutoff") + public Collection findWhereLastReturnedBefore(@Param("cutoff") Date theCutoff); // @Query("SELECT s FROM Search s WHERE s.myCreated < :cutoff") // public Collection findWhereCreatedBefore(@Param("cutoff") Date theCutoff); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java index 726ac86c5f6..dff4f46fa67 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java @@ -71,13 +71,13 @@ public class StaleSearchDeletingSvcImpl implements IStaleSearchDeletingSvc { @Autowired private PlatformTransactionManager myTransactionManager; - protected void deleteSearch(final Search next) { + protected void deleteSearch(final Long theSearchPid) { TransactionTemplate tt = new TransactionTemplate(myTransactionManager); tt.execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search searchToDelete = mySearchDao.findOne(next.getId()); - ourLog.info("Expiring stale search {} / {}", searchToDelete.getId(), searchToDelete.getUuid()); + Search searchToDelete = mySearchDao.findOne(theSearchPid); + ourLog.info("Deleting search {}/{} - Created[{}] -- Last returned[{}]", searchToDelete.getId(), searchToDelete.getUuid(), searchToDelete.getCreated(), searchToDelete.getSearchLastReturned()); mySearchIncludeDao.deleteForSearch(searchToDelete.getId()); mySearchResultDao.deleteForSearch(searchToDelete.getId()); mySearchDao.delete(searchToDelete); @@ -97,13 +97,10 @@ public class StaleSearchDeletingSvcImpl implements IStaleSearchDeletingSvc { ourLog.debug("Searching for searches which are before {}", cutoff); - Collection toDelete = mySearchDao.findWhereLastReturnedBefore(cutoff); + Collection toDelete = mySearchDao.findWhereLastReturnedBefore(cutoff); if (!toDelete.isEmpty()) { - for (final Search next : toDelete) { - - ourLog.info("Deleting search {} - Created[{}] -- Last returned[{}]", next.getUuid(), next.getCreated(), next.getSearchLastReturned()); - + for (final Long next : toDelete) { deleteSearch(next); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index a6dc8ddfb7f..0b855d95a7e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -3320,6 +3320,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { List ids = toUnqualifiedVersionlessIdValues(bundle); assertThat(ids, contains(id1.getValue())); + assertThat(ids, not(contains(id2.getValue()))); } finally { IOUtils.closeQuietly(resp); } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 04c0dc524d2..ee423069a96 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -75,6 +75,11 @@ paging to the database in that case. This fix also resolves a NullPointerException when performing an $everything search. Thanks to Kamal Othman for reporting! + + Correct an issue in JPA server on Postgres where searches with a long search URL + were not able to be automatically purged from the database after they were scheduled + for deletion. Thanks to Ravi Kuchi for reporting! + From 28a5b92fe2717f62ad96701303da0da34b5f0283 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 30 Jun 2017 09:58:32 -0400 Subject: [PATCH 08/14] Enforce a hard limit on meta size --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 153 +++++------------- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 4 +- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 40 ++++- .../fhir/jpa/entity/ResourceHistoryTag.java | 16 +- .../ca/uhn/fhir/jpa/entity/ResourceTag.java | 16 +- .../dstu3/FhirResourceDaoDstu3UpdateTest.java | 136 +++++++++++++++- src/changes/changes.xml | 5 + 7 files changed, 225 insertions(+), 145 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 7dacbe72832..191fcf87046 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -26,52 +26,21 @@ import static org.apache.commons.lang3.StringUtils.trim; import java.io.UnsupportedEncodingException; import java.text.Normalizer; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Set; -import java.util.UUID; -import javax.persistence.EntityManager; -import javax.persistence.NoResultException; -import javax.persistence.PersistenceContext; -import javax.persistence.PersistenceContextType; -import javax.persistence.Tuple; -import javax.persistence.TypedQuery; -import javax.persistence.criteria.CriteriaBuilder; -import javax.persistence.criteria.CriteriaQuery; -import javax.persistence.criteria.Predicate; -import javax.persistence.criteria.Root; +import javax.persistence.*; +import javax.persistence.criteria.*; import javax.xml.stream.events.Characters; import javax.xml.stream.events.XMLEvent; -import org.apache.commons.lang3.NotImplementedException; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.*; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; import org.hl7.fhir.dstu3.model.Bundle.HTTPVerb; import org.hl7.fhir.dstu3.model.IdType; import org.hl7.fhir.dstu3.model.StringType; -import org.hl7.fhir.instance.model.api.IAnyResource; -import org.hl7.fhir.instance.model.api.IBase; -import org.hl7.fhir.instance.model.api.IBaseBundle; -import org.hl7.fhir.instance.model.api.IBaseCoding; -import org.hl7.fhir.instance.model.api.IBaseExtension; -import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; -import org.hl7.fhir.instance.model.api.IBaseReference; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IDomainResource; -import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.instance.model.api.IPrimitiveType; +import org.hl7.fhir.instance.model.api.*; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.PlatformTransactionManager; @@ -81,89 +50,31 @@ import com.google.common.collect.Sets; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; -import ca.uhn.fhir.context.BaseRuntimeChildDefinition; -import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; -import ca.uhn.fhir.context.BaseRuntimeElementDefinition; -import ca.uhn.fhir.context.ConfigurationException; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.FhirVersionEnum; -import ca.uhn.fhir.context.RuntimeChildResourceDefinition; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.context.RuntimeSearchParam; -import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; -import ca.uhn.fhir.jpa.dao.data.IResourceHistoryTableDao; -import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; -import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; -import ca.uhn.fhir.jpa.entity.BaseHasResource; -import ca.uhn.fhir.jpa.entity.BaseResourceIndexedSearchParam; -import ca.uhn.fhir.jpa.entity.BaseTag; -import ca.uhn.fhir.jpa.entity.ForcedId; -import ca.uhn.fhir.jpa.entity.ResourceEncodingEnum; -import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; -import ca.uhn.fhir.jpa.entity.ResourceHistoryTag; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamCoords; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamDate; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamNumber; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamQuantity; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamString; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamToken; -import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamUri; -import ca.uhn.fhir.jpa.entity.ResourceLink; -import ca.uhn.fhir.jpa.entity.ResourceTable; -import ca.uhn.fhir.jpa.entity.ResourceTag; -import ca.uhn.fhir.jpa.entity.Search; -import ca.uhn.fhir.jpa.entity.SearchStatusEnum; -import ca.uhn.fhir.jpa.entity.SearchTypeEnum; -import ca.uhn.fhir.jpa.entity.TagDefinition; -import ca.uhn.fhir.jpa.entity.TagTypeEnum; +import ca.uhn.fhir.context.*; +import ca.uhn.fhir.jpa.dao.data.*; +import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.sp.ISearchParamPresenceSvc; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.util.DeleteConflict; -import ca.uhn.fhir.model.api.IQueryParameterAnd; -import ca.uhn.fhir.model.api.IQueryParameterType; -import ca.uhn.fhir.model.api.IResource; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; -import ca.uhn.fhir.model.api.Tag; -import ca.uhn.fhir.model.api.TagList; +import ca.uhn.fhir.model.api.*; import ca.uhn.fhir.model.base.composite.BaseCodingDt; import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt; import ca.uhn.fhir.model.dstu.resource.BaseResource; -import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.model.primitive.InstantDt; -import ca.uhn.fhir.model.primitive.StringDt; -import ca.uhn.fhir.model.primitive.XhtmlDt; +import ca.uhn.fhir.model.primitive.*; import ca.uhn.fhir.model.valueset.BundleEntryTransactionMethodEnum; -import ca.uhn.fhir.parser.DataFormatException; -import ca.uhn.fhir.parser.IParser; -import ca.uhn.fhir.parser.LenientErrorHandler; +import ca.uhn.fhir.parser.*; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; -import ca.uhn.fhir.rest.method.MethodUtil; -import ca.uhn.fhir.rest.method.QualifiedParamList; -import ca.uhn.fhir.rest.method.RestSearchParameterTypeEnum; -import ca.uhn.fhir.rest.param.DateRangeParam; -import ca.uhn.fhir.rest.param.StringAndListParam; -import ca.uhn.fhir.rest.param.StringParam; -import ca.uhn.fhir.rest.param.TokenAndListParam; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.rest.param.UriAndListParam; -import ca.uhn.fhir.rest.param.UriParam; +import ca.uhn.fhir.rest.method.*; +import ca.uhn.fhir.rest.param.*; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.IBundleProvider; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; -import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; -import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; -import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.CoverageIgnore; -import ca.uhn.fhir.util.FhirTerser; -import ca.uhn.fhir.util.OperationOutcomeUtil; +import ca.uhn.fhir.util.*; public abstract class BaseHapiFhirDao implements IDao { @@ -634,6 +545,14 @@ public abstract class BaseHapiFhirDao implements IDao { } } + private Set getAllTagDefinitions(ResourceTable theEntity) { + HashSet retVal = Sets.newHashSet(); + for (ResourceTag next : theEntity.getTags()) { + retVal.add(next.getTag()); + } + return retVal; + } + protected DaoConfig getConfig() { return myConfig; } @@ -1000,14 +919,6 @@ public abstract class BaseHapiFhirDao implements IDao { return changed; } - private Set getAllTagDefinitions(ResourceTable theEntity) { - HashSet retVal = Sets.newHashSet(); - for (ResourceTag next : theEntity.getTags()) { - retVal.add(next.getTag()); - } - return retVal; - } - @SuppressWarnings("unchecked") private R populateResourceMetadataHapi(Class theResourceType, BaseHasResource theEntity, boolean theForHistoryOperation, IResource res) { R retVal = (R) res; @@ -1806,6 +1717,14 @@ public abstract class BaseHapiFhirDao implements IDao { throw new ResourceVersionConflictException(firstMsg, oo); } + protected void validateMetaCount(int theMetaCount) { + if (myConfig.getResourceMetaCountHardLimit() != null) { + if (theMetaCount > myConfig.getResourceMetaCountHardLimit()) { + throw new UnprocessableEntityException("Resource contains " + theMetaCount + " meta entries (tag/profile/security label), maximum is " + myConfig.getResourceMetaCountHardLimit()); + } + } + } + /** * This method is invoked immediately before storing a new resource, or an update to an existing resource to allow the DAO to ensure that it is valid for persistence. By default, checks for the * "subsetted" tag and rejects resources which have it. Subclasses should call the superclass implementation to preserve this check. @@ -1817,15 +1736,23 @@ public abstract class BaseHapiFhirDao implements IDao { */ protected void validateResourceForStorage(T theResource, ResourceTable theEntityToSave) { Object tag = null; + + int totalMetaCount = 0; + if (theResource instanceof IResource) { IResource res = (IResource) theResource; TagList tagList = ResourceMetadataKeyEnum.TAG_LIST.get(res); if (tagList != null) { tag = tagList.getTag(Constants.TAG_SUBSETTED_SYSTEM, Constants.TAG_SUBSETTED_CODE); } + totalMetaCount += tagList.size(); + totalMetaCount += ResourceMetadataKeyEnum.PROFILES.get(res).size(); } else { IAnyResource res = (IAnyResource) theResource; tag = res.getMeta().getTag(Constants.TAG_SUBSETTED_SYSTEM, Constants.TAG_SUBSETTED_CODE); + totalMetaCount += res.getMeta().getTag().size(); + totalMetaCount += res.getMeta().getProfile().size(); + totalMetaCount += res.getMeta().getSecurity().size(); } if (tag != null) { @@ -1835,6 +1762,8 @@ public abstract class BaseHapiFhirDao implements IDao { String resName = getContext().getResourceDefinition(theResource).getName(); validateChildReferences(theResource, resName); + validateMetaCount(totalMetaCount); + } protected static boolean isValidPid(IIdType theId) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 42033bd85cf..d60d372d957 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -431,7 +431,6 @@ public abstract class BaseHapiFhirResourceDao extends B private void doMetaAdd(MT theMetaAdd, BaseHasResource entity) { List tags = toTagList(theMetaAdd); - //@formatter:off for (TagDefinition nextDef : tags) { boolean hasTag = false; @@ -454,8 +453,9 @@ public abstract class BaseHapiFhirResourceDao extends B } } } - //@formatter:on + validateMetaCount(entity.getTags().size()); + myEntityManager.merge(entity); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 475599260bd..c12ead7edf7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -103,13 +103,19 @@ public class DaoConfig { * update setter javadoc if default changes */ private boolean myIndexContainedResources = true; + private List myInterceptors; + /** * update setter javadoc if default changes */ private int myMaximumExpansionSize = 5000; private int myMaximumSearchResultCountInTransaction = DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION; private ResourceEncodingEnum myResourceEncoding = ResourceEncodingEnum.JSONC; + /** + * update setter javadoc if default changes + */ + private Integer myResourceMetaCountHardLimit = 1000; private Long myReuseCachedSearchResultsForMillis = DEFAULT_REUSE_CACHED_SEARCH_RESULTS_FOR_MILLIS; private boolean mySchedulingDisabled; private boolean mySubscriptionEnabled; @@ -121,7 +127,6 @@ public class DaoConfig { private boolean mySuppressUpdatesWithNoChange = true; private Set myTreatBaseUrlsAsLocal = new HashSet(); private Set myTreatReferencesAsLogical = new HashSet(DEFAULT_LOGICAL_BASE_URLS); - /** * Add a value to the {@link #setTreatReferencesAsLogical(Set) logical references list}. * @@ -135,7 +140,6 @@ public class DaoConfig { } myTreatReferencesAsLogical.add(theTreatReferencesAsLogical); } - /** * When a code system is added that contains more than this number of codes, * the code system will be indexed later in an incremental process in order to @@ -244,6 +248,21 @@ public class DaoConfig { return myResourceEncoding; } + /** + * If set, an individual resource will not be allowed to have more than the + * given number of tags, profiles, and security labels (the limit is for the combined + * total for all of these things on an individual resource). + *

+ * If set to null, no limit will be applied. + *

+ *

+ * The default value for this setting is 1000. + *

+ */ + public Integer getResourceMetaCountHardLimit() { + return myResourceMetaCountHardLimit; + } + /** * If set to a non {@literal null} value (default is {@link #DEFAULT_REUSE_CACHED_SEARCH_RESULTS_FOR_MILLIS non null}) * if an identical search is requested multiple times within this window, the same results will be returned @@ -376,8 +395,6 @@ public class DaoConfig { * and other FHIR features may not behave as expected when referential integrity is not * preserved. Use this feature with caution. *

- * - * @return */ public boolean isEnforceReferentialIntegrityOnDelete() { return myEnforceReferentialIntegrityOnDelete; @@ -697,6 +714,21 @@ public class DaoConfig { myResourceEncoding = theResourceEncoding; } + /** + * If set, an individual resource will not be allowed to have more than the + * given number of tags, profiles, and security labels (the limit is for the combined + * total for all of these things on an individual resource). + *

+ * If set to null, no limit will be applied. + *

+ *

+ * The default value for this setting is 1000. + *

+ */ + public void setResourceMetaCountHardLimit(Integer theResourceMetaCountHardLimit) { + myResourceMetaCountHardLimit = theResourceMetaCountHardLimit; + } + /** * If set to a non {@literal null} value (default is {@link #DEFAULT_REUSE_CACHED_SEARCH_RESULTS_FOR_MILLIS non null}) * if an identical search is requested multiple times within this window, the same results will be returned diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java index 5995895a922..72c2b874320 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java @@ -22,21 +22,13 @@ package ca.uhn.fhir.jpa.entity; import java.io.Serializable; -import javax.persistence.Column; -import javax.persistence.Embeddable; -import javax.persistence.Entity; -import javax.persistence.ForeignKey; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; -import javax.persistence.Id; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; -import javax.persistence.SequenceGenerator; -import javax.persistence.Table; +import javax.persistence.*; @Embeddable @Entity -@Table(name = "HFJ_HISTORY_TAG") +@Table(name = "HFJ_HISTORY_TAG", uniqueConstraints= { + @UniqueConstraint(name="IDX_RESHISTTAG_TAGID", columnNames= {"RES_VER_PID","TAG_ID"}) +}) public class ResourceHistoryTag extends BaseTag implements Serializable { private static final long serialVersionUID = 1L; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java index d54a10f9aeb..688a815f1ab 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java @@ -19,23 +19,15 @@ package ca.uhn.fhir.jpa.entity; * limitations under the License. * #L% */ - -import javax.persistence.Column; -import javax.persistence.Entity; -import javax.persistence.ForeignKey; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; -import javax.persistence.Id; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; -import javax.persistence.SequenceGenerator; -import javax.persistence.Table; +import javax.persistence.*; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; @Entity -@Table(name = "HFJ_RES_TAG") +@Table(name = "HFJ_RES_TAG", uniqueConstraints= { + @UniqueConstraint(name="IDX_RESTAG_TAGID", columnNames= {"RES_ID","TAG_ID"}) +}) public class ResourceTag extends BaseTag { private static final long serialVersionUID = 1L; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3UpdateTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3UpdateTest.java index cda43e6288c..01bc711f944 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3UpdateTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3UpdateTest.java @@ -33,11 +33,10 @@ import org.hl7.fhir.dstu3.model.Resource; import org.hl7.fhir.dstu3.model.UriType; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.AfterClass; -import org.junit.Ignore; -import org.junit.Test; +import org.junit.*; import org.mockito.ArgumentCaptor; +import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.SearchParameterMap; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.rest.api.MethodOutcome; @@ -123,6 +122,137 @@ public class FhirResourceDaoDstu3UpdateTest extends BaseJpaDstu3Test { } + @After + public void afterResetDao() { + myDaoConfig.setResourceMetaCountHardLimit(new DaoConfig().getResourceMetaCountHardLimit()); + } + + @Test + public void testHardMetaCapIsEnforcedOnCreate() { + myDaoConfig.setResourceMetaCountHardLimit(3); + + IIdType id; + { + Patient patient = new Patient(); + patient.getMeta().addTag().setSystem("http://foo").setCode("1"); + patient.getMeta().addTag().setSystem("http://foo").setCode("2"); + patient.getMeta().addTag().setSystem("http://foo").setCode("3"); + patient.getMeta().addTag().setSystem("http://foo").setCode("4"); + patient.setActive(true); + try { + id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + fail(); + } catch (UnprocessableEntityException e) { + assertEquals("Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); + } + } + } + + @Test + public void testHardMetaCapIsEnforcedOnMetaAdd() { + myDaoConfig.setResourceMetaCountHardLimit(3); + + IIdType id; + { + Patient patient = new Patient(); + patient.setActive(true); + id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + } + + { + Meta meta = new Meta(); + meta.addTag().setSystem("http://foo").setCode("1"); + meta.addTag().setSystem("http://foo").setCode("2"); + meta.addTag().setSystem("http://foo").setCode("3"); + meta.addTag().setSystem("http://foo").setCode("4"); + try { + myPatientDao.metaAddOperation(id, meta, null); + fail(); + } catch (UnprocessableEntityException e) { + assertEquals("Resource contains 4 meta entries (tag/profile/security label), maximum is 3", e.getMessage()); + } + + } + } + + @Test + public void testDuplicateTagsOnAddTagsIgnored() { + IIdType id; + { + Patient patient = new Patient(); + patient.setActive(true); + id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + } + + Meta meta = new Meta(); + meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val1"); + meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val2"); + meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val3"); + myPatientDao.metaAddOperation(id, meta, null); + + // Do a read + { + Patient patient = myPatientDao.read(id, mySrd); + List tl = patient.getMeta().getTag(); + assertEquals(1, tl.size()); + assertEquals("http://foo", tl.get(0).getSystem()); + assertEquals("bar", tl.get(0).getCode()); + } + + } + + @Test + public void testDuplicateTagsOnUpdateIgnored() { + IIdType id; + { + Patient patient = new Patient(); + patient.setActive(true); + id = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + } + + { + Patient patient = new Patient(); + patient.setId(id); + patient.setActive(true); + patient.getMeta().addTag().setSystem("http://foo").setCode("bar").setDisplay("Val1"); + patient.getMeta().addTag().setSystem("http://foo").setCode("bar").setDisplay("Val2"); + patient.getMeta().addTag().setSystem("http://foo").setCode("bar").setDisplay("Val3"); + myPatientDao.update(patient, mySrd).getId().toUnqualifiedVersionless(); + } + + // Do a read on second version + { + Patient patient = myPatientDao.read(id, mySrd); + List tl = patient.getMeta().getTag(); + assertEquals(1, tl.size()); + assertEquals("http://foo", tl.get(0).getSystem()); + assertEquals("bar", tl.get(0).getCode()); + } + + // Do a read on first version + { + Patient patient = myPatientDao.read(id.withVersion("1"), mySrd); + List tl = patient.getMeta().getTag(); + assertEquals(0, tl.size()); + } + + Meta meta = new Meta(); + meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val1"); + meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val2"); + meta.addTag().setSystem("http://foo").setCode("bar").setDisplay("Val3"); + myPatientDao.metaAddOperation(id.withVersion("1"), meta, null); + + // Do a read on first version + { + Patient patient = myPatientDao.read(id.withVersion("1"), mySrd); + List tl = patient.getMeta().getTag(); + assertEquals(1, tl.size()); + assertEquals("http://foo", tl.get(0).getSystem()); + assertEquals("bar", tl.get(0).getCode()); + } + + } + @Test public void testMultipleUpdatesWithNoChangesDoesNotResultInAnUpdateForDiscreteUpdates() { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index ee423069a96..35ea92cb170 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -80,6 +80,11 @@ were not able to be automatically purged from the database after they were scheduled for deletion. Thanks to Ravi Kuchi for reporting! + + Add an optional and configurable hard limit on the total number of meta items + (tags, profiles, and security labels) on an individual resource. The default + is 1000. + From c9fcef0372750542c7e92398fa4e660d55e1b3c3 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 30 Jun 2017 16:20:32 -0400 Subject: [PATCH 09/14] Clean up handling of searches nested in batch and transaction --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 7 +- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 9 +- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 34 +- .../uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java | 3 - .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 3 - .../search/DatabaseBackedPagingProvider.java | 1 - .../uhn/fhir/jpa/config/TestDstu3Config.java | 3 +- .../fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java | 22 +- .../fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java | 13 +- ...temProviderTransactionSearchDstu2Test.java | 304 +++++++++++++++ .../dstu3/SystemProviderDstu3Test.java | 3 +- ...temProviderTransactionSearchDstu3Test.java | 358 ++++++++++++++++++ .../provider/dstu2/Dstu2BundleFactory.java | 16 +- .../hapi/rest/server/Dstu3BundleFactory.java | 7 +- src/changes/changes.xml | 17 + 15 files changed, 747 insertions(+), 53 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderTransactionSearchDstu2Test.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 191fcf87046..8c44dd49f88 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1744,9 +1744,12 @@ public abstract class BaseHapiFhirDao implements IDao { TagList tagList = ResourceMetadataKeyEnum.TAG_LIST.get(res); if (tagList != null) { tag = tagList.getTag(Constants.TAG_SUBSETTED_SYSTEM, Constants.TAG_SUBSETTED_CODE); + totalMetaCount += tagList.size(); + } + List profileList = ResourceMetadataKeyEnum.PROFILES.get(res); + if (profileList != null) { + totalMetaCount += profileList.size(); } - totalMetaCount += tagList.size(); - totalMetaCount += ResourceMetadataKeyEnum.PROFILES.get(res).size(); } else { IAnyResource res = (IAnyResource) theResource; tag = res.getMeta().getTag(Constants.TAG_SUBSETTED_SYSTEM, Constants.TAG_SUBSETTED_CODE); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index d60d372d957..6fc9a7f7b87 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -29,6 +29,7 @@ import javax.annotation.PostConstruct; import javax.persistence.NoResultException; import javax.persistence.TypedQuery; +import org.apache.commons.lang3.Validate; import org.hl7.fhir.dstu3.model.IdType; import org.hl7.fhir.instance.model.api.*; import org.springframework.beans.factory.annotation.Autowired; @@ -926,9 +927,11 @@ public abstract class BaseHapiFhirResourceDao extends B notifyInterceptors(RestOperationTypeEnum.SEARCH_TYPE, requestDetails); if (theRequestDetails.isSubRequest()) { - theParams.setLoadSynchronous(true); - int max = myDaoConfig.getMaximumSearchResultCountInTransaction(); - theParams.setLoadSynchronousUpTo(myDaoConfig.getMaximumSearchResultCountInTransaction()); + Integer max = myDaoConfig.getMaximumSearchResultCountInTransaction(); + if (max != null) { + Validate.inclusiveBetween(1, Integer.MAX_VALUE, max.intValue(), "Maximum search result count in transaction ust be a positive integer"); + theParams.setLoadSynchronousUpTo(myDaoConfig.getMaximumSearchResultCountInTransaction()); + } } if (!isPagingProviderDatabaseBacked(theRequestDetails)) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index c12ead7edf7..99f9ae23518 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -57,7 +57,7 @@ public class DaoConfig { * * @see #setMaximumSearchResultCountInTransaction(int) */ - private static final int DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION = 500; + private static final Integer DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION = null; /** * Default value for {@link #setReuseCachedSearchResultsForMillis(Long)}: 60000ms (one minute) @@ -110,7 +110,7 @@ public class DaoConfig { * update setter javadoc if default changes */ private int myMaximumExpansionSize = 5000; - private int myMaximumSearchResultCountInTransaction = DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION; + private Integer myMaximumSearchResultCountInTransaction = DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION; private ResourceEncodingEnum myResourceEncoding = ResourceEncodingEnum.JSONC; /** * update setter javadoc if default changes @@ -233,14 +233,17 @@ public class DaoConfig { } /** - * Provides the maximum number of results which may be returned by a search within a FHIR transaction - * operation. For example, if this value is set to 100 and a FHIR transaction is processed with a sub-request - * for Patient?gender=male, the server will throw an error (and the transaction will fail) if there are more than + * Provides the maximum number of results which may be returned by a search (HTTP GET) which + * is executed as a sub-operation within within a FHIR transaction or + * batch operation. For example, if this value is set to 100 and + * a FHIR transaction is processed with a sub-request for Patient?gender=male, + * the server will throw an error (and the transaction will fail) if there are more than * 100 resources on the server which match this query. - * - * @see #DEFAULT_LOGICAL_BASE_URLS The default value for this setting + *

+ * The default value is null, which means that there is no limit. + *

*/ - public int getMaximumSearchResultCountInTransaction() { + public Integer getMaximumSearchResultCountInTransaction() { return myMaximumSearchResultCountInTransaction; } @@ -699,14 +702,17 @@ public class DaoConfig { } /** - * Provides the maximum number of results which may be returned by a search within a FHIR transaction - * operation. For example, if this value is set to 100 and a FHIR transaction is processed with a sub-request - * for Patient?gender=male, the server will throw an error (and the transaction will fail) if there are more than + * Provides the maximum number of results which may be returned by a search (HTTP GET) which + * is executed as a sub-operation within within a FHIR transaction or + * batch operation. For example, if this value is set to 100 and + * a FHIR transaction is processed with a sub-request for Patient?gender=male, + * the server will throw an error (and the transaction will fail) if there are more than * 100 resources on the server which match this query. - * - * @see #DEFAULT_LOGICAL_BASE_URLS The default value for this setting + *

+ * The default value is null, which means that there is no limit. + *

*/ - public void setMaximumSearchResultCountInTransaction(int theMaximumSearchResultCountInTransaction) { + public void setMaximumSearchResultCountInTransaction(Integer theMaximumSearchResultCountInTransaction) { myMaximumSearchResultCountInTransaction = theMaximumSearchResultCountInTransaction; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java index f2031d0eedf..3f503c089e4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java @@ -106,8 +106,6 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { Bundle resp = new Bundle(); resp.setType(BundleTypeEnum.BATCH_RESPONSE); - OperationOutcome ooResp = new OperationOutcome(); - resp.addEntry().setResource(ooResp); /* * For batch, we handle each entry as a mini-transaction in its own database transaction so that if one fails, it doesn't prevent others @@ -163,7 +161,6 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { long delay = System.currentTimeMillis() - start; ourLog.info("Batch completed in {}ms", new Object[] { delay }); - ooResp.addIssue().setSeverity(IssueSeverityEnum.INFORMATION).setDiagnostics("Batch completed in " + delay + "ms"); return resp; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java index 6809fa97daf..79a3d3a965f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java @@ -114,8 +114,6 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { Bundle resp = new Bundle(); resp.setType(BundleType.BATCHRESPONSE); - OperationOutcome ooResp = new OperationOutcome(); - resp.addEntry().setResource(ooResp); /* * For batch, we handle each entry as a mini-transaction in its own database transaction so that if one fails, it doesn't prevent others @@ -171,7 +169,6 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { long delay = System.currentTimeMillis() - start; ourLog.info("Batch completed in {}ms", new Object[] { delay }); - ooResp.addIssue().setSeverity(IssueSeverity.INFORMATION).setDiagnostics("Batch completed in " + delay + "ms"); return resp; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/DatabaseBackedPagingProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/DatabaseBackedPagingProvider.java index da3ca84ee2d..2e4a9ea0da8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/DatabaseBackedPagingProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/DatabaseBackedPagingProvider.java @@ -77,7 +77,6 @@ public class DatabaseBackedPagingProvider extends BasePagingProvider implements @Override public synchronized String storeResultList(IBundleProvider theList) { String uuid = theList.getUuid(); - Validate.notNull(uuid); return uuid; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java index 4b418e87de0..89366320cb7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java @@ -40,7 +40,6 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 { DataSource dataSource = ProxyDataSourceBuilder .create(retVal) - .multiline() .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) .countQuery() @@ -70,7 +69,7 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 { private Properties jpaProperties() { Properties extraProperties = new Properties(); extraProperties.put("hibernate.jdbc.batch_size", "50"); - extraProperties.put("hibernate.format_sql", "true"); + extraProperties.put("hibernate.format_sql", "false"); extraProperties.put("hibernate.show_sql", "false"); extraProperties.put("hibernate.hbm2ddl.auto", "update"); extraProperties.put("hibernate.dialect", "org.hibernate.dialect.DerbyTenSevenDialect"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java index 7537c60e7f1..3adbb5fd8ea 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java @@ -37,6 +37,7 @@ import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.provider.JpaSystemProviderDstu2; +import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.sp.ISearchParamPresenceSvc; import ca.uhn.fhir.model.dstu2.composite.CodeableConceptDt; @@ -75,7 +76,6 @@ import ca.uhn.fhir.util.TestUtil; @ContextConfiguration(classes= {TestDstu2Config.class, ca.uhn.fhir.jpa.config.WebsocketDstu2DispatcherConfig.class}) //@formatter:on public abstract class BaseJpaDstu2Test extends BaseJpaTest { - @Autowired protected ApplicationContext myAppCtx; @Autowired @@ -114,18 +114,18 @@ public abstract class BaseJpaDstu2Test extends BaseJpaTest { @Autowired @Qualifier("myLocationDaoDstu2") protected IFhirResourceDao myLocationDao; + @Autowired + @Qualifier("myMediaDaoDstu2") + protected IFhirResourceDao myMediaDao; @Autowired - @Qualifier("myMediaDaoDstu2") - protected IFhirResourceDao myMediaDao; +@Qualifier("myMedicationAdministrationDaoDstu2") +protected IFhirResourceDao myMedicationAdministrationDao; @Autowired @Qualifier("myMedicationDaoDstu2") protected IFhirResourceDao myMedicationDao; @Autowired - @Qualifier("myMedicationAdministrationDaoDstu2") - protected IFhirResourceDao myMedicationAdministrationDao; - @Autowired @Qualifier("myMedicationOrderDaoDstu2") protected IFhirResourceDao myMedicationOrderDao; @Autowired @@ -135,6 +135,8 @@ public abstract class BaseJpaDstu2Test extends BaseJpaTest { @Qualifier("myOrganizationDaoDstu2") protected IFhirResourceDao myOrganizationDao; @Autowired + protected DatabaseBackedPagingProvider myPagingProvider; + @Autowired @Qualifier("myPatientDaoDstu2") protected IFhirResourceDaoPatient myPatientDao; @Autowired @@ -150,8 +152,12 @@ public abstract class BaseJpaDstu2Test extends BaseJpaTest { @Qualifier("myResourceProvidersDstu2") protected Object myResourceProviders; @Autowired + protected ISearchCoordinatorSvc mySearchCoordinatorSvc; + @Autowired protected IFulltextSearchSvc mySearchDao; @Autowired + protected ISearchParamPresenceSvc mySearchParamPresenceSvc; + @Autowired @Qualifier("myStructureDefinitionDaoDstu2") protected IFhirResourceDao myStructureDefinitionDao; @Autowired @@ -171,10 +177,6 @@ public abstract class BaseJpaDstu2Test extends BaseJpaTest { @Autowired @Qualifier("myValueSetDaoDstu2") protected IFhirResourceDaoValueSet myValueSetDao; - @Autowired - protected ISearchParamPresenceSvc mySearchParamPresenceSvc; - @Autowired - protected ISearchCoordinatorSvc mySearchCoordinatorSvc; @Before public void beforeCreateInterceptor() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java index 38d94f9af7f..8520877dff2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java @@ -86,8 +86,7 @@ import ca.uhn.fhir.jpa.dao.dstu2.FhirResourceDaoDstu2SearchNoFtTest; import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.provider.dstu3.JpaSystemProviderDstu3; -import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; -import ca.uhn.fhir.jpa.search.IStaleSearchDeletingSvc; +import ca.uhn.fhir.jpa.search.*; import ca.uhn.fhir.jpa.sp.ISearchParamPresenceSvc; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.validation.JpaValidationSupportChainDstu3; @@ -108,13 +107,13 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { private static JpaValidationSupportChainDstu3 ourJpaValidationSupportChainDstu3; private static IFhirResourceDaoValueSet ourValueSetDao; - // @Autowired -// protected HapiWorkerContext myHapiWorkerContext; + // @Autowired + // protected HapiWorkerContext myHapiWorkerContext; @Autowired @Qualifier("myAllergyIntoleranceDaoDstu3") protected IFhirResourceDao myAllergyIntoleranceDao; @Autowired - protected ApplicationContext myAppCtx; + protected ApplicationContext myAppCtx; @Autowired @Qualifier("myAppointmentDaoDstu3") protected IFhirResourceDao myAppointmentDao; @@ -124,7 +123,7 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired @Qualifier("myBundleDaoDstu3") protected IFhirResourceDao myBundleDao; -@Autowired + @Autowired @Qualifier("myCarePlanDaoDstu3") protected IFhirResourceDao myCarePlanDao; @Autowired @@ -195,6 +194,8 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Qualifier("myOrganizationDaoDstu3") protected IFhirResourceDao myOrganizationDao; @Autowired + protected DatabaseBackedPagingProvider myPagingProvider; + @Autowired @Qualifier("myPatientDaoDstu3") protected IFhirResourceDaoPatient myPatientDao; @Autowired diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderTransactionSearchDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderTransactionSearchDstu2Test.java new file mode 100644 index 00000000000..92df70236e4 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderTransactionSearchDstu2Test.java @@ -0,0 +1,304 @@ +package ca.uhn.fhir.jpa.provider; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.*; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.dstu2.BaseJpaDstu2Test; +import ca.uhn.fhir.jpa.rp.dstu2.*; +import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider; +import ca.uhn.fhir.model.dstu2.resource.Bundle; +import ca.uhn.fhir.model.dstu2.resource.Bundle.Entry; +import ca.uhn.fhir.model.dstu2.resource.Patient; +import ca.uhn.fhir.model.dstu2.valueset.*; +import ca.uhn.fhir.rest.client.IGenericClient; +import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor; +import ca.uhn.fhir.rest.server.EncodingEnum; +import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.util.TestUtil; + +public class SystemProviderTransactionSearchDstu2Test extends BaseJpaDstu2Test { + + private static RestfulServer myRestServer; + private static IGenericClient ourClient; + private static FhirContext ourCtx; + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SystemProviderTransactionSearchDstu2Test.class); + private static Server ourServer; + private static String ourServerBase; + private SimpleRequestHeaderInterceptor mySimpleHeaderInterceptor; + + + + + @SuppressWarnings("deprecation") + @After + public void after() { + myRestServer.setUseBrowserFriendlyContentTypes(true); + ourClient.unregisterInterceptor(mySimpleHeaderInterceptor); + myDaoConfig.setMaximumSearchResultCountInTransaction(new DaoConfig().getMaximumSearchResultCountInTransaction()); + } + + @Before + public void before() { + mySimpleHeaderInterceptor = new SimpleRequestHeaderInterceptor(); + ourClient.registerInterceptor(mySimpleHeaderInterceptor); + } + + @Before + public void beforeStartServer() throws Exception { + if (myRestServer == null) { + PatientResourceProvider patientRp = new PatientResourceProvider(); + patientRp.setDao(myPatientDao); + + QuestionnaireResourceProviderDstu2 questionnaireRp = new QuestionnaireResourceProviderDstu2(); + questionnaireRp.setDao(myQuestionnaireDao); + + ObservationResourceProvider observationRp = new ObservationResourceProvider(); + observationRp.setDao(myObservationDao); + + OrganizationResourceProvider organizationRp = new OrganizationResourceProvider(); + organizationRp.setDao(myOrganizationDao); + + RestfulServer restServer = new RestfulServer(ourCtx); + restServer.setResourceProviders(patientRp, questionnaireRp, observationRp, organizationRp); + + restServer.setPlainProviders(mySystemProvider); + + int myPort = RandomServerPortProvider.findFreePort(); + ourServer = new Server(myPort); + + ServletContextHandler proxyHandler = new ServletContextHandler(); + proxyHandler.setContextPath("/"); + + ourServerBase = "http://localhost:" + myPort + "/fhir/context"; + + ServletHolder servletHolder = new ServletHolder(); + servletHolder.setServlet(restServer); + proxyHandler.addServlet(servletHolder, "/fhir/context/*"); + + ourCtx = FhirContext.forDstu2(); + restServer.setFhirContext(ourCtx); + + ourServer.setHandler(proxyHandler); + ourServer.start(); + + ourCtx.getRestfulClientFactory().setSocketTimeout(600 * 1000); + ourClient = ourCtx.newRestfulGenericClient(ourServerBase); + myRestServer = restServer; + } + + myRestServer.setDefaultResponseEncoding(EncodingEnum.XML); + myRestServer.setPagingProvider(myPagingProvider); + } + + + private List create20Patients() { + List ids = new ArrayList(); + for (int i = 0; i < 20; i++) { + Patient patient = new Patient(); + patient.setGender(AdministrativeGenderEnum.MALE); + patient.addIdentifier().setSystem("urn:foo").setValue("A"); + patient.addName().addFamily("abcdefghijklmnopqrstuvwxyz".substring(i, i+1)); + String id = myPatientDao.create(patient).getId().toUnqualifiedVersionless().getValue(); + ids.add(id); + } + return ids; + } + + @Test + public void testBatchWithGetHardLimitLargeSynchronous() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleTypeEnum.BATCH); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerbEnum.GET) + .setUrl("Patient?_count=5"); + + myDaoConfig.setMaximumSearchResultCountInTransaction(100); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertEquals(null, respBundle.getLink("next")); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + + @Test + public void testBatchWithGetNormalSearch() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleTypeEnum.BATCH); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerbEnum.GET) + .setUrl("Patient?_count=5&_sort=name"); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + + String nextPageLink = respBundle.getLink("next").getUrl(); + output = ourClient.loadPage().byUrl(nextPageLink).andReturnBundle(Bundle.class).execute(); + respBundle = output; + assertEquals(5, respBundle.getEntry().size()); + actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(5, 10).toArray(new String[0]))); + } + + /** + * 30 searches in one batch! Whoa! + */ + @Test + public void testBatchWithManyGets() throws Exception { + List ids = create20Patients(); + + + Bundle input = new Bundle(); + input.setType(BundleTypeEnum.BATCH); + for (int i = 0; i < 30; i++) { + input + .addEntry() + .getRequest() + .setMethod(HTTPVerbEnum.GET) + .setUrl("Patient?_count=5&identifier=urn:foo|A,AAAAA" + i); + } + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(30, output.getEntry().size()); + for (int i = 0; i < 30; i++) { + Bundle respBundle = (Bundle) output.getEntry().get(i).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertThat(respBundle.getLink("next").getUrl(), not(nullValue())); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + } + + @Test + public void testTransactionWithGetHardLimitLargeSynchronous() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleTypeEnum.TRANSACTION); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerbEnum.GET) + .setUrl("Patient?_count=5"); + + myDaoConfig.setMaximumSearchResultCountInTransaction(100); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertEquals(null, respBundle.getLink("next")); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + + @Test + public void testTransactionWithGetNormalSearch() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleTypeEnum.TRANSACTION); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerbEnum.GET) + .setUrl("Patient?_count=5&_sort=name"); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + + String nextPageLink = respBundle.getLink("next").getUrl(); + output = ourClient.loadPage().byUrl(nextPageLink).andReturnBundle(Bundle.class).execute(); + respBundle = output; + assertEquals(5, respBundle.getEntry().size()); + actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(5, 10).toArray(new String[0]))); + } + + /** + * 30 searches in one Transaction! Whoa! + */ + @Test + public void testTransactionWithManyGets() throws Exception { + List ids = create20Patients(); + + + Bundle input = new Bundle(); + input.setType(BundleTypeEnum.TRANSACTION); + for (int i = 0; i < 30; i++) { + input + .addEntry() + .getRequest() + .setMethod(HTTPVerbEnum.GET) + .setUrl("Patient?_count=5&identifier=urn:foo|A,AAAAA" + i); + } + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(30, output.getEntry().size()); + for (int i = 0; i < 30; i++) { + Bundle respBundle = (Bundle) output.getEntry().get(i).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertThat(respBundle.getLink("next").getUrl(), not(nullValue())); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + } + + private List toIds(Bundle theRespBundle) { + ArrayList retVal = new ArrayList(); + for (Entry next : theRespBundle.getEntry()) { + retVal.add(next.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + } + return retVal; + } + + @AfterClass + public static void afterClassClearContext() throws Exception { + ourServer.stop(); + TestUtil.clearAllStaticFieldsForUnitTest(); + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java index af577fef5db..1e0d52bba4f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java @@ -58,6 +58,7 @@ import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.jpa.rp.dstu3.ObservationResourceProvider; import ca.uhn.fhir.jpa.rp.dstu3.OrganizationResourceProvider; import ca.uhn.fhir.jpa.rp.dstu3.PatientResourceProvider; +import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider; import ca.uhn.fhir.rest.client.IGenericClient; import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor; @@ -202,7 +203,6 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { organizationRp.setDao(myOrganizationDao); RestfulServer restServer = new RestfulServer(ourCtx); - restServer.setPagingProvider(new FifoMemoryPagingProvider(10).setDefaultPageSize(10)); restServer.setResourceProviders(patientRp, questionnaireRp, observationRp, organizationRp); restServer.setPlainProviders(mySystemProvider); @@ -237,6 +237,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { } myRestServer.setDefaultResponseEncoding(EncodingEnum.XML); + myRestServer.setPagingProvider(myPagingProvider); } @Before diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java new file mode 100644 index 00000000000..2ae03965067 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java @@ -0,0 +1,358 @@ +package ca.uhn.fhir.jpa.provider.dstu3; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.io.IOUtils; +import org.apache.http.Header; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator; +import org.hl7.fhir.dstu3.model.Bundle; +import org.hl7.fhir.dstu3.model.Bundle.*; +import org.hl7.fhir.dstu3.model.DecimalType; +import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; +import org.hl7.fhir.dstu3.model.IdType; +import org.hl7.fhir.dstu3.model.Observation; +import org.hl7.fhir.dstu3.model.OperationDefinition; +import org.hl7.fhir.dstu3.model.OperationOutcome; +import org.hl7.fhir.dstu3.model.Organization; +import org.hl7.fhir.dstu3.model.Parameters; +import org.hl7.fhir.dstu3.model.Patient; +import org.hl7.fhir.dstu3.model.StringType; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.DaoMethodOutcome; +import ca.uhn.fhir.jpa.dao.dstu3.BaseJpaDstu3Test; +import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; +import ca.uhn.fhir.jpa.rp.dstu3.ObservationResourceProvider; +import ca.uhn.fhir.jpa.rp.dstu3.OrganizationResourceProvider; +import ca.uhn.fhir.jpa.rp.dstu3.PatientResourceProvider; +import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; +import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider; +import ca.uhn.fhir.rest.client.IGenericClient; +import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor; +import ca.uhn.fhir.rest.server.Constants; +import ca.uhn.fhir.rest.server.EncodingEnum; +import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider; +import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; +import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; +import ca.uhn.fhir.rest.server.interceptor.ResponseHighlighterInterceptor; +import ca.uhn.fhir.util.TestUtil; +import ca.uhn.fhir.validation.ResultSeverityEnum; + +public class SystemProviderTransactionSearchDstu3Test extends BaseJpaDstu3Test { + + private static RestfulServer myRestServer; + private static IGenericClient ourClient; + private static FhirContext ourCtx; + private static CloseableHttpClient ourHttpClient; + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SystemProviderTransactionSearchDstu3Test.class); + private static Server ourServer; + private static String ourServerBase; + private SimpleRequestHeaderInterceptor mySimpleHeaderInterceptor; + + + + @SuppressWarnings("deprecation") + @After + public void after() { + myRestServer.setUseBrowserFriendlyContentTypes(true); + ourClient.unregisterInterceptor(mySimpleHeaderInterceptor); + myDaoConfig.setMaximumSearchResultCountInTransaction(new DaoConfig().getMaximumSearchResultCountInTransaction()); + } + + @Before + public void before() { + mySimpleHeaderInterceptor = new SimpleRequestHeaderInterceptor(); + ourClient.registerInterceptor(mySimpleHeaderInterceptor); + } + + @Before + public void beforeStartServer() throws Exception { + if (myRestServer == null) { + PatientResourceProvider patientRp = new PatientResourceProvider(); + patientRp.setDao(myPatientDao); + + QuestionnaireResourceProviderDstu3 questionnaireRp = new QuestionnaireResourceProviderDstu3(); + questionnaireRp.setDao(myQuestionnaireDao); + + ObservationResourceProvider observationRp = new ObservationResourceProvider(); + observationRp.setDao(myObservationDao); + + OrganizationResourceProvider organizationRp = new OrganizationResourceProvider(); + organizationRp.setDao(myOrganizationDao); + + RestfulServer restServer = new RestfulServer(ourCtx); + restServer.setResourceProviders(patientRp, questionnaireRp, observationRp, organizationRp); + + restServer.setPlainProviders(mySystemProvider); + + int myPort = RandomServerPortProvider.findFreePort(); + ourServer = new Server(myPort); + + ServletContextHandler proxyHandler = new ServletContextHandler(); + proxyHandler.setContextPath("/"); + + ourServerBase = "http://localhost:" + myPort + "/fhir/context"; + + ServletHolder servletHolder = new ServletHolder(); + servletHolder.setServlet(restServer); + proxyHandler.addServlet(servletHolder, "/fhir/context/*"); + + ourCtx = FhirContext.forDstu3(); + restServer.setFhirContext(ourCtx); + + ourServer.setHandler(proxyHandler); + ourServer.start(); + + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); + HttpClientBuilder builder = HttpClientBuilder.create(); + builder.setConnectionManager(connectionManager); + ourHttpClient = builder.build(); + + ourCtx.getRestfulClientFactory().setSocketTimeout(600 * 1000); + ourClient = ourCtx.newRestfulGenericClient(ourServerBase); + ourClient.setLogRequestAndResponse(true); + myRestServer = restServer; + } + + myRestServer.setDefaultResponseEncoding(EncodingEnum.XML); + myRestServer.setPagingProvider(myPagingProvider); + } + + + private List create20Patients() { + List ids = new ArrayList(); + for (int i = 0; i < 20; i++) { + Patient patient = new Patient(); + patient.setGender(AdministrativeGender.MALE); + patient.addIdentifier().setSystem("urn:foo").setValue("A"); + patient.addName().setFamily("abcdefghijklmnopqrstuvwxyz".substring(i, i+1)); + String id = myPatientDao.create(patient).getId().toUnqualifiedVersionless().getValue(); + ids.add(id); + } + return ids; + } + + @Test + public void testBatchWithGetHardLimitLargeSynchronous() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleType.BATCH); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerb.GET) + .setUrl("Patient?_count=5"); + + myDaoConfig.setMaximumSearchResultCountInTransaction(100); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertEquals(null, respBundle.getLink("next")); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + + @Test + public void testBatchWithGetNormalSearch() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleType.BATCH); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerb.GET) + .setUrl("Patient?_count=5&_sort=name"); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + + String nextPageLink = respBundle.getLink("next").getUrl(); + output = ourClient.loadPage().byUrl(nextPageLink).andReturnBundle(Bundle.class).execute(); + respBundle = output; + assertEquals(5, respBundle.getEntry().size()); + actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(5, 10).toArray(new String[0]))); + } + + /** + * 30 searches in one batch! Whoa! + */ + @Test + public void testBatchWithManyGets() throws Exception { + List ids = create20Patients(); + + + Bundle input = new Bundle(); + input.setType(BundleType.BATCH); + for (int i = 0; i < 30; i++) { + input + .addEntry() + .getRequest() + .setMethod(HTTPVerb.GET) + .setUrl("Patient?_count=5&identifier=urn:foo|A,AAAAA" + i); + } + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(30, output.getEntry().size()); + for (int i = 0; i < 30; i++) { + Bundle respBundle = (Bundle) output.getEntry().get(i).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertThat(respBundle.getLink("next").getUrl(), not(nullValue())); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + } + + @Test + public void testTransactionWithGetHardLimitLargeSynchronous() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleType.TRANSACTION); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerb.GET) + .setUrl("Patient?_count=5"); + + myDaoConfig.setMaximumSearchResultCountInTransaction(100); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertEquals(null, respBundle.getLink("next")); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + + @Test + public void testTransactionWithGetNormalSearch() throws Exception { + List ids = create20Patients(); + + Bundle input = new Bundle(); + input.setType(BundleType.TRANSACTION); + input + .addEntry() + .getRequest() + .setMethod(HTTPVerb.GET) + .setUrl("Patient?_count=5&_sort=name"); + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(1, output.getEntry().size()); + Bundle respBundle = (Bundle) output.getEntry().get(0).getResource(); + assertEquals(5, respBundle.getEntry().size()); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + + String nextPageLink = respBundle.getLink("next").getUrl(); + output = ourClient.loadPage().byUrl(nextPageLink).andReturnBundle(Bundle.class).execute(); + respBundle = output; + assertEquals(5, respBundle.getEntry().size()); + actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(5, 10).toArray(new String[0]))); + } + + /** + * 30 searches in one Transaction! Whoa! + */ + @Test + public void testTransactionWithManyGets() throws Exception { + List ids = create20Patients(); + + + Bundle input = new Bundle(); + input.setType(BundleType.TRANSACTION); + for (int i = 0; i < 30; i++) { + input + .addEntry() + .getRequest() + .setMethod(HTTPVerb.GET) + .setUrl("Patient?_count=5&identifier=urn:foo|A,AAAAA" + i); + } + + Bundle output = ourClient.transaction().withBundle(input).execute(); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + assertEquals(30, output.getEntry().size()); + for (int i = 0; i < 30; i++) { + Bundle respBundle = (Bundle) output.getEntry().get(i).getResource(); + assertEquals(5, respBundle.getEntry().size()); + assertThat(respBundle.getLink("next").getUrl(), not(nullValue())); + List actualIds = toIds(respBundle); + assertThat(actualIds, contains(ids.subList(0, 5).toArray(new String[0]))); + } + } + + private List toIds(Bundle theRespBundle) { + ArrayList retVal = new ArrayList(); + for (BundleEntryComponent next : theRespBundle.getEntry()) { + retVal.add(next.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + } + return retVal; + } + + @AfterClass + public static void afterClassClearContext() throws Exception { + ourServer.stop(); + TestUtil.clearAllStaticFieldsForUnitTest(); + } + +} diff --git a/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/rest/server/provider/dstu2/Dstu2BundleFactory.java b/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/rest/server/provider/dstu2/Dstu2BundleFactory.java index 95280e5d37e..6a4373c0884 100644 --- a/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/rest/server/provider/dstu2/Dstu2BundleFactory.java +++ b/hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/rest/server/provider/dstu2/Dstu2BundleFactory.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.rest.server.provider.dstu2; +import static org.apache.commons.lang3.StringUtils.isBlank; /* * #%L * HAPI FHIR Structures - DSTU2 (FHIR v1.0.0) @@ -10,7 +11,7 @@ package ca.uhn.fhir.rest.server.provider.dstu2; * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -62,6 +63,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.ResourceReferenceInfo; public class Dstu2BundleFactory implements IVersionSpecificBundleFactory { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(Dstu2BundleFactory.class); private Bundle myBundle; private FhirContext myContext; @@ -223,7 +225,7 @@ public class Dstu2BundleFactory implements IVersionSpecificBundleFactory { entry.getRequest().getMethodElement().setValueAsString(httpVerb.getCode()); } populateBundleEntryFullUrl(next, entry); - + BundleEntrySearchModeEnum searchMode = ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.get(next); if (searchMode != null) { entry.getSearch().getModeElement().setValue(searchMode.getCode()); @@ -257,7 +259,7 @@ public class Dstu2BundleFactory implements IVersionSpecificBundleFactory { public void addRootPropertiesToBundle(String theAuthor, String theServerBase, String theCompleteUrl, Integer theTotalResults, BundleTypeEnum theBundleType, IPrimitiveType theLastUpdated) { myBase = theServerBase; - + if (myBundle.getId().isEmpty()) { myBundle.setId(UUID.randomUUID().toString()); } @@ -302,7 +304,7 @@ public class Dstu2BundleFactory implements IVersionSpecificBundleFactory { public void initializeBundleFromBundleProvider(IRestfulServer theServer, IBundleProvider theResult, EncodingEnum theResponseEncoding, String theServerBase, String theCompleteUrl, boolean thePrettyPrint, int theOffset, Integer theLimit, String theSearchId, BundleTypeEnum theBundleType, Set theIncludes) { myBase = theServerBase; - + int numToReturn; String searchId = null; List resourceList; @@ -327,7 +329,7 @@ public class Dstu2BundleFactory implements IVersionSpecificBundleFactory { if (numTotalResults != null) { numToReturn = Math.min(numToReturn, numTotalResults - theOffset); } - + if (numToReturn > 0) { resourceList = theResult.getResources(theOffset, numToReturn + theOffset); } else { @@ -340,7 +342,9 @@ public class Dstu2BundleFactory implements IVersionSpecificBundleFactory { } else { if (numTotalResults == null || numTotalResults > numToReturn) { searchId = pagingProvider.storeResultList(theResult); - Validate.notNull(searchId, "Paging provider returned null searchId"); + if (isBlank(searchId)) { + ourLog.info("Found {} results but paging provider did not provide an ID to use for paging", numTotalResults); + } } } } diff --git a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java index e4a57134cbc..6ad431c94ca 100644 --- a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java +++ b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java @@ -1,5 +1,6 @@ package org.hl7.fhir.dstu3.hapi.rest.server; +import static org.apache.commons.lang3.StringUtils.isBlank; /* * #%L * HAPI FHIR Structures - DSTU2 (FHIR v1.0.0) @@ -44,7 +45,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.ResourceReferenceInfo; public class Dstu3BundleFactory implements IVersionSpecificBundleFactory { - + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(Dstu3BundleFactory.class); private Bundle myBundle; private FhirContext myContext; private String myBase; @@ -336,7 +337,9 @@ public class Dstu3BundleFactory implements IVersionSpecificBundleFactory { } else { if (numTotalResults == null || numTotalResults > numToReturn) { searchId = pagingProvider.storeResultList(theResult); - Validate.notNull(searchId, "Paging provider returned null searchId"); + if (isBlank(searchId)) { + ourLog.info("Found {} results but paging provider did not provide an ID to use for paging", numTotalResults); + } } } } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 35ea92cb170..19b00caebb7 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -85,6 +85,23 @@ (tags, profiles, and security labels) on an individual resource. The default is 1000. + + When executing a search (HTTP GET) as a nested operation in in a transaction or + batch operation, the search now returns a normal page of results with a link to + the next page, like any other search would. Previously the search would return + a small number of results with no paging performed, so this change brings transaction + and batch processing in line with other types of search. + + + JPA server no longer returns an OperationOutcome resource as the first resource + in the Bundle for a response to a batch operation. This behaviour was previously + present, but was not specified in the FHIR specification so it caused confusion and + was inconsistent with behaviour in other servers. + + + Fix a regression in HAPI FHIR 2.5 JPA server where executing a search in a + transaction or batch operation caused an exception. + From 0098a21d7a61fc0b513e7777a33cfac41c45ae42 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 30 Jun 2017 16:29:25 -0400 Subject: [PATCH 10/14] Add license header --- .../BaseRestHookSubscriptionInterceptor.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java index b67564aca1e..aebc05815dc 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/BaseRestHookSubscriptionInterceptor.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.jpa.interceptor; +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2017 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; From 6e181b140d108d41e81a5a5c80f377cb64418ccc Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 30 Jun 2017 21:00:25 -0400 Subject: [PATCH 11/14] Correct an issue when processing transactions in JPA server where updates and creates to resources with tags caused the tags to be created twice in the database. These duplicates were utomatically filtered upon read so this issue was not user-visible, but it coule occasionally lead to performance issues if a resource containing multiple tags was updated many times via transactions. --- .../ca/uhn/fhir/jpa/entity/ResourceTable.java | 4 +- .../ca/uhn/fhir/jpa/entity/ResourceTag.java | 11 ++++- .../jpa/dao/dstu2/FhirSystemDaoDstu2Test.java | 23 ++++----- .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 48 +++++++++++-------- src/changes/changes.xml | 8 ++++ 5 files changed, 57 insertions(+), 37 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java index 1bb19109aca..75824cdcb7d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java @@ -251,7 +251,9 @@ public class ResourceTable extends BaseHasResource implements Serializable { @Override public ResourceTag addTag(TagDefinition theTag) { ResourceTag tag = new ResourceTag(this, theTag); - getTags().add(tag); + if (!getTags().contains(tag)) { + getTags().add(tag); + } return tag; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java index 688a815f1ab..7ab7166db0b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java @@ -21,8 +21,7 @@ package ca.uhn.fhir.jpa.entity; */ import javax.persistence.*; -import org.apache.commons.lang3.builder.EqualsBuilder; -import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.apache.commons.lang3.builder.*; @Entity @Table(name = "HFJ_RES_TAG", uniqueConstraints= { @@ -108,4 +107,12 @@ public class ResourceTag extends BaseTag { return b.toHashCode(); } + @Override + public String toString() { + ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE); + b.append("resId", getResourceId()); + b.append("tag", getTag().getId()); + return b.build(); + } + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java index 33a3b6d610d..6a025e36722 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java @@ -303,36 +303,31 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { request.addEntry().getRequest().setMethod(HTTPVerbEnum.GET).setUrl("Patient/THIS_ID_DOESNT_EXIST"); Bundle resp = mySystemDao.transaction(mySrd, request); - assertEquals(3, resp.getEntry().size()); + assertEquals(2, resp.getEntry().size()); assertEquals(BundleTypeEnum.BATCH_RESPONSE, resp.getTypeElement().getValueAsEnum()); ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); EntryResponse respEntry; - // Bundle.entry[0] is operation outcome - assertEquals(OperationOutcome.class, resp.getEntry().get(0).getResource().getClass()); - assertEquals(IssueSeverityEnum.INFORMATION, ((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getSeverityElement().getValueAsEnum()); - assertThat(((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getDiagnostics(), startsWith("Batch completed in ")); - // Bundle.entry[1] is create response - assertEquals("201 Created", resp.getEntry().get(1).getResponse().getStatus()); - assertThat(resp.getEntry().get(1).getResponse().getLocation(), startsWith("Patient/")); + assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus()); + assertThat(resp.getEntry().get(0).getResponse().getLocation(), startsWith("Patient/")); // Bundle.entry[2] is failed read response - assertEquals(OperationOutcome.class, resp.getEntry().get(2).getResource().getClass()); - assertEquals(IssueSeverityEnum.ERROR, ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getSeverityElement().getValueAsEnum()); - assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getDiagnostics()); - assertEquals("404 Not Found", resp.getEntry().get(2).getResponse().getStatus()); + assertEquals(OperationOutcome.class, resp.getEntry().get(1).getResource().getClass()); + assertEquals(IssueSeverityEnum.ERROR, ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getSeverityElement().getValueAsEnum()); + assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getDiagnostics()); + assertEquals("404 Not Found", resp.getEntry().get(1).getResponse().getStatus()); // Check POST - respEntry = resp.getEntry().get(1).getResponse(); + respEntry = resp.getEntry().get(0).getResponse(); assertEquals("201 Created", respEntry.getStatus()); IdDt createdId = new IdDt(respEntry.getLocation()); assertEquals("Patient", createdId.getResourceType()); myPatientDao.read(createdId, mySrd); // shouldn't fail // Check GET - respEntry = resp.getEntry().get(2).getResponse(); + respEntry = resp.getEntry().get(1).getResponse(); assertThat(respEntry.getStatus(), startsWith("404")); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java index 19ea6a484fb..1090bfdd9d0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java @@ -23,7 +23,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; -import java.util.List; +import java.util.*; import org.apache.commons.io.IOUtils; import org.hl7.fhir.dstu3.model.Appointment; @@ -68,9 +68,7 @@ import org.springframework.transaction.support.TransactionTemplate; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.SearchParameterMap; -import ca.uhn.fhir.jpa.entity.ResourceEncodingEnum; -import ca.uhn.fhir.jpa.entity.ResourceTable; -import ca.uhn.fhir.jpa.entity.TagTypeEnum; +import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.primitive.IdDt; @@ -488,36 +486,31 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { request.addEntry().getRequest().setMethod(HTTPVerb.GET).setUrl("Patient/THIS_ID_DOESNT_EXIST"); Bundle resp = mySystemDao.transaction(mySrd, request); - assertEquals(3, resp.getEntry().size()); + assertEquals(2, resp.getEntry().size()); assertEquals(BundleType.BATCHRESPONSE, resp.getTypeElement().getValue()); ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); BundleEntryResponseComponent respEntry; - // Bundle.entry[0] is operation outcome - assertEquals(OperationOutcome.class, resp.getEntry().get(0).getResource().getClass()); - assertEquals(IssueSeverity.INFORMATION, ((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getSeverityElement().getValue()); - assertThat(((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getDiagnostics(), startsWith("Batch completed in ")); + // Bundle.entry[0] is create response + assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus()); + assertThat(resp.getEntry().get(0).getResponse().getLocation(), startsWith("Patient/")); - // Bundle.entry[1] is create response - assertEquals("201 Created", resp.getEntry().get(1).getResponse().getStatus()); - assertThat(resp.getEntry().get(1).getResponse().getLocation(), startsWith("Patient/")); - - // Bundle.entry[2] is failed read response - assertEquals(OperationOutcome.class, resp.getEntry().get(2).getResource().getClass()); - assertEquals(IssueSeverity.ERROR, ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getSeverityElement().getValue()); - assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getDiagnostics()); - assertEquals("404 Not Found", resp.getEntry().get(2).getResponse().getStatus()); + // Bundle.entry[1] is failed read response + assertEquals(OperationOutcome.class, resp.getEntry().get(1).getResource().getClass()); + assertEquals(IssueSeverity.ERROR, ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getSeverityElement().getValue()); + assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getDiagnostics()); + assertEquals("404 Not Found", resp.getEntry().get(1).getResponse().getStatus()); // Check POST - respEntry = resp.getEntry().get(1).getResponse(); + respEntry = resp.getEntry().get(0).getResponse(); assertEquals("201 Created", respEntry.getStatus()); IdType createdId = new IdType(respEntry.getLocation()); assertEquals("Patient", createdId.getResourceType()); myPatientDao.read(createdId, mySrd); // shouldn't fail // Check GET - respEntry = resp.getEntry().get(2).getResponse(); + respEntry = resp.getEntry().get(1).getResponse(); assertThat(respEntry.getStatus(), startsWith("404")); } @@ -2081,6 +2074,21 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus()); + + + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + Set values = new HashSet(); + for (ResourceTag next : myResourceTagDao.findAll()) { + if (!values.add(next.toString())) { + ourLog.info("Found duplicate tag on resource of type {}", next.getResource().getResourceType()); + ourLog.info("Tag was: {} / {}", next.getTag().getSystem(), next.getTag().getCode()); + } + } + } + }); + } @Test diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 19b00caebb7..fe54a315f06 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -102,6 +102,14 @@ Fix a regression in HAPI FHIR 2.5 JPA server where executing a search in a transaction or batch operation caused an exception. + + Correct an issue when processing transactions in JPA server where updates and + creates to resources with tags caused the tags to be created twice in the + database. These duplicates were utomatically filtered upon read so this issue + was not user-visible, but it coule occasionally lead to performance issues + if a resource containing multiple tags was updated many times via + transactions. + From dd7b1b28c26f3328c1b3c46b65f5da04ec831ee7 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 30 Jun 2017 21:32:01 -0400 Subject: [PATCH 12/14] Fix failing tests --- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 11 ++++++---- .../uhn/fhir/jpa/entity/BaseHasResource.java | 11 +--------- .../java/ca/uhn/fhir/jpa/entity/BaseTag.java | 15 ++++++++------ .../fhir/jpa/entity/ResourceHistoryTable.java | 9 +++++++-- .../ca/uhn/fhir/jpa/entity/ResourceTable.java | 9 ++++++--- .../ca/uhn/fhir/jpa/entity/TagDefinition.java | 20 +++++++++++-------- src/changes/changes.xml | 3 ++- 7 files changed, 44 insertions(+), 34 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 6fc9a7f7b87..c15c9ab2f0c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -119,9 +119,10 @@ public abstract class BaseHapiFhirResourceDao extends B TagDefinition def = getTagOrNull(TagTypeEnum.TAG, theScheme, theTerm, theLabel); if (def != null) { BaseTag newEntity = entity.addTag(def); - - myEntityManager.persist(newEntity); - myEntityManager.merge(entity); + if (newEntity.getTagId() == null) { + myEntityManager.persist(newEntity); + myEntityManager.merge(entity); + } } ourLog.info("Processed addTag {}/{} on {} in {}ms", new Object[] { theScheme, theTerm, theId, w.getMillisAndRestart() }); @@ -450,7 +451,9 @@ public abstract class BaseHapiFhirResourceDao extends B TagDefinition def = getTagOrNull(nextDef.getTagType(), nextDef.getSystem(), nextDef.getCode(), nextDef.getDisplay()); if (def != null) { BaseTag newEntity = entity.addTag(def); - myEntityManager.persist(newEntity); + if (newEntity.getTagId() == null) { + myEntityManager.persist(newEntity); + } } } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseHasResource.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseHasResource.java index 812085c51a1..055ed717096 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseHasResource.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseHasResource.java @@ -23,16 +23,7 @@ package ca.uhn.fhir.jpa.entity; import java.util.Collection; import java.util.Date; -import javax.persistence.Column; -import javax.persistence.EnumType; -import javax.persistence.Enumerated; -import javax.persistence.FetchType; -import javax.persistence.JoinColumn; -import javax.persistence.Lob; -import javax.persistence.MappedSuperclass; -import javax.persistence.OneToOne; -import javax.persistence.Temporal; -import javax.persistence.TemporalType; +import javax.persistence.*; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.model.primitive.IdDt; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseTag.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseTag.java index 62c049fd775..8f15307cce3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseTag.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/BaseTag.java @@ -10,7 +10,7 @@ package ca.uhn.fhir.jpa.entity; * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -32,13 +32,17 @@ public class BaseTag implements Serializable { private static final long serialVersionUID = 1L; - @ManyToOne(cascade= {}) - @JoinColumn(name="TAG_ID", nullable=false) + @ManyToOne(cascade = {}) + @JoinColumn(name = "TAG_ID", nullable = false) private TagDefinition myTag; - @Column(name="TAG_ID", insertable=false,updatable=false) + @Column(name = "TAG_ID", insertable = false, updatable = false) private Long myTagId; + public Long getTagId() { + return myTagId; + } + public TagDefinition getTag() { return myTag; } @@ -46,6 +50,5 @@ public class BaseTag implements Serializable { public void setTag(TagDefinition theTag) { myTag = theTag; } - - + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTable.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTable.java index e8b765460cf..3953ba04854 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTable.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTable.java @@ -93,8 +93,13 @@ public class ResourceHistoryTable extends BaseHasResource implements Serializabl } @Override - public BaseTag addTag(TagDefinition theDef) { - ResourceHistoryTag historyTag = new ResourceHistoryTag(this, theDef); + public ResourceHistoryTag addTag(TagDefinition theTag) { + for (ResourceHistoryTag next : getTags()) { + if (next.getTag().equals(theTag)) { + return next; + } + } + ResourceHistoryTag historyTag = new ResourceHistoryTag(this, theTag); getTags().add(historyTag); return historyTag; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java index 75824cdcb7d..cd2d318436a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java @@ -250,10 +250,13 @@ public class ResourceTable extends BaseHasResource implements Serializable { @Override public ResourceTag addTag(TagDefinition theTag) { - ResourceTag tag = new ResourceTag(this, theTag); - if (!getTags().contains(tag)) { - getTags().add(tag); + for (ResourceTag next : getTags()) { + if (next.getTag().equals(theTag)) { + return next; + } } + ResourceTag tag = new ResourceTag(this, theTag); + getTags().add(tag); return tag; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TagDefinition.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TagDefinition.java index 13cd45265e0..0ef85dd8d40 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TagDefinition.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TagDefinition.java @@ -10,7 +10,7 @@ package ca.uhn.fhir.jpa.entity; * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -45,8 +45,8 @@ import ca.uhn.fhir.model.api.Tag; //@formatter:on @Entity -@Table(name = "HFJ_TAG_DEF", uniqueConstraints = { - @UniqueConstraint(name="IDX_TAGDEF_TYPESYSCODE", columnNames = { "TAG_TYPE", "TAG_SYSTEM", "TAG_CODE" }) +@Table(name = "HFJ_TAG_DEF", uniqueConstraints = { + @UniqueConstraint(name = "IDX_TAGDEF_TYPESYSCODE", columnNames = { "TAG_TYPE", "TAG_SYSTEM", "TAG_CODE" }) }) //@formatter:off public class TagDefinition implements Serializable { @@ -78,6 +78,8 @@ public class TagDefinition implements Serializable { @Enumerated(EnumType.ORDINAL) private TagTypeEnum myTagType; + private Integer myHashCode; + public TagDefinition() { } @@ -110,6 +112,7 @@ public class TagDefinition implements Serializable { public void setCode(String theCode) { myCode = theCode; + myHashCode = null; } public void setDisplay(String theDisplay) { @@ -118,10 +121,12 @@ public class TagDefinition implements Serializable { public void setSystem(String theSystem) { mySystem = theSystem; + myHashCode = null; } public void setTagType(TagTypeEnum theTagType) { myTagType = theTagType; + myHashCode = null; } public Tag toTag() { @@ -155,15 +160,14 @@ public class TagDefinition implements Serializable { @Override public int hashCode() { - HashCodeBuilder b = new HashCodeBuilder(); - if (myId != null) { - b.append(myId); - } else { + if (myHashCode == null) { + HashCodeBuilder b = new HashCodeBuilder(); b.append(myTagType); b.append(mySystem); b.append(myCode); + myHashCode = b.toHashCode(); } - return b.toHashCode(); + return myHashCode; } @Override diff --git a/src/changes/changes.xml b/src/changes/changes.xml index fe54a315f06..c27a14a119e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -100,7 +100,8 @@ Fix a regression in HAPI FHIR 2.5 JPA server where executing a search in a - transaction or batch operation caused an exception. + transaction or batch operation caused an exception. Thanks to Ravi Kuchi for + reporting! Correct an issue when processing transactions in JPA server where updates and From 73a8cf1fca15c52104ec562e2d0f8f23421e3384 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 30 Jun 2017 22:20:43 -0400 Subject: [PATCH 13/14] Don't allow creating resources with references to deleted resources --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 5 +++++ .../dao/dstu3/FhirResourceDaoDstu3Test.java | 18 ++++++++++++++++++ src/changes/changes.xml | 5 +++++ 3 files changed, 28 insertions(+) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 8c44dd49f88..31d108d7ed8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -353,6 +353,11 @@ public abstract class BaseHapiFhirDao implements IDao { "Resource contains reference to " + nextId.getValue() + " but resource with ID " + nextId.getIdPart() + " is actually of type " + target.getResourceType()); } + if (target.getDeleted() != null) { + String resName = targetResourceDef.getName(); + throw new InvalidRequestException("Resource " + resName + "/" + id + " is deleted, specified in path: " + nextPathsUnsplit); + } + if (nextSpDef.getTargets() != null && !nextSpDef.getTargets().contains(typeString)) { continue; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java index cfcef8ce1b4..2f3163b3541 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java @@ -155,6 +155,24 @@ public class FhirResourceDaoDstu3Test extends BaseJpaDstu3Test { } } + @Test + public void testCreateReferenceToDeletedResource() { + Organization org = new Organization(); + org.setActive(true); + IIdType orgId = myOrganizationDao.create(org).getId().toUnqualifiedVersionless(); + + myOrganizationDao.delete(orgId); + + Patient p = new Patient(); + p.getManagingOrganization().setReferenceElement(orgId); + try { + myPatientDao.create(p); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Resource Organization/" + orgId.getIdPart() + " is deleted, specified in path: Patient.managingOrganization", e.getMessage()); + } + } + @Test public void testCreateDuplicateTagsDoesNotCauseDuplicates() { Patient p = new Patient(); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index c27a14a119e..0af0d2e74c6 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -111,6 +111,11 @@ if a resource containing multiple tags was updated many times via transactions. + + JPA server should not allow creation of resources that have a reference to + a resource ID that previously existed but is now deleted. Thanks to Artem + Sopin for reporting! + From 294d080bd3e297e6c8d78746e770c5b1bd5483b2 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sat, 1 Jul 2017 16:28:42 -0400 Subject: [PATCH 14/14] Add config setting for JPA resource counts in metadata --- .../provider/JpaConformanceProviderDstu2.java | 20 ++++++++--- .../dstu3/JpaConformanceProviderDstu3.java | 33 +++++++++++-------- src/changes/changes.xml | 4 +++ 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaConformanceProviderDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaConformanceProviderDstu2.java index 1ae9c54633f..da70d52eb69 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaConformanceProviderDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaConformanceProviderDstu2.java @@ -19,9 +19,7 @@ package ca.uhn.fhir.jpa.provider; * limitations under the License. * #L% */ - -import java.util.List; -import java.util.Map; +import java.util.*; import javax.servlet.http.HttpServletRequest; @@ -51,6 +49,7 @@ public class JpaConformanceProviderDstu2 extends ServerConformanceProvider { private volatile Conformance myCachedValue; private DaoConfig myDaoConfig; private String myImplementationDescription; + private boolean myIncludeResourceCounts; private RestfulServer myRestfulServer; private IFhirSystemDao mySystemDao; @@ -61,6 +60,7 @@ public class JpaConformanceProviderDstu2 extends ServerConformanceProvider { public JpaConformanceProviderDstu2(){ super(); super.setCache(false); + setIncludeResourceCounts(true); } /** @@ -72,13 +72,17 @@ public class JpaConformanceProviderDstu2 extends ServerConformanceProvider { mySystemDao = theSystemDao; myDaoConfig = theDaoConfig; super.setCache(false); + setIncludeResourceCounts(true); } @Override public Conformance getServerConformance(HttpServletRequest theRequest) { Conformance retVal = myCachedValue; - Map counts = mySystemDao.getResourceCounts(); + Map counts = Collections.emptyMap(); + if (myIncludeResourceCounts) { + counts = mySystemDao.getResourceCounts(); + } FhirContext ctx = myRestfulServer.getFhirContext(); @@ -119,6 +123,10 @@ public class JpaConformanceProviderDstu2 extends ServerConformanceProvider { return retVal; } + public boolean isIncludeResourceCounts() { + return myIncludeResourceCounts; + } + public void setDaoConfig(DaoConfig myDaoConfig) { this.myDaoConfig = myDaoConfig; } @@ -128,6 +136,10 @@ public class JpaConformanceProviderDstu2 extends ServerConformanceProvider { myImplementationDescription = theImplDesc; } + public void setIncludeResourceCounts(boolean theIncludeResourceCounts) { + myIncludeResourceCounts = theIncludeResourceCounts; + } + @Override public void setRestfulServer(RestfulServer theRestfulServer) { this.myRestfulServer = theRestfulServer; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaConformanceProviderDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaConformanceProviderDstu3.java index b55b70e4e08..ba417412326 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaConformanceProviderDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaConformanceProviderDstu3.java @@ -1,7 +1,5 @@ package ca.uhn.fhir.jpa.provider.dstu3; -import java.util.Collection; - /* * #%L * HAPI FHIR JPA Server @@ -21,23 +19,18 @@ import java.util.Collection; * limitations under the License. * #L% */ - -import java.util.Map; +import java.util.*; import javax.servlet.http.HttpServletRequest; import org.hl7.fhir.dstu3.model.*; import org.hl7.fhir.dstu3.model.CapabilityStatement.*; import org.hl7.fhir.dstu3.model.Enumerations.SearchParamType; -import org.springframework.beans.factory.annotation.Autowired; -import ca.uhn.fhir.context.BaseRuntimeElementDefinition; -import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.IFhirSystemDao; -import ca.uhn.fhir.jpa.dao.ISearchParamRegistry; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.util.CoverageIgnore; import ca.uhn.fhir.util.ExtensionConstants; @@ -47,9 +40,10 @@ public class JpaConformanceProviderDstu3 extends org.hl7.fhir.dstu3.hapi.rest.se private volatile CapabilityStatement myCachedValue; private DaoConfig myDaoConfig; private String myImplementationDescription; + private boolean myIncludeResourceCounts; private RestfulServer myRestfulServer; private IFhirSystemDao mySystemDao; - + /** * Constructor */ @@ -57,8 +51,9 @@ public class JpaConformanceProviderDstu3 extends org.hl7.fhir.dstu3.hapi.rest.se public JpaConformanceProviderDstu3(){ super(); super.setCache(false); + setIncludeResourceCounts(true); } - + /** * Constructor */ @@ -68,15 +63,17 @@ public class JpaConformanceProviderDstu3 extends org.hl7.fhir.dstu3.hapi.rest.se mySystemDao = theSystemDao; myDaoConfig = theDaoConfig; super.setCache(false); + setIncludeResourceCounts(true); } - + @Override public CapabilityStatement getServerConformance(HttpServletRequest theRequest) { CapabilityStatement retVal = myCachedValue; - Map counts = mySystemDao.getResourceCounts(); - - FhirContext ctx = myRestfulServer.getFhirContext(); + Map counts = Collections.emptyMap(); + if (myIncludeResourceCounts) { + counts = mySystemDao.getResourceCounts(); + } retVal = super.getServerConformance(theRequest); for (CapabilityStatementRestComponent nextRest : retVal.getRest()) { @@ -148,6 +145,10 @@ public class JpaConformanceProviderDstu3 extends org.hl7.fhir.dstu3.hapi.rest.se return retVal; } + public boolean isIncludeResourceCounts() { + return myIncludeResourceCounts; + } + /** * Subclasses may override */ @@ -164,6 +165,10 @@ public class JpaConformanceProviderDstu3 extends org.hl7.fhir.dstu3.hapi.rest.se myImplementationDescription = theImplDesc; } + public void setIncludeResourceCounts(boolean theIncludeResourceCounts) { + myIncludeResourceCounts = theIncludeResourceCounts; + } + @Override public void setRestfulServer(RestfulServer theRestfulServer) { this.myRestfulServer = theRestfulServer; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 0af0d2e74c6..8565c7fc406 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -116,6 +116,10 @@ a resource ID that previously existed but is now deleted. Thanks to Artem Sopin for reporting! + + JpaConformanceProvider now has a configuration setting to enable and + disable adding resource counts to the server metadata. +