Skip to content

Commit

Permalink
issue #3501 - apply fix for #2965 more universally
Browse files Browse the repository at this point in the history
We had duplicate logic in two classes:
* FHIRResource.getRequestBaseUri
* FHIRRestHelper.getRequestBaseUri

For #2965, we fixed the version in FHIRResource but missed the one in
FHIRRestHelper.

Now I combined the two implementations into FHIRRestHelper, made it
static, and removed the version from FHIRResource.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
lmsurpre committed Mar 24, 2022
1 parent bd3b1f4 commit 5f97f94
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 135 deletions.
11 changes: 9 additions & 2 deletions fhir-client/src/main/java/com/ibm/fhir/client/FHIRClient.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2016, 2021
* (C) Copyright IBM Corp. 2016, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -8,12 +8,13 @@

import java.security.KeyStore;

import jakarta.json.JsonObject;
import javax.ws.rs.client.WebTarget;

import com.ibm.fhir.model.resource.Bundle;
import com.ibm.fhir.model.resource.Resource;

import jakarta.json.JsonObject;

/**
* This interface provides a client API for invoking the FHIR Server's REST API.
*/
Expand Down Expand Up @@ -126,6 +127,12 @@ public interface FHIRClient {
*/
public static final String PROPNAME_TENANT_ID = "fhirclient.tenant.id";

/**
* Returns the default FHIR base URL that is configured for this client instance
* @return the FHIR base URL with scheme, host, and path
*/
String getDefaultBaseUrl();

/**
* Returns a JAX-RS 2.0 WebTarget object associated with the REST API endpoint.
* @return a WebTarget instance that can be used to invoke REST APIs.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2016, 2021
* (C) Copyright IBM Corp. 2016, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -819,6 +819,11 @@ private Builder addRequestHeaders(Builder builder, FHIRRequestHeader[] headers)
return builder;
}

@Override
public String getDefaultBaseUrl() {
return baseEndpointURL;
}

@Override
public WebTarget getWebTarget() throws Exception {
return client.target(getBaseEndpointURL());
Expand Down Expand Up @@ -973,7 +978,7 @@ private void setClientProperties(Properties clientProperties) {
this.clientProperties = clientProperties;
}

private String getBaseEndpointURL() {
public String getBaseEndpointURL() {
return baseEndpointURL;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
package com.ibm.fhir.server.test;

import static com.ibm.fhir.model.type.String.string;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.AssertJUnit.assertEquals;
import static org.testng.AssertJUnit.assertNotNull;
import static org.testng.AssertJUnit.assertTrue;
import static org.testng.AssertJUnit.fail;
Expand All @@ -32,6 +32,7 @@
import com.ibm.fhir.model.format.Format;
import com.ibm.fhir.model.generator.FHIRGenerator;
import com.ibm.fhir.model.resource.Bundle;
import com.ibm.fhir.model.resource.Bundle.Entry;
import com.ibm.fhir.model.resource.CapabilityStatement;
import com.ibm.fhir.model.resource.Immunization;
import com.ibm.fhir.model.resource.Observation;
Expand Down Expand Up @@ -78,7 +79,7 @@ public void testMetadataAPI() throws FHIRPathException, FHIRValidationException
CapabilityStatement conf = response.readEntity(CapabilityStatement.class);
assertNotNull(conf);
assertNotNull(conf.getFormat());
assertEquals(6, conf.getFormat().size());
assertEquals(conf.getFormat().size(), 6);
assertNotNull(conf.getVersion());
assertNotNull(conf.getName());

Expand Down Expand Up @@ -160,12 +161,12 @@ public void testMetadataAPI_XML() {
WebTarget target = getWebTarget();
Response response = target.path("metadata").request(FHIRMediaType.APPLICATION_FHIR_XML).get();
assertResponse(response, Response.Status.OK.getStatusCode());
assertEquals(FHIRMediaType.APPLICATION_FHIR_XML_TYPE, response.getMediaType());
assertEquals(response.getMediaType(), FHIRMediaType.APPLICATION_FHIR_XML_TYPE);

CapabilityStatement conf = response.readEntity(CapabilityStatement.class);
assertNotNull(conf);
assertNotNull(conf.getFormat());
assertEquals(6, conf.getFormat().size());
assertEquals(conf.getFormat().size(), 6);
assertNotNull(conf.getVersion());
assertNotNull(conf.getName());
}
Expand All @@ -180,12 +181,12 @@ public void testMetadataAPI_validFhirVersion() {
Collections.singletonMap(FHIRMediaType.FHIR_VERSION_PARAMETER, "4.0"));
Response response = target.path("metadata").request(mediaType).get();
assertResponse(response, Response.Status.OK.getStatusCode());
assertEquals(mediaType, response.getMediaType());
assertEquals(response.getMediaType(), mediaType);

CapabilityStatement conf = response.readEntity(CapabilityStatement.class);
assertNotNull(conf);
assertNotNull(conf.getFormat());
assertEquals(6, conf.getFormat().size());
assertEquals(conf.getFormat().size(), 6);
assertNotNull(conf.getVersion());
assertNotNull(conf.getName());
}
Expand Down Expand Up @@ -554,22 +555,26 @@ public void testUpdateObservation() throws Exception {
}

/**
* Tests the retrieval of the history for a previously saved and updated
* patient.
* Tests the retrieval of the history for a previously saved and updated patient.
*/
@Test(groups = { "server-basic" }, dependsOnMethods = { "testCreatePatient", "testUpdatePatient" })
public void testHistoryPatient() {
WebTarget target = getWebTarget();

// Call the 'history' API to retrieve both the original and updated versions of
// the patient.
// Call the 'history' API to retrieve both the original and updated versions of the patient.
String targetPath = "Patient/" + savedCreatedPatient.getId() + "/_history";
Response response = target.path(targetPath).request(FHIRMediaType.APPLICATION_FHIR_JSON).get();
assertResponse(response, Response.Status.OK.getStatusCode());

Bundle resources = response.readEntity(Bundle.class);
assertNotNull(resources);
assertEquals(2, resources.getEntry().size());
assertEquals(resources.getEntry().size(), 2);

for (Entry entry : resources.getEntry()) {
String fullUrl = entry.getFullUrl().getValue();
assertEquals(fullUrl, getRestBaseURL() + "Patient/" + savedCreatedPatient.getId());
}

Patient updatedPatient = (Patient) resources.getEntry().get(0).getResource();
Patient originalPatient = (Patient) resources.getEntry().get(1).getResource();
// Make sure patient ids are equal, and versionIds are NOT equal.
Expand All @@ -583,22 +588,26 @@ public void testHistoryPatient() {
}

/**
* Tests the retrieval of the history for a previously saved and updated
* observation.
* Tests the retrieval of the history for a previously saved and updated observation.
*/
@Test(groups = { "server-basic" }, dependsOnMethods = { "testCreateObservation", "testUpdateObservation" })
public void testHistoryObservation() {
WebTarget target = getWebTarget();

// Call the 'history' API to retrieve both the original and updated versions of
// the observation.
// Call the 'history' API to retrieve both the original and updated versions of the observation.
String targetPath = "Observation/" + savedCreatedObservation.getId() + "/_history";
Response response = target.path(targetPath).request(FHIRMediaType.APPLICATION_FHIR_JSON).get();
assertResponse(response, Response.Status.OK.getStatusCode());

Bundle resources = response.readEntity(Bundle.class);
assertNotNull(resources);
assertEquals(2, resources.getEntry().size());
assertEquals(resources.getEntry().size(), 2);

for (Entry entry : resources.getEntry()) {
String fullUrl = entry.getFullUrl().getValue();
assertEquals(fullUrl, getRestBaseURL() + "Observation/" + savedCreatedObservation.getId());
}

Observation updatedObservation = (Observation) resources.getEntry().get(0).getResource();
Observation originalObservation = (Observation) resources.getEntry().get(1).getResource();
// Make sure observation ids are equal, and versionIds are NOT equal.
Expand Down Expand Up @@ -728,6 +737,6 @@ public void testSearchObservation() {
assertResponse(response, Response.Status.OK.getStatusCode());
Bundle bundle = response.readEntity(Bundle.class);
assertNotNull(bundle);
assertEquals(1, bundle.getEntry().size());
assertEquals(bundle.getEntry().size(), 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.ibm.fhir.server.resources;

import static com.ibm.fhir.server.util.FHIRRestHelper.getRequestBaseUri;
import static com.ibm.fhir.server.util.IssueTypeToHttpStatusMapper.issueListToStatus;

import java.util.Date;
Expand Down Expand Up @@ -75,7 +76,7 @@ public Response create(@PathParam("type") String type, Resource resource, @Heade
ior = helper.doCreate(type, resource, ifNoneExist);

ResponseBuilder response =
Response.created(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
Response.created(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
resource = ior.getResource();

HTTPReturnPreference returnPreference = FHIRRequestContext.get().getReturnPreference();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ protected OperationOutcome.Issue buildOperationOutcomeIssue(IssueSeverity severi
* the path and query parts
* @return the full URI value as a String
*/
protected String getAbsoluteUri(String baseUri, String relativeUri) {
protected String buildAbsoluteUri(String baseUri, String relativeUri) {
StringBuilder fullUri = new StringBuilder();
fullUri.append(baseUri);
if (!baseUri.endsWith("/")) {
Expand Down Expand Up @@ -440,59 +440,6 @@ protected String getRequestUri() throws Exception {
return FHIRRequestContext.get().getOriginalRequestUri();
}

/**
* This method returns the "base URI" associated with the current request. For example, if a client invoked POST
* 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
* <a href="https://www.hl7.org/fhir/http.html#general">Service Base URL</a>
*/
protected String getRequestBaseUri(String type) throws Exception {
String baseUri = null;

String requestUri = getRequestUri();

// Strip off everything after the path
int queryPathSeparatorLoc = requestUri.indexOf("?");
if (queryPathSeparatorLoc != -1) {
baseUri = requestUri.substring(0, queryPathSeparatorLoc);
} else {
baseUri = requestUri;
}

// Strip off any path elements after the base
if (type != null && !type.isEmpty()) {
int resourceNamePathLocation = baseUri.indexOf("/" + type + "/");
if (resourceNamePathLocation != -1) {
baseUri = requestUri.substring(0, resourceNamePathLocation);
} else {
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")) {
baseUri = baseUri.substring(0, baseUri.length() - "/_history".length());
} else if (baseUri.endsWith("/_search")) {
baseUri = baseUri.substring(0, baseUri.length() - "/_search".length());
} else if (baseUri.contains("/$")) {
baseUri = baseUri.substring(0, baseUri.lastIndexOf("/$"));
}
}

return baseUri;
}

/**
* This method simply returns a URI object containing the specified URI string.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.ibm.fhir.server.resources;

import static com.ibm.fhir.server.spi.operation.FHIROperationUtil.checkAndVerifyOperationAllowed;
import static com.ibm.fhir.server.util.FHIRRestHelper.getRequestBaseUri;
import static com.ibm.fhir.server.util.IssueTypeToHttpStatusMapper.issueListToStatus;

import java.net.URI;
Expand Down Expand Up @@ -505,7 +506,7 @@ private Response buildResponse(FHIROperationContext operationContext, String res
(URI) operationContext.getProperty(FHIROperationContext.PROPNAME_LOCATION_URI);
if (locationURI != null) {
return Response.status(status)
.location(toUri(getAbsoluteUri(getRequestBaseUri(resourceTypeName), locationURI.toString())))
.location(toUri(buildAbsoluteUri(getRequestBaseUri(resourceTypeName), locationURI.toString())))
.entity(resource)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.ibm.fhir.server.resources;

import static com.ibm.fhir.server.util.FHIRRestHelper.getRequestBaseUri;
import static com.ibm.fhir.server.util.IssueTypeToHttpStatusMapper.issueListToStatus;

import java.util.Date;
Expand Down Expand Up @@ -83,7 +84,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id

status = ior.getStatus();
ResponseBuilder response = Response.status(status)
.location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
.location(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));

Resource resource = ior.getResource();
if (resource != null && HTTPReturnPreference.REPRESENTATION == FHIRRequestContext.get().getReturnPreference()) {
Expand Down Expand Up @@ -140,7 +141,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id
ior = helper.doPatch(type, id, patch, ifMatch, null, onlyIfModified);

ResponseBuilder response =
Response.ok().location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
Response.ok().location(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
status = ior.getStatus();
response.status(status);

Expand Down Expand Up @@ -203,7 +204,7 @@ public Response conditionalPatch(@PathParam("type") String type, JsonArray array
ior = helper.doPatch(type, null, patch, ifMatch, searchQueryString, onlyIfModified);

ResponseBuilder response =
Response.ok().location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
Response.ok().location(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
status = ior.getStatus();
response.status(status);

Expand Down Expand Up @@ -272,7 +273,7 @@ public Response conditionalPatch(@PathParam("type") String type, Parameters para

status = ior.getStatus();
ResponseBuilder response = Response.status(status)
.location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
.location(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));

Resource resource = ior.getResource();
if (resource != null && HTTPReturnPreference.REPRESENTATION == FHIRRequestContext.get().getReturnPreference()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.ibm.fhir.server.resources;

import static com.ibm.fhir.server.util.FHIRRestHelper.getRequestBaseUri;
import static com.ibm.fhir.server.util.IssueTypeToHttpStatusMapper.issueListToStatus;

import java.util.Date;
Expand Down Expand Up @@ -75,7 +76,7 @@ public Response update(@PathParam("type") String type, @PathParam("id") String i
ior = helper.doUpdate(type, id, resource, ifMatch, null, onlyIfModified, ifNoneMatch);

ResponseBuilder response = Response.ok()
.location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
.location(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
status = ior.getStatus();
response.status(status);

Expand Down Expand Up @@ -147,7 +148,7 @@ public Response conditionalUpdate(@PathParam("type") String type, Resource resou
ior = helper.doUpdate(type, null, resource, ifMatch, searchQueryString, onlyIfModified, IF_NONE_MATCH_NULL);

ResponseBuilder response =
Response.ok().location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
Response.ok().location(toUri(buildAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
status = ior.getStatus();
response.status(status);

Expand Down
Loading

0 comments on commit 5f97f94

Please sign in to comment.