From 12cd92cb62d6d644781995d0d26e9c76e64cff3e Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 21 Aug 2023 19:08:12 +1000 Subject: [PATCH] WW-5336 Clean up StrutsUtil --- .../org/apache/struts2/util/StrutsUtil.java | 135 ++++++++---------- .../apache/struts2/util/StrutsUtilTest.java | 36 +++-- 2 files changed, 81 insertions(+), 90 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java index f9fc43bf3..70396c46b 100644 --- a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java +++ b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java @@ -18,14 +18,16 @@ */ package org.apache.struts2.util; +import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ObjectFactory; +import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.util.ClassLoaderUtil; import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; +import ognl.OgnlException; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsException; -import org.apache.struts2.views.jsp.ui.OgnlTool; import org.apache.struts2.views.util.UrlHelper; import javax.servlet.RequestDispatcher; @@ -39,7 +41,16 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Hashtable; +import java.util.List; +import java.util.Map; + +import static java.text.MessageFormat.format; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; /** * Struts base utility class, for use in Velocity and Freemarker templates @@ -50,13 +61,12 @@ public class StrutsUtil { protected HttpServletRequest request; protected HttpServletResponse response; - protected Map classes = new Hashtable<>(); - protected OgnlTool ognl; + protected Map> classes = new Hashtable<>(); 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; @@ -67,16 +77,14 @@ public class StrutsUtil { 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) { + clazz = ClassLoaderUtil.loadClass(className, StrutsUtil.class); + classes.put(className, clazz); } - - return objectFactory.buildBean(c, stack.getContext()); + return objectFactory.buildBean(clazz, stack.getContext()); } public boolean isTrue(String expression) { @@ -89,30 +97,20 @@ public class StrutsUtil { } public String include(Object aName) throws Exception { - try { - RequestDispatcher dispatcher = request.getRequestDispatcher(aName.toString()); - - if (dispatcher == null) { - throw new IllegalArgumentException("Cannot find included file " + aName); - } - - ResponseWrapper responseWrapper = new ResponseWrapper(response); - - dispatcher.include(request, responseWrapper); - - return responseWrapper.getData(); - } - catch (Exception e) { - LOG.debug("Cannot include {}", aName, e); - throw e; + RequestDispatcher dispatcher = request.getRequestDispatcher(aName.toString()); + if (dispatcher == null) { + throw new IllegalArgumentException("Cannot find included file " + aName); } + ResponseWrapper responseWrapper = new ResponseWrapper(response); + dispatcher.include(request, responseWrapper); + return responseWrapper.getData(); } public String urlEncode(String s) { try { return URLEncoder.encode(s, "UTF-8"); } catch (UnsupportedEncodingException e) { - LOG.debug("Cannot encode URL [{}]", s, e); + LOG.debug(format("Cannot encode URL [{0}]", s), e); return s; } } @@ -144,7 +142,7 @@ public class StrutsUtil { * @return the url ContextPath. An empty string if one does not exist. */ public String getContext() { - return (request == null)? "" : request.getContextPath(); + return request == null ? "" : request.getContextPath(); } public String translateVariables(String expression) { @@ -168,8 +166,13 @@ public class StrutsUtil { * 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(); + public List makeSelectList(String selectedList, String list, String listKey, String listValue) { + List selectList = new ArrayList<>(); + + Collection items = (Collection) stack.findValue(list); + if (items == null) { + return selectList; + } Collection selectedItems = null; @@ -181,42 +184,27 @@ public class StrutsUtil { } else if (i instanceof Collection) { selectedItems = (Collection) i; } else { - // treat it is a single item - selectedItems = new ArrayList(); - selectedItems.add(i); + selectedItems = singletonList(i); } } - Collection items = (Collection) stack.findValue(list); - - if (items != null) { - for (Object element : items) { - Object key; - - if ((listKey == null) || (listKey.length() == 0)) { - key = element; - } else { - key = ognl.findValue(listKey, element); - } + for (Object element : items) { + Object key; + if (listKey == null || listKey.isEmpty()) { + key = element; + } else { key = findValue(listKey, element); - - Object value = null; - - if ((listValue == null) || (listValue.length() == 0)) { - value = element; - } else { - value = ognl.findValue(listValue, element); - } - - boolean isSelected = false; - - if ((value != null) && (selectedItems != null) && selectedItems.contains(value)) { - isSelected = true; - } - value = findValue(listValue, element); - - selectList.add(new ListEntry(key, value, isSelected)); } + + Object value; + if (listValue == null || listValue.isEmpty()) { + value = element; + } else { + value = findValue(listValue, element); + } + boolean isSelected = value != null && selectedItems != null && selectedItems.contains(value); + + selectList.add(new ListEntry(key, value, isSelected)); } return selectList; @@ -227,14 +215,13 @@ public class StrutsUtil { } public long toLong(int anInt) { - return (long) anInt; + return anInt; } public long toLong(String aLong) { - if (aLong == null) { + if (aLong == null || aLong.isEmpty()) { return 0; } - return Long.parseLong(aLong); } @@ -247,14 +234,7 @@ public class StrutsUtil { } public String toStringSafe(Object obj) { - try { - if (obj != null) { - return String.valueOf(obj); - } - return ""; - } catch (Exception e) { - return "Exception thrown: " + e; - } + return obj == null ? "" : obj.toString(); } static class ResponseWrapper extends HttpServletResponseWrapper { @@ -271,7 +251,6 @@ public class StrutsUtil { public String getData() { writer.flush(); - return strout.toString(); } diff --git a/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java b/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java index 4cf0abb82..c456ab7a3 100644 --- a/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java +++ b/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java @@ -32,6 +32,8 @@ import javax.servlet.ServletResponse; import java.util.ArrayList; import java.util.List; +import static org.assertj.core.api.Assertions.assertThat; + /** * Test case for StrutsUtil. * @@ -146,8 +148,8 @@ public class StrutsUtilTest extends StrutsInternalTestCase { public void testMakeSelectListMethod() { - String[] selectedList = new String[] { "Car", "Airplane", "Bus" }; - List list = new ArrayList(); + String[] selectedList = new String[]{"Car", "Airplane", "Bus"}; + List list = new ArrayList<>(); list.add("Lorry"); list.add("Car"); list.add("Helicopter"); @@ -155,18 +157,18 @@ public class StrutsUtilTest extends StrutsInternalTestCase { stack.getContext().put("mySelectedList", selectedList); stack.getContext().put("myList", list); - List listMade = strutsUtil.makeSelectList("#mySelectedList", "#myList", null, null); + List listMade = strutsUtil.makeSelectList("#mySelectedList", "#myList", null, null); assertEquals(listMade.size(), 3); - assertEquals(((ListEntry)listMade.get(0)).getKey(), "Lorry"); - assertEquals(((ListEntry)listMade.get(0)).getValue(), "Lorry"); - assertFalse(((ListEntry) listMade.get(0)).getIsSelected()); - assertEquals(((ListEntry)listMade.get(1)).getKey(), "Car"); - assertEquals(((ListEntry)listMade.get(1)).getValue(), "Car"); - assertTrue(((ListEntry) listMade.get(1)).getIsSelected()); - assertEquals(((ListEntry)listMade.get(2)).getKey(), "Helicopter"); - assertEquals(((ListEntry)listMade.get(2)).getValue(), "Helicopter"); - assertFalse(((ListEntry) listMade.get(2)).getIsSelected()); + assertEquals(listMade.get(0).getKey(), "Lorry"); + assertEquals(listMade.get(0).getValue(), "Lorry"); + assertFalse(listMade.get(0).getIsSelected()); + assertEquals(listMade.get(1).getKey(), "Car"); + assertEquals(listMade.get(1).getValue(), "Car"); + assertTrue(listMade.get(1).getIsSelected()); + assertEquals(listMade.get(2).getKey(), "Helicopter"); + assertEquals(listMade.get(2).getValue(), "Helicopter"); + assertFalse(listMade.get(2).getIsSelected()); } public void testToInt() { @@ -178,12 +180,22 @@ public class StrutsUtilTest extends StrutsInternalTestCase { assertEquals(strutsUtil.toLong(11), 11L); } + public void testStringToLong() { + assertEquals(11L, strutsUtil.toLong("11")); + assertEquals(0L, strutsUtil.toLong(null)); + assertEquals(0L, strutsUtil.toLong("")); + } public void testToString() { assertEquals(strutsUtil.toString(1), "1"); assertEquals(strutsUtil.toString(11L), "11"); } + public void testToStringSafe() { + assertEquals("1", strutsUtil.toStringSafe(1)); + assertEquals("", strutsUtil.toStringSafe(null)); + } + public void testTranslateVariables() { stack.push(new Object() { public String getFoo() {