Skip to content

Commit ee83ff5

Browse files
snicollsnicoll
committed
Revert "Only support @OptionalParameter annotation with endpoint methods"
This reverts commit 450eb48. See gh-47136
1 parent d0c742b commit ee83ff5

File tree

12 files changed

+143
-86
lines changed

12 files changed

+143
-86
lines changed

‎configuration-metadata/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,9 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
107107

108108
static final String WEB_ENDPOINT_ANNOTATION = "org.springframework.boot.actuate.endpoint.web.annotation.WebEndpoint";
109109

110-
static final String ENDPOINT_READ_OPERATION_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.ReadOperation";
110+
static final String READ_OPERATION_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.ReadOperation";
111111

112-
static final String ENDPOINT_OPTIONAL_PARAMETER_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.OptionalParameter";
112+
static final String OPTIONAL_PARAMETER_ANNOTATION = "org.springframework.boot.actuate.endpoint.annotation.OptionalParameter";
113113

114114
static final String NAME_ANNOTATION = "org.springframework.boot.context.properties.bind.Name";
115115

@@ -158,16 +158,16 @@ protected Set<String> endpointAnnotations() {
158158
REST_CONTROLLER_ENDPOINT_ANNOTATION, SERVLET_ENDPOINT_ANNOTATION, WEB_ENDPOINT_ANNOTATION);
159159
}
160160

161-
protected String endpointReadOperationAnnotation() {
162-
return ENDPOINT_READ_OPERATION_ANNOTATION;
161+
protected String readOperationAnnotation() {
162+
return READ_OPERATION_ANNOTATION;
163163
}
164164

165165
protected String nameAnnotation() {
166166
return NAME_ANNOTATION;
167167
}
168168

169-
protected String endpointOptionalParameterAnnotation() {
170-
return ENDPOINT_OPTIONAL_PARAMETER_ANNOTATION;
169+
protected String optionalParameterAnnotation() {
170+
return OPTIONAL_PARAMETER_ANNOTATION;
171171
}
172172

173173
protected String endpointAccessEnum() {
@@ -194,8 +194,8 @@ public synchronized void init(ProcessingEnvironment env) {
194194
this.metadataEnv = new MetadataGenerationEnvironment(env, configurationPropertiesAnnotation(),
195195
configurationPropertiesSourceAnnotation(), nestedConfigurationPropertyAnnotation(),
196196
deprecatedConfigurationPropertyAnnotation(), constructorBindingAnnotation(), autowiredAnnotation(),
197-
defaultValueAnnotation(), endpointAnnotations(), endpointReadOperationAnnotation(),
198-
endpointOptionalParameterAnnotation(), nameAnnotation());
197+
defaultValueAnnotation(), endpointAnnotations(), readOperationAnnotation(),
198+
optionalParameterAnnotation(), nameAnnotation());
199199
}
200200

201201
@Override
@@ -271,7 +271,8 @@ private void processAnnotatedTypeElement(String prefix, TypeElement element, Deq
271271
}
272272

273273
private void processExecutableElement(String prefix, ExecutableElement element, Deque<TypeElement> seen) {
274-
if ((!element.getModifiers().contains(Modifier.PRIVATE)) && returnsVoid(element)) {
274+
if ((!element.getModifiers().contains(Modifier.PRIVATE))
275+
&& (TypeKind.VOID != element.getReturnType().getKind())) {
275276
Element returns = this.processingEnv.getTypeUtils().asElement(element.getReturnType());
276277
if (returns instanceof TypeElement typeElement) {
277278
ItemMetadata group = ItemMetadata.newGroup(prefix,
@@ -353,7 +354,7 @@ private void processEndpoint(AnnotationMirror annotation, TypeElement element) {
353354
"Permitted level of access for the %s endpoint.".formatted(endpointId), defaultAccess, null);
354355
this.metadataCollector.add(accessProperty,
355356
(existing) -> checkDefaultAccessValueMatchesExisting(existing, defaultAccess, type));
356-
if (isCachableEndpoint(element)) {
357+
if (hasMainReadOperation(element)) {
357358
this.metadataCollector.addIfAbsent(ItemMetadata.newProperty(endpointKey, "cache.time-to-live",
358359
Duration.class.getName(), type, null, "Maximum time that a response can be cached.", "0ms", null));
359360
}
@@ -370,27 +371,28 @@ private void checkDefaultAccessValueMatchesExisting(ItemMetadata existing, Strin
370371
}
371372
}
372373

373-
private boolean isCachableEndpoint(TypeElement element) {
374+
private boolean hasMainReadOperation(TypeElement element) {
374375
for (ExecutableElement method : ElementFilter.methodsIn(element.getEnclosedElements())) {
375-
if (this.metadataEnv.isEndpointReadOperation(method) && returnsVoid(method)
376-
&& !hasMandatoryEndpointParameter(method)) {
376+
if (this.metadataEnv.getReadOperationAnnotation(method) != null
377+
&& (TypeKind.VOID != method.getReturnType().getKind()) && hasNoOrOptionalParameters(method)) {
377378
return true;
378379
}
379380
}
380381
return false;
381382
}
382383

383-
private boolean returnsVoid(ExecutableElement method) {
384-
return TypeKind.VOID != method.getReturnType().getKind();
385-
}
386-
387-
private boolean hasMandatoryEndpointParameter(ExecutableElement method) {
384+
private boolean hasNoOrOptionalParameters(ExecutableElement method) {
388385
for (VariableElement parameter : method.getParameters()) {
389-
if (!this.metadataEnv.hasEndpointOptionalParameterAnnotation(parameter)) {
390-
return true;
386+
if (!isOptionalParameter(parameter)) {
387+
return false;
391388
}
392389
}
393-
return false;
390+
return true;
391+
}
392+
393+
private boolean isOptionalParameter(VariableElement parameter) {
394+
return this.metadataEnv.hasNullableAnnotation(parameter)
395+
|| this.metadataEnv.hasOptionalParameterAnnotation(parameter);
394396
}
395397

396398
private String getPrefix(AnnotationMirror annotation) {

‎configuration-metadata/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataGenerationEnvironment.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ class MetadataGenerationEnvironment {
9696

9797
private final Set<String> endpointAnnotations;
9898

99-
private final String endpointReadOperationAnnotation;
99+
private final String readOperationAnnotation;
100100

101-
private final String endpointOptionalParameterAnnotation;
101+
private final String optionalParameterAnnotation;
102102

103103
private final String nameAnnotation;
104104

@@ -108,7 +108,7 @@ class MetadataGenerationEnvironment {
108108
String configurationPropertiesSourceAnnotation, String nestedConfigurationPropertyAnnotation,
109109
String deprecatedConfigurationPropertyAnnotation, String constructorBindingAnnotation,
110110
String autowiredAnnotation, String defaultValueAnnotation, Set<String> endpointAnnotations,
111-
String endpointReadOperationAnnotation, String endpointOptionalParameterAnnotation, String nameAnnotation) {
111+
String readOperationAnnotation, String optionalParameterAnnotation, String nameAnnotation) {
112112
this.typeUtils = new TypeUtils(environment);
113113
this.elements = environment.getElementUtils();
114114
this.messager = environment.getMessager();
@@ -122,8 +122,8 @@ class MetadataGenerationEnvironment {
122122
this.autowiredAnnotation = autowiredAnnotation;
123123
this.defaultValueAnnotation = defaultValueAnnotation;
124124
this.endpointAnnotations = endpointAnnotations;
125-
this.endpointReadOperationAnnotation = endpointReadOperationAnnotation;
126-
this.endpointOptionalParameterAnnotation = endpointOptionalParameterAnnotation;
125+
this.readOperationAnnotation = readOperationAnnotation;
126+
this.optionalParameterAnnotation = optionalParameterAnnotation;
127127
this.nameAnnotation = nameAnnotation;
128128
}
129129

@@ -370,8 +370,8 @@ Set<TypeElement> getEndpointAnnotationElements() {
370370
.collect(Collectors.toSet());
371371
}
372372

373-
boolean isEndpointReadOperation(Element element) {
374-
return getAnnotation(element, this.endpointReadOperationAnnotation) != null;
373+
AnnotationMirror getReadOperationAnnotation(Element element) {
374+
return getAnnotation(element, this.readOperationAnnotation);
375375
}
376376

377377
AnnotationMirror getNameAnnotation(Element element) {
@@ -382,8 +382,8 @@ boolean hasNullableAnnotation(Element element) {
382382
return getTypeUseAnnotation(element, NULLABLE_ANNOTATION) != null;
383383
}
384384

385-
boolean hasEndpointOptionalParameterAnnotation(Element element) {
386-
return getAnnotation(element, this.endpointOptionalParameterAnnotation) != null;
385+
boolean hasOptionalParameterAnnotation(Element element) {
386+
return getAnnotation(element, this.optionalParameterAnnotation) != null;
387387
}
388388

389389
private boolean isElementDeprecated(Element element) {

‎configuration-metadata/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/EndpointMetadataGenerationTests.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.springframework.boot.configurationsample.endpoint.CustomPropertiesEndpoint;
2929
import org.springframework.boot.configurationsample.endpoint.EnabledEndpoint;
3030
import org.springframework.boot.configurationsample.endpoint.NoAccessEndpoint;
31+
import org.springframework.boot.configurationsample.endpoint.NullableParameterEndpoint;
3132
import org.springframework.boot.configurationsample.endpoint.OptionalParameterEndpoint;
3233
import org.springframework.boot.configurationsample.endpoint.ReadOnlyAccessEndpoint;
3334
import org.springframework.boot.configurationsample.endpoint.SimpleEndpoint;
@@ -194,6 +195,16 @@ void shouldFailIfEndpointWithSameIdButWithConflictingEnabledByDefaultSetting() {
194195
"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");
195196
}
196197

198+
@Test
199+
void endpointWithNullableParameter() {
200+
ConfigurationMetadata metadata = compile(NullableParameterEndpoint.class);
201+
assertThat(metadata)
202+
.has(Metadata.withGroup("management.endpoint.nullable").fromSource(NullableParameterEndpoint.class));
203+
assertThat(metadata).has(access("nullable", Access.UNRESTRICTED));
204+
assertThat(metadata).has(cacheTtl("nullable"));
205+
assertThat(metadata.getItems()).hasSize(3);
206+
}
207+
197208
@Test
198209
void endpointWithOptionalParameter() {
199210
ConfigurationMetadata metadata = compile(OptionalParameterEndpoint.class);

‎configuration-metadata/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/test/TestConfigurationMetadataAnnotationProcessor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,12 @@ protected Set<String> endpointAnnotations() {
126126
}
127127

128128
@Override
129-
protected String endpointReadOperationAnnotation() {
129+
protected String readOperationAnnotation() {
130130
return READ_OPERATION_ANNOTATION;
131131
}
132132

133133
@Override
134-
protected String endpointOptionalParameterAnnotation() {
134+
protected String optionalParameterAnnotation() {
135135
return OPTIONAL_PARAMETER_ANNOTATION;
136136
}
137137

‎configuration-metadata/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/NullableParameterEndpoint.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.configurationsample.endpoint;
18+
19+
import org.jspecify.annotations.Nullable;
20+
21+
import org.springframework.boot.configurationsample.Endpoint;
22+
import org.springframework.boot.configurationsample.ReadOperation;
23+
24+
/**
25+
* An endpoint that uses {@code Nullable} to signal an optional parameter.
26+
*
27+
* @author Wonyong Hwang
28+
*/
29+
@Endpoint(id = "nullable")
30+
public class NullableParameterEndpoint {
31+
32+
@ReadOperation
33+
public String invoke(@Nullable String parameter) {
34+
return "test with " + parameter;
35+
}
36+
37+
}

‎module/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethod.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
* @since 2.0.0
3535
* @see ReflectiveOperationInvoker
3636
*/
37-
public abstract class OperationMethod {
37+
public class OperationMethod {
3838

3939
private static final ParameterNameDiscoverer DEFAULT_PARAMETER_NAME_DISCOVERER = new DefaultParameterNameDiscoverer();
4040

@@ -44,14 +44,26 @@ public abstract class OperationMethod {
4444

4545
private final OperationParameters operationParameters;
4646

47+
/**
48+
* Create a new {@link OperationMethod} instance.
49+
* @param method the source method
50+
* @param operationType the operation type
51+
* @deprecated since 4.0.0 for removal in 4.2.0 in favor of
52+
* {@link #OperationMethod(Method, OperationType, Predicate)}
53+
*/
54+
@Deprecated(since = "4.0.0", forRemoval = true)
55+
public OperationMethod(Method method, OperationType operationType) {
56+
this(method, operationType, (parameter) -> false);
57+
}
58+
4759
/**
4860
* Create a new {@link OperationMethod} instance.
4961
* @param method the source method
5062
* @param operationType the operation type
5163
* @param optionalParameters predicate to test if a parameter is optional
5264
* @since 4.0.0
5365
*/
54-
protected OperationMethod(Method method, OperationType operationType, Predicate<Parameter> optionalParameters) {
66+
public OperationMethod(Method method, OperationType operationType, Predicate<Parameter> optionalParameters) {
5567
Assert.notNull(method, "'method' must not be null");
5668
Assert.notNull(operationType, "'operationType' must not be null");
5769
this.method = method;

‎module/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameter.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.function.Predicate;
2222

2323
import org.springframework.boot.actuate.endpoint.invoke.OperationParameter;
24+
import org.springframework.core.Nullness;
2425

2526
/**
2627
* {@link OperationParameter} created from an {@link OperationMethod}.
@@ -60,7 +61,11 @@ public Class<?> getType() {
6061

6162
@Override
6263
public boolean isMandatory() {
63-
return !this.optional.test(this.parameter);
64+
return !isOptional();
65+
}
66+
67+
private boolean isOptional() {
68+
return Nullness.NULLABLE == Nullness.forParameter(this.parameter) || this.optional.test(this.parameter);
6469
}
6570

6671
@Override

‎module/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameterTests.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ class OperationMethodParameterTests {
4343

4444
private final Method example = ReflectionUtils.findMethod(getClass(), "example", String.class, String.class);
4545

46+
private final Method exampleJSpecifyNullable = ReflectionUtils.findMethod(getClass(), "exampleJSpecifyNullable",
47+
String.class, String.class);
48+
49+
private final Method exampleSpringNullable = ReflectionUtils.findMethod(getClass(), "exampleSpringNullable",
50+
String.class, String.class);
51+
4652
private Method exampleAnnotation = ReflectionUtils.findMethod(getClass(), "exampleAnnotation", String.class);
4753

4854
@Test
@@ -73,6 +79,20 @@ void isMandatoryWhenOptionalAnnotationShouldReturnFalse() {
7379
assertThat(parameter.isMandatory()).isFalse();
7480
}
7581

82+
@Test
83+
void isMandatoryWhenJSpecifyNullableAnnotationShouldReturnFalse() {
84+
OperationMethodParameter parameter = new OperationMethodParameter("name",
85+
this.exampleJSpecifyNullable.getParameters()[1], this::isOptionalParameter);
86+
assertThat(parameter.isMandatory()).isFalse();
87+
}
88+
89+
@Test
90+
void isMandatoryWhenSpringNullableAnnotationShouldReturnFalse() {
91+
OperationMethodParameter parameter = new OperationMethodParameter("name",
92+
this.exampleSpringNullable.getParameters()[1], this::isOptionalParameter);
93+
assertThat(parameter.isMandatory()).isFalse();
94+
}
95+
7696
@Test
7797
void getAnnotationShouldReturnAnnotation() {
7898
OperationMethodParameter parameter = new OperationMethodParameter("name",
@@ -89,6 +109,13 @@ private boolean isOptionalParameter(Parameter parameter) {
89109
void example(String one, @TestOptional String two) {
90110
}
91111

112+
void exampleJSpecifyNullable(String one, @org.jspecify.annotations.Nullable String two) {
113+
}
114+
115+
@SuppressWarnings("deprecation")
116+
void exampleSpringNullable(String one, @org.springframework.lang.Nullable String two) {
117+
}
118+
92119
void exampleAnnotation(@Selector(match = Match.ALL_REMAINING) String allRemaining) {
93120
}
94121

‎module/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodTests.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package org.springframework.boot.actuate.endpoint.invoke.reflect;
1818

1919
import java.lang.reflect.Method;
20+
import java.lang.reflect.Parameter;
21+
import java.util.function.Predicate;
2022

2123
import org.junit.jupiter.api.Test;
2224

@@ -34,35 +36,39 @@
3436
*/
3537
class OperationMethodTests {
3638

39+
private static final Predicate<Parameter> NON_OPTIONAL = (parameter) -> false;
40+
3741
private final Method exampleMethod = ReflectionUtils.findMethod(getClass(), "example", String.class);
3842

3943
@Test
4044
void createWhenMethodIsNullShouldThrowException() {
41-
assertThatIllegalArgumentException().isThrownBy(() -> new TestOperationMethod(null, OperationType.READ))
45+
assertThatIllegalArgumentException()
46+
.isThrownBy(() -> new OperationMethod(null, OperationType.READ, NON_OPTIONAL))
4247
.withMessageContaining("'method' must not be null");
4348
}
4449

4550
@Test
4651
void createWhenOperationTypeIsNullShouldThrowException() {
47-
assertThatIllegalArgumentException().isThrownBy(() -> new TestOperationMethod(this.exampleMethod, null))
52+
assertThatIllegalArgumentException()
53+
.isThrownBy(() -> new OperationMethod(this.exampleMethod, null, NON_OPTIONAL))
4854
.withMessageContaining("'operationType' must not be null");
4955
}
5056

5157
@Test
5258
void getMethodShouldReturnMethod() {
53-
OperationMethod operationMethod = new TestOperationMethod(this.exampleMethod, OperationType.READ);
59+
OperationMethod operationMethod = new OperationMethod(this.exampleMethod, OperationType.READ, NON_OPTIONAL);
5460
assertThat(operationMethod.getMethod()).isEqualTo(this.exampleMethod);
5561
}
5662

5763
@Test
5864
void getOperationTypeShouldReturnOperationType() {
59-
OperationMethod operationMethod = new TestOperationMethod(this.exampleMethod, OperationType.READ);
65+
OperationMethod operationMethod = new OperationMethod(this.exampleMethod, OperationType.READ, NON_OPTIONAL);
6066
assertThat(operationMethod.getOperationType()).isEqualTo(OperationType.READ);
6167
}
6268

6369
@Test
6470
void getParametersShouldReturnParameters() {
65-
OperationMethod operationMethod = new TestOperationMethod(this.exampleMethod, OperationType.READ);
71+
OperationMethod operationMethod = new OperationMethod(this.exampleMethod, OperationType.READ, NON_OPTIONAL);
6672
OperationParameters parameters = operationMethod.getParameters();
6773
assertThat(parameters.getParameterCount()).isOne();
6874
assertThat(parameters.iterator().next().getName()).isEqualTo("name");

0 commit comments

Comments
 (0)