From de701a6b7f8fb719c35cae3dc86dd0c9aabf4a5c Mon Sep 17 00:00:00 2001 From: Alin Date: Mon, 9 Jul 2018 15:35:50 +0300 Subject: [PATCH 01/22] added patch for operation outcome --- .../rest/server/method/BaseOutcomeReturningMethodBinding.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java index ea92f5c5b79..407c4fb2df1 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java @@ -48,7 +48,7 @@ import java.util.Set; abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding { static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseOutcomeReturningMethodBinding.class); - private static EnumSet ourOperationsWhichAllowPreferHeader = EnumSet.of(RestOperationTypeEnum.CREATE, RestOperationTypeEnum.UPDATE); + private static EnumSet ourOperationsWhichAllowPreferHeader = EnumSet.of(RestOperationTypeEnum.CREATE, RestOperationTypeEnum.UPDATE, RestOperationTypeEnum.PATCH); private boolean myReturnVoid; @@ -91,11 +91,11 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding Date: Mon, 9 Jul 2018 16:23:04 +0300 Subject: [PATCH 02/22] fixed the testWritePatchByInstance return code based on fhir doc --- .../interceptor/auth/AuthorizationInterceptorDstu2Test.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java index fe70c2e20bb..3fd96f88573 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java @@ -1786,7 +1786,7 @@ public class AuthorizationInterceptorDstu2Test { httpPost.setEntity(new StringEntity(input, ContentType.parse("application/json-patch+json"))); status = ourClient.execute(httpPost); response = extractResponseAndClose(status); - assertEquals(204, status.getStatusLine().getStatusCode()); + assertEquals(200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); } From 62c7c0fa55c8c9c6545a87519edcb04899e5a859 Mon Sep 17 00:00:00 2001 From: Alin Date: Mon, 9 Jul 2018 16:43:31 +0300 Subject: [PATCH 03/22] fixed the AuthorizationInterceptorR4Test return code based on fhir doc --- .../server/interceptor/AuthorizationInterceptorR4Test.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java index 33ad4feabb9..5df78a7ba23 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java @@ -1647,7 +1647,7 @@ public class AuthorizationInterceptorR4Test { patch.setEntity(new StringEntity(patchBody, ContentType.create(Constants.CT_JSON_PATCH, Charsets.UTF_8))); CloseableHttpResponse status = ourClient.execute(patch); extractResponseAndClose(status); - assertEquals(204, status.getStatusLine().getStatusCode()); + assertEquals(200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); } @@ -2924,7 +2924,7 @@ public class AuthorizationInterceptorR4Test { httpPost.setEntity(new StringEntity(input, ContentType.parse("application/json-patch+json"))); status = ourClient.execute(httpPost); extractResponseAndClose(status); - assertEquals(204, status.getStatusLine().getStatusCode()); + assertEquals(200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); } From 6586fc438da169bcdf5f87c98fb31c504bc5b3eb Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Fri, 3 Aug 2018 13:00:57 -0400 Subject: [PATCH 04/22] Fix concurrency issue in hashmap --- .../server/provider/HashMapResourceProvider.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java index 33529801272..100e8642826 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.server.provider; * 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. @@ -65,8 +65,8 @@ public class HashMapResourceProvider implements IResour private final Class myResourceType; private final FhirContext myFhirContext; private final String myResourceName; - protected Map> myIdToVersionToResourceMap = new LinkedHashMap<>(); - protected Map> myIdToHistory = new LinkedHashMap<>(); + protected Map> myIdToVersionToResourceMap = Collections.synchronizedMap(new LinkedHashMap<>()); + protected Map> myIdToHistory = Collections.synchronizedMap(new LinkedHashMap<>()); protected LinkedList myTypeHistory = new LinkedList<>(); private long myNextId; private AtomicLong myDeleteCount = new AtomicLong(0); @@ -188,9 +188,7 @@ public class HashMapResourceProvider implements IResour } private synchronized TreeMap getVersionToResource(String theIdPart) { - if (!myIdToVersionToResourceMap.containsKey(theIdPart)) { - myIdToVersionToResourceMap.put(theIdPart, new TreeMap<>()); - } + myIdToVersionToResourceMap.computeIfAbsent(theIdPart, t -> new TreeMap<>()); return myIdToVersionToResourceMap.get(theIdPart); } @@ -333,9 +331,7 @@ public class HashMapResourceProvider implements IResour myTypeHistory.addFirst(theResource); // Store to ID history map - if (!myIdToHistory.containsKey(theIdPart)) { - myIdToHistory.put(theIdPart, new LinkedList<>()); - } + myIdToHistory.computeIfAbsent(theIdPart, t -> new LinkedList<>()); myIdToHistory.get(theIdPart).addFirst(theResource); // Return the newly assigned ID including the version ID From c98a1e0c6297a7b783cf1b3b6b4a7189ca786e68 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Mon, 6 Aug 2018 18:37:12 -0400 Subject: [PATCH 05/22] Cleanup of subscription processing --- .../BaseSubscriptionDeliverySubscriber.java | 4 ++-- .../subscription/BaseSubscriptionInterceptor.java | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionDeliverySubscriber.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionDeliverySubscriber.java index ac85562506a..461cb3a798b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionDeliverySubscriber.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionDeliverySubscriber.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription; * 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. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java index 58a01a2b4ac..3ac6b74f99f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java @@ -351,12 +351,16 @@ public abstract class BaseSubscriptionInterceptor exten protected abstract void registerDeliverySubscriber(); - public void registerSubscription(IIdType theId, S theSubscription) { + @SuppressWarnings("UnusedReturnValue") + public CanonicalSubscription registerSubscription(IIdType theId, S theSubscription) { Validate.notNull(theId); Validate.notBlank(theId.getIdPart()); Validate.notNull(theSubscription); - myIdToSubscription.put(theId.getIdPart(), canonicalize(theSubscription)); + CanonicalSubscription canonicalized = canonicalize(theSubscription); + myIdToSubscription.put(theId.getIdPart(), canonicalized); + + return canonicalized; } protected void registerSubscriptionCheckingSubscriber() { @@ -525,11 +529,12 @@ public abstract class BaseSubscriptionInterceptor exten protected abstract void unregisterDeliverySubscriber(); - public void unregisterSubscription(IIdType theId) { + @SuppressWarnings("UnusedReturnValue") + public CanonicalSubscription unregisterSubscription(IIdType theId) { Validate.notNull(theId); Validate.notBlank(theId.getIdPart()); - myIdToSubscription.remove(theId.getIdPart()); + return myIdToSubscription.remove(theId.getIdPart()); } From d59a40d01a5f90b6bdf5d70543e181588d3c3984 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Mon, 6 Aug 2018 19:03:30 -0400 Subject: [PATCH 06/22] Subscription cleanup started (won't build) --- .../BaseSubscriptionInterceptor.java | 97 ++++++++++--------- 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java index 3ac6b74f99f..27ccc048c39 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java @@ -72,16 +72,17 @@ public abstract class BaseSubscriptionInterceptor exten static final String SUBSCRIPTION_TYPE = "Subscription.channel.type"; private static final Integer MAX_SUBSCRIPTION_RESULTS = 1000; private SubscribableChannel myProcessingChannel; - private SubscribableChannel myDeliveryChannel; + private Map myDeliveryChannel; private ExecutorService myProcessingExecutor; private int myExecutorThreadCount; private SubscriptionActivatingSubscriber mySubscriptionActivatingSubscriber; private MessageHandler mySubscriptionCheckingSubscriber; private ConcurrentHashMap myIdToSubscription = new ConcurrentHashMap<>(); + private ConcurrentHashMap myIdToSubscribaleChannel = new ConcurrentHashMap<>(); + private ConcurrentHashMap myIdToDeliveryHandler = new ConcurrentHashMap<>(); private Logger ourLog = LoggerFactory.getLogger(BaseSubscriptionInterceptor.class); private ThreadPoolExecutor myDeliveryExecutor; private LinkedBlockingQueue myProcessingExecutorQueue; - private LinkedBlockingQueue myDeliveryExecutorQueue; private IFhirResourceDao mySubscriptionDao; @Autowired private List> myResourceDaos; @@ -136,6 +137,43 @@ public abstract class BaseSubscriptionInterceptor exten return retVal; } + protected abstract MessageHandler createDeliveryHandler(CanonicalSubscription theSubscription); + + protected SubscribableChannel createDeliveryChannel(CanonicalSubscription theSubscription) { + String subscriptionId = theSubscription.getIdElement(myCtx).getIdPart(); + + LinkedBlockingQueue executorQueue = new LinkedBlockingQueue<>(1000); + BasicThreadFactory threadFactory = new BasicThreadFactory.Builder() + .namingPattern("subscription-delivery-" + subscriptionId + "-%d") + .daemon(false) + .priority(Thread.NORM_PRIORITY) + .build(); + RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { + @Override + public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { + ourLog.info("Note: Executor queue is full ({} elements), waiting for a slot to become available!", executorQueue.size()); + StopWatch sw = new StopWatch(); + try { + executorQueue.put(theRunnable); + } catch (InterruptedException theE) { + throw new RejectedExecutionException("Task " + theRunnable.toString() + + " rejected from " + theE.toString()); + } + ourLog.info("Slot become available after {}ms", sw.getMillis()); + } + }; + ThreadPoolExecutor deliveryExecutor = new ThreadPoolExecutor( + 1, + getExecutorThreadCount(), + 0L, + TimeUnit.MILLISECONDS, + executorQueue, + threadFactory, + rejectedExecutionHandler); + + return new ExecutorSubscribableChannel(deliveryExecutor); + } + protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) { org.hl7.fhir.dstu3.model.Subscription subscription = (org.hl7.fhir.dstu3.model.Subscription) theSubscription; @@ -255,16 +293,8 @@ public abstract class BaseSubscriptionInterceptor exten return (IFhirResourceDao) myResourceTypeToDao.get(theType); } - public SubscribableChannel getDeliveryChannel() { - return myDeliveryChannel; - } - - public void setDeliveryChannel(SubscribableChannel theDeliveryChannel) { - myDeliveryChannel = theDeliveryChannel; - } - public int getExecutorQueueSizeForUnitTests() { - return myProcessingExecutorQueue.size() + myDeliveryExecutorQueue.size(); + return myProcessingExecutorQueue.size(); } public int getExecutorThreadCount() { @@ -332,11 +362,11 @@ public abstract class BaseSubscriptionInterceptor exten mySubscriptionActivatingSubscriber.activateAndRegisterSubscriptionIfRequired(resource); } - for (Enumeration keyEnum = myIdToSubscription.keys(); keyEnum.hasMoreElements(); ) { - String next = keyEnum.nextElement(); + for (String next : new ArrayList<>(myIdToSubscription.keySet())) { if (!allIds.contains(next)) { ourLog.info("Unregistering Subscription/{} as it no longer exists", next); - myIdToSubscription.remove(next); + CanonicalSubscription subscription = myIdToSubscription.get(next); + unregisterSubscription(subscription.getIdElement(myCtx)); } } } @@ -357,8 +387,16 @@ public abstract class BaseSubscriptionInterceptor exten Validate.notBlank(theId.getIdPart()); Validate.notNull(theSubscription); + CanonicalSubscription canonicalized = canonicalize(theSubscription); + SubscribableChannel deliveryChannel = createDeliveryChannel(canonicalized); + MessageHandler deliveryHandler = createDeliveryHandler(canonicalized); + + deliveryChannel.subscribe(deliveryHandler); + + myIdToSubscribaleChannel.put(theId.getIdPart(), deliveryChannel); myIdToSubscription.put(theId.getIdPart(), canonicalized); + myIdToDeliveryHandler.put(theId.getIdPart(), deliveryHandler); return canonicalized; } @@ -474,37 +512,6 @@ public abstract class BaseSubscriptionInterceptor exten setProcessingChannel(new ExecutorSubscribableChannel(myProcessingExecutor)); } - if (getDeliveryChannel() == null) { - myDeliveryExecutorQueue = new LinkedBlockingQueue<>(1000); - BasicThreadFactory threadFactory = new BasicThreadFactory.Builder() - .namingPattern("subscription-delivery-%d") - .daemon(false) - .priority(Thread.NORM_PRIORITY) - .build(); - RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { - @Override - public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { - ourLog.info("Note: Executor queue is full ({} elements), waiting for a slot to become available!", myDeliveryExecutorQueue.size()); - StopWatch sw = new StopWatch(); - try { - myDeliveryExecutorQueue.put(theRunnable); - } catch (InterruptedException theE) { - throw new RejectedExecutionException("Task " + theRunnable.toString() + - " rejected from " + theE.toString()); - } - ourLog.info("Slot become available after {}ms", sw.getMillis()); - } - }; - myDeliveryExecutor = new ThreadPoolExecutor( - 1, - getExecutorThreadCount(), - 0L, - TimeUnit.MILLISECONDS, - myDeliveryExecutorQueue, - threadFactory, - rejectedExecutionHandler); - setDeliveryChannel(new ExecutorSubscribableChannel(myDeliveryExecutor)); - } if (mySubscriptionActivatingSubscriber == null) { mySubscriptionActivatingSubscriber = new SubscriptionActivatingSubscriber(getSubscriptionDao(), getChannelType(), this, myTxManager, myAsyncTaskExecutor); From ec4604c498835f674178ad8568d881493dad1dc3 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 6 Aug 2018 20:58:04 -0400 Subject: [PATCH 07/22] Work on splitting subscriptions out into separate channels --- .../BaseSubscriptionInterceptor.java | 174 ++++++++++-------- .../SubscriptionCheckingSubscriber.java | 8 +- .../email/SubscriptionEmailInterceptor.java | 32 +--- .../SubscriptionRestHookInterceptor.java | 20 +- .../SubscriptionWebsocketHandler.java | 6 +- .../SubscriptionWebsocketInterceptor.java | 29 ++- .../r4/WebsocketWithSubscriptionIdR4Test.java | 2 + 7 files changed, 142 insertions(+), 129 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java index 27ccc048c39..690acc5ccf8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription; * 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. @@ -38,6 +38,9 @@ import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import ca.uhn.fhir.rest.server.interceptor.ServerOperationInterceptorAdapter; import ca.uhn.fhir.util.StopWatch; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.concurrent.BasicThreadFactory; import org.hl7.fhir.exceptions.FHIRException; @@ -50,6 +53,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.task.AsyncTaskExecutor; +import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessageHandler; import org.springframework.messaging.SubscribableChannel; import org.springframework.messaging.support.ExecutorSubscribableChannel; @@ -79,7 +83,7 @@ public abstract class BaseSubscriptionInterceptor exten private MessageHandler mySubscriptionCheckingSubscriber; private ConcurrentHashMap myIdToSubscription = new ConcurrentHashMap<>(); private ConcurrentHashMap myIdToSubscribaleChannel = new ConcurrentHashMap<>(); - private ConcurrentHashMap myIdToDeliveryHandler = new ConcurrentHashMap<>(); + private Multimap myIdToDeliveryHandler = Multimaps.synchronizedListMultimap(ArrayListMultimap.create()); private Logger ourLog = LoggerFactory.getLogger(BaseSubscriptionInterceptor.class); private ThreadPoolExecutor myDeliveryExecutor; private LinkedBlockingQueue myProcessingExecutorQueue; @@ -137,43 +141,6 @@ public abstract class BaseSubscriptionInterceptor exten return retVal; } - protected abstract MessageHandler createDeliveryHandler(CanonicalSubscription theSubscription); - - protected SubscribableChannel createDeliveryChannel(CanonicalSubscription theSubscription) { - String subscriptionId = theSubscription.getIdElement(myCtx).getIdPart(); - - LinkedBlockingQueue executorQueue = new LinkedBlockingQueue<>(1000); - BasicThreadFactory threadFactory = new BasicThreadFactory.Builder() - .namingPattern("subscription-delivery-" + subscriptionId + "-%d") - .daemon(false) - .priority(Thread.NORM_PRIORITY) - .build(); - RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { - @Override - public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { - ourLog.info("Note: Executor queue is full ({} elements), waiting for a slot to become available!", executorQueue.size()); - StopWatch sw = new StopWatch(); - try { - executorQueue.put(theRunnable); - } catch (InterruptedException theE) { - throw new RejectedExecutionException("Task " + theRunnable.toString() + - " rejected from " + theE.toString()); - } - ourLog.info("Slot become available after {}ms", sw.getMillis()); - } - }; - ThreadPoolExecutor deliveryExecutor = new ThreadPoolExecutor( - 1, - getExecutorThreadCount(), - 0L, - TimeUnit.MILLISECONDS, - executorQueue, - threadFactory, - rejectedExecutionHandler); - - return new ExecutorSubscribableChannel(deliveryExecutor); - } - protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) { org.hl7.fhir.dstu3.model.Subscription subscription = (org.hl7.fhir.dstu3.model.Subscription) theSubscription; @@ -272,6 +239,46 @@ public abstract class BaseSubscriptionInterceptor exten return retVal; } + protected SubscribableChannel createDeliveryChannel(CanonicalSubscription theSubscription) { + String subscriptionId = theSubscription.getIdElement(myCtx).getIdPart(); + + LinkedBlockingQueue executorQueue = new LinkedBlockingQueue<>(1000); + BasicThreadFactory threadFactory = new BasicThreadFactory.Builder() + .namingPattern("subscription-delivery-" + subscriptionId + "-%d") + .daemon(false) + .priority(Thread.NORM_PRIORITY) + .build(); + RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { + @Override + public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { + ourLog.info("Note: Executor queue is full ({} elements), waiting for a slot to become available!", executorQueue.size()); + StopWatch sw = new StopWatch(); + try { + executorQueue.put(theRunnable); + } catch (InterruptedException theE) { + throw new RejectedExecutionException("Task " + theRunnable.toString() + + " rejected from " + theE.toString()); + } + ourLog.info("Slot become available after {}ms", sw.getMillis()); + } + }; + ThreadPoolExecutor deliveryExecutor = new ThreadPoolExecutor( + 1, + getExecutorThreadCount(), + 0L, + TimeUnit.MILLISECONDS, + executorQueue, + threadFactory, + rejectedExecutionHandler); + + return new ExecutorSubscribableChannel(deliveryExecutor); + } + + /** + * Returns an empty handler if the interceptor will manually handle registration and unregistration + */ + protected abstract Optional createDeliveryHandler(CanonicalSubscription theSubscription); + public abstract Subscription.SubscriptionChannelType getChannelType(); @SuppressWarnings("unchecked") @@ -293,6 +300,10 @@ public abstract class BaseSubscriptionInterceptor exten return (IFhirResourceDao) myResourceTypeToDao.get(theType); } + protected MessageChannel getDeliveryChannel(CanonicalSubscription theSubscription) { + return myIdToSubscribaleChannel.get(theSubscription.getIdElement(myCtx).getIdPart()); + } + public int getExecutorQueueSizeForUnitTests() { return myProcessingExecutorQueue.size(); } @@ -362,41 +373,36 @@ public abstract class BaseSubscriptionInterceptor exten mySubscriptionActivatingSubscriber.activateAndRegisterSubscriptionIfRequired(resource); } - for (String next : new ArrayList<>(myIdToSubscription.keySet())) { - if (!allIds.contains(next)) { - ourLog.info("Unregistering Subscription/{} as it no longer exists", next); - CanonicalSubscription subscription = myIdToSubscription.get(next); - unregisterSubscription(subscription.getIdElement(myCtx)); - } - } + unregisterAllSubscriptionsNotInCollection(allIds); } @SuppressWarnings("unused") @PreDestroy public void preDestroy() { getProcessingChannel().unsubscribe(mySubscriptionCheckingSubscriber); - - unregisterDeliverySubscriber(); + unregisterAllSubscriptionsNotInCollection(Collections.emptyList()); } - protected abstract void registerDeliverySubscriber(); + public void registerHandler(String theSubscriptionId, MessageHandler theHandler) { + myIdToSubscribaleChannel.get(theSubscriptionId).subscribe(theHandler); + myIdToDeliveryHandler.put(theSubscriptionId, theHandler); + } @SuppressWarnings("UnusedReturnValue") public CanonicalSubscription registerSubscription(IIdType theId, S theSubscription) { Validate.notNull(theId); - Validate.notBlank(theId.getIdPart()); + String subscriptionId = theId.getIdPart(); + Validate.notBlank(subscriptionId); Validate.notNull(theSubscription); - CanonicalSubscription canonicalized = canonicalize(theSubscription); SubscribableChannel deliveryChannel = createDeliveryChannel(canonicalized); - MessageHandler deliveryHandler = createDeliveryHandler(canonicalized); + Optional deliveryHandler = createDeliveryHandler(canonicalized); - deliveryChannel.subscribe(deliveryHandler); + myIdToSubscribaleChannel.put(subscriptionId, deliveryChannel); + myIdToSubscription.put(subscriptionId, canonicalized); - myIdToSubscribaleChannel.put(theId.getIdPart(), deliveryChannel); - myIdToSubscription.put(theId.getIdPart(), canonicalized); - myIdToDeliveryHandler.put(theId.getIdPart(), deliveryHandler); + deliveryHandler.ifPresent(handler -> registerHandler(subscriptionId, handler)); return canonicalized; } @@ -482,19 +488,16 @@ public abstract class BaseSubscriptionInterceptor exten if (getProcessingChannel() == null) { myProcessingExecutorQueue = new LinkedBlockingQueue<>(1000); - RejectedExecutionHandler rejectedExecutionHandler = new RejectedExecutionHandler() { - @Override - public void rejectedExecution(Runnable theRunnable, ThreadPoolExecutor theExecutor) { - ourLog.info("Note: Executor queue is full ({} elements), waiting for a slot to become available!", myProcessingExecutorQueue.size()); - StopWatch sw = new StopWatch(); - try { - myProcessingExecutorQueue.put(theRunnable); - } catch (InterruptedException theE) { - throw new RejectedExecutionException("Task " + theRunnable.toString() + - " rejected from " + theE.toString()); - } - ourLog.info("Slot become available after {}ms", sw.getMillis()); + RejectedExecutionHandler rejectedExecutionHandler = (theRunnable, theExecutor) -> { + ourLog.info("Note: Executor queue is full ({} elements), waiting for a slot to become available!", myProcessingExecutorQueue.size()); + StopWatch sw = new StopWatch(); + try { + myProcessingExecutorQueue.put(theRunnable); + } catch (InterruptedException theE) { + throw new RejectedExecutionException("Task " + theRunnable.toString() + + " rejected from " + theE.toString()); } + ourLog.info("Slot become available after {}ms", sw.getMillis()); }; ThreadFactory threadFactory = new BasicThreadFactory.Builder() .namingPattern("subscription-proc-%d") @@ -512,13 +515,11 @@ public abstract class BaseSubscriptionInterceptor exten setProcessingChannel(new ExecutorSubscribableChannel(myProcessingExecutor)); } - if (mySubscriptionActivatingSubscriber == null) { mySubscriptionActivatingSubscriber = new SubscriptionActivatingSubscriber(getSubscriptionDao(), getChannelType(), this, myTxManager, myAsyncTaskExecutor); } registerSubscriptionCheckingSubscriber(); - registerDeliverySubscriber(); TransactionTemplate transactionTemplate = new TransactionTemplate(myTxManager); transactionTemplate.execute(new TransactionCallbackWithoutResult() { @@ -534,14 +535,39 @@ public abstract class BaseSubscriptionInterceptor exten sendToProcessingChannel(theMsg); } - protected abstract void unregisterDeliverySubscriber(); + private void unregisterAllSubscriptionsNotInCollection(Collection theAllIds) { + for (String next : new ArrayList<>(myIdToSubscription.keySet())) { + if (!theAllIds.contains(next)) { + ourLog.info("Unregistering Subscription/{}", next); + CanonicalSubscription subscription = myIdToSubscription.get(next); + unregisterSubscription(subscription.getIdElement(myCtx)); + } + } + } + + public void unregisterHandler(String theSubscriptionId, MessageHandler next) { + SubscribableChannel channel = myIdToSubscribaleChannel.get(theSubscriptionId); + if (channel != null) { + channel.unsubscribe(next); + } + + myIdToSubscribaleChannel.remove(theSubscriptionId, next); + } @SuppressWarnings("UnusedReturnValue") public CanonicalSubscription unregisterSubscription(IIdType theId) { Validate.notNull(theId); - Validate.notBlank(theId.getIdPart()); - return myIdToSubscription.remove(theId.getIdPart()); + String subscriptionId = theId.getIdPart(); + Validate.notBlank(subscriptionId); + + for (MessageHandler next : new ArrayList<>(myIdToDeliveryHandler.get(subscriptionId))) { + unregisterHandler(subscriptionId, next); + } + + myIdToSubscribaleChannel.remove(subscriptionId); + + return myIdToSubscription.remove(subscriptionId); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionCheckingSubscriber.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionCheckingSubscriber.java index af5678ce1b0..33424b7e15f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionCheckingSubscriber.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionCheckingSubscriber.java @@ -34,6 +34,7 @@ import org.hl7.fhir.r4.model.Subscription; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessagingException; import java.util.List; @@ -117,7 +118,12 @@ public class SubscriptionCheckingSubscriber extends BaseSubscriptionSubscriber { deliveryMsg.setPayloadId(msg.getId(getContext())); ResourceDeliveryJsonMessage wrappedMsg = new ResourceDeliveryJsonMessage(deliveryMsg); - getSubscriptionInterceptor().getDeliveryChannel().send(wrappedMsg); + MessageChannel deliveryChannel = getSubscriptionInterceptor().getDeliveryChannel(nextSubscription); + if (deliveryChannel != null) { + deliveryChannel.send(wrappedMsg); + } else { + ourLog.warn("Do not have deliovery channel for subscription {}", nextSubscription.getIdElement(getContext())); + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java index 560d4aa2145..35ff5fc1b64 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.email; * 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. @@ -21,13 +21,14 @@ package ca.uhn.fhir.jpa.subscription.email; */ import ca.uhn.fhir.jpa.subscription.BaseSubscriptionInterceptor; +import ca.uhn.fhir.jpa.subscription.CanonicalSubscription; import org.apache.commons.lang3.Validate; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.messaging.MessageHandler; -import javax.annotation.PostConstruct; +import java.util.Optional; public class SubscriptionEmailInterceptor extends BaseSubscriptionInterceptor { - private SubscriptionDeliveringEmailSubscriber mySubscriptionDeliverySubscriber; /** * This is set to autowired=false just so that implementors can supply this @@ -37,6 +38,11 @@ public class SubscriptionEmailInterceptor extends BaseSubscriptionInterceptor { private IEmailSender myEmailSender; private String myDefaultFromAddress = "noreply@unknown.com"; + @Override + protected Optional createDeliveryHandler(CanonicalSubscription theSubscription) { + return Optional.of(new SubscriptionDeliveringEmailSubscriber(getSubscriptionDao(), getChannelType(), this)); + } + @Override public org.hl7.fhir.r4.model.Subscription.SubscriptionChannelType getChannelType() { return org.hl7.fhir.r4.model.Subscription.SubscriptionChannelType.EMAIL; @@ -69,23 +75,5 @@ public class SubscriptionEmailInterceptor extends BaseSubscriptionInterceptor { myEmailSender = theEmailSender; } - @Override - protected void registerDeliverySubscriber() { - if (mySubscriptionDeliverySubscriber == null) { - mySubscriptionDeliverySubscriber = new SubscriptionDeliveringEmailSubscriber(getSubscriptionDao(), getChannelType(), this); - } - getDeliveryChannel().subscribe(mySubscriptionDeliverySubscriber); - } -// @PostConstruct -// public void start() { -// Validate.notNull(myEmailSender, "emailSender has not been configured"); -// -// super.start(); -// } - - @Override - protected void unregisterDeliverySubscriber() { - getDeliveryChannel().unsubscribe(mySubscriptionDeliverySubscriber); - } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java index 10cb72b981b..9c0558ba310 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.resthook; * 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. @@ -21,16 +21,16 @@ package ca.uhn.fhir.jpa.subscription.resthook; */ import ca.uhn.fhir.jpa.subscription.BaseSubscriptionInterceptor; +import ca.uhn.fhir.jpa.subscription.CanonicalSubscription; +import org.springframework.messaging.MessageHandler; + +import java.util.Optional; public class SubscriptionRestHookInterceptor extends BaseSubscriptionInterceptor { - private SubscriptionDeliveringRestHookSubscriber mySubscriptionDeliverySubscriber; @Override - protected void registerDeliverySubscriber() { - if (mySubscriptionDeliverySubscriber == null) { - mySubscriptionDeliverySubscriber = new SubscriptionDeliveringRestHookSubscriber(getSubscriptionDao(), getChannelType(), this); - } - getDeliveryChannel().subscribe(mySubscriptionDeliverySubscriber); + protected Optional createDeliveryHandler(CanonicalSubscription theSubscription) { + return Optional.of(new SubscriptionDeliveringRestHookSubscriber(getSubscriptionDao(), getChannelType(), this)); } @Override @@ -38,8 +38,4 @@ public class SubscriptionRestHookInterceptor extends BaseSubscriptionInterceptor return org.hl7.fhir.r4.model.Subscription.SubscriptionChannelType.RESTHOOK; } - @Override - protected void unregisterDeliverySubscriber() { - getDeliveryChannel().unsubscribe(mySubscriptionDeliverySubscriber); - } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketHandler.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketHandler.java index 0d2ae3e3659..33c541c780e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketHandler.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketHandler.java @@ -109,12 +109,14 @@ public class SubscriptionWebsocketHandler extends TextWebSocketHandler implement mySession = theSession; mySubscription = theSubscription; - mySubscriptionWebsocketInterceptor.getDeliveryChannel().subscribe(this); + String subscriptionId = mySubscription.getIdElement(myCtx).getIdPart(); + mySubscriptionWebsocketInterceptor.registerHandler(subscriptionId, this); } @Override public void closing() { - mySubscriptionWebsocketInterceptor.getDeliveryChannel().unsubscribe(this); + String subscriptionId = mySubscription.getIdElement(myCtx).getIdPart(); + mySubscriptionWebsocketInterceptor.unregisterHandler(subscriptionId, this); } private void deliver() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java index dbdbbb1efba..95509a3180e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.websocket; * 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. @@ -23,10 +23,14 @@ package ca.uhn.fhir.jpa.subscription.websocket; import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.dao.data.ISubscriptionTableDao; import ca.uhn.fhir.jpa.subscription.BaseSubscriptionInterceptor; +import ca.uhn.fhir.jpa.subscription.CanonicalSubscription; import org.hl7.fhir.r4.model.Subscription; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.messaging.MessageHandler; import org.springframework.transaction.PlatformTransactionManager; +import java.util.Optional; + public class SubscriptionWebsocketInterceptor extends BaseSubscriptionInterceptor { @Autowired @@ -38,26 +42,15 @@ public class SubscriptionWebsocketInterceptor extends BaseSubscriptionIntercepto @Autowired private IResourceTableDao myResourceTableDao; + @Override + protected Optional createDeliveryHandler(CanonicalSubscription theSubscription) { + return Optional.empty(); + } + @Override public Subscription.SubscriptionChannelType getChannelType() { return Subscription.SubscriptionChannelType.WEBSOCKET; } - @Override - protected void registerDeliverySubscriber() { - /* - * nothing, since individual websocket connections - * register themselves - */ - } - @Override - protected void unregisterDeliverySubscriber() { - - /* - * nothing, since individual websocket connections - * register themselves - */ - - } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/WebsocketWithSubscriptionIdR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/WebsocketWithSubscriptionIdR4Test.java index 90a973e69e6..0ee08decc54 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/WebsocketWithSubscriptionIdR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/WebsocketWithSubscriptionIdR4Test.java @@ -47,6 +47,7 @@ public class WebsocketWithSubscriptionIdR4Test extends BaseResourceProviderR4Tes private WebSocketClient myWebSocketClient; private SocketImplementation mySocketImplementation; + @Override @After public void after() throws Exception { super.after(); @@ -60,6 +61,7 @@ public class WebsocketWithSubscriptionIdR4Test extends BaseResourceProviderR4Tes myWebSocketClient.stop(); } + @Override @Before public void before() throws Exception { super.before(); From d9296b8e42b726564343308a9b3af775e089fee9 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 8 Aug 2018 15:14:39 -0400 Subject: [PATCH 08/22] Add hook for error message --- .../main/java/ca/uhn/fhir/rest/server/RestfulServer.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index 9fd333f42c3..a20cc2f76eb 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -300,7 +300,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer Date: Wed, 8 Aug 2018 17:21:25 -0400 Subject: [PATCH 09/22] Add extendible methods to RestfulServer --- .../uhn/fhir/rest/server/RestfulServer.java | 44 ++++++++++--------- .../provider/HashMapResourceProvider.java | 4 +- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index a20cc2f76eb..55f2d1a9611 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -316,15 +316,11 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theList) { myInterceptors.clear(); if (theList != null) { - myInterceptors.addAll(Arrays.asList(theList)); + myInterceptors.addAll(theList); } } @@ -606,11 +602,8 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theProviders) { - myPlainProviders.clear(); - if (theProviders != null) { - myPlainProviders.addAll(theProviders); - } + public void setPlainProviders(Object... theProv) { + setPlainProviders(Arrays.asList(theProv)); } /** @@ -640,10 +633,10 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theResourceProviders) { + public void setResourceProviders(IResourceProvider... theResourceProviders) { myResourceProviders.clear(); if (theResourceProviders != null) { - myResourceProviders.addAll(theResourceProviders); + myResourceProviders.addAll(Arrays.asList(theResourceProviders)); } } @@ -1516,10 +1509,10 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theList) { + public void setInterceptors(IServerInterceptor... theList) { myInterceptors.clear(); if (theList != null) { - myInterceptors.addAll(theList); + myInterceptors.addAll(Arrays.asList(theList)); } } @@ -1528,8 +1521,11 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theProviders) { + myPlainProviders.clear(); + if (theProviders != null) { + myPlainProviders.addAll(theProviders); + } } /** @@ -1547,10 +1543,10 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theResourceProviders) { myResourceProviders.clear(); if (theResourceProviders != null) { - myResourceProviders.addAll(Arrays.asList(theResourceProviders)); + myResourceProviders.addAll(theResourceProviders); } } @@ -1563,6 +1559,14 @@ public class RestfulServer extends HttpServlet implements IRestfulServer Date: Fri, 10 Aug 2018 09:07:55 -0400 Subject: [PATCH 10/22] Subscription cleanup --- .../java/ca/uhn/fhir/cli/BaseCommand.java | 10 +- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 8 +- .../ResourceHistoryTag.java_70782329243090 | 0 .../BaseSubscriptionInterceptor.java | 30 +++-- .../SubscriptionActivatingSubscriber.java | 16 ++- .../email/SubscriptionEmailInterceptor.java | 4 +- .../SubscriptionRestHookInterceptor.java | 4 +- .../SubscriptionWebsocketInterceptor.java | 4 +- .../ca/uhn/fhir/jpa/util/RestUtilities.java | 115 ------------------ 9 files changed, 46 insertions(+), 145 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java_70782329243090 delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/RestUtilities.java diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseCommand.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseCommand.java index 736f374335b..53c80064dc6 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseCommand.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseCommand.java @@ -108,7 +108,7 @@ public abstract class BaseCommand implements Comparable { if (theOptions.getOption(theOpt) != null) { throw new IllegalStateException("Duplicate option: " + theOpt); } - if (theOptionGroup != null && theOptionGroup.getOptions().stream().anyMatch(t-> theOpt.equals(t.getOpt()))) { + if (theOptionGroup != null && theOptionGroup.getOptions().stream().anyMatch(t -> theOpt.equals(t.getOpt()))) { throw new IllegalStateException("Duplicate option: " + theOpt); } } @@ -116,7 +116,7 @@ public abstract class BaseCommand implements Comparable { if (theOptions.getOption(theLongOpt) != null) { throw new IllegalStateException("Duplicate option: " + theLongOpt); } - if (theOptionGroup != null && theOptionGroup.getOptions().stream().anyMatch(t-> theLongOpt.equals(t.getLongOpt()))) { + if (theOptionGroup != null && theOptionGroup.getOptions().stream().anyMatch(t -> theLongOpt.equals(t.getLongOpt()))) { throw new IllegalStateException("Duplicate option: " + theOpt); } } @@ -362,8 +362,12 @@ public abstract class BaseCommand implements Comparable { throw new ParseException("Invalid target server specified, must begin with 'http' or 'file'."); } + return newClientWithBaseUrl(theCommandLine, baseUrl, theBasicAuthOptionName, theBearerTokenOptionName); + } + + protected IGenericClient newClientWithBaseUrl(CommandLine theCommandLine, String theBaseUrl, String theBasicAuthOptionName, String theBearerTokenOptionName) { myFhirCtx.getRestfulClientFactory().setSocketTimeout(10 * 60 * 1000); - IGenericClient retVal = myFhirCtx.newRestfulGenericClient(baseUrl); + IGenericClient retVal = myFhirCtx.newRestfulGenericClient(theBaseUrl); String basicAuthHeaderValue = getAndParseOptionBasicAuthHeader(theCommandLine, theBasicAuthOptionName); if (isNotBlank(basicAuthHeaderValue)) { 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 6d86ce24f82..8268c3dae9a 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 @@ -706,9 +706,9 @@ public class DaoConfig { * references instead of being treated as real references. *

