Show missing FCM token for user targets

manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java

@@ -256,39 +256,18 @@
           target -> {
             Notification.TargetType targetType = target.getType();
             String targetId = target.getId();
-            AssetQuery assetQuery =
-                new AssetQuery()
-                    .select(new AssetQuery.Select().excludeAttributes())
-                    .types(ConsoleAsset.class)
-                    .attributes(
-                        new AttributePredicate(
-                            ConsoleAsset.CONSOLE_PROVIDERS,
-                            new ValueEmptyPredicate().negate(true),
-                            false,
-                            new NameValuePredicate.Path(
-                                PushNotificationMessage.TYPE, "data", "token")));
-
-            switch (targetType) {
-              case REALM ->
-                  // Any console assets in the target realm
-                  assetQuery.realm(new RealmPredicate(targetId));
-              case USER ->
-                  // Any console assets linked to the target user
-                  assetQuery.userIds(targetId);
-              case ASSET ->
-                  // Any users linked to this asset and then their linked console assets
-                  assetQuery.userIds(
-                      assetStorageService.findUserAssetLinks(null, null, targetId).stream()
-                          .map(ual -> ual.getId().getUserId())
-                          .collect(Collectors.toSet())
-                          .toArray(String[]::new));
-            }
-
+            // Prefer consoles that still have an FCM token so a normal send is unchanged.
+            // #3113: when none do, map FCM consoles anyway so the send records
+            // "No FCM token found" instead of a missing-targets error. Browser consoles
+            // (push version "web") stay out of both queries.
             List<String> consoleAssetIds =
-                assetStorageService.findAll(assetQuery).stream()
-                    .map(Asset::getId)
-                    .distinct()
-                    .toList();
+                consoleIds(
+                    consoleAssetsLinkedTo(targetType, targetId).attributes(nonEmptyFcmToken()));
+            if (consoleAssetIds.isEmpty()) {
+              consoleAssetIds =
+                  consoleIds(
+                      consoleAssetsLinkedTo(targetType, targetId).attributes(fcmProviderVersion()));
+            }
 
             if (!consoleAssetIds.isEmpty()) {
               // Special handling if target type is user (don't need to find all linked users)
@@ -386,6 +365,25 @@
       AbstractNotificationMessage message)
       throws Exception {
 
+    Notification.TargetType targetType = target.getType();
+    String targetId = target.getId();
+
+    if (targetType != Notification.TargetType.ASSET
+        && targetType != Notification.TargetType.CUSTOM) {
+      String msg = "Target type not supported: " + targetType;
+      LOG.warning(msg);
+      throw new Exception(msg);
+    }
+
+    // A console without a token must fail even when FCM itself is not configured. Otherwise the
+    // dev-mode skip below hides the missing token and the API reports success.
+    String fcmToken = consoleFCMTokenMap.get(targetId);
+    if (targetType == Notification.TargetType.ASSET && TextUtil.isNullOrEmpty(fcmToken)) {
+      String msg = "No FCM token found for console: " + targetId;
+      LOG.finer(msg);
+      throw new Exception(msg);
+    }
+
     if (devMode && !valid) {
       // FCM not configured; log the notification instead of sending so the sent record is still
       // created
@@ -399,19 +397,6 @@
       return;
     }
 
-    Notification.TargetType targetType = target.getType();
-    String targetId = target.getId();
-
-    if (targetType != Notification.TargetType.ASSET
-        && targetType != Notification.TargetType.CUSTOM) {
-      String msg = "Target type not supported: " + targetType;
-      LOG.warning(msg);
-      throw new Exception(msg);
-    }
-
-    // Check this asset has an FCM token (i.e. it is registered for push notifications)
-    String fcmToken = consoleFCMTokenMap.get(targetId);
-
     if (TextUtil.isNullOrEmpty(fcmToken)) {
       String msg = "No FCM token found for console: " + targetId;
       LOG.finer(msg);
@@ -598,6 +583,50 @@
     return builder.build();
   }
 
+  private AssetQuery consoleAssetsLinkedTo(Notification.TargetType targetType, String targetId) {
+    AssetQuery assetQuery =
+        new AssetQuery()
+            .select(new AssetQuery.Select().excludeAttributes())
+            .types(ConsoleAsset.class);
+
+    switch (targetType) {
+      case REALM ->
+          // Any console assets in the target realm
+          assetQuery.realm(new RealmPredicate(targetId));
+      case USER ->
+          // Any console assets linked to the target user
+          assetQuery.userIds(targetId);
+      case ASSET ->
+          // Any users linked to this asset and then their linked console assets
+          assetQuery.userIds(
+              assetStorageService.findUserAssetLinks(null, null, targetId).stream()
+                  .map(ual -> ual.getId().getUserId())
+                  .collect(Collectors.toSet())
+                  .toArray(String[]::new));
+    }
+    return assetQuery;
+  }
+
+  private static AttributePredicate nonEmptyFcmToken() {
+    return new AttributePredicate(
+        ConsoleAsset.CONSOLE_PROVIDERS,
+        new ValueEmptyPredicate().negate(true),
+        false,
+        new NameValuePredicate.Path(PushNotificationMessage.TYPE, "data", "token"));
+  }
+
+  private static AttributePredicate fcmProviderVersion() {
+    return new AttributePredicate(
+        ConsoleAsset.CONSOLE_PROVIDERS,
+        new StringPredicate(FCM_PROVIDER_NAME),
+        false,
+        new NameValuePredicate.Path(PushNotificationMessage.TYPE, "version"));
+  }
+
+  private List<String> consoleIds(AssetQuery assetQuery) {
+    return assetStorageService.findAll(assetQuery).stream().map(Asset::getId).distinct().toList();
+  }
+
   protected static boolean isLinkedToFcmProvider(ConsoleAsset asset) {
     return asset
         .getConsoleProviders()

test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy

@@ -407,6 +407,34 @@ class NotificationTest extends Specification implements ManagerContainerTrait {
       assert !notificationTargetIds.contains(testuser3Console1.id)
     }
 
+    when: "all linked consoles for a user lack an FCM token"
+    notificationIds.clear()
+    notificationTargetIds.clear()
+    def testUser3Console2Asset = assetStorageService.find(testuser3Console2.id) as ConsoleAsset
+    testUser3Console2Asset.getConsoleProviders().map {
+      it.get(PushNotificationMessage.TYPE)
+    }.get().getData().put("token", null)
+    assetStorageService.merge(testUser3Console2Asset)
+    conditions.eventually {
+      assert pushNotificationHandler.consoleFCMTokenMap.get(testUser3Console2Asset.id) == null
+    }
+    notification.targets = [new Notification.Target(Notification.TargetType.USER, keycloakTestSetup.testuser3Id)]
+    advancePseudoClock(1, TimeUnit.HOURS, container)
+    adminNotificationResource.sendNotification(null, notification)
+
+    then: "sent notifications record the missing FCM token instead of missing targets"
+    ex = thrown()
+    ex.response.withCloseable { r ->
+      assert r.status == 400
+      return true
+    }
+    conditions.eventually {
+      def console1Notifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console1.id, null, null, null, null, null)
+      def console2Notifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console2.id, null, null, null, null, null)
+      assert console1Notifications.any { it.error?.contains("No FCM token found for console") }
+      assert console2Notifications.any { it.error?.contains("No FCM token found for console") }
+    }
+
     when: "a notification handler throws an exception"
     throwPushHandlerException = true
     advancePseudoClock(1, TimeUnit.HOURS, container)
@@ -434,7 +462,7 @@ class NotificationTest extends Specification implements ManagerContainerTrait {
     conditions.eventually {
       assert adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser2Console.id, null, null, null, null, null).length == 2
       notifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console1.id, null, null, null, null, null)
-      assert notifications.length == 5
+      assert notifications.length == 7
       assert notifications.every {n ->
         PushNotificationMessage pushMessage = n.message as PushNotificationMessage
         pushMessage.getTitle() == "Test Action" &&
@@ -443,7 +471,7 @@ class NotificationTest extends Specification implements ManagerContainerTrait {
                 n.deliveredOn == null &&
                 n.acknowledgedOn == null
       }
-      assert adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console2.id, null, null, null, null, null).length == 7
+      assert adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console2.id, null, null, null, null, null).length == 8
       assert adminNotificationResource.getNotifications(null, null, null, null, null, null, null, anonymousConsole.id, null, null, null, null, null).length == 3
     }
 
