From 4f8298c71176365ec638ca5ea8e5655ef5d1a8ec Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 11 Mar 2015 15:10:44 +0100 Subject: [PATCH] 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. --- .../webmvc/BasePathAwareHandlerMapping.java | 2 - .../webmvc/RepositoryRestHandlerMapping.java | 3 - .../RepositoryRestMvcConfiguration.java | 31 ++++--- .../support/DelegatingHandlerMapping.java | 91 +++++++++++++++++++ .../rest/webmvc/jpa/JpaRepositoryConfig.java | 12 +++ .../DelegatingHandlerMappingUnitTests.java | 64 +++++++++++++ 6 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java index 1e2c41f4d..5c315d277 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java @@ -41,7 +41,6 @@ import javax.servlet.http.HttpServletResponse; 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 class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { Assert.notNull(configuration, "RepositoryRestConfiguration must not be null!"); this.configuration = configuration; - setOrder(Ordered.LOWEST_PRECEDENCE - 150); } /* diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java index 6c3e42366..d122b3f4d 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java @@ -21,7 +21,6 @@ import java.util.Set; 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 class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { this.mappings = mappings; this.configuration = config; - - setOrder(Ordered.LOWEST_PRECEDENCE - 100); } /** diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java index 3a90b8bed..2e5e11577 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java +++ b/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; 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.plugin.core.PluginRegistry; 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 class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon } /** - * 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 mappings = new ArrayList(); + mappings.add(basePathMapping); + mappings.add(repositoryMapping); + + return new DelegatingHandlerMapping(mappings); } @Bean diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java new file mode 100644 index 000000000..eec4d7ca0 --- /dev/null +++ b/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 delegates; + + /** + * Creates a new {@link DelegatingHandlerMapping} for the given delegates. + * + * @param delegates must not be {@literal null}. + */ + public DelegatingHandlerMapping(List 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; + } +} diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java index 565f23ecf..bff2b0c21 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java @@ -18,7 +18,12 @@ package org.springframework.data.rest.webmvc.jpa; 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 class JpaRepositoryConfig extends JpaInfrastructureConfig { 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) {} + } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java new file mode 100644 index 000000000..7771bcba3 --- /dev/null +++ b/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); + } + } +}