Skip to content

Commit

Permalink
issue #2526 - deprecate the /Resource and /DomainResource endpoints
Browse files Browse the repository at this point in the history
Because we use "Resource" as the resource type for whole-system search
and whole-system history interactions, it wasn't as simple as just
checking for abstract types in FHIRRestHelper.validateInteraction.
Instead, I now check that the type in the request path is valid directly
from the JAX-RS endpoints / bundle entry processing.
This has the additional benefit of catching URL path errors up front,
avoiding almost all work associated with a bad request.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
lmsurpre committed Jul 8, 2021
1 parent 1a2b5fd commit 82a7f2b
Show file tree
Hide file tree
Showing 12 changed files with 249 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public Response create(@PathParam("type") String type, Resource resource, @Heade

try {
checkInitComplete();
checkType(type);

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doCreate(type, resource, ifNoneExist);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public Response delete(@PathParam("type") String type, @PathParam("id") String i

try {
checkInitComplete();
checkType(type);

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doDelete(type, id, null);
Expand Down Expand Up @@ -93,6 +94,7 @@ public Response conditionalDelete(@PathParam("type") String type) throws Excepti

try {
checkInitComplete();
checkType(type);

String searchQueryString = httpServletRequest.getQueryString();
if (searchQueryString == null || searchQueryString.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,21 @@
import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.config.PropertyGroup;
import com.ibm.fhir.core.FHIRConstants;
import com.ibm.fhir.exception.FHIROperationException;
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.OperationOutcome;
import com.ibm.fhir.model.resource.OperationOutcome.Issue;
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.CodeableConcept;
import com.ibm.fhir.model.type.Extension;
import com.ibm.fhir.model.type.code.IssueSeverity;
import com.ibm.fhir.model.type.code.IssueType;
import com.ibm.fhir.model.util.FHIRUtil;
import com.ibm.fhir.model.util.ModelSupport;
import com.ibm.fhir.persistence.FHIRPersistence;
import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
import com.ibm.fhir.persistence.helper.FHIRPersistenceHelper;
Expand Down Expand Up @@ -118,6 +122,19 @@ protected void checkInitComplete() throws FHIROperationException {
}
}

/**
* This method will do a quick check of the {type} URL parameter. If the type is not a valid FHIR resource type, then
* we'll throw an error to short-circuit the current in-progress REST API invocation.
*/
protected void checkType(String type) throws FHIROperationException {
if (!ModelSupport.isResourceType(type)) {
throw buildUnsupportedResourceTypeException(type);
}
if (!ModelSupport.isConcreteResourceType(type)) {
log.warning("Use of abstract resource types like '" + type + "' in FHIR URLs is deprecated and will be removed in a future release");
}
}

public FHIRResource() throws Exception {
if (log.isLoggable(Level.FINEST)) {
log.entering(this.getClass().getName(), "FHIRResource ctor");
Expand Down Expand Up @@ -429,4 +446,19 @@ protected String getRequestBaseUri(String type) throws Exception {
protected URI toUri(String uriString) throws URISyntaxException {
return new URI(uriString);
}

protected FHIROperationException buildUnsupportedResourceTypeException(String resourceTypeName) {
String msg = "'" + resourceTypeName + "' is not a valid resource type.";
Issue issue = OperationOutcome.Issue.builder()
.severity(IssueSeverity.FATAL)
.code(IssueType.NOT_SUPPORTED.toBuilder()
.extension(Extension.builder()
.url(FHIRConstants.EXT_BASE + "not-supported-detail")
.value(Code.of("resource"))
.build())
.build())
.details(CodeableConcept.builder().text(string(Encode.forHtml(msg))).build())
.build();
return new FHIROperationException(msg).withIssue(issue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public Response history(@PathParam("type") String type, @PathParam("id") String

try {
checkInitComplete();
checkType(type);

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
bundle = helper.doHistory(type, id, uriInfo.getQueryParameters(), getRequestUri());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ public Response invoke(@PathParam("resourceTypeName") String resourceTypeName,

try {
checkInitComplete();
checkType(resourceTypeName);
checkAndVerifyOperationAllowed(operationName);

FHIROperationContext operationContext =
Expand Down Expand Up @@ -245,6 +246,7 @@ public Response invoke(@PathParam("resourceTypeName") String resourceTypeName,

try {
checkInitComplete();
checkType(resourceTypeName);
checkAndVerifyOperationAllowed(operationName);

FHIROperationContext operationContext =
Expand Down Expand Up @@ -304,6 +306,7 @@ public Response invoke(@PathParam("resourceTypeName") String resourceTypeName,

try {
checkInitComplete();
checkType(resourceTypeName);
checkAndVerifyOperationAllowed(operationName);

FHIROperationContext operationContext =
Expand Down Expand Up @@ -350,6 +353,7 @@ public Response invoke(@PathParam("resourceTypeName") String resourceTypeName,

try {
checkInitComplete();
checkType(resourceTypeName);
checkAndVerifyOperationAllowed(operationName);

FHIROperationContext operationContext =
Expand Down Expand Up @@ -397,6 +401,7 @@ public Response invoke(@PathParam("resourceTypeName") String resourceTypeName,

try {
checkInitComplete();
checkType(resourceTypeName);
checkAndVerifyOperationAllowed(operationName);

FHIROperationContext operationContext =
Expand Down Expand Up @@ -444,6 +449,7 @@ public Response invoke(@PathParam("resourceTypeName") String resourceTypeName,

try {
checkInitComplete();
checkType(resourceTypeName);
checkAndVerifyOperationAllowed(operationName);

FHIROperationContext operationContext =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@

import javax.annotation.security.RolesAllowed;
import javax.enterprise.context.RequestScoped;
import jakarta.json.JsonArray;
import jakarta.json.JsonPatch;
import jakarta.json.JsonValue;
import javax.ws.rs.Consumes;
import javax.ws.rs.HeaderParam;
import javax.ws.rs.Path;
Expand Down Expand Up @@ -46,6 +43,10 @@
import com.ibm.fhir.server.util.FHIRRestHelper;
import com.ibm.fhir.server.util.RestAuditLogger;

import jakarta.json.JsonArray;
import jakarta.json.JsonPatch;
import jakarta.json.JsonValue;

@Path("/")
@Consumes({ FHIRMediaType.APPLICATION_FHIR_JSON, MediaType.APPLICATION_JSON,
FHIRMediaType.APPLICATION_FHIR_XML, MediaType.APPLICATION_XML })
Expand Down Expand Up @@ -73,6 +74,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id

try {
checkInitComplete();
checkType(type);

FHIRPatch patch = createPatch(array);

Expand Down Expand Up @@ -125,6 +127,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id

try {
checkInitComplete();
checkType(type);

FHIRPatch patch;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public Response read(@PathParam("type") String type, @PathParam("id") String id,

try {
checkInitComplete();
checkType(type);

MultivaluedMap<String, String> queryParameters = uriInfo.getQueryParameters();
long modifiedSince = parseIfModifiedSince();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ private Response doSearch(String type) {

try {
checkInitComplete();
checkType(type);

queryParameters = uriInfo.getQueryParameters();
FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
Expand Down Expand Up @@ -115,6 +116,7 @@ private Response doSearchCompartment(String compartment, String compartmentId, S

try {
checkInitComplete();
checkType(type);

queryParameters = uriInfo.getQueryParameters();
FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public Response update(@PathParam("type") String type, @PathParam("id") String i

try {
checkInitComplete();
checkType(type);

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doUpdate(type, id, resource, ifMatch, null, onlyIfModified);
Expand Down Expand Up @@ -116,6 +117,7 @@ public Response conditionalUpdate(@PathParam("type") String type, Resource resou

try {
checkInitComplete();
checkType(type);

String searchQueryString = httpServletRequest.getQueryString();
if (searchQueryString == null || searchQueryString.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public Response vread(@PathParam("type") String type, @PathParam("id") String id

try {
checkInitComplete();
checkType(type);

MultivaluedMap<String, String> queryParameters = uriInfo.getQueryParameters();

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
Expand Down
Loading

0 comments on commit 82a7f2b

Please sign in to comment.