From b3b091e309a0807f37d19729a9fa5d3962eaed21 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 14 Feb 2014 13:20:03 +0100 Subject: [PATCH] DATAREST-95 - General overhaul of HTTP method handling. Code polishing in DomainObjectMerger and related test cases. Fixed the related test cases. Cleanups in ControllerUtils to remove unneeded constants and make sure we really render no content for empty responses. Refactorings in controller classes to reduce code duplication. We now do not allow POST requests for partial updates to property reference resources anymore but require the usage of PATCH. Tweaked test helper methods to correctly implement basic interaction patterns. Related pull request: #127. --- .../rest/core/support/DomainObjectMerger.java | 30 +-- .../data/rest/core/util/Function.java | 2 +- .../core/support/DomainObjectMergerTests.java | 5 +- .../data/rest/webmvc/ControllerUtils.java | 12 +- .../rest/webmvc/PersistentEntityResource.java | 5 +- .../webmvc/RepositoryEntityController.java | 197 +++++++++++------- ...RepositoryPropertyReferenceController.java | 82 +++----- .../webmvc/AbstractWebIntegrationTests.java | 25 +-- ...itoryEntityControllerIntegrationTests.java | 2 +- ...itorySearchControllerIntegrationTests.java | 2 +- .../data/rest/webmvc/jpa/JpaWebTests.java | 45 ++-- 11 files changed, 204 insertions(+), 203 deletions(-) diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/DomainObjectMerger.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/DomainObjectMerger.java index 28f7631be..9d635316e 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/DomainObjectMerger.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/DomainObjectMerger.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package org.springframework.data.rest.core.support; +import static org.springframework.data.rest.core.support.DomainObjectMerger.NullHandlingPolicy.*; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.Association; @@ -60,10 +62,11 @@ public class DomainObjectMerger { * * @param from can be {@literal null}. * @param target can be {@literal null}. + * @param nullPolicy how to handle {@literal null} values in the source object. */ - public void merge(Object from, Object target, final MergeNullPolicy nullPolicy) { + public void merge(Object from, Object target, final NullHandlingPolicy nullPolicy) { - if (null == from || null == target) { + if (from == null || target == null) { return; } @@ -87,10 +90,12 @@ public class DomainObjectMerger { return; } - if (!ObjectUtils.nullSafeEquals(sourceValue, targetValue)) { - if (nullPolicy == MergeNullPolicy.APPLY_NULLS || sourceValue != null) { - targetWrapper.setProperty(persistentProperty, sourceValue); - } + if (ObjectUtils.nullSafeEquals(sourceValue, targetValue)) { + return; + } + + if (nullPolicy == APPLY_NULLS || sourceValue != null) { + targetWrapper.setProperty(persistentProperty, sourceValue); } } }); @@ -106,7 +111,8 @@ public class DomainObjectMerger { PersistentProperty persistentProperty = association.getInverse(); Object fromVal = fromWrapper.getProperty(persistentProperty); - if (null != fromVal && !fromVal.equals(targetWrapper.getProperty(persistentProperty))) { + + if (fromVal != null && !fromVal.equals(targetWrapper.getProperty(persistentProperty))) { targetWrapper.setProperty(persistentProperty, fromVal); } } @@ -114,13 +120,9 @@ public class DomainObjectMerger { } /** - * A switch on whether or not to ignore nulls. - * NOTE: This could have been a simple boolean flag but the enumerated value clearly - * denotes which version is being used. + * Strategy to express whether {@literal null} values should be ignored or set on the target domain object. */ - public static enum MergeNullPolicy { + public static enum NullHandlingPolicy { APPLY_NULLS, IGNORE_NULLS; } - - } diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/util/Function.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/util/Function.java index c099b0ae2..150d63f12 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/util/Function.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/util/Function.java @@ -22,5 +22,5 @@ package org.springframework.data.rest.core.util; */ public interface Function { - T apply(S input); + T apply(S input) throws Exception; } diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/support/DomainObjectMergerTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/support/DomainObjectMergerTests.java index 2bdfddd49..9307c6335 100644 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/support/DomainObjectMergerTests.java +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/support/DomainObjectMergerTests.java @@ -17,6 +17,7 @@ package org.springframework.data.rest.core.support; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.springframework.data.rest.core.support.DomainObjectMerger.NullHandlingPolicy.*; import org.junit.Test; import org.junit.runner.RunWith; @@ -57,7 +58,7 @@ public class DomainObjectMergerTests { Person existingDomainObject = new Person("Frodo", "Baggins"); DomainObjectMerger merger = new DomainObjectMerger(repositories, conversionService); - merger.merge(incoming, existingDomainObject, DomainObjectMerger.MergeNullPolicy.APPLY_NULLS); + merger.merge(incoming, existingDomainObject, APPLY_NULLS); assertThat(existingDomainObject.getFirstName(), equalTo(incoming.getFirstName())); assertThat(existingDomainObject.getLastName(), equalTo(incoming.getLastName())); @@ -76,7 +77,7 @@ public class DomainObjectMergerTests { Person existingDomainObject = new Person("Frodo", "Baggins"); DomainObjectMerger merger = new DomainObjectMerger(repositories, conversionService); - merger.merge(incoming, existingDomainObject, DomainObjectMerger.MergeNullPolicy.APPLY_NULLS); + merger.merge(incoming, existingDomainObject, APPLY_NULLS); assertThat(existingDomainObject.getFirstName(), equalTo(incoming.getFirstName())); assertThat(existingDomainObject.getLastName(), equalTo(incoming.getLastName())); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ControllerUtils.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ControllerUtils.java index 7af93648a..c93b928c3 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ControllerUtils.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ControllerUtils.java @@ -17,10 +17,8 @@ package org.springframework.data.rest.webmvc; import java.util.Collections; -import org.springframework.core.convert.TypeDescriptor; import org.springframework.hateoas.Resource; import org.springframework.hateoas.ResourceSupport; -import org.springframework.hateoas.Resources; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -31,10 +29,7 @@ import org.springframework.http.ResponseEntity; */ public class ControllerUtils { - public static final Resources> EMPTY_RESOURCES = new Resources>( - Collections.> emptyList()); public static final Iterable> EMPTY_RESOURCE_LIST = Collections.emptyList(); - public static final TypeDescriptor STRING_TYPE = TypeDescriptor.valueOf(String.class); /** * Wrap a resource as a {@link ResourceEntity} and attach given headers and status. @@ -49,7 +44,8 @@ public class ControllerUtils { HttpHeaders headers, R resource) { HttpHeaders hdrs = new HttpHeaders(); - if (null != headers) { + + if (headers != null) { hdrs.putAll(headers); } @@ -63,7 +59,7 @@ public class ControllerUtils { * @return */ public static ResponseEntity toEmptyResponse(HttpStatus status) { - return toResponseEntity(status, null, EMPTY_RESOURCES); + return toEmptyResponse(status, null); } /** @@ -74,6 +70,6 @@ public class ControllerUtils { * @return */ public static ResponseEntity toEmptyResponse(HttpStatus status, HttpHeaders headers) { - return toResponseEntity(status, headers, EMPTY_RESOURCES); + return toResponseEntity(status, headers, null); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/PersistentEntityResource.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/PersistentEntityResource.java index 145dcb78e..b5b8eff01 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/PersistentEntityResource.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/PersistentEntityResource.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.data.rest.webmvc; import java.util.Arrays; import org.springframework.data.mapping.PersistentEntity; +import org.springframework.data.mapping.PersistentProperty; import org.springframework.hateoas.Link; import org.springframework.hateoas.Resource; @@ -47,7 +48,7 @@ public class PersistentEntityResource extends Resource { } @JsonIgnore - public PersistentEntity getPersistentEntity() { + public PersistentEntity> getPersistentEntity() { return entity; } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryEntityController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryEntityController.java index 45e1e300a..6a2e96972 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryEntityController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryEntityController.java @@ -15,28 +15,22 @@ */ package org.springframework.data.rest.webmvc; -import static org.springframework.data.rest.webmvc.ControllerUtils.*; +import static org.springframework.data.rest.core.support.DomainObjectMerger.NullHandlingPolicy.*; +import static org.springframework.http.HttpMethod.*; -import java.io.IOException; import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.TypeDescriptor; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; -import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.BeanWrapper; -import org.springframework.data.repository.support.DomainClassConverter; import org.springframework.data.repository.support.Repositories; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; import org.springframework.data.rest.core.event.AfterCreateEvent; @@ -49,6 +43,7 @@ import org.springframework.data.rest.core.invoke.RepositoryInvoker; import org.springframework.data.rest.core.mapping.ResourceMetadata; import org.springframework.data.rest.core.mapping.SearchResourceMappings; import org.springframework.data.rest.core.support.DomainObjectMerger; +import org.springframework.data.rest.core.support.DomainObjectMerger.NullHandlingPolicy; import org.springframework.data.web.PagedResourcesAssembler; import org.springframework.hateoas.EntityLinks; import org.springframework.hateoas.Link; @@ -61,7 +56,10 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.HttpRequestMethodNotSupportedException; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.ResponseBody; /** * @author Jon Brisbin @@ -76,7 +74,6 @@ class RepositoryEntityController extends AbstractRepositoryRestController implem private final EntityLinks entityLinks; private final PersistentEntityResourceAssembler perAssembler; private final RepositoryRestConfiguration config; - private final DomainClassConverter converter; private final ConversionService conversionService; private final DomainObjectMerger domainObjectMerger; @@ -85,7 +82,7 @@ class RepositoryEntityController extends AbstractRepositoryRestController implem @Autowired public RepositoryEntityController(Repositories repositories, RepositoryRestConfiguration config, EntityLinks entityLinks, PagedResourcesAssembler assembler, - PersistentEntityResourceAssembler perAssembler, DomainClassConverter converter, + PersistentEntityResourceAssembler perAssembler, @Qualifier("defaultConversionService") ConversionService conversionService, DomainObjectMerger domainObjectMerger) { super(assembler, perAssembler); @@ -93,7 +90,6 @@ class RepositoryEntityController extends AbstractRepositoryRestController implem this.entityLinks = entityLinks; this.perAssembler = perAssembler; this.config = config; - this.converter = converter; this.conversionService = conversionService; this.domainObjectMerger = domainObjectMerger; } @@ -163,29 +159,26 @@ class RepositoryEntityController extends AbstractRepositoryRestController implem } } + /** + * POST /{repository} - Creates a new entity instances from the collection resource. + * + * @param resourceInformation + * @param payload + * @return + * @throws HttpRequestMethodNotSupportedException + */ @ResponseBody - @RequestMapping(value = BASE_MAPPING, method = RequestMethod.POST, consumes = { "application/json" }) - public ResponseEntity createNewEntity(RootResourceInformation resourceInformation, - PersistentEntityResource incoming) throws HttpRequestMethodNotSupportedException { + @RequestMapping(value = BASE_MAPPING, method = RequestMethod.POST) + public ResponseEntity postEntity(RootResourceInformation resourceInformation, + PersistentEntityResource payload) throws HttpRequestMethodNotSupportedException { resourceInformation.verifySupportedMethod(HttpMethod.POST, ResourceType.COLLECTION); - RepositoryInvoker invoker = resourceInformation.getInvoker(); - - publisher.publishEvent(new BeforeCreateEvent(incoming.getContent())); - Object obj = invoker.invokeSave(incoming.getContent()); - publisher.publishEvent(new AfterCreateEvent(obj)); - - Link selfLink = perAssembler.getSelfLinkFor(obj); - HttpHeaders headers = new HttpHeaders(); - headers.setLocation(URI.create(selfLink.getHref())); - - PersistentEntityResource resource = config.isReturnBodyOnCreate() ? perAssembler.toResource(obj) : null; - return ControllerUtils.toResponseEntity(HttpStatus.CREATED, headers, resource); + return createAndReturn(payload.getContent(), resourceInformation.getInvoker()); } /** - * {@code GET /$repository/$id} + * GET /{repository}/{id} - Returns a single entity. * * @param resourceInformation * @param id @@ -214,78 +207,69 @@ class RepositoryEntityController extends AbstractRepositoryRestController implem } /** - * {@code PUT /$repository/$id} - Updates an existing entity or creates one at exactly that place. + * PUT /{repository}/{id} - Updates an existing entity or creates one at exactly that place. * * @param resourceInformation - * @param incoming + * @param payload * @param id * @return * @throws HttpRequestMethodNotSupportedException */ - @RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.PUT, consumes = { "application/json" }) - public ResponseEntity updateEntity(RootResourceInformation resourceInformation, - PersistentEntityResource incoming, @PathVariable String id) throws HttpRequestMethodNotSupportedException { + @RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.PUT) + public ResponseEntity putEntity(RootResourceInformation resourceInformation, + PersistentEntityResource payload, @PathVariable String id) throws HttpRequestMethodNotSupportedException { resourceInformation.verifySupportedMethod(HttpMethod.PUT, ResourceType.ITEM); + Object domainObject = conversionService.convert(id, resourceInformation.getDomainType()); RepositoryInvoker invoker = resourceInformation.getInvoker(); - Object domainObj = converter.convert(id, STRING_TYPE, - TypeDescriptor.valueOf(resourceInformation.getPersistentEntity().getType())); - if (null == domainObj) { - BeanWrapper incomingWrapper = BeanWrapper.create(incoming.getContent(), conversionService); - PersistentProperty idProp = incoming.getPersistentEntity().getIdProperty(); - incomingWrapper.setProperty(idProp, conversionService.convert(id, idProp.getType())); - return createNewEntity(resourceInformation, incoming); + if (domainObject == null) { + + BeanWrapper incomingWrapper = BeanWrapper.create(payload.getContent(), conversionService); + incomingWrapper.setProperty(payload.getPersistentEntity().getIdProperty(), id); + + return createAndReturn(incomingWrapper.getBean(), invoker); } - domainObjectMerger.merge(incoming.getContent(), domainObj, DomainObjectMerger.MergeNullPolicy.APPLY_NULLS); - - publisher.publishEvent(new BeforeSaveEvent(incoming.getContent())); - Object obj = invoker.invokeSave(domainObj); - publisher.publishEvent(new AfterSaveEvent(obj)); - - Link selfLink = perAssembler.getSelfLinkFor(obj); - HttpHeaders headers = new HttpHeaders(); - headers.setLocation(URI.create(selfLink.getHref())); - - if (config.isReturnBodyOnUpdate()) { - return ControllerUtils.toResponseEntity(HttpStatus.OK, headers, perAssembler.toResource(obj)); - } else { - return ControllerUtils.toResponseEntity(HttpStatus.NO_CONTENT, headers, null); - } + return mergeAndReturn(payload.getContent(), domainObject, invoker, PUT); } - @RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.PATCH, consumes = { "application/json" }) - public ResponseEntity patchEntity( - RepositoryRestRequest request, PersistentEntityResource incoming, - @PathVariable String id) { + /** + * PUT /{repository}/{id} - Updates an existing entity or creates one at exactly that place. + * + * @param resourceInformation + * @param payload + * @param id + * @return + * @throws HttpRequestMethodNotSupportedException + * @throws ResourceNotFoundException + */ + @RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.PATCH) + public ResponseEntity patchEntity(RootResourceInformation resourceInformation, + PersistentEntityResource payload, @PathVariable String id) throws HttpRequestMethodNotSupportedException, + ResourceNotFoundException { - RepositoryInvoker invoker = request.getRepositoryInvoker(); - if (null == invoker || !invoker.exposesSave() || !invoker.exposesFindOne()) { - return new ResponseEntity>(HttpStatus.METHOD_NOT_ALLOWED); - } - - Object domainObj = converter.convert(id, STRING_TYPE, - TypeDescriptor.valueOf(request.getPersistentEntity().getType())); - if (null == domainObj) { - return new ResponseEntity>(HttpStatus.NOT_FOUND); - } - - domainObjectMerger.merge(incoming.getContent(), domainObj, DomainObjectMerger.MergeNullPolicy.IGNORE_NULLS); - - publisher.publishEvent(new BeforeSaveEvent(domainObj)); - Object obj = invoker.invokeSave(domainObj); - publisher.publishEvent(new AfterSaveEvent(domainObj)); - - if (config.isReturnBodyOnUpdate()) { - return ControllerUtils.toResponseEntity(HttpStatus.OK, null, perAssembler.toResource(obj)); - } else { - return ControllerUtils.toEmptyResponse(HttpStatus.NO_CONTENT); + resourceInformation.verifySupportedMethod(HttpMethod.PATCH, ResourceType.ITEM); + + Object domainObject = conversionService.convert(id, resourceInformation.getDomainType()); + + if (domainObject == null) { + throw new ResourceNotFoundException(); } + return mergeAndReturn(payload.getContent(), domainObject, resourceInformation.getInvoker(), PATCH); } + /** + * DELETE /{repository}/{id} - Deletes the entity backing the item resource. + * + * @param resourceInformation + * @param id + * @return + * @throws ResourceNotFoundException + * @throws HttpRequestMethodNotSupportedException + */ @RequestMapping(value = BASE_MAPPING + "/{id}", method = RequestMethod.DELETE) public ResponseEntity deleteEntity(final RootResourceInformation resourceInformation, @PathVariable final String id) throws ResourceNotFoundException, HttpRequestMethodNotSupportedException { @@ -309,4 +293,57 @@ class RepositoryEntityController extends AbstractRepositoryRestController implem return new ResponseEntity(HttpStatus.NO_CONTENT); } + + /** + * Merges the given incoming object into the given domain object. + * + * @param incoming + * @param domainObject + * @param invoker + * @param httpMethod + * @return + */ + private ResponseEntity mergeAndReturn(Object incoming, Object domainObject, + RepositoryInvoker invoker, HttpMethod httpMethod) { + + NullHandlingPolicy nullPolicy = httpMethod.equals(PATCH) ? IGNORE_NULLS : APPLY_NULLS; + domainObjectMerger.merge(incoming, domainObject, nullPolicy); + + publisher.publishEvent(new BeforeSaveEvent(domainObject)); + Object obj = invoker.invokeSave(domainObject); + publisher.publishEvent(new AfterSaveEvent(domainObject)); + + HttpHeaders headers = new HttpHeaders(); + + if (PUT.equals(httpMethod)) { + headers.setLocation(URI.create(perAssembler.getSelfLinkFor(obj).getHref())); + } + + if (config.isReturnBodyOnUpdate()) { + return ControllerUtils.toResponseEntity(HttpStatus.OK, headers, perAssembler.toResource(obj)); + } else { + return ControllerUtils.toEmptyResponse(HttpStatus.NO_CONTENT, headers); + } + } + + /** + * Triggers the creation of the domain object and renders it into the response if needed. + * + * @param domainObject + * @param invoker + * @return + */ + private ResponseEntity createAndReturn(Object domainObject, RepositoryInvoker invoker) { + + publisher.publishEvent(new BeforeCreateEvent(domainObject)); + Object savedObject = invoker.invokeSave(domainObject); + publisher.publishEvent(new AfterCreateEvent(savedObject)); + + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(URI.create(perAssembler.getSelfLinkFor(savedObject).getHref())); + + PersistentEntityResource resource = config.isReturnBodyOnCreate() ? perAssembler.toResource(savedObject) + : null; + return ControllerUtils.toResponseEntity(HttpStatus.CREATED, headers, resource); + } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java index dfbb8964b..b6fa8944c 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java @@ -27,14 +27,14 @@ import java.util.Map; import java.util.Map.Entry; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.core.CollectionFactory; -import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.BeanWrapper; -import org.springframework.data.repository.support.DomainClassConverter; import org.springframework.data.repository.support.Repositories; import org.springframework.data.rest.core.event.AfterLinkDeleteEvent; import org.springframework.data.rest.core.event.AfterLinkSaveEvent; @@ -60,7 +60,6 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.ResponseBody; -import org.springframework.web.util.UriComponentsBuilder; /** * @author Jon Brisbin @@ -69,26 +68,27 @@ import org.springframework.web.util.UriComponentsBuilder; */ @RepositoryRestController @SuppressWarnings({ "unchecked" }) -public class RepositoryPropertyReferenceController extends AbstractRepositoryRestController implements +class RepositoryPropertyReferenceController extends AbstractRepositoryRestController implements ApplicationEventPublisherAware { private static final String BASE_MAPPING = "/{repository}/{id}/{property}"; private final Repositories repositories; private final PersistentEntityResourceAssembler perAssembler; - private final DomainClassConverter converter; + private final ConversionService conversionService; private ApplicationEventPublisher publisher; @Autowired - public RepositoryPropertyReferenceController(Repositories repositories, DomainClassConverter domainClassConverter, + public RepositoryPropertyReferenceController(Repositories repositories, + @Qualifier("defaultConversionService") ConversionService conversionService, PagedResourcesAssembler assembler, PersistentEntityResourceAssembler perAssembler) { super(assembler, perAssembler); this.repositories = repositories; this.perAssembler = perAssembler; - this.converter = domainClassConverter; + this.conversionService = conversionService; } /* @@ -102,8 +102,7 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes @RequestMapping(value = BASE_MAPPING, method = RequestMethod.GET) public ResponseEntity followPropertyReference(final RootResourceInformation repoRequest, - @PathVariable String id, @PathVariable String property) throws ResourceNotFoundException, - HttpRequestMethodNotSupportedException { + @PathVariable String id, @PathVariable String property) throws Exception { final HttpHeaders headers = new HttpHeaders(); @@ -151,8 +150,7 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes @RequestMapping(value = BASE_MAPPING, method = RequestMethod.DELETE) public ResponseEntity deletePropertyReference(final RootResourceInformation repoRequest, - @PathVariable String id, @PathVariable String property) throws ResourceNotFoundException, NoSuchMethodException, - HttpRequestMethodNotSupportedException { + @PathVariable String id, @PathVariable String property) throws Exception { final RepositoryInvoker repoMethodInvoker = repoRequest.getInvoker(); @@ -163,16 +161,16 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes Function handler = new Function() { @Override - public Resource apply(ReferencedProperty prop) { + public Resource apply(ReferencedProperty prop) throws HttpRequestMethodNotSupportedException { if (null == prop.propertyValue) { return null; } if (prop.property.isCollectionLike()) { - throw new IllegalArgumentException(new HttpRequestMethodNotSupportedException("DELETE")); + throw new HttpRequestMethodNotSupportedException("DELETE"); } else if (prop.property.isMap()) { - throw new IllegalArgumentException(new HttpRequestMethodNotSupportedException("DELETE")); + throw new HttpRequestMethodNotSupportedException("DELETE"); } else { prop.wrapper.setProperty(prop.property, null); } @@ -185,24 +183,17 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes } }; - try { - doWithReferencedProperty(repoRequest, id, property, handler, HttpMethod.DELETE); - } catch (IllegalArgumentException iae) { - if (iae.getCause() instanceof HttpRequestMethodNotSupportedException) { - throw (HttpRequestMethodNotSupportedException) iae.getCause(); - } - } + doWithReferencedProperty(repoRequest, id, property, handler, HttpMethod.DELETE); return ControllerUtils.toEmptyResponse(HttpStatus.NO_CONTENT); } - @RequestMapping(value = BASE_MAPPING + "/{propertyId}", method = RequestMethod.GET, produces = { "application/json", - "application/x-spring-data-verbose+json", "application/x-spring-data-compact+json", "text/uri-list" }) + @RequestMapping(value = BASE_MAPPING + "/{propertyId}", method = RequestMethod.GET) public ResponseEntity followPropertyReference(final RootResourceInformation repoRequest, - @PathVariable String id, @PathVariable String property, final @PathVariable String propertyId) - throws ResourceNotFoundException, HttpRequestMethodNotSupportedException { + @PathVariable String id, @PathVariable String property, final @PathVariable String propertyId) throws Exception { final HttpHeaders headers = new HttpHeaders(); + Function handler = new Function() { @Override @@ -240,7 +231,7 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes } else { return new Resource(prop.propertyValue); } - throw new IllegalArgumentException(new ResourceNotFoundException()); + throw new ResourceNotFoundException(); } }; @@ -251,8 +242,7 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes @RequestMapping(value = BASE_MAPPING, method = RequestMethod.GET, produces = { "application/x-spring-data-compact+json", "text/uri-list" }) public ResponseEntity followPropertyReferenceCompact(RootResourceInformation repoRequest, - @PathVariable String id, @PathVariable String property) throws ResourceNotFoundException, - HttpRequestMethodNotSupportedException { + @PathVariable String id, @PathVariable String property) throws Exception { ResponseEntity response = followPropertyReference(repoRequest, id, property); @@ -298,20 +288,20 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes } @RequestMapping(value = BASE_MAPPING, // - method = { RequestMethod.POST, RequestMethod.PUT }, // + method = { RequestMethod.PATCH, RequestMethod.PUT }, // consumes = { "application/json", "application/x-spring-data-compact+json", "text/uri-list" }) @ResponseBody public ResponseEntity createPropertyReference( final RootResourceInformation resourceInformation, final HttpMethod requestMethod, - final @RequestBody Resources incoming, @PathVariable String id, @PathVariable String property, - final UriComponentsBuilder builder) throws HttpRequestMethodNotSupportedException { + final @RequestBody Resources incoming, @PathVariable String id, @PathVariable String property) + throws Exception { final RepositoryInvoker invoker = resourceInformation.getInvoker(); Function handler = new Function() { @Override - public ResourceSupport apply(ReferencedProperty prop) { + public ResourceSupport apply(ReferencedProperty prop) throws HttpRequestMethodNotSupportedException { Class propertyType = prop.property.getType(); @@ -319,8 +309,8 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes Collection coll = CollectionFactory.createCollection(propertyType, 0); - // Either load the exist collection to add to it (POST) - if (HttpMethod.POST.equals(requestMethod)) { + // Either load the exist collection to add to it (PATCH) + if (HttpMethod.PATCH.equals(requestMethod)) { coll = (Collection) prop.propertyValue; } @@ -336,8 +326,8 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes Map m = CollectionFactory.createMap(propertyType, 0); - // Either load the exist collection to add to it (POST) - if (HttpMethod.POST.equals(requestMethod)) { + // Either load the exist collection to add to it (PATCH) + if (HttpMethod.PATCH.equals(requestMethod)) { m = (Map) prop.propertyValue; } @@ -351,9 +341,9 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes } else { - if (HttpMethod.POST.equals(requestMethod)) { - throw new IllegalStateException( - "Cannot POST a reference to this singular property since the property type is not a List or a Map."); + if (HttpMethod.PATCH.equals(requestMethod)) { + throw new HttpRequestMethodNotSupportedException(HttpMethod.PATCH.name(), new String[] { "PATCH" }, + "Cannot PATCH a reference to this singular property since the property type is not a List or a Map."); } if (incoming.getLinks().size() != 1) { @@ -375,17 +365,13 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes doWithReferencedProperty(resourceInformation, id, property, handler, requestMethod); - HttpHeaders headers = new HttpHeaders(); - headers.set("Location", builder.build().toUriString()); - - return ControllerUtils.toEmptyResponse(HttpStatus.CREATED, headers); + return ControllerUtils.toEmptyResponse(HttpStatus.NO_CONTENT); } @RequestMapping(value = BASE_MAPPING + "/{propertyId}", method = RequestMethod.DELETE) @ResponseBody public ResponseEntity deletePropertyReferenceId(final RootResourceInformation repoRequest, - @PathVariable String id, @PathVariable String property, final @PathVariable String propertyId) - throws HttpRequestMethodNotSupportedException { + @PathVariable String id, @PathVariable String property, final @PathVariable String propertyId) throws Exception { final RepositoryInvoker invoker = repoRequest.getInvoker(); @@ -442,13 +428,13 @@ public class RepositoryPropertyReferenceController extends AbstractRepositoryRes } private Object loadPropertyValue(Class type, String href) { + String id = href.substring(href.lastIndexOf('/') + 1); - return converter.convert(id, STRING_TYPE, TypeDescriptor.valueOf(type)); + return conversionService.convert(id, type); } private ResourceSupport doWithReferencedProperty(RootResourceInformation repoRequest, String id, String propertyPath, - Function handler, HttpMethod method) - throws HttpRequestMethodNotSupportedException { + Function handler, HttpMethod method) throws Exception { RepositoryInvoker invoker = repoRequest.getInvoker(); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/AbstractWebIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/AbstractWebIntegrationTests.java index 6217a9d2b..60a84e998 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/AbstractWebIntegrationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/AbstractWebIntegrationTests.java @@ -162,16 +162,9 @@ public abstract class AbstractWebIntegrationTests { MockHttpServletResponse response = mvc.perform(put(href).content(payload.toString()).contentType(mediaType)).// andExpect(status().is(both(greaterThanOrEqualTo(200)).and(lessThan(300)))).// - andExpect(header().string("Location", is(notNullValue()))).// andReturn().getResponse(); - String content = response.getContentAsString(); - - if (StringUtils.hasText(content)) { - return response; - } - - return request(response.getHeader("Location")); + return StringUtils.hasText(response.getContentAsString()) ? response : request(link); } protected MockHttpServletResponse patchAndGet(Link link, Object payload, MediaType mediaType) throws Exception { @@ -183,24 +176,20 @@ public abstract class AbstractWebIntegrationTests { andExpect(status().isNoContent()).// andReturn().getResponse(); - return request(href); + return StringUtils.hasText(response.getContentAsString()) ? response : request(href); } - protected MockHttpServletResponse deleteAndGet(Link link, MediaType mediaType) throws Exception { + protected void deleteAndVerify(Link link) throws Exception { String href = link.isTemplated() ? link.expand().getHref() : link.getHref(); - MockHttpServletResponse response = mvc.perform(delete(href).contentType(mediaType)).// + mvc.perform(delete(href)).// andExpect(status().isNoContent()).// andReturn().getResponse(); - String content = response.getContentAsString(); - - if (StringUtils.hasText(content)) { - return response; - } - - return request(response.getHeader("Location")); + // Check that the resource is unavailable after a DELETE + mvc.perform(get(href)).// + andExpect(status().isNotFound()); } protected Link assertHasLinkWithRel(String rel, MockHttpServletResponse response) throws Exception { diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryEntityControllerIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryEntityControllerIntegrationTests.java index 5d3af02b5..32aeb7c73 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryEntityControllerIntegrationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryEntityControllerIntegrationTests.java @@ -56,6 +56,6 @@ public class RepositoryEntityControllerIntegrationTests extends AbstractControll RootResourceInformation request = getResourceInformation(Address.class); - controller.createNewEntity(request, null); + controller.postEntity(request, null); } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java index bfd64e73a..cb6722630 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java @@ -53,7 +53,7 @@ public class RepositorySearchControllerIntegrationTests extends AbstractControll } @Test - public void rendersCorrectSearchLinksForPersons() { + public void rendersCorrectSearchLinksForPersons() throws Exception { RootResourceInformation request = getResourceInformation(Person.class); ResourceSupport resource = controller.listSearches(request); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java index 9c8725e69..4472dfc81 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.Scanner; -import com.jayway.jsonpath.JsonPath; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -35,11 +34,9 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.data.rest.core.mapping.ResourceMappings; import org.springframework.data.rest.webmvc.AbstractWebIntegrationTests; import org.springframework.hateoas.Link; -import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -185,28 +182,20 @@ public class JpaWebTests extends AbstractWebIntegrationTests { @Test public void createThenPatch() throws Exception { - MockHttpServletResponse bilbo = postAndGet(new Link("/people/"),// - "{ \"firstName\" : \"Bilbo\", \"lastName\" : \"Baggins\" }",// - MediaType.APPLICATION_JSON); + MockHttpServletResponse bilbo = postAndGet(new Link("/people"), + "{ \"firstName\" : \"Bilbo\", \"lastName\" : \"Baggins\" }", MediaType.APPLICATION_JSON); Link bilboLink = assertHasLinkWithRel("self", bilbo); - assertThat((String) JsonPath.read(bilbo.getContentAsString(), "$.firstName"), - equalTo("Bilbo")); - assertThat((String) JsonPath.read(bilbo.getContentAsString(), "$.lastName"), - equalTo("Baggins")); + assertThat((String) JsonPath.read(bilbo.getContentAsString(), "$.firstName"), equalTo("Bilbo")); + assertThat((String) JsonPath.read(bilbo.getContentAsString(), "$.lastName"), equalTo("Baggins")); - MockHttpServletResponse frodo = patchAndGet(bilboLink,// - "{ \"firstName\" : \"Frodo\" }",// - MediaType.APPLICATION_JSON); + MockHttpServletResponse frodo = patchAndGet(bilboLink, "{ \"firstName\" : \"Frodo\" }", MediaType.APPLICATION_JSON); - assertThat((String) JsonPath.read(frodo.getContentAsString(), "$.firstName"), - equalTo("Frodo")); - assertThat((String) JsonPath.read(frodo.getContentAsString(), "$.lastName"), - equalTo("Baggins")); + assertThat((String) JsonPath.read(frodo.getContentAsString(), "$.firstName"), equalTo("Frodo")); + assertThat((String) JsonPath.read(frodo.getContentAsString(), "$.lastName"), equalTo("Baggins")); } - @Test public void listsSiblingsWithContentCorrectly() throws Exception { @@ -238,9 +227,9 @@ public class JpaWebTests extends AbstractWebIntegrationTests { Link frodosSiblingLink = links.get(0); - postAndGet(frodosSiblingLink, links.get(1).getHref(), TEXT_URI_LIST); - postAndGet(frodosSiblingLink, links.get(2).getHref(), TEXT_URI_LIST); - postAndGet(frodosSiblingLink, links.get(3).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingLink, links.get(1).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingLink, links.get(2).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingLink, links.get(3).getHref(), TEXT_URI_LIST); assertSiblingNames(frodosSiblingLink, "Bilbo", "Merry", "Pippin"); } @@ -258,7 +247,7 @@ public class JpaWebTests extends AbstractWebIntegrationTests { Link frodosSiblingLink = links.get(0); - postAndGet(frodosSiblingLink, toUriList(links.get(1), links.get(2), links.get(3)), TEXT_URI_LIST); + patchAndGet(frodosSiblingLink, toUriList(links.get(1), links.get(2), links.get(3)), TEXT_URI_LIST); assertSiblingNames(frodosSiblingLink, "Bilbo", "Merry", "Pippin"); } @@ -281,7 +270,7 @@ public class JpaWebTests extends AbstractWebIntegrationTests { putAndGet(frodosSiblingsLink, links.get(3).getHref(), TEXT_URI_LIST); assertSiblingNames(frodosSiblingsLink, "Pippin"); - postAndGet(frodosSiblingsLink, links.get(2).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingsLink, links.get(2).getHref(), TEXT_URI_LIST); assertSiblingNames(frodosSiblingsLink, "Merry", "Pippin"); } @@ -304,7 +293,7 @@ public class JpaWebTests extends AbstractWebIntegrationTests { putAndGet(frodoSiblingLink, toUriList(links.get(3)), TEXT_URI_LIST); assertSiblingNames(frodoSiblingLink, "Pippin"); - postAndGet(frodoSiblingLink, toUriList(links.get(2)), TEXT_URI_LIST); + patchAndGet(frodoSiblingLink, toUriList(links.get(2)), TEXT_URI_LIST); assertSiblingNames(frodoSiblingLink, "Merry", "Pippin"); } @@ -321,12 +310,12 @@ public class JpaWebTests extends AbstractWebIntegrationTests { Link frodosSiblingsLink = links.get(0); - postAndGet(frodosSiblingsLink, links.get(1).getHref(), TEXT_URI_LIST); - postAndGet(frodosSiblingsLink, links.get(2).getHref(), TEXT_URI_LIST); - postAndGet(frodosSiblingsLink, links.get(3).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingsLink, links.get(1).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingsLink, links.get(2).getHref(), TEXT_URI_LIST); + patchAndGet(frodosSiblingsLink, links.get(3).getHref(), TEXT_URI_LIST); String pippinId = new UriTemplate("/people/{id}").match(links.get(3).getHref()).get("id"); - deleteAndGet(new Link(frodosSiblingsLink.getHref() + "/" + pippinId), TEXT_URI_LIST); + deleteAndVerify(new Link(frodosSiblingsLink.getHref() + "/" + pippinId)); assertSiblingNames(frodosSiblingsLink, "Bilbo", "Merry"); }