Custom "Converters" not working with @Command-Annotations

[tschoellhorn]

It seems that custom "Converter<S, T>"-Beans are not registered correctly when running a SpringBoot-Application and SpringShell. I have attached a full working project with test-cases demonstrating the problem: shell.zip

In the previous 3.x-line of SpringShell this was not a issue.

In a nutshell:

We have commands in the form:

@Command(name = "custom")
public void custom(
	@Option(longName = "msg", required = true) Message msg) {	
	System.out.println(msg.toString());
}

In a Component we define a new Bean implementiung the Converter-Interface. This is initialized from SpringBoot before running the actual command - and yet the Converter cannot be found from the CommandExecutor:

org.springframework.shell.core.command.CommandExecutionException: Unable to execute command custom
	at org.springframework.shell.core.command.CommandExecutor.execute(CommandExecutor.java:74) ~[spring-shell-core-4.0.2.jar:4.0.2]
	at org.springframework.shell.core.NonInteractiveShellRunner.executeCommand(NonInteractiveShellRunner.java:158) ~[spring-shell-core-4.0.2.jar:4.0.2]
	at org.springframework.shell.core.NonInteractiveShellRunner.run(NonInteractiveShellRunner.java:95) ~[spring-shell-core-4.0.2.jar:4.0.2]
	at org.springframework.shell.core.autoconfigure.ShellRunnerAutoConfiguration.lambda$springShellApplicationRunner$0(ShellRunnerAutoConfiguration.java:39) ~[spring-shell-core-autoconfigure-4.0.2.jar:4.0.2]
	at org.springframework.boot.SpringApplication.lambda$callRunner$0(SpringApplication.java:788) ~[spring-boot-4.0.6.jar:4.0.6]
	at org.springframework.util.function.ThrowingConsumer$1.acceptWithException(ThrowingConsumer.java:82) ~[spring-core-7.0.7.jar:7.0.7]
	at org.springframework.util.function.ThrowingConsumer.accept(ThrowingConsumer.java:60) ~[spring-core-7.0.7.jar:7.0.7]
	at org.springframework.util.function.ThrowingConsumer$1.accept(ThrowingConsumer.java:86) ~[spring-core-7.0.7.jar:7.0.7]
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:800) ~[spring-boot-4.0.6.jar:4.0.6]
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:788) ~[spring-boot-4.0.6.jar:4.0.6]
	at org.springframework.boot.SpringApplication.lambda$callRunners$0(SpringApplication.java:776) ~[spring-boot-4.0.6.jar:4.0.6]
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[na:na]
	at java.base/java.util.stream.SortedOps$SizedRefSortingSink.end(SortedOps.java:357) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[na:na]
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) ~[na:na]
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:na]
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[na:na]
	at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:776) ~[spring-boot-4.0.6.jar:4.0.6]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:328) ~[spring-boot-4.0.6.jar:4.0.6]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1365) ~[spring-boot-4.0.6.jar:4.0.6]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1354) ~[spring-boot-4.0.6.jar:4.0.6]
	at shell.SpringShellApplication.main(SpringShellApplication.java:14) ~[classes/:na]
Caused by: org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [java.lang.String] to type [shell.SpringShellApplication$Message]
	at org.springframework.core.convert.support.GenericConversionService.handleConverterNotFound(GenericConversionService.java:289) ~[spring-core-7.0.7.jar:7.0.7]
	at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:184) ~[spring-core-7.0.7.jar:7.0.7]
	at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:165) ~[spring-core-7.0.7.jar:7.0.7]
	at org.springframework.shell.core.command.adapter.MethodInvokerCommandAdapter.prepareArguments(MethodInvokerCommandAdapter.java:153) ~[spring-shell-core-4.0.2.jar:4.0.2]
	at org.springframework.shell.core.command.adapter.MethodInvokerCommandAdapter.doExecute(MethodInvokerCommandAdapter.java:93) ~[spring-shell-core-4.0.2.jar:4.0.2]
	at org.springframework.shell.core.command.AbstractCommand.execute(AbstractCommand.java:166) ~[spring-shell-core-4.0.2.jar:4.0.2]
	at org.springframework.shell.core.command.CommandExecutor.execute(CommandExecutor.java:71) ~[spring-shell-core-4.0.2.jar:4.0.2]
	... 23 common frames omitted