@@ -455,7 +483,7 @@ class NotificationTest extends Specification implements ManagerContainerTrait {
     then: "the notification should have been updated"
     conditions.eventually {
       notifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console1.id, null, null, null, null, null)
-      assert notifications.length == 5
+      assert notifications.length == 7
       assert notifications.count {n -> n.deliveredOn != null} == 1
     }
 
@@ -467,7 +495,7 @@ class NotificationTest extends Specification implements ManagerContainerTrait {
     then: "the notification should have been updated"
     conditions.eventually {
       notifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console1.id, null, null, null, null, null)
-      assert notifications.length == 5
+      assert notifications.length == 7
       assert notifications.count {n ->
         n.deliveredOn != null && n.acknowledgedOn != null && n.acknowledgement == "\"dismissed\""
       } == 1
@@ -584,6 +612,19 @@ class NotificationTest extends Specification implements ManagerContainerTrait {
     when: "a repeat frequency is set and a notification is sent"
     // Clear exception flag
     throwPushHandlerException = false
+    // The missing-token scenario cleared both consoles. Later sends still need a token.
+    def fcmToken = "23123213ad2313b0897efd"
+    [testuser3Console1, testuser3Console2].each { console ->
+      def asset = assetStorageService.find(console.id) as ConsoleAsset
+      asset.getConsoleProviders().map {
+        it.get(PushNotificationMessage.TYPE)
+      }.get().getData().put("token", fcmToken)
+      assetStorageService.merge(asset)
+    }
+    conditions.eventually {
+      assert pushNotificationHandler.consoleFCMTokenMap.get(testuser3Console1.id) == fcmToken
+      assert pushNotificationHandler.consoleFCMTokenMap.get(testuser3Console2.id) == fcmToken
+    }
     // Move clock to beginning of next hour
     def advancement = Instant.ofEpochMilli(getClockTimeOf(container))
             .truncatedTo(ChronoUnit.HOURS)