honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Handle searches with chained slash value (#1503) 

* Work on test case for bug with searches

* Work on tests

* Fix issue with slashes in reference chain parameter for JPA server
This commit is contained in:
James Agnew 2019-09-24 14:31:05 -04:00 committed by GitHub
parent f5788341f2
commit 9b2826f3c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 172 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
README.md
View File

@ -6,12 +6,10 @@ HAPI FHIR - Java API for HL7 FHIR Clients and Servers
[![Coverage Status](https://coveralls.io/repos/jamesagnew/hapi-fhir/badge.svg?branch=master&service=github)](https://coveralls.io/github/jamesagnew/hapi-fhir?branch=master)
[![Maven Central](https://maven-badges.herokuapp.com/maven-central/ca.uhn.hapi.fhir/hapi-fhir-base/badge.svg)](http://search.maven.org/#search|ga|1|ca.uhn.hapi.fhir)
[![License](https://img.shields.io/badge/license-apache%202.0-60C060.svg)](http://jamesagnew.github.io/hapi-fhir/license.html)
* Linux Build: [![Build Status](https://travis-ci.org/jamesagnew/hapi-fhir.svg?branch=master)](https://travis-ci.org/jamesagnew/hapi-fhir)
* Windows Build: <a href="https://ci.appveyor.com/project/jamesagnew/hapi-fhir"><img src="https://ci.appveyor.com/api/projects/status/github/jamesagnew/hapi-fhir?branch=master&svg=true"></a>
[![Build Status](https://dev.azure.com/jamesagnew214/jamesagnew214/_apis/build/status/jamesagnew.hapi-fhir?branchName=master)](https://dev.azure.com/jamesagnew214/jamesagnew214/_build/latest?definitionId=1&branchName=master)
Complete project documentation is available here:
http://jamesagnew.github.io/hapi-fhir/
http://hapifhir.io
A demonstration of this project is available here:
http://hapi.fhir.org/

124
hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java
View File

@ -19,25 +19,27 @@ package ca.uhn.fhir.rest.param;
* limitations under the License.
* #L%
*/
import static ca.uhn.fhir.model.primitive.IdDt.isValidLong;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import java.math.BigDecimal;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.util.CoverageIgnore;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import java.math.BigDecimal;
import static ca.uhn.fhir.model.primitive.IdDt.isValidLong;
import static org.apache.commons.lang3.StringUtils.*;
public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/ {
private String myChain;
private String myResourceType;
private String myBaseUrl;
private String myValue;
private String myIdPart;
private final IdDt myId = new IdDt();
/**
* Constructor
*/
@ -64,12 +66,15 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/
* Constructor
*/
public ReferenceParam(String theResourceType, String theChain, String theValue) {
String qualifier = "";
if (isNotBlank(theResourceType)) {
setValue(theResourceType + "/" + theValue);
} else {
setValue(theValue);
qualifier = ":" + theResourceType;
}
setChain(theChain);
if (isNotBlank(theChain)) {
qualifier = qualifier + "." + theChain;
}
setValueAsQueryToken(null, null, qualifier, theValue);
}
@Override
@ -91,55 +96,54 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/
@Override
String doGetValueAsQueryToken(FhirContext theContext) {
if (isBlank(myId.getResourceType())) {
return myId.getValue(); // e.g. urn:asdjd or 123 or cid:wieiuru or #1
if (isBlank(getResourceType())) {
return myValue; // e.g. urn:asdjd or 123 or cid:wieiuru or #1
} else {
if (isBlank(getChain())) {
return getResourceType() + "/" + myId.getIdPart();
if (isBlank(getChain()) && isNotBlank(getResourceType())) {
return getResourceType() + "/" + getIdPart();
}
return myId.getIdPart();
return myValue;
}
}
@Override
void doSetValueAsQueryToken(FhirContext theContext, String theParamName, String theQualifier, String theValue) {
String q = theQualifier;
String resourceType = null;
boolean skipSetValue = false;
if (isNotBlank(q)) {
if (q.startsWith(":")) {
int nextIdx = q.indexOf('.');
if (nextIdx != -1) {
resourceType = q.substring(1, nextIdx);
myChain = q.substring(nextIdx + 1);
// type is explicitly defined so use it
myId.setParts(null, resourceType, theValue, null);
skipSetValue = true;
myResourceType = q.substring(1, nextIdx);
myValue = theValue;
myIdPart = theValue;
} else {
resourceType = q.substring(1);
myChain = null;
myResourceType = q.substring(1);
myValue = theValue;
myIdPart = theValue;
}
} else if (q.startsWith(".")) {
myChain = q.substring(1);
// type not defined but this is a chain, so treat value as opaque
myId.setParts(null, null, theValue, null);
skipSetValue = true;
myResourceType = null;
myValue = theValue;
myIdPart = theValue;
}
} else {
myChain = null;
myValue = theValue;
IdDt id = new IdDt(theValue);
myResourceType = id.getResourceType();
myIdPart = id.getIdPart();
myBaseUrl = id.getBaseUrl();
}
if (!skipSetValue) {
setValue(theValue);
if (isNotBlank(resourceType) && isBlank(getResourceType())) {
setValue(resourceType + '/' + theValue);
}
}
}
@CoverageIgnore
public String getBaseUrl() {
return myId.getBaseUrl();
return myBaseUrl;
}
@ -147,24 +151,34 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/
return myChain;
}
public ReferenceParam setChain(String theChain) {
myChain = theChain;
return this;
}
@CoverageIgnore
public String getIdPart() {
return myId.getIdPart();
return myIdPart;
}
@CoverageIgnore
public BigDecimal getIdPartAsBigDecimal() {
return myId.getIdPartAsBigDecimal();
return new IdDt(myValue).getIdPartAsBigDecimal();
}
@CoverageIgnore
public Long getIdPartAsLong() {
return myId.getIdPartAsLong();
return new IdDt(myValue).getIdPartAsLong();
}
public String getResourceType() {
return myId.getResourceType();
if (isNotBlank(myResourceType)) {
return myResourceType;
}
if (isBlank(myChain)) {
return new IdDt(myValue).getResourceType();
}
return null;
}
public Class<? extends IBaseResource> getResourceType(FhirContext theCtx) {
@ -175,11 +189,21 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/
}
public String getValue() {
return myId.getValue();
return myValue;
}
public ReferenceParam setValue(String theValue) {
IdDt id = new IdDt(theValue);
String qualifier= null;
if (id.hasResourceType()) {
qualifier = ":" + id.getResourceType();
}
setValueAsQueryToken(null, null, qualifier, id.getIdPart());
return this;
}
public boolean hasResourceType() {
return myId.hasResourceType();
return isNotBlank(myResourceType);
}
@Override
@ -187,16 +211,6 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/
return true;
}
public ReferenceParam setChain(String theChain) {
myChain = theChain;
return this;
}
public ReferenceParam setValue(String theValue) {
myId.setValue(theValue);
return this;
}
/**
* Returns a new param containing the same value as this param, but with the type copnverted
* to {@link DateParam}. This is useful if you are using reference parameters and want to handle

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

@ -573,7 +573,7 @@ public class SearchBuilder implements ISearchBuilder {
private Predicate addPredicateReferenceWithChain(String theResourceName, String theParamName, List<? extends IQueryParameterType> theList, Join<ResourceTable, ResourceLink> theJoin, List<Predicate> theCodePredicates, ReferenceParam theRef, RequestDetails theRequest) {
final List<Class<? extends IBaseResource>> resourceTypes;
String resourceId;
if (!theRef.getValue().matches("[a-zA-Z]+/.*")) {
if (!theRef.hasResourceType()) {
RuntimeSearchParam param = mySearchParamRegistry.getActiveSearchParam(theResourceName, theParamName);
resourceTypes = new ArrayList<>();

48
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java
View File

@ -167,6 +167,54 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
}
@Test
public void testSearchWithSlashes() {
myDaoConfig.setSearchPreFetchThresholds(Lists.newArrayList(10, 50, 10000));
Procedure procedure = new Procedure();
procedure.setStatus(Procedure.ProcedureStatus.COMPLETED);
String procedureId = ourClient.create().resource(procedure).execute().getId().toUnqualifiedVersionless().getValue();
DocumentReference dr = new DocumentReference();
dr.addContent().getAttachment().setContentType("application/vnd.mfer");
String drId = ourClient.create().resource(dr).execute().getId().toUnqualifiedVersionless().getValue();
for (int i = 0; i < 60; i++) {
Observation obs = new Observation();
obs.addPartOf().setReference(procedureId);
obs.addDerivedFrom().setReference(drId);
ourClient.create().resource(obs).execute();
}
ourLog.info("Starting search");
Bundle response = ourClient
.search()
.byUrl("Observation?part-of=" + procedureId + "&derived-from:DocumentReference.contenttype=application/vnd.mfer&_total=accurate&_count=2")
.returnBundle(Bundle.class)
.execute();
int obsCount = 0;
int pageCount = 0;
while (response != null) {
obsCount += response.getEntry().size();
pageCount++;
if (response.getLink("next") != null) {
response = ourClient.loadPage().next(response).execute();
} else {
response = null;
}
ourLog.info("Have loaded {} pages and {} reources", pageCount, obsCount);
}
assertEquals(60, obsCount);
assertEquals(30, pageCount);
}
@Test
public void testManualPagingLinkOffsetDoesntReturnBeyondEnd() {
myDaoConfig.setSearchPreFetchThresholds(Lists.newArrayList(10, 1000));

69
hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java
View File

@ -1,39 +1,60 @@
package ca.uhn.fhir.rest.param;
import static org.junit.Assert.*;
import org.junit.AfterClass;
import org.junit.Test;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.util.TestUtil;
import com.google.common.base.Charsets;
import org.apache.commons.lang3.SerializationUtils;
import org.junit.AfterClass;
import org.junit.Ignore;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.junit.Assert.assertEquals;
public class ReferenceParamTest {
private static final Logger ourLog = LoggerFactory.getLogger(ReferenceParamTest.class);
private FhirContext ourCtx = FhirContext.forDstu3();
@Test
public void testValueWithSlashPersistsAcrossSerialization() {
ReferenceParam param = new ReferenceParam();
param.setValueAsQueryToken(ourCtx, "derived-from", ":DocumentReference.contenttype", "application/vnd.mfer");
assertEquals("application/vnd.mfer", param.getValueAsQueryToken(ourCtx));
assertEquals(":DocumentReference.contenttype", param.getQueryParameterQualifier());
byte[] serialized = SerializationUtils.serialize(param);
ourLog.info("Serialized: {}", new String(serialized, Charsets.US_ASCII));
param = SerializationUtils.deserialize(serialized);
assertEquals("application/vnd.mfer", param.getValueAsQueryToken(ourCtx));
assertEquals(":DocumentReference.contenttype", param.getQueryParameterQualifier());
}
@Test
public void testWithResourceType() {
ReferenceParam rp = new ReferenceParam();
rp.setValueAsQueryToken(ourCtx, null, null, "Location/123");
assertEquals("Location", rp.getResourceType());
assertEquals("123", rp.getIdPart());
assertEquals("Location/123", rp.getValue());
assertEquals(null, rp.getQueryParameterQualifier());
}
@Test
public void testWithResourceType_AbsoluteUrl() {
ReferenceParam rp = new ReferenceParam();
rp.setValueAsQueryToken(ourCtx, null, null, "http://a.b/c/d/e");
assertEquals("d", rp.getResourceType());
assertEquals("e", rp.getIdPart());
assertEquals("http://a.b/c/d/e", rp.getValue());
assertEquals(null, rp.getQueryParameterQualifier());
}
@Test
@ -74,24 +95,26 @@ public class ReferenceParamTest {
assertEquals("name", rp.getChain());
}
@Test
public void testWithResourceTypeAsQualifier() {
ReferenceParam rp = new ReferenceParam();
rp.setValueAsQueryToken(ourCtx, null, ":Location", "123");
assertEquals("Location", rp.getResourceType());
assertEquals("123", rp.getIdPart());
assertEquals("Location/123", rp.getValue());
assertEquals("123", rp.getValue());
assertEquals(null, rp.getQueryParameterQualifier());
}
// TODO: verify this behavior is correct. If type is explicitly specified (i.e. :Location), should it be
// an error if it gets overriden by the resourceType in the url?
/**
* TODO: is this an error?
*/
@Test
public void testWithResourceTypeAsQualifier_RelativeUrl() {
@Ignore
public void testMismatchedTypeAndValueType() {
ReferenceParam rp = new ReferenceParam();
rp.setValueAsQueryToken(ourCtx, null, ":Location", "Patient/123");
assertEquals("Patient", rp.getResourceType());
@ -104,11 +127,11 @@ public class ReferenceParamTest {
// TODO: verify this behavior is correct. Same case as testWithResourceTypeAsQualifier_RelativeUrl()
@Test
public void testWithResourceTypeAsQualifier_AbsoluteUrl() {
ReferenceParam rp = new ReferenceParam();
rp.setValueAsQueryToken(ourCtx, null, ":Location", "http://a.b/c/d/e");
assertEquals("d", rp.getResourceType());
assertEquals("e", rp.getIdPart());
assertEquals("Location", rp.getResourceType());
assertEquals("http://a.b/c/d/e", rp.getIdPart());
assertEquals("http://a.b/c/d/e", rp.getValue());
assertEquals(null, rp.getQueryParameterQualifier());
@ -122,7 +145,7 @@ public class ReferenceParamTest {
rp.setValueAsQueryToken(ourCtx, null, ":Location.name", "FOO");
assertEquals("Location", rp.getResourceType());
assertEquals("FOO", rp.getIdPart());
assertEquals("Location/FOO", rp.getValue());
assertEquals("FOO", rp.getValue());
assertEquals(":Location.name", rp.getQueryParameterQualifier());
assertEquals("name", rp.getChain());
@ -135,7 +158,7 @@ public class ReferenceParamTest {
rp.setValueAsQueryToken(ourCtx, null, ":Patient.identifier", "http://hey.there/a/b|123");
assertEquals("Patient", rp.getResourceType());
assertEquals("http://hey.there/a/b|123", rp.getIdPart());
assertEquals("Patient/http://hey.there/a/b|123", rp.getValue());
assertEquals("http://hey.there/a/b|123", rp.getValue());
assertEquals(":Patient.identifier", rp.getQueryParameterQualifier());
assertEquals("identifier", rp.getChain());
@ -147,8 +170,8 @@ public class ReferenceParamTest {
ReferenceParam rp = new ReferenceParam();
rp.setValueAsQueryToken(ourCtx, null, ":Patient.identifier", "http://hey.there/a/b|");
assertEquals("Patient", rp.getResourceType());
assertEquals("http://hey.there/a/b|", rp.getValue());
assertEquals("http://hey.there/a/b|", rp.getIdPart());
assertEquals("Patient/http://hey.there/a/b|", rp.getValue());
assertEquals(":Patient.identifier", rp.getQueryParameterQualifier());
assertEquals("identifier", rp.getChain());
@ -161,7 +184,7 @@ public class ReferenceParamTest {
rp.setValueAsQueryToken(ourCtx, null, ":Patient.identifier", "|abc");
assertEquals("Patient", rp.getResourceType());
assertEquals("|abc", rp.getIdPart());
assertEquals("Patient/|abc", rp.getValue());
assertEquals("|abc", rp.getValue());
assertEquals(":Patient.identifier", rp.getQueryParameterQualifier());
assertEquals("identifier", rp.getChain());

6
src/changes/changes.xml
View File

@ -193,6 +193,12 @@
resulted in some ValueSets with duplicate codes. This has been corrected by specifying a path with each
filename.
</action>
<action>
A corner case bug in the JPA server was solved: When performing a search that contained chained reference searches
where the value contained slashes (e.g.
<![CDATA[<code>Observation?derived-from:DocumentReference.contenttype=application/vnd.mfer</code>]]>)
the server could fail to load later pages in the search results.
</action>
<action type="add">
A new flag has been added to the JPA migrator tool that causes the migrator to not try to reduce the length
of existing columns in the schema.