From my point of view this should be possible as it is standard Spring-Behaviour to support "Converters".

Additional note: When registering the Command programmitically the Converters are respected. It seems to me, that the Building-process of the Commands via Annotations is the problem.

@Bean
org.springframework.shell.core.command.Command programmatic() {
	Consumer<CommandContext> action = ctx -> 
		System.out.println(ctx.getOptionByName("msg"));
	
	return org.springframework.shell.core.command.Command
		.builder()
		.name("programmatic")
		.options(
			CommandOption.with()
				.longName("msg")
				.type(Message.class).build())
		.execute(action);
}
``

[czpilar]

Fixed in PR #1353

When no ConfigurableConversionService bean is defined CommandFactoryBean falls back to a fresh DefaultConversionService. That fallback now also harvests Converter, GenericConverter and ConverterFactory beans from the application context and registers them on the new service. For Converter<S, T> declared as a @Bean lambda, the runtime class loses its generics, so source/target are resolved from the merged bean definition's ResolvableType and registered via the typed addConverter(source, target, converter) overload.

[tschoellhorn]

Thanks that you could look into the issue. Is there a workaround I could use in the meantime (before the PR is merged into the main line) ?

[czpilar]

Here's a workaround you can add into your application until the fix lands - it replicates the same auto-harvesting logic from the PR as a user-defined ConfigurableConversionService bean, so your existing @Bean Converter<String, Message> lambda from the reproducer keeps working unchanged:

package org.springframework.shell.samples.helloworld.boot;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.core.ResolvableType;
import org.springframework.core.convert.converter.Converter;
import org.springframework.core.convert.converter.ConverterFactory;
import org.springframework.core.convert.converter.GenericConverter;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.shell.core.command.CommandContext;
import org.springframework.shell.core.command.CommandOption;
import org.springframework.shell.core.command.annotation.Command;
import org.springframework.shell.core.command.annotation.Option;

import java.util.function.Function;

@SpringBootApplication
public class SpringShellApplication {

	public static void main(String[] args) {
		SpringApplication.run(SpringShellApplication.class, args);
	}

	@Command(name = "custom")
	public String custom(@Option(longName = "msg", required = true) Message msg) {
		return msg.getMsg();
	}

	@Command(name = "simple")
	public String simple(@Option(longName = "msg", required = true) String msg) {
		return msg;
	}

	@Bean
	org.springframework.shell.core.command.Command programmatic() {
		Function<CommandContext, String> action = ctx -> ctx.getOptionByName("msg").value();
		return org.springframework.shell.core.command.Command.builder()
			.name("programmatic")
			.options(CommandOption.with().longName("msg").type(Message.class).build())
			.execute(action);
	}

	@Bean
	public Converter<String, Message> messageConverter() {
		return source -> {
			Message m = new Message();
			m.setMsg(source);
			return m;
		};
	}

	@Bean
	@SuppressWarnings({"rawtypes", "unchecked"})
	ConfigurableConversionService conversionService(ApplicationContext ctx) {
		DefaultConversionService cs = new DefaultConversionService();
		ctx.getBeansOfType(GenericConverter.class).values().forEach(cs::addConverter);
		ctx.getBeansOfType(ConverterFactory.class).values().forEach(cs::addConverterFactory);
		ctx.getBeansOfType(Converter.class).forEach((name, converter) -> {
			ResolvableType type = ((ctx instanceof ConfigurableApplicationContext cac
				&& cac.getBeanFactory().containsBeanDefinition(name))
				? cac.getBeanFactory().getMergedBeanDefinition(name).getResolvableType()
				: ResolvableType.forClass(converter.getClass()))
				.as(Converter.class);
			Class<?> source = type.getGeneric(0).resolve();
			Class<?> target = type.getGeneric(1).resolve();
			if (source != null && target != null) {
				cs.addConverter(source, target, converter);
			} else {
				cs.addConverter(converter);
			}
		});
		return cs;
	}

