Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9fa7ff4
Initial pass over refactoring the sprawling MDM expansion
Oct 9, 2025
be91a82
Fix up an error in construction
Oct 9, 2025
daa3e48
failing test location
Oct 9, 2025
58db184
Remove dead annotation, fix logic bug in new method for determining w…
Oct 10, 2025
4a9663f
Spotless
Oct 10, 2025
f76048b
Redefine bean in jpaconfig for app ctx loading
Oct 10, 2025
ebb9a6a
comments
Oct 10, 2025
e65b52c
Version bumped to 8.5.8-SNAPSHOT
Oct 10, 2025
eb0953f
wip
Oct 10, 2025
3844dc6
wip maybe revert
Oct 14, 2025
b01f50d
wip maybe revert
Oct 14, 2025
7d452df
wip
tadgh Oct 14, 2025
cc5d682
Revert "wip maybe revert"
tadgh Oct 14, 2025
16e7d2e
Remove dead code
tadgh Oct 14, 2025
1211448
move MDM stuff down the test class stack
tadgh Oct 14, 2025
95176ab
move MDM stuff down the test class stack
tadgh Oct 14, 2025
f38c402
Push all mdm related stuff down into MDM-related classes
tadgh Oct 14, 2025
c6834cc
Patch up mock tests
tadgh Oct 14, 2025
9af2564
Patch up mock tests
tadgh Oct 14, 2025
b40fca6
Remove fixmes
tadgh Oct 14, 2025
754ec3d
Merge branch 'master' into refactor-mdm-expander-holder-out
tadgh Oct 14, 2025
8f2ebce
Remove conditionalonbean
tadgh Oct 15, 2025
00547d5
Remove service annotation
tadgh Oct 15, 2025
8547472
bump hapi version
Oct 17, 2025
4d46c84
Remove another useless configuration class
Oct 17, 2025
5f15470
Make the cacher no longer a bean, as its used only inside of the mdml…
Oct 17, 2025
557da59
Make the cacher no longer a bean, as its used only inside of the mdml…
Oct 17, 2025
c684366
Remove usage of the mdm expansion cache entirely. It doesnt work dist…
Oct 17, 2025
37dad05
Spotless
Oct 17, 2025
42bc3e0
Make IMdmSettings nullable
Oct 19, 2025
39b9837
Merge branch 'master' into refactor-mdm-expander-holder-out
Nov 19, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@
import ca.uhn.fhir.batch2.api.IJobPersistence;
import ca.uhn.fhir.batch2.config.BaseBatch2Config;
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.jpa.bulk.export.job.BulkExportJobConfig;
import ca.uhn.fhir.jpa.dao.data.IBatch2JobInstanceRepository;
import ca.uhn.fhir.jpa.dao.data.IBatch2WorkChunkMetadataViewRepository;
import ca.uhn.fhir.jpa.dao.data.IBatch2WorkChunkRepository;
import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService;
import jakarta.persistence.EntityManager;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;

@Configuration
@Import({BulkExportJobConfig.class})
public class JpaBatch2Config extends BaseBatch2Config {

@Bean
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import ca.uhn.fhir.jpa.model.search.SearchBuilderLoadIncludesParameters;
import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.mdm.svc.MdmExpandersHolder;
import ca.uhn.fhir.mdm.api.IMdmLinkExpandSvc;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
Expand Down Expand Up @@ -86,7 +86,7 @@ public class JpaBulkExportProcessor implements IBulkExportProcessor<JpaPid> {
private EntityManager myEntityManager;
private IHapiTransactionService myHapiTransactionService;
private ISearchParamRegistry mySearchParamRegistry;
private MdmExpandersHolder myMdmExpandersHolder;
private Optional<IMdmLinkExpandSvc> myMdmLinkExpandSvc;

@Autowired
public JpaBulkExportProcessor(
Expand All @@ -99,7 +99,7 @@ public JpaBulkExportProcessor(
EntityManager theEntityManager,
IHapiTransactionService theHapiTransactionService,
ISearchParamRegistry theSearchParamRegistry,
MdmExpandersHolder theMdmExpandersHolder) {
Optional<IMdmLinkExpandSvc> theMdmLinkExpandSvc) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

myContext = theContext;
myBulkExportHelperSvc = theBulkExportHelperSvc;
myStorageSettings = theStorageSettings;
Expand All @@ -109,7 +109,7 @@ public JpaBulkExportProcessor(
myEntityManager = theEntityManager;
myHapiTransactionService = theHapiTransactionService;
mySearchParamRegistry = theSearchParamRegistry;
myMdmExpandersHolder = theMdmExpandersHolder;
myMdmLinkExpandSvc = theMdmLinkExpandSvc;
}

@Override
Expand Down Expand Up @@ -345,15 +345,6 @@ protected RuntimeSearchParam getPatientSearchParamForCurrentResourceType(String
return searchParam;
}

@Override
public void expandMdmResources(List<IBaseResource> theResources) {
for (IBaseResource resource : theResources) {
if (!PATIENT_BULK_EXPORT_FORWARD_REFERENCE_RESOURCE_TYPES.contains(resource.fhirType())) {
myMdmExpandersHolder.getBulkExportMDMResourceExpanderInstance().annotateResource(resource);
}
}
}

/**
* For Patient
**/
Expand Down Expand Up @@ -389,7 +380,7 @@ private void validateSearchParametersForGroup(SearchParameterMap expandedSpMap,
/**
* Given the local myGroupId, perform an expansion to retrieve all resource IDs of member patients.
* if myMdmEnabled is set to true, we also attempt to also expand it into matched
* patients.
* patients, assuming MDM is configured.
*
* @return a Set of Strings representing the resource IDs of all members of a group.
*/
Expand All @@ -404,10 +395,14 @@ private LinkedHashSet<JpaPid> getExpandedPatientList(
LinkedHashSet<JpaPid> patientPidsToExport = new LinkedHashSet<>(members);

if (theParameters.isExpandMdm()) {
RequestPartitionId partitionId = theParameters.getPartitionIdOrAllPartitions();
patientPidsToExport.addAll(myMdmExpandersHolder
.getBulkExportMDMResourceExpanderInstance()
.expandGroup(theParameters.getGroupId(), partitionId));
if (myMdmLinkExpandSvc.isPresent()) {
RequestPartitionId partitionId = theParameters.getPartitionIdOrAllPartitions();
IMdmLinkExpandSvc iMdmLinkExpandSvc = myMdmLinkExpandSvc.get();
patientPidsToExport.addAll(iMdmLinkExpandSvc.expandGroup(theParameters.getGroupId(), partitionId));
} else {
ourLog.warn(
"Attempted to perform MDM expansion during a group-level export operation, but no IMdmLinkExpandSvc was configured. Is MDM Configured correctly?");
}
}
return patientPidsToExport;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@
import ca.uhn.fhir.jpa.dao.SearchBuilderFactory;
import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.mdm.svc.MdmExpandersHolder;
import ca.uhn.fhir.mdm.api.IMdmLinkExpandSvc;
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import jakarta.persistence.EntityManager;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import java.util.Optional;

@Configuration
public class JpaBulkExportConfig {
@Bean
Expand All @@ -48,7 +50,7 @@ public IBulkExportProcessor<JpaPid> jpaBulkExportProcessor(
EntityManager theEntityManager,
IHapiTransactionService theHapiTransactionService,
ISearchParamRegistry theSearchParamRegistry,
MdmExpandersHolder theMdmExpandersHolder) {
Optional<IMdmLinkExpandSvc> theMdmLinkExpandSvc) {
return new JpaBulkExportProcessor(
theFhirContext,
theBulkExportHelperService,
Expand All @@ -59,7 +61,7 @@ public IBulkExportProcessor<JpaPid> jpaBulkExportProcessor(
theEntityManager,
theHapiTransactionService,
theSearchParamRegistry,
theMdmExpandersHolder);
theMdmLinkExpandSvc);
}

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,47 +30,23 @@
import ca.uhn.fhir.jpa.entity.MdmLink;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.mdm.api.IMdmLinkExpandSvc;
import ca.uhn.fhir.mdm.api.IMdmSettings;
import ca.uhn.fhir.mdm.dao.IMdmLinkDao;
import ca.uhn.fhir.mdm.dao.IMdmLinkImplFactory;
import ca.uhn.fhir.mdm.svc.BulkExportMdmEidMatchOnlyResourceExpander;
import ca.uhn.fhir.mdm.svc.BulkExportMdmResourceExpander;
import ca.uhn.fhir.mdm.svc.DisabledMdmLinkExpandSvc;
import ca.uhn.fhir.mdm.svc.MdmEidMatchOnlyExpandSvc;
import ca.uhn.fhir.mdm.svc.MdmExpandersHolder;
import ca.uhn.fhir.mdm.svc.MdmExpansionCacheSvc;
import ca.uhn.fhir.mdm.svc.MdmLinkExpandSvc;
import ca.uhn.fhir.mdm.svc.MdmSearchExpansionSvc;
import ca.uhn.fhir.mdm.util.EIDHelper;
import jakarta.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Primary;

@Configuration
public class MdmJpaConfig {

@Bean
public MdmExpandersHolder mdmLinkExpandSvcHolder(
FhirContext theFhirContext,
IMdmLinkExpandSvc theMdmLinkExpandSvc,
MdmEidMatchOnlyExpandSvc theMdmEidMatchOnlyLinkExpandSvc,
BulkExportMdmEidMatchOnlyResourceExpander theBulkExportMdmEidMatchOnlyResourceExpander,
BulkExportMdmResourceExpander theBulkExportMdmResourceExpander) {
return new MdmExpandersHolder(
theFhirContext,
theMdmLinkExpandSvc,
theMdmEidMatchOnlyLinkExpandSvc,
theBulkExportMdmResourceExpander,
theBulkExportMdmEidMatchOnlyResourceExpander);
}

@Bean
public MdmEidMatchOnlyExpandSvc mdmEidMatchOnlyLinkExpandSvc(DaoRegistry theDaoRegistry) {
return new MdmEidMatchOnlyExpandSvc(theDaoRegistry);
}

@Bean
@Primary
public IMdmLinkExpandSvc mdmLinkExpandSvc() {
return new MdmLinkExpandSvc();
}
private static final Logger ourLog = LoggerFactory.getLogger(MdmJpaConfig.class);

@Bean
public MdmSearchExpansionSvc mdmSearchExpansionSvc() {
Expand All @@ -83,33 +59,41 @@ public IMdmLinkDao<JpaPid, MdmLink> mdmLinkDao() {
}

@Bean
public BulkExportMdmResourceExpander bulkExportMDMResourceExpander(
MdmExpansionCacheSvc theMdmExpansionCacheSvc,
IMdmLinkDao theMdmLinkDao,
IIdHelperService<JpaPid> theIdHelperService,
DaoRegistry theDaoRegistry,
FhirContext theFhirContext) {
return new BulkExportMdmResourceExpander(
theMdmExpansionCacheSvc, theMdmLinkDao, theIdHelperService, theDaoRegistry, theFhirContext);
public IMdmLinkImplFactory<MdmLink> mdmLinkImplFactory() {
return new JpaMdmLinkImplFactory();
}

/**
* Based on the rules laid out in the {@link IMdmSettings} file, construct an {@link IMdmLinkExpandSvc} that is suitable
*/
@Bean
public BulkExportMdmEidMatchOnlyResourceExpander bulkExportMDMEidMatchOnlyResourceExpander(
public IMdmLinkExpandSvc mdmLinkExpandSvc(
EIDHelper theEidHelper,
@Nullable IMdmSettings theMdmSettings,
DaoRegistry theDaoRegistry,
MdmEidMatchOnlyExpandSvc theMdmEidMatchOnlyLinkExpandSvc,
FhirContext theFhirContext,
IIdHelperService<JpaPid> theIdHelperService) {
return new BulkExportMdmEidMatchOnlyResourceExpander(
theDaoRegistry, theMdmEidMatchOnlyLinkExpandSvc, theFhirContext, theIdHelperService);
IIdHelperService theIdHelperService) {
if (theMdmSettings == null) {
return new DisabledMdmLinkExpandSvc();
}
if (theMdmSettings.supportsLinkBasedExpansion()) {
return new MdmLinkExpandSvc();
} else if (theMdmSettings.supportsEidBasedExpansion()) {
return new MdmEidMatchOnlyExpandSvc(theDaoRegistry, theFhirContext, theIdHelperService, theEidHelper);
}
return new DisabledMdmLinkExpandSvc();
}

@Bean
public IMdmLinkImplFactory<MdmLink> mdmLinkImplFactory() {
return new JpaMdmLinkImplFactory();
EIDHelper eidHelper(FhirContext theFhirContext, @Nullable IMdmSettings theMdmSettings) {
if (theMdmSettings == null) {
ourLog.warn("Loading up an EID helper without an IMdmSetting bean defined! MDM will _not_ work!");
}
return new EIDHelper(theFhirContext, theMdmSettings);
}

@Bean
public IMdmClearHelperSvc<JpaPid> helperSvc(IDeleteExpungeSvc<JpaPid> theDeleteExpungeSvc) {
public IMdmClearHelperSvc<JpaPid> mdmClearHelperSvc(IDeleteExpungeSvc<JpaPid> theDeleteExpungeSvc) {
return new MdmClearHelperSvcImpl(theDeleteExpungeSvc);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.api.svc.IIdHelperService;
import ca.uhn.fhir.mdm.svc.IBulkExportMdmResourceExpander;
import ca.uhn.fhir.mdm.api.IMdmLink;
import ca.uhn.fhir.mdm.api.IMdmLinkExpandSvc;
import ca.uhn.fhir.jpa.bulk.export.model.ExportPIDIteratorParameters;
import ca.uhn.fhir.jpa.dao.IResultIterator;
import ca.uhn.fhir.jpa.dao.ISearchBuilder;
Expand All @@ -19,7 +20,6 @@
import ca.uhn.fhir.jpa.model.search.SearchBuilderLoadIncludesParameters;
import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.mdm.svc.MdmExpandersHolder;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
Expand Down Expand Up @@ -52,6 +52,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -131,10 +132,10 @@ public JpaPid next() {
private IIdHelperService<JpaPid> myIdHelperService;

@Mock
private MdmExpandersHolder myMdmExpandersHolder;
private Optional<IMdmLinkExpandSvc> myOptionalMdmLinkExpanderService;

@Mock
private IBulkExportMdmResourceExpander myBulkExportMDMResourceExpander;
private IMdmLinkExpandSvc myMdmLinkExpanderService;

@Mock
private ISearchParamRegistry mySearchParamRegistry;
Expand Down Expand Up @@ -299,9 +300,10 @@ public void getResourcePidIterator_groupExportStyleWithPatientResource_returnsIt
final JpaPid mdmExpandedPatientId = JpaPid.fromId(4567L);
if (theMdm) {

when(myMdmExpandersHolder.getBulkExportMDMResourceExpanderInstance()).thenReturn(myBulkExportMDMResourceExpander);
// mock the call to expandGroup method of the expander
when(myBulkExportMDMResourceExpander.expandGroup(parameters.getGroupId(), getPartitionIdFromParams(thePartitioned)))
when(myOptionalMdmLinkExpanderService.isPresent()).thenReturn(true);
when(myOptionalMdmLinkExpanderService.get()).thenReturn(myMdmLinkExpanderService);
when(myMdmLinkExpanderService.expandGroup(parameters.getGroupId(), getPartitionIdFromParams(thePartitioned)))
.thenReturn(Set.of(mdmExpandedPatientId));
}

Expand Down Expand Up @@ -395,9 +397,10 @@ public void getResourcePidIterator_groupExportStyleWithNonPatientResource_return
.thenReturn(observationResultsIterator);

if (theMdm) {
when(myMdmExpandersHolder.getBulkExportMDMResourceExpanderInstance()).thenReturn(myBulkExportMDMResourceExpander);
// mock the call to expandGroup method of the expander
when(myBulkExportMDMResourceExpander.expandGroup(parameters.getGroupId(), getPartitionIdFromParams(thePartitioned)))
when(myOptionalMdmLinkExpanderService.isPresent()).thenReturn(true);
when(myOptionalMdmLinkExpanderService.get()).thenReturn(myMdmLinkExpanderService);
when(myMdmLinkExpanderService.expandGroup(parameters.getGroupId(), getPartitionIdFromParams(thePartitioned)))
.thenReturn(Collections.emptySet());
}

Expand Down
Loading
Loading