generic merge draft #7418
Draft
generic merge draft #7418
Conversation
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
Implements ExtensionBasedLinkService to manage merge relationships using FHIR extensions instead of native resource fields. This enables tracking "replaces" and "replaced-by" links on any resource type in a version-agnostic manner, supporting the generic merge implementation design. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…mentation Introduces IResourceLinkService interface to abstract resource merge link management: - Provides common interface for tracking "replaces" and "replaced-by" relationships - Implemented by ExtensionBasedLinkService (version-agnostic, works with any resource) - Implemented by PatientNativeLinkService (R4-specific, uses native Patient.link field) PatientNativeLinkService uses FHIR R4 Patient.LinkType.REPLACES and Patient.LinkType.REPLACEDBY to manage merge links via the standard Patient.link field instead of extensions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Created factory class to route between PatientNativeLinkService and ExtensionBasedLinkService based on resource type. Uses case-insensitive comparison for resource type matching. Factory provides: - getServiceForResourceType(String): routes based on resource type string - getServiceForResource(IBaseResource): convenience method using resource instance Test coverage includes parameterized tests for: - Patient resource type variations (case-insensitive) - Non-Patient resource types (Observation, Practitioner, Organization) - Resource instance routing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ubbing Changes: - Inject ResourceLinkServiceFactory into ResourceMergeService and MergeValidationService - Remove unused FhirContext field from PatientNativeLinkService - Add Spring beans for PatientNativeLinkService, ExtensionBasedLinkService, and ResourceLinkServiceFactory in JpaR4Config - Update all test files to use no-arg constructor for PatientNativeLinkService - Fix Mockito UnnecessaryStubbingException in ResourceMergeServiceTest by using lenient() for mocks that are not used in all test paths All 70 tests in ResourceMergeServiceTest now pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changes: - Refactored MergeValidationService to use IResourceLinkService methods instead of directly accessing Patient.link fields - Chunk 1: Replaced replaced-by link validation to use linkService.getReplacedByLinks() - Chunk 2: Replaced replaces link validation to use private helper getReplacesLinksTo() - Removed getReplacesLinksTo() from IResourceLinkService interface (only needed in one place) - Removed duplicate implementations from PatientNativeLinkService and ExtensionBasedLinkService - Added private helper method getReplacesLinksTo() in MergeValidationService - Removed obsolete helper methods: getLinksToResource() and getLinksOfTypeWithNonNullReference() - Removed unused import java.util.stream.Collectors This abstraction prepares the codebase for generic resource merge support beyond Patient resources. All 70 ResourceMergeServiceTest tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Move link service classes and fix bean configuration to align with established batch2 job architecture patterns. Changes: - Move IResourceLinkService, ResourceLinkServiceFactory, PatientNativeLinkService, and ExtensionBasedLinkService from hapi-fhir-jpaserver-base to hapi-fhir-storage-batch2-jobs - Move corresponding test files to batch2-jobs module - Add MergeResourceHelper bean to JpaR4Config (shared service) - Remove duplicate MergeProvenanceSvc from MergeAppCtx - Remove MergeResourceHelper from MergeAppCtx (now in JpaR4Config) - Update MergeResourceHelper to use IResourceLinkService abstraction - Update imports in MergeValidationService and ResourceMergeService Architecture improvements: - MergeAppCtx now contains ONLY batch job-specific beans (job definition, steps, error handlers) following the pattern of BulkExport, Reindex, and DeleteExpunge batch jobs - JpaR4Config contains shared services used by both REST providers and batch jobs - Eliminates duplicate bean definitions - Resolves cross-module dependency issues Verified with 70/70 passing tests in ResourceMergeServiceTest. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…hirTerser Changed copyIdentifiersAndMarkOld method to work with IBaseResource instead of Patient-specific types, enabling identifier copying for any FHIR resource type that supports identifiers. - Add FhirContext and FhirTerser member variables to MergeResourceHelper - Initialize FhirContext from DaoRegistry in constructor for generic operations - Replace Patient.getIdentifier() calls with FhirTerser.getValues() for path-based access - Use FhirTerser.addElement() and cloneInto() for generic identifier cloning - Replace equalsDeep() with TerserUtil.equals() for generic deep equality checks - Update method signature to accept IBaseResource parameters - Gracefully handle resources without identifier elements (returns early) - Fix ResourceMergeServiceTest to mock DaoRegistry.getFhirContext() - Add test case for Bundle merge with identifiers in GenericMergeR4Test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…h FhirTerser Replace Patient-specific setActive(false) call with generic FhirTerser-based implementation that works with any resource type. The method prepareSourcePatientForUpdate is renamed to prepareSourceResourceForUpdate and now accepts IBaseResource parameters. This enables merge operations for resources beyond Patient (Practitioner, Organization, etc.) while gracefully handling resources without an active field (like Observation). All 121 tests passing (ResourceMergeServiceTest, PatientMergeR4Test, MergeBatchTest). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Rename prepareTargetPatientForUpdate to prepareTargetResourceForUpdate and change parameter types from Patient to IBaseResource. This enables generic merge operations for any resource type (Practitioner, Organization, Observation, etc.) while maintaining backward compatibility with existing Patient merge functionality. The method now: - Accepts IBaseResource parameters instead of Patient-specific types - Returns IBaseResource with appropriate casting at call sites - Uses existing generic infrastructure (IResourceLinkService, FhirTerser) - Works seamlessly with resources that lack identifier fields All 121 tests passing (ResourceMergeServiceTest, PatientMergeR4Test, MergeBatchTest). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…DAO lookup for generic resource types Replace static Patient-specific DAO with dynamic DaoRegistry lookup to enable merging of any resource type (Observation, Practitioner, Organization, etc.). Changes: - MergeResourceHelper.updateMergedResourcesAfterReferencesReplaced: Changed method signature from Patient to IBaseResource parameters - Removed myPatientDao field and replaced with myDaoRegistry.getResourceDao(resourceType) calls - Used IFhirResourceDao<IBaseResource> pattern for type-safe generic DAO operations - Updated MergeUpdateTaskReducerStep to work with IBaseResource types - Removed unnecessary Patient casts in ResourceMergeService preview and sync merge paths All 121 tests passing: ResourceMergeServiceTest (70), MergeBatchTest (5), PatientMergeR4Test (46) Full backward compatibility maintained for Patient merge operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… for generic resource validation Replace Patient-specific active field check with FhirTerser-based generic pattern to enable validation of any FHIR resource type with an active field. Changes: - Added FhirTerser field and initialized in constructor - Updated validateSourceAndTargetAreSuitableForMerge signature to accept IBaseResource parameters - Replaced theTargetResource.hasActive() && !theTargetResource.getActive() with FhirTerser.getValues() pattern - Fixed ID comparison to use getIdElement().getValue() instead of getId() - Removed Patient casts in validate() method for source and target resource variables - Maintained temporary casts for validateResultResourceIfExists and MergeValidationResult.validResult (not yet genericized) All 70 tests in ResourceMergeServiceTest passing. Full backward compatibility maintained for Patient merge operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change hasAllIdentifiers() parameter from Patient to IBaseResource - Use FhirTerser.getValues() for version-agnostic identifier access - Convert identifiers to CanonicalIdentifier using fromIdentifier() - Simplify comparison logic using containsAll() instead of manual loop - Remove unused import for org.hl7.fhir.r4.model.Identifier - Add import for java.util.stream.Collectors This change allows identifier validation to work with any FHIR resource type (Patient, Practitioner, Organization, etc.) across FHIR versions (DSTU3, R4, R5). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changes: - MergeValidationResult: Change field types from Patient to IBaseResource - MergeValidationResult: Update constructor and validResult() factory method - MergeValidationResult: Replace Patient import with IBaseResource import - MergeValidationService: Remove Patient casts in validResult() call (line 108) - ResourceMergeService: Add TODO comments for remaining Patient casts (lines 152-154) This change allows MergeValidationResult to hold any FHIR resource type (Patient, Practitioner, Organization, etc.), advancing the genericization of the merge operation infrastructure. Note: Some Patient casts remain temporarily in MergeValidationService (line 105) and ResourceMergeService (lines 152-154) as downstream methods still expect Patient. These will be removed in future commits as those methods are refactored. Tests: All 121 tests pass - ResourceMergeServiceTest: 70/70 ✅ - PatientMergeR4Test: 46/46 ✅ - MergeBatchTest: 5/5 ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… Patient This change genericizes the merge operation infrastructure to support any FHIR resource type (Patient, Practitioner, Organization, Observation, etc.), not just Patient resources. Changes in MergeOperationInputParameters.java: - Update asMergeJobParameters() method signature to accept IBaseResource parameters instead of Patient (lines 104-105) - Remove unused Patient import Changes in ResourceMergeService.java: - Replace myPatientDao field with myDaoRegistry field for dynamic DAO lookup - Update constructor to store DaoRegistry and get FhirContext from registry - Remove Patient casts in validateAndMerge() (lines 152-153) - Update all method signatures to use IBaseResource: * handlePreview() - lines 176-177 * doMerge() - lines 199-200 * doMergeSync() - lines 228-229 * doMergeAsync() - lines 299-300 - Implement dynamic DAO lookup for delete operation (lines 287-289): IFhirResourceDao<IBaseResource> dao = myDaoRegistry.getResourceDao(theSourceResource.fhirType()); dao.delete(theSourceResource.getIdElement(), theRequestDetails); - Remove unused Patient import Changes in ResourceMergeServiceTest.java: - Remove unnecessary mock stubs for myPatientDao field that was removed - Fix UnnecessaryStubbingException errors by removing: * when(myDaoRegistryMock.getResourceDao(Patient.class)).thenReturn(myPatientDaoMock) * when(myPatientDaoMock.getContext()).thenReturn(myFhirContext) Pattern: Dynamic DAO lookup using myDaoRegistry.getResourceDao(theSourceResource.fhirType()) matches the established pattern in MergeResourceHelper, ensuring consistency. Dependencies: All downstream components already support IBaseResource: - MergeResourceHelper methods already accept IBaseResource - IReplaceReferencesSvc works with generic IIdType - MergeValidationResult now stores IBaseResource (previous commit) Tests: All 121 tests pass - ResourceMergeServiceTest: 70/70 ✅ - PatientMergeR4Test: 46/46 ✅ - MergeBatchTest: 5/5 ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replace hardcoded parameter name strings with constants from ProviderConstants to maintain consistency with Patient/$merge implementation and provide single source of truth for parameter definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…fic parameter names Refactored the merge operation parameter naming system to support different parameter names for different merge operations at runtime: - Patient-specific operations ($merge, $hapi.fhir.undo-merge): use "source-patient", "target-patient", "result-patient" parameters - Generic operations ($hapi-fhir-merge): use "source-resource", "target-resource", "result-resource" parameters Changes: - Converted MergeOperationInputParameterNames from concrete class to abstract base class with factory method forOperation() - Created PatientMergeOperationInputParameterNames subclass for Patient-specific operations - Created GenericMergeOperationInputParameterNames subclass for generic operations - Added operationName field to MergeJobParameters for async batch job support with backward compatibility (null defaults to "$merge") - Updated MergeValidationService to use local parameterNames variable and removed fallback logic - Updated MergeUpdateTaskReducerStep to use local parameterNames variable with backward compatibility for in-flight jobs - Updated ResourceMergeService to extract operation name from RequestDetails and pass to job parameters - Updated ResourceUndoMergeService and MergeProvenanceSvc to use PatientMergeOperationInputParameterNames - Added OPERATION_UNDO_MERGE constant to ProviderConstants The factory method defaults to PatientMergeOperationInputParameterNames when operation name is null, ensuring backward compatibility with existing jobs and tests. All existing tests pass (70/70 in ResourceMergeServiceTest, 215/215 in hapi-fhir-storage-batch2-jobs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… findProvenanceByTargetIdAndSourceIdentifiers Refactored MergeProvenanceSvc to accept operation-specific parameter names as a method parameter instead of using an instance field, making the service operation-aware and able to support both Patient-specific and generic merge operations. Changes: - Removed myInputParamNames instance field from MergeProvenanceSvc - Updated findProvenanceByTargetIdAndSourceIdentifiers() to accept MergeOperationInputParameterNames as a parameter - Updated ResourceUndoMergeService to pass myInputParamNames when calling findProvenanceByTargetIdAndSourceIdentifiers() This change allows MergeProvenanceSvc to work with different merge operations that use different parameter naming conventions (e.g., "source-patient-identifier" for Patient merge vs "source-resource-identifier" for generic merge). All tests pass: - ResourceMergeServiceTest: 70/70 tests passing - PatientMergeR4Test: 46/46 tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… merge operations Fixed ClassCastException when using generic merge operation ($hapi-fhir-merge) with non-Patient resources like Practitioner. The validation methods were hardcoded to use Patient types, preventing the generic merge from working with other resource types. Changes: - Changed validateResultResourceIfExists() parameters from Patient to IBaseResource - Changed validateResultResourceReplacesLinkToSourceResource() parameters from Patient to IBaseResource - Removed Patient casts when calling validateResultResourceIfExists() - Removed Patient cast when getting result resource - Removed unused Patient import The underlying helper methods (getReplacesLinksTo, FhirTerser, IResourceLinkService) were already generic, so they work correctly with any resource type. All tests pass: - GenericMergeR4Test: 3/3 tests passing (including new Practitioner merge test) - PatientMergeR4Test: 46/46 tests passing (backward compatibility) - ResourceMergeServiceTest: 70/70 tests passing Fixes issue where testGenericMergeEndpoint_ReturnsOk was failing with: "class org.hl7.fhir.r4.model.Practitioner cannot be cast to class org.hl7.fhir.r4.model.Patient" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changed testGenericMergeEndpoint_ReturnsOk to use string literals "source-resource" and "target-resource" instead of the corresponding ProviderConstants for better readability in the test.
… for async merge operations - Remove Patient-specific import and myPatientDao field - Change all Patient types to IBaseResource throughout the class - Implement dynamic DAO lookup using mergeJobParameters.getSourceId().getResourceType() - Fix backward compatibility parsing to use dynamic resource class from FhirContext - Remove unnecessary cast in getResultResource() method - Extract resourceType calculation to method start to avoid duplication This enables async merge operations to work with any FHIR resource type (Practitioner, Observation, Organization, etc.) instead of being limited to Patient resources. The changes maintain full backward compatibility with existing Patient merge operations. Fixes ClassCastException: "Practitioner cannot be cast to Patient" in async merge jobs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… checking Replace exception-based control flow with explicit field existence checks when accessing the 'active' field during resource merge operations. This improves code clarity and performance by avoiding DataFormatException overhead for resource types that don't have an 'active' field (e.g., Observation). Changes: - MergeValidationService: Use fieldExists() + getSingleValueOrNull() instead of try-catch when validating target resource active status - MergeResourceHelper: Use fieldExists() + getSingleValueOrNull() + setValueAsString() instead of try-catch when setting source resource active=false This refactoring maintains identical behavior while following HAPI FHIR's idiomatic pattern for optional field access across different resource types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ations Implement reusable test framework and comprehensive test suites for validating merge operations across different FHIR resource types, including resources with and without 'active' field support. Test Infrastructure (hapi-fhir-jpaserver-test-utilities): - ResourceMergeTestStrategy: Interface defining resource-specific merge test behavior with methods for creating resources, managing identifiers, handling references, and validating merge outcomes - ObservationMergeTestStrategy: Implementation for Observation resources (no 'active' field, uses extension-based links) - PractitionerMergeTestStrategy: Implementation for Practitioner resources (has 'active' field, uses extension-based links) - MergeTestDataBuilder: Builder pattern for constructing test scenarios with configurable identifiers, references, and result resources - MergeTestScenario: Encapsulates test data including source/target resources, referencing resources, and expected merge outcomes - MergeTestParameters: Represents merge operation parameters (source, target, preview, delete-source, result-resource) - MergeOperationTestHelper: Helper methods for invoking merge operations and asserting expected outcomes - ReferencingResourceConfig: Configuration for creating referencing resources in tests Test Classes (hapi-fhir-jpaserver-test-r4): - ObservationMergeR4Test: 46 comprehensive tests validating Observation merge operations including basic matrix tests, identifier resolution, extension links, provenance creation, status preservation, and DiagnosticReport reference updates - PractitionerMergeR4Test: 42 comprehensive tests validating Practitioner merge operations including basic matrix tests, identifier resolution, extension links, provenance creation, active field handling, and PractitionerRole/Encounter/CarePlan reference updates Key Features: - Strategy pattern enables easy addition of new resource types - Parameterized matrix tests cover all merge operation combinations - Validates both native links (Patient) and extension-based links (Practitioner, Observation) - Tests async and sync merge execution paths - Validates preview mode and delete-source behavior - Tests reference rewriting across multiple resource types - Validates provenance creation for audit trail Test Results: - ObservationMergeR4Test: 46/46 passing - PractitionerMergeR4Test: 42/42 passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…dMergeParameters() - Remove myResultResource field from AbstractMergeTestScenario - Remove getResultResource() from MergeTestScenario interface - Remove cloneResource() method (no longer needed) - Create result resource on-demand in buildMergeParameters() methods - Result resource now created only when needed, not stored as test data - Eliminates unnecessary cloning and state management Benefits: - createTestData() only handles persisted resources (cleaner separation) - buildMergeParameters() responsible for operation parameters - No need to clone result resource (created fresh each time) - Simpler state management in test scenario Tests: All 61 tests passing (PractitionerMergeR4Test: 16, ObservationMergeR4Test: 45)
…st infrastructure In test code, use string literals instead of production constants to reduce coupling and improve test readability. Changes: - MergeOperationTestHelper.java: Replace JpaConstants.OPERATION_HAPI_FHIR_MERGE with "-fhir-merge" and ProviderConstants output param constants with "outcome" and "task" - MergeTestParameters.java: Replace ProviderConstants input param constants with "preview", "delete-source", and "batch-size" - GenericMergeR4Test.java: Replace ProviderConstants identifier param constants with "source-resource-identifier" and "target-resource-identifier" - AbstractGenericMergeR4Test.java: Replace OPERATION_MERGE_OUTPUT_PARAM_TASK with "task" - Remove all unused production constant imports from test files Tests: All 64 tests passing (Practitioner: 16, Observation: 45, Generic: 3)
…simplify identifier extraction This commit improves separation of concerns in the merge test infrastructure: 1. **Validation methods moved to scenarios**: All validation methods (validateSyncMergeOutcome, validateAsyncTaskCreated, validatePreviewOutcome, assertReferencesUpdated, assertReferencesNotUpdated, assertMergeProvenanceCreated) moved from MergeOperationTestHelper to AbstractMergeTestScenario. This creates clearer responsibility boundaries: - MergeOperationTestHelper: pure operation invocation (callMergeOperation, awaitJobCompletion, getJobIdFromTask) - MergeTestScenario: all validation + test data + identifier extraction 2. **Identifier extraction simplified**: Updated getIdentifiersFromResource() in AbstractMergeTestScenario to use FhirTerser instead of reflection. This is more robust and works generically for any FHIR resource type. Removed redundant overrides from PractitionerMergeTestScenario and ObservationMergeTestScenario. 3. **Removed hasIdentifierField() capability detection**: No longer needed since FhirTerser-based extraction works for all resources with identifier fields. Removed from interface and implementations. 4. **Simplified constructor**: MergeOperationTestHelper constructor now takes only IGenericClient and Batch2JobHelper (removed FhirContext and DaoRegistry which were only used for validation). All 90 tests pass (42 Practitioner + 45 Observation + 3 Generic). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…parameters This commit simplifies validation method APIs by removing parameters that the scenario already has access to internally: 1. **validatePreviewOutcome(Parameters)**: Removed expectedUpdateCount parameter. The method now calculates it internally as getTotalReferenceCount() + 2. 2. **assertReferencesUpdated(List<IIdType>)**: Removed sourceId and targetId parameters. The method now retrieves them internally from the scenario. **New overload added**: assertReferencesUpdated(String resourceType) - Convenience method that retrieves referencing resource IDs by type. 3. **assertMergeProvenanceCreated(Parameters)**: Removed sourceId and targetId parameters. The method now retrieves them internally from the scenario. **Benefits:** - Cleaner test code: Reduces boilerplate at all 8 call sites - Less error-prone: Cannot accidentally pass wrong source/target IDs - Better encapsulation: Scenario manages its own state - More intuitive API: Method signatures show what varies vs what's fixed **Example transformation:** Before: scenario.assertReferencesUpdated(scenario.getReferencingResourceIds(resourceType), scenario.getSourceId(), scenario.getTargetId()) After: scenario.assertReferencesUpdated(resourceType) All 90 tests pass (42 Practitioner + 45 Observation + 3 Generic). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Simplified the provenance agent interceptor test by moving validation logic into the scenario's validateResourcesAfterMerge() method. Changes: - Add withExpectedProvenanceAgents() builder method to scenario - Store expected agents as member field in AbstractMergeTestScenario - Update assertMergeProvenanceCreated() to use stored agents - Simplify test method by removing ~30 lines of duplicated logic This improves maintainability by keeping provenance validation calculations in one place and ensures all tests use consistent validation paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements test validation for error handling when Provenance Agent interceptor returns empty list. Ensures system fails fast with InternalErrorException (500) when interceptor contract is violated. Test coverage: - 2 parametrized tests (sync/async) - Validates error message: "HAPI-2723: No Provenance Agent was provided" - Generic for all resource types (Practitioner, Observation) Changes: - Add testMerge_withProvenanceAgentInterceptor_InterceptorReturnsNoAgent_ReturnsInternalError test - Add imports for InternalErrorException, BaseServerResponseException - Rename createTestData() to persistTestData() (spotless formatting) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Refactor GAP #3 test to use a cleaner API by adding an overloaded withMultipleReferencingResources(int count) method to scenario classes. Changes: - Add abstract method to AbstractMergeTestScenario - Implement in PractitionerMergeTestScenario with distribution strategy: * count >= 3: 1 PractitionerRole, 1 Encounter, (count-2) CarePlan * count == 2: 1 PractitionerRole, 1 Encounter * count == 1: 1 PractitionerRole * Added Validate.isTrue(count > 0) assertion - Implement in ObservationMergeTestScenario: all DiagnosticReport - Update testMerge_smallResourceLimit to use new method - Update assertion to match PatientMergeR4Test pattern using myReplaceReferencesTestHelper.extractFailureMessageFromOutcomeParameter() - Remove unused ReferencingTestResourceType import 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add zero-reference guard clause to AbstractMergeTestScenario.validateTaskOutput() - When getTotalReferenceCount() == 0, validate Task.output and Task.contained are empty - Tests can now call validateTaskOutput() unconditionally without skip logic - Add GAP #4 test: testMerge_sourceNotReferencedByAnyResource for generic merge tests - Tests both sync and async merge when source has no referencing resources - Validates merge succeeds, provenance created, links established even with zero references 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add testMerge_errorHandling_multipleTargetMatches() to AbstractGenericMergeR4Test to validate error handling when multiple resources match the target identifier during merge operation resolution. Test behavior: - Creates a duplicate resource with same identifiers as target - Uses identifier-based target resolution (sourceById=true, targetById=false) - Validates UnprocessableEntityException with exact error message: "Multiple resources found matching the identifier(s) specified in 'target-resource-identifier'" This test ensures the merge operation fails safely when target identifier resolution is ambiguous, preventing incorrect merges. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implemented testMerge_errorHandling_multipleSourceMatches() to validate error handling when multiple resources match the source identifier during merge operation resolution. Test creates a duplicate resource with the same identifiers as the source, then triggers identifier-based source resolution which finds both resources, resulting in UnprocessableEntityException with message: "Multiple resources found matching the identifier(s) specified in 'source-resource-identifier'" This mirrors GAP #6 implementation but tests source ambiguity instead of target ambiguity (sourceById=false, targetById=true). Tests verified for both PractitionerMergeR4Test and ObservationMergeR4Test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements test that validates race condition handling when a new resource referencing the source is added mid-job during async merge with deleteSource=true. The test ensures the job fails safely rather than leaving orphaned references, validating transaction isolation in multi-user environments. Changes: - Add createReferencingResource() abstract method to AbstractMergeTestScenario - Implement method in PractitionerMergeTestScenario (returns PractitionerRole) - Implement method in ObservationMergeTestScenario (returns DiagnosticReport) - Add testMerge_concurrentModificationDuringAsyncJob_JobFails() to AbstractGenericMergeR4Test - Test uses small batch size to slow job execution and create timing window - Validates that Task status becomes FAILED when concurrent modification prevents source deletion 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
In AbstractMergeTestScenario.persistTestData(), referencing resources were being created with versioned source IDs (e.g., Practitioner/123/_history/1) instead of versionless IDs (e.g., Practitioner/123). This fix ensures referencing resources use versionless references, which is the FHIR standard for references unless version-specific references are explicitly needed. Change: Added .toUnqualifiedVersionless() when passing source ID to createReferencingResource() at line 249. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add testMerge_MissingRequiredParameters_Returns400BadRequest to AbstractGenericMergeR4Test to validate that completely empty parameters (missing both source and target) return InvalidRequestException with 400 status code. This brings generic merge tests to parity with PatientMergeR4Test coverage for the empty parameters edge case. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Enhanced three existing test methods in AbstractGenericMergeR4Test to validate the specific error messages returned when merge parameters are missing: - testMerge_errorHandling_missingSourceResource: Validates full error message "There are no source resource parameters provided, include either a 'source-resource', or a 'source-resource-identifier' parameter" - testMerge_errorHandling_missingTargetResource: Validates full error message "There are no target resource parameters provided, include either a 'target-resource', or a 'target-resource-identifier' parameter" - testMerge_MissingRequiredParameters_Returns400BadRequest: Validates that both source and target error messages are present when both parameters are missing Uses ReplaceReferencesTestHelper.extractFailureMessageFromOutcomeParameter to extract diagnostic messages from InvalidRequestException response bodies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Extracted repeated exception validation code into a reusable helper method callMergeAndValidateException to reduce duplication and improve test maintainability. Changes: - Added callMergeAndValidateException() helper method that: * Calls merge operation with given parameters * Validates exception type * Extracts diagnostic message from OperationOutcome * Validates all expected message parts are present - Refactored three test methods to use the new helper: * testMerge_errorHandling_missingSourceResource * testMerge_errorHandling_missingTargetResource * testMerge_MissingRequiredParameters_Returns400BadRequest Benefits: - Eliminates 10+ lines of repeated code per test - Single location for exception validation logic - Clearer test intent with concise helper call - Reusable by other tests needing exception validation All tests pass with identical behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ption helper Refactored three more tests to use the callMergeAndValidateException helper method: - testMerge_errorHandling_multipleTargetMatches - testMerge_errorHandling_multipleSourceMatches - testMerge_errorHandling_previouslyMergedSourceAsSource This reduces code duplication by consolidating the exception validation pattern (catch exception, validate type, extract diagnostic message, validate message contents) into a single reusable helper method. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Created callMergeAndExtractDiagnosticMessage base helper method that: - Catches exceptions from merge operations - Validates exception type - Extracts and returns diagnostic message for custom validation Refactored callMergeAndValidateException to use the new base helper, improving composability and DRY principles. Refactored 5 tests to use helpers with complete message validation: - testMerge_errorHandling_sourceEqualsTarget: validates "Source and target resources are the same resource." - testMerge_errorHandling_nonExistentSourceIdentifier: validates complete "No resources found matching..." message - testMerge_errorHandling_nonExistentTargetIdentifier: validates complete "No resources found matching..." message - testMerge_errorHandling_previouslyMergedSourceAsTarget: uses base helper with complete messages in OR logic - testMerge_smallResourceLimit: validates complete HAPI-2597 message with dynamic sourceId Benefits: - 11 out of 12 tests now use helper methods (92% coverage) - Improved test coverage with complete diagnostic message validation - Base helper enables custom validation patterns (OR logic, exact match, etc.) - Reduced duplication by 30+ lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Move validation orchestration logic from test methods into AbstractMergeTestScenario to reduce code duplication and improve maintainability. This refactoring encapsulates execution and validation logic within the scenario, creating a cleaner separation between test scenarios (happy path) and test classes (error handling). Changes to AbstractMergeTestScenario: - Add myHelper field and update constructor to accept MergeOperationTestHelper - Add myAsync configuration field with withAsync() builder method - Add callMergeOperation() method that executes merge and returns Parameters - Add callMergeOperation(sourceById, targetById) overload for identifier-based resolution - Add validateSuccess(Parameters) method that orchestrates complete validation flow - Delegate no-arg callMergeOperation() to overload for code reuse Changes to scenario implementations: - Update PractitionerMergeTestScenario constructor to accept helper parameter - Update ObservationMergeTestScenario constructor to accept helper parameter Changes to test classes: - Update PractitionerMergeR4Test.createScenario() to pass myHelper - Update ObservationMergeR4Test.createScenario() to pass myHelper - Refactor 15 test methods to use scenario.callMergeOperation() and scenario.validateSuccess() - Remove redundant validation from 4 identifier edge case tests (already covered by validateSuccess) - Add Parameters import to ObservationMergeR4Test Changes to MergeOperationTestHelper: - Remove unused callMergeOperation(AbstractMergeTestScenario<?>, boolean) method Net result: -15 lines of code, improved maintainability, cleaner test API 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Restructure test method order for improved readability: - Move testMerge_sourceNotReferencedByAnyResource closer to identifier tests - Group provenance agent interceptor tests together before error handling - Move helper methods (callMergeAndExtractDiagnosticMessage, callMergeAndValidateException) to end of class No logical changes - pure method reordering for better code organization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Rename error handling test methods to follow consistent naming pattern: - Format: testMerge_<description>_failsWithXXXException - Remove _errorHandling_ prefix from method names - Append exception type suffix (e.g., _failsWithInvalidRequestException) Updated methods: - missingSourceResource - missingTargetResource - missingRequiredParameters - sourceEqualsTarget - nonExistentSourceIdentifier - nonExistentTargetIdentifier - multipleTargetMatches - multipleSourceMatches - previouslyMergedSourceAsSource - previouslyMergedSourceAsTarget - smallResourceLimit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Rename all success test methods to follow consistent naming pattern: - Format: testMerge_<description>_succeeds - Apply to identifier resolution, edge cases, and provenance tests Updated methods: - identifierResolution_sync - identifierResolution_async - sourceNotReferencedByAnyResource - identifiers_emptySourceIdentifiers - identifiers_emptyTargetIdentifiers - identifiers_bothEmpty - resultResource_overridesTargetIdentifiers - withProvenanceAgentInterceptor This complements the error test naming standard (_failsWithXXXException) and makes test intent immediately clear from method names. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Simplify testMerge_withProvenanceAgentInterceptor_InterceptorReturnsNoAgent_failsWithInternalErrorException: - Replace inline assertThatThrownBy with callMergeAndValidateException helper - Remove async parameterization (error occurs before async execution) - Remove unused assertThatThrownBy import - Remove HTTP status code validation (InternalErrorException always returns 500) Benefits: - Consistent with other error handling tests - Simpler, more readable test code - Delegates error validation to centralized helper 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Consolidate testMerge_identifierResolution_sync_succeeds and testMerge_identifierResolution_async_succeeds into a single parameterized test with async parameter. Changes: - Merge two tests into testMerge_identifierResolution_succeeds - Add async as third parameter (sourceById, targetById, async) - Remove redundant (true, true) cases already covered by basicInputParameterVariations - Add Javadoc note explaining the coverage overlap - Test 6 cases instead of 8 (no duplication) Benefits: - Eliminates duplicate test coverage - Simpler test structure (1 method instead of 2) - Clear documentation of what's being tested - Faster test execution 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fix failing testMerge_withProvenanceAgentInterceptor_InterceptorReturnsNoAgent_failsWithInternalErrorException by reverting to inline validation with assertThatThrownBy. Root cause: InternalErrorException (HTTP 500) returns OperationOutcome directly in the response body, not wrapped in Parameters. The callMergeAndValidateException helper expects client errors (4xx) which wrap OperationOutcome in Parameters, causing a parse error. Changes: - Revert test to use inline assertThatThrownBy validation - Re-add assertThatThrownBy import - Add Javadoc note explaining why inline validation is used for 500 errors This acknowledges that server errors (5xx) behave differently from client errors (4xx) and require different validation approaches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This change addresses an edge case where the target resource is not actually updated during a merge operation when deleteSource=true and the source has no identifiers to merge. Changes: - Add parameter theExpectTargetToBeUpdated to assertMergeProvenanceCreated() to allow validation when target version stays at 1 instead of incrementing to 2 - Add overloaded validateResourcesAfterMerge() that accepts the same parameter for end-to-end validation control - Add test case testMerge_emptySourceIdentifiers_deleteSource_targetNoopUpdate_succeeds() that validates target version stays at 1 and Provenance.target references v1 when source has no identifiers to merge The parameter-based approach allows existing tests to continue working with default behavior (expectTargetUpdated=true) while enabling validation of the no-op update edge case. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… by base class - Remove testMerge_observationSpecific_diagnosticReportResult() and testMerge_observationSpecific_allStandardReferences() from ObservationMergeR4Test - These tests duplicate validation already performed by AbstractGenericMergeR4Test.validateSuccess() which internally calls validateResourcesAfterMerge() that iterates all referencing resource types - Remove unused validateAsyncTaskCreated() method from AbstractMergeTestScenario - Aligns ObservationMergeR4Test with PractitionerMergeR4Test pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implement PatientMergeTestScenario and PatientMergeGenericEndpointR4Test
using the modern generic merge test framework, providing comprehensive test
coverage with minimal code.
Changes:
- Add PatientMergeTestScenario in test-utilities module
- Creates Patient resources with active=true and identifiers
- Supports Encounter, Observation, and CarePlan referencing resources
- Validates active field behavior (source set to false on merge)
- Distribution logic: 1 Encounter, 1 Observation, remaining CarePlan
- Add PatientMergeGenericEndpointR4Test in test-r4 module
- Minimal implementation with only 2 required methods
- Inherits 48 comprehensive tests from AbstractGenericMergeR4Test
- Tests all merge scenarios, identifier resolution, edge cases, error
handling, provenance validation, and concurrent modification
All 48 tests pass successfully.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Formatting check succeeded! |
…ciency Store only resource IDs (IIdType) instead of full resource objects in AbstractMergeTestScenario to improve memory efficiency and ensure data freshness. Changes: - Replace mySourceResource/myTargetResource fields with myVersionlessSourceId/myVersionlessTargetId - Rename getSourceResource() -> readSourceResource() to make DB reads explicit - Add getSourceIdentifiers()/getTargetIdentifiers() for direct config access - Update all usages in AbstractGenericMergeR4Test to use new API Benefits: - Memory efficiency: stores only IDs instead of full resource objects - Data freshness: all validations read from database, getting latest data - Clearer intent: method names explicitly indicate when DB reads occur 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
No description provided.