From 94c35ae1de7c1d85fd18e16fd4234a5801840b76 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 23 Jul 2019 11:21:42 +0200 Subject: [PATCH] Polish "Make Actuator dedicated ConversionService configurable" See gh-16449 --- .../endpoint/EndpointAutoConfiguration.java | 73 ++++-------- .../EndpointAutoConfigurationTests.java | 109 +++++++----------- .../annotation}/EndpointConverter.java | 17 +-- .../asciidoc/production-ready-features.adoc | 4 +- 4 files changed, 70 insertions(+), 133 deletions(-) rename spring-boot-project/{spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint => spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation}/EndpointConverter.java (65%) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfiguration.java index 212f968945..d32af3609f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfiguration.java @@ -16,21 +16,18 @@ package org.springframework.boot.actuate.autoconfigure.endpoint; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; -import org.springframework.beans.factory.BeanFactory; -import org.springframework.beans.factory.ListableBeanFactory; -import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; +import org.springframework.boot.actuate.endpoint.annotation.EndpointConverter; import org.springframework.boot.actuate.endpoint.invoke.ParameterValueMapper; import org.springframework.boot.actuate.endpoint.invoke.convert.ConversionServiceParameterValueMapper; import org.springframework.boot.actuate.endpoint.invoker.cache.CachingOperationInvokerAdvisor; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.convert.ApplicationConversionService; -import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.ConversionService; @@ -53,9 +50,23 @@ public class EndpointAutoConfiguration { @Bean @ConditionalOnMissingBean public ParameterValueMapper endpointOperationParameterMapper( - ApplicationContext applicationContext) { - return new ConversionServiceParameterValueMapper( - new Factory(applicationContext.getAutowireCapableBeanFactory()).create()); + @EndpointConverter ObjectProvider> converters, + @EndpointConverter ObjectProvider genericConverters) { + ConversionService conversionService = createConversionService( + converters.orderedStream().collect(Collectors.toList()), + genericConverters.orderedStream().collect(Collectors.toList())); + return new ConversionServiceParameterValueMapper(conversionService); + } + + private ConversionService createConversionService(List> converters, + List genericConverters) { + if (genericConverters.isEmpty() && converters.isEmpty()) { + return ApplicationConversionService.getSharedInstance(); + } + ApplicationConversionService conversionService = new ApplicationConversionService(); + converters.forEach(conversionService::addConverter); + genericConverters.forEach(conversionService::addConverter); + return conversionService; } @Bean @@ -64,48 +75,4 @@ public class EndpointAutoConfiguration { return new CachingOperationInvokerAdvisor(new EndpointIdTimeToLivePropertyFunction(environment)); } - private static class Factory { - - @SuppressWarnings("rawtypes") - private final List converters; - - private final List genericConverters; - - Factory(BeanFactory beanFactory) { - this.converters = beans(beanFactory, Converter.class, - EndpointConverter.VALUE); - this.genericConverters = beans(beanFactory, GenericConverter.class, - EndpointConverter.VALUE); - } - - private List beans(BeanFactory beanFactory, Class type, - String qualifier) { - if (beanFactory instanceof ListableBeanFactory) { - return beans(type, qualifier, (ListableBeanFactory) beanFactory); - } - return Collections.emptyList(); - } - - private List beans(Class type, String qualifier, - ListableBeanFactory beanFactory) { - return new ArrayList<>(BeanFactoryAnnotationUtils - .qualifiedBeansOfType(beanFactory, type, qualifier).values()); - } - - public ConversionService create() { - if (this.converters.isEmpty() && this.genericConverters.isEmpty()) { - return ApplicationConversionService.getSharedInstance(); - } - ApplicationConversionService conversionService = new ApplicationConversionService(); - for (Converter converter : this.converters) { - conversionService.addConverter(converter); - } - for (GenericConverter genericConverter : this.genericConverters) { - conversionService.addConverter(genericConverter); - } - return conversionService; - } - - } - } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfigurationTests.java index cba2d1d5a8..f3c0643fa5 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -19,13 +19,14 @@ package org.springframework.boot.actuate.autoconfigure.endpoint; import java.util.Collections; import java.util.Set; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import org.springframework.boot.actuate.endpoint.annotation.EndpointConverter; import org.springframework.boot.actuate.endpoint.invoke.OperationParameter; import org.springframework.boot.actuate.endpoint.invoke.ParameterMappingException; import org.springframework.boot.actuate.endpoint.invoke.ParameterValueMapper; import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.test.context.runner.WebApplicationContextRunner; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.ConverterNotFoundException; @@ -42,72 +43,55 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * * @author Chao Chang */ -public class EndpointAutoConfigurationTests { +class EndpointAutoConfigurationTests { - private static final AutoConfigurations CONFIGURATIONS = AutoConfigurations - .of(EndpointAutoConfiguration.class); - - private WebApplicationContextRunner contextRunner = new WebApplicationContextRunner() - .withConfiguration(CONFIGURATIONS); + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(EndpointAutoConfiguration.class)); @Test - public void mapShouldUseConfigurationConverter() { - this.contextRunner.withUserConfiguration(ConverterConfiguration.class) - .run((context) -> { - ParameterValueMapper parameterValueMapper = context - .getBean(ParameterValueMapper.class); - Object paramValue = parameterValueMapper.mapParameterValue( - new TestOperationParameter(Person.class), "John Smith"); - assertThat(paramValue).isInstanceOf(Person.class); - Person person = (Person) paramValue; - assertThat(person.firstName).isEqualTo("John"); - assertThat(person.lastName).isEqualTo("Smith"); - }); + void mapShouldUseConfigurationConverter() { + this.contextRunner.withUserConfiguration(ConverterConfiguration.class).run((context) -> { + ParameterValueMapper parameterValueMapper = context.getBean(ParameterValueMapper.class); + Object paramValue = parameterValueMapper.mapParameterValue(new TestOperationParameter(Person.class), + "John Smith"); + assertThat(paramValue).isInstanceOf(Person.class); + Person person = (Person) paramValue; + assertThat(person.firstName).isEqualTo("John"); + assertThat(person.lastName).isEqualTo("Smith"); + }); } @Test - public void mapWhenConfigurationConverterIsNotQualifiedShouldNotConvert() { + void mapWhenConfigurationConverterIsNotQualifiedShouldNotConvert() { assertThatExceptionOfType(ParameterMappingException.class).isThrownBy(() -> { - this.contextRunner - .withUserConfiguration(NonQualifiedConverterConfiguration.class) - .run((context) -> { - ParameterValueMapper parameterValueMapper = context - .getBean(ParameterValueMapper.class); - parameterValueMapper.mapParameterValue( - new TestOperationParameter(Person.class), "John Smith"); - }); + this.contextRunner.withUserConfiguration(NonQualifiedConverterConfiguration.class).run((context) -> { + ParameterValueMapper parameterValueMapper = context.getBean(ParameterValueMapper.class); + parameterValueMapper.mapParameterValue(new TestOperationParameter(Person.class), "John Smith"); + }); }).withCauseInstanceOf(ConverterNotFoundException.class); - } @Test - public void mapShouldUseGenericConfigurationConverter() { - this.contextRunner.withUserConfiguration(GenericConverterConfiguration.class) - .run((context) -> { - ParameterValueMapper parameterValueMapper = context - .getBean(ParameterValueMapper.class); - Object paramValue = parameterValueMapper.mapParameterValue( - new TestOperationParameter(Person.class), "John Smith"); - assertThat(paramValue).isInstanceOf(Person.class); - Person person = (Person) paramValue; - assertThat(person.firstName).isEqualTo("John"); - assertThat(person.lastName).isEqualTo("Smith"); - }); + void mapShouldUseGenericConfigurationConverter() { + this.contextRunner.withUserConfiguration(GenericConverterConfiguration.class).run((context) -> { + ParameterValueMapper parameterValueMapper = context.getBean(ParameterValueMapper.class); + Object paramValue = parameterValueMapper.mapParameterValue(new TestOperationParameter(Person.class), + "John Smith"); + assertThat(paramValue).isInstanceOf(Person.class); + Person person = (Person) paramValue; + assertThat(person.firstName).isEqualTo("John"); + assertThat(person.lastName).isEqualTo("Smith"); + }); } @Test - public void mapWhenGenericConfigurationConverterIsNotQualifiedShouldNotConvert() { + void mapWhenGenericConfigurationConverterIsNotQualifiedShouldNotConvert() { assertThatExceptionOfType(ParameterMappingException.class).isThrownBy(() -> { - this.contextRunner - .withUserConfiguration( - NonQualifiedGenericConverterConfiguration.class) - .run((context) -> { - ParameterValueMapper parameterValueMapper = context - .getBean(ParameterValueMapper.class); - parameterValueMapper.mapParameterValue( - new TestOperationParameter(Person.class), "John Smith"); - }); + this.contextRunner.withUserConfiguration(NonQualifiedGenericConverterConfiguration.class).run((context) -> { + ParameterValueMapper parameterValueMapper = context.getBean(ParameterValueMapper.class); + parameterValueMapper.mapParameterValue(new TestOperationParameter(Person.class), "John Smith"); + }); }).withCauseInstanceOf(ConverterNotFoundException.class); @@ -131,8 +115,7 @@ public class EndpointAutoConfigurationTests { } @Override - public Object convert(Object source, TypeDescriptor sourceType, - TypeDescriptor targetType) { + public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { String[] content = StringUtils.split((String) source, " "); return new Person(content[0], content[1]); } @@ -144,7 +127,7 @@ public class EndpointAutoConfigurationTests { @Bean @EndpointConverter - public Converter personConverter() { + Converter personConverter() { return new PersonConverter(); } @@ -154,7 +137,7 @@ public class EndpointAutoConfigurationTests { static class NonQualifiedConverterConfiguration { @Bean - public Converter personConverter() { + Converter personConverter() { return new PersonConverter(); } @@ -165,7 +148,7 @@ public class EndpointAutoConfigurationTests { @Bean @EndpointConverter - public GenericConverter genericPersonConverter() { + GenericConverter genericPersonConverter() { return new GenericPersonConverter(); } @@ -175,7 +158,7 @@ public class EndpointAutoConfigurationTests { static class NonQualifiedGenericConverterConfiguration { @Bean - public GenericConverter genericPersonConverter() { + GenericConverter genericPersonConverter() { return new GenericPersonConverter(); } @@ -192,14 +175,6 @@ public class EndpointAutoConfigurationTests { this.lastName = lastName; } - public String getFirstName() { - return this.firstName; - } - - public String getLastName() { - return this.lastName; - } - } private static class TestOperationParameter implements OperationParameter { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointConverter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointConverter.java similarity index 65% rename from spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointConverter.java rename to spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointConverter.java index 29a5226b69..790028a7ea 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/EndpointConverter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.boot.actuate.autoconfigure.endpoint; +package org.springframework.boot.actuate.endpoint.annotation; import java.lang.annotation.Documented; import java.lang.annotation.ElementType; @@ -23,22 +23,17 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.boot.actuate.endpoint.annotation.Endpoint; /** - * Qualifier for beans that are needed to be converters for {@link Endpoint}. + * Qualifier for beans that are needed to convert {@link Endpoint} input parameters. * * @author Chao Chang + * @since 2.2.0 */ -@Qualifier(EndpointConverter.VALUE) -@Target({ ElementType.TYPE, ElementType.METHOD }) +@Target({ ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.TYPE, ElementType.ANNOTATION_TYPE }) @Retention(RetentionPolicy.RUNTIME) @Documented +@Qualifier public @interface EndpointConverter { - /** - * Concrete value for the {@link Qualifier @Qualifier}. - */ - String VALUE = "org.springframework.boot.actuate.autoconfigure.endpoint.EndpointConverter"; - } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc index 9d7cc9b492..36ed2f2c3c 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc @@ -558,8 +558,8 @@ automatically if you are using Spring Boot's Gradle plugin or if you are using M The parameters passed to endpoint operation methods are, if necessary, automatically converted to the required type. Before calling an operation method, the input received via JMX or an HTTP request is converted to the required types using an instance of -`ApplicationConversionService`. - +`ApplicationConversionService` as well as any `Converter` or `GenericConverter` beans +qualified with `@EndpointConverter`. [[production-ready-endpoints-custom-web]]