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

Improve JdbcUserDetailsManager.userExists method #14649

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Shenker93
Copy link

Hello everyone,

This proposal is about tiny enhancements applied to JdbcUserDetailsManager.userExists() method.
As we do not really use an information returned by select query
select username from users where username = ?,
and only resultset size is important, my idea is to replace it with an ordinary count query.
Some pros of this approach:

  • It works faster on DB side and IMO the code is more straightforward in this case, you see what's the purpose just looking on sql
  • No need to create unnecessary wrappers on Java side. Currently JdbcTemplate creates a list with a one single purpose - to let the code check its size - and then it goes directly to GC. Not used for iterating over it or any modification/read operations.
  • Overloaded version of queryForList() method used there is already marked as @Deprecated as of Spring 5.3 release in favor of queryForList(String, Class, Object...). However that's a separate topic, some other JdbcUserDetailsManager methods also use it, e.g. findUsersInGroup(), that could be resolved in a separate PR to not mix the things up.

Also, a unit test was added in case if userExists() was invoked with a 'null' parameter. That use-case is not something that really-really should be used as most databases expect is [not] null query pattern instead (well, with rare exclusions for null-aware mode support), However as long as this method is designed in this fail-safe manner, let's create a test to be informed about any breaking changes.

@Shenker93 Shenker93 force-pushed the JdbcUserDetailsManager-improvements branch from 5f80800 to 680e87d Compare February 24, 2024 08:25
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants