From c9c1efca0aafd005b82427da2e75995d9bb92b0c Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 14 Oct 2026 15:22:24 -0500 Subject: [PATCH] fix(preferences): create the bootstrap subscription once per user AlertsPreferences.ensure was an unlocked check-then-insert, and a page load fires the home widgets and the bell together. On kof22 this left two identical "alerts (bootstrap rule)" subscriptions for the owner, so every alert notified them twice. First calls are now serialized in the JVM. Across replicas, each first call keeps the oldest bootstrap subscription and deletes the rest. Subscriptions a person made are never touched. Co-Authored-By: Claude Opus 5.5 --- .../qbits/alerts/AlertsPreferences.java | 100 +++++++++++++----- .../alerts/engine/RoutingAndDeliveryTest.java | 70 ++++++++++++ 2 files changed, 143 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/kingsrook/qbits/alerts/AlertsPreferences.java b/src/main/java/com/kingsrook/qbits/alerts/AlertsPreferences.java index 78fc6c0..35f3991 100644 --- a/src/main/java/com/kingsrook/qbits/alerts/AlertsPreferences.java +++ b/src/main/java/com/kingsrook/qbits/alerts/AlertsPreferences.java @@ -17,6 +17,8 @@ package com.kingsrook.qbits.alerts; +import java.io.Serializable; +import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import com.kingsrook.qbits.alerts.engine.AlertsDb; @@ -26,7 +28,12 @@ import com.kingsrook.qbits.alerts.model.AlertsPossibleValues.ScopeType; import com.kingsrook.qbits.alerts.model.AlertsPossibleValues.SubscriberType; import com.kingsrook.qbits.alerts.model.AlertsSubscription; import com.kingsrook.qbits.alerts.model.AlertsUserPreference; +import com.kingsrook.qqq.backend.core.actions.tables.DeleteAction; +import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.tables.delete.DeleteInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.session.QSession; @@ -42,6 +49,9 @@ import com.kingsrook.qqq.backend.core.model.session.QSession; *******************************************************************************/ public final class AlertsPreferences { + /** marks the bootstrap rule's subscription (createdByUserId). */ + static final String BOOTSTRAP_CREATOR = "alerts (bootstrap rule)"; + private static final Set ENSURED = ConcurrentHashMap.newKeySet(); @@ -69,36 +79,72 @@ public final class AlertsPreferences boolean admin = session.hasPermission(AlertsQBitConfig.PERMISSION_ADMIN); String fullName = session.getUser() == null ? null : session.getUser().getFullName(); - runtime.asSystem(() -> - { - if(AlertsDb.first(AlertsUserPreference.TABLE_NAME, AlertsDb.eq("userId", userId)) == null) - { - AlertsDb.insert(AlertsUserPreference.TABLE_NAME, new QRecord() - .withValue("userId", userId) - .withValue("userLabel", fullName == null ? userId : fullName) - .withValue("inAppEnabled", true) - .withValue("emailEnabled", true) - .withValue("quietBypassSeverity", "CRITICAL") - .withValue("digestHour", 8)); - } - if(admin && runtime.getConfig().getBootstrapMinSeverity() != null && AlertsDb.first(AlertsSubscription.TABLE_NAME, AlertsDb.eq("userId", userId)) == null) + //////////////////////////////////////////////////////////////////////////// + // A page load fires the home widgets and the bell together, so first // + // calls for one user race. Serialize them in this JVM; across replicas, // + // the clean-up below keeps one bootstrap subscription (the lowest id). // + //////////////////////////////////////////////////////////////////////////// + synchronized(AlertsPreferences.class) + { + if(ENSURED.contains(userId)) { - QRecord inApp = runtime.getChannels().defaultChannel(ChannelType.IN_APP); - AlertsDb.insert(AlertsSubscription.TABLE_NAME, new QRecord() - .withValue("subscriberType", SubscriberType.USER.name()) - .withValue("userId", userId) - .withValue("scopeType", ScopeType.ALL.name()) - .withValue("minSeverity", runtime.getConfig().getBootstrapMinSeverity().name()) - .withValue("events", "RAISED,ESCALATED,REMINDER,RESOLVED") - .withValue("deliveryMode", DeliveryMode.IMMEDIATE.name()) - .withValue("channelOverrideIds", String.valueOf(inApp.getValue("id"))) - .withValue("enabled", true) - .withValue("createdByUserId", "alerts (bootstrap rule)")); + return; } - return (null); - }); - ENSURED.add(userId); + runtime.asSystem(() -> + { + if(AlertsDb.first(AlertsUserPreference.TABLE_NAME, AlertsDb.eq("userId", userId)) == null) + { + AlertsDb.insert(AlertsUserPreference.TABLE_NAME, new QRecord() + .withValue("userId", userId) + .withValue("userLabel", fullName == null ? userId : fullName) + .withValue("inAppEnabled", true) + .withValue("emailEnabled", true) + .withValue("quietBypassSeverity", "CRITICAL") + .withValue("digestHour", 8)); + } + + if(admin && runtime.getConfig().getBootstrapMinSeverity() != null && AlertsDb.first(AlertsSubscription.TABLE_NAME, AlertsDb.eq("userId", userId)) == null) + { + QRecord inApp = runtime.getChannels().defaultChannel(ChannelType.IN_APP); + AlertsDb.insert(AlertsSubscription.TABLE_NAME, new QRecord() + .withValue("subscriberType", SubscriberType.USER.name()) + .withValue("userId", userId) + .withValue("scopeType", ScopeType.ALL.name()) + .withValue("minSeverity", runtime.getConfig().getBootstrapMinSeverity().name()) + .withValue("events", "RAISED,ESCALATED,REMINDER,RESOLVED") + .withValue("deliveryMode", DeliveryMode.IMMEDIATE.name()) + .withValue("channelOverrideIds", String.valueOf(inApp.getValue("id"))) + .withValue("enabled", true) + .withValue("createdByUserId", BOOTSTRAP_CREATOR)); + } + + removeDuplicateBootstrapSubscriptions(userId); + return (null); + }); + ENSURED.add(userId); + } + } + + + + /******************************************************************************* + ** Keep only the first bootstrap subscription of a user. Two replicas can + ** each see no subscription and insert one; every later first call (and the + ** losers' own calls) delete the extra copies, so the user is not notified + ** twice per alert. Subscriptions a person made are never touched. + *******************************************************************************/ + static void removeDuplicateBootstrapSubscriptions(String userId) throws QException + { + List bootstrap = AlertsDb.query(AlertsSubscription.TABLE_NAME, AlertsDb.eq("userId", userId) + .withCriteria("createdByUserId", QCriteriaOperator.EQUALS, BOOTSTRAP_CREATOR) + .withOrderBy(new QFilterOrderBy("id"))); + if(bootstrap.size() < 2) + { + return; + } + List extra = bootstrap.subList(1, bootstrap.size()).stream().map(r -> (Serializable) r.getValue("id")).toList(); + new DeleteAction().execute(new DeleteInput(AlertsSubscription.TABLE_NAME).withPrimaryKeys(extra).withTransaction(QContext.getQBackendTransaction())); } diff --git a/src/test/java/com/kingsrook/qbits/alerts/engine/RoutingAndDeliveryTest.java b/src/test/java/com/kingsrook/qbits/alerts/engine/RoutingAndDeliveryTest.java index bf6e463..5c2c846 100644 --- a/src/test/java/com/kingsrook/qbits/alerts/engine/RoutingAndDeliveryTest.java +++ b/src/test/java/com/kingsrook/qbits/alerts/engine/RoutingAndDeliveryTest.java @@ -135,6 +135,76 @@ class RoutingAndDeliveryTest extends BaseAlertsTest + /******************************************************************************* + ** First calls for one user race (the home widgets and the bell load + ** together): only one bootstrap subscription is created. + *******************************************************************************/ + @Test + void testConcurrentFirstCallsBootstrapOnce() throws Exception + { + com.kingsrook.qqq.backend.core.context.CapturedContext context = QContext.capture(); + java.util.concurrent.ExecutorService pool = java.util.concurrent.Executors.newFixedThreadPool(4); + try + { + java.util.concurrent.CountDownLatch start = new java.util.concurrent.CountDownLatch(1); + List> futures = new java.util.ArrayList<>(); + for(int i = 0; i < 4; i++) + { + futures.add(pool.submit(() -> + { + QContext.init(context); + start.await(); + AlertsPreferences.ensure(runtime, QContext.getQSession()); + return (null); + })); + } + start.countDown(); + for(java.util.concurrent.Future future : futures) + { + future.get(); + } + } + finally + { + pool.shutdownNow(); + } + assertThat(all(AlertsSubscription.TABLE_NAME)).hasSize(1); + assertThat(all(AlertsUserPreference.TABLE_NAME)).hasSize(1); + } + + + + /******************************************************************************* + ** Another replica raced and left a second bootstrap subscription: the next + ** first call keeps the oldest and leaves a person's own subscriptions alone. + *******************************************************************************/ + @Test + void testDuplicateBootstrapSubscriptionsAreRemoved() throws Exception + { + for(String creator : List.of("alerts (bootstrap rule)", "alerts (bootstrap rule)", ADMIN)) + { + runtime.asSystem(() -> AlertsDb.insert(AlertsSubscription.TABLE_NAME, new QRecord() + .withValue("subscriberType", "USER") + .withValue("userId", ADMIN) + .withValue("scopeType", "ALL") + .withValue("minSeverity", "ERROR") + .withValue("events", "RAISED") + .withValue("deliveryMode", "IMMEDIATE") + .withValue("enabled", true) + .withValue("createdByUserId", creator))); + } + List before = all(AlertsSubscription.TABLE_NAME); + assertThat(before).hasSize(3); + + AlertsPreferences.ensure(runtime, QContext.getQSession()); + + List after = all(AlertsSubscription.TABLE_NAME); + assertThat(after).hasSize(2); + assertThat(after).extracting(r -> r.getValueInteger("id")).containsExactlyInAnyOrder(before.get(0).getValueInteger("id"), before.get(2).getValueInteger("id")); + } + + + /******************************************************************************* ** No subscriptions: nobody is notified. The bootstrap rule gives admins an ** in-app subscription to ERROR and worse on first visit. -- 2.54.0