2
\$\begingroup\$

I want to implement a utility class which manages tokens generated by any API. The idea is simple:

It should store the current token value. The token should be initialized just before first use and then re-initialized after some time has elapsed. If the API returns an error related to the token, it should be regenerated and job should be repeated.

Here we have the utility class:

import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

public abstract class TokenManager {

    private volatile String token;
    private ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor();
    private ScheduledFuture<?> schedule;

    public void useToken(TokenConsumer tokenConsumer) {
        useToken(tokenConsumer, true);
    }

    private void useToken(TokenConsumer tokenConsumer, boolean retryWhenTokenExpired) {
        try {
            tokenConsumer.useToken(getToken());
        } catch (Exception e) {
            if (retryWhenTokenExpired && tokenExpired(e.getMessage())) {
                System.out.println("token expired, retry");
                resetToken();
                useToken(tokenConsumer, false);
            }
        }
    }

    synchronized private String getToken() {
        if (token == null) {
            resetToken();
        }
        return token;
    }

    synchronized private void resetToken() {
        if (schedule != null) {
            schedule.cancel(false);
        }
        System.out.println("reset token");
        token = generateToken();
        int oneMinute = 60 * 1000;
        long delay = getTokenValidityTimeMillis() - oneMinute;
        schedule = scheduler.schedule(this::resetToken, delay > 0 ? delay : getTokenValidityTimeMillis(), TimeUnit.MILLISECONDS);

        System.out.println("reset token finished");
    }

    protected abstract String generateToken();

    protected abstract boolean tokenExpired(String message);

    protected abstract long getTokenValidityTimeMillis();

    @FunctionalInterface
    public interface TokenConsumer {
        void useToken(String token) throws Exception;
    }
}

And here we have its simple implementation with use example:

import java.util.Random;

public class TokenManagerImpl extends TokenManager {
    @Override
    protected String generateToken() {
        //here would go some api call which retrieves token
        try {
            Thread.sleep(2000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        return "token " + new Random().nextInt();
    }

    @Override
    protected boolean tokenExpired(String message) {
        return "Token expired".equals(message);
    }

    @Override
    protected long getTokenValidityTimeMillis() {
        return 1000;
    }

    public static void main(String[] args) {
        TokenManager tokenManager = new TokenManagerImpl();
        TokenConsumer tokenConsumer = token -> {
            //here would go some api call which uses token
            System.out.println("using token : " + token);
        };
        while (true) {
            tokenManager.useToken(tokenConsumer);
            try {
                Thread.sleep(50L);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

useToken

I don't really get your expected behavior from this method. You only retry when expired, what if an exception occured while not yet expired? Nothing happens, not even a log. The same with exceptions that occur when retryWhenTokenExpired is not set, the exception gets swallowed without any trace.

  catch (Exception e) {
    if (retryWhenTokenExpired && tokenExpired(e.getMessage())) {
        System.out.println("token expired, retry");
        resetToken();
        useToken(tokenConsumer, false);
    }
  }

I would make sure the following constraint is true: either a retry is performed or an exception is propagated up the stack. In addition, on expiration, reset the token when requested so. log is an abstraction of your System.out.println.

  catch (Exception e) {
    log("error using token: " + e.getMessage());
    if (!retryWhenTokenExpired) {
        throw e;
    }
    final boolean expired = tokenExpired(e.getMessage());
    if (expired) {
        log("token expired, retry");
        resetToken();
    }
    useToken(tokenConsumer, !expired);
  }
\$\endgroup\$
0
\$\begingroup\$

You helpfully offered an English specification, that spoke of initialization, use, and regeneration.

The code is attempting to maintain invariants related to that spec. It would be helpful to see tests which verify such invariants, or at least javadoc or comments that describe them.

In place of e.printStackTrace() swallowing an exception, I would prefer to see throw new RuntimeException(e) where that is acceptable. In useToken() one could perhaps catch something narrower than Exception.

I can't say I'm very fond of useToken(tokenConsumer, false). It seems more like useToken() should have a while (True) { ... } loop, or while not finished or something. It could have a goal of always refreshing the token when less than 50% of its lifetime remains, prior to attempted use, and then the catch would be more about reporting fatal unexpected error than about retrying.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.