Skip to content

Commit c9de11f

Browse files
committed
refactor: polish dcm4che5 backend after pre-PR audit
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.dowell@gmail.com>
1 parent 865d1f1 commit c9de11f

5 files changed

Lines changed: 108 additions & 10 deletions

File tree

docs/dcm4che5-migration-guide.md

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ To revert, change the value back to `dcm4che2` (or remove the line) and restart.
3434

3535
| Area | Behavior |
3636
|------|----------|
37-
| Channel properties | All DICOM Listener and Sender properties work identically |
37+
| Channel properties | Listener and Sender UI properties work identically (see [Settings with no effect on dcm4che5](#settings-with-no-effect-on-dcm4che5) for two pure-tuning flags that are ignored) |
3838
| Source map keys | Same keys populated: `localApplicationEntityTitle`, `remoteApplicationEntityTitle`, `localAddress`, `localPort`, `remoteAddress`, `remotePort` |
3939
| DICOM object serialization | Byte-level output differs (different FMI implementation version UIDs), but all tag values are semantically equivalent |
4040
| C-STORE, C-ECHO | Both work through the standard channel lifecycle |
@@ -63,6 +63,46 @@ If your channel logic inspects DICOM element names (not tag numbers), be aware t
6363
dicomObject.getString(Tag.PatientName) // works on both
6464
```
6565

66+
### Settings with no effect
67+
68+
A few Listener and Sender UI fields have no corresponding implementation on the dcm4che5 backend. The dcm4che2 backend behavior is preserved unchanged. A `WARN`-level log is emitted on channel start when any of these are set to a non-default value on dcm4che5:
69+
70+
| UI setting | Connector | Note |
71+
|---|---|---|
72+
| `bufSize` | Listener | File buffer size. dcm4che3 manages buffers internally; no equivalent API |
73+
| `bufSize` | Sender | Transcoder buffer size. dcm4che3 manages buffers internally via `DataWriterAdapter` |
74+
| `dest` (Store Received Objects in Directory) | Listener | Silently ignored on **both backends** — a long-standing upstream behavior. `MirthDcmRcv` streams DIMSE data directly to the channel and never consults this setting on either backend |
75+
76+
None of these affect data integrity. Messages still arrive and dispatch through the channel correctly. The `bufSize` flags are pure performance tuning; revert `dicom.library` to `dcm4che2` if the throughput difference matters for your deployment.
77+
78+
All other UI settings — TLS options, AE titles, timeouts, PDU lengths, transfer syntax selection, storage commitment, user identity, and priority — work identically on both backends.
79+
80+
### DICOMUtil API (user transformer scripts)
81+
82+
`DICOMUtil.byteArrayToDicomObject()` and `dicomObjectToByteArray()` now return / accept the version-neutral `OieDicomObject` type. For the vast majority of transformer scripts this change is invisible — Rhino's duck typing plus Object-type overloads on `OieDicomObject` mean existing calls keep working unchanged:
83+
84+
```javascript
85+
// Existing scripts continue to work on default (dcm4che2) without changes:
86+
var dcm = DICOMUtil.byteArrayToDicomObject(bytes, false);
87+
dcm.getString(Tag.PatientName); // same method exists on OieDicomObject
88+
dcm.putString(Tag.PatientName, VR.PN, "SMITH"); // Object-overload routes via VR.toString()
89+
```
90+
91+
Only these specific patterns require a one-line change:
92+
93+
| Pattern | Change |
94+
|---|---|
95+
| `(DicomObject) DICOMUtil.byteArrayToDicomObject(...)` explicit cast | `(DicomObject) DICOMUtil.byteArrayToDicomObject(...).unwrap()` |
96+
| `dcm instanceof DicomObject` | `dcm.unwrap() instanceof DicomObject` |
97+
| Passing the result to a Java API that expects `org.dcm4che2.data.DicomObject` | Pass `dcm.unwrap()` instead |
98+
99+
The recommended version-neutral pattern for new scripts is to use string VR codes instead of library-specific constants:
100+
101+
```javascript
102+
// Works identically on both backends — no dependency on VR class:
103+
dcm.putString(Tag.PatientName, "PN", "SMITH");
104+
```
105+
66106
## TLS Configuration
67107

68108
### Standard UI TLS Options

server/src/com/mirth/connect/connectors/dimse/dicom/OieDicomObject.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ default String getString(int tag, String defaultValue) {
3636
* if the tag is not present.
3737
*/
3838
default int getInt(int tag, int defaultValue) {
39-
OieDicomElement elem = get(tag);
40-
return elem != null ? getInt(tag) : defaultValue;
39+
return contains(tag) ? getInt(tag) : defaultValue;
4140
}
4241

4342
OieDicomElement get(int tag);
@@ -79,14 +78,41 @@ default int getInt(int tag, int defaultValue) {
7978

8079
void putString(int tag, String vr, String value);
8180

81+
/**
82+
* Overload accepting any {@code Object} as the VR argument. Delegates to
83+
* {@link #putString(int, String, String)} using {@code vr.toString()}.
84+
*
85+
* <p>Preserves backward compatibility for transformer scripts that pass a
86+
* library-specific VR constant (e.g., dcm4che2's {@code VR.PN}), whose
87+
* {@code toString()} returns the two-letter VR code.
88+
*/
89+
default void putString(int tag, Object vr, String value) {
90+
putString(tag, vr != null ? vr.toString() : null, value);
91+
}
92+
8293
void putInt(int tag, String vr, int value);
8394

95+
/** Object-VR overload of {@link #putInt(int, String, int)}; see {@link #putString(int, Object, String)}. */
96+
default void putInt(int tag, Object vr, int value) {
97+
putInt(tag, vr != null ? vr.toString() : null, value);
98+
}
99+
84100
void putBytes(int tag, String vr, byte[] value);
85101

102+
/** Object-VR overload of {@link #putBytes(int, String, byte[])}; see {@link #putString(int, Object, String)}. */
103+
default void putBytes(int tag, Object vr, byte[] value) {
104+
putBytes(tag, vr != null ? vr.toString() : null, value);
105+
}
106+
86107
OieDicomElement putSequence(int tag);
87108

88109
OieDicomElement putFragments(int tag, String vr, boolean bigEndian, int capacity);
89110

111+
/** Object-VR overload of {@link #putFragments(int, String, boolean, int)}; see {@link #putString(int, Object, String)}. */
112+
default OieDicomElement putFragments(int tag, Object vr, boolean bigEndian, int capacity) {
113+
return putFragments(tag, vr != null ? vr.toString() : null, bigEndian, capacity);
114+
}
115+
90116
void add(OieDicomElement element);
91117

92118
OieDicomElement remove(int tag);

server/src/com/mirth/connect/connectors/dimse/dicom/dcm5/Dcm5DicomReceiver.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,13 @@ public void setHostname(String hostname) {
273273

274274
@Override
275275
public void setDestination(String destination) {
276-
// Not used — dcm5 receiver dispatches directly to the channel, not to filesystem
277-
logger.trace("setDestination ignored in dcm5 receiver (no filesystem storage): " + destination);
276+
// Matches dcm4che2's de facto behavior: MirthDcmRcv streams DIMSE data directly
277+
// to the channel and never consults DcmRcv.setDestination, so the UI flag has
278+
// been a no-op on both backends. Only reached when the user explicitly set a
279+
// non-blank value in the Listener's "Store Received Objects in Directory" field.
280+
logger.warn("destination={} has no effect on either DICOM backend (the flag has been "
281+
+ "silently ignored upstream for years). Remove this setting to clear the warning.",
282+
destination);
278283
}
279284

280285
@Override
@@ -345,9 +350,11 @@ public void setReceiveBufferSize(int size) {
345350

346351
@Override
347352
public void setFileBufferSize(int size) {
348-
// dcm5 receiver writes directly to ByteArrayOutputStream, not filesystem.
349-
// No file buffer applicable.
350-
logger.trace("setFileBufferSize ignored in dcm5 receiver: " + size);
353+
// dcm5 receiver streams directly to memory, with no file buffer concept.
354+
// Only reached when the user explicitly changed bufSize from default,
355+
// so warn instead of trace to surface the ignored setting.
356+
logger.warn("bufSize={} has no effect on dcm4che5 receiver (dcm4che3 manages buffers internally). "
357+
+ "Revert dicom.library=dcm4che2 if this tuning is load-bearing.", size);
351358
}
352359

353360
@Override

server/src/com/mirth/connect/connectors/dimse/dicom/dcm5/Dcm5DicomSender.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,11 @@ public void setMaxOpsInvoked(int maxOps) {
197197

198198
@Override
199199
public void setTranscoderBufferSize(int size) {
200-
// dcm4che5 does not have a transcoder buffer — transcoding is handled internally.
201-
logger.trace("setTranscoderBufferSize ignored in dcm5 sender: " + size);
200+
// dcm4che5 has no transcoder buffer — transcoding is handled internally via DataWriterAdapter.
201+
// Only reached when the user explicitly changed bufSize from default, so warn instead of
202+
// trace to surface the ignored setting.
203+
logger.warn("bufSize={} has no effect on dcm4che5 sender (dcm4che3 manages transcoder buffers internally). "
204+
+ "Revert dicom.library=dcm4che2 if this tuning is load-bearing.", size);
202205
}
203206

204207
@Override

server/test/com/mirth/connect/connectors/dimse/dicom/dcm5/Dcm5DicomObjectTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,28 @@ public void testGetReturnsNullForMissingTag() {
5757
assertEquals(0, obj.getInt(Tag.Rows));
5858
}
5959

60+
@Test
61+
public void testPutStringAcceptsObjectVrViaToString() {
62+
// Simulates a user transformer script passing a library-specific VR constant
63+
// (e.g., dcm4che2 VR.PN) — the Object overload delegates via toString().
64+
Object libraryVr = new Object() {
65+
@Override public String toString() { return "PN"; }
66+
};
67+
Dcm5DicomObject obj = new Dcm5DicomObject();
68+
obj.putString(Tag.PatientName, libraryVr, "Doe^John");
69+
assertEquals("Doe^John", obj.getString(Tag.PatientName));
70+
}
71+
72+
@Test
73+
public void testPutIntAcceptsObjectVrViaToString() {
74+
Object libraryVr = new Object() {
75+
@Override public String toString() { return "US"; }
76+
};
77+
Dcm5DicomObject obj = new Dcm5DicomObject();
78+
obj.putInt(Tag.Rows, libraryVr, 512);
79+
assertEquals(512, obj.getInt(Tag.Rows));
80+
}
81+
6082
@Test
6183
public void testPutSequence() {
6284
Dcm5DicomObject obj = new Dcm5DicomObject();

0 commit comments

Comments
 (0)