From b9b6a8fe9235b7530e2a339521b2d42726b07167 Mon Sep 17 00:00:00 2001 From: Jon Brisbin Date: Fri, 17 Aug 2012 13:37:26 -0500 Subject: [PATCH] DATAREST-39: Modified the DELETEing of links so that an attempt to delete a required relationship will result in a 405 Method Not Allowed. Had to change the entities and tests to accommodate the new required relationship on the Address entity, which pingponged into a number of changes. --- .../rest/repository/AttributeMetadata.java | 22 ++++++++++++++ .../repository/jpa/JpaAttributeMetadata.java | 9 ++++++ ...itoryAwareMappingHttpMessageConverter.java | 14 ++++----- .../rest/webmvc/RepositoryRestController.java | 25 +++++++++++----- .../data/rest/webmvc/spec/BaseSpec.groovy | 19 +++++++----- .../rest/webmvc/spec/RelationshipsSpec.groovy | 30 +++++++++++++++---- .../webmvc/spec/TopLevelEntitySpec.groovy | 2 +- .../data/rest/test/webmvc/Address.java | 15 ++++++++-- .../data/rest/test/webmvc/Person.java | 5 ++++ .../data/rest/test/webmvc/PersonLoader.java | 20 ++++++++++--- .../src/test/resources/load_data.sh | 4 +-- 11 files changed, 130 insertions(+), 35 deletions(-) diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/AttributeMetadata.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/AttributeMetadata.java index 086ae9dff..2d2823c4e 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/AttributeMetadata.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/AttributeMetadata.java @@ -1,5 +1,6 @@ package org.springframework.data.rest.repository; +import java.lang.annotation.Annotation; import java.util.Collection; import java.util.Map; import java.util.Set; @@ -90,6 +91,27 @@ public interface AttributeMetadata { */ Map asMap(Object target); + /** + * Does this attribute have the given annotation on it? + * + * @param annoType + * The type of annotation to search for. + * + * @return {@literal true} if this annotation exists on this attribute, {@literal false} otherwise. + */ + boolean hasAnnotation(Class annoType); + + /** + * Get the given annotation. + * + * @param annoType + * The type of annotation to get. + * @param + * + * @return The annotation, or {@literal null} if it doesn't exist. + */ + A annotation(Class annoType); + /** * Get the path of this attribute. * diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaAttributeMetadata.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaAttributeMetadata.java index 109bf3861..8b6ebb064 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaAttributeMetadata.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaAttributeMetadata.java @@ -1,6 +1,7 @@ package org.springframework.data.rest.repository.jpa; import java.beans.PropertyDescriptor; +import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.Collection; @@ -126,6 +127,14 @@ public class JpaAttributeMetadata implements AttributeMetadata { return (Map)get(target); } + @Override public boolean hasAnnotation(Class annoType) { + return field.isAnnotationPresent(annoType); + } + + @Override public A annotation(Class annoType) { + return field.getAnnotation(annoType); + } + @Override public Object get(Object target) { try { if(null != getter) { diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryAwareMappingHttpMessageConverter.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryAwareMappingHttpMessageConverter.java index a66b7c1e2..f155a34c9 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryAwareMappingHttpMessageConverter.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryAwareMappingHttpMessageConverter.java @@ -74,13 +74,11 @@ public class RepositoryAwareMappingHttpMessageConverter MediaTypes.COMPACT_JSON, MediaTypes.VERBOSE_JSON )); - - mapper.registerModule(new RepositoryAwareModule()); - setObjectMapper(mapper); } @Override public void afterPropertiesSet() throws Exception { + mapper.registerModule(new RepositoryAwareModule()); for(Module m : modules) { mapper.registerModule(m); } @@ -194,6 +192,12 @@ public class RepositoryAwareMappingHttpMessageConverter } private class RepositoryAwareModule extends SimpleModule { + + SimpleSerializers sers = new SimpleSerializers(); + SimpleDeserializers dsers = new SimpleDeserializers(); + SimpleSerializers keySers = new SimpleSerializers(); + SimpleKeyDeserializers keyDsers = new SimpleKeyDeserializers(); + private RepositoryAwareModule() { super("RepositoryAwareModule", new Version(1, 0, 0, "SNAPSHOT")); } @@ -204,10 +208,6 @@ public class RepositoryAwareMappingHttpMessageConverter new SimpleAbstractTypeResolver() .addMapping(Link.class, ResourceLink.class) ); - SimpleSerializers sers = new SimpleSerializers(); - SimpleDeserializers dsers = new SimpleDeserializers(); - SimpleSerializers keySers = new SimpleSerializers(); - SimpleKeyDeserializers keyDsers = new SimpleKeyDeserializers(); for(RepositoryExporter repoExp : repositoryExporters) { for(String repoName : new ArrayList(repoExp.repositoryNames())) { diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java index ac47bcd49..6514fdc64 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java @@ -20,6 +20,8 @@ import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicReference; +import javax.persistence.ManyToOne; +import javax.persistence.OneToOne; import javax.servlet.http.HttpServletRequest; import org.codehaus.jackson.map.ObjectMapper; @@ -1065,9 +1067,9 @@ public class RepositoryRestController AttributeMetadata idAttr = propRepoMeta.entityMetadata().idAttribute(); String propertyRel = repository + "." + entity.getClass().getSimpleName() + - "." + property + - "." + propRepoMeta.entityMetadata().type().getSimpleName(); + "." + property; if(propVal instanceof Collection) { + propertyRel += "." + propRepoMeta.entityMetadata().type().getSimpleName(); ResourceSet resources = new ResourceSet(); for(Object o : (Collection)propVal) { String propValId = idAttr.get(o).toString(); @@ -1088,6 +1090,7 @@ public class RepositoryRestController } body = resources; } else if(propVal instanceof Map) { + propertyRel += "." + propRepoMeta.entityMetadata().type().getSimpleName(); Map resource = new HashMap(); for(Map.Entry entry : ((Map)propVal).entrySet()) { String propValId = idAttr.get(entry.getValue()).toString(); @@ -1427,6 +1430,7 @@ public class RepositoryRestController @PathVariable String linkedId) throws IOException { RepositoryMetadata repoMeta = repositoryMetadataFor(repository); CrudRepository repo = repoMeta.repository(); + // If I can't load the parent entity, then this method isn't allowed. if(!repoMeta.exportsMethod(CrudMethod.FIND_ONE) || !repoMeta.exportsMethod(CrudMethod.SAVE_ONE)) { return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); } @@ -1440,6 +1444,12 @@ public class RepositoryRestController return notFoundResponse(request); } + // Check if this @*ToOne relationship is optional and if not, fail with a 405 Method Not Allowed + if((attrMeta.hasAnnotation(ManyToOne.class) && !attrMeta.annotation(ManyToOne.class).optional()) + || (attrMeta.hasAnnotation(OneToOne.class) && !attrMeta.annotation(OneToOne.class).optional())) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } + // Find linked entity RepositoryMetadata linkedRepoMeta; if(null == (linkedRepoMeta = repositoryMetadataFor(attrMeta))) { @@ -1460,18 +1470,18 @@ public class RepositoryRestController // Remove linked entity from relationship based on property type if(attrMeta.isCollectionLike()) { Collection c = attrMeta.asCollection(entity); - if(null != c) { + if(null != c && c != Collections.emptyList()) { c.remove(linkedEntity); } } else if(attrMeta.isSetLike()) { Set s = attrMeta.asSet(entity); - if(null != s) { + if(null != s && s != Collections.emptySet()) { s.remove(linkedEntity); } } else if(attrMeta.isMapLike()) { Object keyToRemove = null; Map m = attrMeta.asMap(entity); - if(null != m) { + if(null != m && m != Collections.emptyMap()) { for(Map.Entry entry : m.entrySet()) { Object val = entry.getValue(); if(null != val && val.equals(linkedEntity)) { @@ -1484,7 +1494,7 @@ public class RepositoryRestController } } } else { - attrMeta.set(linkedEntity, entity); + attrMeta.set(null, entity); } publishEvent(new BeforeLinkDeleteEvent(entity, linkedEntity)); @@ -1698,7 +1708,8 @@ public class RepositoryRestController for(String attrName : entityMetadata.linkedAttributes().keySet()) { URI uri = buildUri(baseUri, attrName); - resource.addLink(new ResourceLink(repoRel + "." + entity.getClass().getSimpleName() + "." + attrName, uri)); + String rel = repoRel + "." + entity.getClass().getSimpleName() + "." + attrName; + resource.addLink(new ResourceLink(rel, uri)); } return resource; diff --git a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/BaseSpec.groovy b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/BaseSpec.groovy index b29abd224..78be35f1e 100644 --- a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/BaseSpec.groovy +++ b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/BaseSpec.groovy @@ -39,7 +39,8 @@ abstract class BaseSpec extends Specification { @Autowired AddressRepository addresses @Autowired CustomerRepository customers UriComponentsBuilder baseUri - ObjectMapper mapper = new ObjectMapper() + + def mapper = new ObjectMapper() @Transactional def setup() { @@ -49,12 +50,12 @@ abstract class BaseSpec extends Specification { bindResource(emf, new EntityManagerHolder(emf.createEntityManager())) } - for (Person p : people.findAll()) { - people.delete(p) - } for (Address a : addresses.findAll()) { addresses.delete(a) } + for (Person p : people.findAll()) { + people.delete(p) + } } def readJson(ResponseEntity entity) { @@ -93,15 +94,19 @@ abstract class BaseSpec extends Specification { } def newPerson() { - people.save(new Person(name: "John Doe", addresses: [newAddress("Univille")])) + def p = people.save(new Person(name: "John Doe")) + def a = newAddress("Univille", p) + p.addresses = [a] + people.save(p) } - def newAddress(city) { + def newAddress(city, person) { addresses.save(new Address( ["1234 W. 1st St."] as String[], city, "ST", - "12345" + "12345", + person )) } diff --git a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/RelationshipsSpec.groovy b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/RelationshipsSpec.groovy index c41244038..4a49a03ec 100644 --- a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/RelationshipsSpec.groovy +++ b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/RelationshipsSpec.groovy @@ -2,20 +2,28 @@ package org.springframework.data.rest.webmvc.spec import org.springframework.http.HttpStatus import org.springframework.transaction.annotation.Transactional +import spock.lang.Shared /** * @author Jon Brisbin */ class RelationshipsSpec extends BaseSpec { + @Shared + Long persId + @Shared + Long addrId + + def setup() { + def person = newPerson() + persId = person.id + addrId = person.addresses[0].id + } + @Transactional def "saves entity relationship"() { given: - def person = newPerson() - def persId = person.id - def addr = newAddress("Smallville") - def addrId = addr.id def request = createUriListRequest( "POST", "people/$persId/addresses", @@ -35,7 +43,19 @@ class RelationshipsSpec extends BaseSpec { then: response.statusCode == HttpStatus.OK - readJson(response).city == "Smallville" + readJson(response).city == "Univille" + + } + + @Transactional + def "cannot delete a required relationship"() { + + when: + def request = createRequest("DELETE", "address/$addrId/person/$persId", null) + def response = controller.deleteLink(request, "address", "$addrId", "person", "$persId") + + then: + response.statusCode == HttpStatus.METHOD_NOT_ALLOWED } diff --git a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/TopLevelEntitySpec.groovy b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/TopLevelEntitySpec.groovy index 4f7353490..b99a52aa2 100644 --- a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/TopLevelEntitySpec.groovy +++ b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/TopLevelEntitySpec.groovy @@ -47,7 +47,7 @@ class TopLevelEntitySpec extends BaseSpec { person.name = "Johnnie Doe" person = people.save(person) def persId = person.id - def request = createJsonRequest("PUT", "people/$persId", null, person) + def request = createJsonRequest("PUT", "people/$persId", null, ["name": "Johnnie Doe"]) def retrReq = createRequest("GET", "people/$persId", null) when: diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Address.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Address.java index ac7bfbbb2..3c092b9a6 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Address.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Address.java @@ -1,5 +1,6 @@ package org.springframework.data.rest.test.webmvc; +import javax.persistence.CascadeType; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.Id; @@ -16,17 +17,18 @@ public class Address { private String city; private String province; private String postalCode; - @ManyToOne + @ManyToOne(optional = false, cascade = CascadeType.REMOVE) private Person person; public Address() { } - public Address(String[] lines, String city, String province, String postalCode) { + public Address(String[] lines, String city, String province, String postalCode, Person person) { this.lines = lines; this.city = city; this.province = province; this.postalCode = postalCode; + this.person = person; } public Long getId() { @@ -73,4 +75,13 @@ public class Address { this.person = person; } + @Override public boolean equals(Object o) { + if(!(o instanceof Address)) { + return false; + } + + Address address2 = (Address)o; + return (address2.id == id || (id != null && id.equals(address2.id))); + } + } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Person.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Person.java index 5c8f9427b..30207222a 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Person.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/Person.java @@ -42,6 +42,11 @@ public class Person { this.profiles = profiles; } + public Person(String name, Map profiles) { + this.name = name; + this.profiles = profiles; + } + public Long getId() { return id; } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonLoader.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonLoader.java index 31e7afb52..03cef69fb 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonLoader.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonLoader.java @@ -42,7 +42,6 @@ public class PersonLoader @Override public void afterPropertiesSet() throws Exception { - Address pers1addr = addressRepository.save(new Address(new String[]{"1234 W. 1st St."}, "Univille", "ST", "12345")); Map pers1profiles = new HashMap(); Profile twitter = profileRepository.save(new Profile("twitter", "#!/johndoe")); @@ -53,19 +52,32 @@ public class PersonLoader Person p1 = personRepository.save( new Person( "John Doe", - Arrays.asList(addressRepository.findOne(pers1addr.getId())), pers1profiles ) ); - Address pers2addr = addressRepository.save(new Address(new String[]{"1234 E. 2nd St."}, "Univille", "ST", "12345")); + Address pers1addr = addressRepository.save(new Address(new String[]{"1234 W. 1st St."}, + "Univille", + "ST", + "12345", + p1)); + p1.setAddresses(Arrays.asList(pers1addr)); + personRepository.save(p1); Map pers2profiles = new HashMap(); Profile twitter2 = profileRepository.save(new Profile("twitter", "#!/janedoe")); Profile fb2 = profileRepository.save(new Profile("facebook", "/janedoe")); pers2profiles.put("facebook", fb2); - Person p2 = personRepository.save(new Person("Jane Doe", Arrays.asList(pers2addr), pers2profiles)); + Person p2 = personRepository.save(new Person("Jane Doe", pers2profiles)); + + Address pers2addr = addressRepository.save(new Address(new String[]{"1234 E. 2nd St."}, + "Univille", + "ST", + "12345", + p2)); + p2.setAddresses(Arrays.asList(pers2addr)); + personRepository.save(p2); } diff --git a/spring-data-rest-webmvc/src/test/resources/load_data.sh b/spring-data-rest-webmvc/src/test/resources/load_data.sh index ae4900941..a12367b90 100755 --- a/spring-data-rest-webmvc/src/test/resources/load_data.sh +++ b/spring-data-rest-webmvc/src/test/resources/load_data.sh @@ -4,10 +4,10 @@ curl -v -d '{"name" : "John Doe"}' -H "Content-Type: application/json" http://lo curl -v -d '{"name" : "Jane Doe"}' -H "Content-Type: application/json" http://localhost:8080/people curl -v -d 'http://localhost:8080/people/1 http://localhost:8080/people/2' -H "Content-Type: text/uri-list" http://localhost:8080/family/1/members -curl -v -d '{"postalCode":"12345","province":"MO","lines":["1 W 1st St."],"city":"Univille"}' -H "Content-Type: application/json" http://localhost:8080/address +curl -v -d '{"postalCode":"12345","province":"MO","lines":["1 W 1st St."],"city":"Univille","person": {"href":"http://localhost:8080/people/1"}}' -H "Content-Type: application/json" http://localhost:8080/address curl -v -d "http://localhost:8080/address/1" -H "Content-Type: text/uri-list" http://localhost:8080/people/1/addresses curl -v -d "http://localhost:8080/people/1" -X PUT -H "Content-Type: text/uri-list" http://localhost:8080/address/1/person -curl -v -d '{"postalCode":"54321","province":"MO","lines":["2 W 1st St."],"city":"Univille"}' -H "Content-Type: application/json" http://localhost:8080/address +curl -v -d '{"postalCode":"54321","province":"MO","lines":["2 W 1st St."],"city":"Univille","person": {"href":"http://localhost:8080/people/2"}}' -H "Content-Type: application/json" http://localhost:8080/address curl -v -d "http://localhost:8080/address/2" -H "Content-Type: text/uri-list" http://localhost:8080/people/2/addresses curl -v -d '{"type" : "twitter", "url": "#!/johndoe"}' -H "Content-Type: application/json" http://localhost:8080/profile curl -v -d "http://localhost:8080/profile/1" -H "Content-Type: text/uri-list" http://localhost:8080/people/1/profiles