Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add remaining CDI normal scopes: RouteScoped, UIScoped, VaadinSessionScoped #25

Merged
merged 13 commits into from
Sep 3, 2021
Merged

Add remaining CDI normal scopes: RouteScoped, UIScoped, VaadinSessionScoped #25

merged 13 commits into from
Sep 3, 2021

Conversation

denis-anisimov
Copy link

@denis-anisimov denis-anisimov commented Sep 1, 2021 •
edited
Loading

Fixes #13

Description

Please list all relevant dependencies in this section and provide summary of the change, motivation and context.

Fixes # (issue)

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

Denis Anisimov added 7 commits August 25, 2021 19:14
feat: implement VaadinSessionScope
c525dd8
chore: correct tests for session scope
1ad9751
chore: correct IT test for session
22f6e44
feat: add Vaadin UI scope
488e12e
feat: modify UI scope context aiming normal scope
bd737cf 
- Since Quakrus at the moment doesn't support pseudo scoped properly the only UI scope which works is normal. Use it as a primary scope in the impl.
- Rework IT tests for UI scope.
- Add initial RouteScope support in the same way as UI : normal scope is a primary scope since it's the only supported scope.
chore: remove pseudo scope annotations since they don't work
0fadf91
feat: add RouteScope for quarkus as ability to observes UI events
dd774b9 
- Add RouteScope context impl
- Register Route scope
- Add unit tests for Route scope
- Add ITs for Route Scope
- Fire UI evens using bean manager to be able to observe them via CDI
- Add ITs for UI events
@denis-anisimov denis-anisimov mentioned this pull request Sep 1, 2021
Closed
5 tasks
caalador
caalador previously requested changes Sep 1, 2021
View reviewed changes
...n-test-code/src/main/java/com/vaadin/flow/quarkus/it/routecontext/PreserveOnRefreshBean.java Outdated Show resolved Hide resolved
...tion-tests/common-test-code/src/test/java/com/vaadin/flow/quarkus/it/SessionContextTest.java Outdated Show resolved Hide resolved
runtime/src/main/java/com/vaadin/quarkus/context/RouteScopedContext.java Outdated Show resolved Hide resolved
runtime/src/main/java/com/vaadin/quarkus/context/RouteScopedContext.java Show resolved Hide resolved
runtime/src/test/java/com/vaadin/quarkus/context/BeanManagerProxy.java Outdated Show resolved Hide resolved
runtime/src/test/java/com/vaadin/quarkus/context/SessionUnderTestContext.java Outdated Show resolved Hide resolved
runtime/src/test/java/com/vaadin/quarkus/context/TestHasElement.java Outdated Show resolved Hide resolved
runtime/src/test/java/com/vaadin/quarkus/context/TestRouteScopedContext.java Outdated Show resolved Hide resolved
runtime/src/test/java/com/vaadin/quarkus/context/UiContextTest.java Outdated Show resolved Hide resolved
@caalador caalador dismissed their stale review September 1, 2021 10:57

Seems fine, Mikhail will check the RouteScope tests as I don't have that much knowledge on how they should go.

mshabarov
mshabarov approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@mshabarov mshabarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite I haven't review all the tests and route scope related code, I propose to merge this PR to not block the further development of scopes implementation and recheck it later-on.

@mshabarov mshabarov merged commit 7bf82e9 into master Sep 3, 2021
@mshabarov mshabarov deleted the 13-other-scopes branch September 3, 2021 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caalador caalador caalador left review comments

@mshabarov mshabarov mshabarov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement custom CDI scopes for Quarkus
3 participants
@denis-anisimov @caalador @mshabarov