0
\$\begingroup\$

I am using an CloseableHttpAsyncClient the following way:

public class AsyncHttpClient {

    private static final Logger logger = LogManager.getLogger(Main.class);

    private static CloseableHttpAsyncClient HTTP_ASYNC_CLIENT = null;
    private static HttpPost HTTP_POST = null;
    private static HttpContext HTTP_CONTEXT;

    private static PropertiesLoader PROP_LOADER = new PropertiesLoader();
    private static Properties PROPERTIES = PROP_LOADER.initProp("./config/config.properties");
    private static final int CON_REQUEST_TIMEOUT = Integer.valueOf(PROPERTIES.getProperty("connectionrequesttimeout"));
    private static final int CON_TIMEOUT = Integer.valueOf(PROPERTIES.getProperty("connecttimeout"));
    private static final int CON_SOCKET_TIMEOUT = Integer.valueOf(PROPERTIES.getProperty("sockettimeout"));
    private static final String CON_URL = PROPERTIES.getProperty("conurl");
    private static final int CONMANAGER_MAX_TOTAL = Integer.valueOf(PROPERTIES.getProperty("conmanagermaxtotal"));
    private static final int DEFAULT_MAX_PER_ROUTE = Integer.valueOf(PROPERTIES.getProperty("conmanagermaxdefaultperroute"));

    //Reusable connection
    private static RequestConfig createConnConfig() {

        return RequestConfig.custom()
                .setConnectionRequestTimeout(CON_REQUEST_TIMEOUT)
                .setConnectTimeout(CON_TIMEOUT)
                .setSocketTimeout(CON_SOCKET_TIMEOUT).build();
    }

    private static PoolingNHttpClientConnectionManager createPoolingConnManager() throws IOReactorException {
        IOReactorConfig config = IOReactorConfig.DEFAULT;
        ConnectingIOReactor ioReactor = new DefaultConnectingIOReactor(config);

        PoolingNHttpClientConnectionManager cm = new PoolingNHttpClientConnectionManager(ioReactor);
        cm.setMaxTotal(CONMANAGER_MAX_TOTAL);
        cm.setDefaultMaxPerRoute(DEFAULT_MAX_PER_ROUTE);

        return cm;
    }

    private static synchronized CloseableHttpAsyncClient getCloseableClient() throws IOReactorException {

        if (HTTP_ASYNC_CLIENT == null) {
            logger.info("New Async Client created ");
            HTTP_ASYNC_CLIENT = HttpAsyncClientBuilder.create()
                    .setDefaultRequestConfig(createConnConfig())
                    .setConnectionManager(createPoolingConnManager()).build();

            HTTP_ASYNC_CLIENT.start();
        }
        return HTTP_ASYNC_CLIENT;
    }

    private static synchronized HttpPost getPost() {
        if (HTTP_POST == null) {
            logger.info("New Post created ");
            HTTP_POST = new HttpPost(CON_URL);
            HTTP_POST.setHeader("Accept", "application/json");
            HTTP_POST.setHeader("Content-type", "application/json");
        }
        return HTTP_POST;
    }


    public static void postData(String data) throws Exception {
        //TODO - needs cleanup
        StringEntity entity = new StringEntity(data);
        HttpPost httpPost = getPost();
        httpPost.setEntity(entity);
        getCloseableClient().execute(httpPost, HTTP_CONTEXT, new FutureCallback<HttpResponse>() {
            @Override
            public void completed(HttpResponse httpResponse) {
                logger.info("Completed");

            }

            @Override
            public void failed(Exception e) {
                logger.info("Completed with exception " + e);
            }

            @Override
            public void cancelled() {
                logger.info("Cancelled ");
            }
        });
    }

}

Throughout the program I want only one instance to be alive, I am also using multiple threads to post data using the above class.

\$\endgroup\$
  • \$\begingroup\$ you should add a comment if you are not satisfied with the proposed answers, or accept an answer ;) \$\endgroup\$ – Ronan Dhellemmes Jun 8 '17 at 9:36
1
\$\begingroup\$

Thread safety

You want to have only one instance of CloseableHttpAsyncClient which is why you use static method and static variable, right ?

Sadly, using only static methods and global states is usually a bad idea (impossible to inject object... and thus you can't mock in your unit test, it also increases the coupling between your objects)

If you still want to use static methods, the synchronized method for the HttpAsyncClient creation make it safe against thread race and you are guaranteed to have only one HttpAsyncClient instance so you have that guarantee but you should consider using Dependency Injection instead.

Code format

The indentation is good IMO, however...

private static CloseableHttpAsyncClient HTTP_ASYNC_CLIENT = null;
private static HttpPost HTTP_POST = null;
private static HttpContext HTTP_CONTEXT;

private static PropertiesLoader PROP_LOADER = new PropertiesLoader();
private static Properties PROPERTIES = PROP_LOADER.initProp("./config/config.properties");

You should only use SCREAMING_SNAKE_CASE in java when the field is static final and using the object don't have any side-effects. Here it's not only non-final, CloseableHttpAsyncClient and PropertiesLoader are full of side-effects so SCREAMING_SNAKE_CASE is better avoided.

\$\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.