From f32a19f3e2f854dd7f455987e8d4a5c14e0f0eb5 Mon Sep 17 00:00:00 2001 From: dominictootell Date: Sun, 1 Dec 2013 13:43:48 +0000 Subject: [PATCH] Consume Entity in AsynchronousValidationRequest or a connection is lost remove the convoluted null check try/catch the close, convert test to use internal http server change to .close() on http response --- httpclient-cache/pom.xml | 7 + .../cache/AsynchronousValidationRequest.java | 11 +- ...stStaleWhileRevalidationReleasesConnection.java | 306 +++++++++++++++++++++ 3 files changed, 321 insertions(+), 3 deletions(-) create mode 100644 httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestStaleWhileRevalidationReleasesConnection.java diff --git a/httpclient-cache/pom.xml b/httpclient-cache/pom.xml index 199e8dc..93437dd 100644 --- a/httpclient-cache/pom.xml +++ b/httpclient-cache/pom.xml @@ -84,6 +84,13 @@ easymockclassextension test + + org.apache.httpcomponents + httpclient + ${project.version} + test-jar + test + diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java index 93b7a1d..ab30ea4 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java @@ -37,6 +37,7 @@ import org.apache.http.client.cache.HeaderConstants; import org.apache.http.client.cache.HttpCacheEntry; import org.apache.http.client.methods.HttpExecutionAware; import org.apache.http.client.methods.HttpRequestWrapper; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.conn.routing.HttpRoute; @@ -108,9 +109,13 @@ class AsynchronousValidationRequest implements Runnable { */ protected boolean revalidateCacheEntry() { try { - final HttpResponse httpResponse = cachingExec.revalidateCacheEntry(route, request, context, execAware, cacheEntry); - final int statusCode = httpResponse.getStatusLine().getStatusCode(); - return isNotServerError(statusCode) && isNotStale(httpResponse); + final CloseableHttpResponse httpResponse = cachingExec.revalidateCacheEntry(route, request, context, execAware, cacheEntry); + try { + final int statusCode = httpResponse.getStatusLine().getStatusCode(); + return isNotServerError(statusCode) && isNotStale(httpResponse); + } finally { + httpResponse.close(); + } } catch (final IOException ioe) { log.debug("Asynchronous revalidation failed due to I/O error", ioe); return false; diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestStaleWhileRevalidationReleasesConnection.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestStaleWhileRevalidationReleasesConnection.java new file mode 100644 index 0000000..29b4afb --- /dev/null +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestStaleWhileRevalidationReleasesConnection.java @@ -0,0 +1,306 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.http.impl.client.cache; + +import org.apache.http.HttpEntity; +import org.apache.http.HttpException; +import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; +import org.apache.http.MethodNotSupportedException; +import org.apache.http.Header; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.HttpClient; +import org.apache.http.client.cache.CacheResponseStatus; +import org.apache.http.client.cache.HttpCacheContext; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.entity.ByteArrayEntity; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.protocol.BasicHttpContext; +import org.apache.http.protocol.HttpContext; +import org.apache.http.protocol.HttpRequestHandler; +import org.apache.http.util.EntityUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.util.Locale; + + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import org.apache.http.localserver.LocalTestServer; + +/** + * Test that after background validation that a subsequent request for non cached + * conent can be made. This verifies that the connection has been release back to + * the pool by the AsynchronousValidationRequest. + */ +public class TestStaleWhileRevalidationReleasesConnection { + + private static final EchoViaHeaderHandler cacheHandler = new EchoViaHeaderHandler(); + + protected LocalTestServer localServer; + private int port; + private CloseableHttpClient client; + private final String url = "/static/dom"; + private final String url2 = "2"; + + + @Before + public void start() throws Exception { + this.localServer = new LocalTestServer(null, null); + this.localServer.register(url +"*", new EchoViaHeaderHandler()); + localServer.setTimeout(5000); + this.localServer.start(); + + port = this.localServer.getServiceAddress().getPort(); + + final CacheConfig cacheConfig = CacheConfig.custom() + .setMaxCacheEntries(100) + .setMaxObjectSize(15) //1574 + .setAsynchronousWorkerIdleLifetimeSecs(60) + .setAsynchronousWorkersMax(1) + .setAsynchronousWorkersCore(1) + .setRevalidationQueueSize(100) + .setSharedCache(true) + .build(); + + final HttpClientBuilder clientBuilder = CachingHttpClientBuilder.create().setCacheConfig(cacheConfig); + clientBuilder.setMaxConnTotal(1); + clientBuilder.setMaxConnPerRoute(1); + + final RequestConfig config = RequestConfig.custom() + .setSocketTimeout(10000) + .setConnectTimeout(10000) + .setConnectionRequestTimeout(1000) + .build(); + + clientBuilder.setDefaultRequestConfig(config); + + + client = clientBuilder.build(); + } + + @After + public void stop() { + if (this.localServer != null) { + try { + this.localServer.stop(); + } catch(Exception e) { + e.printStackTrace(); + } + } + + try { + client.close(); + } catch(IOException e) { + e.printStackTrace(); + } + } + + @Test + public void testStaleWhileRevalidate() { + final String urlToCall = "http://localhost:"+port + url; + final HttpContext localContext = new BasicHttpContext(); + Exception requestException = null; + + // This will fetch from backend. + requestException = sendRequest(client, localContext,urlToCall,null); + assertNull(requestException); + + CacheResponseStatus responseStatus = (CacheResponseStatus) localContext.getAttribute(HttpCacheContext.CACHE_RESPONSE_STATUS); + assertEquals(CacheResponseStatus.CACHE_MISS,responseStatus); + + try { + Thread.sleep(1000); + } catch (Exception e) { + + } + // These will be cached + requestException = sendRequest(client, localContext,urlToCall,null); + assertNull(requestException); + + responseStatus = (CacheResponseStatus) localContext.getAttribute(HttpCacheContext.CACHE_RESPONSE_STATUS); + assertEquals(CacheResponseStatus.CACHE_HIT,responseStatus); + + requestException = sendRequest(client, localContext,urlToCall,null); + assertNull(requestException); + + responseStatus = (CacheResponseStatus) localContext.getAttribute(HttpCacheContext.CACHE_RESPONSE_STATUS); + assertEquals(CacheResponseStatus.CACHE_HIT,responseStatus); + + // wait, so that max-age is expired + try { + Thread.sleep(4000); + } catch (Exception e) { + + } + + // This will cause a revalidation to occur + requestException = sendRequest(client, localContext,urlToCall,"This is new content that is bigger than cache limit"); + assertNull(requestException); + + responseStatus = (CacheResponseStatus) localContext.getAttribute(HttpCacheContext.CACHE_RESPONSE_STATUS); + assertEquals(CacheResponseStatus.CACHE_HIT,responseStatus); + + try { + Thread.sleep(1000); + } catch (Exception e) { + + } + + // fetch a different content This will hang due to connection leak in revalidation + requestException = sendRequest(client, localContext,urlToCall+url2,null); + if(requestException!=null) { + requestException.printStackTrace(); + } + assertNull(requestException); + + + } + + static Exception sendRequest(final HttpClient cachingClient, final HttpContext localContext , final String url, final String content) { + final HttpGet httpget = new HttpGet(url); + if(content!=null) { + httpget.setHeader(cacheHandler.getUserContentHeader(),content); + } + + HttpResponse response = null; + try { + response = cachingClient.execute(httpget, localContext); + return null; + } catch (ClientProtocolException e1) { + return e1; + } catch (IOException e1) { + return e1; + } finally { + if(response!=null) { + final HttpEntity entity = response.getEntity(); + try { + EntityUtils.consume(entity); + } catch (IOException e) { + e.printStackTrace(); + } + } + } + } + + public static class EchoViaHeaderHandler + implements HttpRequestHandler { + + private final String CACHE_CONTROL_HEADER = "Cache-Control"; + + private final byte[] DEFAULT_CONTENT; + private final String DEFAULT_CLIENT_CONTROLLED_CONTENT_HEADER; + private final String DEFAULT_RESPONSE_CACHE_HEADER; + + // public default constructor + public EchoViaHeaderHandler() { + this("ECHO-CONTENT","abc".getBytes(), "public, max-age=3, stale-while-revalidate=5"); + } + + public EchoViaHeaderHandler(final String contentHeader,final byte[] content, + final String defaultResponseCacheHeader) { + DEFAULT_CLIENT_CONTROLLED_CONTENT_HEADER = contentHeader; + DEFAULT_CONTENT = content; + DEFAULT_RESPONSE_CACHE_HEADER = defaultResponseCacheHeader; + } + + + /** + * Return the header the user can set the content that will be returned by the server + * @return + */ + public String getUserContentHeader() { + return DEFAULT_CLIENT_CONTROLLED_CONTENT_HEADER; + } + + /** + * Handles a request by echoing the incoming request entity. + * If there is no request entity, an empty document is returned. + * + * @param request the request + * @param response the response + * @param context the context + * + * @throws org.apache.http.HttpException in case of a problem + * @throws java.io.IOException in case of an IO problem + */ + public void handle(final HttpRequest request, + final HttpResponse response, + final HttpContext context) + throws HttpException, IOException { + + final String method = request.getRequestLine().getMethod().toUpperCase(Locale.ENGLISH); + if (!"GET".equals(method) && + !"POST".equals(method) && + !"PUT".equals(method) + ) { + throw new MethodNotSupportedException + (method + " not supported by " + getClass().getName()); + } + + response.setStatusCode(org.apache.http.HttpStatus.SC_OK); + response.addHeader("Cache-Control",getCacheContent(request)); + final byte[] content = getHeaderContent(request); + final ByteArrayEntity bae = new ByteArrayEntity(content); + response.setHeader("Connection","keep-alive"); + + response.setEntity(bae); + + } // handle + + + public byte[] getHeaderContent(final HttpRequest request) { + final Header contentHeader = request.getFirstHeader(DEFAULT_CLIENT_CONTROLLED_CONTENT_HEADER); + if(contentHeader!=null) { + try { + return contentHeader.getValue().getBytes("UTF-8"); + } catch(UnsupportedEncodingException e) { + return contentHeader.getValue().getBytes(); + } + } else { + return DEFAULT_CONTENT; + } + } + + public String getCacheContent(final HttpRequest request) { + final Header contentHeader = request.getFirstHeader(CACHE_CONTROL_HEADER); + if(contentHeader!=null) { + return contentHeader.getValue(); + } else { + return DEFAULT_RESPONSE_CACHE_HEADER; + } + } + + } // class EchoHandler + + } \ No newline at end of file -- 1.7.12.4 (Apple Git-37)