svn commit: r1817722 - in /ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content: ContentManagementEvents.java ContentManagementServices.java ContentManagementWorker.java ConvertTree.java

[mbrohl]

Author: mbrohl
Date: Sun Dec 10 17:51:09 2017
New Revision: 1817722

URL: http://svn.apache.org/viewvc?rev=1817722&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.content.
(OFBIZ-9858)

The patch was modified. Instead of removing variables which were only
intialized with null but used in method calls, I've added some TODO
tags to check them.

Thanks Dennis Balkir for reporting and providing the patch.

Modified:
    ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementEvents.java
    ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
    ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementWorker.java
    ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ConvertTree.java

Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementEvents.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementEvents.java?rev=1817722&r1=1817721&r2=1817722&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementEvents.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementEvents.java Sun Dec 10 17:51:09 2017
@@ -22,6 +22,7 @@ import java.sql.Timestamp;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -157,6 +158,7 @@ public class ContentManagementEvents {
             roleTypeList = StringUtil.split(roles, "|");
         }
         List<String> targetOperationList = UtilMisc.<String>toList("CONTENT_PUBLISH");
+        // TODO check the purpose of this list and if we want to make use of it. Else remove
         List<String> contentPurposeList = null; //UtilMisc.toList("ARTICLE");
         String permittedAction = (String)paramMap.get("permittedAction"); // The content to be linked to one or more sites
         String permittedOperations = (String)paramMap.get("permittedOperations"); // The content to be linked to one or more sites
