More cleanup for contained resources

hapi-fhir-base/examples/src/main/java/example/GenericClientExample.java

@@ -24,7 +24,11 @@ public class GenericClientExample {
       IGenericClient client = ctx.newRestfulGenericClient(serverBase);
 
       // Perform a search
-      Bundle results = client.search().forResource(Patient.class).where(Patient.FAMILY.matches().value("duck")).execute();
+      Bundle results = client
+            .search()
+            .forResource(Patient.class)
+            .where(Patient.FAMILY.matches().value("duck"))
+            .execute();
 
       System.out.println("Found " + results.size() + " patients named 'duck'");
       // END SNIPPET: simple
@@ -44,7 +48,12 @@ public class GenericClientExample {
          // Invoke the server create method (and send pretty-printed JSON
          // encoding to the server
          // instead of the default which is non-pretty printed XML)
-         client.create().resource(patient).prettyPrint().encodedJson().execute();
+         client
+            .create()
+            .resource(patient)
+            .prettyPrint()
+            .encodedJson()
+            .execute();
          // END SNIPPET: create
       }
       {
@@ -63,7 +72,10 @@ public class GenericClientExample {
          // Invoke the server create method (and send pretty-printed JSON
          // encoding to the server
          // instead of the default which is non-pretty printed XML)
-         client.update().resource(patient).execute();
+         client
+            .update()
+            .resource(patient)
+            .execute();
          // END SNIPPET: update
       }
       {

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java

@@ -20,6 +20,8 @@ package ca.uhn.fhir.parser;
  * #L%
  */
 
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
 import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
@@ -55,6 +57,14 @@ public abstract class BaseParser implements IParser {
 		myContext = theContext;
 	}
 
+	protected String fixContainedResourceId(String theValue) {
+		if (StringUtils.isNotBlank(theValue)&&theValue.charAt(0)=='#') {
+			return theValue.substring(1);
+		}
+		return theValue;
+	}
+
+	
 	private void containResourcesForEncoding(ContainedResources theContained, IResource theResource, IResource theTarget) {
 		List<ResourceReferenceDt> allElements = myContext.newTerser().getAllPopulatedChildElementsOfType(theResource, ResourceReferenceDt.class);
 
@@ -70,7 +80,7 @@ public abstract class BaseParser implements IParser {
 		for (ResourceReferenceDt next : allElements) {
 			IResource resource = next.getResource();
 			if (resource != null) {
-				if (resource.getId().isEmpty()) {
+				if (resource.getId().isEmpty() || resource.getId().isLocal()) {
 					theContained.addContained(resource);
 				} else {
 					continue;
@@ -191,6 +201,26 @@ public abstract class BaseParser implements IParser {
 		throw new DataFormatException(nextChild + " has no child of type " + type);
 	}
 
+	protected String determineReferenceText(ResourceReferenceDt theRef) {
+		String reference = theRef.getReference().getValue();
+		if (isBlank(reference)) {
+			if (theRef.getResource() != null) {
+				IdDt containedId = getContainedResources().getResourceId(theRef.getResource());
+				if (containedId != null && !containedId.isEmpty()) {
+					if (containedId.isLocal()) {
+						reference = containedId.getValue();
+					} else {
+						reference = "#" + containedId.getValue();
+					}
+				} else if (theRef.getResource().getId() != null && theRef.getResource().getId().hasIdPart()) {
+					reference = theRef.getResource().getId().getValue();
+				}
+			}
+		}
+		return reference;
+	}
+
+	
 	static class ContainedResources {
 		private long myNextContainedId = 1;
 
@@ -201,11 +231,16 @@ public abstract class BaseParser implements IParser {
 			if (myResourceToId.containsKey(theResource)) {
 				return;
 			}
-
+			
-			// TODO: make this configurable between the two below (and something else?)
+			IdDt newId;
-			IdDt newId = new IdDt(myNextContainedId++);
+			if (theResource.getId().isLocal()) {
-			// newId = new IdDt(UUID.randomUUID().toString());
+				newId = theResource.getId();
-
+			} else {
+				// TODO: make this configurable between the two below (and something else?)
+				// newId = new IdDt(UUID.randomUUID().toString());
+				newId = new IdDt(myNextContainedId++);
+			}
+			
 			myResourceToId.put(theResource, newId);
 			myResources.add(theResource);
 		}

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java

@@ -284,30 +284,14 @@ public class JsonParser extends BaseParser implements IParser {
 		}
 		case RESOURCE_REF: {
 			ResourceReferenceDt referenceDt = (ResourceReferenceDt) theValue;
-			IdDt value = referenceDt.getReference();
 			if (theChildName != null) {
 				theWriter.writeStartObject(theChildName);
 			} else {
 				theWriter.writeStartObject();
 			}
 
-			String reference = value.getValue();
+			String reference = determineReferenceText(referenceDt); 
-			if (StringUtils.isBlank(reference)) {
+					
-				if (value.getResourceType() != null && StringUtils.isNotBlank(value.getIdPart())) {
-					reference = myContext.getResourceDefinition(value.getResourceType()).getName() + '/' + value.getIdPart();
-				}
-			}
-			if (StringUtils.isBlank(reference)) {
-				if (referenceDt.getResource() != null) {
-					IdDt containedId = getContainedResources().getResourceId(referenceDt.getResource());
-					if (containedId != null) {
-						reference = "#" + containedId.getValue();
-					} else if (referenceDt.getResource().getId() != null && referenceDt.getResource().getId().hasIdPart()) {
-						reference = referenceDt.getResource().getId().getValue();
-					}
-				}
-			}
-
 			if (StringUtils.isNotBlank(reference)) {
 				theWriter.write(XmlParser.RESREF_REFERENCE, reference);
 			}
@@ -321,11 +305,14 @@ public class JsonParser extends BaseParser implements IParser {
 			theWriter.writeStartArray(theChildName);
 			ContainedDt value = (ContainedDt) theValue;
 			for (IResource next : value.getContainedResources()) {
-				encodeResourceToJsonStreamWriter(theResDef, next, theWriter, null, true);
+				if (getContainedResources().getResourceId(next)!=null) {
+					continue;
+				}
+				encodeResourceToJsonStreamWriter(theResDef, next, theWriter, null, true, fixContainedResourceId(next.getId().getValue()));
 			}
 			for (IResource next : getContainedResources().getContainedResources()) {
 				IdDt resourceId = getContainedResources().getResourceId(next);
-				encodeResourceToJsonStreamWriter(theResDef, next, theWriter, null, true, resourceId.getValue());
+				encodeResourceToJsonStreamWriter(theResDef, next, theWriter, null, true, fixContainedResourceId(resourceId.getValue()));
 			}
 			theWriter.writeEnd();
 			break;

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java

@@ -828,8 +828,12 @@ class ParserState<T> {
 				ourLog.debug("Discarding contained resource with no ID!");
 			} else {
 				getPreResourceState().getContainedResources().put(res.getId().getValueAsString(), res);
+				if (!res.getId().isLocal()) {
+					res.setId(new IdDt('#' + res.getId().getIdPart()));
+				}
 			}
 			getPreResourceState().getCurrentElement().getContained().getContainedResources().add(res);
+			
 		}
 
 	}

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java

@@ -467,11 +467,14 @@ public class XmlParser extends BaseParser implements IParser {
 			ContainedDt value = (ContainedDt) nextValue;
 			theEventWriter.writeStartElement("contained");
 			for (IResource next : value.getContainedResources()) {
-				encodeResourceToXmlStreamWriter(next, theEventWriter, true);
+				if (getContainedResources().getResourceId(next)!=null) {
+					continue;
+				}
+				encodeResourceToXmlStreamWriter(next, theEventWriter, true, fixContainedResourceId(next.getId().getValue()));
 			}
 			for (IResource next : getContainedResources().getContainedResources()) {
 				IdDt resourceId = getContainedResources().getResourceId(next);
-				encodeResourceToXmlStreamWriter(next, theEventWriter, true, resourceId.getValue());
+				encodeResourceToXmlStreamWriter(next, theEventWriter, true, fixContainedResourceId(resourceId.getValue()));
 			}
 			theEventWriter.writeEndElement();
 			break;
@@ -494,6 +497,7 @@ public class XmlParser extends BaseParser implements IParser {
 
 	}
 
+
 	private void encodeCompositeElementChildrenToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, IElement theElement, XMLStreamWriter theEventWriter,
 			List<? extends BaseRuntimeChildDefinition> children, boolean theIncludedResource) throws XMLStreamException, DataFormatException {
 		for (BaseRuntimeChildDefinition nextChild : children) {
@@ -569,23 +573,7 @@ public class XmlParser extends BaseParser implements IParser {
 	}
 
 	private void encodeResourceReferenceToStreamWriter(XMLStreamWriter theEventWriter, ResourceReferenceDt theRef) throws XMLStreamException {
-		String reference = theRef.getReference().getValue();
+		String reference = determineReferenceText(theRef);
-		// if (StringUtils.isBlank(reference)) {
-		// if (theRef.getResourceType() != null && StringUtils.isNotBlank(theRef.getResourceId())) {
-		// reference = myContext.getResourceDefinition(theRef.getResourceType()).getName() + '/' + theRef.getResourceId();
-		// }
-		// }
-
-		if (isBlank(reference)) {
-			if (theRef.getResource() != null) {
-				IdDt containedId = getContainedResources().getResourceId(theRef.getResource());
-				if (containedId != null) {
-					reference = "#" + containedId.getValue();
-				} else if (theRef.getResource().getId() != null && theRef.getResource().getId().hasIdPart()) {
-					reference = theRef.getResource().getId().getValue();
-				}
-			}
-		}
 		
 		if (StringUtils.isNotBlank(reference)) {
 			theEventWriter.writeStartElement(RESREF_REFERENCE);
@@ -599,6 +587,7 @@ public class XmlParser extends BaseParser implements IParser {
 		}
 	}
 
+
 	private void encodeResourceToXmlStreamWriter(IResource theResource, XMLStreamWriter theEventWriter, boolean theIncludedResource) throws XMLStreamException, DataFormatException {
 		String resourceId = null;
 		if (theIncludedResource && StringUtils.isNotBlank(theResource.getId().getValue())) {

hapi-fhir-base/src/test/java/ca/uhn/fhir/model/primitive/IdDtTest.java

@@ -17,6 +17,15 @@ public class IdDtTest {
 
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(IdDtTest.class);
 
+	@Test
+	public void testDetectLocal() {
+		
+		IdDt id = new IdDt("#123");
+		assertEquals("#123", id.getValue());
+		assertTrue(id.isLocal());
+		
+	}
+	
 	@Test
 	public void testDetermineBase() {
 

hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java

@@ -463,7 +463,7 @@ public class JsonParserTest {
 	}
 
 	@Test
-	public void testEncodeContained() {
+	public void testEncodeContained__() {
 		// Create an organization
 		Organization org = new Organization();
 		org.getName().setValue("Contained Test Organization");
@@ -478,7 +478,7 @@ public class JsonParserTest {
 		List<IResource> resources = new ArrayList<IResource>();
 		resources.add(patient);
 		Bundle b = Bundle.withResources(resources, ourCtx, "http://example.com/base");
-		
+				
 		// Encode the buntdle
 		String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeBundleToString(b);
 		ourLog.info(encoded);
@@ -489,9 +489,73 @@ public class JsonParserTest {
 		ourLog.info(encoded);
 		assertThat(encoded, stringContainsInOrder(Arrays.asList("\"contained\"", "resourceType\":\"Organization", "id\":\"1\"")));
 		assertThat(encoded, containsString("reference\":\"#1\""));
+	}
+	
+	@Test
+	public void testEncodeContained() {
+		IParser xmlParser = 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");
+
+		// Create a patient
+		Patient patient = new Patient();
+		patient.setId("Patient/1333");
+		patient.addIdentifier("urn:mrns", "253345");
+		
+		// Put the organization as a reference in the patient resource
+		patient.getManagingOrganization().setResource(org);
+		
+		String encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("\"contained\":[", "\"id\":\"1\"", "\"identifier\"", "\"reference\":\"#1\"")));
+		
+		// Create a bundle with just the patient resource
+		List<IResource> resources = new ArrayList<IResource>();
+		resources.add(patient);
+		Bundle b = Bundle.withResources(resources, ourCtx, "http://example.com/base");
+		
+		// Encode the bundle
+		encoded = xmlParser.encodeBundleToString(b);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("\"contained\":[", "\"id\":\"1\"", "\"identifier\"", "\"reference\":\"#1\"")));
+		
+		// Re-parse the bundle
+		patient = (Patient) xmlParser.parseResource(xmlParser.encodeResourceToString(patient));
+		assertEquals("#1", patient.getManagingOrganization().getReference().getValue());
+		
+		assertNotNull(patient.getManagingOrganization().getResource());
+		org = (Organization) patient.getManagingOrganization().getResource();
+		assertEquals("#1", org.getId().getValue());
+		assertEquals("Contained Test Organization", org.getName().getValue());
+		
+		// And re-encode a second time
+		encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("\"contained\":[", "\"id\":\"1\"", "\"identifier\"", "\"reference\":\"#1\"")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("\"contained\":", "[", "\"contained\":"))));
+
+		// And re-encode once more, with the references cleared
+		patient.getContained().getContainedResources().clear();
+		patient.getManagingOrganization().setReference((IdDt)null);
+		encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("\"contained\":[", "\"id\":\"1\"", "\"identifier\"", "\"reference\":\"#1\"")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("\"contained\":", "[", "\"contained\":"))));
+
+		// And re-encode once more, with the references cleared and a manually set local ID
+		patient.getContained().getContainedResources().clear();
+		patient.getManagingOrganization().setReference((IdDt)null);
+		patient.getManagingOrganization().getResource().setId(new IdDt("#333"));
+		encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("\"contained\":[", "\"id\":\"333\"", "\"identifier\"", "\"reference\":\"#333\"")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("\"contained\":", "[", "\"contained\":"))));
 		
 	}
+	
+	
 
 	
 	@Test

hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java

@@ -118,6 +118,8 @@ public class XmlParserTest {
 	
 	@Test
 	public void testEncodeContained() {
+		IParser xmlParser = 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");
@@ -130,7 +132,7 @@ public class XmlParserTest {
 		// Put the organization as a reference in the patient resource
 		patient.getManagingOrganization().setResource(org);
 		
-		String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(patient);
+		String encoded = xmlParser.encodeResourceToString(patient);
 		ourLog.info(encoded);
 		assertThat(encoded, containsString("<contained>"));
 		assertThat(encoded, containsString("<reference value=\"#1\"/>"));
@@ -141,11 +143,46 @@ public class XmlParserTest {
 		Bundle b = Bundle.withResources(resources, ourCtx, "http://example.com/base");
 		
 		// Encode the buntdle
-		encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeBundleToString(b);
+		encoded = xmlParser.encodeBundleToString(b);
 		ourLog.info(encoded);
-		assertThat(encoded, containsString("<contained>"));
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("<contained>","id=\"1\"", "</contained>")));
 		assertThat(encoded, containsString("<reference value=\"#1\"/>"));
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("<entry>", "</entry>")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("<entry>", "</entry>", "<entry>"))));
 		
+		// Re-parse the bundle
+		patient = (Patient) xmlParser.parseResource(xmlParser.encodeResourceToString(patient));
+		assertEquals("#1", patient.getManagingOrganization().getReference().getValue());
+		
+		assertNotNull(patient.getManagingOrganization().getResource());
+		org = (Organization) patient.getManagingOrganization().getResource();
+		assertEquals("#1", org.getId().getValue());
+		assertEquals("Contained Test Organization", org.getName().getValue());
+		
+		// And re-encode a second time
+		encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("<contained>", "<Organization ", "id=\"1\"", "</Organization", "</contained>", "<reference value=\"#1\"/>")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("<contained>",  "<Org", "<contained>"))));
+		assertThat(encoded, containsString("<reference value=\"#1\"/>"));
+
+		// And re-encode once more, with the references cleared
+		patient.getContained().getContainedResources().clear();
+		patient.getManagingOrganization().setReference((IdDt)null);
+		encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("<contained>", "<Organization ", "id=\"1\"", "</Organization", "</contained>", "<reference value=\"#1\"/>")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("<contained>",  "<Org", "<contained>"))));
+		assertThat(encoded, containsString("<reference value=\"#1\"/>"));
+
+		// And re-encode once more, with the references cleared and a manually set local ID
+		patient.getContained().getContainedResources().clear();
+		patient.getManagingOrganization().setReference((IdDt)null);
+		patient.getManagingOrganization().getResource().setId(new IdDt("#333"));
+		encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, stringContainsInOrder(Arrays.asList("<contained>", "<Organization ", "id=\"333\"", "</Organization", "</contained>", "<reference value=\"#333\"/>")));
+		assertThat(encoded, not(stringContainsInOrder(Arrays.asList("<contained>",  "<Org", "<contained>"))));
 		
 	}
 	

hapi-fhir-base/src/test/resources/logback-test.xml

@@ -2,7 +2,7 @@
 
 	<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
 		<encoder>
-			<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n
+			<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} [%file:%line] - %msg%n
 			</pattern>
 		</encoder>
 	</appender>