Skip to content

Commit

Permalink
[LIB-362] Wrong private tag handling in Attributes.addSelected()
Browse files Browse the repository at this point in the history
  • Loading branch information
@hczedik
hczedik committed Jun 24, 2015
1 parent 189c91a commit 764d9c4
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 14 deletions.
30 changes: 16 additions & 14 deletions dcm4che-core/src/main/java/org/dcm4che3/data/Attributes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.*;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.TimeZone;
import java.util.regex.Pattern;

import org.dcm4che3.data.IOD.DataElement;
Expand Down Expand Up @@ -2003,7 +2007,7 @@ public boolean testMerge(Attributes other) {
}

public boolean addSelected(Attributes other, Attributes selection) {
return add(other, selection.tags, null, 0, selection.size, selection, false, false, false, null);
return add(other, null, null, 0, 0, selection, false, false, false, null);
}

public boolean addSelected(Attributes other, String privateCreator, int tag) {
Expand Down Expand Up @@ -2178,32 +2182,26 @@ private boolean add(Attributes other, int[] include, int[] exclude,
boolean toggleEndian = bigEndian != other.bigEndian;
boolean modifiedToggleEndian = modified != null
&& bigEndian != modified.bigEndian;
final int[] tags = other.tags;
final int[] otherTags = other.tags;
final VR[] srcVRs = other.vrs;
final Object[] srcValues = other.values;
final int otherSize = other.size;
int numAdd = 0;
String privateCreator = null;
int creatorTag = 0;
for (int i = 0; i < otherSize; i++) {
int tag = tags[i];
int tag = otherTags[i];
VR vr = srcVRs[i];
Object value = srcValues[i];
if (TagUtils.isPrivateCreator(tag)) {
if (contains(tag))
continue; // do not overwrite private creator IDs

if (vr == VR.LO) {
value = other.decodeStringValue(i);
if ((value instanceof String)
&& creatorTagOf((String) value, tag, false) != -1)
continue; // do not add duplicate private creator ID
}
continue; // private creators will be automatically added with the private tags
}

if (include != null && Arrays.binarySearch(include, fromIndex, toIndex, tag) < 0)
continue;
if (exclude != null && Arrays.binarySearch(exclude, fromIndex, toIndex, tag) >= 0)
continue;

if (TagUtils.isPrivateTag(tag)) {
int tmp = TagUtils.creatorTagOf(tag);
if (creatorTag != tmp) {
Expand All @@ -2214,6 +2212,10 @@ && creatorTagOf((String) value, tag, false) != -1)
creatorTag = 0;
privateCreator = null;
}

if (selection != null && !selection.contains(privateCreator, tag))
continue;

if (merge || update) {
int j = indexOf(tag);
if (j >= 0) {
Expand Down Expand Up @@ -2244,7 +2246,7 @@ && creatorTagOf((String) value, tag, false) != -1)
if (value instanceof Sequence) {
set(privateCreator, tag, (Sequence) value,
selection != null
? selection.getNestedDataset(tag)
? selection.getNestedDataset(privateCreator, tag)
: null);
} else if (value instanceof Fragments) {
set(privateCreator, tag, (Fragments) value);
Expand Down
172 changes: 172 additions & 0 deletions dcm4che-core/src/test/java/org/dcm4che3/data/AttributesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,178 @@ public void testItemPointer() {
assertArrayEquals(ipd, d.itemPointers());
}

@Test
public void testAddSelectedWithSelectionAttributes()
{
Attributes original = new Attributes();
Attributes otherPID = new Attributes();
original.setString(Tag.AccessionNumber, VR.SH, "AccessionNumber");
original.setNull(Tag.PatientName, VR.PN);
original.setString(Tag.PatientID, VR.LO, "PatientID");
original.setString(Tag.IssuerOfPatientID, VR.LO, "IssuerOfPatientID");
original.newSequence(Tag.OtherPatientIDsSequence, 1).add(otherPID);
Sequence requestAttributesSequence = original.newSequence(Tag.RequestAttributesSequence, 2);
Attributes rqAttrs1 = new Attributes();
rqAttrs1.setString(Tag.RequestedProcedureID, VR.LO, "RequestedProcedureID1");
rqAttrs1.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID1");
Attributes rqAttrs2 = new Attributes();
rqAttrs2.setString(Tag.RequestedProcedureID, VR.LO, "RequestedProcedureID2");
rqAttrs2.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID2");
requestAttributesSequence.add(rqAttrs1);
requestAttributesSequence.add(rqAttrs2);
original.setString("PrivateCreatorA", 0x00990001, VR.LO, "0099xx01A");
original.setString("PrivateCreatorB", 0x00990001, VR.LO, "0099xx01B");
original.setString("PrivateCreatorB", 0x00990002, VR.LO, "0099xx02B");
otherPID.setString(Tag.PatientID, VR.LO, "OtherPatientID");
otherPID.setString(Tag.IssuerOfPatientID, VR.LO, "OtherIssuerOfPatientID");

Attributes selection = new Attributes();
selection.setNull(Tag.AccessionNumber, VR.SH);
selection.setNull(Tag.PatientName, VR.PN);
// select complete other patient id sequence
selection.newSequence(Tag.OtherPatientIDsSequence, 0);
// sub-selection inside the RequestAttributesSequence
Attributes rqAttrsSelection = new Attributes();
rqAttrsSelection.setNull(Tag.ScheduledProcedureStepID, VR.LO);
selection.newSequence(Tag.RequestAttributesSequence, 1).add(rqAttrsSelection);

// filter the original with the selection
Attributes filtered = new Attributes();
filtered.addSelected(original, selection); // THIS is the method we want to test here

// that is the expected result
Attributes filteredExpected = new Attributes();
filteredExpected.setString(Tag.AccessionNumber, VR.SH, "AccessionNumber");
filteredExpected.setNull(Tag.PatientName, VR.PN);
Attributes filteredExpectedOtherPID = new Attributes();
filteredExpectedOtherPID.setString(Tag.PatientID, VR.LO, "OtherPatientID");
filteredExpectedOtherPID.setString(Tag.IssuerOfPatientID, VR.LO, "OtherIssuerOfPatientID");
filteredExpected.newSequence(Tag.OtherPatientIDsSequence, 1).add(filteredExpectedOtherPID);
Sequence requestAttributesSequenceFilteredExpected = filteredExpected.newSequence(Tag.RequestAttributesSequence, 2);
Attributes rqAttrs1FilteredExpected = new Attributes();
rqAttrs1FilteredExpected.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID1");
Attributes rqAttrs2FilteredExpected = new Attributes();
rqAttrs2FilteredExpected.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID2");
requestAttributesSequenceFilteredExpected.add(rqAttrs1FilteredExpected);
requestAttributesSequenceFilteredExpected.add(rqAttrs2FilteredExpected);

assertEquals(filteredExpected, filtered);
}

@Test
public void testAddSelectedWithSelectionAttributesInsideSequence()
{
Attributes original = new Attributes();
Sequence requestAttributesSequence = original.newSequence(Tag.RequestAttributesSequence, 2);
Attributes rqAttrs1 = new Attributes();
rqAttrs1.setString(Tag.RequestedProcedureID, VR.LO, "RequestedProcedureID1");
rqAttrs1.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID1");
Attributes rqAttrs2 = new Attributes();
rqAttrs2.setString(Tag.RequestedProcedureID, VR.LO, "RequestedProcedureID2");
rqAttrs2.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID2");
requestAttributesSequence.add(rqAttrs1);
requestAttributesSequence.add(rqAttrs2);

Attributes selection = new Attributes();
// sub-selection inside the RequestAttributesSequence
// this test just documents the behavior that for the selection only the first item within a sequence is considered
Attributes rqAttrsSelection = new Attributes();
rqAttrsSelection.setNull(Tag.ScheduledProcedureStepID, VR.LO);
Attributes rqAttrsIgnoredSelection = new Attributes();
rqAttrsIgnoredSelection.setNull(Tag.RequestedProcedureID, VR.LO);
Sequence requestAttrsSeqSelection = selection.newSequence(Tag.RequestAttributesSequence, 2);
requestAttrsSeqSelection.add(rqAttrsSelection);
requestAttrsSeqSelection.add(rqAttrsIgnoredSelection); // this one will not be considered for the selection

// filter the original with the selection
Attributes filtered = new Attributes();
filtered.addSelected(original, selection); // THIS is the method we want to test here

// that is the expected result
Attributes filteredExpected = new Attributes();
Sequence requestAttributesSequenceFilteredExpected = filteredExpected.newSequence(Tag.RequestAttributesSequence, 2);
Attributes rqAttrs1FilteredExpected = new Attributes();
rqAttrs1FilteredExpected.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID1");
Attributes rqAttrs2FilteredExpected = new Attributes();
rqAttrs2FilteredExpected.setString(Tag.ScheduledProcedureStepID, VR.LO, "ScheduledProcedureStepID2");
requestAttributesSequenceFilteredExpected.add(rqAttrs1FilteredExpected);
requestAttributesSequenceFilteredExpected.add(rqAttrs2FilteredExpected);

assertEquals(filteredExpected, filtered);
}

@Test
public void testAddSelectedWithSelectionAttributesPrivateTags()
{
// tests the fix for LIB-362

Attributes original = new Attributes();
original.setString("PrivateCreatorA", 0x00990001, VR.LO, "0099xx01A");
original.setString("PrivateCreatorB", 0x00990001, VR.LO, "0099xx01B");

Attributes selection = new Attributes();
selection.setNull("PrivateCreatorB", 0x00990001, VR.LO);

Attributes filtered = new Attributes();
filtered.addSelected(original, selection); // THIS is the method we want to test here

// that is the expected result
Attributes filteredExpected = new Attributes();
filteredExpected.setString("PrivateCreatorB", 0x00990001, VR.LO, "0099xx01B");

assertEquals(filteredExpected, filtered);
}

@Test
public void testAddSelectedWithSelectionAttributesPrivateTags2()
{
Attributes original = new Attributes();
original.setString("PrivateCreatorA", 0x00990001, VR.LO, "0099xx01A");
original.setString("PrivateCreatorB", 0x00990001, VR.LO, "0099xx01B");
original.setString("PrivateCreatorC", 0x00990001, VR.LO, "0099xx01C");

Attributes selection = new Attributes();
selection.setNull("PrivateCreatorA", 0x00990001, VR.LO);
selection.setNull("PrivateCreatorC", 0x00990001, VR.LO);

Attributes filtered = new Attributes();
filtered.addSelected(original, selection); // THIS is the method we want to test here

// that is the expected result
Attributes filteredExpected = new Attributes();
filteredExpected.setString("PrivateCreatorA", 0x00990001, VR.LO, "0099xx01A");
filteredExpected.setString("PrivateCreatorC", 0x00990001, VR.LO, "0099xx01C");

assertEquals(filteredExpected, filtered);
}

@Test
public void testAddSelectedWithSelectionAttributesInsidePrivateSequence()
{
Attributes original = new Attributes();
original.setString("PrivateCreatorA", 0x00990001, VR.LO, "0099xx01A");
Sequence privateSeq = original.newSequence("PrivateCreatorB", 0x00990001, 2);
privateSeq.add(new Attributes());
privateSeq.get(0).setString(Tag.SOPInstanceUID, VR.UI, "1.2.3.4");
privateSeq.get(0).setString(Tag.SOPClassUID, VR.UI, "4.3.2.1");

Attributes selection = new Attributes();
Sequence privateSeqSelection = selection.newSequence("PrivateCreatorB", 0x00990001, 2);
privateSeqSelection.add(new Attributes());
privateSeqSelection.get(0).setNull(Tag.SOPInstanceUID, VR.UI);

Attributes filtered = new Attributes();
filtered.addSelected(original, selection); // THIS is the method we want to test here

// that is the expected result
Attributes filteredExpected = new Attributes();
Sequence privateSeqExpected = filteredExpected.newSequence("PrivateCreatorB", 0x00990001, 2);
privateSeqExpected.add(new Attributes());
privateSeqExpected.get(0).setString(Tag.SOPInstanceUID, VR.UI, "1.2.3.4");

assertEquals(filteredExpected, filtered);
}

private void assertModified(Attributes modified) {
assertEquals("PatientID", modified.getString(Tag.PatientID));
Attributes modOtherPID = modified.getNestedDataset(Tag.OtherPatientIDsSequence);
Expand Down

0 comments on commit 764d9c4

Please sign in to comment.