Fix #170 - Better addXXX methods in structures. Also don't incorrectly

include IDs in client create requests, and add better getLink methods
to the bew Bundle resource structure.

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

@@ -65,6 +65,7 @@ public abstract class BaseParser implements IParser {
 
 	private ContainedResources myContainedResources;
 	private FhirContext myContext;
+	private boolean myOmitResourceId;
 	private String myServerBaseUrl;
 	private boolean myStripVersionsFromReferences = true;
 	private boolean mySuppressNarratives;
@@ -259,6 +260,10 @@ public abstract class BaseParser implements IParser {
 				&& theIncludedResource == false;
 	}
 
+	public boolean isOmitResourceId() {
+		return myOmitResourceId;
+	}
+
 	@Override
 	public boolean isStripVersionsFromReferences() {
 		return myStripVersionsFromReferences;
@@ -345,6 +350,11 @@ public abstract class BaseParser implements IParser {
 		return parseTagList(new StringReader(theString));
 	}
 
+	public BaseParser setOmitResourceId(boolean theOmitResourceId) {
+		myOmitResourceId = theOmitResourceId;
+		return this;
+	}
+
 	@Override
 	public IParser setServerBaseUrl(String theUrl) {
 		myServerBaseUrl = isNotBlank(theUrl) ? theUrl : null;

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

@@ -186,7 +186,7 @@ public interface IParser {
 	 * 
 	 * @param theStripVersionsFromReferences Set this to <code>false<code> to prevent the parser from removing
 	 * resource versions from references.
-	 * @return Returns an instance of <code>this</code> parser so that method calls can be chained together
+	 * @return Returns a reference to <code>this</code> parser so that method calls can be chained together
 	 */
 	IParser setStripVersionsFromReferences(boolean theStripVersionsFromReferences);
 	
@@ -201,4 +201,23 @@ public interface IParser {
 	 */
 	boolean isStripVersionsFromReferences();
 
+	/**
+	 * If set to <code>true</code> (default is <code>false</code>) the ID of any resources being encoded
+	 * will not be included in the output. Note that this does not apply to contained resources, only to root resources.
+	 * In other words, if this is set to <code>true</code>, contained resources will still have local IDs but the
+	 * outer/containing ID will not have an ID.
+	 * 
+	 * @param theOmitResourceId Should resource IDs be omitted
+	 * @return Returns a reference to <code>this</code> parser so that method calls can be chained together
+	 * @since 1.1
+	 */
+	IParser setOmitResourceId(boolean theOmitResourceId);
+
+	/**
+	 * Returns true if resource IDs should be omitted
+	 * @see #setOmitResourceId(boolean)
+	 * @since 1.1
+	 */
+	boolean isOmitResourceId();
+
 }

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

@@ -683,6 +683,10 @@ public class JsonParser extends BaseParser implements IParser {
 			}
 		}
 
+		if (isOmitResourceId() && !theContainedResource) {
+			resourceId = null;
+		}
+		
 		encodeResourceToJsonStreamWriter(theResDef, theResource, theEventWriter, theObjectNameOrNull, theContainedResource, resourceId);
 	}
 

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

@@ -757,6 +757,10 @@ public class XmlParser extends BaseParser implements IParser {
 			}
 		}
 
+		if (isOmitResourceId() && !theIncludedResource) {
+			resourceId = null;
+		}
+		
 		encodeResourceToXmlStreamWriter(theResource, theEventWriter, theIncludedResource, resourceId);
 	}
 

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/BaseHttpClientInvocationWithContents.java

@@ -63,6 +63,7 @@ abstract class BaseHttpClientInvocationWithContents extends BaseHttpClientInvoca
 	private final FhirContext myContext;
 	private Map<String, List<String>> myIfNoneExistParams;
 	private String myIfNoneExistString;
+	private boolean myOmitResourceId = false;
 	private Map<String, List<String>> myParams;
 	private final IBaseResource myResource;
 	private final List<? extends IBaseResource> myResources;
