From d31f043b3cca1a97dc5fb8b81b85c485667c28cf Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 11 Oct 2019 16:17:58 +0200 Subject: Remove dead code --- .../communication/http/ApacheAsyncHttpClient.java | 183 ---------------- .../communication/http/ApacheHttpInstance.java | 133 ------------ .../http/ApacheAsyncHttpClientTest.java | 239 --------------------- 3 files changed, 555 deletions(-) delete mode 100644 clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClient.java delete mode 100644 clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheHttpInstance.java delete mode 100644 clustercontroller-apputil/src/test/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClientTest.java (limited to 'clustercontroller-apputil/src') diff --git a/clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClient.java b/clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClient.java deleted file mode 100644 index 11d746ef3ce..00000000000 --- a/clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClient.java +++ /dev/null @@ -1,183 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.clustercontroller.apputil.communication.http; - -import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperation; -import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperationImpl; -import com.yahoo.vespa.clustercontroller.utils.communication.http.AsyncHttpClient; -import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpRequest; -import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpResult; -import com.yahoo.vespa.clustercontroller.utils.communication.http.SyncHttpClient; - -import java.util.*; -import java.util.concurrent.Executor; -import java.util.logging.Logger; - -/** - * There are some stuff to work around with the apache client. - * - Whether to use a proxy or not is global, not per request. - * - Timeout is not handled per request (and is not a request timeout but seems like a "something happening on TCP" timeout. - * - It is not thread safe. - * - * This class gets around these issues by creating one instance per unique setting, and ensuring only one request use a given instance at a time. - */ -public class ApacheAsyncHttpClient implements AsyncHttpClient { - - private static final Logger log = Logger.getLogger(ApacheAsyncHttpClient.class.getName()); - - public interface SyncHttpClientFactory { - SyncHttpClient createInstance(String proxyHost, int proxyPort, long timeoutMs); - } - - public static class Settings { - String proxyHost; - int proxyPort; - long timeout; - - Settings(HttpRequest request) { - timeout = request.getTimeoutMillis(); - if (request.getPath() != null - && !request.getPath().isEmpty() - && request.getPath().charAt(0) != '/') - { - proxyHost = request.getHost(); - proxyPort = request.getPort(); - int colo = request.getPath().indexOf(':'); - int slash = request.getPath().indexOf('/', colo); - if (colo < 0 && slash < 0) { - throw new IllegalStateException("Http path '" + request.getPath() + "' looks invalid. " - + "Cannot extract proxy server data. Is it a regular request that " - + "should start with a slash?"); - } - if (colo < 0) { - request.setPort(80); - request.setHost(request.getPath().substring(0, slash)); - } else { - request.setHost(request.getPath().substring(0, colo)); - request.setPort(Integer.valueOf(request.getPath().substring(colo + 1, slash))); - } - request.setPath(request.getPath().substring(slash)); - } - } - - @Override - public boolean equals(Object other) { - Settings o = (Settings) other; - if (timeout != o.timeout || proxyPort != o.proxyPort - || (proxyHost == null ^ o.proxyHost == null) - || (proxyHost != null && !proxyHost.equals(o.proxyHost))) - { - return false; - } - return true; - } - - @Override - public int hashCode() { - return (proxyHost == null ? 0 : proxyHost.hashCode()) ^ proxyPort ^ Long.valueOf(timeout).hashCode(); - } - } - private final Executor executor; - private final SyncHttpClientFactory clientFactory; - private boolean closed = false; - private final int maxInstanceCacheSize; // Maximum number of currently unused instances. - private final Map> apacheInstances = new LinkedHashMap>() { - protected @Override boolean removeEldestEntry(Map.Entry> eldest) { - return getUnusedCacheSize() > maxInstanceCacheSize; - } - }; - - public ApacheAsyncHttpClient(Executor executor) { - this(executor, new SyncHttpClientFactory() { - @Override - public SyncHttpClient createInstance(String proxyHost, int proxyPort, long timeoutMs) { - return new ApacheHttpInstance(proxyHost, proxyPort, timeoutMs); - } - }); - } - - public ApacheAsyncHttpClient(Executor executor, SyncHttpClientFactory clientFactory) { - this.executor = executor; - this.clientFactory = clientFactory; - maxInstanceCacheSize = 16; - log.fine("Starting apache com.yahoo.vespa.clustercontroller.utils.communication.async HTTP client"); - } - - private SyncHttpClient getFittingInstance(Settings settings) { - synchronized (apacheInstances) { - if (closed) throw new IllegalStateException("Http client has been closed for business."); - LinkedList fittingInstances = apacheInstances.get(settings); - if (fittingInstances == null) { - fittingInstances = new LinkedList<>(); - apacheInstances.put(settings, fittingInstances); - } - if (fittingInstances.isEmpty()) { - return clientFactory.createInstance(settings.proxyHost, settings.proxyPort, settings.timeout); - } else { - return fittingInstances.removeFirst(); - } - } - } - private void insertInstance(Settings settings, SyncHttpClient instance) { - synchronized (apacheInstances) { - LinkedList fittingInstances = apacheInstances.get(settings); - if (closed || fittingInstances == null) { - instance.close(); - return; - } - fittingInstances.addLast(instance); - } - } - private int getUnusedCacheSize() { - int size = 0; - synchronized (apacheInstances) { - for (LinkedList list : apacheInstances.values()) { - size += list.size(); - } - } - return size; - } - - @Override - public AsyncOperation execute(HttpRequest r) { - final HttpRequest request = r.clone(); // Gonna modify it to extract proxy information - final Settings settings = new Settings(request); - final SyncHttpClient instance = getFittingInstance(settings); - final AsyncOperationImpl op = new AsyncOperationImpl<>(r.toString(), r.toString(true)); - executor.execute(new Runnable() { - @Override - public void run() { - HttpResult result; - Exception failure = null; - try{ - result = instance.execute(request); - } catch (Exception e) { - result = new HttpResult().setHttpCode(500, "Apache client failed to execute request."); - failure = e; - } - insertInstance(settings, instance); - // Must insert back instance before tagging operation complete to ensure a following - // call can reuse same instance - if (failure != null) { - op.setFailure(failure, result); - } else { - op.setResult(result); - } - } - }); - return op; - } - - @Override - public void close() { - synchronized (apacheInstances) { - closed = true; - for (LinkedList list : apacheInstances.values()) { - for (SyncHttpClient instance : list) { - instance.close(); - } - } - apacheInstances.clear(); - } - } - -} diff --git a/clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheHttpInstance.java b/clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheHttpInstance.java deleted file mode 100644 index 3eafd2ae1d5..00000000000 --- a/clustercontroller-apputil/src/main/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheHttpInstance.java +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.clustercontroller.apputil.communication.http; - -import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpRequest; -import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpResult; -import com.yahoo.vespa.clustercontroller.utils.communication.http.SyncHttpClient; -import org.apache.http.Header; -import org.apache.http.HttpEntity; -import org.apache.http.HttpHost; -import org.apache.http.HttpResponse; -import org.apache.http.client.methods.HttpDelete; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.methods.HttpPut; -import org.apache.http.entity.StringEntity; -import org.apache.http.util.EntityUtils; - -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.logging.Logger; - -// NOTE: these are all deprecated: -import org.apache.http.conn.params.ConnRoutePNames; -import org.apache.http.impl.client.DefaultHttpClient; -import org.apache.http.params.BasicHttpParams; -import org.apache.http.params.HttpConnectionParams; -import org.apache.http.params.HttpParams; - -/** - * Synchronous http client using Apache commons. - */ -public class ApacheHttpInstance implements SyncHttpClient { - - private static final Logger log = Logger.getLogger(ApacheHttpInstance.class.getName()); - DefaultHttpClient client; - - public ApacheHttpInstance(String proxyHost, int proxyPort, long timeoutMs) { - if (timeoutMs > Integer.MAX_VALUE) throw new IllegalArgumentException("Cannot handle timeout not contained in an integer"); - HttpParams httpParams = new BasicHttpParams(); - HttpConnectionParams.setConnectionTimeout(httpParams, (int) timeoutMs); - HttpConnectionParams.setSoTimeout(httpParams, (int) timeoutMs); - - if (proxyHost != null && !proxyHost.isEmpty()) { - httpParams.setParameter(ConnRoutePNames.DEFAULT_PROXY, new HttpHost(proxyHost, proxyPort, "http")); - } - - client = new DefaultHttpClient(httpParams); - } - - /** This function is not threadsafe. */ - public HttpResult execute(HttpRequest r) { - HttpRequest.HttpOp op = r.getHttpOperation(); - if (op == null) { - if (r.getPostContent() != null) { - log.fine("Request " + r + " has no HTTP function specified. Assuming POST as post content is set."); - op = HttpRequest.HttpOp.POST; - } else { - log.fine("Request " + r + " has no HTTP function specified. Assuming GET as post content is set."); - op = HttpRequest.HttpOp.GET; - } - } - if (r.getPostContent() != null - && !(op.equals(HttpRequest.HttpOp.POST) || op.equals(HttpRequest.HttpOp.PUT))) - { - throw new IllegalStateException("A " + op + " operation can't have content"); - } - try { - HttpHost target = new HttpHost(r.getHost(), r.getPort(), "http"); - org.apache.http.HttpRequest req = null; - - String path = r.getPath(); - int uriOption = 0; - for (HttpRequest.KeyValuePair option : r.getUrlOptions()) { - path += (++uriOption == 1 ? '?' : '&'); - path += option.getKey() + '=' + option.getValue(); - } - - switch (op) { - case POST: - HttpPost post = new HttpPost(path); - if (r.getPostContent() != null) { - post.setEntity(new StringEntity(r.getPostContent().toString())); - } - req = post; - break; - case GET: - req = new HttpGet(path); - break; - case PUT: - HttpPut put = new HttpPut(path); - put.setEntity(new StringEntity(r.getPostContent().toString())); - req = put; - break; - case DELETE: - req = new HttpDelete(path); - break; - } - - for (HttpRequest.KeyValuePair header : r.getHeaders()) { - req.addHeader(header.getKey(), header.getValue()); - } - - HttpResponse rsp = client.execute(target, req); - HttpEntity entity = rsp.getEntity(); - - HttpResult result = new HttpResult(); - result.setHttpCode(rsp.getStatusLine().getStatusCode(), rsp.getStatusLine().getReasonPhrase()); - - if (entity != null) { - result.setContent(EntityUtils.toString(entity)); - } - for (Header header : rsp.getAllHeaders()) { - result.addHeader(header.getName(), header.getValue()); - } - - return result; - } catch (Exception e) { - HttpResult result = new HttpResult(); - - StringWriter writer = new StringWriter(); - e.printStackTrace(new PrintWriter(writer)); - writer.flush(); - - result.setHttpCode(500, "Got exception " + writer.toString() + " when sending message."); - return result; - } - } - - public void close() { - client.getConnectionManager().shutdown(); - } - -} diff --git a/clustercontroller-apputil/src/test/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClientTest.java b/clustercontroller-apputil/src/test/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClientTest.java deleted file mode 100644 index c3909e50f0e..00000000000 --- a/clustercontroller-apputil/src/test/java/com/yahoo/vespa/clustercontroller/apputil/communication/http/ApacheAsyncHttpClientTest.java +++ /dev/null @@ -1,239 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.clustercontroller.apputil.communication.http; - -import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperation; -import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncUtils; -import com.yahoo.vespa.clustercontroller.utils.communication.http.*; - -import java.util.LinkedList; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.Executor; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -public class ApacheAsyncHttpClientTest { - - public static class Server { - public class Request { - HttpRequest request; - Object result; - String proxyHost; - int proxyPort; - long timeoutMs; - - public Request(HttpRequest r, String proxyHost, int proxyPort, long timeoutMs) { - request = r; - this.proxyHost = proxyHost; - this.proxyPort = proxyPort; - this.timeoutMs = timeoutMs; - } - - public void answer(Object result) { - synchronized (requests) { - this.result = result; - requests.notifyAll(); - } - } - } - public final LinkedList requests = new LinkedList<>(); - - public Request createRequest(HttpRequest r, String proxyHost, int proxyPort, long timeoutMs) { - return new Request(r, proxyHost, proxyPort, timeoutMs); - } - - public Request waitForRequest() { - synchronized (requests) { - while (true) { - if (!requests.isEmpty()) { - return requests.removeFirst(); - } - try{ requests.wait(); } catch (InterruptedException e) {} - } - } - } - } - - private Server server = new Server(); - private int clientCount = 0; - - public class Client implements SyncHttpClient { - String proxyHost; - int proxyPort; - long timeoutMs; - private boolean running = true; - - public Client(String proxyHost, int proxyPort, long timeoutMs) { - this.proxyHost = proxyHost; - this.proxyPort = proxyPort; - this.timeoutMs = timeoutMs; - } - - @Override - public HttpResult execute(HttpRequest r) { - synchronized (server.requests) { - Server.Request pair = server.createRequest(r, proxyHost, proxyPort, timeoutMs); - server.requests.addLast(pair); - server.requests.notifyAll(); - while (running) { - try{ server.requests.wait(); } catch (InterruptedException e) {} - if (pair.result != null) { - if (pair.result instanceof HttpResult) { - return (HttpResult) pair.result; - } else { - throw new RuntimeException((Exception) pair.result); - } - } else { - } - } - } - return new HttpResult().setHttpCode(500, "Shutting down"); - } - - @Override - public void close() { - synchronized (server.requests) { - running = false; - server.requests.notifyAll(); - } - } - } - - public class ClientFactory implements ApacheAsyncHttpClient.SyncHttpClientFactory { - @Override - public SyncHttpClient createInstance(String proxyHost, int proxyPort, long timeoutMs) { - ++clientCount; - return new Client(proxyHost, proxyPort, timeoutMs); - } - } - - private Executor executor; - - @Before - public void setUp() { - executor = new ThreadPoolExecutor(10, 100, 100, TimeUnit.SECONDS, new ArrayBlockingQueue(1000)); - } - - @Test - public void testOneInstancePerUniqueSettings() { - HttpResult result = new HttpResult().setHttpCode(201, "This worked out good"); - ApacheAsyncHttpClient client = new ApacheAsyncHttpClient(executor, new ClientFactory()); - ProxyAsyncHttpClient proxyClient = new ProxyAsyncHttpClient<>(client, "proxy", 8080); - - // A first request - HttpRequest req = new HttpRequest().setHost("www.yahoo.com").setPath("/foo").setTimeout(400); - AsyncOperation op = proxyClient.execute(req); - Server.Request r = server.waitForRequest(); - assertEquals(r.request.toString(true), "proxy", r.proxyHost); - assertEquals(r.request.toString(true), 8080, r.proxyPort); - assertEquals(r.request.toString(true), 80, r.request.getPort()); - assertEquals(r.request.toString(true), "www.yahoo.com", r.request.getHost()); - assertEquals(400, r.request.getTimeoutMillis()); - r.answer(result); - AsyncUtils.waitFor(op); - assertTrue(op.isDone()); - assertTrue(op.isSuccess()); - assertEquals(result, op.getResult()); - assertEquals(1, clientCount); - - // A second request should reuse first instance - op = proxyClient.execute(req.clone()); - r = server.waitForRequest(); - r.answer(result); - AsyncUtils.waitFor(op); - assertEquals(1, clientCount); - - // Altering timeout create a new one - op = proxyClient.execute(req.clone().setTimeout(800)); - r = server.waitForRequest(); - assertEquals(800, r.request.getTimeoutMillis()); - r.answer(result); - AsyncUtils.waitFor(op); - assertEquals(2, clientCount); - - // And altering proxy will create a new one - ProxyAsyncHttpClient proxyClient2 = new ProxyAsyncHttpClient<>(client, "proxy2", 8080); - op = proxyClient2.execute(req.clone()); - r = server.waitForRequest(); - assertEquals(r.request.toString(true), "proxy2", r.proxyHost); - r.answer(result); - AsyncUtils.waitFor(op); - assertEquals(3, clientCount); - - // And the old ones are still cached, even if port now is specified - op = proxyClient.execute(req.clone().setPort(80)); - r = server.waitForRequest(); - assertEquals(r.request.toString(true), "proxy", r.proxyHost); - assertEquals(r.request.toString(true), 8080, r.proxyPort); - assertEquals(r.request.toString(true), 80, r.request.getPort()); - assertEquals(r.request.toString(true), "www.yahoo.com", r.request.getHost()); - assertEquals(400, r.request.getTimeoutMillis()); - r.answer(result); - AsyncUtils.waitFor(op); - assertEquals(3, clientCount); - - client.close(); - } - - @Test - public void testFailingRequest() { - ApacheAsyncHttpClient client = new ApacheAsyncHttpClient(executor, new ClientFactory()); - HttpRequest req = new HttpRequest(); - AsyncOperation op = client.execute(req); - Server.Request r = server.waitForRequest(); - r.answer(new IllegalStateException("Failed to run")); - AsyncUtils.waitFor(op); - assertEquals(false, op.isSuccess()); - assertTrue(op.getCause().getMessage(), op.getCause().getMessage().contains("Failed to run")); - } - - @Test - public void testClose() { - ApacheAsyncHttpClient client = new ApacheAsyncHttpClient(executor, new ClientFactory()); - HttpRequest req = new HttpRequest(); - AsyncOperation op = client.execute(req); - Server.Request r = server.waitForRequest(); - client.close(); - r.answer(new HttpResult()); - AsyncUtils.waitFor(op); - assertEquals(true, op.isSuccess()); - - try{ - client.execute(req); - assertTrue(false); - } catch (IllegalStateException e) { - assertTrue(e.getMessage(), e.getMessage().contains("Http client has been closed")); - } - } - - @Test - public void testInvalidProxyRequest() { - ApacheAsyncHttpClient client = new ApacheAsyncHttpClient(executor, new ClientFactory()); - HttpRequest req = new HttpRequest().setPath("foo"); - try{ - client.execute(req); - assertTrue(false); - } catch (IllegalStateException e) { - assertTrue(e.getMessage(), e.getMessage().contains("looks invalid")); - } - } - - @Test - public void testNothingButGetCoverage() { - // There is never any hash conflict for equals to be false in actual use - HttpRequest r = new HttpRequest(); - new ApacheAsyncHttpClient.Settings(r.clone().setTimeout(15)) - .equals(new ApacheAsyncHttpClient.Settings(r)); - // Only actual container is meant to use this constructor - ApacheAsyncHttpClient client = new ApacheAsyncHttpClient(executor); - client.execute(new HttpRequest()); - client.close(); - } - -} -- cgit v1.2.3