From 3189819dd5218d296c48ba318c6ba84aec82bbc4 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 16 Sep 2020 14:13:03 -0400 Subject: [PATCH] limit transaction size config (#2087) --- .../2087-limit-transaction-size-config.yaml | 5 + .../ca/uhn/fhir/jpa/api/config/DaoConfig.java | 28 +++++ .../jpa/dao/BaseTransactionProcessor.java | 16 ++- .../dstu3/FhirSystemDaoDstu3SearchTest.java | 46 -------- .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 102 ------------------ .../FhirSystemDaoTransactionDstu3Test.java | 78 ++++++++++++++ 6 files changed, 125 insertions(+), 150 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/2087-limit-transaction-size-config.yaml delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3SearchTest.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoTransactionDstu3Test.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/2087-limit-transaction-size-config.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/2087-limit-transaction-size-config.yaml new file mode 100644 index 00000000000..db383a422f5 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/2087-limit-transaction-size-config.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 2087 +title: "Added new DaoConfig parameter called maximumTransactionBundleSize that if not-null will throw a +PayloadTooLarge exception when the number of resources in a transaction bundle exceeds this size." diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java index 03c8f3f5c5b..038a189d788 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java @@ -80,6 +80,7 @@ public class DaoConfig { * @see #setMaximumSearchResultCountInTransaction(Integer) */ private static final Integer DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION = null; + private static final Integer DEFAULT_MAXIMUM_TRANSACTION_BUNDLE_SIZE = null; private static final Logger ourLog = LoggerFactory.getLogger(DaoConfig.class); private static final int DEFAULT_EXPUNGE_BATCH_SIZE = 800; private IndexEnabledEnum myIndexMissingFieldsEnabled = IndexEnabledEnum.DISABLED; @@ -126,6 +127,8 @@ public class DaoConfig { private boolean myIndexContainedResources = true; private int myMaximumExpansionSize = DEFAULT_MAX_EXPANSION_SIZE; private Integer myMaximumSearchResultCountInTransaction = DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION; + + private Integer myMaximumTransactionBundleSize = DEFAULT_MAXIMUM_TRANSACTION_BUNDLE_SIZE; private ResourceEncodingEnum myResourceEncoding = ResourceEncodingEnum.JSONC; /** * update setter javadoc if default changes @@ -655,6 +658,31 @@ public class DaoConfig { myMaximumSearchResultCountInTransaction = theMaximumSearchResultCountInTransaction; } + /** + * Specifies the maximum number of resources permitted within a single transaction bundle. + * If a transaction bundle is submitted with more than this number of resources, it will be + * rejected with a PayloadTooLarge exception. + *

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

+ */ + public Integer getMaximumTransactionBundleSize() { + return myMaximumTransactionBundleSize; + } + + /** + * Specifies the maximum number of resources permitted within a single transaction bundle. + * If a transaction bundle is submitted with more than this number of resources, it will be + * rejected with a PayloadTooLarge exception. + *

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

+ */ + public DaoConfig setMaximumTransactionBundleSize(Integer theMaximumTransactionBundleSize) { + myMaximumTransactionBundleSize = theMaximumTransactionBundleSize; + return this; + } + /** * This setting controls the number of threads allocated to resource reindexing * (which is only ever used if SearchParameters change, or a manual reindex is diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index afbfdc0c954..1135bd1e6c6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.dao.IJpaDao; @@ -54,6 +55,7 @@ import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.NotModifiedException; +import ca.uhn.fhir.rest.server.exceptions.PayloadTooLargeException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; @@ -124,6 +126,8 @@ public abstract class BaseTransactionProcessor { private MatchResourceUrlService myMatchResourceUrlService; @Autowired private HapiTransactionService myHapiTransactionService; + @Autowired + private DaoConfig myDaoConfig; @PostConstruct public void start() { @@ -342,7 +346,15 @@ public abstract class BaseTransactionProcessor { throw new InvalidRequestException("Unable to process transaction where incoming Bundle.type = " + transactionType); } - ourLog.debug("Beginning {} with {} resources", theActionName, myVersionAdapter.getEntries(theRequest).size()); + int numberOfEntries = myVersionAdapter.getEntries(theRequest).size(); + + if (myDaoConfig.getMaximumTransactionBundleSize() != null && numberOfEntries > myDaoConfig.getMaximumTransactionBundleSize()) { + throw new PayloadTooLargeException("Transaction Bundle Too large. Transaction bundle contains " + + numberOfEntries + + " which exceedes the maximum permitted transaction bundle size of " + myDaoConfig.getMaximumTransactionBundleSize()); + } + + ourLog.debug("Beginning {} with {} resources", theActionName, numberOfEntries); final TransactionDetails transactionDetails = new TransactionDetails(); final StopWatch transactionStopWatch = new StopWatch(); @@ -350,7 +362,7 @@ public abstract class BaseTransactionProcessor { List requestEntries = myVersionAdapter.getEntries(theRequest); // Do all entries have a verb? - for (int i = 0; i < myVersionAdapter.getEntries(theRequest).size(); i++) { + for (int i = 0; i < numberOfEntries; i++) { IBase nextReqEntry = requestEntries.get(i); String verb = myVersionAdapter.getEntryRequestVerb(myContext, nextReqEntry); if (verb == null || !isValidVerb(verb)) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3SearchTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3SearchTest.java deleted file mode 100644 index fd47d44bd85..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3SearchTest.java +++ /dev/null @@ -1,46 +0,0 @@ -package ca.uhn.fhir.jpa.dao.dstu3; - -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Test; - -import ca.uhn.fhir.util.TestUtil; - -public class FhirSystemDaoDstu3SearchTest extends BaseJpaDstu3SystemTest { - - @Test - public void testSearchByParans() { - // code to come.. just here to prevent a failure - } - - /*//@formatter:off - * [ERROR] Search parameter action has conflicting types token and reference - * [ERROR] Search parameter source has conflicting types token and reference - * [ERROR] Search parameter plan has conflicting types reference and token - * [ERROR] Search parameter version has conflicting types token and string - * [ERROR] Search parameter source has conflicting types reference and uri - * [ERROR] Search parameter location has conflicting types reference and uri - * [ERROR] Search parameter title has conflicting types string and token - * [ERROR] Search parameter manufacturer has conflicting types string and reference - * [ERROR] Search parameter address has conflicting types token and string - * [ERROR] Search parameter source has conflicting types reference and string - * [ERROR] Search parameter destination has conflicting types reference and string - * [ERROR] Search parameter responsible has conflicting types reference and string - * [ERROR] Search parameter value has conflicting types token and string - * [ERROR] Search parameter address has conflicting types token and string - * [ERROR] Search parameter address has conflicting types token and string - * [ERROR] Search parameter address has conflicting types token and string - * [ERROR] Search parameter address has conflicting types token and string - * [ERROR] Search parameter action has conflicting types reference and token - * [ERROR] Search parameter version has conflicting types token and string - * [ERROR] Search parameter address has conflicting types token and string - * [ERROR] Search parameter base has conflicting types reference and token - * [ERROR] Search parameter target has conflicting types reference and token - * [ERROR] Search parameter base has conflicting types reference and uri - * [ERROR] Search parameter contact has conflicting types string and token - * [ERROR] Search parameter substance has conflicting types token and reference - * [ERROR] Search parameter provider has conflicting types reference and token - * [ERROR] Search parameter system has conflicting types token and uri - * [ERROR] Search parameter reference has conflicting types reference and uri - * //@formatter:off - */ -} 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 11fac06fce8..fd5d1938250 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 @@ -2888,108 +2888,6 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { assertEquals(1, found.size().intValue()); } - // - // - // /** - // * Issue #55 - // */ - // @Test - // public void testTransactionWithCidIds() throws Exception { - // Bundle request = new Bundle(); - // - // Patient p1 = new Patient(); - // p1.setId("cid:patient1"); - // p1.addIdentifier().setSystem("system").setValue("testTransactionWithCidIds01"); - // res.add(p1); - // - // Observation o1 = new Observation(); - // o1.setId("cid:observation1"); - // o1.getIdentifier().setSystem("system").setValue("testTransactionWithCidIds02"); - // o1.setSubject(new Reference("Patient/cid:patient1")); - // res.add(o1); - // - // Observation o2 = new Observation(); - // o2.setId("cid:observation2"); - // o2.getIdentifier().setSystem("system").setValue("testTransactionWithCidIds03"); - // o2.setSubject(new Reference("Patient/cid:patient1")); - // res.add(o2); - // - // ourSystemDao.transaction(res); - // - // assertTrue(p1.getId().getValue(), p1.getId().getIdPart().matches("^[0-9]+$")); - // assertTrue(o1.getId().getValue(), o1.getId().getIdPart().matches("^[0-9]+$")); - // assertTrue(o2.getId().getValue(), o2.getId().getIdPart().matches("^[0-9]+$")); - // - // assertThat(o1.getSubject().getReference().getValue(), endsWith("Patient/" + p1.getId().getIdPart())); - // assertThat(o2.getSubject().getReference().getValue(), endsWith("Patient/" + p1.getId().getIdPart())); - // - // } - // - // @Test - // public void testTransactionWithDelete() throws Exception { - // Bundle request = new Bundle(); - // - // /* - // * Create 3 - // */ - // - // List res; - // res = new ArrayList(); - // - // Patient p1 = new Patient(); - // p1.addIdentifier().setSystem("urn:system").setValue("testTransactionWithDelete"); - // res.add(p1); - // - // Patient p2 = new Patient(); - // p2.addIdentifier().setSystem("urn:system").setValue("testTransactionWithDelete"); - // res.add(p2); - // - // Patient p3 = new Patient(); - // p3.addIdentifier().setSystem("urn:system").setValue("testTransactionWithDelete"); - // res.add(p3); - // - // ourSystemDao.transaction(res); - // - // /* - // * Verify - // */ - // - // IBundleProvider results = ourPatientDao.search(Patient.SP_IDENTIFIER, new TokenParam("urn:system", - // "testTransactionWithDelete")); - // assertEquals(3, results.size()); - // - // /* - // * Now delete 2 - // */ - // - // request = new Bundle(); - // res = new ArrayList(); - // List existing = results.getResources(0, 3); - // - // p1 = new Patient(); - // p1.setId(existing.get(0).getId()); - // ResourceMetadataKeyEnum.DELETED_AT.put(p1, InstantDt.withCurrentTime()); - // res.add(p1); - // - // p2 = new Patient(); - // p2.setId(existing.get(1).getId()); - // ResourceMetadataKeyEnum.DELETED_AT.put(p2, InstantDt.withCurrentTime()); - // res.add(p2); - // - // ourSystemDao.transaction(res); - // - // /* - // * Verify - // */ - // - // IBundleProvider results2 = ourPatientDao.search(Patient.SP_IDENTIFIER, new TokenParam("urn:system", - // "testTransactionWithDelete")); - // assertEquals(1, results2.size()); - // List existing2 = results2.getResources(0, 1); - // assertEquals(existing2.get(0).getId(), existing.get(2).getId()); - // - // } - @Test public void testTransactionWithRelativeOidIds() { Bundle res = new Bundle(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoTransactionDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoTransactionDstu3Test.java new file mode 100644 index 00000000000..8db6ddc8b7b --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoTransactionDstu3Test.java @@ -0,0 +1,78 @@ +package ca.uhn.fhir.jpa.dao.dstu3; + +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.rest.server.exceptions.PayloadTooLargeException; +import org.hl7.fhir.dstu3.model.Bundle; +import org.hl7.fhir.dstu3.model.Bundle.BundleType; +import org.hl7.fhir.dstu3.model.Bundle.HTTPVerb; +import org.hl7.fhir.dstu3.model.Observation; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +public class FhirSystemDaoTransactionDstu3Test extends BaseJpaDstu3SystemTest { + public static final int TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE = 5; + + @AfterEach + public void after() { + myDaoConfig.setMaximumTransactionBundleSize(new DaoConfig().getMaximumTransactionBundleSize()); + } + + @BeforeEach + public void beforeDisableResultReuse() { + myDaoConfig.setMaximumTransactionBundleSize(TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE); + } + + private Bundle createInputTransactionWithSize(int theSize) { + Bundle retval = new Bundle(); + retval.setType(BundleType.TRANSACTION); + for (int i = 0; i < theSize; ++i) { + Observation obs = new Observation(); + obs.setStatus(Observation.ObservationStatus.FINAL); + retval + .addEntry() + .setFullUrl("urn:uuid:000" + i) + .setResource(obs) + .getRequest() + .setMethod(HTTPVerb.POST); + } + + return retval; + } + + @Test + public void testTransactionTooBig() { + Bundle bundle = createInputTransactionWithSize(TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE + 1); + + try { + mySystemDao.transaction(null, bundle); + fail(); + } catch (PayloadTooLargeException e) { + assertThat(e.getMessage(), containsString("Transaction Bundle Too large. Transaction bundle contains " + + (TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE + 1) + + " which exceedes the maximum permitted transaction bundle size of " + TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE)); + } + } + + @Test + public void testTransactionSmallEnough() { + testTransactionBundleSucceedsWithSize(TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE); + testTransactionBundleSucceedsWithSize(TEST_MAXIMUM_TRANSACTION_BUNDLE_SIZE - 1); + testTransactionBundleSucceedsWithSize(1); + } + + private void testTransactionBundleSucceedsWithSize(int theSize) { + Bundle bundle = createInputTransactionWithSize(theSize); + Bundle response = mySystemDao.transaction(null, bundle); + + assertEquals(theSize, response.getEntry().size()); + assertEquals("201 Created", response.getEntry().get(0).getResponse().getStatus()); + assertEquals("201 Created", response.getEntry().get(theSize - 1).getResponse().getStatus()); + } + +}