Skip to content

Commit d02d1bb

Browse files
committed
Support for returning response bodies on deletion of item resources.
RepositoryRestConfiguration now allows to configure to return a response body for the deletion of item resources. The controller implementation follows the same patter we have already established for creation and updates: unless explicitly enabled or disabled we now consider the presence of an accept header as indicator of whether a response body should be rendered. This could be a "breaking" change for clients having explicitly expected 204 until now even for requests with an Accept header. If that's an issue, those should either explicitly disable the setting, do not submit an Accept header or loosen their expectations to expect either 200 or 2xx as indicator of success in general. Fixes #2225.
1 parent 7c0fc83 commit d02d1bb

File tree

4 files changed

+76
-8
lines changed

4 files changed

+76
-8
lines changed

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/RepositoryRestConfiguration.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public class RepositoryRestConfiguration {
6161
private boolean useHalAsDefaultJsonMediaType = true;
6262
private Boolean returnBodyOnCreate = null;
6363
private Boolean returnBodyOnUpdate = null;
64+
private Boolean returnBodyOnDelete = null;
6465
private List<Class<?>> exposeIdsFor = new ArrayList<Class<?>>();
6566
private ResourceMappingConfiguration domainMappings = new ResourceMappingConfiguration();
6667
private ResourceMappingConfiguration repoMappings = new ResourceMappingConfiguration();
@@ -375,6 +376,30 @@ public RepositoryRestConfiguration setReturnBodyOnUpdate(Boolean returnBodyOnUpd
375376
return this;
376377
}
377378

379+
/**
380+
* Whether to return a response body after deleting an entity considering the given accept header.
381+
*
382+
* @param acceptHeader can be {@literal null} or empty.
383+
* @return
384+
* @since 4.1
385+
*/
386+
public boolean returnBodyOnDelete(String acceptHeader) {
387+
return returnBodyOnDelete == null ? StringUtils.hasText(acceptHeader) : returnBodyOnDelete;
388+
}
389+
390+
/**
391+
* Set whether to return a response body after deleting an entity.
392+
*
393+
* @param returnBodyOnUpdate can be {@literal null}, expressing the decision shall be derived from the presence of an
394+
* {@code Accept} header in the request.
395+
* @return {@literal this}
396+
* @since 4.1
397+
*/
398+
public RepositoryRestConfiguration setReturnBodyOnDelete(Boolean returnBodyOnDelete) {
399+
this.returnBodyOnDelete = returnBodyOnDelete;
400+
return this;
401+
}
402+
378403
/**
379404
* Start configuration a {@link ResourceMapping} for a specific domain type.
380405
*

spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/RepositoryEntityControllerIntegrationTests.java

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@
2222

2323
import java.util.List;
2424
import java.util.Optional;
25+
import java.util.stream.Stream;
2526

27+
import org.junit.jupiter.api.DynamicTest;
2628
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.TestFactory;
2730
import org.mockito.Mockito;
2831
import org.springframework.beans.factory.annotation.Autowired;
2932
import org.springframework.context.annotation.Bean;
@@ -266,7 +269,7 @@ void deletesEntityWithCustomLookupCorrectly() throws Exception {
266269
RootResourceInformation informationSpy = Mockito.spy(resourceInformation);
267270
doReturn(invoker).when(informationSpy).getInvoker();
268271

269-
controller.deleteItemResource(informationSpy, "foo", ETag.from("0"));
272+
controller.deleteItemResource(informationSpy, "foo", ETag.from("0"), assembler, MediaType.ALL_VALUE);
270273

271274
assertThat(repository.findById(address.id)).isEmpty();
272275
}
@@ -283,5 +286,44 @@ void rejectsPutForCreationIfConfigured() throws HttpRequestMethodNotSupportedExc
283286
MediaType.APPLICATION_JSON_VALUE));
284287
}
285288

289+
@TestFactory // #2225
290+
Stream<DynamicTest> returnsResponseBodyForDeleteForAcceptHeaderOrConfig() throws Exception {
291+
292+
record Fixture(String description, Boolean activateReturnBodyOnDelete, String acceptHeader,
293+
HttpStatus expectedStatusCode) {
294+
public String description() {
295+
return description.formatted(expectedStatusCode);
296+
}
297+
}
298+
299+
var fixtures = Stream.of(
300+
new Fixture("No config, no header -> %s", null, null, HttpStatus.NO_CONTENT),
301+
new Fixture("No config, but header -> %s", null, MediaType.ALL_VALUE, HttpStatus.OK),
302+
new Fixture("Enabled, no header -> %s", true, null, HttpStatus.OK),
303+
new Fixture("Enabled, and header -> %s", true, MediaType.ALL_VALUE, HttpStatus.OK),
304+
new Fixture("Disabled, no header -> %s", false, null, HttpStatus.NO_CONTENT),
305+
new Fixture("Disabled, and header -> %s", false, MediaType.ALL_VALUE, HttpStatus.NO_CONTENT));
306+
307+
return DynamicTest.stream(fixtures, Fixture::description, it -> {
308+
309+
try {
310+
311+
// Apply configuration
312+
configuration.setReturnBodyOnDelete(it.activateReturnBodyOnDelete());
313+
314+
var address = repository.save(new Address());
315+
var response = controller.deleteItemResource(getResourceInformation(Address.class), address.id,
316+
ETag.NO_ETAG, assembler, it.acceptHeader());
317+
318+
assertThat(response.getStatusCode()).isEqualTo(it.expectedStatusCode());
319+
320+
} finally {
321+
322+
// Reset configuration
323+
configuration.setReturnBodyOnDelete(null);
324+
}
325+
});
326+
}
327+
286328
interface AddressProjection {}
287329
}

spring-data-rest-tests/spring-data-rest-tests-security/src/test/java/org/springframework/data/rest/tests/security/SecurityIntegrationTests.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@
2929
import org.springframework.data.rest.tests.AbstractWebIntegrationTests;
3030
import org.springframework.data.rest.tests.TestMvcClient;
3131
import org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration;
32-
import org.springframework.http.HttpStatus;
3332
import org.springframework.mock.web.MockHttpServletResponse;
3433
import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
3534
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
3635
import org.springframework.security.core.authority.AuthorityUtils;
3736
import org.springframework.security.core.context.SecurityContextHolder;
3837
import org.springframework.test.context.ContextConfiguration;
3938
import org.springframework.test.context.junit.jupiter.SpringExtension;
40-
import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
4139
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
4240
import org.springframework.web.context.WebApplicationContext;
4341

@@ -140,7 +138,7 @@ void deletePersonAccessGrantedForAdmins() throws Exception {
140138
SecurityContextHolder.clearContext();
141139

142140
mvc.perform(delete(href).with(user("user").roles("USER", "ADMIN")))
143-
.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
141+
.andExpect(status().is2xxSuccessful());
144142
}
145143

146144
@Test // DATAREST-327
@@ -205,7 +203,7 @@ void deleteOrderAccessGrantedForAdmins() throws Exception {
205203
SecurityContextHolder.clearContext();
206204

207205
mvc.perform(delete(href).with(user("user").roles("USER", "ADMIN")))
208-
.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
206+
.andExpect(status().is2xxSuccessful());
209207
}
210208

211209
@Test // DATAREST-327
@@ -240,7 +238,6 @@ void rejectsAccessToItemResourceIfNotAuthorized() throws Exception {
240238
String href = assertHasJsonPathValue("$._embedded.orders[0]._links.self.href", response);
241239

242240
mvc.perform(get(href).with(user("user").roles("USER")))
243-
.andDo(MockMvcResultHandlers.print())
244241
.andExpect(status().isForbidden());
245242
}
246243
}

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryEntityController.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,9 @@ public ResponseEntity<RepresentationModel<?>> patchItemResource(RootResourceInfo
402402
*/
403403
@RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.DELETE)
404404
public ResponseEntity<?> deleteItemResource(RootResourceInformation resourceInformation, @BackendId Serializable id,
405-
ETag eTag) throws ResourceNotFoundException, HttpRequestMethodNotSupportedException {
405+
ETag eTag, PersistentEntityResourceAssembler assembler,
406+
@RequestHeader(value = ACCEPT_HEADER, required = false) String acceptHeader)
407+
throws ResourceNotFoundException, HttpRequestMethodNotSupportedException {
406408

407409
resourceInformation.verifySupportedMethod(HttpMethod.DELETE, ResourceType.ITEM);
408410

@@ -419,7 +421,9 @@ public ResponseEntity<?> deleteItemResource(RootResourceInformation resourceInfo
419421
invoker.invokeDeleteById(entity.getIdentifierAccessor(it).getIdentifier());
420422
publisher.publishEvent(new AfterDeleteEvent(it));
421423

422-
return new ResponseEntity<Object>(HttpStatus.NO_CONTENT);
424+
return config.returnBodyOnDelete(acceptHeader)
425+
? ResponseEntity.ok(assembler.toFullResource(it))
426+
: new ResponseEntity<Object>(HttpStatus.NO_CONTENT);
423427

424428
}).orElseThrow(() -> new ResourceNotFoundException());
425429
}

0 commit comments

Comments
 (0)