#1197 - Polishing.

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.
This commit is contained in:
Oliver Drotbohm
2020-02-11 16:52:21 +01:00
parent 2919d60c46
commit f973afedd8
5 changed files with 106 additions and 250 deletions

View File

@@ -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<Annotation> 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<Annotation> originalAnnotations = new ArrayList<>(Arrays.asList(original));
Set<Class<?>> 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<Annotation> annotations) {
return cacheAndReturn(annotations.toArray(new Annotation[annotations.size()]));
}
private Annotation[] cacheAndReturn(Annotation[] annotations) {
this.combinedAnnotations = annotations;
return annotations;
}
}
}

View File

@@ -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();
}
}
}

View File

@@ -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();
}
}
}

View File

@@ -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<RepresentationModel<?>> root(@RequestParam String view);
}
@RestController
static class WebFluxClass implements WebFluxInterface {
@Override
public Mono<RepresentationModel<?>> root(String view) {
Mono<Link> 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();
}
}
}

View File

@@ -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<Link> verifications) {
Mono<Link> mono = link.toMono();
@@ -254,4 +266,21 @@ class WebFluxLinkBuilderTest {
return Mono.empty();
}
}
// #1189
interface WebFluxInterface {
@GetMapping("/api")
Mono<HttpEntity<?>> root(@RequestParam String view);
}
@RestController
static class WebFluxClass implements WebFluxInterface {
@Override
public Mono<HttpEntity<?>> root(String view) {
return Mono.empty();
}
}
}