From c2ff3c32f1dddd768f84868216e0756cacad6adc Mon Sep 17 00:00:00 2001 From: guqing Date: Wed, 27 Sep 2023 13:59:28 +0800 Subject: [PATCH] refactor: using databuffer instead of string for plugin bundle resource --- .../extension/endpoint/PluginEndpoint.java | 141 ++++++++++-------- .../core/extension/service/PluginService.java | 5 +- .../service/impl/PluginServiceImpl.java | 92 ++++++------ .../endpoint/PluginEndpointTest.java | 60 ++++++-- 4 files changed, 176 insertions(+), 122 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java b/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java index 40df265c921..e8a289640b0 100644 --- a/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java +++ b/application/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java @@ -3,7 +3,6 @@ import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.NOT_REQUIRED; import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED; import static java.util.Comparator.comparing; -import static org.apache.commons.lang3.StringUtils.defaultString; import static org.springdoc.core.fn.builders.apiresponse.Builder.responseBuilder; import static org.springdoc.core.fn.builders.content.Builder.contentBuilder; import static org.springdoc.core.fn.builders.parameter.Builder.parameterBuilder; @@ -22,7 +21,6 @@ import io.swagger.v3.oas.annotations.media.Schema; import java.io.IOException; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -35,6 +33,9 @@ import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -46,6 +47,7 @@ import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Sort; import org.springframework.http.CacheControl; @@ -65,6 +67,7 @@ import org.springframework.web.reactive.function.server.ServerResponse; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebInputException; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; @@ -255,8 +258,8 @@ public RouterFunction endpoint() { private Mono fetchJsBundle(ServerRequest request) { Optional versionOption = request.queryParam("v"); return versionOption.map(s -> - Mono.fromCallable(() -> bufferedPluginBundleResource - .jsBundleResource(s, () -> pluginService.uglifyJsBundle().block()) + Mono.defer(() -> bufferedPluginBundleResource + .getJsBundle(s, pluginService::uglifyJsBundle) ).flatMap(fsRes -> { var bodyBuilder = ServerResponse.ok() .cacheControl(MAX_CACHE_CONTROL) @@ -284,8 +287,8 @@ private Mono fetchJsBundle(ServerRequest request) { private Mono fetchCssBundle(ServerRequest request) { Optional versionOption = request.queryParam("v"); return versionOption.map(s -> - Mono.fromCallable(() -> bufferedPluginBundleResource.cssBundleResource(s, - () -> pluginService.uglifyCssBundle().block()) + Mono.defer(() -> bufferedPluginBundleResource.getCssBundle(s, + pluginService::uglifyCssBundle) ).flatMap(fsRes -> { var bodyBuilder = ServerResponse.ok() .cacheControl(MAX_CACHE_CONTROL) @@ -672,91 +675,107 @@ private Mono writeToTempFile(Publisher content) { @Component static class BufferedPluginBundleResource implements DisposableBean { - private final Object jsBundleLock = new Object(); - private final Object cssBundleLock = new Object(); - private FileSystemResource jsBundleResource; - private FileSystemResource cssBundleResource; + private final AtomicReference jsBundle = new AtomicReference<>(); + private final AtomicReference cssBundle = new AtomicReference<>(); + + private final ReadWriteLock jsLock = new ReentrantReadWriteLock(); + private final ReadWriteLock cssLock = new ReentrantReadWriteLock(); + private Path tempDir; - public FileSystemResource jsBundleResource(String v, Supplier bundleSupplier) { - synchronized (jsBundleLock) { - var jsResource = writeAndGetBundleResource(jsBundleResource, - tempFileName(v, ".js"), bundleSupplier); - this.jsBundleResource = jsResource; - return jsResource; - } + public Mono getJsBundle(String version, + Supplier> jsSupplier) { + var fileName = tempFileName(version, ".js"); + return Mono.defer(() -> { + try { + jsLock.readLock().lock(); + var jsBundleResource = jsBundle.get(); + if (getResourceIfNotChange(fileName, jsBundleResource) != null) { + return Mono.just(jsBundleResource); + } + } finally { + jsLock.readLock().unlock(); + } + + jsLock.writeLock().lock(); + try { + return writeBundle(fileName, jsSupplier) + .doOnNext(jsBundle::set); + } finally { + jsLock.writeLock().unlock(); + } + }).subscribeOn(Schedulers.boundedElastic()); } - public FileSystemResource cssBundleResource(String v, Supplier bundleSupplier) { - synchronized (cssBundleLock) { - var cssResource = writeAndGetBundleResource(cssBundleResource, - tempFileName(v, ".css"), bundleSupplier); - this.cssBundleResource = cssResource; - return cssResource; - } + public Mono getCssBundle(String version, + Supplier> cssSupplier) { + var fileName = tempFileName(version, ".css"); + return Mono.defer(() -> { + try { + cssLock.readLock().lock(); + var cssBundleResource = cssBundle.get(); + if (getResourceIfNotChange(fileName, cssBundleResource) != null) { + return Mono.just(cssBundleResource); + } + } finally { + cssLock.readLock().unlock(); + } + + cssLock.writeLock().lock(); + try { + return writeBundle(fileName, cssSupplier) + .doOnNext(cssBundle::set); + } finally { + cssLock.writeLock().unlock(); + } + }).subscribeOn(Schedulers.boundedElastic()); } - FileSystemResource writeAndGetBundleResource(FileSystemResource resource, String fileName, - Supplier bundleSupplier) { - if (resourceDoesNotExist(resource)) { - return createAndWriteContent(fileName, bundleSupplier, null); - } - if (fileName.equals(resource.getFilename())) { + @Nullable + private Resource getResourceIfNotChange(String fileName, Resource resource) { + if (resource != null && resource.exists() && fileName.equals(resource.getFilename())) { return resource; } - return createAndWriteContent(fileName, bundleSupplier, resource); + return null; } - FileSystemResource createAndWriteContent(String fileName, Supplier bundleSupplier, - @Nullable FileSystemResource fileResource) { - try { - if (fileResource != null) { - Files.deleteIfExists(fileResource.getFile().toPath()); - } - var filePath = generateStorePath(fileName); - var newResource = new FileSystemResource(filePath); - - String content = defaultString(bundleSupplier.get()); - Files.writeString(filePath, content, StandardCharsets.UTF_8); - return newResource; - } catch (IOException e) { - throw new ServerWebInputException("Failed to write bundle file.", null, e); - } + private Mono writeBundle(String fileName, + Supplier> dataSupplier) { + return Mono.defer( + () -> { + var filePath = createTempFileToStore(fileName); + return DataBufferUtils.write(dataSupplier.get(), filePath) + .then(Mono.fromSupplier(() -> new FileSystemResource(filePath))); + }); } - Path generateStorePath(String fileName) { + private synchronized Path createTempFileToStore(String fileName) { try { - if (tempDirDoesNotExist()) { + if (tempDir == null || !Files.exists(tempDir)) { this.tempDir = Files.createTempDirectory("halo-plugin-bundle"); } - return Files.createFile(tempDir.resolve(fileName)); + var path = tempDir.resolve(fileName); + Files.deleteIfExists(path); + return Files.createFile(path); } catch (IOException e) { throw new ServerWebInputException("Failed to create temp file.", null, e); } } - boolean tempDirDoesNotExist() { - return tempDir == null || !Files.exists(tempDir); - } - - String tempFileName(String v, String suffix) { + private String tempFileName(String v, String suffix) { Assert.notNull(v, "Version must not be null"); Assert.notNull(suffix, "Suffix must not be null"); return v + suffix; } - boolean resourceDoesNotExist(Resource resource) { - return resource == null || !resource.exists(); - } - @Override public void destroy() throws Exception { - if (!tempDirDoesNotExist()) { + if (tempDir != null && Files.exists(tempDir)) { FileSystemUtils.deleteRecursively(tempDir); } - this.jsBundleResource = null; - this.cssBundleResource = null; + this.jsBundle.set(null); + this.cssBundle.set(null); } } } diff --git a/application/src/main/java/run/halo/app/core/extension/service/PluginService.java b/application/src/main/java/run/halo/app/core/extension/service/PluginService.java index bcf404dbec6..27e4f43ff73 100644 --- a/application/src/main/java/run/halo/app/core/extension/service/PluginService.java +++ b/application/src/main/java/run/halo/app/core/extension/service/PluginService.java @@ -1,6 +1,7 @@ package run.halo.app.core.extension.service; import java.nio.file.Path; +import org.springframework.core.io.buffer.DataBuffer; import org.springframework.web.server.ServerWebInputException; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -46,14 +47,14 @@ public interface PluginService { * * @return uglified js bundle */ - Mono uglifyJsBundle(); + Flux uglifyJsBundle(); /** * Uglify css bundle from all enabled plugins to a single css bundle string. * * @return uglified css bundle */ - Mono uglifyCssBundle(); + Flux uglifyCssBundle(); /** *

Generate js bundle version for cache control.

diff --git a/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java b/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java index e5c621c107a..ab029aa69d3 100644 --- a/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java +++ b/application/src/main/java/run/halo/app/core/extension/service/impl/PluginServiceImpl.java @@ -9,7 +9,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -21,9 +20,13 @@ import org.pf4j.PluginWrapper; import org.pf4j.RuntimeMode; import org.springframework.core.io.Resource; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.core.io.support.PathMatchingResourcePatternResolver; import org.springframework.stereotype.Component; import org.springframework.util.Assert; +import org.springframework.util.StreamUtils; import org.springframework.web.server.ServerWebInputException; import reactor.core.Exceptions; import reactor.core.publisher.Flux; @@ -133,55 +136,52 @@ public Mono reload(String name) { } @Override - public Mono uglifyJsBundle() { - return Mono.fromSupplier(() -> { - StringBuilder jsBundle = new StringBuilder(); - List pluginNames = new ArrayList<>(); - for (PluginWrapper pluginWrapper : pluginManager.getStartedPlugins()) { - String pluginName = pluginWrapper.getPluginId(); - pluginNames.add(pluginName); - Resource jsBundleResource = - BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, - BundleResourceUtils.JS_BUNDLE); - if (jsBundleResource != null) { - try { - jsBundle.append( - jsBundleResource.getContentAsString(StandardCharsets.UTF_8)); - jsBundle.append("\n"); - } catch (IOException e) { - log.error("Failed to read js bundle of plugin [{}]", pluginName, e); - } + public Flux uglifyJsBundle() { + var startedPlugins = List.copyOf(pluginManager.getStartedPlugins()); + String plugins = """ + this.enabledPluginNames = [%s]; + """.formatted(startedPlugins.stream() + .map(PluginWrapper::getPluginId) + .collect(Collectors.joining("','", "'", "'"))); + return Flux.fromIterable(startedPlugins) + .mapNotNull(pluginWrapper -> { + var pluginName = pluginWrapper.getPluginId(); + return BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, + BundleResourceUtils.JS_BUNDLE); + }) + .flatMap(resource -> { + try { + // Specifying bufferSize as resource content length is + // to append line breaks at the end of each plugin + return DataBufferUtils.read(resource, DefaultDataBufferFactory.sharedInstance, + (int) resource.contentLength()) + .doOnNext(dataBuffer -> { + // add a new line after each plugin bundle to avoid syntax error + dataBuffer.write("\n".getBytes(StandardCharsets.UTF_8)); + }); + } catch (IOException e) { + log.error("Failed to read plugin bundle resource", e); + return Flux.empty(); } - } - - String plugins = """ - this.enabledPluginNames = [%s]; - """.formatted(pluginNames.stream() - .collect(Collectors.joining("','", "'", "'"))); - return jsBundle + plugins; - }); + }) + .concatWith(Flux.defer(() -> { + var dataBuffer = DefaultDataBufferFactory.sharedInstance + .wrap(plugins.getBytes(StandardCharsets.UTF_8)); + return Flux.just(dataBuffer); + })); } @Override - public Mono uglifyCssBundle() { - return Mono.fromSupplier(() -> { - StringBuilder cssBundle = new StringBuilder(); - for (PluginWrapper pluginWrapper : pluginManager.getStartedPlugins()) { + public Flux uglifyCssBundle() { + return Flux.fromIterable(pluginManager.getStartedPlugins()) + .mapNotNull(pluginWrapper -> { String pluginName = pluginWrapper.getPluginId(); - Resource cssBundleResource = - BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, - BundleResourceUtils.CSS_BUNDLE); - if (cssBundleResource != null) { - try { - cssBundle.append( - cssBundleResource.getContentAsString(StandardCharsets.UTF_8)); - } catch (IOException e) { - log.error("Failed to read css bundle of plugin [{}]", pluginName, e); - } - } - } - return cssBundle.toString(); - }); + return BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, + BundleResourceUtils.CSS_BUNDLE); + }) + .flatMap(resource -> DataBufferUtils.read(resource, + DefaultDataBufferFactory.sharedInstance, StreamUtils.BUFFER_SIZE) + ); } @Override @@ -259,7 +259,7 @@ private void satisfiesRequiresVersion(Plugin newPlugin) { if (!VersionUtils.satisfiesRequires(systemVersion, requires)) { throw new UnsatisfiedAttributeValueException(String.format( "Plugin requires a minimum system version of [%s], but the current version is " - + "[%s].", + + "[%s].", requires, systemVersion), "problemDetail.plugin.version.unsatisfied.requires", new String[] {requires, systemVersion}); diff --git a/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java b/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java index 2ff8261b20c..d57f2f96ccf 100644 --- a/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java +++ b/application/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java @@ -35,11 +35,15 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.MediaType; import org.springframework.http.client.MultipartBodyBuilder; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.server.ServerWebInputException; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import run.halo.app.core.extension.Plugin; import run.halo.app.core.extension.Setting; import run.halo.app.core.extension.service.PluginService; @@ -390,24 +394,54 @@ class BufferedPluginBundleResourceTest { private final PluginEndpoint.BufferedPluginBundleResource bufferedPluginBundleResource = new PluginEndpoint.BufferedPluginBundleResource(); + private static Flux getDataBufferFlux(String x) { + var buffer = DefaultDataBufferFactory.sharedInstance + .wrap(x.getBytes(StandardCharsets.UTF_8)); + return Flux.just(buffer); + } + @Test - void writeAndGetResourceTest() throws IOException { - var resource = - bufferedPluginBundleResource.jsBundleResource("1", () -> "first line\nnext line"); - var content = resource.getContentAsString(StandardCharsets.UTF_8); - assertThat(content).isEqualTo("first line\nnext line"); + void writeAndGetResourceTest() { + bufferedPluginBundleResource.getJsBundle("1", + () -> getDataBufferFlux("first line\nnext line")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); // version is matched, should return cached content - resource = - bufferedPluginBundleResource.jsBundleResource("1", () -> "first line\nnext line-1"); - content = resource.getContentAsString(StandardCharsets.UTF_8); - assertThat(content).isEqualTo("first line\nnext line"); + bufferedPluginBundleResource.getJsBundle("1", + () -> getDataBufferFlux("first line\nnext line-1")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); // new version should return new content - resource = - bufferedPluginBundleResource.jsBundleResource("2", () -> "first line\nnext line-2"); - content = resource.getContentAsString(StandardCharsets.UTF_8); - assertThat(content).isEqualTo("first line\nnext line-2"); + bufferedPluginBundleResource.getJsBundle("2", + () -> getDataBufferFlux("first line\nnext line-2")) + .as(StepVerifier::create) + .consumeNextWith(resource -> { + try { + String content = resource.getContentAsString(StandardCharsets.UTF_8); + assertThat(content).isEqualTo("first line\nnext line-2"); + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .verifyComplete(); } } }