Revert "Only support @OptionalParameter annotation with endpoint methods"

This reverts commit 450eb48303.

See gh-47136
This commit is contained in:
Stéphane Nicoll
2025-09-10 14:31:35 +02:00
parent d0c742bc7b
commit ee83ff5f6a
12 changed files with 143 additions and 86 deletions
@@ -107,9 +107,9 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
static final String WEB_ENDPOINT_ANNOTATION = "org.springframework.boot.actuate.endpoint.web.annotation.WebEndpoint";
static final String ENDPOINT_READ_OPERATION_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.ReadOperation";
static final String READ_OPERATION_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.ReadOperation";
static final String ENDPOINT_OPTIONAL_PARAMETER_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.OptionalParameter";
static final String OPTIONAL_PARAMETER_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.OptionalParameter";
static final String NAME_ANNOTATION = "org.springframework.boot.context.properties.bind.Name";
@@ -158,16 +158,16 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
REST_CONTROLLER_ENDPOINT_ANNOTATION, SERVLET_ENDPOINT_ANNOTATION, WEB_ENDPOINT_ANNOTATION);
}
protected String endpointReadOperationAnnotation() {
return ENDPOINT_READ_OPERATION_ANNOTATION;
protected String readOperationAnnotation() {
return READ_OPERATION_ANNOTATION;
}
protected String nameAnnotation() {
return NAME_ANNOTATION;
}
protected String endpointOptionalParameterAnnotation() {
return ENDPOINT_OPTIONAL_PARAMETER_ANNOTATION;
protected String optionalParameterAnnotation() {
return OPTIONAL_PARAMETER_ANNOTATION;
}
protected String endpointAccessEnum() {
@@ -194,8 +194,8 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
this.metadataEnv = new MetadataGenerationEnvironment(env, configurationPropertiesAnnotation(),
configurationPropertiesSourceAnnotation(), nestedConfigurationPropertyAnnotation(),
deprecatedConfigurationPropertyAnnotation(), constructorBindingAnnotation(), autowiredAnnotation(),
defaultValueAnnotation(), endpointAnnotations(), endpointReadOperationAnnotation(),
endpointOptionalParameterAnnotation(), nameAnnotation());
defaultValueAnnotation(), endpointAnnotations(), readOperationAnnotation(),
optionalParameterAnnotation(), nameAnnotation());
}
@Override
@@ -271,7 +271,8 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
}
private void processExecutableElement(String prefix, ExecutableElement element, Deque<TypeElement> seen) {
if ((!element.getModifiers().contains(Modifier.PRIVATE)) && returnsVoid(element)) {
if ((!element.getModifiers().contains(Modifier.PRIVATE))
&& (TypeKind.VOID != element.getReturnType().getKind())) {
Element returns = this.processingEnv.getTypeUtils().asElement(element.getReturnType());
if (returns instanceof TypeElement typeElement) {
ItemMetadata group = ItemMetadata.newGroup(prefix,
@@ -353,7 +354,7 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
"Permitted level of access for the %s endpoint.".formatted(endpointId), defaultAccess, null);
this.metadataCollector.add(accessProperty,
(existing) -> checkDefaultAccessValueMatchesExisting(existing, defaultAccess, type));
if (isCachableEndpoint(element)) {
if (hasMainReadOperation(element)) {
this.metadataCollector.addIfAbsent(ItemMetadata.newProperty(endpointKey, "cache.time-to-live",
Duration.class.getName(), type, null, "Maximum time that a response can be cached.", "0ms", null));
}
@@ -370,27 +371,28 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
}
}
private boolean isCachableEndpoint(TypeElement element) {
private boolean hasMainReadOperation(TypeElement element) {
for (ExecutableElement method : ElementFilter.methodsIn(element.getEnclosedElements())) {
if (this.metadataEnv.isEndpointReadOperation(method) && returnsVoid(method)
&& !hasMandatoryEndpointParameter(method)) {
if (this.metadataEnv.getReadOperationAnnotation(method) != null
&& (TypeKind.VOID != method.getReturnType().getKind()) && hasNoOrOptionalParameters(method)) {
return true;
}
}
return false;
}
private boolean returnsVoid(ExecutableElement method) {
return TypeKind.VOID != method.getReturnType().getKind();
}
private boolean hasMandatoryEndpointParameter(ExecutableElement method) {
private boolean hasNoOrOptionalParameters(ExecutableElement method) {
for (VariableElement parameter : method.getParameters()) {
if (!this.metadataEnv.hasEndpointOptionalParameterAnnotation(parameter)) {
return true;
if (!isOptionalParameter(parameter)) {
return false;
}
}
return false;
return true;
}
private boolean isOptionalParameter(VariableElement parameter) {
return this.metadataEnv.hasNullableAnnotation(parameter)
|| this.metadataEnv.hasOptionalParameterAnnotation(parameter);
}
private String getPrefix(AnnotationMirror annotation) {
@@ -96,9 +96,9 @@ class MetadataGenerationEnvironment {
private final Set<String> endpointAnnotations;
private final String endpointReadOperationAnnotation;
private final String readOperationAnnotation;
private final String endpointOptionalParameterAnnotation;
private final String optionalParameterAnnotation;
private final String nameAnnotation;
@@ -108,7 +108,7 @@ class MetadataGenerationEnvironment {
String configurationPropertiesSourceAnnotation, String nestedConfigurationPropertyAnnotation,
String deprecatedConfigurationPropertyAnnotation, String constructorBindingAnnotation,
String autowiredAnnotation, String defaultValueAnnotation, Set<String> endpointAnnotations,
String endpointReadOperationAnnotation, String endpointOptionalParameterAnnotation, String nameAnnotation) {
String readOperationAnnotation, String optionalParameterAnnotation, String nameAnnotation) {
this.typeUtils = new TypeUtils(environment);
this.elements = environment.getElementUtils();
this.messager = environment.getMessager();
@@ -122,8 +122,8 @@ class MetadataGenerationEnvironment {
this.autowiredAnnotation = autowiredAnnotation;
this.defaultValueAnnotation = defaultValueAnnotation;
this.endpointAnnotations = endpointAnnotations;
this.endpointReadOperationAnnotation = endpointReadOperationAnnotation;
this.endpointOptionalParameterAnnotation = endpointOptionalParameterAnnotation;
this.readOperationAnnotation = readOperationAnnotation;
this.optionalParameterAnnotation = optionalParameterAnnotation;
this.nameAnnotation = nameAnnotation;
}
@@ -370,8 +370,8 @@ class MetadataGenerationEnvironment {
.collect(Collectors.toSet());
}
boolean isEndpointReadOperation(Element element) {
return getAnnotation(element, this.endpointReadOperationAnnotation) != null;
AnnotationMirror getReadOperationAnnotation(Element element) {
return getAnnotation(element, this.readOperationAnnotation);
}
AnnotationMirror getNameAnnotation(Element element) {
@@ -382,8 +382,8 @@ class MetadataGenerationEnvironment {
return getTypeUseAnnotation(element, NULLABLE_ANNOTATION) != null;
}
boolean hasEndpointOptionalParameterAnnotation(Element element) {
return getAnnotation(element, this.endpointOptionalParameterAnnotation) != null;
boolean hasOptionalParameterAnnotation(Element element) {
return getAnnotation(element, this.optionalParameterAnnotation) != null;
}
private boolean isElementDeprecated(Element element) {
@@ -28,6 +28,7 @@ import org.springframework.boot.configurationsample.endpoint.CamelCaseEndpoint;
import org.springframework.boot.configurationsample.endpoint.CustomPropertiesEndpoint;
import org.springframework.boot.configurationsample.endpoint.EnabledEndpoint;
import org.springframework.boot.configurationsample.endpoint.NoAccessEndpoint;
import org.springframework.boot.configurationsample.endpoint.NullableParameterEndpoint;
import org.springframework.boot.configurationsample.endpoint.OptionalParameterEndpoint;
import org.springframework.boot.configurationsample.endpoint.ReadOnlyAccessEndpoint;
import org.springframework.boot.configurationsample.endpoint.SimpleEndpoint;
@@ -194,6 +195,16 @@ class EndpointMetadataGenerationTests extends AbstractMetadataGenerationTests {
"Existing property 'management.endpoint.simple.access' from type org.springframework.boot.configurationsample.endpoint.SimpleEndpoint has a conflicting value. Existing value: unrestricted, new value from type org.springframework.boot.configurationsample.endpoint.SimpleEndpoint3: none");
}
@Test
void endpointWithNullableParameter() {
ConfigurationMetadata metadata = compile(NullableParameterEndpoint.class);
assertThat(metadata)
.has(Metadata.withGroup("management.endpoint.nullable").fromSource(NullableParameterEndpoint.class));
assertThat(metadata).has(access("nullable", Access.UNRESTRICTED));
assertThat(metadata).has(cacheTtl("nullable"));
assertThat(metadata.getItems()).hasSize(3);
}
@Test
void endpointWithOptionalParameter() {
ConfigurationMetadata metadata = compile(OptionalParameterEndpoint.class);
@@ -126,12 +126,12 @@ public class TestConfigurationMetadataAnnotationProcessor extends ConfigurationM
}
@Override
protected String endpointReadOperationAnnotation() {
protected String readOperationAnnotation() {
return READ_OPERATION_ANNOTATION;
}
@Override
protected String endpointOptionalParameterAnnotation() {
protected String optionalParameterAnnotation() {
return OPTIONAL_PARAMETER_ANNOTATION;
}
@@ -0,0 +1,37 @@
/*
* Copyright 2012-present 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.boot.configurationsample.endpoint;
import org.jspecify.annotations.Nullable;
import org.springframework.boot.configurationsample.Endpoint;
import org.springframework.boot.configurationsample.ReadOperation;
/**
* An endpoint that uses {@code Nullable} to signal an optional parameter.
*
* @author Wonyong Hwang
*/
@Endpoint(id = "nullable")
public class NullableParameterEndpoint {
@ReadOperation
public String invoke(@Nullable String parameter) {
return "test with " + parameter;
}
}
@@ -34,7 +34,7 @@ import org.springframework.util.Assert;
* @since 2.0.0
* @see ReflectiveOperationInvoker
*/
public abstract class OperationMethod {
public class OperationMethod {
private static final ParameterNameDiscoverer DEFAULT_PARAMETER_NAME_DISCOVERER = new DefaultParameterNameDiscoverer();
@@ -44,6 +44,18 @@ public abstract class OperationMethod {
private final OperationParameters operationParameters;
/**
* Create a new {@link OperationMethod} instance.
* @param method the source method
* @param operationType the operation type
* @deprecated since 4.0.0 for removal in 4.2.0 in favor of
* {@link #OperationMethod(Method, OperationType, Predicate)}
*/
@Deprecated(since = "4.0.0", forRemoval = true)
public OperationMethod(Method method, OperationType operationType) {
this(method, operationType, (parameter) -> false);
}
/**
* Create a new {@link OperationMethod} instance.
* @param method the source method
@@ -51,7 +63,7 @@ public abstract class OperationMethod {
* @param optionalParameters predicate to test if a parameter is optional
* @since 4.0.0
*/
protected OperationMethod(Method method, OperationType operationType, Predicate<Parameter> optionalParameters) {
public OperationMethod(Method method, OperationType operationType, Predicate<Parameter> optionalParameters) {
Assert.notNull(method, "'method' must not be null");
Assert.notNull(operationType, "'operationType' must not be null");
this.method = method;
@@ -21,6 +21,7 @@ import java.lang.reflect.Parameter;
import java.util.function.Predicate;
import org.springframework.boot.actuate.endpoint.invoke.OperationParameter;
import org.springframework.core.Nullness;
/**
* {@link OperationParameter} created from an {@link OperationMethod}.
@@ -60,7 +61,11 @@ class OperationMethodParameter implements OperationParameter {
@Override
public boolean isMandatory() {
return !this.optional.test(this.parameter);
return !isOptional();
}
private boolean isOptional() {
return Nullness.NULLABLE == Nullness.forParameter(this.parameter) || this.optional.test(this.parameter);
}
@Override
@@ -43,6 +43,12 @@ class OperationMethodParameterTests {
private final Method example = ReflectionUtils.findMethod(getClass(), "example", String.class, String.class);
private final Method exampleJSpecifyNullable = ReflectionUtils.findMethod(getClass(), "exampleJSpecifyNullable",
String.class, String.class);
private final Method exampleSpringNullable = ReflectionUtils.findMethod(getClass(), "exampleSpringNullable",
String.class, String.class);
private Method exampleAnnotation = ReflectionUtils.findMethod(getClass(), "exampleAnnotation", String.class);
@Test
@@ -73,6 +79,20 @@ class OperationMethodParameterTests {
assertThat(parameter.isMandatory()).isFalse();
}
@Test
void isMandatoryWhenJSpecifyNullableAnnotationShouldReturnFalse() {
OperationMethodParameter parameter = new OperationMethodParameter("name",
this.exampleJSpecifyNullable.getParameters()[1], this::isOptionalParameter);
assertThat(parameter.isMandatory()).isFalse();
}
@Test
void isMandatoryWhenSpringNullableAnnotationShouldReturnFalse() {
OperationMethodParameter parameter = new OperationMethodParameter("name",
this.exampleSpringNullable.getParameters()[1], this::isOptionalParameter);
assertThat(parameter.isMandatory()).isFalse();
}
@Test
void getAnnotationShouldReturnAnnotation() {
OperationMethodParameter parameter = new OperationMethodParameter("name",
@@ -89,6 +109,13 @@ class OperationMethodParameterTests {
void example(String one, @TestOptional String two) {
}
void exampleJSpecifyNullable(String one, @org.jspecify.annotations.Nullable String two) {
}
@SuppressWarnings("deprecation")
void exampleSpringNullable(String one, @org.springframework.lang.Nullable String two) {
}
void exampleAnnotation(@Selector(match = Match.ALL_REMAINING) String allRemaining) {
}
@@ -17,6 +17,8 @@
package org.springframework.boot.actuate.endpoint.invoke.reflect;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.function.Predicate;
import org.junit.jupiter.api.Test;
@@ -34,35 +36,39 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
*/
class OperationMethodTests {
private static final Predicate<Parameter> NON_OPTIONAL = (parameter) -> false;
private final Method exampleMethod = ReflectionUtils.findMethod(getClass(), "example", String.class);
@Test
void createWhenMethodIsNullShouldThrowException() {
assertThatIllegalArgumentException().isThrownBy(() -> new TestOperationMethod(null, OperationType.READ))
assertThatIllegalArgumentException()
.isThrownBy(() -> new OperationMethod(null, OperationType.READ, NON_OPTIONAL))
.withMessageContaining("'method' must not be null");
}
@Test
void createWhenOperationTypeIsNullShouldThrowException() {
assertThatIllegalArgumentException().isThrownBy(() -> new TestOperationMethod(this.exampleMethod, null))
assertThatIllegalArgumentException()
.isThrownBy(() -> new OperationMethod(this.exampleMethod, null, NON_OPTIONAL))
.withMessageContaining("'operationType' must not be null");
}
@Test
void getMethodShouldReturnMethod() {
OperationMethod operationMethod = new TestOperationMethod(this.exampleMethod, OperationType.READ);
OperationMethod operationMethod = new OperationMethod(this.exampleMethod, OperationType.READ, NON_OPTIONAL);
assertThat(operationMethod.getMethod()).isEqualTo(this.exampleMethod);
}
@Test
void getOperationTypeShouldReturnOperationType() {
OperationMethod operationMethod = new TestOperationMethod(this.exampleMethod, OperationType.READ);
OperationMethod operationMethod = new OperationMethod(this.exampleMethod, OperationType.READ, NON_OPTIONAL);
assertThat(operationMethod.getOperationType()).isEqualTo(OperationType.READ);
}
@Test
void getParametersShouldReturnParameters() {
OperationMethod operationMethod = new TestOperationMethod(this.exampleMethod, OperationType.READ);
OperationMethod operationMethod = new OperationMethod(this.exampleMethod, OperationType.READ, NON_OPTIONAL);
OperationParameters parameters = operationMethod.getParameters();
assertThat(parameters.getParameterCount()).isOne();
assertThat(parameters.iterator().next().getName()).isEqualTo("name");
@@ -57,7 +57,7 @@ class ReflectiveOperationInvokerTests {
@BeforeEach
void setup() {
this.target = new Example();
this.operationMethod = new TestOperationMethod(ReflectionUtils.findMethod(Example.class, "reverse",
this.operationMethod = new OperationMethod(ReflectionUtils.findMethod(Example.class, "reverse",
ApiVersion.class, SecurityContext.class, String.class), OperationType.READ, this::isOptional);
this.parameterValueMapper = (parameter, value) -> (value != null) ? value.toString() : null;
}
@@ -102,7 +102,7 @@ class ReflectiveOperationInvokerTests {
@Test
void invokeWhenMissingOptionalArgumentShouldInvoke() {
OperationMethod operationMethod = new TestOperationMethod(ReflectionUtils.findMethod(Example.class,
OperationMethod operationMethod = new OperationMethod(ReflectionUtils.findMethod(Example.class,
"reverseOptional", ApiVersion.class, SecurityContext.class, String.class), OperationType.READ,
this::isOptional);
ReflectiveOperationInvoker invoker = new ReflectiveOperationInvoker(this.target, operationMethod,
@@ -1,42 +0,0 @@
/*
* Copyright 2012-present 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.boot.actuate.endpoint.invoke.reflect;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.function.Predicate;
import org.springframework.boot.actuate.endpoint.OperationType;
/**
* Test {@link OperationMethod}.
*
* @author Phillip Webb
*/
public class TestOperationMethod extends OperationMethod {
public static final Predicate<Parameter> NON_OPTIONAL = (parameter) -> false;
public TestOperationMethod(Method method, OperationType operationType) {
this(method, operationType, NON_OPTIONAL);
}
public TestOperationMethod(Method method, OperationType operationType, Predicate<Parameter> optionalParameters) {
super(method, operationType, optionalParameters);
}
}
@@ -37,7 +37,6 @@ import org.springframework.boot.actuate.endpoint.SecurityContext;
import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
import org.springframework.boot.actuate.endpoint.invoke.OperationParameters;
import org.springframework.boot.actuate.endpoint.invoke.reflect.OperationMethod;
import org.springframework.boot.actuate.endpoint.invoke.reflect.TestOperationMethod;
import org.springframework.boot.actuate.endpoint.web.WebServerNamespace;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.util.ReflectionUtils;
@@ -164,7 +163,7 @@ class CachingOperationInvokerAdvisorTests {
private OperationMethod getOperationMethod(String methodName, Class<?>... parameterTypes) {
Method method = ReflectionUtils.findMethod(TestOperations.class, methodName, parameterTypes);
return new TestOperationMethod(method, OperationType.READ,
return new OperationMethod(method, OperationType.READ,
(parameter) -> MergedAnnotations.from(parameter).isPresent(TestOptional.class));
}