diff --git a/hapi-fhir-base/src/changes/changes.xml b/hapi-fhir-base/src/changes/changes.xml index 2ced661c149..e9917647dd5 100644 --- a/hapi-fhir-base/src/changes/changes.xml +++ b/hapi-fhir-base/src/changes/changes.xml @@ -7,6 +7,12 @@ + + API CHANGE:]]> The TagList class previously implemented ArrayList semantics, + but this has been replaced with LinkedHashMap semantics. This means that the list of + tags will no longer accept duplicate tags, but that tag order will still be + preserved. Thanks to Bill de Beaubien for reporting! + Documentation fixes diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java index 6a8c6904859..b38aafa2891 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java @@ -29,28 +29,25 @@ import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; public class Tag extends BaseElement implements IElement { - /** - * Convenience constant containing the "http://hl7.org/fhir/tag" scheme - * value - */ - public static final String HL7_ORG_FHIR_TAG = "http://hl7.org/fhir/tag"; + + public static final String ATTR_LABEL = "label"; + public static final String ATTR_SCHEME = "scheme"; + public static final String ATTR_TERM = "term"; /** - * Convenience constant containing the "http://hl7.org/fhir/tag/security" scheme - * value + * Convenience constant containing the "http://hl7.org/fhir/tag" scheme value + */ + public static final String HL7_ORG_FHIR_TAG = "http://hl7.org/fhir/tag"; + /** + * Convenience constant containing the "http://hl7.org/fhir/tag/profile" scheme value + */ + public static final String HL7_ORG_PROFILE_TAG = "http://hl7.org/fhir/tag/profile"; + /** + * Convenience constant containing the "http://hl7.org/fhir/tag/security" scheme value */ public static final String HL7_ORG_SECURITY_TAG = "http://hl7.org/fhir/tag/security"; - /** - * Convenience constant containing the "http://hl7.org/fhir/tag/profile" scheme - * value - */ - public static final String HL7_ORG_PROFILE_TAG = "http://hl7.org/fhir/tag/profile"; - - public static final String ATTR_TERM = "term"; - public static final String ATTR_LABEL = "label"; - public static final String ATTR_SCHEME = "scheme"; - + private transient Integer myHashCode; private String myLabel; private String myScheme; private String myTerm; @@ -59,7 +56,7 @@ public class Tag extends BaseElement implements IElement { } public Tag(String theTerm) { - this((String)null, theTerm, null); + this((String) null, theTerm, null); } public Tag(String theScheme, String theTerm, String theLabel) { @@ -119,11 +116,17 @@ public class Tag extends BaseElement implements IElement { @Override public int hashCode() { + if (myHashCode != null) { + System.out.println("Tag alread has hashcode " + myHashCode + " - " + myScheme + " - " + myTerm + " - " + myLabel); + return myHashCode; + } final int prime = 31; int result = 1; result = prime * result + ((myLabel == null) ? 0 : myLabel.hashCode()); result = prime * result + ((myScheme == null) ? 0 : myScheme.hashCode()); result = prime * result + ((myTerm == null) ? 0 : myTerm.hashCode()); + myHashCode = result; + System.out.println("Tag has hashcode " + myHashCode + " - " + myScheme + " - " + myTerm + " - " + myLabel); return result; } @@ -134,16 +137,19 @@ public class Tag extends BaseElement implements IElement { public Tag setLabel(String theLabel) { myLabel = theLabel; + myHashCode = null; return this; } public Tag setScheme(String theScheme) { myScheme = theScheme; + myHashCode = null; return this; } public Tag setTerm(String theTerm) { myTerm = theTerm; + myHashCode = null; return this; } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/TagList.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/TagList.java index 458540976bb..6a669990bf2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/TagList.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/TagList.java @@ -20,26 +20,193 @@ package ca.uhn.fhir.model.api; * #L% */ +import java.io.Serializable; import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; -public class TagList extends ArrayList { +/** + * A collection of tags present on a single resource. TagList is backed by a {@link LinkedHashSet}, so the order of added tags will be consistent, but duplicates will not be preserved. + * + *

+ * Thread safety: This class is not thread safe + *

+ */ +public class TagList implements Set, Serializable { - private static final long serialVersionUID = 1L; public static final String ATTR_CATEGORY = "category"; public static final String ELEMENT_NAME = "TagList"; + public static final String ELEMENT_NAME_LC = ELEMENT_NAME.toLowerCase(); + private static final long serialVersionUID = 1L; + private transient List myOrderedTags; + private LinkedHashSet myTagSet = new LinkedHashSet(); + + /** + * Constructor + */ + public TagList() { + super(); + } + + @Override + public String toString() { + StringBuilder b = new StringBuilder(); + b.append("TagList[").append(size()).append(" tag(s)]"); + for (Tag next : this) { + b.append("\n * ").append(next.toString()); + } + return b.toString(); + } + + @Override + public boolean add(Tag theE) { + myOrderedTags = null; + return myTagSet.add(theE); + } + + @Override + public boolean addAll(Collection theC) { + myOrderedTags = null; + return myTagSet.addAll(theC); + } + + public Tag addTag() { + myOrderedTags = null; + return addTag(null, null, null); + } public Tag addTag(String theScheme, String theTerm, String theLabel) { Tag retVal = new Tag(theScheme, theTerm, theLabel); add(retVal); + myOrderedTags = null; return retVal; } - public Tag addTag() { - return addTag(null, null, null); + @Override + public void clear() { + myOrderedTags = null; + myTagSet.clear(); } + @Override + public boolean contains(Object theO) { + return myTagSet.contains(theO); + } + + @Override + public boolean containsAll(Collection theC) { + return myTagSet.containsAll(theC); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + TagList other = (TagList) obj; + if (myTagSet == null) { + if (other.myTagSet != null) + return false; + } else if (!myTagSet.equals(other.myTagSet)) + return false; + return true; + } + + /** + * Returns the tag at a given index - Note that the TagList is backed by a {@link LinkedHashSet}, so the order of added tags will be consistent, but duplicates will not be preserved. + */ + public Tag get(int theIndex) { + if (myOrderedTags == null) { + myOrderedTags = new ArrayList(); + for (Tag next : myTagSet) { + myOrderedTags.add(next); + } + } + return myOrderedTags.get(theIndex); + } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public Tag set(int theIndex, Tag theElement) { + // throw new UnsupportedOperationException(); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public void add(int theIndex, Tag theElement) { + // myTagSet.add(theElement); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public Tag remove(int theIndex) { + // Tag retVal = myTagSet.remove(theIndex); + // myTagSet.remove(retVal); + // return retVal; + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public int indexOf(Object theO) { + // myTagSet.remove(theO); + // return myTagSet.indexOf(theO); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public int lastIndexOf(Object theO) { + // return myTagSet.lastIndexOf(theO); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public ListIterator listIterator() { + // return myTagSet.listIterator(); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public ListIterator listIterator(int theIndex) { + // return myTagSet.listIterator(theIndex); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public List subList(int theFromIndex, int theToIndex) { + // return myTagSet.subList(theFromIndex, theToIndex); + // } + public Tag getTag(String theScheme, String theTerm) { for (Tag next : this) { if (theScheme.equals(next.getScheme()) && theTerm.equals(next.getTerm())) { @@ -59,4 +226,68 @@ public class TagList extends ArrayList { return retVal; } + @Override + public int hashCode() { + return myTagSet.hashCode(); + } + + @Override + public boolean isEmpty() { + return myTagSet.isEmpty(); + } + + @Override + public Iterator iterator() { + return myTagSet.iterator(); + } + + @Override + public boolean remove(Object theO) { + myOrderedTags = null; + return myTagSet.remove(theO); + } + + @Override + public boolean removeAll(Collection theC) { + myOrderedTags = null; + return myTagSet.removeAll(theC); + } + + @Override + public boolean retainAll(Collection theC) { + myOrderedTags = null; + return myTagSet.retainAll(theC); + } + + @Override + public int size() { + return myTagSet.size(); + } + + @Override + public Object[] toArray() { + return myTagSet.toArray(); + } + + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + // public boolean addAll(int theIndex, Collection theC) { + // myTagSet.addAll(theC); + // return myTagSet.addAll(theC); + // } + // + // /** + // * @deprecated TagList will not implement the {@link List} interface in a future release, as tag order is not supposed to be meaningful + // */ + // @Deprecated + // @Override + + @Override + public T[] toArray(T[] theA) { + return myTagSet.toArray(theA); + } + } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java index 4de489f592e..b5b8ba30b66 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java @@ -29,7 +29,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import ca.uhn.fhir.context.BaseRuntimeChildDefinition; import ca.uhn.fhir.context.BaseRuntimeDeclaredChildDefinition; diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagListTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagListTest.java new file mode 100644 index 00000000000..f00e0bf4b2c --- /dev/null +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagListTest.java @@ -0,0 +1,67 @@ +package ca.uhn.fhir.model.api; + +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.junit.Assert.*; + +import java.util.Arrays; + +import org.junit.Test; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.dstu.resource.Patient; + +public class TagListTest { + + private FhirContext myCtx = new FhirContext(); + + @Test + public void testEquals() { + TagList tagList1 = new TagList(); + tagList1.addTag(null, "Dog", "Puppies"); + tagList1.addTag("http://foo", "Cat", "Kittens"); + + TagList tagList2 = new TagList(); + tagList2.addTag(null, "Dog", "Puppies"); + tagList2.addTag("http://foo", "Cat", "Kittens"); + + assertEquals(tagList1,tagList2); + } + + @Test + public void testEqualsIgnoresOrder() { + TagList tagList1 = new TagList(); + tagList1.addTag(null, "Dog", "Puppies"); + tagList1.addTag("http://foo", "Cat", "Kittens"); + + TagList tagList2 = new TagList(); + tagList2.addTag("http://foo", "Cat", "Kittens"); + tagList2.addTag(null, "Dog", "Puppies"); + + assertEquals(tagList1,tagList2); + } + + @Test + public void testPreventDuplication() { + + Patient patient = new Patient(); + patient.addIdentifier("urn:system", "testTagsWithCreateAndReadAndSearch"); + patient.addName().addFamily("Tester").addGiven("Joe"); + TagList tagList = new TagList(); + tagList.addTag(null, "Dog", "Puppies"); + // Add this twice + tagList.addTag("http://foo", "Cat", "Kittens"); + tagList.addTag("http://foo", "Cat", "Kittens"); + + patient.getResourceMetadata().put(ResourceMetadataKeyEnum.TAG_LIST, tagList); + + Bundle b = new Bundle(); + b.addResource(patient, myCtx, "http://foo"); + + String encoded = myCtx.newXmlParser().encodeBundleToString(b); + assertThat(encoded, stringContainsInOrder(Arrays.asList("Cat", "Kittens"))); + assertThat(encoded, not(stringContainsInOrder(Arrays.asList("Cat", "Kittens", "Cat", "Kittens")))); + + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java index f3c6ae7d9fc..dce096145ac 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java @@ -1189,14 +1189,17 @@ public class FhirResourceDaoTest { patient.addName().addFamily("Tester").addGiven("Joe"); TagList tagList = new TagList(); tagList.addTag(null, "Dog", "Puppies"); + // Add this twice + tagList.addTag("http://foo", "Cat", "Kittens"); tagList.addTag("http://foo", "Cat", "Kittens"); patient.getResourceMetadata().put(ResourceMetadataKeyEnum.TAG_LIST, tagList); MethodOutcome outcome = ourPatientDao.create(patient); - assertNotNull(outcome.getId()); - assertFalse(outcome.getId().isEmpty()); + IdDt patientId = outcome.getId(); + assertNotNull(patientId); + assertFalse(patientId.isEmpty()); - Patient retrieved = ourPatientDao.read(outcome.getId()); + Patient retrieved = ourPatientDao.read(patientId); TagList published = (TagList) retrieved.getResourceMetadata().get(ResourceMetadataKeyEnum.TAG_LIST); assertEquals(2, published.size()); assertEquals("Dog", published.get(0).getTerm()); @@ -1217,6 +1220,23 @@ public class FhirResourceDaoTest { assertEquals("Kittens", published.get(1).getLabel()); assertEquals("http://foo", published.get(1).getScheme()); + ourPatientDao.addTag(patientId, "http://foo", "Cat", "Kittens"); + ourPatientDao.addTag(patientId, "http://foo", "Cow", "Calves"); + + retrieved = ourPatientDao.read(patientId); + published = (TagList) retrieved.getResourceMetadata().get(ResourceMetadataKeyEnum.TAG_LIST); + assertEquals(3, published.size()); + assertEquals("Dog", published.get(0).getTerm()); + assertEquals("Puppies", published.get(0).getLabel()); + assertEquals(null, published.get(0).getScheme()); + assertEquals("Cat", published.get(1).getTerm()); + assertEquals("Kittens", published.get(1).getLabel()); + assertEquals("http://foo", published.get(1).getScheme()); + assertEquals("Cow", published.get(2).getTerm()); + assertEquals("Calves", published.get(2).getLabel()); + assertEquals("http://foo", published.get(2).getScheme()); + + } @Test