Correct an issue when processing transactions in JPA server where updates and

creates to resources with tags caused the tags to be created twice in the
            database. These duplicates were utomatically filtered upon read so this issue
            was not user-visible, but it coule occasionally lead to performance issues
            if a resource containing multiple tags was updated many times via
            transactions.

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTable.java

@@ -251,7 +251,9 @@ public class ResourceTable extends BaseHasResource implements Serializable {
 	@Override
 	public ResourceTag addTag(TagDefinition theTag) {
 		ResourceTag tag = new ResourceTag(this, theTag);
+		if (!getTags().contains(tag)) {
 			getTags().add(tag);
+		}
 		return tag;
 	}
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceTag.java

@@ -21,8 +21,7 @@ package ca.uhn.fhir.jpa.entity;
  */
 import javax.persistence.*;
 
-import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.*;
-import org.apache.commons.lang3.builder.HashCodeBuilder;
 
 @Entity
 @Table(name = "HFJ_RES_TAG", uniqueConstraints= {
@@ -108,4 +107,12 @@ public class ResourceTag extends BaseTag {
 		return b.toHashCode();
 	}
 
+	@Override
+	public String toString() {
+		ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE);
+		b.append("resId", getResourceId());
+		b.append("tag", getTag().getId());
+		return b.build();
+	}
+
 }

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java

@@ -303,36 +303,31 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest {
 		request.addEntry().getRequest().setMethod(HTTPVerbEnum.GET).setUrl("Patient/THIS_ID_DOESNT_EXIST");
 
 		Bundle resp = mySystemDao.transaction(mySrd, request);
-		assertEquals(3, resp.getEntry().size());
+		assertEquals(2, resp.getEntry().size());
 		assertEquals(BundleTypeEnum.BATCH_RESPONSE, resp.getTypeElement().getValueAsEnum());
 
 		ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp));
 		EntryResponse respEntry;
 
-		// Bundle.entry[0] is operation outcome
-		assertEquals(OperationOutcome.class, resp.getEntry().get(0).getResource().getClass());
-		assertEquals(IssueSeverityEnum.INFORMATION, ((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getSeverityElement().getValueAsEnum());
-		assertThat(((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getDiagnostics(), startsWith("Batch completed in "));
-
 		// Bundle.entry[1] is create response
-		assertEquals("201 Created", resp.getEntry().get(1).getResponse().getStatus());
+		assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus());
-		assertThat(resp.getEntry().get(1).getResponse().getLocation(), startsWith("Patient/"));
+		assertThat(resp.getEntry().get(0).getResponse().getLocation(), startsWith("Patient/"));
 
 		// Bundle.entry[2] is failed read response
-		assertEquals(OperationOutcome.class, resp.getEntry().get(2).getResource().getClass());
+		assertEquals(OperationOutcome.class, resp.getEntry().get(1).getResource().getClass());
-		assertEquals(IssueSeverityEnum.ERROR, ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getSeverityElement().getValueAsEnum());
+		assertEquals(IssueSeverityEnum.ERROR, ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getSeverityElement().getValueAsEnum());
-		assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getDiagnostics());
+		assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getDiagnostics());
-		assertEquals("404 Not Found", resp.getEntry().get(2).getResponse().getStatus());
+		assertEquals("404 Not Found", resp.getEntry().get(1).getResponse().getStatus());
 
 		// Check POST
-		respEntry = resp.getEntry().get(1).getResponse();
+		respEntry = resp.getEntry().get(0).getResponse();
 		assertEquals("201 Created", respEntry.getStatus());
 		IdDt createdId = new IdDt(respEntry.getLocation());
 		assertEquals("Patient", createdId.getResourceType());
 		myPatientDao.read(createdId, mySrd); // shouldn't fail
 
 		// Check GET
-		respEntry = resp.getEntry().get(2).getResponse();
+		respEntry = resp.getEntry().get(1).getResponse();
 		assertThat(respEntry.getStatus(), startsWith("404"));
 
 	}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java

@@ -23,7 +23,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.StandardCharsets;
-import java.util.List;
+import java.util.*;
 
 import org.apache.commons.io.IOUtils;
 import org.hl7.fhir.dstu3.model.Appointment;
@@ -68,9 +68,7 @@ import org.springframework.transaction.support.TransactionTemplate;
 import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao;
 import ca.uhn.fhir.jpa.dao.DaoConfig;
 import ca.uhn.fhir.jpa.dao.SearchParameterMap;
-import ca.uhn.fhir.jpa.entity.ResourceEncodingEnum;
+import ca.uhn.fhir.jpa.entity.*;
-import ca.uhn.fhir.jpa.entity.ResourceTable;
-import ca.uhn.fhir.jpa.entity.TagTypeEnum;
 import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test;
 import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
 import ca.uhn.fhir.model.primitive.IdDt;
@@ -488,36 +486,31 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest {
 		request.addEntry().getRequest().setMethod(HTTPVerb.GET).setUrl("Patient/THIS_ID_DOESNT_EXIST");
 
 		Bundle resp = mySystemDao.transaction(mySrd, request);
-		assertEquals(3, resp.getEntry().size());
+		assertEquals(2, resp.getEntry().size());
 		assertEquals(BundleType.BATCHRESPONSE, resp.getTypeElement().getValue());
 
 		ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp));
 		BundleEntryResponseComponent respEntry;
 
-		// Bundle.entry[0] is operation outcome
+		// Bundle.entry[0] is create response
-		assertEquals(OperationOutcome.class, resp.getEntry().get(0).getResource().getClass());
+		assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus());
-		assertEquals(IssueSeverity.INFORMATION, ((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getSeverityElement().getValue());
+		assertThat(resp.getEntry().get(0).getResponse().getLocation(), startsWith("Patient/"));
-		assertThat(((OperationOutcome) resp.getEntry().get(0).getResource()).getIssue().get(0).getDiagnostics(), startsWith("Batch completed in "));
 
-		// Bundle.entry[1] is create response
+		// Bundle.entry[1] is failed read response
-		assertEquals("201 Created", resp.getEntry().get(1).getResponse().getStatus());
+		assertEquals(OperationOutcome.class, resp.getEntry().get(1).getResource().getClass());
-		assertThat(resp.getEntry().get(1).getResponse().getLocation(), startsWith("Patient/"));
+		assertEquals(IssueSeverity.ERROR, ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getSeverityElement().getValue());
-
+		assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(1).getResource()).getIssue().get(0).getDiagnostics());
-		// Bundle.entry[2] is failed read response
+		assertEquals("404 Not Found", resp.getEntry().get(1).getResponse().getStatus());
-		assertEquals(OperationOutcome.class, resp.getEntry().get(2).getResource().getClass());
-		assertEquals(IssueSeverity.ERROR, ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getSeverityElement().getValue());
-		assertEquals("Resource Patient/THIS_ID_DOESNT_EXIST is not known", ((OperationOutcome) resp.getEntry().get(2).getResource()).getIssue().get(0).getDiagnostics());
-		assertEquals("404 Not Found", resp.getEntry().get(2).getResponse().getStatus());
 
 		// Check POST
-		respEntry = resp.getEntry().get(1).getResponse();
+		respEntry = resp.getEntry().get(0).getResponse();
 		assertEquals("201 Created", respEntry.getStatus());
 		IdType createdId = new IdType(respEntry.getLocation());
 		assertEquals("Patient", createdId.getResourceType());
 		myPatientDao.read(createdId, mySrd); // shouldn't fail
 
 		// Check GET
-		respEntry = resp.getEntry().get(2).getResponse();
+		respEntry = resp.getEntry().get(1).getResponse();
 		assertThat(respEntry.getStatus(), startsWith("404"));
 
 	}
@@ -2081,6 +2074,21 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest {
 		ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp));
 
 		assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus());
+		
+		
+		new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() {
+			@Override
+			protected void doInTransactionWithoutResult(TransactionStatus theStatus) {
+				Set<String> values = new HashSet<String>();
+				for (ResourceTag next : myResourceTagDao.findAll()) {
+					if (!values.add(next.toString())) {
+						ourLog.info("Found duplicate tag on resource of type {}", next.getResource().getResourceType());
+						ourLog.info("Tag was: {} / {}", next.getTag().getSystem(), next.getTag().getCode());
+					}
+				}
+			}
+		});
+		
 	}
 
 	@Test

src/changes/changes.xml

@@ -102,6 +102,14 @@
 				Fix a regression in HAPI FHIR 2.5 JPA server where executing a search in a
 				transaction or batch operation caused an exception.
 			</action>
+			<action type="fix">
+				Correct an issue when processing transactions in JPA server where updates and
+				creates to resources with tags caused the tags to be created twice in the
+				database. These duplicates were utomatically filtered upon read so this issue
+				was not user-visible, but it coule occasionally lead to performance issues
+				if a resource containing multiple tags was updated many times via
+				transactions.
+			</action>
 		</release>
 		<release version="2.5" date="2017-06-08">
 			<action type="fix">