DATAREST-825 - Fixed exposure of DELETE HTTP method if findOne(…) is not exposed.

Previously our support for the HTTP DELETE method on item resources was requiring a repository's findOne(…) method to be available and exposed. However, the latter might not be desirable as the support of GET and HEAD requests for item resources depends on that.

We now changed that to only checking that a findOne(…) method is declared on the repository as the implementation of RepositoryEntityController.deleteItemResource(…) requires it to be present to be able to trigger the events that intercept deletes for a particular type.

Author: odrotbohm

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/CrudMethodsSupportedHttpMethods.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2015 the original author or authors.
+ * Copyright 2014-2016 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,9 @@
 import static org.springframework.data.rest.core.mapping.ResourceType.*;
 import static org.springframework.http.HttpMethod.*;
 
+import lombok.NonNull;
+import lombok.RequiredArgsConstructor;
+
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.HashSet;
@@ -57,14 +60,15 @@ public CrudMethodsSupportedHttpMethods(CrudMethods crudMethods) {
 	 * @see org.springframework.data.rest.core.mapping.SupportedHttpMethods#getSupportedHttpMethods(org.springframework.data.rest.core.mapping.ResourceType)
 	 */
 	@Override
-	public Set<HttpMethod> getMethodsFor(ResourceType resourcType) {
+	public Set<HttpMethod> getMethodsFor(ResourceType resourceType) {
 
-		Assert.notNull(resourcType, "Resource type must not be null!");
+		Assert.notNull(resourceType, "Resource type must not be null!");
 
 		Set<HttpMethod> methods = new HashSet<HttpMethod>();
 		methods.add(OPTIONS);
 
-		switch (resourcType) {
+		switch (resourceType) {
+
 			case COLLECTION:
 
 				if (exposedMethods.exposesFindAll()) {
@@ -80,7 +84,7 @@ public Set<HttpMethod> getMethodsFor(ResourceType resourcType) {
 
 			case ITEM:
 
-				if (exposedMethods.exposesDelete() && exposedMethods.exposesFindOne()) {
+				if (exposedMethods.exposesDelete()) {
 					methods.add(DELETE);
 				}
 
@@ -97,7 +101,7 @@ public Set<HttpMethod> getMethodsFor(ResourceType resourcType) {
 				break;
 
 			default:
-				throw new IllegalArgumentException(String.format("Unsupported resource type %s!", resourcType));
+				throw new IllegalArgumentException(String.format("Unsupported resource type %s!", resourceType));
 		}
 
 		return Collections.unmodifiableSet(methods);
@@ -134,16 +138,10 @@ public Set<HttpMethod> getMethodsFor(PersistentProperty<?> property) {
 	/**
 	 * @author Oliver Gierke
 	 */
+	@RequiredArgsConstructor
 	private static class DefaultExposureAwareCrudMethods implements ExposureAwareCrudMethods {
 
-		private final CrudMethods crudMethods;
-
-		/**
-		 * @param exposedMethods
-		 */
-		public DefaultExposureAwareCrudMethods(CrudMethods crudMethods) {
-			this.crudMethods = crudMethods;
-		}
+		private final @NonNull CrudMethods crudMethods;
 
 		/* 
 		 * (non-Javadoc)
@@ -160,7 +158,7 @@ public boolean exposesSave() {
 		 */
 		@Override
 		public boolean exposesDelete() {
-			return exposes(crudMethods.getDeleteMethod());
+			return exposes(crudMethods.getDeleteMethod()) && crudMethods.hasFindOneMethod();
 		}
 
 		/* 

spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/CrudMethodsSupportedHttpMethodsUnitTests.java

@@ -121,6 +121,22 @@ public void exposesMethodsForProperties() {
 				allOf(hasItem(GET), not(hasItems(DELETE, PATCH, PUT, POST))));
 	}
 
+	/**
+	 * @see DATAREST-825
+	 */
+	@Test
+	public void supportsDeleteIfFindOneIsHidden() {
+		assertMethodsSupported(getSupportedHttpMethodsFor(HidesFindOne.class), ITEM, true, DELETE, PATCH, PUT, OPTIONS);
+	}
+
+	/**
+	 * @see DATAREST-825
+	 */
+	@Test
+	public void doesNotSupportDeleteIfNoFindOneAvailable() {
+		assertMethodsSupported(getSupportedHttpMethodsFor(NoFindOne.class), ITEM, false, DELETE);
+	}
+
 	private static SupportedHttpMethods getSupportedHttpMethodsFor(Class<?> repositoryInterface) {
 
 		RepositoryMetadata metadata = new DefaultRepositoryMetadata(repositoryInterface);
@@ -148,7 +164,18 @@ interface SampleRepository extends CrudRepository<Object, Long> {}
 	interface HidesDelete extends CrudRepository<Object, Long> {
 
 		@RestResource(exported = false)
-		void delete(Object id);
+		void delete(Object entity);
+	}
+
+	interface HidesFindOne extends CrudRepository<Object, Long> {
+
+		@Override
+		@RestResource(exported = false)
+		Object findOne(Long id);
+	}
+
+	interface NoFindOne extends Repository<Object, Long> {
+		void delete(Object entity);
 	}
 
 	interface EntityRepository extends CrudRepository<Entity, Long> {}