Simplify notification service (#3087)

FAForever · Dec 17, 2023 · 175b61b · 175b61b
1 parent 7e1f13d
commit 175b61b
Show file tree

Hide file tree

Showing 63 changed files with 520 additions and 660 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<T> 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
@@ -54,8 +54,7 @@ public void afterPropertiesSet() {
 
   public void notifyMissingGamePath(boolean immediateUserActionRequired) {
     List<Action> 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
@@ -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<Path> logFile) {
@@ -434,7 +434,7 @@ private void alertOnBadExit(int exitCode, Optional<Path> 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
@@ -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
@@ -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<Node> {
 
   @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<? extends PersistentNotification> notifications) {
+  private void updateNotificationsButton(Collection<PersistentNotification> notifications) {
     int size = notifications.size();
 
-    Severity highestSeverity = notifications.stream()
-        .map(PersistentNotification::getSeverity)
+    Severity highestSeverity = notifications.stream().map(PersistentNotification::severity)
         .max(Enum::compareTo)
         .orElse(null);