From 287162af914231aa5d994813de9a4d1ea64f0196 Mon Sep 17 00:00:00 2001
From: James Agnew <jamesagnew@gmail.com>
Date: Wed, 15 Oct 2014 16:46:28 -0400
Subject: [PATCH] Fix #33 - Server was incorrectly returning contained
 resources as top-level resources

---
 hapi-fhir-base/src/changes/changes.xml        |  5 +++
 .../java/ca/uhn/fhir/parser/JsonParser.java   |  2 +
 .../java/ca/uhn/fhir/parser/XmlParser.java    |  2 +
 .../uhn/fhir/rest/server/RestfulServer.java   | 12 +++++
 .../ca/uhn/fhir/parser/JsonParserTest.java    | 25 ++++++++++-
 .../ca/uhn/fhir/parser/XmlParserTest.java     | 44 ++++++++++++++++---
 .../test/CompleteResourceProviderTest.java    | 22 ++++++++++
 7 files changed, 105 insertions(+), 7 deletions(-)

diff --git a/hapi-fhir-base/src/changes/changes.xml b/hapi-fhir-base/src/changes/changes.xml
index b44974df3b6..7dfd74859e7 100644
--- a/hapi-fhir-base/src/changes/changes.xml
+++ b/hapi-fhir-base/src/changes/changes.xml
@@ -104,6 +104,11 @@
 				Documentation on contained resources contained a typo and did not actually produce contained resources. Thanks
 				to David Hay of Orion Health for reporting!
 			</action>			
+			<action type="fix" issue="33">
+				Server was incorrectly including contained resources being returned as both contained resources, and as 
+				top-level resources in the returned bundle for search operations.
+				Thanks to Bill de Beaubien for reporting!
+			</action>			
 		</release>
 		<release version="0.6" date="2014-Sep-08" description="This release brings a number of new features and bug fixes!">
 			<!-- 
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
index a97fe73a871..cf2498e8766 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
@@ -411,6 +411,8 @@ public class JsonParser extends BaseParser implements IParser {
 							theEventWriter.writeStartArray(childName);
 							inArray = true;
 							encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, nextValue, childDef, null, theIsSubElementWithinResource);
+						} else if (nextChild instanceof RuntimeChildNarrativeDefinition && theIsSubElementWithinResource) {
+							// suppress narratives from contained resources
 						} else {
 							encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, nextValue, childDef, childName, theIsSubElementWithinResource);
 						}
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java
index 28dd8d919b2..e427aa2d827 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java
@@ -549,6 +549,8 @@ public class XmlParser extends BaseParser implements IParser {
 					theEventWriter.writeAttribute("url", extensionUrl);
 					encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, nextValue, childName, childDef, null, theIncludedResource);
 					theEventWriter.writeEndElement();
+				} else if (nextChild instanceof RuntimeChildNarrativeDefinition && theIncludedResource) {
+					// suppress narratives from contained resources
 				} else {
 					encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, nextValue, childName, childDef, extensionUrl, theIncludedResource);
 				}
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java
index 289713eec35..e7cb8f347a4 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java
@@ -1035,6 +1035,13 @@ public class RestfulServer extends HttpServlet {
 		Set<IdDt> addedResourceIds = new HashSet<IdDt>();
 		for (IResource next : theResult) {
 
+			Set<String> containedIds = new HashSet<String>();
+			for (IResource nextContained : next.getContained().getContainedResources()) {
+				if (nextContained.getId().isEmpty()==false) {
+					containedIds.add(nextContained.getId().getValue());
+				}
+			}
+			
 			if (theContext.getNarrativeGenerator() != null) {
 				String title = theContext.getNarrativeGenerator().generateTitle(next);
 				ourLog.trace("Narrative generator created title: {}", title);
@@ -1053,6 +1060,11 @@ public class RestfulServer extends HttpServlet {
 					IResource nextRes = nextRef.getResource();
 					if (nextRes != null) {
 						if (nextRes.getId().hasIdPart()) {
+							if (containedIds.contains(nextRes.getId().getValue())) {
+								// Don't add contained IDs as top level resources
+								continue;
+							}
+							
 							IdDt id = nextRes.getId().toVersionless();
 							if (id.hasResourceType() == false) {
 								String resName = theContext.getResourceDefinition(nextRes).getName();
diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java
index d723c9e28b4..eb5b764a97d 100644
--- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java
+++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java
@@ -491,6 +491,29 @@ public class JsonParserTest {
 		assertThat(encoded, containsString("reference\":\"#1\""));
 	}
 	
+	@Test
+	public void testEncodeContainedWithNarrativeIsSuppresed() {
+		IParser parser = ourCtx.newJsonParser().setPrettyPrint(true);
+
+		// Create an organization, note that the organization does not have an ID
+		Organization org = new Organization();
+		org.getName().setValue("Contained Test Organization");
+		org.getText().setDiv("<div>FOOBAR</div>");
+
+		// Create a patient
+		Patient patient = new Patient();
+		patient.setId("Patient/1333");
+		patient.addIdentifier("urn:mrns", "253345");
+		patient.getText().setDiv("<div>BARFOO</div>");
+		patient.getManagingOrganization().setResource(org);
+
+		String encoded = parser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, not(containsString("FOOBAR")));
+		assertThat(encoded, (containsString("BARFOO")));
+		
+	}	
+	
 	@Test
 	public void testEncodeContained() {
 		IParser xmlParser = ourCtx.newJsonParser().setPrettyPrint(true);
@@ -603,7 +626,7 @@ public class JsonParserTest {
 
 		DiagnosticReport rpt = new DiagnosticReport();
 		Specimen spm = new Specimen();
-		spm.getText().setDiv("AAA");
+		rpt.getText().setDiv("AAA");
 		rpt.addSpecimen().setResource(spm);
 
 		IParser p = new FhirContext(DiagnosticReport.class).newJsonParser().setPrettyPrint(true);
diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java
index ccc3fb155db..ffdfeef5cee 100644
--- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java
+++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java
@@ -1,9 +1,18 @@
 package ca.uhn.fhir.parser;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-import static org.mockito.Matchers.*;
-import static org.mockito.Mockito.*;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.stringContainsInOrder;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -12,7 +21,6 @@ import java.io.StringReader;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
 
@@ -115,6 +123,29 @@ public class XmlParserTest {
 		
 	}
 	
+	@Test
+	public void testEncodeContainedWithNarrativeIsSuppresed() {
+		IParser parser = ourCtx.newXmlParser().setPrettyPrint(true);
+
+		// Create an organization, note that the organization does not have an ID
+		Organization org = new Organization();
+		org.getName().setValue("Contained Test Organization");
+		org.getText().setDiv("<div>FOOBAR</div>");
+
+		// Create a patient
+		Patient patient = new Patient();
+		patient.setId("Patient/1333");
+		patient.addIdentifier("urn:mrns", "253345");
+		patient.getText().setDiv("<div>BARFOO</div>");
+		patient.getManagingOrganization().setResource(org);
+		
+		String encoded = parser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, not(containsString("FOOBAR")));
+		assertThat(encoded, (containsString("BARFOO")));
+		
+	}	
+
 	
 	@Test
 	public void testEncodeContained() {
@@ -388,7 +419,8 @@ public class XmlParserTest {
 
 		DiagnosticReport rpt = new DiagnosticReport();
 		Specimen spm = new Specimen();
-		spm.getText().setDiv("AAA");
+		spm.addIdentifier("urn", "123");
+		rpt.getText().setDiv("AAA");
 		rpt.addSpecimen().setResource(spm);
 
 		IParser p = ourCtx.newXmlParser().setPrettyPrint(true);
diff --git a/hapi-fhir-jpaserver-test/src/test/java/ca/uhn/fhir/jpa/test/CompleteResourceProviderTest.java b/hapi-fhir-jpaserver-test/src/test/java/ca/uhn/fhir/jpa/test/CompleteResourceProviderTest.java
index 43b6be6d257..10b251a3ea9 100644
--- a/hapi-fhir-jpaserver-test/src/test/java/ca/uhn/fhir/jpa/test/CompleteResourceProviderTest.java
+++ b/hapi-fhir-jpaserver-test/src/test/java/ca/uhn/fhir/jpa/test/CompleteResourceProviderTest.java
@@ -174,6 +174,28 @@ public class CompleteResourceProviderTest {
 		assertThat(actual.getText().getDiv().getValueAsString(), containsString("<td>Identifier</td><td>testSearchByResourceChain01</td>"));
 	}
 
+	@Test
+	public void testSaveAndRetrieveWithContained() {
+		Patient p1 = new Patient();
+		p1.addIdentifier().setSystem("urn:system").setValue("testSaveAndRetrieveWithContained01");
+
+		Organization o1 = new Organization();
+		o1.addIdentifier().setSystem("urn:system").setValue("testSaveAndRetrieveWithContained02");
+		
+		p1.getManagingOrganization().setResource(o1);
+		
+		IdDt newId = ourClient.create().resource(p1).execute().getId();
+
+		Patient actual = ourClient.read(Patient.class, newId);
+		assertEquals(1, actual.getContained().getContainedResources().size());
+		assertThat(actual.getText().getDiv().getValueAsString(), containsString("<td>Identifier</td><td>testSaveAndRetrieveWithContained01</td>"));
+		
+		Bundle b = ourClient.search().forResource("Patient").where(Patient.IDENTIFIER.exactly().systemAndCode("urn:system","testSaveAndRetrieveWithContained01")).execute();
+		assertEquals(1, b.size());
+		
+	}
+	
+	
 	@Test
 	public void testSearchByIdentifier() {
 		deleteToken("Patient", Patient.SP_IDENTIFIER, "urn:system", "testSearchByIdentifier01");