Apply suggestions from code review

Co-authored-by: Pearl Dsilva <[email protected]>

Author: DaanHoogland

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapImportUsersCmd.java

@@ -105,14 +105,14 @@ public LdapImportUsersCmd(final LdapManager ldapManager, final DomainService dom
     private void createCloudstackUserAccount(LdapUser user, String accountName, Domain domain) {
         Account account = _accountService.getActiveAccountByName(accountName, domain.getId());
         if (account == null) {
-            logger.debug("No account exists with name: {} creating the account and an user with name: {} in the account", accountName, user.getUsername());
+            logger.debug("No account exists with name: {}. Creating the account and a user with name: {} in the account", accountName, user.getUsername());
             _accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, getAccountType(), getRoleId(),
                     domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP);
         } else {
 //            check if the user exists. if yes, call update
             UserAccount csuser = _accountService.getActiveUserAccount(user.getUsername(), domain.getId());
             if (csuser == null) {
-                logger.debug("No user exists with name: {} creating a user in the account: {}", user.getUsername(), accountName);
+                logger.debug("No user exists with name: {}. Creating a user in the account: {}", user.getUsername(), accountName);
                 _accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(),
                         UUID.randomUUID().toString(), User.Source.LDAP);
             } else {
@@ -201,11 +201,11 @@ private Domain getDomainForName(String name) {
     private Domain getDomain(LdapUser user) {
         Domain domain;
         if (_domain != null) {
-            //this means either domain id or group name is passed and this will be same for all the users in this call. hence returning it.
+            // This means that either a domain id or group name is passed and this will be same for all the users in this call, hence returning it.
             domain = _domain;
         } else {
             if (domainId != null) {
-                // a domain ID is passed. use it for this user and all the users in the same api call (by setting _domain)
+                // a domain ID is passed. Use it for this user and all the users in the same API call (by setting _domain)
                 domain = _domain = _domainService.getDomain(domainId);
             } else {
                 // a group name is passed. use it for this user and all the users in the same api call(by setting _domain)

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java

@@ -223,7 +223,7 @@ private boolean isACloudStackUser(String username) {
         if (CollectionUtils.isNotEmpty(cloudstackUsers)) {
             for (final UserResponse cloudstackUser : cloudstackUsers) {
                 if (username.equals(cloudstackUser.getUsername())) {
-                    logger.trace("found user {} in cloudstack user {}", username, cloudstackUser.getUsername());
+                    logger.trace("Found user {} in CloudStack", cloudstackUser.getUsername());
                     rc = true;
                     break;
                 } else {
@@ -414,7 +414,7 @@ public List<LdapUserResponse> filterPotentialImport(List<LdapUserResponse> input
             logger.trace("should be filtering potential imports!!!");
         }
         // functional possibility do not add only users not yet in cloudstack but include users that would be moved if they are so in ldap?
-        // this means if they are part of an account linked to a ldap group/ou
+        // This means if they are part of an Account linked to an LDAP Group/OU
         input.removeIf(ldapUser ->
                 (
                         (isACloudStackUser(ldapUser))

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java

@@ -91,7 +91,7 @@ public Pair<Boolean, ActionOnFailedAuthentication> authenticate(final String use
                     }
                 } else {
                     if (logger.isTraceEnabled()) {
-                        logger.trace("'this' domain ({}) is not linked to ldap follow normal authentication", domainId);
+                        logger.trace("'this' domain ({}) is not linked to LDAP follow normal authentication", domainId);
                     }
                     rc = authenticate(username, password, domainId, user);
                 }

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java

@@ -180,7 +180,7 @@ private LdapConfigurationResponse addConfigurationInternal(final String hostname
                 context = _ldapContextFactory.createBindContext(providerUrl,domainId);
                 configuration = new LdapConfigurationVO(hostname, port, domainId);
                 _ldapConfigurationDao.persist(configuration);
-                logger.info("Added new ldap server with url: {}{}", providerUrl, domainId == null ? "" : " for domain " + domainId);
+                logger.info("Added a new LDAP server with URL: {}{}", providerUrl, domainId == null ? "" : " for domain " + domainId);
                 return createLdapConfigurationResponse(configuration);
             } catch (NamingException | IOException e) {
                 logger.debug("NamingException while doing an LDAP bind", e);
@@ -215,7 +215,7 @@ public boolean canAuthenticate(final String principal, final String password, fi
             return true;
         } catch (NamingException | IOException e) {/* AuthenticationException is caught as NamingException */
             logger.debug("Exception while doing an LDAP bind for user  {}", principal, e);
-            logger.info("Failed to authenticate user: {}. incorrect password.", principal);
+            logger.info("Failed to authenticate user: {}. Incorrect password.", principal);
             return false;
         }
     }

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDao.java

@@ -29,8 +29,8 @@
 public interface LdapConfigurationDao extends GenericDao<LdapConfigurationVO, Long> {
     /**
      * @deprecated there might well be more then one ldap implementation on a host and or a double binding of several domains
-     * @param hostname the hostname or ip address of the ldap server
-     * @return the ldap configuration for the given hostname
+     * @param hostname the hostname or ip address of the LDAP server
+     * @return the LDAP configuration for the given hostname
      */
     @Deprecated
     LdapConfigurationVO findByHostname(String hostname);