From 14db3d5f638c31d66b354516e368e3475dd5cfa3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 16 Jul 2015 12:38:42 +0200 Subject: [PATCH] DATAREST-617 - Fixed content negotiation for requests with Accept: application/*+json. We now tweak the ProducesRequestConditions for repository controllers to implicitly declare a produces clause of application/*+json to make sure the more concrete produces declarations on methods creating the compact representation are only used as fallbacks. --- .../webmvc/BasePathAwareHandlerMapping.java | 34 +++++++++-- .../webmvc/RepositoryRestHandlerMapping.java | 27 ++++++++ ...oryRestHandlerMappingIntegrationTests.java | 61 +++++++++++++++++++ ...yRepresentationConfigIntegrationTests.java | 23 +++++-- 4 files changed, 133 insertions(+), 12 deletions(-) create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingIntegrationTests.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 76b464205..8dfee41e2 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 @@ -51,6 +51,7 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition; +import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition; import org.springframework.web.servlet.mvc.method.RequestMappingInfo; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; import org.springframework.web.util.UrlPathHelper; @@ -122,9 +123,23 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { return null; } - PatternsRequestCondition patternsCondition = info.getPatternsCondition(); + PatternsRequestCondition patternsCondition = customize(info.getPatternsCondition(), prefix); + ProducesRequestCondition producesCondition = customize(info.getProducesCondition()); - Set patterns = patternsCondition.getPatterns(); + return new RequestMappingInfo(patternsCondition, info.getMethodsCondition(), info.getParamsCondition(), + info.getHeadersCondition(), info.getConsumesCondition(), producesCondition, info.getCustomCondition()); + } + + /** + * Customize the given {@link PatternsRequestCondition} and prefix. + * + * @param condition will never be {@literal null}. + * @param prefix will never be {@literal null}. + * @return + */ + protected PatternsRequestCondition customize(PatternsRequestCondition condition, String prefix) { + + Set patterns = condition.getPatterns(); String[] augmentedPatterns = new String[patterns.size()]; int count = 0; @@ -132,11 +147,18 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { augmentedPatterns[count++] = prefix.concat(pattern); } - PatternsRequestCondition condition = new PatternsRequestCondition(augmentedPatterns, getUrlPathHelper(), - getPathMatcher(), useSuffixPatternMatch(), useTrailingSlashMatch(), getFileExtensions()); + return new PatternsRequestCondition(augmentedPatterns, getUrlPathHelper(), getPathMatcher(), + useSuffixPatternMatch(), useTrailingSlashMatch(), getFileExtensions()); + } - return new RequestMappingInfo(condition, info.getMethodsCondition(), info.getParamsCondition(), - info.getHeadersCondition(), info.getConsumesCondition(), info.getProducesCondition(), info.getCustomCondition()); + /** + * Customize the given {@link ProducesRequestCondition}. Default implementation returns the condition as is. + * + * @param condition will never be {@literal null}. + * @return + */ + protected ProducesRequestCondition customize(ProducesRequestCondition condition) { + return condition; } /* 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 d122b3f4d..7e669eaa6 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 @@ -15,6 +15,8 @@ */ package org.springframework.data.rest.webmvc; +import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -25,10 +27,13 @@ import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; import org.springframework.data.rest.core.mapping.ResourceMappings; import org.springframework.data.rest.webmvc.support.JpaHelper; +import org.springframework.http.MediaType; +import org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter; import org.springframework.orm.jpa.support.OpenEntityManagerInViewInterceptor; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition; import org.springframework.web.servlet.mvc.method.RequestMappingInfo; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; @@ -43,6 +48,9 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl */ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { + private static final MediaType EVERYTHING_JSON_MEDIA_TYPE = new MediaType("application", "*+json", + AbstractJackson2HttpMessageConverter.DEFAULT_CHARSET); + private final ResourceMappings mappings; private final RepositoryRestConfiguration configuration; @@ -128,6 +136,25 @@ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { } } + /* + * (non-Javadoc) + * @see org.springframework.data.rest.webmvc.BasePathAwareHandlerMapping#process(org.springframework.web.servlet.mvc.condition.ProducesRequestCondition) + */ + @Override + protected ProducesRequestCondition customize(ProducesRequestCondition condition) { + + if (!condition.isEmpty()) { + return condition; + } + + HashSet mediaTypes = new LinkedHashSet(); + mediaTypes.add(configuration.getDefaultMediaType().toString()); + mediaTypes.add(MediaType.APPLICATION_JSON_VALUE); + mediaTypes.add(EVERYTHING_JSON_MEDIA_TYPE.toString()); + + return new ProducesRequestCondition(mediaTypes.toArray(new String[mediaTypes.size()])); + } + /** * Returns the first segment of the given repository lookup path. * diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingIntegrationTests.java new file mode 100644 index 000000000..b1a2e8563 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingIntegrationTests.java @@ -0,0 +1,61 @@ +/* + * 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; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.rest.webmvc.mongodb.MongoDbRepositoryConfig; +import org.springframework.data.rest.webmvc.support.DelegatingHandlerMapping; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.HandlerExecutionChain; + +/** + * Integration tests for {@link BasePathAwareHandlerMapping}. + * + * @author Oliver Gierke + * @soundtrack Elephants Crossing - Echo (Irrelephant) + */ +@ContextConfiguration(classes = MongoDbRepositoryConfig.class) +public class RepositoryRestHandlerMappingIntegrationTests extends AbstractControllerIntegrationTests { + + @Autowired DelegatingHandlerMapping mapping; + + /** + * @see DATAREST-617 + */ + @Test + public void usesMethodsWithoutProducesClauseForGeneralJsonRequests() throws Exception { + + MockHttpServletRequest mockRequest = new MockHttpServletRequest("GET", "/users"); + mockRequest.addHeader("Accept", "application/*+json"); + + HandlerExecutionChain chain = mapping.getHandler(mockRequest); + + assertThat(chain, is(notNullValue())); + + Object handler = chain.getHandler(); + assertThat(handler, is(instanceOf(HandlerMethod.class))); + + HandlerMethod method = (HandlerMethod) handler; + assertThat(method.getMethod().getDeclaringClass(), is(typeCompatibleWith(RepositoryEntityController.class))); + assertThat(method.getMethod().getName(), is("getCollectionResource")); + } +} diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/LegacyRepresentationConfigIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/LegacyRepresentationConfigIntegrationTests.java index df4199314..ab3ecd95f 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/LegacyRepresentationConfigIntegrationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/LegacyRepresentationConfigIntegrationTests.java @@ -19,9 +19,13 @@ import static org.hamcrest.CoreMatchers.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import java.util.Arrays; + import org.junit.Test; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; +import org.springframework.data.rest.webmvc.mongodb.MongoDbRepositoryConfig; import org.springframework.http.MediaType; import org.springframework.test.context.ContextConfiguration; @@ -34,6 +38,7 @@ import org.springframework.test.context.ContextConfiguration; public class LegacyRepresentationConfigIntegrationTests extends AbstractRepositoryRestMvcConfigurationIntegrationTests { @Configuration + @Import(MongoDbRepositoryConfig.class) static class Config extends RepositoryRestMvcConfiguration { @Override @@ -44,22 +49,28 @@ public class LegacyRepresentationConfigIntegrationTests extends AbstractReposito } /** - * @see DATAREST-213 + * @see DATAREST-213, DATAREST-617 */ @Test public void returnsJsonIfConfiguredAndRequested() throws Exception { - mvc.perform(get("/").accept(MediaType.APPLICATION_JSON)). // - andExpect(jsonPath("links", is(notNullValue()))); + for (String resource : Arrays.asList("/", "/users")) { + + mvc.perform(get(resource).accept(MediaType.APPLICATION_JSON)). // + andExpect(jsonPath("links", is(notNullValue()))); + } } /** - * @see DATAREST-213 + * @see DATAREST-213, DATAREST-617 */ @Test public void returnsJsonIfConfigured() throws Exception { - mvc.perform(get("/").accept(MediaType.ALL)). // - andExpect(jsonPath("links", is(notNullValue()))); + for (String resource : Arrays.asList("/", "/users")) { + + mvc.perform(get(resource).accept(MediaType.ALL)). // + andExpect(jsonPath("links", is(notNullValue()))); + } } }