Skip to content

Commit 5486712

Browse files
odrotbohmodrotbohm
committed
DATAREST-34 - Polishing.
Moved decision logic on whether to return response bodies int RepositoryRestConfig to ease testability of the controller. Added more unit tests and simplified the controller integration tests accordingly. Had to deactivate some Cassandra related tests as they don't seem to handle nulling of properties correctly. Changed configuration to rely on the presence of the Accept header by default. Deprecated parameterless isReturnBodyFor…(…) methods in favor of the ones taking the Accept header to avoid the null checks on the calling side. Polished JavaDoc for the isReturnBodyOn(Create|Update) methods. Polished formatting in RepositoryEntityController. Original pull request: #167.
1 parent f3c74ac commit 5486712

File tree

8 files changed

+242
-115
lines changed

8 files changed

+242
-115
lines changed

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

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@
2828
import org.springframework.util.StringUtils;
2929

3030
/**
31+
* Spring Data REST configuration options.
32+
*
3133
* @author Jon Brisbin
3234
* @author Oliver Gierke
35+
* @author Jeremy Rickard
3336
*/
3437
@SuppressWarnings("deprecation")
3538
public class RepositoryRestConfiguration {
@@ -46,8 +49,8 @@ public class RepositoryRestConfiguration {
4649
private String sortParamName = "sort";
4750
private MediaType defaultMediaType = MediaTypes.HAL_JSON;
4851
private boolean useHalAsDefaultJsonMediaType = true;
49-
private Boolean returnBodyOnCreate = Boolean.FALSE;
50-
private Boolean returnBodyOnUpdate = Boolean.FALSE;
52+
private Boolean returnBodyOnCreate = null;
53+
private Boolean returnBodyOnUpdate = null;
5154
private List<Class<?>> exposeIdsFor = new ArrayList<Class<?>>();
5255
private ResourceMappingConfiguration domainMappings = new ResourceMappingConfiguration();
5356
private ResourceMappingConfiguration repoMappings = new ResourceMappingConfiguration();
@@ -293,43 +296,85 @@ public RepositoryRestConfiguration useHalAsDefaultJsonMediaType(boolean useHalAs
293296
return this;
294297
}
295298

299+
/**
300+
* Convenience method to activate returning response bodies for all {@code PUT} and {@code POST} requests, i.e. both
301+
* creating and updating entities.
302+
*
303+
* @param returnBody can be {@literal null}, expressing the decision shall be derived from the presence of an
304+
* {@code Accept} header in the request.
305+
* @return
306+
*/
307+
public RepositoryRestConfiguration setReturnBodyForPutAndPost(Boolean returnBody) {
308+
309+
setReturnBodyOnCreate(returnBody);
310+
setReturnBodyOnUpdate(returnBody);
311+
312+
return this;
313+
}
314+
296315
/**
297316
* Whether to return a response body after creating an entity.
298317
*
299-
* @return {@link java.lang.Boolean#TRUE} to return a body on create, {@link java.lang.Boolean#FALSE} otherwise.
300-
* If {@literal null}, defer to HTTP Accept header
318+
* @return {@link java.lang.Boolean#TRUE} to enforce returning a body on create, {@link java.lang.Boolean#FALSE}
319+
* otherwise. If {@literal null} and an {@code Accept} header present in the request will cause a body being
320+
* returned. If the {@code Accept} header is not present, no body will be rendered.
321+
* @deprecated use {@link #returnBodyOnCreate(String)}
301322
*/
323+
@Deprecated
302324
public Boolean isReturnBodyOnCreate() {
303325
return returnBodyOnCreate;
304326
}
305327

328+
/**
329+
* Whether to return a response body after creating an entity considering the given accept header.
330+
*
331+
* @param acceptHeader can be {@literal null} or empty.
332+
* @return
333+
*/
334+
public boolean returnBodyOnCreate(String acceptHeader) {
335+
return returnBodyOnCreate == null ? StringUtils.hasText(acceptHeader) : returnBodyOnCreate;
336+
}
337+
306338
/**
307339
* Set whether to return a response body after creating an entity.
308340
*
309-
* @param returnBodyOnCreate {@link java.lang.Boolean#TRUE} to return a body on create, {@link java.lang.Boolean#FALSE} otherwise.
310-
* If {@literal null}, defer to HTTP Accept header
341+
* @param returnBody can be {@literal null}, expressing the decision shall be derived from the presence of an
342+
* {@code Accept} header in the request.
311343
* @return {@literal this}
312344
*/
313-
public RepositoryRestConfiguration setReturnBodyOnCreate(Boolean returnBodyOnCreate) {
314-
this.returnBodyOnCreate = returnBodyOnCreate;
345+
public RepositoryRestConfiguration setReturnBodyOnCreate(Boolean returnBody) {
346+
this.returnBodyOnCreate = returnBody;
315347
return this;
316348
}
317349

318350
/**
319351
* Whether to return a response body after updating an entity.
320352
*
321-
* @return {@link java.lang.Boolean#TRUE} to return a body on update, {@link java.lang.Boolean#FALSE} otherwise.
322-
* If {@literal null}, defer to HTTP Accept header
353+
* @return {@link java.lang.Boolean#TRUE} to enforce returning a body on create, {@link java.lang.Boolean#FALSE}
354+
* otherwise. If {@literal null} and an {@code Accept} header present in the request will cause a body being
355+
* returned. If the {@code Accept} header is not present, no body will be rendered.
356+
* @deprecated use {@link #returnBodyOnUpdate(String)}
323357
*/
358+
@Deprecated
324359
public Boolean isReturnBodyOnUpdate() {
325360
return returnBodyOnUpdate;
326361
}
327362

363+
/**
364+
* Whether to return a response body after updating an entity considering the given accept header.
365+
*
366+
* @param acceptHeader can be {@literal null} or empty.
367+
* @return
368+
*/
369+
public boolean returnBodyOnUpdate(String acceptHeader) {
370+
return returnBodyOnUpdate == null ? StringUtils.hasText(acceptHeader) : returnBodyOnUpdate;
371+
}
372+
328373
/**
329374
* Set whether to return a response body after updating an entity.
330375
*
331-
* @param returnBodyOnUpdate {@link java.lang.Boolean#TRUE} to return a body on update, {@link java.lang.Boolean#FALSE} otherwise.
332-
* If {@literal null}, defer to HTTP Accept header
376+
* @param returnBody can be {@literal null}, expressing the decision shall be derived from the presence of an
377+
* {@code Accept} header in the request.
333378
* @return {@literal this}
334379
*/
335380
public RepositoryRestConfiguration setReturnBodyOnUpdate(Boolean returnBodyOnUpdate) {

‎spring-data-rest-core/src/test/java/org/springframework/data/rest/core/RepositoryRestConfigurationUnitTests.java‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
* Copyright 2015 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.rest.core;
17+
18+
import static org.hamcrest.CoreMatchers.*;
19+
import static org.junit.Assert.*;
20+
21+
import org.junit.Test;
22+
import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
23+
import org.springframework.http.MediaType;
24+
25+
/**
26+
* Unit tests for {@link RepositoryRestConfiguration}.
27+
*
28+
* @author Oliver Gierke
29+
* @soundtrack Adam F - Circles (Colors)
30+
*/
31+
public class RepositoryRestConfigurationUnitTests {
32+
33+
/**
34+
* @see DATAREST-34
35+
*/
36+
@Test
37+
public void returnsBodiesIfAcceptHeaderPresentByDefault() {
38+
39+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
40+
41+
assertThat(configuration.returnBodyOnCreate(MediaType.APPLICATION_JSON_VALUE), is(true));
42+
assertThat(configuration.returnBodyOnUpdate(MediaType.APPLICATION_JSON_VALUE), is(true));
43+
}
44+
45+
/**
46+
* @see DATAREST-34
47+
*/
48+
@Test
49+
public void doesNotReturnBodiesIfNoAcceptHeaderPresentByDefault() {
50+
51+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
52+
53+
assertThat(configuration.returnBodyOnCreate(null), is(false));
54+
assertThat(configuration.returnBodyOnUpdate(null), is(false));
55+
}
56+
57+
/**
58+
* @see DATAREST-34
59+
*/
60+
@Test
61+
public void doesNotReturnBodiesIfEmptyAcceptHeaderPresentByDefault() {
62+
63+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
64+
65+
assertThat(configuration.returnBodyOnCreate(""), is(false));
66+
assertThat(configuration.returnBodyOnUpdate(""), is(false));
67+
}
68+
69+
/**
70+
* @see DATAREST-34
71+
*/
72+
@Test
73+
public void doesNotReturnBodyForUpdateIfExplicitlyDeactivated() {
74+
75+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
76+
configuration.setReturnBodyOnUpdate(false);
77+
78+
assertThat(configuration.returnBodyOnUpdate(null), is(false));
79+
assertThat(configuration.returnBodyOnUpdate(""), is(false));
80+
assertThat(configuration.returnBodyOnUpdate(MediaType.APPLICATION_JSON_VALUE), is(false));
81+
}
82+
83+
/**
84+
* @see DATAREST-34
85+
*/
86+
@Test
87+
public void doesNotReturnBodyForCreateIfExplicitlyDeactivated() {
88+
89+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
90+
configuration.setReturnBodyOnCreate(false);
91+
92+
assertThat(configuration.returnBodyOnCreate(null), is(false));
93+
assertThat(configuration.returnBodyOnCreate(""), is(false));
94+
assertThat(configuration.returnBodyOnCreate(MediaType.APPLICATION_JSON_VALUE), is(false));
95+
}
96+
97+
/**
98+
* @see DATAREST-34
99+
*/
100+
@Test
101+
public void returnsBodyForUpdateIfExplicitlyActivated() {
102+
103+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
104+
configuration.setReturnBodyOnUpdate(true);
105+
106+
assertThat(configuration.returnBodyOnUpdate(null), is(true));
107+
assertThat(configuration.returnBodyOnUpdate(""), is(true));
108+
assertThat(configuration.returnBodyOnUpdate(MediaType.APPLICATION_JSON_VALUE), is(true));
109+
}
110+
111+
/**
112+
* @see DATAREST-34
113+
*/
114+
@Test
115+
public void returnsBodyForCreateIfExplicitlyActivated() {
116+
117+
RepositoryRestConfiguration configuration = new RepositoryRestConfiguration();
118+
configuration.setReturnBodyOnCreate(true);
119+
120+
assertThat(configuration.returnBodyOnCreate(null), is(true));
121+
assertThat(configuration.returnBodyOnCreate(""), is(true));
122+
assertThat(configuration.returnBodyOnCreate(MediaType.APPLICATION_JSON_VALUE), is(true));
123+
}
124+
}

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

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
* @author Jon Brisbin
7373
* @author Oliver Gierke
7474
* @author Greg Turnquist
75+
* @author Jeremy Rickard
7576
*/
7677
@RepositoryRestController
7778
class RepositoryEntityController extends AbstractRepositoryRestController implements ApplicationEventPublisherAware {
@@ -238,15 +239,13 @@ public Resources<?> getCollectionResourceCompact(RootResourceInformation repoReq
238239
@ResponseBody
239240
@RequestMapping(value = BASE_MAPPING, method = RequestMethod.POST)
240241
public ResponseEntity<ResourceSupport> postCollectionResource(RootResourceInformation resourceInformation,
241-
PersistentEntityResource payload, PersistentEntityResourceAssembler assembler,
242-
@RequestHeader(value= ACCEPT_HEADER, required = false) String acceptHeader)
243-
throws HttpRequestMethodNotSupportedException {
242+
PersistentEntityResource payload, PersistentEntityResourceAssembler assembler, @RequestHeader(
243+
value = ACCEPT_HEADER, required = false) String acceptHeader) throws HttpRequestMethodNotSupportedException {
244244

245245
resourceInformation.verifySupportedMethod(HttpMethod.POST, ResourceType.COLLECTION);
246246

247-
boolean acceptHeaderPresent = acceptHeader != null;
248-
249-
return createAndReturn(payload.getContent(), resourceInformation.getInvoker(), assembler, acceptHeaderPresent);
247+
return createAndReturn(payload.getContent(), resourceInformation.getInvoker(), assembler,
248+
config.returnBodyOnCreate(acceptHeader));
250249
}
251250

252251
/**
@@ -319,8 +318,7 @@ public ResponseEntity<Resource<?>> getItemResource(RootResourceInformation resou
319318

320319
/**
321320
* <code>PUT /{repository}/{id}</code> - Updates an existing entity or creates one at exactly that place.
322-
*
323-
321+
*
324322
* @param resourceInformation
325323
* @param payload
326324
* @param id
@@ -333,7 +331,7 @@ public ResponseEntity<Resource<?>> getItemResource(RootResourceInformation resou
333331
@RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.PUT)
334332
public ResponseEntity<? extends ResourceSupport> putItemResource(RootResourceInformation resourceInformation,
335333
PersistentEntityResource payload, @BackendId Serializable id, PersistentEntityResourceAssembler assembler,
336-
ETag eTag, @RequestHeader(value=ACCEPT_HEADER, required = false) String acceptHeader)
334+
ETag eTag, @RequestHeader(value = ACCEPT_HEADER, required = false) String acceptHeader)
337335
throws HttpRequestMethodNotSupportedException {
338336

339337
resourceInformation.verifySupportedMethod(HttpMethod.PUT, ResourceType.ITEM);
@@ -350,10 +348,9 @@ public ResponseEntity<? extends ResourceSupport> putItemResource(RootResourceInf
350348

351349
eTag.verify(resourceInformation.getPersistentEntity(), domainObject);
352350

353-
boolean acceptHeaderPresent = acceptHeader != null;
354-
355-
return domainObject == null ? createAndReturn(objectToSave, invoker, assembler, acceptHeaderPresent)
356-
: saveAndReturn(objectToSave, invoker, PUT, assembler, acceptHeaderPresent);
351+
return domainObject == null ? createAndReturn(objectToSave, invoker, assembler,
352+
config.returnBodyOnCreate(acceptHeader)) : saveAndReturn(objectToSave, invoker, PUT, assembler,
353+
config.returnBodyOnUpdate(acceptHeader));
357354
}
358355

359356
/**
@@ -373,7 +370,7 @@ public ResponseEntity<? extends ResourceSupport> putItemResource(RootResourceInf
373370
@RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.PATCH)
374371
public ResponseEntity<ResourceSupport> patchItemResource(RootResourceInformation resourceInformation,
375372
PersistentEntityResource payload, @BackendId Serializable id, PersistentEntityResourceAssembler assembler,
376-
ETag eTag,@RequestHeader(value=ACCEPT_HEADER, required = false) String acceptHeader )
373+
ETag eTag, @RequestHeader(value = ACCEPT_HEADER, required = false) String acceptHeader)
377374
throws HttpRequestMethodNotSupportedException, ResourceNotFoundException {
378375

379376
resourceInformation.verifySupportedMethod(HttpMethod.PATCH, ResourceType.ITEM);
@@ -386,9 +383,8 @@ public ResponseEntity<ResourceSupport> patchItemResource(RootResourceInformation
386383

387384
eTag.verify(resourceInformation.getPersistentEntity(), domainObject);
388385

389-
boolean acceptHeaderPresent = acceptHeader != null;
390-
391-
return saveAndReturn(payload.getContent(), resourceInformation.getInvoker(), PATCH, assembler, acceptHeaderPresent);
386+
return saveAndReturn(payload.getContent(), resourceInformation.getInvoker(), PATCH, assembler,
387+
config.returnBodyOnUpdate(acceptHeader));
392388
}
393389

394390
/**
@@ -433,7 +429,7 @@ public ResponseEntity<?> deleteItemResource(RootResourceInformation resourceInfo
433429
* @return
434430
*/
435431
private ResponseEntity<ResourceSupport> saveAndReturn(Object domainObject, RepositoryInvoker invoker,
436-
HttpMethod httpMethod, PersistentEntityResourceAssembler assembler, boolean acceptHeaderPresent) {
432+
HttpMethod httpMethod, PersistentEntityResourceAssembler assembler, boolean returnBody) {
437433

438434
publisher.publishEvent(new BeforeSaveEvent(domainObject));
439435
Object obj = invoker.invokeSave(domainObject);
@@ -446,10 +442,7 @@ private ResponseEntity<ResourceSupport> saveAndReturn(Object domainObject, Repos
446442
addLocationHeader(headers, assembler, obj);
447443
}
448444

449-
boolean returnBodyOnUpdate = (config.isReturnBodyOnUpdate() == null && acceptHeaderPresent)
450-
|| Boolean.TRUE.equals(config.isReturnBodyOnUpdate());
451-
452-
if (returnBodyOnUpdate) {
445+
if (returnBody) {
453446
return ControllerUtils.toResponseEntity(HttpStatus.OK, headers, resource);
454447
} else {
455448
return ControllerUtils.toEmptyResponse(HttpStatus.NO_CONTENT, headers);
@@ -464,17 +457,13 @@ private ResponseEntity<ResourceSupport> saveAndReturn(Object domainObject, Repos
464457
* @return
465458
*/
466459
private ResponseEntity<ResourceSupport> createAndReturn(Object domainObject, RepositoryInvoker invoker,
467-
PersistentEntityResourceAssembler assembler, boolean acceptHeaderPresent) {
460+
PersistentEntityResourceAssembler assembler, boolean returnBody) {
468461

469462
publisher.publishEvent(new BeforeCreateEvent(domainObject));
470463
Object savedObject = invoker.invokeSave(domainObject);
471464
publisher.publishEvent(new AfterCreateEvent(savedObject));
472465

473-
474-
boolean returnBodyOnCreate = (config.isReturnBodyOnCreate() == null && acceptHeaderPresent)
475-
|| Boolean.TRUE.equals(config.isReturnBodyOnCreate());
476-
477-
PersistentEntityResource resource = returnBodyOnCreate ? assembler.toFullResource(savedObject) : null;
466+
PersistentEntityResource resource = returnBody ? assembler.toFullResource(savedObject) : null;
478467

479468
HttpHeaders headers = prepareHeaders(resource);
480469
addLocationHeader(headers, assembler, savedObject);

‎spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/AbstractWebIntegrationTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ protected MockHttpServletResponse putAndGet(Link link, Object payload, MediaType
103103
String href = link.isTemplated() ? link.expand().getHref() : link.getHref();
104104

105105
MockHttpServletResponse response = mvc.perform(put(href).content(payload.toString()).contentType(mediaType)).//
106-
andExpect(status().is(both(greaterThanOrEqualTo(200)).and(lessThan(300)))).//
106+
andExpect(status().is2xxSuccessful()).//
107107
andReturn().getResponse();
108108

109109
return StringUtils.hasText(response.getContentAsString()) ? response : client.request(link);
@@ -115,7 +115,7 @@ protected MockHttpServletResponse patchAndGet(Link link, Object payload, MediaTy
115115

116116
MockHttpServletResponse response = mvc.perform(MockMvcRequestBuilders.request(HttpMethod.PATCH, href).//
117117
content(payload.toString()).contentType(mediaType)).//
118-
andExpect(status().isNoContent()).//
118+
andExpect(status().is2xxSuccessful()).//
119119
andReturn().getResponse();
120120

121121
return StringUtils.hasText(response.getContentAsString()) ? response : client.request(href);

0 commit comments

Comments
 (0)