Skip to content

Conversation

@vpavic
Copy link
Contributor

@vpavic vpavic commented May 11, 2018

At present, auto-configuration of LdapContextSource is conditional on presence of ContextSource bean. However, there are valid use cases which require multiple ContextSource bean, for instance PooledContextSource. With the current arrangement, the auto-configuration of LdapContextSource will back off if user provides PooledContextSource bean, while it would still be reasonable to reuse auto-configured LdapContextSource.

This PR improves LdapContextSource factory method return value and condition to back off only if users actually provide LdapContextSource bean themselves.

With this change in place, users can provide PooledContextSource bean and reuse auto-configured LdapContextSource. PooledContextSource bean should then be marked as @Primary to ensure it is injected in auto-configured LdapTemplate.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 11, 2018
At present, auto-configuration of `LdapContextSource` is conditional on presence of `ContextSource` bean. However, there are valid use cases which require multiple `ContextSource` bean, for instance `PooledContextSource`. With the current arrangement, the auto-configuration of `LdapContextSource` will back off if user provides `PooledContextSource` bean, while it would still be reasonable to reuse auto-configured `LdapContextSource`.

This commit improves `LdapContextSource` factory method return value and condition to back off only if users actually provide `LdapContextSource` bean themselves.
@vpavic vpavic force-pushed the improve-ldap-condition branch from 25d4e76 to 29528b7 Compare May 11, 2018 12:49
snicoll pushed a commit that referenced this pull request May 11, 2018
At present, auto-configuration of `LdapContextSource` is conditional on
presence of a `ContextSource` bean. However, there are valid use cases
which require multiple `ContextSource` bean, for instance
`PooledContextSource`. With the current arrangement, the
auto-configuration of `LdapContextSource` will back off if user provides
a `PooledContextSource` bean, while it would still be reasonable to
reuse the auto-configured `LdapContextSource`.

This commit improves `LdapContextSource` factory method return value and
condition to back off only if users actually provide a
`LdapContextSource` bean themselves.

See gh-13143
@snicoll snicoll closed this in 8e9a873 May 11, 2018
snicoll added a commit that referenced this pull request May 11, 2018
* pr/13143:
  Polish "Improve LDAP auto-configuration conditions"
  Improve LDAP auto-configuration conditions
@snicoll snicoll self-assigned this May 11, 2018
@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels May 11, 2018
@snicoll snicoll modified the milestones: 2.0.3, 2.1.0.M1 May 11, 2018
@snicoll
Copy link
Member

snicoll commented May 11, 2018

Thanks Vedran! I've polished the PR in 8e9a873.

@vpavic vpavic deleted the improve-ldap-condition branch May 11, 2018 16:50
@vpavic
Copy link
Contributor Author

vpavic commented May 11, 2018

I've polished the PR in 8e9a873.

Thanks for that - I rebased on top of changes from #13136 in rush and messed up that test.

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

Labels

type: task A general task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants