From a28cc9f38cbf96e21364909e6c3581ba859ac869 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Sat, 26 Sep 2026 11:17:52 +0700 Subject: [PATCH 1/4] fix(notifications): map user targets without FCM token USER and REALM push target expansion no longer requires a non-empty FCM token in the asset query, so sendMessage can persist the existing "No FCM token found for console" failure instead of MISSING_TARGETS. Fixes #3113 --- .../notification/PushNotificationHandler.java | 11 ++--- .../test/notification/NotificationTest.groovy | 45 ++++++++++++++++--- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index a6281e349b..391ad1774a 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -256,17 +256,12 @@ public List getTargets( target -> { Notification.TargetType targetType = target.getType(); String targetId = target.getId(); + // Do not require an FCM token in the query; consoles without a token must still be + // mapped so sendMessage can record "No FCM token found" (see direct ASSET targets). 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"))); + .types(ConsoleAsset.class); switch (targetType) { case REALM -> diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 605ab9f69b..e6c69a87d2 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -400,11 +400,42 @@ class NotificationTest extends Specification implements ManagerContainerTrait { advancePseudoClock(1, TimeUnit.HOURS, container) adminNotificationResource.sendNotification(null, notification) - then: "the notification should have been sent" + then: "the missing FCM token is recorded for the console without a token and the other console is still sent" + ex = thrown() + ex.response.withCloseable { r -> + assert r.status == 400 + return true + } conditions.eventually { - assert notificationIds.size() == 1 + assert notificationTargetIds.contains(testuser3Console1.id) assert notificationTargetIds.contains(testuser3Console2.id) - assert !notificationTargetIds.contains(testuser3Console1.id) + def failedNotifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console1.id, null, null, null, null, null) + assert failedNotifications.any { it.error?.contains("No FCM token found for console") } + } + + 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) + 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" @@ -434,7 +465,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 == 8 assert notifications.every {n -> PushNotificationMessage pushMessage = n.message as PushNotificationMessage pushMessage.getTitle() == "Test Action" && @@ -443,7 +474,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 +486,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 == 8 assert notifications.count {n -> n.deliveredOn != null} == 1 } @@ -467,7 +498,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 == 8 assert notifications.count {n -> n.deliveredOn != null && n.acknowledgedOn != null && n.acknowledgement == "\"dismissed\"" } == 1 From 02dec2ff1a0f4391dfb9b2969e40dd1f3d60a485 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:42:12 +0700 Subject: [PATCH 2/4] fix(notifications): record missing FCM token when FCM is not configured The dev-mode skip returned before the token check, so a user target with no token looked like success in integration tests. --- .../notification/PushNotificationHandler.java | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index 391ad1774a..ac613b033a 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -381,6 +381,25 @@ public void sendMessage( 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 @@ -394,19 +413,6 @@ public void sendMessage( 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); From d8a5deb1be1740e00c7f226bcc0d3f2bcd687cbb Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Tue, 27 Oct 2026 07:31:46 +0700 Subject: [PATCH 3/4] fix(notifications): map token-less consoles only when none have a token A normal user or realm send still targets consoles that have an FCM token. When none do, map FCM consoles so the missing token is recorded instead of a missing-targets error, and wait for the token cache before that send. --- .../notification/PushNotificationHandler.java | 86 +++++++++++++------ .../test/notification/NotificationTest.groovy | 21 ++--- 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index ac613b033a..f49514ab74 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -256,34 +256,19 @@ public List getTargets( target -> { Notification.TargetType targetType = target.getType(); String targetId = target.getId(); - // Do not require an FCM token in the query; consoles without a token must still be - // mapped so sendMessage can record "No FCM token found" (see direct ASSET targets). - 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)); - } - + // 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 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) @@ -599,6 +584,53 @@ protected static Message buildFCMMessage(long id, PushNotificationMessage pushMe 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 consoleIds(AssetQuery assetQuery) { + return assetStorageService.findAll(assetQuery).stream() + .map(Asset::getId) + .distinct() + .toList(); + } + protected static boolean isLinkedToFcmProvider(ConsoleAsset asset) { return asset .getConsoleProviders() diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index cba80fb6bf..e379a470dc 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -400,17 +400,11 @@ class NotificationTest extends Specification implements ManagerContainerTrait { advancePseudoClock(1, TimeUnit.HOURS, container) adminNotificationResource.sendNotification(null, notification) - then: "the missing FCM token is recorded for the console without a token and the other console is still sent" - ex = thrown() - ex.response.withCloseable { r -> - assert r.status == 400 - return true - } + then: "the notification should have been sent" conditions.eventually { - assert notificationTargetIds.contains(testuser3Console1.id) + assert notificationIds.size() == 1 assert notificationTargetIds.contains(testuser3Console2.id) - def failedNotifications = adminNotificationResource.getNotifications(null, null, null, null, null, null, null, testuser3Console1.id, null, null, null, null, null) - assert failedNotifications.any { it.error?.contains("No FCM token found for console") } + assert !notificationTargetIds.contains(testuser3Console1.id) } when: "all linked consoles for a user lack an FCM token" @@ -421,6 +415,9 @@ class NotificationTest extends Specification implements ManagerContainerTrait { 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) @@ -465,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 == 8 + assert notifications.length == 7 assert notifications.every {n -> PushNotificationMessage pushMessage = n.message as PushNotificationMessage pushMessage.getTitle() == "Test Action" && @@ -486,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 == 8 + assert notifications.length == 7 assert notifications.count {n -> n.deliveredOn != null} == 1 } @@ -498,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 == 8 + assert notifications.length == 7 assert notifications.count {n -> n.deliveredOn != null && n.acknowledgedOn != null && n.acknowledgement == "\"dismissed\"" } == 1 From 3d5a4bcdcf578fb1b46fa7af9c8c2fcac5a72e33 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:21:47 +0700 Subject: [PATCH 4/4] fix(notifications): restore FCM tokens before later sends The missing-token case left both consoles without a token, so the repeat-frequency send returned HTTP 400. Restore the tokens and wait for the handler cache. Format PushNotificationHandler for spotless. --- .../notification/PushNotificationHandler.java | 8 ++------ .../test/notification/NotificationTest.groovy | 13 +++++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index f49514ab74..f1f773f17c 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -266,8 +266,7 @@ public List getTargets( if (consoleAssetIds.isEmpty()) { consoleAssetIds = consoleIds( - consoleAssetsLinkedTo(targetType, targetId) - .attributes(fcmProviderVersion())); + consoleAssetsLinkedTo(targetType, targetId).attributes(fcmProviderVersion())); } if (!consoleAssetIds.isEmpty()) { @@ -625,10 +624,7 @@ private static AttributePredicate fcmProviderVersion() { } private List consoleIds(AssetQuery assetQuery) { - return assetStorageService.findAll(assetQuery).stream() - .map(Asset::getId) - .distinct() - .toList(); + return assetStorageService.findAll(assetQuery).stream().map(Asset::getId).distinct().toList(); } protected static boolean isLinkedToFcmProvider(ConsoleAsset asset) { diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index e379a470dc..e9c53ada56 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -612,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)