From 6a370c60cb579516d4002f8fc432e75b66fb41ec Mon Sep 17 00:00:00 2001 From: Michael Buckley Date: Fri, 19 Jan 2024 12:42:50 -0500 Subject: [PATCH] Cleanup and some tests (#5609) Cleanup warnings, and add some missing tests. --- .../interceptor/model/RequestPartitionId.java | 6 ++- .../java/ca/uhn/fhir/util/TaskChunker.java | 8 +++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 2 +- .../fhir/jpa/dao/index/IdHelperService.java | 3 +- .../uhn/fhir/jpa/entity/PartitionEntity.java | 19 +++++++ .../ca/uhn/fhir/jpa/util/QueryChunker.java | 5 ++ .../fhir/jpa/entity/PartitionEntityTest.java | 31 +++++++++++ .../match/registry/SubscriptionLoader.java | 14 +++-- .../jpa/topic/SubscriptionTopicLoader.java | 4 ++ .../registry/SubscriptionLoaderTest.java | 51 +++++-------------- .../uhn/fhir/jpa/dao/tx/ReindexStepTest.java | 1 + .../cache/BaseResourceCacheSynchronizer.java | 15 ++++-- .../fhir/jpa/api/dao/IFhirResourceDao.java | 7 ++- .../jpa/dao/tx/HapiTransactionService.java | 36 ++++++++----- .../jpa/dao/tx/IHapiTransactionService.java | 9 ++++ .../util/LogbackCaptureTestExtension.java | 9 ++-- 16 files changed, 152 insertions(+), 68 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/entity/PartitionEntityTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java index 91a522e5aea..e4f0b5f1201 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/model/RequestPartitionId.java @@ -38,6 +38,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; @@ -134,6 +135,9 @@ public class RequestPartitionId implements IModelJson { if (hasPartitionNames()) { b.append("names", getPartitionNames()); } + if (myAllPartitions) { + b.append("allPartitions", myAllPartitions); + } return b.build(); } @@ -216,7 +220,7 @@ public class RequestPartitionId implements IModelJson { } public List getPartitionIdsWithoutDefault() { - return getPartitionIds().stream().filter(t -> t != null).collect(Collectors.toList()); + return getPartitionIds().stream().filter(Objects::nonNull).collect(Collectors.toList()); } @Nullable diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TaskChunker.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TaskChunker.java index d28ee419f40..7d4b80f3d08 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TaskChunker.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TaskChunker.java @@ -20,10 +20,13 @@ package ca.uhn.fhir.util; * #L% */ +import jakarta.annotation.Nonnull; + import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.function.Consumer; +import java.util.stream.Stream; /** * This utility takes an input collection, breaks it up into chunks of a @@ -49,4 +52,9 @@ public class TaskChunker { theBatchConsumer.accept(batch); } } + + @Nonnull + public Stream> chunk(Stream theStream, int theChunkSize) { + return StreamUtil.partition(theStream, theChunkSize); + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 688a57c6467..6a79e566d5f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1540,7 +1540,7 @@ public abstract class BaseHapiFhirResourceDao extends B .withRequest(theRequest) .withTransactionDetails(transactionDetails) .withRequestPartitionId(requestPartitionId) - .execute(() -> doReadInTransaction(theId, theRequest, theDeletedOk, requestPartitionId)); + .read(() -> doReadInTransaction(theId, theRequest, theDeletedOk, requestPartitionId)); } private T doReadInTransaction( diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java index 318126e119d..b21c64dba93 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java @@ -158,7 +158,8 @@ public class IdHelperService implements IIdHelperService { String theResourceId, boolean theExcludeDeleted) throws ResourceNotFoundException { - assert myDontCheckActiveTransactionForUnitTest || TransactionSynchronizationManager.isSynchronizationActive(); + assert myDontCheckActiveTransactionForUnitTest || TransactionSynchronizationManager.isSynchronizationActive() + : "no transaction active"; if (theResourceId.contains("/")) { theResourceId = theResourceId.substring(theResourceId.indexOf("/") + 1); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/PartitionEntity.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/PartitionEntity.java index 9735a905278..bcaa9af7f4e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/PartitionEntity.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/PartitionEntity.java @@ -26,6 +26,9 @@ import jakarta.persistence.Id; import jakarta.persistence.Table; import jakarta.persistence.UniqueConstraint; +import java.util.ArrayList; +import java.util.List; + @Entity @Table( name = "HFJ_PARTITION", @@ -82,4 +85,20 @@ public class PartitionEntity { public RequestPartitionId toRequestPartitionId() { return RequestPartitionId.fromPartitionIdAndName(getId(), getName()); } + + /** + * Build a RequestPartitionId from the ids and names in the entities. + * @param thePartitions the entities to use for ids and names + * @return a single RequestPartitionId covering all the entities + */ + public static RequestPartitionId buildRequestPartitionId(List thePartitions) { + List ids = new ArrayList<>(thePartitions.size()); + List names = new ArrayList<>(thePartitions.size()); + for (PartitionEntity nextPartition : thePartitions) { + ids.add(nextPartition.getId()); + names.add(nextPartition.getName()); + } + + return RequestPartitionId.forPartitionIdsAndNames(names, ids, null); + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/QueryChunker.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/QueryChunker.java index f8880324a55..2d19dbd3e80 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/QueryChunker.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/QueryChunker.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.util.TaskChunker; import java.util.Collection; import java.util.List; import java.util.function.Consumer; +import java.util.stream.Stream; /** * As always, Oracle can't handle things that other databases don't mind.. In this @@ -37,4 +38,8 @@ public class QueryChunker extends TaskChunker { public void chunk(Collection theInput, Consumer> theBatchConsumer) { chunk(theInput, SearchBuilder.getMaximumPageSize(), theBatchConsumer); } + + public Stream> chunk(Stream theStream) { + return chunk(theStream, SearchBuilder.getMaximumPageSize()); + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/entity/PartitionEntityTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/entity/PartitionEntityTest.java new file mode 100644 index 00000000000..b684a87df15 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/entity/PartitionEntityTest.java @@ -0,0 +1,31 @@ +package ca.uhn.fhir.jpa.entity; + +import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; + +class PartitionEntityTest { + + @Test + void buildRequestPartitionFromList() { + // given + List entities = List.of( + new PartitionEntity().setId(1).setName("p1"), + new PartitionEntity().setId(2).setName("p2") + ); + + // when + RequestPartitionId requestPartitionId = PartitionEntity.buildRequestPartitionId(entities); + + // then + assertThat(requestPartitionId, notNullValue()); + assertThat(requestPartitionId.getPartitionIds(), equalTo(List.of(1,2))); + assertThat(requestPartitionId.getPartitionNames(), equalTo(List.of("p1","p2"))); + } + +} diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoader.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoader.java index 4e44eb8940f..130fe8edace 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoader.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoader.java @@ -24,6 +24,7 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.subscription.match.matcher.subscriber.SubscriptionActivatingSubscriber; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.subscription.SubscriptionConstants; import jakarta.annotation.Nonnull; import org.apache.commons.lang3.StringUtils; @@ -49,6 +50,9 @@ public class SubscriptionLoader extends BaseResourceCacheSynchronizer { @Autowired private SubscriptionCanonicalizer mySubscriptionCanonicalizer; + @Autowired + protected ISearchParamRegistry mySearchParamRegistry; + /** * Constructor */ @@ -157,11 +161,11 @@ public class SubscriptionLoader extends BaseResourceCacheSynchronizer { } else { error = ""; } - ourLog.error("Subscription " - + theSubscription.getIdElement().getIdPart() - + " could not be activated." - + " This will not prevent startup, but it could lead to undesirable outcomes! " - + (StringUtils.isBlank(error) ? "" : "Error: " + error)); + ourLog.error( + "Subscription {} could not be activated." + + " This will not prevent startup, but it could lead to undesirable outcomes! {}", + theSubscription.getIdElement().getIdPart(), + (StringUtils.isBlank(error) ? "" : "Error: " + error)); } public void syncSubscriptions() { diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicLoader.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicLoader.java index 82c02088ee9..de78e59a741 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicLoader.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicLoader.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.subscription.SubscriptionConstants; import ca.uhn.fhir.util.Logs; import jakarta.annotation.Nonnull; @@ -47,6 +48,9 @@ public class SubscriptionTopicLoader extends BaseResourceCacheSynchronizer { @Autowired private SubscriptionTopicRegistry mySubscriptionTopicRegistry; + @Autowired + protected ISearchParamRegistry mySearchParamRegistry; + /** * Constructor */ diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoaderTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoaderTest.java index f007a6a1150..e262fd813ae 100644 --- a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoaderTest.java +++ b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionLoaderTest.java @@ -13,27 +13,24 @@ import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.server.SimpleBundleProvider; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.subscription.SubscriptionConstants; +import ca.uhn.test.util.LogbackCaptureTestExtension; import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.Appender; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.Subscription; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentCaptor; +import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; -import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.List; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; @@ -43,13 +40,11 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class SubscriptionLoaderTest { - private Logger ourLogger; - @Spy private FhirContext myFhirContext = FhirContext.forR4Cached(); - @Mock - private Appender myAppender; + @RegisterExtension + LogbackCaptureTestExtension myLogCapture = new LogbackCaptureTestExtension(SubscriptionLoader.class); @Mock private SubscriptionRegistry mySubscriptionRegistry; @@ -81,15 +76,8 @@ public class SubscriptionLoaderTest { @InjectMocks private SubscriptionLoader mySubscriptionLoader; - private Level myStoredLogLevel; - @BeforeEach public void init() { - ourLogger = (Logger) LoggerFactory.getLogger(SubscriptionLoader.class); - - myStoredLogLevel = ourLogger.getLevel(); - ourLogger.addAppender(myAppender); - when(myResourceChangeListenerRegistry.registerResourceResourceChangeListener( anyString(), any(SearchParameterMap.class), @@ -102,12 +90,6 @@ public class SubscriptionLoaderTest { mySubscriptionLoader.registerListener(); } - @AfterEach - public void end() { - ourLogger.detachAppender(myAppender); - ourLogger.setLevel(myStoredLogLevel); - } - private IBundleProvider getSubscriptionList(List theReturnedResource) { IBundleProvider subscriptionList = new SimpleBundleProvider(theReturnedResource); @@ -122,26 +104,24 @@ public class SubscriptionLoaderTest { subscription.setId("Subscription/123"); subscription.setError("THIS IS AN ERROR"); - ourLogger.setLevel(Level.ERROR); - // when when(myDaoRegistry.getResourceDao("Subscription")) - .thenReturn(mySubscriptionDao); + .thenReturn(mySubscriptionDao); when(myDaoRegistry.isResourceTypeSupported("Subscription")) - .thenReturn(true); + .thenReturn(true); when(mySubscriptionDao.search(any(SearchParameterMap.class), any(SystemRequestDetails.class))) .thenReturn(getSubscriptionList( Collections.singletonList(subscription) )); when(mySubscriptionActivatingInterceptor.activateSubscriptionIfRequired(any(IBaseResource.class))) - .thenReturn(false); + .thenReturn(false); when(mySubscriptionActivatingInterceptor.isChannelTypeSupported(any(IBaseResource.class))) .thenReturn(true); when(mySubscriptionCanonicalizer.getSubscriptionStatus(any())).thenReturn(SubscriptionConstants.REQUESTED_STATUS); - + // test mySubscriptionLoader.syncDatabaseToCache(); @@ -149,15 +129,10 @@ public class SubscriptionLoaderTest { verify(mySubscriptionDao) .search(any(SearchParameterMap.class), any(SystemRequestDetails.class)); - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(ILoggingEvent.class); - verify(myAppender).doAppend(eventCaptor.capture()); - String actual = "Subscription " + String expected = "Subscription " + subscription.getIdElement().getIdPart() + " could not be activated."; - String msg = eventCaptor.getValue().getMessage(); - Assertions.assertTrue(msg - .contains(actual), - String.format("Expected %s, actual %s", msg, actual)); - Assertions.assertTrue(msg.contains(subscription.getError())); + assertThat(myLogCapture.getLogEvents(), hasItem(LogbackCaptureTestExtension.eventWithLevelAndMessageContains(Level.ERROR, expected))); + assertThat(myLogCapture.getLogEvents(), hasItem(LogbackCaptureTestExtension.eventWithMessageContains(subscription.getError()))); } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/tx/ReindexStepTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/tx/ReindexStepTest.java index 15cf9ed79e2..82acb95053d 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/tx/ReindexStepTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/tx/ReindexStepTest.java @@ -44,6 +44,7 @@ public class ReindexStepTest { ReindexJobParameters reindexJobParameters = new ReindexJobParameters(); reindexJobParameters.setRequestPartitionId(RequestPartitionId.fromPartitionId(expectedPartitionId)); when(myHapiTransactionService.withRequest(any())).thenCallRealMethod(); + when(myHapiTransactionService.buildExecutionBuilder(any())).thenCallRealMethod(); // when myReindexStep.doReindex(data, myDataSink, "index-id", "chunk-id", reindexJobParameters); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/cache/BaseResourceCacheSynchronizer.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/cache/BaseResourceCacheSynchronizer.java index 762eae3545a..bacdb0e64e8 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/cache/BaseResourceCacheSynchronizer.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/cache/BaseResourceCacheSynchronizer.java @@ -29,7 +29,6 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.retry.Retrier; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; -import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.subscription.SubscriptionConstants; import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; @@ -56,9 +55,6 @@ public abstract class BaseResourceCacheSynchronizer implements IResourceChangeLi public static final long REFRESH_INTERVAL = DateUtils.MILLIS_PER_MINUTE; private final String myResourceName; - @Autowired - protected ISearchParamRegistry mySearchParamRegistry; - @Autowired private IResourceChangeListenerRegistry myResourceChangeListenerRegistry; @@ -71,10 +67,19 @@ public abstract class BaseResourceCacheSynchronizer implements IResourceChangeLi private final Semaphore mySyncResourcesSemaphore = new Semaphore(1); private final Object mySyncResourcesLock = new Object(); - public BaseResourceCacheSynchronizer(String theResourceName) { + protected BaseResourceCacheSynchronizer(String theResourceName) { myResourceName = theResourceName; } + protected BaseResourceCacheSynchronizer( + String theResourceName, + IResourceChangeListenerRegistry theResourceChangeListenerRegistry, + DaoRegistry theDaoRegistry) { + myResourceName = theResourceName; + myDaoRegistry = theDaoRegistry; + myResourceChangeListenerRegistry = theResourceChangeListenerRegistry; + } + @PostConstruct public void registerListener() { if (myDaoRegistry.getResourceDaoOrNull(myResourceName) == null) { diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index 93ccff56bf2..01a8becf7d8 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -374,7 +374,7 @@ public interface IFhirResourceDao extends IDao { * The Stream MUST be closed to avoid leaking resources. * @param theParams the search * @param theRequest for partition target info - * @return a Stream than MUST only be used within the calling transaction. + * @return a Stream that MUST only be used within the calling transaction. */ default > Stream searchForIdStream( SearchParameterMap theParams, @@ -384,6 +384,11 @@ public interface IFhirResourceDao extends IDao { return iResourcePersistentIds.stream(); } + default > Stream searchForIdStream( + SearchParameterMap theParams, RequestDetails theRequest) { + return searchForIdStream(theParams, theRequest, null); + } + /** * Takes a map of incoming raw search parameters and translates/parses them into * appropriate {@link IQueryParameterType} instances of the appropriate type diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/HapiTransactionService.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/HapiTransactionService.java index 2ec94ce5f03..46023360cba 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/HapiTransactionService.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/HapiTransactionService.java @@ -104,12 +104,16 @@ public class HapiTransactionService implements IHapiTransactionService { @Override public IExecutionBuilder withRequest(@Nullable RequestDetails theRequestDetails) { - return new ExecutionBuilder(theRequestDetails); + return buildExecutionBuilder(theRequestDetails); } @Override public IExecutionBuilder withSystemRequest() { - return new ExecutionBuilder(null); + return buildExecutionBuilder(null); + } + + protected IExecutionBuilder buildExecutionBuilder(@Nullable RequestDetails theRequestDetails) { + return new ExecutionBuilder(theRequestDetails); } /** @@ -236,15 +240,7 @@ public class HapiTransactionService implements IHapiTransactionService { @Nullable protected T doExecute(ExecutionBuilder theExecutionBuilder, TransactionCallback theCallback) { - final RequestPartitionId requestPartitionId; - if (theExecutionBuilder.myRequestPartitionId != null) { - requestPartitionId = theExecutionBuilder.myRequestPartitionId; - } else if (theExecutionBuilder.myRequestDetails != null) { - requestPartitionId = myRequestPartitionHelperSvc.determineGenericPartitionForRequest( - theExecutionBuilder.myRequestDetails); - } else { - requestPartitionId = null; - } + final RequestPartitionId requestPartitionId = theExecutionBuilder.getEffectiveRequestPartitionId(); RequestPartitionId previousRequestPartitionId = null; if (requestPartitionId != null) { previousRequestPartitionId = ourRequestPartitionThreadLocal.get(); @@ -443,7 +439,8 @@ public class HapiTransactionService implements IHapiTransactionService { } } - protected class ExecutionBuilder implements IExecutionBuilder, TransactionOperations { + // wipmb is Clone ok, or do we want an explicit copy constructor? + protected class ExecutionBuilder implements IExecutionBuilder, TransactionOperations, Cloneable { private final RequestDetails myRequestDetails; private Isolation myIsolation; @@ -451,7 +448,7 @@ public class HapiTransactionService implements IHapiTransactionService { private boolean myReadOnly; private TransactionDetails myTransactionDetails; private Runnable myOnRollback; - private RequestPartitionId myRequestPartitionId; + protected RequestPartitionId myRequestPartitionId; protected ExecutionBuilder(RequestDetails theRequestDetails) { myRequestDetails = theRequestDetails; @@ -533,6 +530,19 @@ public class HapiTransactionService implements IHapiTransactionService { public Propagation getPropagation() { return myPropagation; } + + @Nullable + protected RequestPartitionId getEffectiveRequestPartitionId() { + final RequestPartitionId requestPartitionId; + if (myRequestPartitionId != null) { + requestPartitionId = myRequestPartitionId; + } else if (myRequestDetails != null) { + requestPartitionId = myRequestPartitionHelperSvc.determineGenericPartitionForRequest(myRequestDetails); + } else { + requestPartitionId = null; + } + return requestPartitionId; + } } /** diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/IHapiTransactionService.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/IHapiTransactionService.java index a8d582f2cf3..401f0f66e1a 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/IHapiTransactionService.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/tx/IHapiTransactionService.java @@ -31,6 +31,7 @@ import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionOperations; import java.util.concurrent.Callable; +import java.util.stream.Stream; /** * This class is used to execute code within the context of a database transaction, @@ -107,5 +108,13 @@ public interface IHapiTransactionService { T execute(Callable theTask); T execute(@Nonnull TransactionCallback callback); + + default T read(Callable theCallback) { + return execute(theCallback); + } + + default Stream search(Callable> theCallback) { + return execute(theCallback); + } } } diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/test/util/LogbackCaptureTestExtension.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/test/util/LogbackCaptureTestExtension.java index 062f3e7f875..f5f61161ba4 100644 --- a/hapi-fhir-test-utilities/src/main/java/ca/uhn/test/util/LogbackCaptureTestExtension.java +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/test/util/LogbackCaptureTestExtension.java @@ -23,13 +23,13 @@ import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.read.ListAppender; +import jakarta.annotation.Nonnull; import org.hamcrest.Matcher; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; import org.slf4j.LoggerFactory; -import jakarta.annotation.Nonnull; import java.util.ArrayList; import java.util.List; import java.util.function.Predicate; @@ -37,7 +37,6 @@ import java.util.stream.Collectors; /** * Test helper to collect logback lines. - * * The empty constructor will capture all log events, or you can name a log root to limit the noise. */ public class LogbackCaptureTestExtension implements BeforeEachCallback, AfterEachCallback { @@ -83,7 +82,11 @@ public class LogbackCaptureTestExtension implements BeforeEachCallback, AfterEac this((Logger) LoggerFactory.getLogger(theLoggerName), theLevel); } - /** + public LogbackCaptureTestExtension(Class theClass) { + this(theClass.getName()); + } + + /** * Returns a copy to avoid concurrent modification errors. * @return A copy of the log events so far. */