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.

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 @@ package org.springframework.data.rest.core.mapping;
 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 class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
 	 * @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 class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
 
 			case ITEM:
 
-				if (exposedMethods.exposesDelete() && exposedMethods.exposesFindOne()) {
+				if (exposedMethods.exposesDelete()) {
 					methods.add(DELETE);
 				}
 
@@ -97,7 +101,7 @@ public class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
 				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 class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
 	/**
 	 * @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 class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
 		 */
 		@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 class CrudMethodsSupportedHttpMethodsUnitTests {
 				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 @@ public class CrudMethodsSupportedHttpMethodsUnitTests {
 	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> {}