From 4fddaa94c05093c171fd88dfc28fa311d6ba9e39 Mon Sep 17 00:00:00 2001 From: Jonas Konrad Date: Fri, 2 Dec 2022 12:06:46 +0100 Subject: [PATCH] Support records for `@RequestBean` (#8420) --- .../routes/rules/MissingParameterRule.java | 23 ++- .../rules/RequestBeanParameterRule.java | 16 +- .../RequestBeanParameterRuleSpec.groovy | 143 +++++++++++------- .../test/AbstractTypeElementSpec.groovy | 16 +- 4 files changed, 127 insertions(+), 71 deletions(-) diff --git a/http-validation/src/main/java/io/micronaut/validation/routes/rules/MissingParameterRule.java b/http-validation/src/main/java/io/micronaut/validation/routes/rules/MissingParameterRule.java index 313dab5a412..02953bd2918 100644 --- a/http-validation/src/main/java/io/micronaut/validation/routes/rules/MissingParameterRule.java +++ b/http-validation/src/main/java/io/micronaut/validation/routes/rules/MissingParameterRule.java @@ -15,11 +15,13 @@ */ package io.micronaut.validation.routes.rules; +import io.micronaut.core.annotation.AnnotatedElement; import io.micronaut.core.bind.annotation.Bindable; +import io.micronaut.core.naming.Named; import io.micronaut.http.uri.UriMatchTemplate; +import io.micronaut.inject.ast.ClassElement; import io.micronaut.inject.ast.MethodElement; import io.micronaut.inject.ast.ParameterElement; -import io.micronaut.inject.ast.PropertyElement; import io.micronaut.validation.routes.RouteValidationResult; import java.util.ArrayList; @@ -27,8 +29,10 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Validates all route uri variables are present in the route arguments. @@ -48,16 +52,16 @@ public RouteValidationResult validate(List templates, Paramete .filter(p -> p.hasAnnotation("io.micronaut.http.annotation.Body")) .map(ParameterElement::getType) .filter(Objects::nonNull) - .flatMap(t -> t.getBeanProperties().stream()) - .map(PropertyElement::getName) + .flatMap(MissingParameterRule::findProperties) + .map(Named::getName) .collect(Collectors.toList())); // RequestBean has properties inside routeVariables.addAll(Arrays.stream(parameters) .filter(p -> p.hasAnnotation("io.micronaut.http.annotation.RequestBean")) .map(ParameterElement::getType) - .flatMap(t -> t.getBeanProperties().stream()) - .filter(p -> p.hasStereotype(Bindable.class)) + .flatMap(MissingParameterRule::findProperties) + .filter(p -> p.getAnnotationMetadata().hasStereotype(Bindable.class)) .map(p -> p.getAnnotationMetadata().stringValue(Bindable.class).orElse(p.getName())) .collect(Collectors.toSet())); @@ -72,4 +76,13 @@ public RouteValidationResult validate(List templates, Paramete return new RouteValidationResult(errorMessages.toArray(new String[0])); } + private static Stream findProperties(ClassElement t) { + if (t.isRecord()) { + Optional primaryConstructor = t.getPrimaryConstructor(); + if (primaryConstructor.isPresent()) { + return Arrays.stream(primaryConstructor.get().getParameters()); + } + } + return t.getBeanProperties().stream(); + } } diff --git a/http-validation/src/main/java/io/micronaut/validation/routes/rules/RequestBeanParameterRule.java b/http-validation/src/main/java/io/micronaut/validation/routes/rules/RequestBeanParameterRule.java index aac4a23bd21..5aa576e9226 100644 --- a/http-validation/src/main/java/io/micronaut/validation/routes/rules/RequestBeanParameterRule.java +++ b/http-validation/src/main/java/io/micronaut/validation/routes/rules/RequestBeanParameterRule.java @@ -52,15 +52,17 @@ private List validate(ParameterElement parameterElement) { // @Creator constructor List constructorParameters = Arrays.asList(primaryConstructor.get().getParameters()); - // Check no constructor parameter has any @Bindable annotation - // We could allow this, but this would add some complexity, some annotations that can be used in combination - // with @Bindable works only on fields (e.g. bean validation annotations) and this might confuse Micronaut users - constructorParameters.stream() + if (!parameterElement.getType().isRecord()) { + // Check no constructor parameter has any @Bindable annotation + // We could allow this, but this would add some complexity, some annotations that can be used in combination + // with @Bindable works only on fields (e.g. bean validation annotations) and this might confuse Micronaut users + constructorParameters.stream() .filter(p -> p.hasStereotype(Bindable.class)) .forEach(p -> errors.add("Parameter of Primary Constructor (or @Creator Method) [" + p.getName() + "] for type [" - + parameterElement.getType().getName() + "] has one of @Bindable annotations. This is not supported." - + "\nNote1: Primary constructor is a constructor that have parameters or is annotated with @Creator." - + "\nNote2: In case you have multiple @Creator constructors, first is used as primary constructor.")); + + parameterElement.getType().getName() + "] has one of @Bindable annotations. This is not supported." + + "\nNote1: Primary constructor is a constructor that have parameters or is annotated with @Creator." + + "\nNote2: In case you have multiple @Creator constructors, first is used as primary constructor.")); + } // Check readonly bindable properties can be set via constructor beanProperties.stream() diff --git a/http-validation/src/test/groovy/io/micronaut/validation/routes/RequestBeanParameterRuleSpec.groovy b/http-validation/src/test/groovy/io/micronaut/validation/routes/RequestBeanParameterRuleSpec.groovy index fcb126bbfe6..11a988792a2 100644 --- a/http-validation/src/test/groovy/io/micronaut/validation/routes/RequestBeanParameterRuleSpec.groovy +++ b/http-validation/src/test/groovy/io/micronaut/validation/routes/RequestBeanParameterRuleSpec.groovy @@ -1,6 +1,7 @@ package io.micronaut.validation.routes import io.micronaut.annotation.processing.test.AbstractTypeElementSpec +import spock.lang.IgnoreIf class RequestBeanParameterRuleSpec extends AbstractTypeElementSpec { @@ -21,22 +22,22 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected private static class Bean { - + @Nullable @QueryValue private final String abc; - + public Bean(String abc) { this.abc = abc; } - + public String getAbc() { return abc; } - + } - + } """) @@ -61,35 +62,35 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected private static class Bean { - + @Nullable @QueryValue private final String abc; - + @Nullable @QueryValue private final String def; - + public Bean(String abc) { this.abc = abc; this.def = null; } - - @Creator + + @Creator public Bean(String abc, String def) { this.abc = abc; this.def = def; } - + public String getAbc() { return abc; } - + public String getDef() { return def; } - + } - + } """) @@ -114,25 +115,55 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected private static class Bean { - + @Nullable @QueryValue private String abc; - + @Creator public static Bean of(String abc) { Bean bean = new Bean(); bean.abc = abc; return bean; } - + public String getAbc() { return abc; } - + + } + +} + +""") + then: + noExceptionThrown() + } + + @IgnoreIf({ !jvm.isJava14Compatible() }) + void "test RequestBean compiles with record"() { + when: + buildTypeElement(""" + +package test; + +import io.micronaut.http.annotation.*; +import io.micronaut.core.annotation.*; +import io.micronaut.core.annotation.Nullable; + +@Controller("/foo") +class Foo { + + @Get("/abc/{abc}") + String abc(@RequestBean Bean bean) { + return ""; + } + + @Introspected + public record Bean(@Nullable @PathVariable String abc) { } - + } """) @@ -157,20 +188,20 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected private static class Bean { - + @Nullable @QueryValue private String abc; - + public String getAbc() { return abc; } - + public void setAbc(String abc) { this.abc = abc; } - + } - + } """) @@ -195,18 +226,18 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected public static class Bean { - + @Nullable @QueryValue private String abc; - + public String getAbc() { return abc; } - + } - + } """) @@ -232,28 +263,28 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected public static class Bean { - + @Nullable @QueryValue private String abc; - + @Nullable @QueryValue private String def; - + public Bean(String def) { this.def = def; } - + public String getAbc() { return abc; } - + public String getDef() { return def; } - + } - + } """) @@ -279,23 +310,23 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected public static class Bean { - + @Nullable @QueryValue private String abc; - + @Creator public Bean(@Nullable @QueryValue String abc) { this.abc = abc; } - + public String getAbc() { return abc; } - + } - + } """) @@ -321,31 +352,31 @@ class Foo { String abc(@RequestBean Bean bean) { return ""; } - + @Introspected public static class Bean { - + @Nullable @QueryValue private String abc; - + @Nullable @QueryValue private String def; - + @Creator public Bean(String def) { this.def = def; } - + public String getAbc() { return abc; } - + public void setAbc() { this.abc = abc; } - + public String getDef() { return def; } - + } - + } """) diff --git a/inject-java-test/src/main/groovy/io/micronaut/annotation/processing/test/AbstractTypeElementSpec.groovy b/inject-java-test/src/main/groovy/io/micronaut/annotation/processing/test/AbstractTypeElementSpec.groovy index 0430afc2963..9ec05b0a71b 100644 --- a/inject-java-test/src/main/groovy/io/micronaut/annotation/processing/test/AbstractTypeElementSpec.groovy +++ b/inject-java-test/src/main/groovy/io/micronaut/annotation/processing/test/AbstractTypeElementSpec.groovy @@ -17,11 +17,20 @@ package io.micronaut.annotation.processing.test import com.sun.source.util.JavacTask import groovy.transform.CompileStatic -import io.micronaut.annotation.processing.* +import io.micronaut.annotation.processing.AggregatingTypeElementVisitorProcessor +import io.micronaut.annotation.processing.AnnotationUtils +import io.micronaut.annotation.processing.GenericUtils +import io.micronaut.annotation.processing.JavaAnnotationMetadataBuilder +import io.micronaut.annotation.processing.ModelUtils +import io.micronaut.annotation.processing.TypeElementVisitorProcessor import io.micronaut.annotation.processing.visitor.JavaElementFactory import io.micronaut.annotation.processing.visitor.JavaVisitorContext import io.micronaut.aop.internal.InterceptorRegistryBean -import io.micronaut.context.* +import io.micronaut.context.ApplicationContext +import io.micronaut.context.ApplicationContextBuilder +import io.micronaut.context.ApplicationContextConfiguration +import io.micronaut.context.DefaultApplicationContext +import io.micronaut.context.Qualifier import io.micronaut.context.event.ApplicationEventPublisherFactory import io.micronaut.core.annotation.AnnotationMetadata import io.micronaut.core.annotation.Experimental @@ -59,6 +68,7 @@ import javax.tools.JavaFileObject import java.lang.annotation.Annotation import java.util.stream.Collectors import java.util.stream.StreamSupport + /** * Base class to extend from to allow compilation of Java sources * at runtime to allow testing of compile time behavior. @@ -338,7 +348,7 @@ class Test { return metadata } - protected TypeElement buildTypeElement(String cls) { + protected TypeElement buildTypeElement(@Language('java') String cls) { List elements = [] newJavaParser().parseLines("",