Implement a solid and reusable RepositoryProxyFactory pattern for Repository proxies.#963
Draft
lukedegruchy wants to merge 1 commit intomasterfrom
Draft
Implement a solid and reusable RepositoryProxyFactory pattern for Repository proxies.#963lukedegruchy wants to merge 1 commit intomasterfrom
lukedegruchy wants to merge 1 commit intomasterfrom
Conversation
…ries.proxy() Processors (ActivityDefinition, PlanDefinition, Library, CQL, Questionnaire, QuestionnaireResponse, GraphDefinition, Measure/CareGaps) previously called Repositories.proxy() and createRestRepository() directly, coupling them to the static helper and making proxy behaviour untestable in isolation. This change: - Adds a RepositoryProxyFactory interface in cqf-fhir-utility with a DefaultRepositoryProxyFactory that delegates to Repositories.proxy() - Injects RepositoryProxyFactory into all processors, replacing direct static calls to Repositories.proxy() and createRestRepository() - Removes telescope constructors from processors in favour of a single constructor that requires the factory - Adds withRepository() to R4MeasureProcessor and R4MeasureServiceUtils so R4MultiMeasureService can unconditionally call proxy() and avoid duplicated null-checking logic - Introduces NoOpRepositoryProxyFactory for tests that don't exercise proxy routing, replacing DefaultRepositoryProxyFactory in all test files except R4MultiMeasureServiceProxyTest - Adds R4MultiMeasureServiceProxyTest and TestRepositoryProxyFactory to verify proxy routing with custom endpoint repositories - Wires the factory through HAPI Spring configs (CrBaseConfig, CrProcessorConfig, CrR4Config) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Formatting check succeeded! |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
This MR introduces a
RepositoryProxyFactoryabstraction to decouple all clinical reasoning processors from the staticRepositories.proxy()/Repositories.createRestRepository()helpers. Previously, processors (ActivityDefinition, PlanDefinition, Library, CQL, Questionnaire, QuestionnaireResponse, GraphDefinition, Measure, CareGaps) hard-coded calls to these static methods, making it impossible to substitute proxy behaviour in tests without standing up live FHIR servers. The factory is injected via constructors throughout the processor hierarchy and wired as a Spring bean in the HAPI config layer.RepositoryProxyFactoryinterface andDefaultRepositoryProxyFactoryincqf-fhir-utility— the interface accepts raw endpoint resources (IBaseResource) rather than pre-resolvedIRepositoryinstances, so processors no longer need to callcreateRestRepository()themselves.new Processor(repo)->new Processor(repo, settings)->new Processor(repo, settings, ops)) are removed in favour of a single constructor that requires the factory, enforced withrequireNonNull.proxy()+withRepository()on cached processor/utils instances.withRepository()returnsthiswhen the repository is the same instance, avoiding unnecessary object creation.NoOpRepositoryProxyFactory(always returns the local repo) replacesDefaultRepositoryProxyFactoryin all tests that don't exercise proxy routing.R4MultiMeasureServiceProxyTestwithTestRepositoryProxyFactoryvalidates that endpoint-based routing actually works by splitting resources acrossInMemoryFhirRepositoryinstances.Code Review Suggestions
RepositoryProxyFactoryas a mandatory parameter, and the 1-arg and 2-arg telescope constructors are removed. Verify that no downstream consumers (HAPI FHIR starter, Android SDK, other integrators) rely on the removed constructors — the compiler will catch direct callers, but reflection-based or Spring XML-based instantiation will fail silently at runtime.ApplyRequestBuilderremovedwithDataRepository(),withContentRepository(),withTerminologyRepository()setters — these acceptedIRepositorydirectly and are replaced by endpoint-based wiring. Check whether any HAPI provider or external code calls these removed methods.R4MultiMeasureServiceoriginal proxy condition used&&(all three non-null) while the new code callsproxy()unconditionally — verify this is the intended semantic change. The old code only proxied when all endpoints were non-null; the new code delegates to the factory regardless, which handles partial endpoints correctly viaRepositories.proxy().QuestionnaireResponseProcessoris now constructed insideApplyProcessorwithnew QuestionnaireResponseProcessor(repo, crSettings, null, repositoryProxyFactory)— previously it used a no-arg-ish constructor. ConfirmCrSettingspropagation here is correct (it receives the PlanDefinition processor's settings, not QuestionnaireResponse-specific settings).repositoryProxyFactory()inCrBaseConfigis not annotated with@ConditionalOnMissingBean— if a downstream project (like Smile CDR) defines its ownRepositoryProxyFactorybean, it will conflict. Verify whether this should be conditional.QA Test Suggestions
Setup
cqf-fhir-cr-hapimodule loadedTest Cases
Measure/$evaluate-measurewith nodataEndpoint,contentEndpoint, orterminologyEndpointparameters. Verify the measure evaluates successfully using the local server's data — this exercises the defaultRepositoryProxyFactorypath.$evaluate-measurewith aterminologyEndpointpointing to it. Verify that ValueSet expansion comes from the remote server.PlanDefinition/$applywith and without endpoint parameters. Verify the CarePlan/RequestGroup is returned correctly in both cases.ActivityDefinition/$applywith and without endpoint parameters. Verify the generated resource (e.g. MedicationRequest, ServiceRequest) is correct.Questionnaire/$populatewith a subject. Verify pre-population works against local data.$extract. Verify the extracted resources are correct — this validates theApplyProcessor->QuestionnaireResponseProcessorconstruction chain.Measure/$care-gapswith required parameters. Verify the care gaps bundle is returned with correct composition and detected issues.RepositoryProxyFactory.