Fix #149 - Respect server address strategy for link-self on search

results
This commit is contained in:
James Agnew 2015-05-15 17:51:06 -04:00
parent da3ec668de
commit 7517709edb
8 changed files with 139 additions and 22 deletions

View File

@ -20,6 +20,8 @@ package ca.uhn.fhir.rest.method;
* #L%
*/
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import java.io.IOException;
import java.io.Reader;
import java.lang.reflect.Method;
@ -30,6 +32,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import javax.servlet.http.HttpServletResponse;
@ -45,7 +48,9 @@ import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.api.annotation.ResourceDef;
import ca.uhn.fhir.model.valueset.BundleTypeEnum;
import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.rest.api.RequestTypeEnum;
import ca.uhn.fhir.rest.client.exceptions.InvalidResponseException;
import ca.uhn.fhir.rest.param.ParameterUtil;
import ca.uhn.fhir.rest.server.Constants;
import ca.uhn.fhir.rest.server.EncodingEnum;
import ca.uhn.fhir.rest.server.IBundleProvider;
@ -59,6 +64,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
import ca.uhn.fhir.util.ReflectionUtil;
import ca.uhn.fhir.util.UrlUtil;
abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Object> {
protected static final Set<String> ALLOWED_PARAMS;
@ -92,7 +98,8 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
Class<?> collectionType = ReflectionUtil.getGenericCollectionTypeOfMethodReturnType(theMethod);
if (collectionType != null) {
if (!Object.class.equals(collectionType) && !IResource.class.isAssignableFrom(collectionType)) {
throw new ConfigurationException("Method " + theMethod.getDeclaringClass().getSimpleName() + "#" + theMethod.getName() + " returns an invalid collection generic type: " + collectionType);
throw new ConfigurationException("Method " + theMethod.getDeclaringClass().getSimpleName() + "#" + theMethod.getName() + " returns an invalid collection generic type: "
+ collectionType);
}
}
myResourceListCollectionType = collectionType;
@ -108,7 +115,8 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
} else if (IBundleProvider.class.isAssignableFrom(methodReturnType)) {
myMethodReturnType = MethodReturnTypeEnum.BUNDLE_PROVIDER;
} else {
throw new ConfigurationException("Invalid return type '" + methodReturnType.getCanonicalName() + "' on method '" + theMethod.getName() + "' on type: " + theMethod.getDeclaringClass().getCanonicalName());
throw new ConfigurationException("Invalid return type '" + methodReturnType.getCanonicalName() + "' on method '" + theMethod.getName() + "' on type: "
+ theMethod.getDeclaringClass().getCanonicalName());
}
if (theReturnResourceType != null) {
@ -255,6 +263,36 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
switch (getReturnType()) {
case BUNDLE: {
/*
* Figure out the self-link for this request
*/
String serverBase = theServer.getServerBaseForRequest(theRequest.getServletRequest());
String linkSelf;
StringBuilder b = new StringBuilder();
b.append(serverBase);
if (isNotBlank(theRequest.getRequestPath())) {
b.append('/');
b.append(theRequest.getRequestPath());
}
// For POST the URL parameters get jumbled with the post body parameters so don't include them, they might be huge
if (theRequest.getRequestType() == RequestTypeEnum.GET) {
boolean first = true;
for (Entry<String, String[]> nextParams : theRequest.getParameters().entrySet()) {
for (String nextParamValue : nextParams.getValue()) {
if (first) {
b.append('?');
first = false;
} else {
b.append('&');
}
b.append(UrlUtil.escape(nextParams.getKey()));
b.append('=');
b.append(UrlUtil.escape(nextParamValue));
}
}
}
linkSelf = b.toString();
if (getMethodReturnType() == MethodReturnTypeEnum.BUNDLE_RESOURCE) {
IBaseResource resource;
if (resultObj instanceof IBundleProvider) {
@ -265,12 +303,11 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
}
/*
* We assume that the bundle we got back from the handling method may not have everything populated
* (e.g. self links, bundle type, etc) so we do that here.
* We assume that the bundle we got back from the handling method may not have everything populated (e.g. self links, bundle type, etc) so we do that here.
*/
IVersionSpecificBundleFactory bundleFactory = theServer.getFhirContext().newBundleFactory();
bundleFactory.initializeWithBundleResource(resource);
bundleFactory.addRootPropertiesToBundle(null, theRequest.getFhirServerBase(), theRequest.getCompleteUrl(), count, getResponseBundleType());
bundleFactory.addRootPropertiesToBundle(null, theRequest.getFhirServerBase(), linkSelf, count, getResponseBundleType());
for (int i = theServer.getInterceptors().size() - 1; i >= 0; i--) {
IServerInterceptor next = theServer.getInterceptors().get(i);
@ -280,7 +317,8 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
return;
}
}
RestfulServerUtils.streamResponseAsResource(theServer, response, resource, responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, Constants.STATUS_HTTP_200_OK, respondGzip, theRequest.getFhirServerBase(), isAddContentLocationHeader());
RestfulServerUtils.streamResponseAsResource(theServer, response, resource, responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, Constants.STATUS_HTTP_200_OK, respondGzip,
theRequest.getFhirServerBase(), isAddContentLocationHeader());
break;
} else {
Set<Include> includes = getRequestIncludesFromParams(params);
@ -291,7 +329,8 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
}
IVersionSpecificBundleFactory bundleFactory = theServer.getFhirContext().newBundleFactory();
bundleFactory.initializeBundleFromBundleProvider(theServer, result, responseEncoding, theRequest.getFhirServerBase(), theRequest.getCompleteUrl(), prettyPrint, 0, count, null, getResponseBundleType(), includes);
bundleFactory.initializeBundleFromBundleProvider(theServer, result, responseEncoding, theRequest.getFhirServerBase(), linkSelf, prettyPrint, 0, count, null, getResponseBundleType(),
includes);
Bundle bundle = bundleFactory.getDstu1Bundle();
if (bundle != null) {
for (int i = theServer.getInterceptors().size() - 1; i >= 0; i--) {
@ -313,7 +352,8 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
return;
}
}
RestfulServerUtils.streamResponseAsResource(theServer, response, (IResource) resBundle, responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, Constants.STATUS_HTTP_200_OK, theRequest.isRespondGzip(), theRequest.getFhirServerBase(), isAddContentLocationHeader());
RestfulServerUtils.streamResponseAsResource(theServer, response, (IResource) resBundle, responseEncoding, prettyPrint, requestIsBrowser, narrativeMode,
Constants.STATUS_HTTP_200_OK, theRequest.isRespondGzip(), theRequest.getFhirServerBase(), isAddContentLocationHeader());
}
break;
@ -337,7 +377,8 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
}
}
RestfulServerUtils.streamResponseAsResource(theServer, response, resource, responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, Constants.STATUS_HTTP_200_OK, respondGzip, theRequest.getFhirServerBase(), isAddContentLocationHeader());
RestfulServerUtils.streamResponseAsResource(theServer, response, resource, responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, Constants.STATUS_HTTP_200_OK, respondGzip,
theRequest.getFhirServerBase(), isAddContentLocationHeader());
break;
}
}

