From dfceede0bc37a97a9add54c8e17cb0123880f832 Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Fri, 11 May 2018 08:27:42 +0200 Subject: [PATCH] Improve LDAP auto-configuration conditions 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 --- .../ldap/LdapAutoConfiguration.java | 2 +- .../EmbeddedLdapAutoConfiguration.java | 3 +- .../ldap/LdapAutoConfigurationTests.java | 38 ++++++++++++++++++- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfiguration.java index aef5bcb6c5..794414b98e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfiguration.java @@ -53,7 +53,7 @@ public class LdapAutoConfiguration { @Bean @ConditionalOnMissingBean - public ContextSource ldapContextSource() { + public LdapContextSource ldapContextSource() { LdapContextSource source = new LdapContextSource(); source.setUserDn(this.properties.getUsername()); source.setPassword(this.properties.getPassword()); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/embedded/EmbeddedLdapAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/embedded/EmbeddedLdapAutoConfiguration.java index b7b32510f0..741e377e69 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/embedded/EmbeddedLdapAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ldap/embedded/EmbeddedLdapAutoConfiguration.java @@ -58,7 +58,6 @@ import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.core.io.Resource; import org.springframework.core.type.AnnotatedTypeMetadata; -import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.support.LdapContextSource; import org.springframework.util.StringUtils; @@ -101,7 +100,7 @@ public class EmbeddedLdapAutoConfiguration { @Bean @DependsOn("directoryServer") @ConditionalOnMissingBean - public ContextSource ldapContextSource() { + public LdapContextSource ldapContextSource() { LdapContextSource source = new LdapContextSource(); if (hasCredentials(this.embeddedProperties.getCredential())) { source.setUserDn(this.embeddedProperties.getCredential().getUsername()); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfigurationTests.java index c20bb1f947..2908d817a0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ldap/LdapAutoConfigurationTests.java @@ -20,9 +20,13 @@ import org.junit.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.LdapTemplate; import org.springframework.ldap.core.support.LdapContextSource; +import org.springframework.ldap.pool2.factory.PoolConfig; +import org.springframework.ldap.pool2.factory.PooledContextSource; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -54,7 +58,8 @@ public class LdapAutoConfigurationTests { public void contextSourceWithSingleUrl() { this.contextRunner.withPropertyValues("spring.ldap.urls:ldap://localhost:123") .run((context) -> { - ContextSource contextSource = context.getBean(ContextSource.class); + ContextSource contextSource = context + .getBean(LdapContextSource.class); String[] urls = (String[]) ReflectionTestUtils.getField(contextSource, "urls"); assertThat(urls).containsExactly("ldap://localhost:123"); @@ -67,7 +72,8 @@ public class LdapAutoConfigurationTests { .withPropertyValues( "spring.ldap.urls:ldap://localhost:123,ldap://mycompany:123") .run((context) -> { - ContextSource contextSource = context.getBean(ContextSource.class); + ContextSource contextSource = context + .getBean(LdapContextSource.class); LdapProperties ldapProperties = context.getBean(LdapProperties.class); String[] urls = (String[]) ReflectionTestUtils.getField(contextSource, "urls"); @@ -104,4 +110,32 @@ public class LdapAutoConfigurationTests { .run(context -> assertThat(context).hasSingleBean(LdapTemplate.class)); } + @Test + public void contextSourceWithUserProvidedPooledContextSource() { + this.contextRunner.withUserConfiguration(PooledContextSourceConfig.class) + .run((context) -> { + LdapContextSource contextSource = context + .getBean(LdapContextSource.class); + String[] urls = (String[]) ReflectionTestUtils.getField(contextSource, + "urls"); + assertThat(urls).containsExactly("ldap://localhost:389"); + assertThat(contextSource.isAnonymousReadOnly()).isFalse(); + context.getBean(PooledContextSource.class); + }); + } + + @Configuration + static class PooledContextSourceConfig { + + @Bean + public PooledContextSource pooledContextSource( + LdapContextSource ldapContextSource) { + PooledContextSource pooledContextSource = new PooledContextSource( + new PoolConfig()); + pooledContextSource.setContextSource(ldapContextSource); + return pooledContextSource; + } + + } + }