Fix #240 - Support target resource type in server _include and _revinclude values

hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Include.java

@@ -1,8 +1,9 @@
 package ca.uhn.fhir.model.api;
 
-import org.apache.commons.lang3.builder.ToStringBuilder;
+import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
-import ch.qos.logback.core.db.dialect.MySQLDialect;
+import org.apache.commons.lang3.builder.ToStringBuilder;
 
 /*
  * #%L
@@ -34,9 +35,9 @@ import ch.qos.logback.core.db.dialect.MySQLDialect;
  */
 public class Include {
 
+	private final boolean myImmutable;
 	private boolean myRecurse;
 	private String myValue;
-	private final boolean myImmutable;
 
 	/**
 	 * Constructor for <b>non-recursive</b> include
@@ -91,22 +92,6 @@ public class Include {
 		return new Include(myValue, true);
 	}
 
-	public String getValue() {
-		return myValue;
-	}
-
-	/**
-	 * See the note on equality on the {@link Include class documentation}
-	 */
-	@Override
-	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + (myRecurse ? 1231 : 1237);
-		result = prime * result + ((myValue == null) ? 0 : myValue.hashCode());
-		return result;
-	}
-
 	/**
 	 * See the note on equality on the {@link Include class documentation}
 	 */
@@ -135,6 +120,73 @@ public class Include {
 		return true;
 	}
 
+	/**
+	 * Returns the portion of the value before the first colon
+	 */
+	public String getParamType() {
+		int firstColon = myValue.indexOf(':');
+		if (firstColon == -1 || firstColon == myValue.length() - 1) {
+			return null;
+		}
+		return myValue.substring(0, firstColon);
+	}
+
+	/**
+	 * Returns the portion of the value after the first colon but before the second colon
+	 */
+	public String getParamName() {
+		int firstColon = myValue.indexOf(':');
+		if (firstColon == -1 || firstColon == myValue.length() - 1) {
+			return null;
+		}
+		int secondColon = myValue.indexOf(':', firstColon + 1);
+		if (secondColon != -1) {
+			return myValue.substring(firstColon + 1, secondColon);
+		} else {
+			return myValue.substring(firstColon + 1);
+		}
+	}
+
+	/**
+	 * Returns the portion of the string after the second colon, or null if there are not two colons in the value.
+	 */
+	public String getParamTargetType() {
+		int firstColon = myValue.indexOf(':');
+		if (firstColon == -1 || firstColon == myValue.length() - 1) {
+			return null;
+		}
+		int secondColon = myValue.indexOf(':', firstColon + 1);
+		if (secondColon != -1) {
+			return myValue.substring(secondColon + 1);
+		} else {
+			return null;
+		}
+
+	}
+
+	public String getValue() {
+		return myValue;
+	}
+
+	/**
+	 * See the note on equality on the {@link Include class documentation}
+	 */
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = 1;
+		result = prime * result + (myRecurse ? 1231 : 1237);
+		result = prime * result + ((myValue == null) ? 0 : myValue.hashCode());
+		return result;
+	}
+
+	/**
+	 * Is this object {@link #toLocked() locked}?
+	 */
+	public boolean isLocked() {
+		return myImmutable;
+	}
+
 	public boolean isRecurse() {
 		return myRecurse;
 	}
@@ -150,13 +202,6 @@ public class Include {
 		myValue = theValue;
 	}
 
-	/**
-	 * Is this object {@link #toLocked() locked}?
-	 */
-	public boolean isLocked() {
-		return myImmutable;
-	}
-
 	/**
 	 * Return a new
 	 */
