Skip to content

Instantiate exceptions lazily #7996

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
Merged
merged 2 commits into from
Feb 27, 2020
Merged

Instantiate exceptions lazily #7996

merged 2 commits into from
Feb 27, 2020

Conversation

robotmrv
Copy link
Contributor

Add lazy Exception instantiation and reduce amount of
reactor operators to reduce allocations

Fixes gh-7995

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 19, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Feb 20, 2020

Thanks for the polish, @robotmrv. In preparation for merging, will you make two adjustments to the commit messages, please?

  1. Both commits refer to additional cleanup items. Would you please ensure that one commit is about lazy exceptions and the other is about the additional cleanup?
  2. One of the commits has a long commit message - it's preferred that commit messages be separated into a title of fewer than 50 characters and an optional description for more detail.

@robotmrv
Copy link
Contributor Author

@jzheaux no problem
I thought they would eventually be squashed into one

@jzheaux
Copy link
Contributor

jzheaux commented Feb 20, 2020

I see, @robotmrv. The advantage of separating the commits, in this case, is that it clarifies what precisely was needed to fix the issue vs what was a related polish. This can be useful for reference in the future.

@robotmrv
Instantiate exceptions lazily
18f62d7 
Add lazy Exception instantiation to reduce allocations

Fixes spring-projectsgh-7995
@robotmrv
Copy link
Contributor Author

@jzheaux, done
I've change a little bit second commit. Use Mono#map instead of Mono#handle as it is more efficient.
It now throws from mapper lambda but it seems that reactor-core team is ok with this (see Simon`s Basle comment to the answer https://stackoverflow.com/questions/53595420/correct-way-of-throwing-exceptions-with-reactor/53596358#53596358)

@jzheaux jzheaux self-assigned this Feb 20, 2020
@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 20, 2020
@robotmrv
polish spring-projectsgh-7996
a92f1d7 
Make defensive collection copy as Collections.unmodifiableCollection
does not protect from the source collection direct modification.
Use Mono#map instead of Mono#flatMap as it allocates less.
Use less operators to reduce allocations.
Use lambda parameter instead of outer method parameter
in authenticationManagers#computeIfAbsent()
to make it non capturing so it could be cached by JVM.
Propagate cause for InvalidBearerTokenException.
@jzheaux jzheaux merged commit 9d66f2c into spring-projects:master Feb 27, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Feb 27, 2020

Thanks, @robotmrv! This is now merged into master.

@jzheaux jzheaux added this to the 5.3.0 milestone Feb 27, 2020
@jzheaux jzheaux added the status: duplicate A duplicate of another issue label Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jzheaux jzheaux jzheaux left review comments

Assignees

@jzheaux jzheaux

Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
5.3.0
Development

Successfully merging this pull request may close these issues.

JwtIssuerReactiveAuthenticationManagerResolver eagerly creates Exceptions
3 participants
@robotmrv @jzheaux @spring-projects-issues