WW-5534 Simplify ProxyUtil, add OgnlCache#computeIfAbsent #1236
Conversation
kusalk
force-pushed
the
WW-5534-proxyutil
branch
from
bcf569b to
74ec63c
Compare
kusalk
force-pushed
the
WW-5534-proxyutil
branch
from
74ec63c to
8579a10
Compare
|
|
|
| editable = ProxyUtil.ultimateTargetClass(action); | ||
| } | ||
| reflectionProvider.copy(object, action, ctxMap, prepareExcludes(), includes, editable); | ||
| if (!shouldCopy(object)) { |
There was a problem hiding this comment.
Nothing much to see here, just denested this code for slight readability improvement
| <optional>true</optional> | ||
| </dependency> | ||
| <dependency> | ||
| <!-- Optional used in org.apache.struts2.util.ProxyUtil to detect if object is Spring proxy --> |
There was a problem hiding this comment.
I'm using the same optional dependency technique we used for Hibernate proxy resolution - it allows us to avoid reflecting and duplicating code in ProxyUtil leading to improved maintainability
| /** | ||
| * @since 7.1 | ||
| */ | ||
| default V computeIfAbsent(K key, |
There was a problem hiding this comment.
Adding a default implementation preserves API compatibility in case some Struts users have defined their own OgnlCache implementations using the extension points. It allows them to upgrade to Struts 7.1 without requiring any additional changes
| beanInfo = Introspector.getBeanInfo(clazz, Object.class); | ||
| beanInfoCache.putIfAbsent(clazz, beanInfo); | ||
| try { | ||
| return beanInfoCache.computeIfAbsent(clazz, k -> { |
There was a problem hiding this comment.
By removing the synchronized blcok and using #computeIfAbsent we uplift performance as now we only lock on the class name rather than whenever we enter this method
| throw new IllegalArgumentException(e); | ||
| } | ||
| }); | ||
| } catch (IllegalArgumentException e) { |
There was a problem hiding this comment.
This is rather ugly but it's to ensure backwards compatibility with respect to the exception throwing behaviour. The lambda we pass to #computeIfAbsent doesn't allow us to throw checked exceptions
| } | ||
|
|
||
| return result; | ||
| return targetClassCache.computeIfAbsent(candidate, k -> { |
There was a problem hiding this comment.
Added caching here too to uplift SecurityMemberAccess performance for applications which have chosen to allow Hibernate and Spring proxies
| * | ||
| */ | ||
| public class ProxyUtil { | ||
| private static final String SPRING_ADVISED_CLASS_NAME = "org.springframework.aop.framework.Advised"; |
There was a problem hiding this comment.
Removed these fields which were previously used for reflection based code - we now use the optional Spring dependency with catch clauses
I think I must put back the code in GH Actions about branches, in theory Sonar should figure it out automatically, but it doesn't :\ |
2 participants
WW-5534
These changes prepare us for introducing Spring proxy support for the OGNL allowlist and
@StrutsParametercapabilities in a follow-up PR. Refer to PR comments for details on changes.