Browse Source

Reject multiple @⁠HttpExchange declarations in HTTP interface clients

This commit updates HttpServiceMethod so that multiple @⁠HttpExchange
declarations on the same element (class or method) are rejected.

Closes gh-32049
pull/32064/head
Sam Brannen 2 years ago
parent
commit
6691ff2072
  1. 63
      spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceMethod.java
  2. 60
      spring-web/src/test/java/org/springframework/web/service/invoker/HttpServiceMethodTests.java

63
spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceMethod.java

@ -16,6 +16,7 @@ @@ -16,6 +16,7 @@
package org.springframework.web.service.invoker;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Method;
import java.time.Duration;
import java.util.List;
@ -32,7 +33,11 @@ import org.springframework.core.KotlinDetector; @@ -32,7 +33,11 @@ import org.springframework.core.KotlinDetector;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.core.ReactiveAdapter;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotationPredicates;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.annotation.RepeatableContainers;
import org.springframework.core.annotation.SynthesizingMethodParameter;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
@ -54,6 +59,7 @@ import org.springframework.web.service.annotation.HttpExchange; @@ -54,6 +59,7 @@ import org.springframework.web.service.annotation.HttpExchange;
* @author Rossen Stoyanchev
* @author Sebastien Deleuze
* @author Olga Maciaszek-Sharma
* @author Sam Brannen
* @since 6.0
*/
final class HttpServiceMethod {
@ -145,7 +151,7 @@ final class HttpServiceMethod { @@ -145,7 +151,7 @@ final class HttpServiceMethod {
/**
* Factory for {@link HttpRequestValues} with values extracted from the type
* and method-level {@link HttpExchange @HttpRequest} annotations.
* and method-level {@link HttpExchange @HttpExchange} annotations.
*/
private record HttpRequestValuesInitializer(
@Nullable HttpMethod httpMethod, @Nullable String url,
@ -177,10 +183,20 @@ final class HttpServiceMethod { @@ -177,10 +183,20 @@ final class HttpServiceMethod {
Method method, Class<?> containingClass, @Nullable StringValueResolver embeddedValueResolver,
Supplier<HttpRequestValues.Builder> requestValuesSupplier) {
HttpExchange typeAnnotation = AnnotatedElementUtils.findMergedAnnotation(containingClass, HttpExchange.class);
HttpExchange methodAnnotation = AnnotatedElementUtils.findMergedAnnotation(method, HttpExchange.class);
List<AnnotationDescriptor> methodHttpExchanges = getAnnotationDescriptors(method);
Assert.state(!methodHttpExchanges.isEmpty(),
() -> "Expected @HttpExchange annotation on method " + method);
Assert.state(methodHttpExchanges.size() == 1,
() -> "Multiple @HttpExchange annotations found on method %s, but only one is allowed: %s"
.formatted(method, methodHttpExchanges));
Assert.notNull(methodAnnotation, () -> "Expected @HttpRequest annotation on method " + method.toGenericString());
List<AnnotationDescriptor> typeHttpExchanges = getAnnotationDescriptors(containingClass);
Assert.state(typeHttpExchanges.size() <= 1,
() -> "Multiple @HttpExchange annotations found on %s, but only one is allowed: %s"
.formatted(containingClass, typeHttpExchanges));
HttpExchange methodAnnotation = methodHttpExchanges.get(0).httpExchange;
HttpExchange typeAnnotation = (!typeHttpExchanges.isEmpty() ? typeHttpExchanges.get(0).httpExchange : null);
HttpMethod httpMethod = initHttpMethod(typeAnnotation, methodAnnotation);
String url = initUrl(typeAnnotation, methodAnnotation, embeddedValueResolver);
@ -262,6 +278,43 @@ final class HttpServiceMethod { @@ -262,6 +278,43 @@ final class HttpServiceMethod {
return null;
}
private static List<AnnotationDescriptor> getAnnotationDescriptors(AnnotatedElement element) {
return MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY, RepeatableContainers.none())
.stream(HttpExchange.class)
.filter(MergedAnnotationPredicates.firstRunOf(MergedAnnotation::getAggregateIndex))
.map(AnnotationDescriptor::new)
.distinct()
.toList();
}
private static class AnnotationDescriptor {
private final HttpExchange httpExchange;
private final MergedAnnotation<?> root;
AnnotationDescriptor(MergedAnnotation<HttpExchange> mergedAnnotation) {
this.httpExchange = mergedAnnotation.synthesize();
this.root = mergedAnnotation.getRoot();
}
@Override
public boolean equals(Object obj) {
return (obj instanceof AnnotationDescriptor that && this.httpExchange.equals(that.httpExchange));
}
@Override
public int hashCode() {
return this.httpExchange.hashCode();
}
@Override
public String toString() {
return this.root.synthesize().toString();
}
}
}

60
spring-web/src/test/java/org/springframework/web/service/invoker/HttpServiceMethodTests.java

@ -16,6 +16,11 @@ @@ -16,6 +16,11 @@
package org.springframework.web.service.invoker;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Method;
import java.util.List;
import java.util.Optional;
@ -37,8 +42,10 @@ import org.springframework.lang.Nullable; @@ -37,8 +42,10 @@ import org.springframework.lang.Nullable;
import org.springframework.web.service.annotation.GetExchange;
import org.springframework.web.service.annotation.HttpExchange;
import org.springframework.web.service.annotation.PostExchange;
import org.springframework.web.service.annotation.PutExchange;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.springframework.http.MediaType.APPLICATION_CBOR_VALUE;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
@ -52,6 +59,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; @@ -52,6 +59,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
*
* @author Rossen Stoyanchev
* @author Olga Maciaszek-Sharma
* @author Sam Brannen
*/
class HttpServiceMethodTests {
@ -204,6 +212,33 @@ class HttpServiceMethodTests { @@ -204,6 +212,33 @@ class HttpServiceMethodTests {
assertThat(requestValues.getHeaders().getAccept()).containsExactly(MediaType.APPLICATION_JSON);
}
@Test // gh-32049
void multipleAnnotationsAtClassLevel() {
Class<?> serviceInterface = MultipleClassLevelAnnotationsService.class;
assertThatIllegalStateException()
.isThrownBy(() -> this.proxyFactory.createClient(serviceInterface))
.withMessageContainingAll(
"Multiple @HttpExchange annotations found on " + serviceInterface,
"@" + HttpExchange.class.getName(),
"@" + ExtraHttpExchange.class.getName()
);
}
@Test // gh-32049
void multipleAnnotationsAtMethodLevel() throws NoSuchMethodException {
Class<?> serviceInterface = MultipleMethodLevelAnnotationsService.class;
Method method = serviceInterface.getMethod("post");
assertThatIllegalStateException()
.isThrownBy(() -> this.proxyFactory.createClient(serviceInterface))
.withMessageContainingAll(
"Multiple @HttpExchange annotations found on method " + method,
"@" + PostExchange.class.getName(),
"@" + PutExchange.class.getName()
);
}
protected void verifyReactorClientInvocation(String methodName, @Nullable ParameterizedTypeReference<?> expectedBodyType) {
assertThat(this.reactorClient.getInvokedMethodName()).isEqualTo(methodName);
assertThat(this.reactorClient.getBodyType()).isEqualTo(expectedBodyType);
@ -305,9 +340,34 @@ class HttpServiceMethodTests { @@ -305,9 +340,34 @@ class HttpServiceMethodTests {
}
@SuppressWarnings("unused")
@HttpExchange(url = "${baseUrl}", contentType = APPLICATION_CBOR_VALUE, accept = APPLICATION_CBOR_VALUE)
private interface TypeAndMethodLevelAnnotatedService extends MethodLevelAnnotatedService {
}
@HttpExchange("/exchange")
@ExtraHttpExchange
private interface MultipleClassLevelAnnotationsService {
@PostExchange("/post")
void post();
}
private interface MultipleMethodLevelAnnotationsService {
@PostExchange("/post")
@PutExchange("/post")
void post();
}
@HttpExchange
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
private @interface ExtraHttpExchange {
}
}

Loading…
Cancel
Save