-
Notifications
You must be signed in to change notification settings - Fork 37.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SpEL regression with ArrayList to int array conversion [SPR-7519] #12177
Comments
Chris Beams commented Add Mark and Juergen as watchers |
Andy Clement commented Just my observation so far. When the failure occurs I see that the 'and(int[])' method from the testcase hasn't been selected by the SpEL reflective method resolver because the type converter says it cannot convert from ArrayList to int[]. (ReflectionHelper line 76). No idea if that is a new problem or not... |
Chris Beams commented Does that suggest that it's Mark's problem? Are you still investigating? Please reassign if appropriate. |
Juergen Hoeller commented That could very well be the problem: The ConversionService only claims to convert a List to an array of X now if it either knows the actual element type of the source List (by passing a rich TypeDescriptor in, not just ArrayList.class), or if it has a general Object-to-X converter for the target array element type X. Juergen |
Juergen Hoeller commented I've assigned the issue to myself since I'm pretty sure it is caused by that ConversionService change in 3.0.4... Thanks for researching it, Andy - that helped a lot. Juergen |
Andy Clement commented ok Juergen. To flow the rich typedescriptors down from the MethodReference.getValueInternal() to the actual ReflectiveMethodResolver (if that is what you want to do) looks a bit messy but is probably the right thing to do. The rich descriptor looks like it is an java.util.Collection<org.springframework.integration.Message> so hope that converts to an int[] ok... |
Juergen Hoeller commented I hope that they got a Message-to-int converter registered there in Spring Integration... although that looks a bit odd, doesn't it. Basically we got two choices: Either make the ConversionService more lenient in terms of canConvert (again) which potentially leads to over-eager canConvert=true results (which in turn may break an algorithm that has some fallback after the canConvert check, like our TypeConverterDelegate in the beans module). Or consistently pass the rich TypeDescriptor through instead of the Class argument, as you suggest. I agree that it's the right thing to do in any case since we do not want to lose that detailed context just because we do some internal delegation there. Juergen |
Juergen Hoeller commented Maybe a helpful guideline: With rich context involved, nobody should call TypeConverter.canConvert(Class, Class) but rather always TypeConverter.canConvert(TypeDescriptor, TypeDescriptor) instead. A quick check reveals that it's only really ReflectionHelper calling the Class variant, so when we'd consistently use the TypeDescriptor variant there, we could even drop canConvert(Class, Class) from the expression package's TypeConverter SPI completely. That would avoid accidental use of the Class variant on the way forward (within the expression parser code, that is). Juergen |
Juergen Hoeller commented Seems to work quite well... The only problem is that it requires a signature change in MethodResolver and ConstructorResolver, passing in List<TypeDescriptor> instead of Class[]. Are there any known custom implementations of those SPIs? Otherwise I'd have no concerns changing those, since they're quite internal. Juergen |
Mark Fisher commented Juergen, we no longer even have a custom implementation of MethodResolver in Spring Integration. We did earlier, but once the MethodFilter support was added, we were able to refactor based on that more focused strategy. |
Juergen Hoeller commented This should be fixed now: We're passing the full TypeDescriptor context through to ConversionService calls. Unfortunately this meant changing the ConstructorResolver and MethodResolver interfaces; as far as I investigated, nobody seems to depend on those, so we're hopefully fine there. Feel free to give tonight's 3.0.5 snapshot a try with that specific Spring Integration test case... Juergen |
Chris Beams opened SPR-7519 and commented
I just discovered what appears to be a regression in SpEL support when attempting to upgrade Spring Integration from depending on Spring 3.0.3 to Spring 3.0.4.
See the stack trace here: http://build.springframework.org/browse/INT-TRUNK-3075/test/case/63242567
This can be reproduced with the following steps:
shouldFindSimpleAggregatorMethodWithArray
I suspect the regression occurred in one of the following two commits that have happened since 3.0.3 was released:
Perhaps specifically in Andy's latest
changes to org.springframework.expression/src/main/java/org/springframework/expression/spel/SpelMessage.java
?Affects: 3.0.4, 3.0.5
Issue Links:
Referenced from: commits c33df59
The text was updated successfully, but these errors were encountered: