Fix file upload exception (#581)

* Refactor FileHandler and related implementations

* Add more friendly error throw

* Make tmp directory as default location for multipart file

* Change error message in Chinese

src/main/java/run/halo/app/handler/file/AliOssFileHandler.java

@@ -69,7 +69,7 @@ public class AliOssFileHandler implements FileHandler {
         }
 
         try {
-            String basename = FilenameUtils.getBasename(file.getOriginalFilename());
+            String basename = FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename()));
             String extension = FilenameUtils.getExtension(file.getOriginalFilename());
             String timestamp = String.valueOf(System.currentTimeMillis());
             StringBuilder upFilePath = new StringBuilder();
@@ -116,19 +116,13 @@ public class AliOssFileHandler implements FileHandler {
                 }
             }
 
+            log.info("Uploaded file: [{}] successfully", file.getOriginalFilename());
             return uploadResult;
         } catch (Exception e) {
-            e.printStackTrace();
+            throw new FileOperationException("上传附件 " + file.getOriginalFilename() + " 到阿里云失败 ", e).setErrorData(file.getOriginalFilename());
         } finally {
             ossClient.shutdown();
         }
-
-        // Build result
-        UploadResult result = new UploadResult();
-
-        log.info("File: [{}] uploaded successfully", file.getOriginalFilename());
-
-        return result;
     }
 
     @Override
@@ -154,7 +148,7 @@ public class AliOssFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.ALIOSS.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.ALIOSS.name().equalsIgnoreCase(type);
     }
 }

src/main/java/run/halo/app/handler/file/BaiduBosFileHandler.java

@@ -64,7 +64,7 @@ public class BaiduBosFileHandler implements FileHandler {
         domain = protocol + domain;
 
         try {
-            String basename = FilenameUtils.getBasename(file.getOriginalFilename());
+            String basename = FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename()));
             String extension = FilenameUtils.getExtension(file.getOriginalFilename());
             String timestamp = String.valueOf(System.currentTimeMillis());
             String upFilePath = StringUtils.join(basename, "_", timestamp, ".", extension);
@@ -132,7 +132,7 @@ public class BaiduBosFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.BAIDUBOS.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.BAIDUBOS.name().equalsIgnoreCase(type);
     }
 }

src/main/java/run/halo/app/handler/file/FileHandler.java

@@ -77,7 +77,7 @@ public interface FileHandler {
      * Checks if the given type is supported.
      *
      * @param type attachment type
-     * @return true if supported; false or else
+     * @return true if supported; false otherwise
      */
-    boolean supportType(@Nullable AttachmentType type);
+    boolean supportType(@Nullable String type);
 }

src/main/java/run/halo/app/handler/file/FileHandlers.java

@@ -34,6 +34,7 @@ public class FileHandlers {
     public FileHandlers(ApplicationContext applicationContext) {
         // Add all file handler
         addFileHandlers(applicationContext.getBeansOfType(FileHandler.class).values());
+        log.info("Registered {} file handler(s)", fileHandlers.size());
     }
 
     /**
@@ -46,16 +47,30 @@ public class FileHandlers {
      */
     @NonNull
     public UploadResult upload(@NonNull MultipartFile file, @NonNull AttachmentType attachmentType) {
-        Assert.notNull(file, "Multipart file must not be null");
         Assert.notNull(attachmentType, "Attachment type must not be null");
 
+        return upload(file, attachmentType.name());
+    }
+
+    /**
+     * Uploads files.
+     *
+     * @param file multipart file must not be null
+     * @param type store type
+     * @return upload result
+     * @throws FileOperationException throws when fail to delete attachment or no available file handler to upload it
+     */
+    @NonNull
+    public UploadResult upload(@NonNull MultipartFile file, @Nullable String type) {
+        Assert.notNull(file, "Multipart file must not be null");
+
         for (FileHandler fileHandler : fileHandlers) {
-            if (fileHandler.supportType(attachmentType)) {
+            if (fileHandler.supportType(type)) {
                 return fileHandler.upload(file);
             }
         }
 
-        throw new FileOperationException("No available file handler to upload the file").setErrorData(attachmentType);
+        throw new FileOperationException("No available file handlers to upload the file").setErrorData(type);
     }
 
     /**
@@ -67,15 +82,25 @@ public class FileHandlers {
     public void delete(@NonNull Attachment attachment) {
         Assert.notNull(attachment, "Attachment must not be null");
 
+        delete(attachment.getType().name(), attachment.getFileKey());
+    }
+
+    /**
+     * Deletes attachment.
+     *
+     * @param key file key
+     * @throws FileOperationException throws when fail to delete attachment or no available file handler to delete it
+     */
+    public void delete(@Nullable String type, @NonNull String key) {
         for (FileHandler fileHandler : fileHandlers) {
-            if (fileHandler.supportType(attachment.getType())) {
+            if (fileHandler.supportType(type)) {
                 // Delete the file
-                fileHandler.delete(attachment.getFileKey());
+                fileHandler.delete(key);
                 return;
             }
         }
 
-        throw new FileOperationException("No available file handler to delete the file").setErrorData(attachment);
+        throw new FileOperationException("No available file handlers to delete the file").setErrorData(type);
     }
 
     /**

src/main/java/run/halo/app/handler/file/LocalFileHandler.java

@@ -8,7 +8,6 @@ import org.springframework.util.Assert;
 import org.springframework.web.multipart.MultipartFile;
 import run.halo.app.config.properties.HaloProperties;
 import run.halo.app.exception.FileOperationException;
-import run.halo.app.exception.ServiceException;
 import run.halo.app.model.enums.AttachmentType;
 import run.halo.app.model.support.UploadResult;
 import run.halo.app.service.OptionService;
@@ -101,7 +100,7 @@ public class LocalFileHandler implements FileHandler {
         // Build directory
         String subDir = UPLOAD_SUB_DIR + year + FILE_SEPARATOR + month + FILE_SEPARATOR;
 
-        String originalBasename = FilenameUtils.getBasename(file.getOriginalFilename());
+        String originalBasename = FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename()));
 
         // Get basename
         String basename = originalBasename + '-' + HaloUtils.randomUUIDWithoutDash();
@@ -117,7 +116,7 @@ public class LocalFileHandler implements FileHandler {
         // Get upload path
         Path uploadPath = Paths.get(workDir, subFilePath);
 
-        log.info("Uploading to directory: [{}]", uploadPath.toString());
+        log.info("Uploading file: [{}]to directory: [{}]", file.getOriginalFilename(), uploadPath.toString());
 
         try {
             // TODO Synchronize here
@@ -171,10 +170,10 @@ public class LocalFileHandler implements FileHandler {
                 uploadResult.setThumbPath(subFilePath);
             }
 
+            log.info("Uploaded file: [{}] to directory: [{}] successfully", file.getOriginalFilename(), uploadPath.toString());
             return uploadResult;
         } catch (IOException e) {
-            log.error("Failed to upload file to local: " + uploadPath, e);
-            throw new ServiceException("上传附件失败").setErrorData(uploadPath);
+            throw new FileOperationException("上传附件失败").setErrorData(uploadPath);
         }
     }
 
@@ -184,7 +183,6 @@ public class LocalFileHandler implements FileHandler {
         // Get path
         Path path = Paths.get(workDir, key);
 
-
         // Delete the file key
         try {
             Files.delete(path);
@@ -214,8 +212,8 @@ public class LocalFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.LOCAL.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.LOCAL.name().equalsIgnoreCase(type);
     }
 
     private boolean generateThumbnail(BufferedImage originalImage, Path thumbPath, String extension) {

src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java

@@ -87,7 +87,7 @@ public class QiniuOssFileHandler implements FileHandler {
                 .append("/");
 
         try {
-            String basename = FilenameUtils.getBasename(file.getOriginalFilename());
+            String basename = FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename()));
             String extension = FilenameUtils.getExtension(file.getOriginalFilename());
             String timestamp = String.valueOf(System.currentTimeMillis());
             StringBuilder upFilePath = new StringBuilder();
@@ -108,8 +108,10 @@ public class QiniuOssFileHandler implements FileHandler {
             // Put the file
             Response response = uploadManager.put(file.getInputStream(), upFilePath.toString(), uploadToken, null, null);
 
-            log.debug("QnYun response: [{}]", response.toString());
-            log.debug("QnYun response body: [{}]", response.bodyString());
+            if (log.isDebugEnabled()) {
+                log.debug("QnYun response: [{}]", response.toString());
+                log.debug("QnYun response body: [{}]", response.bodyString());
+            }
 
             response.jsonToObject(QiNiuPutSet.class);
 
@@ -175,7 +177,7 @@ public class QiniuOssFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.QINIUOSS.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.QINIUOSS.name().equalsIgnoreCase(type);
     }
 }

src/main/java/run/halo/app/handler/file/SmmsFileHandler.java

@@ -2,7 +2,6 @@ package run.halo.app.handler.file;
 
 import lombok.Data;
 import lombok.NoArgsConstructor;
-import lombok.ToString;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.*;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
@@ -123,7 +122,7 @@ public class SmmsFileHandler implements FileHandler {
 
         // Build result
         UploadResult result = new UploadResult();
-        result.setFilename(FilenameUtils.getBasename(file.getOriginalFilename()));
+        result.setFilename(FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename())));
         result.setSuffix(FilenameUtils.getExtension(file.getOriginalFilename()));
         result.setMediaType(MediaType.valueOf(Objects.requireNonNull(file.getContentType())));
 
@@ -164,8 +163,8 @@ public class SmmsFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.SMMS.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.SMMS.name().equalsIgnoreCase(type);
     }
 
     /**
@@ -179,7 +178,6 @@ public class SmmsFileHandler implements FileHandler {
     }
 
     @Data
-    @ToString
     @NoArgsConstructor
     private static class SmmsResponse {
 
@@ -195,7 +193,6 @@ public class SmmsFileHandler implements FileHandler {
     }
 
     @Data
-    @ToString(callSuper = true)
     @NoArgsConstructor
     private static class SmmsResponseData {
 

src/main/java/run/halo/app/handler/file/TencentCosFileHandler.java

@@ -78,7 +78,7 @@ public class TencentCosFileHandler implements FileHandler {
         }
 
         try {
-            String basename = FilenameUtils.getBasename(file.getOriginalFilename());
+            String basename = FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename()));
             String extension = FilenameUtils.getExtension(file.getOriginalFilename());
             String timestamp = String.valueOf(System.currentTimeMillis());
             StringBuilder upFilePath = new StringBuilder();
@@ -163,7 +163,7 @@ public class TencentCosFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.TENCENTCOS.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.TENCENTCOS.name().equalsIgnoreCase(type);
     }
 }

src/main/java/run/halo/app/handler/file/UpOssFileHandler.java

@@ -58,7 +58,7 @@ public class UpOssFileHandler implements FileHandler {
 
         try {
             // Get file basename
-            String basename = FilenameUtils.getBasename(file.getOriginalFilename());
+            String basename = FilenameUtils.getBasename(Objects.requireNonNull(file.getOriginalFilename()));
             // Get file extension
             String extension = FilenameUtils.getExtension(file.getOriginalFilename());
             // Get md5 value of the file
@@ -128,7 +128,7 @@ public class UpOssFileHandler implements FileHandler {
     }
 
     @Override
-    public boolean supportType(AttachmentType type) {
-        return AttachmentType.UPOSS.equals(type);
+    public boolean supportType(String type) {
+        return AttachmentType.UPOSS.name().equalsIgnoreCase(type);
     }
 }

src/main/java/run/halo/app/service/impl/AttachmentServiceImpl.java

@@ -234,6 +234,6 @@ public class AttachmentServiceImpl extends AbstractCrudService<Attachment, Integ
      */
     @NonNull
     private AttachmentType getAttachmentType() {
-        return optionService.getEnumByPropertyOrDefault(AttachmentProperties.ATTACHMENT_TYPE, AttachmentType.class, AttachmentType.LOCAL);
+        return Objects.requireNonNull(optionService.getEnumByPropertyOrDefault(AttachmentProperties.ATTACHMENT_TYPE, AttachmentType.class, AttachmentType.LOCAL));
     }
 }

src/main/resources/application.yaml

@@ -20,12 +20,6 @@ spring:
     username: admin
     password: 123456
 
-    # MySQL database configuration.
-#    driver-class-name: com.mysql.cj.jdbc.Driver
-#    url: jdbc:mysql://127.0.0.1:3306/halodb?characterEncoding=utf8&useSSL=false&serverTimezone=Asia/Shanghai&allowPublicKeyRetrieval=true
-#    username: root
-#    password: 123456
-
   h2:
     console:
       settings:
@@ -43,6 +37,7 @@ spring:
     multipart:
       max-file-size: 10240MB
       max-request-size: 10240MB
+      location: ${java.io.tmpdir}
 management:
   endpoints:
     web: