Include version ID in response for deleted resource

This commit is contained in:
James Agnew 2019-07-03 14:44:12 -04:00
parent 5e44161c9d
commit f1848fb1ad
5 changed files with 87 additions and 17 deletions

View File

@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.server.exceptions;
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
* You may obtain a copy of the License at * You may obtain a copy of the License at
* *
* http://www.apache.org/licenses/LICENSE-2.0 * http://www.apache.org/licenses/LICENSE-2.0
* *
* Unless required by applicable law or agreed to in writing, software * Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, * distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@ -36,6 +36,7 @@ public class ResourceGoneException extends BaseServerResponseException {
public static final int STATUS_CODE = Constants.STATUS_HTTP_410_GONE; public static final int STATUS_CODE = Constants.STATUS_HTTP_410_GONE;
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
private IIdType myResourceId;
/** /**
* Constructor which creates an error message based on a given resource ID * Constructor which creates an error message based on a given resource ID
@ -44,6 +45,7 @@ public class ResourceGoneException extends BaseServerResponseException {
*/ */
public ResourceGoneException(IIdType theResourceId) { public ResourceGoneException(IIdType theResourceId) {
super(STATUS_CODE, "Resource " + (theResourceId != null ? theResourceId.getValue() : "") + " is gone/deleted"); super(STATUS_CODE, "Resource " + (theResourceId != null ? theResourceId.getValue() : "") + " is gone/deleted");
myResourceId = theResourceId;
} }
/** /**
@ -53,6 +55,7 @@ public class ResourceGoneException extends BaseServerResponseException {
@Deprecated @Deprecated
public ResourceGoneException(Class<? extends IBaseResource> theClass, BaseIdentifierDt thePatientId) { public ResourceGoneException(Class<? extends IBaseResource> theClass, BaseIdentifierDt thePatientId) {
super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + thePatientId + " is gone/deleted"); super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + thePatientId + " is gone/deleted");
myResourceId = null;
} }
/** /**
@ -63,6 +66,7 @@ public class ResourceGoneException extends BaseServerResponseException {
*/ */
public ResourceGoneException(Class<? extends IBaseResource> theClass, IIdType theResourceId) { public ResourceGoneException(Class<? extends IBaseResource> theClass, IIdType theResourceId) {
super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + theResourceId + " is gone/deleted"); super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + theResourceId + " is gone/deleted");
myResourceId = theResourceId;
} }
/** /**
@ -84,4 +88,12 @@ public class ResourceGoneException extends BaseServerResponseException {
super(STATUS_CODE, theMessage); super(STATUS_CODE, theMessage);
} }
public IIdType getResourceId() {
return myResourceId;
}
public void setResourceId(IIdType theResourceId) {
myResourceId = theResourceId;
}
} }

View File

@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao;
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
* You may obtain a copy of the License at * You may obtain a copy of the License at
* *
* http://www.apache.org/licenses/LICENSE-2.0 * http://www.apache.org/licenses/LICENSE-2.0
* *
* Unless required by applicable law or agreed to in writing, software * Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, * distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@ -54,6 +54,7 @@ import ca.uhn.fhir.util.*;
import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.Validate;
import org.hl7.fhir.instance.model.api.*; import org.hl7.fhir.instance.model.api.*;
import org.hl7.fhir.r4.model.InstantType; import org.hl7.fhir.r4.model.InstantType;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
@ -863,7 +864,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
throw new ResourceNotFoundException("No resource found with PID " + thePid); throw new ResourceNotFoundException("No resource found with PID " + thePid);
} }
if (entity.get().getDeleted() != null) { if (entity.get().getDeleted() != null) {
throw new ResourceGoneException("Resource was deleted at " + new InstantType(entity.get().getDeleted()).getValueAsString()); throw newResourceGoneException(entity.get());
} }
T retVal = toResource(myResourceType, entity.get(), null, false); T retVal = toResource(myResourceType, entity.get(), null, false);
@ -872,6 +873,16 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
return retVal; return retVal;
} }
@NotNull
private ResourceGoneException newResourceGoneException(BaseHasResource theResourceEntity) {
StringBuilder b = new StringBuilder();
b.append("Resource was deleted at ");
b.append(new InstantType(theResourceEntity.getDeleted()).getValueAsString());
ResourceGoneException retVal = new ResourceGoneException(b.toString());
retVal.setResourceId(theResourceEntity.getIdDt());
return retVal;
}
@Override @Override
public T read(IIdType theId) { public T read(IIdType theId) {
@ -902,22 +913,22 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
if (theDeletedOk == false) { if (theDeletedOk == false) {
if (entity.getDeleted() != null) { if (entity.getDeleted() != null) {
throw new ResourceGoneException("Resource was deleted at " + new InstantType(entity.getDeleted()).getValueAsString()); throw newResourceGoneException(entity);
} }
} }
// Interceptor broadcast: STORAGE_PREACCESS_RESOURCES // Interceptor broadcast: STORAGE_PREACCESS_RESOURCES
{ {
SimplePreResourceAccessDetails accessDetails = new SimplePreResourceAccessDetails(retVal); SimplePreResourceAccessDetails accessDetails = new SimplePreResourceAccessDetails(retVal);
HookParams params = new HookParams() HookParams params = new HookParams()
.add(IPreResourceAccessDetails.class, accessDetails) .add(IPreResourceAccessDetails.class, accessDetails)
.add(RequestDetails.class, theRequest) .add(RequestDetails.class, theRequest)
.addIfMatchesType(ServletRequestDetails.class, theRequest); .addIfMatchesType(ServletRequestDetails.class, theRequest);
JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PREACCESS_RESOURCES, params); JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PREACCESS_RESOURCES, params);
if (accessDetails.isDontReturnResourceAtIndex(0)) { if (accessDetails.isDontReturnResourceAtIndex(0)) {
throw new ResourceNotFoundException(theId); throw new ResourceNotFoundException(theId);
}
} }
}
// Interceptor broadcast: STORAGE_PRESHOW_RESOURCES // Interceptor broadcast: STORAGE_PRESHOW_RESOURCES
{ {
@ -1183,7 +1194,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
outcome.setId(id); outcome.setId(id);
if (theEntity.getDeleted() == null) { if (theEntity.getDeleted() == null) {
outcome.setResource(theResource); outcome.setResource(theResource);
} }
outcome.setEntity(theEntity); outcome.setEntity(theEntity);
@ -1204,7 +1215,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
// Note that this will only fire if someone actually goes to use the // Note that this will only fire if someone actually goes to use the
// resource in a response (it's their responsibility to call // resource in a response (it's their responsibility to call
// outcome.fireResourceViewCallback()) // outcome.fireResourceViewCallback())
outcome.registerResourceViewCallback(()->{ outcome.registerResourceViewCallback(() -> {
if (outcome.getResource() != null) { if (outcome.getResource() != null) {
SimplePreResourceShowDetails showDetails = new SimplePreResourceShowDetails(outcome.getResource()); SimplePreResourceShowDetails showDetails = new SimplePreResourceShowDetails(outcome.getResource());
HookParams params = new HookParams() HookParams params = new HookParams()

View File

@ -375,6 +375,34 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
} catch (ResourceGoneException e) { } catch (ResourceGoneException e) {
// good // good
} }
}
@Test
public void testResourceGoneIncludesVersion() {
Patient p = new Patient();
p.addName().setFamily("FAM").addGiven("GIV");
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
ourClient
.delete()
.resourceById(id)
.execute();
CapturingInterceptor captureInterceptor = new CapturingInterceptor();
ourClient.registerInterceptor(captureInterceptor);
try {
ourClient.read().resource("Patient").withId(id.toUnqualifiedVersionless()).execute();
fail();
} catch (ResourceGoneException e) {
// good
}
List<String> locationHeader = captureInterceptor.getLastResponse().getHeaders(Constants.HEADER_LOCATION);
assertEquals(1, locationHeader.size());
assertThat(locationHeader.get(0), containsString(id.getValue() + "/_history/2"));
} }
@Test @Test

View File

@ -1046,6 +1046,20 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
exception = DEFAULT_EXCEPTION_HANDLER.preProcessOutgoingException(requestDetails, e, theRequest); exception = DEFAULT_EXCEPTION_HANDLER.preProcessOutgoingException(requestDetails, e, theRequest);
} }
/*
* If it's a 410 Gone, we want to include a location header inthe response
* if we can, since that can include the resource version which is nice
* for the user.
*/
if (exception instanceof ResourceGoneException) {
IIdType resourceId = ((ResourceGoneException) exception).getResourceId();
if (resourceId != null && resourceId.hasResourceType() && resourceId.hasIdPart()) {
String baseUrl = myServerAddressStrategy.determineServerBase(theRequest.getServletContext(), theRequest);
resourceId = resourceId.withServerBase(baseUrl, resourceId.getResourceType());
requestDetails.getResponse().addHeader(Constants.HEADER_LOCATION, resourceId.getValue());
}
}
/* /*
* Next, interceptors get a shot at handling the exception * Next, interceptors get a shot at handling the exception
*/ */

View File

@ -220,6 +220,11 @@
request and add it to the response headers. Clients may supply the transaction request and add it to the response headers. Clients may supply the transaction
header via the <![CDATA[<code>X-Request-ID</code>]]> header. header via the <![CDATA[<code>X-Request-ID</code>]]> header.
</action> </action>
<action type="add">
When attempting to read a resource that is deleted, a Location header is now
returned that includes the resource ID and the version ID for the deleted
resource.
</action>
</release> </release>
<release version="3.8.0" date="2019-05-30" description="Hippo"> <release version="3.8.0" date="2019-05-30" description="Hippo">
<action type="fix"> <action type="fix">