From 2f6650c84649afd45dafb3c4bcdd834b7be940a2 Mon Sep 17 00:00:00 2001 From: Jon Brisbin Date: Mon, 30 Jul 2012 13:53:13 -0500 Subject: [PATCH] Added before/after link delete events and fixed a bug with deleting links where the updated entity was never saved back to the DB. --- .../rest/repository/RepositoryMetadata.java | 11 ++ .../annotation/HandleAfterLinkDelete.java | 19 +++ .../annotation/HandleBeforeLinkDelete.java | 19 +++ .../AbstractRepositoryEventListener.java | 22 +++ .../context/AfterLinkDeleteEvent.java | 10 ++ .../repository/context/BeforeDeleteEvent.java | 3 +- .../context/BeforeLinkDeleteEvent.java | 10 ++ .../rest/repository/invoke/CrudMethod.java | 63 +++++++++ .../repository/invoke/RepositoryMethod.java | 58 +------- .../repository/jpa/JpaAttributeMetadata.java | 3 +- .../repository/jpa/JpaEntityMetadata.java | 21 ++- .../repository/jpa/JpaRepositoryMetadata.java | 35 ++++- .../data/rest/repository/support/Methods.java | 28 ++++ .../webmvc/RepositoryRestConfiguration.java | 10 ++ .../rest/webmvc/RepositoryRestController.java | 125 ++++++++++++++++-- .../RepositoryRestMvcConfiguration.java | 1 + .../webmvc/ThrowableHttpMessageConverter.java | 48 +++++++ .../data/rest/webmvc/spec/BaseSpec.groovy | 2 +- .../rest/webmvc/spec/QueryMethodsSpec.groovy | 65 +++++++++ .../webmvc/spec/TopLevelEntitySpec.groovy | 15 +++ .../rest/test/webmvc/ApplicationConfig.java | 3 + .../rest/test/webmvc/PersonRepository.java | 11 +- 22 files changed, 497 insertions(+), 85 deletions(-) create mode 100644 spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleAfterLinkDelete.java create mode 100644 spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleBeforeLinkDelete.java create mode 100644 spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/AfterLinkDeleteEvent.java create mode 100644 spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/BeforeLinkDeleteEvent.java create mode 100644 spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/CrudMethod.java create mode 100644 spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/support/Methods.java create mode 100644 spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ThrowableHttpMessageConverter.java create mode 100644 spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/QueryMethodsSpec.groovy diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/RepositoryMetadata.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/RepositoryMetadata.java index 211aff430..c941c3d6b 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/RepositoryMetadata.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/RepositoryMetadata.java @@ -1,10 +1,12 @@ package org.springframework.data.rest.repository; import java.io.Serializable; +import java.lang.reflect.Method; import java.util.Map; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.Repository; +import org.springframework.data.rest.repository.invoke.CrudMethod; import org.springframework.data.rest.repository.invoke.RepositoryQueryMethod; /** @@ -74,4 +76,13 @@ public interface RepositoryMetadata queryMethods(); + /** + * Does this Repository all this method to be exported? + * + * @param method + * + * @return + */ + Boolean exportsMethod(CrudMethod method); + } diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleAfterLinkDelete.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleAfterLinkDelete.java new file mode 100644 index 000000000..68780cbbf --- /dev/null +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleAfterLinkDelete.java @@ -0,0 +1,19 @@ +package org.springframework.data.rest.repository.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * @author Jon Brisbin + */ +@Target({ElementType.METHOD}) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +public @interface HandleAfterLinkDelete { + + Class[] value() default {}; + +} diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleBeforeLinkDelete.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleBeforeLinkDelete.java new file mode 100644 index 000000000..15e3b6e0c --- /dev/null +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/annotation/HandleBeforeLinkDelete.java @@ -0,0 +1,19 @@ +package org.springframework.data.rest.repository.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * @author Jon Brisbin + */ +@Target({ElementType.METHOD}) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +public @interface HandleBeforeLinkDelete { + + Class[] value() default {}; + +} diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/AbstractRepositoryEventListener.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/AbstractRepositoryEventListener.java index b56980aac..96e9b1752 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/AbstractRepositoryEventListener.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/AbstractRepositoryEventListener.java @@ -42,6 +42,10 @@ public abstract class AbstractRepositoryEventListener */ -public class BeforeDeleteEvent - extends RepositoryEvent { +public class BeforeDeleteEvent extends RepositoryEvent { public BeforeDeleteEvent(Object source) { super(source); } diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/BeforeLinkDeleteEvent.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/BeforeLinkDeleteEvent.java new file mode 100644 index 000000000..9dea78cc9 --- /dev/null +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/context/BeforeLinkDeleteEvent.java @@ -0,0 +1,10 @@ +package org.springframework.data.rest.repository.context; + +/** + * @author Jon Brisbin + */ +public class BeforeLinkDeleteEvent extends LinkSaveEvent{ + public BeforeLinkDeleteEvent(Object source, Object linked) { + super(source, linked); + } +} diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/CrudMethod.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/CrudMethod.java new file mode 100644 index 000000000..53d594b9e --- /dev/null +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/CrudMethod.java @@ -0,0 +1,63 @@ +package org.springframework.data.rest.repository.invoke; + +import java.lang.reflect.Method; + +/** + * @author Jon Brisbin + */ +public enum CrudMethod { + + COUNT, + DELETE_ALL, + DELETE_ONE, + DELETE_SOME, + FIND_ALL, + FIND_ONE, + FIND_SOME, + SAVE_ONE, + SAVE_SOME; + + public static CrudMethod fromMethod(Method m) { + String s = m.getName(); + Class[] paramTypes = m.getParameterTypes(); + boolean some = (paramTypes.length > 0 && Iterable.class.isAssignableFrom(paramTypes[0])); + if("count".equals(s)) { + return COUNT; + } else if("delete".equals(s)) { + return (some ? DELETE_SOME : DELETE_ONE); + } else if("deleteAll".equals(s)) { + return DELETE_ALL; + } else if("findAll".equals(s)) { + return (some ? FIND_SOME : FIND_ALL); + } else if("findOne".equals(s)) { + return FIND_ONE; + } else if("save".equals(s)) { + return (some ? SAVE_SOME : SAVE_ONE); + } else { + return null; + } + } + + public String toMethodName() { + switch(this) { + case COUNT: + return "count"; + case DELETE_ALL: + return "deleteAll"; + case DELETE_ONE: + case DELETE_SOME: + return "delete"; + case FIND_ALL: + case FIND_SOME: + return "findAll"; + case FIND_ONE: + return "findOne"; + case SAVE_ONE: + case SAVE_SOME: + return "save"; + default: + return null; + } + } + +} diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/RepositoryMethod.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/RepositoryMethod.java index 03e134200..fdf1093f7 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/RepositoryMethod.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/invoke/RepositoryMethod.java @@ -3,70 +3,16 @@ package org.springframework.data.rest.repository.invoke; import java.lang.annotation.Annotation; import java.lang.reflect.Method; -import org.springframework.core.LocalVariableTableParameterNameDiscoverer; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.repository.query.Param; -import org.springframework.util.ReflectionUtils; +import org.springframework.data.rest.repository.support.Methods; /** * @author Jon Brisbin */ public class RepositoryMethod { - public enum Type { - COUNT, - CUSTOM, - DELETE, - FIND_ALL, - FIND_ONE, - SAVE; - - public static Type fromMethodName(String s) { - if("count".equals(s)) { - return COUNT; - } else if("delete".equals(s)) { - return DELETE; - } else if("findAll".equals(s)) { - return FIND_ALL; - } else if("findOne".equals(s)) { - return FIND_ONE; - } else if("save".equals(s)) { - return SAVE; - } else { - return CUSTOM; - } - } - - public String toMethodName() { - switch(this) { - case COUNT: - return "count"; - case DELETE: - return "delete"; - case FIND_ALL: - return "findAll"; - case FIND_ONE: - return "findOne"; - case SAVE: - return "save"; - default: - return null; - } - } - - } - - public static final ReflectionUtils.MethodFilter USER_METHODS = new ReflectionUtils.MethodFilter() { - @Override public boolean matches(Method method) { - return (!method.isSynthetic() - && !method.isBridge() - && method.getDeclaringClass() != Object.class - && !method.getName().contains("$")); - } - }; - public static final LocalVariableTableParameterNameDiscoverer NAME_DISCOVERER = new LocalVariableTableParameterNameDiscoverer(); - private Method method; private Class[] paramTypes; private String[] paramNames; @@ -84,7 +30,7 @@ public class RepositoryMethod { sortable = true; } } - paramNames = NAME_DISCOVERER.getParameterNames(method); + paramNames = Methods.NAME_DISCOVERER.getParameterNames(method); if(null == paramNames) { paramNames = new String[paramTypes.length]; } 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 06362adaa..7cb15bcba 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 @@ -17,8 +17,7 @@ import org.springframework.util.ReflectionUtils; /** * @author Jon Brisbin */ -public class JpaAttributeMetadata - implements AttributeMetadata { +public class JpaAttributeMetadata implements AttributeMetadata { private String name; private Attribute attribute; diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaEntityMetadata.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaEntityMetadata.java index fdf234a0d..134467138 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaEntityMetadata.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaEntityMetadata.java @@ -12,6 +12,7 @@ import org.springframework.data.repository.support.Repositories; import org.springframework.data.rest.repository.EntityMetadata; import org.springframework.data.rest.repository.annotation.RestResource; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; /** * @author Jon Brisbin @@ -36,22 +37,28 @@ public class JpaEntityMetadata for(Attribute attr : entityType.getAttributes()) { boolean exported = true; Field field = ReflectionUtils.findField(type, attr.getJavaMember().getName()); - if(null != field) { - RestResource fieldResourceAnno = field.getAnnotation(RestResource.class); - if(null != fieldResourceAnno) { - exported = fieldResourceAnno.exported(); - } + if(null == field) { + continue; + } + + RestResource fieldResourceAnno = field.getAnnotation(RestResource.class); + if(null != fieldResourceAnno) { + exported = fieldResourceAnno.exported(); } if(exported) { + String name = attr.getName(); + if(null != fieldResourceAnno && StringUtils.hasText(fieldResourceAnno.path())) { + name = fieldResourceAnno.path(); + } Class attrType = (attr instanceof PluralAttribute ? ((PluralAttribute)attr).getElementType().getJavaType() : attr.getJavaType()); if(repositories.hasRepositoryFor(attrType)) { - linkedAttributes.put(attr.getName(), new JpaAttributeMetadata(entityType, attr)); + linkedAttributes.put(name, new JpaAttributeMetadata(entityType, attr)); } else { if(!(attr instanceof SingularAttribute && ((SingularAttribute)attr).isId()) && !(attr instanceof SingularAttribute && ((SingularAttribute)attr).isVersion())) { - embeddedAttributes.put(attr.getName(), new JpaAttributeMetadata(entityType, attr)); + embeddedAttributes.put(name, new JpaAttributeMetadata(entityType, attr)); } } } diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaRepositoryMetadata.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaRepositoryMetadata.java index 1dfe91846..d21b6b663 100644 --- a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaRepositoryMetadata.java +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/jpa/JpaRepositoryMetadata.java @@ -13,21 +13,23 @@ import org.springframework.data.repository.core.EntityInformation; import org.springframework.data.repository.support.Repositories; import org.springframework.data.rest.repository.RepositoryMetadata; import org.springframework.data.rest.repository.annotation.RestResource; +import org.springframework.data.rest.repository.invoke.CrudMethod; import org.springframework.data.rest.repository.invoke.RepositoryQueryMethod; +import org.springframework.data.rest.repository.support.Methods; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** * @author Jon Brisbin */ -public class JpaRepositoryMetadata - implements RepositoryMetadata { +public class JpaRepositoryMetadata implements RepositoryMetadata { private final String name; private final Class repoClass; private final CrudRepository repository; private final EntityInformation entityInfo; - private final Map queryMethods = new HashMap(); + private final Map crudMethodExposed = new HashMap(); + private final Map queryMethods = new HashMap(); private String rel; private JpaEntityMetadata entityMetadata; @@ -66,6 +68,24 @@ public class JpaRepositoryMetadata } } + ReflectionUtils.doWithMethods( + repoClass, + new ReflectionUtils.MethodCallback() { + @Override public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { + CrudMethod cr = CrudMethod.fromMethod(method); + RestResource rr = method.getAnnotation(RestResource.class); + if(null != rr) { + crudMethodExposed.put(cr, rr.exported()); + } + } + }, + new ReflectionUtils.MethodFilter() { + @Override public boolean matches(Method method) { + return (null != CrudMethod.fromMethod(method) && Methods.USER_METHODS.matches(method)); + } + } + ); + Metamodel metamodel = entityManager.getMetamodel(); entityMetadata = new JpaEntityMetadata(repositories, metamodel.entity(entityInfo.getJavaType())); } @@ -102,6 +122,15 @@ public class JpaRepositoryMetadata return Collections.unmodifiableMap(queryMethods); } + @Override public Boolean exportsMethod(CrudMethod method) { + Boolean b = crudMethodExposed.get(method); + if(null == b) { + return true; + } else { + return b; + } + } + @Override public String toString() { return "JpaRepositoryMetadata{" + "name='" + name + '\'' + diff --git a/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/support/Methods.java b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/support/Methods.java new file mode 100644 index 000000000..48c8919c5 --- /dev/null +++ b/spring-data-rest-repository/src/main/java/org/springframework/data/rest/repository/support/Methods.java @@ -0,0 +1,28 @@ +package org.springframework.data.rest.repository.support; + +import java.lang.reflect.Method; + +import org.springframework.core.LocalVariableTableParameterNameDiscoverer; +import org.springframework.util.ReflectionUtils; + +/** + * @author Jon Brisbin + */ +public abstract class Methods { + + private Methods() { + } + + public static final ReflectionUtils.MethodFilter USER_METHODS = + new ReflectionUtils.MethodFilter() { + @Override public boolean matches(Method method) { + return (!method.isSynthetic() + && !method.isBridge() + && method.getDeclaringClass() != Object.class + && !method.getName().contains("$")); + } + }; + public static final LocalVariableTableParameterNameDiscoverer NAME_DISCOVERER = + new LocalVariableTableParameterNameDiscoverer(); + +} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestConfiguration.java index 67920a1b6..4debccc11 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestConfiguration.java @@ -22,6 +22,7 @@ public class RepositoryRestConfiguration { private String jsonpOnErrParamName = null; private List> customConverters = Collections.emptyList(); private MediaType defaultMediaType = MediaType.APPLICATION_JSON; + private boolean dumpErrors = false; public int getDefaultPageSize() { return defaultPageSize; @@ -100,4 +101,13 @@ public class RepositoryRestConfiguration { return this; } + public boolean isDumpErrors() { + return dumpErrors; + } + + public RepositoryRestConfiguration setDumpErrors(boolean dumpErrors) { + this.dumpErrors = dumpErrors; + return this; + } + } 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 09b7ed8f7..0cc623934 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 @@ -31,6 +31,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.convert.ConversionService; +import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -53,12 +54,15 @@ import org.springframework.data.rest.repository.RepositoryMetadata; import org.springframework.data.rest.repository.RepositoryNotFoundException; import org.springframework.data.rest.repository.annotation.RestResource; import org.springframework.data.rest.repository.context.AfterDeleteEvent; +import org.springframework.data.rest.repository.context.AfterLinkDeleteEvent; import org.springframework.data.rest.repository.context.AfterLinkSaveEvent; import org.springframework.data.rest.repository.context.AfterSaveEvent; import org.springframework.data.rest.repository.context.BeforeDeleteEvent; +import org.springframework.data.rest.repository.context.BeforeLinkDeleteEvent; import org.springframework.data.rest.repository.context.BeforeLinkSaveEvent; import org.springframework.data.rest.repository.context.BeforeSaveEvent; import org.springframework.data.rest.repository.context.RepositoryEvent; +import org.springframework.data.rest.repository.invoke.CrudMethod; import org.springframework.data.rest.repository.invoke.RepositoryMethodResponse; import org.springframework.data.rest.repository.invoke.RepositoryQueryMethod; import org.springframework.format.support.DefaultFormattingConversionService; @@ -348,6 +352,10 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.FIND_ALL)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } + RepositoryMethodResponse response = new RepositoryMethodResponse(); Iterator allEntities = Collections.emptyList().iterator(); @@ -515,8 +523,18 @@ public class RepositoryRestController String[] paramNames = queryMethod.paramNames(); Object[] paramVals = new Object[paramTypes.length]; for(int i = 0; i < paramVals.length; i++) { - String queryVal = request.getServletRequest().getParameter(paramNames[i]); - if(null == queryVal) { + if(Pageable.class.isAssignableFrom(paramTypes[i])) { + // Handle paging + paramVals[i] = pageSort; + continue; + } else if(Sort.class.isAssignableFrom(paramTypes[i])) { + // Handle sorting + paramVals[i] = (null != pageSort ? pageSort.getSort() : null); + continue; + } + + String queryVal; + if(null == (queryVal = request.getServletRequest().getParameter(paramNames[i]))) { continue; } @@ -524,12 +542,6 @@ public class RepositoryRestController if(String.class.isAssignableFrom(paramTypes[i])) { // Param type is a String paramVals[i] = queryVal; - } else if(Pageable.class.isAssignableFrom(paramTypes[i])) { - // Handle paging - paramVals[i] = pageSort; - } else if(Sort.class.isAssignableFrom(paramTypes[i])) { - // Handle sorting - paramVals[i] = (null != pageSort ? pageSort.getSort() : null); } else if(null != (paramRepoMeta = repositoryMetadataFor(paramTypes[i]))) { // Complex parameter is a managed type Serializable id = stringToSerializable(queryVal, @@ -667,6 +679,9 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.SAVE_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } CrudRepository repo = repoMeta.repository(); MediaType incomingMediaType = request.getHeaders().getContentType(); @@ -726,6 +741,9 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.FIND_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() .idAttribute() @@ -790,6 +808,9 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.SAVE_ONE) || !repoMeta.exportsMethod(CrudMethod.FIND_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() .idAttribute() @@ -865,6 +886,9 @@ public class RepositoryRestController @PathVariable String repository, @PathVariable String id) throws IOException { RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.DELETE_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() .idAttribute() @@ -909,6 +933,9 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.FIND_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() .idAttribute() @@ -931,6 +958,10 @@ public class RepositoryRestController } RepositoryMetadata propRepoMeta = repositoryMetadataFor(attrType); + if(!propRepoMeta.exportsMethod(CrudMethod.FIND_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } + Object propVal; if(null == (propVal = attrMeta.get(entity))) { @@ -1001,6 +1032,9 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); final RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.SAVE_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() .idAttribute() @@ -1109,6 +1143,9 @@ public class RepositoryRestController @PathVariable String id, @PathVariable String property) throws IOException { RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.SAVE_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } CrudRepository repo = repoMeta.repository(); Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() @@ -1162,6 +1199,9 @@ public class RepositoryRestController URI baseUri = uriBuilder.build().toUri(); RepositoryMetadata repoMeta = repositoryMetadataFor(repository); + if(!repoMeta.exportsMethod(CrudMethod.FIND_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } AttributeMetadata attrMeta; if(null == (attrMeta = repoMeta.entityMetadata().attribute(property))) { @@ -1226,6 +1266,9 @@ public class RepositoryRestController @PathVariable String linkedId) throws IOException { RepositoryMetadata repoMeta = repositoryMetadataFor(repository); CrudRepository repo = repoMeta.repository(); + if(!repoMeta.exportsMethod(CrudMethod.FIND_ONE) || !repoMeta.exportsMethod(CrudMethod.SAVE_ONE)) { + return negotiateResponse(request, HttpStatus.METHOD_NOT_ALLOWED, new HttpHeaders(), null); + } Serializable serId = stringToSerializable(id, (Class)repoMeta.entityMetadata() .idAttribute() @@ -1283,6 +1326,10 @@ public class RepositoryRestController attrMeta.set(linkedEntity, entity); } + publishEvent(new BeforeLinkDeleteEvent(entity, linkedEntity)); + Object savedEntity = repo.save(entity); + publishEvent(new AfterLinkDeleteEvent(savedEntity, linkedEntity)); + return negotiateResponse(request, HttpStatus.NO_CONTENT, new HttpHeaders(), null); } @@ -1306,11 +1353,67 @@ public class RepositoryRestController return notFoundResponse(request); } + /** + * Handle NPEs as a regular 500 error. + * + * @param e + * @param request + * + * @return + * + * @throws IOException + */ + @ExceptionHandler(NullPointerException.class) + @ResponseBody + public ResponseEntity handleNPE(NullPointerException e, + ServletServerHttpRequest request) throws IOException { + if(LOG.isErrorEnabled()) { + LOG.error(e.getMessage(), e); + } + return negotiateResponse(request, HttpStatus.INTERNAL_SERVER_ERROR, new HttpHeaders(), null); + } + + /** + * Handle {@link InvocationTargetException}s as a 400 Bad Request because these are likely to occur if, e.g. the user + * does not provide a value for a query parameter. + * + * @param e + * @param request + * + * @return + * + * @throws IOException + */ + @ExceptionHandler(InvocationTargetException.class) + @ResponseBody + public ResponseEntity handleInvocationTargetException(InvocationTargetException e, + ServletServerHttpRequest request) throws IOException { + if(LOG.isErrorEnabled()) { + LOG.error(e.getMessage(), e); + } + + for(Throwable cause = e.getCause(); (null != cause && cause != e.getCause()); cause = cause.getCause()) { + if(cause instanceof InvalidDataAccessApiUsageException || cause instanceof IllegalArgumentException) { + return negotiateResponse(request, HttpStatus.BAD_REQUEST, new HttpHeaders(), null); + } + } + + return negotiateResponse(request, HttpStatus.INTERNAL_SERVER_ERROR, new HttpHeaders(), e); + } + + /** + * Handle failures commonly thrown from code tries to read incoming data and convert or cast it to the right type. + * + * @param t + * @param request + * + * @return + * + * @throws IOException + */ @ExceptionHandler( { - NullPointerException.class, IllegalArgumentException.class, - IllegalStateException.class, ClassCastException.class } ) @@ -1320,7 +1423,7 @@ public class RepositoryRestController if(LOG.isErrorEnabled()) { LOG.error(t.getMessage(), t); } - return negotiateResponse(request, HttpStatus.BAD_REQUEST, new HttpHeaders(), null); + return negotiateResponse(request, HttpStatus.BAD_REQUEST, new HttpHeaders(), t); } /** diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestMvcConfiguration.java index b2a00d938..dda706110 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestMvcConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestMvcConfiguration.java @@ -7,6 +7,7 @@ import org.springframework.context.annotation.ImportResource; import org.springframework.data.rest.repository.context.ValidatingRepositoryEventListener; import org.springframework.data.rest.repository.jpa.JpaRepositoryExporter; import org.springframework.orm.jpa.support.PersistenceAnnotationBeanPostProcessor; +import org.springframework.web.method.annotation.ExceptionHandlerMethodResolver; /** * @author Jon Brisbin diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ThrowableHttpMessageConverter.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ThrowableHttpMessageConverter.java new file mode 100644 index 000000000..c63e7f553 --- /dev/null +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ThrowableHttpMessageConverter.java @@ -0,0 +1,48 @@ +package org.springframework.data.rest.webmvc; + +import java.io.IOException; +import java.io.PrintWriter; + +import org.codehaus.jackson.map.ObjectMapper; +import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpOutputMessage; +import org.springframework.http.MediaType; +import org.springframework.http.converter.AbstractHttpMessageConverter; +import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.http.converter.HttpMessageNotWritableException; + +/** + * @author Jon Brisbin + */ +public class ThrowableHttpMessageConverter extends AbstractHttpMessageConverter { + + private final ObjectMapper mapper = new ObjectMapper(); + + @Override protected boolean supports(Class clazz) { + throw new IllegalStateException("supports(Class clazz) not used in " + getClass().getName()); + } + + @Override public boolean canRead(Class clazz, MediaType mediaType) { + return false; + } + + @Override public boolean canWrite(Class clazz, MediaType mediaType) { + return (Throwable.class.isAssignableFrom(clazz) + && (mediaType.getSubtype().contains("json") || mediaType.getSubtype().contains("text"))); + } + + @Override protected Throwable readInternal(Class clazz, HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { + throw new HttpMessageNotReadableException("Cannot read Throwables from input."); + } + + @Override protected void writeInternal(Throwable throwable, HttpOutputMessage outputMessage) + throws IOException, HttpMessageNotWritableException { + if(outputMessage.getHeaders().getContentType().getSubtype().contains("json")) { + outputMessage.getBody().write(mapper.writeValueAsBytes(throwable)); + } else { + throwable.printStackTrace(new PrintWriter(outputMessage.getBody())); + } + } + +} 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 8dcedc104..0d7c3ec30 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 @@ -76,7 +76,7 @@ abstract class BaseSpec extends Specification { method: method ) if (query) { - query.collect {k, v -> req.addParameter(k, v)} + query.collect { String k, String v -> req.addParameter(k, v)} } if (contentType) { req.contentType = contentType diff --git a/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/QueryMethodsSpec.groovy b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/QueryMethodsSpec.groovy new file mode 100644 index 000000000..72860efa4 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/groovy/org/springframework/data/rest/webmvc/spec/QueryMethodsSpec.groovy @@ -0,0 +1,65 @@ +package org.springframework.data.rest.webmvc.spec + +import org.springframework.data.domain.PageRequest +import org.springframework.data.rest.test.webmvc.Person +import org.springframework.data.rest.webmvc.PagingAndSorting +import org.springframework.data.rest.webmvc.RepositoryRestConfiguration +import org.springframework.http.HttpStatus +import spock.lang.Shared + +import java.lang.reflect.InvocationTargetException + +/** + * @author Jon Brisbin + */ +class QueryMethodsSpec extends BaseSpec { + + @Shared + def pageSort = new PagingAndSorting(RepositoryRestConfiguration.DEFAULT, new PageRequest(0, 10)) + + def "exposes query method links to discovery"() { + + given: + def request = createRequest("GET", "people/search", null) + + when: + def response = controller.listQueryMethods(request, baseUri, "people") + def body = readJson(response) + + then: + response.statusCode == HttpStatus.OK + body["_links"].size() == 2 + + } + + def "invokes query methods"() { + + given: + people.save(new Person(name: "John Doe")) + people.save(new Person(name: "Bill Doe")) + def request = createRequest("GET", "people/search/nameStartsWith", ["name": "John"]) + + when: + def response = controller.query(request, pageSort, baseUri, "people", "nameStartsWith") + def body = readJson(response) + + then: + response.statusCode == HttpStatus.OK + body["results"].size() == 1 + + } + + def "blows up on empty query parameters"() { + + given: + def request = createRequest("GET", "people/search/nameStartsWith", null) + + when: + controller.query(request, pageSort, baseUri, "people", "nameStartsWith") + + then: + thrown(InvocationTargetException) + + } + +} 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 02139e276..1f181f898 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 @@ -66,4 +66,19 @@ class TopLevelEntitySpec extends BaseSpec { } + def "won't delete entities whose delete methods are not exported"() { + + given: + def person = people.save(new Person(name: "John Doe")) + def persId = person.id + def request = createRequest("DELETE", "people/$persId", null) + + when: + def response = controller.deleteEntity(request, "people", "$persId") + + then: + response.statusCode == HttpStatus.METHOD_NOT_ALLOWED + + } + } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/ApplicationConfig.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/ApplicationConfig.java index 6277f2ec9..5d120eb39 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/ApplicationConfig.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/ApplicationConfig.java @@ -6,8 +6,10 @@ import javax.sql.DataSource; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; import org.springframework.data.rest.webmvc.RepositoryRestConfiguration; +import org.springframework.data.rest.webmvc.RepositoryRestMvcConfiguration; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; import org.springframework.orm.jpa.JpaDialect; @@ -23,6 +25,7 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; * @author Jon Brisbin */ @Configuration +@Import(RepositoryRestMvcConfiguration.class) @ComponentScan(basePackageClasses = ApplicationConfig.class) @EnableJpaRepositories @EnableTransactionManagement diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonRepository.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonRepository.java index 77d8fb5f9..94492dfa0 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonRepository.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/test/webmvc/PersonRepository.java @@ -14,10 +14,15 @@ import org.springframework.data.rest.repository.annotation.RestResource; @RestResource(path = "people", rel = "peeps") public interface PersonRepository extends PagingAndSortingRepository { - @RestResource(path = "name", rel = "names") - public List findByName(@Param("name") String name); + @Override + @RestResource(exported = false) void delete(Long id); + + @Override + @RestResource(exported = false) void delete(Person entity); + + @RestResource(path = "name", rel = "names") List findByName(@Param("name") String name); @RestResource(path = "nameStartsWith", rel = "nameStartsWith") - public Page findByNameStartsWith(@Param("name") String name, Pageable p); + Page findByNameStartsWith(@Param("name") String name, Pageable p); }