CODEWITHSUNDEEP
spring-webfluxproject-reactor
Alex Faster
Alex Faster

Reputation: 209

How can I make code more reactive; Remove if's, check for empty and do intermediate logging

I am considering, how may I rewrite this code in more reactive way (without if's, throw Exceptions in intermediate steps etc., and best practice for logging intermediate results)

        return identityRepository.findByDeviceIdAndToken(
                deviceId,
                authToken
        ).doOnSuccess(identity -> {
            if (identity == null) {
                log.info(
                        "Pair Auth-Token: {} and Device-ID: {} not found",
                        authToken,
                        deviceId
                );
            }
        })
                .map(MyPrincipal::new)
                .map(
                        principal -> {
                            if (!principal.isCredentialsNonExpired()) {
                                throw new CredentialsExpiredException();
                            }
                            return 
                                    new UsernamePasswordAuthenticationToken(
                                            principal,
                                            null,
                                            Collections.emptyList()
                                    )
                            ;
                        }
                )
                .flatMap(this.authenticationManager::authenticate)
                .map(SecurityContextImpl::new);

Upvotes: 1

Views: 205

Answers (2)

Ajit Soman
Ajit Soman

Reputation: 4074

You can use Optional to eliminate if condition from your code. There are total 2 if condition in your code. Lets first remove if from doOnSuccess method

.doOnSuccess(identity -> {
 if (identity == null) {
  log.info(
   "Pair Auth-Token: {} and Device-ID: {} not found",
   authToken,
   deviceId
  );
 }
})

You can use ifPresentOrElse to remove if condition. It was introduced in java 9:

.doOnSuccess(identity -> Optional.ofNullable(identity)
 .ifPresentOrElse(
  val -> {},
  () -> log.info(
   "Pair Auth-Token: {} and Device-ID: {} not found",
   authToken,
   deviceId)
 )
)

The second if condition is in map method

.map(principal -> {
    if (!principal.isCredentialsNonExpired()) {
        throw new CredentialsExpiredException();
    }
    return new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList());
})

In above code, You are throwing some exception based on a condition. You can use filter along with orElseThrow to throw exception if Optional has become empty due to filter:

.map(principal -> Optional.of(new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()))
        .filter(token -> token.getPrincipal().isCredentialsNonExpired())
        .orElseThrow(CredentialsExpiredException::new))

Upvotes: 1

mslowiak
mslowiak

Reputation: 1838

You can change map logic in this way:

return identityRepository.findByDeviceIdAndToken(
                deviceId,
                authToken
        ).doOnSuccess(identity -> {
            if (identity == null) {
                log.info(
                        "Pair Auth-Token: {} and Device-ID: {} not found",
                        authToken,
                        deviceId
                );
            }
        })
                .map(MyPrincipal::new)
                .filter(principal -> principal.isCredentialsNonExpired())
                .switchIfEmpty(Mono.error(new CredentialsExpiredException()))
                .map(x -> new UsernamePasswordAuthenticationToken(
                                principal,
                                null,
                                Collections.emptyList()
                        )
                )
                .flatMap(this.authenticationManager::authenticate)
                .map(SecurityContextImpl::new);

Upvotes: 0

Related Questions