4

Inside a method, a condition is needed to proceed the logic. An unhandled exception warning message shows up in my IDE. Wrapping the whole block with try-catch doesn't let the message go away.

public void changePassword(String login, String currentClearTextPassword, String newPassword) {

    userRepository.findOneByLogin(login)
            .ifPresent(user -> {
                String currentEncryptedPassword = user.getUserSecret();
                String encryptedInputPassword = "";
                try {
                    encryptedInputPassword = authUtils.encrypt(currentClearTextPassword);
                } catch (Exception ex) {
                    System.err.println("Encryption exception: " + ex.getMessage());
                }
                if (!Objects.equals(encryptedInputPassword, currentEncryptedPassword)) {
                    throw new Exception("Invalid Password");  // <-- unhandled exception
                }
                String encryptedNewPassword = "";
                try {
                    encryptedNewPassword = authUtils.encrypt(newPassword);
                } catch (Exception ex) {
                    System.err.println("Encryption exception: " + ex.getMessage());
                }
                user.setUserSecret(encryptedNewPassword);
                userRepository.save(user);
                log.debug("Changed password for User: {}", user);
            });
}

How to deal with this warning message?

Stuart Marks
  • 127,867
  • 37
  • 205
  • 259
vic
  • 2,548
  • 9
  • 44
  • 74

3 Answers3

3

Treating Exception inside stream operation is a little overhead, I would like to separate the operation and makes it like so :

public void changePassword(String login, String currentClearTextPassword, String newPassword) throws Exception {
    //get the user in Optional
    Optional<User> check = userRepository.findOneByLogin(login);

    //if the user is present 'isPresent()'
    if(check.isPresent()){

        //get the user from the Optional and do your actions
        User user = check.get();

        String currentEncryptedPassword = user.getUserSecret();
        String encryptedInputPassword = "";
        try {
            encryptedInputPassword = authUtils.encrypt(currentClearTextPassword);
        } catch (Exception ex) {
            throw new Exception("Encryption exception: " + ex.getMessage());
        }
        if (!Objects.equals(encryptedInputPassword, currentEncryptedPassword)) {
            throw new Exception("Invalid Password");  // <-- unhandled exception
        }
        String encryptedNewPassword = "";
        try {
            encryptedNewPassword = authUtils.encrypt(newPassword);
        } catch (Exception ex) {
            throw new Exception("Encryption exception: " + ex.getMessage());
        }
        user.setUserSecret(encryptedNewPassword);
        userRepository.save(user);
        log.debug("Changed password for User: {}", user);
    }
}

Beside the Exception should be thrown not just printed.

Youcef LAIDANI
  • 55,661
  • 15
  • 90
  • 140
1

Like @louis-wasserman said you can use an unchecked exception.

So instead of

throw new Exception("Invalid Password");

use:

throw new RuntimeException("Invalid Password"); // for example

Note: It is better to use a custom RuntimeException that fits your use case.

Parisana Ng
  • 487
  • 4
  • 14
0

For the general case, @pari-ngang is correct: within a lambda expression you cannot handle a checked exception outside the calling function. Either use an unchecked or use the try construct in the lambda.

However, needing to do either of these might be a sign of poor structure. Having a large block of code within a lambda is one such sign. Here are a few suggestions for your specific case:

1. As a simplification of @ycf-l's answer, invert the logic and simply return user if not found:

Optional<User> user = userRepository.findOneByLogin(login);
if (!user.isPresent()) {
    return;
}
// ... carry on as before outside of a lambda

2. Seperate the logic of fetching the user from changing the password of a user (you'll still have to pass exceptions back up):

if (user.isPresent()) {
    changePassword(user.get(), currentClearTextPassword, newPassword);
}

3. You can also use libraries like vavr to simplify the try/catch blocks:

String encryptedInputPassword = Try.of(() -> authUtils.encrypt(currentClearTextPassword))
    .onFailure(e -> System.err.println("Encryption exception: " + e.getMessage()))
    .orElse("");

4. If you're willing to pursue vavr, you can also solve the more general problem:

userRepository.findOneByLogin(login)
    .map(user -> Try.of(() -> {
        // Your change password stuff goes here.
        changePassword(user, currentClearTextPassword, newPassword);
        return null; // I don't currently know of any way of passing a runnable.
  }))
  // This pulls the exception out of the lambda function, satisfying the compiler.
  .orElseGet(() -> Try.success(null))
  .getOrElseThrow(Function.identity());

I haven't used vavr much so can't tell you if this is the most efficient/cleanest approach/use of vavr, but it will solve your problem.

Druckles
  • 3,161
  • 2
  • 41
  • 65