svn commit: r1812899 - in /ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro: MacroFormRenderer.java MacroScreenRenderer.java

[mbrohl]

Author: mbrohl
Date: Sun Oct 22 11:53:14 2017
New Revision: 1812899

URL: http://svn.apache.org/viewvc?rev=1812899&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.widget.renderer.macro.
(OFBIZ-9702)

Thanks Julian Leichert for reporting and providing the patch.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
    ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroScreenRenderer.java

Modified: ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java?rev=1812899&r1=1812898&r2=1812899&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java Sun Oct 22 11:53:14 2017
@@ -151,9 +151,7 @@ public final class MacroFormRenderer imp
             Template template = new Template(new UID().toString(), templateReader, FreeMarkerWorker.getDefaultOfbizConfig());
             templateReader.close();
             environment.include(template);
-        } catch (TemplateException e) {
-            Debug.logError(e, "Error rendering screen thru ftl macro: " + macro, module);
-        } catch (IOException e) {
+        } catch (TemplateException | IOException e) {
             Debug.logError(e, "Error rendering screen thru ftl, macro: " + macro, module);
         }
     }
@@ -239,23 +237,25 @@ public final class MacroFormRenderer imp
         sr.append(modelFormField.shouldBeRed(context) ? "true" : "false");
         if (ajaxEnabled) {
             String url = inPlaceEditor.getUrl(context);
-            String extraParameter = "{";
+            StringBuffer extraParameterBuffer = new StringBuffer();
+            String extraParameter;
+
             Map<String, Object> fieldMap = inPlaceEditor.getFieldMap(context);
-            if (fieldMap != null) {
-                Set<Entry<String, Object>> fieldSet = fieldMap.entrySet();
-                Iterator<Entry<String, Object>> fieldIterator = fieldSet.iterator();
-                int count = 0;
-                while (fieldIterator.hasNext()) {
-                    count++;
-                    Entry<String, Object> field = fieldIterator.next();
-                    extraParameter += field.getKey() + ":'" + (String) field.getValue() + "'";
-                    if (count < fieldSet.size()) {
-                        extraParameter += ',';
-                    }
+            Set<Entry<String, Object>> fieldSet = fieldMap.entrySet();
+            Iterator<Entry<String, Object>> fieldIterator = fieldSet.iterator();
+            int count = 0;
+            extraParameterBuffer.append("{");
+            while (fieldIterator.hasNext()) {
+                count++;
+                Entry<String, Object> field = fieldIterator.next();
+                extraParameterBuffer.append(field.getKey() + ":'" + (String) field.getValue() + "'");
+                if (count < fieldSet.size()) {
+                    extraParameterBuffer.append(',');
                 }
 
             }
-            extraParameter += "}";
+            extraParameterBuffer.append("}");
+            extraParameter = extraParameterBuffer.toString();
             sr.append("\" inPlaceEditorUrl=\"");
             sr.append(url);
             sr.append("\" inPlaceEditorParams=\"");
@@ -542,7 +542,7 @@ public final class MacroFormRenderer imp
         StringBuilder timeValues = new StringBuilder();
         if (useTimeDropDown && UtilValidate.isNotEmpty(step)) {
             try {
-                step = Integer.valueOf(stepString).intValue();
+                step = Integer.parseInt(stepString);
             } catch (IllegalArgumentException e) {
                 Debug.logWarning("Invalid value for step property for field[" + paramName + "] with input-method=\"time-dropdown\" " + " Found Value [" + stepString + "]  " + e.getMessage(), module);
             }
@@ -1004,7 +1004,6 @@ public final class MacroFormRenderer imp
 
     public void renderCheckField(Appendable writer, Map<String, Object> context, CheckField checkField) throws IOException {
         ModelFormField modelFormField = checkField.getModelFormField();
-        modelFormField.getModelForm();
         String currentValue = modelFormField.getEntry(context);
         Boolean allChecked = checkField.isAllChecked(context);
         String id = modelFormField.getCurrentContainerId(context);
@@ -1066,7 +1065,6 @@ public final class MacroFormRenderer imp
 
     public void renderRadioField(Appendable writer, Map<String, Object> context, RadioField radioField) throws IOException {
         ModelFormField modelFormField = radioField.getModelFormField();
-        modelFormField.getModelForm();
         List<ModelFormField.OptionValue> allOptionValues = radioField.getAllOptionValues(context, WidgetWorker.getDelegator(context));
         String currentValue = modelFormField.getEntry(context);
         String className = "";
@@ -1279,7 +1277,8 @@ public final class MacroFormRenderer imp
                     String fullTarget = target.expandString(context);
                     targetBuffer.append(fullTarget);
                     String targetType = CommonWidgetModels.Link.DEFAULT_URL_MODE;
-                    if (UtilValidate.isNotEmpty(targetBuffer.toString()) && targetBuffer.toString().toLowerCase().startsWith("javascript:")) {
+                    if (UtilValidate.isNotEmpty(targetBuffer.toString()) && targetBuffer.toString().toLowerCase(Locale
+                            .getDefault()).startsWith("javascript:")) {
                         targetType = "plain";
                     }
                     StringWriter sr = new StringWriter();
@@ -1548,14 +1547,14 @@ public final class MacroFormRenderer imp
             renderEndingBoundaryComment(writer, "Grid Widget - Grid Element", modelForm);
         }
     }
-    
+
     public void renderFormatHeaderOpen(Appendable writer, Map<String, Object> context, ModelForm modelForm) throws IOException {
         StringWriter sr = new StringWriter();
         sr.append("<@renderFormatHeaderOpen ");
         sr.append(" />");
         executeMacro(writer, sr.toString());
     }
-    
+
     public void renderFormatHeaderClose(Appendable writer, Map<String, Object> context, ModelForm modelForm) throws IOException {
         StringWriter sr = new StringWriter();
         sr.append("<@renderFormatHeaderClose");
@@ -1643,7 +1642,7 @@ public final class MacroFormRenderer imp
         sr.append(" formName=\"");
         sr.append(modelForm.getName());
         sr.append("\" itemIndex=");
-        sr.append(Integer.toString(itemIndex));
+        sr.append(String.valueOf(itemIndex));
         sr.append(" altRowStyles=\"");
         sr.append(altRowStyles);
         sr.append("\" evenRowStyle=\"");
@@ -2028,7 +2027,7 @@ public final class MacroFormRenderer imp
                 value2 = (String) parameters.get(name + "_fld1_value");
             }
         }
-        
+
         String titleStyle = "";
         if (UtilValidate.isNotEmpty(modelFormField.getTitleStyle())) {
             titleStyle = modelFormField.getTitleStyle();
@@ -2343,10 +2342,7 @@ public final class MacroFormRenderer imp
         if (UtilValidate.isNotEmpty(queryString)) {
             queryString = UtilHttp.encodeAmpersands(queryString);
         }
-        if (prepLinkText == null) {
-            prepLinkText = "";
-        }
-        if (prepLinkText.indexOf("?") < 0) {
+        if (prepLinkText.indexOf('?') < 0) {
             prepLinkText += "?";
         } else if (!prepLinkText.endsWith("?")) {
             prepLinkText += "&amp;";
@@ -2590,7 +2586,7 @@ public final class MacroFormRenderer imp
         if (!passwordField.getClientAutocompleteField()) {
             autocomplete = "off";
         }
-        
+
         //check for required field style on single forms
         if ("single".equals(modelFormField.getModelForm().getType()) && modelFormField.getRequiredField()) {
             String requiredStyle = modelFormField.getRequiredFieldStyle();
@@ -2601,7 +2597,7 @@ public final class MacroFormRenderer imp
             else
                 className = requiredStyle + " " + className;
         }
-        
+
         String tabindex = modelFormField.getTabindex();
         StringWriter sr = new StringWriter();
         sr.append("<@renderPasswordField ");
@@ -2966,12 +2962,11 @@ public final class MacroFormRenderer imp
             Map<String, Object> ctx = UtilGenerics.checkMap(context);
             Map<String, String> parameters = updateArea.getParameterMap(ctx);
             String targetUrl = updateArea.getAreaTarget(context);
-            String ajaxParams = getAjaxParamsFromTarget(targetUrl);
+            String ajaxParams;
+            StringBuffer ajaxParamsBuffer = new StringBuffer();
+            ajaxParamsBuffer.append(getAjaxParamsFromTarget(targetUrl));
             //add first parameters from updateArea parameters
             if (UtilValidate.isNotEmpty(parameters)) {
-                if (UtilValidate.isEmpty(ajaxParams)) {
-                    ajaxParams = "";
-                }
                 for (Map.Entry<String, String> entry : parameters.entrySet()) {
                     String key = entry.getKey();
                     String value = entry.getValue();
@@ -2979,21 +2974,22 @@ public final class MacroFormRenderer imp
                     if (UtilValidate.isNotEmpty(extraParams) && extraParams.contains(value)) {
                         continue;
                     }
-                    if (ajaxParams.length() > 0 && ajaxParams.indexOf(key) < 0) {
-                        ajaxParams += "&";
+                    if (ajaxParamsBuffer.length() > 0 && ajaxParamsBuffer.indexOf(key) < 0) {
+                        ajaxParamsBuffer.append("&");
                     }
-                    if (ajaxParams.indexOf(key) < 0) {
-                        ajaxParams += key + "=" + value;
+                    if (ajaxParamsBuffer.indexOf(key) < 0) {
+                        ajaxParamsBuffer.append(key).append("=").append(value);
                     }
                 }
             }
             //then add parameters from request. Those parameters could end with an anchor so we must set ajax parameters first
             if (UtilValidate.isNotEmpty(extraParams)) {
-                if (ajaxParams.length() > 0 && !extraParams.startsWith("&")) {
-                    ajaxParams += "&";
+                if (ajaxParamsBuffer.length() > 0 && !extraParams.startsWith("&")) {
+                    ajaxParamsBuffer.append("&");
                 }
-                ajaxParams += extraParams;
+                ajaxParamsBuffer.append(extraParams);
             }
+            ajaxParams = ajaxParamsBuffer.toString();
             ajaxUrl += updateArea.getAreaId() + ",";
             ajaxUrl += this.rh.makeLink(this.request, this.response, UtilHttp.removeQueryStringFromTarget(targetUrl));
             ajaxUrl += "," + ajaxParams;
@@ -3088,7 +3084,7 @@ public final class MacroFormRenderer imp
         if ("hidden-form".equals(realLinkType)) {
             parameterMap.put(viewIndexField, Integer.toString(viewIndex));
             parameterMap.put(viewSizeField, Integer.toString(viewSize));
-            if (modelFormField != null && "multi".equals(modelForm.getType())) {
+            if ("multi".equals(modelForm.getType())) {
                 WidgetWorker.makeHiddenFormLinkAnchor(writer, linkStyle, encodedDescription, confirmation, modelFormField, request, response, context);
                 // this is a bit trickier, since we can't do a nested form we'll have to put the link to submit the form in place, but put the actual form def elsewhere, ie after the big form is closed
                 Map<String, Object> wholeFormContext = UtilGenerics.checkMap(context.get("wholeFormContext"));
@@ -3193,7 +3189,7 @@ public final class MacroFormRenderer imp
             sr.append("linkStyle=\"");
             sr.append(linkStyle == null ? "" : linkStyle);
             sr.append("\" hiddenFormName=\"");
-            sr.append(hiddenFormName == null ? "" : hiddenFormName);
+            sr.append(hiddenFormName);
             sr.append("\" event=\"");
             sr.append(event);
             sr.append("\" action=\"");
@@ -3245,7 +3241,7 @@ public final class MacroFormRenderer imp
             sr.append("linkStyle=\"");
             sr.append(linkStyle == null ? "" : linkStyle);
             sr.append("\" hiddenFormName=\"");
-            sr.append(hiddenFormName == null ? "" : hiddenFormName);
+            sr.append(hiddenFormName);
             sr.append("\" event=\"");
             sr.append(event);
             sr.append("\" action=\"");

Modified: ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroScreenRenderer.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroScreenRenderer.java?rev=1812899&r1=1812898&r2=1812899&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroScreenRenderer.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroScreenRenderer.java Sun Oct 22 11:53:14 2017
@@ -442,7 +442,7 @@ public class MacroScreenRenderer impleme
         String enableEditName = content.getEnableEditName(context);
         String enableEditValue = (String)context.get(enableEditName);
         String urlString = "";
-        if (editRequest != null && editRequest.toUpperCase().indexOf("IMAGE") > 0) {
+        if (editRequest != null && editRequest.toUpperCase(Locale.getDefault()).indexOf("IMAGE") < 0) {
             editMode += " Image";
         }
 
@@ -450,7 +450,8 @@ public class MacroScreenRenderer impleme
             HttpServletResponse response = (HttpServletResponse) context.get("response");
             HttpServletRequest request = (HttpServletRequest) context.get("request");
             if (request != null && response != null) {
-                if (editRequest.indexOf("?") < 0)  editRequest += "?";
+                if (editRequest.indexOf('?') < 0)
+                    editRequest += "?";
                 else editRequest += "&amp;";
                 editRequest += "contentId=" + expandedContentId;
                 ServletContext ctx = (ServletContext) request.getAttribute("servletContext");
@@ -554,14 +555,15 @@ public class MacroScreenRenderer impleme
          String expandedContentId = content.getContentId(context);
          String expandedMapKey = content.getMapKey(context);
          String urlString = "";
-         if (editRequest != null && editRequest.toUpperCase().indexOf("IMAGE") > 0) {
+        if (editRequest != null && !(editRequest.toUpperCase(Locale.getDefault()).indexOf("IMAGE") < 1)) {
              editMode += " Image";
          }
          if (UtilValidate.isNotEmpty(editRequest) && "true".equals(enableEditValue)) {
              HttpServletResponse response = (HttpServletResponse) context.get("response");
              HttpServletRequest request = (HttpServletRequest) context.get("request");
              if (request != null && response != null) {
-                 if (editRequest.indexOf("?") < 0)  editRequest += "?";
+                if (editRequest.indexOf('?') < 0)
+                    editRequest += "?";
                  else editRequest += "&amp;";
                  editRequest += "contentId=" + expandedContentId;
                  if (UtilValidate.isNotEmpty(expandedMapKey)) {
@@ -728,7 +730,7 @@ public class MacroScreenRenderer impleme
             Debug.logWarning("Could not find uiLabelMap in context", module);
         } else {
             ofLabel = uiLabelMap.get("CommonOf");
-            ofLabel = ofLabel.toLowerCase();
+            ofLabel = ofLabel.toLowerCase(Locale.getDefault());
         }
 
         // for legacy support, the viewSizeParam is VIEW_SIZE and viewIndexParam is VIEW_INDEX when the fields are "viewSize" and "viewIndex"
@@ -759,7 +761,7 @@ public class MacroScreenRenderer impleme
         // preparing the link text, so that later in the code we can reuse this and just add the viewIndex
         String prepLinkText = "";
         prepLinkText = targetService;
-        if (prepLinkText.indexOf("?") < 0) {
+        if (prepLinkText.indexOf('?') < 0) {
             prepLinkText += "?";
         } else if (!prepLinkText.endsWith("?")) {
             prepLinkText += "&amp;";
@@ -870,7 +872,7 @@ public class MacroScreenRenderer impleme
         String addPortletHint = "";
         String colWidthLabel = "";
         String setColumnSizeHint = "";
-        
+
         if (uiLabelMap == null) {
             Debug.logWarning("Could not find uiLabelMap in context", module);
         } else {
@@ -917,7 +919,7 @@ public class MacroScreenRenderer impleme
         sr.append(setColumnSizeHint);
         sr.append("\" />");
         executeMacro(writer, sr.toString());
-    }  
+    }
 
     public void renderPortalPageColumnEnd(Appendable writer, Map<String, Object> context, ModelScreenWidget.PortalPage portalPage, GenericValue portalPageColumn) throws GeneralException, IOException {
         StringWriter sr = new StringWriter();
@@ -1024,7 +1026,11 @@ public class MacroScreenRenderer impleme
                 throw new RuntimeException(errMsg);
             }
         }
-        modelScreen.renderScreenString(writer, context, this);
+        if (writer != null && context != null) {
+            modelScreen.renderScreenString(writer, context, this);
+        } else {
+            Debug.logError("Null on some Path: writer" + writer + ", context: " + context, module);
+        }
     }
 
     @Override
@@ -1059,7 +1065,7 @@ public class MacroScreenRenderer impleme
         }
         executeMacro(writer, "<@renderColumnContainerEnd />");
     }
-    
+
     // This is a util method to get the style from a property file
     public static String getFoStyle(String styleName) {
         String value = UtilProperties.getPropertyValue("fo-styles", styleName);