apache / apache/grails-core

Invoke registered converters even if types are compatible

Open
#9,950 5 comments 0 reactions 0 assignees View on GitHub
status: acknowledged
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.