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.

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

Fix for issue #20092 #20103

Merged
merged 6 commits into from
Oct 9, 2024
Merged

Conversation

archiecobbs
Copy link
Contributor

Description

Fix for #20092: VaadinSessionScopes for all sessions are destroyed when any single session expires.

  • Add new method VaadinSession.addSessionDestroyListener() for when you just want one destroy notificatios for a single session, instead of geting notifications for all sessions via VaadinService.addSessionDestroyListener().

  • Refactor the Spring scopes to take advantage of the new method to fix an inefficiency.

  • Mark SpringVaadinSession, which is now empty and useless, for deprecation.

Fixes #20092 (see further discussion there)

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.

…le session expires.

* Add new method VaadinSession.addSessionDestroyListener() for when you just want one destroy
  notificatios for a single session, instead of geting notifications for all sessions via
  VaadinService.addSessionDestroyListener().

* Refactor the Spring scopes to take advantage of the new method to fix an inefficiency.

* Mark SpringVaadinSession, which is now empty and useless, for deprepcation.

Fixes vaadin#20092.
@mshabarov mshabarov added the Contribution PRs coming from the community or external to the team label Oct 3, 2024
@mshabarov
Copy link
Contributor

@archiecobbs thank you for the contribution!

Copy link

github-actions bot commented Oct 8, 2024

Test Results

1 141 files  ± 0  1 141 suites  ±0   1h 28m 21s ⏱️ +16s
7 447 tests ± 0  7 397 ✅ +22  50 💤 ±0  0 ❌  - 10 
7 818 runs  +14  7 758 ✅ +36  60 💤 ±0  0 ❌  - 10 

Results for commit cd79aad. ± Comparison against base commit 1fbf412.

♻️ This comment has been updated with latest results.

@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Oct 8, 2024
@caalador
Copy link
Contributor

caalador commented Oct 8, 2024

Seems good apart from the formatting.
Could you run mvn formatter:format -pl flow-server,vaadin-spring and push the formatted sources.

@archiecobbs
Copy link
Contributor Author

Could you run mvn formatter:format -pl flow-server,vaadin-spring and push the formatted sources.

Done in 7f8ab58. Thanks.

caalador
caalador previously approved these changes Oct 9, 2024
@vaadin-bot vaadin-bot added +1.0.0 and removed +0.0.1 labels Oct 9, 2024
Copy link
Contributor

@tepi tepi left a comment

Choose a reason for hiding this comment

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

Please see comment about SpringVaadinSession

@vaadin-bot vaadin-bot added +0.1.0 and removed +1.0.0 labels Oct 9, 2024
Copy link

sonarcloud bot commented Oct 9, 2024

@tepi tepi requested a review from caalador October 9, 2024 15:12
@tepi
Copy link
Contributor

tepi commented Oct 9, 2024

@caalador looks good to me, but please have another look

@tepi tepi merged commit 2d42aad into vaadin:main Oct 9, 2024
26 checks passed
@archiecobbs
Copy link
Contributor Author

Thanks!

vaadin-bot pushed a commit that referenced this pull request Oct 9, 2024
…le session expires #20092 (#20103)

* fix: VaadinSessionScopes for all sessions are destroyed when any single session expires.

* Add new method VaadinSession.addSessionDestroyListener() for when you just want one destroy
  notificatios for a single session, instead of geting notifications for all sessions via
  VaadinService.addSessionDestroyListener().

* Refactor the Spring scopes to take advantage of the new method to fix an inefficiency.

* Mark SpringVaadinSession, which is now empty and useless, for deprecation.

Fixes #20092.

* Apply formatter.

* Restore public method SpringVaadinSession.addDestroyListener().

* Restore public method SpringVaadinSession.fireSessionDestroy().

---------

Co-authored-by: caalador <[email protected]>
Co-authored-by: Teppo Kurki <[email protected]>
vaadin-bot pushed a commit that referenced this pull request Oct 9, 2024
…le session expires #20092 (#20103)

* fix: VaadinSessionScopes for all sessions are destroyed when any single session expires.

* Add new method VaadinSession.addSessionDestroyListener() for when you just want one destroy
  notificatios for a single session, instead of geting notifications for all sessions via
  VaadinService.addSessionDestroyListener().

* Refactor the Spring scopes to take advantage of the new method to fix an inefficiency.

* Mark SpringVaadinSession, which is now empty and useless, for deprecation.

Fixes #20092.

* Apply formatter.

* Restore public method SpringVaadinSession.addDestroyListener().

* Restore public method SpringVaadinSession.fireSessionDestroy().

---------

Co-authored-by: caalador <[email protected]>
Co-authored-by: Teppo Kurki <[email protected]>
mcollovati pushed a commit that referenced this pull request Oct 9, 2024
…le session expires #20092 (#20103) (#20203)

* fix: VaadinSessionScopes for all sessions are destroyed when any single session expires.

* Add new method VaadinSession.addSessionDestroyListener() for when you just want one destroy
  notificatios for a single session, instead of geting notifications for all sessions via
  VaadinService.addSessionDestroyListener().

* Refactor the Spring scopes to take advantage of the new method to fix an inefficiency.

* Mark SpringVaadinSession, which is now empty and useless, for deprecation.

Fixes #20092.

* Apply formatter.

* Restore public method SpringVaadinSession.addDestroyListener().

* Restore public method SpringVaadinSession.fireSessionDestroy().

---------

Co-authored-by: Archie L. Cobbs <[email protected]>
Co-authored-by: caalador <[email protected]>
Co-authored-by: Teppo Kurki <[email protected]>
mcollovati pushed a commit that referenced this pull request Oct 9, 2024
…le session expires #20092 (#20103) (#20204)

* fix: VaadinSessionScopes for all sessions are destroyed when any single session expires.

* Add new method VaadinSession.addSessionDestroyListener() for when you just want one destroy
  notificatios for a single session, instead of geting notifications for all sessions via
  VaadinService.addSessionDestroyListener().

* Refactor the Spring scopes to take advantage of the new method to fix an inefficiency.

* Mark SpringVaadinSession, which is now empty and useless, for deprecation.

Fixes #20092.

* Apply formatter.

* Restore public method SpringVaadinSession.addDestroyListener().

* Restore public method SpringVaadinSession.fireSessionDestroy().

---------

Co-authored-by: Archie L. Cobbs <[email protected]>
Co-authored-by: caalador <[email protected]>
Co-authored-by: Teppo Kurki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VaadinSessionScopes for all sessions are destroyed when any single session expires
5 participants