From 175b61b468875f17389e5005e2094bd9a3a434f8 Mon Sep 17 00:00:00 2001 From: Sheikah45 <66929319+Sheikah45@users.noreply.github.com> Date: Sun, 17 Dec 2023 13:16:37 -0500 Subject: [PATCH] Simplify notification service (#3087) --- .../client/FafClientApplication.java | 7 +- .../achievements/AchievementService.java | 2 +- .../chat/AbstractChatTabController.java | 4 +- .../client/chat/KittehChatService.java | 9 +- .../faforever/client/config/AsyncConfig.java | 31 +-- .../faforever/client/config/BaseConfig.java | 27 +++ .../faforever/client/coop/CoopController.java | 2 +- .../exception/GlobalExceptionHandler.java | 4 +- .../faforever/client/fa/GameFullNotifier.java | 3 +- .../fx/contextmenu/AbstractMenuItem.java | 5 - .../client/game/GamePathHandler.java | 3 +- .../com/faforever/client/game/GameRunner.java | 14 +- .../client/game/VaultPathHandler.java | 8 +- .../NotificationButtonController.java | 12 +- .../faforever/client/main/MainController.java | 108 +--------- .../com/faforever/client/map/MapService.java | 2 +- .../client/map/MapUploadController.java | 9 +- .../com/faforever/client/mod/ModService.java | 4 +- .../client/mod/ModUploadController.java | 9 +- .../faforever/client/notification/Action.java | 31 ++- .../client/notification/CopyErrorAction.java | 2 +- .../client/notification/GetHelpAction.java | 2 +- .../notification/ImmediateNotification.java | 19 +- .../ImmediateNotificationController.java | 20 +- .../client/notification/Notification.java | 3 + .../notification/NotificationService.java | 133 ++++++------ .../notification/PersistentNotification.java | 28 +-- .../PersistentNotificationController.java | 8 +- .../PersistentNotificationsController.java | 5 +- .../notification/ServerNotification.java | 30 +++ .../ServerNotificationController.java | 20 +- .../client/notification/Severity.java | 2 +- .../client/notification/ToastDisplayer.java | 92 +++++++++ .../notification/TransientNotification.java | 62 +----- .../TransientNotificationController.java | 14 +- .../TransientNotificationsController.java | 4 +- .../player/FriendJoinedGameNotifier.java | 3 +- .../client/player/FriendOnlineNotifier.java | 4 +- .../preferences/ui/SettingsController.java | 13 +- .../client/remote/FafServerAccessor.java | 7 +- .../client/replay/LiveReplayService.java | 3 +- .../client/replay/ReplayCardController.java | 7 +- .../client/replay/ReplayDetailController.java | 2 +- .../faforever/client/replay/ReplayRunner.java | 7 +- .../faforever/client/replay/ReplayServer.java | 4 +- .../client/replay/ReplayService.java | 6 +- .../TeamMatchmakingService.java | 4 +- .../faforever/client/theme/ThemeService.java | 2 - .../com/faforever/client/ui/alert/Alert.java | 190 +++++++++--------- .../client/update/ClientUpdateService.java | 8 +- .../client/chat/KittehChatServiceTest.java | 4 +- .../client/fa/GameFullNotifierTest.java | 5 +- .../faforever/client/game/GameRunnerTest.java | 4 +- .../notification/NotificationServiceTest.java | 62 ++---- ...PersistentNotificationsControllerTest.java | 32 +-- .../ServerNotificationControllerTest.java | 10 +- .../TransientNotificationControllerTest.java | 10 +- .../TransientNotificationsControllerTest.java | 5 +- .../player/FriendJoinedGameNotifierTest.java | 6 +- .../client/remote/ServerAccessorTest.java | 34 ++-- .../client/replay/ReplayRunnerTest.java | 4 +- .../TeamMatchmakingServiceTest.java | 2 +- .../update/ClientUpdateServiceTest.java | 4 +- 63 files changed, 520 insertions(+), 660 deletions(-) create mode 100644 src/main/java/com/faforever/client/notification/Notification.java create mode 100644 src/main/java/com/faforever/client/notification/ServerNotification.java create mode 100644 src/main/java/com/faforever/client/notification/ToastDisplayer.java diff --git a/src/main/java/com/faforever/client/FafClientApplication.java b/src/main/java/com/faforever/client/FafClientApplication.java index 25092c4eb6..a3e06b055c 100644 --- a/src/main/java/com/faforever/client/FafClientApplication.java +++ b/src/main/java/com/faforever/client/FafClientApplication.java @@ -95,12 +95,7 @@ private void closeMainWindow(WindowEvent event) { notificationService.addNotification(new ImmediateNotification(i18n.get("exitWarning.title"), i18n.get("exitWarning.message"), Severity.WARN, - List.of( - new Action(i18n.get("yes"), ev -> { - Platform.exit(); - }), - new Action(i18n.get("no"), ev -> { - }) + List.of(new Action(i18n.get("yes"), Platform::exit), new Action(i18n.get("no"), () -> {}) ))); event.consume(); } else { diff --git a/src/main/java/com/faforever/client/achievements/AchievementService.java b/src/main/java/com/faforever/client/achievements/AchievementService.java index 8b5d8469c6..965950dd73 100644 --- a/src/main/java/com/faforever/client/achievements/AchievementService.java +++ b/src/main/java/com/faforever/client/achievements/AchievementService.java @@ -70,7 +70,7 @@ public Image getImage(AchievementDefinition achievementDefinition, AchievementSt null, ACHIEVEMENT_IMAGE_SIZE, ACHIEVEMENT_IMAGE_SIZE); } catch (MalformedURLException e) { log.warn("Could not load achievement image bad url for achievement: {}", achievementDefinition.getName(), e); - notificationService.addPersistentErrorNotification(e, "achievements.load.badUrl", achievementDefinition.getName()); + notificationService.addPersistentErrorNotification("achievements.load.badUrl", achievementDefinition.getName()); return null; } } diff --git a/src/main/java/com/faforever/client/chat/AbstractChatTabController.java b/src/main/java/com/faforever/client/chat/AbstractChatTabController.java index 90adc83f47..bbc2b9ad56 100644 --- a/src/main/java/com/faforever/client/chat/AbstractChatTabController.java +++ b/src/main/java/com/faforever/client/chat/AbstractChatTabController.java @@ -110,9 +110,9 @@ public abstract class AbstractChatTabController extends TabController { private static final String JOIN_PREFIX = "/join "; private static final String WHOIS_PREFIX = "/whois "; /** - * Added if a message is what IRC calls an "action". + * Added if a message is what IRC calls an "onAction". */ - private static final String ACTION_CSS_CLASS = "action"; + private static final String ACTION_CSS_CLASS = "onAction"; private static final String MESSAGE_CSS_CLASS = "message"; protected final ChatService chatService; diff --git a/src/main/java/com/faforever/client/chat/KittehChatService.java b/src/main/java/com/faforever/client/chat/KittehChatService.java index 4a65d473c1..c8002689cb 100644 --- a/src/main/java/com/faforever/client/chat/KittehChatService.java +++ b/src/main/java/com/faforever/client/chat/KittehChatService.java @@ -311,9 +311,7 @@ private void notifyIfMentioned(String text, ChatChannel chatChannel, ChatChannel if (!chatChannel.isOpen() && notificationPrefs.isPrivateMessageToastEnabled()) { notificationService.addNotification( new TransientNotification(sender.getUsername(), text, IdenticonUtil.createIdenticon(identIconSource), - evt -> { - navigationHandler.navigateTo(new NavigateEvent(NavigationItem.CHAT)); - })); + () -> navigationHandler.navigateTo(new NavigateEvent(NavigationItem.CHAT)))); } } } @@ -328,9 +326,8 @@ private void notifyOnPrivateMessage(String text, ChatChannel chatChannel, String .map(String::valueOf) .orElse(sender); notificationService.addNotification( - new TransientNotification(sender, text, IdenticonUtil.createIdenticon(identIconSource), evt -> { - navigationHandler.navigateTo(new NavigateEvent(NavigationItem.CHAT)); - })); + new TransientNotification(sender, text, IdenticonUtil.createIdenticon(identIconSource), + () -> navigationHandler.navigateTo(new NavigateEvent(NavigationItem.CHAT)))); } } } diff --git a/src/main/java/com/faforever/client/config/AsyncConfig.java b/src/main/java/com/faforever/client/config/AsyncConfig.java index 9c91ccdbc1..4db6b08bbc 100644 --- a/src/main/java/com/faforever/client/config/AsyncConfig.java +++ b/src/main/java/com/faforever/client/config/AsyncConfig.java @@ -4,20 +4,16 @@ import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.aop.interceptor.AsyncUncaughtExceptionHandler; -import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor; -import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.annotation.AsyncConfigurer; import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.scheduling.annotation.EnableScheduling; import org.springframework.scheduling.annotation.SchedulingConfigurer; -import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.scheduling.config.ScheduledTaskRegistrar; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; @EnableAsync @EnableScheduling @@ -27,10 +23,12 @@ public class AsyncConfig implements AsyncConfigurer, SchedulingConfigurer { private final GlobalExceptionHandler globalExceptionHandler; + private final ExecutorService taskExecutor; + private final TaskScheduler taskScheduler; @Override public Executor getAsyncExecutor() { - return taskExecutor(); + return taskExecutor; } @Override @@ -40,27 +38,6 @@ public AsyncUncaughtExceptionHandler getAsyncUncaughtExceptionHandler() { @Override public void configureTasks(ScheduledTaskRegistrar taskRegistrar) { - taskRegistrar.setTaskScheduler(taskScheduler()); - } - - @Bean - public ExecutorService taskExecutor() { - return Executors.newCachedThreadPool(); - } - - @Bean - public TaskScheduler taskScheduler() { - return new ThreadPoolTaskScheduler(); - } - - @Bean - public DestructionAwareBeanPostProcessor threadPoolShutdownProcessor() { - return (Object bean, String beanName) -> { - if ("taskExecutor".equals(beanName)) { - log.info("Shutting down ExecutorService '" + beanName + "'"); - ExecutorService executor = (ExecutorService) bean; - executor.shutdownNow(); - } - }; + taskRegistrar.setTaskScheduler(taskScheduler); } } diff --git a/src/main/java/com/faforever/client/config/BaseConfig.java b/src/main/java/com/faforever/client/config/BaseConfig.java index 8e18ad6c78..37520140a7 100644 --- a/src/main/java/com/faforever/client/config/BaseConfig.java +++ b/src/main/java/com/faforever/client/config/BaseConfig.java @@ -1,11 +1,17 @@ package com.faforever.client.config; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.support.ReloadableResourceBundleMessageSource; +import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.annotation.EnableScheduling; +import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; /** * This configuration has to be imported by other configurations and should only contain beans that are necessary to run @@ -25,4 +31,25 @@ ReloadableResourceBundleMessageSource messageSource() { messageSource.setFallbackToSystemLocale(false); return messageSource; } + + @Bean + public ExecutorService taskExecutor() { + return Executors.newCachedThreadPool(); + } + + @Bean + public TaskScheduler taskScheduler() { + return new ThreadPoolTaskScheduler(); + } + + @Bean + public DestructionAwareBeanPostProcessor threadPoolShutdownProcessor() { + return (Object bean, String beanName) -> { + if ("taskExecutor".equals(beanName)) { + log.info("Shutting down ExecutorService '" + beanName + "'"); + ExecutorService executor = (ExecutorService) bean; + executor.shutdownNow(); + } + }; + } } diff --git a/src/main/java/com/faforever/client/coop/CoopController.java b/src/main/java/com/faforever/client/coop/CoopController.java index 31ba823b85..935fe0f745 100644 --- a/src/main/java/com/faforever/client/coop/CoopController.java +++ b/src/main/java/com/faforever/client/coop/CoopController.java @@ -210,7 +210,7 @@ protected void onInitialize() { selectionModel.selectFirst(); } }, fxApplicationThreadExecutor).exceptionally(throwable -> { - notificationService.addPersistentErrorNotification(throwable, "coop.couldNotLoad", + notificationService.addPersistentErrorNotification("coop.couldNotLoad", throwable.getLocalizedMessage()); return null; }); diff --git a/src/main/java/com/faforever/client/exception/GlobalExceptionHandler.java b/src/main/java/com/faforever/client/exception/GlobalExceptionHandler.java index 012eed4d29..0eb31dec3c 100644 --- a/src/main/java/com/faforever/client/exception/GlobalExceptionHandler.java +++ b/src/main/java/com/faforever/client/exception/GlobalExceptionHandler.java @@ -38,9 +38,9 @@ public void uncaughtException(Thread t, Throwable ex) { @Override public void handleUncaughtException(@NotNull Throwable ex, @NotNull Method method, Object @NotNull ... params) { - if (ex instanceof NotifiableException) { + if (ex instanceof NotifiableException notifiableException) { log.error("Exception on Method {} with parameters {}: ", method.getName(), params, ex); - notificationService.addErrorNotification((NotifiableException) ex); + notificationService.addErrorNotification(notifiableException); } else { log.error("Uncaught Exception on Method {} with parameters {}: ", method.getName(), params, ex); } diff --git a/src/main/java/com/faforever/client/fa/GameFullNotifier.java b/src/main/java/com/faforever/client/fa/GameFullNotifier.java index 858307140d..2b4f40b7f7 100644 --- a/src/main/java/com/faforever/client/fa/GameFullNotifier.java +++ b/src/main/java/com/faforever/client/fa/GameFullNotifier.java @@ -62,8 +62,7 @@ public void run() { notificationService.addNotification(new TransientNotification(i18n.get("game.full"), i18n.get("game.full.action"), mapService.loadPreview(runningGame.getMapFolderName(), - PreviewSize.SMALL), - ignored -> { + PreviewSize.SMALL), () -> { if (platformService.isWindowFocused(faWindowTitle)) { // Switching to the game lobby window from replay window may not work correctly (no interaction) for reasons: // 1) The game has full screen mode diff --git a/src/main/java/com/faforever/client/fx/contextmenu/AbstractMenuItem.java b/src/main/java/com/faforever/client/fx/contextmenu/AbstractMenuItem.java index acf7e6b696..6ce09cdb1d 100644 --- a/src/main/java/com/faforever/client/fx/contextmenu/AbstractMenuItem.java +++ b/src/main/java/com/faforever/client/fx/contextmenu/AbstractMenuItem.java @@ -2,12 +2,7 @@ import javafx.scene.control.MenuItem; import javafx.scene.layout.Region; -import org.springframework.beans.factory.config.ConfigurableBeanFactory; -import org.springframework.context.annotation.Scope; -import org.springframework.stereotype.Component; -@Component -@Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE) public abstract class AbstractMenuItem extends MenuItem { protected T object; diff --git a/src/main/java/com/faforever/client/game/GamePathHandler.java b/src/main/java/com/faforever/client/game/GamePathHandler.java index 5bd23c9b3a..118a71d2d8 100644 --- a/src/main/java/com/faforever/client/game/GamePathHandler.java +++ b/src/main/java/com/faforever/client/game/GamePathHandler.java @@ -54,8 +54,7 @@ public void afterPropertiesSet() { public void notifyMissingGamePath(boolean immediateUserActionRequired) { List actions = Collections.singletonList( - new Action(i18n.get("missingGamePath.locate"), - chooseEvent -> chooseAndValidateGameDirectory()) + new Action(i18n.get("missingGamePath.locate"), this::chooseAndValidateGameDirectory) ); String notificationText = i18n.get("missingGamePath.notification"); diff --git a/src/main/java/com/faforever/client/game/GameRunner.java b/src/main/java/com/faforever/client/game/GameRunner.java index d64b231f71..11196d2b27 100644 --- a/src/main/java/com/faforever/client/game/GameRunner.java +++ b/src/main/java/com/faforever/client/game/GameRunner.java @@ -30,6 +30,7 @@ import com.faforever.client.notification.ImmediateNotification; import com.faforever.client.notification.NotificationService; import com.faforever.client.notification.PersistentNotification; +import com.faforever.client.notification.ServerNotification; import com.faforever.client.notification.Severity; import com.faforever.client.os.OperatingSystem; import com.faforever.client.player.PlayerService; @@ -309,7 +310,7 @@ private void showRatingOutOfBoundsConfirmation(int playerRating, GameBean game, i18n.get("game.joinGameRatingConfirmation.text", game.getRatingMin(), game.getRatingMax(), playerRating), Severity.INFO, List.of( - new Action(i18n.get("game.join"), event -> join(game, password, true)), new Action(i18n.get("game.cancel"))))); + new Action(i18n.get("game.join"), () -> join(game, password, true)), new Action(i18n.get("game.cancel"))))); } public void startSearchMatchmaker() { @@ -340,9 +341,9 @@ public void startSearchMatchmaker() { if (throwable instanceof CancellationException) { log.info("Matchmaking search has been cancelled"); if (isRunning()) { - notificationService.addServerNotification( - new ImmediateNotification(i18n.get("matchmaker.cancelled.title"), i18n.get("matchmaker.cancelled"), - Severity.INFO)); + notificationService.addNotification( + new ServerNotification(i18n.get("matchmaker.cancelled.title"), i18n.get("matchmaker.cancelled"), + Severity.INFO)); killGame(); } } else { @@ -422,8 +423,7 @@ private void askForGameRate() { GameBean game = getRunningGame(); notificationService.addNotification( new PersistentNotification(i18n.get("game.ended", game.getTitle()), Severity.INFO, List.of( - new Action(i18n.get("game.rate"), - actionEvent -> navigationHandler.navigateTo(new ShowReplayEvent(game.getId())))))); + new Action(i18n.get("game.rate"), () -> navigationHandler.navigateTo(new ShowReplayEvent(game.getId())))))); } private void alertOnBadExit(int exitCode, Optional logFile) { @@ -434,7 +434,7 @@ private void alertOnBadExit(int exitCode, Optional logFile) { i18n.get("game.crash", exitCode, logFile.map(Path::toString).orElse("")), WARN, List.of(new Action(i18n.get("game.open.log"), - event -> platformService.reveal( + () -> platformService.reveal( logFile.orElse( operatingSystem.getLoggingDirectory()))), new DismissAction(i18n)))); diff --git a/src/main/java/com/faforever/client/game/VaultPathHandler.java b/src/main/java/com/faforever/client/game/VaultPathHandler.java index fe35099edc..3369b00f9f 100644 --- a/src/main/java/com/faforever/client/game/VaultPathHandler.java +++ b/src/main/java/com/faforever/client/game/VaultPathHandler.java @@ -63,7 +63,7 @@ private void showWarning() { notificationService.addImmediateWarnNotification(i18n.get("vaultBasePath.nonAscii.warning.title"), i18n.get("vaultBasePath.nonAscii.warning.text"), List.of( - new Action(i18n.get("vaultBasePath.nonAscii.warning.changePath"), event -> askForPathAndUpdate()), + new Action(i18n.get("vaultBasePath.nonAscii.warning.changePath"), this::askForPathAndUpdate), new OkAction(i18n)), ignoreCheckbox); } @@ -79,10 +79,12 @@ private void onVaultPathUpdated(Path newPath) { moveDirectoryTask.setOldDirectory(forgedAlliancePrefs.getVaultBaseDirectory()); moveDirectoryTask.setNewDirectory(newPath); moveDirectoryTask.setAfterCopyAction(() -> forgedAlliancePrefs.setVaultBaseDirectory(newPath)); - notificationService.addNotification(new ImmediateNotification(i18n.get("settings.vault.change"), i18n.get("settings.vault.change.message"), Severity.INFO, List.of(new Action(i18n.get("no"), event -> { + notificationService.addNotification( + new ImmediateNotification(i18n.get("settings.vault.change"), i18n.get("settings.vault.change.message"), + Severity.INFO, List.of(new Action(i18n.get("no"), () -> { moveDirectoryTask.setPreserveOldDirectory(false); taskService.submitTask(moveDirectoryTask); - }), new Action(i18n.get("yes"), event -> { + }), new Action(i18n.get("yes"), () -> { moveDirectoryTask.setPreserveOldDirectory(true); taskService.submitTask(moveDirectoryTask); }), new CancelAction(i18n)))); diff --git a/src/main/java/com/faforever/client/headerbar/NotificationButtonController.java b/src/main/java/com/faforever/client/headerbar/NotificationButtonController.java index 3a914be6ea..d914c72915 100644 --- a/src/main/java/com/faforever/client/headerbar/NotificationButtonController.java +++ b/src/main/java/com/faforever/client/headerbar/NotificationButtonController.java @@ -1,6 +1,7 @@ package com.faforever.client.headerbar; import com.faforever.client.fx.FxApplicationThreadExecutor; +import com.faforever.client.fx.JavaFxUtil; import com.faforever.client.fx.NodeController; import com.faforever.client.notification.NotificationService; import com.faforever.client.notification.PersistentNotification; @@ -38,21 +39,20 @@ public class NotificationButtonController extends NodeController { @Override protected void onInitialize() { + JavaFxUtil.bindManagedToVisible(notificationButton); updateNotificationsButton(notificationService.getPersistentNotifications()); - notificationService.addPersistentNotificationListener(change -> fxApplicationThreadExecutor.execute(() -> updateNotificationsButton(change.getSet()))); - - notificationButton.managedProperty().bind(notificationButton.visibleProperty()); + notificationService.getPersistentNotifications() + .subscribe(() -> updateNotificationsButton(notificationService.getPersistentNotifications())); } /** * Updates the number displayed in the notifications button and sets its CSS pseudo class based on the highest * notification {@code Severity} of all current notifications. */ - private void updateNotificationsButton(Collection notifications) { + private void updateNotificationsButton(Collection notifications) { int size = notifications.size(); - Severity highestSeverity = notifications.stream() - .map(PersistentNotification::getSeverity) + Severity highestSeverity = notifications.stream().map(PersistentNotification::severity) .max(Enum::compareTo) .orElse(null); diff --git a/src/main/java/com/faforever/client/main/MainController.java b/src/main/java/com/faforever/client/main/MainController.java index 2ba2472a70..7ec0968480 100644 --- a/src/main/java/com/faforever/client/main/MainController.java +++ b/src/main/java/com/faforever/client/main/MainController.java @@ -14,27 +14,17 @@ import com.faforever.client.navigation.NavigationHandler; import com.faforever.client.notification.Action; import com.faforever.client.notification.ImmediateNotification; -import com.faforever.client.notification.ImmediateNotificationController; import com.faforever.client.notification.NotificationService; import com.faforever.client.notification.PersistentNotification; -import com.faforever.client.notification.ServerNotificationController; import com.faforever.client.notification.Severity; -import com.faforever.client.notification.TransientNotificationsController; -import com.faforever.client.preferences.NotificationPrefs; import com.faforever.client.preferences.WindowPrefs; import com.faforever.client.theme.UiService; import com.faforever.client.ui.StageHolder; -import com.faforever.client.ui.alert.Alert; -import com.faforever.client.ui.alert.animation.AlertAnimation; import com.faforever.client.ui.tray.TrayIconManager; import com.faforever.client.ui.tray.event.UpdateApplicationBadgeEvent; import com.faforever.client.user.LoginService; -import com.faforever.client.util.PopupUtil; import com.google.common.annotations.VisibleForTesting; -import javafx.beans.InvalidationListener; -import javafx.beans.Observable; import javafx.collections.ObservableList; -import javafx.geometry.Rectangle2D; import javafx.scene.Node; import javafx.scene.image.Image; import javafx.scene.layout.Background; @@ -45,8 +35,6 @@ import javafx.scene.layout.HBox; import javafx.scene.layout.Pane; import javafx.scene.layout.StackPane; -import javafx.stage.Popup; -import javafx.stage.PopupWindow.AnchorLocation; import javafx.stage.Screen; import javafx.stage.Stage; import lombok.RequiredArgsConstructor; @@ -77,7 +65,6 @@ public class MainController extends NodeController implements Initializing private final NotificationService notificationService; private final UiService uiService; private final LoginService loginService; - private final NotificationPrefs notificationPrefs; private final WindowPrefs windowPrefs; private final FxApplicationThreadExecutor fxApplicationThreadExecutor; private final TrayIconManager trayIconManager; @@ -89,8 +76,6 @@ public class MainController extends NodeController implements Initializing public HBox headerBar; public HeaderBarController headerBarController; - @VisibleForTesting - protected Popup transientNotificationsPopup; private FxStage fxStage; @Override @@ -124,18 +109,6 @@ private static void hideSplashScreen() { @Override protected void onInitialize() { - TransientNotificationsController transientNotificationsController = uiService.loadFxml("theme/transient_notifications.fxml"); - transientNotificationsPopup = PopupUtil.createPopup(transientNotificationsController.getRoot()); - transientNotificationsPopup.getScene().getRoot().getStyleClass().add("transient-notification"); - - transientNotificationsController.getRoot() - .getChildren() - .addListener(new ToastDisplayer(transientNotificationsController)); - - notificationService.addImmediateNotificationListener(notification -> fxApplicationThreadExecutor.execute(() -> displayImmediateNotification(notification))); - notificationService.addServerNotificationListener(notification -> fxApplicationThreadExecutor.execute(() -> displayServerNotification(notification))); - notificationService.addTransientNotificationListener(notification -> fxApplicationThreadExecutor.execute(() -> transientNotificationsController.addNotification(notification))); - navigationHandler.navigationEventProperty().subscribe(this::onNavigateEvent); } @@ -146,19 +119,6 @@ private void displayView(NodeController controller, NavigateEvent navigateEve JavaFxUtil.setAnchors(node, 0d); } - private Rectangle2D getTransientNotificationAreaBounds() { - ObservableList screens = Screen.getScreens(); - - int toastScreenIndex = notificationPrefs.getToastScreen(); - Screen screen; - if (toastScreenIndex < screens.size()) { - screen = screens.get(Math.max(0, toastScreenIndex)); - } else { - screen = Screen.getPrimary(); - } - return screen.getVisualBounds(); - } - public void display() { trayIconManager.onSetApplicationBadgeEvent(UpdateApplicationBadgeEvent.ofNewValue(0)); @@ -291,14 +251,14 @@ void openStartTab() { private void askUserForPreferenceOverStartTab() { windowPrefs.setNavigationItem(NavigationItem.NEWS); - List actions = Collections.singletonList(new Action(i18n.get("startTab.configure"), event -> - makePopUpAskingForPreferenceInStartTab())); + List actions = Collections.singletonList( + new Action(i18n.get("startTab.configure"), this::makePopUpAskingForPreferenceInStartTab)); notificationService.addNotification(new PersistentNotification(i18n.get("startTab.wantToConfigure"), Severity.INFO, actions)); } private void makePopUpAskingForPreferenceInStartTab() { StartTabChooseController startTabChooseController = uiService.loadFxml("theme/start_tab_choose.fxml"); - Action saveAction = new Action(i18n.get("startTab.save"), event -> { + Action saveAction = new Action(i18n.get("startTab.save"), () -> { NavigationItem newSelection = startTabChooseController.getSelected(); windowPrefs.setNavigationItem(newSelection); navigationHandler.navigateTo(new NavigateEvent(newSelection)); @@ -325,70 +285,8 @@ public void onNavigateEvent(NavigateEvent navigateEvent) { displayView(controller, navigateEvent); } - private void displayImmediateNotification(ImmediateNotification notification) { - Alert dialog = new Alert<>(fxStage.getStage(), fxApplicationThreadExecutor); - - ImmediateNotificationController controller = ((ImmediateNotificationController) uiService.loadFxml("theme/immediate_notification.fxml")) - .setNotification(notification) - .setCloseListener(dialog::close); - - dialog.setContent(controller.getDialogLayout()); - dialog.setAnimation(AlertAnimation.TOP_ANIMATION); - dialog.show(); - } - - private void displayServerNotification(ImmediateNotification notification) { - Alert dialog = new Alert<>(fxStage.getStage(), fxApplicationThreadExecutor); - - ServerNotificationController controller = ((ServerNotificationController) uiService.loadFxml("theme/server_notification.fxml")) - .setNotification(notification) - .setCloseListener(dialog::close); - - dialog.setContent(controller.getDialogLayout()); - dialog.setAnimation(AlertAnimation.TOP_ANIMATION); - dialog.show(); - } - public void setFxStage(FxStage fxWindow) { this.fxStage = fxWindow; } - public class ToastDisplayer implements InvalidationListener { - private final TransientNotificationsController transientNotificationsController; - - public ToastDisplayer(TransientNotificationsController transientNotificationsController) { - this.transientNotificationsController = transientNotificationsController; - } - - @Override - public void invalidated(Observable observable) { - boolean enabled = notificationPrefs.isTransientNotificationsEnabled(); - if (transientNotificationsController.getRoot().getChildren().isEmpty() || !enabled) { - transientNotificationsPopup.hide(); - return; - } - - Rectangle2D visualBounds = getTransientNotificationAreaBounds(); - double anchorX = visualBounds.getMaxX() - 1; - double anchorY = visualBounds.getMaxY() - 1; - AnchorLocation location = switch (notificationPrefs.getToastPosition()) { - case BOTTOM_RIGHT -> AnchorLocation.CONTENT_BOTTOM_RIGHT; - case TOP_RIGHT -> { - anchorY = visualBounds.getMinY(); - yield AnchorLocation.CONTENT_TOP_RIGHT; - } - case BOTTOM_LEFT -> { - anchorX = visualBounds.getMinX(); - yield AnchorLocation.CONTENT_BOTTOM_LEFT; - } - case TOP_LEFT -> { - anchorX = visualBounds.getMinX(); - anchorY = visualBounds.getMinY(); - yield AnchorLocation.CONTENT_TOP_LEFT; - } - }; - transientNotificationsPopup.setAnchorLocation(location); - transientNotificationsPopup.show(mainRoot.getScene().getWindow(), anchorX, anchorY); - } - } } diff --git a/src/main/java/com/faforever/client/map/MapService.java b/src/main/java/com/faforever/client/map/MapService.java index f60a00d3ea..081bfa0002 100644 --- a/src/main/java/com/faforever/client/map/MapService.java +++ b/src/main/java/com/faforever/client/map/MapService.java @@ -621,7 +621,7 @@ public CompletableFuture downloadAllMatchmakerMaps(MatchmakerQueueBean mat .flatMap(mapVersion -> Mono.fromCompletionStage(() -> downloadAndInstallMap(mapVersion, null, null)) .onErrorResume(throwable -> { log.warn("Unable to download map `{}`", mapVersion.getFolderName(), throwable); - notificationService.addPersistentErrorNotification(throwable, "map.download.error", mapVersion.getFolderName()); + notificationService.addPersistentErrorNotification("map.download.error", mapVersion.getFolderName()); return Mono.empty(); })) .then() diff --git a/src/main/java/com/faforever/client/map/MapUploadController.java b/src/main/java/com/faforever/client/map/MapUploadController.java index f2815d70dd..e9333d4c34 100644 --- a/src/main/java/com/faforever/client/map/MapUploadController.java +++ b/src/main/java/com/faforever/client/map/MapUploadController.java @@ -11,8 +11,8 @@ import com.faforever.client.notification.CopyErrorAction; import com.faforever.client.notification.DismissAction; import com.faforever.client.notification.GetHelpAction; -import com.faforever.client.notification.ImmediateNotification; import com.faforever.client.notification.NotificationService; +import com.faforever.client.notification.ServerNotification; import com.faforever.client.reporting.ReportingService; import com.faforever.client.task.CompletableTask; import com.faforever.client.util.ConcurrentUtil; @@ -156,7 +156,7 @@ public void onCancelUploadClicked() { private void onUploadFailed(Throwable throwable) { enterMapInfoState(); if (throwable instanceof ApiException) { - notificationService.addServerNotification(new ImmediateNotification( + notificationService.addNotification(new ServerNotification( i18n.get("errorTitle"), i18n.get("mapVault.upload.failed", throwable.getLocalizedMessage()), ERROR, asList( new GetHelpAction(i18n, reportingService), @@ -164,10 +164,9 @@ private void onUploadFailed(Throwable throwable) { ) )); } else { - notificationService.addServerNotification(new ImmediateNotification( + notificationService.addNotification(new ServerNotification( i18n.get("errorTitle"), i18n.get("mapVault.upload.failed", throwable.getLocalizedMessage()), ERROR, throwable, - asList( - new Action(i18n.get("mapVault.upload.retry"), event -> onUploadClicked()), + asList(new Action(i18n.get("mapVault.upload.retry"), this::onUploadClicked), new CopyErrorAction(i18n, reportingService, throwable), new GetHelpAction(i18n, reportingService), new DismissAction(i18n) diff --git a/src/main/java/com/faforever/client/mod/ModService.java b/src/main/java/com/faforever/client/mod/ModService.java index edcd1f65e9..09929d19b9 100644 --- a/src/main/java/com/faforever/client/mod/ModService.java +++ b/src/main/java/com/faforever/client/mod/ModService.java @@ -166,7 +166,7 @@ private Thread startDirectoryWatcher(Path modsDirectory) { .subscribe(null, throwable -> { log.error("Mod could not be read: `{}`", modPath, throwable); notificationService.addPersistentWarnNotification( - List.of(new Action(i18n.get("corruptedMods.show"), evt -> platformService.reveal(modPath))), + List.of(new Action(i18n.get("corruptedMods.show"), () -> platformService.reveal(modPath))), "corruptedModsError.notification", modPath.getFileName()); }); } @@ -203,7 +203,7 @@ protected Void call() { log.warn("Corrupt mod: `{}`", modPath, e); notificationService.addPersistentWarnNotification( - List.of(new Action(i18n.get("corruptedMods.show"), event -> platformService.reveal(modPath))), + List.of(new Action(i18n.get("corruptedMods.show"), () -> platformService.reveal(modPath))), "corruptedModsError.notification", modPath.getFileName()); } } diff --git a/src/main/java/com/faforever/client/mod/ModUploadController.java b/src/main/java/com/faforever/client/mod/ModUploadController.java index d9774b5633..e8d3964ea3 100644 --- a/src/main/java/com/faforever/client/mod/ModUploadController.java +++ b/src/main/java/com/faforever/client/mod/ModUploadController.java @@ -11,8 +11,8 @@ import com.faforever.client.notification.CopyErrorAction; import com.faforever.client.notification.DismissAction; import com.faforever.client.notification.GetHelpAction; -import com.faforever.client.notification.ImmediateNotification; import com.faforever.client.notification.NotificationService; +import com.faforever.client.notification.ServerNotification; import com.faforever.client.reporting.ReportingService; import com.faforever.client.task.CompletableTask; import com.faforever.client.util.ConcurrentUtil; @@ -137,7 +137,7 @@ public void onCancelUploadClicked() { private void onUploadFailed(Throwable throwable) { enterModInfoState(); if (throwable instanceof ApiException) { - notificationService.addServerNotification(new ImmediateNotification( + notificationService.addNotification(new ServerNotification( i18n.get("errorTitle"), i18n.get("modVault.upload.failed", throwable.getLocalizedMessage()), ERROR, asList( new GetHelpAction(i18n, reportingService), @@ -145,10 +145,9 @@ private void onUploadFailed(Throwable throwable) { ) )); } else { - notificationService.addServerNotification(new ImmediateNotification( + notificationService.addNotification(new ServerNotification( i18n.get("errorTitle"), i18n.get("modVault.upload.failed", throwable.getLocalizedMessage()), ERROR, throwable, - asList( - new Action(i18n.get("modVault.upload.retry"), event -> onUploadClicked()), + asList(new Action(i18n.get("modVault.upload.retry"), this::onUploadClicked), new CopyErrorAction(i18n, reportingService, throwable), new GetHelpAction(i18n, reportingService), new DismissAction(i18n) diff --git a/src/main/java/com/faforever/client/notification/Action.java b/src/main/java/com/faforever/client/notification/Action.java index 78604b6730..76628c5007 100644 --- a/src/main/java/com/faforever/client/notification/Action.java +++ b/src/main/java/com/faforever/client/notification/Action.java @@ -1,56 +1,53 @@ package com.faforever.client.notification; -import javafx.event.Event; import lombok.Getter; -import java.util.function.Consumer; - /** - * Notifications have actions associated with it. This class represents such an action, which is usually displayed to + * Notifications have actions associated with it. This class represents such an onAction, which is usually displayed to * the user as a button. */ public class Action { @Getter private final String title; - private final Consumer callback; + private final Runnable onAction; @Getter private final Type type; - public Action(Consumer callback) { - this(null, Type.OK_DONE, callback); + public Action(Runnable onAction) { + this(null, Type.OK_DONE, onAction); } /** - * Creates an action that calls the specified callback when executed. Also, a type is specified that + * Creates an onAction that calls the specified callback when executed. Also, a type is specified that */ - public Action(String title, Type type, Consumer callback) { + public Action(String title, Type type, Runnable onAction) { this.title = title; this.type = type; - this.callback = callback; + this.onAction = onAction; } /** - * Creates an action that does nothing. + * Creates an onAction that does nothing. */ public Action(String title) { this(title, Type.OK_DONE, null); } /** - * Creates an action that calls the specified callback when executed. The action will have the default action type + * Creates an onAction that calls the specified callback when executed. The onAction will have the default onAction type * {@link com.faforever.client.notification.Action.Type#OK_DONE}. */ - public Action(String title, Consumer callback) { - this(title, Type.OK_DONE, callback); + public Action(String title, Runnable onAction) { + this(title, Type.OK_DONE, onAction); } /** * Calls the specified callback, if any. Subclasses may override. */ - public void call(Event event) { - if (callback != null) { - callback.accept(event); + public void run() { + if (onAction != null) { + onAction.run(); } } diff --git a/src/main/java/com/faforever/client/notification/CopyErrorAction.java b/src/main/java/com/faforever/client/notification/CopyErrorAction.java index 0fdaf53d2d..b7726f6ca0 100644 --- a/src/main/java/com/faforever/client/notification/CopyErrorAction.java +++ b/src/main/java/com/faforever/client/notification/CopyErrorAction.java @@ -6,6 +6,6 @@ public class CopyErrorAction extends Action { public CopyErrorAction(I18n i18n, ReportingService reportingService, Throwable throwable) { - super(i18n.get("copyError"), Type.OK_STAY, event -> reportingService.copyError(throwable)); + super(i18n.get("copyError"), Type.OK_STAY, () -> reportingService.copyError(throwable)); } } diff --git a/src/main/java/com/faforever/client/notification/GetHelpAction.java b/src/main/java/com/faforever/client/notification/GetHelpAction.java index ada60f3ce3..3f9d0af0ca 100644 --- a/src/main/java/com/faforever/client/notification/GetHelpAction.java +++ b/src/main/java/com/faforever/client/notification/GetHelpAction.java @@ -6,6 +6,6 @@ public class GetHelpAction extends Action { public GetHelpAction(I18n i18n, ReportingService reportingService) { - super(i18n.get("getHelp"), Type.OK_STAY, event -> reportingService.getHelp()); + super(i18n.get("getHelp"), Type.OK_STAY, reportingService::getHelp); } } diff --git a/src/main/java/com/faforever/client/notification/ImmediateNotification.java b/src/main/java/com/faforever/client/notification/ImmediateNotification.java index 96e39117fd..e1481fb33b 100644 --- a/src/main/java/com/faforever/client/notification/ImmediateNotification.java +++ b/src/main/java/com/faforever/client/notification/ImmediateNotification.java @@ -1,27 +1,16 @@ package com.faforever.client.notification; import javafx.scene.Parent; -import lombok.Getter; -import lombok.RequiredArgsConstructor; -import lombok.Setter; import java.util.List; /** * A notification that requires the user's immediate attention. It is displayed until the user performs a suggested - * action or dismisses it. The notification consists of a title, a text, an optional image and zero or more actions. + * onAction or dismisses it. The notification consists of a title, a text, an optional image and zero or more actions. */ -@RequiredArgsConstructor -@Getter -@Setter -public class ImmediateNotification { - - private final String title; - private final String text; - private final Severity severity; - private final Throwable throwable; - private final List actions; - private final Parent customUI; +public record ImmediateNotification( + String title, String text, Severity severity, Throwable throwable, List actions, Parent customUI +) implements Notification { public ImmediateNotification(String title, String text, Severity severity) { this(title, text, severity, null); diff --git a/src/main/java/com/faforever/client/notification/ImmediateNotificationController.java b/src/main/java/com/faforever/client/notification/ImmediateNotificationController.java index a1602be71b..07fb2d29ad 100644 --- a/src/main/java/com/faforever/client/notification/ImmediateNotificationController.java +++ b/src/main/java/com/faforever/client/notification/ImmediateNotificationController.java @@ -47,9 +47,9 @@ protected void onInitialize() { dialogLayout.setBody(immediateNotificationRoot); } - public ImmediateNotificationController setNotification(ImmediateNotification notification) { + public void setNotification(ImmediateNotification notification) { StringWriter writer = new StringWriter(); - Throwable throwable = notification.getThrowable(); + Throwable throwable = notification.throwable(); if (throwable != null) { throwable.printStackTrace(new PrintWriter(writer)); exceptionTextArea.setVisible(true); @@ -62,22 +62,21 @@ public ImmediateNotificationController setNotification(ImmediateNotification not versionText.setVisible(false); } - dialogLayout.setHeading(new Label(notification.getTitle())); - notificationText.setText(notification.getText()); + dialogLayout.setHeading(new Label(notification.title())); + notificationText.setText(notification.text()); - Optional.ofNullable(notification.getActions()) + Optional.ofNullable(notification.actions()) .map(actions -> actions.stream().map(this::createButton).collect(Collectors.toList())) .ifPresent(dialogLayout::setActions); - if (notification.getCustomUI() != null) { - immediateNotificationRoot.getChildren().add(notification.getCustomUI()); + if (notification.customUI() != null) { + immediateNotificationRoot.getChildren().add(notification.customUI()); } - return this; } private Button createButton(Action action) { Button button = new Button(action.getTitle()); button.setOnAction(event -> { - action.call(event); + action.run(); if (action.getType() == Action.Type.OK_DONE) { dismiss(); } @@ -100,9 +99,8 @@ public Region getRoot() { return immediateNotificationRoot; } - public ImmediateNotificationController setCloseListener(Runnable closeListener) { + public void setCloseListener(Runnable closeListener) { this.closeListener = closeListener; - return this; } public DialogLayout getDialogLayout() { diff --git a/src/main/java/com/faforever/client/notification/Notification.java b/src/main/java/com/faforever/client/notification/Notification.java new file mode 100644 index 0000000000..6b17270973 --- /dev/null +++ b/src/main/java/com/faforever/client/notification/Notification.java @@ -0,0 +1,3 @@ +package com.faforever.client.notification; + +public sealed interface Notification permits ImmediateNotification, PersistentNotification, ServerNotification, TransientNotification {} diff --git a/src/main/java/com/faforever/client/notification/NotificationService.java b/src/main/java/com/faforever/client/notification/NotificationService.java index 2a06b92135..a427cf3ac7 100644 --- a/src/main/java/com/faforever/client/notification/NotificationService.java +++ b/src/main/java/com/faforever/client/notification/NotificationService.java @@ -3,20 +3,21 @@ import com.faforever.client.exception.MajorNotifiableException; import com.faforever.client.exception.MinorNotifiableException; import com.faforever.client.exception.NotifiableException; -import com.faforever.client.fx.JavaFxUtil; +import com.faforever.client.fx.FxApplicationThreadExecutor; import com.faforever.client.i18n.I18n; import com.faforever.client.reporting.ReportingService; +import com.faforever.client.theme.UiService; +import com.faforever.client.ui.StageHolder; +import com.faforever.client.ui.alert.Alert; +import com.faforever.client.ui.alert.animation.AlertAnimation; +import javafx.collections.FXCollections; import javafx.collections.ObservableSet; -import javafx.collections.SetChangeListener; import javafx.scene.Parent; import lombok.RequiredArgsConstructor; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Service; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.Set; import java.util.TreeSet; import static com.faforever.client.notification.Severity.ERROR; @@ -32,81 +33,37 @@ @RequiredArgsConstructor public class NotificationService { - private final ObservableSet persistentNotifications = synchronizedObservableSet(observableSet(new TreeSet<>())); - private final List onTransientNotificationListeners = new ArrayList<>(); - private final List onImmediateNotificationListeners = new ArrayList<>(); - private final List onServerNotificationListeners = new ArrayList<>(); + private final UiService uiService; private final ReportingService reportingService; private final I18n i18n; + private final ToastDisplayer toastDisplayer; + private final FxApplicationThreadExecutor fxApplicationThreadExecutor; - /** - * Adds a {@link PersistentNotification} to be displayed. - */ - - public void addNotification(PersistentNotification notification) { - persistentNotifications.add(notification); - } - - /** - * Adds a {@link TransientNotification} to be displayed. - */ - - public void addNotification(TransientNotification notification) { - onTransientNotificationListeners.forEach(listener -> listener.onTransientNotification(notification)); - } - - /** - * Adds a {@link ImmediateNotification} to be displayed. - */ - - public void addNotification(ImmediateNotification notification) { - onImmediateNotificationListeners.forEach(listener -> listener.onImmediateNotification(notification)); - } - - /** - * Adds a {@link ImmediateNotification} to be displayed. - */ - - public void addServerNotification(ImmediateNotification notification) { - onServerNotificationListeners.forEach(listener -> listener.onImmediateNotification(notification)); - } - - /** - * Adds a listener to be notified about added/removed {@link PersistentNotification}s - */ - - public void addPersistentNotificationListener(SetChangeListener listener) { - JavaFxUtil.addListener(persistentNotifications, listener); - } - - /** - * Adds a listener to be notified whenever a {@link TransientNotification} has been fired. - */ - - public void addTransientNotificationListener(OnTransientNotificationListener listener) { - onTransientNotificationListeners.add(listener); - } - - - public Set getPersistentNotifications() { - return Collections.unmodifiableSet(persistentNotifications); - } - + private final ObservableSet persistentNotifications = synchronizedObservableSet(observableSet(new TreeSet<>())); - public void removeNotification(PersistentNotification notification) { - persistentNotifications.remove(notification); + public ObservableSet getPersistentNotifications() { + return FXCollections.unmodifiableObservableSet(persistentNotifications); } - - public void addImmediateNotificationListener(OnImmediateNotificationListener listener) { - onImmediateNotificationListeners.add(listener); + public void addNotification(Notification notification) { + switch (notification) { + case ImmediateNotification immediateNotification -> displayImmediateNotification(immediateNotification); + case ServerNotification serverNotification -> displayServerNotification(serverNotification); + case PersistentNotification persistentNotification -> persistentNotifications.add(persistentNotification); + case TransientNotification transientNotification -> toastDisplayer.addNotification(transientNotification); + } } - public void addServerNotificationListener(OnImmediateNotificationListener listener) { - onServerNotificationListeners.add(listener); + public void removeNotification(Notification notification) { + switch (notification) { + case PersistentNotification persistentNotification -> persistentNotifications.remove(persistentNotification); + case ImmediateNotification immediateNotification -> {} + case ServerNotification serverNotification -> {} + case TransientNotification transientNotification -> {} + } } - public void addPersistentErrorNotification(Throwable throwable, String messageKey, Object... args) { + public void addPersistentErrorNotification(String messageKey, Object... args) { addNotification(new PersistentNotification(i18n.get(messageKey, args), ERROR, singletonList(new GetHelpAction(i18n, reportingService)))); } @@ -122,9 +79,10 @@ public void addImmediateErrorNotification(Throwable throwable, String messageKey public void addErrorNotification(NotifiableException throwable) { switch (throwable) { case MajorNotifiableException majorNotifiableException -> - addImmediateErrorNotification(throwable.getCause(), throwable.getI18nKey(), throwable.getI18nArgs()); + addImmediateErrorNotification(majorNotifiableException.getCause(), majorNotifiableException.getI18nKey(), + majorNotifiableException.getI18nArgs()); case MinorNotifiableException minorNotifiableException -> - addPersistentErrorNotification(throwable.getCause(), throwable.getI18nKey(), throwable.getI18nArgs()); + addPersistentErrorNotification(minorNotifiableException.getI18nKey(), minorNotifiableException.getI18nArgs()); } } @@ -139,4 +97,35 @@ public void addImmediateWarnNotification(String title, String text, List public void addImmediateInfoNotification(String messageKey, Object... args) { addNotification(new ImmediateNotification("", i18n.get(messageKey, args), INFO, List.of(new OkAction(i18n)))); } + + private void displayImmediateNotification(ImmediateNotification notification) { + ImmediateNotificationController controller = uiService.loadFxml("theme/immediate_notification.fxml"); + + controller.setNotification(notification); + + fxApplicationThreadExecutor.execute(() -> { + Alert dialog = new Alert<>(StageHolder.getStage()); + + controller.setCloseListener(dialog::close); + + dialog.setContent(controller.getDialogLayout()); + dialog.setAnimation(AlertAnimation.TOP_ANIMATION); + dialog.show(); + }); + } + + private void displayServerNotification(ServerNotification notification) { + ServerNotificationController controller = uiService.loadFxml("theme/server_notification.fxml"); + controller.setNotification(notification); + + fxApplicationThreadExecutor.execute(() -> { + Alert dialog = new Alert<>(StageHolder.getStage()); + + controller.setCloseListener(dialog::close); + + dialog.setContent(controller.getDialogLayout()); + dialog.setAnimation(AlertAnimation.TOP_ANIMATION); + dialog.show(); + }); + } } diff --git a/src/main/java/com/faforever/client/notification/PersistentNotification.java b/src/main/java/com/faforever/client/notification/PersistentNotification.java index c849f93b27..7e64e62f18 100644 --- a/src/main/java/com/faforever/client/notification/PersistentNotification.java +++ b/src/main/java/com/faforever/client/notification/PersistentNotification.java @@ -5,37 +5,17 @@ import java.util.List; /** - * A notification that keeps displaying until the user performs a suggested action or dismisses it. The notification + * A notification that keeps displaying until the user performs a suggested onAction or dismisses it. The notification * consist of a severity, a text and zero or more actions and is always rendered with a close button. */ -public class PersistentNotification implements Comparable { - - private final String text; - private final Severity severity; - private final List actions; +public record PersistentNotification( + String text, Severity severity, List actions +) implements Comparable, Notification { public PersistentNotification(String text, Severity severity) { this(text, severity, null); } - public PersistentNotification(String text, Severity severity, List actions) { - this.text = text; - this.severity = severity; - this.actions = actions; - } - - public String getText() { - return text; - } - - public List getActions() { - return actions; - } - - public Severity getSeverity() { - return severity; - } - @Override public int compareTo(@NotNull PersistentNotification o) { return text.compareTo(o.text); diff --git a/src/main/java/com/faforever/client/notification/PersistentNotificationController.java b/src/main/java/com/faforever/client/notification/PersistentNotificationController.java index 89e6382673..608ffdd729 100644 --- a/src/main/java/com/faforever/client/notification/PersistentNotificationController.java +++ b/src/main/java/com/faforever/client/notification/PersistentNotificationController.java @@ -50,9 +50,9 @@ protected void onInitialize() { */ public void setNotification(PersistentNotification notification) { this.notification = notification; - messageLabel.setText(notification.getText()); - setImageBasedOnSeverity(notification.getSeverity()); - setActions(notification.getActions()); + messageLabel.setText(notification.text()); + setImageBasedOnSeverity(notification.severity()); + setActions(notification.actions()); } private void setImageBasedOnSeverity(Severity severity) { @@ -86,7 +86,7 @@ private void setActions(List actions) { Button button = new Button(action.getTitle()); button.setFocusTraversable(false); button.setOnAction(event -> { - action.call(event); + action.run(); if (action.getType() == Action.Type.OK_DONE) { dismiss(); } diff --git a/src/main/java/com/faforever/client/notification/PersistentNotificationsController.java b/src/main/java/com/faforever/client/notification/PersistentNotificationsController.java index 77999d4f3e..760b739a3d 100644 --- a/src/main/java/com/faforever/client/notification/PersistentNotificationsController.java +++ b/src/main/java/com/faforever/client/notification/PersistentNotificationsController.java @@ -5,6 +5,7 @@ import com.faforever.client.fx.NodeController; import com.faforever.client.theme.UiService; import javafx.collections.ObservableList; +import javafx.collections.SetChangeListener; import javafx.scene.Node; import javafx.scene.control.Label; import javafx.scene.layout.Pane; @@ -35,7 +36,7 @@ public class PersistentNotificationsController extends NodeController { @Override protected void onInitialize() { - notificationService.addPersistentNotificationListener(change -> { + notificationService.getPersistentNotifications().addListener((SetChangeListener) change -> { if (change.wasAdded()) { PersistentNotification addedNotifications = change.getElementAdded(); addNotification(addedNotifications); @@ -71,7 +72,7 @@ private void removeNotification(PersistentNotification removedNotifications) { } private void playNotificationSound(PersistentNotification notification) { - switch (notification.getSeverity()) { + switch (notification.severity()) { case INFO -> audioService.playInfoNotificationSound(); case WARN -> audioService.playWarnNotificationSound(); case ERROR -> audioService.playErrorNotificationSound(); diff --git a/src/main/java/com/faforever/client/notification/ServerNotification.java b/src/main/java/com/faforever/client/notification/ServerNotification.java new file mode 100644 index 0000000000..2ba0ff4a4d --- /dev/null +++ b/src/main/java/com/faforever/client/notification/ServerNotification.java @@ -0,0 +1,30 @@ +package com.faforever.client.notification; + +import javafx.scene.Parent; + +import java.util.List; + +/** + * A notification that requires the user's immediate attention. It is displayed until the user performs a suggested + * onAction or dismisses it. The notification consists of a title, a text, an optional image and zero or more actions. + */ +public record ServerNotification( + String title, String text, Severity severity, Throwable throwable, List actions, Parent customUI +) implements Notification { + + public ServerNotification(String title, String text, Severity severity) { + this(title, text, severity, null); + } + + public ServerNotification(String title, String text, Severity severity, List actions) { + this(title, text, severity, null, actions, null); + } + + public ServerNotification(String title, String text, Severity severity, Throwable throwable, List actions) { + this(title, text, severity, throwable, actions, null); + } + + public ServerNotification(String title, String text, Severity severity, List actions, Parent customUI) { + this(title, text, severity, null, actions, customUI); + } +} diff --git a/src/main/java/com/faforever/client/notification/ServerNotificationController.java b/src/main/java/com/faforever/client/notification/ServerNotificationController.java index 063ce8a08e..8a55c94105 100644 --- a/src/main/java/com/faforever/client/notification/ServerNotificationController.java +++ b/src/main/java/com/faforever/client/notification/ServerNotificationController.java @@ -48,9 +48,9 @@ protected void onInitialize() { dialogLayout.setBody(serverNotificationRoot); } - public ServerNotificationController setNotification(ImmediateNotification notification) { + public void setNotification(ServerNotification notification) { StringWriter writer = new StringWriter(); - Throwable throwable = notification.getThrowable(); + Throwable throwable = notification.throwable(); if (throwable != null) { throwable.printStackTrace(new PrintWriter(writer)); exceptionTextArea.setVisible(true); @@ -59,22 +59,21 @@ public ServerNotificationController setNotification(ImmediateNotification notifi exceptionTextArea.setVisible(false); } - dialogLayout.setHeading(new Label(notification.getTitle())); - fxApplicationThreadExecutor.execute(() -> errorMessageView.getEngine().loadContent(notification.getText())); + dialogLayout.setHeading(new Label(notification.title())); + fxApplicationThreadExecutor.execute(() -> errorMessageView.getEngine().loadContent(notification.text())); - Optional.ofNullable(notification.getActions()) + Optional.ofNullable(notification.actions()) .map(actions -> actions.stream().map(this::createButton).collect(Collectors.toList())) .ifPresent(dialogLayout::setActions); - if (notification.getCustomUI() != null) { - serverNotificationRoot.getChildren().add(notification.getCustomUI()); + if (notification.customUI() != null) { + serverNotificationRoot.getChildren().add(notification.customUI()); } - return this; } private Button createButton(Action action) { Button button = new Button(action.getTitle()); button.setOnAction(event -> { - action.call(event); + action.run(); if (action.getType() == Action.Type.OK_DONE) { dismiss(); } @@ -99,9 +98,8 @@ public Region getRoot() { return serverNotificationRoot; } - public ServerNotificationController setCloseListener(Runnable closeListener) { + public void setCloseListener(Runnable closeListener) { this.closeListener = closeListener; - return this; } public DialogLayout getDialogLayout() { diff --git a/src/main/java/com/faforever/client/notification/Severity.java b/src/main/java/com/faforever/client/notification/Severity.java index da17feba93..971b8b76d9 100644 --- a/src/main/java/com/faforever/client/notification/Severity.java +++ b/src/main/java/com/faforever/client/notification/Severity.java @@ -13,7 +13,7 @@ public enum Severity { /** * Use this severity to inform the user about recoverable errors (something that went wrong but can be fixed) that - * requires his action to fix. + * requires his onAction to fix. */ WARN, diff --git a/src/main/java/com/faforever/client/notification/ToastDisplayer.java b/src/main/java/com/faforever/client/notification/ToastDisplayer.java new file mode 100644 index 0000000000..e6bba50df2 --- /dev/null +++ b/src/main/java/com/faforever/client/notification/ToastDisplayer.java @@ -0,0 +1,92 @@ +package com.faforever.client.notification; + +import com.faforever.client.fx.FxApplicationThreadExecutor; +import com.faforever.client.preferences.NotificationPrefs; +import com.faforever.client.preferences.ToastPosition; +import com.faforever.client.theme.UiService; +import com.faforever.client.ui.StageHolder; +import com.faforever.client.util.PopupUtil; +import javafx.beans.property.ObjectProperty; +import javafx.beans.value.ObservableValue; +import javafx.collections.ObservableList; +import javafx.stage.Popup; +import javafx.stage.PopupWindow.AnchorLocation; +import javafx.stage.Screen; +import lombok.RequiredArgsConstructor; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.stereotype.Component; + +@RequiredArgsConstructor +@Component +public class ToastDisplayer implements InitializingBean { + private final UiService uiService; + private final NotificationPrefs notificationPrefs; + private final FxApplicationThreadExecutor fxApplicationThreadExecutor; + + private TransientNotificationsController transientNotificationsController; + private Popup transientNotificationsPopup; + private ObservableValue anchorDetails; + + @Override + public void afterPropertiesSet() { + transientNotificationsController = uiService.loadFxml("theme/transient_notifications.fxml"); + transientNotificationsPopup = PopupUtil.createPopup(transientNotificationsController.getRoot()); + transientNotificationsPopup.getScene().getRoot().getStyleClass().add("transient-notification"); + + notificationPrefs.transientNotificationsEnabledProperty().subscribe(enabled -> { + if (enabled) { + fxApplicationThreadExecutor.execute(() -> transientNotificationsPopup.hide()); + } + }); + + ObjectProperty positionProperty = notificationPrefs.toastPositionProperty(); + anchorDetails = notificationPrefs.toastScreenProperty() + .map(this::findScreen) + .map(Screen::getVisualBounds) + .flatMap(bounds -> positionProperty.map( + position -> switch (notificationPrefs.getToastPosition()) { + case ToastPosition.BOTTOM_RIGHT -> + new AnchorDetails(AnchorLocation.CONTENT_BOTTOM_RIGHT, + bounds.getMaxX() - 1, bounds.getMaxY() - 1); + case ToastPosition.TOP_RIGHT -> + new AnchorDetails(AnchorLocation.CONTENT_TOP_RIGHT, bounds.getMaxX() - 1, + bounds.getMinY()); + case ToastPosition.BOTTOM_LEFT -> + new AnchorDetails(AnchorLocation.CONTENT_BOTTOM_LEFT, bounds.getMinX(), + bounds.getMaxY() - 1); + case ToastPosition.TOP_LEFT -> + new AnchorDetails(AnchorLocation.CONTENT_TOP_LEFT, bounds.getMinX(), + bounds.getMinY()); + })); + + anchorDetails.map(AnchorDetails::anchorLocation).subscribe(transientNotificationsPopup::setAnchorLocation); + transientNotificationsController.getRoot().getChildren().subscribe(this::updateToast); + } + + public void addNotification(TransientNotification notification) { + transientNotificationsController.addNotification(notification); + } + + private Screen findScreen(Number toastScreenIndex) { + ObservableList screens = Screen.getScreens(); + if (toastScreenIndex.intValue() < screens.size()) { + return screens.get(Math.max(0, toastScreenIndex.intValue())); + } else { + return Screen.getPrimary(); + } + } + + private void updateToast() { + boolean enabled = notificationPrefs.isTransientNotificationsEnabled(); + if (transientNotificationsController.getRoot().getChildren().isEmpty() || !enabled) { + transientNotificationsPopup.hide(); + return; + } + + AnchorDetails anchorDetails = this.anchorDetails.getValue(); + + transientNotificationsPopup.show(StageHolder.getStage(), anchorDetails.anchorX(), anchorDetails.anchorY()); + } + + private record AnchorDetails(AnchorLocation anchorLocation, double anchorX, double anchorY) {} +} diff --git a/src/main/java/com/faforever/client/notification/TransientNotification.java b/src/main/java/com/faforever/client/notification/TransientNotification.java index 6eb77b477a..e4822da87c 100644 --- a/src/main/java/com/faforever/client/notification/TransientNotification.java +++ b/src/main/java/com/faforever/client/notification/TransientNotification.java @@ -1,77 +1,27 @@ package com.faforever.client.notification; -import javafx.event.Event; import javafx.scene.image.Image; import org.jetbrains.annotations.NotNull; -import java.util.Objects; -import java.util.function.Consumer; - /** - * A notification that is displayed for a short amount of time, or until the user the user performs a suggested actionCallback - * or dismisses it. The notification consist of a text, an optional image, an optional actionCallback and is always - * rendered with a close button. The actionCallback is executed when the user clicks on the notification. + * A notification that is displayed for a short amount of time, or until the user the user performs a suggested + * onAction or dismisses it. The notification consist of a text, an optional image, an optional onAction and + * is always rendered with a close button. The onAction is executed when the user clicks on the notification. */ -public class TransientNotification implements Comparable { - - private final String title; - private final String text; - private final Image image; - private final Consumer callback; +public record TransientNotification( + String title, String text, Image image, Runnable onAction +) implements Comparable, Notification { public TransientNotification(String title, String text) { this(title, text, null, null); } - public TransientNotification(String title, String text, Image image, Consumer callback) { - this.title = title; - this.text = text; - this.image = image; - this.callback = callback; - } - public TransientNotification(String title, String text, Image image) { this(title, text, image, null); } - public String getTitle() { - return title; - } - - public String getText() { - return text; - } - - public Image getImage() { - return image; - } - - public Consumer getCallback() { - return callback; - } - @Override public int compareTo(@NotNull TransientNotification o) { return text.compareTo(o.text); } - - @Override - public int hashCode() { - return Objects.hash(title, text, image, callback); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - TransientNotification that = (TransientNotification) o; - return Objects.equals(title, that.title) && - Objects.equals(text, that.text) && - Objects.equals(image, that.image) && - Objects.equals(callback, that.callback); - } } diff --git a/src/main/java/com/faforever/client/notification/TransientNotificationController.java b/src/main/java/com/faforever/client/notification/TransientNotificationController.java index 8b5b73f42d..10485194bb 100644 --- a/src/main/java/com/faforever/client/notification/TransientNotificationController.java +++ b/src/main/java/com/faforever/client/notification/TransientNotificationController.java @@ -7,7 +7,6 @@ import javafx.animation.KeyValue; import javafx.animation.Timeline; import javafx.beans.value.ChangeListener; -import javafx.event.Event; import javafx.scene.Node; import javafx.scene.control.Label; import javafx.scene.image.ImageView; @@ -23,7 +22,6 @@ import org.springframework.stereotype.Component; import java.util.Objects; -import java.util.function.Consumer; import static javafx.util.Duration.millis; @@ -39,7 +37,7 @@ public class TransientNotificationController extends NodeController { public Label titleLabel; public ImageView imageView; private ChangeListener animationListener; - private Consumer action; + private Runnable action; private Timeline timeline; private int toastDisplayTime; @@ -93,10 +91,10 @@ private void dismiss() { } public void setNotification(TransientNotification notification) { - titleLabel.setText(notification.getTitle()); - messageLabel.setText(notification.getText()); - imageView.setImage(notification.getImage()); - action = notification.getCallback(); + titleLabel.setText(notification.title()); + messageLabel.setText(notification.text()); + imageView.setImage(notification.image()); + action = notification.onAction(); } public void onCloseButtonClicked() { @@ -112,7 +110,7 @@ public void onClicked(MouseEvent event) { if (event.getButton().equals(MouseButton.SECONDARY)) { dismiss(); } else if (action != null) { - action.accept(event); + action.run(); } } } diff --git a/src/main/java/com/faforever/client/notification/TransientNotificationsController.java b/src/main/java/com/faforever/client/notification/TransientNotificationsController.java index acde69eb74..a983178b0c 100644 --- a/src/main/java/com/faforever/client/notification/TransientNotificationsController.java +++ b/src/main/java/com/faforever/client/notification/TransientNotificationsController.java @@ -1,5 +1,6 @@ package com.faforever.client.notification; +import com.faforever.client.fx.FxApplicationThreadExecutor; import com.faforever.client.fx.NodeController; import com.faforever.client.preferences.NotificationPrefs; import com.faforever.client.preferences.ToastPosition; @@ -21,6 +22,7 @@ public class TransientNotificationsController extends NodeController { private final UiService uiService; private final NotificationPrefs notificationPrefs; + private final FxApplicationThreadExecutor fxApplicationThreadExecutor; public VBox transientNotificationsRoot; @@ -46,6 +48,6 @@ public void addNotification(TransientNotification notification) { TransientNotificationController controller = uiService.loadFxml("theme/transient_notification.fxml"); controller.setNotification(notification); Region controllerRoot = controller.getRoot(); - transientNotificationsRoot.getChildren().add(0, controllerRoot); + fxApplicationThreadExecutor.execute(() -> transientNotificationsRoot.getChildren().addFirst(controllerRoot)); } } diff --git a/src/main/java/com/faforever/client/player/FriendJoinedGameNotifier.java b/src/main/java/com/faforever/client/player/FriendJoinedGameNotifier.java index 5b32983bd4..fb953de804 100644 --- a/src/main/java/com/faforever/client/player/FriendJoinedGameNotifier.java +++ b/src/main/java/com/faforever/client/player/FriendJoinedGameNotifier.java @@ -34,8 +34,7 @@ public void onFriendJoinedGame(PlayerBean player, GameBean game) { notificationService.addNotification(new TransientNotification( i18n.get("friend.joinedGameNotification.title", player.getUsername(), game.getTitle()), i18n.get("friend.joinedGameNotification.action"), - IdenticonUtil.createIdenticon(player.getId()), - event -> gameRunner.join(game) + IdenticonUtil.createIdenticon(player.getId()), () -> gameRunner.join(game) )); } } diff --git a/src/main/java/com/faforever/client/player/FriendOnlineNotifier.java b/src/main/java/com/faforever/client/player/FriendOnlineNotifier.java index d5fdf35a66..e94dd93785 100644 --- a/src/main/java/com/faforever/client/player/FriendOnlineNotifier.java +++ b/src/main/java/com/faforever/client/player/FriendOnlineNotifier.java @@ -43,9 +43,7 @@ void onPlayerOnline(PlayerBean player) { i18n.get("friend.nowOnlineNotification.title", player.getUsername()), i18n.get("friend.nowOnlineNotification.action"), IdenticonUtil.createIdenticon(player.getId()), - actionEvent -> { - chatService.onInitiatePrivateChat(player.getUsername()); - } + () -> chatService.onInitiatePrivateChat(player.getUsername()) )); } } diff --git a/src/main/java/com/faforever/client/preferences/ui/SettingsController.java b/src/main/java/com/faforever/client/preferences/ui/SettingsController.java index 615eb27d4f..aa93e02f2f 100644 --- a/src/main/java/com/faforever/client/preferences/ui/SettingsController.java +++ b/src/main/java/com/faforever/client/preferences/ui/SettingsController.java @@ -246,8 +246,9 @@ private void onThemeChanged(Theme newValue) { themeService.setTheme(newValue); if (themeService.doesThemeNeedRestart(newValue)) { notificationService.addNotification(new PersistentNotification(i18n.get("theme.needsRestart.message", newValue.getDisplayName()), Severity.WARN, - Collections.singletonList(new Action(i18n.get("theme.needsRestart.quit"), event -> Platform.exit())))); - // FIXME reload application (stage & application context) https://github.com/FAForever/downlords-faf-client/issues/1794 + Collections.singletonList( + new Action(i18n.get("theme.needsRestart.quit"), + Platform::exit)))); } } @@ -533,14 +534,10 @@ void onLanguageSelected(Locale locale) { availableLanguagesListener.invalidated(i18n.getAvailableLanguages()); + // FIXME reload application (stage & application context) notificationService.addNotification(new PersistentNotification( i18n.get(locale, "settings.languages.restart.message"), - Severity.WARN, - Collections.singletonList(new Action(i18n.get(locale, "settings.languages.restart"), - event -> { - Platform.exit(); - // FIXME reload application (stage & application context) - }) + Severity.WARN, List.of(new Action(i18n.get(locale, "settings.languages.restart"), Platform::exit) ))); } diff --git a/src/main/java/com/faforever/client/remote/FafServerAccessor.java b/src/main/java/com/faforever/client/remote/FafServerAccessor.java index e564dd7912..707c133fe2 100644 --- a/src/main/java/com/faforever/client/remote/FafServerAccessor.java +++ b/src/main/java/com/faforever/client/remote/FafServerAccessor.java @@ -13,6 +13,7 @@ import com.faforever.client.notification.DismissAction; import com.faforever.client.notification.ImmediateNotification; import com.faforever.client.notification.NotificationService; +import com.faforever.client.notification.ServerNotification; import com.faforever.client.notification.Severity; import com.faforever.client.update.Version; import com.faforever.commons.lobby.ConnectionStatus; @@ -271,9 +272,9 @@ private void onNotice(NoticeInfo noticeMessage) { default -> Severity.INFO; }; } - notificationService.addServerNotification( - new ImmediateNotification(i18n.get("messageFromServer"), noticeMessage.getText(), severity, - Collections.singletonList(new DismissAction(i18n)))); + notificationService.addNotification( + new ServerNotification(i18n.get("messageFromServer"), noticeMessage.getText(), severity, + Collections.singletonList(new DismissAction(i18n)))); } public void restoreGameSession(int id) { diff --git a/src/main/java/com/faforever/client/replay/LiveReplayService.java b/src/main/java/com/faforever/client/replay/LiveReplayService.java index c1f8069f75..81351d0bb1 100644 --- a/src/main/java/com/faforever/client/replay/LiveReplayService.java +++ b/src/main/java/com/faforever/client/replay/LiveReplayService.java @@ -90,8 +90,7 @@ private void notifyUserWhenReplayAvailable(GameBean game) { clearTrackingLiveReplayProperty(); notificationService.addNotification(new PersistentNotification( i18n.get("vault.liveReplays.replayAvailable", game.getTitle()), - Severity.INFO, - List.of(new Action(i18n.get("game.watch"), (event) -> replayRunner.runWithLiveReplay(game))))); + Severity.INFO, List.of(new Action(i18n.get("game.watch"), () -> replayRunner.runWithLiveReplay(game))))); }, Instant.from(game.getStartTime().plusSeconds(getWatchDelaySeconds()))); } diff --git a/src/main/java/com/faforever/client/replay/ReplayCardController.java b/src/main/java/com/faforever/client/replay/ReplayCardController.java index 009a82fde5..982e6f1ac2 100644 --- a/src/main/java/com/faforever/client/replay/ReplayCardController.java +++ b/src/main/java/com/faforever/client/replay/ReplayCardController.java @@ -179,7 +179,12 @@ public void onWatchButtonClicked() { public void onDeleteButtonClicked() { notificationService.addNotification(new ImmediateNotification(i18n.get("replay.deleteNotification.heading", entity.get() - .getTitle()), i18n.get("replay.deleteNotification.info"), Severity.INFO, Arrays.asList(new Action(i18n.get("cancel")), new Action(i18n.get("delete"), event -> deleteReplay())))); + .getTitle()), + i18n.get("replay.deleteNotification.info"), + Severity.INFO, + Arrays.asList(new Action(i18n.get("cancel")), + new Action(i18n.get("delete"), + this::deleteReplay)))); } private void deleteReplay() { diff --git a/src/main/java/com/faforever/client/replay/ReplayDetailController.java b/src/main/java/com/faforever/client/replay/ReplayDetailController.java index c6e6483bb9..133a706516 100644 --- a/src/main/java/com/faforever/client/replay/ReplayDetailController.java +++ b/src/main/java/com/faforever/client/replay/ReplayDetailController.java @@ -505,7 +505,7 @@ public void onDeleteButtonClicked() { new ImmediateNotification(i18n.get("replay.deleteNotification.heading", replay.get().getTitle()), i18n.get("replay.deleteNotification.info"), Severity.INFO, Arrays.asList(new Action(i18n.get("cancel")), - new Action(i18n.get("delete"), event -> deleteReplay())))); + new Action(i18n.get("delete"), this::deleteReplay)))); } public void setOnDeleteListener(Runnable onDeleteListener) { diff --git a/src/main/java/com/faforever/client/replay/ReplayRunner.java b/src/main/java/com/faforever/client/replay/ReplayRunner.java index f9253a3de2..1534b141c5 100644 --- a/src/main/java/com/faforever/client/replay/ReplayRunner.java +++ b/src/main/java/com/faforever/client/replay/ReplayRunner.java @@ -78,8 +78,7 @@ public void afterPropertiesSet() { @VisibleForTesting CompletableFuture downloadMapAskIfError(String mapFolderName) { - return mapService.downloadIfNecessary(mapFolderName) - .exceptionallyCompose(throwable -> shouldStartWithOutMap(throwable)); + return mapService.downloadIfNecessary(mapFolderName).exceptionallyCompose(this::shouldStartWithOutMap); } /** @@ -117,10 +116,10 @@ private CompletableFuture shouldStartWithOutMap(Throwable throwable) { CountDownLatch userAnswered = new CountDownLatch(1); AtomicReference shouldStart = new AtomicReference<>(false); - List actions = Arrays.asList(new Action(i18n.get("replay.ignoreMapNotFound"), event -> { + List actions = Arrays.asList(new Action(i18n.get("replay.ignoreMapNotFound"), () -> { shouldStart.set(true); userAnswered.countDown(); - }), new Action(i18n.get("replay.abortAfterMapNotFound"), event -> userAnswered.countDown())); + }), new Action(i18n.get("replay.abortAfterMapNotFound"), userAnswered::countDown)); notificationService.addNotification(new ImmediateNotification(i18n.get("replay.mapDownloadFailed"), i18n.get("replay.mapDownloadFailed.wannaContinue"), diff --git a/src/main/java/com/faforever/client/replay/ReplayServer.java b/src/main/java/com/faforever/client/replay/ReplayServer.java index 01a802d577..51cea98ab7 100644 --- a/src/main/java/com/faforever/client/replay/ReplayServer.java +++ b/src/main/java/com/faforever/client/replay/ReplayServer.java @@ -119,8 +119,8 @@ public CompletableFuture start(int gameId) { future.completeExceptionally(e); log.warn("Error in replay server", e); notificationService.addNotification(new PersistentNotification( - i18n.get("replayServer.listeningFailed"), - Severity.WARN, Collections.singletonList(new Action(i18n.get("replayServer.retry"), event -> start(gameId))) + i18n.get("replayServer.listeningFailed"), Severity.WARN, + Collections.singletonList(new Action(i18n.get("replayServer.retry"), () -> start(gameId))) )); } }).start(); diff --git a/src/main/java/com/faforever/client/replay/ReplayService.java b/src/main/java/com/faforever/client/replay/ReplayService.java index 4da9d93121..f009134625 100644 --- a/src/main/java/com/faforever/client/replay/ReplayService.java +++ b/src/main/java/com/faforever/client/replay/ReplayService.java @@ -240,7 +240,11 @@ private void moveCorruptedReplayFile(Path replayFile) { return; } - notificationService.addNotification(new PersistentNotification(i18n.get("corruptedReplayFiles.notification"), WARN, singletonList(new Action(i18n.get("corruptedReplayFiles.show"), event -> platformService.reveal(replayFile))))); + notificationService.addNotification(new PersistentNotification(i18n.get("corruptedReplayFiles.notification"), WARN, + singletonList( + new Action(i18n.get("corruptedReplayFiles.show"), + () -> platformService.reveal( + replayFile))))); } public boolean deleteReplayFile(Path replayFile) { diff --git a/src/main/java/com/faforever/client/teammatchmaking/TeamMatchmakingService.java b/src/main/java/com/faforever/client/teammatchmaking/TeamMatchmakingService.java index 07efd1c778..7bd7700c7c 100644 --- a/src/main/java/com/faforever/client/teammatchmaking/TeamMatchmakingService.java +++ b/src/main/java/com/faforever/client/teammatchmaking/TeamMatchmakingService.java @@ -66,7 +66,6 @@ import javafx.collections.ObservableMap; import javafx.collections.ObservableSet; import javafx.collections.transformation.FilteredList; -import javafx.event.Event; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -84,7 +83,6 @@ import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Consumer; import java.util.stream.Collectors; import static com.faforever.client.chat.ChatService.PARTY_CHANNEL_SUFFIX; @@ -319,7 +317,7 @@ private void onValidQueueChange(Change change) { } private void sendInviteNotifications(PlayerBean player) { - Consumer callback = event -> this.acceptPartyInvite(player); + Runnable callback = () -> this.acceptPartyInvite(player); notificationService.addNotification(new TransientNotification(i18n.get("teammatchmaking.notification.invite.title"), i18n.get( diff --git a/src/main/java/com/faforever/client/theme/ThemeService.java b/src/main/java/com/faforever/client/theme/ThemeService.java index 6e5989ecba..b245576f27 100644 --- a/src/main/java/com/faforever/client/theme/ThemeService.java +++ b/src/main/java/com/faforever/client/theme/ThemeService.java @@ -31,7 +31,6 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; -import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Lazy; import org.springframework.core.io.ClassPathResource; import org.springframework.stereotype.Service; @@ -112,7 +111,6 @@ public class ThemeService implements InitializingBean, DisposableBean { private static final String METADATA_FILE_NAME = "theme.properties"; private final ExecutorService executorService; - private final ApplicationContext applicationContext; private final DataPrefs dataPrefs; private final Preferences preferences; private final FxApplicationThreadExecutor fxApplicationThreadExecutor; diff --git a/src/main/java/com/faforever/client/ui/alert/Alert.java b/src/main/java/com/faforever/client/ui/alert/Alert.java index 4e672f9662..020cfab7dd 100644 --- a/src/main/java/com/faforever/client/ui/alert/Alert.java +++ b/src/main/java/com/faforever/client/ui/alert/Alert.java @@ -1,7 +1,5 @@ package com.faforever.client.ui.alert; -import com.faforever.client.fx.FxApplicationThreadExecutor; -import com.faforever.client.fx.SimpleInvalidationListener; import com.faforever.client.ui.alert.animation.AlertAnimation; import com.faforever.client.ui.effects.DepthManager; import com.sun.javafx.event.EventHandlerManager; @@ -16,6 +14,7 @@ import javafx.geometry.Insets; import javafx.geometry.VPos; import javafx.scene.Node; +import javafx.scene.Scene; import javafx.scene.control.ButtonType; import javafx.scene.control.Dialog; import javafx.scene.control.DialogEvent; @@ -27,14 +26,15 @@ import javafx.scene.paint.Color; import javafx.stage.StageStyle; import javafx.stage.Window; +import javafx.util.Subscription; +import java.util.ArrayList; import java.util.List; /** Ported from JFoenix since we wanted to get rid of the JFoenix dependency */ public class Alert extends Dialog { - private final StackPane contentContainer; - private final FxApplicationThreadExecutor fxApplicationThreadExecutor; + private final StackPane contentContainer = new StackPane(); private final EventHandlerManager eventHandlerManager = new EventHandlerManager(this); @@ -51,17 +51,12 @@ public class Alert extends Dialog { (AlertAnimation.CENTER_ANIMATION); private final BooleanProperty hideOnEscape = new SimpleBooleanProperty(this, "hideOnEscape", true); - private final SimpleInvalidationListener widthListener = this::updateWidth; - private final SimpleInvalidationListener heightListener = this::updateHeight; - private final SimpleInvalidationListener xListener = this::updateX; - private final SimpleInvalidationListener yListener = this::updateY; + private final List layoutSubscriptions = new ArrayList<>(); private boolean animateClosing = true; private Animation transition = null; - public Alert(Window window, FxApplicationThreadExecutor fxApplicationThreadExecutor) { - this.fxApplicationThreadExecutor = fxApplicationThreadExecutor; - contentContainer = new StackPane(); + public Alert(Window window) { contentContainer.getStyleClass().add("alert-content-container"); // add depth effect final Node materialNode = DepthManager.createMaterialNode(contentContainer, 2); @@ -69,71 +64,7 @@ public Alert(Window window, FxApplicationThreadExecutor fxApplicationThreadExecu materialNode.addEventHandler(MouseEvent.MOUSE_CLICKED, Event::consume); // create custom dialog pane (will layout children in center) - final DialogPane dialogPane = new DialogPane() { - private boolean performingLayout = false; - - { - getButtonTypes().add(ButtonType.CLOSE); - Node closeButton = this.lookupButton(ButtonType.CLOSE); - closeButton.managedProperty().bind(closeButton.visibleProperty()); - closeButton.setVisible(false); - } - - @Override - protected double computePrefHeight(double width) { - Window owner = getOwner(); - if (owner != null) { - return owner.getHeight(); - } - return super.computePrefHeight(width); - } - - @Override - protected double computePrefWidth(double height) { - Window owner = getOwner(); - if (owner != null) { - return owner.getWidth(); - } - return super.computePrefWidth(height); - } - - @Override - public void requestLayout() { - if (performingLayout) { - return; - } - super.requestLayout(); - } - - @Override - protected void layoutChildren() { - performingLayout = true; - List managed = getManagedChildren(); - final double width = getWidth(); - double height = getHeight(); - double top = getInsets().getTop(); - double right = getInsets().getRight(); - double left = getInsets().getLeft(); - double bottom = getInsets().getBottom(); - double contentWidth = width - left - right; - double contentHeight = height - top - bottom; - for (Node child : managed) { - layoutInArea(child, left, top, contentWidth, contentHeight, - 0, Insets.EMPTY, HPos.CENTER, VPos.CENTER); - } - performingLayout = false; - } - - @Override - public String getUserAgentStylesheet() { - return getClass().getResource("/css/controls/alert.css").toExternalForm(); - } - - @Override - protected Node createButtonBar() { - return null; - } - }; + final DialogPane dialogPane = new AlertDialogPane(); dialogPane.getStyleClass().add("alert-overlay"); dialogPane.setContent(materialNode); setDialogPane(dialogPane); @@ -152,6 +83,18 @@ protected Node createButtonBar() { }); } + addEventHandlers(dialogPane); + + getDialogPane().getScene().getWindow().addEventFilter(KeyEvent.KEY_PRESSED, keyEvent -> { + if (keyEvent.getCode() == KeyCode.ESCAPE) { + if (!isHideOnEscape()) { + keyEvent.consume(); + } + } + }); + } + + private void addEventHandlers(DialogPane dialogPane) { // handle animation / owner window layout changes eventHandlerManager.addEventHandler(DialogEvent.DIALOG_SHOWING, event -> { addLayoutListeners(); @@ -177,14 +120,6 @@ protected Node createButtonBar() { } }); eventHandlerManager.addEventHandler(DialogEvent.DIALOG_HIDDEN, event -> removeLayoutListeners()); - - getDialogPane().getScene().getWindow().addEventFilter(KeyEvent.KEY_PRESSED, keyEvent -> { - if (keyEvent.getCode() == KeyCode.ESCAPE) { - if (!isHideOnEscape()) { - keyEvent.consume(); - } - } - }); } // this method ensure not null value for current animation @@ -195,25 +130,18 @@ private AlertAnimation getCurrentAnimation() { } private void removeLayoutListeners() { - Window stage = getOwner(); - if (stage != null) { - stage.getScene().widthProperty().removeListener(widthListener); - stage.getScene().heightProperty().removeListener(heightListener); - stage.xProperty().removeListener(xListener); - stage.yProperty().removeListener(yListener); - } + layoutSubscriptions.forEach(Subscription::unsubscribe); + layoutSubscriptions.clear(); } private void addLayoutListeners() { Window stage = getOwner(); if (stage != null) { - if (widthListener == null) { - throw new RuntimeException("Owner can only be set using the constructor"); - } - stage.getScene().widthProperty().addListener(widthListener); - stage.getScene().heightProperty().addListener(heightListener); - stage.xProperty().addListener(xListener); - stage.yProperty().addListener(yListener); + Scene scene = stage.getScene(); + layoutSubscriptions.add(scene.widthProperty().subscribe(this::updateWidth)); + layoutSubscriptions.add(scene.heightProperty().subscribe(this::updateHeight)); + layoutSubscriptions.add(stage.xProperty().subscribe(this::updateX)); + layoutSubscriptions.add(stage.yProperty().subscribe(this::updateY)); } } @@ -262,7 +190,7 @@ public void hideWithAnimation() { } else { animateClosing = false; transition = null; - fxApplicationThreadExecutor.execute(this::hide); + hide(); } } } @@ -316,4 +244,68 @@ public final BooleanProperty hideOnEscapeProperty() { return hideOnEscape; } + private class AlertDialogPane extends DialogPane { + private boolean performingLayout = false; + + public AlertDialogPane() { + getButtonTypes().add(ButtonType.CLOSE); + Node closeButton = this.lookupButton(ButtonType.CLOSE); + closeButton.managedProperty().bind(closeButton.visibleProperty()); + closeButton.setVisible(false); + } + + @Override + protected double computePrefHeight(double width) { + Window owner = getOwner(); + if (owner != null) { + return owner.getHeight(); + } + return super.computePrefHeight(width); + } + + @Override + protected double computePrefWidth(double height) { + Window owner = getOwner(); + if (owner != null) { + return owner.getWidth(); + } + return super.computePrefWidth(height); + } + + @Override + public void requestLayout() { + if (performingLayout) { + return; + } + super.requestLayout(); + } + + @Override + protected void layoutChildren() { + performingLayout = true; + List managed = getManagedChildren(); + final double width = getWidth(); + double height = getHeight(); + double top = getInsets().getTop(); + double right = getInsets().getRight(); + double left = getInsets().getLeft(); + double bottom = getInsets().getBottom(); + double contentWidth = width - left - right; + double contentHeight = height - top - bottom; + for (Node child : managed) { + layoutInArea(child, left, top, contentWidth, contentHeight, 0, Insets.EMPTY, HPos.CENTER, VPos.CENTER); + } + performingLayout = false; + } + + @Override + public String getUserAgentStylesheet() { + return getClass().getResource("/css/controls/alert.css").toExternalForm(); + } + + @Override + protected Node createButtonBar() { + return null; + } + } } diff --git a/src/main/java/com/faforever/client/update/ClientUpdateService.java b/src/main/java/com/faforever/client/update/ClientUpdateService.java index 2141c22a6c..877ca0ad1f 100644 --- a/src/main/java/com/faforever/client/update/ClientUpdateService.java +++ b/src/main/java/com/faforever/client/update/ClientUpdateService.java @@ -111,11 +111,12 @@ private void notificationOnUpdate(CompletableFuture updateInfoSuppli List actions = new ArrayList<>(); if (operatingSystem.supportsUpdateInstall()) { - actions.add(new Action(i18n.get("clientUpdateAvailable.downloadAndInstall"), event -> downloadAndInstallInBackground(updateInfo))); + actions.add(new Action(i18n.get("clientUpdateAvailable.downloadAndInstall"), + () -> downloadAndInstallInBackground(updateInfo))); } actions.add(new Action(i18n.get("clientUpdateAvailable.releaseNotes"), Action.Type.OK_STAY, - event -> platformService.showDocument(updateInfo.releaseNotesUrl().toExternalForm()) + () -> platformService.showDocument(updateInfo.releaseNotesUrl().toExternalForm()) )); notificationService.addNotification(new PersistentNotification( @@ -156,7 +157,8 @@ public DownloadUpdateTask downloadAndInstallInBackground(UpdateInfo updateInfo) log.error("Error while downloading client update", throwable); notificationService.addNotification( new PersistentNotification(i18n.get("clientUpdateDownloadFailed.notification"), WARN, singletonList( - new Action(i18n.get("clientUpdateDownloadFailed.retry"), event -> downloadAndInstallInBackground(updateInfo)) + new Action(i18n.get("clientUpdateDownloadFailed.retry"), + () -> downloadAndInstallInBackground(updateInfo)) )) ); return null; diff --git a/src/test/java/com/faforever/client/chat/KittehChatServiceTest.java b/src/test/java/com/faforever/client/chat/KittehChatServiceTest.java index 0432ff9255..09ec34249d 100644 --- a/src/test/java/com/faforever/client/chat/KittehChatServiceTest.java +++ b/src/test/java/com/faforever/client/chat/KittehChatServiceTest.java @@ -500,7 +500,7 @@ public void testChatMessageEventTriggeredByChannelMessage() throws Exception { public void testChatMessageEventTriggeredByChannelAction() throws Exception { CompletableFuture chatMessageFuture = new CompletableFuture<>(); - String message = "chat action"; + String message = "chat onAction"; connect(); @@ -692,7 +692,7 @@ public void testAddChannelUserListListener() { public void testSendActionInBackground() throws Exception { connect(); - String action = "test action"; + String action = "test onAction"; CompletableFuture future = new CompletableFuture<>(); ChatChannel chatChannel = new ChatChannel(DEFAULT_CHANNEL_NAME); diff --git a/src/test/java/com/faforever/client/fa/GameFullNotifierTest.java b/src/test/java/com/faforever/client/fa/GameFullNotifierTest.java index 19eea473cc..5880c42f0a 100644 --- a/src/test/java/com/faforever/client/fa/GameFullNotifierTest.java +++ b/src/test/java/com/faforever/client/fa/GameFullNotifierTest.java @@ -10,7 +10,6 @@ import com.faforever.client.notification.NotificationService; import com.faforever.client.notification.TransientNotification; import com.faforever.client.test.ServiceTest; -import javafx.event.Event; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -78,7 +77,7 @@ public void testFocusToFaWindowFromNonFaWindow() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(TransientNotification.class); verify(notificationService).addNotification(argumentCaptor.capture()); - argumentCaptor.getValue().getCallback().accept(any(Event.class)); + argumentCaptor.getValue().onAction().run(); verify(platformService).focusWindow(clientProperties.getForgedAlliance().getWindowTitle(), 1L); verify(platformService, never()).minimizeFocusedWindow(); } @@ -95,7 +94,7 @@ public void testFocusToFaWindowFromAnotherFaWindow() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(TransientNotification.class); verify(notificationService).addNotification(argumentCaptor.capture()); - argumentCaptor.getValue().getCallback().accept(any(Event.class)); + argumentCaptor.getValue().onAction().run(); verify(platformService).minimizeFocusedWindow(); verify(platformService).focusWindow(clientProperties.getForgedAlliance().getWindowTitle(), 1L); } diff --git a/src/test/java/com/faforever/client/game/GameRunnerTest.java b/src/test/java/com/faforever/client/game/GameRunnerTest.java index d000a95369..331932cb06 100644 --- a/src/test/java/com/faforever/client/game/GameRunnerTest.java +++ b/src/test/java/com/faforever/client/game/GameRunnerTest.java @@ -499,7 +499,7 @@ public void testJoinGameRatingTooLow() throws Exception { ImmediateNotification value = captor.getValue(); - value.getActions().getFirst().call(null); + value.actions().getFirst().run(); verify(fafServerAccessor).requestJoinGame(anyInt(), any()); } @@ -520,7 +520,7 @@ public void testJoinGameRatingTooHigh() throws Exception { ImmediateNotification value = captor.getValue(); - value.getActions().getFirst().call(null); + value.actions().getFirst().run(); verify(fafServerAccessor).requestJoinGame(anyInt(), any()); } diff --git a/src/test/java/com/faforever/client/notification/NotificationServiceTest.java b/src/test/java/com/faforever/client/notification/NotificationServiceTest.java index fd9b68c399..44014bdeb7 100644 --- a/src/test/java/com/faforever/client/notification/NotificationServiceTest.java +++ b/src/test/java/com/faforever/client/notification/NotificationServiceTest.java @@ -1,10 +1,10 @@ package com.faforever.client.notification; +import com.faforever.client.fx.FxApplicationThreadExecutor; import com.faforever.client.i18n.I18n; import com.faforever.client.reporting.ReportingService; import com.faforever.client.test.ServiceTest; -import javafx.collections.SetChangeListener; -import javafx.collections.SetChangeListener.Change; +import com.faforever.client.theme.UiService; import org.junit.jupiter.api.Test; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -16,6 +16,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class NotificationServiceTest extends ServiceTest { @@ -26,61 +27,42 @@ public class NotificationServiceTest extends ServiceTest { private ReportingService reportingService; @Mock private I18n i18n; + @Mock + private ToastDisplayer toastDisplayer; + @Mock + private UiService uiService; + @Mock + private FxApplicationThreadExecutor fxApplicationThreadExecutor; @Test - public void testAddNotificationPersistent() throws Exception { - instance.addNotification(new PersistentNotification("text", Severity.INFO)); - } - - @Test - public void testAddNotificationImmediate() throws Exception { - instance.addNotification(new ImmediateNotification("title", "text", Severity.INFO)); - } - - @Test - public void testAddNotificationTransient() throws Exception { + public void testAddTransientNotification() throws Exception { instance.addNotification(new TransientNotification("title", "text")); - } - - @Test - @SuppressWarnings("unchecked") - public void testAddPersistentNotificationListener() throws Exception { - SetChangeListener listener = mock(SetChangeListener.class); - - instance.addPersistentNotificationListener(listener); - instance.addNotification(mock(PersistentNotification.class)); - verify(listener).onChanged(any(Change.class)); + verify(toastDisplayer).addNotification(any()); } @Test - @SuppressWarnings("unchecked") - public void testAddTransientNotificationListener() throws Exception { - OnTransientNotificationListener listener = mock(OnTransientNotificationListener.class); + public void testAddImmediateNotification() throws Exception { + ImmediateNotificationController notificationController = mock(ImmediateNotificationController.class); + when(uiService.loadFxml("theme/immediate_notification.fxml")).thenReturn(notificationController); - instance.addTransientNotificationListener(listener); + instance.addNotification(new ImmediateNotification("", "", Severity.INFO)); - TransientNotification notification = mock(TransientNotification.class); - instance.addNotification(notification); - - verify(listener).onTransientNotification(notification); + verify(notificationController).setNotification(any()); } @Test - @SuppressWarnings("unchecked") - public void testAddImmediateNotificationListener() throws Exception { - OnImmediateNotificationListener listener = mock(OnImmediateNotificationListener.class); + public void testAddServerNotification() throws Exception { + ServerNotificationController notificationController = mock(ServerNotificationController.class); + when(uiService.loadFxml("theme/server_notification.fxml")).thenReturn(notificationController); - instance.addImmediateNotificationListener(listener); - - ImmediateNotification notification = mock(ImmediateNotification.class); - instance.addNotification(notification); + instance.addNotification(new ServerNotification("", "", Severity.INFO)); - verify(listener).onImmediateNotification(notification); + verify(notificationController).setNotification(any()); } @Test - public void testGetPersistentNotifications() throws Exception { + public void testPersistentNotification() throws Exception { assertThat(instance.getPersistentNotifications(), empty()); PersistentNotification notification = mock(PersistentNotification.class); diff --git a/src/test/java/com/faforever/client/notification/PersistentNotificationsControllerTest.java b/src/test/java/com/faforever/client/notification/PersistentNotificationsControllerTest.java index 2404a63e22..c20d5744c4 100644 --- a/src/test/java/com/faforever/client/notification/PersistentNotificationsControllerTest.java +++ b/src/test/java/com/faforever/client/notification/PersistentNotificationsControllerTest.java @@ -3,12 +3,11 @@ import com.faforever.client.audio.AudioService; import com.faforever.client.test.PlatformTest; import com.faforever.client.theme.UiService; -import javafx.collections.SetChangeListener; -import javafx.collections.SetChangeListener.Change; +import javafx.collections.FXCollections; +import javafx.collections.ObservableSet; import javafx.scene.layout.Pane; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -16,7 +15,6 @@ import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -34,20 +32,18 @@ public class PersistentNotificationsControllerTest extends PlatformTest { @Mock private UiService uiService; + private ObservableSet persistentNotifications; + @BeforeEach public void setUp() throws Exception { + persistentNotifications = FXCollections.observableSet(); + when(notificationService.getPersistentNotifications()).thenReturn(persistentNotifications); + instance.persistentNotificationsRoot = new Pane(); loadFxml("theme/persistent_notifications.fxml", clazz -> instance); } - @Test - @SuppressWarnings("unchecked") - public void testPostConstruct() throws Exception { - verify(notificationService).getPersistentNotifications(); - verify(notificationService).addPersistentNotificationListener(any(SetChangeListener.class)); - } - @Test public void testGetRoot() throws Exception { assertEquals(instance.persistentNotificationsRoot, instance.getRoot()); @@ -72,19 +68,9 @@ private void onNotificationAdded(Severity severity) { when(uiService.loadFxml("theme/persistent_notification.fxml")).thenReturn(notificationController); - ArgumentCaptor argument = ArgumentCaptor.forClass(SetChangeListener.class); - verify(notificationService).addPersistentNotificationListener(argument.capture()); - - SetChangeListener listener = argument.getValue(); - - PersistentNotification notification = mock(PersistentNotification.class); - when(notification.getSeverity()).thenReturn(severity); - - Change change = mock(Change.class); - when(change.wasAdded()).thenReturn(true); - when(change.getElementAdded()).thenReturn(notification); + PersistentNotification notification = new PersistentNotification("", severity); - listener.onChanged(change); + persistentNotifications.add(notification); verify(notificationController).setNotification(notification); } diff --git a/src/test/java/com/faforever/client/notification/ServerNotificationControllerTest.java b/src/test/java/com/faforever/client/notification/ServerNotificationControllerTest.java index 5eb9034f8e..09b451e37e 100644 --- a/src/test/java/com/faforever/client/notification/ServerNotificationControllerTest.java +++ b/src/test/java/com/faforever/client/notification/ServerNotificationControllerTest.java @@ -11,7 +11,7 @@ import org.mockito.Mock; import org.testfx.util.WaitForAsyncUtils; -import java.util.Collections; +import java.util.List; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; @@ -35,7 +35,7 @@ public void setUp() throws Exception { @Test public void testSetNotificationWithoutActions() { - ImmediateNotification notification = new ImmediateNotification("title", "text", Severity.INFO); + ServerNotification notification = new ServerNotification("title", "text", Severity.INFO); instance.setNotification(notification); WaitForAsyncUtils.waitForFxEvents(); @@ -47,10 +47,8 @@ public void testSetNotificationWithoutActions() { @Test public void testSetNotificationWithActions() { - ImmediateNotification notification = new ImmediateNotification("title", "text", Severity.INFO, - Collections.singletonList( - new Action("actionTitle") - )); + ServerNotification notification = new ServerNotification("title", "text", Severity.INFO, + List.of(new Action("actionTitle"))); instance.setNotification(notification); WaitForAsyncUtils.waitForFxEvents(); diff --git a/src/test/java/com/faforever/client/notification/TransientNotificationControllerTest.java b/src/test/java/com/faforever/client/notification/TransientNotificationControllerTest.java index 701a040169..43aa149db7 100644 --- a/src/test/java/com/faforever/client/notification/TransientNotificationControllerTest.java +++ b/src/test/java/com/faforever/client/notification/TransientNotificationControllerTest.java @@ -2,7 +2,6 @@ import com.faforever.client.preferences.NotificationPrefs; import com.faforever.client.test.PlatformTest; -import javafx.event.Event; import javafx.scene.image.Image; import javafx.scene.input.MouseButton; import javafx.scene.input.MouseEvent; @@ -11,8 +10,6 @@ import org.mockito.InjectMocks; import org.mockito.Spy; -import java.util.function.Consumer; - import static com.faforever.client.fx.MouseEvents.generateClick; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; @@ -59,14 +56,15 @@ public void testOnRightClick() throws Exception { assertEquals("timeline has not been set", assertThrows(Exception.class, () -> instance.onClicked(generateClick(MouseButton.SECONDARY, 1))).getMessage()); } + @SuppressWarnings("unchecked") @Test public void testOnLeftClick() throws Exception { TransientNotification notificationMock = mock(TransientNotification.class); - Consumer actionMock = mock(Consumer.class); - when(notificationMock.getCallback()).thenReturn(actionMock); + Runnable actionMock = mock(Runnable.class); + when(notificationMock.onAction()).thenReturn(actionMock); instance.setNotification(notificationMock); MouseEvent mouseEvent = generateClick(MouseButton.PRIMARY, 1); instance.onClicked(mouseEvent); - verify(actionMock).accept(mouseEvent); + verify(actionMock).run(); } } diff --git a/src/test/java/com/faforever/client/notification/TransientNotificationsControllerTest.java b/src/test/java/com/faforever/client/notification/TransientNotificationsControllerTest.java index 3d148649f2..d0daed380f 100644 --- a/src/test/java/com/faforever/client/notification/TransientNotificationsControllerTest.java +++ b/src/test/java/com/faforever/client/notification/TransientNotificationsControllerTest.java @@ -76,10 +76,11 @@ public void testAddNotification() throws Exception { when(uiService.loadFxml("theme/transient_notification.fxml")).thenReturn(controller); TransientNotification notification1 = new TransientNotification("title1", "text1"); - instance.addNotification(notification1); + runOnFxThreadAndWait(() -> instance.addNotification(notification1)); TransientNotification notification2 = new TransientNotification("title2", "text2"); - instance.addNotification(notification2); + runOnFxThreadAndWait(() -> instance.addNotification(notification2)); + assertThat(instance.transientNotificationsRoot.getChildren(), hasSize(2)); verify(controller).setNotification(notification1); diff --git a/src/test/java/com/faforever/client/player/FriendJoinedGameNotifierTest.java b/src/test/java/com/faforever/client/player/FriendJoinedGameNotifierTest.java index a50dce7468..c5aea29994 100644 --- a/src/test/java/com/faforever/client/player/FriendJoinedGameNotifierTest.java +++ b/src/test/java/com/faforever/client/player/FriendJoinedGameNotifierTest.java @@ -54,9 +54,9 @@ public void onFriendJoinedGame() throws Exception { verify(notificationService).addNotification(captor.capture()); TransientNotification notification = captor.getValue(); - assertThat(notification.getTitle(), is("junit joined My Game")); - assertThat(notification.getText(), is("Click to join")); - assertThat(notification.getImage(), notNullValue()); + assertThat(notification.title(), is("junit joined My Game")); + assertThat(notification.text(), is("Click to join")); + assertThat(notification.image(), notNullValue()); } @Test diff --git a/src/test/java/com/faforever/client/remote/ServerAccessorTest.java b/src/test/java/com/faforever/client/remote/ServerAccessorTest.java index 558a40b1d7..2e20334041 100644 --- a/src/test/java/com/faforever/client/remote/ServerAccessorTest.java +++ b/src/test/java/com/faforever/client/remote/ServerAccessorTest.java @@ -14,8 +14,8 @@ import com.faforever.client.game.NewGameInfo; import com.faforever.client.i18n.I18n; import com.faforever.client.io.UidService; -import com.faforever.client.notification.ImmediateNotification; import com.faforever.client.notification.NotificationService; +import com.faforever.client.notification.ServerNotification; import com.faforever.client.notification.Severity; import com.faforever.client.test.ServiceTest; import com.faforever.client.update.Version; @@ -319,13 +319,13 @@ public void testOnNotice() throws Exception { sendFromServer(noticeMessage); - ArgumentCaptor captor = ArgumentCaptor.forClass(ImmediateNotification.class); - verify(notificationService, timeout(1000)).addServerNotification(captor.capture()); + ArgumentCaptor captor = ArgumentCaptor.forClass(ServerNotification.class); + verify(notificationService, timeout(1000)).addNotification(captor.capture()); - ImmediateNotification notification = captor.getValue(); - assertThat(notification.getSeverity(), is(Severity.WARN)); - assertThat(notification.getText(), is("foo bar")); - assertThat(notification.getTitle(), is("Message from Server")); + ServerNotification notification = captor.getValue(); + assertThat(notification.severity(), is(Severity.WARN)); + assertThat(notification.text(), is("foo bar")); + assertThat(notification.title(), is("Message from Server")); verify(i18n).get("messageFromServer"); } @@ -462,9 +462,7 @@ public void testClosePlayersGame() { instance.closePlayersGame(1); assertMessageContainsComponents( - "admin", - "user_id", - "action", + "admin", "user_id", "action", String.valueOf(1)); } @@ -474,9 +472,7 @@ public void testClosePlayersLobby() { instance.closePlayersLobby(1); assertMessageContainsComponents( - "admin", - "user_id", - "action", + "admin", "user_id", "action", String.valueOf(1)); } @@ -486,9 +482,7 @@ public void testBroadcastMessage() { instance.broadcastMessage("Test"); assertMessageContainsComponents( - "admin", - "message", - "action", + "admin", "message", "action", "Test"); } @@ -498,9 +492,7 @@ public void testGetAvailableAvatars() throws Exception { instance.getAvailableAvatars(); assertMessageContainsComponents( - "avatar", - "list_avatar", - "action" + "avatar", "list_avatar", "action" ); AvatarListInfo avatarList = new AvatarListInfo( @@ -613,9 +605,7 @@ public void testSelectAvatar() throws MalformedURLException { instance.selectAvatar(url); - assertMessageContainsComponents( - "avatar", - "action", + assertMessageContainsComponents("avatar", "action", url.toString() ); } diff --git a/src/test/java/com/faforever/client/replay/ReplayRunnerTest.java b/src/test/java/com/faforever/client/replay/ReplayRunnerTest.java index 64bca450db..a5ccccaf1f 100644 --- a/src/test/java/com/faforever/client/replay/ReplayRunnerTest.java +++ b/src/test/java/com/faforever/client/replay/ReplayRunnerTest.java @@ -105,7 +105,7 @@ public void testDownloadMapWithErrorIgnore() { verify(notificationService).addNotification(captor.capture()); ImmediateNotification notification = captor.getValue(); - notification.getActions().getFirst().call(null); + notification.actions().getFirst().run(); assertDoesNotThrow(() -> future.get(1, TimeUnit.SECONDS)); } @@ -123,7 +123,7 @@ public void testDownloadMapWithError() { verify(notificationService).addNotification(captor.capture()); ImmediateNotification notification = captor.getValue(); - notification.getActions().get(1).call(null); + notification.actions().get(1).run(); assertThrows(ExecutionException.class, () -> future.get(1, TimeUnit.SECONDS)); } diff --git a/src/test/java/com/faforever/client/teammatchmaking/TeamMatchmakingServiceTest.java b/src/test/java/com/faforever/client/teammatchmaking/TeamMatchmakingServiceTest.java index b27746a887..73994a8f4a 100644 --- a/src/test/java/com/faforever/client/teammatchmaking/TeamMatchmakingServiceTest.java +++ b/src/test/java/com/faforever/client/teammatchmaking/TeamMatchmakingServiceTest.java @@ -214,7 +214,7 @@ public void testOnInviteMessage() { ArgumentCaptor captorPersistent = ArgumentCaptor.forClass(PersistentNotification.class); verify(notificationService).addNotification(captorPersistent.capture()); PersistentNotification persistentNotification = captorPersistent.getValue(); - assertThat(persistentNotification.getSeverity(), is(INFO)); + assertThat(persistentNotification.severity(), is(INFO)); verify(i18n, times(2)).get("teammatchmaking.notification.invite.message", player.getUsername()); } diff --git a/src/test/java/com/faforever/client/update/ClientUpdateServiceTest.java b/src/test/java/com/faforever/client/update/ClientUpdateServiceTest.java index ba7dde2ce0..e612fdbd1b 100644 --- a/src/test/java/com/faforever/client/update/ClientUpdateServiceTest.java +++ b/src/test/java/com/faforever/client/update/ClientUpdateServiceTest.java @@ -115,7 +115,7 @@ public void testCheckForUpdateInBackgroundUpdateAvailable() throws Exception { PersistentNotification persistentNotification = captor.getValue(); verify(i18n).get("clientUpdateAvailable.notification", "v0.4.9.0-RC1", Bytes.formatSize(56079360L, i18n.getUserSpecificLocale())); - assertThat(persistentNotification.getSeverity(), is(INFO)); + assertThat(persistentNotification.severity(), is(INFO)); } /** @@ -144,7 +144,7 @@ public void testCheckForBetaUpdateInBackgroundUpdateAvailable(boolean supportsUp PersistentNotification persistentNotification = captor.getValue(); verify(i18n).get("clientUpdateAvailable.prereleaseNotification", "v0.4.9.1-alpha", Bytes.formatSize(56079360L, i18n.getUserSpecificLocale())); - assertThat(persistentNotification.getSeverity(), is(INFO)); + assertThat(persistentNotification.severity(), is(INFO)); } @Test