@@ -182,7 +184,8 @@ public class ContentManagementEvents {
         // make a map of the values that are passed in using the top subSite as the key.
         // Content can only be linked to one subsite under a top site (ends with "_MASTER")
         Map<String, String> siteIdLookup = new HashMap<String, String>();
-        for (String param : paramMap.keySet()) {
+        for (Entry<String, Object> entry : paramMap.entrySet()) {
+            String param = entry.getKey();
             int pos = param.indexOf("select_");
             if (pos >= 0) {
                 String siteId = param.substring(7);
@@ -238,6 +241,7 @@ public class ContentManagementEvents {
                         serviceIn.put("contentIdTo", currentSubContentId);
                         serviceIn.put("roleTypeList", roleTypeList);
                         serviceIn.put("targetOperationList", targetOperationList);
+                        // TODO check if this should be removed (see above)
                         serviceIn.put("contentPurposeList", contentPurposeList);
                         results = dispatcher.runSync("createContentAssoc", serviceIn);
                         responseMessage = (String)results.get(ModelService.RESPONSE_MESSAGE);
@@ -257,6 +261,7 @@ public class ContentManagementEvents {
                         serviceIn.put("contentIdTo", contentId);
                         serviceIn.put("roleTypeList", roleTypeList);
                         serviceIn.put("targetOperationList", targetOperationList);
+                        // TODO check if this should be removed (see above)
                         serviceIn.put("contentPurposeList", contentPurposeList);
                         results = dispatcher.runSync("createContentAssoc", serviceIn);
                         if (!statusIdUpdated) {

Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java?rev=1817722&r1=1817721&r2=1817722&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java Sun Dec 10 17:51:09 2017
@@ -234,7 +234,7 @@ public class ContentManagementServices {
             Debug.logInfo("in persist... dataResourceTypeId(0):" + dataResourceTypeId, module);
         }
         if (UtilValidate.isNotEmpty(dataResourceTypeId)) {
-            Map<String, Object> dataResourceResult = new HashMap<String, Object>();
+            Map<String, Object> dataResourceResult;
             try {
                 dataResourceResult = persistDataResourceAndDataMethod(dctx, context);
             } catch (GenericServiceException e) {
@@ -310,12 +310,10 @@ public class ContentManagementServices {
             // Add ContentPurposes if this is a create operation
             if (contentId != null && !contentExists) {
                 try {
-                    if (contentPurposeList != null) {
-                        Set<String> contentPurposeSet = UtilMisc.makeSetWritable(contentPurposeList);
-                        for (String contentPurposeTypeId : contentPurposeSet) {
-                            GenericValue contentPurpose = delegator.makeValue("ContentPurpose", UtilMisc.toMap("contentId", contentId, "contentPurposeTypeId", contentPurposeTypeId));
-                            contentPurpose.create();
-                        }
+                    Set<String> contentPurposeSet = UtilMisc.makeSetWritable(contentPurposeList);
+                    for (String contentPurposeTypeId : contentPurposeSet) {
+                        GenericValue contentPurpose = delegator.makeValue("ContentPurpose", UtilMisc.toMap("contentId", contentId, "contentPurposeTypeId", contentPurposeTypeId));
+                        contentPurpose.create();
                     }
                 } catch (GenericEntityException e) {
                     return ServiceUtil.returnError(e.toString());
@@ -502,7 +500,7 @@ public class ContentManagementServices {
     public static Map<String, Object> persistDataResourceAndData(DispatchContext dctx, Map<String, ? extends Object> context) {
       LocalDispatcher dispatcher = dctx.getDispatcher();
       Locale locale = (Locale) context.get("locale");
-      Map<String, Object> result = new HashMap<String, Object>();
+      Map<String, Object> result;
       try {
           ModelService checkPermModel = dispatcher.getDispatchContext().getModelService("checkContentPermission");
           Map<String, Object> ctx = checkPermModel.makeValid(context, ModelService.IN_PARAM);
@@ -984,7 +982,7 @@ public class ContentManagementServices {
         String contentId = (String)context.get("contentId");
         visitedSet.add(contentId);
         String contentTypeId = "PAGE_NODE";
-        if (pageMode != null && pageMode.toLowerCase().indexOf("outline") >= 0)
+        if (pageMode != null && pageMode.toLowerCase(Locale.getDefault()).indexOf("outline") >= 0)
             contentTypeId = "OUTLINE_NODE";
         GenericValue thisContent = null;
         try {
@@ -1023,7 +1021,7 @@ public class ContentManagementServices {
         String contentId = (String)context.get("contentId");
         String pageMode = (String)context.get("pageMode");
         String contentTypeId = "OUTLINE_NODE";
-        if (pageMode != null && pageMode.toLowerCase().indexOf("page") >= 0) {
+        if (pageMode != null && pageMode.toLowerCase(Locale.getDefault()).indexOf("page") >= 0) {
             contentTypeId = "PAGE_NODE";
         }
         GenericValue thisContent = null;
@@ -1360,7 +1358,7 @@ public class ContentManagementServices {
 
     public static Map<String, Object> updateContentSubscriptionByProduct(DispatchContext dctx, Map<String, ? extends Object> rcontext) throws GenericServiceException{
         Map<String, Object> context = UtilMisc.makeMapWritable(rcontext);
-        Map<String, Object> result = new HashMap<String, Object>();
+        Map<String, Object> result;
         Delegator delegator = dctx.getDelegator();
         Locale locale = (Locale) context.get("locale");
         LocalDispatcher dispatcher = dctx.getDispatcher();
@@ -1370,10 +1368,6 @@ public class ContentManagementServices {
             qty = Integer.valueOf(1);
         }
 
-        Timestamp orderCreatedDate = (Timestamp) context.get("orderCreatedDate");
-        if (orderCreatedDate == null) {
-            orderCreatedDate = UtilDateTime.nowTimestamp();
-        }
         GenericValue productContent = null;
            try {
             List<GenericValue> lst = EntityQuery.use(delegator).from("ProductContent")

Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementWorker.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementWorker.java?rev=1817722&r1=1817721&r2=1817722&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementWorker.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementWorker.java Sun Dec 10 17:51:09 2017
@@ -27,6 +27,7 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
@@ -284,6 +285,7 @@ public final class ContentManagementWork
             String contentId = (String)webSitePP.get("contentId");
             String templateTitle = (String)webSitePP.get("templateTitle");
             GenericValue content = delegator.makeValue("Content", UtilMisc.toMap("contentId", contentId));
+            // TODO check if we want statusId to be filled/used, else this should be removed
             String statusId = null;
             String entityAction = permittedAction;
             if (entityAction == null) {
@@ -477,8 +479,7 @@ public final class ContentManagementWork
             String contentId = arr[0];
             String description = arr[1];
             List<Object []> subPointList = new LinkedList<Object[]>();
-            Object nullObj = null;
-            Object [] subArr = {contentId, subPointList, description, nullObj};
+            Object [] subArr = {contentId, subPointList, description, null};
             publishPointMap.put(contentId, subArr);
             publishPointMapAll.put(contentId, contentId);
             List<GenericValue> subPublishPointList = getAllPublishPoints(delegator, contentId);
@@ -486,8 +487,7 @@ public final class ContentManagementWork
                 String contentId2 = (String)webSitePublishPoint2.get("contentId");
                 String description2 = (String)webSitePublishPoint2.get("templateTitle");
                 publishPointMapAll.put(contentId2, contentId);
-                Timestamp obj = null;
-                Object [] subArr2 = {contentId2, description2, obj};
+                Object [] subArr2 = {contentId2, description2, null};
                 subPointList.add(subArr2);
             }
         }
@@ -522,7 +522,8 @@ public final class ContentManagementWork
         }
 
         List<Object []> publishedLinkList = new LinkedList<Object[]>();
-        for (String contentId : publishPointMap.keySet()) {
+        for (Entry<String, Object> entry : publishPointMap.entrySet()) {
+            String contentId = entry.getKey();
             Object [] subPointArr = (Object [])publishPointMap.get(contentId);
             publishedLinkList.add(subPointArr);
         }
@@ -533,6 +534,7 @@ public final class ContentManagementWork
         GenericValue authorContent = null;
         try {
             List<String> assocTypes = UtilMisc.toList("AUTHOR");
+            // TODO check if we want contentTypes to be filled/used, else this should be removed
             List<String> contentTypes = null;
             Map<String, Object> results =  ContentServicesComplex.getAssocAndContentAndDataResourceCacheMethod(delegator, contentId, null, "To", null, null, assocTypes, contentTypes, Boolean.TRUE, null, null);
             List<GenericValue> valueList = UtilGenerics.checkList(results.get("entityList"));
@@ -556,6 +558,7 @@ public final class ContentManagementWork
         for (GenericValue content : allDepartmentPoints) {
             String contentId = (String)content.get("contentId");
             String contentName = (String)content.get("contentName");
+            // TODO check if we want statusId to be filled/used, else this should be removed
             String statusId = null;
             String entityAction = permittedAction;
             if (entityAction == null)

Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ConvertTree.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ConvertTree.java?rev=1817722&r1=1817721&r2=1817722&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ConvertTree.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/ConvertTree.java Sun Dec 10 17:51:09 2017
@@ -19,8 +19,10 @@
 package org.apache.ofbiz.content;
 
 import java.io.BufferedReader;
+import java.io.FileInputStream;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -29,6 +31,7 @@ import java.util.Map;
 
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilDateTime;
+import org.apache.ofbiz.base.util.UtilIO;
 import org.apache.ofbiz.base.util.UtilMisc;
 import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.base.util.UtilValidate;
@@ -77,7 +80,7 @@ In order ta make this service active add
             BufferedReader input = null;
             try {
                 if (UtilValidate.isNotEmpty(file)) {
-                    input = new BufferedReader(new FileReader(file));
+                    input = new BufferedReader(new InputStreamReader(new FileInputStream(file), UtilIO.getUtf8()));
                     String line = null;
                     int size = 0;
                     if (file != null) {
@@ -151,7 +154,7 @@ In order ta make this service active add
                                                     GenericValue contentAss = contentAssChecks.next();
                                                     GenericValue contentcheck = EntityQuery.use(delegator).from("Content").where("contentId", contentAss.get("contentId")).queryOne();
                                                     if (contentcheck!=null) {
-                                                        if (contentcheck.get("contentName").equals(contentName) && contentNameMatch == false) {
+                                                        if (contentcheck.get("contentName").equals(contentName)) {
                                                             contentNameMatch = true;
                                                             contentId = contentcheck.get("contentId").toString();
                                                         }
@@ -226,7 +229,7 @@ In order ta make this service active add
                 }
              }
              finally {
-                 input.close();
+                 if (input != null) input.close();
              }
              return ServiceUtil.returnSuccess(sucMsg);
         } catch (IOException e) {