Make getSharedInstance() unmodifiable
Update `ApplicationConversionService.getSharedInstance()` so that the instance returned is unmodifiable and converters cannot be added or removed from it. Closes gh-26088
This commit is contained in:
		
							parent
							
								
									47709ec0e4
								
							
						
					
					
						commit
						5581ec0e29
					
				|  | @ -60,8 +60,6 @@ import org.springframework.context.support.AbstractApplicationContext; | ||||||
| import org.springframework.context.support.GenericApplicationContext; | import org.springframework.context.support.GenericApplicationContext; | ||||||
| import org.springframework.core.GenericTypeResolver; | import org.springframework.core.GenericTypeResolver; | ||||||
| import org.springframework.core.annotation.AnnotationAwareOrderComparator; | import org.springframework.core.annotation.AnnotationAwareOrderComparator; | ||||||
| import org.springframework.core.convert.ConversionService; |  | ||||||
| import org.springframework.core.convert.support.ConfigurableConversionService; |  | ||||||
| import org.springframework.core.env.CommandLinePropertySource; | import org.springframework.core.env.CommandLinePropertySource; | ||||||
| import org.springframework.core.env.CompositePropertySource; | import org.springframework.core.env.CompositePropertySource; | ||||||
| import org.springframework.core.env.ConfigurableEnvironment; | import org.springframework.core.env.ConfigurableEnvironment; | ||||||
|  | @ -512,8 +510,7 @@ public class SpringApplication { | ||||||
| 	 */ | 	 */ | ||||||
| 	protected void configureEnvironment(ConfigurableEnvironment environment, String[] args) { | 	protected void configureEnvironment(ConfigurableEnvironment environment, String[] args) { | ||||||
| 		if (this.addConversionService) { | 		if (this.addConversionService) { | ||||||
| 			ConversionService conversionService = ApplicationConversionService.getSharedInstance(); | 			environment.setConversionService(new ApplicationConversionService()); | ||||||
| 			environment.setConversionService((ConfigurableConversionService) conversionService); |  | ||||||
| 		} | 		} | ||||||
| 		configurePropertySources(environment, args); | 		configurePropertySources(environment, args); | ||||||
| 		configureProfiles(environment, args); | 		configureProfiles(environment, args); | ||||||
|  | @ -633,7 +630,7 @@ public class SpringApplication { | ||||||
| 			} | 			} | ||||||
| 		} | 		} | ||||||
| 		if (this.addConversionService) { | 		if (this.addConversionService) { | ||||||
| 			context.getBeanFactory().setConversionService(ApplicationConversionService.getSharedInstance()); | 			context.getBeanFactory().setConversionService(context.getEnvironment().getConversionService()); | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -16,6 +16,7 @@ | ||||||
| 
 | 
 | ||||||
| package org.springframework.boot.convert; | package org.springframework.boot.convert; | ||||||
| 
 | 
 | ||||||
|  | import java.lang.annotation.Annotation; | ||||||
| import java.util.LinkedHashSet; | import java.util.LinkedHashSet; | ||||||
| import java.util.Set; | import java.util.Set; | ||||||
| 
 | 
 | ||||||
|  | @ -23,11 +24,13 @@ import org.springframework.beans.factory.ListableBeanFactory; | ||||||
| import org.springframework.core.convert.ConversionService; | import org.springframework.core.convert.ConversionService; | ||||||
| import org.springframework.core.convert.TypeDescriptor; | import org.springframework.core.convert.TypeDescriptor; | ||||||
| import org.springframework.core.convert.converter.Converter; | import org.springframework.core.convert.converter.Converter; | ||||||
|  | import org.springframework.core.convert.converter.ConverterFactory; | ||||||
| import org.springframework.core.convert.converter.ConverterRegistry; | import org.springframework.core.convert.converter.ConverterRegistry; | ||||||
| import org.springframework.core.convert.converter.GenericConverter; | import org.springframework.core.convert.converter.GenericConverter; | ||||||
| import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; | import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; | ||||||
| import org.springframework.core.convert.support.ConfigurableConversionService; | import org.springframework.core.convert.support.ConfigurableConversionService; | ||||||
| import org.springframework.core.convert.support.DefaultConversionService; | import org.springframework.core.convert.support.DefaultConversionService; | ||||||
|  | import org.springframework.format.AnnotationFormatterFactory; | ||||||
| import org.springframework.format.Formatter; | import org.springframework.format.Formatter; | ||||||
| import org.springframework.format.FormatterRegistry; | import org.springframework.format.FormatterRegistry; | ||||||
| import org.springframework.format.Parser; | import org.springframework.format.Parser; | ||||||
|  | @ -52,15 +55,96 @@ public class ApplicationConversionService extends FormattingConversionService { | ||||||
| 
 | 
 | ||||||
| 	private static volatile ApplicationConversionService sharedInstance; | 	private static volatile ApplicationConversionService sharedInstance; | ||||||
| 
 | 
 | ||||||
|  | 	private boolean unmodifiable; | ||||||
|  | 
 | ||||||
| 	public ApplicationConversionService() { | 	public ApplicationConversionService() { | ||||||
| 		this(null); | 		this(null); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	public ApplicationConversionService(StringValueResolver embeddedValueResolver) { | 	public ApplicationConversionService(StringValueResolver embeddedValueResolver) { | ||||||
|  | 		this(embeddedValueResolver, false); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	private ApplicationConversionService(StringValueResolver embeddedValueResolver, boolean unmodifiable) { | ||||||
| 		if (embeddedValueResolver != null) { | 		if (embeddedValueResolver != null) { | ||||||
| 			setEmbeddedValueResolver(embeddedValueResolver); | 			setEmbeddedValueResolver(embeddedValueResolver); | ||||||
| 		} | 		} | ||||||
| 		configure(this); | 		configure(this); | ||||||
|  | 		this.unmodifiable = unmodifiable; | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addPrinter(Printer<?> printer) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addPrinter(printer); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addParser(Parser<?> parser) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addParser(parser); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addFormatter(Formatter<?> formatter) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addFormatter(formatter); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addFormatterForFieldType(Class<?> fieldType, Formatter<?> formatter) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addFormatterForFieldType(fieldType, formatter); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addConverter(Converter<?, ?> converter) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addConverter(converter); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addFormatterForFieldType(Class<?> fieldType, Printer<?> printer, Parser<?> parser) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addFormatterForFieldType(fieldType, printer, parser); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addFormatterForFieldAnnotation( | ||||||
|  | 			AnnotationFormatterFactory<? extends Annotation> annotationFormatterFactory) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addFormatterForFieldAnnotation(annotationFormatterFactory); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public <S, T> void addConverter(Class<S> sourceType, Class<T> targetType, | ||||||
|  | 			Converter<? super S, ? extends T> converter) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addConverter(sourceType, targetType, converter); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addConverter(GenericConverter converter) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addConverter(converter); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void addConverterFactory(ConverterFactory<?, ?> factory) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.addConverterFactory(factory); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	@Override | ||||||
|  | 	public void removeConvertible(Class<?> sourceType, Class<?> targetType) { | ||||||
|  | 		assertModifiable(); | ||||||
|  | 		super.removeConvertible(sourceType, targetType); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	private void assertModifiable() { | ||||||
|  | 		if (this.unmodifiable) { | ||||||
|  | 			throw new UnsupportedOperationException("This ApplicationConversionService cannot be modified"); | ||||||
|  | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	/** | 	/** | ||||||
|  | @ -101,7 +185,7 @@ public class ApplicationConversionService extends FormattingConversionService { | ||||||
| 			synchronized (ApplicationConversionService.class) { | 			synchronized (ApplicationConversionService.class) { | ||||||
| 				sharedInstance = ApplicationConversionService.sharedInstance; | 				sharedInstance = ApplicationConversionService.sharedInstance; | ||||||
| 				if (sharedInstance == null) { | 				if (sharedInstance == null) { | ||||||
| 					sharedInstance = new ApplicationConversionService(); | 					sharedInstance = new ApplicationConversionService(null, true); | ||||||
| 					ApplicationConversionService.sharedInstance = sharedInstance; | 					ApplicationConversionService.sharedInstance = sharedInstance; | ||||||
| 				} | 				} | ||||||
| 			} | 			} | ||||||
|  |  | ||||||
|  | @ -21,6 +21,7 @@ import java.util.List; | ||||||
| import java.util.Locale; | import java.util.Locale; | ||||||
| import java.util.Set; | import java.util.Set; | ||||||
| 
 | 
 | ||||||
|  | import org.assertj.core.api.ThrowableAssert.ThrowingCallable; | ||||||
| import org.junit.jupiter.api.Test; | import org.junit.jupiter.api.Test; | ||||||
| 
 | 
 | ||||||
| import org.springframework.context.ConfigurableApplicationContext; | import org.springframework.context.ConfigurableApplicationContext; | ||||||
|  | @ -34,6 +35,7 @@ import org.springframework.format.Parser; | ||||||
| import org.springframework.format.Printer; | import org.springframework.format.Printer; | ||||||
| 
 | 
 | ||||||
| import static org.assertj.core.api.Assertions.assertThat; | import static org.assertj.core.api.Assertions.assertThat; | ||||||
|  | import static org.assertj.core.api.Assertions.assertThatExceptionOfType; | ||||||
| import static org.mockito.Mockito.mock; | import static org.mockito.Mockito.mock; | ||||||
| import static org.mockito.Mockito.verify; | import static org.mockito.Mockito.verify; | ||||||
| import static org.mockito.Mockito.verifyNoMoreInteractions; | import static org.mockito.Mockito.verifyNoMoreInteractions; | ||||||
|  | @ -113,6 +115,28 @@ class ApplicationConversionServiceTests { | ||||||
| 		assertThat(conversionService.isConvertViaObjectSourceType(sourceType, targetType)).isFalse(); | 		assertThat(conversionService.isConvertViaObjectSourceType(sourceType, targetType)).isFalse(); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	@Test | ||||||
|  | 	void sharedInstanceCannotBeModified() { | ||||||
|  | 		ApplicationConversionService instance = (ApplicationConversionService) ApplicationConversionService | ||||||
|  | 				.getSharedInstance(); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addPrinter(null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addParser(null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addFormatter(null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addFormatterForFieldType(null, null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addConverter((Converter<?, ?>) null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addFormatterForFieldType(null, null, null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addFormatterForFieldAnnotation(null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addConverter(null, null, null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addConverter((GenericConverter) null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.addConverterFactory(null)); | ||||||
|  | 		assertUnmodifiableExceptionThrown(() -> instance.removeConvertible(null, null)); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	private void assertUnmodifiableExceptionThrown(ThrowingCallable throwingCallable) { | ||||||
|  | 		assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(throwingCallable) | ||||||
|  | 				.withMessage("This ApplicationConversionService cannot be modified"); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
| 	static class ExampleGenericConverter implements GenericConverter { | 	static class ExampleGenericConverter implements GenericConverter { | ||||||
| 
 | 
 | ||||||
| 		@Override | 		@Override | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue