From 5f7ea18f7c7141d06fb698ec84cc3682edb541f1 Mon Sep 17 00:00:00 2001 From: John Niang Date: Wed, 15 Feb 2023 13:38:12 +0800 Subject: [PATCH] Auto rename attachment if it exists (#3305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /milestone 2.3.x #### What this PR does / why we need it: Before this PR, halo will throw an FileAlreadyExists exception if users upload file which already exists. But now, we will rename the attachment automatically on filename conflict. e.g.: ```bash halo.run -> halo-xyz.run .run -> xyz.run halo -> halo-xyz ``` #### Which issue(s) this PR fixes: Fixes https://github.com/halo-dev/halo/issues/3218 #### Special notes for your reviewer: ![image](https://user-images.githubusercontent.com/16865714/218670068-87389289-1248-48e8-82a4-1bcf939e64e3.png) #### Does this PR introduce a user-facing change? ```release-note 附件已存在时自动重命名 ``` --- .../LocalAttachmentUploadHandler.java | 56 ++++++++-- .../halo/app/infra/utils/FileNameUtils.java | 29 +++++ .../app/infra/utils/FileNameUtilsTest.java | 101 +++++++++++------- 3 files changed, 139 insertions(+), 47 deletions(-) diff --git a/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java b/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java index 6e97211ef..45608eb3c 100644 --- a/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java +++ b/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java @@ -1,6 +1,7 @@ package run.halo.app.core.extension.attachment.endpoint; import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static run.halo.app.infra.utils.FileNameUtils.randomFileName; import static run.halo.app.infra.utils.FileUtils.checkDirectoryTraversal; import java.io.IOException; @@ -12,16 +13,20 @@ import java.util.ArrayList; import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; import lombok.Data; import lombok.extern.slf4j.Slf4j; +import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.http.MediaType; import org.springframework.stereotype.Component; import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; import reactor.core.Exceptions; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; +import reactor.util.retry.Retry; import run.halo.app.core.extension.attachment.Attachment; import run.halo.app.core.extension.attachment.Attachment.AttachmentSpec; import run.halo.app.core.extension.attachment.Constant; @@ -76,18 +81,17 @@ class LocalAttachmentUploadHandler implements AttachmentHandler { } }) .subscribeOn(Schedulers.boundedElastic()) - // save the attachment - .then(DataBufferUtils.write(file.content(), attachmentPath, CREATE_NEW)) - .then(Mono.fromCallable(() -> { - log.info("Wrote attachment {} into {}", file.filename(), attachmentPath); + .then(writeContent(file.content(), attachmentPath, true)) + .map(path -> { + log.info("Wrote attachment {} into {}", file.filename(), path); // TODO check the file extension var metadata = new Metadata(); metadata.setName(UUID.randomUUID().toString()); - var relativePath = attachmentsRoot.relativize(attachmentPath).toString(); + var relativePath = attachmentsRoot.relativize(path).toString(); var pathSegments = new ArrayList(); pathSegments.add("upload"); - for (Path p : uploadRoot.relativize(attachmentPath)) { + for (Path p : uploadRoot.relativize(path)) { pathSegments.add(p.toString()); } @@ -100,17 +104,16 @@ class LocalAttachmentUploadHandler implements AttachmentHandler { Constant.LOCAL_REL_PATH_ANNO_KEY, relativePath, Constant.URI_ANNO_KEY, uri)); var spec = new AttachmentSpec(); - spec.setSize(attachmentPath.toFile().length()); - file.headers().getContentType(); + spec.setSize(path.toFile().length()); spec.setMediaType(Optional.ofNullable(file.headers().getContentType()) .map(MediaType::toString) .orElse(null)); - spec.setDisplayName(file.filename()); + spec.setDisplayName(path.getFileName().toString()); var attachment = new Attachment(); attachment.setMetadata(metadata); attachment.setSpec(spec); return attachment; - })) + }) .onErrorMap(FileAlreadyExistsException.class, e -> new AttachmentAlreadyExistsException(e.getFile())); }); @@ -165,4 +168,37 @@ class LocalAttachmentUploadHandler implements AttachmentHandler { private String location; } + + /** + * Write content into file. We will detect duplicate filename and auto-rename it with 3 times + * retry. + * + * @param content is file content + * @param targetPath is target path + * @return file path + */ + private Mono writeContent(Flux content, + Path targetPath, + boolean renameIfExists) { + return Mono.defer(() -> { + final var pathRef = new AtomicReference<>(targetPath); + return Mono.defer( + // we have to use defer method to obtain a fresh path + () -> DataBufferUtils.write(content, pathRef.get(), CREATE_NEW)) + .retryWhen(Retry.max(3) + .filter(t -> { + if (renameIfExists) { + return t instanceof FileAlreadyExistsException; + } + return false; + }) + .doAfterRetry(signal -> { + // rename the path + var oldPath = pathRef.get(); + var fileName = randomFileName(oldPath.toString(), 4); + pathRef.set(oldPath.resolveSibling(fileName)); + })) + .then(Mono.fromSupplier(pathRef::get)); + }); + } } diff --git a/src/main/java/run/halo/app/infra/utils/FileNameUtils.java b/src/main/java/run/halo/app/infra/utils/FileNameUtils.java index 10cc028fe..36ef71e26 100644 --- a/src/main/java/run/halo/app/infra/utils/FileNameUtils.java +++ b/src/main/java/run/halo/app/infra/utils/FileNameUtils.java @@ -1,5 +1,9 @@ package run.halo.app.infra.utils; +import com.google.common.io.Files; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.commons.lang3.StringUtils; + public final class FileNameUtils { private FileNameUtils() { @@ -12,4 +16,29 @@ public final class FileNameUtils { var extPattern = "(? + * Case 1: halo.run -> halo-xyz.run + * Case 2: .run -> xyz.run + * Case 3: halo -> halo-xyz + * + * + * @param filename is name of file. + * @param length is for generating random string with specific length. + * @return File name with random string. + */ + public static String randomFileName(String filename, int length) { + var nameWithoutExt = Files.getNameWithoutExtension(filename); + var ext = Files.getFileExtension(filename); + var random = RandomStringUtils.randomAlphabetic(length).toLowerCase(); + if (StringUtils.isBlank(nameWithoutExt)) { + return random + "." + ext; + } + if (StringUtils.isBlank(ext)) { + return nameWithoutExt + "-" + random; + } + return nameWithoutExt + "-" + random + "." + ext; + } } diff --git a/src/test/java/run/halo/app/infra/utils/FileNameUtilsTest.java b/src/test/java/run/halo/app/infra/utils/FileNameUtilsTest.java index ded4d54f0..98b9c7f8e 100644 --- a/src/test/java/run/halo/app/infra/utils/FileNameUtilsTest.java +++ b/src/test/java/run/halo/app/infra/utils/FileNameUtilsTest.java @@ -2,50 +2,77 @@ package run.halo.app.infra.utils; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static run.halo.app.infra.utils.FileNameUtils.randomFileName; +import static run.halo.app.infra.utils.FileNameUtils.removeFileExtension; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; class FileNameUtilsTest { - @Test - public void shouldNotRemoveExtIfNoExt() { - assertEquals("halo", FileNameUtils.removeFileExtension("halo", true)); - assertEquals("halo", FileNameUtils.removeFileExtension("halo", false)); + @Nested + class RemoveFileExtensionTest { + + @Test + public void shouldNotRemoveExtIfNoExt() { + assertEquals("halo", removeFileExtension("halo", true)); + assertEquals("halo", removeFileExtension("halo", false)); + } + + @Test + public void shouldRemoveExtIfHasOnlyOneExt() { + assertEquals("halo", removeFileExtension("halo.run", true)); + assertEquals("halo", removeFileExtension("halo.run", false)); + } + + @Test + public void shouldNotRemoveExtIfDotfile() { + assertEquals(".halo", removeFileExtension(".halo", true)); + assertEquals(".halo", removeFileExtension(".halo", false)); + } + + @Test + public void shouldRemoveExtIfDotfileHasOneExt() { + assertEquals(".halo", removeFileExtension(".halo.run", true)); + assertEquals(".halo", removeFileExtension(".halo.run", false)); + } + + @Test + public void shouldRemoveExtIfHasTwoExt() { + assertEquals("halo", removeFileExtension("halo.tar.gz", true)); + assertEquals("halo.tar", removeFileExtension("halo.tar.gz", false)); + } + + @Test + public void shouldRemoveExtIfDotfileHasTwoExt() { + assertEquals(".halo", removeFileExtension(".halo.tar.gz", true)); + assertEquals(".halo.tar", removeFileExtension(".halo.tar.gz", false)); + } + + @Test + void shouldReturnNullIfFilenameIsNull() { + assertNull(removeFileExtension(null, true)); + assertNull(removeFileExtension(null, false)); + } } - @Test - public void shouldRemoveExtIfHasOnlyOneExt() { - assertEquals("halo", FileNameUtils.removeFileExtension("halo.run", true)); - assertEquals("halo", FileNameUtils.removeFileExtension("halo.run", false)); - } + @Nested + class AppendRandomFileNameTest { + @Test + void normalFileName() { + String randomFileName = randomFileName("halo.run", 3); + assertEquals(12, randomFileName.length()); + assertTrue(randomFileName.startsWith("halo-")); + assertTrue(randomFileName.endsWith(".run")); - @Test - public void shouldNotRemoveExtIfDotfile() { - assertEquals(".halo", FileNameUtils.removeFileExtension(".halo", true)); - assertEquals(".halo", FileNameUtils.removeFileExtension(".halo", false)); - } + randomFileName = randomFileName(".run", 3); + assertEquals(7, randomFileName.length()); + assertTrue(randomFileName.endsWith(".run")); - @Test - public void shouldRemoveExtIfDotfileHasOneExt() { - assertEquals(".halo", FileNameUtils.removeFileExtension(".halo.run", true)); - assertEquals(".halo", FileNameUtils.removeFileExtension(".halo.run", false)); + randomFileName = randomFileName("halo", 3); + assertEquals(8, randomFileName.length()); + assertTrue(randomFileName.startsWith("halo-")); + } } - - @Test - public void shouldRemoveExtIfHasTwoExt() { - assertEquals("halo", FileNameUtils.removeFileExtension("halo.tar.gz", true)); - assertEquals("halo.tar", FileNameUtils.removeFileExtension("halo.tar.gz", false)); - } - - @Test - public void shouldRemoveExtIfDotfileHasTwoExt() { - assertEquals(".halo", FileNameUtils.removeFileExtension(".halo.tar.gz", true)); - assertEquals(".halo.tar", FileNameUtils.removeFileExtension(".halo.tar.gz", false)); - } - - @Test - void shouldReturnNullIfFilenameIsNull() { - assertNull(FileNameUtils.removeFileExtension(null, true)); - assertNull(FileNameUtils.removeFileExtension(null, false)); - } -} \ No newline at end of file +}