6700 alternate api (hapifhir#6702)

tadgh · web-flow · commit 0a79f4e153aa · 2025-02-12T18:48:03.000Z
* Make the Canonicalizer non-null-default-partition-id aware

* Rename test

* Changelog

* Rework

* Remove dead IT

* remove fixme
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
@@ -333,11 +333,13 @@ public static RequestPartitionId allPartitions() {
 	}
 
 	@Nonnull
+	//	TODO GGG: This is a now-bad usage and we should remove it. we cannot assume null means default.
 	public static RequestPartitionId defaultPartition() {
 		return fromPartitionIds(Collections.singletonList(null));
 	}
 
 	@Nonnull
+	//	TODO GGG: This is a now-bad usage and we should remove it. we cannot assume null means default.
 	public static RequestPartitionId defaultPartition(@Nullable LocalDate thePartitionDate) {
 		return fromPartitionIds(Collections.singletonList(null), thePartitionDate);
 	}
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6700-cross-partition-checker.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6700-cross-partition-checker.yaml
@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 6700
+title: "Previously if a non-null default partition ID was selected when partitioning is enabled, the subscription matcher would fail to find cross-partition subscriptions, causing subscriptions to appear to not be working. This has been corrected."
diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/matcher/subscriber/SubscriptionRegisteringSubscriberTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/matcher/subscriber/SubscriptionRegisteringSubscriberTest.java
@@ -4,7 +4,6 @@
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
-import ca.uhn.fhir.jpa.model.entity.StorageSettings;
 import ca.uhn.fhir.jpa.subscription.match.registry.SubscriptionCanonicalizer;
 import ca.uhn.fhir.jpa.subscription.match.registry.SubscriptionRegistry;
 import ca.uhn.fhir.jpa.subscription.model.ResourceModifiedJsonMessage;
diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionCanonicalizerTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionCanonicalizerTest.java
@@ -1,28 +1,36 @@
 package ca.uhn.fhir.jpa.subscription.match.registry;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.interceptor.model.ReadPartitionIdRequestDetails;
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.model.config.SubscriptionSettings;
+import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.subscription.model.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.model.CanonicalSubscriptionChannelType;
 import ca.uhn.fhir.jpa.subscription.model.CanonicalTopicSubscriptionFilter;
 import ca.uhn.fhir.model.api.ExtensionDt;
 import ca.uhn.fhir.model.primitive.BooleanDt;
+import ca.uhn.fhir.rest.api.Constants;
+import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.subscription.SubscriptionConstants;
 import ca.uhn.fhir.subscription.SubscriptionTestDataHelper;
 import ca.uhn.fhir.util.HapiExtensions;
 import jakarta.annotation.Nonnull;
 import org.assertj.core.api.AssertionsForClassTypes;
+import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.BooleanType;
 import org.hl7.fhir.r4.model.Extension;
 import org.hl7.fhir.r4.model.Subscription;
 import org.hl7.fhir.r5.model.Coding;
 import org.hl7.fhir.r5.model.Enumerations;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import java.util.Set;
 import java.util.stream.Stream;
 
 import static ca.uhn.fhir.rest.api.Constants.CT_FHIR_JSON_NEW;
@@ -37,6 +45,8 @@
 
 class SubscriptionCanonicalizerTest {
 
+	private static final Integer NON_NULL_DEFAULT_PARTITION_ID = 666;
+
 	FhirContext r4Context = FhirContext.forR4();
 
 	private final SubscriptionCanonicalizer testedSC = new SubscriptionCanonicalizer(r4Context, new SubscriptionSettings());
@@ -172,6 +182,90 @@ private static Stream<RequestPartitionId> crossPartitionParams() {
 		return Stream.of(null, RequestPartitionId.fromPartitionId(1), RequestPartitionId.defaultPartition()) ;
 	}
 
+	private class FakeNonNullDefaultPartitionIDHelper implements IRequestPartitionHelperSvc {
+
+		@Override
+		public @Nullable Integer getDefaultPartitionId() {
+			return NON_NULL_DEFAULT_PARTITION_ID;
+		}
+
+		@Override
+		public boolean isDefaultPartition(@NotNull RequestPartitionId theRequestPartitionId) {
+			return  theRequestPartitionId.getPartitionIds().get(0).equals(NON_NULL_DEFAULT_PARTITION_ID);
+		}
+
+		@Override
+		public boolean hasDefaultPartitionId(@NotNull RequestPartitionId theRequestPartitionId) {
+			return theRequestPartitionId.getPartitionIds().stream().anyMatch(part -> part.equals(NON_NULL_DEFAULT_PARTITION_ID));
+		}
+
+		@Override
+		public RequestPartitionId determineReadPartitionForRequest(@Nullable RequestDetails theRequest, @NotNull ReadPartitionIdRequestDetails theDetails) {
+			return null;
+		}
+
+		@Override
+		public RequestPartitionId determineGenericPartitionForRequest(RequestDetails theRequestDetails) {
+			return null;
+		}
+
+		@Override
+		public @NotNull RequestPartitionId determineCreatePartitionForRequest(@Nullable RequestDetails theRequest, @NotNull IBaseResource theResource, @NotNull String theResourceType) {
+			return null;
+		}
+
+		@Override
+		public @NotNull Set<Integer> toReadPartitions(@NotNull RequestPartitionId theRequestPartitionId) {
+			return Set.of();
+		}
+
+		@Override
+		public boolean isResourcePartitionable(String theResourceType) {
+			return false;
+		}
+
+		@Override
+		public RequestPartitionId validateAndNormalizePartitionIds(RequestPartitionId theRequestPartitionId) {
+			return null;
+		}
+
+		@Override
+		public RequestPartitionId validateAndNormalizePartitionNames(RequestPartitionId theRequestPartitionId) {
+			return null;
+		}
+	}
+	@Test
+	public void testNonNullDefaultPartitionIDCanonicalizesToCrossPartition() {
+		IRequestPartitionHelperSvc myHelperSvc = new FakeNonNullDefaultPartitionIDHelper();
+		final SubscriptionSettings subscriptionSettings = new SubscriptionSettings();
+
+		subscriptionSettings.setCrossPartitionSubscriptionEnabled(true);
+		final SubscriptionCanonicalizer subscriptionCanonicalizer = new SubscriptionCanonicalizer(FhirContext.forR4(), subscriptionSettings);
+		subscriptionCanonicalizer.setPartitionHelperSvc(myHelperSvc);
+		Subscription subscription = buildMdmSubscriptionR4("test-subscription", "Patient?");
+		CanonicalSubscription canonicalize = subscriptionCanonicalizer.canonicalize(subscription);
+
+		assertThat(canonicalize.isCrossPartitionEnabled()).isTrue();
+
+	}
+	private Subscription buildMdmSubscriptionR4(String theId, String theCriteria) {
+		Subscription retval = new Subscription();
+		retval.setId(theId);
+		retval.setReason("MDM Simulacrum Subscription");
+		retval.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		retval.setCriteria(theCriteria);
+		retval.addExtension()
+			.setUrl(HapiExtensions.EXTENSION_SUBSCRIPTION_CROSS_PARTITION)
+			.setValue(new BooleanType().setValue(true));
+		Subscription.SubscriptionChannelComponent channel = retval.getChannel();
+		channel.setType(Subscription.SubscriptionChannelType.MESSAGE);
+		channel.setEndpoint("channel:test");
+		channel.setPayload(Constants.CT_JSON);
+		RequestPartitionId retPartId = RequestPartitionId.fromPartitionId(NON_NULL_DEFAULT_PARTITION_ID);
+		retval.setUserData(Constants.RESOURCE_PARTITION_ID, retPartId);
+		return retval;
+	}
+
 	@ParameterizedTest
 	@MethodSource("crossPartitionParams")
 	void testSubscriptionCrossPartitionEnableProperty_forDstu2WithExtensionAndPartitions(RequestPartitionId theRequestPartitionId) {
diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionRegistryTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionRegistryTest.java
@@ -2,7 +2,6 @@
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
-import ca.uhn.fhir.jpa.model.entity.StorageSettings;
 import ca.uhn.fhir.jpa.subscription.channel.subscription.ISubscriptionDeliveryChannelNamer;
 import ca.uhn.fhir.jpa.subscription.channel.subscription.SubscriptionChannelRegistry;
 import ca.uhn.fhir.jpa.subscription.model.CanonicalSubscription;
diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriberTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriberTest.java
@@ -6,6 +6,7 @@
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.model.config.SubscriptionSettings;
+import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.subscription.match.matcher.subscriber.SubscriptionCriteriaParser;
 import ca.uhn.fhir.jpa.subscription.match.matcher.subscriber.SubscriptionMatchDeliverer;
 import ca.uhn.fhir.jpa.subscription.match.matcher.subscriber.SubscriptionMatchingSubscriber;
@@ -33,6 +34,7 @@
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
+import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.Collections;
 import java.util.List;
@@ -55,8 +57,12 @@
 public class SubscriptionMatchingSubscriberTest extends BaseBlockingQueueSubscribableChannelDstu3Test {
 	private final IFhirResourceDao<Subscription> myMockSubscriptionDao = Mockito.mock(IFhirResourceDao.class);
 
+	@Autowired
+	private IRequestPartitionHelperSvc myRequestPartitionHelperSvc;
+
 	@BeforeEach
 	public void beforeEach() {
+		when(myRequestPartitionHelperSvc.isDefaultPartition(any(RequestPartitionId.class))).thenReturn(Boolean.TRUE);
 		when(myMockSubscriptionDao.getResourceType()).thenReturn(Subscription.class);
 		myDaoRegistry.register(myMockSubscriptionDao);
 	}
diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/subscriber/websocket/WebsocketConnectionValidatorTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/subscriber/websocket/WebsocketConnectionValidatorTest.java
@@ -6,6 +6,7 @@
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.cache.IResourceChangeListenerRegistry;
 import ca.uhn.fhir.jpa.model.config.PartitionSettings;
+import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
 import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryResourceMatcher;
 import ca.uhn.fhir.jpa.searchparam.matcher.SearchParamMatcher;
@@ -66,6 +67,8 @@ public class WebsocketConnectionValidatorTest {
 	@MockBean
 	SubscriptionRegistry mySubscriptionRegistry;
 	@MockBean
+	IRequestPartitionHelperSvc myRequestPartitionHelperSvc;
+	@MockBean
 	ISearchParamRegistry mySearchParamRegistry;
 	@MockBean
 	SubscriptionSettings mySubscriptionSettings;
diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionCanonicalizer.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/subscription/match/registry/SubscriptionCanonicalizer.java
@@ -24,6 +24,7 @@
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.model.config.SubscriptionSettings;
+import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.subscription.match.matcher.matching.SubscriptionMatchingStrategy;
 import ca.uhn.fhir.jpa.subscription.model.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.model.CanonicalSubscriptionChannelType;
@@ -71,11 +72,20 @@ public class SubscriptionCanonicalizer {
 	final FhirContext myFhirContext;
 	private final SubscriptionSettings mySubscriptionSettings;
 
+	private IRequestPartitionHelperSvc myHelperSvc;
+
 	@Autowired
 	public SubscriptionCanonicalizer(FhirContext theFhirContext, SubscriptionSettings theSubscriptionSettings) {
 		myFhirContext = theFhirContext;
 		mySubscriptionSettings = theSubscriptionSettings;
 	}
+	// TODO GGG: Eventually, we will unify autowiring styles. It is this way now as this is the least destrctive method
+	// to accomplish a minimal MR. I recommend moving all dependencies to setter autowiring, but that is for another
+	// day.
+	@Autowired
+	public void setPartitionHelperSvc(IRequestPartitionHelperSvc thePartitionHelperSvc) {
+		myHelperSvc = thePartitionHelperSvc;
+	}
 
 	// TODO:  LD:  remove this constructor once all callers call the 2 arg constructor above
 
@@ -787,7 +797,9 @@ private boolean handleCrossPartition(IBaseResource theSubscription) {
 		boolean isSubscriptionCreatedOnDefaultPartition = false;
 
 		if (nonNull(requestPartitionId)) {
-			isSubscriptionCreatedOnDefaultPartition = requestPartitionId.isDefaultPartition();
+			isSubscriptionCreatedOnDefaultPartition = myHelperSvc == null
+					? requestPartitionId.isDefaultPartition()
+					: myHelperSvc.isDefaultPartition(requestPartitionId);
 		}
 
 		boolean isSubscriptionDefinededAsCrossPartitionSubscription =

Original file line number	Diff line number	Diff line change
`@@ -333,11 +333,13 @@ public static RequestPartitionId allPartitions() {`
`333`	`333`	`}`
`334`	`334`
`335`	`335`	`@Nonnull`
	`336`	`+ // TODO GGG: This is a now-bad usage and we should remove it. we cannot assume null means default.`
`336`	`337`	`public static RequestPartitionId defaultPartition() {`
`337`	`338`	`return fromPartitionIds(Collections.singletonList(null));`
`338`	`339`	`}`
`339`	`340`
`340`	`341`	`@Nonnull`
	`342`	`+ // TODO GGG: This is a now-bad usage and we should remove it. we cannot assume null means default.`
`341`	`343`	`public static RequestPartitionId defaultPartition(@Nullable LocalDate thePartitionDate) {`
`342`	`344`	`return fromPartitionIds(Collections.singletonList(null), thePartitionDate);`
`343`	`345`	`}`