diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/QualifiedParamList.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/QualifiedParamList.java index 962e33fdcb0..65db09c0116 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/QualifiedParamList.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/QualifiedParamList.java @@ -1,5 +1,12 @@ package ca.uhn.fhir.rest.api; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.IQueryParameterOr; +import ca.uhn.fhir.model.api.IQueryParameterType; + +import java.util.ArrayList; +import java.util.StringTokenizer; + import static org.apache.commons.lang3.StringUtils.isBlank; /* @@ -22,13 +29,6 @@ import static org.apache.commons.lang3.StringUtils.isBlank; * #L% */ -import java.util.ArrayList; -import java.util.StringTokenizer; - -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.model.api.IQueryParameterOr; -import ca.uhn.fhir.model.api.IQueryParameterType; - public class QualifiedParamList extends ArrayList { private static final long serialVersionUID = 1L; @@ -83,16 +83,16 @@ public class QualifiedParamList extends ArrayList { prev = null; continue; } - + if (str.equals(",")) { - if (countTrailingSlashes(prev) % 2 == 1) { - int idx = retVal.size() - 1; - String existing = retVal.get(idx); - prev = existing.substring(0, existing.length() - 1) + ','; - retVal.set(idx, prev); - } else { - prev = null; - } + if (countTrailingSlashes(prev) % 2 == 1) { + int idx = retVal.size() - 1; + String existing = retVal.get(idx); + prev = existing.substring(0, existing.length() - 1) + ','; + retVal.set(idx, prev); + } else { + prev = null; + } continue; } @@ -108,11 +108,18 @@ public class QualifiedParamList extends ArrayList { } + // If no value was found, at least add that empty string as a value. It should get ignored later, but at + // least this lets us give a sensible error message if the parameter name was bad. See + // ResourceProviderR4Test#testParameterWithNoValueThrowsError_InvalidChainOnCustomSearch for an example + if (retVal.size() == 0) { + retVal.add(""); + } + return retVal; } private static int countTrailingSlashes(String theString) { - if(theString==null) { + if (theString == null) { return 0; } int retVal = 0; 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 ec9c8c621dd..db7cd9497fb 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 @@ -559,11 +559,11 @@ public abstract class BaseHapiFhirResourceDao extends B myEntityManager.merge(entity); } - private void validateExpungeEnabled() { - if (!myDaoConfig.isExpungeEnabled()) { - throw new MethodNotAllowedException("$expunge is not enabled on this server"); - } - } + private void validateExpungeEnabled() { + if (!myDaoConfig.isExpungeEnabled()) { + throw new MethodNotAllowedException("$expunge is not enabled on this server"); + } + } @Override @Transactional(propagation = Propagation.NEVER) @@ -892,8 +892,8 @@ public abstract class BaseHapiFhirResourceDao extends B boolean hasExternalizedBinaryReference = ((IBaseHasExtensions) nextBase64) .getExtension() .stream() - .filter(t-> t.getUserData(JpaConstants.EXTENSION_EXT_SYSTEMDEFINED) == null) - .anyMatch(t-> t.getUrl().equals(EXT_EXTERNALIZED_BINARY_ID)); + .filter(t -> t.getUserData(JpaConstants.EXTENSION_EXT_SYSTEMDEFINED) == null) + .anyMatch(t -> t.getUrl().equals(EXT_EXTERNALIZED_BINARY_ID)); if (hasExternalizedBinaryReference) { String msg = getContext().getLocalizer().getMessage(BaseHapiFhirDao.class, "externalizedBinaryStorageExtensionFoundInRequestBody", EXT_EXTERNALIZED_BINARY_ID); throw new InvalidRequestException(msg); @@ -1326,12 +1326,10 @@ public abstract class BaseHapiFhirResourceDao extends B RuntimeSearchParam paramDef = mySearchParamRegistry.getSearchParamByName(resourceDef, qualifiedParamName.getParamName()); for (String nextValue : theSource.get(nextParamName)) { - if (isNotBlank(nextValue)) { - QualifiedParamList qualifiedParam = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(qualifiedParamName.getWholeQualifier(), nextValue); - List paramList = Collections.singletonList(qualifiedParam); - IQueryParameterAnd parsedParam = ParameterUtil.parseQueryParams(getContext(), paramDef, nextParamName, paramList); - theTarget.add(qualifiedParamName.getParamName(), parsedParam); - } + QualifiedParamList qualifiedParam = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(qualifiedParamName.getWholeQualifier(), nextValue); + List paramList = Collections.singletonList(qualifiedParam); + IQueryParameterAnd parsedParam = ParameterUtil.parseQueryParams(getContext(), paramDef, nextParamName, paramList); + theTarget.add(qualifiedParamName.getParamName(), parsedParam); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/validation/JpaValidationSupportChainR5.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/validation/JpaValidationSupportChainR5.java index 21ffcb84df7..e1094f9a585 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/validation/JpaValidationSupportChainR5.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/validation/JpaValidationSupportChainR5.java @@ -51,7 +51,8 @@ public class JpaValidationSupportChainR5 extends ValidationSupportChain { public JpaValidationSupportChainR5() { super(); } - + + @PreDestroy public void flush() { myDefaultProfileValidationSupport.flush(); } @@ -83,10 +84,4 @@ public class JpaValidationSupportChainR5 extends ValidationSupportChain { addValidationSupport(new SnapshotGeneratingValidationSupport(myFhirContext, this)); } - @PreDestroy - public void preDestroy() { - flush(); - } - - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java index 1b4c239f66c..3d1bc8d6e33 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java @@ -1,8 +1,13 @@ package ca.uhn.fhir.jpa.dao.r4; +import ca.uhn.fhir.interceptor.api.Hook; +import ca.uhn.fhir.interceptor.api.HookParams; +import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor; +import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken; +import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.rest.api.server.IBundleProvider; @@ -10,11 +15,13 @@ import ca.uhn.fhir.rest.param.*; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.util.TestUtil; +import org.hamcrest.Matchers; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.Appointment.AppointmentStatus; import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender; import org.junit.*; +import org.mockito.ArgumentCaptor; import org.mockito.internal.util.collections.ListUtil; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; @@ -23,8 +30,11 @@ import org.springframework.transaction.support.TransactionTemplate; import java.util.List; +import static org.hamcrest.Matchers.startsWith; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4SearchCustomSearchParamTest.class); @@ -181,7 +191,7 @@ public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test IBundleProvider outcome = myPatientDao.search(params); List ids = toUnqualifiedVersionlessIdValues(outcome); ourLog.info("IDS: " + ids); - assertThat(ids, contains(pid.getValue())); + assertThat(ids, Matchers.contains(pid.getValue())); } @@ -1326,6 +1336,39 @@ public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test } + + @Test + public void testCompositeWithInvalidTarget() { + SearchParameter sp = new SearchParameter(); + sp.addBase("Patient"); + sp.setCode("myDoctor"); + sp.setType(Enumerations.SearchParamType.COMPOSITE); + sp.setTitle("My Doctor"); + sp.setStatus(org.hl7.fhir.r4.model.Enumerations.PublicationStatus.ACTIVE); + sp.addComponent() + .setDefinition("http://foo"); + mySearchParameterDao.create(sp); + + IAnonymousInterceptor interceptor = mock(IAnonymousInterceptor.class); + myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.JPA_PERFTRACE_WARNING, interceptor); + + try { + mySearchParamRegistry.forceRefresh(); + + ArgumentCaptor paramsCaptor = ArgumentCaptor.forClass(HookParams.class); + verify(interceptor, times(1)).invoke(any(), paramsCaptor.capture()); + + StorageProcessingMessage msg = paramsCaptor.getValue().get(StorageProcessingMessage.class); + assertThat(msg.getMessage(), containsString("refers to unknown component foo, ignoring this parameter")); + + } finally { + myInterceptorRegistry.unregisterInterceptor(interceptor); + } + } + + + + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java index 55361aed6d1..960f5f95bd2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java @@ -116,6 +116,48 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); } + @Test + public void testParameterWithNoValueThrowsError_InvalidChainOnCustomSearch() throws IOException { + SearchParameter searchParameter = new SearchParameter(); + searchParameter.addBase("BodySite").addBase("Procedure"); + searchParameter.setCode("focalAccess"); + searchParameter.setType(Enumerations.SearchParamType.REFERENCE); + searchParameter.setExpression("Procedure.extension('Procedure#focalAccess')"); + searchParameter.setXpathUsage(SearchParameter.XPathUsageType.NORMAL); + searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); + ourClient.create().resource(searchParameter).execute(); + + mySearchParamRegistry.forceRefresh(); + + HttpGet get = new HttpGet(ourServerBase + "/Procedure?focalAccess.a%20ne%20e"); + try (CloseableHttpResponse resp = ourHttpClient.execute(get)) { + String output = IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8); + assertThat(output, containsString("Invalid parameter chain: focalAccess.a ne e")); + assertEquals(400, resp.getStatusLine().getStatusCode()); + } + } + + @Test + public void testParameterWithNoValueThrowsError_InvalidRootParam() throws IOException { + SearchParameter searchParameter = new SearchParameter(); + searchParameter.addBase("BodySite").addBase("Procedure"); + searchParameter.setCode("focalAccess"); + searchParameter.setType(Enumerations.SearchParamType.REFERENCE); + searchParameter.setExpression("Procedure.extension('Procedure#focalAccess')"); + searchParameter.setXpathUsage(SearchParameter.XPathUsageType.NORMAL); + searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); + ourClient.create().resource(searchParameter).execute(); + + mySearchParamRegistry.forceRefresh(); + + HttpGet get = new HttpGet(ourServerBase + "/Procedure?a"); + try (CloseableHttpResponse resp = ourHttpClient.execute(get)) { + String output = IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8); + assertThat(output, containsString("Unknown search parameter "a"")); + assertEquals(400, resp.getStatusLine().getStatusCode()); + } + } + @Test public void testSearchForTokenValueOnlyUsesValueHash() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImplTest.java index a11ad70f804..615ecb143ce 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImplTest.java @@ -239,6 +239,24 @@ public class ResourceReindexingSvcImplTest extends BaseJpaTest { verifyNoMoreInteractions(myReindexJobDao); } + @Test + public void testReindexThrowsError() { + mockNothingToExpunge(); + mockSingleReindexingJob("Patient"); + List values = Arrays.asList(0L, 1L, 2L, 3L); + when(myResourceTableDao.findIdsOfResourcesWithinUpdatedRangeOrderedFromOldest(myPageRequestCaptor.capture(), myTypeCaptor.capture(), myLowCaptor.capture(), myHighCaptor.capture())).thenReturn(new SliceImpl<>(values)); + when(myResourceTableDao.findById(anyLong())).thenThrow(new NullPointerException("A MESSAGE")); + + int count = mySvc.forceReindexingPass(); + assertEquals(0, count); + + // Make sure we didn't do anything unexpected + verify(myReindexJobDao, times(1)).findAll(any(), eq(false)); + verify(myReindexJobDao, times(1)).findAll(any(), eq(true)); + verify(myReindexJobDao, times(1)).setSuspendedUntil(any()); + verifyNoMoreInteractions(myReindexJobDao); + } + private void mockWhenResourceTableFindById(long[] theUpdatedTimes, String[] theResourceTypes) { when(myResourceTableDao.findById(any())).thenAnswer(t -> { ResourceTable retVal = new ResourceTable(); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java index 70950c52bc8..61770fc83c5 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParams.java @@ -149,8 +149,7 @@ public final class ResourceIndexedSearchParams { * @param theResourceType E.g. Patient * @param thePartsChoices E.g. [[gender=male], [name=SMITH, name=JOHN]] */ - public static Set extractCompositeStringUniquesValueChains(String - theResourceType, List> thePartsChoices) { + public static Set extractCompositeStringUniquesValueChains(String theResourceType, List> thePartsChoices) { for (List next : thePartsChoices) { next.removeIf(StringUtils::isBlank); diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java index aecf2227d02..252834ab82c 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java @@ -3,14 +3,17 @@ package ca.uhn.fhir.jpa.searchparam.extractor; import ca.uhn.fhir.jpa.model.entity.ForcedId; import ca.uhn.fhir.jpa.model.entity.ResourceLink; import ca.uhn.fhir.jpa.model.entity.ResourceTable; -import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.param.ReferenceParam; -import org.hl7.fhir.instance.model.api.IIdType; +import com.google.common.collect.Lists; import org.junit.Before; import org.junit.Test; import java.util.Date; +import java.util.List; +import java.util.Set; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; import static org.junit.Assert.*; public class ResourceIndexedSearchParamsTest { @@ -79,4 +82,30 @@ public class ResourceIndexedSearchParamsTest { return retval; } + + @Test + public void testExtractCompositeStringUniquesValueChains() { + List> partsChoices; + Set values; + + partsChoices = Lists.newArrayList( + Lists.newArrayList("gender=male"), + Lists.newArrayList("name=SMITH", "name=JOHN") + ); + values = ResourceIndexedSearchParams.extractCompositeStringUniquesValueChains("Patient", partsChoices); + assertThat(values.toString(), values, containsInAnyOrder("Patient?gender=male&name=JOHN","Patient?gender=male&name=SMITH")); + + partsChoices = Lists.newArrayList( + Lists.newArrayList("gender=male", ""), + Lists.newArrayList("name=SMITH", "name=JOHN", "") + ); + values = ResourceIndexedSearchParams.extractCompositeStringUniquesValueChains("Patient", partsChoices); + assertThat(values.toString(), values, containsInAnyOrder("Patient?gender=male&name=JOHN","Patient?gender=male&name=SMITH")); + + partsChoices = Lists.newArrayList( + ); + values = ResourceIndexedSearchParams.extractCompositeStringUniquesValueChains("Patient", partsChoices); + assertThat(values.toString(), values, empty()); + } + } diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoader.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoader.java index 339d8a0117b..1f5e4ae998b 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoader.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoader.java @@ -83,6 +83,12 @@ public class SubscriptionLoader { } } + @VisibleForTesting + void acquireSemaphoreForUnitTest() throws InterruptedException { + mySyncSubscriptionsSemaphore.acquire(); + } + + @PostConstruct public void registerScheduledJob() { ScheduledJobDefinition jobDetail = new ScheduledJobDefinition(); diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/SubscriptionLoaderTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoaderTest.java similarity index 78% rename from hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/SubscriptionLoaderTest.java rename to hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoaderTest.java index 929563099b7..04cbcaafd6e 100755 --- a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/SubscriptionLoaderTest.java +++ b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionLoaderTest.java @@ -1,6 +1,8 @@ -package ca.uhn.fhir.jpa.subscription.module.standalone; +package ca.uhn.fhir.jpa.subscription.module.cache; +import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionLoader; import ca.uhn.fhir.jpa.subscription.module.config.MockFhirClientSubscriptionProvider; +import ca.uhn.fhir.jpa.subscription.module.standalone.BaseBlockingQueueSubscribableChannelDstu3Test; import org.hl7.fhir.dstu3.model.Subscription; import org.junit.After; import org.junit.Before; @@ -43,4 +45,13 @@ public class SubscriptionLoaderTest extends BaseBlockingQueueSubscribableChannel mySubscriptionActivatedPost.awaitExpected(); assertEquals(0, myMockFhirClientSubscriptionProvider.getFailCount()); } + + + @Test + public void testMultipleThreadsDontBlock() throws InterruptedException { + SubscriptionLoader svc = new SubscriptionLoader(); + svc.acquireSemaphoreForUnitTest(); + svc.syncSubscriptions(); + } + } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java index 53844569823..3976e8fadf8 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.rest.server.method; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.model.api.IResource; @@ -103,7 +104,8 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi // type let's grab it if (!Modifier.isAbstract(theReturnResourceType.getModifiers()) && !Modifier.isInterface(theReturnResourceType.getModifiers())) { Class resourceType = (Class) theReturnResourceType; - myResourceName = theContext.getResourceDefinition(resourceType).getName(); + RuntimeResourceDefinition resourceDefinition = theContext.getResourceDefinition(resourceType); + myResourceName = resourceDefinition.getName(); } } } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java new file mode 100644 index 00000000000..adeb846f749 --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java @@ -0,0 +1,170 @@ +package ca.uhn.fhir.rest.server.method; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.model.api.annotation.ResourceDef; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.annotation.IdParam; +import ca.uhn.fhir.rest.annotation.Read; +import ca.uhn.fhir.rest.api.RequestTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import org.hl7.fhir.instance.model.api.IBaseMetaType; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.lang.reflect.Method; +import java.util.List; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@SuppressWarnings("unchecked") +@RunWith(MockitoJUnitRunner.class) +public class ReadMethodBindingTest { + + @Mock + private FhirContext myCtx; + + @Mock + private RequestDetails myRequestDetails; + + @Mock + private RuntimeResourceDefinition definition; + + @Test + public void testIncomingServerRequestMatchesMethod_Read() throws NoSuchMethodException { + + class MyProvider { + @Read(version = false) + public IBaseResource read(@IdParam IIdType theIdType) { + return null; + } + } + + when(myCtx.getResourceDefinition(any(Class.class))).thenReturn(definition); + when(definition.getName()).thenReturn("Patient"); + when(myRequestDetails.getResourceName()).thenReturn("Patient"); + when(myRequestDetails.getRequestType()).thenReturn(RequestTypeEnum.GET); + + // Read + ReadMethodBinding binding = createBinding(new MyProvider()); + when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); + assertTrue(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + + // VRead + when(myRequestDetails.getResourceName()).thenReturn("Patient"); + when(myRequestDetails.getOperation()).thenReturn("_history"); + assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + + } + + @Test + public void testIncomingServerRequestMatchesMethod_VRead() throws NoSuchMethodException { + + class MyProvider { + @Read(version = true) + public IBaseResource read(@IdParam IIdType theIdType) { + return null; + } + } + + when(myCtx.getResourceDefinition(any(Class.class))).thenReturn(definition); + when(definition.getName()).thenReturn("Patient"); + when(myRequestDetails.getResourceName()).thenReturn("Patient"); + when(myRequestDetails.getRequestType()).thenReturn(RequestTypeEnum.GET); + + // Read - wrong resource type + ReadMethodBinding binding = createBinding(new MyProvider()); + when(myRequestDetails.getResourceName()).thenReturn("Observation"); + when(myRequestDetails.getId()).thenReturn(new IdDt("Observation/123")); + assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + + // Read + when(myRequestDetails.getResourceName()).thenReturn("Patient"); + when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); + assertTrue(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + + // VRead + when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123/_history/123")); + when(myRequestDetails.getOperation()).thenReturn("_history"); + assertTrue(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + + // Some other operation + when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123/_history/123")); + when(myRequestDetails.getOperation()).thenReturn("$foo"); + assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + + } + + public ReadMethodBinding createBinding(Object theProvider) throws NoSuchMethodException { + Method method = theProvider.getClass().getMethod("read", IIdType.class); + return new ReadMethodBinding(MyPatient.class, method, myCtx, theProvider); + } + + @ResourceDef(name = "Patient") + private static class MyPatient implements IBaseResource { + + @Override + public IBaseMetaType getMeta() { + return null; + } + + @Override + public IIdType getIdElement() { + return null; + } + + @Override + public IBaseResource setId(String theId) { + return null; + } + + @Override + public IBaseResource setId(IIdType theId) { + return null; + } + + @Override + public FhirVersionEnum getStructureFhirVersionEnum() { + return null; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public boolean hasFormatComment() { + return false; + } + + @Override + public List getFormatCommentsPre() { + return null; + } + + @Override + public List getFormatCommentsPost() { + return null; + } + + @Override + public Object getUserData(String theName) { + return null; + } + + @Override + public void setUserData(String theName, Object theValue) { + + } + } + +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b1726af62dc..94800269bb0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -433,6 +433,12 @@ JavaScript dependency libraries. This reduces our Git repository size and should make it easier to stay up-to-date. + + In the (fairly unlikely) circumstance that a JPA server was called with a parameter where the parameter name referenced + a custom search parameter with an invalid chain attached, and the value was missing entirely (e.g. + ProcedureRequest?someCustomParameter.BAD_NAME=]]>, the server would ignore this + parameter instead of incorrectly returning an error. This has been corrected. +