From 9acad5d55d6a0ffa7588a7bdbf58760b4950ed4f Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 21 Aug 2013 12:14:40 +0200 Subject: [PATCH] DATAREST-111 - Slightly raised precedence of RepositoryRestHandlerMapping. Set order of RepositoryRestHandlerMapping to LOWEST_PRECEDENCE - 100 to give core Spring Framework components the chance to hook into the right place in the chain. Refactored RepositoryRestHandlerMapping to really check for the exposed path and added test cases for handler method resolution. --- .../rest/core/mapping/ResourceMappings.java | 19 ++++ .../ResourceMappingsIntegrationTest.java | 16 +++ .../webmvc/RepositoryRestHandlerMapping.java | 83 +++++++++++--- .../RepositoryRestMvcConfiguration.java | 7 +- ...RepositoryRestHandlerMappingUnitTests.java | 103 ++++++++++++++++++ 5 files changed, 211 insertions(+), 17 deletions(-) create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingUnitTests.java diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/ResourceMappings.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/ResourceMappings.java index cea4b6a2b..5587b3319 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/ResourceMappings.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/ResourceMappings.java @@ -161,6 +161,25 @@ public class ResourceMappings implements Iterable { return metadata.isExported(); } + /** + * Returns whether we export a top-level resource for the given path. + * + * @param path must not be {@literal null} or empty. + * @return + */ + public boolean exportsTopLevelResourceFor(String path) { + + Assert.hasText(path); + + for (ResourceMetadata metadata : cache.values()) { + if (metadata.getPath().matches(path)) { + return metadata.isExported(); + } + } + + return false; + } + /** * Returns whether we have a {@link ResourceMapping} for the given type. * diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/ResourceMappingsIntegrationTest.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/ResourceMappingsIntegrationTest.java index cafc00179..1eb9cb7ff 100644 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/ResourceMappingsIntegrationTest.java +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/ResourceMappingsIntegrationTest.java @@ -97,4 +97,20 @@ public class ResourceMappingsIntegrationTest { assertThat(mapping.getPath(), is(new Path("siblings"))); assertThat(mapping.isExported(), is(true)); } + + /** + * @see DATAREST-111 + */ + @Test + public void exposesResourceByPath() { + + assertThat(mappings.exportsTopLevelResourceFor("people"), is(true)); + assertThat(mappings.exportsTopLevelResourceFor("orders"), is(true)); + + ResourceMetadata creditCardMapping = mappings.getMappingFor(CreditCard.class); + assertThat(creditCardMapping, is(notNullValue())); + assertThat(creditCardMapping.getPath(), is(new Path("creditCards"))); + assertThat(creditCardMapping.isExported(), is(false)); + assertThat(mappings.exportsTopLevelResourceFor("creditCards"), is(false)); + } } 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 9875c3ba5..cba198db0 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 @@ -1,3 +1,18 @@ +/* + * Copyright 2012-2013 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.springframework.util.StringUtils.*; @@ -8,15 +23,14 @@ import java.util.List; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.data.repository.support.Repositories; 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.orm.jpa.support.OpenEntityManagerInViewInterceptor; +import org.springframework.util.Assert; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; @@ -27,37 +41,66 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl * REST exporter to function properly. * * @author Jon Brisbin + * @author Oliver Gierke */ public class RepositoryRestHandlerMapping extends RequestMappingHandlerMapping { - @Autowired private Repositories repositories; - @Autowired private RepositoryRestConfiguration config; - @Autowired(required = false) private JpaHelper jpaHelper; - private final ResourceMappings mappings; + private final RepositoryRestConfiguration config; + + private JpaHelper jpaHelper; + + /** + * Creates a new {@link RepositoryRestHandlerMapping} for the given {@link ResourceMappings} and + * {@link RepositoryRestConfiguration}. + * + * @param mappings must not be {@literal null}. + * @param config must not be {@literal null}. + */ + public RepositoryRestHandlerMapping(ResourceMappings mappings, RepositoryRestConfiguration config) { + + Assert.notNull(mappings, "ResourceMappings must not be null!"); + Assert.notNull(config, "RepositoryRestConfiguration must not be null!"); - public RepositoryRestHandlerMapping(ResourceMappings mappings) { - setOrder(Ordered.LOWEST_PRECEDENCE); this.mappings = mappings; + this.config = config; + + setOrder(Ordered.LOWEST_PRECEDENCE - 100); } + /** + * @param jpaHelper the jpaHelper to set + */ + public void setJpaHelper(JpaHelper jpaHelper) { + this.jpaHelper = jpaHelper; + } + + /* + * (non-Javadoc) + * @see org.springframework.web.servlet.handler.AbstractHandlerMethodMapping#lookupHandlerMethod(java.lang.String, javax.servlet.http.HttpServletRequest) + */ @Override protected HandlerMethod lookupHandlerMethod(String lookupPath, HttpServletRequest origRequest) throws Exception { + String acceptType = origRequest.getHeader("Accept"); + if (null == acceptType) { acceptType = config.getDefaultMediaType().toString(); } + List acceptHeaderTypes = MediaType.parseMediaTypes(acceptType); List acceptableTypes = new ArrayList(); + for (MediaType mt : acceptHeaderTypes) { - if (("*".equals(mt.getType()) && ("*".equals(mt.getSubtype())) || ("application".equals(mt.getType()) && "*" - .equals(mt.getSubtype())))) { + if ("*".equals(mt.getType()) && "*".equals(mt.getSubtype()) || "application".equals(mt.getType()) + && "*".equals(mt.getSubtype())) { mt = config.getDefaultMediaType(); } if (!acceptableTypes.contains(mt)) { acceptableTypes.add(mt); } } + if (acceptableTypes.size() > 1) { acceptType = collectionToDelimitedString(acceptableTypes, ","); } else if (acceptableTypes.size() == 1) { @@ -72,29 +115,38 @@ public class RepositoryRestHandlerMapping extends RequestMappingHandlerMapping { if (requestUri.startsWith("/")) { requestUri = requestUri.substring(1); } + if (!hasText(requestUri)) { return super.lookupHandlerMethod(lookupPath, request); } + String[] parts = requestUri.split("/"); + if (parts.length == 0) { // Root request return super.lookupHandlerMethod(lookupPath, request); } - for (Class domainType : repositories) { - if (mappings.exportsMappingFor(domainType)) { - return super.lookupHandlerMethod(lookupPath, request); - } + if (mappings.exportsTopLevelResourceFor(parts[0])) { + return super.lookupHandlerMethod(lookupPath, request); } return null; } + /* + * (non-Javadoc) + * @see org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping#isHandler(java.lang.Class) + */ @Override protected boolean isHandler(Class beanType) { return AnnotationUtils.findAnnotation(beanType, RepositoryRestController.class) != null; } + /* + * (non-Javadoc) + * @see org.springframework.web.servlet.handler.AbstractHandlerMapping#extendInterceptors(java.util.List) + */ @Override protected void extendInterceptors(List interceptors) { if (null != jpaHelper) { @@ -105,6 +157,7 @@ public class RepositoryRestHandlerMapping extends RequestMappingHandlerMapping { } private static class DefaultAcceptTypeHttpServletRequest extends HttpServletRequestWrapper { + private final String defaultAcceptType; private DefaultAcceptTypeHttpServletRequest(HttpServletRequest request, String defaultAcceptType) { @@ -114,6 +167,7 @@ public class RepositoryRestHandlerMapping extends RequestMappingHandlerMapping { @Override public String getHeader(String name) { + if ("accept".equals(name.toLowerCase())) { return defaultAcceptType; } else { @@ -121,5 +175,4 @@ public class RepositoryRestHandlerMapping extends RequestMappingHandlerMapping { } } } - } 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 2cbc3d812..b1d02f0db 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 @@ -146,7 +146,6 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon } @Bean - @Lazy public JpaHelper jpaHelper() { if (IS_JPA_AVAILABLE) { return new JpaHelper(); @@ -325,7 +324,11 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon */ @Bean public RequestMappingHandlerMapping repositoryExporterHandlerMapping() { - return new RepositoryRestHandlerMapping(resourceMappings()); + + RepositoryRestHandlerMapping mapping = new RepositoryRestHandlerMapping(resourceMappings(), config()); + mapping.setJpaHelper(jpaHelper()); + + return mapping; } @Bean diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingUnitTests.java new file mode 100644 index 000000000..def2a908c --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingUnitTests.java @@ -0,0 +1,103 @@ +/* + * Copyright 2013 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.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import java.lang.reflect.Method; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.rest.core.config.RepositoryRestConfiguration; +import org.springframework.data.rest.core.mapping.ResourceMappings; +import org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.web.method.HandlerMethod; + +/** + * Unit tests for {@link RepositoryRestHandlerMapping}. + * + * @author Oliver Gierke + */ +@RunWith(MockitoJUnitRunner.class) +public class RepositoryRestHandlerMappingUnitTests { + + static final ApplicationContext CONTEXT = new AnnotationConfigApplicationContext(RepositoryRestMvcConfiguration.class); + + @Mock ResourceMappings mappings; + + RepositoryRestConfiguration configuration; + RepositoryRestHandlerMapping handlerMapping; + MockHttpServletRequest mockRequest; + Method listEntitiesMethod; + + @Before + public void setUp() throws Exception { + + configuration = new RepositoryRestConfiguration(); + + handlerMapping = new RepositoryRestHandlerMapping(mappings, configuration); + handlerMapping.setApplicationContext(CONTEXT); + handlerMapping.afterPropertiesSet(); + + mockRequest = new MockHttpServletRequest(); + + listEntitiesMethod = RepositoryEntityController.class.getMethod("listEntities", RepositoryRestRequest.class, + Pageable.class, Sort.class); + } + + @Test(expected = IllegalArgumentException.class) + public void rejectsNullMappings() { + new RepositoryRestHandlerMapping(null, configuration); + } + + @Test(expected = IllegalArgumentException.class) + public void rejectsNullConfiguration() { + new RepositoryRestHandlerMapping(mappings, null); + } + + /** + * @see DATAREST-111 + */ + @Test + public void returnsNullForUriNotMapped() throws Exception { + assertThat(handlerMapping.lookupHandlerMethod("/foo", mockRequest), is(nullValue())); + } + + /** + * @see DATAREST-111 + */ + @Test + public void looksUpRepositoryEntityControllerMethodCorrectly() throws Exception { + + when(mappings.exportsTopLevelResourceFor("people")).thenReturn(true); + mockRequest = new MockHttpServletRequest("GET", "/people"); + + HandlerMethod method = handlerMapping.lookupHandlerMethod("/people", mockRequest); + + assertThat(method, is(notNullValue())); + assertThat(method.getMethod(), is(listEntitiesMethod)); + } +}