Skip to content

issue #2965 - modify getRequestBaseUri logic #2966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lmsurpre merged 2 commits into from
Nov 10, 2021
Merged

issue #2965 - modify getRequestBaseUri logic #2966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e17122
issue #2965 - modify getRequestBaseUri logic
lmsurpre Nov 9, 2021
3a27bb2
issue #2965 - document known limitation for custom base urls
lmsurpre Nov 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/src/pages/guides/FHIRServerUsersGuide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`|
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <code>"<resource-type>/<id>/_history/<version>"</code>. Note that the server will turn this into an absolute URL prior to
* <code>"[resource-type]/[id]/_history/[version]"</code>. 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());
}

/**
Expand Down
9 changes: 4 additions & 5 deletions fhir-persistence/src/main/java/com/ibm/fhir/persistence/FHIRPersistence.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -38,7 +37,7 @@ public interface FHIRPersistence {
*/
@Deprecated
<T extends Resource> SingleResourceResult<T> 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
Expand Down Expand Up @@ -142,7 +141,7 @@ default <T extends Resource> SingleResourceResult<T> delete(FHIRPersistenceConte
* @throws FHIRPersistenceException
*/
MultiResourceResult<Resource> search(FHIRPersistenceContext context, Class<? extends Resource> resourceType) throws FHIRPersistenceException;

/**
* Returns true iff the persistence layer implementation supports transactions.
*/
Expand All @@ -167,7 +166,7 @@ default <T extends Resource> SingleResourceResult<T> 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.
Expand Down
68 changes: 29 additions & 39 deletions fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRServerTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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());
}

/**
Expand Down Expand Up @@ -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: <code>[base]/<resource-type>/<id>/_history/<version></code>
* should be: <code>[base]/[resource-type]/[id]/_history/[version]</code>
*
* @param response the response object for a REST API invocation
* @return the logical id value
Expand Down Expand Up @@ -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 <HumanName> nameList = new ArrayList<HumanName>();
for(HumanName humanName: patient.getName()) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
38 changes: 38 additions & 0 deletions ...-server-test/src/test/java/com/ibm/fhir/server/test/OriginalRequestRewriteServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Patient> 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");
}
}
10 changes: 5 additions & 5 deletions fhir-server-test/src/test/java/com/ibm/fhir/server/test/ServerSpecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
Loading