From a57dead4be7e19c423df867852380b38c1f5ddbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Mon, 13 Jun 2022 14:44:49 +0200 Subject: [PATCH] Add support for serialization in RequestMappingReflectiveProcessor Support reflection-based serialization of parameters annotated with @RequestBody and return values annotated with @ResponseBody. It leverages a new BindingReflectionHintsRegistrar class that is designed to register transitively the types usually needed for binding and reflection-based serialization on fields, constructors and properties. Generics are taken in account as well. Closes gh-28518 --- .../BindingReflectionHintsRegistrar.java | 114 ++++++++++++ .../BindingReflectionHintsRegistrarTests.java | 174 +++++++++++++++++ .../RequestMappingReflectiveProcessor.java | 45 ++++- ...equestMappingReflectiveProcessorTests.java | 176 ++++++++++++++++++ 4 files changed, 503 insertions(+), 6 deletions(-) create mode 100644 spring-core/src/main/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrar.java create mode 100644 spring-core/src/test/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrarTests.java create mode 100644 spring-web/src/test/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessorTests.java diff --git a/spring-core/src/main/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrar.java b/spring-core/src/main/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrar.java new file mode 100644 index 00000000000..85cd0f4698a --- /dev/null +++ b/spring-core/src/main/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrar.java @@ -0,0 +1,114 @@ +/* + * Copyright 2002-2022 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.aot.hint.support; + +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.lang.reflect.Method; +import java.lang.reflect.Type; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.function.Consumer; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.aot.hint.ExecutableHint; +import org.springframework.aot.hint.ExecutableMode; +import org.springframework.aot.hint.MemberCategory; +import org.springframework.aot.hint.ReflectionHints; +import org.springframework.core.MethodParameter; +import org.springframework.core.ResolvableType; + +/** + * Register the necessary reflection hints so that the specified type can be bound/serialized at runtime. + * Fields, constructors and property methods are registered, except for a set of types like those in + * {@code java.} package where just the type is registered. Types are discovered transitively and + * generic types are registered as well. + * + * @author Sebastien Deleuze + * @since 6.0 + */ +public class BindingReflectionHintsRegistrar { + + private static final Log logger = LogFactory.getLog(BindingReflectionHintsRegistrar.class); + + private static final Consumer INVOKE = builder -> builder + .withMode(ExecutableMode.INVOKE); + + public void registerReflectionHints(ReflectionHints hints, Type... types) { + for (Type type : types) { + Set> referencedTypes = new LinkedHashSet<>(); + collectReferencedTypes(new HashSet<>(), referencedTypes, type); + referencedTypes.forEach(referencedType -> hints.registerType(referencedType, builder -> { + if (shouldRegisterMembers(referencedType)) { + builder.withMembers(MemberCategory.DECLARED_FIELDS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS); + try { + BeanInfo beanInfo = Introspector.getBeanInfo(referencedType); + PropertyDescriptor[] propertyDescriptors = beanInfo.getPropertyDescriptors(); + for (PropertyDescriptor propertyDescriptor : propertyDescriptors) { + Method writeMethod = propertyDescriptor.getWriteMethod(); + if (writeMethod != null && writeMethod.getDeclaringClass() != Object.class) { + hints.registerMethod(writeMethod, INVOKE); + MethodParameter methodParameter = MethodParameter.forExecutable(writeMethod, 0); + registerReflectionHints(hints, methodParameter.getGenericParameterType()); + } + Method readMethod = propertyDescriptor.getReadMethod(); + if (readMethod != null && readMethod.getDeclaringClass() != Object.class) { + hints.registerMethod(readMethod, INVOKE); + MethodParameter methodParameter = MethodParameter.forExecutable(readMethod, -1); + registerReflectionHints(hints, methodParameter.getGenericParameterType()); + } + } + } + catch (IntrospectionException ex) { + if (logger.isDebugEnabled()) { + logger.debug("Ignoring referenced type [" + referencedType.getName() + "]: " + ex.getMessage()); + } + } + } + })); + } + } + + /** + * Return whether the members of the type should be registered transitively. + * @param type the type to evaluate + * @return {@code true} if the members of the type should be registered transitively + */ + protected boolean shouldRegisterMembers(Class type) { + return !type.getCanonicalName().startsWith("java."); + } + + private void collectReferencedTypes(Set seen, Set> types, Type type) { + if (seen.contains(type)) { + return; + } + seen.add(type); + ResolvableType resolvableType = ResolvableType.forType(type); + Class clazz = resolvableType.resolve(); + if (clazz != null) { + types.add(clazz); + for (ResolvableType genericResolvableType : resolvableType.getGenerics()) { + collectReferencedTypes(seen, types, genericResolvableType.getType()); + } + } + } +} diff --git a/spring-core/src/test/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrarTests.java b/spring-core/src/test/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrarTests.java new file mode 100644 index 00000000000..1e692c480bb --- /dev/null +++ b/spring-core/src/test/java/org/springframework/aot/hint/support/BindingReflectionHintsRegistrarTests.java @@ -0,0 +1,174 @@ +/* + * Copyright 2002-2022 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.aot.hint.support; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import org.springframework.aot.hint.ExecutableMode; +import org.springframework.aot.hint.MemberCategory; +import org.springframework.aot.hint.RuntimeHints; +import org.springframework.aot.hint.TypeReference; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link BindingReflectionHintsRegistrar}. + * + * @author Sebastien Deleuze + */ +public class BindingReflectionHintsRegistrarTests { + + private final BindingReflectionHintsRegistrar bindingRegistrar = new BindingReflectionHintsRegistrar(); + private final RuntimeHints hints = new RuntimeHints(); + + @Test + void registerTypeForSerializationWithEmptyClass() { + bindingRegistrar.registerReflectionHints(this.hints.reflection(), SampleEmptyClass.class); + assertThat(this.hints.reflection().typeHints()).singleElement() + .satisfies(typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleEmptyClass.class)); + assertThat(typeHint.getMemberCategories()).containsExactlyInAnyOrder( + MemberCategory.DECLARED_FIELDS, MemberCategory.INVOKE_DECLARED_CONSTRUCTORS); + assertThat(typeHint.constructors()).isEmpty(); + assertThat(typeHint.fields()).isEmpty(); + assertThat(typeHint.methods()).isEmpty(); + }); + } + + @Test + void registerTypeForSerializationWithNoProperty() { + bindingRegistrar.registerReflectionHints(this.hints.reflection(), SampleClassWithNoProperty.class); + assertThat(this.hints.reflection().typeHints()).singleElement() + .satisfies(typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleClassWithNoProperty.class))); + } + + @Test + void registerTypeForSerializationWithGetter() { + bindingRegistrar.registerReflectionHints(this.hints.reflection(), SampleClassWithGetter.class); + assertThat(this.hints.reflection().typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class)); + assertThat(typeHint.getMemberCategories()).isEmpty(); + assertThat(typeHint.constructors()).isEmpty(); + assertThat(typeHint.fields()).isEmpty(); + assertThat(typeHint.methods()).isEmpty(); + }, + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleClassWithGetter.class)); + assertThat(typeHint.methods()).singleElement().satisfies(methodHint -> { + assertThat(methodHint.getName()).isEqualTo("getName"); + assertThat(methodHint.getModes()).containsOnly(ExecutableMode.INVOKE); + }); + }); + } + + @Test + void registerTypeForSerializationWithSetter() { + bindingRegistrar.registerReflectionHints(this.hints.reflection(), SampleClassWithSetter.class); + assertThat(this.hints.reflection().typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class)); + assertThat(typeHint.getMemberCategories()).isEmpty(); + assertThat(typeHint.constructors()).isEmpty(); + assertThat(typeHint.fields()).isEmpty(); + assertThat(typeHint.methods()).isEmpty(); + }, + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleClassWithSetter.class)); + assertThat(typeHint.methods()).singleElement().satisfies(methodHint -> { + assertThat(methodHint.getName()).isEqualTo("setName"); + assertThat(methodHint.getModes()).containsOnly(ExecutableMode.INVOKE); + }); + }); + } + + @Test + void registerTypeForSerializationWithListProperty() { + bindingRegistrar.registerReflectionHints(this.hints.reflection(), SampleClassWithListProperty.class); + assertThat(this.hints.reflection().typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class)); + assertThat(typeHint.getMemberCategories()).isEmpty(); + assertThat(typeHint.constructors()).isEmpty(); + assertThat(typeHint.fields()).isEmpty(); + assertThat(typeHint.methods()).isEmpty(); + }, + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(List.class)); + assertThat(typeHint.getMemberCategories()).isEmpty(); + assertThat(typeHint.constructors()).isEmpty(); + assertThat(typeHint.fields()).isEmpty(); + assertThat(typeHint.methods()).isEmpty(); + }, + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleClassWithListProperty.class)); + assertThat(typeHint.methods()).satisfiesExactlyInAnyOrder( + methodHint -> { + assertThat(methodHint.getName()).isEqualTo("setNames"); + assertThat(methodHint.getModes()).containsOnly(ExecutableMode.INVOKE); + }, + methodHint -> { + assertThat(methodHint.getName()).isEqualTo("getNames"); + assertThat(methodHint.getModes()).containsOnly(ExecutableMode.INVOKE); + }); + }); + } + + static class SampleEmptyClass { + } + + static class SampleClassWithNoProperty { + + String name() { + return null; + } + } + + static class SampleClassWithGetter { + + public String getName() { + return null; + } + + public SampleEmptyClass unmanaged() { + return null; + } + } + + static class SampleClassWithSetter { + + public void setName(String name) { + } + + public SampleEmptyClass unmanaged() { + return null; + } + } + + static class SampleClassWithListProperty { + + public List getNames() { + return null; + } + + public void setNames(List names) { + } + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessor.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessor.java index 9f652769788..da62dd77cb3 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessor.java @@ -16,26 +16,59 @@ package org.springframework.web.bind.annotation; +import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import org.springframework.aot.hint.ExecutableMode; import org.springframework.aot.hint.ReflectionHints; import org.springframework.aot.hint.annotation.ReflectiveProcessor; -import org.springframework.aot.hint.annotation.SimpleReflectiveProcessor; +import org.springframework.aot.hint.support.BindingReflectionHintsRegistrar; +import org.springframework.core.MethodParameter; +import org.springframework.core.annotation.AnnotatedElementUtils; /** * {@link ReflectiveProcessor} implementation for {@link RequestMapping} * annotated types. On top of registering reflection hints for invoking - * the annotated method, this implementation handles return types that - * are serialized as well as TBD. + * the annotated method, this implementation handles return types annotated + * with {@link ResponseBody} and parameters annotated with {@link RequestBody} + * which are serialized as well. + * * * @author Stephane Nicoll + * @author Sebastien Deleuze * @since 6.0 */ -class RequestMappingReflectiveProcessor extends SimpleReflectiveProcessor { +class RequestMappingReflectiveProcessor implements ReflectiveProcessor { + + private BindingReflectionHintsRegistrar bindingRegistrar = new BindingReflectionHintsRegistrar(); @Override + public void registerReflectionHints(ReflectionHints hints, AnnotatedElement element) { + if (element instanceof Class type) { + registerTypeHint(hints, type); + } + else if (element instanceof Method method) { + registerMethodHint(hints, method); + } + } + + protected void registerTypeHint(ReflectionHints hints, Class type) { + hints.registerType(type, hint -> {}); + } + protected void registerMethodHint(ReflectionHints hints, Method method) { - super.registerMethodHint(hints, method); - // TODO + hints.registerMethod(method, hint -> hint.setModes(ExecutableMode.INVOKE)); + for (Parameter parameter : method.getParameters()) { + MethodParameter methodParameter = MethodParameter.forParameter(parameter); + if (methodParameter.hasParameterAnnotation(RequestBody.class)) { + this.bindingRegistrar.registerReflectionHints(hints, methodParameter.getGenericParameterType()); + } + } + MethodParameter returnType = MethodParameter.forExecutable(method, -1); + if (AnnotatedElementUtils.hasAnnotation(returnType.getContainingClass(), ResponseBody.class) || + returnType.hasMethodAnnotation(ResponseBody.class)) { + this.bindingRegistrar.registerReflectionHints(hints, returnType.getGenericParameterType()); + } } } diff --git a/spring-web/src/test/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessorTests.java b/spring-web/src/test/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessorTests.java new file mode 100644 index 00000000000..43c4a1f5bdb --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/bind/annotation/RequestMappingReflectiveProcessorTests.java @@ -0,0 +1,176 @@ +/* + * Copyright 2002-2022 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.web.bind.annotation; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.aot.hint.MemberCategory; +import org.springframework.aot.hint.ReflectionHints; +import org.springframework.aot.hint.TypeReference; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link RequestMappingReflectiveProcessor}. + * + * @author Sebastien Deleuze + */ +public class RequestMappingReflectiveProcessorTests { + + private final RequestMappingReflectiveProcessor processor = new RequestMappingReflectiveProcessor(); + + private final ReflectionHints hints = new ReflectionHints(); + + @Test + void registerReflectiveHintsForMethodWithResponseBody() throws NoSuchMethodException { + Method method = SampleController.class.getDeclaredMethod("get"); + processor.registerReflectionHints(hints, method); + assertThat(hints.typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleController.class)), + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(Response.class)); + assertThat(typeHint.getMemberCategories()).containsExactlyInAnyOrder( + MemberCategory.INVOKE_DECLARED_CONSTRUCTORS, + MemberCategory.DECLARED_FIELDS); + assertThat(typeHint.methods()).satisfiesExactlyInAnyOrder( + hint -> assertThat(hint.getName()).isEqualTo("getMessage"), + hint -> assertThat(hint.getName()).isEqualTo("setMessage")); + }, + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class))); + } + + @Test + void registerReflectiveHintsForMethodWithRequestBody() throws NoSuchMethodException { + Method method = SampleController.class.getDeclaredMethod("post", Request.class); + processor.registerReflectionHints(hints, method); + assertThat(hints.typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleController.class)), + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(Request.class)); + assertThat(typeHint.getMemberCategories()).containsExactlyInAnyOrder( + MemberCategory.INVOKE_DECLARED_CONSTRUCTORS, + MemberCategory.DECLARED_FIELDS); + assertThat(typeHint.methods()).satisfiesExactlyInAnyOrder( + hint -> assertThat(hint.getName()).isEqualTo("getMessage"), + hint -> assertThat(hint.getName()).isEqualTo("setMessage")); + }, + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class))); + } + + @Test + void registerReflectiveHintsForMethodWithRestController() throws NoSuchMethodException { + Method method = SampleRestController.class.getDeclaredMethod("get"); + processor.registerReflectionHints(hints, method); + assertThat(hints.typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleRestController.class)), + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(Response.class)); + assertThat(typeHint.getMemberCategories()).containsExactlyInAnyOrder( + MemberCategory.INVOKE_DECLARED_CONSTRUCTORS, + MemberCategory.DECLARED_FIELDS); + assertThat(typeHint.methods()).satisfiesExactlyInAnyOrder( + hint -> assertThat(hint.getName()).isEqualTo("getMessage"), + hint -> assertThat(hint.getName()).isEqualTo("setMessage")); + }, + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class))); + } + + @Test + void registerReflectiveHintsForMethodWithString() throws NoSuchMethodException { + Method method = SampleController.class.getDeclaredMethod("message"); + processor.registerReflectionHints(hints, method); + assertThat(hints.typeHints()).satisfiesExactlyInAnyOrder( + typeHint -> assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleController.class)), + typeHint -> { + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(String.class)); + assertThat(typeHint.constructors()).isEmpty(); + assertThat(typeHint.fields()).isEmpty(); + assertThat(typeHint.methods()).isEmpty(); + }); + } + + @Test + void registerReflectiveHintsForClassWithMapping() { + processor.registerReflectionHints(hints, SampleControllerWithClassMapping.class); + assertThat(hints.typeHints()).singleElement().satisfies(typeHint -> + assertThat(typeHint.getType()).isEqualTo(TypeReference.of(SampleControllerWithClassMapping.class))); + } + + static class SampleController { + + @GetMapping + @ResponseBody + Response get() { + return new Response("response"); + } + + @PostMapping + void post(@RequestBody Request request) { + } + + @GetMapping + @ResponseBody + String message() { + return ""; + } + } + + @RestController + static class SampleRestController { + + @GetMapping + Response get() { + return new Response("response"); + } + } + + @RequestMapping("/prefix") + static class SampleControllerWithClassMapping { + } + + static class Request { + + private String message; + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } + } + + static class Response { + + private String message; + + public Response(String message) { + this.message = message; + } + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } + } +}