Skip to content

Commit

Permalink
Support records for @RequestBean (#8420)
Browse files Browse the repository at this point in the history
  • Loading branch information
yawkat committed Dec 2, 2022
1 parent 54ffd2f commit 4fddaa9
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 71 deletions.
Expand Up @@ -15,20 +15,24 @@
*/
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;
import java.util.Arrays;
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.
Expand All @@ -48,16 +52,16 @@ public RouteValidationResult validate(List<UriMatchTemplate> 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()));

Expand All @@ -72,4 +76,13 @@ public RouteValidationResult validate(List<UriMatchTemplate> templates, Paramete
return new RouteValidationResult(errorMessages.toArray(new String[0]));
}

private static Stream<? extends AnnotatedElement> findProperties(ClassElement t) {
if (t.isRecord()) {
Optional<MethodElement> primaryConstructor = t.getPrimaryConstructor();
if (primaryConstructor.isPresent()) {
return Arrays.stream(primaryConstructor.get().getParameters());
}
}
return t.getBeanProperties().stream();
}
}
Expand Up @@ -52,15 +52,17 @@ private List<String> validate(ParameterElement parameterElement) {
// @Creator constructor
List<ParameterElement> 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()
Expand Down
@@ -1,6 +1,7 @@
package io.micronaut.validation.routes

import io.micronaut.annotation.processing.test.AbstractTypeElementSpec
import spock.lang.IgnoreIf

class RequestBeanParameterRuleSpec extends AbstractTypeElementSpec {

Expand All @@ -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; }
}
}
""")
Expand All @@ -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; }
}
}
""")
Expand All @@ -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) {
}
}
""")
Expand All @@ -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; }
}
}
""")
Expand All @@ -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; }
}
}
""")
Expand All @@ -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; }
}
}
""")
Expand All @@ -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; }
}
}
""")
Expand All @@ -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; }
}
}
""")
Expand Down

0 comments on commit 4fddaa9

Please sign in to comment.