refactor: cannot reload when settingName is not configured before (#2745)

#### What type of PR is this?
/kind improvement
/area core
/milestone 2.0.0-rc.1
/kind api-change

#### What this PR does / why we need it:
- 修复 theme.yaml 之前没有配置过 settingName 会无法 reload 的问题
- 将 `/themes/{name}/reload-setting` 的 API 修改为 `/themes/{name}/reload`

#### Which issue(s) this PR fixes:

Fixes #2735

#### Special notes for your reviewer:
/cc @halo-dev/sig-halo 
#### Does this PR introduce a user-facing change?

```release-note
修复 theme.yaml 之前没有配置过 settingName 会无法 reload 的问题
```
pull/2753/head^2
guqing 2022-11-28 11:00:17 +08:00 committed by GitHub
parent 4f4f35c67f
commit de983a2e46
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 312 additions and 160 deletions

View File

@ -6,7 +6,6 @@ import static org.springdoc.core.fn.builders.parameter.Builder.parameterBuilder;
import static org.springdoc.core.fn.builders.requestbody.Builder.requestBodyBuilder;
import static org.springdoc.core.fn.builders.schema.Builder.schemaBuilder;
import static org.springframework.web.reactive.function.server.RequestPredicates.contentType;
import static run.halo.app.core.extension.theme.ThemeUtils.loadThemeManifest;
import static run.halo.app.infra.utils.DataBufferUtils.toInputStream;
import io.swagger.v3.oas.annotations.enums.ParameterIn;
@ -17,7 +16,6 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.springdoc.webflux.core.fn.SpringdocRouteBuilder;
import org.springframework.http.MediaType;
import org.springframework.http.codec.multipart.FilePart;
@ -32,12 +30,10 @@ import org.springframework.web.reactive.function.server.ServerResponse;
import org.springframework.web.server.ServerWebInputException;
import reactor.core.Exceptions;
import reactor.core.publisher.Mono;
import run.halo.app.core.extension.Setting;
import run.halo.app.core.extension.Theme;
import run.halo.app.core.extension.endpoint.CustomEndpoint;
import run.halo.app.extension.ListResult;
import run.halo.app.extension.ReactiveExtensionClient;
import run.halo.app.extension.Unstructured;
import run.halo.app.extension.router.IListRequest;
import run.halo.app.extension.router.QueryParamBuildUtil;
import run.halo.app.infra.ThemeRootGetter;
@ -92,8 +88,8 @@ public class ThemeEndpoint implements CustomEndpoint {
.content(contentBuilder().mediaType(MediaType.MULTIPART_FORM_DATA_VALUE)
.schema(schemaBuilder().implementation(UpgradeRequest.class))))
.build())
.PUT("themes/{name}/reload-setting", this::reloadSetting,
builder -> builder.operationId("ReloadThemeSetting")
.PUT("themes/{name}/reload", this::reloadTheme,
builder -> builder.operationId("Reload")
.description("Reload theme setting.")
.tag(tag)
.parameter(parameterBuilder()
@ -211,42 +207,9 @@ public class ThemeEndpoint implements CustomEndpoint {
);
}
// TODO Extract the method into ThemeService
Mono<ServerResponse> reloadSetting(ServerRequest request) {
Mono<ServerResponse> reloadTheme(ServerRequest request) {
String name = request.pathVariable("name");
return client.fetch(Theme.class, name)
.filter(theme -> StringUtils.isNotBlank(theme.getSpec().getSettingName()))
.flatMap(theme -> {
String settingName = theme.getSpec().getSettingName();
return ThemeUtils.loadThemeSetting(getThemePath(theme))
.stream()
.filter(unstructured ->
settingName.equals(unstructured.getMetadata().getName()))
.findFirst()
.map(setting -> Unstructured.OBJECT_MAPPER.convertValue(setting, Setting.class))
.map(setting -> client.fetch(Setting.class, settingName)
.flatMap(persistent -> {
// update spec to persisted setting
persistent.setSpec(setting.getSpec());
return client.update(persistent);
})
.switchIfEmpty(Mono.defer(() -> client.create(setting)))
.thenReturn(theme)
)
.orElse(Mono.just(theme));
})
.flatMap(themeToUse -> {
Path themePath = themeRoot.get().resolve(themeToUse.getMetadata().getName());
Path themeManifestPath = ThemeUtils.resolveThemeManifest(themePath);
if (themeManifestPath == null) {
return Mono.error(new IllegalArgumentException(
"The manifest file [theme.yaml] is required."));
}
Unstructured unstructured = loadThemeManifest(themeManifestPath);
Theme newTheme = Unstructured.OBJECT_MAPPER.convertValue(unstructured, Theme.class);
themeToUse.setSpec(newTheme.getSpec());
return client.update(themeToUse);
})
return themeService.reloadTheme(name)
.flatMap(theme -> ServerResponse.ok()
.contentType(MediaType.APPLICATION_JSON)
.bodyValue(theme));

View File

@ -10,6 +10,7 @@ public interface ThemeService {
Mono<Theme> upgrade(String themeName, InputStream is);
Mono<Theme> reloadTheme(String name);
// TODO Migrate other useful methods in ThemeEndpoint in the future.
}

View File

@ -2,6 +2,7 @@ package run.halo.app.core.extension.theme;
import static java.nio.file.Files.createTempDirectory;
import static org.springframework.util.FileSystemUtils.copyRecursively;
import static run.halo.app.core.extension.theme.ThemeUtils.loadThemeManifest;
import static run.halo.app.core.extension.theme.ThemeUtils.locateThemeManifest;
import static run.halo.app.infra.utils.FileUtils.deleteRecursivelyAndSilently;
import static run.halo.app.infra.utils.FileUtils.unzip;
@ -16,6 +17,7 @@ import java.util.function.Predicate;
import java.util.zip.ZipInputStream;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.springframework.retry.RetryException;
import org.springframework.stereotype.Service;
import org.springframework.util.Assert;
@ -32,6 +34,7 @@ import run.halo.app.extension.ConfigMap;
import run.halo.app.extension.ReactiveExtensionClient;
import run.halo.app.extension.Unstructured;
import run.halo.app.infra.ThemeRootGetter;
import run.halo.app.infra.exception.AsyncRequestTimeoutException;
import run.halo.app.infra.exception.ThemeInstallationException;
@Slf4j
@ -163,6 +166,60 @@ public class ThemeServiceImpl implements ThemeService {
});
}
@Override
public Mono<Theme> reloadTheme(String name) {
return client.fetch(Theme.class, name)
.flatMap(oldTheme -> {
String settingName = oldTheme.getSpec().getSettingName();
return waitForSettingDeleted(settingName)
.doOnError(error -> {
log.error("Failed to delete setting: {}", settingName,
ExceptionUtils.getRootCause(error));
throw new AsyncRequestTimeoutException("Reload theme timeout.");
});
})
.then(Mono.defer(() -> {
Path themePath = themeRoot.get().resolve(name);
Path themeManifestPath = ThemeUtils.resolveThemeManifest(themePath);
if (themeManifestPath == null) {
throw new IllegalArgumentException(
"The manifest file [theme.yaml] is required.");
}
Unstructured unstructured = loadThemeManifest(themeManifestPath);
Theme newTheme = Unstructured.OBJECT_MAPPER.convertValue(unstructured,
Theme.class);
return client.fetch(Theme.class, name)
.map(oldTheme -> {
newTheme.getMetadata().setVersion(oldTheme.getMetadata().getVersion());
return newTheme;
})
.flatMap(client::update);
}))
.flatMap(theme -> {
String settingName = theme.getSpec().getSettingName();
return Flux.fromIterable(ThemeUtils.loadThemeSetting(getThemePath(theme)))
.map(setting -> Unstructured.OBJECT_MAPPER.convertValue(setting, Setting.class))
.filter(setting -> setting.getMetadata().getName().equals(settingName))
.next()
.flatMap(client::create)
.thenReturn(theme);
});
}
private Mono<Void> waitForSettingDeleted(String settingName) {
return client.fetch(Setting.class, settingName)
.flatMap(setting -> client.delete(setting)
.flatMap(deleted -> client.fetch(Setting.class, settingName)
.doOnNext(latest -> {
throw new RetryException("Setting is not deleted yet.");
})
.retryWhen(Retry.fixedDelay(10, Duration.ofMillis(100))
.filter(t -> t instanceof RetryException))
)
)
.then();
}
private Path getThemePath(Theme theme) {
return themeRoot.get().resolve(theme.getMetadata().getName());
}

View File

@ -0,0 +1,49 @@
package run.halo.app.infra.exception;
import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ProblemDetail;
import org.springframework.lang.NonNull;
import org.springframework.web.ErrorResponse;
/**
* <p>Exception to be thrown when an async request times out.</p>
* By default the exception will be handled as a {@link HttpStatus#REQUEST_TIMEOUT} error.
*
* @author guqing
* @since 2.0.0
*/
public class AsyncRequestTimeoutException extends RuntimeException implements ErrorResponse {
public AsyncRequestTimeoutException() {
super();
}
public AsyncRequestTimeoutException(String message) {
super(message);
}
public AsyncRequestTimeoutException(String message, Throwable cause) {
super(message, cause);
}
public AsyncRequestTimeoutException(Throwable cause) {
super(cause);
}
protected AsyncRequestTimeoutException(String message, Throwable cause,
boolean enableSuppression, boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
}
@Override
@NonNull
public HttpStatusCode getStatusCode() {
return HttpStatus.REQUEST_TIMEOUT;
}
@Override
@NonNull
public ProblemDetail getBody() {
return ProblemDetail.forStatusAndDetail(getStatusCode(), getMessage());
}
}

View File

@ -15,7 +15,7 @@ rules:
resources: [ "themes" ]
verbs: [ "*" ]
- apiGroups: [ "api.console.halo.run" ]
resources: [ "themes", "themes/reload-setting" ]
resources: [ "themes", "themes/reload" ]
verbs: [ "*" ]
- nonResourceURLs: [ "/apis/api.console.halo.run/themes/install" ]
verbs: [ "create" ]

View File

@ -1,11 +1,9 @@
package run.halo.app.core.extension.theme;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
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 static org.springframework.web.reactive.function.BodyInserters.fromMultipartData;
@ -15,18 +13,14 @@ import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import org.json.JSONException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.skyscreamer.jsonassert.JSONAssert;
import org.springframework.core.io.FileSystemResource;
import org.springframework.http.MediaType;
import org.springframework.http.client.MultipartBodyBuilder;
@ -35,13 +29,9 @@ import org.springframework.util.FileSystemUtils;
import org.springframework.util.ResourceUtils;
import org.springframework.web.server.ServerWebInputException;
import reactor.core.publisher.Mono;
import run.halo.app.core.extension.Setting;
import run.halo.app.core.extension.Theme;
import run.halo.app.extension.AbstractExtension;
import run.halo.app.extension.Metadata;
import run.halo.app.extension.ReactiveExtensionClient;
import run.halo.app.infra.ThemeRootGetter;
import run.halo.app.infra.utils.JsonUtils;
/**
* Tests for {@link ThemeEndpoint}.
@ -52,9 +42,6 @@ import run.halo.app.infra.utils.JsonUtils;
@ExtendWith(MockitoExtension.class)
class ThemeEndpointTest {
@Mock
ReactiveExtensionClient extensionClient;
@Mock
ThemeRootGetter themeRoot;
@ -166,112 +153,11 @@ class ThemeEndpointTest {
}
@Test
void reloadSetting() throws IOException {
Theme theme = new Theme();
theme.setMetadata(new Metadata());
theme.getMetadata().setName("fake-theme");
theme.setSpec(new Theme.ThemeSpec());
theme.getSpec().setDisplayName("Hello");
theme.getSpec().setSettingName("fake-setting");
when(extensionClient.fetch(Theme.class, "fake-theme"))
.thenReturn(Mono.just(theme));
Setting setting = new Setting();
setting.setMetadata(new Metadata());
setting.setSpec(new Setting.SettingSpec());
setting.getSpec().setForms(List.of());
when(extensionClient.fetch(Setting.class, "fake-setting"))
.thenReturn(Mono.just(setting));
// when(haloProperties.getWorkDir()).thenReturn(tmpHaloWorkDir);
Path themeWorkDir = themeRoot.get().resolve(theme.getMetadata().getName());
if (!Files.exists(themeWorkDir)) {
Files.createDirectories(themeWorkDir);
}
Files.writeString(themeWorkDir.resolve("settings.yaml"), """
apiVersion: v1alpha1
kind: Setting
metadata:
name: fake-setting
spec:
forms:
- group: sns
label:
formSchema:
- $el: h1
children: Register
""");
Files.writeString(themeWorkDir.resolve("theme.yaml"), """
apiVersion: v1alpha1
kind: Theme
metadata:
name: fake-theme
spec:
displayName: Fake Theme
""");
when(extensionClient.update(any(Theme.class)))
.thenReturn(Mono.just(theme));
when(extensionClient.update(any(Setting.class)))
.thenReturn(Mono.just(setting));
ArgumentCaptor<AbstractExtension> captor = ArgumentCaptor.forClass(Setting.class);
void reloadTheme() {
when(themeService.reloadTheme(any())).thenReturn(Mono.empty());
webTestClient.put()
.uri("/themes/fake-theme/reload-setting")
.uri("/themes/fake/reload")
.exchange()
.expectStatus()
.isOk()
.expectBody(Setting.class)
.value(settingRes -> {
verify(extensionClient, times(2)).update(captor.capture());
verify(extensionClient, times(0)).create(any(Setting.class));
List<AbstractExtension> allValues = captor.getAllValues();
assertThat(allValues.get(0)).isInstanceOfAny(Setting.class);
Setting newSetting = (Setting) allValues.get(0);
Theme newTheme = (Theme) allValues.get(1);
try {
JSONAssert.assertEquals("""
{
"spec": {
"forms": [
{
"group": "sns",
"label": "社交资料",
"formSchema": [
{
"$el": "h1",
"children": "Register"
}
]
}
]
},
"apiVersion": "v1alpha1",
"kind": "Setting",
"metadata": {}
}
""",
JsonUtils.objectToJson(newSetting),
true);
JSONAssert.assertEquals("""
{
"spec": {
"displayName": "Fake Theme",
"version": "*",
"require": "*"
},
"apiVersion": "theme.halo.run/v1alpha1",
"kind": "Theme",
"metadata": {
"name": "fake-theme"
}
}
""",
JsonUtils.objectToJson(newTheme),
true);
} catch (JSONException e) {
throw new RuntimeException(e);
}
});
.expectStatus().isOk();
}
}

View File

@ -2,6 +2,8 @@ package run.halo.app.core.extension.theme;
import static java.nio.file.Files.createTempDirectory;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.times;
@ -14,7 +16,9 @@ import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.function.Consumer;
import org.json.JSONException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
@ -23,10 +27,13 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.stubbing.Answer;
import org.skyscreamer.jsonassert.JSONAssert;
import org.springframework.util.ResourceUtils;
import org.springframework.web.server.ServerWebInputException;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import run.halo.app.core.extension.Setting;
import run.halo.app.core.extension.Theme;
import run.halo.app.extension.Metadata;
import run.halo.app.extension.ReactiveExtensionClient;
@ -34,6 +41,7 @@ import run.halo.app.extension.Unstructured;
import run.halo.app.extension.exception.ExtensionException;
import run.halo.app.infra.ThemeRootGetter;
import run.halo.app.infra.exception.ThemeInstallationException;
import run.halo.app.infra.utils.JsonUtils;
@ExtendWith(MockitoExtension.class)
class ThemeServiceImplTest {
@ -179,4 +187,192 @@ class ThemeServiceImplTest {
}
}
@Test
void reloadThemeWhenSettingNameSetBeforeThenDeleteSetting() throws IOException {
Theme theme = new Theme();
theme.setMetadata(new Metadata());
theme.getMetadata().setName("fake-theme");
theme.setSpec(new Theme.ThemeSpec());
theme.getSpec().setDisplayName("Hello");
theme.getSpec().setSettingName("fake-setting");
when(client.fetch(Theme.class, "fake-theme"))
.thenReturn(Mono.just(theme));
when(client.delete(any(Setting.class))).thenReturn(Mono.empty());
Setting setting = new Setting();
setting.setMetadata(new Metadata());
setting.setSpec(new Setting.SettingSpec());
setting.getSpec().setForms(List.of());
when(client.fetch(Setting.class, "fake-setting"))
.thenReturn(Mono.just(setting));
Path themeWorkDir = themeRoot.get().resolve(theme.getMetadata().getName());
if (!Files.exists(themeWorkDir)) {
Files.createDirectories(themeWorkDir);
}
Files.writeString(themeWorkDir.resolve("settings.yaml"), """
apiVersion: v1alpha1
kind: Setting
metadata:
name: fake-setting
spec:
forms:
- group: sns
label:
formSchema:
- $el: h1
children: Register
""");
Files.writeString(themeWorkDir.resolve("theme.yaml"), """
apiVersion: v1alpha1
kind: Theme
metadata:
name: fake-theme
spec:
displayName: Fake Theme
""");
when(client.update(any(Theme.class)))
.thenAnswer((Answer<Mono<Theme>>) invocation -> {
Theme argument = invocation.getArgument(0);
return Mono.just(argument);
});
themeService.reloadTheme("fake-theme")
.as(StepVerifier::create)
.consumeNextWith(themeUpdated -> {
try {
JSONAssert.assertEquals("""
{
"spec": {
"displayName": "Fake Theme",
"version": "*",
"require": "*"
},
"apiVersion": "theme.halo.run/v1alpha1",
"kind": "Theme",
"metadata": {
"name": "fake-theme"
}
}
""",
JsonUtils.objectToJson(themeUpdated),
true);
} catch (JSONException e) {
throw new RuntimeException(e);
}
})
.verifyComplete();
// delete fake-setting
verify(client, times(1)).delete(any(Setting.class));
// Will not be created
verify(client, times(0)).create(any(Setting.class));
}
@Test
void reloadThemeWhenSettingNameNotSetBefore() throws IOException {
Theme theme = new Theme();
theme.setMetadata(new Metadata());
theme.getMetadata().setName("fake-theme");
theme.setSpec(new Theme.ThemeSpec());
theme.getSpec().setDisplayName("Hello");
when(client.fetch(Theme.class, "fake-theme"))
.thenReturn(Mono.just(theme));
Setting setting = new Setting();
setting.setMetadata(new Metadata());
setting.setSpec(new Setting.SettingSpec());
setting.getSpec().setForms(List.of());
when(client.fetch(eq(Setting.class), eq(null))).thenReturn(Mono.empty());
Path themeWorkDir = themeRoot.get().resolve(theme.getMetadata().getName());
if (!Files.exists(themeWorkDir)) {
Files.createDirectories(themeWorkDir);
}
Files.writeString(themeWorkDir.resolve("settings.yaml"), """
apiVersion: v1alpha1
kind: Setting
metadata:
name: fake-setting
spec:
forms:
- group: sns
label:
formSchema:
- $el: h1
children: Register
""");
Files.writeString(themeWorkDir.resolve("theme.yaml"), """
apiVersion: v1alpha1
kind: Theme
metadata:
name: fake-theme
spec:
displayName: Fake Theme
settingName: fake-setting
""");
when(client.update(any(Theme.class)))
.thenAnswer((Answer<Mono<Theme>>) invocation -> {
Theme argument = invocation.getArgument(0);
return Mono.just(argument);
});
when(client.create(any(Setting.class)))
.thenAnswer((Answer<Mono<Setting>>) invocation -> {
Setting argument = invocation.getArgument(0);
JSONAssert.assertEquals("""
{
"spec": {
"forms": [
{
"group": "sns",
"label": "社交资料",
"formSchema": [
{
"$el": "h1",
"children": "Register"
}
]
}
]
},
"apiVersion": "v1alpha1",
"kind": "Setting",
"metadata": {
"name": "fake-setting"
}
}
""",
JsonUtils.objectToJson(argument),
true);
return Mono.just(invocation.getArgument(0));
});
themeService.reloadTheme("fake-theme")
.as(StepVerifier::create)
.consumeNextWith(themeUpdated -> {
try {
JSONAssert.assertEquals("""
{
"spec": {
"settingName": "fake-setting",
"displayName": "Fake Theme",
"version": "*",
"require": "*"
},
"apiVersion": "theme.halo.run/v1alpha1",
"kind": "Theme",
"metadata": {
"name": "fake-theme"
}
}
""",
JsonUtils.objectToJson(themeUpdated),
true);
} catch (JSONException e) {
throw new RuntimeException(e);
}
})
.verifyComplete();
}
}