Skip to content

[WIP] Add support for mandatory single result in LdapClient search operations #1446

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

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

[WIP] Add support for mandatory single result in LdapClient search operations #1446

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
143 changes: 143 additions & 0 deletions core/src/main/java/org/springframework/ldap/core/LdapClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

package org.springframework.ldap.core;

import java.util.Set;
import java.util.List;
import java.util.LinkedHashSet;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.stream.Stream;
Expand All @@ -29,9 +32,11 @@
import javax.naming.directory.ModificationItem;
import javax.naming.directory.SearchControls;

import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;

import org.springframework.LdapDataEntry;
import org.springframework.dao.support.DataAccessUtils;
import org.springframework.ldap.NameAlreadyBoundException;
import org.springframework.ldap.PartialResultException;
import org.springframework.ldap.query.LdapQuery;
Expand All @@ -41,6 +46,7 @@
* An LDAP Client
*
* @author Josh Cummings
* @author Andrey Litvitski
* @since 3.1
*/
public interface LdapClient {
Expand Down Expand Up @@ -364,6 +370,86 @@ interface SearchSpec {
*/
SearchSpec query(LdapQuery query);

/**
* Retrieve a single search result as a required {@link LdapDataEntry} instance.
* @return the single result object (never {@code null})
*/
Copy link
Collaborator

@jzheaux jzheaux Mar 20, 2026

Choose a reason for hiding this comment

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

Let's add @since 4.1 to new classes and methods.

default <O extends LdapDataEntry> O single() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return map(cast).single();
}
Comment on lines +376 to +380
Copy link
Contributor Author

@therepanic therepanic Mar 16, 2026

Choose a reason for hiding this comment

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

Perhaps there's a better idea for the default implementation?

Also, should we create tests for the default implementations of these methods? Besides, I don't think we need to reimplement them in DefaultLdapClient?

Copy link
Collaborator

@jzheaux jzheaux Mar 20, 2026

Choose a reason for hiding this comment

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

Yes, let's please test them. It will help us to know when we inadvertently make a non-passive change down the road.


/**
* Retrieve a single search result, if available, as an {@link Optional} handle.
* @return an Optional handle with a single {@link LdapDataEntry} result or none
*/
default <O extends LdapDataEntry> Optional<O> optional() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return map(cast).optional();
}

/**
* Retrieve the result as a pre-resolved list of mapped objects, retaining the
* order from the original database result.
* @return the result as a detached List, containing mapped objects
*/
default <O extends LdapDataEntry> List<O> list() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return map(cast).list();
}

/**
* Retrieve the result as a lazily resolved stream of mapped objects, retaining
* the order from the original database result.
* @return the result Stream, containing mapped objects, needing to be closed once
* fully processed (for example, through a try-with-resources clause)
*/
default <O extends LdapDataEntry> Stream<O> stream() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return map(cast).stream();
}

/**
* Map each search result to an object using the given {@link ContextMapper}
* strategy.
* @param mapper the {@link ContextMapper} strategy to use to map each result
* @return a {@link MappedSearchSpec} for specifying the result cardinality
*/
default <T> MappedSearchSpec<T> map(ContextMapper<T> mapper) {
return new MappedSearchSpec<T>() {
@Override
public List<T> list() {
return toList(mapper);
}

@Override
public Stream<T> stream() {
return toStream(mapper);
}
};
}
Comment on lines +418 to +430
Copy link
Contributor Author

@therepanic therepanic Mar 16, 2026

Choose a reason for hiding this comment

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

This is currently a poorly implemented implementation that uses methods we've depricated. Should we literally take the default implementation for these methods from DefaultLdapClient?

Copy link
Collaborator

@jzheaux jzheaux Mar 20, 2026

Choose a reason for hiding this comment

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

I'd prefer we not use deprecated code where possible. If needed, the copy-paste can be abstracted away in a package-private utility class. Are there any other concerns other than the duplication?


/**
* Map each search result to an object using the given {@link AttributesMapper}
* strategy.
* @param mapper the {@link AttributesMapper} strategy to use to map each result
* @return a {@link MappedSearchSpec} for specifying the result cardinality
*/
default <T> MappedSearchSpec<T> map(AttributesMapper<T> mapper) {
return new MappedSearchSpec<T>() {
@Override
public List<T> list() {
return toList(mapper);
}

@Override
public Stream<T> stream() {
return toStream(mapper);
}
};
}

@Deprecated(since = "4.1.0")
default <O extends LdapDataEntry> @Nullable O toEntry() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return toObject(cast);
Expand All @@ -379,6 +465,7 @@ interface SearchSpec {
* @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the
* result set contains more than one result
*/
@Deprecated(since = "4.1.0")
<O> @Nullable O toObject(ContextMapper<O> mapper);

/**
Expand All @@ -388,8 +475,10 @@ interface SearchSpec {
* @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the
* result set contains more than one result
*/
@Deprecated(since = "4.1.0")
<O> @Nullable O toObject(AttributesMapper<O> mapper);

@Deprecated(since = "4.1.0")
default <O extends LdapDataEntry> List<O> toEntryList() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return toList(cast);
Expand All @@ -402,6 +491,7 @@ default <O extends LdapDataEntry> List<O> toEntryList() {
* @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the
* result set contains more than one result
*/
@Deprecated(since = "4.1.0")
<O> List<O> toList(ContextMapper<O> mapper);

/**
Expand All @@ -411,8 +501,10 @@ default <O extends LdapDataEntry> List<O> toEntryList() {
* @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the
* result set contains more than one result
*/
@Deprecated(since = "4.1.0")
<O> List<O> toList(AttributesMapper<O> mapper);

@Deprecated(since = "4.1.0")
default <O extends LdapDataEntry> Stream<O> toEntryStream() {
ContextMapper<O> cast = (ctx) -> (O) ctx;
return toStream(cast);
Expand All @@ -425,6 +517,7 @@ default <O extends LdapDataEntry> Stream<O> toEntryStream() {
* @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the
* result set contains more than one result
*/
@Deprecated(since = "4.1.0")
<O> Stream<O> toStream(ContextMapper<O> mapper);

/**
Expand All @@ -434,6 +527,7 @@ default <O extends LdapDataEntry> Stream<O> toEntryStream() {
* @throws org.springframework.dao.IncorrectResultSizeDataAccessException if the
* result set contains more than one result
*/
@Deprecated(since = "4.1.0")
<O> Stream<O> toStream(AttributesMapper<O> mapper);

}
Expand Down Expand Up @@ -571,4 +665,53 @@ interface UnbindSpec {

}

interface MappedSearchSpec<T extends @Nullable Object> {

/**
* Retrieve the result as a lazily resolved stream of mapped objects, retaining
* the order from the original database result.
* @return the result Stream, containing mapped objects, needing to be closed once
Comment on lines +668 to +673
Copy link
Contributor Author

@therepanic therepanic Mar 16, 2026

Choose a reason for hiding this comment

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

The implementation of this class is essentially copied from JdbcClient

* fully processed (for example, through a try-with-resources clause)
*/
Stream<T> stream();

/**
* Retrieve the result as a pre-resolved list of mapped objects, retaining the
* order from the original database result.
* @return the result as a detached List, containing mapped objects
*/
List<T> list();

/**
* Retrieve the result as an order-preserving set of mapped objects.
* @return the result as a detached Set, containing mapped objects
* @see #list()
* @see LinkedHashSet
*/
default Set<T> set() {
return new LinkedHashSet<>(list());
}

/**
* Retrieve a single result as a required object instance.
* @return the single result object (never {@code null})
* @see #optional()
* @see DataAccessUtils#requiredSingleResult(Collection)
*/
default @NonNull T single() {
return DataAccessUtils.requiredSingleResult(list());
}

/**
* Retrieve a single result, if available, as an {@link Optional} handle.
* @return an Optional handle with a single result object or none
* @see #single()
* @see DataAccessUtils#optionalResult(Collection)
*/
default Optional<T> optional() {
return DataAccessUtils.optionalResult(list());
}

}

}
Loading