SpEL does not expose field-level annotations when setting bean properties [SPR-6745]

[spring-projects-issues]

Keith Donald opened SPR-6745 and commented

This means annotation-driven type conversion does not work when binding to JavaBean properties with SpEL. This can be worked around by defining annotations on the method parameter of a setter method, and the return value of a getter, but is not ideal.

SpEL should be consistent with BeanWrapper with its TypeDescriptor configuration when getting and setting JavaBean properties. This likely means use of a BeanTypeDescriptor in those cases.

This was noticed when attempting to use a SpEL-based data mapper to map String fields in a Map to a JavaBean, where annotation-driven parsing rules were specified on JavaBean fields. This works fine with a BeanWrapper since the BeanTypeDescriptor exposes annotations from the field, getter, and setter. Unfortunately, SpEl just uses the default TypeDescriptor in these cases which only exposes parameter annotations.

---

Affects: 3.0 GA

Issue Links:

• Allow SpEl expressions to be evaluated from JSPs [SPR-6827] #11493 Allow SpEl expressions to be evaluated from JSPs ("is depended on by")
• Create a Unified EL implementation with Spring EL [SPR-6080] #10748 Create a Unified EL implementation with Spring EL

Referenced from: commits 117b138

[spring-projects-issues]

Keith Donald commented

Adding Andy to Watch list.

[spring-projects-issues]

Andy Clement commented

I've made the changes to get Keiths new testcase to pass (EvalTagTests). To summarize the changes:

Had to enable the use of source type descriptors on the typeconverter interface. Had to modify the reflective property accessor to build BeanTypeDescriptors rather than just TypeDescriptors.

---

TypeConverter/StandardTypeConverter

• updated to allow source typedescriptors to flow through it (rather than guessing them from the actual source value)

ExpressionUtils

• factored out some common code from SpelExpression into a second helper method. over time change callers of the first ExpressionUtils method to just use this new one.

SpelNode/SpelNodeImpl

• new method that can return the TypedValue object (the original getValue() unwraps the result value)

SpelExpression

• getValue() calls changed to getTypedValue() calls where the user is requesting a particular result type.
• removed special conversion logic and defer all conversions to ExpressionUtils

BeanTypeDescriptor

• copied (!) from the beans project as we only need it and don't want a new dependency on beans

ReflectiveConstructorExecutor

• bug! Been there always but uncovered now that source descriptors are used in conversions rather than just asking the object what type it is

Various changes throughout where the conversion service was being used so that a source type descriptor is passed.

---

Although Keiths test now passes, more work is needed in ReflectivePropertyAccessor for a few reasons, but I wouldn't do that so close to a release:

• too much duplication in there and its messy
• the typeDescriptorCache looks unnecessary
• BeanTypeDescriptors are built separately for property reads and property writes. I did not want to switch to letting the PropertyDescriptor discover setters/getters (with limited time until release) and I didn't want to introduce the new cost of reflectively hunting for the opposing accessor when it may not be used (so hunting for the setter when a field is being read, just so the BeanTypeDescriptor would be fully populated). This means that metadata attached to the 'other accessor' may not make it through to the conversion. So if you annotate the setter, it wont (I dont think) affect the conversion service call for a property read. And if you annotate the getter, it wont affect the conversion service call for a property write. More thought and testing required here. Keith can probably knock up a test that shows this limitation - I'll need to discuss it with him how much of a likely real world issue it is.

One final thing to mention. SpEL continues to avoid any calls to the conversion service if the result of the expression appears (from a simple isAssignableFrom check) to match what the user asked for. This means no extra metadata that might have been on an accessed property will be used to trigger a format - because the conversion service won't get called. We may want to change this so that the conversion is always called, or at least always called when there is a non-simple source type descriptors, to give it a chance to perform a format.

I would probably close this issue to cover what has been done and open two more to cover the previous two sections that could be looked at when there is more time.

[spring-projects-issues]

Keith Donald commented

Hi Andy,

To your two points. I think we should address both in separate JIRAs as you described, along with cleaning up ReflectivePropertyAccessor, post 3.0.1 when we have more time.

Most users will place the annotation on the field or the getter; setter is rare. Data binding back is presently handled by BeanWrapper, which can already handle the setter case with no issue, so that shouldn't hurt us for general Spring MVC usage. It will be a problem for EL-based mapping using the new mapping framework I've been working on, but that's scheduled for 3.1.

Not respecting format annotation metadata for String properties is more of an issue, since MaskFormatting of Strings (e.g. phone numbers, SSN's) is more of a common thing. With that said, we don't provide any @MaskFormat annotation out of the box yet, so it's up to users to implement their own mask support at present. Furthermore, any such custom mask formatting will work if users use the spring:form tags, since those again use BeanWrapper (only spring:eval usage would not respect such annotations then).

So in summary, as long as we address these remaining EL issues in the next 3.0.x maintenance release, I think we'll be fine.

[spring-projects-issues]

Andy Clement commented

all work planned has now been done for this I believe...

Spin off JIRAs for issues I mentioned are:

https://jira.springsource.org/browse/SPR-6831
"[spel] overhaul ReflectivePropertyAccessor"

https://jira.springsource.org/browse/SPR-6832
"[spel] always call the ConversionService even when getValue() returns value of the expected type, to allow for formatting"