* A logical reference is a reference which is treated as an identifier, and - * does not neccesarily resolve. See {@link "http://hl7.org/fhir/references.html"} for + * does not neccesarily resolve. See references for * a description of logical references. For example, the valueset - * {@link "http://hl7.org/fhir/valueset-quantity-comparator.html"} is a logical + * valueset-quantity-comparator is a logical * reference. *

*

@@ -731,9 +731,9 @@ public class DaoConfig { * references instead of being treated as real references. *

* A logical reference is a reference which is treated as an identifier, and - * does not neccesarily resolve. See {@link "http://hl7.org/fhir/references.html"} for + * does not neccesarily resolve. See references for * a description of logical references. For example, the valueset - * {@link "http://hl7.org/fhir/valueset-quantity-comparator.html"} is a logical + * valueset-quantity-comparator is a logical * reference. *

*

diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java_70782329243090 b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTag.java_70782329243090 new file mode 100644 index 00000000000..e69de29bb2d diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java index 690acc5ccf8..1a6273d842e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription; * 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. @@ -50,6 +50,7 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Subscription; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.task.AsyncTaskExecutor; @@ -82,7 +83,7 @@ public abstract class BaseSubscriptionInterceptor exten private SubscriptionActivatingSubscriber mySubscriptionActivatingSubscriber; private MessageHandler mySubscriptionCheckingSubscriber; private ConcurrentHashMap myIdToSubscription = new ConcurrentHashMap<>(); - private ConcurrentHashMap myIdToSubscribaleChannel = new ConcurrentHashMap<>(); + private ConcurrentHashMap mySubscribableChannel = new ConcurrentHashMap<>(); private Multimap myIdToDeliveryHandler = Multimaps.synchronizedListMultimap(ArrayListMultimap.create()); private Logger ourLog = LoggerFactory.getLogger(BaseSubscriptionInterceptor.class); private ThreadPoolExecutor myDeliveryExecutor; @@ -301,7 +302,7 @@ public abstract class BaseSubscriptionInterceptor exten } protected MessageChannel getDeliveryChannel(CanonicalSubscription theSubscription) { - return myIdToSubscribaleChannel.get(theSubscription.getIdElement(myCtx).getIdPart()); + return mySubscribableChannel.get(theSubscription.getIdElement(myCtx).getIdPart()); } public int getExecutorQueueSizeForUnitTests() { @@ -384,7 +385,7 @@ public abstract class BaseSubscriptionInterceptor exten } public void registerHandler(String theSubscriptionId, MessageHandler theHandler) { - myIdToSubscribaleChannel.get(theSubscriptionId).subscribe(theHandler); + mySubscribableChannel.get(theSubscriptionId).subscribe(theHandler); myIdToDeliveryHandler.put(theSubscriptionId, theHandler); } @@ -399,7 +400,7 @@ public abstract class BaseSubscriptionInterceptor exten SubscribableChannel deliveryChannel = createDeliveryChannel(canonicalized); Optional deliveryHandler = createDeliveryHandler(canonicalized); - myIdToSubscribaleChannel.put(subscriptionId, deliveryChannel); + mySubscribableChannel.put(subscriptionId, deliveryChannel); myIdToSubscription.put(subscriptionId, canonicalized); deliveryHandler.ifPresent(handler -> registerHandler(subscriptionId, handler)); @@ -545,13 +546,20 @@ public abstract class BaseSubscriptionInterceptor exten } } - public void unregisterHandler(String theSubscriptionId, MessageHandler next) { - SubscribableChannel channel = myIdToSubscribaleChannel.get(theSubscriptionId); + public void unregisterHandler(String theSubscriptionId, MessageHandler theMessageHandler) { + SubscribableChannel channel = mySubscribableChannel.get(theSubscriptionId); if (channel != null) { - channel.unsubscribe(next); + if (channel instanceof DisposableBean) { + try { + ((DisposableBean) channel).destroy(); + } catch (Exception e) { + ourLog.error("Failed to destroy channel bean", e); + } + } + channel.unsubscribe(theMessageHandler); } - myIdToSubscribaleChannel.remove(theSubscriptionId, next); + mySubscribableChannel.remove(theSubscriptionId, theMessageHandler); } @SuppressWarnings("UnusedReturnValue") @@ -565,7 +573,7 @@ public abstract class BaseSubscriptionInterceptor exten unregisterHandler(subscriptionId, next); } - myIdToSubscribaleChannel.remove(subscriptionId); + mySubscribableChannel.remove(subscriptionId); return myIdToSubscription.remove(subscriptionId); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java index 89681e0c0da..13cdb3b0b74 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java @@ -126,15 +126,12 @@ public class SubscriptionActivatingSubscriber { activateSubscription(activeStatus, theSubscription, requestedStatus); } } else if (activeStatus.equals(statusString)) { - if (!mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) { - ourLog.info("Registering active subscription {}", theSubscription.getIdElement().toUnqualified().getValue()); - } - mySubscriptionInterceptor.registerSubscription(theSubscription.getIdElement(), theSubscription); + registerSubscriptionUnlessAlreadyRegistered(theSubscription); } else { if (mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) { ourLog.info("Removing {} subscription {}", statusString, theSubscription.getIdElement().toUnqualified().getValue()); + mySubscriptionInterceptor.unregisterSubscription(theSubscription.getIdElement()); } - mySubscriptionInterceptor.unregisterSubscription(theSubscription.getIdElement()); } } @@ -145,7 +142,7 @@ public class SubscriptionActivatingSubscriber { try { SubscriptionUtil.setStatus(myCtx, subscription, theActiveStatus); mySubscriptionDao.update(subscription); - mySubscriptionInterceptor.registerSubscription(subscription.getIdElement(), subscription); + registerSubscriptionUnlessAlreadyRegistered(subscription); } catch (final UnprocessableEntityException e) { ourLog.info("Changing status of {} to ERROR", subscription.getIdElement()); SubscriptionUtil.setStatus(myCtx, subscription, "error"); @@ -179,6 +176,13 @@ public class SubscriptionActivatingSubscriber { } + private void registerSubscriptionUnlessAlreadyRegistered(IBaseResource theSubscription) { + if (!mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) { + ourLog.info("Registering active subscription {}", theSubscription.getIdElement().toUnqualified().getValue()); + mySubscriptionInterceptor.registerSubscription(theSubscription.getIdElement(), theSubscription); + } + } + @VisibleForTesting public static void setWaitForSubscriptionActivationSynchronouslyForUnitTest(boolean theWaitForSubscriptionActivationSynchronouslyForUnitTest) { ourWaitForSubscriptionActivationSynchronouslyForUnitTest = theWaitForSubscriptionActivationSynchronouslyForUnitTest; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java index 35ff5fc1b64..270bf3ee9b1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/email/SubscriptionEmailInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.email; * 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. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java index 9c0558ba310..83ff6a3257d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/resthook/SubscriptionRestHookInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.resthook; * 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. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java index 95509a3180e..9189ddc6cc2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/websocket/SubscriptionWebsocketInterceptor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.websocket; * 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. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/RestUtilities.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/RestUtilities.java deleted file mode 100644 index 94acf15c4e4..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/RestUtilities.java +++ /dev/null @@ -1,115 +0,0 @@ -package ca.uhn.fhir.jpa.util; - -/*- - * #%L - * HAPI FHIR JPA Server - * %% - * Copyright (C) 2014 - 2018 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.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; -import org.apache.http.client.methods.HttpDelete; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.methods.HttpPut; -import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.DefaultHttpClient; -import org.apache.http.util.EntityUtils; - -import javax.xml.ws.http.HTTPException; -import java.io.IOException; -import java.util.concurrent.TimeUnit; - -/** - * Rest service utilities. Generally used in the tests - */ -public class RestUtilities { - - public static final String CONTEXT_PATH = ""; - public static final String APPLICATION_JSON = "application/json"; - - /** - * Get the response for a CXF REST service without an object parameter - * - * @param url - * @param typeRequest - * @return - * @throws IOException - */ - public static String getResponse(String url, MethodRequest typeRequest) throws IOException { - return getResponse(url, (StringEntity) null, typeRequest); - } - - /** - * Get the response for a CXF REST service with an object parameter - * - * @param url - * @param parameterEntity - * @param typeRequest - * @return - * @throws IOException - */ - public static String getResponse(String url, StringEntity parameterEntity, MethodRequest typeRequest) throws IOException { - HttpClient httpclient = new DefaultHttpClient(); - HttpResponse response; - - switch (typeRequest) { - case POST: - HttpPost httppost = new HttpPost(url); - httppost.setHeader("Content-type", APPLICATION_JSON); - if (parameterEntity != null) { - httppost.setEntity(parameterEntity); - } - response = httpclient.execute(httppost); - break; - case PUT: - HttpPut httpPut = new HttpPut(url); - httpPut.setHeader("Content-type", APPLICATION_JSON); - if (parameterEntity != null) { - httpPut.setEntity(parameterEntity); - } - response = httpclient.execute(httpPut); - break; - case DELETE: - HttpDelete httpDelete = new HttpDelete(url); - httpDelete.setHeader("Content-type", APPLICATION_JSON); - response = httpclient.execute(httpDelete); - break; - case GET: - HttpGet httpGet = new HttpGet(url); - httpGet.setHeader("Content-type", APPLICATION_JSON); - response = httpclient.execute(httpGet); - break; - default: - throw new IllegalArgumentException("Cannot handle type request " + typeRequest); - } - - if (response.getStatusLine().getStatusCode() < 200 || response.getStatusLine().getStatusCode() >= 300) { - throw new HTTPException(response.getStatusLine().getStatusCode()); - } - - if (response.getStatusLine().getStatusCode() == 204) { - return ""; - } - - //Closes connections that have already been closed by the server - //org.apache.http.NoHttpResponseException: The target server failed to respond - httpclient.getConnectionManager().closeIdleConnections(1, TimeUnit.SECONDS); - - return EntityUtils.toString(response.getEntity()); - } -} From 98d794e30f3557c52a7e6604ecf3f91978fa22a0 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 10 Aug 2018 13:07:06 -0400 Subject: [PATCH 11/22] Bug fix in subscription processing --- .../SubscriptionActivatingSubscriber.java | 7 ++- .../ca/uhn/fhir/jpa/config/TestR4Config.java | 2 +- .../subscription/r4/RestHookTestR4Test.java | 47 +++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java index 13cdb3b0b74..128355ff284 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java @@ -177,10 +177,13 @@ public class SubscriptionActivatingSubscriber { } private void registerSubscriptionUnlessAlreadyRegistered(IBaseResource theSubscription) { - if (!mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) { + if (mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) { + ourLog.info("Updating already-registered active subscription {}", theSubscription.getIdElement().toUnqualified().getValue()); + mySubscriptionInterceptor.unregisterSubscription(theSubscription.getIdElement()); + } else { ourLog.info("Registering active subscription {}", theSubscription.getIdElement().toUnqualified().getValue()); - mySubscriptionInterceptor.registerSubscription(theSubscription.getIdElement(), theSubscription); } + mySubscriptionInterceptor.registerSubscription(theSubscription.getIdElement(), theSubscription); } @VisibleForTesting diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java index 5557e57e067..943fc6b0451 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java @@ -105,7 +105,7 @@ public class TestR4Config extends BaseJavaConfigR4 { DataSource dataSource = ProxyDataSourceBuilder .create(retVal) - .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") +// .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) .countQuery(new ThreadQueryCountHolder()) .build(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestR4Test.java index 4de1ab758ac..a2782ccdffc 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestR4Test.java @@ -355,6 +355,53 @@ public class RestHookTestR4Test extends BaseResourceProviderR4Test { Assert.assertFalse(observation2.getId().isEmpty()); } + + @Test + public void testUpdateSubscriptionToMatchLater() throws Exception { + String payload = "application/xml"; + + String code = "1000000050"; + String criteriaBad = "Observation?code=SNOMED-CT|" + code + "111&_format=xml"; + + ourLog.info("** About to create non-matching subscription"); + + Subscription subscription2 = createSubscription(criteriaBad, payload, ourListenerServerBase); + + ourLog.info("** About to send observation that wont match"); + + Observation observation1 = sendObservation(code, "SNOMED-CT"); + + // Criteria didn't match, shouldn't see any updates + waitForQueueToDrain(); + Thread.sleep(1000); + assertEquals(0, ourUpdatedObservations.size()); + + Subscription subscriptionTemp = myClient.read().resource(Subscription.class).withId(subscription2.getId()).execute(); + Assert.assertNotNull(subscriptionTemp); + String criteriaGood = "Observation?code=SNOMED-CT|" + code + "&_format=xml"; + subscriptionTemp.setCriteria(criteriaGood); + ourLog.info("** About to update subscription"); + myClient.update().resource(subscriptionTemp).withId(subscriptionTemp.getIdElement()).execute(); + waitForQueueToDrain(); + + ourLog.info("** About to send Observation 2"); + Observation observation2 = sendObservation(code, "SNOMED-CT"); + waitForQueueToDrain(); + + // Should see a subscription notification this time + waitForSize(0, ourCreatedObservations); + waitForSize(1, ourUpdatedObservations); + + myClient.delete().resourceById(new IdType("Subscription/" + subscription2.getId())).execute(); + + Observation observationTemp3 = sendObservation(code, "SNOMED-CT"); + + // No more matches + Thread.sleep(1000); + assertEquals(1, ourUpdatedObservations.size()); + } + + @Test public void testRestHookSubscriptionApplicationXmlJson() throws Exception { String payload = "application/fhir+xml"; From acfe442cf99334d7c040c7a8a6db2a619e76d46c Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sat, 11 Aug 2018 09:55:28 -0400 Subject: [PATCH 12/22] Add equivalency flag to reverse lookup --- .../dao/r4/FhirResourceDaoConceptMapR4.java | 4 ++ .../jpa/term/BaseHapiTerminologySvcImpl.java | 23 ++++++- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 6 ++ .../r4/FhirResourceDaoR4ConceptMapTest.java | 69 +++++++++++++++---- .../derby_maintenance.txt | 35 ++++++++++ src/changes/changes.xml | 4 ++ 6 files changed, 127 insertions(+), 14 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoConceptMapR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoConceptMapR4.java index 5c0ff79bf03..6a37a9b1260 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoConceptMapR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoConceptMapR4.java @@ -143,6 +143,10 @@ public class FhirResourceDaoConceptMapR4 extends FhirResourceDaoR4 i translationMatch.setSource(new UriType(element.getConceptMapUrl())); + if (element.getConceptMapGroupElementTargets().size() == 1) { + translationMatch.setEquivalence(new CodeType(element.getConceptMapGroupElementTargets().get(0).getEquivalence().toCode())); + } + retVal.addMatch(translationMatch); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java index 61d6f45cc8e..f5b39ae2ea8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java @@ -1343,14 +1343,18 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, predicates = new ArrayList<>(); coding = translationQuery.getCoding(); + String targetCode = null; + String targetCodeSystem = null; if (coding.hasCode()) { predicates.add(criteriaBuilder.equal(targetJoin.get("myCode"), coding.getCode())); + targetCode = coding.getCode(); } else { throw new InvalidRequestException("A code must be provided for translation to occur."); } if (coding.hasSystem()) { predicates.add(criteriaBuilder.equal(groupJoin.get("myTarget"), coding.getSystem())); + targetCodeSystem = coding.getSystem(); } if (coding.hasVersion()) { @@ -1384,7 +1388,24 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, Iterator scrollableResultsIterator = new ScrollableResultsIterator<>(scrollableResults); while (scrollableResultsIterator.hasNext()) { - elements.add(scrollableResultsIterator.next()); + TermConceptMapGroupElement nextElement = scrollableResultsIterator.next(); + nextElement.getConceptMapGroupElementTargets().size(); + myEntityManager.detach(nextElement); + + if (isNotBlank(targetCode) && isNotBlank(targetCodeSystem)) { + for (Iterator iter = nextElement.getConceptMapGroupElementTargets().iterator(); iter.hasNext(); ) { + TermConceptMapGroupElementTarget next = iter.next(); + if (targetCodeSystem.equals(next.getSystem())) { + if (targetCode.equals(next.getCode())) { + continue; + } + } + + iter.remove(); + } + } + + elements.add(nextElement); } ourLastResultsFromTranslationWithReverseCache = false; // For testing. diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index d0b1ce49979..be4e5aabc1e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -395,6 +395,12 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { target.setDisplay("Target Code 45678"); target.setEquivalence(ConceptMapEquivalence.WIDER); + // Add a duplicate + target = element.addTarget(); + target.setCode("45678"); + target.setDisplay("Target Code 45678"); + target.setEquivalence(ConceptMapEquivalence.WIDER); + group = conceptMap.addGroup(); group.setSource(CS_URL); group.setSourceVersion("Version 3"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java index c96b8e5ce18..dbbdd237dbd 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java @@ -613,7 +613,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(1, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); @@ -625,6 +625,49 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { }); } + @Test + public void testTranslateWithReverseHavingEquivalence() { + ConceptMap conceptMap = myConceptMapDao.read(myConceptMapId); + + ourLog.info("ConceptMap:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(conceptMap)); + + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + /* + * Provided: + * source code + * source code system + * target code system + * reverse = true + */ + TranslationRequest translationRequest = new TranslationRequest(); + translationRequest.getCodeableConcept().addCoding() + .setSystem(CS_URL_3) + .setCode("67890"); + translationRequest.setTargetSystem(new UriType(CS_URL)); + translationRequest.setReverse(true); + + TranslationResult translationResult = myConceptMapDao.translate(translationRequest, null); + + assertTrue(translationResult.getResult().booleanValue()); + assertEquals("Matches found!", translationResult.getMessage().getValueAsString()); + + assertEquals(1, translationResult.getMatches().size()); + + TranslationMatch translationMatch = translationResult.getMatches().get(0); + Coding concept = translationMatch.getConcept(); + assertEquals("12345", concept.getCode()); + assertEquals("Source Code 12345", concept.getDisplay()); + assertEquals(CS_URL, concept.getSystem()); + assertEquals("Version 3", concept.getVersion()); + assertFalse(concept.getUserSelected()); + assertEquals(CM_URL, translationMatch.getSource().getValueAsString()); + assertEquals("wider", translationMatch.getEquivalence().getCode()); + } + }); + } + @Test public void testTranslateWithReverseByCodeSystemsAndSourceCodeUnmapped() { ConceptMap conceptMap = myConceptMapDao.read(myConceptMapId); @@ -680,7 +723,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(2, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("equal", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("12345", concept.getCode()); assertEquals("Source Code 12345", concept.getDisplay()); @@ -690,7 +733,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(CM_URL, translationMatch.getSource().getValueAsString()); translationMatch = translationResult.getMatches().get(1); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); @@ -733,7 +776,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(1, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("equal", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("12345", concept.getCode()); assertEquals("Source Code 12345", concept.getDisplay()); @@ -776,7 +819,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(1, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); @@ -817,7 +860,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(2, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("equal", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("12345", concept.getCode()); assertEquals("Source Code 12345", concept.getDisplay()); @@ -827,7 +870,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(CM_URL, translationMatch.getSource().getValueAsString()); translationMatch = translationResult.getMatches().get(1); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); @@ -870,7 +913,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(2, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("equal", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("12345", concept.getCode()); assertEquals("Source Code 12345", concept.getDisplay()); @@ -880,7 +923,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(CM_URL, translationMatch.getSource().getValueAsString()); translationMatch = translationResult.getMatches().get(1); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); @@ -921,7 +964,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(2, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("equal", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("12345", concept.getCode()); assertEquals("Source Code 12345", concept.getDisplay()); @@ -931,7 +974,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(CM_URL, translationMatch.getSource().getValueAsString()); translationMatch = translationResult.getMatches().get(1); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); @@ -972,7 +1015,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(2, translationResult.getMatches().size()); TranslationMatch translationMatch = translationResult.getMatches().get(0); - assertNull(translationMatch.getEquivalence()); + assertEquals("equal", translationMatch.getEquivalence().getCode()); Coding concept = translationMatch.getConcept(); assertEquals("12345", concept.getCode()); assertEquals("Source Code 12345", concept.getDisplay()); @@ -982,7 +1025,7 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { assertEquals(CM_URL, translationMatch.getSource().getValueAsString()); translationMatch = translationResult.getMatches().get(1); - assertNull(translationMatch.getEquivalence()); + assertEquals("narrower", translationMatch.getEquivalence().getCode()); concept = translationMatch.getConcept(); assertEquals("78901", concept.getCode()); assertEquals("Source Code 78901", concept.getDisplay()); diff --git a/hapi-fhir-jpaserver-uhnfhirtest/derby_maintenance.txt b/hapi-fhir-jpaserver-uhnfhirtest/derby_maintenance.txt index b2e6da3861a..5c38a909b06 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/derby_maintenance.txt +++ b/hapi-fhir-jpaserver-uhnfhirtest/derby_maintenance.txt @@ -195,3 +195,38 @@ drop column SP_ID from table HFJ_RES_PARAM_PRESENT; drop index IDX_SEARCHPARM_RESTYPE_SPNAME; drop index IDX_RESPARMPRESENT_SPID_RESID; drop table HFJ_SEARCH_PARM; + + + +# Delete everything +update hfj_res_ver set forced_id_pid = null where res_id in (select res_id from hfj_resource); +update hfj_resource set forced_id_pid = null where res_id in (select res_id from hfj_resource); +delete from hfj_history_tag where res_id in (select res_id from hfj_resource); +delete from hfj_res_ver where res_id in (select res_id from hfj_resource); +delete from hfj_forced_id where resource_pid in (select res_id from hfj_resource); +delete from hfj_res_link where src_resource_id in (select res_id from hfj_resource); +delete from hfj_res_link where target_resource_id in (select res_id from hfj_resource); +delete from hfj_spidx_coords; +delete from hfj_spidx_date; +delete from hfj_spidx_number; +delete from hfj_spidx_quantity; +delete from hfj_spidx_string; +delete from hfj_spidx_token; +delete from hfj_spidx_uri; +delete from hfj_res_tag; +delete from hfj_search_result; +delete from hfj_res_param_present; +delete from hfj_idx_cmp_string_uniq; +delete from hfj_subscription_stats; +DELETE FROM TRM_CONCEPT_MAP_GRP_ELM_TGT; +DELETE FROM TRM_CONCEPT_MAP_GRP_ELEMENT; +DELETE FROM TRM_CONCEPT_MAP_GROUP; +DELETE FROM TRM_CONCEPT_MAP; +DELETE FROM TRM_CONCEPT_DESIG; +DELETE FROM TRM_CONCEPT_PC_LINK; +DELETE FROM TRM_CONCEPT_PROPERTY; +DELETE FROM TRM_CONCEPT; +DELETE FROM TRM_CODESYSTEM_VER; +DELETE FROM TRM_CODESYSTEM; +delete from hfj_resource; + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a97c12ab28f..4733be32e91 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -249,6 +249,10 @@ queue (only the ID), which should reduce the memory/disk footprint of the queue when it grows long. + + When performing a ConceptMap/$translate operation with reverse="true" in the arguments, + the equivalency flag is now set on the response just as it is for a non-reverse lookup. + From 159377ac16aabaf1170ff7f3d9dccfdd8d779872 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 11 Aug 2018 13:46:34 -0400 Subject: [PATCH 13/22] Add details to ResponseHighlighterInterceptor --- .../ResponseHighlighterInterceptor.java | 329 ++++++++++-------- src/changes/changes.xml | 4 + 2 files changed, 185 insertions(+), 148 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java index 3045580b603..1b7e81e81ff 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java @@ -13,7 +13,9 @@ import ca.uhn.fhir.rest.server.RestfulServerUtils.ResponseEncoding; import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.UrlUtil; +import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringEscapeUtils; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -440,206 +442,237 @@ public class ResponseHighlighterInterceptor extends InterceptorAdapter { } theServletResponse.setContentType(Constants.CT_HTML_WITH_UTF8); - StringBuilder b = new StringBuilder(); - b.append("\n"); - b.append(" \n"); - b.append(" \n"); - b.append(" \n"); - b.append(" \n"); - b.append("\n"); - b.append(" "); + StringBuilder outputBuffer = new StringBuilder(); + outputBuffer.append("\n"); + outputBuffer.append(" \n"); + outputBuffer.append(" \n"); + outputBuffer.append(" \n"); + outputBuffer.append(" \n"); + outputBuffer.append("\n"); + outputBuffer.append(" "); - b.append("

"); - b.append("This result is being rendered in HTML for easy viewing. "); - b.append("You may access this content as "); + outputBuffer.append("

"); + outputBuffer.append("This result is being rendered in HTML for easy viewing. "); + outputBuffer.append("You may access this content as "); - b.append("Raw JSON or "); + outputBuffer.append("Raw JSON or "); - b.append("Raw XML, "); + outputBuffer.append("Raw XML, "); - b.append(" or view this content in "); + outputBuffer.append(" or view this content in "); - b.append("HTML JSON "); + outputBuffer.append("HTML JSON "); - b.append("or "); - b.append("HTML XML."); + outputBuffer.append("or "); + outputBuffer.append("HTML XML."); Date startTime = (Date) theServletRequest.getAttribute(RestfulServer.REQUEST_START_TIME); if (startTime != null) { long time = System.currentTimeMillis() - startTime.getTime(); - b.append(" Response generated in "); - b.append(time); - b.append("ms."); + outputBuffer.append(" Response generated in "); + outputBuffer.append(time); + outputBuffer.append("ms."); } - b.append("

"); + outputBuffer.append("

"); - b.append("\n"); + outputBuffer.append("\n"); // status (e.g. HTTP 200 OK) String statusName = Constants.HTTP_STATUS_NAMES.get(theServletResponse.getStatus()); statusName = defaultString(statusName); - b.append("
"); - b.append("HTTP "); - b.append(theServletResponse.getStatus()); - b.append(" "); - b.append(statusName); - b.append("
"); + outputBuffer.append("
"); + outputBuffer.append("HTTP "); + outputBuffer.append(theServletResponse.getStatus()); + outputBuffer.append(" "); + outputBuffer.append(statusName); + outputBuffer.append("
"); - b.append("\n"); - b.append("\n"); + outputBuffer.append("\n"); + outputBuffer.append("\n"); try { if (isShowRequestHeaders()) { - streamRequestHeaders(theServletRequest, b); + streamRequestHeaders(theServletRequest, outputBuffer); } if (isShowResponseHeaders()) { - streamResponseHeaders(theRequestDetails, theServletResponse, b); + streamResponseHeaders(theRequestDetails, theServletResponse, outputBuffer); } } catch (Throwable t) { // ignore (this will hit if we're running in a servlet 2.5 environment) } - b.append("

Response Body

"); + outputBuffer.append("

Response Body

"); - b.append("
"); + outputBuffer.append("
"); // Response Body - b.append("
");
+			outputBuffer.append("
");
 			StringBuilder target = new StringBuilder();
 			int linesCount = format(encoded, target, encoding);
-			b.append(target);
-			b.append("
");
+			outputBuffer.append(target);
+			outputBuffer.append("
"); // Line Numbers - b.append("
");
+			outputBuffer.append("
");
 			for (int i = 1; i <= linesCount; i++) {
-				b.append("
");
+				outputBuffer.append("");
+				outputBuffer.append("");
+				outputBuffer.append(i);
+				outputBuffer.append("
");
 			}
-			b.append("
");
+			outputBuffer.append("
"); - b.append("
"); + outputBuffer.append("
"); - b.append("\n"); + outputBuffer.append("\n"); InputStream jsStream = ResponseHighlighterInterceptor.class.getResourceAsStream("ResponseHighlighter.js"); String jsStr = jsStream != null ? IOUtils.toString(jsStream, "UTF-8") : "console.log('ResponseHighlighterInterceptor: javascript theResource not found')"; jsStr = jsStr.replace("FHIR_BASE", theRequestDetails.getServerBaseForRequest()); - b.append("\n"); + outputBuffer.append("\n"); - b.append(""); - b.append(""); - String out = b.toString(); + StopWatch writeSw = new StopWatch(); + theServletResponse.getWriter().append(outputBuffer); + theServletResponse.getWriter().flush(); + + theServletResponse.getWriter().append("
"); + theServletResponse.getWriter().append("Wrote "); + writeLength(theServletResponse, encoded.length()); + theServletResponse.getWriter().append(" ("); + writeLength(theServletResponse, outputBuffer.length()); + theServletResponse.getWriter().append(" total including HTML)"); + + theServletResponse.getWriter().append(" in estimated "); + theServletResponse.getWriter().append(writeSw.toString()); + theServletResponse.getWriter().append("
"); + + + theServletResponse.getWriter().append(""); + theServletResponse.getWriter().append(""); - theServletResponse.getWriter().append(out); theServletResponse.getWriter().close(); } catch (IOException e) { throw new InternalErrorException(e); } } + private void writeLength(HttpServletResponse theServletResponse, int theLength) throws IOException { + double kb = ((double)theLength) / FileUtils.ONE_KB; + if (kb <= 1000) { + theServletResponse.getWriter().append(String.format("%.1f", kb)).append(" KB"); + } else { + double mb = kb / 1000; + theServletResponse.getWriter().append(String.format("%.1f", mb)).append(" MB"); + } + } + private void streamResponseHeaders(RequestDetails theRequestDetails, HttpServletResponse theServletResponse, StringBuilder b) { if (theServletResponse.getHeaderNames().isEmpty() == false) { b.append("

Response Headers

"); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a97c12ab28f..367af4091be 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -249,6 +249,10 @@ queue (only the ID), which should reduce the memory/disk footprint of the queue when it grows long. + + ResponseHighlighterInterceptor now displays the total size of the output and + an estimate of the transfer time at the bottom of the response. + From 6d10260d8f9be68420a85b9c5dabe29df51ec317 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 11 Aug 2018 15:12:33 -0400 Subject: [PATCH 14/22] Unit test cleanup --- .../java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java | 38 +++++++++++++------ .../fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java | 1 + ...nterceptorRegisteredToDaoConfigR4Test.java | 6 +-- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java index 556ff29c3ed..572d0a8c111 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java @@ -34,7 +34,6 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Rule; import org.mockito.Mockito; -import org.springframework.orm.hibernate5.HibernateTransactionManager; import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; @@ -49,6 +48,7 @@ import java.sql.SQLException; import java.util.*; import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import static ca.uhn.fhir.util.TestUtil.randomizeLocale; import static org.junit.Assert.*; @@ -77,16 +77,6 @@ public abstract class BaseJpaTest { BaseHapiFhirResourceDao.setDisableIncrementOnUpdateForUnitTest(false); } - @Before - public void beforeCreateSrd() { - mySrd = mock(ServletRequestDetails.class, Mockito.RETURNS_DEEP_STUBS); - when(mySrd.getRequestOperationCallback()).thenReturn(myRequestOperationCallback); - myServerInterceptorList = new ArrayList<>(); - when(mySrd.getServer().getInterceptors()).thenReturn(myServerInterceptorList); - when(mySrd.getUserData()).thenReturn(new HashMap<>()); - when(mySrd.getHeaders(eq(JpaConstants.HEADER_META_SNAPSHOT_MODE))).thenReturn(new ArrayList<>()); - } - @After public void afterValidateNoTransaction() { PlatformTransactionManager txManager = getTxManager(); @@ -103,6 +93,7 @@ public abstract class BaseJpaTest { if (currentSession != null) { currentSession.doWork(new Work() { + @Override public void execute(Connection connection) throws SQLException { isReadOnly.set(connection.isReadOnly()); } @@ -113,6 +104,16 @@ public abstract class BaseJpaTest { } } + @Before + public void beforeCreateSrd() { + mySrd = mock(ServletRequestDetails.class, Mockito.RETURNS_DEEP_STUBS); + when(mySrd.getRequestOperationCallback()).thenReturn(myRequestOperationCallback); + myServerInterceptorList = new ArrayList<>(); + when(mySrd.getServer().getInterceptors()).thenReturn(myServerInterceptorList); + when(mySrd.getUserData()).thenReturn(new HashMap<>()); + when(mySrd.getHeaders(eq(JpaConstants.HEADER_META_SNAPSHOT_MODE))).thenReturn(new ArrayList<>()); + } + @Before public void beforeRandomizeLocale() { randomizeLocale(); @@ -299,6 +300,7 @@ public abstract class BaseJpaTest { return retVal.toArray(new String[retVal.size()]); } + @SuppressWarnings("RedundantThrows") @AfterClass public static void afterClassClearContext() throws Exception { TestUtil.clearAllStaticFieldsForUnitTest(); @@ -360,7 +362,19 @@ public abstract class BaseJpaTest { } } if (sw.getMillis() >= 15000) { - fail("Size " + theList.size() + " is != target " + theTarget + " - Got: " + theList.toString()); + String describeResults = theList + .stream() + .map(t -> { + if (t == null) { + return "null"; + } + if (t instanceof IBaseResource) { + return ((IBaseResource)t).getIdElement().getValue(); + } + return t.toString(); + }) + .collect(Collectors.joining(", ")); + fail("Size " + theList.size() + " is != target " + theTarget + " - Got: " + describeResults); } } 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 c6aa63ae750..fccf834173b 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 @@ -235,6 +235,7 @@ public abstract class BaseJpaDstu2Test extends BaseJpaTest { return retVal; } + @SuppressWarnings("RedundantThrows") @AfterClass public static void afterClassClearContext() throws Exception { TestUtil.clearAllStaticFieldsForUnitTest(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestWithInterceptorRegisteredToDaoConfigR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestWithInterceptorRegisteredToDaoConfigR4Test.java index 32e418e82a8..d8568ecdd5e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestWithInterceptorRegisteredToDaoConfigR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookTestWithInterceptorRegisteredToDaoConfigR4Test.java @@ -150,7 +150,7 @@ public class RestHookTestWithInterceptorRegisteredToDaoConfigR4Test extends Base waitForSize(0, ourCreatedObservations); waitForSize(4, ourUpdatedObservations); - Observation observation3 = myClient.read(Observation.class, observationTemp3.getId()); + Observation observation3 = myClient.read().resource(Observation.class).withId(observationTemp3.getId()).execute(); CodeableConcept codeableConcept = new CodeableConcept(); observation3.setCode(codeableConcept); Coding coding = codeableConcept.addCoding(); @@ -223,7 +223,7 @@ public class RestHookTestWithInterceptorRegisteredToDaoConfigR4Test extends Base waitForSize(0, ourCreatedObservations); waitForSize(4, ourUpdatedObservations); - Observation observation3 = myClient.read(Observation.class, observationTemp3.getId()); + Observation observation3 = myClient.read().resource(Observation.class).withId(observationTemp3.getId()).execute(); CodeableConcept codeableConcept = new CodeableConcept(); observation3.setCode(codeableConcept); Coding coding = codeableConcept.addCoding(); @@ -236,7 +236,7 @@ public class RestHookTestWithInterceptorRegisteredToDaoConfigR4Test extends Base waitForSize(0, ourCreatedObservations); waitForSize(4, ourUpdatedObservations); - Observation observation3a = myClient.read(Observation.class, observationTemp3.getId()); + Observation observation3a = myClient.read().resource(Observation.class).withId(observationTemp3.getId()).execute(); CodeableConcept codeableConcept1 = new CodeableConcept(); observation3a.setCode(codeableConcept1); From 7eb36c3392b3e24e888f0e83b7aa3c43b56699d7 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 11 Aug 2018 16:14:53 -0400 Subject: [PATCH 15/22] Work on subscription --- ...ashMapResourceProviderConceptMapDstu3.java | 14 +- .../HashMapResourceProviderConceptMapR4.java | 15 +- .../AbstractHashMapResourceProvider.java | 198 ------------------ .../provider/HashMapResourceProvider.java | 12 +- 4 files changed, 25 insertions(+), 214 deletions(-) delete mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/AbstractHashMapResourceProvider.java diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapDstu3.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapDstu3.java index 7ca6a49417f..9550a66ab17 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapDstu3.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapDstu3.java @@ -25,7 +25,7 @@ import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.rest.annotation.*; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.provider.AbstractHashMapResourceProvider; +import ca.uhn.fhir.rest.server.provider.HashMapResourceProvider; import com.google.common.base.Charsets; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; @@ -42,7 +42,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; /** * This is a subclass to implement FHIR operations specific to DSTU3 ConceptMap - * resources. Its superclass, {@link AbstractHashMapResourceProvider}, is a simple + * resources. Its superclass, {@link HashMapResourceProvider}, is a simple * implementation of the resource provider interface that uses a HashMap to * store all resources in memory. *

@@ -53,7 +53,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; *

  • Conditional update for DSTU3 ConceptMap resources by ConceptMap.url
    • * */ -public class HashMapResourceProviderConceptMapDstu3 extends AbstractHashMapResourceProvider { +public class HashMapResourceProviderConceptMapDstu3 extends HashMapResourceProvider { @SuppressWarnings("unchecked") public HashMapResourceProviderConceptMapDstu3(FhirContext theFhirContext) { super(theFhirContext, ConceptMap.class); @@ -84,10 +84,10 @@ public class HashMapResourceProviderConceptMapDstu3 extends AbstractHashMapResou return retVal; } + @Override @Update - public MethodOutcome updateConceptMapConditional( + public MethodOutcome update( @ResourceParam ConceptMap theConceptMap, - @IdParam IdType theId, @ConditionalUrlParam String theConditional) { MethodOutcome methodOutcome = new MethodOutcome(); @@ -112,14 +112,14 @@ public class HashMapResourceProviderConceptMapDstu3 extends AbstractHashMapResou List conceptMaps = searchByUrl(url); if (!conceptMaps.isEmpty()) { - methodOutcome = update(conceptMaps.get(0)); + methodOutcome = super.update(conceptMaps.get(0), null); } else { methodOutcome = create(theConceptMap); } } } else { - methodOutcome = update(theConceptMap); + methodOutcome = super.update(theConceptMap, null); } return methodOutcome; diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapR4.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapR4.java index d958ae5a3c4..471af74a751 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapR4.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HashMapResourceProviderConceptMapR4.java @@ -25,7 +25,7 @@ import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.rest.annotation.*; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.provider.AbstractHashMapResourceProvider; +import ca.uhn.fhir.rest.server.provider.HashMapResourceProvider; import com.google.common.base.Charsets; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; @@ -42,7 +42,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; /** * This is a subclass to implement FHIR operations specific to R4 ConceptMap - * resources. Its superclass, {@link AbstractHashMapResourceProvider}, is a simple + * resources. Its superclass, {@link HashMapResourceProvider}, is a simple * implementation of the resource provider interface that uses a HashMap to * store all resources in memory. *

    @@ -53,7 +53,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; *

  • Conditional update for R4 ConceptMap resources by ConceptMap.url
    • * */ -public class HashMapResourceProviderConceptMapR4 extends AbstractHashMapResourceProvider { +public class HashMapResourceProviderConceptMapR4 extends HashMapResourceProvider { @SuppressWarnings("unchecked") public HashMapResourceProviderConceptMapR4(FhirContext theFhirContext) { super(theFhirContext, ConceptMap.class); @@ -84,16 +84,15 @@ public class HashMapResourceProviderConceptMapR4 extends AbstractHashMapResource return retVal; } + @Override @Update - public MethodOutcome updateConceptMapConditional( + public MethodOutcome update( @ResourceParam ConceptMap theConceptMap, - @IdParam IdType theId, @ConditionalUrlParam String theConditional) { MethodOutcome methodOutcome = new MethodOutcome(); if (theConditional != null) { - String url = null; try { @@ -112,14 +111,14 @@ public class HashMapResourceProviderConceptMapR4 extends AbstractHashMapResource List conceptMaps = searchByUrl(url); if (!conceptMaps.isEmpty()) { - methodOutcome = update(conceptMaps.get(0)); + methodOutcome = super.update(conceptMaps.get(0), null); } else { methodOutcome = create(theConceptMap); } } } else { - methodOutcome = update(theConceptMap); + methodOutcome = super.update(theConceptMap, null); } return methodOutcome; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/AbstractHashMapResourceProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/AbstractHashMapResourceProvider.java deleted file mode 100644 index 0756072e34b..00000000000 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/AbstractHashMapResourceProvider.java +++ /dev/null @@ -1,198 +0,0 @@ -package ca.uhn.fhir.rest.server.provider; - -/*- - * #%L - * HAPI FHIR - Server Framework - * %% - * Copyright (C) 2014 - 2018 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 ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.rest.annotation.*; -import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.server.IResourceProvider; -import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; -import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.*; - -/** - * This class is a simple implementation of the resource provider - * interface that uses a HashMap to store all resources in memory. - * It is essentially a copy of {@link ca.uhn.fhir.rest.server.provider.HashMapResourceProvider} - * with the {@link Update} and {@link ResourceParam} annotations removed from method - * {@link ca.uhn.fhir.rest.server.provider.HashMapResourceProvider#update(IBaseResource)}. - * Non-generic subclasses of this abstract class may implement their own annotated methods (e.g. a conditional - * update method specifically for ConceptMap resources). - *

    - * This class currently supports the following FHIR operations: - *

    - *
      - *
    • Create
      • - *
    • Update existing resource
      • - *
    • Update non-existing resource (e.g. create with client-supplied ID)
      • - *
    • Delete
      • - *
    • Search by resource type with no parameters
      • - *
    - * - * @param The resource type to support - */ -public class AbstractHashMapResourceProvider implements IResourceProvider { - private static final Logger ourLog = LoggerFactory.getLogger(AbstractHashMapResourceProvider.class); - private final Class myResourceType; - private final FhirContext myFhirContext; - private final String myResourceName; - protected Map> myIdToVersionToResourceMap = new HashMap<>(); - private long myNextId; - - /** - * Constructor - * - * @param theFhirContext The FHIR context - * @param theResourceType The resource type to support - */ - @SuppressWarnings("WeakerAccess") - public AbstractHashMapResourceProvider(FhirContext theFhirContext, Class theResourceType) { - myFhirContext = theFhirContext; - myResourceType = theResourceType; - myResourceName = myFhirContext.getResourceDefinition(theResourceType).getName(); - clear(); - } - - /** - * Clear all data held in this resource provider - */ - public void clear() { - myNextId = 1; - myIdToVersionToResourceMap.clear(); - } - - @Create - public MethodOutcome create(@ResourceParam T theResource) { - long idPart = myNextId++; - String idPartAsString = Long.toString(idPart); - Long versionIdPart = 1L; - - IIdType id = store(theResource, idPartAsString, versionIdPart); - - return new MethodOutcome() - .setCreated(true) - .setId(id); - } - - @Delete - public MethodOutcome delete(@IdParam IIdType theId) { - TreeMap versions = myIdToVersionToResourceMap.get(theId.getIdPart()); - if (versions == null || versions.isEmpty()) { - throw new ResourceNotFoundException(theId); - } - - long nextVersion = versions.lastEntry().getKey() + 1L; - IIdType id = store(null, theId.getIdPart(), nextVersion); - - return new MethodOutcome() - .setId(id); - } - - @Override - public Class getResourceType() { - return myResourceType; - } - - private synchronized TreeMap getVersionToResource(String theIdPart) { - if (!myIdToVersionToResourceMap.containsKey(theIdPart)) { - myIdToVersionToResourceMap.put(theIdPart, new TreeMap()); - } - return myIdToVersionToResourceMap.get(theIdPart); - } - - @Read(version = true) - public IBaseResource read(@IdParam IIdType theId) { - TreeMap versions = myIdToVersionToResourceMap.get(theId.getIdPart()); - if (versions == null || versions.isEmpty()) { - throw new ResourceNotFoundException(theId); - } - - if (theId.hasVersionIdPart()) { - Long versionId = theId.getVersionIdPartAsLong(); - if (!versions.containsKey(versionId)) { - throw new ResourceNotFoundException(theId); - } else { - T resource = versions.get(versionId); - if (resource == null) { - throw new ResourceGoneException(theId); - } - return resource; - } - - } else { - return versions.lastEntry().getValue(); - } - } - - @Search - public List search() { - List retVal = new ArrayList<>(); - - for (TreeMap next : myIdToVersionToResourceMap.values()) { - if (next.isEmpty() == false) { - retVal.add(next.lastEntry().getValue()); - } - } - - return retVal; - } - - private IIdType store(@ResourceParam T theResource, String theIdPart, Long theVersionIdPart) { - IIdType id = myFhirContext.getVersion().newIdType(); - id.setParts(null, myResourceName, theIdPart, Long.toString(theVersionIdPart)); - if (theResource != null) { - theResource.setId(id); - } - - TreeMap versionToResource = getVersionToResource(theIdPart); - versionToResource.put(theVersionIdPart, theResource); - - ourLog.info("Storing resource with ID: {}", id.getValue()); - - return id; - } - - public MethodOutcome update(T theResource) { - String idPartAsString = theResource.getIdElement().getIdPart(); - TreeMap versionToResource = getVersionToResource(idPartAsString); - - Long versionIdPart; - Boolean created; - if (versionToResource.isEmpty()) { - versionIdPart = 1L; - created = true; - } else { - versionIdPart = versionToResource.lastKey() + 1L; - created = false; - } - - IIdType id = store(theResource, idPartAsString, versionIdPart); - - return new MethodOutcome() - .setCreated(created) - .setId(id); - } -} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java index e876efa85fc..59a14d9f2b7 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java @@ -34,6 +34,7 @@ import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import ca.uhn.fhir.util.ValidateUtil; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -44,6 +45,8 @@ import org.slf4j.LoggerFactory; import java.util.*; import java.util.concurrent.atomic.AtomicLong; +import static org.apache.commons.lang3.StringUtils.isBlank; + /** * This class is a simple implementation of the resource provider * interface that uses a HashMap to store all resources in memory. @@ -338,8 +341,15 @@ public class HashMapResourceProvider implements IResour return id; } + /** + * @param theConditional This is provided only so that subclasses can implement if they want + */ @Update - public MethodOutcome update(@ResourceParam T theResource) { + public MethodOutcome update( + @ResourceParam T theResource, + @ConditionalUrlParam String theConditional) { + + ValidateUtil.isTrueOrThrowInvalidRequest(isBlank(theConditional), "This server doesn't support conditional update"); String idPartAsString = theResource.getIdElement().getIdPart(); TreeMap versionToResource = getVersionToResource(idPartAsString); From 0571e2a3fa15978bab0a618ad5aebae7846a6aef Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 11 Aug 2018 16:26:45 -0400 Subject: [PATCH 16/22] Downgrade OSGI versions --- osgi/hapi-fhir-karaf-features/pom.xml | 4 ++-- osgi/hapi-fhir-karaf-integration-tests/pom.xml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osgi/hapi-fhir-karaf-features/pom.xml b/osgi/hapi-fhir-karaf-features/pom.xml index 8a921d1a981..1ca1eeb8774 100644 --- a/osgi/hapi-fhir-karaf-features/pom.xml +++ b/osgi/hapi-fhir-karaf-features/pom.xml @@ -17,8 +17,8 @@ features.xml - 1.10.1 - 6.0.0 + 1.8.6 + 3.2.2 diff --git a/osgi/hapi-fhir-karaf-integration-tests/pom.xml b/osgi/hapi-fhir-karaf-integration-tests/pom.xml index 20a8d47e6cb..d8d98bd71ae 100644 --- a/osgi/hapi-fhir-karaf-integration-tests/pom.xml +++ b/osgi/hapi-fhir-karaf-integration-tests/pom.xml @@ -33,7 +33,7 @@ - 4.12.0 + 4.9.1 From bf68adfdba027c5cb2327ff101848cea3a9220fc Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 11 Aug 2018 16:56:46 -0400 Subject: [PATCH 17/22] Restore working POM for Karaf version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 71c4c33e5d3..5792e8842b5 100644 --- a/pom.xml +++ b/pom.xml @@ -489,7 +489,7 @@ 1.2.0 - 4.2.0 + 4.1.4 1.0.10 2.6.2 1.11 From 0f6188b73c6dfe90a79600badb82b4cae27c1eae Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 12 Aug 2018 09:56:47 -0400 Subject: [PATCH 18/22] Improve error message for unknown UUIDs in transaction bundles --- .../jpa/config/dstu3/BaseDstu3Config.java | 21 +- .../uhn/fhir/jpa/config/r4/BaseR4Config.java | 17 +- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 8 +- .../fhir/jpa/dao/TransactionProcessor.java | 944 ++++++++++++++++++ .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 835 +--------------- ...ansactionProcessorVersionAdapterDstu3.java | 133 +++ .../uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java | 812 +-------------- .../TransactionProcessorVersionAdapterR4.java | 133 +++ .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 27 + src/changes/changes.xml | 9 + 10 files changed, 1306 insertions(+), 1633 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java index 3ec6c62431e..04cf5103996 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java @@ -3,10 +3,8 @@ package ca.uhn.fhir.jpa.config.dstu3; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.ParserOptions; import ca.uhn.fhir.jpa.config.BaseConfig; -import ca.uhn.fhir.jpa.dao.FulltextSearchSvcImpl; -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.*; +import ca.uhn.fhir.jpa.dao.dstu3.TransactionProcessorVersionAdapterDstu3; import ca.uhn.fhir.jpa.dao.dstu3.SearchParamExtractorDstu3; import ca.uhn.fhir.jpa.dao.dstu3.SearchParamRegistryDstu3; import ca.uhn.fhir.jpa.provider.dstu3.TerminologyUploaderProviderDstu3; @@ -21,6 +19,7 @@ import org.apache.commons.lang3.time.DateUtils; import org.hl7.fhir.dstu3.hapi.ctx.IValidationSupport; import org.hl7.fhir.dstu3.hapi.validation.CachingValidationSupport; import org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator; +import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.r4.utils.IResourceValidator; import org.springframework.beans.factory.annotation.Autowire; import org.springframework.context.annotation.Bean; @@ -38,9 +37,9 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; * 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. @@ -70,6 +69,16 @@ public class BaseDstu3Config extends BaseConfig { return retVal; } + @Bean + public TransactionProcessor.ITransactionProcessorVersionAdapter transactionProcessorVersionFacade() { + return new TransactionProcessorVersionAdapterDstu3(); + } + + @Bean + public TransactionProcessor transactionProcessor() { + return new TransactionProcessor<>(); + } + @Bean(name = "myInstanceValidatorDstu3") @Lazy public IValidatorModule instanceValidatorDstu3() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/r4/BaseR4Config.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/r4/BaseR4Config.java index d55a3bc43fb..5dbcf649ff0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/r4/BaseR4Config.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/r4/BaseR4Config.java @@ -3,12 +3,10 @@ package ca.uhn.fhir.jpa.config.r4; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.ParserOptions; import ca.uhn.fhir.jpa.config.BaseConfig; -import ca.uhn.fhir.jpa.dao.FulltextSearchSvcImpl; -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.*; import ca.uhn.fhir.jpa.dao.r4.SearchParamExtractorR4; import ca.uhn.fhir.jpa.dao.r4.SearchParamRegistryR4; +import ca.uhn.fhir.jpa.dao.r4.TransactionProcessorVersionAdapterR4; import ca.uhn.fhir.jpa.graphql.JpaStorageServices; import ca.uhn.fhir.jpa.provider.r4.TerminologyUploaderProviderR4; import ca.uhn.fhir.jpa.term.HapiTerminologySvcR4; @@ -23,6 +21,7 @@ import org.hl7.fhir.r4.hapi.ctx.IValidationSupport; import org.hl7.fhir.r4.hapi.rest.server.GraphQLProvider; import org.hl7.fhir.r4.hapi.validation.CachingValidationSupport; import org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.utils.GraphQLEngine; import org.hl7.fhir.r4.utils.IResourceValidator.BestPracticeWarningLevel; import org.springframework.beans.factory.annotation.Autowire; @@ -73,6 +72,16 @@ public class BaseR4Config extends BaseConfig { return retVal; } + @Bean + public TransactionProcessor.ITransactionProcessorVersionAdapter transactionProcessorVersionFacade() { + return new TransactionProcessorVersionAdapterR4(); + } + + @Bean + public TransactionProcessor transactionProcessor() { + return new TransactionProcessor<>(); + } + @Bean(name = "myGraphQLProvider") @Lazy public GraphQLProvider graphQLProvider() { 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 92a9dbca55c..cf26d4f1f2f 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 @@ -206,7 +206,7 @@ public abstract class BaseHapiFhirDao implements IDao, private ApplicationContext myApplicationContext; private Map, IFhirResourceDao> myResourceTypeToDao; - protected void clearRequestAsProcessingSubRequest(ServletRequestDetails theRequestDetails) { + public static void clearRequestAsProcessingSubRequest(ServletRequestDetails theRequestDetails) { if (theRequestDetails != null) { theRequestDetails.getUserData().remove(PROCESSING_SUB_REQUEST); } @@ -1156,7 +1156,7 @@ public abstract class BaseHapiFhirDao implements IDao, return false; } - protected void markRequestAsProcessingSubRequest(ServletRequestDetails theRequestDetails) { + public static void markRequestAsProcessingSubRequest(ServletRequestDetails theRequestDetails) { if (theRequestDetails != null) { theRequestDetails.getUserData().put(PROCESSING_SUB_REQUEST, Boolean.TRUE); } @@ -1170,7 +1170,7 @@ public abstract class BaseHapiFhirDao implements IDao, return builder; } - protected void notifyInterceptors(RestOperationTypeEnum theOperationType, ActionRequestDetails theRequestDetails) { + public void notifyInterceptors(RestOperationTypeEnum theOperationType, ActionRequestDetails theRequestDetails) { if (theRequestDetails.getId() != null && theRequestDetails.getId().hasResourceType() && isNotBlank(theRequestDetails.getResourceType())) { if (theRequestDetails.getId().getResourceType().equals(theRequestDetails.getResourceType()) == false) { throw new InternalErrorException( @@ -2288,7 +2288,7 @@ public abstract class BaseHapiFhirDao implements IDao, } } - protected void validateDeleteConflictsEmptyOrThrowException(List theDeleteConflicts) { + public void validateDeleteConflictsEmptyOrThrowException(List theDeleteConflicts) { if (theDeleteConflicts.isEmpty()) { return; } 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 new file mode 100644 index 00000000000..cfa1376716c --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java @@ -0,0 +1,944 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails; +import ca.uhn.fhir.jpa.util.DeleteConflict; +import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; +import ca.uhn.fhir.parser.DataFormatException; +import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.PreferReturnEnum; +import ca.uhn.fhir.rest.api.RequestTypeEnum; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.param.ParameterUtil; +import ca.uhn.fhir.rest.server.RestfulServerUtils; +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.interceptor.IServerInterceptor; +import ca.uhn.fhir.rest.server.method.BaseMethodBinding; +import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.util.FhirTerser; +import ca.uhn.fhir.util.ResourceReferenceInfo; +import ca.uhn.fhir.util.StopWatch; +import ca.uhn.fhir.util.UrlUtil; +import com.google.common.collect.ArrayListMultimap; +import org.apache.commons.lang3.Validate; +import org.apache.http.NameValuePair; +import org.hibernate.Session; +import org.hibernate.internal.SessionImpl; +import org.hl7.fhir.dstu3.model.Bundle; +import org.hl7.fhir.exceptions.FHIRException; +import org.hl7.fhir.instance.model.api.*; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionCallback; +import org.springframework.transaction.support.TransactionTemplate; + +import javax.persistence.EntityManager; +import javax.persistence.PersistenceContext; +import javax.persistence.PersistenceContextType; +import java.util.*; + +import static org.apache.commons.lang3.StringUtils.*; + +public class TransactionProcessor { + + public static final String URN_PREFIX = "urn:"; + private static final Logger ourLog = LoggerFactory.getLogger(TransactionProcessor.class); + private BaseHapiFhirDao myDao; + @Autowired + private PlatformTransactionManager myTxManager; + @PersistenceContext(type = PersistenceContextType.TRANSACTION) + private EntityManager myEntityManager; + @Autowired + private FhirContext myContext; + @Autowired + private ITransactionProcessorVersionAdapter myVersionAdapter; + + public static boolean isPlaceholder(IIdType theId) { + if (theId != null && theId.getValue() != null) { + return theId.getValue().startsWith("urn:oid:") || theId.getValue().startsWith("urn:uuid:"); + } + return false; + } + + private static String toStatusString(int theStatusCode) { + return Integer.toString(theStatusCode) + " " + defaultString(Constants.HTTP_STATUS_NAMES.get(theStatusCode)); + } + + private void populateEntryWithOperationOutcome(BaseServerResponseException caughtEx, BUNDLEENTRY nextEntry) { + myVersionAdapter.populateEntryWithOperationOutcome(caughtEx, nextEntry); + } + + private void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, IIdType nextResourceId, DaoMethodOutcome outcome, + BUNDLEENTRY newEntry, String theResourceType, IBaseResource theRes, ServletRequestDetails theRequestDetails) { + IIdType newId = outcome.getId().toUnqualifiedVersionless(); + IIdType resourceId = isPlaceholder(nextResourceId) ? nextResourceId : nextResourceId.toUnqualifiedVersionless(); + if (newId.equals(resourceId) == false) { + idSubstitutions.put(resourceId, newId); + if (isPlaceholder(resourceId)) { + /* + * The correct way for substitution IDs to be is to be with no resource type, but we'll accept the qualified kind too just to be lenient. + */ + IIdType id = myContext.getVersion().newIdType(); + id.setValue(theResourceType + '/' + resourceId.getValue()); + idSubstitutions.put(id, newId); + } + } + idToPersistedOutcome.put(newId, outcome); + if (outcome.getCreated().booleanValue()) { + myVersionAdapter.setResponseStatus(newEntry, toStatusString(Constants.STATUS_HTTP_201_CREATED)); + } else { + myVersionAdapter.setResponseStatus(newEntry, toStatusString(Constants.STATUS_HTTP_200_OK)); + } + Date lastModifier = getLastModified(theRes); + myVersionAdapter.setResponseLastModified(newEntry, lastModifier); + + if (theRequestDetails != null) { + if (outcome.getResource() != null) { + String prefer = theRequestDetails.getHeader(Constants.HEADER_PREFER); + PreferReturnEnum preferReturn = RestfulServerUtils.parsePreferHeader(prefer); + if (preferReturn != null) { + if (preferReturn == PreferReturnEnum.REPRESENTATION) { + myVersionAdapter.setResource(newEntry, outcome.getResource()); + } + } + } + } + + } + + private Date getLastModified(IBaseResource theRes) { + return theRes.getMeta().getLastUpdated(); + } + + private String performIdSubstitutionsInMatchUrl(Map theIdSubstitutions, String theMatchUrl) { + String matchUrl = theMatchUrl; + if (isNotBlank(matchUrl)) { + for (Map.Entry nextSubstitutionEntry : theIdSubstitutions.entrySet()) { + IIdType nextTemporaryId = nextSubstitutionEntry.getKey(); + IIdType nextReplacementId = nextSubstitutionEntry.getValue(); + String nextTemporaryIdPart = nextTemporaryId.getIdPart(); + String nextReplacementIdPart = nextReplacementId.getValueAsString(); + if (isUrn(nextTemporaryId) && nextTemporaryIdPart.length() > URN_PREFIX.length()) { + matchUrl = matchUrl.replace(nextTemporaryIdPart, nextReplacementIdPart); + matchUrl = matchUrl.replace(UrlUtil.escapeUrlParam(nextTemporaryIdPart), nextReplacementIdPart); + } + } + } + return matchUrl; + } + + private boolean isUrn(IIdType theId) { + return defaultString(theId.getValue()).startsWith(URN_PREFIX); + } + + + public void setDao(BaseHapiFhirDao theDao) { + myDao = theDao; + } + + public BUNDLE transaction(RequestDetails theRequestDetails, BUNDLE theRequest) { + if (theRequestDetails != null) { + IServerInterceptor.ActionRequestDetails requestDetails = new IServerInterceptor.ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null); + myDao.notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails); + } + + String actionName = "Transaction"; + return processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, theRequest, actionName); + } + + private BUNDLE processTransactionAsSubRequest(ServletRequestDetails theRequestDetails, BUNDLE theRequest, String theActionName) { + BaseHapiFhirDao.markRequestAsProcessingSubRequest(theRequestDetails); + try { + return processTransaction(theRequestDetails, theRequest, theActionName); + } finally { + BaseHapiFhirDao.clearRequestAsProcessingSubRequest(theRequestDetails); + } + } + + private BUNDLE batch(final RequestDetails theRequestDetails, BUNDLE theRequest) { + ourLog.info("Beginning batch with {} resources", myVersionAdapter.getEntries(theRequest).size()); + long start = System.currentTimeMillis(); + + TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); + txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + + BUNDLE resp = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.BATCHRESPONSE.toCode()); + + /* + * 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 + */ + + for (final BUNDLEENTRY nextRequestEntry : myVersionAdapter.getEntries(theRequest)) { + + BaseServerResponseExceptionHolder caughtEx = new BaseServerResponseExceptionHolder(); + + TransactionCallback callback = theStatus -> { + BUNDLE subRequestBundle = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.TRANSACTION.toCode()); + myVersionAdapter.addEntry(subRequestBundle, nextRequestEntry); + + BUNDLE subResponseBundle = processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, subRequestBundle, "Batch sub-request"); + return subResponseBundle; + }; + + try { + // FIXME: this doesn't need to be a callback + BUNDLE nextResponseBundle = callback.doInTransaction(null); + + BUNDLEENTRY subResponseEntry = myVersionAdapter.getEntries(nextResponseBundle).get(0); + myVersionAdapter.addEntry(resp, subResponseEntry); + + /* + * If the individual entry didn't have a resource in its response, bring the sub-transaction's OperationOutcome across so the client can see it + */ + if (myVersionAdapter.getResource(subResponseEntry) == null) { + BUNDLEENTRY nextResponseBundleFirstEntry = myVersionAdapter.getEntries(nextResponseBundle).get(0); + myVersionAdapter.setResource(subResponseEntry, myVersionAdapter.getResource(nextResponseBundleFirstEntry)); + } + + } catch (BaseServerResponseException e) { + caughtEx.setException(e); + } catch (Throwable t) { + ourLog.error("Failure during BATCH sub transaction processing", t); + caughtEx.setException(new InternalErrorException(t)); + } + + if (caughtEx.getException() != null) { + BUNDLEENTRY nextEntry = myVersionAdapter.addEntry(resp); + + populateEntryWithOperationOutcome(caughtEx.getException(), nextEntry); + + myVersionAdapter.setResponseStatus(nextEntry, toStatusString(caughtEx.getException().getStatusCode())); + } + + } + + long delay = System.currentTimeMillis() - start; + ourLog.info("Batch completed in {}ms", new Object[]{delay}); + + return resp; + } + + private BUNDLE processTransaction(final ServletRequestDetails theRequestDetails, final BUNDLE theRequest, final String theActionName) { + validateDependencies(); + + String transactionType = myVersionAdapter.getBundleType(theRequest); + if (org.hl7.fhir.r4.model.Bundle.BundleType.BATCH.toCode().equals(transactionType)) { + return batch(theRequestDetails, theRequest); + } + + if (transactionType == null) { + String message = "Transaction Bundle did not specify valid Bundle.type, assuming " + Bundle.BundleType.TRANSACTION.toCode(); + ourLog.warn(message); + transactionType = org.hl7.fhir.r4.model.Bundle.BundleType.TRANSACTION.toCode(); + } + if (!org.hl7.fhir.r4.model.Bundle.BundleType.TRANSACTION.toCode().equals(transactionType)) { + throw new InvalidRequestException("Unable to process transaction where incoming Bundle.type = " + transactionType); + } + + ourLog.debug("Beginning {} with {} resources", theActionName, myVersionAdapter.getEntries(theRequest).size()); + + final Date updateTime = new Date(); + final StopWatch transactionStopWatch = new StopWatch(); + + final Set allIds = new LinkedHashSet<>(); + final Map idSubstitutions = new HashMap<>(); + final Map idToPersistedOutcome = new HashMap<>(); + List requestEntries = myVersionAdapter.getEntries(theRequest); + + // Do all entries have a verb? + for (int i = 0; i < myVersionAdapter.getEntries(theRequest).size(); i++) { + BUNDLEENTRY nextReqEntry = requestEntries.get(i); + String verb = myVersionAdapter.getEntryRequestVerb(nextReqEntry); + if (verb == null || !isValidVerb(verb)) { + throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionEntryHasInvalidVerb", verb, i)); + } + } + + /* + * We want to execute the transaction request bundle elements in the order + * specified by the FHIR specification (see TransactionSorter) so we save the + * original order in the request, then sort it. + * + * Entries with a type of GET are removed from the bundle so that they + * can be processed at the very end. We do this because the incoming resources + * are saved in a two-phase way in order to deal with interdependencies, and + * we want the GET processing to use the final indexing state + */ + final BUNDLE response = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.TRANSACTIONRESPONSE.toCode()); + List getEntries = new ArrayList<>(); + final IdentityHashMap originalRequestOrder = new IdentityHashMap<>(); + for (int i = 0; i < requestEntries.size(); i++) { + originalRequestOrder.put(requestEntries.get(i), i); + myVersionAdapter.addEntry(response); + if (myVersionAdapter.getEntryRequestVerb(requestEntries.get(i)).equals("GET")) { + getEntries.add(requestEntries.get(i)); + } + } + + /* + * See FhirSystemDaoDstu3Test#testTransactionWithPlaceholderIdInMatchUrl + * Basically if the resource has a match URL that references a placeholder, + * we try to handle the resource with the placeholder first. + */ + Set placeholderIds = new HashSet<>(); + final List entries = requestEntries; + for (BUNDLEENTRY nextEntry : entries) { + String fullUrl = myVersionAdapter.getFullUrl(nextEntry); + if (isNotBlank(fullUrl) && fullUrl.startsWith(URN_PREFIX)) { + placeholderIds.add(fullUrl); + } + } + Collections.sort(entries, new TransactionSorter(placeholderIds)); + + /* + * All of the write operations in the transaction (PUT, POST, etc.. basically anything + * except GET) are performed in their own database transaction before we do the reads. + * We do this because the reads (specifically the searches) often spawn their own + * secondary database transaction and if we allow that within the primary + * database transaction we can end up with deadlocks if the server is under + * heavy load with lots of concurrent transactions using all available + * database connections. + */ + TransactionTemplate txManager = new TransactionTemplate(myTxManager); + Map entriesToProcess = txManager.execute(status -> { + Map retVal = doTransactionWriteOperations(theRequestDetails, theActionName, updateTime, allIds, idSubstitutions, idToPersistedOutcome, response, originalRequestOrder, entries, transactionStopWatch); + + transactionStopWatch.startTask("Commit writes to database"); + return retVal; + }); + transactionStopWatch.endCurrentTask(); + + for (Map.Entry nextEntry : entriesToProcess.entrySet()) { + String responseLocation = nextEntry.getValue().getIdDt().toUnqualified().getValue(); + String responseEtag = nextEntry.getValue().getIdDt().getVersionIdPart(); + myVersionAdapter.setResponseLocation(nextEntry.getKey(), responseLocation); + myVersionAdapter.setResponseETag(nextEntry.getKey(), responseEtag); + } + + /* + * Loop through the request and process any entries of type GET + */ + if (getEntries.size() > 0) { + transactionStopWatch.startTask("Process " + getEntries.size() + " GET entries"); + } + for (BUNDLEENTRY nextReqEntry : getEntries) { + Integer originalOrder = originalRequestOrder.get(nextReqEntry); + BUNDLEENTRY nextRespEntry = myVersionAdapter.getEntries(response).get(originalOrder); + + ServletSubRequestDetails requestDetails = new ServletSubRequestDetails(); + requestDetails.setServletRequest(theRequestDetails.getServletRequest()); + requestDetails.setRequestType(RequestTypeEnum.GET); + requestDetails.setServer(theRequestDetails.getServer()); + + String url = extractTransactionUrlOrThrowException(nextReqEntry, "GET"); + + int qIndex = url.indexOf('?'); + ArrayListMultimap paramValues = ArrayListMultimap.create(); + requestDetails.setParameters(new HashMap<>()); + if (qIndex != -1) { + String params = url.substring(qIndex); + List parameters = BaseHapiFhirDao.translateMatchUrl(params); + for (NameValuePair next : parameters) { + paramValues.put(next.getName(), next.getValue()); + } + for (Map.Entry> nextParamEntry : paramValues.asMap().entrySet()) { + String[] nextValue = nextParamEntry.getValue().toArray(new String[nextParamEntry.getValue().size()]); + requestDetails.addParameter(nextParamEntry.getKey(), nextValue); + } + url = url.substring(0, qIndex); + } + + requestDetails.setRequestPath(url); + requestDetails.setFhirServerBase(theRequestDetails.getFhirServerBase()); + + theRequestDetails.getServer().populateRequestDetailsFromRequestPath(requestDetails, url); + BaseMethodBinding method = theRequestDetails.getServer().determineResourceMethod(requestDetails, url); + if (method == null) { + throw new IllegalArgumentException("Unable to handle GET " + url); + } + + if (isNotBlank(myVersionAdapter.getEntryRequestIfMatch(nextReqEntry))) { + requestDetails.addHeader(Constants.HEADER_IF_MATCH, myVersionAdapter.getEntryRequestIfMatch(nextReqEntry)); + } + if (isNotBlank(myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry))) { + requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry)); + } + if (isNotBlank(myVersionAdapter.getEntryRequestIfNoneMatch(nextReqEntry))) { + requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, myVersionAdapter.getEntryRequestIfNoneMatch(nextReqEntry)); + } + + Validate.isTrue(method instanceof BaseResourceReturningMethodBinding, "Unable to handle GET {}", url); + try { + IBaseResource resource = ((BaseResourceReturningMethodBinding) method).doInvokeServer(theRequestDetails.getServer(), requestDetails); + if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { + resource = filterNestedBundle(requestDetails, resource); + } + myVersionAdapter.setResource(nextRespEntry, resource); + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(Constants.STATUS_HTTP_200_OK)); + } catch (NotModifiedException e) { + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); + } catch (BaseServerResponseException e) { + ourLog.info("Failure processing transaction GET {}: {}", url, e.toString()); + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(e.getStatusCode())); + populateEntryWithOperationOutcome(e, nextRespEntry); + } + + } + transactionStopWatch.endCurrentTask(); + + ourLog.info("Transaction timing:\n{}", transactionStopWatch.formatTaskDurations()); + + return response; + } + + private boolean isValidVerb(String theVerb) { + try { + return org.hl7.fhir.r4.model.Bundle.HTTPVerb.fromCode(theVerb) != null; + } catch (FHIRException theE) { + return false; + } + } + + /** + * This method is called for nested bundles (e.g. if we received a transaction with an entry that + * was a GET search, this method is called on the bundle for the search result, that will be placed in the + * outer bundle). This method applies the _summary and _content parameters to the output of + * that bundle. + *

    + * TODO: This isn't the most efficient way of doing this.. hopefully we can come up with something better in the future. + */ + private IBaseResource filterNestedBundle(RequestDetails theRequestDetails, IBaseResource theResource) { + IParser p = myContext.newJsonParser(); + RestfulServerUtils.configureResponseParser(theRequestDetails, p); + return p.parseResource(theResource.getClass(), p.encodeResourceToString(theResource)); + } + + public void setEntityManager(EntityManager theEntityManager) { + myEntityManager = theEntityManager; + } + + private void validateDependencies() { + Validate.notNull(myEntityManager); + Validate.notNull(myContext); + Validate.notNull(myDao); + Validate.notNull(myTxManager); + } + + private IIdType newIdType(String theValue) { + return myContext.getVersion().newIdType().setValue(theValue); + } + + private Map doTransactionWriteOperations(ServletRequestDetails theRequestDetails, String theActionName, Date theUpdateTime, Set theAllIds, + Map theIdSubstitutions, Map theIdToPersistedOutcome, BUNDLE theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries, StopWatch theTransactionStopWatch) { + Set deletedResources = new HashSet<>(); + List deleteConflicts = new ArrayList<>(); + Map entriesToProcess = new IdentityHashMap<>(); + Set nonUpdatedEntities = new HashSet<>(); + Set updatedEntities = new HashSet<>(); + Map> conditionalRequestUrls = new HashMap<>(); + + /* + * Loop through the request and process any entries of type + * PUT, POST or DELETE + */ + for (int i = 0; i < theEntries.size(); i++) { + + if (i % 100 == 0) { + ourLog.debug("Processed {} non-GET entries out of {}", i, theEntries.size()); + } + + BUNDLEENTRY nextReqEntry = theEntries.get(i); + IBaseResource res = myVersionAdapter.getResource(nextReqEntry); + IIdType nextResourceId = null; + if (res != null) { + + nextResourceId = res.getIdElement(); + + if (!nextResourceId.hasIdPart()) { + if (isNotBlank(myVersionAdapter.getFullUrl(nextReqEntry))) { + nextResourceId = newIdType(myVersionAdapter.getFullUrl(nextReqEntry)); + } + } + + if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+\\:.*") && !isPlaceholder(nextResourceId)) { + throw new InvalidRequestException("Invalid placeholder ID found: " + nextResourceId.getIdPart() + " - Must be of the form 'urn:uuid:[uuid]' or 'urn:oid:[oid]'"); + } + + if (nextResourceId.hasIdPart() && !nextResourceId.hasResourceType() && !isPlaceholder(nextResourceId)) { + nextResourceId = newIdType(toResourceName(res.getClass()), nextResourceId.getIdPart()); + res.setId(nextResourceId); + } + + /* + * Ensure that the bundle doesn't have any duplicates, since this causes all kinds of weirdness + */ + if (isPlaceholder(nextResourceId)) { + if (!theAllIds.add(nextResourceId)) { + throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextResourceId)); + } + } else if (nextResourceId.hasResourceType() && nextResourceId.hasIdPart()) { + IIdType nextId = nextResourceId.toUnqualifiedVersionless(); + if (!theAllIds.add(nextId)) { + throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextId)); + } + } + + } + + String verb = myVersionAdapter.getEntryRequestVerb(nextReqEntry); + String resourceType = res != null ? myContext.getResourceDefinition(res).getName() : null; + Integer order = theOriginalRequestOrder.get(nextReqEntry); + BUNDLEENTRY nextRespEntry = myVersionAdapter.getEntries(theResponse).get(order); + + theTransactionStopWatch.startTask("Bundle.entry[" + i + "]: " + verb + " " + defaultString(resourceType)); + + switch (verb) { + case "POST": { + // CREATE + @SuppressWarnings("rawtypes") + IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); + res.setId((String) null); + DaoMethodOutcome outcome; + String matchUrl = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry); + matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); + outcome = resourceDao.create(res, matchUrl, false, theRequestDetails); + if (nextResourceId != null) { + handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); + } + entriesToProcess.put(nextRespEntry, outcome.getEntity()); + if (outcome.getCreated() == false) { + nonUpdatedEntities.add(outcome.getEntity()); + } else { + if (isNotBlank(matchUrl)) { + conditionalRequestUrls.put(matchUrl, res.getClass()); + } + } + + break; + } + case "DELETE": { + // DELETE + String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); + UrlUtil.UrlParts parts = UrlUtil.parseUrl(url); + ca.uhn.fhir.jpa.dao.IFhirResourceDao dao = toDao(parts, verb, url); + int status = Constants.STATUS_HTTP_204_NO_CONTENT; + if (parts.getResourceId() != null) { + IIdType deleteId = newIdType(parts.getResourceType(), parts.getResourceId()); + if (!deletedResources.contains(deleteId.getValueAsString())) { + DaoMethodOutcome outcome = dao.delete(deleteId, deleteConflicts, theRequestDetails); + if (outcome.getEntity() != null) { + deletedResources.add(deleteId.getValueAsString()); + entriesToProcess.put(nextRespEntry, outcome.getEntity()); + } + } + } else { + String matchUrl = parts.getResourceType() + '?' + parts.getParams(); + matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); + DeleteMethodOutcome deleteOutcome = dao.deleteByUrl(matchUrl, deleteConflicts, theRequestDetails); + List allDeleted = deleteOutcome.getDeletedEntities(); + for (ResourceTable deleted : allDeleted) { + deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless().getValueAsString()); + } + if (allDeleted.isEmpty()) { + status = Constants.STATUS_HTTP_204_NO_CONTENT; + } + + myVersionAdapter.setResponseOutcome(nextRespEntry, deleteOutcome.getOperationOutcome()); + } + + myVersionAdapter.setResponseStatus(nextRespEntry, toStatusString(status)); + + break; + } + case "PUT": { + // UPDATE + @SuppressWarnings("rawtypes") + IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); + + String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); + + DaoMethodOutcome outcome; + UrlUtil.UrlParts parts = UrlUtil.parseUrl(url); + if (isNotBlank(parts.getResourceId())) { + String version = null; + if (isNotBlank(myVersionAdapter.getEntryRequestIfMatch(nextReqEntry))) { + version = ParameterUtil.parseETagValue(myVersionAdapter.getEntryRequestIfMatch(nextReqEntry)); + } + res.setId(newIdType(parts.getResourceType(), parts.getResourceId(), version)); + outcome = resourceDao.update(res, null, false, theRequestDetails); + } else { + res.setId((String) null); + String matchUrl; + if (isNotBlank(parts.getParams())) { + matchUrl = parts.getResourceType() + '?' + parts.getParams(); + } else { + matchUrl = parts.getResourceType(); + } + matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); + outcome = resourceDao.update(res, matchUrl, false, theRequestDetails); + if (Boolean.TRUE.equals(outcome.getCreated())) { + conditionalRequestUrls.put(matchUrl, res.getClass()); + } + } + + if (outcome.getCreated() == Boolean.FALSE) { + updatedEntities.add(outcome.getEntity()); + } + + handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); + entriesToProcess.put(nextRespEntry, outcome.getEntity()); + break; + } + case "GET": + default: + break; + + } + + theTransactionStopWatch.endCurrentTask(); + } + + + /* + * Make sure that there are no conflicts from deletions. E.g. we can't delete something + * if something else has a reference to it.. Unless the thing that has a reference to it + * was also deleted as a part of this transaction, which is why we check this now at the + * end. + */ + + deleteConflicts.removeIf(next -> + deletedResources.contains(next.getTargetId().toUnqualifiedVersionless().getValue())); + myDao.validateDeleteConflictsEmptyOrThrowException(deleteConflicts); + + /* + * Perform ID substitutions and then index each resource we have saved + */ + + FhirTerser terser = myContext.newTerser(); + theTransactionStopWatch.startTask("Index " + theIdToPersistedOutcome.size() + " resources"); + for (DaoMethodOutcome nextOutcome : theIdToPersistedOutcome.values()) { + IBaseResource nextResource = nextOutcome.getResource(); + if (nextResource == null) { + continue; + } + + // References + List allRefs = terser.getAllResourceReferences(nextResource); + for (ResourceReferenceInfo nextRef : allRefs) { + IIdType nextId = nextRef.getResourceReference().getReferenceElement(); + if (!nextId.hasIdPart()) { + continue; + } + if (theIdSubstitutions.containsKey(nextId)) { + IIdType newId = theIdSubstitutions.get(nextId); + ourLog.debug(" * Replacing resource ref {} with {}", nextId, newId); + nextRef.getResourceReference().setReference(newId.getValue()); + } else if (nextId.getValue().startsWith("urn:")) { + throw new InvalidRequestException("Unable to satisfy placeholder ID " + nextId.getValue() + " found in element named '" + nextRef.getName() + "' within resource of type: " + nextResource.getIdElement().getResourceType()); + } else { + ourLog.debug(" * Reference [{}] does not exist in bundle", nextId); + } + } + + // URIs + Class> uriType = (Class>) myContext.getElementDefinition("uri").getImplementingClass(); + List> allUris = terser.getAllPopulatedChildElementsOfType(nextResource, uriType); + for (IPrimitiveType nextRef : allUris) { + if (nextRef instanceof IIdType) { + continue; // No substitution on the resource ID itself! + } + IIdType nextUriString = newIdType(nextRef.getValueAsString()); + if (theIdSubstitutions.containsKey(nextUriString)) { + IIdType newId = theIdSubstitutions.get(nextUriString); + ourLog.debug(" * Replacing resource ref {} with {}", nextUriString, newId); + nextRef.setValueAsString(newId.getValue()); + } else { + ourLog.debug(" * Reference [{}] does not exist in bundle", nextUriString); + } + } + + IPrimitiveType deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource); + Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; + + if (updatedEntities.contains(nextOutcome.getEntity())) { + myDao.updateInternal(theRequestDetails, nextResource, true, false, theRequestDetails, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); + } else if (!nonUpdatedEntities.contains(nextOutcome.getEntity())) { + myDao.updateEntity(theRequestDetails, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theUpdateTime, false, true); + } + } + + theTransactionStopWatch.endCurrentTask(); + theTransactionStopWatch.startTask("Flush writes to database"); + + flushJpaSession(); + + theTransactionStopWatch.endCurrentTask(); + if (conditionalRequestUrls.size() > 0) { + theTransactionStopWatch.startTask("Check for conflicts in conditional resources"); + } + + /* + * Double check we didn't allow any duplicates we shouldn't have + */ + for (Map.Entry> nextEntry : conditionalRequestUrls.entrySet()) { + String matchUrl = nextEntry.getKey(); + Class resType = nextEntry.getValue(); + if (isNotBlank(matchUrl)) { + IFhirResourceDao resourceDao = myDao.getDao(resType); + Set val = resourceDao.processMatchUrl(matchUrl); + if (val.size() > 1) { + throw new InvalidRequestException( + "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); + } + } + } + + theTransactionStopWatch.endCurrentTask(); + + for (IIdType next : theAllIds) { + IIdType replacement = theIdSubstitutions.get(next); + if (replacement == null) { + continue; + } + if (replacement.equals(next)) { + continue; + } + ourLog.debug("Placeholder resource ID \"{}\" was replaced with permanent ID \"{}\"", next, replacement); + } + return entriesToProcess; + } + + private IIdType newIdType(String theResourceType, String theResourceId, String theVersion) { + org.hl7.fhir.r4.model.IdType id = new org.hl7.fhir.r4.model.IdType(theResourceType, theResourceId, theVersion); + return myContext.getVersion().newIdType().setValue(id.getValue()); + } + + private IIdType newIdType(String theToResourceName, String theIdPart) { + return newIdType(theToResourceName, theIdPart, null); + } + + private IFhirResourceDao getDaoOrThrowException(Class theClass) { + return myDao.getDaoOrThrowException(theClass); + } + + protected void flushJpaSession() { + SessionImpl session = (SessionImpl) myEntityManager.unwrap(Session.class); + int insertionCount = session.getActionQueue().numberOfInsertions(); + int updateCount = session.getActionQueue().numberOfUpdates(); + + StopWatch sw = new StopWatch(); + myEntityManager.flush(); + ourLog.debug("Session flush took {}ms for {} inserts and {} updates", sw.getMillis(), insertionCount, updateCount); + } + + protected String toResourceName(Class theResourceType) { + return myContext.getResourceDefinition(theResourceType).getName(); + } + + public void setContext(FhirContext theContext) { + myContext = theContext; + } + + private String extractTransactionUrlOrThrowException(BUNDLEENTRY nextEntry, String verb) { + String url = myVersionAdapter.getEntryRequestUrl(nextEntry); + if (isBlank(url)) { + throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionMissingUrl", verb)); + } + return url; + } + + private ca.uhn.fhir.jpa.dao.IFhirResourceDao toDao(UrlUtil.UrlParts theParts, String theVerb, String theUrl) { + RuntimeResourceDefinition resType; + try { + resType = myContext.getResourceDefinition(theParts.getResourceType()); + } catch (DataFormatException e) { + String msg = myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); + throw new InvalidRequestException(msg); + } + IFhirResourceDao dao = null; + if (resType != null) { + dao = myDao.getDao(resType.getImplementingClass()); + } + if (dao == null) { + String msg = myContext.getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); + throw new InvalidRequestException(msg); + } + + // if (theParts.getResourceId() == null && theParts.getParams() == null) { + // String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); + // throw new InvalidRequestException(msg); + // } + + return dao; + } + + public interface ITransactionProcessorVersionAdapter { + + void setResponseStatus(BUNDLEENTRY theBundleEntry, String theStatus); + + void setResponseLastModified(BUNDLEENTRY theBundleEntry, Date theLastModified); + + void setResource(BUNDLEENTRY theBundleEntry, IBaseResource theResource); + + IBaseResource getResource(BUNDLEENTRY theBundleEntry); + + String getBundleType(BUNDLE theRequest); + + void populateEntryWithOperationOutcome(BaseServerResponseException theCaughtEx, BUNDLEENTRY theEntry); + + BUNDLE createBundle(String theBundleType); + + List getEntries(BUNDLE theRequest); + + void addEntry(BUNDLE theBundle, BUNDLEENTRY theEntry); + + BUNDLEENTRY addEntry(BUNDLE theBundle); + + String getEntryRequestVerb(BUNDLEENTRY theEntry); + + String getFullUrl(BUNDLEENTRY theEntry); + + String getEntryIfNoneExist(BUNDLEENTRY theEntry); + + String getEntryRequestUrl(BUNDLEENTRY theEntry); + + void setResponseLocation(BUNDLEENTRY theEntry, String theResponseLocation); + + void setResponseETag(BUNDLEENTRY theEntry, String theEtag); + + String getEntryRequestIfMatch(BUNDLEENTRY theEntry); + + String getEntryRequestIfNoneExist(BUNDLEENTRY theEntry); + + String getEntryRequestIfNoneMatch(BUNDLEENTRY theEntry); + + void setResponseOutcome(BUNDLEENTRY theEntry, IBaseOperationOutcome theOperationOutcome); + } + + private static class BaseServerResponseExceptionHolder { + private BaseServerResponseException myException; + + public BaseServerResponseException getException() { + return myException; + } + + public void setException(BaseServerResponseException myException) { + this.myException = myException; + } + } + + /** + * Transaction Order, per the spec: + *

    + * Process any DELETE interactions + * Process any POST interactions + * Process any PUT interactions + * Process any GET interactions + */ + //@formatter:off + public class TransactionSorter implements Comparator { + + private Set myPlaceholderIds; + + public TransactionSorter(Set thePlaceholderIds) { + myPlaceholderIds = thePlaceholderIds; + } + + @Override + public int compare(BUNDLEENTRY theO1, BUNDLEENTRY theO2) { + int o1 = toOrder(theO1); + int o2 = toOrder(theO2); + + if (o1 == o2) { + String matchUrl1 = toMatchUrl(theO1); + String matchUrl2 = toMatchUrl(theO2); + if (isBlank(matchUrl1) && isBlank(matchUrl2)) { + return 0; + } + if (isBlank(matchUrl1)) { + return -1; + } + if (isBlank(matchUrl2)) { + return 1; + } + + boolean match1containsSubstitutions = false; + boolean match2containsSubstitutions = false; + for (String nextPlaceholder : myPlaceholderIds) { + if (matchUrl1.contains(nextPlaceholder)) { + match1containsSubstitutions = true; + } + if (matchUrl2.contains(nextPlaceholder)) { + match2containsSubstitutions = true; + } + } + + if (match1containsSubstitutions && match2containsSubstitutions) { + return 0; + } + if (!match1containsSubstitutions && !match2containsSubstitutions) { + return 0; + } + if (match1containsSubstitutions) { + return 1; + } else { + return -1; + } + } + + return o1 - o2; + } + + private String toMatchUrl(BUNDLEENTRY theEntry) { + String verb = myVersionAdapter.getEntryRequestVerb(theEntry); + if (verb.equals("POST")) { + return myVersionAdapter.getEntryIfNoneExist(theEntry); + } + if (verb.equals("PUT") || verb.equals("DELETE")) { + String url = extractTransactionUrlOrThrowException(theEntry, verb); + UrlUtil.UrlParts parts = UrlUtil.parseUrl(url); + if (isBlank(parts.getResourceId())) { + return parts.getResourceType() + '?' + parts.getParams(); + } + } + return null; + } + + private int toOrder(BUNDLEENTRY theO1) { + int o1 = 0; + if (myVersionAdapter.getEntryRequestVerb(theO1) != null) { + switch (myVersionAdapter.getEntryRequestVerb(theO1)) { + case "DELETE": + o1 = 1; + break; + case "POST": + o1 = 2; + break; + case "PUT": + o1 = 3; + break; + case "GET": + o1 = 4; + break; + default: + o1 = 0; + break; + } + } + return o1; + } + + } + +} 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 2204f1c49f9..fd7cbbef632 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 @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao.dstu3; * 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. @@ -20,610 +20,34 @@ package ca.uhn.fhir.jpa.dao.dstu3; * #L% */ -import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.dao.BaseHapiFhirSystemDao; -import ca.uhn.fhir.jpa.dao.DaoMethodOutcome; -import ca.uhn.fhir.jpa.dao.DeleteMethodOutcome; -import ca.uhn.fhir.jpa.dao.IFhirResourceDao; -import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.dao.TransactionProcessor; import ca.uhn.fhir.jpa.entity.TagDefinition; -import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails; -import ca.uhn.fhir.jpa.util.DeleteConflict; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; -import ca.uhn.fhir.parser.DataFormatException; -import ca.uhn.fhir.parser.IParser; -import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.PreferReturnEnum; -import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.param.ParameterUtil; -import ca.uhn.fhir.rest.server.RestfulServerUtils; -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.interceptor.IServerInterceptor.ActionRequestDetails; -import ca.uhn.fhir.rest.server.method.BaseMethodBinding; -import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.FhirTerser; -import ca.uhn.fhir.util.StopWatch; -import ca.uhn.fhir.util.UrlUtil; -import ca.uhn.fhir.util.UrlUtil.UrlParts; -import com.google.common.collect.ArrayListMultimap; -import org.apache.commons.lang3.Validate; -import org.apache.http.NameValuePair; -import org.hl7.fhir.dstu3.model.*; +import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent; -import org.hl7.fhir.dstu3.model.Bundle.BundleEntryResponseComponent; -import org.hl7.fhir.dstu3.model.Bundle.BundleType; -import org.hl7.fhir.dstu3.model.Bundle.HTTPVerb; -import org.hl7.fhir.dstu3.model.OperationOutcome.IssueSeverity; -import org.hl7.fhir.instance.model.api.*; +import org.hl7.fhir.dstu3.model.Meta; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionCallback; -import org.springframework.transaction.support.TransactionTemplate; +import javax.annotation.PostConstruct; import javax.persistence.TypedQuery; -import java.util.*; -import java.util.Map.Entry; - -import static org.apache.commons.lang3.StringUtils.*; +import java.util.Collection; +import java.util.List; public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoDstu3.class); - @Autowired - private PlatformTransactionManager myTxManager; + private TransactionProcessor myTransactionProcessor; - private Bundle batch(final RequestDetails theRequestDetails, Bundle theRequest) { - ourLog.info("Beginning batch with {} resources", theRequest.getEntry().size()); - long start = System.currentTimeMillis(); - - TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); - - Bundle resp = new Bundle(); - resp.setType(BundleType.BATCHRESPONSE); - - /* - * 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 - */ - - for (final BundleEntryComponent nextRequestEntry : theRequest.getEntry()) { - - BaseServerResponseExceptionHolder caughtEx = new BaseServerResponseExceptionHolder(); - - TransactionCallback callback = new TransactionCallback() { - @Override - public Bundle doInTransaction(TransactionStatus theStatus) { - Bundle subRequestBundle = new Bundle(); - subRequestBundle.setType(BundleType.TRANSACTION); - subRequestBundle.addEntry(nextRequestEntry); - - Bundle subResponseBundle = transaction((ServletRequestDetails) theRequestDetails, subRequestBundle, "Batch sub-request"); - return subResponseBundle; - } - }; - - try { - Bundle nextResponseBundle = callback.doInTransaction(null); - - BundleEntryComponent subResponseEntry = nextResponseBundle.getEntry().get(0); - resp.addEntry(subResponseEntry); - - /* - * If the individual entry didn't have a resource in its response, bring the sub-transaction's OperationOutcome across so the client can see it - */ - if (subResponseEntry.getResource() == null) { - subResponseEntry.setResource(nextResponseBundle.getEntry().get(0).getResource()); - } - - } catch (BaseServerResponseException e) { - caughtEx.setException(e); - } catch (Throwable t) { - ourLog.error("Failure during BATCH sub transaction processing", t); - caughtEx.setException(new InternalErrorException(t)); - } - - if (caughtEx.getException() != null) { - BundleEntryComponent nextEntry = resp.addEntry(); - - populateEntryWithOperationOutcome(caughtEx.getException(), nextEntry); - - BundleEntryResponseComponent nextEntryResp = nextEntry.getResponse(); - nextEntryResp.setStatus(toStatusString(caughtEx.getException().getStatusCode())); - } - - } - - long delay = System.currentTimeMillis() - start; - ourLog.info("Batch completed in {}ms", new Object[] {delay}); - - return resp; + @PostConstruct + public void start() { + myTransactionProcessor.setDao(this); } - private Bundle doTransaction(final ServletRequestDetails theRequestDetails, final Bundle theRequest, final String theActionName) { - BundleType transactionType = theRequest.getTypeElement().getValue(); - if (transactionType == BundleType.BATCH) { - return batch(theRequestDetails, theRequest); - } - - if (transactionType == null) { - String message = "Transaction Bundle did not specify valid Bundle.type, assuming " + BundleType.TRANSACTION.toCode(); - ourLog.warn(message); - transactionType = BundleType.TRANSACTION; - } - if (transactionType != BundleType.TRANSACTION) { - throw new InvalidRequestException("Unable to process transaction where incoming Bundle.type = " + transactionType.toCode()); - } - - ourLog.debug("Beginning {} with {} resources", theActionName, theRequest.getEntry().size()); - - final Date updateTime = new Date(); - final StopWatch transactionStopWatch = new StopWatch(); - - final Set allIds = new LinkedHashSet<>(); - final Map idSubstitutions = new HashMap<>(); - final Map idToPersistedOutcome = new HashMap<>(); - - // Do all entries have a verb? - for (int i = 0; i < theRequest.getEntry().size(); i++) { - BundleEntryComponent nextReqEntry = theRequest.getEntry().get(i); - HTTPVerb verb = nextReqEntry.getRequest().getMethodElement().getValue(); - if (verb == null) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionEntryHasInvalidVerb", nextReqEntry.getRequest().getMethod(), i)); - } - } - - /* - * We want to execute the transaction request bundle elements in the order - * specified by the FHIR specification (see TransactionSorter) so we save the - * original order in the request, then sort it. - * - * Entries with a type of GET are removed from the bundle so that they - * can be processed at the very end. We do this because the incoming resources - * are saved in a two-phase way in order to deal with interdependencies, and - * we want the GET processing to use the final indexing state - */ - final Bundle response = new Bundle(); - List getEntries = new ArrayList(); - final IdentityHashMap originalRequestOrder = new IdentityHashMap(); - for (int i = 0; i < theRequest.getEntry().size(); i++) { - originalRequestOrder.put(theRequest.getEntry().get(i), i); - response.addEntry(); - if (theRequest.getEntry().get(i).getRequest().getMethodElement().getValue() == HTTPVerb.GET) { - getEntries.add(theRequest.getEntry().get(i)); - } - } - - /* - * See FhirSystemDaoDstu3Test#testTransactionWithPlaceholderIdInMatchUrl - * Basically if the resource has a match URL that references a placeholder, - * we try to handle the resource with the placeholder first. - */ - Set placeholderIds = new HashSet<>(); - final List entries = theRequest.getEntry(); - for (BundleEntryComponent nextEntry : entries) { - if (isNotBlank(nextEntry.getFullUrl()) && nextEntry.getFullUrl().startsWith(IdType.URN_PREFIX)) { - placeholderIds.add(nextEntry.getFullUrl()); - } - } - Collections.sort(entries, new TransactionSorter(placeholderIds)); - - /* - * All of the write operations in the transaction (PUT, POST, etc.. basically anything - * except GET) are performed in their own database transaction before we do the reads. - * We do this because the reads (specifically the searches) often spawn their own - * secondary database transaction and if we allow that within the primary - * database transaction we can end up with deadlocks if the server is under - * heavy load with lots of concurrent transactions using all available - * database connections. - */ - TransactionTemplate txManager = new TransactionTemplate(myTxManager); - Map entriesToProcess = txManager.execute(new TransactionCallback>() { - @Override - public Map doInTransaction(TransactionStatus status) { - Map retVal = doTransactionWriteOperations(theRequestDetails, theActionName, updateTime, allIds, idSubstitutions, idToPersistedOutcome, response, originalRequestOrder, entries, transactionStopWatch); - - transactionStopWatch.startTask("Commit writes to database"); - return retVal; - } - }); - transactionStopWatch.endCurrentTask(); - - for (Entry nextEntry : entriesToProcess.entrySet()) { - String responseLocation = nextEntry.getValue().getIdDt().toUnqualified().getValue(); - String responseEtag = nextEntry.getValue().getIdDt().getVersionIdPart(); - nextEntry.getKey().getResponse().setLocation(responseLocation); - nextEntry.getKey().getResponse().setEtag(responseEtag); - } - - /* - * Loop through the request and process any entries of type GET - */ - if (getEntries.size() > 0) { - transactionStopWatch.startTask("Process " + getEntries.size() + " GET entries"); - } - for (BundleEntryComponent nextReqEntry : getEntries) { - Integer originalOrder = originalRequestOrder.get(nextReqEntry); - BundleEntryComponent nextRespEntry = response.getEntry().get(originalOrder); - - ServletSubRequestDetails requestDetails = new ServletSubRequestDetails(); - requestDetails.setServletRequest(theRequestDetails.getServletRequest()); - requestDetails.setRequestType(RequestTypeEnum.GET); - requestDetails.setServer(theRequestDetails.getServer()); - - String url = extractTransactionUrlOrThrowException(nextReqEntry, HTTPVerb.GET); - - int qIndex = url.indexOf('?'); - ArrayListMultimap paramValues = ArrayListMultimap.create(); - requestDetails.setParameters(new HashMap<>()); - if (qIndex != -1) { - String params = url.substring(qIndex); - List parameters = translateMatchUrl(params); - for (NameValuePair next : parameters) { - paramValues.put(next.getName(), next.getValue()); - } - for (Entry> nextParamEntry : paramValues.asMap().entrySet()) { - String[] nextValue = nextParamEntry.getValue().toArray(new String[nextParamEntry.getValue().size()]); - requestDetails.addParameter(nextParamEntry.getKey(), nextValue); - } - url = url.substring(0, qIndex); - } - - requestDetails.setRequestPath(url); - requestDetails.setFhirServerBase(theRequestDetails.getFhirServerBase()); - - theRequestDetails.getServer().populateRequestDetailsFromRequestPath(requestDetails, url); - BaseMethodBinding method = theRequestDetails.getServer().determineResourceMethod(requestDetails, url); - if (method == null) { - throw new IllegalArgumentException("Unable to handle GET " + url); - } - - if (isNotBlank(nextReqEntry.getRequest().getIfMatch())) { - requestDetails.addHeader(Constants.HEADER_IF_MATCH, nextReqEntry.getRequest().getIfMatch()); - } - if (isNotBlank(nextReqEntry.getRequest().getIfNoneExist())) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, nextReqEntry.getRequest().getIfNoneExist()); - } - if (isNotBlank(nextReqEntry.getRequest().getIfNoneMatch())) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, nextReqEntry.getRequest().getIfNoneMatch()); - } - - Validate.isTrue(method instanceof BaseResourceReturningMethodBinding, "Unable to handle GET {}", url); - try { - IBaseResource resource = ((BaseResourceReturningMethodBinding) method).doInvokeServer(theRequestDetails.getServer(), requestDetails); - if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { - resource = filterNestedBundle(requestDetails, resource); - } - nextRespEntry.setResource((Resource) resource); - nextRespEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); - } catch (NotModifiedException e) { - nextRespEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); - } catch (BaseServerResponseException e) { - ourLog.info("Failure processing transaction GET {}: {}", url, e.toString()); - nextRespEntry.getResponse().setStatus(toStatusString(e.getStatusCode())); - populateEntryWithOperationOutcome(e, nextRespEntry); - } - - } - transactionStopWatch.endCurrentTask(); - - response.setType(BundleType.TRANSACTIONRESPONSE); - - ourLog.info("Transaction timing:\n{}", transactionStopWatch.formatTaskDurations()); - - return response; - } - - private Map doTransactionWriteOperations(ServletRequestDetails theRequestDetails, String theActionName, Date theUpdateTime, Set theAllIds, - Map theIdSubstitutions, Map theIdToPersistedOutcome, Bundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries, StopWatch theTransactionStopWatch) { - Set deletedResources = new HashSet<>(); - List deleteConflicts = new ArrayList<>(); - Map entriesToProcess = new IdentityHashMap<>(); - Set nonUpdatedEntities = new HashSet<>(); - Set updatedEntities = new HashSet<>(); - Map> conditionalRequestUrls = new HashMap<>(); - - /* - * Loop through the request and process any entries of type - * PUT, POST or DELETE - */ - for (int i = 0; i < theEntries.size(); i++) { - - if (i % 100 == 0) { - ourLog.debug("Processed {} non-GET entries out of {}", i, theEntries.size()); - } - - BundleEntryComponent nextReqEntry = theEntries.get(i); - Resource res = nextReqEntry.getResource(); - IdType nextResourceId = null; - if (res != null) { - - nextResourceId = res.getIdElement(); - - if (!nextResourceId.hasIdPart()) { - if (isNotBlank(nextReqEntry.getFullUrl())) { - nextResourceId = new IdType(nextReqEntry.getFullUrl()); - } - } - - if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+\\:.*") && !isPlaceholder(nextResourceId)) { - throw new InvalidRequestException("Invalid placeholder ID found: " + nextResourceId.getIdPart() + " - Must be of the form 'urn:uuid:[uuid]' or 'urn:oid:[oid]'"); - } - - if (nextResourceId.hasIdPart() && !nextResourceId.hasResourceType() && !isPlaceholder(nextResourceId)) { - nextResourceId = new IdType(toResourceName(res.getClass()), nextResourceId.getIdPart()); - res.setId(nextResourceId); - } - - /* - * Ensure that the bundle doesn't have any duplicates, since this causes all kinds of weirdness - */ - if (isPlaceholder(nextResourceId)) { - if (!theAllIds.add(nextResourceId)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextResourceId)); - } - } else if (nextResourceId.hasResourceType() && nextResourceId.hasIdPart()) { - IdType nextId = nextResourceId.toUnqualifiedVersionless(); - if (!theAllIds.add(nextId)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextId)); - } - } - - } - - HTTPVerb verb = nextReqEntry.getRequest().getMethodElement().getValue(); - String resourceType = res != null ? getContext().getResourceDefinition(res).getName() : null; - BundleEntryComponent nextRespEntry = theResponse.getEntry().get(theOriginalRequestOrder.get(nextReqEntry)); - - theTransactionStopWatch.startTask("Bundle.entry[" + i + "]: " + verb.name() + " " + defaultString(resourceType)); - - switch (verb) { - case POST: { - // CREATE - @SuppressWarnings("rawtypes") - IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); - res.setId((String) null); - DaoMethodOutcome outcome; - String matchUrl = nextReqEntry.getRequest().getIfNoneExist(); - matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - outcome = resourceDao.create(res, matchUrl, false, theRequestDetails); - if (nextResourceId != null) { - handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); - } - entriesToProcess.put(nextRespEntry, outcome.getEntity()); - if (outcome.getCreated() == false) { - nonUpdatedEntities.add(outcome.getEntity()); - } else { - if (isNotBlank(matchUrl)) { - conditionalRequestUrls.put(matchUrl, res.getClass()); - } - } - - break; - } - case DELETE: { - // DELETE - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); - UrlParts parts = UrlUtil.parseUrl(url); - ca.uhn.fhir.jpa.dao.IFhirResourceDao dao = toDao(parts, verb.toCode(), url); - int status = Constants.STATUS_HTTP_204_NO_CONTENT; - if (parts.getResourceId() != null) { - IdType deleteId = new IdType(parts.getResourceType(), parts.getResourceId()); - if (!deletedResources.contains(deleteId.getValueAsString())) { - DaoMethodOutcome outcome = dao.delete(deleteId, deleteConflicts, theRequestDetails); - if (outcome.getEntity() != null) { - deletedResources.add(deleteId.getValueAsString()); - entriesToProcess.put(nextRespEntry, outcome.getEntity()); - } - } - } else { - String matchUrl = parts.getResourceType() + '?' + parts.getParams(); - matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - DeleteMethodOutcome deleteOutcome = dao.deleteByUrl(matchUrl, deleteConflicts, theRequestDetails); - List allDeleted = deleteOutcome.getDeletedEntities(); - for (ResourceTable deleted : allDeleted) { - deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless().getValueAsString()); - } - if (allDeleted.isEmpty()) { - status = Constants.STATUS_HTTP_204_NO_CONTENT; - } - - nextRespEntry.getResponse().setOutcome((Resource) deleteOutcome.getOperationOutcome()); - } - - nextRespEntry.getResponse().setStatus(toStatusString(status)); - - break; - } - case PUT: { - // UPDATE - @SuppressWarnings("rawtypes") - IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); - - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); - - DaoMethodOutcome outcome; - UrlParts parts = UrlUtil.parseUrl(url); - if (isNotBlank(parts.getResourceId())) { - String version = null; - if (isNotBlank(nextReqEntry.getRequest().getIfMatch())) { - version = ParameterUtil.parseETagValue(nextReqEntry.getRequest().getIfMatch()); - } - res.setId(new IdType(parts.getResourceType(), parts.getResourceId(), version)); - outcome = resourceDao.update(res, null, false, theRequestDetails); - } else { - res.setId((String) null); - String matchUrl; - if (isNotBlank(parts.getParams())) { - matchUrl = parts.getResourceType() + '?' + parts.getParams(); - } else { - matchUrl = parts.getResourceType(); - } - matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - outcome = resourceDao.update(res, matchUrl, false, theRequestDetails); - if (Boolean.TRUE.equals(outcome.getCreated())) { - conditionalRequestUrls.put(matchUrl, res.getClass()); - } - } - - if (outcome.getCreated() == Boolean.FALSE) { - updatedEntities.add(outcome.getEntity()); - } - - handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); - entriesToProcess.put(nextRespEntry, outcome.getEntity()); - break; - } - case GET: - case NULL: - break; - - } - - theTransactionStopWatch.endCurrentTask(); - } - - /* - * Make sure that there are no conflicts from deletions. E.g. we can't delete something - * if something else has a reference to it.. Unless the thing that has a reference to it - * was also deleted as a part of this transaction, which is why we check this now at the - * end. - */ - - deleteConflicts.removeIf(next -> - deletedResources.contains(next.getTargetId().toUnqualifiedVersionless().getValue())); - validateDeleteConflictsEmptyOrThrowException(deleteConflicts); - - /* - * Perform ID substitutions and then index each resource we have saved - */ - - FhirTerser terser = getContext().newTerser(); - theTransactionStopWatch.startTask("Index " + theIdToPersistedOutcome.size() + " resources"); - for (DaoMethodOutcome nextOutcome : theIdToPersistedOutcome.values()) { - IBaseResource nextResource = nextOutcome.getResource(); - if (nextResource == null) { - continue; - } - - // References - List allRefs = terser.getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class); - for (IBaseReference nextRef : allRefs) { - IIdType nextId = nextRef.getReferenceElement(); - if (!nextId.hasIdPart()) { - continue; - } - if (theIdSubstitutions.containsKey(nextId)) { - IdType newId = theIdSubstitutions.get(nextId); - ourLog.debug(" * Replacing resource ref {} with {}", nextId, newId); - nextRef.setReference(newId.getValue()); - } else if (nextId.getValue().startsWith("urn:")) { - throw new InvalidRequestException("Unable to satisfy placeholder ID: " + nextId.getValue()); - } else { - ourLog.debug(" * Reference [{}] does not exist in bundle", nextId); - } - } - - // URIs - List allUris = terser.getAllPopulatedChildElementsOfType(nextResource, UriType.class); - for (UriType nextRef : allUris) { - if (nextRef instanceof IIdType) { - continue; // No substitution on the resource ID itself! - } - IdType nextUriString = new IdType(nextRef.getValueAsString()); - if (theIdSubstitutions.containsKey(nextUriString)) { - IdType newId = theIdSubstitutions.get(nextUriString); - ourLog.debug(" * Replacing resource ref {} with {}", nextUriString, newId); - nextRef.setValue(newId.getValue()); - } else { - ourLog.debug(" * Reference [{}] does not exist in bundle", nextUriString); - } - } - - IPrimitiveType deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource); - Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; - - if (updatedEntities.contains(nextOutcome.getEntity())) { - updateInternal(theRequestDetails, nextResource, true, false, theRequestDetails, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); - } else if (!nonUpdatedEntities.contains(nextOutcome.getEntity())) { - updateEntity(theRequestDetails, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theUpdateTime, false, true); - } - } - - theTransactionStopWatch.endCurrentTask(); - theTransactionStopWatch.startTask("Flush writes to database"); - - flushJpaSession(); - - theTransactionStopWatch.endCurrentTask(); - if (conditionalRequestUrls.size() > 0) { - theTransactionStopWatch.startTask("Check for conflicts in conditional resources"); - } - - /* - * Double check we didn't allow any duplicates we shouldn't have - */ - for (Entry> nextEntry : conditionalRequestUrls.entrySet()) { - String matchUrl = nextEntry.getKey(); - Class resType = nextEntry.getValue(); - if (isNotBlank(matchUrl)) { - IFhirResourceDao resourceDao = getDao(resType); - Set val = resourceDao.processMatchUrl(matchUrl); - if (val.size() > 1) { - throw new InvalidRequestException( - "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); - } - } - } - - theTransactionStopWatch.endCurrentTask(); - - for (IdType next : theAllIds) { - IdType replacement = theIdSubstitutions.get(next); - if (replacement == null) { - continue; - } - if (replacement.equals(next)) { - continue; - } - ourLog.debug("Placeholder resource ID \"{}\" was replaced with permanent ID \"{}\"", next, replacement); - } - return entriesToProcess; - } - - private String extractTransactionUrlOrThrowException(BundleEntryComponent nextEntry, HTTPVerb verb) { - String url = nextEntry.getRequest().getUrl(); - if (isBlank(url)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionMissingUrl", verb.name())); - } - return url; - } - - /** - * This method is called for nested bundles (e.g. if we received a transaction with an entry that - * was a GET search, this method is called on the bundle for the search result, that will be placed in the - * outer bundle). This method applies the _summary and _content parameters to the output of - * that bundle. - *

    - * TODO: This isn't the most efficient way of doing this.. hopefully we can come up with something better in the future. - */ - private IBaseResource filterNestedBundle(RequestDetails theRequestDetails, IBaseResource theResource) { - IParser p = getContext().newJsonParser(); - RestfulServerUtils.configureResponseParser(theRequestDetails, p); - return p.parseResource(theResource.getClass(), p.encodeResourceToString(theResource)); - } - - @Override public Meta metaGetOperation(RequestDetails theRequestDetails) { // Notify interceptors @@ -634,60 +58,10 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { TypedQuery q = myEntityManager.createQuery(sql, TagDefinition.class); List tagDefinitions = q.getResultList(); - Meta retVal = toMeta(tagDefinitions); - - return retVal; + return toMeta(tagDefinitions); } - private String performIdSubstitutionsInMatchUrl(Map theIdSubstitutions, String theMatchUrl) { - String matchUrl = theMatchUrl; - if (isNotBlank(matchUrl)) { - for (Entry nextSubstitutionEntry : theIdSubstitutions.entrySet()) { - IdType nextTemporaryId = nextSubstitutionEntry.getKey(); - IdType nextReplacementId = nextSubstitutionEntry.getValue(); - String nextTemporaryIdPart = nextTemporaryId.getIdPart(); - String nextReplacementIdPart = nextReplacementId.getValueAsString(); - if (nextTemporaryId.isUrn() && nextTemporaryIdPart.length() > IdType.URN_PREFIX.length()) { - matchUrl = matchUrl.replace(nextTemporaryIdPart, nextReplacementIdPart); - matchUrl = matchUrl.replace(UrlUtil.escapeUrlParam(nextTemporaryIdPart), nextReplacementIdPart); - } - } - } - return matchUrl; - } - - private void populateEntryWithOperationOutcome(BaseServerResponseException caughtEx, BundleEntryComponent nextEntry) { - OperationOutcome oo = new OperationOutcome(); - oo.addIssue().setSeverity(IssueSeverity.ERROR).setDiagnostics(caughtEx.getMessage()); - nextEntry.getResponse().setOutcome(oo); - } - - private ca.uhn.fhir.jpa.dao.IFhirResourceDao toDao(UrlParts theParts, String theVerb, String theUrl) { - RuntimeResourceDefinition resType; - try { - resType = getContext().getResourceDefinition(theParts.getResourceType()); - } catch (DataFormatException e) { - String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - throw new InvalidRequestException(msg); - } - IFhirResourceDao dao = null; - if (resType != null) { - dao = getDao(resType.getImplementingClass()); - } - if (dao == null) { - String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - throw new InvalidRequestException(msg); - } - - // if (theParts.getResourceId() == null && theParts.getParams() == null) { - // String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - // throw new InvalidRequestException(msg); - // } - - return dao; - } - - protected Meta toMeta(Collection tagDefinitions) { + private Meta toMeta(Collection tagDefinitions) { Meta retVal = new Meta(); for (TagDefinition next : tagDefinitions) { switch (next.getTagType()) { @@ -708,187 +82,8 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { @Transactional(propagation = Propagation.NEVER) @Override public Bundle transaction(RequestDetails theRequestDetails, Bundle theRequest) { - if (theRequestDetails != null) { - ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null); - notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails); - } - - String actionName = "Transaction"; - return transaction((ServletRequestDetails) theRequestDetails, theRequest, actionName); + return myTransactionProcessor.transaction(theRequestDetails, theRequest); } - private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRequest, String theActionName) { - super.markRequestAsProcessingSubRequest(theRequestDetails); - try { - return doTransaction(theRequestDetails, theRequest, theActionName); - } finally { - super.clearRequestAsProcessingSubRequest(theRequestDetails); - } - } - - private static void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, IdType nextResourceId, DaoMethodOutcome outcome, - BundleEntryComponent newEntry, String theResourceType, IBaseResource theRes, ServletRequestDetails theRequestDetails) { - IdType newId = (IdType) outcome.getId().toUnqualifiedVersionless(); - IdType resourceId = isPlaceholder(nextResourceId) ? nextResourceId : nextResourceId.toUnqualifiedVersionless(); - if (newId.equals(resourceId) == false) { - idSubstitutions.put(resourceId, newId); - if (isPlaceholder(resourceId)) { - /* - * The correct way for substitution IDs to be is to be with no resource type, but we'll accept the qualified kind too just to be lenient. - */ - idSubstitutions.put(new IdType(theResourceType + '/' + resourceId.getValue()), newId); - } - } - idToPersistedOutcome.put(newId, outcome); - if (outcome.getCreated().booleanValue()) { - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_201_CREATED)); - } else { - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); - } - newEntry.getResponse().setLastModified(((Resource) theRes).getMeta().getLastUpdated()); - - if (theRequestDetails != null) { - if (outcome.getResource() != null) { - String prefer = theRequestDetails.getHeader(Constants.HEADER_PREFER); - PreferReturnEnum preferReturn = RestfulServerUtils.parsePreferHeader(prefer); - if (preferReturn != null) { - if (preferReturn == PreferReturnEnum.REPRESENTATION) { - newEntry.setResource((Resource) outcome.getResource()); - } - } - } - } - - } - - private static boolean isPlaceholder(IdType theId) { - if (theId != null && theId.getValue() != null) { - if (theId.getValue().startsWith("urn:oid:") || theId.getValue().startsWith("urn:uuid:")) { - return true; - } - } - return false; - } - - private static String toStatusString(int theStatusCode) { - return Integer.toString(theStatusCode) + " " + defaultString(Constants.HTTP_STATUS_NAMES.get(theStatusCode)); - } - - /** - * Transaction Order, per the spec: - *

    - * Process any DELETE interactions - * Process any POST interactions - * Process any PUT interactions - * Process any GET interactions - */ - //@formatter:off - public class TransactionSorter implements Comparator { - - private Set myPlaceholderIds; - - public TransactionSorter(Set thePlaceholderIds) { - myPlaceholderIds = thePlaceholderIds; - } - - @Override - public int compare(BundleEntryComponent theO1, BundleEntryComponent theO2) { - int o1 = toOrder(theO1); - int o2 = toOrder(theO2); - - if (o1 == o2) { - String matchUrl1 = toMatchUrl(theO1); - String matchUrl2 = toMatchUrl(theO2); - if (isBlank(matchUrl1) && isBlank(matchUrl2)) { - return 0; - } - if (isBlank(matchUrl1)) { - return -1; - } - if (isBlank(matchUrl2)) { - return 1; - } - - boolean match1containsSubstitutions = false; - boolean match2containsSubstitutions = false; - for (String nextPlaceholder : myPlaceholderIds) { - if (matchUrl1.contains(nextPlaceholder)) { - match1containsSubstitutions = true; - } - if (matchUrl2.contains(nextPlaceholder)) { - match2containsSubstitutions = true; - } - } - - if (match1containsSubstitutions && match2containsSubstitutions) { - return 0; - } - if (!match1containsSubstitutions && !match2containsSubstitutions) { - return 0; - } - if (match1containsSubstitutions) { - return 1; - } else { - return -1; - } - } - - return o1 - o2; - } - - private String toMatchUrl(BundleEntryComponent theEntry) { - HTTPVerb verb = theEntry.getRequest().getMethod(); - if (verb == HTTPVerb.POST) { - return theEntry.getRequest().getIfNoneExist(); - } - if (verb == HTTPVerb.PUT || verb == HTTPVerb.DELETE) { - String url = extractTransactionUrlOrThrowException(theEntry, verb); - UrlParts parts = UrlUtil.parseUrl(url); - if (isBlank(parts.getResourceId())) { - return parts.getResourceType() + '?' + parts.getParams(); - } - } - return null; - } - - private int toOrder(BundleEntryComponent theO1) { - int o1 = 0; - if (theO1.getRequest().getMethodElement().getValue() != null) { - switch (theO1.getRequest().getMethodElement().getValue()) { - case DELETE: - o1 = 1; - break; - case POST: - o1 = 2; - break; - case PUT: - o1 = 3; - break; - case GET: - o1 = 4; - break; - case NULL: - o1 = 0; - break; - } - } - return o1; - } - - } - - //@formatter:off - - private static class BaseServerResponseExceptionHolder { - private BaseServerResponseException myException; - - public BaseServerResponseException getException() { - return myException; - } - - public void setException(BaseServerResponseException myException) { - this.myException = myException; - } - } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java new file mode 100644 index 00000000000..1b8d712f6ac --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java @@ -0,0 +1,133 @@ +package ca.uhn.fhir.jpa.dao.dstu3; + +import ca.uhn.fhir.jpa.dao.TransactionProcessor; +import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import org.hl7.fhir.dstu3.model.Bundle; +import org.hl7.fhir.dstu3.model.OperationOutcome; +import org.hl7.fhir.dstu3.model.Resource; +import org.hl7.fhir.exceptions.FHIRException; +import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; +import org.hl7.fhir.instance.model.api.IBaseResource; + +import java.util.Date; +import java.util.List; + +public class TransactionProcessorVersionAdapterDstu3 implements TransactionProcessor.ITransactionProcessorVersionAdapter { + @Override + public void setResponseStatus(Bundle.BundleEntryComponent theBundleEntry, String theStatus) { + theBundleEntry.getResponse().setStatus(theStatus); + } + + @Override + public void setResponseLastModified(Bundle.BundleEntryComponent theBundleEntry, Date theLastModified) { + theBundleEntry.getResponse().setLastModified(theLastModified); + } + + @Override + public void setResource(Bundle.BundleEntryComponent theBundleEntry, IBaseResource theResource) { + theBundleEntry.setResource((Resource) theResource); + } + + @Override + public IBaseResource getResource(Bundle.BundleEntryComponent theBundleEntry) { + return theBundleEntry.getResource(); + } + + @Override + public String getBundleType(Bundle theRequest) { + if (theRequest.getType() == null) { + return null; + } + return theRequest.getTypeElement().getValue().toCode(); + } + + @Override + public void populateEntryWithOperationOutcome(BaseServerResponseException theCaughtEx, Bundle.BundleEntryComponent theEntry) { + OperationOutcome oo = new OperationOutcome(); + oo.addIssue().setSeverity(OperationOutcome.IssueSeverity.ERROR).setDiagnostics(theCaughtEx.getMessage()); + theEntry.getResponse().setOutcome(oo); + } + + @Override + public Bundle createBundle(String theBundleType) { + Bundle resp = new Bundle(); + try { + resp.setType(Bundle.BundleType.fromCode(theBundleType)); + } catch (FHIRException theE) { + throw new InternalErrorException("Unknown bundle type: " + theBundleType); + } + return resp; + } + + @Override + public List getEntries(Bundle theRequest) { + return theRequest.getEntry(); + } + + @Override + public void addEntry(Bundle theBundle, Bundle.BundleEntryComponent theEntry) { + theBundle.addEntry(theEntry); + } + + @Override + public Bundle.BundleEntryComponent addEntry(Bundle theBundle) { + return theBundle.addEntry(); + } + + @Override + public String getEntryRequestVerb(Bundle.BundleEntryComponent theEntry) { + String retVal = null; + Bundle.HTTPVerb value = theEntry.getRequest().getMethodElement().getValue(); + if (value != null) { + retVal = value.toCode(); + } + return retVal; + } + + @Override + public String getFullUrl(Bundle.BundleEntryComponent theEntry) { + return theEntry.getFullUrl(); + } + + @Override + public String getEntryIfNoneExist(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfNoneExist(); + } + + @Override + public String getEntryRequestUrl(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getUrl(); + } + + @Override + public void setResponseLocation(Bundle.BundleEntryComponent theEntry, String theResponseLocation) { + theEntry.getResponse().setLocation(theResponseLocation); + } + + @Override + public void setResponseETag(Bundle.BundleEntryComponent theEntry, String theEtag) { + theEntry.getResponse().setEtag(theEtag); + } + + @Override + public String getEntryRequestIfMatch(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfMatch(); + } + + @Override + public String getEntryRequestIfNoneExist(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfNoneExist(); + } + + @Override + public String getEntryRequestIfNoneMatch(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfNoneMatch(); + } + + @Override + public void setResponseOutcome(Bundle.BundleEntryComponent theEntry, IBaseOperationOutcome theOperationOutcome) { + theEntry.getResponse().setOutcome((Resource) theOperationOutcome); + } + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java index ad74a60f642..00e7fe1650e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao.r4; * 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. @@ -20,587 +20,34 @@ package ca.uhn.fhir.jpa.dao.r4; * #L% */ -import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.dao.BaseHapiFhirSystemDao; -import ca.uhn.fhir.jpa.dao.DaoMethodOutcome; -import ca.uhn.fhir.jpa.dao.DeleteMethodOutcome; -import ca.uhn.fhir.jpa.dao.IFhirResourceDao; -import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.dao.TransactionProcessor; import ca.uhn.fhir.jpa.entity.TagDefinition; -import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails; -import ca.uhn.fhir.jpa.util.DeleteConflict; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; -import ca.uhn.fhir.parser.DataFormatException; -import ca.uhn.fhir.parser.IParser; -import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.PreferReturnEnum; -import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.param.ParameterUtil; -import ca.uhn.fhir.rest.server.RestfulServerUtils; -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.interceptor.IServerInterceptor.ActionRequestDetails; -import ca.uhn.fhir.rest.server.method.BaseMethodBinding; -import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.FhirTerser; -import ca.uhn.fhir.util.UrlUtil; -import ca.uhn.fhir.util.UrlUtil.UrlParts; -import com.google.common.collect.ArrayListMultimap; -import org.apache.commons.lang3.Validate; -import org.apache.http.NameValuePair; -import org.hl7.fhir.instance.model.api.*; -import org.hl7.fhir.r4.model.*; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent; -import org.hl7.fhir.r4.model.Bundle.BundleEntryResponseComponent; -import org.hl7.fhir.r4.model.Bundle.BundleType; -import org.hl7.fhir.r4.model.Bundle.HTTPVerb; -import org.hl7.fhir.r4.model.OperationOutcome.IssueSeverity; +import org.hl7.fhir.r4.model.Meta; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionCallback; -import org.springframework.transaction.support.TransactionTemplate; +import javax.annotation.PostConstruct; import javax.persistence.TypedQuery; -import java.util.*; -import java.util.Map.Entry; - -import static org.apache.commons.lang3.StringUtils.*; +import java.util.Collection; +import java.util.List; public class FhirSystemDaoR4 extends BaseHapiFhirSystemDao { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoR4.class); @Autowired - private PlatformTransactionManager myTxManager; + private TransactionProcessor myTransactionProcessor; - private Bundle batch(final RequestDetails theRequestDetails, Bundle theRequest) { - ourLog.info("Beginning batch with {} resources", theRequest.getEntry().size()); - long start = System.currentTimeMillis(); - - TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); - - Bundle resp = new Bundle(); - resp.setType(BundleType.BATCHRESPONSE); - - /* - * 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 - */ - - for (final BundleEntryComponent nextRequestEntry : theRequest.getEntry()) { - - BaseServerResponseExceptionHolder caughtEx = new BaseServerResponseExceptionHolder(); - - TransactionCallback callback = new TransactionCallback() { - @Override - public Bundle doInTransaction(TransactionStatus theStatus) { - Bundle subRequestBundle = new Bundle(); - subRequestBundle.setType(BundleType.TRANSACTION); - subRequestBundle.addEntry(nextRequestEntry); - - Bundle subResponseBundle = transaction((ServletRequestDetails) theRequestDetails, subRequestBundle, "Batch sub-request"); - return subResponseBundle; - } - }; - - try { - Bundle nextResponseBundle = callback.doInTransaction(null); - - BundleEntryComponent subResponseEntry = nextResponseBundle.getEntry().get(0); - resp.addEntry(subResponseEntry); - - /* - * If the individual entry didn't have a resource in its response, bring the sub-transaction's OperationOutcome across so the client can see it - */ - if (subResponseEntry.getResource() == null) { - subResponseEntry.setResource(nextResponseBundle.getEntry().get(0).getResource()); - } - - } catch (BaseServerResponseException e) { - caughtEx.setException(e); - } catch (Throwable t) { - ourLog.error("Failure during BATCH sub transaction processing", t); - caughtEx.setException(new InternalErrorException(t)); - } - - if (caughtEx.getException() != null) { - BundleEntryComponent nextEntry = resp.addEntry(); - - populateEntryWithOperationOutcome(caughtEx.getException(), nextEntry); - - BundleEntryResponseComponent nextEntryResp = nextEntry.getResponse(); - nextEntryResp.setStatus(toStatusString(caughtEx.getException().getStatusCode())); - } - - } - - long delay = System.currentTimeMillis() - start; - ourLog.info("Batch completed in {}ms", new Object[] {delay}); - - return resp; - } - - private Bundle doTransaction(final ServletRequestDetails theRequestDetails, final Bundle theRequest, final String theActionName) { - BundleType transactionType = theRequest.getTypeElement().getValue(); - if (transactionType == BundleType.BATCH) { - return batch(theRequestDetails, theRequest); - } - - if (transactionType == null) { - String message = "Transaction Bundle did not specify valid Bundle.type, assuming " + BundleType.TRANSACTION.toCode(); - ourLog.warn(message); - transactionType = BundleType.TRANSACTION; - } - if (transactionType != BundleType.TRANSACTION) { - throw new InvalidRequestException("Unable to process transaction where incoming Bundle.type = " + transactionType.toCode()); - } - - ourLog.debug("Beginning {} with {} resources", theActionName, theRequest.getEntry().size()); - - long start = System.currentTimeMillis(); - final Date updateTime = new Date(); - - final Set allIds = new LinkedHashSet<>(); - final Map idSubstitutions = new HashMap<>(); - final Map idToPersistedOutcome = new HashMap<>(); - - // Do all entries have a verb? - for (int i = 0; i < theRequest.getEntry().size(); i++) { - BundleEntryComponent nextReqEntry = theRequest.getEntry().get(i); - HTTPVerb verb = nextReqEntry.getRequest().getMethodElement().getValue(); - if (verb == null) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionEntryHasInvalidVerb", nextReqEntry.getRequest().getMethod(), i)); - } - } - - /* - * We want to execute the transaction request bundle elements in the order - * specified by the FHIR specification (see TransactionSorter) so we save the - * original order in the request, then sort it. - * - * Entries with a type of GET are removed from the bundle so that they - * can be processed at the very end. We do this because the incoming resources - * are saved in a two-phase way in order to deal with interdependencies, and - * we want the GET processing to use the final indexing state - */ - final Bundle response = new Bundle(); - List getEntries = new ArrayList<>(); - final IdentityHashMap originalRequestOrder = new IdentityHashMap<>(); - for (int i = 0; i < theRequest.getEntry().size(); i++) { - originalRequestOrder.put(theRequest.getEntry().get(i), i); - response.addEntry(); - if (theRequest.getEntry().get(i).getRequest().getMethodElement().getValue() == HTTPVerb.GET) { - getEntries.add(theRequest.getEntry().get(i)); - } - } - - /* - * See FhirSystemDaoR4Test#testTransactionWithPlaceholderIdInMatchUrl - * Basically if the resource has a match URL that references a placeholder, - * we try to handle the resource with the placeholder first. - */ - Set placeholderIds = new HashSet(); - final List entries = theRequest.getEntry(); - for (BundleEntryComponent nextEntry : entries) { - if (isNotBlank(nextEntry.getFullUrl()) && nextEntry.getFullUrl().startsWith(IdType.URN_PREFIX)) { - placeholderIds.add(nextEntry.getFullUrl()); - } - } - Collections.sort(entries, new TransactionSorter(placeholderIds)); - - /* - * All of the write operations in the transaction (PUT, POST, etc.. basically anything - * except GET) are performed in their own database transaction before we do the reads. - * We do this because the reads (specifically the searches) often spawn their own - * secondary database transaction and if we allow that within the primary - * database transaction we can end up with deadlocks if the server is under - * heavy load with lots of concurrent transactions using all available - * database connections. - */ - TransactionTemplate txManager = new TransactionTemplate(myTxManager); - Map entriesToProcess = txManager.execute(new TransactionCallback>() { - @Override - public Map doInTransaction(TransactionStatus status) { - return doTransactionWriteOperations(theRequestDetails, theRequest, theActionName, updateTime, allIds, idSubstitutions, idToPersistedOutcome, response, originalRequestOrder, entries); - } - }); - for (Entry nextEntry : entriesToProcess.entrySet()) { - String responseLocation = nextEntry.getValue().getIdDt().toUnqualified().getValue(); - String responseEtag = nextEntry.getValue().getIdDt().getVersionIdPart(); - nextEntry.getKey().getResponse().setLocation(responseLocation); - nextEntry.getKey().getResponse().setEtag(responseEtag); - } - - /* - * Loop through the request and process any entries of type GET - */ - for (int i = 0; i < getEntries.size(); i++) { - BundleEntryComponent nextReqEntry = getEntries.get(i); - Integer originalOrder = originalRequestOrder.get(nextReqEntry); - BundleEntryComponent nextRespEntry = response.getEntry().get(originalOrder); - - ServletSubRequestDetails requestDetails = new ServletSubRequestDetails(); - requestDetails.setServletRequest(theRequestDetails.getServletRequest()); - requestDetails.setRequestType(RequestTypeEnum.GET); - requestDetails.setServer(theRequestDetails.getServer()); - - String url = extractTransactionUrlOrThrowException(nextReqEntry, HTTPVerb.GET); - - int qIndex = url.indexOf('?'); - ArrayListMultimap paramValues = ArrayListMultimap.create(); - requestDetails.setParameters(new HashMap()); - if (qIndex != -1) { - String params = url.substring(qIndex); - List parameters = translateMatchUrl(params); - for (NameValuePair next : parameters) { - paramValues.put(next.getName(), next.getValue()); - } - for (java.util.Map.Entry> nextParamEntry : paramValues.asMap().entrySet()) { - String[] nextValue = nextParamEntry.getValue().toArray(new String[nextParamEntry.getValue().size()]); - requestDetails.addParameter(nextParamEntry.getKey(), nextValue); - } - url = url.substring(0, qIndex); - } - - requestDetails.setRequestPath(url); - requestDetails.setFhirServerBase(theRequestDetails.getFhirServerBase()); - - theRequestDetails.getServer().populateRequestDetailsFromRequestPath(requestDetails, url); - BaseMethodBinding method = theRequestDetails.getServer().determineResourceMethod(requestDetails, url); - if (method == null) { - throw new IllegalArgumentException("Unable to handle GET " + url); - } - - if (isNotBlank(nextReqEntry.getRequest().getIfMatch())) { - requestDetails.addHeader(Constants.HEADER_IF_MATCH, nextReqEntry.getRequest().getIfMatch()); - } - if (isNotBlank(nextReqEntry.getRequest().getIfNoneExist())) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, nextReqEntry.getRequest().getIfNoneExist()); - } - if (isNotBlank(nextReqEntry.getRequest().getIfNoneMatch())) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, nextReqEntry.getRequest().getIfNoneMatch()); - } - - Validate.isTrue(method instanceof BaseResourceReturningMethodBinding, "Unable to handle GET {0}", url); - try { - IBaseResource resource = ((BaseResourceReturningMethodBinding) method).doInvokeServer(theRequestDetails.getServer(), requestDetails); - if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { - resource = filterNestedBundle(requestDetails, resource); - } - nextRespEntry.setResource((Resource) resource); - nextRespEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); - } catch (NotModifiedException e) { - nextRespEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); - } catch (BaseServerResponseException e) { - ourLog.info("Failure processing transaction GET {}: {}", url, e.toString()); - nextRespEntry.getResponse().setStatus(toStatusString(e.getStatusCode())); - populateEntryWithOperationOutcome(e, nextRespEntry); - } - - } - - long delay = System.currentTimeMillis() - start; - ourLog.info(theActionName + " completed in {}ms", new Object[] {delay}); - - response.setType(BundleType.TRANSACTIONRESPONSE); - return response; - } - - @SuppressWarnings("unchecked") - private Map doTransactionWriteOperations(RequestDetails theRequestDetails, Bundle theRequest, String theActionName, Date theUpdateTime, Set theAllIds, - Map theIdSubstitutions, Map theIdToPersistedOutcome, Bundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries) { - Set deletedResources = new HashSet<>(); - List deleteConflicts = new ArrayList<>(); - Map entriesToProcess = new IdentityHashMap<>(); - Set nonUpdatedEntities = new HashSet<>(); - Set updatedEntities = new HashSet<>(); - Map> conditionalRequestUrls = new HashMap<>(); - - /* - * Loop through the request and process any entries of type - * PUT, POST or DELETE - */ - for (int i = 0; i < theEntries.size(); i++) { - - if (i % 100 == 0) { - ourLog.debug("Processed {} non-GET entries out of {}", i, theEntries.size()); - } - - BundleEntryComponent nextReqEntry = theEntries.get(i); - Resource res = nextReqEntry.getResource(); - IdType nextResourceId = null; - if (res != null) { - - nextResourceId = res.getIdElement(); - - if (!nextResourceId.hasIdPart()) { - if (isNotBlank(nextReqEntry.getFullUrl())) { - nextResourceId = new IdType(nextReqEntry.getFullUrl()); - } - } - - if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+\\:.*") && !isPlaceholder(nextResourceId)) { - throw new InvalidRequestException("Invalid placeholder ID found: " + nextResourceId.getIdPart() + " - Must be of the form 'urn:uuid:[uuid]' or 'urn:oid:[oid]'"); - } - - if (nextResourceId.hasIdPart() && !nextResourceId.hasResourceType() && !isPlaceholder(nextResourceId)) { - nextResourceId = new IdType(toResourceName(res.getClass()), nextResourceId.getIdPart()); - res.setId(nextResourceId); - } - - /* - * Ensure that the bundle doesn't have any duplicates, since this causes all kinds of weirdness - */ - if (isPlaceholder(nextResourceId)) { - if (!theAllIds.add(nextResourceId)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextResourceId)); - } - } else if (nextResourceId.hasResourceType() && nextResourceId.hasIdPart()) { - IdType nextId = nextResourceId.toUnqualifiedVersionless(); - if (!theAllIds.add(nextId)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionContainsMultipleWithDuplicateId", nextId)); - } - } - - } - - HTTPVerb verb = nextReqEntry.getRequest().getMethodElement().getValue(); - - String resourceType = res != null ? getContext().getResourceDefinition(res).getName() : null; - BundleEntryComponent nextRespEntry = theResponse.getEntry().get(theOriginalRequestOrder.get(nextReqEntry)); - - switch (verb) { - case POST: { - // CREATE - @SuppressWarnings("rawtypes") - IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); - res.setId((String) null); - DaoMethodOutcome outcome; - String matchUrl = nextReqEntry.getRequest().getIfNoneExist(); - matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - outcome = resourceDao.create(res, matchUrl, false, theRequestDetails); - if (nextResourceId != null) { - handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); - } - entriesToProcess.put(nextRespEntry, outcome.getEntity()); - if (outcome.getCreated() == false) { - nonUpdatedEntities.add(outcome.getEntity()); - } else { - if (isNotBlank(matchUrl)) { - conditionalRequestUrls.put(matchUrl, res.getClass()); - } - } - - break; - } - case DELETE: { - // DELETE - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); - UrlParts parts = UrlUtil.parseUrl(url); - ca.uhn.fhir.jpa.dao.IFhirResourceDao dao = toDao(parts, verb.toCode(), url); - int status = Constants.STATUS_HTTP_204_NO_CONTENT; - if (parts.getResourceId() != null) { - IdType deleteId = new IdType(parts.getResourceType(), parts.getResourceId()); - if (!deletedResources.contains(deleteId.getValueAsString())) { - DaoMethodOutcome outcome = dao.delete(deleteId, deleteConflicts, theRequestDetails); - if (outcome.getEntity() != null) { - deletedResources.add(deleteId.getValueAsString()); - entriesToProcess.put(nextRespEntry, outcome.getEntity()); - } - } - } else { - String matchUrl = parts.getResourceType() + '?' + parts.getParams(); - matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - DeleteMethodOutcome deleteOutcome = dao.deleteByUrl(matchUrl, deleteConflicts, theRequestDetails); - List allDeleted = deleteOutcome.getDeletedEntities(); - for (ResourceTable deleted : allDeleted) { - deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless().getValueAsString()); - } - if (allDeleted.isEmpty()) { - status = Constants.STATUS_HTTP_204_NO_CONTENT; - } - - nextRespEntry.getResponse().setOutcome((Resource) deleteOutcome.getOperationOutcome()); - } - - nextRespEntry.getResponse().setStatus(toStatusString(status)); - - break; - } - case PUT: { - // UPDATE - @SuppressWarnings("rawtypes") - IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); - - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); - - DaoMethodOutcome outcome; - UrlParts parts = UrlUtil.parseUrl(url); - if (isNotBlank(parts.getResourceId())) { - String version = null; - if (isNotBlank(nextReqEntry.getRequest().getIfMatch())) { - version = ParameterUtil.parseETagValue(nextReqEntry.getRequest().getIfMatch()); - } - res.setId(new IdType(parts.getResourceType(), parts.getResourceId(), version)); - outcome = resourceDao.update(res, null, false, theRequestDetails); - } else { - res.setId((String) null); - String matchUrl; - if (isNotBlank(parts.getParams())) { - matchUrl = parts.getResourceType() + '?' + parts.getParams(); - } else { - matchUrl = parts.getResourceType(); - } - matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - outcome = resourceDao.update(res, matchUrl, false, theRequestDetails); - if (Boolean.TRUE.equals(outcome.getCreated())) { - conditionalRequestUrls.put(matchUrl, res.getClass()); - } - } - - if (outcome.getCreated() == Boolean.FALSE) { - updatedEntities.add(outcome.getEntity()); - } - - handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); - entriesToProcess.put(nextRespEntry, outcome.getEntity()); - break; - } - case GET: - case NULL: - case HEAD: - case PATCH: - break; - - } - } - - /* - * Make sure that there are no conflicts from deletions. E.g. we can't delete something - * if something else has a reference to it.. Unless the thing that has a reference to it - * was also deleted as a part of this transaction, which is why we check this now at the - * end. - */ - - deleteConflicts.removeIf(next -> - deletedResources.contains(next.getTargetId().toUnqualifiedVersionless().getValue())); - validateDeleteConflictsEmptyOrThrowException(deleteConflicts); - - /* - * Perform ID substitutions and then index each resource we have saved - */ - - FhirTerser terser = getContext().newTerser(); - for (DaoMethodOutcome nextOutcome : theIdToPersistedOutcome.values()) { - IBaseResource nextResource = nextOutcome.getResource(); - if (nextResource == null) { - continue; - } - - // References - List allRefs = terser.getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class); - for (IBaseReference nextRef : allRefs) { - IIdType nextId = nextRef.getReferenceElement(); - if (!nextId.hasIdPart()) { - continue; - } - if (theIdSubstitutions.containsKey(nextId)) { - IdType newId = theIdSubstitutions.get(nextId); - ourLog.debug(" * Replacing resource ref {} with {}", nextId, newId); - nextRef.setReference(newId.getValue()); - } else if (nextId.getValue().startsWith("urn:")) { - throw new InvalidRequestException("Unable to satisfy placeholder ID: " + nextId.getValue()); - } else { - ourLog.debug(" * Reference [{}] does not exist in bundle", nextId); - } - } - - // URIs - List allUris = terser.getAllPopulatedChildElementsOfType(nextResource, UriType.class); - for (UriType nextRef : allUris) { - if (nextRef instanceof IIdType) { - continue; // No substitution on the resource ID itself! - } - IdType nextUriString = new IdType(nextRef.getValueAsString()); - if (theIdSubstitutions.containsKey(nextUriString)) { - IdType newId = theIdSubstitutions.get(nextUriString); - ourLog.debug(" * Replacing resource ref {} with {}", nextUriString, newId); - nextRef.setValue(newId.getValue()); - } else { - ourLog.debug(" * Reference [{}] does not exist in bundle", nextUriString); - } - } - - IPrimitiveType deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource); - Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; - - if (updatedEntities.contains(nextOutcome.getEntity())) { - updateInternal(theRequestDetails, nextResource, true, false, theRequestDetails, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); - } else if (!nonUpdatedEntities.contains(nextOutcome.getEntity())) { - updateEntity(theRequestDetails, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theUpdateTime, false, true); - } - } - - flushJpaSession(); - - /* - * Double check we didn't allow any duplicates we shouldn't have - */ - for (Entry> nextEntry : conditionalRequestUrls.entrySet()) { - String matchUrl = nextEntry.getKey(); - Class resType = nextEntry.getValue(); - if (isNotBlank(matchUrl)) { - IFhirResourceDao resourceDao = getDao(resType); - Set val = resourceDao.processMatchUrl(matchUrl); - if (val.size() > 1) { - throw new InvalidRequestException( - "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); - } - } - } - - for (IdType next : theAllIds) { - IdType replacement = theIdSubstitutions.get(next); - if (replacement == null) { - continue; - } - if (replacement.equals(next)) { - continue; - } - ourLog.debug("Placeholder resource ID \"{}\" was replaced with permanent ID \"{}\"", next, replacement); - } - return entriesToProcess; - } - - private String extractTransactionUrlOrThrowException(BundleEntryComponent nextEntry, HTTPVerb verb) { - String url = nextEntry.getRequest().getUrl(); - if (isBlank(url)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionMissingUrl", verb.name())); - } - return url; - } - - /** - * This method is called for nested bundles (e.g. if we received a transaction with an entry that - * was a GET search, this method is called on the bundle for the search result, that will be placed in the - * outer bundle). This method applies the _summary and _content parameters to the output of - * that bundle. - *

    - * TODO: This isn't the most efficient way of doing this.. hopefully we can come up with something better in the future. - */ - private IBaseResource filterNestedBundle(RequestDetails theRequestDetails, IBaseResource theResource) { - IParser p = getContext().newJsonParser(); - RestfulServerUtils.configureResponseParser(theRequestDetails, p); - return p.parseResource(theResource.getClass(), p.encodeResourceToString(theResource)); + @PostConstruct + public void start() { + myTransactionProcessor.setDao(this); } @@ -617,53 +64,6 @@ public class FhirSystemDaoR4 extends BaseHapiFhirSystemDao { return toMeta(tagDefinitions); } - private String performIdSubstitutionsInMatchUrl(Map theIdSubstitutions, String theMatchUrl) { - String matchUrl = theMatchUrl; - if (isNotBlank(matchUrl)) { - for (Entry nextSubstitutionEntry : theIdSubstitutions.entrySet()) { - IdType nextTemporaryId = nextSubstitutionEntry.getKey(); - IdType nextReplacementId = nextSubstitutionEntry.getValue(); - String nextTemporaryIdPart = nextTemporaryId.getIdPart(); - String nextReplacementIdPart = nextReplacementId.getValueAsString(); - if (nextTemporaryId.isUrn() && nextTemporaryIdPart.length() > IdType.URN_PREFIX.length()) { - matchUrl = matchUrl.replace(nextTemporaryIdPart, nextReplacementIdPart); - matchUrl = matchUrl.replace(UrlUtil.escapeUrlParam(nextTemporaryIdPart), nextReplacementIdPart); - } - } - } - return matchUrl; - } - - private void populateEntryWithOperationOutcome(BaseServerResponseException caughtEx, BundleEntryComponent nextEntry) { - OperationOutcome oo = new OperationOutcome(); - oo.addIssue().setSeverity(IssueSeverity.ERROR).setDiagnostics(caughtEx.getMessage()); - nextEntry.getResponse().setOutcome(oo); - } - - private ca.uhn.fhir.jpa.dao.IFhirResourceDao toDao(UrlParts theParts, String theVerb, String theUrl) { - RuntimeResourceDefinition resType; - try { - resType = getContext().getResourceDefinition(theParts.getResourceType()); - } catch (DataFormatException e) { - String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - throw new InvalidRequestException(msg); - } - IFhirResourceDao dao = null; - if (resType != null) { - dao = getDao(resType.getImplementingClass()); - } - if (dao == null) { - String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - throw new InvalidRequestException(msg); - } - - // if (theParts.getResourceId() == null && theParts.getParams() == null) { - // String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - // throw new InvalidRequestException(msg); - // } - - return dao; - } protected Meta toMeta(Collection tagDefinitions) { Meta retVal = new Meta(); @@ -686,193 +86,7 @@ public class FhirSystemDaoR4 extends BaseHapiFhirSystemDao { @Transactional(propagation = Propagation.NEVER) @Override public Bundle transaction(RequestDetails theRequestDetails, Bundle theRequest) { - if (theRequestDetails != null) { - ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null); - notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails); - } - - String actionName = "Transaction"; - return transaction((ServletRequestDetails) theRequestDetails, theRequest, actionName); - } - - private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRequest, String theActionName) { - super.markRequestAsProcessingSubRequest(theRequestDetails); - try { - return doTransaction(theRequestDetails, theRequest, theActionName); - } finally { - super.clearRequestAsProcessingSubRequest(theRequestDetails); - } - } - - private static void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, IdType nextResourceId, DaoMethodOutcome outcome, - BundleEntryComponent newEntry, String theResourceType, IBaseResource theRes, RequestDetails theRequestDetails) { - IdType newId = (IdType) outcome.getId().toUnqualifiedVersionless(); - IdType resourceId = isPlaceholder(nextResourceId) ? nextResourceId : nextResourceId.toUnqualifiedVersionless(); - if (newId.equals(resourceId) == false) { - idSubstitutions.put(resourceId, newId); - if (isPlaceholder(resourceId)) { - /* - * The correct way for substitution IDs to be is to be with no resource type, but we'll accept the qualified kind too just to be lenient. - */ - idSubstitutions.put(new IdType(theResourceType + '/' + resourceId.getValue()), newId); - } - } - idToPersistedOutcome.put(newId, outcome); - if (outcome.getCreated().booleanValue()) { - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_201_CREATED)); - } else { - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); - } - newEntry.getResponse().setLastModified(((Resource) theRes).getMeta().getLastUpdated()); - - if (theRequestDetails != null) { - if (outcome.getResource() != null) { - String prefer = theRequestDetails.getHeader(Constants.HEADER_PREFER); - PreferReturnEnum preferReturn = RestfulServerUtils.parsePreferHeader(prefer); - if (preferReturn != null) { - if (preferReturn == PreferReturnEnum.REPRESENTATION) { - newEntry.setResource((Resource) outcome.getResource()); - } - } - } - } - - } - - private static boolean isPlaceholder(IdType theId) { - if (theId != null && theId.getValue() != null) { - if (theId.getValue().startsWith("urn:oid:") || theId.getValue().startsWith("urn:uuid:")) { - return true; - } - } - return false; - } - - private static String toStatusString(int theStatusCode) { - return Integer.toString(theStatusCode) + " " + defaultString(Constants.HTTP_STATUS_NAMES.get(theStatusCode)); - } - - /** - * Transaction Order, per the spec: - *

    - * Process any DELETE interactions - * Process any POST interactions - * Process any PUT interactions - * Process any GET interactions - */ - //@formatter:off - public class TransactionSorter implements Comparator { - - private Set myPlaceholderIds; - - public TransactionSorter(Set thePlaceholderIds) { - myPlaceholderIds = thePlaceholderIds; - } - - @Override - public int compare(BundleEntryComponent theO1, BundleEntryComponent theO2) { - int o1 = toOrder(theO1); - int o2 = toOrder(theO2); - - if (o1 == o2) { - String matchUrl1 = toMatchUrl(theO1); - String matchUrl2 = toMatchUrl(theO2); - if (isBlank(matchUrl1) && isBlank(matchUrl2)) { - return 0; - } - if (isBlank(matchUrl1)) { - return -1; - } - if (isBlank(matchUrl2)) { - return 1; - } - - boolean match1containsSubstitutions = false; - boolean match2containsSubstitutions = false; - for (String nextPlaceholder : myPlaceholderIds) { - if (matchUrl1.contains(nextPlaceholder)) { - match1containsSubstitutions = true; - } - if (matchUrl2.contains(nextPlaceholder)) { - match2containsSubstitutions = true; - } - } - - if (match1containsSubstitutions && match2containsSubstitutions) { - return 0; - } - if (!match1containsSubstitutions && !match2containsSubstitutions) { - return 0; - } - if (match1containsSubstitutions) { - return 1; - } else { - return -1; - } - } - - return o1 - o2; - } - - private String toMatchUrl(BundleEntryComponent theEntry) { - HTTPVerb verb = theEntry.getRequest().getMethod(); - if (verb == HTTPVerb.POST) { - return theEntry.getRequest().getIfNoneExist(); - } - if (verb == HTTPVerb.PUT || verb == HTTPVerb.DELETE) { - String url = extractTransactionUrlOrThrowException(theEntry, verb); - UrlParts parts = UrlUtil.parseUrl(url); - if (isBlank(parts.getResourceId())) { - return parts.getResourceType() + '?' + parts.getParams(); - } - } - return null; - } - - private int toOrder(BundleEntryComponent theO1) { - int o1 = 0; - if (theO1.getRequest().getMethodElement().getValue() != null) { - switch (theO1.getRequest().getMethodElement().getValue()) { - case DELETE: - o1 = 1; - break; - case POST: - o1 = 2; - break; - case PUT: - o1 = 3; - break; - case PATCH: - o1 = 4; - break; - case HEAD: - o1 = 5; - break; - case GET: - o1 = 6; - break; - case NULL: - o1 = 0; - break; - } - } - return o1; - } - - } - - //@formatter:off - - private static class BaseServerResponseExceptionHolder { - private BaseServerResponseException myException; - - public BaseServerResponseException getException() { - return myException; - } - - public void setException(BaseServerResponseException myException) { - this.myException = myException; - } + return myTransactionProcessor.transaction(theRequestDetails, theRequest); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java new file mode 100644 index 00000000000..44b73c08a5d --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java @@ -0,0 +1,133 @@ +package ca.uhn.fhir.jpa.dao.r4; + +import ca.uhn.fhir.jpa.dao.TransactionProcessor; +import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.OperationOutcome; +import org.hl7.fhir.r4.model.Resource; +import org.hl7.fhir.exceptions.FHIRException; +import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; +import org.hl7.fhir.instance.model.api.IBaseResource; + +import java.util.Date; +import java.util.List; + +public class TransactionProcessorVersionAdapterR4 implements TransactionProcessor.ITransactionProcessorVersionAdapter { + @Override + public void setResponseStatus(Bundle.BundleEntryComponent theBundleEntry, String theStatus) { + theBundleEntry.getResponse().setStatus(theStatus); + } + + @Override + public void setResponseLastModified(Bundle.BundleEntryComponent theBundleEntry, Date theLastModified) { + theBundleEntry.getResponse().setLastModified(theLastModified); + } + + @Override + public void setResource(Bundle.BundleEntryComponent theBundleEntry, IBaseResource theResource) { + theBundleEntry.setResource((Resource) theResource); + } + + @Override + public IBaseResource getResource(Bundle.BundleEntryComponent theBundleEntry) { + return theBundleEntry.getResource(); + } + + @Override + public String getBundleType(Bundle theRequest) { + if (theRequest.getType() == null) { + return null; + } + return theRequest.getTypeElement().getValue().toCode(); + } + + @Override + public void populateEntryWithOperationOutcome(BaseServerResponseException theCaughtEx, Bundle.BundleEntryComponent theEntry) { + OperationOutcome oo = new OperationOutcome(); + oo.addIssue().setSeverity(OperationOutcome.IssueSeverity.ERROR).setDiagnostics(theCaughtEx.getMessage()); + theEntry.getResponse().setOutcome(oo); + } + + @Override + public Bundle createBundle(String theBundleType) { + Bundle resp = new Bundle(); + try { + resp.setType(Bundle.BundleType.fromCode(theBundleType)); + } catch (FHIRException theE) { + throw new InternalErrorException("Unknown bundle type: " + theBundleType); + } + return resp; + } + + @Override + public List getEntries(Bundle theRequest) { + return theRequest.getEntry(); + } + + @Override + public void addEntry(Bundle theBundle, Bundle.BundleEntryComponent theEntry) { + theBundle.addEntry(theEntry); + } + + @Override + public Bundle.BundleEntryComponent addEntry(Bundle theBundle) { + return theBundle.addEntry(); + } + + @Override + public String getEntryRequestVerb(Bundle.BundleEntryComponent theEntry) { + String retVal = null; + Bundle.HTTPVerb value = theEntry.getRequest().getMethodElement().getValue(); + if (value != null) { + retVal = value.toCode(); + } + return retVal; + } + + @Override + public String getFullUrl(Bundle.BundleEntryComponent theEntry) { + return theEntry.getFullUrl(); + } + + @Override + public String getEntryIfNoneExist(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfNoneExist(); + } + + @Override + public String getEntryRequestUrl(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getUrl(); + } + + @Override + public void setResponseLocation(Bundle.BundleEntryComponent theEntry, String theResponseLocation) { + theEntry.getResponse().setLocation(theResponseLocation); + } + + @Override + public void setResponseETag(Bundle.BundleEntryComponent theEntry, String theEtag) { + theEntry.getResponse().setEtag(theEtag); + } + + @Override + public String getEntryRequestIfMatch(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfMatch(); + } + + @Override + public String getEntryRequestIfNoneExist(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfNoneExist(); + } + + @Override + public String getEntryRequestIfNoneMatch(Bundle.BundleEntryComponent theEntry) { + return theEntry.getRequest().getIfNoneMatch(); + } + + @Override + public void setResponseOutcome(Bundle.BundleEntryComponent theEntry, IBaseOperationOutcome theOperationOutcome) { + theEntry.getResponse().setOutcome((Resource) theOperationOutcome); + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index 24af953ad46..4c7a7c05070 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -16,6 +16,8 @@ import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.util.TestUtil; import org.apache.commons.io.IOUtils; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.*; @@ -42,6 +44,7 @@ import java.util.Set; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -2132,6 +2135,30 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { assertNull(nextEntry.getResource()); } + + @Test + public void testTransactionWithUnknownTemnporaryIdReference() { + String methodName = "testTransactionWithUnknownTemnporaryIdReference"; + + Bundle request = new Bundle(); + + Patient p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); + + p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + p.getManagingOrganization().setReference(IdType.newRandomUuid().getValue()); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); + + try { + mySystemDao.transaction(mySrd, request); + fail(); + } catch (InvalidRequestException e) { + assertThat(e.getMessage(), Matchers.matchesPattern("Unable to satisfy placeholder ID urn:uuid:[0-9a-z-]+ found in element named 'managingOrganization' within resource of type: Patient")); + } + } + @Test public void testTransactionSearchWithCount() { String methodName = "testTransactionSearchWithCount"; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4733be32e91..d01eb6ab8c2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -253,6 +253,15 @@ When performing a ConceptMap/$translate operation with reverse="true" in the arguments, the equivalency flag is now set on the response just as it is for a non-reverse lookup. + + When executing a FHIR transaction in JPA server, if the request bundle contains + placeholder IDs references (i.e. "urn:uuid:*" references) that can not be resolved + anywhere else in the bundle, a user friendly error is now returned. Previously, + a cryptic error containing only the UUID was returned. As a part of this change, + transaction processing has now been consolidated into a single codebase for DSTU3 + / R4 (and future) versions of FHIR. This should greatly improve maintainability + and consistency for transaction processing. + From 0a1ae541e64a8a6fd6f0ed658bcdf6252a47d542 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 12 Aug 2018 12:12:09 -0400 Subject: [PATCH 19/22] Fix some test failures --- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 126 +++++++----------- .../r4/ResourceProviderR4ConceptMapTest.java | 80 +++++------ .../jpa/term/TerminologySvcImplR4Test.java | 14 +- 3 files changed, 103 insertions(+), 117 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index 4c7a7c05070..fee9fdb8591 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -16,7 +16,6 @@ import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.util.TestUtil; import org.apache.commons.io.IOUtils; -import org.hamcrest.Matcher; import org.hamcrest.Matchers; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -44,7 +43,6 @@ import java.util.Set; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; -import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -53,6 +51,11 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoR4Test.class); + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + @After public void after() { myDaoConfig.setAllowInlineMatchUrlReferences(false); @@ -190,7 +193,6 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } - @Test public void testBatchCreateWithBadRead() { Bundle request = new Bundle(); @@ -1523,26 +1525,6 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } } - @Test - public void testTransactionDoesNotAllowDanglingTemporaryIds() throws Exception { - String input = IOUtils.toString(getClass().getResourceAsStream("/cdr-bundle.json"), StandardCharsets.UTF_8); - Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, input); - - BundleEntryComponent entry = bundle.addEntry(); - Patient p = new Patient(); - p.getManagingOrganization().setReference("urn:uuid:30ce60cf-f7cb-4196-961f-cadafa8b7ff5"); - entry.setResource(p); - entry.getRequest().setMethod(HTTPVerb.POST); - entry.getRequest().setUrl("Patient"); - - try { - mySystemDao.transaction(mySrd, bundle); - fail(); - } catch (InvalidRequestException e) { - assertEquals("Unable to satisfy placeholder ID: urn:uuid:30ce60cf-f7cb-4196-961f-cadafa8b7ff5", e.getMessage()); - } - } - @Test public void testTransactionDoesNotLeavePlaceholderIds() { String input; @@ -1627,7 +1609,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { map.add(Patient.SP_IDENTIFIER, new TokenParam("foo", "bar")); search = myPatientDao.search(map); assertThat(toUnqualifiedVersionlessIdValues(search), contains(createdPatientId.toUnqualifiedVersionless().getValue())); - pat = (Patient) search.getResources(0,1).get(0); + pat = (Patient) search.getResources(0, 1).get(0); assertEquals("foo", pat.getIdentifierFirstRep().getSystem()); // Observation map = new SearchParameterMap(); @@ -1635,7 +1617,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { map.add(Observation.SP_IDENTIFIER, new TokenParam("foo", "dog")); search = myObservationDao.search(map); assertThat(toUnqualifiedVersionlessIdValues(search), contains(createdObservationId.toUnqualifiedVersionless().getValue())); - obs = (Observation) search.getResources(0,1).get(0); + obs = (Observation) search.getResources(0, 1).get(0); assertEquals("foo", obs.getIdentifierFirstRep().getSystem()); assertEquals(createdPatientId.toUnqualifiedVersionless().getValue(), obs.getSubject().getReference()); @@ -1692,7 +1674,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { map.add(Patient.SP_IDENTIFIER, new TokenParam("foo", "bar")); search = myPatientDao.search(map); assertThat(toUnqualifiedVersionlessIdValues(search), contains(createdPatientId.toUnqualifiedVersionless().getValue())); - pat = (Patient) search.getResources(0,1).get(0); + pat = (Patient) search.getResources(0, 1).get(0); assertEquals("foo", pat.getIdentifierFirstRep().getSystem()); // Observation map = new SearchParameterMap(); @@ -1700,7 +1682,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { map.add(Observation.SP_IDENTIFIER, new TokenParam("foo", "dog")); search = myObservationDao.search(map); assertThat(toUnqualifiedVersionlessIdValues(search), contains(createdObservationId.toUnqualifiedVersionless().getValue())); - obs = (Observation) search.getResources(0,1).get(0); + obs = (Observation) search.getResources(0, 1).get(0); assertEquals("foo", obs.getIdentifierFirstRep().getSystem()); assertEquals(createdPatientId.toUnqualifiedVersionless().getValue(), obs.getSubject().getReference()); @@ -1758,7 +1740,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { map.add(Patient.SP_IDENTIFIER, new TokenParam("foo", "bar")); search = myPatientDao.search(map); assertThat(toUnqualifiedVersionlessIdValues(search), contains(createdPatientId.toUnqualifiedVersionless().getValue())); - pat = (Patient) search.getResources(0,1).get(0); + pat = (Patient) search.getResources(0, 1).get(0); assertEquals("foo", pat.getIdentifierFirstRep().getSystem()); // Observation map = new SearchParameterMap(); @@ -1766,7 +1748,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { map.add(Observation.SP_IDENTIFIER, new TokenParam("foo", "dog")); search = myObservationDao.search(map); assertThat(toUnqualifiedVersionlessIdValues(search), contains(createdObservationId.toUnqualifiedVersionless().getValue())); - obs = (Observation) search.getResources(0,1).get(0); + obs = (Observation) search.getResources(0, 1).get(0); assertEquals("foo", obs.getIdentifierFirstRep().getSystem()); assertEquals(createdPatientId.toUnqualifiedVersionless().getValue(), obs.getSubject().getReference()); assertEquals(ObservationStatus.FINAL, obs.getStatus()); @@ -2135,7 +2117,6 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { assertNull(nextEntry.getResource()); } - @Test public void testTransactionWithUnknownTemnporaryIdReference() { String methodName = "testTransactionWithUnknownTemnporaryIdReference"; @@ -2155,7 +2136,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { mySystemDao.transaction(mySrd, request); fail(); } catch (InvalidRequestException e) { - assertThat(e.getMessage(), Matchers.matchesPattern("Unable to satisfy placeholder ID urn:uuid:[0-9a-z-]+ found in element named 'managingOrganization' within resource of type: Patient")); + assertThat(e.getMessage(), Matchers.matchesPattern("Unable to satisfy placeholder ID urn:uuid:[0-9a-z-]+ found in element named 'managingOrganization' within resource of type: Patient")); } } @@ -3074,44 +3055,6 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { assertEquals(1, found.size().intValue()); } - @Test - public void testTransactionWithRelativeOidIds() { - Bundle res = new Bundle(); - res.setType(BundleType.TRANSACTION); - - Patient p1 = new Patient(); - p1.setId("urn:oid:0.1.2.3"); - p1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds01"); - res.addEntry().setResource(p1).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); - - Observation o1 = new Observation(); - o1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds02"); - o1.setSubject(new Reference("urn:oid:0.1.2.3")); - res.addEntry().setResource(o1).getRequest().setMethod(HTTPVerb.POST).setUrl("Observation"); - - Observation o2 = new Observation(); - o2.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds03"); - o2.setSubject(new Reference("urn:oid:0.1.2.3")); - res.addEntry().setResource(o2).getRequest().setMethod(HTTPVerb.POST).setUrl("Observation"); - - Bundle resp = mySystemDao.transaction(mySrd, res); - - ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); - - assertEquals(BundleType.TRANSACTIONRESPONSE, resp.getTypeElement().getValue()); - assertEquals(3, resp.getEntry().size()); - - assertTrue(resp.getEntry().get(0).getResponse().getLocation(), new IdType(resp.getEntry().get(0).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); - assertTrue(resp.getEntry().get(1).getResponse().getLocation(), new IdType(resp.getEntry().get(1).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); - assertTrue(resp.getEntry().get(2).getResponse().getLocation(), new IdType(resp.getEntry().get(2).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); - - o1 = myObservationDao.read(new IdType(resp.getEntry().get(1).getResponse().getLocation()), mySrd); - o2 = myObservationDao.read(new IdType(resp.getEntry().get(2).getResponse().getLocation()), mySrd); - assertThat(o1.getSubject().getReferenceElement().getValue(), endsWith("Patient/" + p1.getIdElement().getIdPart())); - assertThat(o2.getSubject().getReferenceElement().getValue(), endsWith("Patient/" + p1.getIdElement().getIdPart())); - - } - // // // /** @@ -3214,6 +3157,44 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { // // } + @Test + public void testTransactionWithRelativeOidIds() { + Bundle res = new Bundle(); + res.setType(BundleType.TRANSACTION); + + Patient p1 = new Patient(); + p1.setId("urn:oid:0.1.2.3"); + p1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds01"); + res.addEntry().setResource(p1).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); + + Observation o1 = new Observation(); + o1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds02"); + o1.setSubject(new Reference("urn:oid:0.1.2.3")); + res.addEntry().setResource(o1).getRequest().setMethod(HTTPVerb.POST).setUrl("Observation"); + + Observation o2 = new Observation(); + o2.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds03"); + o2.setSubject(new Reference("urn:oid:0.1.2.3")); + res.addEntry().setResource(o2).getRequest().setMethod(HTTPVerb.POST).setUrl("Observation"); + + Bundle resp = mySystemDao.transaction(mySrd, res); + + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); + + assertEquals(BundleType.TRANSACTIONRESPONSE, resp.getTypeElement().getValue()); + assertEquals(3, resp.getEntry().size()); + + assertTrue(resp.getEntry().get(0).getResponse().getLocation(), new IdType(resp.getEntry().get(0).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); + assertTrue(resp.getEntry().get(1).getResponse().getLocation(), new IdType(resp.getEntry().get(1).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); + assertTrue(resp.getEntry().get(2).getResponse().getLocation(), new IdType(resp.getEntry().get(2).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); + + o1 = myObservationDao.read(new IdType(resp.getEntry().get(1).getResponse().getLocation()), mySrd); + o2 = myObservationDao.read(new IdType(resp.getEntry().get(2).getResponse().getLocation()), mySrd); + assertThat(o1.getSubject().getReferenceElement().getValue(), endsWith("Patient/" + p1.getIdElement().getIdPart())); + assertThat(o2.getSubject().getReferenceElement().getValue(), endsWith("Patient/" + p1.getIdElement().getIdPart())); + + } + /** * This is not the correct way to do it, but we'll allow it to be lenient */ @@ -3257,7 +3238,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { @Test public void testTransactionWithReplacement() { - byte[] bytes = new byte[] {0, 1, 2, 3, 4}; + byte[] bytes = new byte[]{0, 1, 2, 3, 4}; Binary binary = new Binary(); binary.setId(IdType.newRandomUuid()); @@ -3426,9 +3407,4 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ConceptMapTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ConceptMapTest.java index 904b248e682..920d44d5586 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ConceptMapTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ConceptMapTest.java @@ -988,9 +988,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(1, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1069,9 +1069,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1083,9 +1083,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1135,9 +1135,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1149,9 +1149,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1205,9 +1205,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(4, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1219,9 +1219,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1247,9 +1247,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(3); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1298,9 +1298,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1312,9 +1312,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1365,9 +1365,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1379,9 +1379,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1432,9 +1432,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(1, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1485,9 +1485,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(1, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1536,9 +1536,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1550,9 +1550,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1601,9 +1601,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1615,9 +1615,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); @@ -1659,9 +1659,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(2, getNumberOfParametersByName(respParams, "match")); param = getParametersByName(respParams, "match").get(0); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); ParametersParameterComponent part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("equal", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); Coding coding = (Coding) part.getValue(); assertEquals("12345", coding.getCode()); @@ -1673,9 +1673,9 @@ public class ResourceProviderR4ConceptMapTest extends BaseResourceProviderR4Test assertEquals(CM_URL, ((UriType) part.getValue()).getValueAsString()); param = getParametersByName(respParams, "match").get(1); - assertEquals(2, param.getPart().size()); + assertEquals(3, param.getPart().size()); part = getPartByName(param, "equivalence"); - assertFalse(part.hasValue()); + assertEquals("narrower", ((CodeType)part.getValue()).getCode()); part = getPartByName(param, "concept"); coding = (Coding) part.getValue(); assertEquals("78901", coding.getCode()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java index 920febf026c..bbec7646802 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java @@ -190,12 +190,22 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test { assertEquals("Version 1", element.getSystemVersion()); assertEquals(VS_URL, element.getValueSet()); assertEquals(CM_URL, element.getConceptMapUrl()); - assertEquals(1, element.getConceptMapGroupElementTargets().size()); + + assertEquals(2, element.getConceptMapGroupElementTargets().size()); target = element.getConceptMapGroupElementTargets().get(0); - ourLog.info("ConceptMap.group(0).element(1).target(0):\n" + target.toString()); + assertEquals("45678", target.getCode()); + assertEquals("Target Code 45678", target.getDisplay()); + assertEquals(CS_URL_2, target.getSystem()); + assertEquals("Version 2", target.getSystemVersion()); + assertEquals(ConceptMapEquivalence.WIDER, target.getEquivalence()); + assertEquals(VS_URL_2, target.getValueSet()); + assertEquals(CM_URL, target.getConceptMapUrl()); + // We had deliberately added a duplicate, and here it is... + target = element.getConceptMapGroupElementTargets().get(1); + ourLog.info("ConceptMap.group(0).element(1).target(1):\n" + target.toString()); assertEquals("45678", target.getCode()); assertEquals("Target Code 45678", target.getDisplay()); assertEquals(CS_URL_2, target.getSystem()); From 52cfdb7b8b1e570c0d5022a1d00c36827c28da4f Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 12 Aug 2018 13:43:31 -0400 Subject: [PATCH 20/22] Add some test logging --- .../ca/uhn/fhir/jpa/config/BaseConfig.java | 10 ++++---- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 24 +++++++++++++------ .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 20 ---------------- ...hirResourceDaoR4UniqueSearchParamTest.java | 18 +++++++------- pom.xml | 2 +- 5 files changed, 33 insertions(+), 41 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index a30a11b5a6a..2abb8c59d09 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -50,6 +50,7 @@ import org.springframework.scheduling.concurrent.ScheduledExecutorFactoryBean; import org.springframework.scheduling.config.ScheduledTaskRegistrar; import javax.annotation.Nonnull; +import java.util.concurrent.ScheduledExecutorService; @Configuration @EnableScheduling @@ -100,10 +101,11 @@ public abstract class BaseConfig implements SchedulingConfigurer { } @Bean() - public ScheduledExecutorFactoryBean scheduledExecutorService() { + public ScheduledExecutorService scheduledExecutorService() { ScheduledExecutorFactoryBean b = new ScheduledExecutorFactoryBean(); b.setPoolSize(5); - return b; + b.afterPropertiesSet(); + return b.getObject(); } @Bean(autowire = Autowire.BY_TYPE) @@ -147,8 +149,8 @@ public abstract class BaseConfig implements SchedulingConfigurer { @Bean(name = TASK_EXECUTOR_NAME) public TaskScheduler taskScheduler() { ConcurrentTaskScheduler retVal = new ConcurrentTaskScheduler(); - retVal.setConcurrentExecutor(scheduledExecutorService().getObject()); - retVal.setScheduledExecutor(scheduledExecutorService().getObject()); + retVal.setConcurrentExecutor(scheduledExecutorService()); + retVal.setScheduledExecutor(scheduledExecutorService()); return retVal; } 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 2d76bb864d8..278ead2e201 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 @@ -90,6 +90,8 @@ public class SearchBuilder implements ISearchBuilder { private static Long NO_MORE = -1L; private static HandlerTypeEnum ourLastHandlerMechanismForUnitTest; private static SearchParameterMap ourLastHandlerParamsForUnitTest; + private static String ourLastHandlerThreadForUnitTest; + private static boolean ourTrackHandlersForUnitTest; private List myAlsoIncludePids; private CriteriaBuilder myBuilder; private BaseHapiFhirDao myCallingDao; @@ -1295,8 +1297,11 @@ public class SearchBuilder implements ISearchBuilder { } Set uniqueQueryStrings = BaseHapiFhirDao.extractCompositeStringUniquesValueChains(myResourceName, params); - ourLastHandlerParamsForUnitTest = theParams; - ourLastHandlerMechanismForUnitTest = HandlerTypeEnum.UNIQUE_INDEX; + if (ourTrackHandlersForUnitTest) { + ourLastHandlerParamsForUnitTest = theParams; + ourLastHandlerMechanismForUnitTest = HandlerTypeEnum.UNIQUE_INDEX; + ourLastHandlerThreadForUnitTest = Thread.currentThread().getName(); + } return new UniqueIndexIterator(uniqueQueryStrings); } @@ -1307,8 +1312,11 @@ public class SearchBuilder implements ISearchBuilder { } } - ourLastHandlerParamsForUnitTest = theParams; - ourLastHandlerMechanismForUnitTest = HandlerTypeEnum.STANDARD_QUERY; + if (ourTrackHandlersForUnitTest) { + ourLastHandlerParamsForUnitTest = theParams; + ourLastHandlerMechanismForUnitTest = HandlerTypeEnum.STANDARD_QUERY; + ourLastHandlerThreadForUnitTest = Thread.currentThread().getName(); + } return new QueryIterator(); } @@ -1745,7 +1753,7 @@ public class SearchBuilder implements ISearchBuilder { boolean matchAll = "*".equals(nextInclude.getValue()); if (matchAll) { String sql; - sql = "SELECT r FROM ResourceLink r WHERE r." + searchFieldName + " IN (:target_pids)"; + sql = "SELECT r FROM ResourceLink r WHERE r." + searchFieldName + " IN (:target_pids) "; TypedQuery q = theEntityManager.createQuery(sql, ResourceLink.class); q.setParameter("target_pids", nextRoundMatches); List results = q.getResultList(); @@ -2108,14 +2116,16 @@ public class SearchBuilder implements ISearchBuilder { } @VisibleForTesting - public static SearchParameterMap getLastHandlerParamsForUnitTest() { - return ourLastHandlerParamsForUnitTest; + public static String getLastHandlerParamsForUnitTest() { + return ourLastHandlerParamsForUnitTest.toString() + " on thread [" + ourLastHandlerThreadForUnitTest +"]"; } @VisibleForTesting public static void resetLastHandlerMechanismForUnitTest() { ourLastHandlerMechanismForUnitTest = null; ourLastHandlerParamsForUnitTest = null; + ourLastHandlerThreadForUnitTest = null; + ourTrackHandlersForUnitTest = true; } static Predicate[] toArray(List thePredicates) { 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 da8ff3e43a7..effc8d746bd 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 @@ -1409,26 +1409,6 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { } } - @Test - public void testTransactionDoesNotAllowDanglingTemporaryIds() throws Exception { - String input = IOUtils.toString(getClass().getResourceAsStream("/cdr-bundle.json"), StandardCharsets.UTF_8); - Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, input); - - BundleEntryComponent entry = bundle.addEntry(); - Patient p = new Patient(); - p.getManagingOrganization().setReference("urn:uuid:30ce60cf-f7cb-4196-961f-cadafa8b7ff5"); - entry.setResource(p); - entry.getRequest().setMethod(HTTPVerb.POST); - entry.getRequest().setUrl("Patient"); - - try { - mySystemDao.transaction(mySrd, bundle); - fail(); - } catch (InvalidRequestException e) { - assertEquals("Unable to satisfy placeholder ID: urn:uuid:30ce60cf-f7cb-4196-961f-cadafa8b7ff5", e.getMessage()); - } - } - @Test public void testTransactionDoesNotLeavePlaceholderIds() throws Exception { String input; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UniqueSearchParamTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UniqueSearchParamTest.java index ea57f398263..093fb4a5328 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UniqueSearchParamTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UniqueSearchParamTest.java @@ -1,11 +1,9 @@ package ca.uhn.fhir.jpa.dao.r4; -import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.SearchBuilder; import ca.uhn.fhir.jpa.dao.SearchParameterMap; import ca.uhn.fhir.jpa.entity.ResourceIndexedCompositeStringUnique; -import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.search.JpaRuntimeSearchParam; import ca.uhn.fhir.jpa.util.JpaConstants; import ca.uhn.fhir.rest.api.server.IBundleProvider; @@ -29,7 +27,6 @@ import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nonnull; import java.util.Collections; import java.util.List; -import java.util.Optional; import java.util.UUID; import static org.hamcrest.Matchers.*; @@ -56,6 +53,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { public void before() { myDaoConfig.setDefaultSearchParamsCanBeOverridden(true); myDaoConfig.setSchedulingDisabled(true); + SearchBuilder.resetLastHandlerMechanismForUnitTest(); } private void createUniqueBirthdateAndGenderSps() { @@ -94,6 +92,8 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { mySearchParameterDao.update(sp); mySearchParamRegsitry.forceRefresh(); + + SearchBuilder.resetLastHandlerMechanismForUnitTest(); } @@ -755,7 +755,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { params.add("birthdate", new DateParam("2011-01-01")); IBundleProvider results = myPatientDao.search(params); assertThat(toUnqualifiedVersionlessIdValues(results), containsInAnyOrder(id1.getValue())); - assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest().toString(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); + assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); } @@ -780,7 +780,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { IBundleProvider results = myPatientDao.search(params); String searchId = results.getUuid(); assertThat(toUnqualifiedVersionlessIdValues(results), containsInAnyOrder(id1)); - assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest().toString(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); + assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); // Other order SearchBuilder.resetLastHandlerMechanismForUnitTest(); @@ -799,14 +799,14 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { params.add("birthdate", new DateParam("2011-01-03")); results = myPatientDao.search(params); assertThat(toUnqualifiedVersionlessIdValues(results), empty()); - assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest().toString(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); + assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); SearchBuilder.resetLastHandlerMechanismForUnitTest(); params = new SearchParameterMap(); params.add("birthdate", new DateParam("2011-01-03")); results = myPatientDao.search(params); assertThat(toUnqualifiedVersionlessIdValues(results), empty()); - assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest().toString(), SearchBuilder.HandlerTypeEnum.STANDARD_QUERY, SearchBuilder.getLastHandlerMechanismForUnitTest()); + assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest(), SearchBuilder.HandlerTypeEnum.STANDARD_QUERY, SearchBuilder.getLastHandlerMechanismForUnitTest()); } @@ -872,7 +872,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { SearchBuilder.resetLastHandlerMechanismForUnitTest(); IIdType id1 = myPatientDao.update(pt1, "Patient?name=FAMILY1&organization:Organization=ORG").getId().toUnqualifiedVersionless(); - assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest().toString(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); + assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); uniques = myResourceIndexedCompositeStringUniqueDao.findAll(); assertEquals(1, uniques.size()); assertEquals("Patient/" + id1.getIdPart(), uniques.get(0).getResource().getIdDt().toUnqualifiedVersionless().getValue()); @@ -886,7 +886,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test { SearchBuilder.resetLastHandlerMechanismForUnitTest(); id1 = myPatientDao.update(pt1, "Patient?name=FAMILY1&organization:Organization=ORG").getId().toUnqualifiedVersionless(); - assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest().toString(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); + assertEquals(SearchBuilder.getLastHandlerParamsForUnitTest(), SearchBuilder.HandlerTypeEnum.UNIQUE_INDEX, SearchBuilder.getLastHandlerMechanismForUnitTest()); uniques = myResourceIndexedCompositeStringUniqueDao.findAll(); assertEquals(1, uniques.size()); assertEquals("Patient/" + id1.getIdPart(), uniques.get(0).getResource().getIdDt().toUnqualifiedVersionless().getValue()); diff --git a/pom.xml b/pom.xml index 71c4c33e5d3..dd463ab52ed 100644 --- a/pom.xml +++ b/pom.xml @@ -818,7 +818,7 @@ org.apache.commons commons-dbcp2 - 2.1.1 + 2.5.0 org.apache.commons From d63cd267ab9b25e775c93661256cafd01eaf3e84 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sun, 12 Aug 2018 16:52:13 -0400 Subject: [PATCH 21/22] Test fixes --- .../jpa/config/dstu3/BaseDstu3Config.java | 4 ++-- .../fhir/jpa/dao/TransactionProcessor.java | 20 +++++++++++++++++++ .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 4 ++-- ...ansactionProcessorVersionAdapterDstu3.java | 20 +++++++++++++++++++ .../uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java | 4 ++-- .../TransactionProcessorVersionAdapterR4.java | 20 +++++++++++++++++++ ...rceptorRegisteredToDaoConfigDstu2Test.java | 2 +- ...tivatesPreExistingSubscriptionsR4Test.java | 3 +-- 8 files changed, 68 insertions(+), 9 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java index 04cf5103996..2381a0bcf15 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/dstu3/BaseDstu3Config.java @@ -37,9 +37,9 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; * 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. 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 cfa1376716c..554a3e58723 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 @@ -1,5 +1,25 @@ package ca.uhn.fhir.jpa.dao; +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2018 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 ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.entity.ResourceTable; 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 fd7cbbef632..32e664faeb0 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 @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao.dstu3; * 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. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java index 1b8d712f6ac..ddb198db705 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/TransactionProcessorVersionAdapterDstu3.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.jpa.dao.dstu3; +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2018 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 ca.uhn.fhir.jpa.dao.TransactionProcessor; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java index 00e7fe1650e..9d06dbe3d38 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao.r4; * 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. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java index 44b73c08a5d..db5be3e65c0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/TransactionProcessorVersionAdapterR4.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.jpa.dao.r4; +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2018 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 ca.uhn.fhir.jpa.dao.TransactionProcessor; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestWithInterceptorRegisteredToDaoConfigDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestWithInterceptorRegisteredToDaoConfigDstu2Test.java index 51e57322d66..2ea0529282a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestWithInterceptorRegisteredToDaoConfigDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/RestHookTestWithInterceptorRegisteredToDaoConfigDstu2Test.java @@ -206,7 +206,7 @@ public class RestHookTestWithInterceptorRegisteredToDaoConfigDstu2Test extends B subscriptionTemp.setCriteria(criteria1); ourClient.update().resource(subscriptionTemp).withId(subscriptionTemp.getIdElement()).execute(); - + waitForQueueToDrain(); Observation observation2 = sendObservation(code, "SNOMED-CT"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookActivatesPreExistingSubscriptionsR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookActivatesPreExistingSubscriptionsR4Test.java index ad9eaacd2d4..f7ca0923140 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookActivatesPreExistingSubscriptionsR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/r4/RestHookActivatesPreExistingSubscriptionsR4Test.java @@ -52,6 +52,7 @@ public class RestHookActivatesPreExistingSubscriptionsR4Test extends BaseResourc @Before public void beforeSetSubscriptionActivatingInterceptor() { SubscriptionActivatingSubscriber.setWaitForSubscriptionActivationSynchronouslyForUnitTest(true); + getRestHookSubscriptionInterceptor().initSubscriptions(); } @@ -102,8 +103,6 @@ public class RestHookActivatesPreExistingSubscriptionsR4Test extends BaseResourc createSubscription(criteria1, payload, ourListenerServerBase); createSubscription(criteria2, payload, ourListenerServerBase); - assertFalse(hasRestHookSubscriptionInterceptor()); - ourRestServer.registerInterceptor(getRestHookSubscriptionInterceptor()); getRestHookSubscriptionInterceptor().initSubscriptions(); From 080559027bc0608fe7bd40d6bbecd4d72039461b Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sun, 12 Aug 2018 17:56:42 -0400 Subject: [PATCH 22/22] Credit for #1022 --- pom.xml | 4 ++++ src/changes/changes.xml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/pom.xml b/pom.xml index 8128088772d..fe024fdedee 100644 --- a/pom.xml +++ b/pom.xml @@ -468,6 +468,10 @@ jbalbien + + alinleonard + Alin Leonard + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index e4362f3952c..dc6939a414e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -266,6 +266,10 @@ ResponseHighlighterInterceptor now displays the total size of the output and an estimate of the transfer time at the bottom of the response. + + The Prefer header is now honoured for HTTP PATCH requests. Thanks to + Alin Leonard for the Pull Request! +