From f973afedd8b29a6ee3fe8dfd520eb5b224150e76 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Tue, 11 Feb 2020 16:52:21 +0100 Subject: [PATCH] #1197 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switched to a less complicated implementation of the interface method parameter annotation lookup by using Spring's ClassUtils.getInterfaceMethodIfPossible(…). Simplified test cases to pure unit test on the link builder APIs. We don't need to fully execute a complete MVC/WebFlux request/response cycle to verify the link creation to pick up the parameter annotations from the interfaces. Original pull request: #1194. --- .../hateoas/server/core/MethodParameters.java | 83 +++++++------ .../WebMvcLinkBuilderInterfaceClassTest.java | 103 ---------------- .../server/mvc/WebMvcLinkBuilderUnitTest.java | 28 +++++ .../WebFluxLinkBuilderInterfaceClassTest.java | 113 ------------------ .../reactive/WebFluxLinkBuilderTest.java | 29 +++++ 5 files changed, 106 insertions(+), 250 deletions(-) delete mode 100644 src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderInterfaceClassTest.java delete mode 100644 src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderInterfaceClassTest.java diff --git a/src/main/java/org/springframework/hateoas/server/core/MethodParameters.java b/src/main/java/org/springframework/hateoas/server/core/MethodParameters.java index bcb901ce..ad13a723 100644 --- a/src/main/java/org/springframework/hateoas/server/core/MethodParameters.java +++ b/src/main/java/org/springframework/hateoas/server/core/MethodParameters.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -31,6 +32,7 @@ import org.springframework.core.ParameterNameDiscoverer; import org.springframework.core.annotation.SynthesizingMethodParameter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.ConcurrentReferenceHashMap; /** @@ -149,14 +151,14 @@ public class MethodParameters { * set over discovering it. * * @author Oliver Gierke + * @author Greg Turnquist */ private static class AnnotationNamingMethodParameter extends SynthesizingMethodParameter { private final AnnotationAttribute attribute; - private String name; - @Nullable - private volatile Annotation[] combinedAnnotations; + private String name; + private @Nullable Annotation[] combinedAnnotations; /** * Creates a new {@link AnnotationNamingMethodParameter} for the given {@link Method}'s parameter with the given @@ -197,41 +199,54 @@ public class MethodParameters { } /** - * Return the annotations associated with the specific method/constructor parameter and any parent interfaces. + * Overriding the original behavior to also include parameter annotations declared on original interface method + * declaration for which the parameter is a member of the implementation method. */ @Override public Annotation[] getParameterAnnotations() { - Annotation[] anns = this.combinedAnnotations; - if (anns == null) { - anns = super.getParameterAnnotations(); - Class[] interfaces = getDeclaringClass().getInterfaces(); - for (Class iface : interfaces) { - try { - Method method = iface.getMethod(getExecutable().getName(), getExecutable().getParameterTypes()); - Annotation[] paramAnns = method.getParameterAnnotations()[getParameterIndex()]; - if (paramAnns.length > 0) { - List merged = new ArrayList<>(anns.length + paramAnns.length); - merged.addAll(Arrays.asList(anns)); - for (Annotation fieldAnn : paramAnns) { - boolean existingType = false; - for (Annotation ann : anns) { - if (ann.annotationType() == fieldAnn.annotationType()) { - existingType = true; - break; - } - } - if (!existingType) { - merged.add(fieldAnn); - } - } - anns = merged.toArray(new Annotation[]{}); - } - } catch (NoSuchMethodException ex) { - } - } - this.combinedAnnotations = anns; + + if (combinedAnnotations != null) { + return combinedAnnotations; } - return anns; + + Method method = getMethod(); + + if (method == null) { + throw new IllegalStateException("No method available for " + this.toString() + "!"); + } + + Annotation[] original = super.getParameterAnnotations(); + Method interfaceMethod = ClassUtils.getInterfaceMethodIfPossible(method); + + // No interface or method not declared in interface + if (method.equals(interfaceMethod)) { + return cacheAndReturn(original); + } + + // Lookup annotations and their types of the interface method parameter + MethodParameter interfaceParameter = new MethodParameter(interfaceMethod, getParameterIndex()); + List originalAnnotations = new ArrayList<>(Arrays.asList(original)); + Set> originalAnnotationTypes = originalAnnotations.stream() // + .map(Object::getClass) // + .collect(Collectors.toSet()); + + // Add annotations which have not been declared on the target method + Arrays.stream(interfaceParameter.getParameterAnnotations()) // + .filter(it -> !originalAnnotationTypes.contains(it.annotationType())) // + .forEach(originalAnnotations::add); + + return cacheAndReturn(originalAnnotations); + } + + private Annotation[] cacheAndReturn(List annotations) { + return cacheAndReturn(annotations.toArray(new Annotation[annotations.size()])); + } + + private Annotation[] cacheAndReturn(Annotation[] annotations) { + + this.combinedAnnotations = annotations; + + return annotations; } } } diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderInterfaceClassTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderInterfaceClassTest.java deleted file mode 100644 index 652c07fe..00000000 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderInterfaceClassTest.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2020 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 - * - * https://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.hateoas.server.mvc; - -import static org.hamcrest.Matchers.*; -import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.*; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; -import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.hateoas.MediaTypes; -import org.springframework.hateoas.RepresentationModel; -import org.springframework.hateoas.config.EnableHypermediaSupport; -import org.springframework.hateoas.config.EnableHypermediaSupport.HypermediaType; -import org.springframework.http.HttpHeaders; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit.jupiter.SpringExtension; -import org.springframework.test.context.web.WebAppConfiguration; -import org.springframework.test.web.servlet.MockMvc; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.RestController; -import org.springframework.web.context.WebApplicationContext; -import org.springframework.web.servlet.config.annotation.EnableWebMvc; - -/** - * @author Greg Turnquist - */ -@ExtendWith(SpringExtension.class) -@WebAppConfiguration -@ContextConfiguration -public class WebMvcLinkBuilderInterfaceClassTest { - - @Autowired WebApplicationContext context; - - MockMvc mockMvc; - - @BeforeEach - void setUp() { - this.mockMvc = webAppContextSetup(this.context).build(); - } - - @Test - void parentInterfaceCanHoldSpringWebAnnotations() throws Exception { - - this.mockMvc.perform(get("http://example.com/api?view=short").accept(MediaTypes.HAL_JSON_VALUE)) // - .andDo(print()) // - .andExpect(status().isOk()) // - .andExpect(header().string(HttpHeaders.CONTENT_TYPE, MediaTypes.HAL_JSON_VALUE)) // - .andExpect(jsonPath("$._links.*", hasSize(1))) // - .andExpect(jsonPath("$._links.self.href", is("http://example.com/api?view=short"))); - } - - interface WebMvcInterface { - - @GetMapping("/api") - RepresentationModel root(@RequestParam String view); - } - - @RestController - static class WebMvcClass implements WebMvcInterface { - - @Override - public RepresentationModel root(String view) { - - RepresentationModel model = new RepresentationModel<>(); - model.add(linkTo(methodOn(WebMvcClass.class).root(view)).withSelfRel()); - return model; - } - } - - @Configuration - @EnableWebMvc - @EnableHypermediaSupport(type = { HypermediaType.HAL }) - static class TestConfig { - - @Bean - WebMvcClass concreteController() { - return new WebMvcClass(); - } - } - -} diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java index d3ca04da..90999f54 100644 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java @@ -30,11 +30,14 @@ import org.springframework.hateoas.TemplateVariable; import org.springframework.hateoas.TemplateVariable.VariableType; import org.springframework.hateoas.TestUtils; import org.springframework.http.HttpEntity; +import org.springframework.http.ResponseEntity; import org.springframework.util.MultiValueMap; +import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; @@ -614,6 +617,14 @@ class WebMvcLinkBuilderUnitTest extends TestUtils { assertThat(link.expand().getHref()).endsWith("/foo?offset=1"); } + @Test // #1189 + void parentInterfaceCanHoldSpringWebAnnotations() { + + Link link = linkTo(methodOn(WebMvcClass.class).root("short")).withSelfRel(); + + assertThat(link.getHref()).endsWith("/api?view=short"); + } + private static UriComponents toComponents(Link link) { return UriComponentsBuilder.fromUriString(link.expand().getHref()).build(); } @@ -724,4 +735,21 @@ class WebMvcLinkBuilderUnitTest extends TestUtils { interface ChildControllerWithRootMapping extends ParentControllerWithoutRootMapping { } + + // #1189 + + interface WebMvcInterface { + + @GetMapping("/api") + HttpEntity root(@RequestParam String view); + } + + @RestController + static class WebMvcClass implements WebMvcInterface { + + @Override + public HttpEntity root(String view) { + return ResponseEntity.noContent().build(); + } + } } diff --git a/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderInterfaceClassTest.java b/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderInterfaceClassTest.java deleted file mode 100644 index 65573920..00000000 --- a/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderInterfaceClassTest.java +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Copyright 2020 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 - * - * https://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.hateoas.server.reactive; - -import static org.assertj.core.api.AssertionsForInterfaceTypes.*; -import static org.springframework.hateoas.server.reactive.WebFluxLinkBuilder.*; - -import reactor.core.publisher.Mono; -import reactor.test.StepVerifier; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.ApplicationContext; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.hateoas.IanaLinkRelations; -import org.springframework.hateoas.Link; -import org.springframework.hateoas.MediaTypes; -import org.springframework.hateoas.RepresentationModel; -import org.springframework.hateoas.config.EnableHypermediaSupport; -import org.springframework.hateoas.config.EnableHypermediaSupport.HypermediaType; -import org.springframework.hateoas.config.WebClientConfigurer; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit.jupiter.SpringExtension; -import org.springframework.test.context.web.WebAppConfiguration; -import org.springframework.test.web.reactive.server.WebTestClient; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.RestController; -import org.springframework.web.reactive.config.EnableWebFlux; - -/** - * @author Greg Turnquist - */ -@ExtendWith(SpringExtension.class) -@WebAppConfiguration -@ContextConfiguration -public class WebFluxLinkBuilderInterfaceClassTest { - - @Autowired WebTestClient testClient; - - @Test - void parentInterfaceCanHoldSpringWebAnnotations() throws Exception { - - this.testClient.get().uri("http://example.com/api?view=short") // - .accept(MediaTypes.HAL_JSON) // - .exchange() // - .expectStatus().isOk() // - .expectHeader().contentType(MediaTypes.HAL_JSON) // - .returnResult(RepresentationModel.class) // - .getResponseBody() // - .as(StepVerifier::create) // - .expectNextMatches(resourceSupport -> { - - assertThat(resourceSupport.getLinks())// - .containsExactly(Link.of("http://example.com/api?view=short", IanaLinkRelations.SELF)); - - return true; - }) // - .verifyComplete(); - } - - interface WebFluxInterface { - - @GetMapping("/api") - Mono> root(@RequestParam String view); - } - - @RestController - static class WebFluxClass implements WebFluxInterface { - - @Override - public Mono> root(String view) { - - Mono selfLink = linkTo(methodOn(WebFluxClass.class).root(view)).withSelfRel().toMono(); - - return selfLink.map(RepresentationModel::new); - } - } - - @Configuration - @EnableWebFlux - @EnableHypermediaSupport(type = { HypermediaType.HAL }) - static class TestConfig { - - @Bean - WebFluxClass concreteController() { - return new WebFluxClass(); - } - - @Bean - WebTestClient webTestClient(WebClientConfigurer webClientConfigurer, ApplicationContext ctx) { - - return WebTestClient.bindToApplicationContext(ctx).build().mutate() - .exchangeStrategies(webClientConfigurer.hypermediaExchangeStrategies()).build(); - } - } - -} diff --git a/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java b/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java index b2f4f851..c1365572 100644 --- a/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java +++ b/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java @@ -33,10 +33,12 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.hateoas.IanaLinkRelations; import org.springframework.hateoas.Link; import org.springframework.hateoas.server.reactive.WebFluxLinkBuilder.WebFluxLink; +import org.springframework.http.HttpEntity; import org.springframework.lang.Nullable; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ServerWebExchange; @@ -214,6 +216,16 @@ class WebFluxLinkBuilderTest { }); } + @Test // #1189 + void detectsParameterAnnotationOnInterfaceDeclarations() throws Exception { + + WebFluxLink link = linkTo(methodOn(WebFluxClass.class).root("any")).withSelfRel(); + + verify(null, link, it -> { + assertThat(it.getHref()).endsWith("/api?view=any"); + }); + } + private void verify(@Nullable MockServerHttpRequest request, WebFluxLink link, Consumer verifications) { Mono mono = link.toMono(); @@ -254,4 +266,21 @@ class WebFluxLinkBuilderTest { return Mono.empty(); } } + + // #1189 + + interface WebFluxInterface { + + @GetMapping("/api") + Mono> root(@RequestParam String view); + } + + @RestController + static class WebFluxClass implements WebFluxInterface { + + @Override + public Mono> root(String view) { + return Mono.empty(); + } + } }