[
https://issues.apache.org/jira/browse/WW-5336?focusedWorklogId=877381&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-877381
]
ASF GitHub Bot logged work on WW-5336:
--------------------------------------
Author: ASF GitHub Bot
Created on: 22/Aug/23 02:06
Start Date: 22/Aug/23 02:06
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #735:
URL: https://github.com/apache/struts/pull/735#discussion_r1300825252
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -124,6 +123,17 @@ public Object findValue(String expression, String
className) throws ClassNotFoun
return stack.findValue(expression, Class.forName(className));
}
+ public Object findValue(String expr, Object context) {
Review Comment:
Copied this here from OgnlTool
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -124,6 +123,17 @@ public Object findValue(String expression, String
className) throws ClassNotFoun
return stack.findValue(expression, Class.forName(className));
}
+ public Object findValue(String expr, Object context) {
+ try {
+ return ognl.getValue(expr,
ActionContext.getContext().getContextMap(), context);
+ } catch (OgnlException e) {
+ if (e.getReason() instanceof SecurityException) {
+ LOG.error(format("Could not evaluate this expression due to
security constraints: [{0}]", expr), e);
Review Comment:
Fixed broken logging
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -50,32 +61,30 @@ public class StrutsUtil {
protected HttpServletRequest request;
protected HttpServletResponse response;
- protected Map<String, Class> classes = new Hashtable<>();
- protected OgnlTool ognl;
+ protected Map<String, Class<?>> classes = new HashMap<>();
Review Comment:
This class is constructed per request thread so I don't believe we need
concurrency handling here
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -50,32 +61,30 @@ public class StrutsUtil {
protected HttpServletRequest request;
protected HttpServletResponse response;
- protected Map<String, Class> classes = new Hashtable<>();
- protected OgnlTool ognl;
+ protected Map<String, Class<?>> classes = new HashMap<>();
+ protected OgnlUtil ognl;
protected ValueStack stack;
- private UrlHelper urlHelper;
- private ObjectFactory objectFactory;
+ private final UrlHelper urlHelper;
+ private final ObjectFactory objectFactory;
public StrutsUtil(ValueStack stack, HttpServletRequest request,
HttpServletResponse response) {
this.stack = stack;
this.request = request;
this.response = response;
- this.ognl =
stack.getActionContext().getContainer().getInstance(OgnlTool.class);
+ this.ognl =
stack.getActionContext().getContainer().getInstance(OgnlUtil.class);
this.urlHelper =
stack.getActionContext().getContainer().getInstance(UrlHelper.class);
this.objectFactory =
stack.getActionContext().getContainer().getInstance(ObjectFactory.class);
}
- public Object bean(Object aName) throws Exception {
- String name = aName.toString();
- Class c = classes.get(name);
-
- if (c == null) {
- c = ClassLoaderUtil.loadClass(name, StrutsUtil.class);
- classes.put(name, c);
+ public Object bean(Object name) throws Exception {
+ String className = name.toString();
+ Class<?> clazz = classes.get(className);
+ if (clazz == null) {
Review Comment:
Using #computeIfAbsent here would change the exception handling semantics so
I've forgone it
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -156,71 +166,64 @@ public String translateVariables(String expression) {
* to use as the value of the ListEntry
* @return a List of ListEntry
*/
- public List makeSelectList(String selectedList, String list, String
listKey, String listValue) {
- List selectList = new ArrayList();
-
- Collection selectedItems = null;
-
- Object i = stack.findValue(selectedList);
-
- if (i != null) {
- if (i.getClass().isArray()) {
- selectedItems = Arrays.asList((Object[]) i);
- } else if (i instanceof Collection) {
- selectedItems = (Collection) i;
- } else {
- // treat it is a single item
- selectedItems = new ArrayList();
- selectedItems.add(i);
- }
- }
+ public List<ListEntry> makeSelectList(String selectedList, String list,
String listKey, String listValue) {
Review Comment:
Fixed cognitive complexity and readability
Issue Time Tracking
-------------------
Worklog Id: (was: 877381)
Time Spent: 1h 10m (was: 1h)
> Merge OgnlTool class into StrutsUtil class
> ------------------------------------------
>
> Key: WW-5336
> URL: https://issues.apache.org/jira/browse/WW-5336
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
> Reporter: Kusal Kithul-Godage
> Priority: Minor
> Fix For: 6.3.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> I can't see any benefit in having OgnlTool as a separate class/bean.
> Will deprecate to ensure backwards compatibility maintained.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
