Introduce ViewRouter Component #337

taldekar · 2025-01-28T19:36:14Z

Issue #335

Description of changes:
Introduced a ViewRouter component to determine which view to display based on an order of resolution logic.

Open Questions:

Checks to determine browser compatibility and UI asset availability needs to be implemented, but which components should be responsible for this requires some more thought.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

* Add functionality for hot subscribers that track latest event * Add test to ensure event stream isolation * Fix missing subscription disposal in test * Use autoConnect to implicitly manage stream connection

plugin/src/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouter.java

plugin/src/software/aws/toolkits/eclipse/amazonq/views/router/ViewId.java

plugin/src/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouter.java

plugin/src/software/aws/toolkits/eclipse/amazonq/plugin/Activator.java

plugin/src/software/aws/toolkits/eclipse/amazonq/broker/EventBroker.java

shruti0085 · 2025-01-30T21:31:02Z

plugin/tst/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouterTest.java

+    @ParameterizedTest
+    @MethodSource("provideStateSource")
+    void testActiveViewResolutionBasedOnPluginState(final LspState lspState, final AuthState authState,
+            final ViewId activeViewId) {


can this be renamed to "expectedActiveViewId"?
Is there a way to do an assertion on what is expected and what is the actual active view ?

I'll update the name. Changing this to an assertion is possible using ArgumentCaptors like so. Should I transition to this approach. I've verified that verify calls on the mock fail if the ViewIds are not what is expected. The ViewRouter doesn't really expose any methods to retrieve the activeView, so assertions are a bit difficult.

shruti0085 · 2025-01-30T21:31:37Z

plugin/tst/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouterTest.java

+        RxJavaPlugins.setComputationSchedulerHandler(scheduler -> Schedulers.trampoline());
+
+        publishSubject = PublishSubject.create();
+        authStateObservable = publishSubject.ofType(AuthState.class);


Do any of the observables need disposal?

I don't believe so, but I'll verify this.

Observables do not need to be disposed. But on a related note I did add functionality to track any subscriptions in the event broker and added a method that can be used to dispose off them.

What would be a example of a use case where we would want to dispose off a subscription?
By disposing observable I was talking about the situation when we want to stop listening for a certain event. By disposing off a subscription I am unclear what exactly that does?

Disposing off the subscription basically cancels it. So we stop listening to events. Observables are the stream of data, and those don't need to be disposed. For the ViewContainer component, before the check to see if the parent composite is disposed, if we didn't dispose the listener then it would try to call update child view when the parent composite was disposed and error out. Cases like these it would make sense to dispose of the listener.

plugin/tst/software/aws/toolkits/eclipse/amazonq/broker/EventBrokerTest.java

shruti0085 · 2025-01-31T18:33:52Z

plugin/tst/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouterTest.java

+        RxJavaPlugins.setComputationSchedulerHandler(scheduler -> Schedulers.trampoline());
+
+        publishSubject = PublishSubject.create();
+        authStateObservable = publishSubject.ofType(AuthState.class);


What would be a example of a use case where we would want to dispose off a subscription?
By disposing observable I was talking about the situation when we want to stop listening for a certain event. By disposing off a subscription I am unclear what exactly that does?

plugin/src/software/aws/toolkits/eclipse/amazonq/views/router/ViewId.java

plugin/src/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouter.java

…st state (#342)

This reverts commit 374e549.

This reverts commit 13c55ac.

* improved handling of lsp failure state * add static mock to lsp connection test case * Integrate Event Broker in LspStatusManager * Add ViewRouter POC * Fix code formatting bug * Add method to retrieve observables * Add listeners for combined state streams and view update request * Add listeners for combined state streams and view update request * Add listeners for combined state streams and view update request * Add functionality for hot event streams that track latest event (#340) * Add functionality for hot subscribers that track latest event * Add test to ensure event stream isolation * Fix missing subscription disposal in test * Use autoConnect to implicitly manage stream connection * Remove active view update request listener * Add ViewRouter tests * Remove public constructor * Remove public constructor and unused event in tests * Add comments * Add documentation for EventBroker * Remove ViewRouter initialization * Remove LspInitializingView from ID enum * Add documentation for ViewRouter * Refactor and enhance EventBroker tests * Refactor ViewRouter tests for clarity * Remove PluginState class into separate file * Add documentation to subscription management logic * Add support for notifying multiple late-subscribers over time of latest state (#342) * Remove CODE_REFERENCE_VIEW * Rename newActiveViewId to newActiveView * Rename ViewId class to AmazonQViewType * Revert "Integrate Event Broker in LspStatusManager" This reverts commit 374e549. * Revert "improved handling of lsp failure state" This reverts commit 13c55ac. * Refactor EventBroker and enhance tests (#343) --------- Co-authored-by: Nicolas Borges <[email protected]>

Nicolas Borges and others added 5 commits January 27, 2025 17:24

improved handling of lsp failure state

13c55ac

add static mock to lsp connection test case

548a9da

Integrate Event Broker in LspStatusManager

374e549

Add ViewRouter POC

6a9f8d3

Fix code formatting bug

d1746a4

taldekar requested a review from shruti0085 January 28, 2025 19:42

nborges-aws mentioned this pull request Jan 28, 2025

Introduce View Container and base view components #338

Merged

taldekar changed the base branch from main to nickdb/lspErrorHandling January 29, 2025 14:19

taldekar added 3 commits January 29, 2025 09:59

Add method to retrieve observables

091b6e9

Add listeners for combined state streams and view update request

fa11959

Add listeners for combined state streams and view update request

f934e6c

taldekar requested a review from breedloj January 29, 2025 15:04

taldekar added 3 commits January 29, 2025 10:15

Add listeners for combined state streams and view update request

a095805

Add functionality for hot event streams that track latest event (#340)

4ad24a1

* Add functionality for hot subscribers that track latest event * Add test to ensure event stream isolation * Fix missing subscription disposal in test * Use autoConnect to implicitly manage stream connection

Remove active view update request listener

d8b62b4

breedloj approved these changes Jan 30, 2025

View reviewed changes

taldekar added 4 commits January 30, 2025 14:37

Add ViewRouter tests

eca096f

Remove public constructor

d1096a6

Remove public constructor and unused event in tests

24f9c21

Add comments

eb53a8c

shruti0085 reviewed Jan 30, 2025

View reviewed changes

plugin/src/software/aws/toolkits/eclipse/amazonq/views/router/ViewRouter.java Outdated Show resolved Hide resolved

shruti0085 reviewed Jan 30, 2025

View reviewed changes

taldekar added 8 commits January 30, 2025 18:55

Add documentation for EventBroker

fb1600e

Remove ViewRouter initialization

da7b9f9

Remove LspInitializingView from ID enum

6cea24d

Add documentation for ViewRouter

17c0ebb

Refactor and enhance EventBroker tests

f4da1d3

Refactor ViewRouter tests for clarity

65c07f7

Remove PluginState class into separate file

9f01b0b

Add documentation to subscription management logic

ebdbcbf

taldekar requested a review from shruti0085 January 31, 2025 00:14

shruti0085 reviewed Jan 31, 2025

View reviewed changes

taldekar added 4 commits January 31, 2025 15:09

Add support for notifying multiple late-subscribers over time of late…

0276e79

…st state (#342)

Remove CODE_REFERENCE_VIEW

78d7439

Rename newActiveViewId to newActiveView

77fc653

Rename ViewId class to AmazonQViewType

581b0eb

shruti0085 approved these changes Jan 31, 2025

View reviewed changes

taldekar changed the base branch from nickdb/lspErrorHandling to feature/viewRefactor January 31, 2025 21:21

taldekar added 4 commits January 31, 2025 16:33

Revert "Integrate Event Broker in LspStatusManager"

b395464

This reverts commit 374e549.

Revert "improved handling of lsp failure state"

c49a354

This reverts commit 13c55ac.

Refactor EventBroker and enhance tests (#343)

4ed1bf1

Merge branch 'feature/viewRefactor' into taldekar/ViewRouter

3b95fcd

taldekar merged commit f975e6f into feature/viewRefactor Feb 4, 2025

taldekar deleted the taldekar/ViewRouter branch February 4, 2025 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce ViewRouter Component #337

Introduce ViewRouter Component #337

taldekar commented Jan 28, 2025 •

edited

Loading

shruti0085 Jan 30, 2025

taldekar Jan 30, 2025 •

edited

Loading

shruti0085 Jan 30, 2025

taldekar Jan 30, 2025

taldekar Jan 30, 2025

shruti0085 Jan 31, 2025

taldekar Jan 31, 2025

shruti0085 Jan 31, 2025

Introduce ViewRouter Component #337

Introduce ViewRouter Component #337

Conversation

taldekar commented Jan 28, 2025 • edited Loading

shruti0085 Jan 30, 2025

Choose a reason for hiding this comment

taldekar Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

shruti0085 Jan 30, 2025

Choose a reason for hiding this comment

taldekar Jan 30, 2025

Choose a reason for hiding this comment

taldekar Jan 30, 2025

Choose a reason for hiding this comment

shruti0085 Jan 31, 2025

Choose a reason for hiding this comment

taldekar Jan 31, 2025

Choose a reason for hiding this comment

shruti0085 Jan 31, 2025

Choose a reason for hiding this comment

taldekar commented Jan 28, 2025 •

edited

Loading

taldekar Jan 30, 2025 •

edited

Loading