Drop PerRequestConfig, close #591

AsyncHttpClient · Jul 4, 2014 · feeb504 · feeb504
1 parent ad77819
commit feeb504
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 133 deletions.
diff --git a/src/main/java/com/ning/http/client/PerRequestConfig.java b/src/main/java/com/ning/http/client/PerRequestConfig.java
diff --git a/src/main/java/com/ning/http/client/Request.java b/src/main/java/com/ning/http/client/Request.java
@@ -201,11 +201,10 @@ public static interface EntityWriter {
     boolean isRedirectOverrideSet();
 
     /**
-     * Return Per request configuration.
-     *
-     * @return Per request configuration.
+     * Overrides the config default value
+     * @return the request timeout
      */
-    PerRequestConfig getPerRequestConfig();
+    int getRequestTimeoutInMs();
 
     /**
      * Return the HTTP Range header value, or

diff --git a/src/main/java/com/ning/http/client/RequestBuilderBase.java b/src/main/java/com/ning/http/client/RequestBuilderBase.java
@@ -72,7 +72,7 @@ private static final class RequestImpl implements Request {
         private Realm realm;
         private File file;
         private Boolean followRedirects;
-        private PerRequestConfig perRequestConfig;
+        private int requestTimeoutInMs;
         private long rangeOffset;
         public String charset;
         private boolean useRawUrl;
@@ -104,7 +104,7 @@ public RequestImpl(Request prototype) {
                 this.realm = prototype.getRealm();
                 this.file = prototype.getFile();
                 this.followRedirects = prototype.isRedirectOverrideSet() ? prototype.isRedirectEnabled() : null;
-                this.perRequestConfig = prototype.getPerRequestConfig();
+                this.requestTimeoutInMs = prototype.getRequestTimeoutInMs();
                 this.rangeOffset = prototype.getRangeOffset();
                 this.charset = prototype.getBodyEncoding();
                 this.useRawUrl = prototype.isUseRawUrl();
@@ -291,8 +291,8 @@ public boolean isRedirectOverrideSet() {
             return followRedirects != null;
         }
 
-        public PerRequestConfig getPerRequestConfig() {
-            return perRequestConfig;
+        public int getRequestTimeoutInMs() {
+            return requestTimeoutInMs;
         }
 
         public long getRangeOffset() {
@@ -616,8 +616,8 @@ public T setFollowRedirects(boolean followRedirects) {
         return derived.cast(this);
     }
 
-    public T setPerRequestConfig(PerRequestConfig perRequestConfig) {
-        request.perRequestConfig = perRequestConfig;
+    public T setRequestTimeoutInMs(int requestTimeoutInMs) {
+        request.requestTimeoutInMs = requestTimeoutInMs;
         return derived.cast(this);
     }
 

diff --git a/src/main/java/com/ning/http/client/providers/apache/ApacheAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/apache/ApacheAsyncHttpProvider.java
@@ -27,7 +27,6 @@
 import com.ning.http.client.ListenableFuture;
 import com.ning.http.client.MaxRedirectException;
 import com.ning.http.client.Part;
-import com.ning.http.client.PerRequestConfig;
 import com.ning.http.client.ProgressAsyncHandler;
 import com.ning.http.client.ProxyServer;
 import com.ning.http.client.Realm;
@@ -202,7 +201,7 @@ public <T> ListenableFuture<T> execute(Request request, AsyncHandler<T> handler)
             idleConnectionTimeoutThread = null;
         }
 
-        int requestTimeout = requestTimeout(config, request.getPerRequestConfig());
+        int requestTimeout = AsyncHttpProviderUtils.requestTimeout(config, request);
         if (config.getIdleConnectionTimeoutInMs() > 0 && requestTimeout != -1 && requestTimeout < config.getIdleConnectionTimeoutInMs()) {
             idleConnectionTimeoutThread = new IdleConnectionTimeoutThread();
             idleConnectionTimeoutThread.setConnectionTimeout(config.getIdleConnectionTimeoutInMs());
@@ -469,7 +468,7 @@ public T call() {
                     uri = AsyncHttpProviderUtils.createNonEmptyPathURI(request.getUrl());
                 }
 
-                int delay = requestTimeout(config, future.getRequest().getPerRequestConfig());
+                int delay = AsyncHttpProviderUtils.requestTimeout(config, future.getRequest());
                 if (delay != -1) {
                     ReaperFuture reaperFuture = new ReaperFuture(future);
                     Future scheduledFuture = reaper.scheduleAtFixedRate(reaperFuture, delay, 500, TimeUnit.MILLISECONDS);
@@ -665,11 +664,7 @@ private Throwable filterException(Throwable t) {
                 t = new ConnectException(t.getMessage());
 
             } else if (t instanceof NoHttpResponseException) {
-                int responseTimeoutInMs = config.getRequestTimeoutInMs();
-
-                if (request.getPerRequestConfig() != null && request.getPerRequestConfig().getRequestTimeoutInMs() != -1) {
-                    responseTimeoutInMs = request.getPerRequestConfig().getRequestTimeoutInMs();
-                }
+                int responseTimeoutInMs = AsyncHttpProviderUtils.requestTimeout(config, request);
                 t = new TimeoutException(String.format("No response received after %s", responseTimeoutInMs));
 
             } else if (t instanceof SSLHandshakeException) {
@@ -876,26 +871,11 @@ public synchronized void run() {
             if (this.apacheResponseFuture != null && this.apacheResponseFuture.hasExpired()) {
                 logger.debug("Request Timeout expired for " + this.apacheResponseFuture);
 
-                int requestTimeout = config.getRequestTimeoutInMs();
-                PerRequestConfig p = this.apacheResponseFuture.getRequest().getPerRequestConfig();
-                if (p != null && p.getRequestTimeoutInMs() != -1) {
-                    requestTimeout = p.getRequestTimeoutInMs();
-                }
+                int requestTimeout = AsyncHttpProviderUtils.requestTimeout(config, this.apacheResponseFuture.getRequest());
                 apacheResponseFuture.abort(new TimeoutException(String.format("No response received after %s", requestTimeout)));
 
                 this.apacheResponseFuture = null;
             }
         }
     }
-
-    protected static int requestTimeout(AsyncHttpClientConfig config, PerRequestConfig perRequestConfig) {
-        int result;
-        if (perRequestConfig != null) {
-            int prRequestTimeout = perRequestConfig.getRequestTimeoutInMs();
-            result = (prRequestTimeout != 0 ? prRequestTimeout : config.getRequestTimeoutInMs());
-        } else {
-            result = config.getRequestTimeoutInMs();
-        }
-        return result;
-    }
 }
diff --git a/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java
@@ -120,7 +120,6 @@
 import com.ning.http.client.ListenableFuture;
 import com.ning.http.client.MaxRedirectException;
 import com.ning.http.client.Part;
-import com.ning.http.client.PerRequestConfig;
 import com.ning.http.client.ProxyServer;
 import com.ning.http.client.Realm;
 import com.ning.http.client.Request;
@@ -360,12 +359,9 @@ public long getTimeout(FilterChainContext ctx) {
                                 if (context.isWSRequest) {
                                     return clientConfig.getWebSocketIdleTimeoutInMs();
                                 }
-                                final PerRequestConfig config = context.request.getPerRequestConfig();
-                                if (config != null) {
-                                    final long timeout = config.getRequestTimeoutInMs();
-                                    if (timeout > 0) {
-                                        return timeout;
-                                    }
+                                final long timeout = context.request.getRequestTimeoutInMs();
+                                if (timeout > 0) {
+                                    return timeout;
                                 }
                             }
                             return timeout;
@@ -441,14 +437,11 @@ public void onTimeout(Connection connection) {
 
     void touchConnection(final Connection c, final Request request) {
 
-        final PerRequestConfig config = request.getPerRequestConfig();
-        if (config != null) {
-            final long timeout = config.getRequestTimeoutInMs();
-            if (timeout > 0) {
-                final long newTimeout = System.currentTimeMillis() + timeout;
-                if (resolver != null) {
-                    resolver.setTimeoutMillis(c, newTimeout);
-                }
+        final long perRequestTimeout = request.getRequestTimeoutInMs();
+        if (perRequestTimeout > 0) {
+            final long newTimeout = System.currentTimeMillis() + perRequestTimeout;
+            if (resolver != null) {
+                resolver.setTimeoutMillis(c, newTimeout);
             }
         } else {
             final long timeout = clientConfig.getRequestTimeoutInMs();

diff --git a/src/main/java/com/ning/http/client/providers/jdk/JDKAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/jdk/JDKAsyncHttpProvider.java
@@ -63,7 +63,6 @@
 import com.ning.http.client.HttpResponseStatus;
 import com.ning.http.client.ListenableFuture;
 import com.ning.http.client.MaxRedirectException;
-import com.ning.http.client.PerRequestConfig;
 import com.ning.http.client.ProgressAsyncHandler;
 import com.ning.http.client.ProxyServer;
 import com.ning.http.client.Realm;
@@ -153,9 +152,7 @@ public <T> ListenableFuture<T> execute(Request request, AsyncHandler<T> handler,
             throw new IOException(e.getMessage());
         }
 
-        PerRequestConfig conf = request.getPerRequestConfig();
-        int requestTimeout = (conf != null && conf.getRequestTimeoutInMs() != 0) ?
-                conf.getRequestTimeoutInMs() : config.getRequestTimeoutInMs();
+        int requestTimeout = AsyncHttpProviderUtils.requestTimeout(config, request);
 
         JDKDelegateFuture delegate = null;
         if (future != null) {
@@ -434,11 +431,7 @@ private Throwable filterException(Throwable t) {
                 t = new ConnectException(t.getMessage());
 
             } else if (t instanceof SocketTimeoutException) {
-                int responseTimeoutInMs = config.getRequestTimeoutInMs();
-
-                if (request.getPerRequestConfig() != null && request.getPerRequestConfig().getRequestTimeoutInMs() != -1) {
-                    responseTimeoutInMs = request.getPerRequestConfig().getRequestTimeoutInMs();
-                }
+                int responseTimeoutInMs = AsyncHttpProviderUtils.requestTimeout(config, request);
                 t = new TimeoutException(String.format("No response received after %s", responseTimeoutInMs));
 
             } else if (t instanceof SSLHandshakeException) {
@@ -452,9 +445,7 @@ private Throwable filterException(Throwable t) {
 
         private void configure(UriComponents uri, HttpURLConnection urlConnection, Request request) throws IOException, AuthenticationException {
 
-            PerRequestConfig conf = request.getPerRequestConfig();
-            int requestTimeout = (conf != null && conf.getRequestTimeoutInMs() != 0) ?
-                    conf.getRequestTimeoutInMs() : config.getRequestTimeoutInMs();
+            int requestTimeout = AsyncHttpProviderUtils.requestTimeout(config, request);
 
             urlConnection.setConnectTimeout(config.getConnectionTimeoutInMs());
 

diff --git a/src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java
@@ -123,7 +123,6 @@
 import com.ning.http.client.HttpResponseStatus;
 import com.ning.http.client.ListenableFuture;
 import com.ning.http.client.MaxRedirectException;
-import com.ning.http.client.PerRequestConfig;
 import com.ning.http.client.ProgressAsyncHandler;
 import com.ning.http.client.ProxyServer;
 import com.ning.http.client.RandomAccessBody;
@@ -1139,17 +1138,6 @@ private <T> ListenableFuture<T> doConnect(final Request request, final AsyncHand
         return c.future();
     }
 
-    protected static int requestTimeoutInMs(AsyncHttpClientConfig config, PerRequestConfig perRequestConfig) {
-        int result;
-        if (perRequestConfig != null) {
-            int prRequestTimeout = perRequestConfig.getRequestTimeoutInMs();
-            result = (prRequestTimeout != 0 ? prRequestTimeout : config.getRequestTimeoutInMs());
-        } else {
-            result = config.getRequestTimeoutInMs();
-        }
-        return result;
-    }
-
     private void closeChannel(final ChannelHandlerContext ctx) {
         connectionsPool.removeAll(ctx.getChannel());
         finishChannel(ctx);
@@ -1711,7 +1699,7 @@ public static <T> NettyResponseFuture<T> newFuture(UriComponents uri, Request re
                 request,//
                 asyncHandler,//
                 nettyRequest,//
-                requestTimeoutInMs(config, request.getPerRequestConfig()),//
+                AsyncHttpProviderUtils.requestTimeout(config, request),//
                 config.getIdleConnectionTimeoutInMs(),//
                 provider,//
                 request.getConnectionPoolKeyStrategy(),//

diff --git a/src/main/java/com/ning/http/util/AsyncHttpProviderUtils.java b/src/main/java/com/ning/http/util/AsyncHttpProviderUtils.java
@@ -226,6 +226,6 @@ public static String keepAliveHeaderValue(AsyncHttpClientConfig config) {
     }
 
     public static int requestTimeout(AsyncHttpClientConfig config, Request request) {
-        return (request.getPerRequestConfig() != null && request.getPerRequestConfig().getRequestTimeoutInMs() != 0) ? request.getPerRequestConfig().getRequestTimeoutInMs() : config.getRequestTimeoutInMs();
+        return request.getRequestTimeoutInMs() != 0 ? request.getRequestTimeoutInMs() : config.getRequestTimeoutInMs();
     }
 }
diff --git a/src/test/java/com/ning/http/client/async/PerRequestTimeoutTest.java b/src/test/java/com/ning/http/client/async/PerRequestTimeoutTest.java
@@ -21,7 +21,6 @@
 import com.ning.http.client.AsyncHttpClient;
 import com.ning.http.client.AsyncHttpClientConfig;
 import com.ning.http.client.HttpResponseBodyPart;
-import com.ning.http.client.PerRequestConfig;
 import com.ning.http.client.Response;
 import org.eclipse.jetty.continuation.Continuation;
 import org.eclipse.jetty.continuation.ContinuationSupport;
@@ -98,10 +97,8 @@ public void run() {
     @Test(groups = { "standalone", "default_provider" })
     public void testRequestTimeout() throws IOException {
         AsyncHttpClient client = getAsyncHttpClient(null);
-        PerRequestConfig requestConfig = new PerRequestConfig();
-        requestConfig.setRequestTimeoutInMs(100);
         try {
-            Future<Response> responseFuture = client.prepareGet(getTargetUrl()).setPerRequestConfig(requestConfig).execute();
+            Future<Response> responseFuture = client.prepareGet(getTargetUrl()).setRequestTimeoutInMs(100).execute();
             Response response = responseFuture.get(2000, TimeUnit.MILLISECONDS);
             assertNull(response);
             client.close();
@@ -120,10 +117,8 @@ public void testRequestTimeout() throws IOException {
     @Test(groups = { "standalone", "default_provider" })
     public void testGlobalDefaultPerRequestInfiniteTimeout() throws IOException {
         AsyncHttpClient client = getAsyncHttpClient(new AsyncHttpClientConfig.Builder().setRequestTimeoutInMs(100).build());
-        PerRequestConfig requestConfig = new PerRequestConfig();
-        requestConfig.setRequestTimeoutInMs(-1);
         try {
-            Future<Response> responseFuture = client.prepareGet(getTargetUrl()).setPerRequestConfig(requestConfig).execute();
+            Future<Response> responseFuture = client.prepareGet(getTargetUrl()).setRequestTimeoutInMs(-1).execute();
             Response response = responseFuture.get();
             assertNotNull(response);
             client.close();

diff --git a/src/test/java/com/ning/http/client/async/netty/NettyRequestThrottleTimeoutTest.java b/src/test/java/com/ning/http/client/async/netty/NettyRequestThrottleTimeoutTest.java
@@ -86,12 +86,10 @@ public void testRequestTimeout() throws IOException {
                 public void run() {
                     try {
                         requestThrottle.acquire();
-                        PerRequestConfig requestConfig = new PerRequestConfig();
-                        requestConfig.setRequestTimeoutInMs(SLEEPTIME_MS/2);
                         Future<Response> responseFuture = null;
                         try {
                              responseFuture =
-                                    client.prepareGet(getTargetUrl()).setPerRequestConfig(requestConfig).execute(new AsyncCompletionHandler<Response>() {
+                                    client.prepareGet(getTargetUrl()).setRequestTimeoutInMs(SLEEPTIME_MS/2).execute(new AsyncCompletionHandler<Response>() {
 
                                         @Override
                                         public Response onCompleted(Response response) throws Exception {