From dfceede0bc37a97a9add54c8e17cb0123880f832 Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Fri, 11 May 2018 08:27:42 +0200 Subject: [PATCH 1/2] 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; + } + + } + } From 8e9a873787aeb4a87c758c892b22a8f842d4f996 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 11 May 2018 17:48:29 +0200 Subject: [PATCH 2/2] Polish "Improve LDAP auto-configuration conditions" Closes gh-13143 --- .../boot/autoconfigure/ldap/LdapAutoConfigurationTests.java | 3 ++- .../src/main/asciidoc/spring-boot-features.adoc | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) 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 2908d817a0..9e54fd144a 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 @@ -22,6 +22,7 @@ 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.context.annotation.Primary; import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.LdapTemplate; import org.springframework.ldap.core.support.LdapContextSource; @@ -120,7 +121,6 @@ public class LdapAutoConfigurationTests { "urls"); assertThat(urls).containsExactly("ldap://localhost:389"); assertThat(contextSource.isAnonymousReadOnly()).isFalse(); - context.getBean(PooledContextSource.class); }); } @@ -128,6 +128,7 @@ public class LdapAutoConfigurationTests { static class PooledContextSourceConfig { @Bean + @Primary public PooledContextSource pooledContextSource( LdapContextSource ldapContextSource) { PooledContextSource pooledContextSource = new PooledContextSource( diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index c464502ca2..7568434c1b 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -4539,6 +4539,11 @@ URLs of your server in your application.properties, as shown in the following ex If you need to customize connection settings, you can use the `spring.ldap.base` and `spring.ldap.base-environment` properties. +A `LdapContextSource` is auto-configured based on these settings. If you need to customize +it, for instance to use a `PooledContextSource`, you can still inject the auto-configured +`LdapContextSource`. Make sure to flag your customized `ContextSource` as `@Primary` so +that the auto-configured `LdapTemplate` uses it. + [[boot-features-ldap-spring-data-repositories]]