Invoke registered converters even if types are compatible
- Dominant language
- Groovy
- Stars
- 2.9k
- Forks
- 975
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 92
Description
I wrote the following converter which I hoped would convert phone numbers to a canonical format
``` groovy
@Log4j
class PhoneNumberConverter implements ValueConverter {
@Override
boolean canConvert(Object number) {
number instanceof String
}
@Override
Object convert(Object number) {
'00' + number
}
@Override
Class getTargetType() {
String
}
}
```
I registered this in `resources.groovy`, but it wasn't being invoked by the databinder. I asked about this on Slack channel and @jeffbrown said
> it is a scenario that hasn’t been accounted for. I would have to investigate to validate this but what I expect is happening is that the converters are only engaged if the value being bound isn’t compatible with the type of reference it is being bound to, which is not the case in your scenario. Feel free to file a feature request and we can investigate possibilities.
So it seems that converters are only invoked if the original value and the target type are incompatible. It wasn't at all obvious to me from the docs that this is the case, but it means that using converters for String -> String transformations is impossible
I'm proposing to change the behaviour, such that converters are always invoked if `canConvert` returns true. i.e. even if the orginal value and the target of the binding are compatible.
Rather than applying this change to all registered `ValueConverter` instances - which might be a breaking change - it might be preferable to introduce a new type, e.g.
``` java
public interface ValueTransformer extends ValueConverter {}
```
and only apply the behavioural change described above to `ValueTransformer` instances. You could add a default method to `ValueConverter` instead of introducing a new type, but of course this would only work on JDK 8.
Contributor guide
Research direction
Start by tracing how the databinder selects registered ValueConverter instances and review the registration in resources.groovy. Determine how String-to-String transformations should be invoked, then add coverage showing the proposed behavior for compatible types and confirm existing converter behavior is preserved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- groovy
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100