Given the following Function
implementations...
Get User Credentials From Master Database
private Function<Long, Optional<UserCredentials>>
getUserCredentialsFromMaster() {
return userId -> Optional.ofNullable(userId)
.flatMap(masterUserRepository::findById)
.map(User::getCredentials);
}
Get User Credentials From Secondary Database
private Function<Long, Optional<UserCredentials>>
getUserCredentialsFromSecondary() {
return userId -> Optional.ofNullable(userId)
.flatMap(secondaryUserRepository::findById)
.map(User::getCredentials);
}
I need to execute either getUserCredentialsFromMaster
or getUserCredentialsFromSecondary
depending on where userId
comes from.
Here below is my attempt:
Consider domain class UserProfile
public class UserProfile {
Long id;
Long internalUserId; // if internalUserId is null then externalUserId is not
Long externalUserId; // and vice-versa
}
Attempt to obtain UserCredentials
:
final UserProfile userProfile = userProfileRepository.findById(userProvileId);
final UserCredentials userCredentials =
Optional.ofNullable(userProfile.internalUserId)
.flatMap(getUserCredentialsFromMaster())
.orElse(
Optional.ofNullable(userProfile.externalUserId)
.flatMap(getUserCredentialsFromSecondary())
.orElseThrow(UserCredentialsNotFound::new));
internalUserId
is not null
but the statements above always throw UserCredentialsNotFound
. I've tried to rewrite getUserCredentialsFromMaster
and getUserCredentialsFromSecondary
as plain Java methods invoked from an if-then-else block, and it worked as expected.
Am I missing something?
You're getting an exception because the argument of the Optional.orElse()
is evaluated eagerly. I.e. it would be evaluated even if the Optional
on which it was invoked is not empty.
But as you have said, either "internalUserId
is null
then externalUserId
is not null
and vice-versa" and orElseThrow()
produces an exception.
Firstly, note that Optional
wasn't designed to perform null-checks. The design goal of the Optional
is to serve as a return type, and its method ofNullable()
is supposed to wrap a nullable return value, not to substitute a null-check.
You might be interested in reading:
And there's nothing wrong with implicit null-checks, it's a bit of a problem when there are a lot of them in the code, but it's rather an immediate consequence of how particular behavior in the application was implemented, then the issue related to the tools offered by the language.
The cleaner way to address this problem would be to get read of the functions and define a method producing a Supplier
based on the provided id
and UserRepository
:
private Supplier<Optional<UserCredentials>> getUserCredentials(Long id, UserRepository repository) {
return id == null ?
Optional::empty :
() -> repository.findById(id).map(User::getCredentials);
}
Which can be used in the following way:
UserCredentials userCredentials = Stream.of(
getUserCredentials(userProfile.internalUserId, masterUserRepository),
getUserCredentials(userProfile.internalUserId, secondaryUserRepository)
)
.map(Supplier::get)
.flatMap(Optional::stream)
.findFirst()
.orElseThrow(UserCredentialsNotFound::new);
As a remedy you can use Java 9 Optional.or()
which expects a Supplier
of Optional
which would be evaluated only if needed.
UserCredentials userCredentials = Optional.ofNullable(userProfile.internalUserId)
.flatMap(getUserCredentialsFromMaster())
.or(() ->
Optional.ofNullable(userProfile.externalUserId)
.flatMap(getUserCredentialsFromSecondary())
)
.orElseThrow(UserCredentialsNotFound::new);
Alternatively, you can make use Optional.orElseGet()
which takes a Supplier
of resulting value:
UserCredentials userCredentials = Optional.ofNullable(userProfile.internalUserId)
.flatMap(getUserCredentialsFromMaster())
.orElseGet(() ->
Optional.ofNullable(userProfile.externalUserId)
.flatMap(getUserCredentialsFromSecondary())
.orElseThrow(UserCredentialsNotFound::new)
);