svn commit: r1817623 - /ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java

[mbrohl]

Author: mbrohl
Date: Sat Dec  9 15:09:57 2017
New Revision: 1817623

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

Thanks Julian Leichert for reporting and providing the patch.

Modified:
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java?rev=1817623&r1=1817622&r2=1817623&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java Sat Dec  9 15:09:57 2017
@@ -105,7 +105,7 @@ public class ScaleImage {
 
         /* IMAGE */
         // get Name and Extension
-        index = filenameToUse.lastIndexOf(".");
+        index = filenameToUse.lastIndexOf('.');
         String imgExtension = filenameToUse.substring(index + 1);
         // paths
         
@@ -118,14 +118,13 @@ public class ScaleImage {
         imageUrlPrefix = imageUrlPrefix.endsWith("/") ? imageUrlPrefix.substring(0, imageUrlPrefix.length()-1) : imageUrlPrefix;
         FlexibleStringExpander filenameExpander;
         String fileLocation = null;
-        String type = null;
         String id = null;
-        if (viewType.toLowerCase().contains("main")) {
+        if (viewType.toLowerCase(Locale.getDefault()).contains("main")) {
             String filenameFormat = EntityUtilProperties.getPropertyValue("catalog", "image.filename.format", (Delegator) context.get("delegator"));
             filenameExpander = FlexibleStringExpander.getInstance(filenameFormat);
             id = (String) context.get("productId");
             fileLocation = filenameExpander.expandString(UtilMisc.toMap("location", "products", "id", id, "type", "original"));
-        } else if (viewType.toLowerCase().contains("additional") && viewNumber != null && !"0".equals(viewNumber)) {
+        } else if (viewType.toLowerCase(Locale.getDefault()).contains("additional") && viewNumber != null && !"0".equals(viewNumber)) {
             String filenameFormat = EntityUtilProperties.getPropertyValue("catalog", "image.filename.additionalviewsize.format", (Delegator) context.get("delegator"));
             filenameExpander = FlexibleStringExpander.getInstance(filenameFormat);
             id = (String) context.get("productId");
@@ -136,11 +135,11 @@ public class ScaleImage {
             }    
             fileLocation = filenameExpander.expandString(UtilMisc.toMap("location", "products", "id", id, "viewtype", viewType, "sizetype", "original"));
         } else {
-            return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ProductImageViewType", UtilMisc.toMap("viewType", type), locale));
+            return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ProductImageViewType", UtilMisc.toMap("viewType", viewType), locale));
         }
 
-        if (fileLocation.lastIndexOf("/") != -1) {
-            fileLocation.substring(0, fileLocation.lastIndexOf("/") + 1); // adding 1 to include the trailing slash
+        if (fileLocation.lastIndexOf('/') != -1) {
+            fileLocation = fileLocation.substring(0, fileLocation.lastIndexOf('/') + 1); // adding 1 to include the trailing slash
         }
         
         /* get original BUFFERED IMAGE */
@@ -172,15 +171,15 @@ public class ScaleImage {
 
                     // Build full path for the new scaled image
                     String newFileLocation = null;
-                    filenameToUse = sizeType + filenameToUse.substring(filenameToUse.lastIndexOf("."));
-                    if (viewType.toLowerCase().contains("main")) {
+                    filenameToUse = sizeType + filenameToUse.substring(filenameToUse.lastIndexOf('.'));
+                    if (viewType.toLowerCase(Locale.getDefault()).contains("main")) {
                         newFileLocation = filenameExpander.expandString(UtilMisc.toMap("location", "products", "id", id, "type", sizeType));
-                    } else if (viewType.toLowerCase().contains("additional")) {
+                    } else if (viewType.toLowerCase(Locale.getDefault()).contains("additional")) {
                         newFileLocation = filenameExpander.expandString(UtilMisc.toMap("location", "products", "id", id, "viewtype", viewType, "sizetype", sizeType));
                     }
                     String newFilePathPrefix = "";
-                    if (newFileLocation.lastIndexOf("/") != -1) {
-                        newFilePathPrefix = newFileLocation.substring(0, newFileLocation.lastIndexOf("/") + 1); // adding 1 to include the trailing slash
+                    if (newFileLocation != null && newFileLocation.lastIndexOf('/') != -1) {
+                        newFilePathPrefix = newFileLocation.substring(0, newFileLocation.lastIndexOf('/') + 1); // adding 1 to include the trailing slash
                     }    
                     // Directory
                     String targetDirectory = imageServerPath + "/" + newFilePathPrefix;
@@ -201,7 +200,8 @@ public class ScaleImage {
                                 File[] files = targetDir.listFiles();
                                 for (File file : files) {
                                     if (file.isFile() && file.getName().startsWith(id)) {
-                                        file.delete();
+                                        if (!file.delete())
+                                            Debug.logError("File :" + file.getName() + ", couldn't be deleted", module);
                                     }
                                 }
                             } catch (SecurityException e) {
@@ -286,7 +286,7 @@ public class ScaleImage {
 
         /* IMAGE */
         // get Name and Extension
-        index = filenameToUse.lastIndexOf(".");
+        index = filenameToUse.lastIndexOf('.');
         String imgName = filenameToUse.substring(0, index - 1);
         String imgExtension = filenameToUse.substring(index + 1);
         // paths
@@ -304,7 +304,7 @@ public class ScaleImage {
         if (viewType.toLowerCase().contains("main")) {
             type = "original";
             id = imgName;
-        } else if (viewType.toLowerCase().contains("additional") && viewNumber != null && !"0".equals(viewNumber)) {
+        } else if (viewType.toLowerCase(Locale.getDefault()).contains("additional") && viewNumber != null && !"0".equals(viewNumber)) {
             type = "additional";
             id = imgName + "_View_" + viewNumber;
         } else {
@@ -314,8 +314,8 @@ public class ScaleImage {
         FlexibleStringExpander mainFilenameExpander = FlexibleStringExpander.getInstance(mainFilenameFormat);
         String fileLocation = mainFilenameExpander.expandString(UtilMisc.toMap("location", "products", "id", context.get("productId"), "type", type));
         String filePathPrefix = "";
-        if (fileLocation.lastIndexOf("/") != -1) {
-            filePathPrefix = fileLocation.substring(0, fileLocation.lastIndexOf("/") + 1); // adding 1 to include the trailing slash
+        if (fileLocation.lastIndexOf('/') != -1) {
+            filePathPrefix = fileLocation.substring(0, fileLocation.lastIndexOf('/') + 1); // adding 1 to include the trailing slash
         }
         
         if (context.get("contentId") != null){
@@ -340,7 +340,7 @@ public class ScaleImage {
 
             // new Filename Format
             FlexibleStringExpander addFilenameExpander = mainFilenameExpander;
-            if (viewType.toLowerCase().contains("additional")) {
+            if (viewType.toLowerCase(Locale.getDefault()).contains("additional")) {
                 String addFilenameFormat = EntityUtilProperties.getPropertyValue("catalog", "image.filename.additionalviewsize.format", (Delegator) context.get("delegator"));
                 addFilenameExpander = FlexibleStringExpander.getInstance(addFilenameFormat);
             }
@@ -354,14 +354,14 @@ public class ScaleImage {
 
                     // write the New Scaled Image
                     String newFileLocation = null;
-                    if (viewType.toLowerCase().contains("main")) {
+                    if (viewType.toLowerCase(Locale.getDefault()).contains("main")) {
                         newFileLocation = mainFilenameExpander.expandString(UtilMisc.toMap("location", "products", "id", id, "type", sizeType));
-                    } else if (viewType.toLowerCase().contains("additional")) {
+                    } else if (viewType.toLowerCase(Locale.getDefault()).contains("additional")) {
                         newFileLocation = addFilenameExpander.expandString(UtilMisc.toMap("location", "products","id", id, "viewtype", viewType, "sizetype", sizeType));
                     }
                     String newFilePathPrefix = "";
-                    if (newFileLocation.lastIndexOf("/") != -1) {
-                        newFilePathPrefix = newFileLocation.substring(0, newFileLocation.lastIndexOf("/") + 1); // adding 1 to include the trailing slash
+                    if (newFileLocation != null && newFileLocation.lastIndexOf('/') != -1) {
+                        newFilePathPrefix = newFileLocation.substring(0, newFileLocation.lastIndexOf('/') + 1); // adding 1 to include the trailing slash
                     }
 
                     String targetDirectory = imageServerPath + "/" + newFilePathPrefix;