Updates to OGNL 3.2.x OgnlRuntime #64
Conversation
pom.xml changes: - downgrade javassist 3.24.0-GA -> 3.23.1-GA, to restore compatiblity with JDK 7 Code changes: - added clarifying comments to getStaticField(), cleanup. - added a finally guard in invokeMethod(). - added a finally guard in getFieldValue(), cleanup. - added additional limiter (final) to setFieldValue(), cleanup. - added two test units for protected and public members. - updated PrivateMemberTest with additional tests.
| @@ -68,7 +68,7 @@ | |||
| <dependency> | |||
| <groupId>org.javassist</groupId> | |||
| <artifactId>javassist</artifactId> | |||
| <version>3.24.0-GA</version> | |||
| <version>3.23.1-GA</version> | |||
There was a problem hiding this comment.
Uh... I missed that out :\
There was a problem hiding this comment.
Hello @lukaszlenart .
The only reason I noticed it was I ran the Maven build with JDK7 (and it failed due to "Unsupported major.minor version 52.0" related to Javassist). :)
It seems like they jumped from source/target 1.6 right to 1.8 in 3.24.0-GA (the commit that made the change didn't explain why ...)
There was a problem hiding this comment.
Hello @lukaszlenart .
In addition to the JDK version support levels for Javassist 3.24 vs. 3.23, I ran across a note of possible issues with 3.23.x (jboss-javassist/javassist#224). It's possible that falling back to Javassist 3.22 for OGNL 3.2.x, or opening an issue report with Javassist and asking for restoration of JDK 7 support in 3.25+ might be necessary ...
There was a problem hiding this comment.
Does OGNL even need javassist anymore? I'm not sure if anyone ever used those features other than me in Tapestry 4. It might be safe to just drop that feature set instead. Up to you. =)
There was a problem hiding this comment.
Hello @jkuhnert .
Not sure if anyone out there needs Javassist for OGNL. If neither you nor @lukaszlenart are aware of any active Javassist feature usage, maybe it could marked for deprecation in the OGNL 3.2.x line (or simply phased out in future 3.2.x and noted in the release notes) ?
I was just trying to keep it compiling on JDK 7. :)
There was a problem hiding this comment.
It's used in ExpressionCompiler and I don't know if this can be easily replaced
There was a problem hiding this comment.
Hello @lukaszlenart .
Based on your comment about ExpressionCompiler and a brief look at the code, it certainly looks like Javassist is pretty fundamental to its design...
As a "heads-up" the Javassist team accepted a PR (jboss-javassist/javassist#230) to restore JDK 7 runtime compatibility. That could mean that the next Javassist 3.24.x or 3.25 release might work for the OGNL 3.2.x line, whenever the next Javassist official build is released.
There was a problem hiding this comment.
Great news, thanks a lot!!!
Updates to OGNL 3.2.x
OgnlRuntime:pom.xmlchanges:javassist3.24.0-GA -> 3.23.1-GA, to restore compatiblity with JDK 7.Code changes:
getStaticField(), cleanup.invokeMethod().getFieldValue(), cleanup.setFieldValue(), cleanup.PrivateMemberTestwith additional tests, cleanup.