Merge branch 'master' of github.com:jamesagnew/hapi-fhir

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">