@@ -174,32 +219,52 @@ public class Include {
 	}
 
 	/**
-	 * Creates and returns a new copy of this Include with the given type. The 
+	 * Creates and returns a new copy of this Include with the given type. The following table shows what will be
-	 * following table shows what will be returned:
+	 * returned:
 	 * <table>
-	 * <tr><th>Initial Contents</th><th>theResourceType</th><th>Output</th></tr>
+	 * <tr>
-	 * <tr><td>Patient:careProvider</th><th>Organization</th><th>Patient:careProvider:Organization</th></tr>
+	 * <th>Initial Contents</th>
-	 * <tr><td>Patient:careProvider:Practitioner</th><th>Organization</th><th>Patient:careProvider:Organization</th></tr>
+	 * <th>theResourceType</th>
-	 * <tr><td>Patient</th><th>(any)</th><th>{@link IllegalStateException}</th></tr>
+	 * <th>Output</th>
+	 * </tr>
+	 * <tr>
+	 * <td>Patient:careProvider</th>
+	 * <th>Organization</th>
+	 * <th>Patient:careProvider:Organization</th>
+	 * </tr>
+	 * <tr>
+	 * <td>Patient:careProvider:Practitioner</th>
+	 * <th>Organization</th>
+	 * <th>Patient:careProvider:Organization</th>
+	 * </tr>
+	 * <tr>
+	 * <td>Patient</th>
+	 * <th>(any)</th>
+	 * <th>{@link IllegalStateException}</th>
+	 * </tr>
 	 * </table>
 	 * 
-	 * @param theResourceType The resource type (e.g. "Organization")
+	 * @param theResourceType
-	 * @return A new copy of the include. Note that if this include is {@link #toLocked() locked}, the returned include will be too
+	 *           The resource type (e.g. "Organization")
+	 * @return A new copy of the include. Note that if this include is {@link #toLocked() locked}, the returned include
+	 *         will be too
 	 */
 	public Include withType(String theResourceType) {
 		StringBuilder b = new StringBuilder();
-		b.append(myValue);
+		
-		int firstColon = myValue.indexOf(':');
+		String paramType = getParamType();
-		if (firstColon == -1 || firstColon == b.length() - 1) {
+		String paramName = getParamName();
+		if (isBlank(paramType) || isBlank(paramName)) {
 			throw new IllegalStateException("This include does not contain a value in the format [ResourceType]:[paramName]");
 		}
-		int secondColon = myValue.indexOf(':', firstColon + 1);
+		b.append(paramType);
-		if (secondColon != -1) {
-			b.delete(secondColon, b.length());
-		}
-		
 		b.append(":");
+		b.append(paramName);
+		
+		if (isNotBlank(theResourceType)) {
+			b.append(':');
 			b.append(theResourceType);
+		}
 		Include retVal = new Include(b.toString(), myRecurse, myImmutable);
 		return retVal;
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java

@@ -1,5 +1,7 @@
 package ca.uhn.fhir.jpa.dao;
 
+import static org.apache.commons.lang3.StringUtils.defaultString;
+
 /*
  * #%L
  * HAPI FHIR JPA Server
@@ -1378,19 +1380,18 @@ public class SearchBuilder {
 					if (myContext.getVersion().getVersion() == FhirVersionEnum.DSTU1) {
 						paths = Collections.singletonList(nextInclude.getValue());
 					} else {
-						int colonIdx = nextInclude.getValue().indexOf(':');
+						String resType = nextInclude.getParamType();
-						if (colonIdx < 2) {
+						if (isBlank(resType)) {
 							continue;
 						}
-						String resType = nextInclude.getValue().substring(0, colonIdx);
 						RuntimeResourceDefinition def = myContext.getResourceDefinition(resType);
 						if (def == null) {
 							ourLog.warn("Unknown resource type in include/revinclude=" + nextInclude.getValue());
 							continue;
 						}
 
-						String paramName = nextInclude.getValue().substring(colonIdx + 1);
+						String paramName = nextInclude.getParamName();
-						RuntimeSearchParam param = def.getSearchParam(paramName);
+						RuntimeSearchParam param = isNotBlank(paramName) ? def.getSearchParam(paramName) : null;
 						if (param == null) {
 							ourLog.warn("Unknown param name in include/revinclude=" + nextInclude.getValue());
 							continue;
@@ -1399,11 +1400,20 @@ public class SearchBuilder {
 						paths = param.getPathsSplit();
 					}
 
+					String targetResourceType = defaultString(nextInclude.getParamTargetType(), null);
 					for (String nextPath : paths) {
-						String sql = "SELECT r FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchFieldName + " IN (:target_pids)";
+						String sql;
+						if (targetResourceType != null) {
+							sql = "SELECT r FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchFieldName + " IN (:target_pids) AND r.myTargetResourceType = :target_resource_type";
+						} else {
+							sql = "SELECT r FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchFieldName + " IN (:target_pids)";
+						}
 						TypedQuery<ResourceLink> q = myEntityManager.createQuery(sql, ResourceLink.class);
 						q.setParameter("src_path", nextPath);
 						q.setParameter("target_pids", nextRoundMatches);
+						if (targetResourceType != null) {
+							q.setParameter("target_resource_type", targetResourceType);
+						}
 						List<ResourceLink> results = q.getResultList();
 						for (ResourceLink resourceLink : results) {
 							if (theReverseMode) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceLink.java

@@ -36,6 +36,7 @@ import javax.persistence.Table;
 import org.apache.commons.lang3.Validate;
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.hibernate.annotations.ColumnDefault;
 import org.hibernate.search.annotations.Field;
 
 @Entity
@@ -64,6 +65,11 @@ public class ResourceLink implements Serializable {
 	@Column(name = "SRC_RESOURCE_ID", insertable = false, updatable = false, nullable = false)
 	private Long mySourceResourcePid;
 
+	@Column(name = "SOURCE_RESOURCE_TYPE", nullable=false, length=ResourceTable.RESTYPE_LEN)
+	@ColumnDefault("''") // TODO: remove this (it's only here for simplifying upgrades of 1.3 -> 1.4)
+	@Field()
+	private String mySourceResourceType;
+
 	@ManyToOne(optional = false, fetch=FetchType.LAZY)
 	@JoinColumn(name = "TARGET_RESOURCE_ID", referencedColumnName = "RES_ID", nullable = false)
 	private ResourceTable myTargetResource;
@@ -72,17 +78,20 @@ public class ResourceLink implements Serializable {
 	@Field()
 	private Long myTargetResourcePid;
 
+	@Column(name = "TARGET_RESOURCE_TYPE", nullable=false, length=ResourceTable.RESTYPE_LEN)
+	@ColumnDefault("''") // TODO: remove this (it's only here for simplifying upgrades of 1.3 -> 1.4)
+	@Field()
+	private String myTargetResourceType;
+
 	public ResourceLink() {
 		super();
 	}
 
 	public ResourceLink(String theSourcePath, ResourceTable theSourceResource, ResourceTable theTargetResource) {
 		super();
-		mySourcePath = theSourcePath;
+		setSourcePath(theSourcePath);
-		mySourceResource = theSourceResource;
+		setSourceResource(theSourceResource);
-		mySourceResourcePid = theSourceResource.getId();
+		setTargetResource(theTargetResource);
-		myTargetResource = theTargetResource;
-		myTargetResourcePid = theTargetResource.getId();
 	}
 
 	@Override
@@ -140,12 +149,14 @@ public class ResourceLink implements Serializable {
 	public void setSourceResource(ResourceTable theSourceResource) {
 		mySourceResource = theSourceResource;
 		mySourceResourcePid = theSourceResource.getId();
+		mySourceResourceType = theSourceResource.getResourceType();
 	}
 
 	public void setTargetResource(ResourceTable theTargetResource) {
 		Validate.notNull(theTargetResource);
 		myTargetResource = theTargetResource;
 		myTargetResourcePid = theTargetResource.getId();
+		myTargetResourceType = theTargetResource.getResourceType();
 	}
 
 	@Override

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchNoFtTest.java

@@ -1462,6 +1462,13 @@ public class FhirResourceDaoDstu2SearchNoFtTest extends BaseJpaDstu2Test {
 			patientId2 = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
 		}
 
+		{
+			// Typed include
+			SearchParameterMap params = new SearchParameterMap();
+			params.addInclude(Patient.INCLUDE_CAREPROVIDER.withType("Practitioner"));
+			List<IIdType> ids = toUnqualifiedVersionlessIds(myPatientDao.search(params));
+			assertThat(ids, containsInAnyOrder(patientId, patientId2, practId2));
+		}
 		{
 			// No includes
 			SearchParameterMap params = new SearchParameterMap();
@@ -1545,13 +1552,6 @@ public class FhirResourceDaoDstu2SearchNoFtTest extends BaseJpaDstu2Test {
 			List<IIdType> ids = toUnqualifiedVersionlessIds(myPatientDao.search(params));
 			assertThat(ids, containsInAnyOrder(orgId, patientId, patientId2, practId2));
 		}
-		{
-//			// Typed include
-//			SearchParameterMap params = new SearchParameterMap();
-//			params.addInclude(Patient.INCLUDE_CAREPROVIDER.withType("Practitioner"));
-//			List<IIdType> ids = toUnqualifiedVersionlessIds(myPatientDao.search(params));
-//			assertThat(ids, containsInAnyOrder(patientId, patientId2, practId2));
-		}
 	}
 
 	@SuppressWarnings("unused")

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

@@ -177,6 +177,8 @@ public class IncludeTest {
 		assertEquals("Patient:careProvider:Practitioner", new Include("Patient:careProvider", true).withType("Practitioner").getValue());
 		assertEquals(true, new Include("Patient:careProvider", true).withType("Practitioner").isRecurse());
 		assertEquals(false, new Include("Patient:careProvider:Organization", true).withType("Practitioner").isLocked());
+		assertEquals("Practitioner", new Include("Patient:careProvider", true).withType("Practitioner").getParamTargetType());
+		assertEquals(null, new Include("Patient:careProvider", true).getParamTargetType());
 
 		assertEquals("Patient:careProvider:Practitioner", new Include("Patient:careProvider:Organization", true).withType("Practitioner").getValue());
 		assertEquals(true, new Include("Patient:careProvider:Organization", true).toLocked().withType("Practitioner").isLocked());

src/changes/changes.xml

@@ -71,6 +71,11 @@
 				valueString instead of valueMarkdown. After discussion with Grahame, this appears to
 				be incorrect behaviour so it has been fixed.
 			</action>
+			<action type="add" issue="240">
+				Support target parameter type in _include / _revinclude values, e.g.
+				_include=Patient:careProvider:Organization. Thanks to Joe Portner
+				for reporting!
+			<action>
 		</release>
 		<release version="1.3" date="2015-11-14">
 			<action type="add">