Should I prefer the `ifPresent` method to the `isPresent` one?

Question

I have this code using the latter:

Optional<String> subject = Optional.ofNullable(claims.get().getSubject());
if (subject.isPresent()) {
  UserDetails userDetails = userDetailsService.loadUserByUsername(subject.get());
  UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken(userDetails,
      null, userDetails.getAuthorities());
  authentication.setDetails(new WebAuthenticationDetailsSource().buildDetails(request));
  logger.debug("Security - The request authenticated fine from the JWT Access token");
  return authentication;
} else {
  throw new BadCredentialsException("The authentication token " + optToken + " did not contain a subject.");
}

I'm trying to refactor this using the ifPresent method.

Should I have the userDetailsService.loadUserByUsername service call before the functional method call ? If so, how to ? How to return an object of type different than the functional method type ?

I'm on Java 12.

https://stackoverflow.com/questions/41485751/java-8-optional-ifpresent-return-object-orelsethrow-exception I think your question is already answered here. — piyush tyagi, May 04 '19 at 15:38

marstran · Accepted Answer · 2019-05-04T15:36:08.667

Use the map method to transform the value of the Optional. After transforming it, you can use the orElseThrow method to unpack the Optional, throwing an exception if it was empty.

Something like this:

return Optional.ofNullable(claims.get().getSubject())
               .map(userDetailsService::loadUserByUsername)
               .map(userDetails -> {
                   UsernamePasswordAuthenticationToken authentication = 
                       new UsernamePasswordAuthenticationToken(
                           userDetails, null, userDetails.getAuthorities());
                   authentication.setDetails(
                       new WebAuthenticationDetailsSource().buildDetails(request));
                   return authentication;
               })
               .orElseThrow(() -> new BadCredentialsException(
                  "The authentication token " + optToken + " did not contain a subject."));

However, in your specific case, it might be simpler to not use Optional at all. You could just check for null immediately.

String subject = claims.get().getSubject();
if (subject == null) {
    throw new BadCredentialsException(
        "The authentication token " + optToken + " did not contain a subject.");
}

UsernamePasswordAuthenticationToken authentication = ... ;

score 2 · Answer 2 · answered May 04 '19 at 15:24

In this case you could use orElseThrow, which throws an exception if the value is not present:

String subjectValue = subject.orElseThrow(() ->
    new BadCredentialsException("The authentication token " + optToken + " did not contain a subject."));
...

score 0 · Answer 3 · answered May 04 '19 at 15:31

If you really want to use ifPresent you could do something like

subject.ifPresent(s -> {
     UserDetails userDetails = loadUserByUsername(s);
     ...
});

but since you are throwing on a missing subject why not simply do

String subject = Optional.ofNullable(claims.get().getSubject())
        .orElseThrow(() -> new BadCredentialsException(...));
UserDetails userDetails = loadUserByUsername(subject);
...

Should I prefer the `ifPresent` method to the `isPresent` one?

3 Answers3