|
Here's a patch against XWork 2.0.4 that provides the behavior I'm looking for. It seems like a dirty hack, especially where it's traversing Action classes and looking for properties. The good news is it does work - maybe someone can take these ideas and clean up the implementation? ;-)
Do you have access to xwork? If you don't, you might want to talk to Rainer (not really sure how getting commits rights to xwork works for S2 committers). In the meanwhile I can commit it if you want.
This seems to be a more robust patch. However, it might have performance implications so I'll leave it up to developers of this project to accept it or not.
Example application that proves patch works OK - uses Struts 2.0.12-SNAPSHOT and XWork 2.0.4 (with patch).
Thanks Matt for the patch. Applied to SVN trunk as rev 1647 and for xwork 2.1.0
I'm having second thoughts about this fix. It seems wrong that you'd get a warning thrown when calling a getter, which is basically what findValue is, especially since there isn't a valueExists() method to try to check for the value first. Also, the warning isn't thrown on the findValue(String,Class), which is basically the same, but tries to convert the found value into a certain type.
It seems the best place to be logging a warning would be in the calling code, because many times, it is fine if there is no value found. Always throwing a warning seems to be a misuse of logging to me. I'm not convinced my implementation is a good one. However, I do believe the functionality is necessary. Of all the frameworks I examined, Struts 2 was the most deficient in this area.
http://raibledesigns.com/rd/entry/does_struts_2_suck ;-) Reading more, I agree it is important to throw this error, but it is a bit more complicated than that. In all the examples you give in your blog post, you are using the EL to retrieve a property for a specific object. However, in the Struts 2 example, you are retrieving a property from the value stack, which is basically a list of objects. There are two cases where the findValue() method would return null:
1. The property cannot be found on any objects in the value stack 2. The property can be found, but is null These are two different cases and only in case #1 do you want an error. In fact, only in some cases you want an error from #1. Take this example of a stack: Map > MyActionClass If I asked for "foo", s2 would first try to call map.get("foo"), and if was null, then try to find the getFoo() getter on the MyActionClass object. If both are null because the property didn't exist, should an error really be thrown? Maybe we were really just looking for the property in the map and never expected the property to be in the Action. This is exactly how the tags work, as they put the tag parameters into a map and push it on the stack, then later, findValue() is called. It is never expected to find the properties on the action, so should an exception or warning be thrown? So implementation aside, I think this should happen: 1. If findValue() finds a property but it is null, no warning should be thrown 2. If findValue() can't find a getter in even just one object in the stack, an exception should be thrown 3. We stop using the stack to resolve tag parameters in Struts 2 Fixed as part of https://issues.apache.org/struts/browse/WW-3051
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Resolved
Major
Add ability to log or throw exceptions for expressions that contain non-existant properties or invalid methods
http://www.nabble.com/How-can-I-tell-Struts-2-to-throw-log-exceptions-for-invalid-OGNL-Expressions-tf4100102.html