From 1d020ff550cf9dd760e433aa4381f7b0f20feec5 Mon Sep 17 00:00:00 2001 From: bestsort Date: Sat, 14 Mar 2020 10:05:53 +0800 Subject: [PATCH] Optimize FileHandler's logic (#639) * optimize FileHandler's logic * fix code style * fix: maybe happen OOM when file upload, see #529 * refactor: HashMap -> ConcurrentHashMap * remove unused class --- .../app/exception/RepeatTypeException.java | 15 +++++ .../app/handler/file/AliOssFileHandler.java | 5 +- .../app/handler/file/BaiduBosFileHandler.java | 4 +- .../halo/app/handler/file/FileHandler.java | 11 ++-- .../halo/app/handler/file/FileHandlers.java | 66 ++++++------------- .../app/handler/file/LocalFileHandler.java | 7 +- .../app/handler/file/QiniuOssFileHandler.java | 4 +- .../app/handler/file/SmmsFileHandler.java | 4 +- .../handler/file/TencentCosFileHandler.java | 4 +- .../app/handler/file/UpOssFileHandler.java | 4 +- .../app/service/impl/ThemeServiceImpl.java | 1 - 11 files changed, 58 insertions(+), 67 deletions(-) create mode 100644 src/main/java/run/halo/app/exception/RepeatTypeException.java diff --git a/src/main/java/run/halo/app/exception/RepeatTypeException.java b/src/main/java/run/halo/app/exception/RepeatTypeException.java new file mode 100644 index 000000000..75c76fab4 --- /dev/null +++ b/src/main/java/run/halo/app/exception/RepeatTypeException.java @@ -0,0 +1,15 @@ +package run.halo.app.exception; + +/** + * repeat type exception + * @author bestsort + * @date 3/13/20 5:03 PM + */ +public class RepeatTypeException extends ServiceException { + public RepeatTypeException(String message) { + super(message); + } + public RepeatTypeException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/run/halo/app/handler/file/AliOssFileHandler.java b/src/main/java/run/halo/app/handler/file/AliOssFileHandler.java index 47ca3e8c4..2e9812a8b 100644 --- a/src/main/java/run/halo/app/handler/file/AliOssFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/AliOssFileHandler.java @@ -148,7 +148,8 @@ public class AliOssFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.ALIOSS.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.ALIOSS; } + } diff --git a/src/main/java/run/halo/app/handler/file/BaiduBosFileHandler.java b/src/main/java/run/halo/app/handler/file/BaiduBosFileHandler.java index 78c371bd4..5d0325e59 100644 --- a/src/main/java/run/halo/app/handler/file/BaiduBosFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/BaiduBosFileHandler.java @@ -132,7 +132,7 @@ public class BaiduBosFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.BAIDUBOS.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.BAIDUBOS; } } diff --git a/src/main/java/run/halo/app/handler/file/FileHandler.java b/src/main/java/run/halo/app/handler/file/FileHandler.java index abad7797a..020f5c871 100644 --- a/src/main/java/run/halo/app/handler/file/FileHandler.java +++ b/src/main/java/run/halo/app/handler/file/FileHandler.java @@ -7,6 +7,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.web.multipart.MultipartFile; import run.halo.app.exception.FileOperationException; +import run.halo.app.model.enums.AttachmentType; import run.halo.app.model.support.UploadResult; import static run.halo.app.model.support.HaloConst.FILE_SEPARATOR; @@ -73,10 +74,8 @@ public interface FileHandler { void delete(@NonNull String key); /** - * Checks if the given type is supported. - * - * @param type attachment type - * @return true if supported; false otherwise + * Get attachment type is supported. + * @return attachment type */ - boolean supportType(@Nullable String type); -} + AttachmentType getAttachmentType(); +} \ No newline at end of file diff --git a/src/main/java/run/halo/app/handler/file/FileHandlers.java b/src/main/java/run/halo/app/handler/file/FileHandlers.java index 2adb4a591..0e043d3d5 100644 --- a/src/main/java/run/halo/app/handler/file/FileHandlers.java +++ b/src/main/java/run/halo/app/handler/file/FileHandlers.java @@ -9,12 +9,13 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.web.multipart.MultipartFile; import run.halo.app.exception.FileOperationException; +import run.halo.app.exception.RepeatTypeException; import run.halo.app.model.entity.Attachment; import run.halo.app.model.enums.AttachmentType; import run.halo.app.model.support.UploadResult; import java.util.Collection; -import java.util.LinkedList; +import java.util.concurrent.ConcurrentHashMap; /** * File handler manager. @@ -29,7 +30,7 @@ public class FileHandlers { /** * File handler container. */ - private final Collection fileHandlers = new LinkedList<>(); + private final ConcurrentHashMap fileHandlers = new ConcurrentHashMap<>(16); public FileHandlers(ApplicationContext applicationContext) { // Add all file handler @@ -47,31 +48,9 @@ public class FileHandlers { */ @NonNull public UploadResult upload(@NonNull MultipartFile file, @NonNull AttachmentType attachmentType) { - Assert.notNull(attachmentType, "Attachment type must not be null"); - - return upload(file, attachmentType.name()); + return getSupportedType(attachmentType).upload(file); } - /** - * 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(type)) { - return fileHandler.upload(file); - } - } - - throw new FileOperationException("No available file handlers to upload the file").setErrorData(type); - } /** * Deletes attachment. @@ -81,26 +60,8 @@ 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(type)) { - // Delete the file - fileHandler.delete(key); - return; - } - } - - throw new FileOperationException("No available file handlers to delete the file").setErrorData(type); + getSupportedType(attachment.getType()) + .delete(attachment.getFileKey()); } /** @@ -112,8 +73,21 @@ public class FileHandlers { @NonNull public FileHandlers addFileHandlers(@Nullable Collection fileHandlers) { if (!CollectionUtils.isEmpty(fileHandlers)) { - this.fileHandlers.addAll(fileHandlers); + for (FileHandler handler : fileHandlers) { + if (this.fileHandlers.containsKey(handler.getAttachmentType())) { + throw new RepeatTypeException("Same attachment type implements must be unique"); + } + this.fileHandlers.put(handler.getAttachmentType(), handler); + } } return this; } + + private FileHandler getSupportedType(AttachmentType type) { + FileHandler handler = fileHandlers.getOrDefault(type, fileHandlers.get(AttachmentType.LOCAL)); + if (handler == null) { + throw new FileOperationException("No available file handlers to operate the file").setErrorData(type); + } + return handler; + } } diff --git a/src/main/java/run/halo/app/handler/file/LocalFileHandler.java b/src/main/java/run/halo/app/handler/file/LocalFileHandler.java index 06cbbfbc2..57e72175c 100644 --- a/src/main/java/run/halo/app/handler/file/LocalFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/LocalFileHandler.java @@ -214,8 +214,8 @@ public class LocalFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.LOCAL.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.LOCAL; } private boolean generateThumbnail(BufferedImage originalImage, Path thumbPath, String extension) { @@ -234,6 +234,9 @@ public class LocalFileHandler implements FileHandler { result = true; } catch (Throwable t) { log.warn("Failed to generate thumbnail: " + thumbPath, t); + } finally { + // Disposes of this graphics context and releases any system resources that it is using. + originalImage.getGraphics().dispose(); } return result; } diff --git a/src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java b/src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java index 99421a1c8..105d6810f 100644 --- a/src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java @@ -177,7 +177,7 @@ public class QiniuOssFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.QINIUOSS.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.QINIUOSS; } } diff --git a/src/main/java/run/halo/app/handler/file/SmmsFileHandler.java b/src/main/java/run/halo/app/handler/file/SmmsFileHandler.java index 41511b053..5474fbf9d 100644 --- a/src/main/java/run/halo/app/handler/file/SmmsFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/SmmsFileHandler.java @@ -165,8 +165,8 @@ public class SmmsFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.SMMS.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.SMMS; } /** diff --git a/src/main/java/run/halo/app/handler/file/TencentCosFileHandler.java b/src/main/java/run/halo/app/handler/file/TencentCosFileHandler.java index 5d70ca792..a7cbd62d9 100644 --- a/src/main/java/run/halo/app/handler/file/TencentCosFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/TencentCosFileHandler.java @@ -163,7 +163,7 @@ public class TencentCosFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.TENCENTCOS.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.TENCENTCOS; } } diff --git a/src/main/java/run/halo/app/handler/file/UpOssFileHandler.java b/src/main/java/run/halo/app/handler/file/UpOssFileHandler.java index cdfbc1a0b..8832fc0a9 100644 --- a/src/main/java/run/halo/app/handler/file/UpOssFileHandler.java +++ b/src/main/java/run/halo/app/handler/file/UpOssFileHandler.java @@ -128,7 +128,7 @@ public class UpOssFileHandler implements FileHandler { } @Override - public boolean supportType(String type) { - return AttachmentType.UPOSS.name().equalsIgnoreCase(type); + public AttachmentType getAttachmentType() { + return AttachmentType.UPOSS; } } diff --git a/src/main/java/run/halo/app/service/impl/ThemeServiceImpl.java b/src/main/java/run/halo/app/service/impl/ThemeServiceImpl.java index e459da4c5..25c142f61 100644 --- a/src/main/java/run/halo/app/service/impl/ThemeServiceImpl.java +++ b/src/main/java/run/halo/app/service/impl/ThemeServiceImpl.java @@ -934,7 +934,6 @@ public class ThemeServiceImpl implements ThemeService { return true; } } - return false; } }