This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch feature/WW-5371-modern-upload
in repository https://gitbox.apache.org/repos/asf/struts.git


The following commit(s) were added to refs/heads/feature/WW-5371-modern-upload 
by this push:
     new 3ce3f8264 WW-5371 Simplifies file upload logic and extracts constants
3ce3f8264 is described below

commit 3ce3f82646f667be262fadc1420a2d9251577c17
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Mon Dec 11 08:55:20 2023 +0100

    WW-5371 Simplifies file upload logic and extracts constants
---
 .../showcase/fileupload/FileUploadAction.java      |  3 --
 .../dispatcher/multipart/StrutsUploadedFile.java   |  5 ++
 .../struts2/dispatcher/multipart/UploadedFile.java |  4 +-
 .../interceptor/AbstractFileUploadInterceptor.java | 20 +++++---
 .../interceptor/ActionFileUploadInterceptor.java   | 56 +++++++++-------------
 .../struts2/interceptor/FileUploadInterceptor.java |  6 +--
 6 files changed, 47 insertions(+), 47 deletions(-)

diff --git 
a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java
 
b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java
index 246f8ad70..c2ac471f4 100644
--- 
a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java
+++ 
b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java
@@ -24,7 +24,6 @@ import com.opensymphony.xwork2.ActionSupport;
 import org.apache.struts2.action.UploadedFilesAware;
 import org.apache.struts2.dispatcher.multipart.UploadedFile;
 
-import java.io.File;
 import java.util.List;
 
 /**
@@ -32,8 +31,6 @@ import java.util.List;
  */
 public class FileUploadAction extends ActionSupport implements 
UploadedFilesAware {
 
-    private static final long serialVersionUID = 5156288255337069381L;
-
     private String contentType;
     private UploadedFile uploadedFile;
     private String fileName;
diff --git 
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java
 
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java
index 55233d7d7..5976f578f 100644
--- 
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java
+++ 
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java
@@ -26,6 +26,11 @@ public class StrutsUploadedFile implements UploadedFile {
     private final String contentType;
     private final String originalName;
 
+    /**
+     * Use builder instead of constructor
+     * @param file an uploaded file
+     * @deprecated since Struts 6.4.0
+     */
     @Deprecated
     public StrutsUploadedFile(File file) {
         this.file = file;
diff --git 
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java 
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java
index 896c681be..ada27ff6c 100644
--- 
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java
+++ 
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java
@@ -18,10 +18,12 @@
  */
 package org.apache.struts2.dispatcher.multipart;
 
+import java.io.Serializable;
+
 /**
  * Virtual representation of a uploaded file used by {@link MultiPartRequest}
  */
-public interface UploadedFile {
+public interface UploadedFile extends Serializable {
 
     Long length();
 
diff --git 
a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
 
b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
index 3857a7b4e..c8da384d2 100644
--- 
a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
+++ 
b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
@@ -45,6 +45,14 @@ public abstract class AbstractFileUploadInterceptor extends 
AbstractInterceptor
 
     private static final Logger LOG = 
LogManager.getLogger(AbstractFileUploadInterceptor.class);
 
+    public static final String STRUTS_MESSAGES_BYPASS_REQUEST_KEY = 
"struts.messages.bypass.request";
+    public static final String STRUTS_MESSAGES_ERROR_UPLOADING_KEY = 
"struts.messages.error.uploading";
+    public static final String STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY = 
"struts.messages.error.file.too.large";
+    public static final String STRUTS_MESSAGES_INVALID_FILE_KEY = 
"struts.messages.invalid.file";
+    public static final String STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY = 
"struts.messages.invalid.content.type";
+    public static final String 
STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY = 
"struts.messages.error.content.type.not.allowed";
+    public static final String 
STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY = 
"struts.messages.error.file.extension.not.allowed";
+
     protected Long maximumSize;
     protected Set<String> allowedTypesSet = Collections.emptySet();
     protected Set<String> allowedExtensionsSet = Collections.emptySet();
@@ -109,31 +117,31 @@ public abstract class AbstractFileUploadInterceptor 
extends AbstractInterceptor
 
         // If it's null the upload failed
         if (file == null) {
-            String errMsg = getTextMessage(action, 
"struts.messages.error.uploading", new String[]{inputName});
+            String errMsg = getTextMessage(action, 
STRUTS_MESSAGES_ERROR_UPLOADING_KEY, new String[]{inputName});
             if (validation != null) {
                 validation.addFieldError(inputName, errMsg);
             }
             LOG.warn(errMsg);
         } else if (file.getContent() == null) {
-            String errMsg = getTextMessage(action, 
"struts.messages.error.uploading", new String[]{filename});
+            String errMsg = getTextMessage(action, 
STRUTS_MESSAGES_ERROR_UPLOADING_KEY, new String[]{filename});
             if (validation != null) {
                 validation.addFieldError(inputName, errMsg);
             }
             LOG.warn(errMsg);
         } else if (maximumSize != null && maximumSize < file.length()) {
-            String errMsg = getTextMessage(action, 
"struts.messages.error.file.too.large", new String[]{inputName, filename, 
file.getName(), "" + file.length(), getMaximumSizeStr(action)});
+            String errMsg = getTextMessage(action, 
STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY, new String[]{inputName, filename, 
file.getName(), "" + file.length(), getMaximumSizeStr(action)});
             if (validation != null) {
                 validation.addFieldError(inputName, errMsg);
             }
             LOG.warn(errMsg);
         } else if ((!allowedTypesSet.isEmpty()) && 
(!containsItem(allowedTypesSet, contentType))) {
-            String errMsg = getTextMessage(action, 
"struts.messages.error.content.type.not.allowed", new String[]{inputName, 
filename, file.getName(), contentType});
+            String errMsg = getTextMessage(action, 
STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY, new String[]{inputName, 
filename, file.getName(), contentType});
             if (validation != null) {
                 validation.addFieldError(inputName, errMsg);
             }
             LOG.warn(errMsg);
         } else if ((!allowedExtensionsSet.isEmpty()) && 
(!hasAllowedExtension(allowedExtensionsSet, filename))) {
-            String errMsg = getTextMessage(action, 
"struts.messages.error.file.extension.not.allowed", new String[]{inputName, 
filename, file.getName(), contentType});
+            String errMsg = getTextMessage(action, 
STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY, new String[]{inputName, 
filename, file.getName(), contentType});
             if (validation != null) {
                 validation.addFieldError(inputName, errMsg);
             }
@@ -237,7 +245,7 @@ public abstract class AbstractFileUploadInterceptor extends 
AbstractInterceptor
                 if (textProvider.hasKey(error.getTextKey())) {
                     errorMessage = textProvider.getText(error.getTextKey(), 
Arrays.asList(error.getArgs()));
                 } else {
-                    errorMessage = 
textProvider.getText("struts.messages.error.uploading", 
error.getDefaultMessage());
+                    errorMessage = 
textProvider.getText(STRUTS_MESSAGES_ERROR_UPLOADING_KEY, 
error.getDefaultMessage());
                 }
                 validation.addActionError(errorMessage);
             }
diff --git 
a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
 
b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
index 4cff8617e..1642a890b 100644
--- 
a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
+++ 
b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
@@ -180,8 +180,10 @@ public class ActionFileUploadInterceptor extends 
AbstractFileUploadInterceptor {
     public String intercept(ActionInvocation invocation) throws Exception {
         HttpServletRequest request = 
invocation.getInvocationContext().getServletRequest();
         if (!(request instanceof MultiPartRequestWrapper)) {
-            ActionProxy proxy = invocation.getProxy();
-            LOG.debug(getTextMessage("struts.messages.bypass.request", new 
String[]{proxy.getNamespace(), proxy.getActionName()}));
+            if (LOG.isDebugEnabled()) {
+                ActionProxy proxy = invocation.getProxy();
+                LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, 
new String[]{proxy.getNamespace(), proxy.getActionName()}));
+            }
             return invocation.invoke();
         }
 
@@ -199,50 +201,36 @@ public class ActionFileUploadInterceptor extends 
AbstractFileUploadInterceptor {
 
         // bind allowed Files
         Enumeration<String> fileParameterNames = 
multiWrapper.getFileParameterNames();
+        List<UploadedFile> acceptedFiles = new ArrayList<>();
+
         while (fileParameterNames != null && 
fileParameterNames.hasMoreElements()) {
             // get the value of this input tag
             String inputName = fileParameterNames.nextElement();
+            UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName);
 
-            // get the content type
-            String[] contentType = multiWrapper.getContentTypes(inputName);
-
-            if (isNonEmpty(contentType)) {
-                // get the name of the file from the input tag
-                String[] fileName = multiWrapper.getFileNames(inputName);
-
-                if (isNonEmpty(fileName)) {
-                    // get a File object for the uploaded File
-                    UploadedFile[] files = multiWrapper.getFiles(inputName);
-                    if (files != null && files.length > 0) {
-                        List<UploadedFile> acceptedFiles = new 
ArrayList<>(files.length);
-
-                        for (int index = 0; index < files.length; index++) {
-                            if (acceptFile(action, files[index], 
fileName[index], contentType[index], inputName)) {
-                                acceptedFiles.add(files[index]);
-                            }
-                        }
-
-                        if (acceptedFiles.isEmpty()) {
-                            LOG.debug("No files have been uploaded/accepted");
-                        } else {
-                            LOG.debug("Passing: {} uploaded file(s) to 
action", acceptedFiles.size());
-                            action.withUploadedFiles(acceptedFiles);
-                        }
-                    }
-                } else {
-                    if (LOG.isWarnEnabled()) {
-                        LOG.warn(getTextMessage(action, 
"struts.messages.invalid.file", new String[]{inputName}));
-                    }
+            if (uploadedFiles == null || uploadedFiles.length == 0) {
+                if (LOG.isWarnEnabled()) {
+                    LOG.warn(getTextMessage(action, 
STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName}));
                 }
             } else {
-                if (LOG.isWarnEnabled()) {
-                    LOG.warn(getTextMessage(action, 
"struts.messages.invalid.content.type", new String[]{inputName}));
+                for (UploadedFile uploadedFile : uploadedFiles) {
+                    if (acceptFile(action, uploadedFile, 
uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) {
+                        acceptedFiles.add(uploadedFile);
+                    }
                 }
             }
         }
 
+        if (acceptedFiles.isEmpty()) {
+            LOG.debug("No files have been uploaded/accepted");
+        } else {
+            LOG.debug("Passing: {} uploaded file(s) to action", 
acceptedFiles.size());
+            action.withUploadedFiles(acceptedFiles);
+        }
+
         // invoke action
         return invocation.invoke();
     }
 
 }
+
diff --git 
a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java 
b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
index 4bcdec07b..287e66078 100644
--- 
a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
+++ 
b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
@@ -191,7 +191,7 @@ public class FileUploadInterceptor extends 
AbstractFileUploadInterceptor {
         if (!(request instanceof MultiPartRequestWrapper)) {
             if (LOG.isDebugEnabled()) {
                 ActionProxy proxy = invocation.getProxy();
-                LOG.debug(getTextMessage("struts.messages.bypass.request", new 
String[]{proxy.getNamespace(), proxy.getActionName()}));
+                LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, 
new String[]{proxy.getNamespace(), proxy.getActionName()}));
             }
 
             return invocation.invoke();
@@ -243,12 +243,12 @@ public class FileUploadInterceptor extends 
AbstractFileUploadInterceptor {
                     }
                 } else {
                     if (LOG.isWarnEnabled()) {
-                        LOG.warn(getTextMessage(action, 
"struts.messages.invalid.file", new String[]{inputName}));
+                        LOG.warn(getTextMessage(action, 
STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName}));
                     }
                 }
             } else {
                 if (LOG.isWarnEnabled()) {
-                    LOG.warn(getTextMessage(action, 
"struts.messages.invalid.content.type", new String[]{inputName}));
+                    LOG.warn(getTextMessage(action, 
STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY, new String[]{inputName}));
                 }
             }
         }

Reply via email to