View File

@ -39,6 +39,7 @@ public class Request extends RequestDetails {
private String myFhirServerBase;
private String myOperation;
private String myRequestPath;
private boolean myRespondGzip;
private String mySecondaryOperation;
private HttpServletRequest myServletRequest;
@ -54,6 +55,16 @@ public class Request extends RequestDetails {
}
/**
* The part of the request URL that comes after the server base.
* <p>
* Will not contain a leading '/'
* </p>
*/
public String getRequestPath() {
return myRequestPath;
}
public String getSecondaryOperation() {
return mySecondaryOperation;
}
@ -111,6 +122,11 @@ public class Request extends RequestDetails {
}
public void setRequestPath(String theRequestPath) {
assert theRequestPath.length() == 0 || theRequestPath.charAt(0) != '/';
myRequestPath = theRequestPath;
}
public void setRespondGzip(boolean theRespondGzip) {
myRespondGzip = theRespondGzip;
}

View File

@ -34,6 +34,7 @@ import org.apache.commons.lang3.time.DateUtils;
import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.model.primitive.IntegerDt;
import ca.uhn.fhir.util.UrlUtil;
public class ParameterUtil {
@ -228,11 +229,8 @@ public class ParameterUtil {
return null;
}
try {
return URLEncoder.encode(escape(theValue), "UTF-8");
} catch (UnsupportedEncodingException e) {
throw new Error("UTF-8 not supported on this platform");
}
String escaped = escape(theValue);
return UrlUtil.escape(escaped);
}
/**

View File

@ -379,6 +379,9 @@ public class RestfulServer extends HttpServlet {
return myServerAddressStrategy;
}
/**
* Returns the server base URL (with no trailing '/') for a given request
*/
public String getServerBaseForRequest(HttpServletRequest theRequest) {
String fhirServerBase;
fhirServerBase = myServerAddressStrategy.determineServerBase(getServletContext(), theRequest);
@ -645,6 +648,7 @@ public class RestfulServer extends HttpServlet {
requestDetails.setCompleteUrl(completeUrl);
requestDetails.setServletRequest(theRequest);
requestDetails.setServletResponse(theResponse);
requestDetails.setRequestPath(requestPath);
String pagingAction = theRequest.getParameter(Constants.PARAM_PAGINGACTION);
if (getPagingProvider() != null && isNotBlank(pagingAction)) {

View File

@ -4,6 +4,7 @@ import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLDecoder;
import java.net.URLEncoder;
/*
* #%L
@ -140,4 +141,18 @@ public class UrlUtil {
}
}
/**
* URL encode a value
*/
public static String escape(String theValue) {
if (theValue == null) {
return null;
}
try {
return URLEncoder.encode(theValue, "UTF-8");
} catch (UnsupportedEncodingException e) {
throw new Error("UTF-8 not supported on this platform");
}
}
}

View File

@ -409,7 +409,8 @@ public abstract class BaseFhirResourceDao<T extends IResource> extends BaseFhirD
subQ.select(subQfrom.get("mySourceResourcePid").as(Long.class));
// subQ.where(builder.equal(subQfrom.get("myParamName"), theParamName));
subQ.where(createResourceLinkPathPredicate(theParamName, builder, subQfrom));
Predicate path = createResourceLinkPathPredicate(theParamName, builder, subQfrom);
subQ.where(path);
Predicate joinPredicate = builder.not(builder.in(from.get("myId")).value(subQ));
Predicate typePredicate = builder.equal(from.get("myResourceType"), myResourceName);
@ -646,9 +647,8 @@ public abstract class BaseFhirResourceDao<T extends IResource> extends BaseFhirD
private Predicate createResourceLinkPathPredicate(String theParamName, CriteriaBuilder builder, Root<? extends ResourceLink> from) {
RuntimeSearchParam param = getContext().getResourceDefinition(getResourceType()).getSearchParam(theParamName);
String path = param.getPath();
Predicate type = builder.equal(from.get("mySourcePath"), path);
List<String> path = param.getPathsSplit();
Predicate type = from.get("mySourcePath").in(path);
return type;
}

View File

@ -70,10 +70,13 @@ public class SearchTest {
private static Server ourServer;
private static NarrativeModeEnum ourLastNarrativeMode;
private static RestfulServer ourServlet;
private static IServerAddressStrategy ourDefaultAddressStrategy;
@Before
public void before() {
ourLastNarrativeMode=null;
ourServlet.setServerAddressStrategy(ourDefaultAddressStrategy);
}
@Test
@ -134,6 +137,7 @@ public class SearchTest {
assertEquals(null, p.getNameFirstRep().getFamilyFirstRep().getValue());
}
@Test
public void testReturnLinks() throws Exception {
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_query=findWithLinks");
@ -155,6 +159,38 @@ public class SearchTest {
}
/**
* #149
*/
@Test
public void testReturnLinksWithAddressStrategy() throws Exception {
ourServlet.setServerAddressStrategy(new HardcodedServerAddressStrategy("https://blah.com/base"));
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_query=findWithLinks");
CloseableHttpResponse status = ourClient.execute(httpGet);
String responseContent = IOUtils.toString(status.getEntity().getContent());
IOUtils.closeQuietly(status.getEntity().getContent());
assertEquals(200, status.getStatusLine().getStatusCode());
Bundle bundle = ourCtx.newXmlParser().parseBundle(responseContent);
ourLog.info(responseContent);
assertEquals(1, bundle.getEntries().size());
assertEquals("https://blah.com/base", bundle.getLinkBase().getValue());
assertEquals("https://blah.com/base/Patient?_query=findWithLinks", bundle.getLinkSelf().getValue());
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("https://blah.com/base/Patient/9988", bundle.getEntries().get(0).getLinkAlternate().getValue());
assertEquals("http://foo/Patient?_id=1", ResourceMetadataKeyEnum.LINK_SEARCH.get(p));
assertEquals("https://blah.com/base/Patient/9988", ResourceMetadataKeyEnum.LINK_ALTERNATE.get(p));
}
@Test
public void testSearchById() throws Exception {
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=aaa");
@ -385,11 +421,11 @@ public class SearchTest {
DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider();
ServletHandler proxyHandler = new ServletHandler();
RestfulServer servlet = new RestfulServer();
servlet.getFhirContext().setNarrativeGenerator(new DefaultThymeleafNarrativeGenerator());
ourServlet = new RestfulServer();
ourServlet.getFhirContext().setNarrativeGenerator(new DefaultThymeleafNarrativeGenerator());
servlet.setResourceProviders(patientProvider, new DummyObservationResourceProvider());
ServletHolder servletHolder = new ServletHolder(servlet);
ourServlet.setResourceProviders(patientProvider, new DummyObservationResourceProvider());
ServletHolder servletHolder = new ServletHolder(ourServlet);
proxyHandler.addServletWithMapping(servletHolder, "/*");
ourServer.setHandler(proxyHandler);
ourServer.start();
@ -399,8 +435,10 @@ public class SearchTest {
builder.setConnectionManager(connectionManager);
ourClient = builder.build();
ourDefaultAddressStrategy = ourServlet.getServerAddressStrategy();
}
public static class DummyObservationResourceProvider implements IResourceProvider {
@Override
@ -492,6 +530,7 @@ public class SearchTest {
return retVal;
}
@Search(queryName = "findPatientByAAA")
public List<Patient> findPatientByAAA02Named(@OptionalParam(name = "AAA") StringParam theParam) {
ArrayList<Patient> retVal = new ArrayList<Patient>();

View File

@ -18,6 +18,10 @@
<action type="fix" issue="164">
Correct performance issue with :missing=true search requests where the parameter is a resource link. Thanks to wanghaisheng for all his help in testing this.
</action>
<action type="fix" issue="149">
The self link in the Bundle returned by searches on the server does not respect the
server's address strategy (which resulted in an internal IP being shown on fhirtest.uhn.ca)
</action>
</release>
<release version="1.0" date="2015-May-8">
<action type="add">