From 20b568a44adf1fd7b91ab300a799f7127a9224e2 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Tue, 7 Jul 2026 15:13:38 -0700 Subject: [PATCH 01/10] feat(notifications): add hosted file-based in-IDE notifications Adds a client-side notifications feature to the Eclipse plugin, matching the schema and behavior of the other Amazon Q IDE plugins. On startup (after the language server is ready) a background poller fetches a hosted JSON file over HTTPS every 10 minutes and shows targeted in-IDE toasts. - Schema 2.x "combined" payload model + a polymorphic condition DSL (==, !=, >, >=, <, <=, anyOf, noneOf, and, or, not) with a single Jackson deserializer. - Rules engine gates display on compute/os/ide/extension/authx conditions (semver for ide/extension versions; SNAPSHOT builds and not-installed extensions are never shown). - ETag-cached fetcher that degrades gracefully: any failure (absent file, 403/404, empty, malformed, offline) resolves to "show nothing" and only logs. Supports a file:// endpoint for local testing. - Toasts: Info/Warning auto-dismiss; Critical persists until dismissed. Actions: ShowUrl, UpdateExtension, OpenChangelog, plus More and Dismiss. Dismissals persist for 60 days; emergencies re-show until dismissed. - User preference "Show Amazon Q notifications" (default on) as a kill switch; telemetry (toolkit_showNotification / toolkit_invokeAction) is independent of notification polling and respects the telemetry opt-in. - 42 unit tests covering parsing, the DSL, rules, fetch/degradation, dismissal, and filtering/dedup. Wired into LspStartupActivity (start) and Activator.stop (clean shutdown). --- .../amazonq/lsp/LspStartupActivity.java | 2 + .../AmazonQNotificationPopup.java | 141 ++++++++++++ .../notifications/DismissedNotification.java | 36 +++ .../notifications/FeatureAuthDetails.java | 8 + .../NotificationActionFactory.java | 73 ++++++ .../notifications/NotificationConstants.java | 28 +++ .../notifications/NotificationData.java | 107 +++++++++ .../NotificationDismissalConfiguration.java | 29 +++ .../NotificationDismissalStore.java | 65 ++++++ .../notifications/NotificationExpression.java | 51 +++++ .../NotificationExpressionDeserializer.java | 92 ++++++++ .../NotificationPollingService.java | 85 +++++++ .../NotificationPreferences.java | 34 +++ .../NotificationTelemetryProvider.java | 66 ++++++ .../notifications/NotificationsFetcher.java | 189 ++++++++++++++++ .../notifications/NotificationsList.java | 17 ++ .../notifications/ProcessNotifications.java | 119 ++++++++++ .../amazonq/notifications/RulesEngine.java | 165 ++++++++++++++ .../amazonq/notifications/SystemDetails.java | 18 ++ .../notifications/SystemDetailsCollector.java | 109 +++++++++ .../eclipse/amazonq/plugin/Activator.java | 2 + .../AmazonQPreferenceInitializer.java | 2 + .../preferences/AmazonQPreferencePage.java | 16 ++ .../eclipse/amazonq/util/Constants.java | 2 + .../NotificationActionFactoryTest.java | 93 ++++++++ .../NotificationDismissalStoreTest.java | 80 +++++++ .../notifications/NotificationMiscTest.java | 55 +++++ .../NotificationParsingTest.java | 211 ++++++++++++++++++ .../NotificationsFetcherTest.java | 126 +++++++++++ .../ProcessNotificationsTest.java | 118 ++++++++++ .../notifications/RulesEngineTest.java | 153 +++++++++++++ 31 files changed, 2292 insertions(+) create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/DismissedNotification.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/FeatureAuthDetails.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactory.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationConstants.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalConfiguration.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpression.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpressionDeserializer.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationTelemetryProvider.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsList.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetails.java create mode 100644 plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactoryTest.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationMiscTest.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/LspStartupActivity.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/LspStartupActivity.java index 9b661a14a..3a43ed19a 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/LspStartupActivity.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/LspStartupActivity.java @@ -18,6 +18,7 @@ import org.eclipse.ui.PlatformUI; import software.aws.toolkits.eclipse.amazonq.broker.events.QDeveloperProfileState; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationPollingService; import software.aws.toolkits.eclipse.amazonq.plugin.Activator; import software.aws.toolkits.eclipse.amazonq.providers.browser.AmazonQBrowserProvider; import software.aws.toolkits.eclipse.amazonq.telemetry.ToolkitTelemetryProvider; @@ -83,6 +84,7 @@ private void schedulePostStartupJobs() { Display.getDefault().asyncExec(() -> attachAutoTriggerListenersIfApplicable()); Display.getDefault().asyncExec(() -> showKiroSunsetNotification()); checkForUpdates(); + NotificationPollingService.getInstance().start(); }); } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java new file mode 100644 index 000000000..83cf88e76 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java @@ -0,0 +1,141 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.List; + +import org.eclipse.jface.resource.ImageDescriptor; +import org.eclipse.swt.SWT; +import org.eclipse.swt.events.SelectionAdapter; +import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.graphics.Image; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Label; +import org.eclipse.ui.ISharedImages; +import org.eclipse.ui.PlatformUI; + +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationSeverity; +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; +import software.aws.toolkits.eclipse.amazonq.util.ToolkitNotification; + +/** + * A toast notification that renders a severity icon, wrapped description, and N action buttons built from a hosted + * notification's actions. INFO/WARNING keep the base auto-close timer; CRITICAL overrides {@link #scheduleAutoClose()} + * to a no-op so it persists until the user dismisses it (the COE "reach the user" requirement). + */ +public final class AmazonQNotificationPopup extends ToolkitNotification { + + /** A rendered action button: a label plus the handler to run when clicked. */ + public record NotificationAction(String label, Runnable onClick) { } + + private final String description; + // Mylyn's default auto-close is 8s, which is too short to read a multi-line known-issue message. + private static final long TRANSIENT_DELAY_CLOSE_MS = 20_000L; + + private final NotificationSeverity severity; + private final boolean persistent; + private final List actions; + + public AmazonQNotificationPopup(final Display display, final String title, final String description, + final NotificationSeverity severity, final List actions) { + super(display, title, description); + this.description = description; + this.severity = severity; + this.persistent = severity == NotificationSeverity.CRITICAL; + this.actions = actions == null ? List.of() : List.copyOf(actions); + // CRITICAL persists until dismissed (delayClose = 0 => scheduleAutoClose is a no-op); others stay readable. + final long delayClose = persistent ? 0L : TRANSIENT_DELAY_CLOSE_MS; + setDelayClose(delayClose); + Activator.getLogger().info("AmazonQNotificationPopup created: severity=" + severity + + " persistent=" + persistent + " delayCloseMs=" + delayClose); + } + + @Override + protected void scheduleAutoClose() { + // Belt-and-suspenders: never schedule an auto-close for a persistent (CRITICAL) notification. + if (!persistent) { + super.scheduleAutoClose(); + } + } + + @Override + protected void createContentArea(final Composite parent) { + final Composite container = new Composite(parent, SWT.NONE); + container.setLayout(new GridLayout(2, false)); + container.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); + + final Label iconLabel = new Label(container, SWT.NONE); + final Image icon = createSeverityImage(severity); + if (icon != null) { + iconLabel.setImage(icon); + // close() is final in the base class, so dispose the icon via a listener instead of overriding close(). + iconLabel.addDisposeListener(e -> { + if (!icon.isDisposed()) { + icon.dispose(); + } + }); + } + iconLabel.setLayoutData(new GridData(SWT.BEGINNING, SWT.BEGINNING, false, false)); + + final Label messageLabel = new Label(container, SWT.WRAP); + messageLabel.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); + messageLabel.setText(description != null ? description : ""); + + if (!actions.isEmpty()) { + createActionButtons(parent); + } + } + + private void createActionButtons(final Composite parent) { + final Composite buttonRow = new Composite(parent, SWT.NONE); + final GridLayout layout = new GridLayout(actions.size(), false); + layout.marginWidth = 0; + layout.marginHeight = 0; + buttonRow.setLayout(layout); + buttonRow.setLayoutData(new GridData(SWT.END, SWT.CENTER, true, false)); + + for (final NotificationAction action : actions) { + final Button button = new Button(buttonRow, SWT.PUSH); + button.setText(action.label()); + button.setLayoutData(new GridData(SWT.END, SWT.CENTER, false, false)); + button.addSelectionListener(new SelectionAdapter() { + @Override + public void widgetSelected(final SelectionEvent e) { + action.onClick().run(); + close(); + } + }); + } + } + + private static Image createSeverityImage(final NotificationSeverity severity) { + try { + final ISharedImages sharedImages = PlatformUI.getWorkbench().getSharedImages(); + final ImageDescriptor descriptor = sharedImages.getImageDescriptor(iconKey(severity)); + if (descriptor == null) { + return null; + } + // createImage(false) returns null (rather than throwing) if the image can't be loaded. + return descriptor.createImage(false); + } catch (Exception e) { + return null; + } + } + + static String iconKey(final NotificationSeverity severity) { + switch (severity) { + case CRITICAL: + return ISharedImages.IMG_OBJS_ERROR_TSK; + case WARNING: + return ISharedImages.IMG_OBJS_WARN_TSK; + case INFO: + default: + return ISharedImages.IMG_OBJS_INFO_TSK; + } + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/DismissedNotification.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/DismissedNotification.java new file mode 100644 index 000000000..4e2d9cf08 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/DismissedNotification.java @@ -0,0 +1,36 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +/** A dismissed notification id plus the epoch-millis timestamp it was dismissed (for retention cleanup). Gson-friendly. */ +public final class DismissedNotification { + + private String id; + private long dismissedAtEpochMs; + + public DismissedNotification() { + // no-arg constructor for Gson + } + + public DismissedNotification(final String id, final long dismissedAtEpochMs) { + this.id = id; + this.dismissedAtEpochMs = dismissedAtEpochMs; + } + + public String getId() { + return id; + } + + public void setId(final String id) { + this.id = id; + } + + public long getDismissedAtEpochMs() { + return dismissedAtEpochMs; + } + + public void setDismissedAtEpochMs(final long dismissedAtEpochMs) { + this.dismissedAtEpochMs = dismissedAtEpochMs; + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/FeatureAuthDetails.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/FeatureAuthDetails.java new file mode 100644 index 000000000..f77c792fd --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/FeatureAuthDetails.java @@ -0,0 +1,8 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +/** Snapshot of a feature's auth/connection state, used by the rules engine's {@code authx} matching. */ +public record FeatureAuthDetails(String connectionType, String region, String connectionState) { +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactory.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactory.java new file mode 100644 index 000000000..997ef2447 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactory.java @@ -0,0 +1,73 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.jface.dialogs.MessageDialog; +import org.eclipse.swt.widgets.Display; + +import software.aws.toolkits.eclipse.amazonq.notifications.AmazonQNotificationPopup.NotificationAction; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationFollowupAction; +import software.aws.toolkits.eclipse.amazonq.util.Constants; +import software.aws.toolkits.eclipse.amazonq.util.PluginUtils; + +/** + * Builds the rendered action buttons for a notification from its hosted {@code actions[]}, ports the JetBrains mapping: + * {@code ShowUrl} (case-sensitive) only supplies the URL for the always-present "More" button; {@code UpdateExtension} + * and {@code OpenChangelog} open pages; unknown types are ignored. + */ +public final class NotificationActionFactory { + + private static final String SHOW_URL = "ShowUrl"; + private static final String UPDATE_EXTENSION = "UpdateExtension"; + private static final String OPEN_CHANGELOG = "OpenChangelog"; + + private NotificationActionFactory() { + // prevent instantiation + } + + public static List createActions(final String notificationId, + final List followupActions, final String title, final String description) { + final List result = new ArrayList<>(); + String moreUrl = null; + + if (followupActions != null) { + for (final NotificationFollowupAction action : followupActions) { + final String type = action.type(); + if (SHOW_URL.equals(type)) { + if (action.content() != null && action.content().enUs() != null) { + moreUrl = action.content().enUs().url(); + } + } else if (UPDATE_EXTENSION.equals(type)) { + result.add(new NotificationAction("Update", () -> { + NotificationTelemetryProvider.emitInvokeAction(notificationId, UPDATE_EXTENSION); + PluginUtils.openWebpage(Constants.AMAZON_Q_UPDATE_SITE_URL); + })); + } else if (OPEN_CHANGELOG.equals(type)) { + result.add(new NotificationAction("View changelog", () -> { + NotificationTelemetryProvider.emitInvokeAction(notificationId, OPEN_CHANGELOG); + PluginUtils.openWebpage(Constants.AMAZON_Q_CHANGELOG_URL); + })); + } + } + } + + final String capturedUrl = moreUrl; + result.add(new NotificationAction("More", () -> { + NotificationTelemetryProvider.emitInvokeAction(notificationId, "More"); + showMoreDialog(title, description, capturedUrl); + })); + return result; + } + + private static void showMoreDialog(final String title, final String description, final String url) { + if (url != null && !url.isBlank()) { + PluginUtils.handleExternalLinkClick(url); + } else { + MessageDialog.openInformation(Display.getDefault().getActiveShell(), title, description); + } + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationConstants.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationConstants.java new file mode 100644 index 000000000..91b592c91 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationConstants.java @@ -0,0 +1,28 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +/** Endpoint, cache, and storage-key constants for the hosted-file notifications feature. */ +public final class NotificationConstants { + + /** Production hosted-file endpoint for Eclipse notifications (schema 2.x combined). */ + public static final String NOTIFICATIONS_ENDPOINT = + "https://idetoolkits-hostedfiles.amazonaws.com/Notifications/Eclipse/combined/2.x.json"; + + /** Subdirectory (under the plugin state dir) that holds the cached notifications file. */ + public static final String NOTIFICATIONS_SUBDIRECTORY = "notifications"; + + /** Filename of the cached notifications payload. */ + public static final String NOTIFICATIONS_CACHE_FILENAME = "notifications.json"; + + /** PluginStore key under which dismissed-notification state is persisted. */ + public static final String DISMISSAL_STORAGE_KEY = "qNotificationDismissals"; + + /** Environment variable that overrides the endpoint (for local dev/testing). */ + public static final String NOTIFICATIONS_ENDPOINT_ENV = "AMAZONQ_NOTIFICATIONS_ENDPOINT"; + + private NotificationConstants() { + // prevent instantiation + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java new file mode 100644 index 000000000..0ce5e2b94 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java @@ -0,0 +1,107 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.List; +import java.util.Locale; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * A single hosted notification (schema 2.x). Ported from the JetBrains notification model so payloads + * stay compatible across IDEs. Unknown JSON keys are ignored (the shared mapper disables + * FAIL_ON_UNKNOWN_PROPERTIES), so a missing optional block deserializes to {@code null}. + */ +public record NotificationData( + String id, + NotificationSchedule schedule, + String severity, + NotificationDisplayCondition condition, + NotificationContent content, + List actions) { + + /** How often the notification is shown. */ + public enum NotificationScheduleType { + /** Shown once per IDE session (on the first poll). */ + STARTUP, + /** Shown on every poll until dismissed. */ + EMERGENCY; + + /** + * Maps the raw JSON value case-insensitively: only {@code "startup"} yields + * {@link #STARTUP}; anything else (including typos and {@code null}) yields {@link #EMERGENCY}. + */ + @JsonCreator + public static NotificationScheduleType fromString(final String value) { + return value != null && "startup".equals(value.toLowerCase(Locale.ROOT)) + ? STARTUP + : EMERGENCY; + } + } + + /** Notification severity; drives the toast style. */ + public enum NotificationSeverity { + INFO, + WARNING, + CRITICAL; + + /** Maps the exact-case JSON value; any unrecognized or {@code null} value yields {@link #INFO}. */ + public static NotificationSeverity fromString(final String value) { + if ("Critical".equals(value)) { + return CRITICAL; + } + if ("Warning".equals(value)) { + return WARNING; + } + return INFO; + } + } + + /** Wrapper around the schedule type as it appears in JSON: {@code { "type": "Startup" }}. */ + public record NotificationSchedule(NotificationScheduleType type) { } + + /** + * Display conditions. All present blocks must match (logical AND); a {@code null} block is skipped. + * Note {@code extension} is a singular-named array, matching the JetBrains field the rules engine reads. + */ + public record NotificationDisplayCondition( + ComputeType compute, + SystemType os, + SystemType ide, + List extension, + List authx) { } + + /** Compute-environment condition. */ + public record ComputeType(NotificationExpression type, NotificationExpression architecture) { } + + /** OS or IDE condition (type + version). */ + public record SystemType(NotificationExpression type, NotificationExpression version) { } + + /** Installed-extension condition, matched by id + optional version expression. */ + public record ExtensionType(String id, NotificationExpression version) { } + + /** Authentication/connection condition for a feature (for example {@code "q"}). */ + public record AuthxType( + String feature, + NotificationExpression type, + NotificationExpression region, + NotificationExpression connectionState, + NotificationExpression ssoScopes) { } + + /** Localized notification content. Only the {@code en-US} locale is consumed. */ + public record NotificationContent(@JsonProperty("en-US") LocalizedContent enUs) { } + + /** Title/description for a single locale. */ + public record LocalizedContent(String title, String description) { } + + /** A follow-up action (button) on the notification. */ + public record NotificationFollowupAction(String type, NotificationFollowupActionContent content) { } + + /** Localized action content. */ + public record NotificationFollowupActionContent(@JsonProperty("en-US") LocalizedAction enUs) { } + + /** Title and optional URL for a single locale's action. */ + public record LocalizedAction(String title, String url) { } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalConfiguration.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalConfiguration.java new file mode 100644 index 000000000..4b61eb6a5 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalConfiguration.java @@ -0,0 +1,29 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.ArrayList; +import java.util.List; + +/** + * Concrete wrapper persisted via {@code PluginStore.putObject}/{@code getObject}. A concrete class (rather than a raw + * generic collection) is required because {@code getObject(key, Class)} deserializes with Gson reflecting into the + * declared field type, which correctly recovers the {@link DismissedNotification} element type. + */ +public final class NotificationDismissalConfiguration { + + private List dismissedNotifications = new ArrayList<>(); + + public NotificationDismissalConfiguration() { + // no-arg constructor for Gson + } + + public List getDismissedNotifications() { + return dismissedNotifications; + } + + public void setDismissedNotifications(final List dismissedNotifications) { + this.dismissedNotifications = dismissedNotifications; + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java new file mode 100644 index 000000000..b1f44909f --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java @@ -0,0 +1,65 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.time.Duration; +import java.time.Instant; +import java.util.List; + +import software.aws.toolkits.eclipse.amazonq.configuration.PluginStore; +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; + +/** + * Persists dismissed-notification ids (with a 60-day retention) via {@link PluginStore}. All mutations funnel through + * synchronized methods so a load-modify-write (which spans two PluginStore calls) cannot lose entries under concurrency. + */ +public final class NotificationDismissalStore { + + private static final Duration RETENTION = Duration.ofDays(60); + + private final PluginStore pluginStore; + + public NotificationDismissalStore() { + this(Activator.getPluginStore()); + } + + public NotificationDismissalStore(final PluginStore pluginStore) { + this.pluginStore = pluginStore; + } + + public synchronized boolean isDismissed(final String id) { + return loadAndClean().getDismissedNotifications().stream().anyMatch(d -> d.getId().equals(id)); + } + + public synchronized void dismiss(final String id) { + final NotificationDismissalConfiguration config = loadAndClean(); + final List dismissed = config.getDismissedNotifications(); + if (dismissed.stream().anyMatch(d -> d.getId().equals(id))) { + return; + } + dismissed.add(new DismissedNotification(id, Instant.now().toEpochMilli())); + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, config); + } + + private NotificationDismissalConfiguration loadAndClean() { + NotificationDismissalConfiguration config; + try { + config = pluginStore.getObject(NotificationConstants.DISMISSAL_STORAGE_KEY, + NotificationDismissalConfiguration.class); + } catch (Exception e) { + Activator.getLogger().warn("Corrupt notification dismissal state; resetting", e); + config = null; + } + if (config == null || config.getDismissedNotifications() == null) { + return new NotificationDismissalConfiguration(); + } + final Instant cutoff = Instant.now().minus(RETENTION); + final boolean removedAny = config.getDismissedNotifications() + .removeIf(d -> Instant.ofEpochMilli(d.getDismissedAtEpochMs()).isBefore(cutoff)); + if (removedAny) { + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, config); + } + return config; + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpression.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpression.java new file mode 100644 index 000000000..97b8130dc --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpression.java @@ -0,0 +1,51 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.List; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; + +/** + * A notification display-condition expression, encoded in the hosted JSON as a single-key wrapper + * object (for example { "==": "1.0" } or { "and": [ ... ] }). The operator + * is the wrapper key; the value is a bare string, an array of strings, or nested expressions. + * Ported from the JetBrains schema-2.x notification model so the rules engine can match 1:1. + */ +@JsonDeserialize(using = NotificationExpressionDeserializer.class) +public sealed interface NotificationExpression { + + /** Matches when the actual value equals the given value (==). */ + record ComparisonCondition(String value) implements NotificationExpression { } + + /** Matches when the actual value does not equal the given value (!=). */ + record NotEqualsCondition(String value) implements NotificationExpression { } + + /** Matches when the actual value is greater than the given value (>). */ + record GreaterThanCondition(String value) implements NotificationExpression { } + + /** Matches when the actual value is greater than or equal to the given value (>=). */ + record GreaterThanOrEqualsCondition(String value) implements NotificationExpression { } + + /** Matches when the actual value is less than the given value (<). */ + record LessThanCondition(String value) implements NotificationExpression { } + + /** Matches when the actual value is less than or equal to the given value (<=). */ + record LessThanOrEqualsCondition(String value) implements NotificationExpression { } + + /** Matches when the actual value is contained in the given list (anyOf). */ + record AnyOfCondition(List value) implements NotificationExpression { } + + /** Matches when the actual value is not contained in the given list (noneOf). */ + record NoneOfCondition(List value) implements NotificationExpression { } + + /** Matches when every nested expression matches (and). */ + record AndCondition(List expectedValueList) implements NotificationExpression { } + + /** Matches when any nested expression matches (or). */ + record OrCondition(List expectedValueList) implements NotificationExpression { } + + /** Matches when the nested expression does not match (not). */ + record NotCondition(NotificationExpression expectedValue) implements NotificationExpression { } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpressionDeserializer.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpressionDeserializer.java new file mode 100644 index 000000000..7535b7a76 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationExpressionDeserializer.java @@ -0,0 +1,92 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; + +/** + * Deserializes a {@link NotificationExpression} from its single-key operator-wrapper object form, + * for example { ">=": "1.0" }, { "anyOf": ["a", "b"] }, or + * { "and": [ { ">=": "1.0" }, { "<": "2.0" } ] }. The and, + * or, and not operators nest recursively. + */ +public final class NotificationExpressionDeserializer extends JsonDeserializer { + + @Override + public NotificationExpression deserialize(final JsonParser parser, final DeserializationContext ctxt) throws IOException { + JsonNode node = parser.getCodec().readTree(parser); + if (node == null || !node.isObject() || node.size() != 1) { + throw new JsonMappingException(parser, "Notification expression must be a single-key operator object"); + } + + Map.Entry entry = node.fields().next(); + String operator = entry.getKey(); + JsonNode value = entry.getValue(); + + switch (operator) { + case "==": + return new NotificationExpression.ComparisonCondition(value.asText()); + case "!=": + return new NotificationExpression.NotEqualsCondition(value.asText()); + case ">": + return new NotificationExpression.GreaterThanCondition(value.asText()); + case ">=": + return new NotificationExpression.GreaterThanOrEqualsCondition(value.asText()); + case "<": + return new NotificationExpression.LessThanCondition(value.asText()); + case "<=": + return new NotificationExpression.LessThanOrEqualsCondition(value.asText()); + case "anyOf": + return new NotificationExpression.AnyOfCondition(toStringList(parser, value, operator)); + case "noneOf": + return new NotificationExpression.NoneOfCondition(toStringList(parser, value, operator)); + case "and": + return new NotificationExpression.AndCondition(toExpressionList(parser, value, operator)); + case "or": + return new NotificationExpression.OrCondition(toExpressionList(parser, value, operator)); + case "not": + return new NotificationExpression.NotCondition(toExpression(parser, value)); + default: + throw new JsonMappingException(parser, "Unknown notification expression operator: " + operator); + } + } + + private List toStringList(final JsonParser parser, final JsonNode value, final String operator) throws JsonMappingException { + if (!value.isArray()) { + throw new JsonMappingException(parser, operator + " must contain an array of values"); + } + List values = new ArrayList<>(); + for (JsonNode element : value) { + values.add(element.asText()); + } + return values; + } + + private List toExpressionList(final JsonParser parser, final JsonNode value, final String operator) + throws IOException { + if (!value.isArray()) { + throw new JsonMappingException(parser, operator + " must contain an array of expressions"); + } + List expressions = new ArrayList<>(); + Iterator elements = value.elements(); + while (elements.hasNext()) { + expressions.add(toExpression(parser, elements.next())); + } + return expressions; + } + + private NotificationExpression toExpression(final JsonParser parser, final JsonNode value) throws IOException { + return parser.getCodec().treeToValue(value, NotificationExpression.class); + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java new file mode 100644 index 000000000..10eafbab0 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java @@ -0,0 +1,85 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.time.Duration; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicBoolean; + +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; +import software.aws.toolkits.eclipse.amazonq.util.ThreadingUtils; + +/** + * App-level singleton that polls the notifications endpoint every 10 minutes on the shared worker pool, self-rescheduling + * after each poll. The poll body is total (fetch never throws; the work is wrapped so an escaped error cannot cancel the + * loop) and the re-arm happens in a {@code finally}. {@link #stop()} must be called early in {@code Activator.stop()} to + * cancel the pending future and prevent a re-arm during teardown. + */ +public final class NotificationPollingService { + + private static final NotificationPollingService INSTANCE = new NotificationPollingService(); + private static final long POLL_INTERVAL_MS = Duration.ofMinutes(10).toMillis(); + + private final AtomicBoolean started = new AtomicBoolean(false); + private volatile boolean stopped; + private volatile ScheduledFuture scheduledPoll; + private volatile NotificationsFetcher fetcher; + private volatile ProcessNotifications processor; + + private NotificationPollingService() { + // singleton + } + + public static NotificationPollingService getInstance() { + return INSTANCE; + } + + /** Starts polling once per app lifetime; no-op if the kill-switch is off or polling already started. */ + public void start() { + if (!NotificationPreferences.isNotificationsEnabled()) { + return; + } + if (!started.compareAndSet(false, true)) { + return; + } + this.fetcher = new NotificationsFetcher(NotificationPreferences.resolveEndpoint()); + this.processor = new ProcessNotifications(new NotificationDismissalStore()); + pollOnce(); + } + + private void pollOnce() { + if (stopped || !NotificationPreferences.isNotificationsEnabled()) { + return; + } + try { + fetcher.fetch().ifPresent(processor::process); + } catch (Throwable t) { + Activator.getLogger().warn("Notifications poll failed", t); + NotificationTelemetryProvider.emitPollFailure("Failed to poll for notifications"); + } finally { + reschedule(); + } + } + + private void reschedule() { + if (stopped || !NotificationPreferences.isNotificationsEnabled()) { + return; + } + try { + scheduledPoll = (ScheduledFuture) ThreadingUtils.scheduleAsyncTaskWithDelay(this::pollOnce, POLL_INTERVAL_MS); + } catch (RejectedExecutionException e) { + Activator.getLogger().info("Notifications polling stopped (worker pool shutting down)"); + } + } + + /** Cancels the pending poll and prevents further rescheduling; safe to call during shutdown. */ + public void stop() { + stopped = true; + final ScheduledFuture current = scheduledPoll; + if (current != null) { + current.cancel(false); + } + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java new file mode 100644 index 000000000..8b1e9b043 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java @@ -0,0 +1,34 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; +import software.aws.toolkits.eclipse.amazonq.preferences.AmazonQPreferencePage; + +/** Reads the notifications kill-switch preference and resolves the endpoint (preference > env > prod default). */ +public final class NotificationPreferences { + + private NotificationPreferences() { + // prevent instantiation + } + + /** Whether the notifications feature is enabled (kill-switch); defaults to {@code true}. */ + public static boolean isNotificationsEnabled() { + return Activator.getDefault().getPreferenceStore().getBoolean(AmazonQPreferencePage.NOTIFICATIONS_OPT_IN); + } + + /** Resolves the endpoint URL. Precedence: preference override -> environment variable -> production default. */ + public static String resolveEndpoint() { + final String pref = Activator.getDefault().getPreferenceStore() + .getString(AmazonQPreferencePage.NOTIFICATIONS_ENDPOINT_OVERRIDE); + if (pref != null && !pref.isBlank()) { + return pref; + } + final String env = System.getenv(NotificationConstants.NOTIFICATIONS_ENDPOINT_ENV); + if (env != null && !env.isBlank()) { + return env; + } + return NotificationConstants.NOTIFICATIONS_ENDPOINT; + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationTelemetryProvider.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationTelemetryProvider.java new file mode 100644 index 000000000..2e9f305ee --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationTelemetryProvider.java @@ -0,0 +1,66 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.time.Instant; + +import software.amazon.awssdk.services.toolkittelemetry.model.MetricDatum; +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; +import software.aws.toolkits.telemetry.TelemetryDefinitions.Component; +import software.aws.toolkits.telemetry.TelemetryDefinitions.Result; +import software.aws.toolkits.telemetry.ToolkitTelemetry; + +/** + * Emits notification telemetry ({@code toolkit_showNotification} / {@code toolkit_invokeAction}). Emission routes + * through {@code DefaultTelemetryService.emitMetric}, which respects the telemetry opt-in independently of the + * notifications feature. The metric {@code id} is the raw notification id (no {@code TARGETED_NOTIFICATION:} prefix). + */ +public final class NotificationTelemetryProvider { + + private NotificationTelemetryProvider() { + // prevent instantiation + } + + /** A notification was shown to the user. */ + public static void emitShowNotification(final String notificationId) { + final MetricDatum datum = ToolkitTelemetry.ShowNotificationEvent() + .id(notificationId) + .component(Component.INFOBAR) + .result(Result.SUCCEEDED) + .passive(true) + .createTime(Instant.now()) + .value(1.0) + .build(); + Activator.getTelemetryService().emitMetric(datum); + } + + /** A poll cycle failed to retrieve notifications. */ + public static void emitPollFailure(final String reason) { + final MetricDatum datum = ToolkitTelemetry.ShowNotificationEvent() + .id("") + .component(Component.FILESYSTEM) + .result(Result.FAILED) + .reason(reason) + .passive(true) + .createTime(Instant.now()) + .value(1.0) + .build(); + Activator.getTelemetryService().emitMetric(datum); + } + + /** The user clicked an action button on a notification. */ + public static void emitInvokeAction(final String notificationId, final String actionType) { + final MetricDatum datum = ToolkitTelemetry.InvokeActionEvent() + .id(notificationId) + .source(notificationId) + .action(actionType) + .component(Component.INFOBAR) + .result(Result.SUCCEEDED) + .passive(false) + .createTime(Instant.now()) + .value(1.0) + .build(); + Activator.getTelemetryService().emitMetric(datum); + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java new file mode 100644 index 000000000..bfa304329 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java @@ -0,0 +1,189 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.time.Duration; +import java.util.Optional; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; +import software.aws.toolkits.eclipse.amazonq.util.HttpClientFactory; +import software.aws.toolkits.eclipse.amazonq.util.ObjectMapperFactory; +import software.aws.toolkits.eclipse.amazonq.util.PluginUtils; + +/** + * Fetches the hosted notifications payload with ETag conditional GET + on-disk caching, modeled on + * {@code VersionManifestFetcher}. {@link #fetch()} is a TOTAL function: any failure (absent file, 403/404, empty body, + * malformed JSON, network error) resolves to {@link Optional#empty()} and is only logged — it never throws and never + * surfaces a user-facing popup, so a not-yet-deployed endpoint is a silent no-op. + */ +public final class NotificationsFetcher { + + private static final int TIMEOUT_SECONDS = 30; + private static final int MAX_RETRIES = 3; + private static final long RETRY_BASE_DELAY_MS = 1000L; + private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getInstance(); + + private final String endpointUrl; + private final HttpClient httpClient; + private final Path cachePath; + + public NotificationsFetcher(final String endpointUrl) { + this(endpointUrl, null, null); + } + + public NotificationsFetcher(final String endpointUrl, final HttpClient httpClient, final Path cachePath) { + // Trim stray whitespace/newlines (a common copy-paste artifact when the endpoint is set via env var / preference). + this.endpointUrl = endpointUrl == null ? null : endpointUrl.trim(); + this.httpClient = httpClient != null ? httpClient : HttpClientFactory.getInstance(); + this.cachePath = cachePath != null ? cachePath + : PluginUtils.getPluginDir(NotificationConstants.NOTIFICATIONS_SUBDIRECTORY) + .resolve(NotificationConstants.NOTIFICATIONS_CACHE_FILENAME); + } + + /** Never throws. Returns the parsed notifications, or empty when there is nothing to show. */ + public Optional fetch() { + try { + if (endpointUrl == null || endpointUrl.isBlank()) { + return getResourceFromCache(); + } + if (endpointUrl.regionMatches(true, 0, "file:", 0, 5)) { + return readLocalFile(endpointUrl); + } + return fetchRemoteWithRetries(); + } catch (Exception e) { + Activator.getLogger().warn("Unexpected error fetching notifications", e); + return Optional.empty(); + } + } + + private Optional fetchRemoteWithRetries() { + final Optional cached = getResourceFromCache(); + final String cachedEtag = Activator.getPluginStore().get(endpointUrl); + final String etagToRequest = cached.isPresent() && cachedEtag != null ? cachedEtag : null; + + Exception lastTransient = null; + for (int attempt = 0; attempt < MAX_RETRIES; attempt++) { + try { + final HttpResponse response = getResourceFromRemote(etagToRequest); + final int status = response.statusCode(); + + if (status == HttpURLConnection.HTTP_NOT_MODIFIED) { + if (cached.isPresent()) { + return cached; + } + // ETag stored but cache is gone/invalid: clear it so the next poll re-fetches fresh. + Activator.getLogger().warn("Notifications returned 304 but cache is missing; clearing ETag"); + Activator.getPluginStore().remove(endpointUrl); + return Optional.empty(); + } + if (status == HttpURLConnection.HTTP_OK) { + return validateAndCache(response.body()); + } + // 403/404 (file not deployed yet) and any other non-2xx: not an error condition, show nothing. + Activator.getLogger().info("No notifications available (HTTP " + status + ")"); + return Optional.empty(); + } catch (IOException | InterruptedException e) { + if (e instanceof InterruptedException) { + Thread.currentThread().interrupt(); + return cached; + } + lastTransient = e; + sleepBeforeRetry(attempt); + } + } + Activator.getLogger().warn("Failed to fetch notifications after retries; using cache if present", lastTransient); + return cached; + } + + private HttpResponse getResourceFromRemote(final String etag) throws IOException, InterruptedException { + final HttpRequest.Builder requestBuilder = HttpRequest.newBuilder().uri(URI.create(endpointUrl)) + .timeout(Duration.ofSeconds(TIMEOUT_SECONDS)); + Optional.ofNullable(etag).ifPresent(tag -> requestBuilder.header("If-None-Match", tag)); + return httpClient.send(requestBuilder.build(), HttpResponse.BodyHandlers.ofString()); + } + + private Optional readLocalFile(final String fileUrl) { + try { + // Prefer strict URI parsing; fall back to stripping the scheme for a plain path if the URI is not + // strictly legal (e.g. an un-encoded path pasted as file:///...). + Path path; + try { + path = Path.of(URI.create(fileUrl)); + } catch (IllegalArgumentException e) { + path = Path.of(fileUrl.replaceFirst("(?i)^file://", "")); + } + return validate(Files.readString(path)); + } catch (Exception e) { + Activator.getLogger().warn("Failed to read local notifications file: " + fileUrl, e); + return Optional.empty(); + } + } + + private Optional getResourceFromCache() { + try { + if (Files.exists(cachePath)) { + final Optional parsed = validate(Files.readString(cachePath)); + if (parsed.isEmpty()) { + Files.deleteIfExists(cachePath); + Activator.getLogger().info("Deleted corrupt cached notifications file"); + } + return parsed; + } + } catch (Exception e) { + Activator.getLogger().warn("Error reading cached notifications", e); + } + return Optional.empty(); + } + + private Optional validate(final String content) { + if (content == null || content.isBlank()) { + return Optional.empty(); + } + try { + return Optional.ofNullable(OBJECT_MAPPER.readValue(content, NotificationsList.class)); + } catch (Exception e) { + Activator.getLogger().warn("Failed to parse notifications payload", e); + return Optional.empty(); + } + } + + private Optional validateAndCache(final String body) { + final Optional parsed = validate(body); + if (parsed.isEmpty()) { + // Do not cache a bad body; keep any prior valid cache untouched. + return getResourceFromCache(); + } + try { + final Path tmp = cachePath.resolveSibling(cachePath.getFileName() + ".tmp"); + Files.createDirectories(cachePath.getParent()); + Files.writeString(tmp, body); + Files.move(tmp, cachePath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + } catch (Exception e) { + Activator.getLogger().warn("Failed to cache notifications file", e); + } + return parsed; + } + + private void sleepBeforeRetry(final int attempt) { + if (attempt >= MAX_RETRIES - 1) { + return; + } + try { + Thread.sleep(RETRY_BASE_DELAY_MS * (1L << attempt)); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsList.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsList.java new file mode 100644 index 000000000..fc128e8ce --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsList.java @@ -0,0 +1,17 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.List; + +/** + * Root of the hosted notifications file: {@code { "schema": { "version": "2.0" }, "notifications": [ ... ] }}. + * The schema version is parsed but not validated (matching the JetBrains client), and {@code notifications} + * may be {@code null} or empty when there is nothing to show. + */ +public record NotificationsList(Schema schema, List notifications) { + + /** Schema descriptor; only the version string is carried. */ + public record Schema(String version) { } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java new file mode 100644 index 000000000..c94656561 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java @@ -0,0 +1,119 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.eclipse.swt.widgets.Display; +import org.eclipse.ui.PlatformUI; + +import software.aws.toolkits.eclipse.amazonq.notifications.AmazonQNotificationPopup.NotificationAction; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.LocalizedContent; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationScheduleType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationSeverity; +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; + +/** + * Filters a fetched notifications payload and shows the survivors. Runs on the poll (worker) thread: it snapshots the + * system/auth state once, applies STARTUP-once + dismissal + rules + in-session dedup, then marshals each surviving + * toast onto the SWT UI thread. STARTUP notifications show only on the first poll of a session; an undismissed EMERGENCY + * shows once per session (guarded by an in-memory set) rather than re-toasting on every poll. + */ +public final class ProcessNotifications { + + /** Renders a notification that has passed all filtering. Injectable so tests can observe without SWT. */ + public interface NotificationDisplay { + void show(String id, NotificationData notification, LocalizedContent content, + List actions); + } + + private final AtomicBoolean isFirstPoll = new AtomicBoolean(true); + private final Set shownThisSession = ConcurrentHashMap.newKeySet(); + private final NotificationDismissalStore dismissalStore; + private final NotificationDisplay display; + + public ProcessNotifications(final NotificationDismissalStore dismissalStore) { + this(dismissalStore, ProcessNotifications::showToast); + } + + public ProcessNotifications(final NotificationDismissalStore dismissalStore, final NotificationDisplay display) { + this.dismissalStore = dismissalStore; + this.display = display; + } + + public void process(final NotificationsList list) { + if (list == null || list.notifications() == null || list.notifications().isEmpty()) { + return; + } + final boolean isStartupPoll = isFirstPoll.compareAndSet(true, false); + final SystemDetails sys = SystemDetailsCollector.collect(); + + for (final NotificationData notification : list.notifications()) { + processOne(notification, isStartupPoll, sys); + } + } + + private void processOne(final NotificationData notification, final boolean isStartupPoll, final SystemDetails sys) { + final String id = notification.id(); + if (id == null) { + return; + } + final boolean isStartup = notification.schedule() != null + && notification.schedule().type() == NotificationScheduleType.STARTUP; + if (isStartup && !isStartupPoll) { + return; + } + if (dismissalStore.isDismissed(id)) { + return; + } + if (!RulesEngine.displayNotification(notification, sys)) { + return; + } + if (notification.content() == null || notification.content().enUs() == null) { + Activator.getLogger().info("Skipping notification with no en-US content: " + id); + return; + } + final LocalizedContent content = notification.content().enUs(); + if (isBlank(content.title()) || isBlank(content.description())) { + Activator.getLogger().info("Skipping notification with blank title/description: " + id); + return; + } + if (!shownThisSession.add(id)) { + return; + } + final List actions = new ArrayList<>(NotificationActionFactory.createActions( + id, notification.actions(), content.title(), content.description())); + // The explicit "Dismiss" button persists the dismissal so the notification does not reappear; + // closing/auto-fading or clicking another action does NOT dismiss (an emergency re-shows next session). + actions.add(new NotificationAction("Dismiss", () -> dismissalStore.dismiss(id))); + NotificationTelemetryProvider.emitShowNotification(id); + display.show(id, notification, content, actions); + } + + private static void showToast(final String id, final NotificationData notification, final LocalizedContent content, + final List actions) { + final NotificationSeverity severity = NotificationSeverity.fromString(notification.severity()); + Activator.getLogger().info("Showing notification toast: " + id + " (severity=" + severity + ")"); + Display.getDefault().asyncExec(() -> { + if (!PlatformUI.isWorkbenchRunning()) { + Activator.getLogger().info("Workbench not running; skipping notification toast: " + id); + return; + } + try { + new AmazonQNotificationPopup(Display.getCurrent(), content.title(), content.description(), severity, + actions).open(); + } catch (Exception e) { + Activator.getLogger().error("Failed to render notification toast: " + id, e); + } + }); + } + + private static boolean isBlank(final String s) { + return s == null || s.isBlank(); + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java new file mode 100644 index 000000000..38b150d77 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java @@ -0,0 +1,165 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.List; +import java.util.Map; + +import org.apache.maven.artifact.versioning.ArtifactVersion; + +import software.aws.toolkits.eclipse.amazonq.lsp.manager.fetcher.ArtifactUtils; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.AuthxType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.ComputeType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.ExtensionType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationDisplayCondition; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.SystemType; + +/** + * Pure evaluator that decides whether a notification's display conditions match the current system/auth state. + * Ported 1:1 from the JetBrains RulesEngine: a null condition matches everyone; a present condition is an AND of its + * five optional blocks; version comparisons use semver only for {@code ide.version} and {@code extension.version}. + */ +public final class RulesEngine { + + private static final String CLEAN_SEMVER = "^\\d+(\\.\\d+)*$"; + + private RulesEngine() { + // prevent instantiation + } + + /** {@code condition == null} shows the notification to everyone. */ + public static boolean displayNotification(final NotificationData notification, final SystemDetails sys) { + final NotificationDisplayCondition condition = notification.condition(); + return condition == null || matchesAllRules(condition, sys); + } + + static boolean matchesAllRules(final NotificationDisplayCondition c, final SystemDetails sys) { + final boolean compute = c.compute() == null + || matchesCompute(c.compute(), sys.computeType(), sys.computeArchitecture()); + final boolean os = c.os() == null || matchesOs(c.os(), sys.osType(), sys.osVersion()); + final boolean ide = c.ide() == null || matchesIde(c.ide(), sys.ideType(), sys.ideVersion()); + final boolean extension = matchesExtension(c.extension(), sys.pluginVersions()); + final boolean authx = matchesAuth(c.authx(), sys); + return compute && os && ide && extension && authx; + } + + private static boolean matchesCompute(final ComputeType nc, final String type, final String arch) { + final boolean typeMatch = nc.type() == null || evaluateNotificationExpression(nc.type(), type); + final boolean archMatch = nc.architecture() == null || evaluateNotificationExpression(nc.architecture(), arch); + return typeMatch && archMatch; + } + + private static boolean matchesOs(final SystemType no, final String os, final String osVersion) { + final boolean typeMatch = no.type() == null || evaluateNotificationExpression(no.type(), os); + final boolean versionMatch = no.version() == null || evaluateNotificationExpression(no.version(), osVersion); + return typeMatch && versionMatch; + } + + private static boolean matchesIde(final SystemType ni, final String ide, final String ideVersion) { + final boolean typeMatch = ni.type() == null || evaluateNotificationExpression(ni.type(), ide); + final boolean versionMatch = ni.version() == null || evaluateNotificationExpression(ni.version(), ideVersion, true); + return typeMatch && versionMatch; + } + + private static boolean matchesExtension(final List ne, final Map installedVersions) { + if (ne == null || ne.isEmpty()) { + return true; + } + boolean anyInstalled = false; + for (final ExtensionType ext : ne) { + final String installed = installedVersions.get(ext.id()); + if (installed == null) { + continue; + } + anyInstalled = true; + // Development builds must never receive notifications. + if (installed.toLowerCase(java.util.Locale.ROOT).contains("snapshot")) { + return false; + } + if (ext.version() != null && !evaluateNotificationExpression(ext.version(), installed, true)) { + return false; + } + } + // Declared but none of the extensions are installed -> do not show. + return anyInstalled; + } + + private static boolean matchesAuth(final List na, final SystemDetails sys) { + if (na == null || na.isEmpty()) { + return true; + } + for (final AuthxType feature : na) { + if (!"q".equals(feature.feature())) { + // Faithful to JetBrains: any non-"q" feature passes. + continue; + } + final FeatureAuthDetails auth = sys.qAuth(); + if (auth == null) { + return false; + } + final boolean typeMatch = feature.type() == null + || evaluateNotificationExpression(feature.type(), auth.connectionType()); + final boolean regionMatch = feature.region() == null + || evaluateNotificationExpression(feature.region(), auth.region()); + final boolean stateMatch = feature.connectionState() == null + || evaluateNotificationExpression(feature.connectionState(), auth.connectionState()); + if (!(typeMatch && regionMatch && stateMatch)) { + return false; + } + } + return true; + } + + /** Evaluates an expression against an actual value, using string comparison for ordering operators. */ + public static boolean evaluateNotificationExpression(final NotificationExpression expr, final String value) { + return evaluateNotificationExpression(expr, value, false); + } + + /** Evaluates an expression; when {@code useSemver} is true, ordering operators compare versions semantically. */ + public static boolean evaluateNotificationExpression(final NotificationExpression expr, final String value, + final boolean useSemver) { + if (expr instanceof NotificationExpression.ComparisonCondition c) { + return c.value().equals(value); + } else if (expr instanceof NotificationExpression.NotEqualsCondition c) { + return !c.value().equals(value); + } else if (expr instanceof NotificationExpression.GreaterThanCondition c) { + return compare(value, c.value(), useSemver) > 0; + } else if (expr instanceof NotificationExpression.GreaterThanOrEqualsCondition c) { + return compare(value, c.value(), useSemver) >= 0; + } else if (expr instanceof NotificationExpression.LessThanCondition c) { + return compare(value, c.value(), useSemver) < 0; + } else if (expr instanceof NotificationExpression.LessThanOrEqualsCondition c) { + return compare(value, c.value(), useSemver) <= 0; + } else if (expr instanceof NotificationExpression.AnyOfCondition c) { + return c.value().contains(value); + } else if (expr instanceof NotificationExpression.NoneOfCondition c) { + return !c.value().contains(value); + } else if (expr instanceof NotificationExpression.NotCondition c) { + return !evaluateNotificationExpression(c.expectedValue(), value, useSemver); + } else if (expr instanceof NotificationExpression.OrCondition c) { + return c.expectedValueList().stream().anyMatch(e -> evaluateNotificationExpression(e, value, useSemver)); + } else if (expr instanceof NotificationExpression.AndCondition c) { + return c.expectedValueList().stream().allMatch(e -> evaluateNotificationExpression(e, value, useSemver)); + } + return true; + } + + private static int compare(final String actual, final String expected, final boolean useSemver) { + return useSemver ? compareSemver(actual, expected) : actual.compareTo(expected); + } + + private static int compareSemver(final String actual, final String expected) { + // Match JetBrains: fall back to lexical comparison when either side is not clean numeric semver. + if (!isCleanSemver(actual) || !isCleanSemver(expected)) { + return actual.compareTo(expected); + } + final ArtifactVersion actualVersion = ArtifactUtils.parseVersion(actual); + final ArtifactVersion expectedVersion = ArtifactUtils.parseVersion(expected); + return actualVersion.compareTo(expectedVersion); + } + + private static boolean isCleanSemver(final String v) { + return v != null && v.matches(CLEAN_SEMVER); + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetails.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetails.java new file mode 100644 index 000000000..c69d022d6 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetails.java @@ -0,0 +1,18 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.Map; + +/** Immutable snapshot of the current system + auth state that a notification's conditions are evaluated against. */ +public record SystemDetails( + String computeType, + String computeArchitecture, + String osType, + String osVersion, + String ideType, + String ideVersion, + Map pluginVersions, + FeatureAuthDetails qAuth) { +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java new file mode 100644 index 000000000..03c94f023 --- /dev/null +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java @@ -0,0 +1,109 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import java.util.Map; + +import org.eclipse.core.runtime.Platform; +import org.osgi.framework.Bundle; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.Version; + +import software.aws.toolkits.eclipse.amazonq.lsp.auth.model.AuthState; +import software.aws.toolkits.eclipse.amazonq.lsp.auth.model.AuthStateType; +import software.aws.toolkits.eclipse.amazonq.lsp.auth.model.LoginType; +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; + +/** Resolves the current system + auth state into an immutable {@link SystemDetails} snapshot for the rules engine. */ +public final class SystemDetailsCollector { + + private static final String PLATFORM_BUNDLE_ID = "org.eclipse.platform"; + private static final String UNKNOWN = "Unknown"; + + private SystemDetailsCollector() { + // prevent instantiation + } + + /** Snapshots {@code getAuthState()} once and resolves everything into an immutable {@link SystemDetails}. */ + public static SystemDetails collect() { + final Bundle pluginBundle = FrameworkUtil.getBundle(SystemDetailsCollector.class); + final String pluginId = pluginBundle != null ? pluginBundle.getSymbolicName() : UNKNOWN; + final String pluginVersion = pluginBundle != null ? pluginBundle.getVersion().toString() : UNKNOWN; + + return new SystemDetails( + "Local", + Platform.getOSArch(), + System.getProperty("os.name"), + System.getProperty("os.version"), + "Eclipse", + resolveIdeVersion(), + Map.of(pluginId, pluginVersion), + resolveQAuth()); + } + + /** Amazon Q Eclipse bundle symbolic name — the key notification payloads use for {@code extension.id}. */ + public static String pluginId() { + final Bundle pluginBundle = FrameworkUtil.getBundle(SystemDetailsCollector.class); + return pluginBundle != null ? pluginBundle.getSymbolicName() : UNKNOWN; + } + + private static String resolveIdeVersion() { + final Bundle platform = Platform.getBundle(PLATFORM_BUNDLE_ID); + if (platform == null) { + return UNKNOWN; + } + final Version v = platform.getVersion(); + return v.getMajor() + "." + v.getMinor() + "." + v.getMicro(); + } + + private static FeatureAuthDetails resolveQAuth() { + final AuthState authState = Activator.getLoginService().getAuthState(); + if (authState == null) { + return new FeatureAuthDetails(UNKNOWN, UNKNOWN, "NotConnected"); + } + return new FeatureAuthDetails( + mapConnectionType(authState.loginType()), + mapRegion(authState), + mapConnectionState(authState.authStateType())); + } + + private static String mapConnectionType(final LoginType loginType) { + if (loginType == null) { + return UNKNOWN; + } + switch (loginType) { + case BUILDER_ID: + return "BuilderId"; + case IAM_IDENTITY_CENTER: + return "Idc"; + default: + return UNKNOWN; + } + } + + private static String mapConnectionState(final AuthStateType authStateType) { + if (authStateType == null) { + return "NotConnected"; + } + switch (authStateType) { + case LOGGED_IN: + return "Connected"; + case EXPIRED: + return "Expired"; + case LOGGED_OUT: + default: + return "NotConnected"; + } + } + + private static String mapRegion(final AuthState authState) { + if (authState.loginParams() != null && authState.loginParams().getLoginIdcParams() != null) { + final String region = authState.loginParams().getLoginIdcParams().getRegion(); + if (region != null && !region.isBlank()) { + return region; + } + } + return UNKNOWN; + } +} diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java index 233bfb2a5..39f965adf 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java @@ -22,6 +22,7 @@ import software.aws.toolkits.eclipse.amazonq.util.DefaultCodeReferenceLoggingService; import software.aws.toolkits.eclipse.amazonq.util.LoggingService; import software.aws.toolkits.eclipse.amazonq.util.PluginLogger; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationPollingService; import software.aws.toolkits.eclipse.amazonq.util.ThreadingUtils; import software.aws.toolkits.eclipse.amazonq.views.router.ViewRouter; import software.aws.toolkits.eclipse.workspace.WorkspaceChangeListener; @@ -63,6 +64,7 @@ public Activator() { @Override public final void stop(final BundleContext context) throws Exception { + NotificationPollingService.getInstance().stop(); AmazonQBrowserProvider.getInstance().dispose(); super.stop(context); plugin = null; diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java index fc40c1512..63f700700 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java @@ -22,6 +22,8 @@ public final void initializeDefaultPreferences() { store.setDefault(AmazonQPreferencePage.Q_DATA_SHARING, true); store.setDefault(AmazonQPreferencePage.HTTPS_PROXY, ""); store.setDefault(AmazonQPreferencePage.CA_CERT, ""); + store.setDefault(AmazonQPreferencePage.NOTIFICATIONS_OPT_IN, true); + store.setDefault(AmazonQPreferencePage.NOTIFICATIONS_ENDPOINT_OVERRIDE, ""); store.addPropertyChangeListener(event -> { ThreadingUtils.executeAsyncTask(() -> { Activator.getLspProvider().getAmazonQServer() diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java index 7f55c482f..7dee941f9 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java @@ -40,6 +40,8 @@ public class AmazonQPreferencePage extends FieldEditorPreferencePage implements public static final String Q_DATA_SHARING = "qDataSharing"; public static final String HTTPS_PROXY = "httpsProxy"; public static final String CA_CERT = "customCaCert"; + public static final String NOTIFICATIONS_OPT_IN = "notificationsOptIn"; + public static final String NOTIFICATIONS_ENDPOINT_OVERRIDE = "notificationsEndpointOverride"; private Boolean isTelemetryOptInChecked; private Boolean isQDataSharingOptInChecked; @@ -74,6 +76,8 @@ protected final void createFieldEditors() { createTelemetryOptInField(); createHorizontalSeparator(); createQDataSharingField(); + createHeading("Notifications"); + createNotificationsOptInField(); createHeading("Proxy Settings"); createHttpsProxyField(); createCaCertField(); @@ -154,6 +158,18 @@ public void widgetSelected(final SelectionEvent event) { }); } + private void createNotificationsOptInField() { + Composite notificationsOptInComposite = new Composite(getFieldEditorParent(), SWT.NONE); + notificationsOptInComposite.setLayout(new GridLayout(2, false)); + GridData notificationsOptInCompositeData = new GridData(SWT.FILL, SWT.CENTER, true, false); + notificationsOptInCompositeData.horizontalIndent = 20; + notificationsOptInComposite.setLayoutData(notificationsOptInCompositeData); + + BooleanFieldEditor notificationsOptIn = new BooleanFieldEditor(NOTIFICATIONS_OPT_IN, + "Show Amazon Q notifications about known issues and available fixes", notificationsOptInComposite); + addField(notificationsOptIn); + } + private void createQDataSharingField() { Composite qDataSharingComposite = new Composite(getFieldEditorParent(), SWT.NONE); qDataSharingComposite.setLayout(new GridLayout(2, false)); diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java index e2b009eb7..8b3b24a25 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java @@ -59,4 +59,6 @@ private Constants() { public static final String INLINE_CHAT_EXPIRED_AUTH_BODY = "Login status expired; please open Q plugin window to reauthenticate."; public static final String INLINE_CHAT_CONTEXT_ID = "org.eclipse.ui.inlineChatContext"; public static final String INLINE_SUGGESTIONS_CONTEXT_ID = "org.eclipse.ui.suggestionsContext"; + public static final String AMAZON_Q_CHANGELOG_URL = "https://github.com/aws/amazon-q-eclipse/blob/main/CHANGELOG.md"; + public static final String AMAZON_Q_UPDATE_SITE_URL = "https://marketplace.eclipse.org/content/amazon-q"; } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactoryTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactoryTest.java new file mode 100644 index 000000000..68a7ab313 --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationActionFactoryTest.java @@ -0,0 +1,93 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mockStatic; + +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.MockedStatic; + +import software.aws.toolkits.eclipse.amazonq.extensions.implementation.ActivatorStaticMockExtension; +import software.aws.toolkits.eclipse.amazonq.notifications.AmazonQNotificationPopup.NotificationAction; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.LocalizedAction; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationFollowupAction; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationFollowupActionContent; +import software.aws.toolkits.eclipse.amazonq.util.Constants; +import software.aws.toolkits.eclipse.amazonq.util.PluginUtils; + +/** Coverage for the pure action-to-button mapping (ShowUrl-is-not-a-button, always-append-More, unknown-ignored). */ +public final class NotificationActionFactoryTest { + + @RegisterExtension + private static ActivatorStaticMockExtension activatorExtension = new ActivatorStaticMockExtension(); + + private static NotificationFollowupAction action(final String type, final String title, final String url) { + return new NotificationFollowupAction(type, + new NotificationFollowupActionContent(new LocalizedAction(title, url))); + } + + @Test + void noActionsStillYieldsMoreButton() { + final List result = NotificationActionFactory.createActions("id", null, "t", "d"); + assertEquals(1, result.size()); + assertEquals("More", result.get(0).label()); + } + + @Test + void updateAndChangelogBecomeButtonsPlusMore() { + final List result = NotificationActionFactory.createActions("id", + List.of(action("UpdateExtension", "Update", null), action("OpenChangelog", "Changelog", null)), + "t", "d"); + assertEquals(3, result.size()); + assertEquals("Update", result.get(0).label()); + assertEquals("View changelog", result.get(1).label()); + assertEquals("More", result.get(2).label()); + } + + @Test + void showUrlSuppliesMoreUrlButNoOwnButton() { + final List result = NotificationActionFactory.createActions("id", + List.of(action("ShowUrl", "Click me", "https://x.test")), "t", "d"); + assertEquals(1, result.size()); + assertEquals("More", result.get(0).label()); + + try (MockedStatic pluginUtils = mockStatic(PluginUtils.class)) { + result.get(0).onClick().run(); + pluginUtils.verify(() -> PluginUtils.handleExternalLinkClick(eq("https://x.test"))); + } + } + + @Test + void unknownActionTypeIgnored() { + final List result = NotificationActionFactory.createActions("id", + List.of(action("ShowMarketplace", "Go", null)), "t", "d"); + assertEquals(1, result.size()); + assertEquals("More", result.get(0).label()); + } + + @Test + void lowercaseShowurlIsNotTreatedAsShowUrl() { + // Case-sensitive: "showurl" is unknown, so it is ignored (no URL captured, only More remains). + final List result = NotificationActionFactory.createActions("id", + List.of(action("showurl", "x", "https://x.test")), "t", "d"); + assertEquals(1, result.size()); + } + + @Test + void updateButtonOpensUpdateSite() { + final List result = NotificationActionFactory.createActions("id", + List.of(action("UpdateExtension", "Update", null)), "t", "d"); + try (MockedStatic pluginUtils = mockStatic(PluginUtils.class)) { + result.get(0).onClick().run(); + pluginUtils.verify(() -> PluginUtils.openWebpage(eq(Constants.AMAZON_Q_UPDATE_SITE_URL))); + } + assertTrue(true); + } +} diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java new file mode 100644 index 000000000..3a53c1241 --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java @@ -0,0 +1,80 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.time.Instant; +import java.util.List; + +import org.eclipse.core.internal.preferences.EclipsePreferences; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import software.aws.toolkits.eclipse.amazonq.configuration.DefaultPluginStore; +import software.aws.toolkits.eclipse.amazonq.configuration.PluginStore; +import software.aws.toolkits.eclipse.amazonq.extensions.implementation.ActivatorStaticMockExtension; + +/** Round-trip + retention + concurrency-safety coverage for the dismissal store, using a real Gson-backed PluginStore. */ +public final class NotificationDismissalStoreTest { + + @RegisterExtension + private static ActivatorStaticMockExtension activatorExtension = new ActivatorStaticMockExtension(); + + private PluginStore pluginStore; + + @BeforeEach + void setUp() { + pluginStore = new DefaultPluginStore(new EclipsePreferences()); + } + + @Test + void dismissThenIsDismissedRoundTrips() { + final NotificationDismissalStore store = new NotificationDismissalStore(pluginStore); + assertFalse(store.isDismissed("n1")); + store.dismiss("n1"); + assertTrue(store.isDismissed("n1")); + // A fresh store reading the same PluginStore sees the persisted dismissal. + assertTrue(new NotificationDismissalStore(pluginStore).isDismissed("n1")); + } + + @Test + void dismissIsIdempotent() { + final NotificationDismissalStore store = new NotificationDismissalStore(pluginStore); + store.dismiss("n1"); + store.dismiss("n1"); + final NotificationDismissalConfiguration config = + pluginStore.getObject(NotificationConstants.DISMISSAL_STORAGE_KEY, NotificationDismissalConfiguration.class); + assertTrue(config.getDismissedNotifications().size() == 1); + } + + @Test + void expiredDismissalsAreCleanedOnRead() { + final NotificationDismissalConfiguration config = new NotificationDismissalConfiguration(); + final long old = Instant.now().minus(Duration.ofDays(61)).toEpochMilli(); + config.setDismissedNotifications(new java.util.ArrayList<>(List.of(new DismissedNotification("stale", old)))); + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, config); + + final NotificationDismissalStore store = new NotificationDismissalStore(pluginStore); + assertFalse(store.isDismissed("stale")); + } + + @Test + void recentDismissalsSurviveCleanup() { + final NotificationDismissalConfiguration config = new NotificationDismissalConfiguration(); + final long recent = Instant.now().minus(Duration.ofDays(5)).toEpochMilli(); + config.setDismissedNotifications(new java.util.ArrayList<>(List.of(new DismissedNotification("fresh", recent)))); + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, config); + + assertTrue(new NotificationDismissalStore(pluginStore).isDismissed("fresh")); + } + + @Test + void noStateReturnsNotDismissed() { + assertFalse(new NotificationDismissalStore(pluginStore).isDismissed("anything")); + } +} diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationMiscTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationMiscTest.java new file mode 100644 index 000000000..4853591de --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationMiscTest.java @@ -0,0 +1,55 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.ui.ISharedImages; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import software.aws.toolkits.eclipse.amazonq.extensions.implementation.ActivatorStaticMockExtension; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationSeverity; +import software.aws.toolkits.eclipse.amazonq.plugin.Activator; +import software.aws.toolkits.eclipse.amazonq.preferences.AmazonQPreferencePage; + +/** Coverage for the icon mapping and the preference/endpoint resolver. */ +public final class NotificationMiscTest { + + @RegisterExtension + private static ActivatorStaticMockExtension activatorExtension = new ActivatorStaticMockExtension(); + + @Test + void iconKeyMapsSeverity() { + assertEquals(ISharedImages.IMG_OBJS_ERROR_TSK, AmazonQNotificationPopup.iconKey(NotificationSeverity.CRITICAL)); + assertEquals(ISharedImages.IMG_OBJS_WARN_TSK, AmazonQNotificationPopup.iconKey(NotificationSeverity.WARNING)); + assertEquals(ISharedImages.IMG_OBJS_INFO_TSK, AmazonQNotificationPopup.iconKey(NotificationSeverity.INFO)); + } + + @Test + void notificationsEnabledReadsPreference() { + final IPreferenceStore store = Activator.getDefault().getPreferenceStore(); + when(store.getBoolean(eq(AmazonQPreferencePage.NOTIFICATIONS_OPT_IN))).thenReturn(true); + assertTrue(NotificationPreferences.isNotificationsEnabled()); + + when(store.getBoolean(eq(AmazonQPreferencePage.NOTIFICATIONS_OPT_IN))).thenReturn(false); + assertFalse(NotificationPreferences.isNotificationsEnabled()); + } + + @Test + void resolveEndpointPrefersOverrideThenDefault() { + final IPreferenceStore store = Activator.getDefault().getPreferenceStore(); + when(store.getString(eq(AmazonQPreferencePage.NOTIFICATIONS_ENDPOINT_OVERRIDE))).thenReturn("https://override.test/x.json"); + assertEquals("https://override.test/x.json", NotificationPreferences.resolveEndpoint()); + + when(store.getString(eq(AmazonQPreferencePage.NOTIFICATIONS_ENDPOINT_OVERRIDE))).thenReturn(""); + // With no override and (in a test JVM) no env var, falls through to the prod default. + assertEquals(NotificationConstants.NOTIFICATIONS_ENDPOINT, NotificationPreferences.resolveEndpoint()); + } +} diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java new file mode 100644 index 000000000..fccf606b6 --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java @@ -0,0 +1,211 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.AuthxType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.ExtensionType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationScheduleType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationSeverity; +import software.aws.toolkits.eclipse.amazonq.util.ObjectMapperFactory; + +/** + * Parse-only coverage for the notification data model and the condition-DSL deserializer. This is the + * Phase-1 de-risking test: it proves the JetBrains schema-2.x payload (including the polymorphic operator + * DSL) round-trips through Jackson in Java. + */ +public final class NotificationParsingTest { + + private final ObjectMapper mapper = ObjectMapperFactory.getInstance(); + + // The canonical JetBrains example payload (exercises every operator + nesting). Note the file uses the + // "extensions" (plural) key, which is a latent bug in that fixture: our model reads the singular + // "extension", so this block deserializes to null here (asserted below), matching the JetBrains client. + private static final String EXAMPLE_JSON = """ + { + "schema": { "version": "2.0" }, + "notifications": [ + { + "id": "example_id_12344", + "schedule": { "type": "StartUp" }, + "severity": "Critical", + "condition": { + "compute": { + "type": { "or": [ { "==": "ec2" }, { "==": "desktop" } ] }, + "architecture": { "!=": "x64" } + }, + "os": { + "type": { "anyOf": [ "Darwin", "Linux" ] }, + "version": { "<=": "23.0.1.0" } + }, + "ide": { + "type": { "noneOf": [ "PyCharm", "IDEA" ] }, + "version": { "and": [ { ">=": "1.0" }, { "<": "2.0" } ] } + }, + "extension": [ + { "id": "aws.toolkit", "version": { "!=": "1.3334" } }, + { "id": "amazon.q", "version": { "!=": "3.37.0" } } + ], + "authx": [ { + "feature": "q", + "type": { "anyOf": [ "IamIdentityCenter", "AwsBuilderId" ] }, + "region": { "==": "us-east-1" }, + "connectionState": { "!=": "Connected" }, + "ssoScopes": { "noneOf": [ "codewhisperer:scope1", "sso:account:access" ] } + } ] + }, + "content": { + "en-US": { "title": "Look at this!", "description": "Some bug is there" } + }, + "actions": [ + { "type": "ShowMarketplace", "content": { "en-US": { "title": "Go to market" } } }, + { "type": "ShowUrl", "content": { "en-US": { "title": "Click me!", "url": "http://nowhere" } } } + ] + } + ] + } + """; + + @Test + void parsesFullExamplePayload() throws Exception { + NotificationsList list = mapper.readValue(EXAMPLE_JSON, NotificationsList.class); + + assertEquals("2.0", list.schema().version()); + assertEquals(1, list.notifications().size()); + + NotificationData n = list.notifications().get(0); + assertEquals("example_id_12344", n.id()); + assertEquals(NotificationScheduleType.STARTUP, n.schedule().type()); + assertEquals(NotificationSeverity.CRITICAL, NotificationSeverity.fromString(n.severity())); + assertEquals("Look at this!", n.content().enUs().title()); + assertEquals("Some bug is there", n.content().enUs().description()); + } + + @Test + void parsesEveryDslOperator() throws Exception { + NotificationsList list = mapper.readValue(EXAMPLE_JSON, NotificationsList.class); + NotificationData.NotificationDisplayCondition cond = list.notifications().get(0).condition(); + + // or / == (compute.type), != (compute.architecture) + assertInstanceOf(NotificationExpression.OrCondition.class, cond.compute().type()); + NotificationExpression.OrCondition or = (NotificationExpression.OrCondition) cond.compute().type(); + assertEquals(2, or.expectedValueList().size()); + assertInstanceOf(NotificationExpression.ComparisonCondition.class, or.expectedValueList().get(0)); + assertEquals("ec2", ((NotificationExpression.ComparisonCondition) or.expectedValueList().get(0)).value()); + assertInstanceOf(NotificationExpression.NotEqualsCondition.class, cond.compute().architecture()); + + // anyOf (os.type), <= (os.version) + NotificationExpression.AnyOfCondition anyOf = (NotificationExpression.AnyOfCondition) cond.os().type(); + assertEquals(List.of("Darwin", "Linux"), anyOf.value()); + assertInstanceOf(NotificationExpression.LessThanOrEqualsCondition.class, cond.os().version()); + + // noneOf (ide.type), and[>=, <] (ide.version) + assertInstanceOf(NotificationExpression.NoneOfCondition.class, cond.ide().type()); + NotificationExpression.AndCondition and = (NotificationExpression.AndCondition) cond.ide().version(); + assertEquals(2, and.expectedValueList().size()); + assertInstanceOf(NotificationExpression.GreaterThanOrEqualsCondition.class, and.expectedValueList().get(0)); + assertInstanceOf(NotificationExpression.LessThanCondition.class, and.expectedValueList().get(1)); + } + + @Test + void parsesSingularExtensionArray() throws Exception { + // The example fixture uses the plural "extension" key here (we corrected it in EXAMPLE_JSON), and our + // model reads the singular field name — so the array is populated, not silently dropped. + NotificationsList list = mapper.readValue(EXAMPLE_JSON, NotificationsList.class); + List extensions = list.notifications().get(0).condition().extension(); + + assertEquals(2, extensions.size()); + assertEquals("aws.toolkit", extensions.get(0).id()); + assertEquals("amazon.q", extensions.get(1).id()); + assertInstanceOf(NotificationExpression.NotEqualsCondition.class, extensions.get(0).version()); + } + + @Test + void parsesAuthxAndActions() throws Exception { + NotificationsList list = mapper.readValue(EXAMPLE_JSON, NotificationsList.class); + NotificationData n = list.notifications().get(0); + + AuthxType authx = n.condition().authx().get(0); + assertEquals("q", authx.feature()); + assertInstanceOf(NotificationExpression.AnyOfCondition.class, authx.type()); + assertInstanceOf(NotificationExpression.NoneOfCondition.class, authx.ssoScopes()); + + // Both actions parse (ShowMarketplace is unknown to the handler but must still deserialize cleanly). + assertEquals(2, n.actions().size()); + assertEquals("ShowMarketplace", n.actions().get(0).type()); + assertEquals("ShowUrl", n.actions().get(1).type()); + assertEquals("http://nowhere", n.actions().get(1).content().enUs().url()); + } + + @Test + void scheduleTypeMapsCaseInsensitivelyAndDefaultsToEmergency() { + assertEquals(NotificationScheduleType.STARTUP, NotificationScheduleType.fromString("startup")); + assertEquals(NotificationScheduleType.STARTUP, NotificationScheduleType.fromString("StartUp")); + assertEquals(NotificationScheduleType.EMERGENCY, NotificationScheduleType.fromString("Emergency")); + assertEquals(NotificationScheduleType.EMERGENCY, NotificationScheduleType.fromString("typo")); + assertEquals(NotificationScheduleType.EMERGENCY, NotificationScheduleType.fromString(null)); + } + + @Test + void severityMapsExactCaseAndDefaultsToInfo() { + assertEquals(NotificationSeverity.CRITICAL, NotificationSeverity.fromString("Critical")); + assertEquals(NotificationSeverity.WARNING, NotificationSeverity.fromString("Warning")); + assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString("Info")); + assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString("critical")); + assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString(null)); + } + + @Test + void parsesEmptyNotificationsList() throws Exception { + NotificationsList list = mapper.readValue( + "{ \"schema\": { \"version\": \"2.0\" }, \"notifications\": [] }", NotificationsList.class); + assertTrue(list.notifications().isEmpty()); + } + + @Test + void ignoresUnknownTopLevelKeysAndAllowsNullCondition() throws Exception { + NotificationsList list = mapper.readValue(""" + { + "schema": { "version": "2.0" }, + "futureField": "ignored", + "notifications": [ + { "id": "n1", "schedule": { "type": "Emergency" }, "severity": "Info", + "content": { "en-US": { "title": "t", "description": "d" } } } + ] + } + """, NotificationsList.class); + + NotificationData n = list.notifications().get(0); + assertNull(n.condition()); + assertNull(n.actions()); + assertEquals(NotificationScheduleType.EMERGENCY, n.schedule().type()); + } + + @Test + void rejectsMalformedExpression() { + // An operator object with two keys is not a valid single-operator expression. + String badJson = """ + { + "schema": { "version": "2.0" }, + "notifications": [ + { "id": "n1", "schedule": { "type": "Emergency" }, "severity": "Info", + "condition": { "os": { "type": { "==": "a", "!=": "b" } } }, + "content": { "en-US": { "title": "t", "description": "d" } } } + ] + } + """; + assertThrows(Exception.class, () -> mapper.readValue(badJson, NotificationsList.class)); + } +} diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java new file mode 100644 index 000000000..7c3aadff6 --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java @@ -0,0 +1,126 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.io.TempDir; + +import software.aws.toolkits.eclipse.amazonq.configuration.PluginStore; +import software.aws.toolkits.eclipse.amazonq.extensions.implementation.ActivatorStaticMockExtension; + +/** Covers the ETag fetch + graceful-degradation matrix with an injected mock HttpClient. */ +public final class NotificationsFetcherTest { + + private static final String URL = "https://example.com/Notifications/Eclipse/combined/2.x.json"; + private static final String VALID_JSON = + "{ \"schema\": { \"version\": \"2.0\" }, \"notifications\": [] }"; + + @RegisterExtension + private static ActivatorStaticMockExtension activatorExtension = new ActivatorStaticMockExtension(); + + private HttpClient httpClient; + private PluginStore pluginStore; + + @TempDir + private Path cacheDir; + + @BeforeEach + void setUp() { + httpClient = mock(HttpClient.class); + pluginStore = activatorExtension.getMock(PluginStore.class); + } + + private NotificationsFetcher fetcher() { + return new NotificationsFetcher(URL, httpClient, cacheDir.resolve("notifications.json")); + } + + @SuppressWarnings("unchecked") + private HttpResponse response(final int status, final String body) { + final HttpResponse resp = mock(HttpResponse.class); + when(resp.statusCode()).thenReturn(status); + when(resp.body()).thenReturn(body); + when(resp.headers()).thenReturn(java.net.http.HttpHeaders.of(java.util.Map.of(), (a, b) -> true)); + return resp; + } + + @Test + @SuppressWarnings("unchecked") + void ok200ParsesAndCaches() throws Exception { + doReturn(response(200, VALID_JSON)).when(httpClient).send(any(HttpRequest.class), any()); + final Optional result = fetcher().fetch(); + assertTrue(result.isPresent()); + assertTrue(Files.exists(cacheDir.resolve("notifications.json"))); + } + + @Test + @SuppressWarnings("unchecked") + void notFound404IsSilentNoOp() throws Exception { + doReturn(response(404, "")).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isEmpty()); + verify(activatorExtension.getMock(software.aws.toolkits.eclipse.amazonq.util.LoggingService.class), never()) + .error(any(String.class), any(Throwable.class)); + } + + @Test + @SuppressWarnings("unchecked") + void malformed200FallsBackToEmptyWhenNoCache() throws Exception { + doReturn(response(200, "{ not json")).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isEmpty()); + // A bad body must not be cached. + assertFalse(Files.exists(cacheDir.resolve("notifications.json"))); + } + + @Test + @SuppressWarnings("unchecked") + void notModified304WithMissingCacheClearsEtag() throws Exception { + when(pluginStore.get(URL)).thenReturn("\"etag-1\""); + doReturn(response(304, "")).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isEmpty()); + verify(pluginStore).remove(URL); + } + + @Test + @SuppressWarnings("unchecked") + void timeoutReturnsEmptyWithoutThrowing() throws Exception { + doThrow(new java.io.IOException("timeout")).when(httpClient).send(any(HttpRequest.class), any()); + assertDoesNotThrow(() -> assertTrue(fetcher().fetch().isEmpty())); + } + + @Test + void fileSchemeReadsLocalFileWithoutHttp() throws Exception { + final Path local = cacheDir.resolve("local.json"); + Files.writeString(local, VALID_JSON); + final NotificationsFetcher fileFetcher = + new NotificationsFetcher(local.toUri().toString(), httpClient, cacheDir.resolve("notifications.json")); + assertTrue(fileFetcher.fetch().isPresent()); + verify(httpClient, never()).send(any(), any()); + } + + @Test + void blankEndpointReadsCacheOnly() throws Exception { + final NotificationsFetcher blank = new NotificationsFetcher("", httpClient, cacheDir.resolve("notifications.json")); + assertEquals(Optional.empty(), blank.fetch()); + } +} diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java new file mode 100644 index 000000000..659c649a9 --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java @@ -0,0 +1,118 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import software.aws.toolkits.eclipse.amazonq.extensions.implementation.ActivatorStaticMockExtension; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.LocalizedContent; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationContent; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationSchedule; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationScheduleType; + +/** + * Covers the filtering/dedup/dismissal decisions that are impractical to verify manually (they depend on + * multiple 10-minute polls). Uses an injected display callback to capture which notifications would be shown. + */ +public final class ProcessNotificationsTest { + + @RegisterExtension + private static ActivatorStaticMockExtension activatorExtension = new ActivatorStaticMockExtension(); + + private NotificationDismissalStore dismissalStore; + private List shown; + private ProcessNotifications processor; + + @BeforeEach + void setUp() { + dismissalStore = mock(NotificationDismissalStore.class); + shown = new ArrayList<>(); + processor = new ProcessNotifications(dismissalStore, (id, n, c, actions) -> shown.add(id)); + } + + private static NotificationData notif(final String id, final NotificationScheduleType type) { + return new NotificationData(id, new NotificationSchedule(type), "Info", null, + new NotificationContent(new LocalizedContent("Title", "Description")), null); + } + + private static NotificationsList list(final NotificationData... notifications) { + return new NotificationsList(new NotificationsList.Schema("2.0"), List.of(notifications)); + } + + @Test + void startupShownOnlyOnFirstPoll() { + final NotificationsList payload = list(notif("startup1", NotificationScheduleType.STARTUP)); + processor.process(payload); + assertEquals(List.of("startup1"), shown); + + // Second poll: startup notification must NOT be shown again. + shown.clear(); + processor.process(payload); + assertTrue(shown.isEmpty()); + } + + @Test + void emergencyNotReshownInSameSession() { + final NotificationsList payload = list(notif("emerg1", NotificationScheduleType.EMERGENCY)); + processor.process(payload); + assertEquals(List.of("emerg1"), shown); + + // Same undismissed emergency on the next poll is suppressed by the in-session guard. + shown.clear(); + processor.process(payload); + assertTrue(shown.isEmpty()); + } + + @Test + void newEmergencyStillShownAfterAPriorOne() { + processor.process(list(notif("emerg1", NotificationScheduleType.EMERGENCY))); + shown.clear(); + processor.process(list(notif("emerg2", NotificationScheduleType.EMERGENCY))); + assertEquals(List.of("emerg2"), shown); + } + + @Test + void dismissedNotificationNeverShown() { + when(dismissalStore.isDismissed("emerg1")).thenReturn(true); + processor.process(list(notif("emerg1", NotificationScheduleType.EMERGENCY))); + assertTrue(shown.isEmpty()); + } + + @Test + void ruleFilteredNotificationNotShown() { + final NotificationData gated = new NotificationData("gated", + new NotificationSchedule(NotificationScheduleType.EMERGENCY), "Info", + new NotificationData.NotificationDisplayCondition(null, + new NotificationData.SystemType(new NotificationExpression.ComparisonCondition("NoSuchOS"), null), + null, null, null), + new NotificationContent(new LocalizedContent("t", "d")), null); + processor.process(list(gated)); + assertTrue(shown.isEmpty()); + } + + @Test + void blankContentSkipped() { + final NotificationData blank = new NotificationData("blank", + new NotificationSchedule(NotificationScheduleType.EMERGENCY), "Info", null, + new NotificationContent(new LocalizedContent("", "")), null); + processor.process(list(blank)); + assertTrue(shown.isEmpty()); + } + + @Test + void emptyListIsNoOp() { + processor.process(new NotificationsList(new NotificationsList.Schema("2.0"), List.of())); + assertTrue(shown.isEmpty()); + } +} diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java new file mode 100644 index 000000000..31272c76c --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java @@ -0,0 +1,153 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.AuthxType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.ComputeType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.ExtensionType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationDisplayCondition; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationSchedule; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.NotificationScheduleType; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationData.SystemType; + +/** Pure, deterministic coverage for the rules engine (top JaCoCo contributor; no mocks needed). */ +public final class RulesEngineTest { + + private static final String PLUGIN_ID = "amazon-q-eclipse"; + + private static SystemDetails sys(final String pluginVersion, final FeatureAuthDetails qAuth) { + return new SystemDetails("Local", "aarch64", "Mac OS X", "14.5.0", "Eclipse", "4.30.0", + Map.of(PLUGIN_ID, pluginVersion), qAuth); + } + + private static SystemDetails defaultSys() { + return sys("1.70.0", new FeatureAuthDetails("BuilderId", "us-east-1", "Connected")); + } + + private static NotificationData notification(final NotificationDisplayCondition condition) { + return new NotificationData("id", new NotificationSchedule(NotificationScheduleType.EMERGENCY), "Info", + condition, null, null); + } + + private static NotificationExpression eq(final String v) { + return new NotificationExpression.ComparisonCondition(v); + } + + @Test + void nullConditionShowsToEveryone() { + assertTrue(RulesEngine.displayNotification(notification(null), defaultSys())); + } + + @Test + void equalsAndNotEquals() { + assertTrue(RulesEngine.evaluateNotificationExpression(eq("a"), "a")); + assertFalse(RulesEngine.evaluateNotificationExpression(eq("a"), "b")); + assertTrue(RulesEngine.evaluateNotificationExpression(new NotificationExpression.NotEqualsCondition("a"), "b")); + assertFalse(RulesEngine.evaluateNotificationExpression(new NotificationExpression.NotEqualsCondition("a"), "a")); + } + + @Test + void semverOrderingForVersions() { + // 3.101.0 is NEWER than 3.74.0 numerically (lexical would say "1" < "7"). + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.GreaterThanCondition("3.74.0"), "3.101.0", true)); + assertFalse(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.LessThanCondition("3.74.0"), "3.101.0", true)); + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.GreaterThanOrEqualsCondition("1.0"), "1.0", true)); + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.LessThanOrEqualsCondition("2.0"), "2.0", true)); + } + + @Test + void malformedSemverFallsBackToLexical() { + // "abc" is not clean semver -> lexical compare: "abc" > "1.0". + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.GreaterThanCondition("1.0"), "abc", true)); + } + + @Test + void anyOfNoneOfNotOrAnd() { + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.AnyOfCondition(List.of("Darwin", "Linux")), "Darwin")); + assertFalse(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.NoneOfCondition(List.of("Darwin")), "Darwin")); + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.NotCondition(eq("x")), "y")); + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.OrCondition(List.of(eq("a"), eq("b"))), "b")); + assertFalse(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.AndCondition(List.of(eq("a"), eq("b"))), "a")); + } + + @Test + void computeOsIdeConditionsAreAnded() { + final NotificationDisplayCondition cond = new NotificationDisplayCondition( + new ComputeType(eq("Local"), null), + new SystemType(new NotificationExpression.AnyOfCondition(List.of("Mac OS X")), null), + new SystemType(eq("Eclipse"), new NotificationExpression.GreaterThanOrEqualsCondition("4.0.0")), + null, null); + assertTrue(RulesEngine.displayNotification(notification(cond), defaultSys())); + + final NotificationDisplayCondition mismatch = new NotificationDisplayCondition( + new ComputeType(eq("Remote"), null), null, null, null, null); + assertFalse(RulesEngine.displayNotification(notification(mismatch), defaultSys())); + } + + @Test + void extensionNoneInstalledDoesNotShow() { + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, + List.of(new ExtensionType("not.installed.ext", null)), null); + assertFalse(RulesEngine.displayNotification(notification(cond), defaultSys())); + } + + @Test + void extensionInstalledWithMatchingVersionShows() { + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, + List.of(new ExtensionType(PLUGIN_ID, new NotificationExpression.LessThanCondition("2.0.0"))), null); + assertTrue(RulesEngine.displayNotification(notification(cond), defaultSys())); + } + + @Test + void snapshotPluginVersionNeverShows() { + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, + List.of(new ExtensionType(PLUGIN_ID, null)), null); + assertFalse(RulesEngine.displayNotification(notification(cond), sys("1.70.0-SNAPSHOT", + new FeatureAuthDetails("BuilderId", "us-east-1", "Connected")))); + } + + @Test + void authxMatchingForQFeature() { + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, null, + List.of(new AuthxType("q", null, null, eq("Connected"), null))); + assertTrue(RulesEngine.displayNotification(notification(cond), defaultSys())); + + final NotificationDisplayCondition wantExpired = new NotificationDisplayCondition(null, null, null, null, + List.of(new AuthxType("q", null, null, eq("Expired"), null))); + assertFalse(RulesEngine.displayNotification(notification(wantExpired), defaultSys())); + } + + @Test + void authxNonQFeaturePasses() { + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, null, + List.of(new AuthxType("codeCatalyst", eq("whatever"), null, null, null))); + assertTrue(RulesEngine.displayNotification(notification(cond), defaultSys())); + } + + @Test + void loggedOutIsEvaluableAsNotConnected() { + final SystemDetails loggedOut = sys("1.70.0", new FeatureAuthDetails("Unknown", "Unknown", "NotConnected")); + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, null, + List.of(new AuthxType("q", null, null, eq("NotConnected"), null))); + assertTrue(RulesEngine.displayNotification(notification(cond), loggedOut)); + } +} From 26c67f7cf9178771886ddbce5f6d84edbd4bfe9f Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Wed, 15 Jul 2026 12:27:22 -0700 Subject: [PATCH 02/10] fix(notifications): point changelog action at the releases page The OpenChangelog notification action opened https://github.com/aws/amazon-q-eclipse/blob/main/CHANGELOG.md, which 404s (there is no CHANGELOG.md in the repo). Point AMAZON_Q_CHANGELOG_URL at the GitHub releases page (the de-facto changelog for the repo) so 'View changelog' resolves. --- .../software/aws/toolkits/eclipse/amazonq/util/Constants.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java index 8b3b24a25..82294da69 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java @@ -59,6 +59,6 @@ private Constants() { public static final String INLINE_CHAT_EXPIRED_AUTH_BODY = "Login status expired; please open Q plugin window to reauthenticate."; public static final String INLINE_CHAT_CONTEXT_ID = "org.eclipse.ui.inlineChatContext"; public static final String INLINE_SUGGESTIONS_CONTEXT_ID = "org.eclipse.ui.suggestionsContext"; - public static final String AMAZON_Q_CHANGELOG_URL = "https://github.com/aws/amazon-q-eclipse/blob/main/CHANGELOG.md"; + public static final String AMAZON_Q_CHANGELOG_URL = "https://github.com/aws/amazon-q-eclipse/releases"; public static final String AMAZON_Q_UPDATE_SITE_URL = "https://marketplace.eclipse.org/content/amazon-q"; } From 6158f7464b92c09bed995d96a59356e4564d62f0 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Wed, 15 Jul 2026 12:44:21 -0700 Subject: [PATCH 03/10] docs(notifications): reword persistence comment to drop internal term Replace an internal-only reference in the CRITICAL-persistence javadoc with a provider-neutral phrasing; no behavior change. --- .../eclipse/amazonq/notifications/AmazonQNotificationPopup.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java index 83cf88e76..c54dfbbac 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/AmazonQNotificationPopup.java @@ -26,7 +26,7 @@ /** * A toast notification that renders a severity icon, wrapped description, and N action buttons built from a hosted * notification's actions. INFO/WARNING keep the base auto-close timer; CRITICAL overrides {@link #scheduleAutoClose()} - * to a no-op so it persists until the user dismisses it (the COE "reach the user" requirement). + * to a no-op so it persists until the user dismisses it, ensuring critical alerts reach the user. */ public final class AmazonQNotificationPopup extends ToolkitNotification { From 883509027c7ab3ad140da787b6ece1fc65e39ab7 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Fri, 17 Jul 2026 12:46:20 -0700 Subject: [PATCH 04/10] fix(notifications): harden targeting, startup, and fetch correctness Correctness/robustness fixes surfaced by review of the notifications feature: - ETag: persist the response ETag after a successful 200 so the conditional GET (If-None-Match) actually works; previously the ETag was never written, so every poll re-downloaded and the 304 path was unreachable. Also clean up the temp cache file if the atomic move fails. - Version targeting: sanitize the plugin Bundle-Version to major.minor.micro (drop the OSGi qualifier) before rules evaluation, so extension.version conditions compare with semver instead of lexical ordering (e.g. 2.7.4 < 2.7.10). - Dev builds: suppress polling on unreleased/dev builds (qualifier == "qualifier" or contains "snapshot") unless an explicit endpoint override is set, so dev builds don't receive production notifications while local testing still works. - STARTUP-once: consume the startup window only when a STARTUP notification actually renders, not merely when the first poll runs, so one filtered on the first poll can still show later in the session once it qualifies. - Telemetry accuracy: emit showNotification and mark the id shown only after the toast actually renders (completion callback); on skip/failure the id is un-marked so it retries on a later poll. - First poll no longer runs synchronously in start(); it is scheduled so start() does not block the shared startup worker thread on network I/O. - Batch resilience: skip null notification elements and isolate per-notification processing so one bad entry cannot abort the whole poll. - Severity parsing is case-insensitive (a mis-cased "critical" no longer downgrades to an auto-fading INFO toast). - Tighten the stop()/reschedule() race so a poll cannot be armed after shutdown. Adds tests: ETag write + If-None-Match, 304-with-cache, 500/offline degradation, clean-version extension targeting, case-insensitive severity, STARTUP filtered-then- qualifies, startup-window-after-render, render-failure retry, null batch element. --- .../notifications/NotificationData.java | 9 ++- .../NotificationPollingService.java | 22 ++++++- .../NotificationPreferences.java | 14 ++++ .../notifications/NotificationsFetcher.java | 20 +++++- .../notifications/ProcessNotifications.java | 51 ++++++++++++--- .../notifications/SystemDetailsCollector.java | 31 ++++++++- .../NotificationParsingTest.java | 10 ++- .../NotificationsFetcherTest.java | 58 +++++++++++++++++ .../ProcessNotificationsTest.java | 65 ++++++++++++++++++- .../notifications/RulesEngineTest.java | 20 ++++++ 10 files changed, 277 insertions(+), 23 deletions(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java index 0ce5e2b94..a06c7553b 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationData.java @@ -47,12 +47,15 @@ public enum NotificationSeverity { WARNING, CRITICAL; - /** Maps the exact-case JSON value; any unrecognized or {@code null} value yields {@link #INFO}. */ + /** Maps the JSON value case-insensitively; any unrecognized or {@code null} value yields {@link #INFO}. */ public static NotificationSeverity fromString(final String value) { - if ("Critical".equals(value)) { + if (value == null) { + return INFO; + } + if ("Critical".equalsIgnoreCase(value)) { return CRITICAL; } - if ("Warning".equals(value)) { + if ("Warning".equalsIgnoreCase(value)) { return WARNING; } return INFO; diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java index 10eafbab0..ab3d083ed 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java @@ -41,12 +41,20 @@ public void start() { if (!NotificationPreferences.isNotificationsEnabled()) { return; } + // Development/unreleased builds must not receive production notifications. Allow an explicit endpoint + // override (preference or env var) so local/demo testing against a test endpoint still works. + if (SystemDetailsCollector.isDevBuild() && !NotificationPreferences.hasEndpointOverride()) { + Activator.getLogger().info("Notifications polling skipped: development build with no endpoint override"); + return; + } if (!started.compareAndSet(false, true)) { return; } this.fetcher = new NotificationsFetcher(NotificationPreferences.resolveEndpoint()); this.processor = new ProcessNotifications(new NotificationDismissalStore()); - pollOnce(); + // Schedule the first poll instead of running it inline so start() never blocks its caller (the shared + // startup worker thread) on network I/O. + scheduledPoll = schedulePoll(0L); } private void pollOnce() { @@ -67,10 +75,20 @@ private void reschedule() { if (stopped || !NotificationPreferences.isNotificationsEnabled()) { return; } + scheduledPoll = schedulePoll(POLL_INTERVAL_MS); + // If stop() ran concurrently between the guard above and the assignment, cancel the future we just armed + // so a poll cannot fire after shutdown. + if (stopped && scheduledPoll != null) { + scheduledPoll.cancel(false); + } + } + + private ScheduledFuture schedulePoll(final long delayMs) { try { - scheduledPoll = (ScheduledFuture) ThreadingUtils.scheduleAsyncTaskWithDelay(this::pollOnce, POLL_INTERVAL_MS); + return (ScheduledFuture) ThreadingUtils.scheduleAsyncTaskWithDelay(this::pollOnce, delayMs); } catch (RejectedExecutionException e) { Activator.getLogger().info("Notifications polling stopped (worker pool shutting down)"); + return null; } } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java index 8b1e9b043..399f70fb9 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPreferences.java @@ -31,4 +31,18 @@ public static String resolveEndpoint() { } return NotificationConstants.NOTIFICATIONS_ENDPOINT; } + + /** + * Whether an explicit endpoint override (preference or environment variable) is set. Used to let a + * development/PDE build opt in to polling a test endpoint, which is otherwise suppressed on dev builds. + */ + public static boolean hasEndpointOverride() { + final String pref = Activator.getDefault().getPreferenceStore() + .getString(AmazonQPreferencePage.NOTIFICATIONS_ENDPOINT_OVERRIDE); + if (pref != null && !pref.isBlank()) { + return true; + } + final String env = System.getenv(NotificationConstants.NOTIFICATIONS_ENDPOINT_ENV); + return env != null && !env.isBlank(); + } } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java index bfa304329..807029d3a 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java @@ -89,7 +89,7 @@ private Optional fetchRemoteWithRetries() { return Optional.empty(); } if (status == HttpURLConnection.HTTP_OK) { - return validateAndCache(response.body()); + return validateAndCache(response); } // 403/404 (file not deployed yet) and any other non-2xx: not an error condition, show nothing. Activator.getLogger().info("No notifications available (HTTP " + status + ")"); @@ -159,19 +159,33 @@ private Optional validate(final String content) { } } - private Optional validateAndCache(final String body) { + private Optional validateAndCache(final HttpResponse response) { + final String body = response.body(); final Optional parsed = validate(body); if (parsed.isEmpty()) { // Do not cache a bad body; keep any prior valid cache untouched. return getResourceFromCache(); } + Path tmp = null; try { - final Path tmp = cachePath.resolveSibling(cachePath.getFileName() + ".tmp"); + tmp = cachePath.resolveSibling(cachePath.getFileName() + ".tmp"); Files.createDirectories(cachePath.getParent()); Files.writeString(tmp, body); Files.move(tmp, cachePath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + // Persist the ETag ONLY after the cache write succeeds, so a later If-None-Match 304 can be honored by + // the on-disk cache. Storing the ETag without a matching cache would make 304s unserveable. + response.headers().firstValue("ETag") + .ifPresent(etag -> Activator.getPluginStore().put(endpointUrl, etag)); } catch (Exception e) { Activator.getLogger().warn("Failed to cache notifications file", e); + // Clean up a leaked temp file if the atomic move did not consume it. + if (tmp != null) { + try { + Files.deleteIfExists(tmp); + } catch (Exception cleanupError) { + Activator.getLogger().warn("Failed to delete temp notifications file", cleanupError); + } + } } return parsed; } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java index c94656561..571b3813a 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotifications.java @@ -26,13 +26,18 @@ */ public final class ProcessNotifications { - /** Renders a notification that has passed all filtering. Injectable so tests can observe without SWT. */ + /** + * Renders a notification that has passed all filtering. Injectable so tests can observe without SWT. + * The {@code completion} consumer must be invoked with {@code true} once the toast has actually rendered + * (so we only then commit telemetry + consume the startup window) or {@code false} if rendering was skipped + * or failed (so the notification can be retried on a later poll). + */ public interface NotificationDisplay { void show(String id, NotificationData notification, LocalizedContent content, - List actions); + List actions, java.util.function.Consumer completion); } - private final AtomicBoolean isFirstPoll = new AtomicBoolean(true); + private final AtomicBoolean startupWindowOpen = new AtomicBoolean(true); private final Set shownThisSession = ConcurrentHashMap.newKeySet(); private final NotificationDismissalStore dismissalStore; private final NotificationDisplay display; @@ -50,22 +55,34 @@ public void process(final NotificationsList list) { if (list == null || list.notifications() == null || list.notifications().isEmpty()) { return; } - final boolean isStartupPoll = isFirstPoll.compareAndSet(true, false); + // Whether STARTUP notifications are still eligible this session. Consumed only once a STARTUP notification + // actually survives all filters and is displayed (see processOne) — NOT merely because the first poll ran — + // so a STARTUP item that is dismissed/rule-filtered/blank on the first poll can still show on a later poll + // in the same session once it qualifies. + final boolean startupEligible = startupWindowOpen.get(); final SystemDetails sys = SystemDetailsCollector.collect(); for (final NotificationData notification : list.notifications()) { - processOne(notification, isStartupPoll, sys); + if (notification == null) { + continue; + } + try { + processOne(notification, startupEligible, sys); + } catch (Exception e) { + Activator.getLogger().warn("Skipping notification that failed to process: " + notification.id(), e); + } } } - private void processOne(final NotificationData notification, final boolean isStartupPoll, final SystemDetails sys) { + private void processOne(final NotificationData notification, final boolean startupEligible, + final SystemDetails sys) { final String id = notification.id(); if (id == null) { return; } final boolean isStartup = notification.schedule() != null && notification.schedule().type() == NotificationScheduleType.STARTUP; - if (isStartup && !isStartupPoll) { + if (isStartup && !startupEligible) { return; } if (dismissalStore.isDismissed(id)) { @@ -91,24 +108,38 @@ private void processOne(final NotificationData notification, final boolean isSta // The explicit "Dismiss" button persists the dismissal so the notification does not reappear; // closing/auto-fading or clicking another action does NOT dismiss (an emergency re-shows next session). actions.add(new NotificationAction("Dismiss", () -> dismissalStore.dismiss(id))); - NotificationTelemetryProvider.emitShowNotification(id); - display.show(id, notification, content, actions); + final boolean isStartupNotification = isStartup; + // Telemetry + startup-window consumption are committed only after the toast actually renders (completion + // == true). If rendering is skipped/failed, un-mark it so a later poll can retry. + display.show(id, notification, content, actions, rendered -> { + if (Boolean.TRUE.equals(rendered)) { + NotificationTelemetryProvider.emitShowNotification(id); + if (isStartupNotification) { + startupWindowOpen.set(false); + } + } else { + shownThisSession.remove(id); + } + }); } private static void showToast(final String id, final NotificationData notification, final LocalizedContent content, - final List actions) { + final List actions, final java.util.function.Consumer completion) { final NotificationSeverity severity = NotificationSeverity.fromString(notification.severity()); Activator.getLogger().info("Showing notification toast: " + id + " (severity=" + severity + ")"); Display.getDefault().asyncExec(() -> { if (!PlatformUI.isWorkbenchRunning()) { Activator.getLogger().info("Workbench not running; skipping notification toast: " + id); + completion.accept(false); return; } try { new AmazonQNotificationPopup(Display.getCurrent(), content.title(), content.description(), severity, actions).open(); + completion.accept(true); } catch (Exception e) { Activator.getLogger().error("Failed to render notification toast: " + id, e); + completion.accept(false); } }); } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java index 03c94f023..e47194b70 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/SystemDetailsCollector.java @@ -29,7 +29,9 @@ private SystemDetailsCollector() { public static SystemDetails collect() { final Bundle pluginBundle = FrameworkUtil.getBundle(SystemDetailsCollector.class); final String pluginId = pluginBundle != null ? pluginBundle.getSymbolicName() : UNKNOWN; - final String pluginVersion = pluginBundle != null ? pluginBundle.getVersion().toString() : UNKNOWN; + // Use a clean major.minor.micro string (drop the OSGi qualifier, e.g. "2.7.4.202607161757") so the rules + // engine compares extension.version with semver, not lexical ordering. See resolveIdeVersion for the same. + final String pluginVersion = pluginBundle != null ? cleanVersion(pluginBundle.getVersion()) : UNKNOWN; return new SystemDetails( "Local", @@ -48,12 +50,37 @@ public static String pluginId() { return pluginBundle != null ? pluginBundle.getSymbolicName() : UNKNOWN; } + /** + * Whether this is an unreleased/development build. Tycho replaces the {@code .qualifier} segment with a numeric + * build timestamp at release time, so a bundle whose qualifier is still the literal {@code "qualifier"} (PDE/dev + * launch) or contains {@code "snapshot"} is a development build that should not receive production notifications. + */ + public static boolean isDevBuild() { + final Bundle pluginBundle = FrameworkUtil.getBundle(SystemDetailsCollector.class); + if (pluginBundle == null) { + return true; + } + final String qualifier = pluginBundle.getVersion().getQualifier(); + if (qualifier == null || qualifier.isBlank()) { + return false; + } + final String q = qualifier.toLowerCase(java.util.Locale.ROOT); + return q.equals("qualifier") || q.contains("snapshot"); + } + private static String resolveIdeVersion() { final Bundle platform = Platform.getBundle(PLATFORM_BUNDLE_ID); if (platform == null) { return UNKNOWN; } - final Version v = platform.getVersion(); + return cleanVersion(platform.getVersion()); + } + + /** Renders an OSGi {@link Version} as clean {@code major.minor.micro}, dropping the qualifier segment. */ + private static String cleanVersion(final Version v) { + if (v == null) { + return UNKNOWN; + } return v.getMajor() + "." + v.getMinor() + "." + v.getMicro(); } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java index fccf606b6..b9bdece27 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java @@ -159,11 +159,17 @@ void scheduleTypeMapsCaseInsensitivelyAndDefaultsToEmergency() { } @Test - void severityMapsExactCaseAndDefaultsToInfo() { + void severityMapsCaseInsensitivelyAndDefaultsToInfo() { assertEquals(NotificationSeverity.CRITICAL, NotificationSeverity.fromString("Critical")); assertEquals(NotificationSeverity.WARNING, NotificationSeverity.fromString("Warning")); assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString("Info")); - assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString("critical")); + // Case-insensitive: a mis-cased "critical" must NOT silently downgrade to INFO. + assertEquals(NotificationSeverity.CRITICAL, NotificationSeverity.fromString("critical")); + assertEquals(NotificationSeverity.CRITICAL, NotificationSeverity.fromString("CRITICAL")); + assertEquals(NotificationSeverity.WARNING, NotificationSeverity.fromString("warning")); + assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString("info")); + // Unrecognized and null still default to INFO. + assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString("bogus")); assertEquals(NotificationSeverity.INFO, NotificationSeverity.fromString(null)); } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java index 7c3aadff6..9b4685980 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java @@ -65,6 +65,16 @@ private HttpResponse response(final int status, final String body) { return resp; } + @SuppressWarnings("unchecked") + private HttpResponse responseWithEtag(final int status, final String body, final String etag) { + final HttpResponse resp = mock(HttpResponse.class); + when(resp.statusCode()).thenReturn(status); + when(resp.body()).thenReturn(body); + when(resp.headers()).thenReturn( + java.net.http.HttpHeaders.of(java.util.Map.of("ETag", java.util.List.of(etag)), (a, b) -> true)); + return resp; + } + @Test @SuppressWarnings("unchecked") void ok200ParsesAndCaches() throws Exception { @@ -123,4 +133,52 @@ void blankEndpointReadsCacheOnly() throws Exception { final NotificationsFetcher blank = new NotificationsFetcher("", httpClient, cacheDir.resolve("notifications.json")); assertEquals(Optional.empty(), blank.fetch()); } + + @Test + @SuppressWarnings("unchecked") + void ok200WithEtagStoresEtagForConditionalGet() throws Exception { + doReturn(responseWithEtag(200, VALID_JSON, "\"etag-42\"")).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isPresent()); + // The ETag must be persisted (keyed by URL) so the next poll can send If-None-Match. + verify(pluginStore).put(URL, "\"etag-42\""); + } + + @Test + @SuppressWarnings("unchecked") + void subsequentPollSendsIfNoneMatchWhenCachedAndEtagPresent() throws Exception { + // Seed a cached file and a stored ETag, then confirm the next request carries If-None-Match. + Files.writeString(cacheDir.resolve("notifications.json"), VALID_JSON); + when(pluginStore.get(URL)).thenReturn("\"etag-42\""); + final org.mockito.ArgumentCaptor captor = org.mockito.ArgumentCaptor.forClass(HttpRequest.class); + doReturn(response(304, "")).when(httpClient).send(captor.capture(), any()); + fetcher().fetch(); + assertEquals(Optional.of("\"etag-42\""), captor.getValue().headers().firstValue("If-None-Match")); + } + + @Test + @SuppressWarnings("unchecked") + void notModified304WithCachePresentReturnsCachedWithoutReDownload() throws Exception { + Files.writeString(cacheDir.resolve("notifications.json"), VALID_JSON); + when(pluginStore.get(URL)).thenReturn("\"etag-42\""); + doReturn(response(304, "")).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isPresent()); + } + + @Test + @SuppressWarnings("unchecked") + void serverError500IsSilentNoOp() throws Exception { + doReturn(response(500, "")).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isEmpty()); + verify(activatorExtension.getMock(software.aws.toolkits.eclipse.amazonq.util.LoggingService.class), never()) + .error(any(String.class), any(Throwable.class)); + } + + @Test + @SuppressWarnings("unchecked") + void networkFailureFallsBackToCacheWhenPresent() throws Exception { + Files.writeString(cacheDir.resolve("notifications.json"), VALID_JSON); + doThrow(new java.io.IOException("offline")).when(httpClient).send(any(HttpRequest.class), any()); + // With a valid cache, a persistent network failure serves the cached payload rather than empty. + assertTrue(fetcher().fetch().isPresent()); + } } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java index 659c649a9..67dd54fd5 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/ProcessNotificationsTest.java @@ -38,7 +38,19 @@ public final class ProcessNotificationsTest { void setUp() { dismissalStore = mock(NotificationDismissalStore.class); shown = new ArrayList<>(); - processor = new ProcessNotifications(dismissalStore, (id, n, c, actions) -> shown.add(id)); + // Display that reports a successful render (completion=true), mirroring a real rendered toast. + processor = new ProcessNotifications(dismissalStore, (id, n, c, actions, completion) -> { + shown.add(id); + completion.accept(true); + }); + } + + /** Builds a processor whose display reports render failure, to exercise the retry/no-commit path. */ + private ProcessNotifications processorWithFailingDisplay() { + return new ProcessNotifications(dismissalStore, (id, n, c, actions, completion) -> { + shown.add(id); + completion.accept(false); + }); } private static NotificationData notif(final String id, final NotificationScheduleType type) { @@ -115,4 +127,55 @@ void emptyListIsNoOp() { processor.process(new NotificationsList(new NotificationsList.Schema("2.0"), List.of())); assertTrue(shown.isEmpty()); } + + @Test + void startupFilteredOnFirstPollStillShowsOnceItQualifies() { + // Poll 1: the STARTUP notification is dismissed, so it is filtered out. + when(dismissalStore.isDismissed("startup1")).thenReturn(true); + final NotificationsList payload = list(notif("startup1", NotificationScheduleType.STARTUP)); + processor.process(payload); + assertTrue(shown.isEmpty()); + + // Poll 2 (same session): it is no longer dismissed. The startup window must still be open, because it was + // consumed by an actual display, not merely by the first poll running. + shown.clear(); + when(dismissalStore.isDismissed("startup1")).thenReturn(false); + processor.process(payload); + assertEquals(List.of("startup1"), shown); + } + + @Test + void startupWindowConsumedOnlyAfterActualDisplay() { + final NotificationsList payload = list(notif("startup1", NotificationScheduleType.STARTUP)); + processor.process(payload); + assertEquals(List.of("startup1"), shown); + + // A DIFFERENT startup notification appearing later in the same session must NOT show: the window closed + // once startup1 rendered. + shown.clear(); + processor.process(list(notif("startup2", NotificationScheduleType.STARTUP))); + assertTrue(shown.isEmpty()); + } + + @Test + void renderFailureIsRetriedOnNextPoll() { + final ProcessNotifications failing = processorWithFailingDisplay(); + final NotificationsList payload = list(notif("emerg1", NotificationScheduleType.EMERGENCY)); + failing.process(payload); + assertEquals(List.of("emerg1"), shown); + + // Because rendering failed (completion=false), the id was un-marked, so the next poll retries it. + shown.clear(); + failing.process(payload); + assertEquals(List.of("emerg1"), shown); + } + + @Test + void nullElementInBatchIsSkipped() { + final List withNull = new ArrayList<>(); + withNull.add(null); + withNull.add(notif("emerg1", NotificationScheduleType.EMERGENCY)); + processor.process(new NotificationsList(new NotificationsList.Schema("2.0"), withNull)); + assertEquals(List.of("emerg1"), shown); + } } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java index 31272c76c..ff12e2dfc 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java @@ -117,6 +117,26 @@ void extensionInstalledWithMatchingVersionShows() { assertTrue(RulesEngine.displayNotification(notification(cond), defaultSys())); } + @Test + void extensionVersionEqualsMatchesCleanReleaseVersion() { + // SystemDetailsCollector sanitizes the OSGi Bundle-Version (e.g. "2.7.4.202607161757") to clean + // major.minor.micro "2.7.4" before it reaches the engine, so exact-equality targeting works. + final SystemDetails release = sys("2.7.4", new FeatureAuthDetails("BuilderId", "us-east-1", "Connected")); + final NotificationDisplayCondition eqCond = new NotificationDisplayCondition(null, null, null, + List.of(new ExtensionType(PLUGIN_ID, eq("2.7.4"))), null); + assertTrue(RulesEngine.displayNotification(notification(eqCond), release)); + + // Boundary operators compare with semver, not lexical: 2.7.4 <= 2.7.10 and 2.7.4 < 2.7.10. + final NotificationDisplayCondition ltCond = new NotificationDisplayCondition(null, null, null, + List.of(new ExtensionType(PLUGIN_ID, new NotificationExpression.LessThanCondition("2.7.10"))), null); + assertTrue(RulesEngine.displayNotification(notification(ltCond), release)); + + // A user already on the fixed version must NOT match "< 2.7.4". + final NotificationDisplayCondition fixedCond = new NotificationDisplayCondition(null, null, null, + List.of(new ExtensionType(PLUGIN_ID, new NotificationExpression.LessThanCondition("2.7.4"))), null); + assertFalse(RulesEngine.displayNotification(notification(fixedCond), release)); + } + @Test void snapshotPluginVersionNeverShows() { final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, null, null, From 93c9d98a3a17214faa657df9cd9c770c9fb027c0 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Fri, 17 Jul 2026 12:51:57 -0700 Subject: [PATCH 05/10] fix(notifications): make polling restartable, harden dismissal store, bound fetch - Kill-switch is now reversible in-session: a preference-change listener pauses polling when disabled and resumes it when re-enabled, instead of requiring an IDE restart. NotificationPollingService is refactored into a restartable lifecycle (start / pause via onEnabledPreferenceChanged / permanent shutdown) with an injectable scheduler + suppliers. - Dismissal store: persist the reset when stored state is corrupt (so it stops re-parsing and re-warning on the bad value every poll), and make the id comparison null-safe so a stored entry with a null id cannot NPE and abort all notification processing. - Fetch: tighten the per-request timeout (30s -> 10s) and backoff base (1s -> 0.5s) so a poll cannot occupy a shared worker-pool thread for tens of seconds on a slow/blocked network. Adds NotificationPollingServiceTest (start-once, kill-switch off/on resume, dev-build gating, reschedule-at-interval, shutdown-cancels-and-prevents-rearm, start-after-shutdown) and dismissal tests for corrupt-state repair and null-id safety. --- .../NotificationDismissalStore.java | 15 +- .../NotificationPollingService.java | 138 ++++++++++---- .../notifications/NotificationsFetcher.java | 7 +- .../eclipse/amazonq/plugin/Activator.java | 2 +- .../AmazonQPreferenceInitializer.java | 5 + .../NotificationDismissalStoreTest.java | 27 +++ .../NotificationPollingServiceTest.java | 171 ++++++++++++++++++ 7 files changed, 324 insertions(+), 41 deletions(-) create mode 100644 plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java index b1f44909f..b65674eb8 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStore.java @@ -29,13 +29,14 @@ public NotificationDismissalStore(final PluginStore pluginStore) { } public synchronized boolean isDismissed(final String id) { - return loadAndClean().getDismissedNotifications().stream().anyMatch(d -> d.getId().equals(id)); + // Anchor equals() on the argument so a persisted entry with a null id cannot NPE and abort processing. + return loadAndClean().getDismissedNotifications().stream().anyMatch(d -> id.equals(d.getId())); } public synchronized void dismiss(final String id) { final NotificationDismissalConfiguration config = loadAndClean(); final List dismissed = config.getDismissedNotifications(); - if (dismissed.stream().anyMatch(d -> d.getId().equals(id))) { + if (dismissed.stream().anyMatch(d -> id.equals(d.getId()))) { return; } dismissed.add(new DismissedNotification(id, Instant.now().toEpochMilli())); @@ -44,15 +45,23 @@ public synchronized void dismiss(final String id) { private NotificationDismissalConfiguration loadAndClean() { NotificationDismissalConfiguration config; + boolean corrupt = false; try { config = pluginStore.getObject(NotificationConstants.DISMISSAL_STORAGE_KEY, NotificationDismissalConfiguration.class); } catch (Exception e) { Activator.getLogger().warn("Corrupt notification dismissal state; resetting", e); config = null; + corrupt = true; } if (config == null || config.getDismissedNotifications() == null) { - return new NotificationDismissalConfiguration(); + final NotificationDismissalConfiguration fresh = new NotificationDismissalConfiguration(); + // If the stored bytes were corrupt, persist the reset once so we stop re-parsing (and re-warning on) + // the bad value every poll. + if (corrupt) { + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, fresh); + } + return fresh; } final Instant cutoff = Instant.now().minus(RETENTION); final boolean removedAny = config.getDismissedNotifications() diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java index ab3d083ed..26a5ed54f 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java @@ -6,59 +6,110 @@ import java.time.Duration; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiFunction; +import java.util.function.Supplier; import software.aws.toolkits.eclipse.amazonq.plugin.Activator; import software.aws.toolkits.eclipse.amazonq.util.ThreadingUtils; /** - * App-level singleton that polls the notifications endpoint every 10 minutes on the shared worker pool, self-rescheduling - * after each poll. The poll body is total (fetch never throws; the work is wrapped so an escaped error cannot cancel the - * loop) and the re-arm happens in a {@code finally}. {@link #stop()} must be called early in {@code Activator.stop()} to - * cancel the pending future and prevent a re-arm during teardown. + * App-level singleton that polls the notifications endpoint every 10 minutes on the shared worker pool, + * self-rescheduling after each poll. The poll body is total (fetch never throws) and the re-arm happens in a + * {@code finally}. + * + *

Lifecycle: {@link #start()} is called once at startup; {@link #shutdown()} must be called early in + * {@code Activator.stop()} to permanently cancel polling during teardown. {@link #onEnabledPreferenceChanged()} lets + * the notifications kill-switch pause/resume polling within a session without an IDE restart. + * + *

The scheduler and collaborators are injectable via a package-private constructor so unit tests can drive + * start/stop/reschedule deterministically without SWT, the network, or a real thread pool. */ public final class NotificationPollingService { private static final NotificationPollingService INSTANCE = new NotificationPollingService(); private static final long POLL_INTERVAL_MS = Duration.ofMinutes(10).toMillis(); - private final AtomicBoolean started = new AtomicBoolean(false); - private volatile boolean stopped; + /** Abstracts the scheduler so tests can inject a deterministic one; returns a cancellable handle or null. */ + interface PollScheduler { + ScheduledFuture schedule(Runnable task, long delayMs); + } + + private final Supplier enabledSupplier; + private final Supplier devBuildSupplier; + private final Supplier endpointOverrideSupplier; + private final Supplier fetcherSupplier; + private final Supplier processorSupplier; + private final PollScheduler scheduler; + + private volatile boolean shutdown; + private volatile boolean running; private volatile ScheduledFuture scheduledPoll; private volatile NotificationsFetcher fetcher; private volatile ProcessNotifications processor; private NotificationPollingService() { - // singleton + this( + NotificationPreferences::isNotificationsEnabled, + SystemDetailsCollector::isDevBuild, + NotificationPreferences::hasEndpointOverride, + () -> new NotificationsFetcher(NotificationPreferences.resolveEndpoint()), + () -> new ProcessNotifications(new NotificationDismissalStore()), + defaultScheduler()); + } + + // Package-private for tests. + NotificationPollingService(final Supplier enabledSupplier, final Supplier devBuildSupplier, + final Supplier endpointOverrideSupplier, final Supplier fetcherSupplier, + final Supplier processorSupplier, final PollScheduler scheduler) { + this.enabledSupplier = enabledSupplier; + this.devBuildSupplier = devBuildSupplier; + this.endpointOverrideSupplier = endpointOverrideSupplier; + this.fetcherSupplier = fetcherSupplier; + this.processorSupplier = processorSupplier; + this.scheduler = scheduler; + } + + private static PollScheduler defaultScheduler() { + final BiFunction> sched = + (task, delay) -> (ScheduledFuture) ThreadingUtils.scheduleAsyncTaskWithDelay(task, delay); + return (task, delayMs) -> { + try { + return sched.apply(task, delayMs); + } catch (RejectedExecutionException e) { + Activator.getLogger().info("Notifications polling stopped (worker pool shutting down)"); + return null; + } + }; } public static NotificationPollingService getInstance() { return INSTANCE; } - /** Starts polling once per app lifetime; no-op if the kill-switch is off or polling already started. */ - public void start() { - if (!NotificationPreferences.isNotificationsEnabled()) { + /** Starts polling once per app lifetime; no-op if disabled, a dev build without override, or already running. */ + public synchronized void start() { + if (shutdown || running) { + return; + } + if (!enabledSupplier.get()) { return; } // Development/unreleased builds must not receive production notifications. Allow an explicit endpoint // override (preference or env var) so local/demo testing against a test endpoint still works. - if (SystemDetailsCollector.isDevBuild() && !NotificationPreferences.hasEndpointOverride()) { + if (devBuildSupplier.get() && !endpointOverrideSupplier.get()) { Activator.getLogger().info("Notifications polling skipped: development build with no endpoint override"); return; } - if (!started.compareAndSet(false, true)) { - return; - } - this.fetcher = new NotificationsFetcher(NotificationPreferences.resolveEndpoint()); - this.processor = new ProcessNotifications(new NotificationDismissalStore()); + running = true; + this.fetcher = fetcherSupplier.get(); + this.processor = processorSupplier.get(); // Schedule the first poll instead of running it inline so start() never blocks its caller (the shared // startup worker thread) on network I/O. - scheduledPoll = schedulePoll(0L); + scheduledPoll = scheduler.schedule(this::pollOnce, 0L); } - private void pollOnce() { - if (stopped || !NotificationPreferences.isNotificationsEnabled()) { + void pollOnce() { + if (shutdown || !running || !enabledSupplier.get()) { return; } try { @@ -71,33 +122,50 @@ private void pollOnce() { } } - private void reschedule() { - if (stopped || !NotificationPreferences.isNotificationsEnabled()) { + private synchronized void reschedule() { + if (shutdown || !running || !enabledSupplier.get()) { return; } - scheduledPoll = schedulePoll(POLL_INTERVAL_MS); - // If stop() ran concurrently between the guard above and the assignment, cancel the future we just armed - // so a poll cannot fire after shutdown. - if (stopped && scheduledPoll != null) { + scheduledPoll = scheduler.schedule(this::pollOnce, POLL_INTERVAL_MS); + // If shutdown ran concurrently between the guard above and the assignment, cancel what we just armed so a + // poll cannot fire after teardown. + if (shutdown && scheduledPoll != null) { scheduledPoll.cancel(false); } } - private ScheduledFuture schedulePoll(final long delayMs) { - try { - return (ScheduledFuture) ThreadingUtils.scheduleAsyncTaskWithDelay(this::pollOnce, delayMs); - } catch (RejectedExecutionException e) { - Activator.getLogger().info("Notifications polling stopped (worker pool shutting down)"); - return null; + /** + * Reacts to a change in the notifications kill-switch preference: starts polling if it was turned on, or pauses + * (cancels the pending poll) if it was turned off. Unlike {@link #shutdown()}, this is reversible in-session. + */ + public synchronized void onEnabledPreferenceChanged() { + if (shutdown) { + return; + } + if (enabledSupplier.get()) { + start(); + } else { + pause(); } } - /** Cancels the pending poll and prevents further rescheduling; safe to call during shutdown. */ - public void stop() { - stopped = true; + private synchronized void pause() { + running = false; + cancelPending(); + } + + /** Permanently cancels polling for teardown; not resumable. */ + public synchronized void shutdown() { + shutdown = true; + running = false; + cancelPending(); + } + + private void cancelPending() { final ScheduledFuture current = scheduledPoll; if (current != null) { current.cancel(false); + scheduledPoll = null; } } } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java index 807029d3a..35c7f0a06 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java @@ -30,9 +30,12 @@ */ public final class NotificationsFetcher { - private static final int TIMEOUT_SECONDS = 30; + // A small hosted JSON over CloudFront returns in well under a second; a 10s timeout bounds how long a poll can + // occupy the shared worker thread while still tolerating a slow network. Worst case per poll is bounded to + // roughly MAX_RETRIES * TIMEOUT_SECONDS + total backoff. + private static final int TIMEOUT_SECONDS = 10; private static final int MAX_RETRIES = 3; - private static final long RETRY_BASE_DELAY_MS = 1000L; + private static final long RETRY_BASE_DELAY_MS = 500L; private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getInstance(); private final String endpointUrl; diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java index 39f965adf..fcd3221a6 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java @@ -64,7 +64,7 @@ public Activator() { @Override public final void stop(final BundleContext context) throws Exception { - NotificationPollingService.getInstance().stop(); + NotificationPollingService.getInstance().shutdown(); AmazonQBrowserProvider.getInstance().dispose(); super.stop(context); plugin = null; diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java index 63f700700..8f5c2fe8f 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferenceInitializer.java @@ -9,6 +9,7 @@ import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.lsp4j.DidChangeConfigurationParams; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationPollingService; import software.aws.toolkits.eclipse.amazonq.plugin.Activator; import software.aws.toolkits.eclipse.amazonq.util.ThreadingUtils; @@ -25,6 +26,10 @@ public final void initializeDefaultPreferences() { store.setDefault(AmazonQPreferencePage.NOTIFICATIONS_OPT_IN, true); store.setDefault(AmazonQPreferencePage.NOTIFICATIONS_ENDPOINT_OVERRIDE, ""); store.addPropertyChangeListener(event -> { + // React to the notifications kill-switch so it can pause/resume polling within a session (no restart). + if (AmazonQPreferencePage.NOTIFICATIONS_OPT_IN.equals(event.getProperty())) { + NotificationPollingService.getInstance().onEnabledPreferenceChanged(); + } ThreadingUtils.executeAsyncTask(() -> { Activator.getLspProvider().getAmazonQServer() .thenAccept(server -> server.getWorkspaceService().didChangeConfiguration( diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java index 3a53c1241..5893e9d4d 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java @@ -77,4 +77,31 @@ void recentDismissalsSurviveCleanup() { void noStateReturnsNotDismissed() { assertFalse(new NotificationDismissalStore(pluginStore).isDismissed("anything")); } + + @Test + void corruptStateIsResetAndPersistedOnRead() { + // Store bytes that are not a valid NotificationDismissalConfiguration JSON. + pluginStore.put(NotificationConstants.DISMISSAL_STORAGE_KEY, "}{ not valid json"); + final NotificationDismissalStore store = new NotificationDismissalStore(pluginStore); + // Must not throw, and treats the corrupt state as empty. + assertFalse(store.isDismissed("n1")); + // The corrupt bytes must be overwritten with a valid empty config, so a fresh read no longer sees garbage. + final NotificationDismissalConfiguration repaired = + pluginStore.getObject(NotificationConstants.DISMISSAL_STORAGE_KEY, NotificationDismissalConfiguration.class); + assertTrue(repaired != null && repaired.getDismissedNotifications().isEmpty()); + // And a subsequent dismiss still works after repair. + store.dismiss("n1"); + assertTrue(store.isDismissed("n1")); + } + + @Test + void nullIdEntryDoesNotThrow() { + final NotificationDismissalConfiguration config = new NotificationDismissalConfiguration(); + config.setDismissedNotifications(new java.util.ArrayList<>(List.of( + new DismissedNotification(null, Instant.now().toEpochMilli())))); + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, config); + final NotificationDismissalStore store = new NotificationDismissalStore(pluginStore); + // A stored entry with a null id must not NPE when checking a real id. + assertFalse(store.isDismissed("n1")); + } } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java new file mode 100644 index 000000000..f1d475486 --- /dev/null +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java @@ -0,0 +1,171 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.eclipse.amazonq.notifications; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import software.aws.toolkits.eclipse.amazonq.extensions.implementation.ActivatorStaticMockExtension; +import software.aws.toolkits.eclipse.amazonq.notifications.NotificationPollingService.PollScheduler; + +/** + * Deterministic lifecycle coverage for {@link NotificationPollingService} using the package-private injectable + * constructor: a manual scheduler (records scheduled tasks without a real thread pool) plus toggleable + * enabled/dev-build/override suppliers. + */ +public final class NotificationPollingServiceTest { + + @RegisterExtension + private static ActivatorStaticMockExtension activatorExtension = new ActivatorStaticMockExtension(); + + /** Records scheduled tasks and lets the test fire them manually; returns a mock cancellable future. */ + private static final class ManualScheduler implements PollScheduler { + private final List tasks = new ArrayList<>(); + private final List delays = new ArrayList<>(); + private final List> futures = new ArrayList<>(); + private int scheduleCount; + + @Override + public ScheduledFuture schedule(final Runnable task, final long delayMs) { + scheduleCount++; + tasks.add(task); + delays.add(delayMs); + final ScheduledFuture future = mock(ScheduledFuture.class); + futures.add(future); + return future; + } + + void fireLast() { + tasks.get(tasks.size() - 1).run(); + } + } + + private AtomicBoolean enabled; + private AtomicBoolean devBuild; + private AtomicBoolean override; + private NotificationsFetcher fetcher; + private ProcessNotifications processor; + private ManualScheduler scheduler; + private AtomicInteger fetcherBuilds; + + @BeforeEach + void setUp() { + enabled = new AtomicBoolean(true); + devBuild = new AtomicBoolean(false); + override = new AtomicBoolean(false); + fetcher = mock(NotificationsFetcher.class); + when(fetcher.fetch()).thenReturn(Optional.empty()); + processor = mock(ProcessNotifications.class); + scheduler = new ManualScheduler(); + fetcherBuilds = new AtomicInteger(); + } + + private NotificationPollingService service() { + return new NotificationPollingService(enabled::get, devBuild::get, override::get, + () -> { + fetcherBuilds.incrementAndGet(); + return fetcher; + }, + () -> processor, scheduler); + } + + @Test + void startSchedulesFirstPollWithZeroDelayAndDoesNotBlock() { + service().start(); + assertEquals(1, scheduler.scheduleCount); + assertEquals(0L, scheduler.delays.get(0)); + } + + @Test + void startIsIdempotent() { + final NotificationPollingService s = service(); + s.start(); + s.start(); + assertEquals(1, scheduler.scheduleCount, "second start() must be a no-op"); + assertEquals(1, fetcherBuilds.get()); + } + + @Test + void disabledKillSwitchDoesNotPoll() { + enabled.set(false); + service().start(); + assertEquals(0, scheduler.scheduleCount); + } + + @Test + void devBuildWithoutOverrideDoesNotPoll() { + devBuild.set(true); + service().start(); + assertEquals(0, scheduler.scheduleCount); + } + + @Test + void devBuildWithOverrideDoesPoll() { + devBuild.set(true); + override.set(true); + service().start(); + assertEquals(1, scheduler.scheduleCount); + } + + @Test + void pollReschedulesAtInterval() { + service().start(); + scheduler.fireLast(); // run the first poll + assertEquals(2, scheduler.scheduleCount, "poll must self-reschedule"); + assertTrue(scheduler.delays.get(1) > 0, "reschedule uses the poll interval, not 0"); + } + + @Test + void shutdownCancelsPendingAndPreventsReschedule() { + final NotificationPollingService s = service(); + s.start(); + s.shutdown(); + // Firing a poll after shutdown must not re-arm. + final int countAtShutdown = scheduler.scheduleCount; + scheduler.fireLast(); + assertEquals(countAtShutdown, scheduler.scheduleCount, "no reschedule after shutdown"); + } + + @Test + void killSwitchToggleOffThenOnResumesPolling() { + final NotificationPollingService s = service(); + s.start(); + assertEquals(1, scheduler.scheduleCount); + + // Turn off -> pause. + enabled.set(false); + s.onEnabledPreferenceChanged(); + // Firing a stale scheduled task while disabled must not reschedule. + scheduler.fireLast(); + assertEquals(1, scheduler.scheduleCount); + + // Turn back on -> resumes without an IDE restart. + enabled.set(true); + s.onEnabledPreferenceChanged(); + assertEquals(2, scheduler.scheduleCount, "re-enabling must restart polling"); + } + + @Test + void startAfterShutdownStaysDown() { + final NotificationPollingService s = service(); + s.shutdown(); + s.start(); + assertEquals(0, scheduler.scheduleCount); + assertFalse(devBuild.get()); + } +} From 004606eb012c275c9d1d4b4991bb748de03c4355 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Fri, 17 Jul 2026 12:56:55 -0700 Subject: [PATCH 06/10] fix(notifications): cap payload size, null-safe rule comparisons, more coverage - Cap the fetched payload at ~1MB so a mis-pointed or oversized endpoint cannot buffer an arbitrary amount into memory and attempt to parse it. - Rules engine: make equality/ordering operators null-safe on the actual system value so a null os.version/ide.version (etc.) evaluates to a non-match instead of throwing NPE mid-poll. - Document that authx.ssoScopes is intentionally a no-op until Eclipse collects SSO scopes, so payloads don't rely on it for targeting. Tests: oversized-payload ignored, null-actual-value non-match (+ null os.version ordering), null/duplicate-id and non-array-anyOf parsing behavior. --- .../notifications/NotificationsFetcher.java | 9 ++++ .../amazonq/notifications/RulesEngine.java | 18 +++++--- .../NotificationParsingTest.java | 41 +++++++++++++++++++ .../NotificationsFetcherTest.java | 14 +++++++ .../notifications/RulesEngineTest.java | 23 +++++++++++ 5 files changed, 99 insertions(+), 6 deletions(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java index 35c7f0a06..83d64dc26 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcher.java @@ -36,6 +36,8 @@ public final class NotificationsFetcher { private static final int TIMEOUT_SECONDS = 10; private static final int MAX_RETRIES = 3; private static final long RETRY_BASE_DELAY_MS = 500L; + /** Upper bound on the notifications payload size (~1MB of JSON). Real payloads are a few KB. */ + private static final int MAX_PAYLOAD_CHARS = 1_000_000; private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getInstance(); private final String endpointUrl; @@ -154,6 +156,13 @@ private Optional validate(final String content) { if (content == null || content.isBlank()) { return Optional.empty(); } + // Guard against an unexpectedly large payload (a mis-pointed endpoint, or a hijacked/oversized file) so a + // poll cannot buffer an arbitrary amount into memory + attempt to parse it. + if (content.length() > MAX_PAYLOAD_CHARS) { + Activator.getLogger().warn("Notifications payload exceeds " + MAX_PAYLOAD_CHARS + + " chars (" + content.length() + "); ignoring"); + return Optional.empty(); + } try { return Optional.ofNullable(OBJECT_MAPPER.readValue(content, NotificationsList.class)); } catch (Exception e) { diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java index 38b150d77..779ad25bb 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngine.java @@ -104,6 +104,10 @@ private static boolean matchesAuth(final List na, final SystemDetails || evaluateNotificationExpression(feature.region(), auth.region()); final boolean stateMatch = feature.connectionState() == null || evaluateNotificationExpression(feature.connectionState(), auth.connectionState()); + // NOTE: ssoScopes is intentionally not evaluated here — Eclipse's SystemDetailsCollector does not yet + // collect the connection's SSO scopes, so there is no actual value to compare against. Payloads should + // not rely on ssoScopes for Eclipse targeting until it is collected (tracked as a follow-up); an + // ssoScopes clause is currently a no-op rather than a match/mismatch. if (!(typeMatch && regionMatch && stateMatch)) { return false; } @@ -120,17 +124,19 @@ public static boolean evaluateNotificationExpression(final NotificationExpressio public static boolean evaluateNotificationExpression(final NotificationExpression expr, final String value, final boolean useSemver) { if (expr instanceof NotificationExpression.ComparisonCondition c) { - return c.value().equals(value); + // Anchor on the payload value so a null system value is a non-match, not an NPE. + return c.value() != null && c.value().equals(value); } else if (expr instanceof NotificationExpression.NotEqualsCondition c) { - return !c.value().equals(value); + return c.value() == null || !c.value().equals(value); } else if (expr instanceof NotificationExpression.GreaterThanCondition c) { - return compare(value, c.value(), useSemver) > 0; + // A null actual system value cannot satisfy an ordering constraint. + return value != null && compare(value, c.value(), useSemver) > 0; } else if (expr instanceof NotificationExpression.GreaterThanOrEqualsCondition c) { - return compare(value, c.value(), useSemver) >= 0; + return value != null && compare(value, c.value(), useSemver) >= 0; } else if (expr instanceof NotificationExpression.LessThanCondition c) { - return compare(value, c.value(), useSemver) < 0; + return value != null && compare(value, c.value(), useSemver) < 0; } else if (expr instanceof NotificationExpression.LessThanOrEqualsCondition c) { - return compare(value, c.value(), useSemver) <= 0; + return value != null && compare(value, c.value(), useSemver) <= 0; } else if (expr instanceof NotificationExpression.AnyOfCondition c) { return c.value().contains(value); } else if (expr instanceof NotificationExpression.NoneOfCondition c) { diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java index b9bdece27..5e9adf3b9 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationParsingTest.java @@ -214,4 +214,45 @@ void rejectsMalformedExpression() { """; assertThrows(Exception.class, () -> mapper.readValue(badJson, NotificationsList.class)); } + + @Test + void nullNotificationElementParsesAsNullEntry() throws Exception { + // A JSON null in the notifications array deserializes to a null element; ProcessNotifications must skip it + // (covered in ProcessNotificationsTest.nullElementInBatchIsSkipped) rather than aborting the batch. + NotificationsList list = mapper.readValue(""" + { "schema": { "version": "2.0" }, "notifications": [ null, + { "id": "n1", "schedule": { "type": "Emergency" }, "severity": "Info", + "content": { "en-US": { "title": "t", "description": "d" } } } ] } + """, NotificationsList.class); + assertEquals(2, list.notifications().size()); + assertNull(list.notifications().get(0)); + assertEquals("n1", list.notifications().get(1).id()); + } + + @Test + void duplicateIdsBothParse() throws Exception { + // Duplicate ids are not rejected at parse time; in-session dedup is by id at display time. + NotificationsList list = mapper.readValue(""" + { "schema": { "version": "2.0" }, "notifications": [ + { "id": "dup", "schedule": { "type": "Emergency" }, "severity": "Info", + "content": { "en-US": { "title": "a", "description": "d" } } }, + { "id": "dup", "schedule": { "type": "Emergency" }, "severity": "Critical", + "content": { "en-US": { "title": "b", "description": "d" } } } ] } + """, NotificationsList.class); + assertEquals(2, list.notifications().size()); + assertEquals("dup", list.notifications().get(0).id()); + assertEquals("dup", list.notifications().get(1).id()); + } + + @Test + void anyOfWithNonArrayValueIsRejected() { + // anyOf expects an array; a scalar must not silently deserialize into a valid expression. + String badJson = """ + { "schema": { "version": "2.0" }, "notifications": [ + { "id": "n1", "schedule": { "type": "Emergency" }, "severity": "Info", + "condition": { "os": { "type": { "anyOf": "notAnArray" } } }, + "content": { "en-US": { "title": "t", "description": "d" } } } ] } + """; + assertThrows(Exception.class, () -> mapper.readValue(badJson, NotificationsList.class)); + } } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java index 9b4685980..3762b2f4d 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationsFetcherTest.java @@ -181,4 +181,18 @@ void networkFailureFallsBackToCacheWhenPresent() throws Exception { // With a valid cache, a persistent network failure serves the cached payload rather than empty. assertTrue(fetcher().fetch().isPresent()); } + + @Test + @SuppressWarnings("unchecked") + void oversizedPayloadIsIgnored() throws Exception { + // A payload beyond the size cap must not be parsed or cached. + final StringBuilder huge = new StringBuilder("{ \"schema\": { \"version\": \"2.0\" }, \"notifications\": ["); + while (huge.length() < 1_100_000) { + huge.append("{\"id\":\"x\",\"schedule\":{\"type\":\"Emergency\"},\"severity\":\"Info\"},"); + } + huge.append("] }"); + doReturn(response(200, huge.toString())).when(httpClient).send(any(HttpRequest.class), any()); + assertTrue(fetcher().fetch().isEmpty()); + assertFalse(Files.exists(cacheDir.resolve("notifications.json"))); + } } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java index ff12e2dfc..055181aaa 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/RulesEngineTest.java @@ -170,4 +170,27 @@ void loggedOutIsEvaluableAsNotConnected() { List.of(new AuthxType("q", null, null, eq("NotConnected"), null))); assertTrue(RulesEngine.displayNotification(notification(cond), loggedOut)); } + + @Test + void nullActualValueDoesNotThrowAndIsNonMatch() { + // A null system value against ordering/equality operators must be a non-match, never an NPE. + assertFalse(RulesEngine.evaluateNotificationExpression(eq("x"), null)); + assertFalse(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.GreaterThanCondition("1.0"), null, true)); + assertFalse(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.LessThanCondition("1.0"), null, true)); + // != against a null actual is a match (the actual is not equal to the expected value). + assertTrue(RulesEngine.evaluateNotificationExpression( + new NotificationExpression.NotEqualsCondition("x"), null)); + } + + @Test + void nullOsVersionWithOrderingConditionDoesNotThrow() { + final SystemDetails noOsVersion = new SystemDetails("Local", "aarch64", "Mac OS X", null, "Eclipse", "4.30.0", + Map.of(PLUGIN_ID, "1.70.0"), new FeatureAuthDetails("BuilderId", "us-east-1", "Connected")); + final NotificationDisplayCondition cond = new NotificationDisplayCondition(null, + new SystemType(null, new NotificationExpression.GreaterThanCondition("10.0")), null, null, null); + // Must evaluate to "does not match" rather than throwing. + assertFalse(RulesEngine.displayNotification(notification(cond), noOsVersion)); + } } From a156ddc4695b412b0e38b3a9533607904945b8fd Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Fri, 17 Jul 2026 13:54:21 -0700 Subject: [PATCH 07/10] refactor(notifications): address adversarial re-review nits Follow-ups from an adversarial re-review of the hardening commits (no functional bugs were found; these are robustness/clarity refinements): - NotificationPollingService.start(): set running=true only after the scheduler actually accepts the first poll, so a worker-pool rejection at startup leaves the service restartable instead of latched into a started-but-never-scheduled state. Add a test for the rejection-then-retry path. - Remove a dead post-assignment shutdown recheck in reschedule() (both methods are synchronized on the same monitor, so it could never fire) and clarify the comment. - Make the dismissal corrupt-state test deterministic: seed the wrong-shape value through the same putObject/getObject byte path the store uses, so Gson reliably throws, rather than relying on Base64-decoding-of-non-Base64 behavior. - Drop a vacuous assertion in the polling test. --- .../NotificationPollingService.java | 17 +++++++------ .../NotificationDismissalStoreTest.java | 6 +++-- .../NotificationPollingServiceTest.java | 24 +++++++++++++++++-- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java index 26a5ed54f..53a36d942 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java @@ -100,12 +100,18 @@ public synchronized void start() { Activator.getLogger().info("Notifications polling skipped: development build with no endpoint override"); return; } - running = true; this.fetcher = fetcherSupplier.get(); this.processor = processorSupplier.get(); // Schedule the first poll instead of running it inline so start() never blocks its caller (the shared // startup worker thread) on network I/O. - scheduledPoll = scheduler.schedule(this::pollOnce, 0L); + final ScheduledFuture scheduled = scheduler.schedule(this::pollOnce, 0L); + if (scheduled == null) { + // The worker pool rejected the task (e.g. shutting down). Leave running=false so a later re-enable + // can retry rather than latching into a started-but-never-scheduled state. + return; + } + running = true; + scheduledPoll = scheduled; } void pollOnce() { @@ -126,12 +132,9 @@ private synchronized void reschedule() { if (shutdown || !running || !enabledSupplier.get()) { return; } + // reschedule() and shutdown() are both synchronized on this monitor, so shutdown cannot interleave here; + // the entry guard above plus shutdown()'s cancelPending() are sufficient to stop post-teardown polls. scheduledPoll = scheduler.schedule(this::pollOnce, POLL_INTERVAL_MS); - // If shutdown ran concurrently between the guard above and the assignment, cancel what we just armed so a - // poll cannot fire after teardown. - if (shutdown && scheduledPoll != null) { - scheduledPoll.cancel(false); - } } /** diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java index 5893e9d4d..4c44e6750 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationDismissalStoreTest.java @@ -80,8 +80,10 @@ void noStateReturnsNotDismissed() { @Test void corruptStateIsResetAndPersistedOnRead() { - // Store bytes that are not a valid NotificationDismissalConfiguration JSON. - pluginStore.put(NotificationConstants.DISMISSAL_STORAGE_KEY, "}{ not valid json"); + // Persist a value of the WRONG shape via the same byte path getObject reads (putObject), so getObject's + // Gson.fromJson deterministically throws when coercing it to NotificationDismissalConfiguration. Using a + // bare String here serializes to a JSON string literal, which cannot deserialize into the config object. + pluginStore.putObject(NotificationConstants.DISMISSAL_STORAGE_KEY, "not-a-config-object"); final NotificationDismissalStore store = new NotificationDismissalStore(pluginStore); // Must not throw, and treats the corrupt state as empty. assertFalse(store.isDismissed("n1")); diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java index f1d475486..50125d383 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java @@ -4,7 +4,6 @@ package software.aws.toolkits.eclipse.amazonq.notifications; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -166,6 +165,27 @@ void startAfterShutdownStaysDown() { s.shutdown(); s.start(); assertEquals(0, scheduler.scheduleCount); - assertFalse(devBuild.get()); + } + + @Test + void schedulerRejectionLeavesServiceRestartable() { + // A scheduler that rejects (returns null, as the real one does on RejectedExecutionException) must not + // latch the service into a started-but-never-scheduled state: a later start() can retry. + final AtomicBoolean reject = new AtomicBoolean(true); + final PollScheduler rejecting = (task, delayMs) -> { + if (reject.get()) { + return null; + } + return scheduler.schedule(task, delayMs); + }; + final NotificationPollingService s = new NotificationPollingService(enabled::get, devBuild::get, + override::get, () -> fetcher, () -> processor, rejecting); + s.start(); + assertEquals(0, scheduler.scheduleCount, "rejected schedule armed nothing"); + + // Pool recovers; a subsequent start() succeeds because running was never latched true. + reject.set(false); + s.start(); + assertEquals(1, scheduler.scheduleCount, "start() retries after an earlier rejection"); } } From e88a318f17afa25d32beca3cc08eb23bdaed9578 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Tue, 21 Jul 2026 09:38:05 -0700 Subject: [PATCH 08/10] fix(notifications): set running=true before scheduling the first poll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression from the earlier lifecycle refactor: the first poll is scheduled with delay 0, so on a real ScheduledExecutorService the pool thread can run pollOnce() before start() returns. pollOnce() early-returns unless running==true, and running was being set AFTER scheduling — so the very first poll could silently no-op (no fetch, no toast, no reschedule, no log). Set running=true before scheduling (the field is volatile so the poll thread observes it) and roll it back only if the scheduler rejects the task. The prior unit tests used a scheduler that deferred execution, so they never reproduced the delay-0 inline-execution race. Add InlineScheduler + firstPollExecutingInlineAtScheduleTimeStillRuns, which runs the delay-0 task synchronously at schedule time and fails against the old ordering. --- .../NotificationPollingService.java | 11 +++++-- .../NotificationPollingServiceTest.java | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java index 53a36d942..853f3f766 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingService.java @@ -102,15 +102,20 @@ public synchronized void start() { } this.fetcher = fetcherSupplier.get(); this.processor = processorSupplier.get(); + // Mark running BEFORE scheduling: the first poll is scheduled with delay 0, so on a real thread pool the + // poll can execute before this method returns. pollOnce() early-returns unless running==true, so if we set + // the flag after scheduling the very first poll can silently no-op (no fetch, no toast, no reschedule). + // running is volatile, so the poll thread observes this write. Roll it back if the scheduler rejects. + running = true; // Schedule the first poll instead of running it inline so start() never blocks its caller (the shared // startup worker thread) on network I/O. final ScheduledFuture scheduled = scheduler.schedule(this::pollOnce, 0L); if (scheduled == null) { - // The worker pool rejected the task (e.g. shutting down). Leave running=false so a later re-enable - // can retry rather than latching into a started-but-never-scheduled state. + // The worker pool rejected the task (e.g. shutting down). Reset running so a later re-enable can retry + // rather than latching into a started-but-never-scheduled state. + running = false; return; } - running = true; scheduledPoll = scheduled; } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java index 50125d383..c4111a3c5 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/notifications/NotificationPollingServiceTest.java @@ -54,6 +54,24 @@ void fireLast() { } } + /** + * Scheduler that runs the task SYNCHRONOUSLY inside schedule() — reproducing a real ScheduledExecutorService + * executing a delay-0 first poll before start() returns. This is the race that silently no-op'd the first poll + * when running was set after scheduling. + */ + private static final class InlineScheduler implements PollScheduler { + private int scheduleCount; + + @Override + public ScheduledFuture schedule(final Runnable task, final long delayMs) { + scheduleCount++; + if (delayMs == 0L) { + task.run(); + } + return mock(ScheduledFuture.class); + } + } + private AtomicBoolean enabled; private AtomicBoolean devBuild; private AtomicBoolean override; @@ -167,6 +185,19 @@ void startAfterShutdownStaysDown() { assertEquals(0, scheduler.scheduleCount); } + @Test + void firstPollExecutingInlineAtScheduleTimeStillRuns() { + // Regression: the first poll is scheduled with delay 0; if the scheduler runs it synchronously (as a real + // pool thread can, before start() returns), pollOnce must see running==true and actually fetch — not + // silently no-op. Verify the poll reached the fetcher. + when(fetcher.fetch()).thenReturn(Optional.empty()); + final InlineScheduler inline = new InlineScheduler(); + final NotificationPollingService s = new NotificationPollingService(enabled::get, devBuild::get, + override::get, () -> fetcher, () -> processor, inline); + s.start(); + org.mockito.Mockito.verify(fetcher).fetch(); + } + @Test void schedulerRejectionLeavesServiceRestartable() { // A scheduler that rejects (returns null, as the real one does on RejectedExecutionException) must not From 5617e95ffe9f667e568106458fa5d9a9019b2fed Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Tue, 21 Jul 2026 10:54:23 -0700 Subject: [PATCH 09/10] fix: bundle commons-logging instead of Require-Bundle (fixes load on Eclipse 4.40+) The plugin bundles Apache HttpClient 4.x (via the AWS SDK apache-client), which requires org.apache.commons.logging at runtime. That was satisfied by a platform Require-Bundle on 'org.apache.commons.logging'. Newer Eclipse (4.40+) renamed that Orbit bundle to 'org.apache.commons.commons-logging', so the old Require-Bundle no longer resolves and the ENTIRE plugin fails to load on current Eclipse. Fix: bundle commons-logging.jar as a plugin lib like the other third-party deps (add commons-logging to maven-dependency-plugin includeGroupIds + Bundle-Classpath) and drop the platform Require-Bundle. The plugin now self-supplies commons-logging and no longer depends on the platform bundle's name, so it loads on 4.32 (baseline) through 4.40+ regardless of Orbit renames. Verified: built jar contains target/dependency/commons-logging.jar; installed into a bare Eclipse 4.40 dropins/ (which lacks the old bundle name) -> plugin loads cleanly, no unresolved- bundle errors, notifications render. 499 tests still green. --- plugin/META-INF/MANIFEST.MF | 2 +- plugin/pom.xml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/META-INF/MANIFEST.MF b/plugin/META-INF/MANIFEST.MF index 4bef5eaf0..96ca976b1 100644 --- a/plugin/META-INF/MANIFEST.MF +++ b/plugin/META-INF/MANIFEST.MF @@ -27,7 +27,6 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="3.31.0", org.eclipse.jetty.util;bundle-version="12.0.9", org.eclipse.jetty.http;bundle-version="12.0.9", org.eclipse.core.net;bundle-version="1.5.400", - org.apache.commons.logging;bundle-version="1.2.0", slf4j.api;bundle-version="2.0.13", org.apache.commons.lang3;bundle-version="3.14.0", org.apache.commons.text;bundle-version="1.10.0", @@ -44,6 +43,7 @@ Bundle-Classpath: ., target/dependency/checksums.jar, target/dependency/cognitoidentity.jar, target/dependency/commons-codec.jar, + target/dependency/commons-logging.jar, target/dependency/delight-rhino-sandbox.jar, target/dependency/endpoints-spi.jar, target/dependency/http-auth-aws-eventstream.jar, diff --git a/plugin/pom.xml b/plugin/pom.xml index 4f382bd96..949349b03 100644 --- a/plugin/pom.xml +++ b/plugin/pom.xml @@ -194,6 +194,7 @@ com.fasterxml.jackson, com.nimbusds,jakarta.inject, commons-codec, + commons-logging, org.apache.httpcomponents, org.reactivestreams, org.apache.maven, From e4b19acead36e29b116349291903071c3a1d2bb9 Mon Sep 17 00:00:00 2001 From: Aarush Arora Date: Tue, 21 Jul 2026 16:35:40 -0700 Subject: [PATCH 10/10] fix(notifications): position each stacked toast by its own width (fixes off-screen shift) A notification toast could be shifted right, off the edge of the screen. Root cause was in repositionNotifications() (run on close()): it derived a single x from the CLOSING shell's width and applied it to every surviving toast, ignoring that each survivor has its own width. Because toast widths differ (a short Info toast is ~250px; a Critical toast with an action-button row hits the 400px cap) and Info toasts auto-dismiss while Critical toasts persist, the common case was a narrow toast closing while a wide one remained -> the wide toast was placed at the narrow toast's x and its right edge hung ~150px off-screen. Unlike the open path (Mylyn's open() self-corrects via fixupDisplayBounds), reposition had no such guard. Fix: right-align and stack each surviving toast by its own getShell().getSize(), mirroring initializeBounds(). Also floor x at the client-area left edge in both initializeBounds() and repositionNotifications() as belt-and-suspenders against any residual width/DPI skew, so a toast can never be pushed off-screen to the right. --- .../amazonq/util/ToolkitNotification.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ToolkitNotification.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ToolkitNotification.java index 1ad9461d9..dc64d0284 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ToolkitNotification.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ToolkitNotification.java @@ -76,8 +76,10 @@ protected final void initializeBounds() { // Recompute height with the constrained width so wrapped text and buttons are accounted for int height = Math.max(getShell().computeSize(width, SWT.DEFAULT).y, MIN_HEIGHT); Point size = new Point(width, height); - // Calculate the position for the new notification - int x = clArea.x + clArea.width - size.x - PADDING_EDGE; + // Calculate the position for the new notification. Floor x at the client-area left edge so the + // shell can never be pushed off-screen to the right even if the width term is skewed (e.g. a DPI + // scaling mismatch on high-DPI/multi-monitor Windows). + int x = Math.max(clArea.x, clArea.x + clArea.width - size.x - PADDING_EDGE); int y = clArea.height + clArea.y - size.y - PADDING_EDGE; for (ToolkitNotification notification : activeNotifications) { if (!notification.getShell().isDisposed()) { @@ -96,16 +98,18 @@ private Rectangle getPrimaryClientArea() { private void repositionNotifications() { Rectangle clArea = getPrimaryClientArea(); - Point initialSize = getShell().computeSize(SWT.DEFAULT, SWT.DEFAULT); - int height = Math.max(initialSize.y, MIN_HEIGHT); - int width = Math.min(initialSize.x, MAX_WIDTH); - int x = clArea.x + clArea.width - width - PADDING_EDGE; - int y = clArea.height + clArea.y - height - PADDING_EDGE; + // Right-align and stack each surviving notification by ITS OWN size. Previously a single x/y was + // derived from the closing shell's width/height and applied to every survivor, so when a narrow + // toast closed while a wider one remained, the wider toast was placed too far right and its right + // edge hung off the screen. Mirror initializeBounds(): position each shell from its own getSize(). + int y = clArea.y + clArea.height - PADDING_EDGE; for (ToolkitNotification notification : activeNotifications) { if (!notification.getShell().isDisposed()) { Point size = notification.getShell().getSize(); + int x = Math.max(clArea.x, clArea.x + clArea.width - size.x - PADDING_EDGE); + y -= size.y; notification.getShell().setLocation(x, y); - y -= size.y + NOTIFICATIONS_GAP; + y -= NOTIFICATIONS_GAP; } } }