mirror of https://github.com/halo-dev/halo
feat: support validate requires version for theme (#3150)
#### What type of PR is this? /kind feature /milestone 2.2.x /kind api-change #### What this PR does / why we need it: 主题支持 requires 版本验证 - 主题安装和更新时会校验 requires,如果不通过则抛出异常。 - 主题重载或者 halo 升级后导致主题 requires 不满足条件,则会在 status 中将 phase 标记为 failed ⚠️ 特别说明: - 由于之前主题的 require 字段沿用了 1.x 版本,并没有发现与插件的 requires 属性命名不相同,因此添加了一个 requires 属性,并将 require 标记为过时,后续移除它,这个改动不算是破坏性,但如果主题需要版本校验则需要更改 theme.yaml 为 requires - 校验后如果有错误则 console 端应该提示主题升级,这样可以避免用户不知情导致很长时间主题端模板都渲染错误问题。可以通过判断 status.phase 是否为 FAILED 来展示 conditions 的第一个 item 的信息。 - 例如 halo 升级后 requires 不满足条件这种情况,并且这个主题被启用,也不会去取消激活(requires 不满足条件不会导致主题无法激活)。 **Console 需要做适配** 综上,console 应该需要: - 修改 `Halo 版本要求` 的字段取值,从 *require* -> *requires* - 提供错误信息展示,让用户知道主题不适配应该升级等错误。 #### Which issue(s) this PR fixes: Fixes #3088 #### Special notes for your reviewer: how to test it? - 测试安装、升级在 requires 满足条件和不满足时的状况是否符合描述中所述情况。 - 主题重载时加载了一个新的 requires 值触发不满足系统要求条件会在 status 中有错误信息,但不影响主题激活 /cc @halo-dev/sig-halo #### Does this PR introduce a user-facing change? ```release-note 主题支持 requires 版本验证,原 Halo 版本要求属性从 require 更名为 requires ```pull/3166/head^2
parent
cb850a85c9
commit
da07d75df3
|
@ -5,10 +5,13 @@ import java.util.List;
|
|||
import lombok.Data;
|
||||
import lombok.EqualsAndHashCode;
|
||||
import lombok.ToString;
|
||||
import org.apache.commons.lang3.ObjectUtils;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.springframework.lang.NonNull;
|
||||
import org.springframework.util.Assert;
|
||||
import run.halo.app.extension.AbstractExtension;
|
||||
import run.halo.app.extension.GVK;
|
||||
import run.halo.app.infra.ConditionList;
|
||||
|
||||
/**
|
||||
* <p>Theme extension.</p>
|
||||
|
@ -53,8 +56,12 @@ public class Theme extends AbstractExtension {
|
|||
|
||||
private String version;
|
||||
|
||||
@Deprecated(forRemoval = true, since = "2.2.0")
|
||||
@Schema(description = "Deprecated, use `requires` instead.")
|
||||
private String require;
|
||||
|
||||
private String requires;
|
||||
|
||||
private String settingName;
|
||||
|
||||
private String configMapName;
|
||||
|
@ -64,26 +71,53 @@ public class Theme extends AbstractExtension {
|
|||
|
||||
@NonNull
|
||||
public String getVersion() {
|
||||
if (StringUtils.isBlank(this.version)) {
|
||||
return WILDCARD;
|
||||
}
|
||||
return version;
|
||||
return StringUtils.defaultString(this.version, WILDCARD);
|
||||
}
|
||||
|
||||
/**
|
||||
* if requires is not empty, then return requires, else return require or {@code WILDCARD}.
|
||||
*
|
||||
* @return requires to satisfies system version
|
||||
*/
|
||||
@NonNull
|
||||
public String getRequire() {
|
||||
if (StringUtils.isBlank(this.require)) {
|
||||
return WILDCARD;
|
||||
public String getRequires() {
|
||||
if (StringUtils.isNotBlank(this.requires)) {
|
||||
return this.requires;
|
||||
}
|
||||
return require;
|
||||
return StringUtils.defaultString(this.require, WILDCARD);
|
||||
}
|
||||
}
|
||||
|
||||
@Data
|
||||
public static class ThemeStatus {
|
||||
private ThemePhase phase;
|
||||
private ConditionList conditions;
|
||||
private String location;
|
||||
}
|
||||
|
||||
/**
|
||||
* Null-safe get {@link ConditionList} from theme status.
|
||||
*
|
||||
* @param theme theme must not be null
|
||||
* @return condition list
|
||||
*/
|
||||
public static ConditionList nullSafeConditionList(Theme theme) {
|
||||
Assert.notNull(theme, "The theme must not be null");
|
||||
ThemeStatus status = ObjectUtils.defaultIfNull(theme.getStatus(), new ThemeStatus());
|
||||
theme.setStatus(status);
|
||||
|
||||
ConditionList conditions =
|
||||
ObjectUtils.defaultIfNull(status.getConditions(), new ConditionList());
|
||||
status.setConditions(conditions);
|
||||
return conditions;
|
||||
}
|
||||
|
||||
public enum ThemePhase {
|
||||
READY,
|
||||
FAILED,
|
||||
UNKNOWN,
|
||||
}
|
||||
|
||||
@Data
|
||||
@ToString
|
||||
public static class Author {
|
||||
|
|
|
@ -2,6 +2,7 @@ package run.halo.app.core.extension.reconciler;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Path;
|
||||
import java.time.Instant;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
@ -24,9 +25,13 @@ import run.halo.app.extension.controller.Controller;
|
|||
import run.halo.app.extension.controller.ControllerBuilder;
|
||||
import run.halo.app.extension.controller.Reconciler;
|
||||
import run.halo.app.extension.controller.Reconciler.Request;
|
||||
import run.halo.app.infra.Condition;
|
||||
import run.halo.app.infra.ConditionStatus;
|
||||
import run.halo.app.infra.SystemVersionSupplier;
|
||||
import run.halo.app.infra.exception.ThemeUninstallException;
|
||||
import run.halo.app.infra.properties.HaloProperties;
|
||||
import run.halo.app.infra.utils.JsonUtils;
|
||||
import run.halo.app.infra.utils.VersionUtils;
|
||||
import run.halo.app.theme.ThemePathPolicy;
|
||||
|
||||
/**
|
||||
|
@ -41,6 +46,7 @@ public class ThemeReconciler implements Reconciler<Request> {
|
|||
|
||||
private final ExtensionClient client;
|
||||
private final ThemePathPolicy themePathPolicy;
|
||||
private final SystemVersionSupplier systemVersionSupplier;
|
||||
|
||||
private final RetryTemplate retryTemplate = RetryTemplate.builder()
|
||||
.maxAttempts(20)
|
||||
|
@ -48,9 +54,11 @@ public class ThemeReconciler implements Reconciler<Request> {
|
|||
.retryOn(IllegalStateException.class)
|
||||
.build();
|
||||
|
||||
public ThemeReconciler(ExtensionClient client, HaloProperties haloProperties) {
|
||||
public ThemeReconciler(ExtensionClient client, HaloProperties haloProperties,
|
||||
SystemVersionSupplier systemVersionSupplier) {
|
||||
this.client = client;
|
||||
themePathPolicy = new ThemePathPolicy(haloProperties.getWorkDir());
|
||||
this.systemVersionSupplier = systemVersionSupplier;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -77,12 +85,35 @@ public class ThemeReconciler implements Reconciler<Request> {
|
|||
|
||||
private void reconcileStatus(String name) {
|
||||
client.fetch(Theme.class, name).ifPresent(theme -> {
|
||||
Theme oldTheme = JsonUtils.deepCopy(theme);
|
||||
final Theme oldTheme = JsonUtils.deepCopy(theme);
|
||||
if (theme.getStatus() == null) {
|
||||
theme.setStatus(new Theme.ThemeStatus());
|
||||
}
|
||||
Theme.ThemeStatus status = theme.getStatus();
|
||||
|
||||
Path themePath = themePathPolicy.generate(theme);
|
||||
theme.getStatus().setLocation(themePath.toAbsolutePath().toString());
|
||||
status.setLocation(themePath.toAbsolutePath().toString());
|
||||
if (status.getPhase() == null) {
|
||||
status.setPhase(Theme.ThemePhase.READY);
|
||||
}
|
||||
|
||||
// Check if this theme version is match requires param.
|
||||
String normalVersion = systemVersionSupplier.get().getNormalVersion();
|
||||
String requires = theme.getSpec().getRequires();
|
||||
if (!VersionUtils.satisfiesRequires(normalVersion, requires)) {
|
||||
status.setPhase(Theme.ThemePhase.FAILED);
|
||||
Condition condition = Condition.builder()
|
||||
.type(Theme.ThemePhase.FAILED.name())
|
||||
.status(ConditionStatus.FALSE)
|
||||
.reason("UnsatisfiedRequiresVersion")
|
||||
.message(String.format(
|
||||
"Theme requires a minimum system version of [%s], and you have [%s].",
|
||||
requires, normalVersion))
|
||||
.lastTransitionTime(Instant.now())
|
||||
.build();
|
||||
Theme.nullSafeConditionList(theme).add(condition);
|
||||
}
|
||||
|
||||
if (!oldTheme.equals(theme)) {
|
||||
client.update(theme);
|
||||
}
|
||||
|
|
|
@ -17,6 +17,7 @@ import java.util.Objects;
|
|||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.zip.ZipInputStream;
|
||||
import lombok.AllArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.springframework.dao.OptimisticLockingFailureException;
|
||||
|
@ -37,21 +38,22 @@ import run.halo.app.extension.ConfigMap;
|
|||
import run.halo.app.extension.ExtensionUtil;
|
||||
import run.halo.app.extension.ReactiveExtensionClient;
|
||||
import run.halo.app.extension.Unstructured;
|
||||
import run.halo.app.infra.SystemVersionSupplier;
|
||||
import run.halo.app.infra.ThemeRootGetter;
|
||||
import run.halo.app.infra.exception.ThemeUpgradeException;
|
||||
import run.halo.app.infra.exception.UnsatisfiedAttributeValueException;
|
||||
import run.halo.app.infra.utils.VersionUtils;
|
||||
|
||||
@Slf4j
|
||||
@Service
|
||||
@AllArgsConstructor
|
||||
public class ThemeServiceImpl implements ThemeService {
|
||||
|
||||
private final ReactiveExtensionClient client;
|
||||
|
||||
private final ThemeRootGetter themeRoot;
|
||||
|
||||
public ThemeServiceImpl(ReactiveExtensionClient client, ThemeRootGetter themeRoot) {
|
||||
this.client = client;
|
||||
this.themeRoot = themeRoot;
|
||||
}
|
||||
private final SystemVersionSupplier systemVersionSupplier;
|
||||
|
||||
@Override
|
||||
public Mono<Theme> install(InputStream is) {
|
||||
|
@ -139,6 +141,18 @@ public class ThemeServiceImpl implements ThemeService {
|
|||
"Theme manifest kind must be Theme.");
|
||||
return client.create(themeManifest)
|
||||
.map(theme -> Unstructured.OBJECT_MAPPER.convertValue(theme, Theme.class))
|
||||
.doOnNext(theme -> {
|
||||
String systemVersion = systemVersionSupplier.get().getNormalVersion();
|
||||
String requires = theme.getSpec().getRequires();
|
||||
if (!VersionUtils.satisfiesRequires(systemVersion, requires)) {
|
||||
throw new UnsatisfiedAttributeValueException(
|
||||
String.format("The theme requires a minimum system version of %s, "
|
||||
+ "but the current version is %s.",
|
||||
requires, systemVersion),
|
||||
"problemDetail.theme.version.unsatisfied.requires",
|
||||
new String[] {requires, systemVersion});
|
||||
}
|
||||
})
|
||||
.flatMap(theme -> {
|
||||
var unstructureds = ThemeUtils.loadThemeResources(getThemePath(theme));
|
||||
if (unstructureds.stream()
|
||||
|
|
|
@ -34,6 +34,7 @@ problemDetail.theme.upgrade.missingManifest=Missing theme manifest file "theme.y
|
|||
problemDetail.theme.upgrade.nameMismatch=The current theme name {0} did not match the installed theme name.
|
||||
problemDetail.theme.install.missingManifest=Missing theme manifest file "theme.yaml" or "theme.yml".
|
||||
problemDetail.theme.install.alreadyExists=Theme {0} already exists.
|
||||
problemDetail.theme.version.unsatisfied.requires=The theme requires a minimum system version of {0}, but the current version is {1}.
|
||||
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}.
|
||||
|
|
|
@ -5,3 +5,5 @@ problemDetail.title.run.halo.app.infra.exception.AttachmentAlreadyExistsExceptio
|
|||
problemDetail.run.halo.app.infra.exception.AttachmentAlreadyExistsException=文件 {0} 已存在,建议更名后重试。
|
||||
|
||||
problemDetail.plugin.version.unsatisfied.requires=插件要求一个最小的系统版本为 {0}, 但当前版本为 {1}。
|
||||
|
||||
problemDetail.theme.version.unsatisfied.requires=主题要求一个最小的系统版本为 {0}, 但当前版本为 {1}。
|
|
@ -45,7 +45,7 @@ class ThemeTest {
|
|||
themeSpec.setSettingName("test-setting");
|
||||
|
||||
themeSpec.setVersion(null);
|
||||
themeSpec.setRequire(null);
|
||||
themeSpec.setRequires(null);
|
||||
JSONAssert.assertEquals("""
|
||||
{
|
||||
"spec": {
|
||||
|
@ -59,7 +59,7 @@ class ThemeTest {
|
|||
"website": "https://test.com",
|
||||
"repo": "https://test.com",
|
||||
"version": "*",
|
||||
"require": "*",
|
||||
"requires": "*",
|
||||
"settingName": "test-setting",
|
||||
"configMapName": "test-config-map"
|
||||
},
|
||||
|
@ -74,9 +74,9 @@ class ThemeTest {
|
|||
true);
|
||||
|
||||
themeSpec.setVersion("1.0.0");
|
||||
themeSpec.setRequire("2.0.0");
|
||||
themeSpec.setRequires("2.0.0");
|
||||
assertThat(themeSpec.getVersion()).isEqualTo("1.0.0");
|
||||
assertThat(themeSpec.getRequire()).isEqualTo("2.0.0");
|
||||
assertThat(themeSpec.getRequires()).isEqualTo("2.0.0");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
@ -4,10 +4,12 @@ import static org.assertj.core.api.Assertions.assertThat;
|
|||
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.lenient;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import com.github.zafarkhaja.semver.Version;
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
|
@ -38,6 +40,7 @@ import run.halo.app.extension.ExtensionClient;
|
|||
import run.halo.app.extension.Metadata;
|
||||
import run.halo.app.extension.MetadataOperator;
|
||||
import run.halo.app.extension.controller.Reconciler;
|
||||
import run.halo.app.infra.SystemVersionSupplier;
|
||||
import run.halo.app.infra.properties.HaloProperties;
|
||||
import run.halo.app.infra.utils.JsonUtils;
|
||||
import run.halo.app.theme.ThemePathPolicy;
|
||||
|
@ -57,6 +60,9 @@ class ThemeReconcilerTest {
|
|||
@Mock
|
||||
private HaloProperties haloProperties;
|
||||
|
||||
@Mock
|
||||
private SystemVersionSupplier systemVersionSupplier;
|
||||
|
||||
@Mock
|
||||
private File defaultTheme;
|
||||
|
||||
|
@ -66,6 +72,7 @@ class ThemeReconcilerTest {
|
|||
void setUp() throws IOException {
|
||||
tempDirectory = Files.createTempDirectory("halo-theme-");
|
||||
defaultTheme = ResourceUtils.getFile("classpath:themes/default");
|
||||
lenient().when(systemVersionSupplier.get()).thenReturn(Version.valueOf("0.0.0"));
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
|
@ -80,7 +87,7 @@ class ThemeReconcilerTest {
|
|||
when(haloProperties.getWorkDir()).thenReturn(testWorkDir);
|
||||
|
||||
final ThemeReconciler themeReconciler =
|
||||
new ThemeReconciler(extensionClient, haloProperties);
|
||||
new ThemeReconciler(extensionClient, haloProperties, systemVersionSupplier);
|
||||
final ThemePathPolicy themePathPolicy = new ThemePathPolicy(testWorkDir);
|
||||
|
||||
Theme theme = new Theme();
|
||||
|
@ -127,7 +134,7 @@ class ThemeReconcilerTest {
|
|||
when(haloProperties.getWorkDir()).thenReturn(testWorkDir);
|
||||
|
||||
final ThemeReconciler themeReconciler =
|
||||
new ThemeReconciler(extensionClient, haloProperties);
|
||||
new ThemeReconciler(extensionClient, haloProperties, systemVersionSupplier);
|
||||
|
||||
final int[] retryFlags = {0, 0};
|
||||
when(extensionClient.fetch(eq(Setting.class), eq("theme-test-setting")))
|
||||
|
@ -166,7 +173,7 @@ class ThemeReconcilerTest {
|
|||
when(haloProperties.getWorkDir()).thenReturn(testWorkDir);
|
||||
|
||||
final ThemeReconciler themeReconciler =
|
||||
new ThemeReconciler(extensionClient, haloProperties);
|
||||
new ThemeReconciler(extensionClient, haloProperties, systemVersionSupplier);
|
||||
|
||||
final int[] retryFlags = {0};
|
||||
when(extensionClient.fetch(eq(Setting.class), eq("theme-test-setting")))
|
||||
|
@ -211,7 +218,7 @@ class ThemeReconcilerTest {
|
|||
when(haloProperties.getWorkDir()).thenReturn(testWorkDir);
|
||||
|
||||
final ThemeReconciler themeReconciler =
|
||||
new ThemeReconciler(extensionClient, haloProperties);
|
||||
new ThemeReconciler(extensionClient, haloProperties, systemVersionSupplier);
|
||||
|
||||
Theme theme = new Theme();
|
||||
Metadata metadata = new Metadata();
|
||||
|
|
|
@ -13,6 +13,7 @@ import static org.mockito.Mockito.when;
|
|||
import static run.halo.app.infra.utils.FileUtils.deleteRecursivelyAndSilently;
|
||||
import static run.halo.app.infra.utils.FileUtils.zip;
|
||||
|
||||
import com.github.zafarkhaja.semver.Version;
|
||||
import java.io.IOException;
|
||||
import java.net.URISyntaxException;
|
||||
import java.nio.file.Files;
|
||||
|
@ -44,6 +45,7 @@ import run.halo.app.extension.Metadata;
|
|||
import run.halo.app.extension.ReactiveExtensionClient;
|
||||
import run.halo.app.extension.Unstructured;
|
||||
import run.halo.app.extension.exception.ExtensionException;
|
||||
import run.halo.app.infra.SystemVersionSupplier;
|
||||
import run.halo.app.infra.ThemeRootGetter;
|
||||
import run.halo.app.infra.exception.ThemeInstallationException;
|
||||
import run.halo.app.infra.utils.JsonUtils;
|
||||
|
@ -57,6 +59,9 @@ class ThemeServiceImplTest {
|
|||
@Mock
|
||||
ThemeRootGetter themeRoot;
|
||||
|
||||
@Mock
|
||||
private SystemVersionSupplier systemVersionSupplier;
|
||||
|
||||
@InjectMocks
|
||||
ThemeServiceImpl themeService;
|
||||
|
||||
|
@ -68,6 +73,8 @@ class ThemeServiceImplTest {
|
|||
lenient().when(themeRoot.get()).thenReturn(tmpDir.resolve("themes"));
|
||||
// init the folder
|
||||
Files.createDirectory(themeRoot.get());
|
||||
|
||||
lenient().when(systemVersionSupplier.get()).thenReturn(Version.valueOf("0.0.0"));
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
|
@ -253,7 +260,7 @@ class ThemeServiceImplTest {
|
|||
"spec": {
|
||||
"displayName": "Fake Theme",
|
||||
"version": "*",
|
||||
"require": "*"
|
||||
"requires": "*"
|
||||
},
|
||||
"apiVersion": "theme.halo.run/v1alpha1",
|
||||
"kind": "Theme",
|
||||
|
@ -370,7 +377,7 @@ class ThemeServiceImplTest {
|
|||
"settingName": "fake-setting",
|
||||
"displayName": "Fake Theme",
|
||||
"version": "*",
|
||||
"require": "*"
|
||||
"requires": "*"
|
||||
},
|
||||
"apiVersion": "theme.halo.run/v1alpha1",
|
||||
"kind": "Theme",
|
||||
|
|
Loading…
Reference in New Issue