From 8e1712295fffb7299459374e877ed99151a792aa Mon Sep 17 00:00:00 2001 From: Lee Surprenant Date: Tue, 9 Nov 2021 18:15:01 -0500 Subject: [PATCH 1/2] issue #2965 - modify getRequestBaseUri logic Previously, we looked for the last instnace of `/[resourceType]` in the path and assumed that was the first thing after the baseUrl. However, if a resourceId happened to match the resource type, this proved erroneous. The updated logic looks for the first instance of `/[resourceType]/` in the path instead. If that doesn't exist, then we fall back to the old approach of using the last index of `/[resourceType]` (now that we know it should be safe to do so). This updated logic could break if someone insisted on including a baseUrl that includes a path segment that matches `/[resourceType]/` (e.g. https://example.com/my/Patient/api/ ) but I think that is an acceptable risk because that would be very dumb to do. Alternatives would be to either A. rely solely on a configured baseUrl; or B. do more processing of the URL to ensure we're stripping a path and not a hostname Signed-off-by: Lee Surprenant --- .../com/ibm/fhir/model/util/FHIRUtil.java | 8 +-- .../fhir/server/test/FHIRServerTestBase.java | 68 ++++++++----------- .../ibm/fhir/server/test/ServerSpecTest.java | 10 +-- .../com/ibm/fhir/server/test/UpdateTest.java | 58 ++++++++++++---- .../fhir/server/resources/FHIRResource.java | 14 +++- .../com/ibm/fhir/server/resources/Update.java | 12 ++-- .../ibm/fhir/server/util/FHIRRestHelper.java | 5 +- 7 files changed, 100 insertions(+), 75 deletions(-) diff --git a/fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java b/fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java index aca3165dde4..051910d87d1 100644 --- a/fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java +++ b/fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java @@ -259,18 +259,14 @@ public static OperationOutcome buildOperationOutcome(String message, IssueType i /** * Builds a relative "Location" header value for the specified resource. This will be a string of the form - * "//_history/". Note that the server will turn this into an absolute URL prior to + * "[resource-type]/[id]/_history/[version]". Note that the server will turn this into an absolute URL prior to * returning it to the client. * * @param resource * the resource for which the location header value should be returned */ public static URI buildLocationURI(String type, Resource resource) { - String resourceTypeName = resource.getClass().getSimpleName(); - if (!resourceTypeName.equals(type)) { - resourceTypeName = type; - } - return URI.create(resourceTypeName + "/" + resource.getId() + "/_history/" + resource.getMeta().getVersionId().getValue()); + return URI.create(type + "/" + resource.getId() + "/_history/" + resource.getMeta().getVersionId().getValue()); } /** diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRServerTestBase.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRServerTestBase.java index 75c4502aba9..6659b5cd55d 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRServerTestBase.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRServerTestBase.java @@ -580,7 +580,7 @@ public void assertResponse(Response response, int expectedStatusCode) { assertNotNull(response); assertEquals(expectedStatusCode, response.getStatus()); } - + /** * Verify that the status code in the Response is of the expected status * family. @@ -594,7 +594,7 @@ protected void assertResponse(Response response, Response.Status.Family family) if( ! response.getStatusInfo().getFamily().equals(family) ) { message = response.readEntity(String.class); } - assertEquals(message, response.getStatusInfo().getFamily(), family); + assertEquals(message, family, response.getStatusInfo().getFamily()); } /** @@ -667,10 +667,24 @@ protected void assertValidationOperationOutcome(OperationOutcome oo, String msgP // Misc. common functions used by testcases. // + /** + * Assert that the locationURI is formatted appropriately with the expected baseURL, resourceType, resourceId, and + * and versionId. + * + * @throws Exception + * @implNote Uses {@link #getRestBaseURL()} to construct the base URL for the expected location URI + */ + public void validateLocationURI(String locationURI, String resourceType, + String expectedResourceId, String expectedVersionId) throws Exception { + String expectedLocation = getRestBaseURL() + "/" + resourceType + "/" + + expectedResourceId + "/_history/" + expectedVersionId; + assertEquals(expectedLocation, locationURI.toString()); + } + /** * For the specified response, this function will extract the logical id value * from the response's Location header. The format of a location header value - * should be: [base]///_history/ + * should be: [base]/[resource-type]/[id]/_history/[version] * * @param response the response object for a REST API invocation * @return the logical id value @@ -734,20 +748,6 @@ private String getAbsoluteFilename(String filename) { return null; } - protected String[] getLocationURITokens(String locationURI) { - String[] temp = locationURI.split("/"); - String[] tokens; - if (temp.length > 4) { - tokens = new String[4]; - for (int i = 0; i < 4; i++) { - tokens[i] = temp[temp.length - 4 + i]; - } - } else { - tokens = temp; - } - return tokens; - } - protected Patient setUniqueFamilyName(Patient patient, String uniqueName) { List nameList = new ArrayList(); for(HumanName humanName: patient.getName()) { @@ -787,10 +787,10 @@ public void checkForIssuesWithValidation(Resource resource, boolean failOnValida } System.out.println("count = [" + issues.size() + "]"); - assertEquals(nonWarning,0); + assertEquals(0, nonWarning); if(failOnWarning) { - assertEquals(allOtherIssues,0); + assertEquals(0, allOtherIssues); } } else { @@ -799,9 +799,9 @@ public void checkForIssuesWithValidation(Resource resource, boolean failOnValida } /** - * Parses a location URI into the resourceType, resourceId, and (optionally) the version id. + * Parses a location URI into the resourceType, resource id, and version id. * @param location - * @return + * @return A string array with three parts: resourceType, resourceId, and versionId (in that order) */ public static String[] parseLocationURI(String location) { String[] result = null; @@ -810,25 +810,15 @@ public static String[] parseLocationURI(String location) { } String[] tokens = location.split("/"); + // 1 2 3 4 5 6 7 8 9 10 + // https: // localhost:9443 fhir-server api v4 Patient id _history version + assertEquals(10, tokens.length); + // Check if we should expect 4 tokens or only 2. - if (location.contains("_history")) { - if (tokens.length >= 4) { - result = new String[3]; - result[0] = tokens[tokens.length - 4]; - result[1] = tokens[tokens.length - 3]; - result[2] = tokens[tokens.length - 1]; - } else { - throw new IllegalArgumentException("Incorrect location value specified: " + location); - } - } else { - if (tokens.length >= 2) { - result = new String[2]; - result[0] = tokens[tokens.length - 2]; - result[1] = tokens[tokens.length - 1]; - } else { - throw new IllegalArgumentException("Incorrect location value specified: " + location); - } - } + result = new String[3]; + result[0] = tokens[tokens.length - 4]; + result[1] = tokens[tokens.length - 3]; + result[2] = tokens[tokens.length - 1]; return result; } diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ServerSpecTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ServerSpecTest.java index ef22908fe3d..30dfd5303a1 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ServerSpecTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/ServerSpecTest.java @@ -661,7 +661,7 @@ public void testConditionalUpdateObservation2() throws Exception { String obsId = UUID.randomUUID().toString(); Observation obs = TestUtil.readLocalResource("Observation1.json"); obs = obs.toBuilder() - .subject(Reference.builder().reference(string(fakePatientRef)).build()) + .subject(Reference.builder().reference(fakePatientRef).build()) .build(); // First conditional update should find no matches, so we should get back a 201 @@ -673,11 +673,11 @@ public void testConditionalUpdateObservation2() throws Exception { String locationURI = response.getLocation(); assertNotNull(locationURI); - String[] tokens = parseLocationURI(locationURI); - String resourceId = tokens[1]; + // get the server-assigned resource id from the location header + String resourceId = getLocationLogicalId(response.getResponse()); - // Second conditional update should find 1 match, but because there is a un-matching - // resourceId in the input resource, so we should get back a 400 error. + // Second conditional update should find 1 match, but because there is an un-matching + // resourceId in the input resource, we should get back a 400 error. query = new FHIRParameters().searchParam("_id", resourceId); obs = obs.toBuilder().id(obsId).build(); response = client.conditionalUpdate(obs, query); diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/UpdateTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/UpdateTest.java index 78893a96119..0f098c6e66f 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/UpdateTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/UpdateTest.java @@ -68,6 +68,8 @@ public void testUpdateCreate1() throws Exception { assertNotNull(response); assertResponse(response.getResponse(), Response.Status.CREATED.getStatusCode()); + validateLocationURI(response.getLocation(), "Patient", newId, "1"); + // Now read the resource to verify it's there. response = client.read("Patient", newId); assertNotNull(response); @@ -97,9 +99,9 @@ public void testUpdateCreate2() throws Exception { assertNotNull(response); assertResponse(response.getResponse(), Response.Status.OK.getStatusCode()); String locationURI = response.getLocation(); + + validateLocationURI(locationURI, "Patient", patient.getId(), "2"); String[] locationTokens = parseLocationURI(locationURI); - assertEquals(3, locationTokens.length); - assertEquals("2", locationTokens[2]); // Now read the resource to verify it's there. response = client.vread(locationTokens[0], locationTokens[1], locationTokens[2]); @@ -129,9 +131,9 @@ public void testUpdateIfModified() throws Exception { assertNotNull(response); assertResponse(response.getResponse(), Response.Status.OK.getStatusCode()); String locationURI = response.getLocation(); + + validateLocationURI(locationURI, "Patient", patient.getId(), "2"); String[] locationTokens = parseLocationURI(locationURI); - assertEquals(3, locationTokens.length); - assertEquals("2", locationTokens[2]); // Now read the resource to verify it's there. response = client.vread(locationTokens[0], locationTokens[1], locationTokens[2]); @@ -153,9 +155,9 @@ public void testUpdateIfModified() throws Exception { assertNotNull(response); assertResponse(response.getResponse(), Response.Status.OK.getStatusCode()); String locationURI = response.getLocation(); + + validateLocationURI(locationURI, "Patient", patient.getId(), "3"); String[] locationTokens = parseLocationURI(locationURI); - assertEquals(3, locationTokens.length); - assertEquals("3", locationTokens[2]); // Now read the resource to verify it's there. response = client.vread(locationTokens[0], locationTokens[1], locationTokens[2]); @@ -298,16 +300,15 @@ public void testUpdateCreate3() throws Exception { assertResponse(response.getResponse(), Response.Status.CREATED.getStatusCode()); String locationURI = response.getLocation(); - String[] locationTokens = response.parseLocation(response.getLocation()); - String deletedId = locationTokens[1]; + String resourceId = getLocationLogicalId(response.getResponse()); // Read the new patient. - response = client.read(locationTokens[0], locationTokens[1]); + response = client.read("Patient", resourceId); assertResponse(response.getResponse(), Response.Status.OK.getStatusCode()); Patient createdPatient = response.getResource(Patient.class); assertNotNull(createdPatient); - response = client.delete("Patient", deletedId); + response = client.delete("Patient", resourceId); assertNotNull(response); if (isDeleteSupported()) { assertResponse(response.getResponse(), Response.Status.OK.getStatusCode()); @@ -318,7 +319,7 @@ public void testUpdateCreate3() throws Exception { } // Read the new patient. - response = client.read("Patient", deletedId); + response = client.read("Patient", resourceId); assertResponse(response.getResponse(), Response.Status.GONE.getStatusCode()); // Update the patient. @@ -327,9 +328,38 @@ public void testUpdateCreate3() throws Exception { response = client.update(createdPatient); assertResponse(response.getResponse(), Response.Status.CREATED.getStatusCode()); locationURI = response.getLocation(); - locationTokens = parseLocationURI(locationURI); - assertEquals(3, locationTokens.length); - assertEquals("3", locationTokens[2]); + validateLocationURI(locationURI, "Patient", resourceId, "3"); + } + } + + /** + * Test the "update/create" behavior with an purposefully nasty id. + */ + @Test(dependsOnMethods = {"retrieveConfig"}) + public void testUpdateCreate4() throws Exception { + assertNotNull(updateCreateEnabled); + + // If the "Update/Create" feature is enabled, then test the normal update behavior. + if (updateCreateEnabled.booleanValue()) { + + String nastyId = "Patient" + UUID.randomUUID(); + Patient patient = TestUtil.getMinimalResource(Patient.class).toBuilder() + .id(nastyId) + .build(); + + // Create the new resource via the update operation. + FHIRClient client = getFHIRClient(); + FHIRResponse response = client.update(patient); + assertNotNull(response); + assertResponse(response.getResponse(), Response.Status.CREATED.getStatusCode()); + validateLocationURI(response.getLocationURI().toString(), "Patient", nastyId, "1"); + + // Now read the resource to verify it's there. + response = client.read("Patient", getLocationLogicalId(response.getResponse())); + assertNotNull(response); + assertResponse(response.getResponse(), Response.Status.OK.getStatusCode()); + Patient responsePatient = response.getResource(Patient.class); + assertNotNull(responsePatient); } } diff --git a/fhir-server/src/main/java/com/ibm/fhir/server/resources/FHIRResource.java b/fhir-server/src/main/java/com/ibm/fhir/server/resources/FHIRResource.java index 905dd0d2ae6..435f6cb926e 100644 --- a/fhir-server/src/main/java/com/ibm/fhir/server/resources/FHIRResource.java +++ b/fhir-server/src/main/java/com/ibm/fhir/server/resources/FHIRResource.java @@ -399,6 +399,9 @@ protected String getRequestUri() throws Exception { * https://myhost:9443/fhir-server/api/v4/Patient to create a Patient resource, this method would return * "https://myhost:9443/fhir-server/api/v4". * + * @param type + * The resource type associated with the request URI (e.g. "Patient" in the case of + * https://myhost:9443/fhir-server/api/v4/Patient), or null if there is no such resource type * @return The base endpoint URI associated with the current request. * @throws Exception if an error occurs while reading the config * @implNote This method uses {@link #getRequestUri()} to get the original request URI and then strips it to the @@ -419,12 +422,17 @@ protected String getRequestBaseUri(String type) throws Exception { // Strip off any path elements after the base if (type != null && !type.isEmpty()) { - int resourceNamePathLocation = baseUri.lastIndexOf("/" + type); + int resourceNamePathLocation = baseUri.indexOf("/" + type + "/"); if (resourceNamePathLocation != -1) { baseUri = requestUri.substring(0, resourceNamePathLocation); } else { - // Assume the request was a batch/transaction and just use the requestUri as the base - baseUri = requestUri; + resourceNamePathLocation = baseUri.lastIndexOf("/" + type); + if (resourceNamePathLocation != -1) { + baseUri = requestUri.substring(0, resourceNamePathLocation); + } else { + // Assume the request was a batch/transaction and just use the requestUri as the base + baseUri = requestUri; + } } } else { if (baseUri.endsWith("/_history")) { diff --git a/fhir-server/src/main/java/com/ibm/fhir/server/resources/Update.java b/fhir-server/src/main/java/com/ibm/fhir/server/resources/Update.java index 14788d81810..b98754a4957 100644 --- a/fhir-server/src/main/java/com/ibm/fhir/server/resources/Update.java +++ b/fhir-server/src/main/java/com/ibm/fhir/server/resources/Update.java @@ -57,8 +57,8 @@ public Update() throws Exception { @PUT @Path("{type}/{id}") public Response update(@PathParam("type") String type, @PathParam("id") String id, Resource resource, - @HeaderParam(HttpHeaders.IF_MATCH) String ifMatch, - @HeaderParam(FHIRConstants.UPDATE_IF_MODIFIED_HEADER) boolean onlyIfModified, + @HeaderParam(HttpHeaders.IF_MATCH) String ifMatch, + @HeaderParam(FHIRConstants.UPDATE_IF_MODIFIED_HEADER) boolean onlyIfModified, @HeaderParam(HttpHeaders.IF_NONE_MATCH) String ifNoneMatchHdr) { log.entering(this.getClass().getName(), "update(String,String,Resource)"); Date startTime = new Date(); @@ -73,8 +73,8 @@ public Response update(@PathParam("type") String type, @PathParam("id") String i FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl()); ior = helper.doUpdate(type, id, resource, ifMatch, null, onlyIfModified, ifNoneMatch); - ResponseBuilder response = - Response.ok().location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString()))); + ResponseBuilder response = Response.ok() + .location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString()))); status = ior.getStatus(); response.status(status); @@ -181,13 +181,13 @@ private Integer encodeIfNoneMatch(String value) throws FHIROperationException { if (value == null || value.isEmpty()) { return null; } - + if ("*".equals(value)) { // FHIR resource version numbers start at 1, so we use 0 to represent // all values "*" return IF_NONE_MATCH_ZERO; } - + // Values of the form W/"1" where 1 is the version number would result // in less-than-intuitive behavior and so are not supported at this time if (value.length() > 4 && value.startsWith("W/\"") && value.endsWith("\"")) { diff --git a/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java b/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java index 1a9bc843f27..800cffcac1a 100644 --- a/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java +++ b/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java @@ -445,7 +445,8 @@ private FHIRRestOperationResponse doPatchOrUpdate(String type, String id, FHIRPa } // Persist the resource - FHIRRestOperationResponse ior = doPatchOrUpdatePersist(event, type, id, patch != null, metaResponse.getResource(), metaResponse.getPrevResource(), warnings, metaResponse.isDeleted(), + FHIRRestOperationResponse ior = doPatchOrUpdatePersist(event, type, id, patch != null, + metaResponse.getResource(), metaResponse.getPrevResource(), warnings, metaResponse.isDeleted(), ifNoneMatch); txn.commit(); @@ -1606,7 +1607,7 @@ private Map validateBundle(Bundle bundle) throws Exception { throw buildRestException(msg, IssueType.VALUE); } String fullUrlPlusVersion = fullUrl; - if (resource != null && resource.getMeta() != null + if (resource != null && resource.getMeta() != null && resource.getMeta().getVersionId() != null && resource.getMeta().getVersionId().hasValue()) { fullUrlPlusVersion = fullUrl + resource.getMeta().getVersionId().getValue(); } else { From 3a27bb21bf84ed2ebbb720df03290faf6851dfe0 Mon Sep 17 00:00:00 2001 From: Lee Surprenant Date: Wed, 10 Nov 2021 12:39:50 -0500 Subject: [PATCH 2/2] issue #2965 - document known limitation for custom base urls and add a test to OriginalRequestRewriteServerTest for the exact examples used in the docs Signed-off-by: Lee Surprenant --- docs/src/pages/guides/FHIRServerUsersGuide.md | 4 +- .../ibm/fhir/persistence/FHIRPersistence.java | 9 ++--- .../OriginalRequestRewriteServerTest.java | 38 +++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/docs/src/pages/guides/FHIRServerUsersGuide.md b/docs/src/pages/guides/FHIRServerUsersGuide.md index 746a2683b7b..05f27c77a81 100644 --- a/docs/src/pages/guides/FHIRServerUsersGuide.md +++ b/docs/src/pages/guides/FHIRServerUsersGuide.md @@ -2049,7 +2049,7 @@ This section contains reference information about each of the configuration prop |`fhirServer/core/maxPageSize`|integer|Sets the maximum page size for search and history request results. If a user-specified `_count` parameter value exceeds the maximum page size, then a warning is logged and the maximum page size will be used.| |`fhirServer/core/maxPageIncludeCount`|integer|Sets the maximum number of 'include' resources allowed per page for search and history request results. If the number of 'include' resources returned for a page of results from a search or history request will exceed the maximum number of 'include' resources allowed per page, then an error will be returned in the request results.| |`fhirServer/core/capabilitiesUrl`|string|The URL that is embedded in the default Capabilities statement| -|`fhirServer/core/externalBaseUrl`|string|The base URL that is embedded in the Search bundle response, as of version 4.9.0.| +|`fhirServer/core/externalBaseUrl`|string|The base URL that is embedded in the Search bundle response, as of version 4.9.0. Note that the base URL must not include a path segment that matches any FHIR resource type name (case-sensitive). For example, "https://example.com" or "https://example.com/my/patient/api" are fine, but "https://example.com/my/Patient/api" is not.| |`fhirServer/validation/failFast`|boolean|Indicates whether validation should fail fast on create and update interactions| |`fhirServer/term/capabilitiesUrl`|string|The URL that is embedded in the Terminology Capabilities statement using `mode=terminology`| |`fhirServer/term/disableCaching`|boolean|Indicates whether caching is disabled for the FHIR terminology module, this includes caching in `CodeSystemSupport`, `ValueSetSupport`, `GraphTermServiceProvider`, and `RemoteTermServiceProvider`| @@ -2720,7 +2720,7 @@ IBM FHIR Server Supports the following custom HTTP Headers: |------------------|----------------------------| |`X-FHIR-TENANT-ID`|Specifies which tenant config should be used for the request. Default is `default`. The header name can be overridden via config property `fhirServer/core/tenantIdHeaderName`.| |`X-FHIR-DSID`|Specifies which datastore config should be used for the request. Default is `default`. The header name can be overridden via config property `fhirServer/core/dataSourceIdHeaderName`.| -|`X-FHIR-FORWARDED-URL`|The original (user-facing) request URL; used for constructing absolute URLs within the server response. Only enabled when explicitly configured in the default fhir-server-config.json. If either the config property or the header itself is missing, the server will use the actual request URL. The header name can be overridden via config property `fhirServer/core/originalRequestUriHeaderName`. Note, `fhirServer/core/externalBaseUrl` overrides the `X-FHIR-FORWARDED-URL` and is used to construct the absolute URL.| +|`X-FHIR-FORWARDED-URL`|The original (user-facing) request URL; used for constructing absolute URLs within the server response. Only enabled when explicitly configured in the default fhir-server-config.json. If either the config property or the header itself is missing, the server will use the actual request URL. The header name can be overridden via config property `fhirServer/core/originalRequestUriHeaderName`. Note that `fhirServer/core/externalBaseUrl` overrides the `X-FHIR-FORWARDED-URL` and is used to construct the absolute URL. Also note that the base URL's value must not include a path segment that matches any FHIR resource type name (case-sensitive). For example, "https://example.com" or "https://example.com/my/patient/api" are fine, but "https://example.com/my/Patient/api" is not.| |`X-FHIR-UPDATE-IF-MODIFIED`|When set to true, for update and patch requests, the server will perform a resource comparison and only perform the update if the contents of the resource have changed. For all other values, the update will be executed as normal.| # 6 Related topics diff --git a/fhir-persistence/src/main/java/com/ibm/fhir/persistence/FHIRPersistence.java b/fhir-persistence/src/main/java/com/ibm/fhir/persistence/FHIRPersistence.java index 31be0bc1946..09e28f27e2a 100644 --- a/fhir-persistence/src/main/java/com/ibm/fhir/persistence/FHIRPersistence.java +++ b/fhir-persistence/src/main/java/com/ibm/fhir/persistence/FHIRPersistence.java @@ -12,7 +12,6 @@ import com.ibm.fhir.model.resource.OperationOutcome; import com.ibm.fhir.model.resource.Resource; -import com.ibm.fhir.model.type.Instant; import com.ibm.fhir.persistence.context.FHIRPersistenceContext; import com.ibm.fhir.persistence.erase.EraseDTO; import com.ibm.fhir.persistence.exception.FHIRPersistenceException; @@ -28,7 +27,7 @@ public interface FHIRPersistence { /** * Stores a new FHIR Resource in the datastore. Id assignment handled by the implementation. - * This method has been deprecated. Instead, generate the logical id first and use the + * This method has been deprecated. Instead, generate the logical id first and use the * createWithMeta(context, resource) call instead. * @param context the FHIRPersistenceContext instance associated with the current request * @param resource the FHIR Resource instance to be created in the datastore @@ -38,7 +37,7 @@ public interface FHIRPersistence { */ @Deprecated SingleResourceResult create(FHIRPersistenceContext context, T resource) throws FHIRPersistenceException; - + /** * Stores a new FHIR Resource in the datastore. The resource is not modified before it is stored. It * must therefore already include correct Meta fields. Should be used instead of @@ -142,7 +141,7 @@ default SingleResourceResult delete(FHIRPersistenceConte * @throws FHIRPersistenceException */ MultiResourceResult search(FHIRPersistenceContext context, Class resourceType) throws FHIRPersistenceException; - + /** * Returns true iff the persistence layer implementation supports transactions. */ @@ -167,7 +166,7 @@ default SingleResourceResult delete(FHIRPersistenceConte default boolean isDeleteSupported() { return false; } - + /** * Returns true iff the persistence layer implementation supports update/create and it has been * configured in the persistence config. diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/OriginalRequestRewriteServerTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/OriginalRequestRewriteServerTest.java index 0a5bd095719..ff07072cdb8 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/OriginalRequestRewriteServerTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/OriginalRequestRewriteServerTest.java @@ -97,4 +97,42 @@ public void testWholeSystemSearch_BaseUrlRewrite() throws Exception { assertTrue(responseBundle.getEntry().stream().allMatch(e -> e.getFullUrl().getValue().startsWith(ORIGINAL_URI + "/Patient")), "All search response bundle entry fullUrls start with the passed in uri"); } + + /** + * Create a Patient and ensure the Location header reflects the OriginalRequestUri + */ + @Test + public void testCreatePatientWithCustomBaseUrls() throws Exception { + WebTarget target = getWebTarget(); + String customBaseUrl; + Patient patient = TestUtil.getMinimalResource(Patient.class); + Entity entity = Entity.entity(patient, FHIRMediaType.APPLICATION_FHIR_JSON); + Response response; + + // Build a new Patient and then call the 'create' API. + customBaseUrl = "https://example.com"; + response = target.path("Patient").request() + .header("X-FHIR-Forwarded-URL", customBaseUrl + "/Patient") + .post(entity, Response.class); + assertResponse(response, Response.Status.CREATED.getStatusCode()); + assertTrue(response.getLocation().toString().startsWith(customBaseUrl), + response.getLocation().toString() + " should begin with " + customBaseUrl); + + customBaseUrl = "https://example.com/my/patient/api"; + response = target.path("Patient").request() + .header("X-FHIR-Forwarded-URL", customBaseUrl + "/Patient") + .post(entity, Response.class); + assertResponse(response, Response.Status.CREATED.getStatusCode()); + assertTrue(response.getLocation().toString().startsWith(customBaseUrl), + response.getLocation().toString() + " should begin with " + customBaseUrl); + + customBaseUrl = "https://example.com/my/Patient/api"; + response = target.path("Patient").request() + .header("X-FHIR-Forwarded-URL", customBaseUrl + "/Patient") + .post(entity, Response.class); + assertResponse(response, Response.Status.CREATED.getStatusCode()); + assertFalse(response.getLocation().toString().startsWith(customBaseUrl), + response.getLocation().toString() + " doesn't begin with " + customBaseUrl + + " because of a documented limitation"); + } }