Delegate to ConversionService in DefaultInputObjectMapper #1735
Conversation
Switch to using a ConversionService from Spring to perform conversion in DefaultInputObjectMapper; this greatly simplifies the implementation, and allows for extensibility in the future.
| @@ -156,28 +156,6 @@ internal class InputObjectMapperTest { | |||
| assertThat(mapToObject.simpleString).isEqualTo("hello") | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Are these tests not relevant anymore? Did the behavior change?
There was a problem hiding this comment.
Yes, the behavior is different now that we're delegating to the ConversionService; if there's a registered type converter for the source and target type, we will perform a conversion instead of failing if possible, whereas previously the conversion was explicitly implemented for a narrower set of types (Map, Collections, Enum).
There was a problem hiding this comment.
I added a test case for this new behavior.
| } else { | ||
| trySetField(declaredField, instance, newList) | ||
| } | ||
| } else if (fieldClass.isEnum) { |
There was a problem hiding this comment.
I think we don't have a test covering the enum field case. Should we add one for that as well?
There was a problem hiding this comment.
There is coverage for Enums already in InputArgumentTest.
There was a problem hiding this comment.
I went ahead and added an additional test case for this.
|
Since this is close to the quiet period freeze, should we perhaps hold off on merging this till after? |
Add a test case covering an Enum field, as well as conversion of an input type that doesn't match the target type exactly (e.g., List<String> -> Set<Int>).
Switch to using a ConversionService from Spring to perform conversion in DefaultInputObjectMapper; this greatly simplifies the implementation, and allows for extensibility in the future.