I am using RestTemplate as my HttpClient to execute a URL while the server will return a JSON string as the response. The customer will call this library by passing DataKey object which has userId in it. Earlier I was using AsyncRestTemplate which is part of Spring 4 but in my company they are not supporting Spring 4 in their parent pom so going back to Spring 3 for now.
- Using the given
userId, I will find out what are the machines that I can hit to get the data and then store those machines in aLinkedList, 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 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, first 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.
I am making a library in which I will have synchronous (getSyncData) and asynchronous (getAsyncData) methods in it.
getSyncData()- waits until I have a result, returns the result.getAsyncData()- returns aFutureimmediately which can be processed after other things are done, if needed.
Below is my DataClient class which will be called by customer and they will pass DataKey object to either getSyncData or getAsyncData method depending on what they want to call. In general some customer will call getSyncData method and some customer might call getAsyncData method.
public class DataClient implements Client {
private RestTemplate restTemplate = new RestTemplate();
private ExecutorService service = Executors.newFixedThreadPool(15);
@Override
public DataResponse getSyncData(DataKey key) {
DataResponse response = null;
Future<DataResponse> responseFuture = null;
try {
responseFuture = getAsyncData(key);
response = responseFuture.get(key.getTimeout(), key.getUnitOfTime());
} catch (TimeoutException ex) {
response = new DataResponse(DataErrorEnum.CLIENT_TIMEOUT, DataStatusEnum.ERROR);
responseFuture.cancel(true); // terminating the tasks that have got timed out
} catch (Exception ex) {
response = new DataResponse(DataErrorEnum.ERROR_CLIENT, DataStatusEnum.ERROR);
}
return response;
}
@Override
public Future<DataResponse> getAsyncData(DataKey key) {
DataFetcherTask task = new DataFetcherTask(key, restTemplate);
Future<DataResponse> future = service.submit(task);
return future;
}
}
DataFetcherTask class:
public class DataFetcherTask implements Callable<DataResponse> {
private DataKey key;
private RestTemplate restTemplate;
public DataFetcherTask(DataKey key, RestTemplate restTemplate) {
this.key = checkNotNull(key);
this.restTemplate = checkNotNull(restTemplate);
}
// can we simplify this?
// I tried thinking a lot but not sure how to split this up which follows SRP
@Override
public DataResponse call() {
DataResponse dataResponse = null;
ResponseEntity<String> response = null;
MappingsHolder mappings = ShardMapping.getMappings(key.getTypeOfFlow());
// given a userId, find all the hostnames
// it can also have four hostname or one hostname or six hostname as well in the list
LinkedList<String> hostnames = mappings.getListOfHostnames(key.getUserId());
for (String hostname : hostnames) {
// If host name is null or host name is in local block list, skip sending request to this host
if (StringUtils.isEmpty(hostname) || ShardMapping.isBlockHost(hostname)) {
continue;
}
try {
String url = generateURL(hostname);
response = restTemplate.exchange(url, HttpMethod.GET, key.getEntity(), String.class);
// if the status code is NO_CONTENT, then send that as well
// otherwise send OK
if (response.getStatusCode() == HttpStatus.NO_CONTENT) {
dataResponse = new DataResponse(response.getBody(), DataErrorEnum.NO_CONTENT,
DataStatusEnum.SUCCESS);
} else {
dataResponse = new DataResponse(response.getBody(), DataErrorEnum.OK,
DataStatusEnum.SUCCESS);
}
break;
// below codes are duplicated looks like
} catch (HttpClientErrorException ex) {
HttpStatusCodeException httpException = (HttpStatusCodeException) ex;
DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
String errorMessage = httpException.getResponseBodyAsString();
dataResponse = new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
return dataResponse;
} catch (HttpServerErrorException ex) {
HttpStatusCodeException httpException = (HttpStatusCodeException) ex;
DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
String errorMessage = httpException.getResponseBodyAsString();
dataResponse = new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
return dataResponse;
} catch (RestClientException ex) {
// if it comes here, then it means some of the servers are down so adding it into block list
ShardMapping.blockHost(hostname);
}
}
// if hostnames are empty, then sending different ERROR ENUM code.
if (CollectionUtils.isEmpty(hostnames)) {
dataResponse = new DataResponse(null, DataErrorEnum.PERT_ERROR, DataStatusEnum.ERROR);
} else if (response == null) { // either all the servers are down or all the servers were in block list
dataResponse = new DataResponse(null, DataErrorEnum.SERVICE_UNAVAILABLE, DataStatusEnum.ERROR);
}
return dataResponse;
}
}
My block list keeps getting updated from another background thread every 1 minute. If any server is down and not responding, then I need to block that server by using this -
ShardMapping.blockHost(hostname);
And to check whether any server is in block list or not, I use this -
ShardMapping.isBlockHost(hostname);
I am returning SERVICE_UNAVAILABLE if servers are down or in block list, on the basis of response == null check, not sure whether it's a right approach or not.
I want to know if I am following the Single Responsibility Principle properly here or not and if the above code can be simplified?
