From c8cc9f2710e1b5e8a360db292176ef114f0a945d Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Fri, 26 May 2023 22:56:12 +0800 Subject: [PATCH] refactor: exception prompts during plugin installation (#3993) 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.6.x #### What this PR does / why we need it: 优化插件安装失败的提示信息 插件安装和升级时由于包格式不正确改为如下提示(Localization) image how to test it? 使用下面的插件安装和升级会提示 plugin.yaml 缺失 [failed-plugins.zip](https://github.com/halo-dev/halo/files/11560921/failed-plugins.zip) see #3843 for more details #### Which issue(s) this PR fixes: Fixes #3843 #### Does this PR introduce a user-facing change? ```release-note 优化插件安装失败的提示信息 ``` --- .../service/impl/PluginServiceImpl.java | 84 +++++++++++-------- .../run/halo/app/plugin/YamlPluginFinder.java | 3 +- .../resources/config/i18n/messages.properties | 1 + .../config/i18n/messages_zh.properties | 1 + 4 files changed, 51 insertions(+), 38 deletions(-) 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 9eb2933b1..5f7d9a834 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 @@ -28,6 +28,7 @@ import run.halo.app.extension.MetadataUtil; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.infra.SystemVersionSupplier; import run.halo.app.infra.exception.PluginAlreadyExistsException; +import run.halo.app.infra.exception.PluginInstallationException; import run.halo.app.infra.exception.UnsatisfiedAttributeValueException; import run.halo.app.infra.utils.FileUtils; import run.halo.app.infra.utils.VersionUtils; @@ -70,49 +71,47 @@ public class PluginServiceImpl implements PluginService { @Override public Mono install(Path path) { - return Mono.defer(() -> { - final var pluginFinder = new YamlPluginFinder(); - final var pluginInPath = pluginFinder.find(path); - // validate the plugin version - satisfiesRequiresVersion(pluginInPath); + return findPluginManifest(path) + .flatMap(pluginInPath -> { + // validate the plugin version + satisfiesRequiresVersion(pluginInPath); - return client.fetch(Plugin.class, pluginInPath.getMetadata().getName()) - .flatMap(oldPlugin -> Mono.error( - new PluginAlreadyExistsException(oldPlugin.getMetadata().getName()))) - .switchIfEmpty(Mono.defer( - () -> copyToPluginHome(pluginInPath) - .map(pluginFinder::find) - .doOnNext(p -> { - // Disable auto enable after installation - p.getSpec().setEnabled(false); - }) - .flatMap(client::create))); - - }); + return client.fetch(Plugin.class, pluginInPath.getMetadata().getName()) + .flatMap(oldPlugin -> Mono.error( + new PluginAlreadyExistsException(oldPlugin.getMetadata().getName()))) + .switchIfEmpty(Mono.defer( + () -> copyToPluginHome(pluginInPath) + .flatMap(this::findPluginManifest) + .doOnNext(p -> { + // Disable auto enable after installation + p.getSpec().setEnabled(false); + }) + .flatMap(client::create)) + ); + }); } @Override public Mono upgrade(String name, Path path) { - return Mono.defer(() -> { - // pre-check the plugin in the path - final var pluginFinder = new YamlPluginFinder(); - final var pluginInPath = pluginFinder.find(path); - Validate.notNull(pluginInPath.statusNonNull().getLoadLocation()); - satisfiesRequiresVersion(pluginInPath); - if (!Objects.equals(name, pluginInPath.getMetadata().getName())) { - return Mono.error(new ServerWebInputException( - "The provided plugin " + pluginInPath.getMetadata().getName() - + " didn't match the given plugin " + name)); - } + return findPluginManifest(path) + .flatMap(pluginInPath -> { + // pre-check the plugin in the path + Validate.notNull(pluginInPath.statusNonNull().getLoadLocation()); + satisfiesRequiresVersion(pluginInPath); + if (!Objects.equals(name, pluginInPath.getMetadata().getName())) { + return Mono.error(new ServerWebInputException( + "The provided plugin " + pluginInPath.getMetadata().getName() + + " didn't match the given plugin " + name)); + } - // check if the plugin exists - return client.fetch(Plugin.class, name) - .switchIfEmpty(Mono.error(() -> new ServerWebInputException( - "The given plugin with name " + name + " was not found."))) - // copy plugin into plugin home - .flatMap(prevPlugin -> copyToPluginHome(pluginInPath)) - .flatMap(pluginPath -> updateReloadAnno(name, pluginPath)); - }); + // check if the plugin exists + return client.fetch(Plugin.class, name) + .switchIfEmpty(Mono.error(() -> new ServerWebInputException( + "The given plugin with name " + name + " was not found."))) + // copy plugin into plugin home + .flatMap(prevPlugin -> copyToPluginHome(pluginInPath)) + .flatMap(pluginPath -> updateReloadAnno(name, pluginPath)); + }); } @Override @@ -125,6 +124,17 @@ public class PluginServiceImpl implements PluginService { return updateReloadAnno(name, pluginWrapper.getPluginPath()); } + Mono findPluginManifest(Path path) { + return Mono.fromSupplier( + () -> { + final var pluginFinder = new YamlPluginFinder(); + return pluginFinder.find(path); + }) + .onErrorMap(e -> new PluginInstallationException("Failed to parse the plugin manifest", + "problemDetail.plugin.missingManifest", null) + ); + } + private Mono updateReloadAnno(String name, Path pluginPath) { return client.get(Plugin.class, name) .flatMap(plugin -> { diff --git a/application/src/main/java/run/halo/app/plugin/YamlPluginFinder.java b/application/src/main/java/run/halo/app/plugin/YamlPluginFinder.java index fae21eb0f..882ed99c4 100644 --- a/application/src/main/java/run/halo/app/plugin/YamlPluginFinder.java +++ b/application/src/main/java/run/halo/app/plugin/YamlPluginFinder.java @@ -75,8 +75,9 @@ public class YamlPluginFinder { } protected Plugin readPluginDescriptor(Path pluginPath) { - Path propertiesPath = getManifestPath(pluginPath, propertiesFileName); + Path propertiesPath = null; try { + propertiesPath = getManifestPath(pluginPath, propertiesFileName); if (propertiesPath == null) { throw new PluginRuntimeException("Cannot find the plugin manifest path"); } diff --git a/application/src/main/resources/config/i18n/messages.properties b/application/src/main/resources/config/i18n/messages.properties index c79daaeb4..8519b8089 100644 --- a/application/src/main/resources/config/i18n/messages.properties +++ b/application/src/main/resources/config/i18n/messages.properties @@ -45,3 +45,4 @@ problemDetail.theme.version.unsatisfied.requires=The theme requires a minimum sy problemDetail.directoryTraversal=Directory traversal detected. Base path is {0}, but real path is {1}. problemDetail.plugin.version.unsatisfied.requires=Plugin requires a minimum system version of {0}, but the current version is {1}. +problemDetail.plugin.missingManifest=Missing plugin manifest file "plugin.yaml" or manifest file does not conform to the specification. diff --git a/application/src/main/resources/config/i18n/messages_zh.properties b/application/src/main/resources/config/i18n/messages_zh.properties index 636502628..6c429e30d 100644 --- a/application/src/main/resources/config/i18n/messages_zh.properties +++ b/application/src/main/resources/config/i18n/messages_zh.properties @@ -14,6 +14,7 @@ problemDetail.user.signUpFailed.disallowed=系统不允许注册新用户。 problemDetail.user.duplicateName=用户名 {0} 已存在,请更换用户名后重试。 problemDetail.plugin.version.unsatisfied.requires=插件要求一个最小的系统版本为 {0}, 但当前版本为 {1}。 +problemDetail.plugin.missingManifest=缺少 plugin.yaml 配置文件或配置文件不符合规范。 problemDetail.theme.version.unsatisfied.requires=主题要求一个最小的系统版本为 {0}, 但当前版本为 {1}。 problemDetail.theme.install.missingManifest=缺少 theme.yaml 配置文件或配置文件不符合规范。 \ No newline at end of file