0

I am working on an api to make batch requests through HTTP. Suggested by this (How to make batch http call in java), I decided to use Multithreading.

Here is the implementation:

public class GetItems {
//Control the number of connections made to the server.
PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager();
cm.setMaxTotal(120);
cm.setDefaultMaxPerRoute(30);
HttpClient httpClient = HttpClients.custom()
    .setConnectionManager(cm)
    .build();

//Using concurrent map to store the result from different threads
private Map<String, ItemAttributesFromDatapath> itemDataMap = Maps.newConcurrentMap();

//requests' batch
private List<Request> requests;

public List<String> call() {
        List<Thread> threads = Lists.newArrayList();
        requests.forEach(request -> {
            Thread thread = new Thread(new GetTask(client, request));
            thread.start();
            threads.add(thread);
        });

        threads.forEach(thread -> {
            try {
                thread.join();
            } catch (Exception ex) {
                LOGGER.error(ex);
            }
        });

}

//Inner class for multi-thread.
public class GetTask implements Runnable{

        private CatalogHttpRequest request;
        private CatalogHttpClient client;
        private String asin;

        public GetTask(HttpClient client, final HttpRequest request) {
            this.request = request;
            this.client = client;
        }

        @Override
        public void run(){
            try{
               HttpResponse response = client.executeHttpRequest(request);
                try {
                    if (response.isOK()) {
                        //some logic here
                        String item = handle(response)
                        itemDataMap.put(itemId, item);

                    } else {
                        String message = "Is Client Error";
                        LOGGER.error(message);

                    }
                } catch (Exception ex){
                    LOGGER.error(ex.getMessage());

                }
            } catch (Exception ex) {
                LOGGER.error(ex.getMessage());
            }
        }
    }
}

Then I tried to write unit-test,

@Test
public void testBatchRequest() throws Exception{
HttpClient = mockClient(...);
GetItems batch = new GetItems(...);
List<String> ret = batch.call();}

Inside mockClient method, I mock some class using Mockito:

private HttpClient mockSuccessfulClient(InputStream responseStream, int code) throws IOException{

        HttpEntity entity = Mockito.mock(HttpEntity.class);
        when(entity.getContent()).thenReturn(responseStream);

        HttpResponse response = Mockito.mock(CloseableHttpResponse.class);
        when(response.getEntity()).thenReturn(entity);

        StatusLine statusLine = Mockito.mock(StatusLine.class);
        when(statusLine.getStatusCode()).thenReturn(code);
        when(response.getStatusLine()).thenReturn(statusLine);

        //CloseableHttpClient httpClient = Mockito.mock(CloseableHttpClient.class);
        //when(httpClient.execute(any())).thenReturn(response);


        CatalogHttpClient client = Mockito.mock(HttpClient.class);
        when(client.executeHttpRequest(any())).thenReturn(response);

        return client;
    }

For the unit-test right now, the return list is empty, which is incorrect.

I tried to debug the code by setting up breaking point and found the main code didn't execute run() method inside GetTask when thread starts.

Since I am not familiar with multi-thread stuff. This problem may seem dumb to you, but can you help me figure out what's wrong in my code and how to write the proper test for my case.

Also, I am not sure if the ConcurrentMap is a good practice to store the results across multiple threads?

Meng Li
  • 65
  • 1
  • 7
  • 'Batch' and 'multi-threading' are mutually exclusive. – user207421 Jul 06 '17 at 03:12
  • Are you sure that `run()` didn't run? You should see new threads appear in your debug window. I can't imagine that `run()` wouldn't be running, unless it throws an exception immediately. Also, not what you asked, but there's no point in nesting one `try` inside another when the two `catch` clauses do exactly the same thing. – Dawood ibn Kareem Jul 06 '17 at 04:01
  • Please post a [mcve]. (1) Your code doesn't compile, so we can't test it easily. (2) The initialization of the variable `request` is missing, so we can't check it (if it's empty, that's the problem). (3) Similarly, the return from `batch()` is important but missing. (4) The HTTP stuff does not appear to be directly related to the problem, so you should remove it (including the mocks). If the problem goes away, add things back. Keep simplifying until you have a small example that exhibits the problem. You might even find the bug on your own this way. – tom Jul 11 '17 at 10:46

0 Answers0