From b2776931066d0d286d0ff5acc6dbab9101531ef5 Mon Sep 17 00:00:00 2001 From: "michael.i.calderero" Date: Tue, 14 Nov 2017 19:00:28 -0600 Subject: [PATCH 1/2] ensure to use resourceType properly when it is explicitly provided in a chained reference parameter --- .../uhn/fhir/rest/param/ReferenceParam.java | 12 ++++-- .../fhir/rest/param/ReferenceParamTest.java | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java index 572e0ef1a46..b8575226487 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java @@ -104,12 +104,16 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/ 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; } else { resourceType = q.substring(1); } @@ -118,10 +122,12 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/ } } - setValue(theValue); + if (!skipSetValue) { + setValue(theValue); - if (isNotBlank(resourceType) && isBlank(getResourceType())) { - setValue(resourceType + '/' + theValue); + if (isNotBlank(resourceType) && isBlank(getResourceType())) { + setValue(resourceType + '/' + theValue); + } } } diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java index e6c108a1951..055b47a1a69 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java @@ -19,6 +19,7 @@ public class ReferenceParamTest { rp.setValueAsQueryToken(ourCtx, null, null, "Location/123"); assertEquals("Location", rp.getResourceType()); assertEquals("123", rp.getIdPart()); + assertEquals("Location/123", rp.getValue()); assertEquals(null, rp.getQueryParameterQualifier()); } @@ -30,6 +31,7 @@ public class ReferenceParamTest { rp.setValueAsQueryToken(ourCtx, null, ":Location", "123"); assertEquals("Location", rp.getResourceType()); assertEquals("123", rp.getIdPart()); + assertEquals("Location/123", rp.getValue()); assertEquals(null, rp.getQueryParameterQualifier()); } @@ -42,11 +44,51 @@ public class ReferenceParamTest { rp.setValueAsQueryToken(ourCtx, null, ":Location.name", "FOO"); assertEquals("Location", rp.getResourceType()); assertEquals("FOO", rp.getIdPart()); + assertEquals("Location/FOO", rp.getValue()); assertEquals(":Location.name", rp.getQueryParameterQualifier()); assertEquals("name", rp.getChain()); } + @Test + public void testWithResourceTypeAsQualifierAndChain_IdentifierUrlAndValue() { + + ReferenceParam rp = new ReferenceParam(); + 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(":Patient.identifier", rp.getQueryParameterQualifier()); + assertEquals("identifier", rp.getChain()); + + } + + @Test + public void testWithResourceTypeAsQualifierAndChain_IdentifierUrlOnly() { + + 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.getIdPart()); + assertEquals("Patient/http://hey.there/a/b|", rp.getValue()); + assertEquals(":Patient.identifier", rp.getQueryParameterQualifier()); + assertEquals("identifier", rp.getChain()); + + } + + @Test + public void testWithResourceTypeAsQualifierAndChain_ValueOnlyNoUrl() { + + ReferenceParam rp = new ReferenceParam(); + rp.setValueAsQueryToken(ourCtx, null, ":Patient.identifier", "|abc"); + assertEquals("Patient", rp.getResourceType()); + assertEquals("|abc", rp.getIdPart()); + assertEquals("Patient/|abc", rp.getValue()); + assertEquals(":Patient.identifier", rp.getQueryParameterQualifier()); + assertEquals("identifier", rp.getChain()); + + } + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); From 874390df61fdc466cef7ef6e059997a62baf17f0 Mon Sep 17 00:00:00 2001 From: "michael.i.calderero" Date: Wed, 15 Nov 2017 15:03:53 -0600 Subject: [PATCH 2/2] treat reference param values as opaque and not an url when the param is chained --- .../uhn/fhir/rest/param/ReferenceParam.java | 3 + .../fhir/rest/param/ReferenceParamTest.java | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java index b8575226487..88f3605fb1c 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java @@ -119,6 +119,9 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/ } } 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; } } diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java index 055b47a1a69..a61aa706ac2 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java @@ -23,6 +23,57 @@ public class ReferenceParamTest { 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 + public void testWithNoResourceTypeAsQualifierAndChain() { + + ReferenceParam rp = new ReferenceParam(); + rp.setValueAsQueryToken(ourCtx, null, ".name", "FOO"); + assertEquals(null, rp.getResourceType()); + assertEquals("FOO", rp.getIdPart()); + assertEquals("FOO", rp.getValue()); + assertEquals(".name", rp.getQueryParameterQualifier()); + assertEquals("name", rp.getChain()); + + } + + @Test + public void testWithNoResourceTypeAsQualifierAndChain_RelativeUrl() { + + ReferenceParam rp = new ReferenceParam(); + rp.setValueAsQueryToken(ourCtx, null, ".name", "Patient/1233"); + assertEquals(null, rp.getResourceType()); + assertEquals("Patient/1233", rp.getIdPart()); + assertEquals("Patient/1233", rp.getValue()); + assertEquals(".name", rp.getQueryParameterQualifier()); + assertEquals("name", rp.getChain()); + + } + + @Test + public void testWithNoResourceTypeAsQualifierAndChain_AbsoluteUrl() { + + ReferenceParam rp = new ReferenceParam(); + rp.setValueAsQueryToken(ourCtx, null, ".name", "http://something.strange/a/b/c"); + assertEquals(null, rp.getResourceType()); + assertEquals("http://something.strange/a/b/c", rp.getIdPart()); + assertEquals("http://something.strange/a/b/c", rp.getValue()); + assertEquals(".name", rp.getQueryParameterQualifier()); + assertEquals("name", rp.getChain()); + + } @Test public void testWithResourceTypeAsQualifier() { @@ -36,6 +87,33 @@ public class ReferenceParamTest { } + // 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? + @Test + public void testWithResourceTypeAsQualifier_RelativeUrl() { + + ReferenceParam rp = new ReferenceParam(); + rp.setValueAsQueryToken(ourCtx, null, ":Location", "Patient/123"); + assertEquals("Patient", rp.getResourceType()); + assertEquals("123", rp.getIdPart()); + assertEquals("Patient/123", rp.getValue()); + assertEquals(null, rp.getQueryParameterQualifier()); + + } + + // 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("http://a.b/c/d/e", rp.getValue()); + assertEquals(null, rp.getQueryParameterQualifier()); + + } + @Test public void testWithResourceTypeAsQualifierAndChain() {