From ca799f7b2151616a7c3d47747e58704ed5daa7ce Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Fri, 19 Jan 2024 15:31:27 +0100 Subject: [PATCH] Tolerate actuator endpoints with the same id Closes gh-39249 --- ...figurationMetadataAnnotationProcessor.java | 27 ++++++++++---- .../MetadataCollector.java | 29 ++++++++++++++- .../EndpointMetadataGenerationTests.java | 24 ++++++++++++- .../endpoint/SimpleEndpoint2.java | 35 ++++++++++++++++++ .../endpoint/SimpleEndpoint3.java | 36 +++++++++++++++++++ 5 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint2.java create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint3.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java index 93fe3392d2..0c4fc7c151 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -59,6 +59,7 @@ import org.springframework.boot.configurationprocessor.metadata.ItemMetadata; * @author Kris De Volder * @author Jonas Keßler * @author Scott Frederick + * @author Moritz Halbritter * @since 1.2.0 */ @SupportedAnnotationTypes({ ConfigurationMetadataAnnotationProcessor.AUTO_CONFIGURATION_ANNOTATION, @@ -291,18 +292,30 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor return; // Can't process that endpoint } String endpointKey = ItemMetadata.newItemMetadataPrefix("management.endpoint.", endpointId); - Boolean enabledByDefault = (Boolean) elementValues.get("enableByDefault"); + boolean enabledByDefault = (boolean) elementValues.getOrDefault("enableByDefault", true); String type = this.metadataEnv.getTypeUtils().getQualifiedName(element); - this.metadataCollector.add(ItemMetadata.newGroup(endpointKey, type, type, null)); - this.metadataCollector.add(ItemMetadata.newProperty(endpointKey, "enabled", Boolean.class.getName(), type, null, - String.format("Whether to enable the %s endpoint.", endpointId), - (enabledByDefault != null) ? enabledByDefault : true, null)); + this.metadataCollector.addIfAbsent(ItemMetadata.newGroup(endpointKey, type, type, null)); + this.metadataCollector.add( + ItemMetadata.newProperty(endpointKey, "enabled", Boolean.class.getName(), type, null, + "Whether to enable the %s endpoint.".formatted(endpointId), enabledByDefault, null), + (existing) -> checkEnabledValueMatchesExisting(existing, enabledByDefault, type)); if (hasMainReadOperation(element)) { - this.metadataCollector.add(ItemMetadata.newProperty(endpointKey, "cache.time-to-live", + 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)); } } + private void checkEnabledValueMatchesExisting(ItemMetadata existing, boolean enabledByDefault, String sourceType) { + boolean existingDefaultValue = (boolean) existing.getDefaultValue(); + if (enabledByDefault == existingDefaultValue) { + return; + } + throw new IllegalStateException( + "Existing property '%s' from type %s has a conflicting value. Existing value: %b, new value from type %s: %b" + .formatted(existing.getName(), existing.getSourceType(), existingDefaultValue, sourceType, + enabledByDefault)); + } + private boolean hasMainReadOperation(TypeElement element) { for (ExecutableElement method : ElementFilter.methodsIn(element.getEnclosedElements())) { if (this.metadataEnv.getReadOperationAnnotation(method) != null diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java index c6fe7f81d7..2aa1a55a07 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2024 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. @@ -20,6 +20,7 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.function.Consumer; import javax.annotation.processing.ProcessingEnvironment; import javax.annotation.processing.RoundEnvironment; @@ -35,6 +36,7 @@ import org.springframework.boot.configurationprocessor.metadata.ItemMetadata; * * @author Andy Wilkinson * @author Kris De Volder + * @author Moritz Halbritter * @since 1.2.2 */ public class MetadataCollector { @@ -76,6 +78,24 @@ public class MetadataCollector { this.metadataItems.add(metadata); } + public void add(ItemMetadata metadata, Consumer onConflict) { + ItemMetadata existing = find(metadata.getName()); + if (existing != null) { + onConflict.accept(existing); + return; + } + add(metadata); + } + + public boolean addIfAbsent(ItemMetadata metadata) { + ItemMetadata existing = find(metadata.getName()); + if (existing != null) { + return false; + } + add(metadata); + return true; + } + public boolean hasSimilarGroup(ItemMetadata metadata) { if (!metadata.isOfItemType(ItemMetadata.ItemType.GROUP)) { throw new IllegalStateException("item " + metadata + " must be a group"); @@ -105,6 +125,13 @@ public class MetadataCollector { return metadata; } + private ItemMetadata find(String name) { + return this.metadataItems.stream() + .filter((candidate) -> name.equals(candidate.getName())) + .findFirst() + .orElse(null); + } + private boolean shouldBeMerged(ItemMetadata itemMetadata) { String sourceType = itemMetadata.getSourceType(); return (sourceType != null && !deletedInCurrentBuild(sourceType) && !processedInCurrentBuild(sourceType)); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/EndpointMetadataGenerationTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/EndpointMetadataGenerationTests.java index 4c77c1794b..e9d88b28ba 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/EndpointMetadataGenerationTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/EndpointMetadataGenerationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -27,16 +27,20 @@ import org.springframework.boot.configurationsample.endpoint.CustomPropertiesEnd import org.springframework.boot.configurationsample.endpoint.DisabledEndpoint; import org.springframework.boot.configurationsample.endpoint.EnabledEndpoint; import org.springframework.boot.configurationsample.endpoint.SimpleEndpoint; +import org.springframework.boot.configurationsample.endpoint.SimpleEndpoint2; +import org.springframework.boot.configurationsample.endpoint.SimpleEndpoint3; import org.springframework.boot.configurationsample.endpoint.SpecificEndpoint; import org.springframework.boot.configurationsample.endpoint.incremental.IncrementalEndpoint; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** * Metadata generation tests for Actuator endpoints. * * @author Stephane Nicoll * @author Scott Frederick + * @author Moritz Halbritter */ class EndpointMetadataGenerationTests extends AbstractMetadataGenerationTests { @@ -148,6 +152,24 @@ class EndpointMetadataGenerationTests extends AbstractMetadataGenerationTests { assertThat(metadata.getItems()).hasSize(3); } + @Test + void shouldTolerateEndpointWithSameId() { + ConfigurationMetadata metadata = compile(SimpleEndpoint.class, SimpleEndpoint2.class); + assertThat(metadata).has(Metadata.withGroup("management.endpoint.simple").fromSource(SimpleEndpoint.class)); + assertThat(metadata).has(enabledFlag("simple", "simple", true)); + assertThat(metadata).has(cacheTtl("simple")); + assertThat(metadata.getItems()).hasSize(3); + } + + @Test + void shouldFailIfEndpointWithSameIdButWithConflictingEnabledByDefaultSetting() { + assertThatRuntimeException().isThrownBy(() -> compile(SimpleEndpoint.class, SimpleEndpoint3.class)) + .havingRootCause() + .isInstanceOf(IllegalStateException.class) + .withMessage( + "Existing property 'management.endpoint.simple.enabled' from type org.springframework.boot.configurationsample.endpoint.SimpleEndpoint has a conflicting value. Existing value: true, new value from type org.springframework.boot.configurationsample.endpoint.SimpleEndpoint3: false"); + } + private Metadata.MetadataItemCondition enabledFlag(String endpointId, String endpointSuffix, Boolean defaultValue) { return Metadata.withEnabledFlag("management.endpoint." + endpointSuffix + ".enabled") .withDefaultValue(defaultValue) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint2.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint2.java new file mode 100644 index 0000000000..971064af44 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint2.java @@ -0,0 +1,35 @@ +/* + * Copyright 2012-2024 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.springframework.boot.configurationsample.Endpoint; +import org.springframework.boot.configurationsample.ReadOperation; + +/** + * A simple endpoint with no default override, with the same id as {@link SimpleEndpoint}. + * + * @author Moritz Halbritter + */ +@Endpoint(id = "simple") +public class SimpleEndpoint2 { + + @ReadOperation + public String invoke() { + return "test"; + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint3.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint3.java new file mode 100644 index 0000000000..48c88f16f4 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/endpoint/SimpleEndpoint3.java @@ -0,0 +1,36 @@ +/* + * Copyright 2012-2024 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.springframework.boot.configurationsample.Endpoint; +import org.springframework.boot.configurationsample.ReadOperation; + +/** + * A simple endpoint with no default override, with the same id as {@link SimpleEndpoint}, + * but not enabled by default. + * + * @author Moritz Halbritter + */ +@Endpoint(id = "simple", enableByDefault = false) +public class SimpleEndpoint3 { + + @ReadOperation + public String invoke() { + return "test"; + } + +}