From fd528a0f7ddd8309ff21c2b7e3522d9332b5aa6c Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 7 Dec 2020 17:35:51 -0500 Subject: [PATCH] Improve error message on unsupported resource type (#2221) * Improve error message on unsupported resource type * Add changelog --- .../ca/uhn/fhir/i18n/hapi-messages.properties | 2 + ...ove-error-message-on-unsupported-type.yaml | 5 + .../jpa/dao/BaseTransactionProcessor.java | 10 +- .../fhir/jpa/dao/TransactionProcessor.java | 5 +- .../jpa/dao/TransactionProcessorTest.java | 127 ++++++++++++++++++ 5 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2221-improve-error-message-on-unsupported-type.yaml create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties index aa90487d8f5..5b9821cd33b 100644 --- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties +++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties @@ -4,6 +4,7 @@ ca.uhn.fhir.jpa.term.BaseTermReadSvcImpl.valueSetNotYetExpanded=ValueSet "{0}" h ca.uhn.fhir.jpa.term.BaseTermReadSvcImpl.valueSetNotYetExpanded_OffsetNotAllowed=ValueSet expansion can not combine "offset" with "ValueSet.compose.exclude" unless the ValueSet has been pre-expanded. ValueSet "{0}" must be pre-expanded for this operation to work. + # Core Library Messages ca.uhn.fhir.context.FhirContext.unknownResourceName=Unknown resource name "{0}" (this name is not known in FHIR version "{1}") ca.uhn.fhir.context.FhirContext.noStructures=Could not find any HAPI-FHIR structure JARs on the classpath. Note that as of HAPI-FHIR v0.8, a separate FHIR strcture JAR must be added to your classpath (or project pom.xml if you are using Maven) @@ -108,6 +109,7 @@ ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.invalidSortParameter=Unknown _sort p ca.uhn.fhir.rest.api.PatchTypeEnum.missingPatchContentType=Missing or invalid content type for PATCH operation ca.uhn.fhir.rest.api.PatchTypeEnum.invalidPatchContentType=Invalid Content-Type for PATCH operation: {0} +ca.uhn.fhir.jpa.dao.BaseTransactionProcessor.unsupportedResourceType=Resource {0} is not supported on this server. Supported resource types: {1} ca.uhn.fhir.jpa.dao.TransactionProcessor.missingMandatoryResource=Missing required resource in Bundle.entry[{1}].resource for operation {0} ca.uhn.fhir.jpa.dao.TransactionProcessor.missingPatchBody=Unable to determine PATCH body from request ca.uhn.fhir.jpa.dao.TransactionProcessor.fhirPatchShouldNotUseBinaryResource=Binary PATCH detected with FHIR content type. FHIR Patch should use Parameters resource. diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2221-improve-error-message-on-unsupported-type.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2221-improve-error-message-on-unsupported-type.yaml new file mode 100644 index 00000000000..cb62a83c709 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2221-improve-error-message-on-unsupported-type.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 2221 +title: "The error message returned by the transaction processor has been improved for the case where a transaction + uses an unsupported/disabled resource types." 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 7d154752181..e501bc98168 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 @@ -102,6 +102,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import static ca.uhn.fhir.util.StringUtil.toUtf8String; import static org.apache.commons.lang3.StringUtils.defaultString; @@ -1019,7 +1020,14 @@ public abstract class BaseTransactionProcessor { } private IFhirResourceDao getDaoOrThrowException(Class theClass) { - return myDaoRegistry.getResourceDao(theClass); + IFhirResourceDao dao = myDaoRegistry.getResourceDaoOrNull(theClass); + if (dao == null) { + Set types = new TreeSet<>(myDaoRegistry.getRegisteredDaoTypes()); + String type = myContext.getResourceType(theClass); + String msg = myContext.getLocalizer().getMessage(BaseTransactionProcessor.class, "unsupportedResourceType", type, types.toString()); + throw new InvalidRequestException(msg); + } + return dao; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java index f67e85a36f9..10086c50367 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java @@ -42,12 +42,15 @@ import java.util.stream.Collectors; public class TransactionProcessor extends BaseTransactionProcessor { private static final Logger ourLog = LoggerFactory.getLogger(TransactionProcessor.class); - @PersistenceContext(type = PersistenceContextType.TRANSACTION) private EntityManager myEntityManager; @Autowired(required = false) private HapiFhirHibernateJpaDialect myHapiFhirHibernateJpaDialect; + public void setEntityManagerForUnitTest(EntityManager theEntityManager) { + myEntityManager = theEntityManager; + } + @Override protected void validateDependencies() { super.validateDependencies(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java new file mode 100644 index 00000000000..fc33a513b13 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java @@ -0,0 +1,127 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; +import ca.uhn.fhir.interceptor.executor.InterceptorService; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.dao.r4.TransactionProcessorVersionAdapterR4; +import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import org.hibernate.Session; +import org.hibernate.internal.SessionImpl; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.MedicationKnowledge; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Answers; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionCallback; + +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = TransactionProcessorTest.MyConfig.class) +public class TransactionProcessorTest { + + @Autowired + private TransactionProcessor myTransactionProcessor; + @MockBean + private EntityManagerFactory myEntityManagerFactory; + @MockBean(answer = Answers.RETURNS_DEEP_STUBS) + private EntityManager myEntityManager; + @MockBean + private PlatformTransactionManager myPlatformTransactionManager; + @MockBean + private MatchResourceUrlService myMatchResourceUrlService; + @MockBean + private HapiTransactionService myHapiTransactionService; + @MockBean(answer = Answers.RETURNS_DEEP_STUBS) + private SessionImpl mySession; + + @BeforeEach + public void before() { + when(myHapiTransactionService.execute(any(), any())).thenAnswer(t->{ + TransactionCallback callback = t.getArgument(1, TransactionCallback.class); + return callback.doInTransaction(null); + }); + + myTransactionProcessor.setEntityManagerForUnitTest(myEntityManager); + + when(myEntityManager.unwrap(eq(Session.class))).thenReturn(mySession); + } + + @Test + public void testTransactionWithDisabledResourceType() { + + Bundle input = new Bundle(); + input.setType(Bundle.BundleType.TRANSACTION); + + MedicationKnowledge medKnowledge = new MedicationKnowledge(); + medKnowledge.setStatus(MedicationKnowledge.MedicationKnowledgeStatus.ACTIVE); + input + .addEntry() + .setResource(medKnowledge) + .getRequest() + .setMethod(Bundle.HTTPVerb.POST) + .setUrl("/MedicationKnowledge"); + + try { + myTransactionProcessor.transaction(null, input); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Resource MedicationKnowledge is not supported on this server. Supported resource types: []", e.getMessage()); + } + } + + + @Configuration + public static class MyConfig { + + @Bean + public DaoRegistry daoRegistry() { + return new DaoRegistry(); + } + + @Bean + public FhirContext fhirContext() { + return FhirContext.forCached(FhirVersionEnum.R4); + } + + @Bean + public TransactionProcessor transactionProcessor() { + return new TransactionProcessor(); + } + + @Bean + public InterceptorService interceptorService() { + return new InterceptorService(); + } + + @Bean + public DaoConfig daoConfig() { + return new DaoConfig(); + } + + @Bean + public BaseTransactionProcessor.ITransactionProcessorVersionAdapter versionAdapter() { + return new TransactionProcessorVersionAdapterR4(); + } + + + } +}