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.

Sign up for GitHub

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

Added config interceptor for in-memory server #1000

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

Added config interceptor for in-memory server #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions modules/ROOT/pages/testing.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,54 @@ To use it, create a bean similar to the following:
====

Also, `org.springframework.ldap.test.unboundid.LdapTestUtils` provide methods to programmatically work with an embedded LDAP server.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this to earlier in the doc and demonstrate it through the EmbeddedLdapServerFactoryBean class?

'''

Moreover, all the options enumerated ahead will support a variation that allows us to pass an `EmbeddedLdapServer.ConfigInterceptor` as a parameter.
This functional interface extends `Consumer<InMemoryDirectoryServerConfig>` and will be invoked before initializing the server.

This is how we can implement the `ConfigInterceptor` to make the in-memory server use a temporary file for logging the code:
====
[source,java]
----
class UpdateCodeLogDetails implements EmbeddedLdapServer.ConfigInterceptor {
@Override
public void accept(InMemoryDirectoryServerConfig config) {
config.setCodeLogDetails(tempLogFile, true);
}
}
----
====

Now, let's update our configuration to initialize our custom `ConfigInterceptor` and pass as a parameter to `TestContextSourceFactoryBean`:

====
[source,xml]
----
<bean id="updateCodeLogDetails" class="org.springframework.ldap.test.unboundid.TestContextSourceFactoryBeanTests.UpdateCodeLogDetails">
</bean>

<bean id="contextSource" class="org.springframework.ldap.test.unboundid.TestContextSourceFactoryBean">
<property name="defaultPartitionSuffix" value="dc=jayway,dc=se" />
<property name="defaultPartitionName" value="jayway" />
<property name="principal" value="uid=admin,ou=system" />
<property name="password" value="secret" />
<property name="ldifFile" value="setup_data.ldif" />
<property name="port" value="18881" />
<property name="configInterceptor" ref="updateCodeLogDetails" />
</bean>
----
====

Even if we choose to programmatically start the embedded LDAP server using `LdapTestUtils`,
we can still supply the `ConfigInterceptor` as a lambda expression:

====

[source,java]
----
LdapTestUtils.startEmbeddedServer(1888, "dc=jayway,dc=se", "jayway",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to focus on using EmbeddedLdapServer directly. Also, please consider wrapping this in a @Bean definition so that it's equivalent to the XML snippet.

config -> config.setCodeLogDetails(tempLogFile, true));
----
====

21 changes: 19 additions & 2 deletions test-support/src/main/java/org/springframework/ldap/test/unboundid/EmbeddedLdapServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.ldap.test.unboundid;

import java.util.function.Consumer;

import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
import com.unboundid.ldap.listener.InMemoryListenerConfig;
Expand All @@ -38,13 +40,18 @@ private EmbeddedLdapServer(InMemoryDirectoryServer directoryServer) {

public static EmbeddedLdapServer newEmbeddedServer(String defaultPartitionName, String defaultPartitionSuffix,
int port) throws Exception {
return newEmbeddedServer(defaultPartitionName, defaultPartitionSuffix, port, ConfigInterceptor.doNothing());
}

public static EmbeddedLdapServer newEmbeddedServer(String defaultPartitionName, String defaultPartitionSuffix,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to not continue extending the list of parameters for this method. In fact, I think the existing static approach should likely be deprecated at some point.

Instead, let's create a Builder class that constructs an EmbeddedLdapServer. This will allow some reasonable defaults for the port (0) and partition name (the first dc) that the builder can take on, ultimately simplifying construction, e.g.:

Name suffix = LdapUtils.newLdapName("dc=jayway,dc=se");
EmbeddedLdapServer ldap = EmbeddedLdapServer
    .withPartitionSuffix(suffix)
    .config((config) -> config.setCodeLogDetails(...)).build();

Separating this concern allows the instance to be started up more strategically.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In going this route, will you please in a separate commit add a start method? It should wrap any UnboundID exceptions and rethrow a RuntimeException.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @jzheaux, I'll start working on these comments.

Since we're add the start() method, what do you think about implementing AutoClosable as well (and call the already existing shotdown() ) ?
For those who create the server programmatically using the builder, this will allow them to create it in a try-with-resources block.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this is a good idea, @etrandafir93. Will you please open a ticket to add the start() and AutoCloseable and then reference that ticket in this separate commit?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzheaux sure, I created #1025. I can also do it in 2 different PRs - or do you prefer commits in the same PR?

int port, ConfigInterceptor configInterceptor) throws Exception {

InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig(defaultPartitionSuffix);
config.addAdditionalBindCredentials("uid=admin,ou=system", "secret");

config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("LDAP", port));

config.setEnforceSingleStructuralObjectClass(false);
config.setEnforceAttributeSyntaxCompliance(true);
configInterceptor.accept(config);

Entry entry = new Entry(new DN(defaultPartitionSuffix));
entry.addAttribute("objectClass", "top", "domain", "extensibleObject");
Expand All @@ -56,6 +63,16 @@ public static EmbeddedLdapServer newEmbeddedServer(String defaultPartitionName,
return new EmbeddedLdapServer(directoryServer);
}

@FunctionalInterface
public interface ConfigInterceptor extends Consumer<InMemoryDirectoryServerConfig> {

static ConfigInterceptor doNothing() {
return config -> {
};
}

}

public void shutdown() throws Exception {
this.directoryServer.shutDown(true);
}
Expand Down
10 changes: 9 additions & 1 deletion .../src/main/java/org/springframework/ldap/test/unboundid/EmbeddedLdapServerFactoryBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.ldap.test.unboundid;

import org.springframework.beans.factory.config.AbstractFactoryBean;
import org.springframework.ldap.test.unboundid.EmbeddedLdapServer.ConfigInterceptor;

/**
* @author Mattias Hellborg Arthursson
Expand All @@ -29,6 +30,8 @@ public class EmbeddedLdapServerFactoryBean extends AbstractFactoryBean<EmbeddedL

private String partitionSuffix;

private ConfigInterceptor configInterceptor = ConfigInterceptor.doNothing();

@Override
public Class<?> getObjectType() {
return EmbeddedLdapServer.class;
Expand All @@ -42,13 +45,18 @@ public void setPartitionSuffix(String partitionSuffix) {
this.partitionSuffix = partitionSuffix;
}

public void setConfigInterceptor(ConfigInterceptor configInterceptor) {
this.configInterceptor = configInterceptor;
}

public void setPort(int port) {
this.port = port;
}

@Override
protected EmbeddedLdapServer createInstance() throws Exception {
return EmbeddedLdapServer.newEmbeddedServer(this.partitionName, this.partitionSuffix, this.port);
return EmbeddedLdapServer.newEmbeddedServer(this.partitionName, this.partitionSuffix, this.port,
this.configInterceptor);
}

@Override
Expand Down
30 changes: 25 additions & 5 deletions test-support/src/main/java/org/springframework/ldap/test/unboundid/LdapTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,22 @@
import javax.naming.directory.DirContext;
import javax.naming.ldap.LdapName;

import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldif.LDIFReader;
import org.apache.commons.io.IOUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.core.io.Resource;
import org.springframework.ldap.UncategorizedLdapException;
import org.springframework.ldap.core.ContextSource;
import org.springframework.ldap.core.LdapAttributes;
import org.springframework.ldap.core.support.DefaultDirObjectFactory;
import org.springframework.ldap.ldif.parser.LdifParser;
import org.springframework.ldap.support.LdapUtils;
import org.springframework.ldap.test.unboundid.EmbeddedLdapServer.ConfigInterceptor;

import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldif.LDIFReader;

/**
* Utilities for starting, stopping and populating an in-process Apache Directory Server
Expand Down Expand Up @@ -73,12 +75,30 @@ private LdapTestUtils() {
* @throws IllegalStateException if an embedded server is already started.
*/
public static void startEmbeddedServer(int port, String defaultPartitionSuffix, String defaultPartitionName) {
startEmbeddedServer(port, defaultPartitionSuffix, defaultPartitionName, ConfigInterceptor.doNothing());
}

/**
* Start an embedded Apache Directory Server. Only one embedded server will be
* permitted in the same JVM.
* @param port the port on which the server will be listening.
* @param defaultPartitionSuffix The default base suffix that will be used for the
* LDAP server.
* @param defaultPartitionName The name to use in the directory server configuration
* for the default base suffix.
* @param configInterceptor a function that can intercept and modify the
* {@link InMemoryDirectoryServerConfig} before initiating the in-memory server.
* @throws IllegalStateException if an embedded server is already started.
*/
public static void startEmbeddedServer(int port, String defaultPartitionSuffix, String defaultPartitionName,
ConfigInterceptor configInterceptor) {
if (embeddedServer != null) {
throw new IllegalStateException("An embedded server is already started");
}

try {
embeddedServer = EmbeddedLdapServer.newEmbeddedServer(defaultPartitionName, defaultPartitionSuffix, port);
embeddedServer = EmbeddedLdapServer.newEmbeddedServer(defaultPartitionName, defaultPartitionSuffix, port,
configInterceptor);
}
catch (Exception ex) {
throw new UncategorizedLdapException("Failed to start embedded server", ex);
Expand Down
10 changes: 9 additions & 1 deletion ...t/src/main/java/org/springframework/ldap/test/unboundid/TestContextSourceFactoryBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.springframework.ldap.core.support.DefaultDirObjectFactory;
import org.springframework.ldap.core.support.LdapContextSource;
import org.springframework.ldap.support.LdapUtils;
import org.springframework.ldap.test.unboundid.EmbeddedLdapServer.ConfigInterceptor;

/**
* @author Mattias Hellborg Arthursson
Expand Down Expand Up @@ -53,6 +54,8 @@ public class TestContextSourceFactoryBean extends AbstractFactoryBean<ContextSou

private ContextSource contextSource;

private ConfigInterceptor configInterceptor = ConfigInterceptor.doNothing();

public void setAuthenticationSource(AuthenticationSource authenticationSource) {
this.authenticationSource = authenticationSource;
}
Expand Down Expand Up @@ -97,8 +100,13 @@ public void setContextSource(ContextSource contextSource) {
this.contextSource = contextSource;
}

public void setConfigInterceptor(ConfigInterceptor configInterceptor) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's please leave this out of the TestContextSourceFactoryBean class for now. I'd prefer that folks configure the server and the LdifPopulator separately.

this.configInterceptor = configInterceptor;
}

protected ContextSource createInstance() throws Exception {
LdapTestUtils.startEmbeddedServer(this.port, this.defaultPartitionSuffix, this.defaultPartitionName);
LdapTestUtils.startEmbeddedServer(this.port, this.defaultPartitionSuffix, this.defaultPartitionName,
this.configInterceptor);

if (this.contextSource == null) {
// If not explicitly configured, create a new instance.
Expand Down
49 changes: 43 additions & 6 deletions .../test/java/org/springframework/ldap/test/unboundid/TestContextSourceFactoryBeanTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@

package org.springframework.ldap.test.unboundid;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;

import javax.naming.NamingException;
import javax.naming.directory.Attributes;

import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Test;

import org.springframework.context.support.ClassPathXmlApplicationContext;
Expand All @@ -35,15 +40,31 @@ public class TestContextSourceFactoryBeanTests {

ClassPathXmlApplicationContext ctx;

@After
public void setup() {
if (this.ctx != null) {
this.ctx.close();
}
static String tempLogFile;

@BeforeClass
public static void before() throws IOException {
tempLogFile = Files.createTempFile("ldap-log-", ".txt").toAbsolutePath().toString();
}

@Test
public void testServerStartup_withCustomConfig() {
this.ctx = new ClassPathXmlApplicationContext(
"/applicationContext-testContextSource-withCustomInterceptor.xml");
LdapTemplate ldapTemplate = this.ctx.getBean(LdapTemplate.class);
assertThat(ldapTemplate).isNotNull();

ldapTemplate.search(LdapQueryBuilder.query().where("objectclass").is("person"), new AttributesMapper<>() {
public String mapFromAttributes(Attributes attrs) throws NamingException {
return (String) attrs.get("cn").get();
}
});

assertThat(Path.of(tempLogFile)).isNotEmptyFile();
}

@Test
public void testServerStartup() throws Exception {
public void testServerStartup() {
this.ctx = new ClassPathXmlApplicationContext("/applicationContext-testContextSource.xml");
LdapTemplate ldapTemplate = this.ctx.getBean(LdapTemplate.class);
assertThat(ldapTemplate).isNotNull();
Expand All @@ -57,4 +78,20 @@ public String mapFromAttributes(Attributes attrs) throws NamingException {
assertThat(list.size()).isEqualTo(5);
}

@After
public void setup() {
if (this.ctx != null) {
this.ctx.close();
}
}

static class UpdateCodeLogDetails implements EmbeddedLdapServer.ConfigInterceptor {

@Override
public void accept(InMemoryDirectoryServerConfig config) {
config.setCodeLogDetails(tempLogFile, true);
}

}

}
23 changes: 23 additions & 0 deletions ...support/src/test/resources/applicationContext-testContextSource-withCustomInterceptor.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd">

<bean id="ldapTemplate" class="org.springframework.ldap.core.LdapTemplate">
<property name="contextSource" ref="contextSource" />
</bean>

<bean id="updateCodeLogDetails" class="org.springframework.ldap.test.unboundid.TestContextSourceFactoryBeanTests.UpdateCodeLogDetails">
</bean>

<bean id="contextSource" class="org.springframework.ldap.test.unboundid.TestContextSourceFactoryBean">
<property name="defaultPartitionSuffix" value="dc=jayway,dc=se" />
<property name="defaultPartitionName" value="jayway" />
<property name="principal" value="uid=admin,ou=system" />
<property name="password" value="secret" />
<property name="ldifFile" value="setup_data.ldif" />
<property name="port" value="18881" />
<property name="configInterceptor" ref="updateCodeLogDetails" />
</bean>

</beans>