@@ -234,6 +235,8 @@ abstract class BaseHttpClientInvocationWithContents extends BaseHttpClientInvoca
 			encoding = EncodingEnum.XML;
 			parser = myContext.newXmlParser();
 		}
+		
+		parser.setOmitResourceId(myOmitResourceId);
 
 		AbstractHttpEntity entity;
 		if (myParams != null) {
@@ -303,6 +306,7 @@ abstract class BaseHttpClientInvocationWithContents extends BaseHttpClientInvoca
 	}
 
 	protected abstract HttpRequestBase createRequest(StringBuilder theUrl, AbstractHttpEntity theEntity);
+
 	private StringBuilder newHeaderBuilder(StringBuilder theUrlBase) {
 		StringBuilder b = new StringBuilder();
 		b.append(theUrlBase);
@@ -311,7 +315,6 @@ abstract class BaseHttpClientInvocationWithContents extends BaseHttpClientInvoca
 		}
 		return b;
 	}
-
 	public void setIfNoneExistParams(Map<String, List<String>> theIfNoneExist) {
 		myIfNoneExistParams = theIfNoneExist;
 	}
@@ -320,4 +323,8 @@ abstract class BaseHttpClientInvocationWithContents extends BaseHttpClientInvoca
 		myIfNoneExistString = theIfNoneExistString;
 	}
 
+	public void setOmitResourceId(boolean theOmitResourceId) {
+		myOmitResourceId = theOmitResourceId;
+	}
+
 }

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/MethodUtil.java

@@ -1,6 +1,7 @@
 package ca.uhn.fhir.rest.method;
 
-import static org.apache.commons.lang3.StringUtils.*;
+import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 import java.io.IOException;
 import java.io.PushbackReader;
@@ -24,10 +25,10 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.http.client.utils.DateUtils;
-import org.hl7.fhir.instance.model.api.IBaseResource;
-import org.hl7.fhir.instance.model.api.IRefImplResource;
-import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.instance.model.api.IBaseMetaType;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.instance.model.api.IIdType;
+import org.hl7.fhir.instance.model.api.IRefImplResource;
 
 import ca.uhn.fhir.context.ConfigurationException;
 import ca.uhn.fhir.context.FhirContext;
