Improve error logging for failed user creation (FINERACT-1070)

fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java

@@ -20,7 +20,6 @@ package org.apache.fineract.useradministration.service;
 
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -107,11 +106,10 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
         this.topicDomainService = topicDomainService;
     }
 
-    @Transactional
     @Override
+    @Transactional
     @Caching(evict = { @CacheEvict(value = "users", allEntries = true), @CacheEvict(value = "usersByUsername", allEntries = true) })
     public CommandProcessingResult createUser(final JsonCommand command) {
-
         try {
             this.context.authenticatedUser();
 
@@ -125,17 +123,17 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
             final String[] roles = command.arrayValueOfParameterNamed("roles");
             final Set<Role> allRoles = assembleSetOfRoles(roles);
 
-            AppUser appUser;
-
             final String staffIdParamName = "staffId";
             final Long staffId = command.longValueOfParameterNamed(staffIdParamName);
 
-            Staff linkedStaff = null;
+            Staff linkedStaff;
             if (staffId != null) {
                 linkedStaff = this.staffRepositoryWrapper.findByOfficeWithNotFoundDetection(staffId, userOffice.getId());
+            } else {
+                linkedStaff = null;
             }
 
-            Collection<Client> clients = null;
+            Collection<Client> clients;
             if (command.hasParameter(AppUserConstants.IS_SELF_SERVICE_USER)
                     && command.booleanPrimitiveValueOfParameterNamed(AppUserConstants.IS_SELF_SERVICE_USER)
                     && command.hasParameter(AppUserConstants.CLIENTS)) {
@@ -145,9 +143,11 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
                     clientIds.add(clientElement.getAsLong());
                 }
                 clients = this.clientRepositoryWrapper.findAll(clientIds);
+            } else {
+                clients = null;
             }
 
-            appUser = AppUser.fromJson(userOffice, linkedStaff, allRoles, clients, command);
+            AppUser appUser = AppUser.fromJson(userOffice, linkedStaff, allRoles, clients, command);
 
             final Boolean sendPasswordToEmail = command.booleanObjectValueOfParameterNamed("sendPasswordToEmail");
             this.userDomainService.create(appUser, sendPasswordToEmail);
@@ -160,34 +160,29 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
                     .withOfficeId(userOffice.getId()) //
                     .build();
         } catch (final DataIntegrityViolationException dve) {
-            handleDataIntegrityIssues(command, dve.getMostSpecificCause(), dve);
+            throw handleDataIntegrityIssues(command, dve.getMostSpecificCause(), dve);
-            return CommandProcessingResult.empty();
         } catch (final JpaSystemException | PersistenceException | AuthenticationServiceException dve) {
+            LOG.error("createUser: JpaSystemException | PersistenceException | AuthenticationServiceException", dve);
             Throwable throwable = ExceptionUtils.getRootCause(dve.getCause());
-            handleDataIntegrityIssues(command, throwable, dve);
+            throw handleDataIntegrityIssues(command, throwable, dve);
-            return new CommandProcessingResultBuilder() //
-                    .withCommandId(command.commandId()) //
-                    .build();
         } catch (final PlatformEmailSendException e) {
-            final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
+            LOG.error("createUser: PlatformEmailSendException", e);
 
             final String email = command.stringValueOfParameterNamed("email");
             final ApiParameterError error = ApiParameterError.parameterError("error.msg.user.email.invalid",
-                    "The parameter email is invalid.", "email", email);
+                    "Sending email failed; is parameter email is invalid? More details available in server log: " + e.getMessage(), "email",
-            dataValidationErrors.add(error);
+                    email);
 
             throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist", "Validation errors exist.",
-                    dataValidationErrors, e);
+                    List.of(error), e);
         }
     }
 
-    @Transactional
     @Override
+    @Transactional
     @Caching(evict = { @CacheEvict(value = "users", allEntries = true), @CacheEvict(value = "usersByUsername", allEntries = true) })
     public CommandProcessingResult updateUser(final Long userId, final JsonCommand command) {
-
         try {
-
             this.context.authenticatedUser(new CommandWrapperBuilder().updateUser(null).build());
 
             this.fromApiJsonDeserializer.validateForUpdate(command.json());
@@ -251,57 +246,40 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
                     .with(changes) //
                     .build();
         } catch (final DataIntegrityViolationException dve) {
-            handleDataIntegrityIssues(command, dve.getMostSpecificCause(), dve);
+            throw handleDataIntegrityIssues(command, dve.getMostSpecificCause(), dve);
-            return CommandProcessingResult.empty();
         } catch (final JpaSystemException | PersistenceException | AuthenticationServiceException dve) {
+            LOG.error("updateUser: JpaSystemException | PersistenceException | AuthenticationServiceException", dve);
             Throwable throwable = ExceptionUtils.getRootCause(dve.getCause());
-            handleDataIntegrityIssues(command, throwable, dve);
+            throw handleDataIntegrityIssues(command, throwable, dve);
-            return new CommandProcessingResultBuilder() //
-                    .withCommandId(command.commandId()) //
-                    .build();
         }
     }
 
     /**
-     * encode the new submitted password retrieve the last n used password check if the current submitted password,
+     * Encode the new submitted password and retrieve the last N used passwords to check if the current submitted
-     * match with one of them
+     * password matches with one of them.
-     *
-     * @param user
-     * @param command
-     * @return
      */
     private AppUserPreviousPassword getCurrentPasswordToSaveAsPreview(final AppUser user, final JsonCommand command) {
-
         final String passWordEncodedValue = user.getEncodedPassword(command, this.platformPasswordEncoder);
 
         AppUserPreviousPassword currentPasswordToSaveAsPreview = null;
 
         if (passWordEncodedValue != null) {
-
             PageRequest pageRequest = PageRequest.of(0, AppUserApiConstant.numberOfPreviousPasswords, Sort.Direction.DESC, "removalDate");
-
             final List<AppUserPreviousPassword> nLastUsedPasswords = this.appUserPreviewPasswordRepository.findByUserId(user.getId(),
                     pageRequest);
-
             for (AppUserPreviousPassword aPreviewPassword : nLastUsedPasswords) {
-
                 if (aPreviewPassword.getPassword().equals(passWordEncodedValue)) {
-
                     throw new PasswordPreviouslyUsedException();
-
                 }
             }
 
             currentPasswordToSaveAsPreview = new AppUserPreviousPassword(user);
-
         }
 
         return currentPasswordToSaveAsPreview;
-
     }
 
     private Set<Role> assembleSetOfRoles(final String[] rolesArray) {
-
         final Set<Role> allRoles = new HashSet<>();
 
         if (!ObjectUtils.isEmpty(rolesArray)) {
@@ -315,11 +293,10 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
         return allRoles;
     }
 
-    @Transactional
     @Override
+    @Transactional
     @Caching(evict = { @CacheEvict(value = "users", allEntries = true), @CacheEvict(value = "usersByUsername", allEntries = true) })
     public CommandProcessingResult deleteUser(final Long userId) {
-
         final AppUser user = this.appUserRepository.findById(userId).orElseThrow(() -> new UserNotFoundException(userId));
         if (user.isDeleted()) {
             throw new UserNotFoundException(userId);
@@ -333,23 +310,24 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit
     }
 
     /*
-     * Guaranteed to throw an exception no matter what the data integrity issue is.
+     * Return an exception to throw, no matter what the data integrity issue is.
      */
-    private void handleDataIntegrityIssues(final JsonCommand command, final Throwable realCause, final Exception dve) {
+    private PlatformDataIntegrityException handleDataIntegrityIssues(final JsonCommand command, final Throwable realCause,
+            final Exception dve) {
         if (realCause.getMessage().contains("'username_org'")) {
             final String username = command.stringValueOfParameterNamed("username");
             final StringBuilder defaultMessageBuilder = new StringBuilder("User with username ").append(username)
                     .append(" already exists.");
-            throw new PlatformDataIntegrityException("error.msg.user.duplicate.username", defaultMessageBuilder.toString(), "username",
+            return new PlatformDataIntegrityException("error.msg.user.duplicate.username", defaultMessageBuilder.toString(), "username",
                     username);
         }
 
         if (realCause.getMessage().contains("'unique_self_client'")) {
-            throw new PlatformDataIntegrityException("error.msg.user.self.service.user.already.exist",
+            return new PlatformDataIntegrityException("error.msg.user.self.service.user.already.exist",
                     "Self Service User Id is already created. Go to Admin->Users to edit or delete the self-service user.");
         }
 
-        LOG.error("Error occured.", dve);
+        LOG.error("handleDataIntegrityIssues: Neither duplicate username nor existing user; unknown error occured", dve);
-        throw new PlatformDataIntegrityException("error.msg.unknown.data.integrity.issue", "Unknown data integrity issue with resource.");
+        return new PlatformDataIntegrityException("error.msg.unknown.data.integrity.issue", "Unknown data integrity issue with resource.");
     }
 }

integration-tests/src/test/java/org/apache/fineract/integrationtests/client/ReportsTest.java

@@ -25,7 +25,7 @@ import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 /**
- * Integration Test for /staff API.
+ * Integration Test for /runreports/ API.
  *
  * @author Michael Vorburger.ch
  */