spring-projects / spring-ldap

Spring LDAP
https://spring.io/spring-ldap
Apache License 2.0
339 stars 479 forks source link

Add support to return a Java 8 `Stream` from `LdapTemplate` #586

Closed mp911de closed 1 year ago

mp911de commented 2 years ago

It would be great if LdapTemplate could bridge NamingEnumeration into a Java 8 Stream exposing the following methods:

Stream<T> find(LdapQuery query, Class<T> clazz);

Stream<T> find(LdapQuery query, ContextMapper<T> mapper);

This would be useful in the context of Spring Data to natively expose the LDAP result as stream without collecting results into a Collection. Since NamingEnumeration is closeable, the Stream needs to be closed after consumption but that is a regular requirement when used in other modules such as JdbcTemplate.queryForStream(…) or JPA stream queries.

501 seems a related ticket.

jzheaux commented 1 year ago

Thanks for the suggestion, @mp911de.

How did you decide on those two methods? Several LdapTemplate methods return a List and could possibly benefit from a Stream equivalent. Or, IOW, I'm wondering if any other methods should be considered.

jzheaux commented 1 year ago

After some further analysis, it seems search(LdapQuery, ContextMapper<T>) is a better fit. Also, search(LdapQuery, AttributeMapper<T>) could benefit from a Stream option.

What I'd recommend are the following three new methods:

Stream<T> findStream(LdapQuery query, Class<T> clazz);

Stream<T> searchStream(LdapQuery query, AttributeMapper<T> mapper);

Stream<T> searchStream(LdapQuery query, ContextMapper<T> mapper);
mp911de commented 1 year ago

findStream/searchStream are going into the right direction. Maybe just stream for a less clunky name.

A method returning Stream and accepting ContextMapper seems weird at first sight (one could always call stream.map(mapper::mapFromContext) but given that mapFromContext can throw an exception, the design proposal makes sense.