diff --git a/Spring Data Commons.sonargraph b/Spring Data Commons.sonargraph index a7846f6545..f21de9ad70 100644 --- a/Spring Data Commons.sonargraph +++ b/Spring Data Commons.sonargraph @@ -16,6 +16,7 @@ + diff --git a/pom.xml b/pom.xml index 64f7fc515c..0e084b0e11 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ 1.9.7 2.2.2 - 0.8.0.RELEASE + 0.9.0.BUILD-SNAPSHOT DATACMNS diff --git a/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java index f3dfc1463c..e107e2e00b 100644 --- a/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -76,9 +76,20 @@ public HateoasPageableHandlerMethodArgumentResolver(HateoasSortHandlerMethodArgu this.sortResolver = getDefaultedSortResolver(sortResolver); } - private static HateoasSortHandlerMethodArgumentResolver getDefaultedSortResolver( - HateoasSortHandlerMethodArgumentResolver sortResolver) { - return sortResolver == null ? DEFAULT_SORT_RESOLVER : sortResolver; + /** + * Returns the template variable for the pagination parameters. + * + * @param parameter can be {@literal null}. + * @return + * @since 1.7 + */ + public String getPaginationTemplateVariables(MethodParameter parameter) { + + String pagePropertyName = getParameterNameToUse(getPageParameterName(), parameter); + String sizePropertyName = getParameterNameToUse(getSizeParameterName(), parameter); + + String sortTemplateVariables = sortResolver.getSortTemplateVariables(parameter); + return String.format("{?%s,%s}{&%s", pagePropertyName, sizePropertyName, sortTemplateVariables.substring(2)); } /* @@ -105,4 +116,9 @@ public void enhance(UriComponentsBuilder builder, MethodParameter parameter, Obj this.sortResolver.enhance(builder, parameter, pageable.getSort()); } + + private static HateoasSortHandlerMethodArgumentResolver getDefaultedSortResolver( + HateoasSortHandlerMethodArgumentResolver sortResolver) { + return sortResolver == null ? DEFAULT_SORT_RESOLVER : sortResolver; + } } diff --git a/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java index 988b6aca4f..f27939bd01 100644 --- a/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -35,6 +35,17 @@ public class HateoasSortHandlerMethodArgumentResolver extends SortHandlerMethodArgumentResolver implements UriComponentsContributor { + /** + * Returns the tempalte variables for the sort parameter. + * + * @param parameter must not be {@literal null}. + * @return + * @since 1.7 + */ + public String getSortTemplateVariables(MethodParameter parameter) { + return String.format("{?%s}", getSortParameter(parameter)); + } + /* * (non-Javadoc) * @see org.springframework.hateoas.mvc.UriComponentsContributor#enhance(org.springframework.web.util.UriComponentsBuilder, org.springframework.core.MethodParameter, java.lang.Object) diff --git a/src/main/java/org/springframework/data/web/MethodParameterAwarePagedResourcesAssembler.java b/src/main/java/org/springframework/data/web/MethodParameterAwarePagedResourcesAssembler.java new file mode 100644 index 0000000000..c4d41fcfea --- /dev/null +++ b/src/main/java/org/springframework/data/web/MethodParameterAwarePagedResourcesAssembler.java @@ -0,0 +1,57 @@ +/* + * Copyright 2014 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.web; + +import org.springframework.core.MethodParameter; +import org.springframework.util.Assert; +import org.springframework.web.util.UriComponents; + +/** + * Custom {@link PagedResourcesAssembler} that is aware of the {@link MethodParameter} it shall create links for. + * + * @author Oliver Gierke + * @since 1.7 + */ +class MethodParameterAwarePagedResourcesAssembler extends PagedResourcesAssembler { + + private final MethodParameter parameter; + + /** + * Creates a new {@link MethodParameterAwarePagedResourcesAssembler} using the given {@link MethodParameter}, + * {@link HateoasPageableHandlerMethodArgumentResolver} and base URI. + * + * @param parameter must not be {@literal null}. + * @param resolver can be {@literal null}. + * @param baseUri can be {@literal null}. + */ + public MethodParameterAwarePagedResourcesAssembler(MethodParameter parameter, + HateoasPageableHandlerMethodArgumentResolver resolver, UriComponents baseUri) { + + super(resolver, baseUri); + + Assert.notNull(parameter, "Method parameter must not be null!"); + this.parameter = parameter; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.web.PagedResourcesAssembler#getMethodParameter() + */ + @Override + protected MethodParameter getMethodParameter() { + return parameter; + } +} diff --git a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java index 83732cc85f..f58629116a 100644 --- a/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java +++ b/src/main/java/org/springframework/data/web/PagedResourcesAssembler.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -20,9 +20,11 @@ import java.util.ArrayList; import java.util.List; +import org.springframework.core.MethodParameter; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.hateoas.Link; +import org.springframework.hateoas.LinkTemplate; import org.springframework.hateoas.PagedResources; import org.springframework.hateoas.PagedResources.PageMetadata; import org.springframework.hateoas.Resource; @@ -141,16 +143,30 @@ private PagedResources addPaginationLinks(PagedRe foo(resources, page.previousPageable(), uri, Link.REL_PREVIOUS); } + resources.add(new LinkTemplate(uri + pageableResolver.getPaginationTemplateVariables(getMethodParameter()), + Link.REL_SELF)); + return resources; } private void foo(PagedResources resources, Pageable pageable, String uri, String rel) { UriComponentsBuilder builder = fromUriString(uri); - pageableResolver.enhance(builder, null, pageable); + pageableResolver.enhance(builder, getMethodParameter(), pageable); resources.add(new Link(builder.build().toUriString(), rel)); } + /** + * Return the {@link MethodParameter} to be used to potentially qualify the paging and sorting request parameters to. + * Default implementations returns {@literal null}, which means the parameters will not be qualified. + * + * @return + * @since 1.7 + */ + protected MethodParameter getMethodParameter() { + return null; + } + /** * Creates a new {@link PageMetadata} instance from the given {@link Page}. * diff --git a/src/main/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolver.java b/src/main/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolver.java index 4f3c5b3690..7db819a30c 100644 --- a/src/main/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -15,9 +15,16 @@ */ package org.springframework.data.web; +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.MethodParameter; +import org.springframework.data.domain.Pageable; import org.springframework.hateoas.Link; import org.springframework.hateoas.MethodLinkBuilderFactory; +import org.springframework.hateoas.core.MethodParameters; import org.springframework.hateoas.mvc.ControllerLinkBuilderFactory; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; @@ -36,6 +43,11 @@ */ public class PagedResourcesAssemblerArgumentResolver implements HandlerMethodArgumentResolver { + private static final Logger LOGGER = LoggerFactory.getLogger(PagedResourcesAssemblerArgumentResolver.class); + + private static final String SUPERFLOUS_QUALIFIER = "Found qualified {} parameter, but a unique unqualified {} parameter. Using that one, but you might wanna check your controller method configuration!"; + private static final String PARAMETER_AMBIGUITY = "Discovered muliple parameters of type Pageable but no qualifier annotations to disambiguate!"; + private final HateoasPageableHandlerMethodArgumentResolver resolver; private final MethodLinkBuilderFactory linkBuilderFactory; @@ -73,6 +85,72 @@ public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer m Link linkToMethod = linkBuilderFactory.linkTo(parameter.getMethod(), new Object[0]).withSelfRel(); UriComponents fromUriString = UriComponentsBuilder.fromUriString(linkToMethod.getHref()).build(); - return new PagedResourcesAssembler(resolver, fromUriString); + MethodParameter pageableParameter = findMatchingPageableParameter(parameter); + + if (pageableParameter != null) { + return new MethodParameterAwarePagedResourcesAssembler(pageableParameter, resolver, fromUriString); + } else { + return new PagedResourcesAssembler(resolver, fromUriString); + } + } + + /** + * Returns finds the {@link MethodParameter} for a {@link Pageable} instance matching the given + * {@link MethodParameter} requesting a {@link PagedResourcesAssembler}. + * + * @param parameter must not be {@literal null}. + * @return + */ + private static final MethodParameter findMatchingPageableParameter(MethodParameter parameter) { + + MethodParameters parameters = new MethodParameters(parameter.getMethod()); + List pageableParameters = parameters.getParametersOfType(Pageable.class); + Qualifier assemblerQualifier = parameter.getParameterAnnotation(Qualifier.class); + + if (pageableParameters.isEmpty()) { + return null; + } + + if (pageableParameters.size() == 1) { + + MethodParameter pageableParameter = pageableParameters.get(0); + MethodParameter matchingParameter = returnIfQualifiersMatch(pageableParameter, assemblerQualifier); + + if (matchingParameter == null) { + LOGGER.info(SUPERFLOUS_QUALIFIER, PagedResourcesAssembler.class.getSimpleName(), Pageable.class.getName()); + } + + return pageableParameter; + } + + if (assemblerQualifier == null) { + throw new IllegalStateException(PARAMETER_AMBIGUITY); + } + + for (MethodParameter pageableParameter : pageableParameters) { + + MethodParameter matchingParameter = returnIfQualifiersMatch(pageableParameter, assemblerQualifier); + + if (matchingParameter != null) { + return matchingParameter; + } + } + + throw new IllegalStateException(PARAMETER_AMBIGUITY); + } + + private static MethodParameter returnIfQualifiersMatch(MethodParameter pageableParameter, Qualifier assemblerQualifier) { + + if (assemblerQualifier == null) { + return pageableParameter; + } + + Qualifier pageableParameterQualifier = pageableParameter.getParameterAnnotation(Qualifier.class); + + if (pageableParameterQualifier == null) { + return null; + } + + return pageableParameterQualifier.value().equals(assemblerQualifier.value()) ? pageableParameter : null; } } diff --git a/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java index 0100dcbf38..e9d8695a5b 100644 --- a/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/HateoasPageableHandlerMethodArgumentResolverUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -76,6 +76,19 @@ public void preventsPageSizeFromExceedingMayValueIfConfiguredOnWrite() throws Ex assertUriStringFor(new PageRequest(0, 200), "page=0&size=100"); } + /** + * @see DATACMNS-418 + */ + @Test + public void returnCorrectTemplateVariables() { + + HateoasPageableHandlerMethodArgumentResolver resolver = getResolver(); + assertThat(resolver.getPaginationTemplateVariables(null), is("{?page,size}{&sort}")); + + resolver.setPageParameterName("foo"); + assertThat(resolver.getPaginationTemplateVariables(null), is("{?foo,size}{&sort}")); + } + @Override protected HateoasPageableHandlerMethodArgumentResolver getResolver() { diff --git a/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java index 65b6d39750..99727e6d72 100644 --- a/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/HateoasSortHandlerMethodArgumentResolverUnitTests.java @@ -51,6 +51,16 @@ public void replacesExistingRequestParameters() throws Exception { assertUriStringFor(SORT, "/?sort=firstname,lastname,desc", "/?sort=foo,asc"); } + /** + * @see DATACMNS-418 + */ + @Test + public void returnCorrectTemplateVariables() { + + HateoasSortHandlerMethodArgumentResolver resolver = new HateoasSortHandlerMethodArgumentResolver(); + assertThat(resolver.getSortTemplateVariables(null), is("{?sort}")); + } + private void assertUriStringFor(Sort sort, String expected) throws Exception { assertUriStringFor(sort, expected, "/"); } diff --git a/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java new file mode 100644 index 0000000000..d04609e09f --- /dev/null +++ b/src/test/java/org/springframework/data/web/PagedResourcesAssemblerArgumentResolverUnitTests.java @@ -0,0 +1,182 @@ +/* + * Copyright 2014 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.web; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import java.lang.reflect.Method; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.core.MethodParameter; +import org.springframework.data.domain.Pageable; +import org.springframework.web.bind.annotation.RequestMapping; + +/** + * Unit tests for {@link PagedResourcesAssemblerArgumentResolver}. + * + * @author Oliver Gierke + * @since 1.7 + */ +public class PagedResourcesAssemblerArgumentResolverUnitTests { + + PagedResourcesAssemblerArgumentResolver resolver; + + public @Rule ExpectedException exception = ExpectedException.none(); + + @Before + public void setUp() { + + WebTestUtils.initWebTest(); + + HateoasPageableHandlerMethodArgumentResolver hateoasPageableHandlerMethodArgumentResolver = new HateoasPageableHandlerMethodArgumentResolver(); + this.resolver = new PagedResourcesAssemblerArgumentResolver(hateoasPageableHandlerMethodArgumentResolver, null); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void createsPlainAssemblerWithoutContext() throws Exception { + + Method method = Controller.class.getMethod("noContext", PagedResourcesAssembler.class); + Object result = resolver.resolveArgument(new MethodParameter(method, 0), null, null, null); + + assertThat(result, is(instanceOf(PagedResourcesAssembler.class))); + assertThat(result, is(not(instanceOf(MethodParameterAwarePagedResourcesAssembler.class)))); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void selectsUniquePageableParameter() throws Exception { + + Method method = Controller.class.getMethod("unique", PagedResourcesAssembler.class, Pageable.class); + assertSelectsParameter(method, 1); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void selectsUniquePageableParameterForQualifiedAssembler() throws Exception { + + Method method = Controller.class.getMethod("unnecessarilyQualified", PagedResourcesAssembler.class, Pageable.class); + assertSelectsParameter(method, 1); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void selectsUniqueQualifiedPageableParameter() throws Exception { + + Method method = Controller.class.getMethod("qualifiedUnique", PagedResourcesAssembler.class, Pageable.class); + assertSelectsParameter(method, 1); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void selectsQualifiedPageableParameter() throws Exception { + + Method method = Controller.class.getMethod("qualified", PagedResourcesAssembler.class, Pageable.class, + Pageable.class); + assertSelectsParameter(method, 1); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void rejectsAmbiguousPageableParameters() throws Exception { + assertRejectsAmbiguity("unqualifiedAmbiguity"); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void rejectsAmbiguousPageableParametersForQualifiedAssembler() throws Exception { + assertRejectsAmbiguity("assemblerQualifiedAmbiguity"); + } + + /** + * @see DATACMNS-418 + */ + @Test + public void rejectsAmbiguityWithoutMatchingQualifiers() throws Exception { + assertRejectsAmbiguity("noMatchingQualifiers"); + } + + private void assertSelectsParameter(Method method, int expectedIndex) throws Exception { + + MethodParameter parameter = new MethodParameter(method, 0); + + Object result = resolver.resolveArgument(parameter, null, null, null); + assertMethodParameterAwarePagedResourcesAssemblerFor(result, new MethodParameter(method, expectedIndex)); + } + + private static void assertMethodParameterAwarePagedResourcesAssemblerFor(Object result, MethodParameter parameter) { + + assertThat(result, is(instanceOf(MethodParameterAwarePagedResourcesAssembler.class))); + MethodParameterAwarePagedResourcesAssembler assembler = (MethodParameterAwarePagedResourcesAssembler) result; + + assertThat(assembler.getMethodParameter(), is(parameter)); + } + + private void assertRejectsAmbiguity(String methodName) throws Exception { + + Method method = Controller.class.getMethod(methodName, PagedResourcesAssembler.class, Pageable.class, + Pageable.class); + + exception.expect(IllegalStateException.class); + resolver.resolveArgument(new MethodParameter(method, 0), null, null, null); + } + + @RequestMapping("/") + static interface Controller { + + void noContext(PagedResourcesAssembler resolver); + + void unique(PagedResourcesAssembler assembler, Pageable pageable); + + void unnecessarilyQualified(@Qualifier("qualified") PagedResourcesAssembler assembler, Pageable pageable); + + void qualifiedUnique(@Qualifier("qualified") PagedResourcesAssembler assembler, + @Qualifier("qualified") Pageable pageable); + + void qualified(@Qualifier("qualified") PagedResourcesAssembler resolver, + @Qualifier("qualified") Pageable pageable, Pageable unqualified); + + void unqualifiedAmbiguity(PagedResourcesAssembler assembler, Pageable pageable, Pageable unqualified); + + void assemblerQualifiedAmbiguity(@Qualifier("qualified") PagedResourcesAssembler assembler, + Pageable pageable, Pageable unqualified); + + void noMatchingQualifiers(@Qualifier("qualified") PagedResourcesAssembler assembler, Pageable pageable, + @Qualifier("qualified2") Pageable unqualified); + + @RequestMapping("/{variable}/foo") + void methodWithPathVariable(PagedResourcesAssembler assembler); + } +}