You don't even need ResponseEntitys! Just use getForObject with a String.class like:

final RestTemplate restTemplate = new RestTemplate();
final String response = restTemplate.getForObject("https://httpbin.org/ip", String.class);

System.out.println(response);

It will print something like:

{
  "origin": "1.2.3.4"
}

Parse the JSON with a library (e.g. Jackson) and grab the token field.

String json = restTemplate.getForObject(url, String.class);
ObjectMapper mapper = new ObjectMapper();
return mapper.readTree(json).get("token").asText();

First define an object to hold the entity coming back in the array.. e.g.

@JsonIgnoreProperties(ignoreUnknown = true)
public class Rate {
    private String name;
    private String code;
    private Double rate;
    // add getters and setters
}

Then you can consume the service and get a strongly typed list via:

ResponseEntity<List<Rate>> rateResponse =
        restTemplate.exchange("https://bitpay.com/api/rates",
                    HttpMethod.GET, null, new ParameterizedTypeReference<List<Rate>>() {
            });
List<Rate> rates = rateResponse.getBody();

The other solutions above will also work, but I like getting a strongly typed list back instead of an Object[].

1. I would consider creating an AsyncClient and a SyncClient interface/classes instead of one. I guess users of the current Client class would use only one of the two methods. Furthermore, getSyncData does not use any of the fields of DataClient currently.

   Async version:

   public class AsyncDataClient {
   
       private RestTemplate restTemplate = new RestTemplate();
       private ExecutorService service = Executors.newFixedThreadPool(15);
   
       public Future<DataResponse> getAsyncData(DataKey key) {
           DataFetcherTask task = new DataFetcherTask(key, restTemplate);
           Future<DataResponse> future = service.submit(task);
   
           return future;
       }
   }
   

   Sync version:

   public class SyncDataClient {
   
       private AsyncDataClient asyncDataClient;
   
       public SyncDataClient(final AsyncDataClient asyncDataClient) {
           this.asyncDataClient = checkNotNull(asyncDataClient, "asyncDataClient cannot be null");
       }
   
       public DataResponse getSyncData(DataKey key) {
           ...
           responseFuture = asyncDataClient.getAsyncData(key);
           ...
       }
   }
   

   See also: Interface segregation principle

2. In getSyncData the response variable is used for multiple purposes. It could store a valid response and error responses too. I would use separate variables for these purposes for better readability and smaller variable scope:

   public DataResponse getSyncData(DataKey key) {
       Future<DataResponse> responseFuture = null;
   
       try {
           responseFuture = asyncDataClient.getAsyncData(key);
           final DataResponse response = responseFuture.get(key.getTimeout(), key.getUnitOfTime());
           return response;
       } catch (TimeoutException ex) {
           final DataResponse response = new DataResponse(DataErrorEnum.CLIENT_TIMEOUT, DataStatusEnum.ERROR);
           responseFuture.cancel(true); // terminating the tasks that have got
                                        // timed out
           return response;
       } catch (Exception ex) {
           return new DataResponse(DataErrorEnum.ERROR_CLIENT, DataStatusEnum.ERROR);
       }
   }
   

   It also makes easier to figure out that the only occasion when this method returns null is the try block, when the future returns null.

   See also: Effective Java, Second Edition, Item 45: Minimize the scope of local variables

3. In this catch block you loose the cause of the error:

   } catch (Exception ex) {
       response = new DataResponse(DataErrorEnum.ERROR_CLIENT, DataStatusEnum.ERROR);
   }
   

   An operator would be helpful for (at least) a debug level log message here. It could save you lots of debugging time.

4. DataKey contains at least these methods:

   • getTimeout()
   • getUnitOfTime()
   • getTypeOfFlow()
   • getUserId()
   • getEntity()

   It reminds me a DataRequest object name instead. Consider renaming. For me, DataKey is closer to a cache key class or something like that. Furthermore, getEntity is still smells even in a request class. It might be a third parameter of getAsyncData and the constructor of DataFetcherTask as well.

5. The of hostnames reference could be a simple List<String> instead of

   LinkedList<String> hostnames = ...
   

   As far as I see the code does not use any LinkedList specific methods.

   See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

6. This:

   if (CollectionUtils.isEmpty(hostnames)) {
   

   Could be changed to

    if (hostnames.isEmpty()) {
   

   CollectionUtils checks nulls as well, but if it's a null you get a NullPointerException in the for loop earlier.

7. Instead of StringUtils.isEmpty(hostname) I usually prefer StringUtils.isBlank which handles whitespace-only strings too.

8. I don't know how complex is your generateURL method but I would consider moving it to an UrlGenerator method. I would also call it generateUrl for a little bit better readability.

   From Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions:

   While uppercase may be more common, a strong argument can made in favor of capitalizing only the first letter: even if multiple acronyms occur back-to-back, you can still tell where one word starts and the next word ends. Which class name would you rather see, HTTPURL or HttpUrl?

9. You could move this part at the beginning of your method as a guard clause:

   // if hostnames are empty, then sending different ERROR ENUM code.
   if (hostnames.isEmpty()) {
       dataResponse = new DataResponse(null, DataErrorEnum.PERT_ERROR, DataStatusEnum.ERROR);
   

   For example:

   List<String> hostnames = mappings.getListOfHostnames(key.getUserId());
   // if hostnames are empty, then sending different ERROR ENUM code.
   if (hostnames.isEmpty()) {
       return new DataResponse(null, DataErrorEnum.PERT_ERROR, DataStatusEnum.ERROR);
   }
   

10. Instead of the break statement in the loop you could return immediately:

    if (response.getStatusCode() == HttpStatus.NO_CONTENT) {
        return new DataResponse(response.getBody(), DataErrorEnum.NO_CONTENT, DataStatusEnum.SUCCESS);
    } else {
        return new DataResponse(response.getBody(), DataErrorEnum.OK, DataStatusEnum.SUCCESS);
    }
    

    It also helps to make the scope of dataResponse variable smaller. Actually, you don't need it at all and you could get rid of the null comparison at the end of the method too:

    @Override
    public DataResponse call() {
        ...
        List<String> hostnames = mappings.getListOfHostnames(key.getUserId());
        if (hostnames.isEmpty()) {
            return new DataResponse(null, DataErrorEnum.PERT_ERROR, DataStatusEnum.ERROR);
        }
    
        for (String hostname: hostnames) {
            ...
            try {
                ...
                ResponseEntity<String> response = restTemplate.exchange(url, HttpMethod.GET, key.getEntity(), String.class);
                ...
                if (response.getStatusCode() == HttpStatus.NO_CONTENT) {
                    return new DataResponse(response.getBody(), DataErrorEnum.NO_CONTENT, DataStatusEnum.SUCCESS);
                } else {
                    return new DataResponse(response.getBody(), DataErrorEnum.OK, DataStatusEnum.SUCCESS);
                }
            } catch (HttpClientErrorException ex) {
                ...
                return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            } catch (HttpServerErrorException ex) {
                ...
                return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            } 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);
            }
        }
    
        // either  all the servers are down or all the servers were in block list
        return new DataResponse(null, DataErrorEnum.SERVICE_UNAVAILABLE, DataStatusEnum.ERROR);
    }
    

    Also note the scope change of response.

11. I don't see why you need this casting:

    } catch (HttpClientErrorException ex) {
        HttpStatusCodeException httpException = (HttpStatusCodeException) ex;
    

    and this one:

    } catch (HttpServerErrorException ex) {
        HttpStatusCodeException httpException = (HttpStatusCodeException) ex;
    

    Since HttpStatusCodeException is a superclass of both HttpClientErrorException and HttpServerErrorException the following is the same:

        } catch (HttpClientErrorException ex) {
            HttpStatusCodeException httpException = ex;
            ...
        } catch (HttpServerErrorException ex) {
            HttpStatusCodeException httpException = ex;
    

    Furthermore, HttpStatusCodeException has only these two subclasses in Spring and the body of both catch clauses are the same so you could simply catch only HttpStatusCodeException here:

    } catch (HttpStatusCodeException httpException) {
        DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
        String errorMessage = httpException.getResponseBodyAsString();
        return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
    } 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);
    }
    

    Keep in my that anyone can create a new subclass of HttpStatusCodeException so that's might not what you want.

    If you're using Java 7 or later, you could use multi-catch with one catch block:

    } catch (HttpClientErrorException | HttpServerErrorException ex) {
    

    Another solution is extracting out the common code into a method:

     private DataResponse createErrorResponse(HttpStatusCodeException httpException) {
        DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
        String errorMessage = httpException.getResponseBodyAsString();
        return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
    }
    

    Usage:

    } catch (HttpClientErrorException ex) {
        return createErrorResponse(ex);
    } catch (HttpServerErrorException ex) {
        return createErrorResponse(ex);
    

12. responseFuture.cancel(true) is in a catch block. I have not checked but I would try to move it into a finally block. If another (non-timeout exception) happens you won't use the future anyway.

13. getUnitOfTime does not suggest any connection with the getTimeout method. I would rename it to getTimeoutUnit.

OK, eventually, one of my co-workers helped me figure out why, believe it or not, it's this simple:

The endpoint was like this: "http:localhost:8080/api/v1/items?itemIds=" + URLEncoder.encode(itemIds, "UTF-8");

However, it should be "http:localhost:8080/api/v1/items?itemIds=" + itemIds;

itemIds is just a comma-separated list.

After URLEncoder encoding via "UTF-8" schema, this comma-separated list becomes itemIds=5400028914%2C5400029138%2C5400029138%2C5400029138%2C5400029138%2C5400028401%2C5400028918%2C5400028076

from

itemIds=5400028914,5400029138,5400029138,5400029138,5400029138,5400028401,5400028918,5400028076,5400028726

We don't need to URLEncoder to encode the URL when using RestTemplate, anyone could help me deepen my understanding here please?

Thanks!

If your RestTemplate is configured with the default HttpMessageConverters, which is provided by Jackson2ObjectMapperBuilder, you may directly get a JsonNode from restTemplate.getForObject.

For example,

ArrayNode resources = restTemplate.getForObject("/resources", ArrayNode.class);

Or,

ObjectNode resource = restTemplate.getForObject("/resources/123", ObjectNode.class);

Note that ArrayNode and ObjectNode are sub-classes of JsonNode.

I could solve my issue with RC.'s help. I'll post the answer to help other people.

The problem was that Accept header is automatically set to APPLICATION/JSON so I had to change the way to invoke the service in order to provide the Accept header I want.

I changed this:

String response = getRestTemplate().getForObject(url, String.class);

To this in order to make the application work:

// Set XML content type explicitly to force response in XML (If not spring gets response in JSON)
HttpHeaders headers = new HttpHeaders();
headers.setAccept(Arrays.asList(MediaType.APPLICATION_XML));
HttpEntity<String> entity = new HttpEntity<String>("parameters", headers);

ResponseEntity<String> response = getRestTemplate().exchange(url, HttpMethod.GET, entity, String.class);
String responseBody = response.getBody();

The postForLocation method returns the value for the Location header. You should use postForObject with the String class, which returns the server's response.

So like this:

String result = restTemplate.postForObject(API_URL + "account/authenticate/?email={email}&token={token}", String.class, args);

This will return the response as a string.

1. As of the "main key is result":

   a. I would create a wrapper class for the actual payload if you deal with just one of this kind of web service:

   public class Return{
       // Class property cannot be called "return" because it is Java reserved name.
       @JsonProperty("return")
       private User[] array;
       .... getter and setter
   }
   

   b. If you deal with multiple webservices where actual payload is in "return" field I would create a generic wrapper class :

   public class Return<T>{
       // Class property cannot be called "return" because it is Java reserved name.
       @JsonProperty("return")
       private T[] array;
       .... getter and setter
   }
   

   Call to RestRemplate:

   ResponseEntity<Return<User>> response = restTemplate.exchange(URL_GET, 
           HttpMethod.GET, request, new ParameterizedTypeReference<Return<User>>(){});
   User[] usersArray = response2.getBody().getArray();
   

2. As of the property value in JSON attribute called "value" I would create two custom JsonDeserializer(s): one for single value and one for array of values and annotate each property with @JsonDeserialize where it applies:

   Single value deserializer:

   public class StringValueDeserializer  extends JsonDeserializer<String>{
   
       @Override
       public String deserialize(JsonParser parser, DeserializationContext ctxt)
               throws IOException, JsonProcessingException {
           ObjectCodec codec = parser.getCodec();
           TreeNode node = codec.readTree(parser);
           JsonNode value = (JsonNode)node.get("value");
   
           if (value != null){
               return value.asText();
           }
           return null;
       }
   }
   

   Array of values derializer:

   public class StringArrayValueDeserializer  extends JsonDeserializer<List<String>>{
   
       @Override
       public List<String> deserialize(JsonParser parser, DeserializationContext ctxt)
           throws IOException, JsonProcessingException {
   
           List<String> ret = new ArrayList<>();
   
           ObjectCodec codec = parser.getCodec();
           TreeNode node = codec.readTree(parser);
   
           if (node.isArray()){
               for (JsonNode n : (ArrayNode)node){
                   JsonNode value = n.get("value");
                   if (value != null){
                       ret.add(value.asText());
                   }
               }
           }
           return ret;
       }
   }
   

   Here you are new User.class:

   public class User {
   
       private boolean admin;
   
       private String alias;
   
       private String email;
   
       @JsonDeserialize(using = StringValueDeserializer.class)
       private String emailId;
   
       @JsonDeserialize(using = StringArrayValueDeserializer.class)
       private ArrayList<String> groups;
   
       @JsonDeserialize(using = StringValueDeserializer.class)
       private String id;
   
       private boolean locked;
   
       private int loggedInCount;
   
       private boolean master;
   
       private String sms;
   
       @JsonDeserialize(using = StringValueDeserializer.class)
       private String smsId;
   
       private String type;
   
       private String username;
       .... getter and setter
   }
   

Good luck!

This technique worked for me:

HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
            
HttpEntity<String> entity = new HttpEntity<String>(requestJson, headers);
ResponseEntity<String> response = restTemplate.put(url, entity);

Since you are using Spring Boot, it usually comes bundled with handy tools for JSON parsing. Spring Boot wires per default jackson into your application.

The first thing, you'll need is a (reduced) POJO model of the response.

@JsonIgnoreProperties(ignoreUnknown = true)
public class ResponsePojo {
   @JsonProperty("<jsonFieldName>") // only required, if fieldName != jsonFieldName
   private List<String> residents; 

/* getter & setter ommitted */
}

In your calling code, use something like

ResponsePojo response = restTemplate.getForObject(url, ResponsePojo.class);
response.getResidents() gives you access to the contents of 'resident' array

What happens behind the scenes ?

RestTemplate sends your request and tries to parse the response into your ResponsePojo object. Since the pojo is a reduced representation of the response, we provided the annotation @JsonIgnoreProperties(ignoreUnknown = true). This tells the parser, that it should simple ignore any field in the json, which cannot be mapped to your pojo. Since a field is provided, with the exact name like in json, the parser is able to identify and map them accordingly.