@@ -136,11 +137,17 @@ public class MethodUtil {
 		StringBuilder urlExtension = new StringBuilder();
 		urlExtension.append(resourceName);
 
-		if (StringUtils.isNotBlank(theId)) {
+		boolean dstu1 = theContext.getVersion().getVersion().equals(FhirVersionEnum.DSTU1);
-			urlExtension.append('/');
+		if (dstu1) {
-			urlExtension.append(theId);
+			/*
+			 * This was allowable at one point, but as of DSTU2 it isn't.
+			 */
+			if (StringUtils.isNotBlank(theId)) {
+				urlExtension.append('/');
+				urlExtension.append(theId);
+			}
 		}
-
+		
 		HttpPostClientInvocation retVal;
 		if (StringUtils.isBlank(theResourceBody)) {
 			retVal = new HttpPostClientInvocation(theContext, theResource, urlExtension.toString());
@@ -149,6 +156,9 @@ public class MethodUtil {
 		}
 		addTagsToPostOrPut(theContext, theResource, retVal);
 
+		if (!dstu1) {
+			retVal.setOmitResourceId(true);
+		}
 		// addContentTypeHeaderBasedOnDetectedType(retVal, theResourceBody);
 
 		return retVal;

hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IBaseBundle.java

@@ -23,4 +23,25 @@ package org.hl7.fhir.instance.model.api;
 
 public interface IBaseBundle extends IBaseResource {
 
+	/**
+	 * Constant for links provided in the bundle. This constant is used in the
+	 * link.type field to indicate that the given link is for
+	 * the next page of results.  
+	 */
+	public static final String LINK_NEXT = "next";
+	
+	/**
+	 * Constant for links provided in the bundle. This constant is used in the
+	 * link.type field to indicate that the given link is for
+	 * the previous page of results.  
+	 */
+	public static final String LINK_PREV = "prev";
+
+	/**
+	 * Constant for links provided in the bundle. This constant is used in the
+	 * link.type field to indicate that the given link is for
+	 * this bundle.  
+	 */
+	public static final String LINK_SELF = "self";
+	
 }

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

@@ -75,8 +75,7 @@ public class XmlParserTest {
 
 	private static FhirContext ourCtx;
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(XmlParserTest.class);
-
+	
-
 	/**
 	 * see #144 and #146
 	 */

hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java

@@ -152,15 +152,15 @@ public class SearchTest {
 		IOUtils.closeQuietly(status.getEntity().getContent());
 		assertEquals(200, status.getStatusLine().getStatusCode());
 		Bundle bundle = ourCtx.newXmlParser().parseBundle(responseContent);
-		assertEquals(1, bundle.getEntries().size());
+		assertEquals(10, bundle.getEntries().size());
 
 		Patient p = bundle.getResources(Patient.class).get(0);
 		assertEquals("AAANamed", p.getIdentifierFirstRep().getValue().getValue());
 		assertEquals("http://foo/Patient?_id=1", bundle.getEntries().get(0).getLinkSearch().getValue());
-		assertEquals("http://localhost:" + ourPort + "/Patient/9988", bundle.getEntries().get(0).getLinkAlternate().getValue());
+		assertEquals("http://localhost:" + ourPort + "/Patient/99881", bundle.getEntries().get(0).getLinkAlternate().getValue());
 
 		assertEquals("http://foo/Patient?_id=1", ResourceMetadataKeyEnum.LINK_SEARCH.get(p));
-		assertEquals("http://localhost:" + ourPort + "/Patient/9988", ResourceMetadataKeyEnum.LINK_ALTERNATE.get(p));
+		assertEquals("http://localhost:" + ourPort + "/Patient/99881", ResourceMetadataKeyEnum.LINK_ALTERNATE.get(p));
 
 	}
 

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/model/dstu2/BundleTest.java

@@ -0,0 +1,28 @@
+package ca.uhn.fhir.model.dstu2;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+import ca.uhn.fhir.model.dstu2.resource.Bundle;
+import ca.uhn.fhir.model.dstu2.resource.Bundle.Link;
+
+public class BundleTest {
+
+	@Test
+	public void testGetLink() {
+		
+		Bundle b = new Bundle();
+		Link link = b.getLink(Bundle.LINK_NEXT);
+		
+		assertNull(link);
+		
+		Link link2 = b.getLinkOrCreate(Bundle.LINK_NEXT);
+		link = b.getLink(Bundle.LINK_NEXT);
+		
+		assertNotNull(link);
+		assertNotNull(link2);
+		assertSame(link, link2);
+	}
+	
+}

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/narrative/DefaultThymeleafNarrativeGeneratorTestDstu2.java

@@ -53,7 +53,7 @@ public class DefaultThymeleafNarrativeGeneratorTestDstu2 {
 		Patient value = new Patient();
 
 		value.addIdentifier().setSystem("urn:names").setValue("123456");
-		value.addName().addFamily("blow").addGiven("joe").addGiven(null).addGiven("john");
+		value.addName().addFamily("blow").addGiven("joe").addGiven((String)null).addGiven("john");
 		value.getAddressFirstRep().addLine("123 Fake Street").addLine("Unit 1");
 		value.getAddressFirstRep().setCity("Toronto").setState("ON").setCountry("Canada");
 

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java

@@ -53,6 +53,19 @@ public class JsonParserDstu2Test {
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(JsonParserDstu2Test.class);
 	private static final FhirContext ourCtx = FhirContext.forDstu2();
 
+	@Test
+	public void testOmitResourceId() {
+		Patient p = new Patient();
+		p.setId("123");
+		p.addName().addFamily("ABC");
+		
+		assertThat(ourCtx.newJsonParser().encodeResourceToString(p), stringContainsInOrder("123", "ABC"));
+		assertThat(ourCtx.newJsonParser().setOmitResourceId(true).encodeResourceToString(p), containsString("ABC"));
+		assertThat(ourCtx.newJsonParser().setOmitResourceId(true).encodeResourceToString(p), not(containsString("123")));
+	}
+
+
+	
 	@Test
 	public void testParseBundleWithBinary() {
 		Binary patient = new Binary();

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java

@@ -67,6 +67,18 @@ public class XmlParserDstu2Test {
 		XMLUnit.setIgnoreWhitespace(true);
 	}
 
+	@Test
+	public void testOmitResourceId() {
+		Patient p = new Patient();
+		p.setId("123");
+		p.addName().addFamily("ABC");
+		
+		assertThat(ourCtx.newXmlParser().encodeResourceToString(p), stringContainsInOrder("123", "ABC"));
+		assertThat(ourCtx.newXmlParser().setOmitResourceId(true).encodeResourceToString(p), containsString("ABC"));
+		assertThat(ourCtx.newXmlParser().setOmitResourceId(true).encodeResourceToString(p), not(containsString("123")));
+	}
+
+	
 	@Test
 	public void testEncodeExtensionWithResourceContent() {
 		IParser parser = ourCtx.newXmlParser();

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu2Test.java

@@ -788,6 +788,48 @@ public class GenericClientDstu2Test {
 
 	}
 
+	@Test
+	public void testCreate() throws Exception {
+		ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
+		when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse);
+		when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), Constants.STATUS_HTTP_204_NO_CONTENT, ""));
+		when(myHttpResponse.getEntity().getContent()).then(new Answer<ReaderInputStream>() {
+			@Override
+			public ReaderInputStream answer(InvocationOnMock theInvocation) throws Throwable {
+				return new ReaderInputStream(new StringReader(""), Charset.forName("UTF-8"));
+			}
+		});
+
+		IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
+
+		int idx = 0;
+
+		Patient p = new Patient();
+		p.addName().addFamily("FOOFAMILY");
+
+		client.create().resource(p).execute();
+		
+		assertEquals(1, capt.getAllValues().get(idx).getHeaders(Constants.HEADER_CONTENT_TYPE).length);
+		assertEquals(EncodingEnum.XML.getResourceContentType() + Constants.HEADER_SUFFIX_CT_UTF_8, capt.getAllValues().get(idx).getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue());
+		assertThat(extractBody(capt, idx), containsString("<family value=\"FOOFAMILY\"/>"));
+		assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(idx).getURI().toString());
+		assertEquals("POST", capt.getAllValues().get(idx).getRequestLine().getMethod());
+		idx++;
+
+		p.setId("123");
+		
+		client.create().resource(p).execute();
+		assertEquals(1, capt.getAllValues().get(idx).getHeaders(Constants.HEADER_CONTENT_TYPE).length);
+		assertEquals(EncodingEnum.XML.getResourceContentType() + Constants.HEADER_SUFFIX_CT_UTF_8, capt.getAllValues().get(idx).getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue());
+		String body = extractBody(capt, idx);
+		assertThat(body, containsString("<family value=\"FOOFAMILY\"/>"));
+		assertThat(body, not(containsString("123")));
+		assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(idx).getURI().toString());
+		assertEquals("POST", capt.getAllValues().get(idx).getRequestLine().getMethod());
+		idx++;
+
+	}
+
 	@Test
 	public void testUpdateConditional() throws Exception {
 		ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/OperationClientTest.java

@@ -1,7 +1,8 @@
 package ca.uhn.fhir.rest.client;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.io.InputStream;
 import java.io.StringReader;
@@ -51,7 +52,7 @@ public class OperationClientTest {
 
 		ourHttpClient = mock(HttpClient.class, new ReturnsDeepStubs());
 		ourCtx.getRestfulClientFactory().setHttpClient(ourHttpClient);
-		ourCtx.getRestfulClientFactory().setServerValidationModeEnum(ServerValidationModeEnum.NEVER);
+		ourCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER);
 
 		ourHttpResponse = mock(HttpResponse.class, new ReturnsDeepStubs());
 	}
@@ -228,7 +229,6 @@ public class OperationClientTest {
 		assertEquals(0, request.getParameter().size());
 		idx++;
 
-
 	}
 
 	@Test

hapi-tinder-plugin/src/main/resources/vm/resource.vm

@@ -113,5 +113,54 @@ public class ${className} extends ca.uhn.fhir.model.${version}.resource.BaseReso
 	}
 
 #end
+#if ( ${className} ==  "Bundle" )
+	/**
+	 * Returns the {@link ${hash}getLink() link} which matches a given {@link Link${hash}getRelation() relation}. 
+	 * If no link is found which matches the given relation, returns <code>null</code>. If more than one
+	 * link is found which matches the given relation, returns the first matching Link.
+	 * 
+	 * @param theRelation
+	 *            The relation, such as "next", or "self. See the constants such as {@link IBaseBundle#LINK_SELF} and {@link IBaseBundle#LINK_NEXT}.
+	 * @return Returns a matching Link, or <code>null</code>
+	 * @see IBaseBundle#LINK_NEXT
+	 * @see IBaseBundle#LINK_PREV
+	 * @see IBaseBundle#LINK_SELF
+	 */
+	public Link getLink(String theRelation) {
+		org.apache.commons.lang3.Validate.notBlank(theRelation, "theRelation may not be null or empty");
+		for (Link next : getLink()) {
+			if (theRelation.equals(next.getRelation())) {
+				return next;
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * Returns the {@link ${hash}getLink() link} which matches a given {@link Link${hash}getRelation() relation}. 
+	 * If no link is found which matches the given relation, creates a new Link with the
+	 * given relation and adds it to this Bundle. If more than one
+	 * link is found which matches the given relation, returns the first matching Link.
+	 * 
+	 * @param theRelation
+	 *            The relation, such as "next", or "self. See the constants such as {@link IBaseBundle#LINK_SELF} and {@link IBaseBundle#LINK_NEXT}.
+	 * @return Returns a matching Link, or <code>null</code>
+	 * @see IBaseBundle#LINK_NEXT
+	 * @see IBaseBundle#LINK_PREV
+	 * @see IBaseBundle#LINK_SELF
+	 */
+	public Link getLinkOrCreate(String theRelation) {
+		org.apache.commons.lang3.Validate.notBlank(theRelation, "theRelation may not be null or empty");
+		for (Link next : getLink()) {
+			if (theRelation.equals(next.getRelation())) {
+				return next;
+			}
+		}
+		Link retVal = new Link();
+		retVal.setRelation(theRelation);
+		getLink().add(retVal);
+		return retVal;
+	}
+#end
 
 }

hapi-tinder-plugin/src/main/resources/vm/templates.vm

@@ -157,6 +157,23 @@
 		return newType; 
 	}
 
+	/**
+	 * Adds a given new value for <b>${child.elementName}</b> ($esc.html(${child.shortName}))
+	 *
+	 * <p>
+	 * <b>Definition:</b>
+	 * $esc.html(${child.definition})
+	 * </p>
+	 * @param theValue The ${child.elementName} to add (must not be <code>null</code>)
+	 */
+	public ${child.declaringClassNameComplete} add${child.methodName}(${child.singleType} theValue) {
+		if (theValue == null) {
+			throw new NullPointerException("theValue must not be null");
+		}
+		get${child.methodName}().add(theValue);
+		return this;
+	}
+
 	/**
 	 * Gets the first repetition for <b>${child.elementName}</b> ($esc.html(${child.shortName})),
 	 * creating it if it does not already exist.

src/changes/changes.xml

@@ -27,6 +27,15 @@
 				if the server detects that the request is coming from a browser. This interceptor has been added
 				to fhirtest.uhn.ca responses.
 			</action>
+			<action type="fix">
+				Performing a create operation in a client used an incorrect URL if the
+				resource had an ID set. ID should be ignored for creates. Thanks to 
+				Peter Girard for reporting!
+			</action>
+			<action type="add" issue="170">
+				Add better addXXX() methods to structures, which take the datatype being added as a parameter. Thanks to Claude Nanjo for the
+				suggestion!
+			</action>
 		</release>
 		<release version="1.0" date="2015-May-8">
 			<action type="add">