Spring Framework Version:5.3.1
When I was writing the formatter special column. I need to write a demo about the detailed use of FormatterRegistry
. You know
Formattingconversionservice
can be considered as its unique implementation.
In the process of using Formattingconversionservice
, I found a bug in it by accident. Here is my demo source code:
@Data
@NoArgsConstructor
@AllArgsConstructor
public class Person {
private Integer id;
private String name;
}
public class Demo {
// Customize a parser: String -> Integer
private static class IntegerParser implements Parser<Integer> {
@Override
public Integer parse(String text, Locale locale) throws ParseException {
return NumberUtils.parseNumber(text, Integer.class);
}
}
@Test
public void test4() {
FormattingConversionService formattingConversionService = new FormattingConversionService();
FormatterRegistry formatterRegistry = formattingConversionService;
ConversionService conversionService = formattingConversionService;
// 注册格式化器
formatterRegistry.addFormatterForFieldType(Person.class, null, new IntegerParser());
formatterRegistry.addConverter(new Converter<Integer, Person>() {
@Override
public Person convert(Integer source) {
return new Person(source, "YourBatman");
}
});
System.out.println(conversionService.canConvert(String.class, Person.class));
System.out.println(conversionService.convert("1", Person.class));
}
}
Run the program,throw an NullPointerException
exception:
java.lang.NullPointerException
at org.springframework.format.support.FormattingConversionService$PrinterConverter.resolvePrinterObjectType(FormattingConversionService.java:179)
at org.springframework.format.support.FormattingConversionService$PrinterConverter.<init>(FormattingConversionService.java:155)
at org.springframework.format.support.FormattingConversionService.addFormatterForFieldType(FormattingConversionService.java:95)
at cn.yourbatman.formatter.Demo.test4(Demo.java:86)
...
The location of the exception is obvious. In FormattingConversionService:
FormattingConversionService:
@Override
public void addFormatterForFieldType(Class<?> fieldType, Printer<?> printer, Parser<?> parser) {
addConverter(new PrinterConverter(fieldType, printer, this));
addConverter(new ParserConverter(fieldType, parser, this));
}
private static class PrinterConverter implements GenericConverter {
...
public PrinterConverter(Class<?> fieldType, Printer<?> printer, ConversionService conversionService) {
this.fieldType = fieldType;
// Where the NullPointerException exception occurred
this.printerObjectType = TypeDescriptor.valueOf(resolvePrinterObjectType(printer));
this.printer = printer;
this.conversionService = conversionService;
}
...
}
If I modify my demo just like this:
@Test
public void test4() {
...
// 注册格式化器
// just use new IntegerPrinter() instead of null for this second param
formatterRegistry.addFormatterForFieldType(Person.class, new IntegerPrinter(), new IntegerParser());
...
}
Run the program again.Normal output:
true
Person(id=1, name=YourBatman)
I think the implementation of FormattingConversionService
is not rigorous. I just want to register a Parser
for specified type, but Printer
doesn't need. Even if I specify the second param null must also work natural.There's no reason to tie them together.
So I think it's a bug.I have the following suggestions to fix it:
FormattingConversionService:
@Override
public void addFormatterForFieldType(Class<?> fieldType, Printer<?> printer, Parser<?> parser) {
if (printer != null) {
addConverter(new PrinterConverter(fieldType, printer, this));
}
if (parser != null) {
addConverter(new ParserConverter(fieldType, parser, this));
}
}
If printer/parser is null,There's no point in registering
PrinterConverter/ParserConverter
in theConverterRegistry
.InFormattingConversionService
all registered places need to be judge null.
In addition, at the API level,I suggest adding the following two APIs:
FormatterRegistry:
void addPrinter(Class<?> fieldType, Printer<?> printer);
void addParser(Class<?> fieldType, Parser<?> parser);
Thanks for reading my advice.
Comment From: snicoll
The NullPointerException
is legit. The printer
argument is not marked as @Nullable
so it must be provided. Not providing it is a user error so I am not sure why you're reporting a bug for this.