rouazana commented on a change in pull request #478:
URL: https://github.com/apache/james-project/pull/478#discussion_r648060705



##########
File path: 
server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUsersDAO.java
##########
@@ -260,133 +195,150 @@ private boolean userInGroupsMembershipList(String 
userDN,
         return results;
     }
 
-    /**
-     * For a given name, this method makes ldap search in userBase with filter 
{@link LdapRepositoryConfiguration#userIdAttribute}=name
-     * and objectClass={@link LdapRepositoryConfiguration#userObjectClass} and 
builds {@link User} based on search result.
-     *
-     * @param name
-     *            The userId which should be value of the field {@link 
LdapRepositoryConfiguration#userIdAttribute}
-     * @return A {@link ReadOnlyLDAPUser} instance which is initialized with 
the
-     *         userId of this user and ldap connection information with which
-     *         the user was searched. Return null if such a user was not found.
-     * @throws NamingException
-     *             Propagated by the underlying LDAP communication layer.
-     */
-    private ReadOnlyLDAPUser searchAndBuildUser(Username name) throws 
NamingException {
-        SearchControls sc = new SearchControls();
-        sc.setSearchScope(SearchControls.SUBTREE_SCOPE);
-        sc.setReturningAttributes(new String[] { 
ldapConfiguration.getUserIdAttribute() });
-        sc.setCountLimit(1);
-
-        String filterTemplate = "(&({0}={1})(objectClass={2})" +
-            StringUtils.defaultString(ldapConfiguration.getFilter(), "") +
-            ")";
-
-        String sanitizedFilter = FilterEncoder.format(
-            filterTemplate,
-            ldapConfiguration.getUserIdAttribute(),
-            name.asString(),
-            ldapConfiguration.getUserObjectClass());
+    private ReadOnlyLDAPUser searchAndBuildUser(Username name) throws 
LDAPException {
+        LDAPConnection connection = ldapConnectionPool.getConnection();
+        try {
+            String sanitizedFilter = FilterEncoder.format(
+                filterTemplate,
+                ldapConfiguration.getUserIdAttribute(),
+                name.asString(),
+                ldapConfiguration.getUserObjectClass());
+
+            SearchResult searchResult = 
connection.search(ldapConfiguration.getUserBase(),
+                SearchScope.SUB,
+                sanitizedFilter,
+                ldapConfiguration.getUserIdAttribute());
+
+            SearchResultEntry result = searchResult.getSearchEntries()
+                .stream()
+                .findFirst()
+                .orElse(null);
+            if (result == null) {
+                return null;
+            }
 
-        NamingEnumeration<SearchResult> sr = 
ldapContext.search(ldapConfiguration.getUserBase(), sanitizedFilter, sc);
+            if (!ldapConfiguration.getRestriction().isActivated()
+                || userInGroupsMembershipList(result.getDN(), 
ldapConfiguration.getRestriction().getGroupMembershipLists(connection))) {
 
-        if (!sr.hasMore()) {
+                return new ReadOnlyLDAPUser(name, result.getDN(), 
ldapConnectionPool, ldapConfiguration);
+            }
             return null;
+        } finally {
+            ldapConnectionPool.releaseConnection(connection);
         }
-
-        SearchResult r = sr.next();
-        Attribute userName = 
r.getAttributes().get(ldapConfiguration.getUserIdAttribute());
-
-        if (!ldapConfiguration.getRestriction().isActivated()
-            || userInGroupsMembershipList(r.getNameInNamespace(), 
ldapConfiguration.getRestriction().getGroupMembershipLists(ldapContext))) {
-            return new 
ReadOnlyLDAPUser(Username.of(userName.get().toString()), 
r.getNameInNamespace(), ldapContext);
-        }
-
-        return null;
     }
 
-    /**
-     * Given a userDN, this method retrieves the user attributes from the LDAP
-     * server, so as to extract the items that are of interest to James.
-     * Specifically it extracts the userId, which is extracted from the LDAP
-     * attribute whose name is given by the value of the field
-     * {@link LdapRepositoryConfiguration#userIdAttribute}.
-     *
-     * @param userDN
-     *            The distinguished-name of the user whose details are to be
-     *            extracted from the LDAP repository.
-     * @return A {@link ReadOnlyLDAPUser} instance which is initialized with 
the
-     *         userId of this user and ldap connection information with which
-     *         the userDN and attributes were obtained.
-     * @throws NamingException
-     *             Propagated by the underlying LDAP communication layer.
-     */
-    private Optional<ReadOnlyLDAPUser> buildUser(String userDN) throws 
NamingException {
-      Attributes userAttributes = ldapContext.getAttributes(userDN);
-      Optional<Attribute> userName = 
Optional.ofNullable(userAttributes.get(ldapConfiguration.getUserIdAttribute()));
-      return userName
-          .map(Throwing.<Attribute, String>function(u -> 
u.get().toString()).sneakyThrow())
-          .map(Username::of)
-          .map(username -> new ReadOnlyLDAPUser(username, userDN, 
ldapContext));
+    private Optional<ReadOnlyLDAPUser> buildUser(String userDN) throws 
LDAPException {
+        LDAPConnection connection = ldapConnectionPool.getConnection();
+        try {
+            SearchResultEntry userAttributes = connection.getEntry(userDN);
+            Optional<String> userName = 
Optional.ofNullable(userAttributes.getAttributeValue(ldapConfiguration.getUserIdAttribute()));
+            return userName
+                .map(Username::of)
+                .map(username -> new ReadOnlyLDAPUser(username, userDN, 
ldapConnectionPool, ldapConfiguration));
+        } finally {
+            ldapConnectionPool.releaseConnection(connection);
+        }
     }
 
     @Override
     public boolean contains(Username name) throws UsersRepositoryException {
-        return getUserByName(name).isPresent();
+        try {
+            return Mono.fromCallable(() -> doContains(name))
+                .retryWhen(ldapConfiguration.retrySpec())

Review comment:
       Which error? You should retry only when connection-related errors.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to