Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine exception detail with i18n #3042

Merged
merged 5 commits into from
Dec 26, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ public Mono<Comment> create(Comment comment) {
.flatMap(commentSetting -> {
if (Boolean.FALSE.equals(commentSetting.getEnable())) {
return Mono.error(
new AccessDeniedException("The comment function has been turned off."));
new AccessDeniedException("The comment function has been turned off.",
"problemDetail.comment.turnedOff", null));
}
if (checkCommentOwner(comment, commentSetting.getSystemUserOnly())) {
return Mono.error(
new AccessDeniedException("Allow system user comments only."));
new AccessDeniedException("Allow only system users to comment.",
"problemDetail.comment.systemUsersOnly", null));
}

if (comment.getSpec().getTop() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ public Mono<Reply> create(String commentName, Reply reply) {
.map(commentSetting -> {
if (Boolean.FALSE.equals(commentSetting.getEnable())) {
throw new AccessDeniedException(
"The comment function has been turned off.");
"The comment function has been turned off.",
"problemDetail.comment.turnedOff", null);
}
if (checkReplyOwner(reply, commentSetting.getSystemUserOnly())) {
throw new AccessDeniedException("Allow system user reply only.");
throw new AccessDeniedException("Allow only system users to comment.",
"problemDetail.comment.systemUsersOnly", null);
}
reply.getSpec().setApproved(
Boolean.FALSE.equals(commentSetting.getRequireReviewForNew()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand All @@ -26,6 +27,7 @@
import run.halo.app.core.extension.attachment.Constant;
import run.halo.app.core.extension.attachment.Policy;
import run.halo.app.extension.Metadata;
import run.halo.app.infra.exception.AttachmentAlreadyExistsException;
import run.halo.app.infra.properties.HaloProperties;
import run.halo.app.infra.utils.JsonUtils;

Expand Down Expand Up @@ -108,7 +110,9 @@ public Mono<Attachment> upload(UploadContext uploadOption) {
attachment.setMetadata(metadata);
attachment.setSpec(spec);
return attachment;
}));
}))
.onErrorMap(FileAlreadyExistsException.class,
e -> new AttachmentAlreadyExistsException(e.getFile()));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import run.halo.app.core.extension.User;
import run.halo.app.core.extension.service.UserService;
import run.halo.app.extension.ReactiveExtensionClient;
import run.halo.app.extension.exception.ExtensionNotFoundException;
import run.halo.app.infra.exception.UserNotFoundException;
import run.halo.app.infra.utils.JsonUtils;

@Component
Expand Down Expand Up @@ -115,7 +117,9 @@ Mono<ServerResponse> me(ServerRequest request) {
return ReactiveSecurityContextHolder.getContext()
.flatMap(ctx -> {
var name = ctx.getAuthentication().getName();
return client.get(User.class, name);
return client.get(User.class, name)
.onErrorMap(ExtensionNotFoundException.class,
e -> new UserNotFoundException(name));
})
.flatMap(user -> ServerResponse.ok()
.contentType(MediaType.APPLICATION_JSON)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
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.dao.OptimisticLockingFailureException;
import org.springframework.retry.RetryException;
import org.springframework.stereotype.Service;
Expand All @@ -36,8 +35,7 @@
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;
import run.halo.app.infra.exception.ThemeUpgradeException;

@Slf4j
@Service
Expand Down Expand Up @@ -77,9 +75,10 @@ public Mono<Theme> upgrade(String themeName, InputStream is) {
.flatMap(oldTheme -> {
try (var zis = new ZipInputStream(is)) {
unzip(zis, tempDir.get());
return locateThemeManifest(tempDir.get())
.switchIfEmpty(Mono.error(() -> new ThemeInstallationException(
"Missing theme manifest file: theme.yaml or theme.yml")));
return locateThemeManifest(tempDir.get()).switchIfEmpty(Mono.error(
() -> new ThemeUpgradeException(
"Missing theme manifest file: theme.yaml or theme.yml",
"problemDetail.theme.upgrade.missingManifest", null)));
} catch (IOException e) {
return Mono.error(e);
}
Expand All @@ -97,7 +96,9 @@ public Mono<Theme> upgrade(String themeName, InputStream is) {
log.error("Want theme name: {}, but provided: {}", themeName,
newTheme.getMetadata().getName());
}
throw new ServerWebInputException("please make sure the theme name is correct");
throw new ThemeUpgradeException("Please make sure the theme name is correct",
"problemDetail.theme.upgrade.nameMismatch",
new Object[] {newTheme.getMetadata().getName(), themeName});
}
})
.flatMap(newTheme -> {
Expand Down Expand Up @@ -173,12 +174,7 @@ 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.");
});
return waitForSettingDeleted(settingName);
})
.then(Mono.defer(() -> {
Path themePath = themeRoot.get().resolve(name);
Expand Down Expand Up @@ -235,9 +231,8 @@ 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.");
})
.flatMap(s -> Mono.error(
() -> new RetryException("Re-check if the setting is deleted.")))
.retryWhen(Retry.fixedDelay(10, Duration.ofMillis(100))
.filter(t -> t instanceof RetryException))
)
Expand Down Expand Up @@ -272,9 +267,9 @@ Mono<Void> waitForThemeDeleted(String themeName) {
throw new RetryException("Re-check if the theme is deleted successfully");
})
.retryWhen(Retry.fixedDelay(20, Duration.ofMillis(100))
.filter(t -> t instanceof RetryException))
.onErrorMap(Exceptions::isRetryExhausted,
throwable -> new ServerErrorException("Wait timeout for theme deleted", throwable))
.filter(t -> t instanceof RetryException)
.onRetryExhaustedThrow((spec, signal) ->
new ServerErrorException("Wait timeout for theme deleted", null)))
.then();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,16 @@ static Mono<Unstructured> unzipThemeTo(InputStream inputStream, Path themeWorkDi
})
.flatMap(is -> ThemeUtils.locateThemeManifest(tempDir.get()))
.switchIfEmpty(
Mono.error(() -> new ThemeInstallationException("Missing theme manifest")))
Mono.error(() -> new ThemeInstallationException("Missing theme manifest",
"problemDetail.theme.install.missingManifest", null)))
.map(themeManifestPath -> {
var theme = loadThemeManifest(themeManifestPath);
var themeName = theme.getMetadata().getName();
var themeTargetPath = themeWorkDir.resolve(themeName);
try {
if (!override && !FileUtils.isEmpty(themeTargetPath)) {
throw new ThemeInstallationException("Theme already exists.");
throw new ThemeInstallationException("Theme already exists.",
"problemDetail.theme.install.alreadyExists", new Object[] {themeName});
}
// install theme to theme work dir
copyRecursively(themeManifestPath.getParent(), themeTargetPath);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/run/halo/app/extension/GroupVersionKind.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static GroupVersionKind fromAPIVersionAndKind(String apiVersion, String k
return new GroupVersionKind(gv.group(), gv.version(), kind);
}

public static <T extends AbstractExtension> GroupVersionKind fromExtension(Class<T> extension) {
public static <T extends Extension> GroupVersionKind fromExtension(Class<T> extension) {
GVK gvk = extension.getAnnotation(GVK.class);
return new GroupVersionKind(gvk.group(), gvk.version(), gvk.kind());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public <E extends Extension> ExtensionStore convertTo(E extension) {
if (!validation.isValid()) {
log.debug("Failed to validate Extension: {}, and errors were: {}",
extension.getClass(), validation.results());
throw new SchemaViolationException("Failed to validate Extension "
+ extension.getClass(), validation.results());
throw new SchemaViolationException(extension.groupVersionKind(),
validation.results());
}

var version = extension.getMetadata().getVersion();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,15 @@ public Mono<Unstructured> fetch(GroupVersionKind gvk, String name) {
@Override
public <E extends Extension> Mono<E> get(Class<E> type, String name) {
return fetch(type, name)
.switchIfEmpty(Mono.error(() -> new ExtensionNotFoundException(
"Extension " + type.getName() + " with name " + name + " not found")));
.switchIfEmpty(Mono.error(() -> {
var gvk = GroupVersionKind.fromExtension(type);
return new ExtensionNotFoundException(gvk, name);
}));
}

private Mono<Unstructured> get(GroupVersionKind gvk, String name) {
return fetch(gvk, name)
.switchIfEmpty(Mono.error(() -> new ExtensionNotFoundException(
"Extension " + gvk + " with name " + name + " not found")));
.switchIfEmpty(Mono.error(() -> new ExtensionNotFoundException(gvk, name)));
}

@Override
Expand Down
7 changes: 2 additions & 5 deletions src/main/java/run/halo/app/extension/Scheme.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ public static Scheme buildFromType(Class<? extends Extension> type) {
@NonNull
public static GVK getGvkFromType(@NonNull Class<? extends Extension> type) {
var gvk = type.getAnnotation(GVK.class);
if (gvk == null) {
throw new ExtensionException(
String.format("Annotation %s needs to be on Extension %s", GVK.class.getName(),
type.getName()));
}
Assert.notNull(gvk,
"Missing annotation " + GVK.class.getName() + " on type " + type.getName());
return gvk;
}
}
2 changes: 1 addition & 1 deletion src/main/java/run/halo/app/extension/SchemeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ default Optional<Scheme> fetch(@NonNull GroupVersionKind gvk) {
@NonNull
default Scheme get(@NonNull GroupVersionKind gvk) {
return fetch(gvk).orElseThrow(
() -> new SchemeNotFoundException("Scheme was not found for " + gvk));
() -> new SchemeNotFoundException(gvk));
}

@NonNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,11 @@
*/
public class ExtensionConvertException extends ExtensionException {

public ExtensionConvertException() {
public ExtensionConvertException(String reason) {
super(reason);
}

public ExtensionConvertException(String message) {
super(message);
}

public ExtensionConvertException(String message, Throwable cause) {
super(message, cause);
}

public ExtensionConvertException(Throwable cause) {
super(cause);
}

public ExtensionConvertException(String message, Throwable cause, boolean enableSuppression,
boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
public ExtensionConvertException(String reason, Throwable cause) {
super(reason, cause);
}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,26 @@
package run.halo.app.extension.exception;

import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.web.server.ResponseStatusException;

/**
* ExtensionException is the superclass of those exceptions that can be thrown by Extension module.
*
* @author johnniang
*/
public class ExtensionException extends RuntimeException {

public ExtensionException() {
}
public class ExtensionException extends ResponseStatusException {

public ExtensionException(String message) {
super(message);
public ExtensionException(String reason) {
this(reason, null);
}

public ExtensionException(String message, Throwable cause) {
super(message, cause);
public ExtensionException(String reason, Throwable cause) {
this(HttpStatus.INTERNAL_SERVER_ERROR, reason, cause, null, new Object[] {reason});
}

public ExtensionException(Throwable cause) {
super(cause);
protected ExtensionException(HttpStatusCode status, String reason, Throwable cause,
String messageDetailCode, Object[] messageDetailArguments) {
super(status, reason, cause, messageDetailCode, messageDetailArguments);
}

public ExtensionException(String message, Throwable cause, boolean enableSuppression,
boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
}

}
Original file line number Diff line number Diff line change
@@ -1,24 +1,13 @@
package run.halo.app.extension.exception;

public class ExtensionNotFoundException extends ExtensionException {

public ExtensionNotFoundException() {
}

public ExtensionNotFoundException(String message) {
super(message);
}
import org.springframework.http.HttpStatus;
import run.halo.app.extension.GroupVersionKind;

public ExtensionNotFoundException(String message, Throwable cause) {
super(message, cause);
}
public class ExtensionNotFoundException extends ExtensionException {

public ExtensionNotFoundException(Throwable cause) {
super(cause);
public ExtensionNotFoundException(GroupVersionKind gvk, String name) {
super(HttpStatus.NOT_FOUND, "Extension " + gvk + "/" + name + " was not found.",
null, null, new Object[] {gvk, name});
}

public ExtensionNotFoundException(String message, Throwable cause, boolean enableSuppression,
boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package run.halo.app.extension.exception;

import org.openapi4j.core.validation.ValidationResults;
import org.springframework.http.HttpStatus;
import run.halo.app.extension.GroupVersionKind;

/**
* This exception is thrown when Schema is violation.
Expand All @@ -14,28 +16,9 @@ public class SchemaViolationException extends ExtensionException {
*/
private final ValidationResults errors;

public SchemaViolationException(ValidationResults errors) {
this.errors = errors;
}

public SchemaViolationException(String message, ValidationResults errors) {
super(message);
this.errors = errors;
}

public SchemaViolationException(String message, Throwable cause, ValidationResults errors) {
super(message, cause);
this.errors = errors;
}

public SchemaViolationException(Throwable cause, ValidationResults errors) {
super(cause);
this.errors = errors;
}

public SchemaViolationException(String message, Throwable cause, boolean enableSuppression,
boolean writableStackTrace, ValidationResults errors) {
super(message, cause, enableSuppression, writableStackTrace);
public SchemaViolationException(GroupVersionKind gvk, ValidationResults errors) {
super(HttpStatus.BAD_REQUEST, "Failed to validate " + gvk, null, null,
new Object[] {gvk, errors});
this.errors = errors;
}

Expand Down
Loading