Calling other machines from the LinkedList if one is blocked or down

This is a follow on to : Simplifying asynchronous "executeAsync" method along with "onFailure" callback.

I am using AsyncRestTemplate as my HttpClient to execute my URL and the server will return back a json string as the response. Customer will call this library by passing DataKey object which has userId in it.

• Given a userId, I will find out what are the machines that I can hit to get the data and then store those machines in a LinkedList so that I can execute them sequentially.

• After that I will check whether the first hostname is in block list or not. If it is not there in the block list, then I will make a URL with the first hostname in the list and execute it and if the response is successful then return back the response. But let's say if that first hostname is in the block list, then I will try to get the second hostname in the list and make the url and execute it so basically find the hostname which is not in block list before making the URL.

• Now, let's say if we selected first hostname which was not in the block list and executed the URL and somehow server was down or not responding, then I will execute the second hostname in the list and keep doing this until you get a successful response. But make sure they were not in the block list as well so we need to follow above point.

• If all servers are down or in block list, then I can simply log and return the error that service is unavailable.

Below is my DataClient class which will be called by customer and they will pass DataKey object to getData method.

```
public class DataClient implements Client, DataFetcher {

private final AsyncRestTemplate restTemplate = new AsyncRestTemplate(new HttpComponentsAsyncClientHttpRequestFactory());

@Override
public ListenableFuture getData(DataKey key) {
final SettableFuture responseFuture = SettableFuture.create();

// given a userId, find all the hostnames
// so it can also have four hostname or one hos

-
Name of getHostnamesInfo is misleading a little bit. It returns to listOfHostnames, so calling it getHostnames or getHostnamesForUser would be less confusing.

-
First, to DRY the code, I'd try to extract out some hostname list handling logic to a separate class:

public class Hosts {

     private final LinkedList hostsnames = newLinkedList();

     public Hosts(final List hosts) {
         checkNotNull(hosts, "hosts cannot be null");
         this.hostsnames.addAll(hosts);
     }

     public Optional getNextAvailableHostname() {
         while (!hostsnames.isEmpty()) {
             String firstHostname = hostsnames.removeFirst();
             if (!ClientUtils.isEmpty(firstHostname) && !ShardMapping.isBlockHost(firstHostname)) {
                 return Optional.of(firstHostname);
             }
         }
         return Optional.absent();
     }

     public boolean isEmpty() {
         return hostsnames.isEmpty();
     }
 }

It makes getData simpler:

@Override
    public ListenableFuture getData(final DataKey key) {
        // given a userId, find all the hostnames
        // so it can also have four hostname or one hostname or six hostname as
        // well in the list
        final LinkedList hostnames = getHostnames(key.getUserId());
        final Hosts hosts = new Hosts(hostnames);
        final Optional nextAvailableHost = hosts.getNextAvailableHostname();
        if (!nextAvailableHost.isPresent()) {
            final SettableFuture responseFuture = SettableFuture.create();
            responseFuture.set(new DataResponse(null, DataErrorEnum.SERVICE_UNAVAILABLE, DataStatusEnum.ERROR));
            return responseFuture;
        }
        checkState(nextAvailableHost.isPresent());

        final SettableFuture responseFuture = SettableFuture.create();
        executeForServers(responseFuture, key, nextAvailableHost.get(), hosts);

        return responseFuture;
    }

and improves a little bit the RetryCallback.onFailure too:

@Override
    public void onFailure(final Throwable ex) {
        if (ex instanceof SocketException) {
            // if it comes here, then it means some of the servers are down so adding it into block list
            ShardMapping.blockHost(hostname);
            
            final Optional nextAvailableHost = hosts.getNextAvailableHostname();
            if (nextAvailableHost.isPresent()) {
                dataFetcher.executeForServers(responseFuture, key, nextAvailableHost.get(), hosts);
                return;
            }

            // either  all the servers are down or all the servers were in block list
            if (hosts.isEmpty()) {
                responseFuture.set(new DataResponse(null, DataErrorEnum.SERVICE_UNAVAILABLE, DataStatusEnum.ERROR));
            }
        } else { // this is for 4xx (HttpClientErrorException) and 5xx (HttpServerErrorException) error coming from server side
            HttpStatusCodeException httpException = (HttpStatusCodeException) ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(), error,
                    DataStatusEnum.ERROR));
        }
        // is there any other error I should look into?
    }

-
You might notice some other duplication here, especially if you invert the condition in getData:

final SettableFuture responseFuture = SettableFuture.create();

 final Optional nextAvailableHost = hosts.getNextAvailableHostname();
 if (nextAvailableHost.isPresent()) {
     executeForServers(responseFuture, key, nextAvailableHost.get(), hosts);
 } else {
     responseFuture.set(new DataResponse(null, DataErrorEnum.SERVICE_UNAVAILABLE, DataStatusEnum.ERROR));
 }

 return responseFuture;

I have created a temporary execute method for it in the Hosts class:

public void execute(final SettableFuture responseFuture, final DataKey key, final DataFetcher dataFetcher) {
        final Optional nextAvailableHost = getNextAvailableHostname();
        if (nextAvailableHost.isPresent()) {
            dataFetcher.executeForServers(responseFuture, key, nextAvailableHost.get(), this);
            return;
        }

        responseFuture.set(new DataResponse(null, DataErrorEnum.SERVICE_UNAVAILABLE, DataStatusEnum.ERROR));
    }

It probably has a bad name and probably it should not be in the Hosts class but it's enough in this internal step.

Now getData looks like this:

@Override
    public ListenableFuture getData(final DataKey key) {
        final LinkedList hostnames = getHostnames(key.getUserId());
        final Hosts hosts = new Hosts(hostnames);
        final SettableFuture responseFuture = SettableFuture.create();
        hosts.execute(responseFuture, key, this);
        return responseFuture;
    }

And onFailure:

```
@Override
public void onFailure(final Throwable ex) {
if (ex instanceof SocketException) {

StackExchange Code Review Q#86478, answer score: 7