Skip to content

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Sep 10, 2025

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 10, 2025
@rwinch
Copy link
Member

rwinch commented Sep 12, 2025

Thank you for the pull request. However, I think that we want to deprecate those methods as well. We won't remove the methods until JdbcDaoSupport is removed, so I'd prefer to leave this until Framework removes the class. I'm going to close this pull request.

If you are looking for a pull request, Spring Security should have alternative ways to inject the DataSource or JdbcOperations that are not deprecated (most likely a constructor argument) and is missing some of these. JdbcDaoImpl does not have a way to inject either, so you can add a two new constructors for users to leverage without getting a deprecation warning. You would then deprecate the default constructor for removal. JdbcUserDetailsManager has a DataSource constructor, but you can add a constructor that accepts JdbcOperations and deprecate the default constructor.

@rwinch rwinch closed this Sep 12, 2025
@rwinch rwinch self-assigned this Sep 12, 2025
@rwinch rwinch added status: declined A suggestion or change that we don't feel we should currently apply in: core An issue in spring-security-core and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 12, 2025
@quaff
Copy link
Contributor Author

quaff commented Sep 12, 2025

If breaking changes is acceptable, I think it's better to drop default constructor and setter injection methods:

	private final JdbcOperations jdbcOperations;

	public JdbcDaoImpl(JdbcOperations jdbcOperations) {
		this.jdbcOperations = jdbcOperations;
	}

	public JdbcDaoImpl(DataSource dataSource) {
		this.jdbcOperations = new JdbcTemplate(dataSource);
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants