2
\$\begingroup\$

I am working on a library in which I need to execute my URL using AsyncRestTemplate and after that I will get a json response back if it is successful. I am also making DataResponse object whether the response is successful or not which I will return back to the caller of my library.

If the response is not successful then I need to log the error and make the DataResponse object appropriately depending on what type of error it is. I already have an enum DataErrorEnum which contains most of the status code (HttpClientErrorException and HttpServerErrorException and some predefined) as shown below -

public enum DataErrorEnum {

    OK(200, "NONE"),
    BAD_REQUEST(400, "Server Bad Request"),
    UNAUTHORIZED(401, " Server Unauthorized"),
    SERVER_TIMEOUT(408, "Server Timeout"),
    INTERNAL_SERVER_ERROR(500, "Internal Server Error"),
    SERVERS_DOWN(6000, "Servers Down"),
    ERROR_SERVICE(6002, "Random Error Occurred on Server");

    // rest of the code
}

So I will be logging like this depending on what the error is since it will help to categorize how many were BAD_REQUEST or UNAUTHORIZED or any other status codes -

DataLogging.logErrors(ex, DataErrorEnum.BAD_REQUEST, keys); // if bad request
DataLogging.logErrors(ex, DataErrorEnum.UNAUTHORIZED, keys); // if unauthorized
DataLogging.logErrors(ex, DataErrorEnum.INTERNAL_SERVER_ERROR, keys); // if server error
DataLogging.logErrors(ex, DataErrorEnum.ERROR_SERVICE, keys); // if some random error occurred on server

And similarly I will return the DataResponse object as well for the error case like this -

new DataResponse(result.getBody(), DataErrorEnum.BAD_REQUEST, DataStatusEnum.ERROR)
new DataResponse(result.getBody(), DataErrorEnum.UNAUTHORIZED, DataStatusEnum.ERROR)
new DataResponse(result.getBody(), DataErrorEnum.INTERNAL_SERVER_ERROR, DataStatusEnum.ERROR)
new DataResponse(result.getBody(), DataErrorEnum.ERROR_SERVICE, DataStatusEnum.ERROR)

Below is the code I have so far which works fine but it is looking more messy in onFailure method. Is there any way to make this more generic in onFailure method? If I need to add couple more status code check, then I might need to add another if else block by which the code will grow very long and will not look good at all.

private AsyncRestTemplate restTemplate = new AsyncRestTemplate();

@Override
public Future<DataResponse> executeAsync(final DataKey keys) {
    final SettableFuture<DataResponse> responseFuture = SettableFuture.create();
    // for now I only have one machine in the list
    List<String> hostnames = Arrays.asList("machineA");
    executeForServers(responseFuture, keys, hostnames.get(0));
    return responseFuture;
}

private void executeForServers(final SettableFuture<DataResponse> responseFuture, final DataKey keys,
        final String hostName) {

    restTemplate.exchange(generateURL(hostName, keys), HttpMethod.GET, keys.getEntity(), String.class).addCallback(
            new ListenableFutureCallback<ResponseEntity<String>>() {
                @Override
                public void onSuccess(ResponseEntity<String> result) {
                    responseFuture.set(new DataResponse(result.getBody(), DataErrorEnum.OK,
                            DataStatusEnum.SUCCESS));
                }

                // can we simplify this by making more generic?
                @Override
                public void onFailure(Throwable ex) {
                    if (ex instanceof HttpClientErrorException) {
                        HttpClientErrorException httpException = (HttpClientErrorException) ex;
                        HttpStatus statusCode = httpException.getStatusCode();
                        if (statusCode == HttpStatus.BAD_REQUEST) {
                            DataLogging.logErrors(ex, DataErrorEnum.BAD_REQUEST, keys); // if bad request
                            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                                    DataErrorEnum.BAD_REQUEST, DataStatusEnum.ERROR));
                        } else if (statusCode == HttpStatus.UNAUTHORIZED) {
                            DataLogging.logErrors(ex, DataErrorEnum.UNAUTHORIZED, keys); // if unauthorized
                            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                                    DataErrorEnum.UNAUTHORIZED, DataStatusEnum.ERROR));
                        } else if (statusCode == HttpStatus.REQUEST_TIMEOUT) {
                            DataLogging.logErrors(ex, DataErrorEnum.SERVER_TIMEOUT, keys); // if server timeout
                            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                                    DataErrorEnum.SERVER_TIMEOUT, DataStatusEnum.ERROR));
                        } else {
                            DataLogging.logErrors(ex, DataErrorEnum.ERROR_SERVICE, keys); // some random error on server
                            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                                    DataErrorEnum.ERROR_SERVICE, DataStatusEnum.ERROR));
                        }
                    } else if (ex instanceof HttpServerErrorException) {
                        HttpServerErrorException httpException = (HttpServerErrorException) ex;
                        HttpStatus statusCode = httpException.getStatusCode();

                        if (statusCode == HttpStatus.INTERNAL_SERVER_ERROR) {
                            DataLogging.logErrors(ex, DataErrorEnum.SERVER_TIMEOUT, keys); // if server error
                            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                                    DataErrorEnum.SERVER_TIMEOUT, DataStatusEnum.ERROR));
                        } else {
                            DataLogging.logErrors(ex, DataErrorEnum.ERROR_SERVICE, keys); // some random error on server                            
                            responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                                    DataErrorEnum.ERROR_SERVICE, DataStatusEnum.ERROR));
                        }
                    } else if (ex instanceof RestClientException) {
                            DataLogging.logErrors(ex, DataErrorEnum.SERVERS_DOWN, keys); // server down
                            responseFuture.set(new DataResponse(null, DataErrorEnum.SERVERS_DOWN,
                                    DataStatusEnum.ERROR));
                    }
                }
            });

}

Is there any way to simplify this messy code?

\$\endgroup\$
1
\$\begingroup\$

It seems that the main difference is the type of DataErrorEnum used. So you can extract that to it's own method:

private static DataErrorEnum getErrorByException(HttpStatusCodeException ex) {
    HttpStatus statusCode = httpException.getStatusCode();
    switch (statusCode) {
        case HttpStatus.BAD_REQUEST:
            return DataErrorEnum.BAD_REQUEST;
        [...]
    }
}

And then your onFailure method will look a lot simpler:

@Override
public void onFailure(Throwable ex) {
    HttpStatusCodeException httpException = (HttpStatusCodeException) ex;
    DataErrorEnum error = getErrorByException(httpException);
    DataLogging.logErrors(ex, error, keys);
    responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
                    error, DataStatusEnum.ERROR));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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