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.
This commit is contained in:
Oliver Drotbohm
2023-02-21 17:25:28 +01:00
parent 7c0fc837a3
commit d02d1bb0c1
4 changed files with 76 additions and 8 deletions

View File

@@ -22,8 +22,11 @@ import static org.springframework.http.HttpMethod.*;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;
import org.junit.jupiter.api.DynamicTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestFactory;
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
@@ -266,7 +269,7 @@ class RepositoryEntityControllerIntegrationTests extends AbstractControllerInteg
RootResourceInformation informationSpy = Mockito.spy(resourceInformation);
doReturn(invoker).when(informationSpy).getInvoker();
controller.deleteItemResource(informationSpy, "foo", ETag.from("0"));
controller.deleteItemResource(informationSpy, "foo", ETag.from("0"), assembler, MediaType.ALL_VALUE);
assertThat(repository.findById(address.id)).isEmpty();
}
@@ -283,5 +286,44 @@ class RepositoryEntityControllerIntegrationTests extends AbstractControllerInteg
MediaType.APPLICATION_JSON_VALUE));
}
@TestFactory // #2225
Stream<DynamicTest> returnsResponseBodyForDeleteForAcceptHeaderOrConfig() throws Exception {
record Fixture(String description, Boolean activateReturnBodyOnDelete, String acceptHeader,
HttpStatus expectedStatusCode) {
public String description() {
return description.formatted(expectedStatusCode);
}
}
var fixtures = Stream.of(
new Fixture("No config, no header -> %s", null, null, HttpStatus.NO_CONTENT),
new Fixture("No config, but header -> %s", null, MediaType.ALL_VALUE, HttpStatus.OK),
new Fixture("Enabled, no header -> %s", true, null, HttpStatus.OK),
new Fixture("Enabled, and header -> %s", true, MediaType.ALL_VALUE, HttpStatus.OK),
new Fixture("Disabled, no header -> %s", false, null, HttpStatus.NO_CONTENT),
new Fixture("Disabled, and header -> %s", false, MediaType.ALL_VALUE, HttpStatus.NO_CONTENT));
return DynamicTest.stream(fixtures, Fixture::description, it -> {
try {
// Apply configuration
configuration.setReturnBodyOnDelete(it.activateReturnBodyOnDelete());
var address = repository.save(new Address());
var response = controller.deleteItemResource(getResourceInformation(Address.class), address.id,
ETag.NO_ETAG, assembler, it.acceptHeader());
assertThat(response.getStatusCode()).isEqualTo(it.expectedStatusCode());
} finally {
// Reset configuration
configuration.setReturnBodyOnDelete(null);
}
});
}
interface AddressProjection {}
}

View File

@@ -29,7 +29,6 @@ import org.springframework.data.map.repository.config.EnableMapRepositories;
import org.springframework.data.rest.tests.AbstractWebIntegrationTests;
import org.springframework.data.rest.tests.TestMvcClient;
import org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration;
import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
@@ -37,7 +36,6 @@ import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;
@@ -140,7 +138,7 @@ class SecurityIntegrationTests extends AbstractWebIntegrationTests {
SecurityContextHolder.clearContext();
mvc.perform(delete(href).with(user("user").roles("USER", "ADMIN")))
.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
.andExpect(status().is2xxSuccessful());
}
@Test // DATAREST-327
@@ -205,7 +203,7 @@ class SecurityIntegrationTests extends AbstractWebIntegrationTests {
SecurityContextHolder.clearContext();
mvc.perform(delete(href).with(user("user").roles("USER", "ADMIN")))
.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
.andExpect(status().is2xxSuccessful());
}
@Test // DATAREST-327
@@ -240,7 +238,6 @@ class SecurityIntegrationTests extends AbstractWebIntegrationTests {
String href = assertHasJsonPathValue("$._embedded.orders[0]._links.self.href", response);
mvc.perform(get(href).with(user("user").roles("USER")))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isForbidden());
}
}