	public static class Message {

		String msg;

		public String getMsg() {
			return msg;
		}

		public void setMsg(String msg) {
			this.msg = msg;
		}

	}

}

With registered bean:

Without registered bean:

[tschoellhorn]

Thanks for the workaround. It works great in the testases I provided.

Yet, in the larger project I try to upgrade, the Converter-Beans are more complicated and have dependencies to the database. Thus, they are initialized after these ConversionService and thus have not been published to the ApplicationContext. I tried to remedy that with @dependsOn - but in that case I receive a circular definition:

***************************
APPLICATION FAILED TO START
***************************

Description:

The dependencies of some of the beans in the application context form a cycle:

┌─────┐
|  conversionService defined in class path resource [wdc/crawler3/Config.class]
↑     ↓
|  panelConverter (field private wdc.crawler3.core.dao.PanelDAO wdc.crawler3.core.main.PanelConverter.panelDAO)
↑     ↓
|  panelDAO defined in file [/vagrant/wdc-crawler3/target/classes/wdc/crawler3/core/dao/PanelDAO.class]
↑     ↓
|  getJdbcTemplate defined in class path resource [wdc/crawler3/Config.class]
↑     ↓
|  dataSourceScriptDatabaseInitializer defined in class path resource [org/springframework/boot/jdbc/autoconfigure/DataSourceInitializationAutoConfiguration.class]
↑     ↓
|  spring.sql.init-org.springframework.boot.sql.autoconfigure.init.SqlInitializationProperties
└─────┘

Now, this seems a general issue which might affect also your fix. I did not find an easy way to include that case in the tests. Any ideas how to proceed?

[tschoellhorn]

I could provide a more complex and realistic use case including tests. The main difference is that the Converter-Bean itself depends on another bean (in that case JdbcTemplate). Now it does not get registered with this solution. I guess the real fix will have the same problems. Please find attached the updated test-cases. It has nothing to do with lambda-expressions.

shell.zip

[czpilar]

I tried the reproducer against the PR (4.0.3-SNAPSHOT), removed the workaround first - all three tests pass, even with MessageConverter as a @Component that @Autowireds JdbcTemplate.

About the cycle: it isn't caused by the harvesting logic itself, it's caused by the bean's name and type. A bean named conversionService of type ConfigurableConversionService is also picked up by Spring as the conversion service for @ConfigurationProperties binding. So SqlInitializationProperties needs conversionService, conversionService needs MessageConverter, MessageConverter needs JdbcTemplate, JdbcTemplate needs DataSource, DataSource needs the SQL initializer, the SQL initializer needs SqlInitializationProperties -> loop closed.

The fix doesn't trigger this because its DefaultConversionService is a plain local variable inside CommandFactoryBean#getConfigurableConversionService, never registered as a bean. Spring's property binding keeps using its own default. By the time CommandRegistry is being created and our harvesting runs, DataSource and JdbcTemplate already exist, so MessageConverter instantiates without trouble.

If you want to keep using a workaround until the PR is merged to the main and released, just rename the bean - e.g. @Bean("shellConversionService") ConfigurableConversionService …. CommandFactoryBean looks it up by type so it still finds it, but Spring's property binder only looks for the literal name conversionService, so the cycle breaks.

@Bean("shellConversionService")
@SuppressWarnings({"rawtypes", "unchecked"})
public ConfigurableConversionService conversionService(ApplicationContext ctx) {
...
}

[tschoellhorn]

Thanks for the "fix" of the fix. I did not catch the loop myself - sorry. Now I am confident that your fix also will actually close the issue. Hope your fix will be integrated soon.

[czpilar]

@tschoellhorn No problem. I’m glad to help this project with my contributions, as we use it in our company projects and I also use it in my personal projects.