-
Couldn't load subscription status.
- Fork 10
feature/Notifications #390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0730975
ff4e3bf
005b6ee
a747467
85a3f4b
d89007e
6c5a9cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,10 @@ public QLspConnectionProvider() throws IOException { | |
| var serverCommand = Paths.get(lspInstallResult.getServerDirectory(), lspInstallResult.getServerCommand()); | ||
| List<String> commands = new ArrayList<>(); | ||
| commands.add(serverCommand.toString()); | ||
| commands.add(lspInstallResult.getServerCommandArgs()); | ||
| //commands.add(lspInstallResult.getServerCommandArgs()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Be sure to add a comment here about you're doing this for development on the feature branch and this should not be checked into main. We don't want this making it into prod. |
||
| commands.add("--inspect=6012"); | ||
| commands.add("/Users/aseemxs/Language-servers/language-servers/app/aws-lsp-notification-runtimes/out/standalone.js"); | ||
| commands.add("--nolazy"); | ||
| commands.add("--stdio"); | ||
| commands.add("--set-credentials-encryption-key"); | ||
| setCommands(commands); | ||
|
|
@@ -55,6 +58,7 @@ protected final void addEnvironmentVariables(final Map<String, String> env) { | |
| } | ||
| env.put("ENABLE_INLINE_COMPLETION", "true"); | ||
| env.put("ENABLE_TOKEN_PROVIDER", "true"); | ||
| env.put("AWS_Q_ENDPOINT_URL", "https://rts.gamma-us-east-1.codewhisperer.ai.aws.dev/"); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package software.aws.toolkits.eclipse.amazonq.lsp.model; | ||
aseemxs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| public record EventIdentifier(String id) { } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package software.aws.toolkits.eclipse.amazonq.lsp.model; | ||
|
|
||
| public record NotificationAction(String text, String type) { } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Notification actions may include a URI property (wait for the Flare protocol updates). |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package software.aws.toolkits.eclipse.amazonq.lsp.model; | ||
|
|
||
| public record NotificationContent(String text, String title) { } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package software.aws.toolkits.eclipse.amazonq.lsp.model; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.eclipse.lsp4j.MessageType; | ||
|
|
||
| public record NotificationParams( | ||
| MessageType type, | ||
| NotificationContent content, | ||
| String id, | ||
| List<NotificationAction> actions | ||
| ) { } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package software.aws.toolkits.eclipse.amazonq.notification; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.HashMap; | ||
| import java.util.Collections; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.ScheduledExecutorService; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
|
|
||
| import software.aws.toolkits.eclipse.amazonq.plugin.Activator; | ||
| import software.aws.toolkits.eclipse.amazonq.lsp.AmazonQLspClient; | ||
|
|
||
| /** | ||
| * Maintains client state information for the notification system. | ||
| * | ||
| * This class tracks both static and dynamic information about the Eclipse environment | ||
| * that the notification server uses to determine which notifications to show. | ||
| */ | ||
| public class NotificationConfiguration{ | ||
|
|
||
| // Singleton, needed one for whole eclipse session | ||
| private static NotificationConfiguration instance; | ||
|
|
||
| private final AmazonQLspClient lspClient; | ||
|
|
||
| private final List<String> contexts = new ArrayList<>(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a HashSet for contexts. |
||
|
|
||
| private final Map<String, List<String>> clientStates = new HashMap<>(); | ||
|
|
||
| private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); | ||
| private final AtomicBoolean pendingUpdate = new AtomicBoolean(false); | ||
| private static final int DEBOUNCE_DELAY_MS = 1000; //1 sec debounce | ||
|
|
||
| /** | ||
| * Gets the singleton instance of NotificationConfiguration. | ||
| * | ||
| * @param lspClient The LSP client to use for communication with the server | ||
| * @return The NotificationConfiguration instance | ||
| */ | ||
| public static synchronized NotificationConfiguration getInstance(AmazonQLspClient lspClient) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider splitting this to explicit instantiation instead of lazy instantiation so that not all code that wants to call getInstance must also have an instance of AmazonQLspClient easily available as well. |
||
| if (instance == null) { | ||
| instance = new NotificationConfiguration(lspClient); | ||
| } | ||
| return instance; | ||
| } | ||
|
|
||
| /** | ||
| * Private constructor to enforce singleton pattern. | ||
| * | ||
| * @param lspClient The LSP client to use for communication | ||
| */ | ||
| private NotificationConfiguration(AmazonQLspClient lspClient) { | ||
| this.lspClient = lspClient; | ||
| initializeStaticClientStates(); | ||
| } | ||
|
|
||
| /** | ||
| * Initialize static client states that won't change during the Eclipse session. | ||
| */ | ||
| private void initializeStaticClientStates() { | ||
| clientStates.put("IDE/VERSION", Collections.singletonList(getEclipseVersion())); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider defining these as constants in a single location (e.g. constants at the top of this class or a separate NotificationClientStates class) so they're easily discoverable and documentable in the code. The runbook will want to link to the code for explicit client state definitions and descriptions rather than being reproduced (yet another place to keep in sync) in the runbook. Also consider camelCase for the literal client state names as people tend not to like SCREAMING_SNAKE_CASE anymore. |
||
|
|
||
| // Add other static values as needed | ||
| // clientStates.put("ANOTHER_STATIC_VALUE", getSomeOtherStaticValue()); | ||
| } | ||
|
|
||
| private String getEclipseVersion() { | ||
| //placeholder - need to use the actual Eclipse API to get the version | ||
| return org.eclipse.core.runtime.Platform.getProduct().getDefiningBundle().getVersion().toString(); | ||
| } | ||
|
|
||
| /** | ||
| * Add a context to the active contexts list. | ||
| * | ||
| * @param context The context to add | ||
| */ | ||
| public synchronized void addContext(String context) { | ||
| if (!contexts.contains(context)) { | ||
| contexts.add(context); | ||
| notifyConfigurationChanged(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Remove a context from the active contexts list. | ||
| * | ||
| * @param context The context to remove | ||
| */ | ||
| public synchronized void removeContext(String context) { | ||
| if (contexts.contains(context)) { | ||
| contexts.remove(context); | ||
| notifyConfigurationChanged(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Set SSO scopes. | ||
| * | ||
| * @param scopes The list of SSO scopes | ||
| */ | ||
| public synchronized void setSsoScopes(List<String> scopes) { | ||
| List<String> scopesCopy = new ArrayList<>(scopes); | ||
| clientStates.put("SSO_SCOPES", scopesCopy); | ||
| notifyConfigurationChanged(); | ||
| } | ||
|
|
||
| /** | ||
| * Get the current notification configuration. | ||
| * | ||
| * @return A map representing the current notification configuration | ||
| */ | ||
| public synchronized Map<String, Object> getConfiguration() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure this needs to be public. Will client code need this? The format of this will be a little different, but doesn't need to be updated now. |
||
| Map<String, Object> config = new HashMap<>(); | ||
| config.putAll(clientStates); | ||
| config.put("CONTEXT", new ArrayList<>(contexts)); | ||
|
|
||
| return config; | ||
| } | ||
|
|
||
| /** | ||
| * Notify the server of a configuration change in a try catch block using didChangeConfiguration | ||
| */ | ||
| private void notifyConfigurationChanged() { | ||
| if (pendingUpdate.compareAndSet(false, true)) { | ||
| scheduler.schedule(() -> { | ||
| try { | ||
| lspClient.didChangeConfiguration(null); | ||
| Activator.getLogger().info("sent didChangeConfiguration notification"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: Capitalize "sent" |
||
| } catch (Exception e) { | ||
| Activator.getLogger().error("Error sending didChangeConfiguration notification", e); | ||
| } finally { | ||
| pendingUpdate.set(false); | ||
| } | ||
| }, DEBOUNCE_DELAY_MS, TimeUnit.MILLISECONDS); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Handles the getConfiguration request from the server. | ||
| * | ||
| * @param section The configuration section requested | ||
| * @return The requested configuration section | ||
| */ | ||
| public Object handleGetConfiguration(String section) { | ||
| if ("notifications".equals(section)) { | ||
| return getConfiguration(); | ||
| } | ||
|
|
||
| // Handle other sections or return null if not recognized | ||
| return null; | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to worry about now, but I may try to convert this to showNotifications (if Flare will agree) and send an array of Notification instead of just one.