Adjust return type for fluent delete() operation (#1805)

* Add cascading delete to client

* Add changelog

* Client delete should return MethodOutcome

* Rerturn more appropriate type for delete operations

* Refactor cascade detection to be reusable
This commit is contained in:
James Agnew 2020-04-21 15:23:11 -04:00 committed by GitHub
parent f218ade480
commit c716216b34
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 195 additions and 133 deletions

View File

@ -21,9 +21,10 @@ package ca.uhn.fhir.rest.gclient;
*/
import ca.uhn.fhir.rest.api.DeleteCascadeModeEnum;
import ca.uhn.fhir.rest.api.MethodOutcome;
import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
public interface IDeleteTyped extends IClientExecutable<IDeleteTyped, IBaseOperationOutcome> {
public interface IDeleteTyped extends IClientExecutable<IDeleteTyped, MethodOutcome> {
/**
* Delete cascade mode - Note that this is a HAPI FHIR specific feature and is not supported on all servers.

View File

@ -72,6 +72,7 @@ public interface IQuery<Y> extends IBaseQuery<IQuery<Y>>, IClientExecutable<IQue
* on a single page.
*
* @deprecated This parameter is badly named, since FHIR calls this parameter "_count" and not "_limit". Use {@link #count(int)} instead (it also sets the _count parameter)
* @see #count(int)
*/
@Deprecated
IQuery<Y> limitTo(int theLimitTo);

View File

@ -604,7 +604,7 @@ public class GenericClient extends BaseClient implements IGenericClient {
}
private class DeleteInternal extends BaseSearch<IDeleteTyped, IDeleteWithQueryTyped, IBaseOperationOutcome> implements IDelete, IDeleteTyped, IDeleteWithQuery, IDeleteWithQueryTyped {
private class DeleteInternal extends BaseSearch<IDeleteTyped, IDeleteWithQueryTyped, MethodOutcome> implements IDelete, IDeleteTyped, IDeleteWithQuery, IDeleteWithQueryTyped {
private boolean myConditional;
private IIdType myId;
@ -613,7 +613,7 @@ public class GenericClient extends BaseClient implements IGenericClient {
private DeleteCascadeModeEnum myCascadeMode;
@Override
public IBaseOperationOutcome execute() {
public MethodOutcome execute() {
Map<String, List<String>> additionalParams = new HashMap<>();
if (myCascadeMode != null) {
@ -635,7 +635,8 @@ public class GenericClient extends BaseClient implements IGenericClient {
} else {
invocation = DeleteMethodBinding.createDeleteInvocation(getFhirContext(), mySearchUrl, getParamMap());
}
OperationOutcomeResponseHandler binding = new OperationOutcomeResponseHandler();
OutcomeResponseHandler binding = new OutcomeResponseHandler();
return invoke(additionalParams, binding, invocation);
}
@ -1389,30 +1390,6 @@ public class GenericClient extends BaseClient implements IGenericClient {
}
}
private final class OperationOutcomeResponseHandler implements IClientResponseHandler<IBaseOperationOutcome> {
@Override
public IBaseOperationOutcome invokeClient(String theResponseMimeType, InputStream theResponseInputStream, int theResponseStatusCode, Map<String, List<String>> theHeaders)
throws BaseServerResponseException {
EncodingEnum respType = EncodingEnum.forContentType(theResponseMimeType);
if (respType == null) {
return null;
}
IParser parser = respType.newParser(myContext);
IBaseOperationOutcome retVal;
try {
// TODO: handle if something else than OO comes back
retVal = (IBaseOperationOutcome) parser.parseResource(theResponseInputStream);
} catch (DataFormatException e) {
ourLog.warn("Failed to parse OperationOutcome response", e);
return null;
}
MethodUtil.parseClientRequestResourceHeaders(null, theHeaders, retVal);
return retVal;
}
}
private final class OutcomeResponseHandler implements IClientResponseHandler<MethodOutcome> {
private PreferReturnEnum myPrefer;

View File

@ -236,17 +236,23 @@ public class GenericClientExample {
// START SNIPPET: conformance
// Retrieve the server's conformance statement and print its
// description
CapabilityStatement conf = client.capabilities().ofType(CapabilityStatement.class).execute();
CapabilityStatement conf = client
.capabilities()
.ofType(CapabilityStatement.class)
.execute();
System.out.println(conf.getDescriptionElement().getValue());
// END SNIPPET: conformance
}
{
// START SNIPPET: delete
IBaseOperationOutcome resp = client.delete().resourceById(new IdType("Patient", "1234")).execute();
MethodOutcome response = client
.delete()
.resourceById(new IdType("Patient", "1234"))
.execute();
// outcome may be null if the server didn't return one
if (resp != null) {
OperationOutcome outcome = (OperationOutcome) resp;
OperationOutcome outcome = (OperationOutcome) response.getOperationOutcome();
if (outcome != null) {
System.out.println(outcome.getIssueFirstRep().getDetails().getCodingFirstRep().getCode());
}
// END SNIPPET: delete
@ -356,7 +362,8 @@ public class GenericClientExample {
.revInclude(Provenance.INCLUDE_TARGET)
.lastUpdated(new DateRangeParam("2011-01-01", null))
.sort().ascending(Patient.BIRTHDATE)
.sort().descending(Patient.NAME).limitTo(123)
.sort().descending(Patient.NAME)
.count(123)
.returnBundle(Bundle.class)
.execute();
// END SNIPPET: searchAdv

View File

@ -33,3 +33,20 @@
These classes have not changed in terms of functionality, but existing projects may need to adjust some
package import statements.
"
- item:
issue: "1804"
type: "change"
title: "**Breaking Change**:
The Generic/Fluent **delete()** operation now returns a [MethodOutcome](/apidocs/hapi-fhir-base/ca/uhn/fhir/rest/api/MethodOutcome.html)
object instead of an OperationOutcome. The OperationOutcomoe is still available direcly by querying
the MethodOutcome object, but this change makes the delete() method more consistent with
other similar methods in the API.
"
- item:
type: "change"
title: "**Breaking Change**:
Some R4 and R5 structure fields containing a `code` value with a **Required (closed) binding**
did not use the java Enum type that was generated for the given field. These have been changed
to use the Enum values where possible. This change does not remove any functionality from the model
but may require a small amount of re-coding to deal with new setter/getter types on a few fields.
"

View File

@ -168,7 +168,7 @@ public class AbstractJaxRsResourceProviderDstu3Test {
@Test
public void testDeletePatient() {
when(mock.delete(idCaptor.capture(), conditionalCaptor.capture())).thenReturn(new MethodOutcome());
final IBaseOperationOutcome results = client.delete().resourceById("Patient", "1").execute();
final IBaseOperationOutcome results = client.delete().resourceById("Patient", "1").execute().getOperationOutcome();
assertEquals("1", idCaptor.getValue().getIdPart());
}

View File

@ -164,7 +164,7 @@ public class AbstractJaxRsResourceProviderTest {
@Test
public void testDeletePatient() {
when(mock.delete(idCaptor.capture(), conditionalCaptor.capture())).thenReturn(new MethodOutcome());
final IBaseOperationOutcome results = client.delete().resourceById("Patient", "1").execute();
final IBaseOperationOutcome results = client.delete().resourceById("Patient", "1").execute().getOperationOutcome();
assertEquals("1", idCaptor.getValue().getIdPart());
}

View File

@ -34,8 +34,10 @@ import ca.uhn.fhir.jpa.delete.DeleteConflictOutcome;
import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.DeleteCascadeModeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.ResponseDetails;
import ca.uhn.fhir.rest.server.RestfulServerUtils;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.util.OperationOutcomeUtil;
import org.apache.commons.lang3.Validate;
@ -45,6 +47,9 @@ import org.hl7.fhir.r4.model.OperationOutcome;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.validation.constraints.Null;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
@ -92,7 +97,12 @@ public class CascadingDeleteInterceptor {
public DeleteConflictOutcome handleDeleteConflicts(DeleteConflictList theConflictList, RequestDetails theRequest) {
ourLog.debug("Have delete conflicts: {}", theConflictList);
if (!shouldCascade(theRequest)) {
if (shouldCascade(theRequest) == DeleteCascadeModeEnum.NONE) {
// Add a message to the response
String message = theRequest.getFhirContext().getLocalizer().getMessage(CascadingDeleteInterceptor.class, "noParam");
theRequest.getUserData().put(CASCADED_DELETES_FAILED_KEY, message);
return null;
}
@ -180,28 +190,12 @@ public class CascadingDeleteInterceptor {
/**
* Subclasses may override
*
* @param theRequest The REST request
* @param theRequest The REST request (may be null)
* @return Returns true if cascading delete should be allowed
*/
@SuppressWarnings("WeakerAccess")
protected boolean shouldCascade(RequestDetails theRequest) {
if (theRequest != null) {
String[] cascadeParameters = theRequest.getParameters().get(Constants.PARAMETER_CASCADE_DELETE);
if (cascadeParameters != null && Arrays.asList(cascadeParameters).contains(Constants.CASCADE_DELETE)) {
return true;
}
String cascadeHeader = theRequest.getHeader(Constants.HEADER_CASCADE);
if (Constants.CASCADE_DELETE.equals(cascadeHeader)) {
return true;
}
// Add a message to the response
String message = theRequest.getFhirContext().getLocalizer().getMessage(CascadingDeleteInterceptor.class, "noParam");
theRequest.getUserData().put(CASCADED_DELETES_FAILED_KEY, message);
}
return false;
@Nonnull
protected DeleteCascadeModeEnum shouldCascade(@Nullable RequestDetails theRequest) {
return RestfulServerUtils.extractDeleteCascadeParameter(theRequest);
}

View File

@ -814,15 +814,13 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
myDaoConfig.setAllowMultipleDelete(true);
//@formatter:off
IBaseOperationOutcome response = ourClient
MethodOutcome response = ourClient
.delete()
.resourceConditionalByType(Patient.class)
.where(Patient.IDENTIFIER.exactly().code(methodName))
.execute();
//@formatter:on
String encoded = myFhirCtx.newXmlParser().encodeResourceToString(response);
String encoded = myFhirCtx.newXmlParser().encodeResourceToString(response.getOperationOutcome());
ourLog.info(encoded);
assertThat(encoded, containsString(
"<issue><severity value=\"information\"/><code value=\"informational\"/><diagnostics value=\"Successfully deleted 2 resource(s) in "));
@ -1028,8 +1026,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
p.addName().setFamily("FAM");
IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless();
IBaseOperationOutcome resp = ourClient.delete().resourceById(id).execute();
OperationOutcome oo = (OperationOutcome) resp;
MethodOutcome resp = ourClient.delete().resourceById(id).execute();
OperationOutcome oo = (OperationOutcome) resp.getOperationOutcome();
assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Successfully deleted 1 resource(s) in "));
}

View File

@ -1257,15 +1257,13 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
myDaoConfig.setAllowMultipleDelete(true);
//@formatter:off
IBaseOperationOutcome response = ourClient
MethodOutcome response = ourClient
.delete()
.resourceConditionalByType(Patient.class)
.where(Patient.IDENTIFIER.exactly().code(methodName))
.execute();
//@formatter:on
String encoded = myFhirCtx.newXmlParser().encodeResourceToString(response);
String encoded = myFhirCtx.newXmlParser().encodeResourceToString(response.getOperationOutcome());
ourLog.info(encoded);
assertThat(encoded, containsString(
"<issue><severity value=\"information\"/><code value=\"informational\"/><diagnostics value=\"Successfully deleted 2 resource(s) in "));
@ -1478,8 +1476,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
p.addName().setFamily("FAM");
IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless();
IBaseOperationOutcome resp = ourClient.delete().resourceById(id).execute();
OperationOutcome oo = (OperationOutcome) resp;
MethodOutcome resp = ourClient.delete().resourceById(id).execute();
OperationOutcome oo = (OperationOutcome) resp.getOperationOutcome();
assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Successfully deleted 1 resource(s) in "));
}

View File

@ -31,6 +31,7 @@ import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.model.valueset.BundleTypeEnum;
import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.DeleteCascadeModeEnum;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.api.PreferHeader;
import ca.uhn.fhir.rest.api.PreferReturnEnum;
@ -78,6 +79,7 @@ public class RestfulServerUtils {
private static final HashSet<String> TEXT_ENCODE_ELEMENTS = new HashSet<>(Arrays.asList("*.text", "*.id", "*.meta", "*.(mandatory)"));
private static Map<FhirVersionEnum, FhirContext> myFhirContextMap = Collections.synchronizedMap(new HashMap<>());
private static EnumSet<RestOperationTypeEnum> ourOperationsWhichAllowPreferHeader = EnumSet.of(RestOperationTypeEnum.CREATE, RestOperationTypeEnum.UPDATE, RestOperationTypeEnum.PATCH);
private enum NarrativeModeEnum {
NORMAL, ONLY, SUPPRESS;
@ -696,8 +698,6 @@ public class RestfulServerUtils {
return retVal;
}
private static EnumSet<RestOperationTypeEnum> ourOperationsWhichAllowPreferHeader = EnumSet.of(RestOperationTypeEnum.CREATE, RestOperationTypeEnum.UPDATE, RestOperationTypeEnum.PATCH);
public static boolean respectPreferHeader(RestOperationTypeEnum theRestOperationType) {
return ourOperationsWhichAllowPreferHeader.contains(theRestOperationType);
}
@ -749,8 +749,6 @@ public class RestfulServerUtils {
}
public static boolean prettyPrintResponse(IRestfulServerDefaults theServer, RequestDetails theRequest) {
Map<String, String[]> requestParams = theRequest.getParameters();
String[] pretty = requestParams.get(Constants.PARAM_PRETTY);
@ -954,5 +952,22 @@ public class RestfulServerUtils {
}
/**
* @since 5.0.0
*/
public static DeleteCascadeModeEnum extractDeleteCascadeParameter(RequestDetails theRequest) {
if (theRequest != null) {
String[] cascadeParameters = theRequest.getParameters().get(Constants.PARAMETER_CASCADE_DELETE);
if (cascadeParameters != null && Arrays.asList(cascadeParameters).contains(Constants.CASCADE_DELETE)) {
return DeleteCascadeModeEnum.DELETE;
}
String cascadeHeader = theRequest.getHeader(Constants.HEADER_CASCADE);
if (Constants.CASCADE_DELETE.equals(cascadeHeader)) {
return DeleteCascadeModeEnum.DELETE;
}
}
return DeleteCascadeModeEnum.NONE;
}
}

View File

@ -444,7 +444,7 @@ public class GenericClientR4Test {
});
IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
IBaseOperationOutcome outcome;
MethodOutcome outcome;
// Regular delete
outcome = client
@ -1707,7 +1707,6 @@ public class GenericClientR4Test {
idx++;
}
@SuppressWarnings("deprecation")
@Test
public void testSearchByQuantity() throws Exception {
final String msg = "{\"resourceType\":\"Bundle\",\"id\":null,\"base\":\"http://localhost:57931/fhir/contextDev\",\"total\":1,\"link\":[{\"relation\":\"self\",\"url\":\"http://localhost:57931/fhir/contextDev/Patient?identifier=urn%3AMultiFhirVersionTest%7CtestSubmitPatient01&_format=json\"}],\"entry\":[{\"resource\":{\"resourceType\":\"Patient\",\"id\":\"1\",\"meta\":{\"versionId\":\"1\",\"lastUpdated\":\"2014-12-20T18:41:29.706-05:00\"},\"identifier\":[{\"system\":\"urn:MultiFhirVersionTest\",\"value\":\"testSubmitPatient01\"}]}}]}";

View File

@ -5,6 +5,7 @@ import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.*;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum;
import ca.uhn.fhir.rest.client.exceptions.FhirClientConnectionException;
import ca.uhn.fhir.rest.client.exceptions.NonFhirResponseException;
import ca.uhn.fhir.rest.client.impl.BaseClient;
import ca.uhn.fhir.rest.client.impl.GenericClient;
@ -386,24 +387,82 @@ public class GenericClientTest {
IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
OperationOutcome outcome = (OperationOutcome) client.delete().resourceById("Patient", "123")
.withAdditionalHeader("myHeaderName", "myHeaderValue").execute();
MethodOutcome outcome = client
.delete()
.resourceById("Patient", "123")
.withAdditionalHeader("myHeaderName", "myHeaderValue")
.execute();
oo = (OperationOutcome) outcome.getOperationOutcome();
assertEquals("http://example.com/fhir/Patient/123", capt.getValue().getURI().toString());
assertEquals("DELETE", capt.getValue().getMethod());
Assert.assertEquals("testDelete01", outcome.getIssueFirstRep().getLocation().get(0).getValue());
Assert.assertEquals("testDelete01", oo.getIssueFirstRep().getLocation().get(0).getValue());
assertEquals("myHeaderValue", capt.getValue().getFirstHeader("myHeaderName").getValue());
}
@Test
public void testDeleteInvalidResponse() throws Exception {
OperationOutcome oo = new OperationOutcome();
oo.addIssue().addLocation("testDelete01");
String ooStr = ourCtx.newXmlParser().encodeResourceToString(oo);
ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse);
when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 201, "OK"));
when(myHttpResponse.getAllHeaders()).thenReturn(new Header[]{new BasicHeader(Constants.HEADER_LOCATION, "/Patient/44/_history/22")});
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8"));
when(myHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader("LKJHLKJGLKJKLL"), StandardCharsets.UTF_8));
outcome = (OperationOutcome) client.delete().resourceById(new IdType("Location", "123", "456")).prettyPrint().encodedJson().execute();
assertEquals("http://example.com/fhir/Location/123?_pretty=true", capt.getAllValues().get(1).getURI().toString());
assertEquals("DELETE", capt.getValue().getMethod());
IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
Assert.assertEquals(null, outcome);
// Try with invalid response
try {
client
.delete()
.resourceById(new IdType("Location", "123", "456"))
.prettyPrint()
.encodedJson()
.execute();
} catch (FhirClientConnectionException e) {
assertEquals(0, e.getStatusCode());
assertThat(e.getMessage(), containsString("Failed to parse response from server when performing DELETE to URL"));
}
}
@Test
public void testDeleteNoResponse() throws Exception {
OperationOutcome oo = new OperationOutcome();
oo.addIssue().addLocation("testDelete01");
String ooStr = ourCtx.newXmlParser().encodeResourceToString(oo);
ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse);
when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 201, "OK"));
when(myHttpResponse.getAllHeaders()).thenReturn(new Header[]{new BasicHeader(Constants.HEADER_LOCATION, "/Patient/44/_history/22")});
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8"));
when(myHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(ooStr), StandardCharsets.UTF_8));
IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
MethodOutcome outcome = client
.delete()
.resourceById("Patient", "123")
.withAdditionalHeader("myHeaderName", "myHeaderValue")
.execute();
oo = (OperationOutcome) outcome.getOperationOutcome();
assertEquals("http://example.com/fhir/Patient/123", capt.getValue().getURI().toString());
assertEquals("DELETE", capt.getValue().getMethod());
Assert.assertEquals("testDelete01", oo.getIssueFirstRep().getLocation().get(0).getValue());
assertEquals("myHeaderValue", capt.getValue().getFirstHeader("myHeaderName").getValue());
}
@Test
public void testHistory() throws Exception {
@ -413,12 +472,8 @@ public class GenericClientTest {
when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse);
when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK"));
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8"));
when(myHttpResponse.getEntity().getContent()).thenAnswer(new Answer<InputStream>() {
@Override
public InputStream answer(InvocationOnMock theInvocation) throws Throwable {
return new ReaderInputStream(new StringReader(msg), StandardCharsets.UTF_8);
}
});
when(myHttpResponse.getEntity().getContent()).thenAnswer(t ->
new ReaderInputStream(new StringReader(msg), StandardCharsets.UTF_8));
IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
@ -428,7 +483,7 @@ public class GenericClientTest {
response = client
.history()
.onServer()
.andReturnBundle(Bundle.class)
.returnBundle(Bundle.class)
.withAdditionalHeader("myHeaderName", "myHeaderValue")
.execute();
assertEquals("http://example.com/fhir/_history", capt.getAllValues().get(idx).getURI().toString());
@ -439,7 +494,7 @@ public class GenericClientTest {
response = client
.history()
.onType(Patient.class)
.andReturnBundle(Bundle.class)
.returnBundle(Bundle.class)
.withAdditionalHeader("myHeaderName", "myHeaderValue1")
.withAdditionalHeader("myHeaderName", "myHeaderValue2")
.execute();