Add support for mandatory single result in `LdapClient` search operations

[therepanic]

Refers: spring-projects/spring-security#18627 (comment)

[therepanic]

In any case, I'd also like to address this issue. Specifically, in our case (see spring-projects/spring-security#18627), we'd like to ensure that either the result isn't null or throw a specific error without null checks.

I'd be happy to implement this. Any suggestions? I think the contract needs to be expanded somehow.

[jzheaux]

Thanks for logging this, @therepanic. I think we should take some care when adding this feature.

First, there is an important difference between JdbcClient#MappedQuerySpec and LdapClient#SearchSpec that would be nice to reconcile at this point.

In JdbcClient we have:

statement().query(query).single() // requires one result
statement().query(query).optional() // allows result to be null

And in LdapClient we have:

search().query(query).toEntry() // allows result to be null

It would be nice to be able to do:

search().query(query).single() // requires one result
search().query(query).optional() // allows result to be null

to align with JdbcClient semantics and naming convention.

I would propose, then, making the following changes:

1. Add single, optional, list, and stream, deprecating toEntry, toObject, toList, and toStream
2. Add map(ContextMapper) and map(AttributeMapper) methods that return a new interface (say, MappedSearchSpec) that also has single, optional, list, and stream.

This separates mapping operations from cardinality terminal methods:

search().query(query).single() // requires one LdapDataEntry result
search().query(query).map(contextMapper).single() // requires one result, mapped by ContextMapper
search().query(query).map(attributeMapper).single() // requires one result, mapped by AttributeMapper

This would change the call referenced in the related Spring Security PR to:

ldapClient.search().query(query).single();

and remove the check for null.

Are you able to provide a PR along these lines? Note that any new methods in SearchSpec need to have default implementations in order to preserve backward compatibility.

[therepanic]

Yes, I will be happy to help.

[jzheaux]

@therepanic, I just wanted to look ahead a bit in this comment. It would be nice for this feature to make it into LDAP 4.1; I usually target feature completion with the M3 milestone of a minor release. This would be in the second week of April (I'll update this comment once that's officially scheduled).

Do you feel you have time to put together a PR within the next two weeks? If so, let's keep going as planned. If not, let's flip it; I'll add the PR and have you review it.

[therepanic]

Everything is ok @jzheaux, I will try to do PR today/tomorrow.

[jzheaux]

Awesome! No rush, @therepanic, even if you need more time than that, don't sweat it. Looking forward to collaborating with you on this enhancement.

[jzheaux]

Closing in favor of the associated PR, #1446