DATAREST-490 - Makes sure content negotiation works for manual resource overrides with dedicated media type.

In case a manually implemented controller is registered to add a dedicated media type for a Spring Data REST resource (e.g. an HTML representation for item resources) we now continue to serve JSON in case the request indicates it wants to see it.

Previously this didn't work as the manually implemented controller method was detected to partially match (only the produces-clause not matching) and thus an exception being thrown from the HandlerMapping lookup. This caused the HandlerMapping registered for repositories not being considered at all.

This is now fixed by hiding the HandlerMapping instances we register behind a DelegatingHandlerMapping that continues to try delegates even in a case of an exception being caused in a particular resolution attempt. Should all resolution attempts fail, we then throw the original exception if one occurred in the first place.

Author: odrotbohm

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java

@@ -41,7 +41,6 @@
 import javax.servlet.http.HttpSession;
 import javax.servlet.http.Part;
 
-import org.springframework.core.Ordered;
 import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
 import org.springframework.util.Assert;
 import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition;
@@ -71,7 +70,6 @@ public BasePathAwareHandlerMapping(RepositoryRestConfiguration configuration) {
 
 		Assert.notNull(configuration, "RepositoryRestConfiguration must not be null!");
 		this.configuration = configuration;
-		setOrder(Ordered.LOWEST_PRECEDENCE - 150);
 	}
 
 	/* 

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java

@@ -21,7 +21,6 @@
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
-import org.springframework.core.Ordered;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
 import org.springframework.data.rest.core.mapping.ResourceMappings;
@@ -65,8 +64,6 @@ public RepositoryRestHandlerMapping(ResourceMappings mappings, RepositoryRestCon
 
 		this.mappings = mappings;
 		this.configuration = config;
-
-		setOrder(Ordered.LOWEST_PRECEDENCE - 100);
 	}
 
 	/**

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java

@@ -85,6 +85,7 @@
 import org.springframework.data.rest.webmvc.spi.BackendIdConverter.DefaultIdConverter;
 import org.springframework.data.rest.webmvc.support.BackendIdHandlerMethodArgumentResolver;
 import org.springframework.data.rest.webmvc.support.DefaultedPageableHandlerMethodArgumentResolver;
+import org.springframework.data.rest.webmvc.support.DelegatingHandlerMapping;
 import org.springframework.data.rest.webmvc.support.ETagArgumentResolver;
 import org.springframework.data.rest.webmvc.support.HttpMethodHandlerMethodArgumentResolver;
 import org.springframework.data.rest.webmvc.support.JpaHelper;
@@ -115,9 +116,9 @@
 import org.springframework.util.ClassUtils;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.method.support.HandlerMethodArgumentResolver;
+import org.springframework.web.servlet.HandlerMapping;
 import org.springframework.web.servlet.mvc.method.annotation.ExceptionHandlerExceptionResolver;
 import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter;
-import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping;
 
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.Module;
@@ -494,23 +495,31 @@ public RequestMappingHandlerAdapter repositoryExporterHandlerAdapter() {
 	}
 
 	/**
-	 * Special {@link org.springframework.web.servlet.HandlerMapping} that only recognizes handler methods defined in the
-	 * provided controller classes.
+	 * The {@link HandlerMapping} to delegate requests to Spring Data REST controllers. Sets up a
+	 * {@link DelegatingHandlerMapping} to make sure manually implemented {@link BasePathAwareController} instances that
+	 * register custom handlers for certain media types don't cause the {@link RepositoryRestHandlerMapping} to be
+	 * omitted.
 	 * 
+	 * @see DATAREST-490
 	 * @return
 	 */
 	@Bean
-	public RequestMappingHandlerMapping repositoryExporterHandlerMapping() {
+	public DelegatingHandlerMapping restHandlerMapping() {
 
-		RepositoryRestHandlerMapping mapping = new RepositoryRestHandlerMapping(resourceMappings(), config());
-		mapping.setJpaHelper(jpaHelper());
+		RepositoryRestHandlerMapping repositoryMapping = new RepositoryRestHandlerMapping(resourceMappings(), config());
+		repositoryMapping.setJpaHelper(jpaHelper());
+		repositoryMapping.setApplicationContext(applicationContext);
+		repositoryMapping.afterPropertiesSet();
 
-		return mapping;
-	}
+		BasePathAwareHandlerMapping basePathMapping = new BasePathAwareHandlerMapping(config());
+		basePathMapping.setApplicationContext(applicationContext);
+		basePathMapping.afterPropertiesSet();
 
-	@Bean
-	public RequestMappingHandlerMapping fallbackMapping() {
-		return new BasePathAwareHandlerMapping(config());
+		List<HandlerMapping> mappings = new ArrayList<HandlerMapping>();
+		mappings.add(basePathMapping);
+		mappings.add(repositoryMapping);
+
+		return new DelegatingHandlerMapping(mappings);
 	}
 
 	@Bean

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java

@@ -0,0 +1,91 @@
+/*
+ * Copyright 2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.webmvc.support;
+
+import java.util.List;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.springframework.core.Ordered;
+import org.springframework.util.Assert;
+import org.springframework.web.HttpMediaTypeException;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
+import org.springframework.web.servlet.HandlerExecutionChain;
+import org.springframework.web.servlet.HandlerMapping;
+
+/**
+ * A {@link HandlerMapping} that considers a {@link List} of delegates. It will keep on traversing the delegates in case
+ * an {@link HttpMediaTypeNotAcceptableException} is thrown while trying to lookup the handler on a particular delegate.
+ * 
+ * @author Oliver Gierke
+ * @soundtrack Benny Greb - Stabila (Moving Parts)
+ */
+public class DelegatingHandlerMapping implements HandlerMapping, Ordered {
+
+	private final List<HandlerMapping> delegates;
+
+	/**
+	 * Creates a new {@link DelegatingHandlerMapping} for the given delegates.
+	 * 
+	 * @param delegates must not be {@literal null}.
+	 */
+	public DelegatingHandlerMapping(List<HandlerMapping> delegates) {
+
+		Assert.notNull(delegates, "Delegates must not be null!");
+
+		this.delegates = delegates;
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.core.Ordered#getOrder()
+	 */
+	@Override
+	public int getOrder() {
+		return Ordered.LOWEST_PRECEDENCE - 100;
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.web.servlet.HandlerMapping#getHandler(javax.servlet.http.HttpServletRequest)
+	 */
+	@Override
+	public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception {
+
+		HttpMediaTypeException exception = null;
+
+		for (HandlerMapping delegate : delegates) {
+
+			try {
+
+				HandlerExecutionChain result = delegate.getHandler(request);
+
+				if (result != null) {
+					return result;
+				}
+
+			} catch (HttpMediaTypeNotAcceptableException o_O) {
+				exception = o_O;
+			}
+		}
+
+		if (exception != null) {
+			throw exception;
+		}
+
+		return null;
+	}
+}

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java

@@ -18,7 +18,12 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
+import org.springframework.data.rest.webmvc.BasePathAwareController;
+import org.springframework.http.MediaType;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
 
 /**
  * Test configuration for JPA.
@@ -40,4 +45,11 @@ public BookIdConverter bookIdConverter() {
 	public TestDataPopulator testDataPopulator() {
 		return new TestDataPopulator();
 	}
+
+	@BasePathAwareController
+	static class BooksHtmlController {
+
+		@RequestMapping(value = "/books/{id}", method = RequestMethod.GET, produces = MediaType.TEXT_HTML_VALUE)
+		void person(@PathVariable String id) {}
+	}
 }

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.webmvc.support;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import java.util.Arrays;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
+import org.springframework.web.servlet.HandlerMapping;
+
+/**
+ * Unit tests for {@link DelegatingHandlerMapping}.
+ * 
+ * @author Oliver Gierke
+ * @soundtrack Benny Greb - Stabila (Moving Parts)
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class DelegatingHandlerMappingUnitTests {
+
+	@Mock HandlerMapping first, second;
+	@Mock HttpServletRequest request;
+
+	/**
+	 * @see DATAREST-490
+	 */
+	@Test
+	@SuppressWarnings("unchecked")
+	public void testname() throws Exception {
+
+		HandlerMapping handlerMapping = new DelegatingHandlerMapping(Arrays.asList(first, second));
+
+		when(first.getHandler(request)).thenThrow(HttpMediaTypeNotAcceptableException.class);
+
+		try {
+
+			handlerMapping.getHandler(request);
+			fail(String.format("Expected %s!", HttpMediaTypeNotAcceptableException.class.getSimpleName()));
+
+		} catch (HttpMediaTypeNotAcceptableException o_O) {
+			verify(second, times(1)).getHandler(request);
+		}
+	}
+}