feat: add optional dcm4che5 DICOM backend with runtime selection5 backend#299
Draft
jessesaga wants to merge 3 commits intoOpenIntegrationEngine:mainfrom
Draft
feat: add optional dcm4che5 DICOM backend with runtime selection5 backend#299jessesaga wants to merge 3 commits intoOpenIntegrationEngine:mainfrom
jessesaga wants to merge 3 commits intoOpenIntegrationEngine:mainfrom
Conversation
Add a dcm4che5 backend for the DICOM connector alongside the existing
dcm4che2 backend. Backend selection is controlled at startup by the new
dicom.library property in mirth.properties ("dcm4che2" default, "dcm4che5"
to opt in). Existing channel configurations work unchanged on either
backend.
Architecture:
- A version-neutral abstraction layer (OieDicomObject, OieDicomElement,
OieDicomSender, OieDicomReceiver, OieDicomConverter, OieVR,
DicomLibraryFactory) decouples connector logic from library specifics.
- Dcm2* implementations wrap the existing dcm4che2 MirthDcmSnd/MirthDcmRcv.
- Dcm5* implementations build an equivalent topology from dcm4che5's
Device/Connection/ApplicationEntity primitives with the same external
behaviour -- source-map keys, C-STORE/C-ECHO handling, storage
commitment, transfer-syntax defaults.
- MirthLauncher resolves a new variant="dicom.library:xxx" attribute on
extension <library> entries so only the configured backend's JARs land
on the classpath at runtime.
Build:
- server/build.xml splits the DICOM extension into three JARs:
dicom-server.jar (version-neutral, always loaded)
dicom-server-dcm2.jar (loaded when dicom.library=dcm4che2)
dicom-server-dcm5.jar (loaded when dicom.library=dcm4che5)
The dcm2 JAR bundles the stock dcm4che-tool-dcmrcv/dcmsnd classes via
zipfileset with duplicate="preserve" so patched classes take priority.
- Adds dcm4che-core-5.34.3.jar and dcm4che-net-5.34.3.jar to
server/lib/extensions/dimse/ and updates THIRD-PARTY-README.txt with
MPL 1.1 attribution for the new version.
Backward compatibility:
- DICOMUtil.byteArrayToDicomObject and dicomObjectToByteArray keep their
public signatures; the return type becomes OieDicomObject, which mirrors
the dcm4che2 DicomObject surface methods (contains, size, isEmpty,
getBytes, getDate, getFloat/getDouble with defaults, getStrings, getInts,
vrOf, nameOf, vm, getNestedDicomObject).
- OieDicomElement.vr() returns an OieVR adapter (code/padding/toString)
matching dcm4che2 VR's runtime shape so scripts calling elem.vr().code()
or String(elem.vr()) continue to work.
- Verified by running identical JavaScript transformer scripts on the
pre-patch baseline and on the patched branch: existing scripts that
work on dcm4che2 continue to work on both backends unchanged.
Tests:
- Unit and integration coverage for the abstraction layer, both backends,
cross-library parity, loopback C-STORE, C-ECHO, error handling, storage
commitment, and TLS with ephemeral self-signed keystores. Integration
tests use ephemeral ports and localhost only; all complete within a
couple of seconds.
- Ready-to-import manual test channels and an API regression script under
server/tests/dicom-channels/ that exercise the full OieDicomObject and
DICOMUtil surface through a live DICOM pipeline.
Documentation:
- docs/dcm4che5-migration-guide.md covers enabling the backend, behavioural
differences, TLS configuration, custom DICOMConfiguration migration,
JAR architecture, verification, and rollback.
Signed-off-by: Jesse Dowell <jesse@saga-it.com>
MirthLauncher already filters extension libraries by their `variant` attribute (e.g., `dicom.library:dcm4che5`) when building the server classpath, but WebStartServlet served all CLIENT/SHARED libraries to the Administrator regardless of variant. That left the Administrator downloading both dcm4che2 and dcm4che5 library JARs whenever both were declared — harmless (different package namespaces), but wasteful and inconsistent with server-side selection. Mirror MirthLauncher.shouldLoadLibrary in WebStartServlet so the Administrator receives only the libraries matching the server's current configuration. Signed-off-by: Jesse Dowell <jesse@saga-it.com>
Followups from a pre-PR audit of the dcm4che5 backend: - Add Object-VR overloads on OieDicomObject for putString / putInt / putBytes / putFragments, delegating via toString(). Preserves backward compatibility for transformer scripts passing a library-specific VR constant (e.g., dcm4che2's VR.PN) when the method surface is the version-neutral wrapper. - Simplify OieDicomObject.getInt(int, int) default from a two-step get(tag) + getInt(tag) lookup to a single contains(tag) + getInt(tag). - Upgrade trace->warn for Dcm5 no-op setters (setFileBufferSize, setTranscoderBufferSize, setDestination). These paths are only reached when the user explicitly set a non-default value, so the warn has zero noise on default configs and surfaces an otherwise-silent ignored setting. Also documents the long-standing upstream behavior of DICOMReceiver 'dest' being ignored on both backends (MirthDcmRcv's onCStoreRQ override streams directly to the channel and never consults DcmRcv.setDestination). - Document no-op settings and DICOMUtil API migration in docs/dcm4che5-migration-guide.md. Signed-off-by: Jesse Dowell <jesse@saga-it.com>
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
2 participants
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.
I realize this is a large changeset but I'm not sure how this can be done incrementally. I considered splitting it, but the abstraction layer is dead code without a second backend, and the variant-filtered JAR loading only exists to pick between the two. Open to restructuring if there's something that would make review easier.
Why: at Saga IT we've had to work around limitations in the dcm4che2 implementation and we believe the newer dcm4che5 version will have fewer bugs and potentially allow room for performance improvements - not to mention receiving security updates.
Caveat: only tested locally end-to-end, including mTLS DIMSE channels on both backends. This code has not been tested in real world workloads.
Summary
Adds an optional dcm4che5 backend to the DICOM Listener / Sender connector alongside the existing dcm4che2 backend. Selection is per-server via a new
dicom.libraryproperty inmirth.properties. The default remainsdcm4che2- existing installations see zero behavioral changes on upgrade.dcm4che2 has not had a release since 2015. dcm4che5 is actively maintained, has better support for modern Java and TLS, and receives security fixes. This PR gives users the option to move forward without forcing the migration.
Backward compatibility
The default codepath is entirely unchanged. For installations that don't opt in:
dicom.library, unset, defaults todcm4che2DICOMConfigurationclasses continue to load (with fallback logic for pre-PR implementations)DICOMUtil:byteArrayToDicomObject()now returnsOieDicomObjectinstead ofDicomObject. Duck-typed scripts work unchanged thanks to Object-VR overloads. Scripts doing explicit(DicomObject)casts orinstanceof DicomObjectchecks need.unwrap()- documented in the migration guide.Approach
com.mirth.connect.connectors.dimse.dicom:OieDicomObject,OieDicomElement,OieDicomSender,OieDicomReceiver,OieDicomConverter,OieVR. Decouples Mirth's DICOM code from a specific library version.dcm2/wraps existingMirthDcmSnd/MirthDcmRcv(unchanged in behavior).dcm5/composes fromDevice+Connection+ApplicationEntity+ service handlers in idiomatic dcm4che5 style.DicomLibraryFactoryreadsdicom.libraryat startup and instantiates the matching backend via reflection. Default isdcm4che2.variant="dicom.library:<value>"attribute.MirthLauncher(server) andWebStartServlet(Administrator) both honor it, so only the active backend's JARs are loaded on the server and shipped to the Administrator.Testing
server/test/.../dimse/dicom/integration/.Migration
Migration guide for users opting into dcm4che5:
docs/dcm4che5-migration-guide.md. Covers enable/disable, element-name difference (keyword vs PS3.6), async C-STORE dispatch, TLS cipher configuration, and three UI settings that are no-ops on dcm5 (twobufSizetuning flags;dest, which is in fact a silent no-op on dcm4che2 upstream as well).Dependencies
Adds
dcm4che-core-5.34.3anddcm4che-net-5.34.3(MPL-1.1). License entries added toserver/docs/thirdparty/THIRD-PARTY-README.txt.Not in this PR
Manual testing approaches taken beyond test cases
dicom.libraryset): existing DIMSE channels deploy and process messages identically to pre-upgradedicom.library = dcm4che5, restart: channels re-deploy cleanly, C-STORE and C-ECHO behavior verifieddicom.library = dcm4che2, restart: behavior returns to default