Fix #519 - Issues with JPA capabilitystatement related to the _id param

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java

@@ -28,17 +28,8 @@ import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Field;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Properties;
-import java.util.Set;
 
 import org.apache.commons.io.IOUtils;
 import org.hl7.fhir.instance.model.api.IBase;
@@ -408,7 +399,23 @@ class ModelScanner {
 		Map<String, RuntimeSearchParam> nameToParam = new HashMap<String, RuntimeSearchParam>();
 		Map<Field, SearchParamDefinition> compositeFields = new LinkedHashMap<Field, SearchParamDefinition>();
 
-		for (Field nextField : theClass.getFields()) {
+		/*
+		 * Make sure we pick up fields in interfaces too.. This ensures that we
+		 * grab the _id field which generally gets picked up via interface
+		 */
+		Set<Field> fields = new HashSet<Field>(Arrays.asList(theClass.getFields()));
+		Class<?> nextClass = theClass;
+		do {
+			for (Class<?> nextInterface : nextClass.getInterfaces()) {
+				fields.addAll(Arrays.asList(nextInterface.getFields()));
+			}
+			nextClass = nextClass.getSuperclass();
+		} while (nextClass.equals(Object.class) == false);
+		
+		/*
+		 * Now scan the fields for search params
+		 */
+		for (Field nextField : fields) {
 			SearchParamDefinition searchParam = pullAnnotation(nextField, SearchParamDefinition.class);
 			if (searchParam != null) {
 				RestSearchParameterTypeEnum paramType = RestSearchParameterTypeEnum.forCode(searchParam.type().toLowerCase());

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/TokenClientParam.java

@@ -6,6 +6,8 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
+import org.apache.commons.lang3.ObjectUtils;
+
 import ca.uhn.fhir.model.base.composite.BaseIdentifierDt;
 
 /*
@@ -34,6 +36,8 @@ import ca.uhn.fhir.model.base.composite.BaseIdentifierDt;
  */
 public class TokenClientParam extends BaseClientParam  implements IParam {
 
+	private static final String[] EMPTY_STRING_LIST = new String[0];
+	
 	private String myParamName;
 
 	public TokenClientParam(String theParamName) {
@@ -77,6 +81,16 @@ public class TokenClientParam extends BaseClientParam  implements IParam {
 				return new TokenCriterion(getParamName(), defaultString(theSystem), theCode);
 			}
 
+			@Override
+			public ICriterion<?> systemAndValues(String theSystem, String... theValues) {
+				return new TokenCriterion(getParamName(), defaultString(theSystem), convertToList(theValues));
+			}
+
+			private List<String> convertToList(String[] theValues) {
+				String[] values = ObjectUtils.defaultIfNull(theValues, EMPTY_STRING_LIST);
+				return Arrays.asList(values);
+			}
+
 			@Override
 			public ICriterion<?> systemAndValues(String theSystem, Collection<String> theValues) {
 				return new TokenCriterion(getParamName(), defaultString(theSystem), theValues);
@@ -111,6 +125,7 @@ public class TokenClientParam extends BaseClientParam  implements IParam {
 		 */
 		ICriterion<TokenClientParam> code(String theIdentifier);
 
+
 		/**
 		 * Creates a search criterion that matches against the given identifier (system and code if both are present, or whatever is present)
 		 * 
@@ -160,15 +175,6 @@ public class TokenClientParam extends BaseClientParam  implements IParam {
 		 */
 		ICriterion<TokenClientParam> systemAndCode(String theSystem, String theCode);
 
-		/**
-		 * Creates a search criterion that matches a given system with a collection of possible 
-		 * values (this will be used to form a comma-separated OR query)
-		 * 
-		 * @param theSystem The system, which will be used with each value
-		 * @param theValues The values
-		 */
-		public ICriterion<?> systemAndValues(String theSystem, Collection<String> theValues);
-
 		/**
 		 * Creates a search criterion that matches against the given system and identifier
 		 * 
@@ -180,6 +186,24 @@ public class TokenClientParam extends BaseClientParam  implements IParam {
 		 */
 		ICriterion<TokenClientParam> systemAndIdentifier(String theSystem, String theIdentifier);
 
+		/**
+		 * Creates a search criterion that matches a given system with a collection of possible 
+		 * values (this will be used to form a comma-separated OR query)
+		 * 
+		 * @param theSystem The system, which will be used with each value
+		 * @param theValues The values
+		 */
+		ICriterion<?> systemAndValues(String theSystem, String... theValues);
+
+		/**
+		 * Creates a search criterion that matches a given system with a collection of possible 
+		 * values (this will be used to form a comma-separated OR query)
+		 * 
+		 * @param theSystem The system, which will be used with each value
+		 * @param theValues The values
+		 */
+		public ICriterion<?> systemAndValues(String theSystem, Collection<String> theValues);
+
 	}
 
 }

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

@@ -1,7 +1,7 @@
 package org.hl7.fhir.instance.model.api;
 
 import ca.uhn.fhir.model.api.annotation.SearchParamDefinition;
-import ca.uhn.fhir.rest.gclient.StringClientParam;
+import ca.uhn.fhir.rest.gclient.TokenClientParam;
 
 /*
  * #%L
@@ -35,7 +35,7 @@ public interface IAnyResource extends IBaseResource {
 	/**
 	 * Search parameter constant for <b>_id</b>
 	 */
-	@SearchParamDefinition(name="_id", path="", description="The ID of the resource", type="string"  )
+	@SearchParamDefinition(name="_id", path="", description="The ID of the resource", type="token"  )
 	public static final String SP_RES_ID = "_id";
 
 	/**
@@ -46,7 +46,7 @@ public interface IAnyResource extends IBaseResource {
 	 * Path: <b>Resource._id</b><br>
 	 * </p>
 	 */
-	public static final StringClientParam RES_ID = new StringClientParam(IAnyResource.SP_RES_ID);
+	public static final TokenClientParam RES_ID = new TokenClientParam(IAnyResource.SP_RES_ID);
 
 	String getId();
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java

@@ -2283,17 +2283,25 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 			id2 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless();
 		}
 
-		//@formatter:off
 		Bundle found = ourClient
 			.search()
 			.forResource(Patient.class)
-			.where(BaseResource.RES_ID.matches().values(id1.getIdPart(), id2.getIdPart()))
+			.where(BaseResource.RES_ID.exactly().systemAndValues(null, id1.getIdPart(), id2.getIdPart()))
-			.and(BaseResource.RES_ID.matches().value(id1.getIdPart()))
+			.and(BaseResource.RES_ID.exactly().systemAndCode(null, id1.getIdPart()))
 			.returnBundle(Bundle.class)
 			.execute();
-		//@formatter:on
 
 		assertThat(toUnqualifiedVersionlessIds(found), containsInAnyOrder(id1));
+		
+		found = ourClient
+				.search()
+				.forResource(Patient.class)
+				.where(BaseResource.RES_ID.exactly().systemAndValues(null, id1.getIdPart(), id2.getIdPart()))
+				.returnBundle(Bundle.class)
+				.execute();
+
+		assertThat(toUnqualifiedVersionlessIds(found), containsInAnyOrder(id1, id2));
+
 	}
 
 	@Test

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ServerDstu3Test.java

@@ -0,0 +1,69 @@
+package ca.uhn.fhir.jpa.provider.dstu3;
+
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.stringContainsInOrder;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.hl7.fhir.dstu3.model.CapabilityStatement;
+import org.hl7.fhir.dstu3.model.CapabilityStatement.CapabilityStatementRestResourceComponent;
+import org.hl7.fhir.dstu3.model.CapabilityStatement.CapabilityStatementRestResourceSearchParamComponent;
+import org.junit.AfterClass;
+import org.junit.Test;
+
+import ca.uhn.fhir.util.TestUtil;
+
+public class ServerDstu3Test extends BaseResourceProviderDstu3Test {
+
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ServerDstu3Test.class);
+
+	
+	
+	/**
+	 * See #519
+	 */
+	@Test
+	public void saveIdParamOnlyAppearsOnce() throws IOException {
+		HttpGet get = new HttpGet(ourServerBase + "/metadata?_pretty=true&_format=xml");
+		CloseableHttpResponse resp = ourHttpClient.execute(get);
+		try {
+			ourLog.info(resp.toString());
+			assertEquals(200, resp.getStatusLine().getStatusCode());
+			
+			String respString = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(respString);
+			
+			CapabilityStatement cs = myFhirCtx.newXmlParser().parseResource(CapabilityStatement.class, respString);
+			
+			for (CapabilityStatementRestResourceComponent nextResource : cs.getRest().get(0).getResource()) {
+				ourLog.info("Testing resource: " + nextResource.getType());
+				Set<String> sps = new HashSet<String>();
+				for (CapabilityStatementRestResourceSearchParamComponent nextSp : nextResource.getSearchParam()) {
+					if (sps.add(nextSp.getName()) == false) {
+						fail("Duplicate search parameter " + nextSp.getName() + " for resource " + nextResource.getType());
+					}
+				}
+				
+				if (!sps.contains("_id")) {
+					fail("No search parameter _id for resource " + nextResource.getType());
+				}
+			}			
+		} finally {
+		IOUtils.closeQuietly(resp.getEntity().getContent());
+		}
+	}
+
+
+	@AfterClass
+	public static void afterClassClearContext() {
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+}

hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/Binary.java

@@ -47,7 +47,7 @@ import org.hl7.fhir.exceptions.FHIRException;
  * A binary resource can contain any content, whether text, image, pdf, zip archive, etc.
  */
 @ResourceDef(name="Binary", profile="http://hl7.org/fhir/Profile/Binary")
-public class Binary extends BaseBinary implements IBaseBinary {
+public class Binary extends BaseBinary implements IBaseBinary, IAnyResource {
 
     /**
      * MimeType of the binary content represented as a standard MimeType (BCP 13).

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/context/ModelScannerDstu3Test.java

@@ -36,6 +36,15 @@ public class ModelScannerDstu3Test {
 		TestUtil.clearAllStaticFieldsForUnitTest();
 	}
 
+	@Test
+	public void testScanBundle() {
+		FhirContext ctx = FhirContext.forDstu3();
+		RuntimeResourceDefinition def = ctx.getResourceDefinition("Bundle");
+		
+		assertNotNull(def.getSearchParam("composition"));
+		assertNotNull(def.getSearchParam("_id"));
+	}
+	
 
 	@Test
 	public void testBundleMustImplementIBaseBundle() throws DataFormatException {

hapi-tinder-plugin/src/main/java/ca/uhn/fhir/tinder/TinderJpaRestServerMojo.java

@@ -236,10 +236,14 @@ public class TinderJpaRestServerMojo extends AbstractMojo {
 
 		TinderJpaRestServerMojo mojo = new TinderJpaRestServerMojo();
 		mojo.myProject = new MavenProject();
-		mojo.version = "dstu3";
+		mojo.version = "dstu2";
 		mojo.packageBase = "ca.uhn.test";
 		mojo.configPackageBase = "ca.uhn.test";
-		mojo.baseResourceNames = new ArrayList<String>(Arrays.asList("structuredefinition", "observation"));
+		mojo.baseResourceNames = new ArrayList<String>(Arrays.asList(
+//				"observation"
+//				"communicationrequest"
+				"binary"
+				));
 		mojo.targetDirectory = new File("target/generated/valuesets");
 		mojo.targetResourceDirectory = new File("target/generated/valuesets");
 		mojo.targetResourceSpringBeansFile = "tmp_beans.xml";

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

@@ -45,14 +45,6 @@ public class ${className}ResourceProvider extends
 			
 			ca.uhn.fhir.rest.method.RequestDetails theRequestDetails,
 			
-			@Description(shortDefinition="The resource identity")
-			@OptionalParam(name="_id")
-			StringAndListParam theId, 
-
-			@Description(shortDefinition="The resource language")
-			@OptionalParam(name="_language")
-			StringAndListParam theResourceLanguage, 
-
 			@Description(shortDefinition="Search the contents of the resource's data using a fulltext search")
 			@OptionalParam(name=ca.uhn.fhir.rest.server.Constants.PARAM_CONTENT)
 			StringAndListParam theFtContent, 
@@ -141,8 +133,6 @@ public class ${className}ResourceProvider extends
 		startRequest(theServletRequest);
 		try {
 			SearchParameterMap paramMap = new SearchParameterMap();
-			paramMap.add("_id", theId);
-			paramMap.add("_language", theResourceLanguage);
 			paramMap.add(ca.uhn.fhir.rest.server.Constants.PARAM_CONTENT, theFtContent);
 			paramMap.add(ca.uhn.fhir.rest.server.Constants.PARAM_TEXT, theFtText);
 			paramMap.add(ca.uhn.fhir.rest.server.Constants.PARAM_TAG, theSearchForTag);

src/changes/changes.xml

@@ -192,6 +192,12 @@
 				before the interceptor could see it. Thanks to
 				Eeva Turkka for reporting!
 			</action>
+			<action type="fix" issue="519">
+				JPA server exported CapabilityStatement includes 
+				double entries for the _id parameter and uses the
+				wrong type (string instead of token). Thanks to 
+				Robert Lichtenberger for reporting!
+			</action>
 		</release>
 		<release version="2.1" date="2016-11-11">
 			<action type="add">