From 5bcaf3d99b6b3303bfca343f9f8ef8c7b67dc367 Mon Sep 17 00:00:00 2001 From: Bondar Bogdan <36962546+bogdan-bondar@users.noreply.github.com> Date: Thu, 22 Apr 2021 08:19:45 +0200 Subject: [PATCH] UI error handling refactoring (#1106) * refactored HawkbitUIErrorHandler to delegate error details extraction to external extractor beans * refactored ui error handling, allowed ui error details extractors to return a list of error details * added license headers, restructured package structure * adapted javadocs * fixed sonar findings * fixed license header * added tests for HawkbitUIErrorHandler * refactored ConstraintViolationErrorExtractor, added test for extractors * changed UI tests feature to Management UI * fixed the parent/child error type resolution by ui error details extractor, added test Signed-off-by: Bogdan Bondar --- .../java/org/eclipse/hawkbit/app/MyUI.java | 7 +- .../eclipse/hawkbit/ui/AbstractHawkbitUI.java | 9 +- .../hawkbit/ui/MgmtUiConfiguration.java | 29 +++ .../ui/components/HawkbitUIErrorHandler.java | 146 ----------- .../hawkbit/ui/{ => error}/ErrorView.java | 2 +- .../ui/error/HawkbitUIErrorHandler.java | 151 ++++++++++++ .../hawkbit/ui/error/UiErrorDetails.java | 67 +++++ ...AbstractSingleUiErrorDetailsExtractor.java | 35 +++ .../ConstraintViolationErrorExtractor.java | 67 +++++ .../extractors/UiErrorDetailsExtractor.java | 51 ++++ .../extractors/UploadErrorExtractor.java | 27 ++ .../ui/error/HawkbitUIErrorHandlerTest.java | 232 ++++++++++++++++++ .../error/UiErrorDetailsExtractorsTest.java | 169 +++++++++++++ .../ui/utils/HawkbitCommonUtilTest.java | 2 +- 14 files changed, 840 insertions(+), 154 deletions(-) delete mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/components/HawkbitUIErrorHandler.java rename hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/{ => error}/ErrorView.java (98%) create mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandler.java create mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/UiErrorDetails.java create mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/AbstractSingleUiErrorDetailsExtractor.java create mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/ConstraintViolationErrorExtractor.java create mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UiErrorDetailsExtractor.java create mode 100644 hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UploadErrorExtractor.java create mode 100644 hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandlerTest.java create mode 100644 hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/UiErrorDetailsExtractorsTest.java diff --git a/hawkbit-runtime/hawkbit-update-server/src/main/java/org/eclipse/hawkbit/app/MyUI.java b/hawkbit-runtime/hawkbit-update-server/src/main/java/org/eclipse/hawkbit/app/MyUI.java index f73548e99..1a3cf138d 100644 --- a/hawkbit-runtime/hawkbit-update-server/src/main/java/org/eclipse/hawkbit/app/MyUI.java +++ b/hawkbit-runtime/hawkbit-update-server/src/main/java/org/eclipse/hawkbit/app/MyUI.java @@ -9,9 +9,9 @@ package org.eclipse.hawkbit.app; import org.eclipse.hawkbit.ui.AbstractHawkbitUI; -import org.eclipse.hawkbit.ui.ErrorView; import org.eclipse.hawkbit.ui.UiProperties; import org.eclipse.hawkbit.ui.components.NotificationUnreadButton; +import org.eclipse.hawkbit.ui.error.ErrorView; import org.eclipse.hawkbit.ui.menu.DashboardMenu; import org.eclipse.hawkbit.ui.push.EventPushStrategy; import org.eclipse.hawkbit.ui.push.UIEventProvider; @@ -21,6 +21,7 @@ import org.springframework.context.ApplicationContext; import org.vaadin.spring.events.EventBus.UIEventBus; import com.vaadin.annotations.Push; +import com.vaadin.server.ErrorHandler; import com.vaadin.shared.communication.PushMode; import com.vaadin.shared.ui.ui.Transport; import com.vaadin.spring.annotation.SpringUI; @@ -49,9 +50,9 @@ public class MyUI extends AbstractHawkbitUI { MyUI(final EventPushStrategy pushStrategy, final UIEventBus eventBus, final UIEventProvider eventProvider, final SpringViewProvider viewProvider, final ApplicationContext context, final DashboardMenu dashboardMenu, final ErrorView errorview, final NotificationUnreadButton notificationUnreadButton, - final UiProperties uiProperties, final VaadinMessageSource i18n) { + final UiProperties uiProperties, final VaadinMessageSource i18n, final ErrorHandler uiErrorHandler) { super(pushStrategy, eventBus, eventProvider, viewProvider, context, dashboardMenu, errorview, - notificationUnreadButton, uiProperties, i18n); + notificationUnreadButton, uiProperties, i18n, uiErrorHandler); } } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/AbstractHawkbitUI.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/AbstractHawkbitUI.java index 39f5efece..422b3bb89 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/AbstractHawkbitUI.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/AbstractHawkbitUI.java @@ -8,8 +8,8 @@ */ package org.eclipse.hawkbit.ui; -import org.eclipse.hawkbit.ui.components.HawkbitUIErrorHandler; import org.eclipse.hawkbit.ui.components.NotificationUnreadButton; +import org.eclipse.hawkbit.ui.error.ErrorView; import org.eclipse.hawkbit.ui.menu.DashboardEvent.PostViewChangeEvent; import org.eclipse.hawkbit.ui.menu.DashboardMenu; import org.eclipse.hawkbit.ui.menu.DashboardMenuItem; @@ -34,6 +34,7 @@ import com.vaadin.navigator.View; import com.vaadin.navigator.ViewChangeListener; import com.vaadin.navigator.ViewProvider; import com.vaadin.server.ClientConnector.DetachListener; +import com.vaadin.server.ErrorHandler; import com.vaadin.server.Responsive; import com.vaadin.server.VaadinRequest; import com.vaadin.spring.navigator.SpringViewProvider; @@ -73,6 +74,7 @@ public abstract class AbstractHawkbitUI extends UI implements DetachListener { private final SpringViewProvider viewProvider; private final transient ApplicationContext context; private final transient EventPushStrategy pushStrategy; + private final transient ErrorHandler uiErrorHandler; private final transient HawkbitEntityEventListener entityEventsListener; @@ -80,7 +82,7 @@ public abstract class AbstractHawkbitUI extends UI implements DetachListener { final UIEventProvider eventProvider, final SpringViewProvider viewProvider, final ApplicationContext context, final DashboardMenu dashboardMenu, final ErrorView errorview, final NotificationUnreadButton notificationUnreadButton, final UiProperties uiProperties, - final VaadinMessageSource i18n) { + final VaadinMessageSource i18n, final ErrorHandler uiErrorHandler) { this.pushStrategy = pushStrategy; this.viewProvider = viewProvider; this.context = context; @@ -89,6 +91,7 @@ public abstract class AbstractHawkbitUI extends UI implements DetachListener { this.notificationUnreadButton = notificationUnreadButton; this.uiProperties = uiProperties; this.i18n = i18n; + this.uiErrorHandler = uiErrorHandler; this.entityEventsListener = new HawkbitEntityEventListener(eventBus, eventProvider, notificationUnreadButton); } @@ -184,7 +187,7 @@ public abstract class AbstractHawkbitUI extends UI implements DetachListener { setNavigator(navigator); if (UI.getCurrent().getErrorHandler() == null) { - UI.getCurrent().setErrorHandler(new HawkbitUIErrorHandler()); + UI.getCurrent().setErrorHandler(uiErrorHandler); } LOG.debug("Current locale of the application is : {}", getLocale()); diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/MgmtUiConfiguration.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/MgmtUiConfiguration.java index ae28c99ad..13350a45e 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/MgmtUiConfiguration.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/MgmtUiConfiguration.java @@ -8,7 +8,13 @@ */ package org.eclipse.hawkbit.ui; +import java.util.List; + import org.eclipse.hawkbit.im.authentication.PermissionService; +import org.eclipse.hawkbit.ui.error.HawkbitUIErrorHandler; +import org.eclipse.hawkbit.ui.error.extractors.ConstraintViolationErrorExtractor; +import org.eclipse.hawkbit.ui.error.extractors.UiErrorDetailsExtractor; +import org.eclipse.hawkbit.ui.error.extractors.UploadErrorExtractor; import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -19,6 +25,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.PropertySource; import org.vaadin.spring.servlet.Vaadin4SpringServlet; +import com.vaadin.server.ErrorHandler; import com.vaadin.server.SystemMessagesProvider; import com.vaadin.server.VaadinServlet; @@ -76,6 +83,28 @@ public class MgmtUiConfiguration { return new LocalizedSystemMessagesProvider(uiProperties, i18n); } + /** + * UI Error handler bean. + * + * @return UI Error handler + */ + @Bean + @ConditionalOnMissingBean + ErrorHandler uiErrorHandler(final VaadinMessageSource i18n, + final List uiErrorDetailsExtractor) { + return new HawkbitUIErrorHandler(i18n, uiErrorDetailsExtractor); + } + + @Bean + UiErrorDetailsExtractor uploadErrorExtractor() { + return new UploadErrorExtractor(); + } + + @Bean + UiErrorDetailsExtractor constraintViolationErrorExtractor(final VaadinMessageSource i18n) { + return new ConstraintViolationErrorExtractor(i18n); + } + /** * Vaadin4Spring servlet bean. * diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/components/HawkbitUIErrorHandler.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/components/HawkbitUIErrorHandler.java deleted file mode 100644 index e7c41f996..000000000 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/components/HawkbitUIErrorHandler.java +++ /dev/null @@ -1,146 +0,0 @@ -/** - * Copyright (c) 2015 Bosch Software Innovations GmbH and others. - * - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Eclipse Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - */ -package org.eclipse.hawkbit.ui.components; - -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -import javax.validation.ConstraintViolation; -import javax.validation.ConstraintViolationException; - -import org.eclipse.hawkbit.ui.common.notification.ParallelNotification; -import org.eclipse.hawkbit.ui.utils.SPUIStyleDefinitions; -import org.eclipse.hawkbit.ui.utils.SpringContextHolder; -import org.eclipse.hawkbit.ui.utils.UINotification; -import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.util.StringUtils; - -import com.vaadin.icons.VaadinIcons; -import com.vaadin.server.ClientConnector.ConnectorErrorEvent; -import com.vaadin.server.DefaultErrorHandler; -import com.vaadin.server.ErrorEvent; -import com.vaadin.server.Page; -import com.vaadin.server.UploadException; -import com.vaadin.shared.Connector; -import com.vaadin.ui.Component; -import com.vaadin.ui.Notification; -import com.vaadin.ui.UI; - -/** - * Default handler for Hawkbit UI. - */ -public class HawkbitUIErrorHandler extends DefaultErrorHandler { - - private static final long serialVersionUID = 1L; - private static final Logger LOG = LoggerFactory.getLogger(HawkbitUIErrorHandler.class); - - @Override - public void error(final ErrorEvent event) { - - // filter upload exceptions - if (event.getThrowable() instanceof UploadException) { - return; - } - - final Notification notification = buildNotification(getRootExceptionFrom(event)); - if (event instanceof ConnectorErrorEvent) { - final Connector connector = ((ConnectorErrorEvent) event).getConnector(); - if (connector instanceof UI) { - final UI uiInstance = (UI) connector; - uiInstance.access(() -> notification.show(uiInstance.getPage())); - return; - } - } - - final Optional originError = getPageOriginError(event); - if (originError.isPresent()) { - notification.show(originError.get()); - return; - } - - notification.show(Page.getCurrent()); - } - - private static Throwable getRootExceptionFrom(final ErrorEvent event) { - return getRootCauseOf(event.getThrowable()); - } - - private static Throwable getRootCauseOf(final Throwable ex) { - - if (ex.getCause() != null) { - return getRootCauseOf(ex.getCause()); - } - - return ex; - } - - private static Optional getPageOriginError(final ErrorEvent event) { - - final Component errorOrigin = findAbstractComponent(event); - - if (errorOrigin != null && errorOrigin.getUI() != null) { - return Optional.ofNullable(errorOrigin.getUI().getPage()); - } - - return Optional.empty(); - } - - /** - * Method to build a notification based on an exception. - * - * @param ex - * the throwable - * @return a hawkbit error notification message - */ - protected ParallelNotification buildNotification(final Throwable ex) { - - LOG.error("Error in UI: ", ex); - - final String errorMessage = extractMessageFrom(ex); - final VaadinMessageSource i18n = SpringContextHolder.getInstance().getBean(VaadinMessageSource.class); - - return buildErrorNotification(i18n.getMessage("caption.error"), errorMessage); - } - - /** - * Method to build a error notification based on caption and description. - * - * @param caption - * Caption - * @param description - * Description - * @return a hawkbit error notification message - */ - protected static ParallelNotification buildErrorNotification(final String caption, final String description) { - return UINotification.buildNotification(SPUIStyleDefinitions.SP_NOTIFICATION_ERROR_MESSAGE_STYLE, caption, - description, VaadinIcons.EXCLAMATION_CIRCLE, true); - } - - private static String extractMessageFrom(final Throwable ex) { - - if (!(ex instanceof ConstraintViolationException)) { - if (!StringUtils.isEmpty(ex.getMessage())) { - return ex.getMessage(); - } - return ex.getClass().getSimpleName(); - } - - final Set> violations = ((ConstraintViolationException) ex).getConstraintViolations(); - - if (violations == null) { - return ex.getClass().getSimpleName(); - } else { - return violations.stream().map(violation -> violation.getPropertyPath() + " " + violation.getMessage()) - .collect(Collectors.joining(System.lineSeparator())); - } - } -} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/ErrorView.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/ErrorView.java similarity index 98% rename from hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/ErrorView.java rename to hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/ErrorView.java index bdd3cae62..ba38d9292 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/ErrorView.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/ErrorView.java @@ -6,7 +6,7 @@ * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html */ -package org.eclipse.hawkbit.ui; +package org.eclipse.hawkbit.ui.error; import org.eclipse.hawkbit.ui.menu.DashboardMenu; import org.eclipse.hawkbit.ui.menu.DashboardMenuItem; diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandler.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandler.java new file mode 100644 index 000000000..78495fbe0 --- /dev/null +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandler.java @@ -0,0 +1,151 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error; + +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.eclipse.hawkbit.ui.common.notification.ParallelNotification; +import org.eclipse.hawkbit.ui.error.extractors.UiErrorDetailsExtractor; +import org.eclipse.hawkbit.ui.utils.SPUIStyleDefinitions; +import org.eclipse.hawkbit.ui.utils.UINotification; +import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.vaadin.icons.VaadinIcons; +import com.vaadin.server.ClientConnector.ConnectorErrorEvent; +import com.vaadin.server.DefaultErrorHandler; +import com.vaadin.server.ErrorEvent; +import com.vaadin.server.ErrorHandler; +import com.vaadin.server.Page; +import com.vaadin.shared.Connector; +import com.vaadin.ui.Component; +import com.vaadin.ui.Notification; +import com.vaadin.ui.UI; + +/** + * Default handler for Hawkbit UI. + */ +public class HawkbitUIErrorHandler implements ErrorHandler { + private static final long serialVersionUID = 1L; + private static final Logger LOG = LoggerFactory.getLogger(HawkbitUIErrorHandler.class); + + private final VaadinMessageSource i18n; + private final transient List uiErrorDetailsExtractors; + + /** + * Constructor for HawkbitUIErrorHandler. + * + * @param i18n + * Message source used for localization + * @param uiErrorDetailsExtractors + * ui error details extractors + */ + public HawkbitUIErrorHandler(final VaadinMessageSource i18n, + final List uiErrorDetailsExtractors) { + this.i18n = i18n; + this.uiErrorDetailsExtractors = uiErrorDetailsExtractors; + } + + @Override + public void error(final ErrorEvent event) { + final Page currentPage = getPageFrom(event); + final List errorDetails = extractErrorDetails(event); + + if (errorDetails.isEmpty()) { + showGenericErrorNotification(currentPage, event); + return; + } + + errorDetails.stream().filter(UiErrorDetails::isPresent) + .forEach(details -> showSpecificErrorNotification(currentPage, details)); + } + + /** + * Method to find the {@link Page} to show notification on. + * + * @param event + * error event + * @return current {@link Page} for error notification + */ + protected Page getPageFrom(final ErrorEvent event) { + if (event instanceof ConnectorErrorEvent) { + final Connector connector = ((ConnectorErrorEvent) event).getConnector(); + if (connector instanceof UI) { + final UI uiInstance = (UI) connector; + return uiInstance.getPage(); + } + } + + final Optional errorOriginPage = getErrorOriginPage(event); + if (errorOriginPage.isPresent()) { + return errorOriginPage.get(); + } + + return Page.getCurrent(); + } + + private static Optional getErrorOriginPage(final ErrorEvent event) { + final Component errorOrigin = DefaultErrorHandler.findAbstractComponent(event); + + if (errorOrigin != null && errorOrigin.getUI() != null) { + return Optional.ofNullable(errorOrigin.getUI().getPage()); + } + + return Optional.empty(); + } + + private List extractErrorDetails(final ErrorEvent event) { + return uiErrorDetailsExtractors.stream() + .map(extractor -> extractor.extractErrorDetailsFrom(event.getThrowable())).flatMap(Collection::stream) + .collect(Collectors.toList()); + } + + private void showGenericErrorNotification(final Page page, final ErrorEvent event) { + LOG.error("Unexpected Ui error occured", event.getThrowable()); + + final Notification notification = buildErrorNotification(i18n.getMessage("caption.error"), + i18n.getMessage("message.error")); + showErrorNotification(page, notification); + } + + private void showSpecificErrorNotification(final Page page, final UiErrorDetails details) { + final Notification notification = buildErrorNotification(details.getCaption(), details.getDescription()); + showErrorNotification(page, notification); + } + + /** + * Method to build an error notification based on caption and description. + * + * @param caption + * notification caption + * @param description + * notification description + * @return a hawkbit error notification message + */ + protected static ParallelNotification buildErrorNotification(final String caption, final String description) { + return UINotification.buildNotification(SPUIStyleDefinitions.SP_NOTIFICATION_ERROR_MESSAGE_STYLE, caption, + description, VaadinIcons.EXCLAMATION_CIRCLE, true); + } + + /** + * Method to show notification on the given page. + * + * @param page + * page to show notification on + * @param notification + * notification to show + */ + protected void showErrorNotification(final Page page, final Notification notification) { + page.getUI().access(() -> notification.show(page)); + } +} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/UiErrorDetails.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/UiErrorDetails.java new file mode 100644 index 000000000..1f381fbc7 --- /dev/null +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/UiErrorDetails.java @@ -0,0 +1,67 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error; + +/** + * Details of UI errors for building the error notification. + */ +public final class UiErrorDetails { + private static final UiErrorDetails EMPTY = new UiErrorDetails(); + + private final String caption; + private final String description; + + private UiErrorDetails() { + this(null, null); + } + + private UiErrorDetails(final String caption, final String description) { + this.caption = caption; + this.description = description; + } + + public String getCaption() { + return caption; + } + + public String getDescription() { + return description; + } + + /** + * Checks if error details are not empty. + * + * @return if error details are populated + */ + public boolean isPresent() { + return caption != null && description != null; + } + + /** + * Creates empty error details that should be ignored by error handler. + * + * @return empty error details + */ + public static UiErrorDetails empty() { + return EMPTY; + } + + /** + * Creates error details that should be processed by error handler. + * + * @param caption + * error details caption + * @param description + * error details description + * @return error details + */ + public static UiErrorDetails create(final String caption, final String description) { + return new UiErrorDetails(caption, description); + } +} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/AbstractSingleUiErrorDetailsExtractor.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/AbstractSingleUiErrorDetailsExtractor.java new file mode 100644 index 000000000..8a418a934 --- /dev/null +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/AbstractSingleUiErrorDetailsExtractor.java @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error.extractors; + +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +import org.eclipse.hawkbit.ui.error.UiErrorDetails; + +/** + * Base class for single UI error details extractors. + */ +public abstract class AbstractSingleUiErrorDetailsExtractor implements UiErrorDetailsExtractor { + + @Override + public List extractErrorDetailsFrom(final Throwable error) { + return findDetails(error).map(Collections::singletonList).orElseGet(Collections::emptyList); + } + + /** + * Extracts single ui error details from given error. + * + * @param error + * error to extract details from + * @return ui error details if found + */ + protected abstract Optional findDetails(Throwable error); +} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/ConstraintViolationErrorExtractor.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/ConstraintViolationErrorExtractor.java new file mode 100644 index 000000000..00fd4ac60 --- /dev/null +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/ConstraintViolationErrorExtractor.java @@ -0,0 +1,67 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error.extractors; + +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import javax.validation.ConstraintViolation; +import javax.validation.ConstraintViolationException; + +import org.apache.commons.lang3.StringUtils; +import org.eclipse.hawkbit.ui.error.UiErrorDetails; +import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; +import org.springframework.util.CollectionUtils; + +/** + * UI error details extractor for {@link ConstraintViolationException}. + */ +public class ConstraintViolationErrorExtractor extends AbstractSingleUiErrorDetailsExtractor { + private final VaadinMessageSource i18n; + + /** + * Constructor for ConstraintViolationErrorExtractor. + * + * @param i18n + * Message source used for localization + */ + public ConstraintViolationErrorExtractor(final VaadinMessageSource i18n) { + this.i18n = i18n; + } + + @Override + protected Optional findDetails(final Throwable error) { + return findExceptionOf(error, ConstraintViolationException.class).map(ex -> { + final StringBuilder descriptionBuilder = new StringBuilder(getBasicDescription(ex, error)); + getViolationsDescription(ex).ifPresent(violationsDescription -> descriptionBuilder.append(":") + .append(System.lineSeparator()).append(violationsDescription)); + + return UiErrorDetails.create(i18n.getMessage("caption.error"), descriptionBuilder.toString()); + }); + } + + private static String getBasicDescription(final ConstraintViolationException ex, final Throwable error) { + return StringUtils.isEmpty(ex.getMessage()) ? error.getClass().getSimpleName() : ex.getMessage(); + } + + private static Optional getViolationsDescription(final ConstraintViolationException ex) { + final Set> violations = ex.getConstraintViolations(); + if (!CollectionUtils.isEmpty(violations)) { + return Optional.of(formatViolations(violations)); + } + + return Optional.empty(); + } + + private static String formatViolations(final Set> violations) { + return violations.stream().map(violation -> violation.getPropertyPath() + " " + violation.getMessage()) + .collect(Collectors.joining(System.lineSeparator())); + } +} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UiErrorDetailsExtractor.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UiErrorDetailsExtractor.java new file mode 100644 index 000000000..e9edbae56 --- /dev/null +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UiErrorDetailsExtractor.java @@ -0,0 +1,51 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error.extractors; + +import java.util.List; +import java.util.Optional; + +import org.eclipse.hawkbit.ui.error.UiErrorDetails; + +/** + * Base interface for extracting ui error details from given error. + */ +@FunctionalInterface +public interface UiErrorDetailsExtractor { + + /** + * Extracts ui error details from given error. + * + * @param error + * error to extract details from + * @return ui error details + */ + List extractErrorDetailsFrom(final Throwable error); + + /** + * Tries to find out if error matches the given exception type. + * + * @param error + * error to match + * @param exceptionType + * the type of exception to match + * @return casted error if matched + */ + default Optional findExceptionOf(final Throwable error, final Class exceptionType) { + if (exceptionType.isAssignableFrom(error.getClass())) { + return Optional.of((T) error); + } + + if (error.getCause() != null) { + return findExceptionOf(error.getCause(), exceptionType); + } + + return Optional.empty(); + } +} diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UploadErrorExtractor.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UploadErrorExtractor.java new file mode 100644 index 000000000..284dd50ad --- /dev/null +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/error/extractors/UploadErrorExtractor.java @@ -0,0 +1,27 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error.extractors; + +import java.util.Optional; + +import org.eclipse.hawkbit.ui.error.UiErrorDetails; + +import com.vaadin.server.UploadException; + +/** + * UI error details extractor for {@link UploadException}. + */ +public class UploadErrorExtractor extends AbstractSingleUiErrorDetailsExtractor { + + @Override + protected Optional findDetails(final Throwable error) { + // UploadException is ignored + return findExceptionOf(error, UploadException.class).map(ex -> UiErrorDetails.empty()); + } +} diff --git a/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandlerTest.java b/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandlerTest.java new file mode 100644 index 000000000..856bd4ed5 --- /dev/null +++ b/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/HawkbitUIErrorHandlerTest.java @@ -0,0 +1,232 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.eclipse.hawkbit.ui.error.extractors.UiErrorDetailsExtractor; +import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import com.vaadin.server.ErrorEvent; +import com.vaadin.server.Page; +import com.vaadin.ui.Notification; +import com.vaadin.ui.UI; + +import io.qameta.allure.Description; +import io.qameta.allure.Feature; +import io.qameta.allure.Step; +import io.qameta.allure.Story; + +@Feature("Unit Tests - Management UI") +@Story("Test the UI error handling with different error extractors") +@ExtendWith(MockitoExtension.class) +class HawkbitUIErrorHandlerTest { + + private final static String DEFAULT_CAPTION_CODE = "caption.error"; + private final static String DEFAULT_CAPTION = "defaultCaption"; + private final static String DEFAULT_DESCRIPTION_CODE = "message.error"; + private final static String DEFAULT_DESCRIPTION = "defaultDescription"; + private final static String MATCHING_CAPTION = "matchingCaption"; + private final static String MATCHING_DESCRIPTION = "matchingDescription"; + private final static String UNMATCHING_CAPTION = "unmatchingCaption"; + private final static String UNMATCHING_DESCRIPTION = "unmatchingDescription"; + + @Mock + private VaadinMessageSource i18n; + + @Mock + private UI ui; + + @Mock + private Page page; + + private HawkbitUIErrorHandler errorHandler; + + @Test + @Description("Generic error notification is shown in case no error details extractors are provided") + void showsGenericErrorNotificationOnMissingExtractors() { + initUi(); + initI18n(); + + handleErrorWithExtractors(new UnmatchingException(), Collections.emptyList()); + + verifyDefaultI18nCalled(); + + final Notification shownNotification = captureShownNotification().getValue(); + verifyNotificationContent(shownNotification, DEFAULT_CAPTION, DEFAULT_DESCRIPTION); + } + + @Step + private void handleErrorWithExtractors(final Throwable error, final List extractors) { + errorHandler = Mockito.spy(new HawkbitUIErrorHandler(i18n, extractors)); + errorHandler.error(new ErrorEvent(error)); + } + + @Step + private void verifyDefaultI18nCalled() { + verify(i18n).getMessage(eq(DEFAULT_CAPTION_CODE)); + verify(i18n).getMessage(eq(DEFAULT_DESCRIPTION_CODE)); + } + + @Step + private ArgumentCaptor captureShownNotification() { + return captureShownNotifications(1); + } + + @Step + private ArgumentCaptor captureShownNotifications(final int count) { + final ArgumentCaptor notificationCaptor = ArgumentCaptor.forClass(Notification.class); + verify(errorHandler, times(count)).showErrorNotification(eq(page), notificationCaptor.capture()); + + return notificationCaptor; + } + + @Step + private void verifyNotificationContent(final Notification notification, final String caption, + final String description) { + assertThat(notification.getCaption()).isEqualTo(caption); + assertThat(notification.getDescription()).isEqualTo(description); + } + + @Test + @Description("Generic error notification is shown in case error doesn't match any provided error details extractor") + void showsGenericErrorNotificationOnNonMatchingExtractor() { + initUi(); + initI18n(); + + final UiErrorDetailsExtractor matchingExtractor = getMatchingErrorDetailsExtractor(); + handleErrorWithExtractors(new UnmatchingException(), Collections.singletonList(matchingExtractor)); + + verifyDefaultI18nCalled(); + + final Notification shownNotification = captureShownNotification().getValue(); + verifyNotificationContent(shownNotification, DEFAULT_CAPTION, DEFAULT_DESCRIPTION); + } + + @Test + @Description("Nothing is shown in case error details extractor ignored the error") + void showsNothingOnIgnoreErrorDetailsExtractor() { + final UiErrorDetailsExtractor ignoreMatchingExtractor = getMatchingIgnoreErrorDetailsExtractor(); + handleErrorWithExtractors(new MatchingException(), Collections.singletonList(ignoreMatchingExtractor)); + + verify(errorHandler, times(0)).showErrorNotification(eq(page), any()); + } + + @Test + @Description("Multiple error notifications are shown in case a list of error details is provided by " + + "specific single error details extractor") + void showsMultipleErrorNotificationsOnSingleMatchingExtractor() { + initUi(); + + final UiErrorDetailsExtractor matchingMultipleExtractor = getMatchingErrorDetailsExtractor( + UiErrorDetails.create("matchingCaption1", "matchingDescription1"), + UiErrorDetails.create("matchingCaption2", "matchingDescription2")); + + handleErrorWithExtractors(new MatchingException(), Collections.singletonList(matchingMultipleExtractor)); + + final List shownNotifications = captureShownNotifications(2).getAllValues(); + verifyNotificationContent(shownNotifications.get(0), "matchingCaption1", "matchingDescription1"); + verifyNotificationContent(shownNotifications.get(1), "matchingCaption2", "matchingDescription2"); + } + + @Test + @Description("Specific error notification is shown only in case of multiple error details extractors") + void showsSingleErrorNotificationOnMatchingErrorDetailsOnly() { + initUi(); + + final UiErrorDetailsExtractor matchingExtractor = getMatchingErrorDetailsExtractor(); + final UiErrorDetailsExtractor unmatchingExtractor = getUnmatchingErrorDetailsExtractor(); + handleErrorWithExtractors(new MatchingException(), Arrays.asList(matchingExtractor, unmatchingExtractor)); + + final Notification shownNotification = captureShownNotification().getValue(); + verifyNotificationContent(shownNotification, MATCHING_CAPTION, MATCHING_DESCRIPTION); + } + + @Test + @Description("Specific error notification is shown only in case of multiple matching error details " + + "extractors if one of the extractors ignores the error") + void showsSingleErrorNotificationOnOneExtractorIgnore() { + initUi(); + + final UiErrorDetailsExtractor matchingExtractor = getMatchingErrorDetailsExtractor(); + final UiErrorDetailsExtractor ignoreMatchingExtractor = getMatchingIgnoreErrorDetailsExtractor(); + handleErrorWithExtractors(new MatchingException(), Arrays.asList(matchingExtractor, ignoreMatchingExtractor)); + + final Notification shownNotification = captureShownNotification().getValue(); + verifyNotificationContent(shownNotification, MATCHING_CAPTION, MATCHING_DESCRIPTION); + } + + @Test + @Description("Multiple error notifications are shown in case of multiple matching error details extractors") + void showsMultipleErrorNotificationsOnMatchingExtractors() { + initUi(); + + final UiErrorDetailsExtractor matchingExtractor = getMatchingErrorDetailsExtractor(); + final UiErrorDetailsExtractor anotherMatchingExtractor = getMatchingErrorDetailsExtractor( + UiErrorDetails.create("anotherMatchingCaption", "anotherMatchingDescription")); + handleErrorWithExtractors(new MatchingException(), Arrays.asList(matchingExtractor, anotherMatchingExtractor)); + + final List shownNotifications = captureShownNotifications(2).getAllValues(); + verifyNotificationContent(shownNotifications.get(0), MATCHING_CAPTION, MATCHING_DESCRIPTION); + verifyNotificationContent(shownNotifications.get(1), "anotherMatchingCaption", "anotherMatchingDescription"); + } + + private void initUi() { + when(ui.getPage()).thenReturn(page); + when(page.getUI()).thenReturn(ui); + UI.setCurrent(ui); + } + + private void initI18n() { + when(i18n.getMessage(eq(DEFAULT_CAPTION_CODE))).thenReturn(DEFAULT_CAPTION); + when(i18n.getMessage(eq(DEFAULT_DESCRIPTION_CODE))).thenReturn(DEFAULT_DESCRIPTION); + } + + private UiErrorDetailsExtractor getMatchingErrorDetailsExtractor() { + return getMatchingErrorDetailsExtractor(UiErrorDetails.create(MATCHING_CAPTION, MATCHING_DESCRIPTION)); + } + + private UiErrorDetailsExtractor getMatchingErrorDetailsExtractor(final UiErrorDetails... errorDetails) { + return (error) -> error instanceof MatchingException ? Arrays.asList(errorDetails) : Collections.emptyList(); + } + + private UiErrorDetailsExtractor getMatchingIgnoreErrorDetailsExtractor() { + return (error) -> error instanceof MatchingException ? Collections.singletonList(UiErrorDetails.empty()) + : Collections.emptyList(); + } + + private UiErrorDetailsExtractor getUnmatchingErrorDetailsExtractor() { + return (error) -> error instanceof UnmatchingException + ? Collections.singletonList(UiErrorDetails.create(UNMATCHING_CAPTION, UNMATCHING_DESCRIPTION)) + : Collections.emptyList(); + } + + private static class MatchingException extends Exception { + private static final long serialVersionUID = 1L; + } + + private static class UnmatchingException extends Exception { + private static final long serialVersionUID = 1L; + } +} diff --git a/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/UiErrorDetailsExtractorsTest.java b/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/UiErrorDetailsExtractorsTest.java new file mode 100644 index 000000000..d1e921301 --- /dev/null +++ b/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/error/UiErrorDetailsExtractorsTest.java @@ -0,0 +1,169 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.ui.error; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import javax.validation.ConstraintViolation; +import javax.validation.ConstraintViolationException; + +import org.eclipse.hawkbit.ui.error.extractors.ConstraintViolationErrorExtractor; +import org.eclipse.hawkbit.ui.error.extractors.UiErrorDetailsExtractor; +import org.eclipse.hawkbit.ui.error.extractors.UploadErrorExtractor; +import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import com.google.gwt.validation.client.impl.ConstraintViolationImpl; +import com.google.gwt.validation.client.impl.PathImpl; +import com.vaadin.server.UploadException; + +import io.qameta.allure.Description; +import io.qameta.allure.Feature; +import io.qameta.allure.Story; + +@Feature("Unit Tests - Management UI") +@Story("Test correct behaviour of the UI error details extractors") +@ExtendWith(MockitoExtension.class) +class UiErrorDetailsExtractorsTest { + + private final static String DEFAULT_CAPTION = "defaultCaption"; + private final static String VIOLATION_DESCRIPTION = "violationDescription"; + + @Mock + private VaadinMessageSource i18n; + + @Test + @Description("Extractor finds the matching exception in case of a nested error") + void nestedExceptionIsFoundByExtractor() { + final UiErrorDetailsExtractor extractor = (error) -> Collections.emptyList(); + final MatchingException error = new MatchingException( + new MatchingSecondLevelException(new MatchingThirdLevelException())); + + final Optional matchingError = extractor.findExceptionOf(error, MatchingException.class); + final Optional parentError = extractor.findExceptionOf(error, + MatchingSecondLevelException.class); + final Optional grandparentError = extractor.findExceptionOf(error, + MatchingThirdLevelException.class); + + assertThat(matchingError).isPresent().hasValue(error); + assertThat(parentError).isPresent().hasValue((MatchingSecondLevelException) error.getCause()); + assertThat(grandparentError).isPresent().hasValue((MatchingThirdLevelException) error.getCause().getCause()); + } + + @Test + @Description("Extractor finds the matching exception in case of parent/child relationships between error classes") + void parentExceptionIsFoundByExtractor() { + final UiErrorDetailsExtractor extractor = (error) -> Collections.emptyList(); + final MatchingException error = new MatchingException(); + + final Optional matchingError = extractor.findExceptionOf(error, + ParentMatchingException.class); + + assertThat(matchingError).isPresent().hasValue(error); + } + + @Test + @Description("Extractor finds details of constraint violation exception") + void extractsConstraintViolationException() { + when(i18n.getMessage("caption.error")).thenReturn(DEFAULT_CAPTION); + + final UiErrorDetailsExtractor extractor = new ConstraintViolationErrorExtractor(i18n); + final MatchingException emptyMessageError = new MatchingException(new ConstraintViolationException(null)); + final MatchingException emptyViolationsError = new MatchingException( + new ConstraintViolationException(VIOLATION_DESCRIPTION, null)); + final ConstraintViolationException violationsError = new ConstraintViolationException(VIOLATION_DESCRIPTION, + buildConstraintViolations()); + + final List emptyMessageErrorDetails = extractor.extractErrorDetailsFrom(emptyMessageError); + final List emptyViolationsErrorDetails = extractor + .extractErrorDetailsFrom(emptyViolationsError); + final List violationsErrorDetails = extractor.extractErrorDetailsFrom(violationsError); + + assertThat(emptyMessageErrorDetails).hasSize(1).first().satisfies(details -> { + assertThat(details.getCaption()).isEqualTo(DEFAULT_CAPTION); + assertThat(details.getDescription()).isEqualTo("MatchingException"); + }); + assertThat(emptyViolationsErrorDetails).hasSize(1).first().satisfies(details -> { + assertThat(details.getCaption()).isEqualTo(DEFAULT_CAPTION); + assertThat(details.getDescription()).isEqualTo(VIOLATION_DESCRIPTION); + }); + assertThat(violationsErrorDetails).hasSize(1).first().satisfies(details -> { + assertThat(details.getCaption()).isEqualTo(DEFAULT_CAPTION); + assertThat(details.getDescription()).contains(VIOLATION_DESCRIPTION).contains("firstProperty invalid") + .contains("secondProperty invalid"); + }); + } + + @Test + @Description("Extractor ignores details of upload exception (errors are handled explicitely)") + void ignoresUploadException() { + final UiErrorDetailsExtractor extractor = new UploadErrorExtractor(); + final UploadException uploadError = new UploadException("ignored"); + + final List errorDetails = extractor.extractErrorDetailsFrom(uploadError); + + assertThat(errorDetails).hasSize(1).first().isEqualTo(UiErrorDetails.empty()); + } + + private Set> buildConstraintViolations() { + final ConstraintViolationImpl violation1 = ConstraintViolationImpl.builder() + .setPropertyPath(new PathImpl().append("firstProperty")).setMessage("invalid").build(); + final ConstraintViolationImpl violation2 = ConstraintViolationImpl.builder() + .setPropertyPath(new PathImpl().append("secondProperty")).setMessage("invalid").build(); + + return new HashSet<>(Arrays.asList(violation1, violation2)); + } + + private static class ParentMatchingException extends Exception { + private static final long serialVersionUID = 1L; + + public ParentMatchingException() { + super(); + } + + public ParentMatchingException(final Throwable cause) { + super(cause); + } + } + + private static class MatchingException extends ParentMatchingException { + private static final long serialVersionUID = 1L; + + public MatchingException() { + super(); + } + + public MatchingException(final Throwable cause) { + super(cause); + } + } + + private static class MatchingSecondLevelException extends Exception { + private static final long serialVersionUID = 1L; + + public MatchingSecondLevelException(final Throwable cause) { + super(cause); + } + } + + private static class MatchingThirdLevelException extends Exception { + private static final long serialVersionUID = 1L; + } +} diff --git a/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtilTest.java b/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtilTest.java index df73d9fd1..b6b7000b1 100644 --- a/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtilTest.java +++ b/hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtilTest.java @@ -24,7 +24,7 @@ import io.qameta.allure.Description; import io.qameta.allure.Feature; import io.qameta.allure.Story; -@Feature("Unit Tests - Localization helper") +@Feature("Unit Tests - Management UI") @Story("Test the locale configuration and prioritization") public class HawkbitCommonUtilTest {