From 776a923c229fab34039d73ab078bc264b0cb0c2f Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Tue, 9 Aug 2016 14:41:37 +0100 Subject: [PATCH 1/9] Adds HttpExecutor interface --- .../util/http/executor/Credentials.java | 31 +++++ .../util/http/executor/HttpConfig.java | 59 +++++++++ .../util/http/executor/HttpExecutor.java | 19 +++ .../util/http/executor/HttpRequest.java | 110 +++++++++++++++++ .../util/http/executor/HttpRequestImpl.java | 71 +++++++++++ .../util/http/executor/HttpResponse.java | 112 ++++++++++++++++++ .../util/http/executor/HttpResponseImpl.java | 61 ++++++++++ 7 files changed, 463 insertions(+) create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java new file mode 100644 index 0000000000..2ca1c8ac5c --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java @@ -0,0 +1,31 @@ +package org.apache.brooklyn.util.http.executor; + +import com.google.common.annotations.Beta; + +@Beta +public class Credentials { + + private Credentials() {} + + public static BasicAuth basic(String username, String password) { + return new BasicAuth(username, password); + } + + public static class BasicAuth { + private final String username; + private final String password; + + protected BasicAuth(String username, String password) { + this.username = username; + this.password = password; + } + + public String username() { + return username; + } + + public String password() { + return password; + } + } +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java new file mode 100644 index 0000000000..d0f536933f --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java @@ -0,0 +1,59 @@ +package org.apache.brooklyn.util.http.executor; + +import com.google.common.annotations.Beta; + +@Beta +public class HttpConfig { + + @Beta + public static class Builder { + private boolean laxRedirect; + private boolean trustAll; + private boolean trustSelfSigned; + + public Builder laxRedirect(boolean val) { + laxRedirect = val; + return this; + } + + public Builder trustAll(boolean val) { + trustAll = val; + return this; + } + + public Builder trustSelfSigned(boolean val) { + trustSelfSigned = val; + return this; + } + + public HttpConfig build() { + return new HttpConfig(this); + } + } + + public static Builder builder() { + return new Builder(); + } + + private final boolean laxRedirect; + private final boolean trustAll; + private final boolean trustSelfSigned; + + protected HttpConfig(Builder builder) { + laxRedirect = builder.laxRedirect; + trustAll = builder.trustAll; + trustSelfSigned = builder.trustSelfSigned; + } + + public boolean laxRedirect() { + return laxRedirect; + } + + public boolean trustAll() { + return trustAll; + } + + public boolean trustSelfSigned() { + return trustSelfSigned; + } +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java new file mode 100644 index 0000000000..96891e4786 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java @@ -0,0 +1,19 @@ +package org.apache.brooklyn.util.http.executor; + +import java.io.IOException; + +/** + * An abstraction for executing HTTP requests, allowing an appropriate implementation to be chosen. + */ +public interface HttpExecutor { + + /** + * Synchronously send the request, and return its response. + * + * To avoid leaking resources callers must close the response's content (or the response itself). + * + * @throws IOException if a problem occurred talking to the server. + * @throws RuntimeException (and subclasses) if an unexpected error occurs creating the request. + */ + HttpResponse execute(HttpRequest request) throws IOException; +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java new file mode 100644 index 0000000000..e11085403e --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java @@ -0,0 +1,110 @@ +package org.apache.brooklyn.util.http.executor; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.net.URI; +import java.util.Map; + +import javax.annotation.Nullable; + +import com.google.common.annotations.Beta; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; + +@Beta +public interface HttpRequest { + + // TODO Should we use InputStream for body (rather than byte[]). That would mean we don't have + // to hold the entire body in-memory, which might be important for really big payloads! + + @Beta + public static class Builder { + protected boolean isHttps; + protected URI uri; + protected String method; + protected byte[] body; + protected Multimap headers = ArrayListMultimap.create(); + protected Credentials.BasicAuth credentials; + protected HttpConfig config; + + public Builder isHttps(boolean val) { + isHttps = val; + return this; + } + + public Builder uri(URI val) { + uri = checkNotNull(val, "uri"); + return this; + } + + public Builder method(String val) { + method = checkNotNull(val, "method"); + return this; + } + + /** + * This val must not be modified after being passed in - the object will be used while executing the request. + */ + public Builder body(@Nullable byte[] val) { + body = val; + return this; + } + + public Builder headers(Multimap val) { + headers.putAll(checkNotNull(val, "headers")); + return this; + } + + public Builder headers(Map val) { + if (checkNotNull(val, "headers").keySet().contains(null)) { + throw new NullPointerException("Headers must not contain null key"); + } + for (Map.Entry entry : val.entrySet()) { + header(entry.getKey(), entry.getValue()); + } + return this; + } + + public Builder header(String key, String val) { + headers.put(checkNotNull(key, "key"), val); + return this; + } + + public Builder credentials(@Nullable Credentials.BasicAuth val) { + credentials = val; + return this; + } + + public Builder config(@Nullable HttpConfig val) { + config = val; + return this; + } + + public HttpRequest build() { + return new HttpRequestImpl(this); + } + } + + boolean isHttps(); + + URI uri(); + + String method(); + + /** + * The payload of the request, or null if no payload (e.g. for GET requests). + */ + @Nullable + byte[] body(); + + Multimap headers(); + + @Nullable + Credentials.BasicAuth credentials(); + + /** + * Additional optional configuration to customize how the call is done. + */ + @Nullable + HttpConfig config(); +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java new file mode 100644 index 0000000000..b3c21d2517 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java @@ -0,0 +1,71 @@ +package org.apache.brooklyn.util.http.executor; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.net.URI; + +import org.apache.brooklyn.util.http.executor.Credentials.BasicAuth; + +import com.google.common.annotations.Beta; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; + +/** + * A (mostly) immutable http request. However, (for efficiency reasons) we do not copy the body. + */ +@Beta +public class HttpRequestImpl implements HttpRequest { + + protected final boolean isHttps; + protected final URI uri; + protected final String method; + protected final byte[] body; + protected final Multimap headers; + protected final Credentials.BasicAuth credentials; + protected final HttpConfig config; + + protected HttpRequestImpl(HttpRequest.Builder builder) { + this.isHttps = builder.isHttps; + this.uri = checkNotNull(builder.uri, "uri"); + this.method = checkNotNull(builder.method, "method"); + this.body = builder.body; + this.headers = Multimaps.unmodifiableMultimap(ArrayListMultimap.create(checkNotNull(builder.headers, "headers"))); + this.credentials = builder.credentials; + this.config = builder.config; + } + + @Override + public boolean isHttps() { + return isHttps; + } + @Override + public URI uri() { + return uri; + } + + @Override + public String method() { + return method; + } + + @Override + public byte[] body() { + return body; + } + + @Override + public Multimap headers() { + return headers; + } + + @Override + public BasicAuth credentials() { + return credentials; + } + + @Override + public HttpConfig config() { + return config; + } +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java new file mode 100644 index 0000000000..18fb5e8e23 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java @@ -0,0 +1,112 @@ +package org.apache.brooklyn.util.http.executor; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.io.ByteArrayInputStream; +import java.io.Closeable; +import java.io.InputStream; +import java.util.Map; + +import javax.annotation.Nullable; + +import com.google.common.annotations.Beta; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; + +/** + * An HTTP response. Instances of this class are not immutable: the response body is a + * one-shot value that may be consumed only once and then closed. All other properties + * are immutable. + * + * This class implements {@link Closeable}. Closing it simply closes its response streams (if any). + */ +@Beta +public interface HttpResponse extends Closeable { + + @Beta + public static class Builder { + protected int code; + protected String reasonPhrase; + protected Multimap headers = ArrayListMultimap.create(); + protected long contentLength = -1; + protected InputStream content; + + public Builder code(int val) { + code = val; + return this; + } + + public Builder reasonPhrase(@Nullable String val) { + reasonPhrase = val; + return this; + } + + public Builder headers(Multimap val) { + headers.putAll(headers); + return this; + } + + public Builder headers(Map val) { + if (checkNotNull(val, "headers").keySet().contains(null)) { + throw new NullPointerException("Headers must not contain null key"); + } + for (Map.Entry entry : val.entrySet()) { + header(entry.getKey(), entry.getValue()); + } + return this; + } + + public Builder header(String key, String val) { + headers.put(key, val); + return this; + } + + public Builder content(@Nullable InputStream val) { + if (val != null) { + contentLength = -1; + content = val; + } + return this; + } + + public Builder content(@Nullable byte[] val) { + if (val != null) { + contentLength = val.length; + content = new ByteArrayInputStream(val); + } + return this; + } + + public HttpResponse build() { + return new HttpResponseImpl(this); + } + } + + /** + * The HTTP status code. + */ + public int code(); + + /** + * The HTTP reason phrase. + */ + @Nullable + public String reasonPhrase(); + + public Multimap headers(); + + /** + * The length of the content, if known. + * + * @return the number of bytes of the content, or a negative number if unknown. If the + * content length is known but exceeds {@link java.lang.Long#MAX_VALUE Long.MAX_VALUE}, + * a negative number is returned. + */ + public long getContentLength(); + + /** + * The response body, or null if no body. + * @return + */ + public InputStream getContent(); +} \ No newline at end of file diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java new file mode 100644 index 0000000000..59c4acd3be --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java @@ -0,0 +1,61 @@ +package org.apache.brooklyn.util.http.executor; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.io.IOException; +import java.io.InputStream; + +import org.apache.brooklyn.util.stream.Streams; + +import com.google.common.annotations.Beta; +import com.google.common.collect.Multimap; + +@Beta +public class HttpResponseImpl implements HttpResponse { + + private final int code; + private final String reasonPhrase; + private final Multimap headers; + private final long contentLength; + private final InputStream content; + + protected HttpResponseImpl(HttpResponse.Builder builder) { + code = builder.code; + reasonPhrase = builder.reasonPhrase; + headers = checkNotNull(builder.headers, "headers"); + contentLength = builder.contentLength; + content = builder.content; + } + + @Override + public void close() throws IOException { + if (content != null) { + Streams.closeQuietly(content); + } + } + + @Override + public int code() { + return code; + } + + @Override + public String reasonPhrase() { + return reasonPhrase; + } + + @Override + public Multimap headers() { + return headers; + } + + @Override + public long getContentLength() { + return contentLength; + } + + @Override + public InputStream getContent() { + return content; + } +} From e785c29e8b7d298f22c0d7d44e993264f4dca0a5 Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Tue, 9 Aug 2016 14:41:58 +0100 Subject: [PATCH 2/9] Adds apache-httpclient impl of HttpExecutor --- .../apache/brooklyn/util/http/HttpTool.java | 70 ++++++++++++-- .../apacheclient/HttpExecutorImpl.java | 88 ++++++++++++++++++ .../apacheclient/HttpResponseWrapper.java | 91 +++++++++++++++++++ 3 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpTool.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpTool.java index 862109f999..719f304f91 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpTool.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpTool.java @@ -32,10 +32,17 @@ import java.util.Collection; import java.util.Map; import java.util.Map.Entry; -import java.util.concurrent.*; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import com.google.common.base.Throwables; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLSession; + import org.apache.brooklyn.util.crypto.SslTrustUtils; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.net.URLParamEncoder; @@ -82,13 +89,10 @@ import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Optional; +import com.google.common.base.Throwables; import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLSession; - /** * A utility tool for HTTP operations. */ @@ -250,6 +254,21 @@ public static class HttpClientBuilder { private boolean trustAll; private boolean trustSelfSigned; + public static HttpClientBuilder fromBuilder(HttpClientBuilder other) { + HttpClientBuilder result = httpClientBuilder(); + result.clientConnectionManager = other.clientConnectionManager; + result.httpParams = other.httpParams; + result.uri = other.uri; + result.port = other.port; + result.credentials = other.credentials; + result.laxRedirect = other.laxRedirect; + result.https = other.https; + result.socketFactory = other.socketFactory; + result.reuseStrategy = other.reuseStrategy; + result.trustAll = other.trustAll; + result.trustSelfSigned = other.trustSelfSigned; + return result; + } public HttpClientBuilder clientConnectionManager(ClientConnectionManager val) { this.clientConnectionManager = checkNotNull(val, "clientConnectionManager"); return this; @@ -311,11 +330,17 @@ public HttpClientBuilder socketFactory(SchemeSocketFactory val) { return this; } public HttpClientBuilder trustAll() { - this.trustAll = true; - return this; + return trustAll(true); } public HttpClientBuilder trustSelfSigned() { - this.trustSelfSigned = true; + return trustSelfSigned(true); + } + public HttpClientBuilder trustAll(boolean val) { + trustAll = val; + return this; + } + public HttpClientBuilder trustSelfSigned(boolean val) { + this.trustSelfSigned = val; return this; } public HttpClient build() { @@ -468,11 +493,26 @@ public static HttpToolResponse httpGet(HttpClient httpClient, URI uri, Map headers) { + HttpGet req = new HttpGetBuilder(uri).headers(headers).build(); + return execAndConsume(httpClient, req); + } + + public static HttpToolResponse httpPost(HttpClient httpClient, URI uri, Multimap headers, byte[] body) { + HttpPost req = new HttpPostBuilder(uri).headers(headers).body(body).build(); + return execAndConsume(httpClient, req); + } + public static HttpToolResponse httpPost(HttpClient httpClient, URI uri, Map headers, byte[] body) { HttpPost req = new HttpPostBuilder(uri).headers(headers).body(body).build(); return execAndConsume(httpClient, req); } + public static HttpToolResponse httpPut(HttpClient httpClient, URI uri, Multimap headers, byte[] body) { + HttpPut req = new HttpPutBuilder(uri).headers(headers).body(body).build(); + return execAndConsume(httpClient, req); + } + public static HttpToolResponse httpPut(HttpClient httpClient, URI uri, Map headers, byte[] body) { HttpPut req = new HttpPutBuilder(uri).headers(headers).body(body).build(); return execAndConsume(httpClient, req); @@ -483,11 +523,21 @@ public static HttpToolResponse httpPost(HttpClient httpClient, URI uri, Map headers) { + public static HttpToolResponse httpDelete(HttpClient httpClient, URI uri, Multimap headers) { HttpDelete req = new HttpDeleteBuilder(uri).headers(headers).build(); return execAndConsume(httpClient, req); } + public static HttpToolResponse httpDelete(HttpClient httpClient, URI uri, Map headers) { + HttpDelete req = new HttpDeleteBuilder(uri).headers(headers).build(); + return execAndConsume(httpClient, req); + } + + public static HttpToolResponse httpHead(HttpClient httpClient, URI uri, Multimap headers) { + HttpHead req = new HttpHeadBuilder(uri).headers(headers).build(); + return execAndConsume(httpClient, req); + } + public static HttpToolResponse httpHead(HttpClient httpClient, URI uri, Map headers) { HttpHead req = new HttpHeadBuilder(uri).headers(headers).build(); return execAndConsume(httpClient, req); diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java new file mode 100644 index 0000000000..03631a0651 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java @@ -0,0 +1,88 @@ +package org.apache.brooklyn.util.http.executor.apacheclient; + +import java.io.IOException; + +import org.apache.brooklyn.util.http.HttpTool; +import org.apache.brooklyn.util.http.HttpTool.HttpClientBuilder; +import org.apache.brooklyn.util.http.HttpToolResponse; +import org.apache.brooklyn.util.http.executor.HttpConfig; +import org.apache.brooklyn.util.http.executor.HttpExecutor; +import org.apache.brooklyn.util.http.executor.HttpRequest; +import org.apache.brooklyn.util.http.executor.HttpResponse; +import org.apache.http.auth.Credentials; +import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.client.HttpClient; + +import com.google.common.annotations.Beta; +import com.google.common.base.Optional; + +@Beta +public class HttpExecutorImpl implements HttpExecutor { + + private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; + + private static final HttpConfig DEFAULT_CONFIG = HttpConfig.builder() + .laxRedirect(false) + .trustAll(false) + .trustSelfSigned(false) + .build(); + + public static HttpExecutorImpl newInstance() { + return newInstance(HttpTool.httpClientBuilder()); + } + + /** + * This {@code baseBuilder} is used to construct a new {@link HttpClient} for each call to {@link #execute(HttpRequest)}. + * Callers are strongly discouraged from modifying the baseBuilder after passing it in! + */ + public static HttpExecutorImpl newInstance(HttpTool.HttpClientBuilder baseBuilder) { + return new HttpExecutorImpl(baseBuilder); + } + + private final HttpClientBuilder baseBuilder; + + protected HttpExecutorImpl(HttpClientBuilder baseBuilder) { + this.baseBuilder = baseBuilder; + } + + @Override + public HttpResponse execute(HttpRequest request) throws IOException { + HttpConfig config = (request.config() != null) ? request.config() : DEFAULT_CONFIG; + Credentials creds = (request.credentials() != null) ? new UsernamePasswordCredentials(request.credentials().username(), request.credentials().password()) : null; + HttpClient httpClient = HttpTool.HttpClientBuilder.fromBuilder(baseBuilder) + .uri(request.uri()) + .https(request.isHttps()) + .credential(Optional.fromNullable(creds)) + .laxRedirect(config.laxRedirect()) + .trustSelfSigned(config.trustSelfSigned()) + .trustAll(config.trustAll()) + .build(); + + HttpToolResponse response; + + switch (request.method().toLowerCase()) { + case "get": + response = HttpTool.httpGet(httpClient, request.uri(), request.headers()); + break; + case "head": + response = HttpTool.httpHead(httpClient, request.uri(), request.headers()); + break; + case "post": + response = HttpTool.httpPost(httpClient, request.uri(), request.headers(), orEmpty(request.body())); + break; + case "put": + response = HttpTool.httpPut(httpClient, request.uri(), request.headers(), orEmpty(request.body())); + break; + case "delete": + response = HttpTool.httpDelete(httpClient, request.uri(), request.headers()); + break; + default: + throw new IllegalArgumentException("Unsupported method '"+request.method()+"' for URI "+request.uri()); + } + return new HttpResponseWrapper(response); + } + + protected byte[] orEmpty(byte[] val) { + return (val != null) ? val : EMPTY_BYTE_ARRAY; + } +} diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java new file mode 100644 index 0000000000..dc15eacc0c --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java @@ -0,0 +1,91 @@ +package org.apache.brooklyn.util.http.executor.apacheclient; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.List; +import java.util.Map; + +import org.apache.brooklyn.util.guava.Maybe; +import org.apache.brooklyn.util.http.HttpToolResponse; +import org.apache.brooklyn.util.http.executor.HttpResponse; +import org.apache.http.HttpEntity; +import org.apache.http.util.EntityUtils; + +import com.google.common.annotations.Beta; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; + +@Beta +public class HttpResponseWrapper implements HttpResponse { + + private final HttpToolResponse delegate; + private transient volatile byte[] content; + private transient volatile Multimap headers; + + public HttpResponseWrapper(HttpToolResponse delegate) { + this.delegate = checkNotNull(delegate, "response"); + } + + @Override + public void close() throws IOException { + Maybe apacheResponse = delegate.getResponse(); + if (apacheResponse.isPresent()) { + HttpEntity entity = apacheResponse.get().getEntity(); + if (entity != null) { + EntityUtils.consumeQuietly(apacheResponse.get().getEntity()); + } + } + } + + @Override + public int code() { + return delegate.getResponseCode(); + } + + @Override + public String reasonPhrase() { + return delegate.getReasonPhrase(); + } + + @Override + public Multimap headers() { + return headersImpl(); + + } + + @Override + public long getContentLength() { + byte[] content = getContentImpl(); + return (content == null) ? -1 : content.length; + } + + @Override + public InputStream getContent() { + byte[] content = getContentImpl(); + return (content == null) ? null : new ByteArrayInputStream(content); + } + + protected byte[] getContentImpl() { + if (content == null) { + content = delegate.getContent(); + } + return content; + } + + protected Multimap headersImpl() { + // The magic number "3" comes from ArrayListMultimap.DEFAULT_VALUES_PER_KEY + if (headers == null) { + Map> headerLists = delegate.getHeaderLists(); + Multimap headers = ArrayListMultimap.create(headerLists.size(), 3); + for (Map.Entry> entry : headerLists.entrySet()) { + headers.putAll(entry.getKey(), entry.getValue()); + } + this.headers = Multimaps.unmodifiableMultimap(headers); + } + return headers; + } +} From 5faa686bfc4752554eaf20facdffbce6d0850673 Mon Sep 17 00:00:00 2001 From: Yavor Yanchev Date: Tue, 16 Aug 2016 09:39:48 +0300 Subject: [PATCH 3/9] Refactoring HttpFeed to use the new HttpExecutor interface. --- .../core/location/AbstractLocation.java | 30 ++++ .../core/location/LocationConfigKeys.java | 5 +- .../location/cloud/CloudLocationConfig.java | 5 + .../apache/brooklyn/feed/http/HttpFeed.java | 136 +++++++++++------- .../location/byon/ByonLocationResolver.java | 2 + .../http/executor/HttpExecutorFactory.java | 29 ++++ .../executor/HttpExecutorFactoryImpl.java | 75 ++++++++++ .../brooklyn/feed/http/HttpFeedTest.java | 16 ++- .../util/http/executor/Credentials.java | 18 +++ .../util/http/executor/HttpConfig.java | 18 +++ .../util/http/executor/HttpExecutor.java | 18 +++ .../util/http/executor/HttpRequest.java | 18 +++ .../util/http/executor/HttpRequestImpl.java | 18 +++ .../util/http/executor/HttpResponse.java | 18 +++ .../util/http/executor/HttpResponseImpl.java | 18 +++ .../apacheclient/HttpExecutorImpl.java | 26 ++++ .../apacheclient/HttpResponseWrapper.java | 18 +++ 17 files changed, 411 insertions(+), 57 deletions(-) create mode 100644 core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java create mode 100644 core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java index 1ca8c2fbe7..fb41282406 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java @@ -65,6 +65,7 @@ import org.apache.brooklyn.core.objs.AbstractBrooklynObject; import org.apache.brooklyn.core.objs.AbstractConfigurationSupportInternal; import org.apache.brooklyn.util.collections.SetFromLiveMap; +import org.apache.brooklyn.util.core.ClassLoaderUtils; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.TypeCoercions; @@ -718,6 +719,35 @@ public boolean removeChild(Location child) { return removed; } + public void init() { + super.init(); + loadExtension(); + } + + public void rebind() { + super.rebind(); + loadExtension(); + } + + private void loadExtension() { + try { + Map extensions = getConfig(LocationConfigKeys.EXTENSIONS); + if (extensions != null) { + for (Map.Entry extension: extensions.entrySet()) { + Class extensionClassType = new ClassLoaderUtils(this, getManagementContext()).loadClass(extension.getKey()); + + if (!hasExtension(extensionClassType)) { + Object extensionClass = new ClassLoaderUtils(this, getManagementContext()).loadClass(extension.getValue()).newInstance(); + addExtension((Class)extensionClassType, extensionClass); + } + } + } + } catch (Exception e) { + LOG.error("Location extension can not be loaded {}", e); + throw Exceptions.propagate(e); + } + } + protected void onChanged() { // currently changes simply trigger re-persistence; there is no intermediate listener as we do for EntityChangeListener if (isManaged()) { diff --git a/core/src/main/java/org/apache/brooklyn/core/location/LocationConfigKeys.java b/core/src/main/java/org/apache/brooklyn/core/location/LocationConfigKeys.java index e8e8db6d74..715f39a5f4 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/LocationConfigKeys.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/LocationConfigKeys.java @@ -24,6 +24,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.MapConfigKey; import org.apache.brooklyn.util.os.Os; import com.google.common.base.CaseFormat; @@ -35,7 +36,9 @@ public class LocationConfigKeys { public static final ConfigKey DISPLAY_NAME = ConfigKeys.newStringConfigKey("displayName"); public static final ConfigKey ENABLED = ConfigKeys.newBooleanConfigKey("enabled", "Whether the location is enabled for listing and use " + "(only supported for selected locations)", true); - + + public static final MapConfigKey EXTENSIONS = new MapConfigKey(String.class, "extensions", "Location extensions"); + public static final ConfigKey ACCESS_IDENTITY = ConfigKeys.newStringConfigKey("identity"); public static final ConfigKey ACCESS_CREDENTIAL = ConfigKeys.newStringConfigKey("credential"); diff --git a/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java b/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java index f749f64490..77d8e06840 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/cloud/CloudLocationConfig.java @@ -27,6 +27,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.MapConfigKey; import org.apache.brooklyn.core.location.LocationConfigKeys; import org.apache.brooklyn.util.core.flags.SetFromFlag; @@ -36,6 +37,10 @@ public interface CloudLocationConfig { public static final ConfigKey CLOUD_REGION_ID = LocationConfigKeys.CLOUD_REGION_ID; public static final ConfigKey CLOUD_AVAILABILITY_ZONE_ID = LocationConfigKeys.CLOUD_AVAILABILITY_ZONE_ID; + + @SetFromFlag("extensions") + public static final MapConfigKey EXTENSION = LocationConfigKeys.EXTENSIONS; + @SetFromFlag("identity") public static final ConfigKey ACCESS_IDENTITY = LocationConfigKeys.ACCESS_IDENTITY; @SetFromFlag("credential") diff --git a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java index d3556be641..b0d4383238 100644 --- a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java +++ b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java @@ -20,8 +20,10 @@ import static com.google.common.base.Preconditions.checkNotNull; +import java.io.IOException; import java.net.URI; import java.net.URL; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,6 +31,7 @@ import java.util.concurrent.TimeUnit; import org.apache.brooklyn.api.entity.EntityLocal; +import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.Entities; @@ -36,13 +39,20 @@ import org.apache.brooklyn.core.feed.AttributePollHandler; import org.apache.brooklyn.core.feed.DelegatingPollHandler; import org.apache.brooklyn.core.feed.Poller; -import org.apache.brooklyn.util.http.HttpTool; +import org.apache.brooklyn.core.location.Locations; +import org.apache.brooklyn.core.location.Machines; +import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.http.HttpToolResponse; -import org.apache.brooklyn.util.http.HttpTool.HttpClientBuilder; +import org.apache.brooklyn.util.http.executor.Credentials.BasicAuth; +import org.apache.brooklyn.util.http.executor.HttpConfig; +import org.apache.brooklyn.util.http.executor.HttpExecutor; +import org.apache.brooklyn.util.http.executor.HttpExecutorFactory; +import org.apache.brooklyn.util.http.executor.HttpRequest; +import org.apache.brooklyn.util.http.executor.HttpResponse; +import org.apache.brooklyn.util.http.executor.apacheclient.HttpExecutorImpl; import org.apache.brooklyn.util.time.Duration; import org.apache.http.auth.Credentials; import org.apache.http.auth.UsernamePasswordCredentials; -import org.apache.http.client.HttpClient; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -56,6 +66,7 @@ import com.google.common.collect.Maps; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; +import com.google.common.io.ByteStreams; import com.google.common.reflect.TypeToken; /** @@ -127,6 +138,7 @@ public static class Builder { private boolean suspended = false; private Credentials credentials; private String uniqueTag; + private HttpExecutor httpExecutor; private volatile boolean built; public Builder entity(EntityLocal val) { @@ -207,6 +219,10 @@ public Builder uniqueTag(String uniqueTag) { this.uniqueTag = uniqueTag; return this; } + public Builder httpExecutor(HttpExecutor val) { + this.httpExecutor = val; + return this; + } public HttpFeed build() { built = true; HttpFeed result = new HttpFeed(this); @@ -222,6 +238,7 @@ protected void finalize() { } private static class HttpPollIdentifier { + final HttpExecutor httpExecutor; final String method; final Supplier uriProvider; final Map headers; @@ -229,8 +246,9 @@ private static class HttpPollIdentifier { final Optional credentials; final Duration connectionTimeout; final Duration socketTimeout; - private HttpPollIdentifier(String method, Supplier uriProvider, Map headers, byte[] body, - Optional credentials, Duration connectionTimeout, Duration socketTimeout) { + private HttpPollIdentifier(HttpExecutor httpExecutor, String method, Supplier uriProvider, Map headers, + byte[] body, Optional credentials, Duration connectionTimeout, Duration socketTimeout) { + this.httpExecutor = httpExecutor; this.method = checkNotNull(method, "method").toLowerCase(); this.uriProvider = checkNotNull(uriProvider, "uriProvider"); this.headers = checkNotNull(headers, "headers"); @@ -262,6 +280,7 @@ public boolean equals(Object other) { Objects.equal(uriProvider, o.uriProvider) && Objects.equal(headers, o.headers) && Objects.equal(body, o.body) && + Objects.equal(httpExecutor, o.httpExecutor) && Objects.equal(credentials, o.credentials); } } @@ -275,7 +294,23 @@ public HttpFeed() { protected HttpFeed(Builder builder) { setConfig(ONLY_IF_SERVICE_UP, builder.onlyIfServiceUp); Map baseHeaders = ImmutableMap.copyOf(checkNotNull(builder.headers, "headers")); - + + HttpExecutor httpExecutor; + if (builder.httpExecutor != null) { + httpExecutor = builder.httpExecutor; + } else { + HttpExecutorFactory httpExecutorFactory = null; + Collection locations = Locations.getLocationsCheckingAncestors(builder.entity.getLocations(), builder.entity); + Maybe location = Machines.findUniqueElement(locations, Location.class); + if (location.isPresent() && location.get().hasExtension(HttpExecutorFactory.class)) { + httpExecutorFactory = location.get().getExtension(HttpExecutorFactory.class); + Map httpExecutorProps = location.get().getAllConfig(true); + httpExecutor = httpExecutorFactory.getHttpExecutor(httpExecutorProps); + } else { + httpExecutor = HttpExecutorImpl.newInstance(); + } + } + SetMultimap> polls = HashMultimap.>create(); for (HttpPollConfig config : builder.polls) { if (!config.isEnabled()) continue; @@ -302,7 +337,7 @@ protected HttpFeed(Builder builder) { } checkNotNull(baseUriProvider); - polls.put(new HttpPollIdentifier(method, baseUriProvider, headers, body, credentials, connectionTimeout, socketTimeout), configCopy); + polls.put(new HttpPollIdentifier(httpExecutor, method, baseUriProvider, headers, body, credentials, connectionTimeout, socketTimeout), configCopy); } setConfig(POLLS, polls); initUniqueTag(builder.uniqueTag, polls.values()); @@ -311,7 +346,7 @@ protected HttpFeed(Builder builder) { @Override protected void preStart() { SetMultimap> polls = getConfig(POLLS); - + for (final HttpPollIdentifier pollInfo : polls.keySet()) { // Though HttpClients are thread safe and can take advantage of connection pooling // and authentication caching, the httpcomponents documentation says: @@ -319,7 +354,6 @@ protected void preStart() { // threads of execution, it is highly recommended that each thread maintains its // own dedicated instance of HttpContext. // http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html - final HttpClient httpClient = createHttpClient(pollInfo); Set> configs = polls.get(pollInfo); long minPeriod = Integer.MAX_VALUE; @@ -331,52 +365,54 @@ protected void preStart() { } Callable pollJob; - - if (pollInfo.method.equals("get")) { - pollJob = new Callable() { - public HttpToolResponse call() throws Exception { - if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo); - return HttpTool.httpGet(httpClient, pollInfo.uriProvider.get(), pollInfo.headers); - }}; - } else if (pollInfo.method.equals("post")) { - pollJob = new Callable() { - public HttpToolResponse call() throws Exception { - if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo); - return HttpTool.httpPost(httpClient, pollInfo.uriProvider.get(), pollInfo.headers, pollInfo.body); - }}; - } else if (pollInfo.method.equals("head")) { - pollJob = new Callable() { - public HttpToolResponse call() throws Exception { - if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo); - return HttpTool.httpHead(httpClient, pollInfo.uriProvider.get(), pollInfo.headers); - }}; - } else { - throw new IllegalStateException("Unexpected http method: "+pollInfo.method); - } - - getPoller().scheduleAtFixedRate(pollJob, new DelegatingPollHandler(handlers), minPeriod); - } - } + pollJob = new Callable() { + public HttpToolResponse call() throws Exception { + if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo); - // TODO Should we really trustAll for https? Make configurable? - private HttpClient createHttpClient(HttpPollIdentifier pollIdentifier) { - URI uri = pollIdentifier.uriProvider.get(); - HttpClientBuilder builder = HttpTool.httpClientBuilder() - .trustAll() - .laxRedirect(true); - if (uri != null) builder.uri(uri); - if (uri != null) builder.credential(pollIdentifier.credentials); - if (pollIdentifier.connectionTimeout != null) { - builder.connectionTimeout(pollIdentifier.connectionTimeout); - } - if (pollIdentifier.socketTimeout != null) { - builder.socketTimeout(pollIdentifier.socketTimeout); + BasicAuth creds = null; + if (pollInfo.credentials.isPresent()) { + creds = org.apache.brooklyn.util.http.executor.Credentials.basic( + pollInfo.credentials.get().getUserPrincipal().getName(), + pollInfo.credentials.get().getPassword()); + } + + HttpResponse response = pollInfo.httpExecutor.execute(new HttpRequest.Builder() + .headers(pollInfo.headers) + .uri(pollInfo.uriProvider.get()) + .credentials(creds) + .method(pollInfo.method) + .body(pollInfo.body) + .config(HttpConfig.builder() + .trustSelfSigned(true) + .trustAll(true) + .laxRedirect(true) + .build()) + .build()); + return createHttpToolRespose(response); + }}; + getPoller().scheduleAtFixedRate(pollJob, new DelegatingPollHandler(handlers), minPeriod); } - return builder.build(); } @SuppressWarnings("unchecked") protected Poller getPoller() { - return (Poller) super.getPoller(); + return (Poller) super.getPoller(); + } + + @SuppressWarnings("unchecked") + private HttpToolResponse createHttpToolRespose(HttpResponse response) throws IOException { + int responseCode = response.code(); + if (responseCode == 400) { // Unprocessable Entity - https://stackoverflow.com/questions/6123425/rest-response-code-for-invalid-data + throw new IOException(" Unprocessable Entity: " + response.reasonPhrase()); + } + Map> headers = (Map>) (Map) response.headers().asMap(); + + return new HttpToolResponse(responseCode, + headers, + ByteStreams.toByteArray(response.getContent()), + System.currentTimeMillis(), + 0, + 0); } } + diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java index ca478fd872..05824dd02b 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.core.location.AbstractLocationResolver; +import org.apache.brooklyn.core.location.LocationConfigKeys; import org.apache.brooklyn.core.mgmt.internal.LocalLocationManager; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; @@ -108,6 +109,7 @@ protected ConfigBag extractConfig(Map locationFlags, String spec, LocationR MutableMap defaultProps = MutableMap.of(); defaultProps.addIfNotNull("user", user); defaultProps.addIfNotNull("port", port); + defaultProps.addIfNotNull(LocationConfigKeys.EXTENSIONS.getName(), config.get(LocationConfigKeys.EXTENSIONS)); List hostAddresses; diff --git a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java new file mode 100644 index 0000000000..b6489f4010 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java @@ -0,0 +1,29 @@ +/* + * 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. + */ +package org.apache.brooklyn.util.http.executor; + +import java.util.Map; + +public interface HttpExecutorFactory { + public static final String HTTP_EXECUTOR_CLASS = "httpExecutorClass"; + + public static final String HTTP_EXECUTOR_CLASS_CONFIG_PREFIX = "httpExecutorClass."; + + HttpExecutor getHttpExecutor(Map props); +} diff --git a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java new file mode 100644 index 0000000000..1d392e172a --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java @@ -0,0 +1,75 @@ +/* + * 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. + */ +package org.apache.brooklyn.util.http.executor; + +import java.util.Map; +import java.util.Map.Entry; + +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.core.ClassLoaderUtils; +import org.apache.brooklyn.util.core.flags.TypeCoercions; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.http.executor.apacheclient.HttpExecutorImpl; +import org.apache.brooklyn.util.text.StringPredicates; +import org.apache.brooklyn.util.text.Strings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Maps; + +public class HttpExecutorFactoryImpl implements HttpExecutorFactory { + private static final Logger LOG = LoggerFactory.getLogger(HttpExecutorFactoryImpl.class); + + public HttpExecutorFactoryImpl() { + // no-op + } + + @Override + public HttpExecutor getHttpExecutor(Map props) { + HttpExecutor httpExecutor; + + String httpExecutorClass = (String) props.get(HTTP_EXECUTOR_CLASS); + if (httpExecutorClass != null) { + Map executorProps = Maps.filterKeys(props, StringPredicates.isStringStartingWith(HTTP_EXECUTOR_CLASS_CONFIG_PREFIX)); + if (executorProps.size() > 0) { + Map httpExecutorProps = MutableMap.of(); + for (Entry entry: executorProps.entrySet()) { + String keyName = Strings.removeFromStart((String)entry.getKey(), HTTP_EXECUTOR_CLASS_CONFIG_PREFIX); + httpExecutorProps.put(keyName, TypeCoercions.coerce(entry.getValue(), String.class)); + } + + try { + httpExecutor = (HttpExecutor) new ClassLoaderUtils(HttpExecutorFactoryImpl.class).loadClass(httpExecutorClass).getConstructor(Map.class).newInstance(httpExecutorProps); + } catch (Exception e) { + throw Exceptions.propagate(e); + } + } else { + LOG.error("Missing parameters for: " + HTTP_EXECUTOR_CLASS); + throw Exceptions.propagate(new IllegalArgumentException("Missing parameters for: " + HTTP_EXECUTOR_CLASS)); + } + } else { + LOG.warn(HTTP_EXECUTOR_CLASS + " parameter not provided. Using the default implementation " + HttpExecutorImpl.class.getName()); + httpExecutor = HttpExecutorImpl.newInstance(); + } + + return httpExecutor; + } +} + + diff --git a/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java b/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java index 63c3c5b9ee..1ecef40e0f 100644 --- a/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java +++ b/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java @@ -71,12 +71,12 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { private static final long TIMEOUT_MS = 10*1000; - private BetterMockWebServer server; - private URL baseUrl; + protected BetterMockWebServer server; + protected URL baseUrl; - private Location loc; - private EntityLocal entity; - private HttpFeed feed; + protected Location loc; + protected EntityLocal entity; + protected HttpFeed feed; @BeforeMethod(alwaysRun=true) @Override @@ -89,11 +89,15 @@ public void setUp() throws Exception { server.play(); baseUrl = server.getUrl("/"); - loc = app.newLocalhostProvisioningLocation(); + loc = newLocation(); entity = app.createAndManageChild(EntitySpec.create(TestEntity.class)); app.start(ImmutableList.of(loc)); } + protected Location newLocation() { + return app.newLocalhostProvisioningLocation(); + } + @AfterMethod(alwaysRun=true) @Override public void tearDown() throws Exception { diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java index 2ca1c8ac5c..f46045d59c 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import com.google.common.annotations.Beta; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java index d0f536933f..e9ac8eb67a 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import com.google.common.annotations.Beta; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java index 96891e4786..26d8b1a519 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import java.io.IOException; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java index e11085403e..d7b6539c70 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import static com.google.common.base.Preconditions.checkNotNull; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java index b3c21d2517..c2e19e9fcf 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import static com.google.common.base.Preconditions.checkNotNull; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java index 18fb5e8e23..c3baaf62bc 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponse.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import static com.google.common.base.Preconditions.checkNotNull; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java index 59c4acd3be..a28aa059df 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor; import static com.google.common.base.Preconditions.checkNotNull; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java index 03631a0651..e9738430f7 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java @@ -1,6 +1,25 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor.apacheclient; import java.io.IOException; +import java.util.Map; import org.apache.brooklyn.util.http.HttpTool; import org.apache.brooklyn.util.http.HttpTool.HttpClientBuilder; @@ -40,6 +59,13 @@ public static HttpExecutorImpl newInstance(HttpTool.HttpClientBuilder baseBuilde } private final HttpClientBuilder baseBuilder; + + /** + * A must have constructor. + */ + public HttpExecutorImpl(Map props) { + this(HttpTool.httpClientBuilder()); + } protected HttpExecutorImpl(HttpClientBuilder baseBuilder) { this.baseBuilder = baseBuilder; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java index dc15eacc0c..186f65e84f 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpResponseWrapper.java @@ -1,3 +1,21 @@ +/* + * 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. + */ package org.apache.brooklyn.util.http.executor.apacheclient; import static com.google.common.base.Preconditions.checkNotNull; From 3a4f870ff2d6bf4087609f343754dc436030e789 Mon Sep 17 00:00:00 2001 From: Yavor Yanchev Date: Mon, 29 Aug 2016 15:09:15 +0300 Subject: [PATCH 4/9] Improving error log handling --- .../org/apache/brooklyn/core/location/AbstractLocation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java index fb41282406..fa510cad84 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java @@ -743,7 +743,7 @@ private void loadExtension() { } } } catch (Exception e) { - LOG.error("Location extension can not be loaded {}", e); + LOG.error("Location extension can not be loaded (rethrowing): {}", new Object[] {e}); throw Exceptions.propagate(e); } } From acfd95625da146da267435cde8a70e41f6ee8242 Mon Sep 17 00:00:00 2001 From: Valentin Aitken Date: Thu, 1 Sep 2016 14:39:37 +0300 Subject: [PATCH 5/9] No https --- .../util/http/executor/HttpExecutorFactoryImpl.java | 2 +- .../apache/brooklyn/util/http/executor/HttpRequest.java | 8 -------- .../brooklyn/util/http/executor/HttpRequestImpl.java | 6 ------ .../util/http/executor/apacheclient/HttpExecutorImpl.java | 1 - 4 files changed, 1 insertion(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java index 1d392e172a..e9b769a7e7 100644 --- a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java +++ b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java @@ -55,7 +55,7 @@ public HttpExecutor getHttpExecutor(Map props) { } try { - httpExecutor = (HttpExecutor) new ClassLoaderUtils(HttpExecutorFactoryImpl.class).loadClass(httpExecutorClass).getConstructor(Map.class).newInstance(httpExecutorProps); + httpExecutor = (HttpExecutor) new ClassLoaderUtils(getClass()).loadClass(httpExecutorClass).getConstructor(Map.class).newInstance(httpExecutorProps); } catch (Exception e) { throw Exceptions.propagate(e); } diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java index d7b6539c70..4659766c14 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java @@ -37,7 +37,6 @@ public interface HttpRequest { @Beta public static class Builder { - protected boolean isHttps; protected URI uri; protected String method; protected byte[] body; @@ -45,11 +44,6 @@ public static class Builder { protected Credentials.BasicAuth credentials; protected HttpConfig config; - public Builder isHttps(boolean val) { - isHttps = val; - return this; - } - public Builder uri(URI val) { uri = checkNotNull(val, "uri"); return this; @@ -103,8 +97,6 @@ public HttpRequest build() { } } - boolean isHttps(); - URI uri(); String method(); diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java index c2e19e9fcf..0c76da802a 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java @@ -35,7 +35,6 @@ @Beta public class HttpRequestImpl implements HttpRequest { - protected final boolean isHttps; protected final URI uri; protected final String method; protected final byte[] body; @@ -44,7 +43,6 @@ public class HttpRequestImpl implements HttpRequest { protected final HttpConfig config; protected HttpRequestImpl(HttpRequest.Builder builder) { - this.isHttps = builder.isHttps; this.uri = checkNotNull(builder.uri, "uri"); this.method = checkNotNull(builder.method, "method"); this.body = builder.body; @@ -53,10 +51,6 @@ protected HttpRequestImpl(HttpRequest.Builder builder) { this.config = builder.config; } - @Override - public boolean isHttps() { - return isHttps; - } @Override public URI uri() { return uri; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java index e9738430f7..b2d2ada1a2 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java @@ -77,7 +77,6 @@ public HttpResponse execute(HttpRequest request) throws IOException { Credentials creds = (request.credentials() != null) ? new UsernamePasswordCredentials(request.credentials().username(), request.credentials().password()) : null; HttpClient httpClient = HttpTool.HttpClientBuilder.fromBuilder(baseBuilder) .uri(request.uri()) - .https(request.isHttps()) .credential(Optional.fromNullable(creds)) .laxRedirect(config.laxRedirect()) .trustSelfSigned(config.trustSelfSigned()) From a22d622b468f98b8c18465289c2a87dc1a2db724 Mon Sep 17 00:00:00 2001 From: Yavor Yanchev Date: Fri, 2 Sep 2016 19:49:55 +0300 Subject: [PATCH 6/9] Adding HttpExecutorImplTest --- .../http/executor/HttpExecutorImplTest.java | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java diff --git a/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java b/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java new file mode 100644 index 0000000000..148dff0da2 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java @@ -0,0 +1,140 @@ +/* + * 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. + */ +package org.apache.brooklyn.util.http.executor; + +import static org.testng.AssertJUnit.assertTrue; +import static org.testng.Assert.assertEquals; + +import java.net.URL; +import java.util.Collection; +import java.util.Map; + +import org.apache.brooklyn.util.core.http.BetterMockWebServer; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableMap; +import com.google.common.io.ByteStreams; +import com.google.mockwebserver.MockResponse; +import com.google.mockwebserver.RecordedRequest; + +public class HttpExecutorImplTest { + protected BetterMockWebServer server; + protected URL baseUrl; + protected HttpExecutorFactoryImpl factory; + + protected String HTTP_HEADER_KEY = "content-type"; + protected String HTTP_HEADER_VALUE = "application/json"; + protected String HTTP_BODY = "{\"foo\":\"myfoo\"}"; + + @BeforeMethod(alwaysRun=true) + public void setUp() throws Exception { + factory = new HttpExecutorFactoryImpl(); + server = BetterMockWebServer.newInstanceLocalhost(); + server.play(); + baseUrl = server.getUrl("/"); + } + + @AfterMethod + public void afterMethod() throws Exception { + if (server != null) server.shutdown(); + } + + + @Test + public void testHttpRequest() throws Exception { + server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + HttpExecutor executor = factory.getHttpExecutor(getProps()); + HttpResponse response = executor.execute(new HttpRequest.Builder() + .method("get") + .uri(baseUrl.toURI()) + .build()); + assertEquals(response.code(), 200); + } + + @Test + public void testHttpHeader() throws Exception { + server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + HttpExecutor executor = factory.getHttpExecutor(getProps()); + HttpResponse response = executor.execute(new HttpRequest.Builder() + .method("get") + .uri(baseUrl.toURI()) + .build()); + Map> headers = response.headers().asMap(); + + assertTrue(headers.get(HTTP_HEADER_KEY).contains(HTTP_HEADER_VALUE)); + } + @Test + public void testHttpBody() throws Exception { + server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + HttpExecutor executor = factory.getHttpExecutor(getProps()); + HttpResponse response = executor.execute(new HttpRequest.Builder() + .method("post") + .body(HTTP_BODY.getBytes()) + .uri(baseUrl.toURI()) + .build()); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getBody(), ByteStreams.toByteArray(response.getContent())); + } + @Test + public void testHttpPostRequest() throws Exception { + server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + HttpExecutor executor = factory.getHttpExecutor(getProps()); + @SuppressWarnings("unused") + HttpResponse response = executor.execute(new HttpRequest.Builder() + .method("post") + .body(HTTP_BODY.getBytes()) + .uri(baseUrl.toURI()) + .build()); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getPath(), baseUrl.getPath()); + assertEquals(request.getMethod(), "POST"); + assertEquals(new String(request.getBody()), HTTP_BODY); + } + @Test + public void testHttpPutRequest() throws Exception { + server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + HttpExecutor executor = factory.getHttpExecutor(getProps()); + @SuppressWarnings("unused") + HttpResponse response = executor.execute(new HttpRequest.Builder() + .method("put") + .uri(baseUrl.toURI()) + .build()); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getMethod(), "PUT"); + } + @Test + public void testHttpDeleteRequest() throws Exception { + server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + HttpExecutor executor = factory.getHttpExecutor(getProps()); + @SuppressWarnings("unused") + HttpResponse response = executor.execute(new HttpRequest.Builder() + .method("delete") + .uri(baseUrl.toURI()) + .build()); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getMethod(), "DELETE"); + } + + protected Map getProps() { + return ImmutableMap.of(); + } +} + From 2903a55ad3405993158ce66fcf0d81dbe379040f Mon Sep 17 00:00:00 2001 From: Yavor Yanchev Date: Fri, 2 Sep 2016 20:40:39 +0300 Subject: [PATCH 7/9] Addressing PR comments --- .../org/apache/brooklyn/feed/http/HttpFeed.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java index b0d4383238..8480b4c7bb 100644 --- a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java +++ b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java @@ -32,6 +32,7 @@ import org.apache.brooklyn.api.entity.EntityLocal; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.api.location.MachineLocation; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.Entities; @@ -301,7 +302,7 @@ protected HttpFeed(Builder builder) { } else { HttpExecutorFactory httpExecutorFactory = null; Collection locations = Locations.getLocationsCheckingAncestors(builder.entity.getLocations(), builder.entity); - Maybe location = Machines.findUniqueElement(locations, Location.class); + Maybe location = Machines.findUniqueElement(locations, MachineLocation.class); if (location.isPresent() && location.get().hasExtension(HttpExecutorFactory.class)) { httpExecutorFactory = location.get().getExtension(HttpExecutorFactory.class); Map httpExecutorProps = location.get().getAllConfig(true); @@ -402,7 +403,15 @@ protected Poller getPoller() { @SuppressWarnings("unchecked") private HttpToolResponse createHttpToolRespose(HttpResponse response) throws IOException { int responseCode = response.code(); - if (responseCode == 400) { // Unprocessable Entity - https://stackoverflow.com/questions/6123425/rest-response-code-for-invalid-data + + /* From https://tools.ietf.org/html/rfc4918#section-11.2 + The 422 (Unprocessable Entity) status code means the server + understands the content type of the request entity (hence a + 415(Unsupported Media Type) status code is inappropriate), and the + syntax of the request entity is correct (thus a 400 (Bad Request) + status code is inappropriate) but was unable to process the contained + instructions. */ + if (responseCode == 422) { throw new IOException(" Unprocessable Entity: " + response.reasonPhrase()); } Map> headers = (Map>) (Map) response.headers().asMap(); From d71602c00e63435337146ed6288bfc4f5aca6e2a Mon Sep 17 00:00:00 2001 From: Valentin Aitken Date: Fri, 2 Sep 2016 22:27:10 +0300 Subject: [PATCH 8/9] Add more assertions in HttpExecutorImplTest --- .../http/executor/HttpExecutorImplTest.java | 180 ++++++++++++------ .../util/http/executor/HttpRequest.java | 18 +- .../util/http/executor/HttpRequestImpl.java | 11 +- 3 files changed, 149 insertions(+), 60 deletions(-) diff --git a/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java b/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java index 148dff0da2..48d5a7f737 100644 --- a/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java @@ -22,10 +22,15 @@ import static org.testng.Assert.assertEquals; import java.net.URL; -import java.util.Collection; import java.util.Map; +import java.util.Random; +import com.google.common.base.Function; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Collections2; import org.apache.brooklyn.util.core.http.BetterMockWebServer; +import org.apache.commons.codec.binary.Base64; +import org.bouncycastle.util.Strings; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -35,13 +40,15 @@ import com.google.mockwebserver.MockResponse; import com.google.mockwebserver.RecordedRequest; +import javax.annotation.Nullable; + public class HttpExecutorImplTest { protected BetterMockWebServer server; protected URL baseUrl; protected HttpExecutorFactoryImpl factory; - protected String HTTP_HEADER_KEY = "content-type"; - protected String HTTP_HEADER_VALUE = "application/json"; + protected String HTTP_RESPONSE_HEADER_KEY = "content-type"; + protected String HTTP_RESPONSE_HEADER_VALUE = "application/json"; protected String HTTP_BODY = "{\"foo\":\"myfoo\"}"; @BeforeMethod(alwaysRun=true) @@ -57,84 +64,147 @@ public void afterMethod() throws Exception { if (server != null) server.shutdown(); } - @Test public void testHttpRequest() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + MockResponse serverResponse = new MockResponse().setResponseCode(200).addHeader(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE).setBody(HTTP_BODY); + server.enqueue(serverResponse); HttpExecutor executor = factory.getHttpExecutor(getProps()); - HttpResponse response = executor.execute(new HttpRequest.Builder() - .method("get") + HttpRequest executorRequest = new HttpRequest.Builder() + .method("GET") .uri(baseUrl.toURI()) - .build()); - assertEquals(response.code(), 200); + .build(); + HttpResponse executorResponse = executor.execute(executorRequest); + assertRequestAndResponse(server.takeRequest(), serverResponse, executorRequest, executorResponse); } @Test - public void testHttpHeader() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + public void testHttpPostRequest() throws Exception { + MockResponse serverResponse = new MockResponse().setResponseCode(200).addHeader(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE).setBody(HTTP_BODY); + server.enqueue(serverResponse); HttpExecutor executor = factory.getHttpExecutor(getProps()); - HttpResponse response = executor.execute(new HttpRequest.Builder() - .method("get") + HttpRequest executorRequest = new HttpRequest.Builder().headers(ImmutableMap.of("RequestHeader", "RequestHeaderValue")) + .method("POST") + .body(HTTP_BODY.getBytes()) .uri(baseUrl.toURI()) - .build()); - Map> headers = response.headers().asMap(); + .build(); + HttpResponse executorResponse = executor.execute(executorRequest); + assertRequestAndResponse(server.takeRequest(), serverResponse, executorRequest, executorResponse); - assertTrue(headers.get(HTTP_HEADER_KEY).contains(HTTP_HEADER_VALUE)); - } - @Test - public void testHttpBody() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); - HttpExecutor executor = factory.getHttpExecutor(getProps()); - HttpResponse response = executor.execute(new HttpRequest.Builder() - .method("post") - .body(HTTP_BODY.getBytes()) + // Big POST request with random bytes + serverResponse = new MockResponse().setResponseCode(200).addHeader(HTTP_RESPONSE_HEADER_KEY + "Test", HTTP_RESPONSE_HEADER_VALUE).setBody(HTTP_BODY); + server.enqueue(serverResponse); + executor = factory.getHttpExecutor(getProps()); + byte[] requestBody = new byte[10 * 1024 * 1024]; + Random r = new Random(); + for (int i = 0; i < requestBody.length; i++) { + requestBody[i] = (byte)(r.nextInt() % 256); + } + executorRequest = new HttpRequest.Builder() + .method("POST") + .body(requestBody) .uri(baseUrl.toURI()) - .build()); - RecordedRequest request = server.takeRequest(); - assertEquals(request.getBody(), ByteStreams.toByteArray(response.getContent())); + .build(); + executorResponse = executor.execute(executorRequest); + assertRequestAndResponse(server.takeRequest(), serverResponse, executorRequest, executorResponse); } + @Test - public void testHttpPostRequest() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + public void testHttpPutRequest() throws Exception { + MockResponse serverResponse = new MockResponse().setResponseCode(200).addHeader(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE).setBody(HTTP_BODY); + server.enqueue(serverResponse); HttpExecutor executor = factory.getHttpExecutor(getProps()); - @SuppressWarnings("unused") - HttpResponse response = executor.execute(new HttpRequest.Builder() - .method("post") - .body(HTTP_BODY.getBytes()) + HttpRequest executorRequest = new HttpRequest.Builder() + .method("PUT") .uri(baseUrl.toURI()) - .build()); - RecordedRequest request = server.takeRequest(); - assertEquals(request.getPath(), baseUrl.getPath()); - assertEquals(request.getMethod(), "POST"); - assertEquals(new String(request.getBody()), HTTP_BODY); + .build(); + HttpResponse executorResponse = executor.execute(executorRequest); + assertRequestAndResponse(server.takeRequest(), serverResponse, executorRequest, executorResponse); } + @Test - public void testHttpPutRequest() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + public void testHttpDeleteRequest() throws Exception { + MockResponse serverResponse = new MockResponse().setResponseCode(200).addHeader(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE).setBody(HTTP_BODY); + server.enqueue(serverResponse); HttpExecutor executor = factory.getHttpExecutor(getProps()); - @SuppressWarnings("unused") - HttpResponse response = executor.execute(new HttpRequest.Builder() - .method("put") + HttpRequest executorRequest = new HttpRequest.Builder() + .method("DELETE") + .uri(baseUrl.toURI()) + .build(); + HttpResponse executorResponse = executor.execute(executorRequest); + assertRequestAndResponse(server.takeRequest(), serverResponse, executorRequest, executorResponse); + + // No Headers returned + serverResponse = new MockResponse().setResponseCode(200).setBody(HTTP_BODY); + server.enqueue(serverResponse); + executor = factory.getHttpExecutor(getProps()); + executorRequest = new HttpRequest.Builder() + .method("DELETE") .uri(baseUrl.toURI()) - .build()); - RecordedRequest request = server.takeRequest(); - assertEquals(request.getMethod(), "PUT"); + .build(); + executorResponse = executor.execute(executorRequest); + assertRequestAndResponse(server.takeRequest(), serverResponse, executorRequest, executorResponse); } + @Test - public void testHttpDeleteRequest() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).addHeader(HTTP_HEADER_KEY + ":" + HTTP_HEADER_VALUE).setBody(HTTP_BODY)); + public void testHttpPasswordRequest() throws Exception { + MockResponse firstServerResponse = new MockResponse().setResponseCode(401).addHeader("WWW-Authenticate", "Basic realm=\"User Visible Realm\"").setBody("Not Authenticated"); + server.enqueue(firstServerResponse); + MockResponse secondServerResponse = new MockResponse().setResponseCode(200).addHeader(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE).setBody(HTTP_BODY); + server.enqueue(secondServerResponse); + final String USER = "brooklyn", + PASSWORD = "apache"; + String authUnencoded = USER + ":" + PASSWORD; + String authEncoded = Base64.encodeBase64String(Strings.toByteArray(authUnencoded)); HttpExecutor executor = factory.getHttpExecutor(getProps()); - @SuppressWarnings("unused") - HttpResponse response = executor.execute(new HttpRequest.Builder() - .method("delete") + HttpRequest executorRequest = new HttpRequest.Builder().headers(ImmutableMap.of("RequestHeader", "RequestHeaderValue")) + .method("GET") .uri(baseUrl.toURI()) - .build()); - RecordedRequest request = server.takeRequest(); - assertEquals(request.getMethod(), "DELETE"); + .credentials(new Credentials.BasicAuth("brooklyn", "apache")) + .build(); + HttpResponse executorResponse = executor.execute(executorRequest); + + RecordedRequest recordedFirstRequest = server.takeRequest(); + RecordedRequest recordedSecondRequest = server.takeRequest(); + + assertRequestAndResponse(recordedFirstRequest, + firstServerResponse, + executorRequest, + new HttpResponse.Builder() + .header("WWW-Authenticate", "Basic realm=\"User Visible Realm\"") + .header("Content-Length", "" + "Not Authenticated".length()) + .content("Not Authenticated".getBytes()) + .build()); + ArrayListMultimap newHeaders = ArrayListMultimap.create(executorRequest.headers()); + newHeaders.put("Authorization", "Basic " + authEncoded); + assertRequestAndResponse(recordedSecondRequest, + secondServerResponse, + new HttpRequest.Builder().from((HttpRequestImpl)executorRequest).headers(newHeaders).build(), + executorResponse); + } + + private void assertRequestAndResponse(RecordedRequest serverRequest, MockResponse serverResponse, HttpRequest executorRequest, HttpResponse executorResponse) throws Exception { + assertEquals(serverRequest.getMethod(), executorRequest.method()); + Function, String> headersMapper = new Function, String>() { + @Nullable + @Override + public String apply(@Nullable Map.Entry input) { + return input.getKey() + ": " + input.getValue(); + } + }; + assertTrue(serverRequest.getHeaders().containsAll(Collections2.transform(executorRequest.headers().entries(), headersMapper))); + assertEquals(serverRequest.getPath(), executorRequest.uri().getPath()); + if (executorRequest.body() != null) { + assertEquals(serverRequest.getBody(), executorRequest.body()); + } else { + assertEquals(serverRequest.getBody(), new byte[0]); + } + + assertEquals(serverResponse.getBody(), ByteStreams.toByteArray(executorResponse.getContent())); + assertTrue(serverResponse.getHeaders().containsAll(Collections2.transform(executorResponse.headers().entries(), headersMapper))); + assertTrue(Collections2.transform(executorResponse.headers().entries(), headersMapper).containsAll(serverResponse.getHeaders())); } protected Map getProps() { return ImmutableMap.of(); } } - diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java index 4659766c14..a2de4b7ac6 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java @@ -34,7 +34,7 @@ public interface HttpRequest { // TODO Should we use InputStream for body (rather than byte[]). That would mean we don't have // to hold the entire body in-memory, which might be important for really big payloads! - + @Beta public static class Builder { protected URI uri; @@ -55,13 +55,13 @@ public Builder method(String val) { } /** - * This val must not be modified after being passed in - the object will be used while executing the request. + * This val must not be modified after being passed in - the object will be used while executing the request. */ public Builder body(@Nullable byte[] val) { body = val; return this; } - + public Builder headers(Multimap val) { headers.putAll(checkNotNull(val, "headers")); return this; @@ -92,11 +92,21 @@ public Builder config(@Nullable HttpConfig val) { return this; } + public Builder from(HttpRequestImpl httpRequest) { + this.uri = checkNotNull(httpRequest.uri, "uri"); + this.method = checkNotNull(httpRequest.method, "method"); + this.body = httpRequest.body; + this.headers = ArrayListMultimap.create(checkNotNull(httpRequest.headers, "headers")); + this.credentials = httpRequest.credentials; + this.config = httpRequest.config; + return this; + } + public HttpRequest build() { return new HttpRequestImpl(this); } } - + URI uri(); String method(); diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java index 0c76da802a..cf10752ad4 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java @@ -50,7 +50,16 @@ protected HttpRequestImpl(HttpRequest.Builder builder) { this.credentials = builder.credentials; this.config = builder.config; } - + + public HttpRequestImpl(HttpRequestImpl httpRequest) { + this.uri = checkNotNull(httpRequest.uri, "uri"); + this.method = checkNotNull(httpRequest.method, "method"); + this.body = httpRequest.body; + this.headers = Multimaps.unmodifiableMultimap(ArrayListMultimap.create(checkNotNull(httpRequest.headers, "headers"))); + this.credentials = httpRequest.credentials; + this.config = httpRequest.config; + } + @Override public URI uri() { return uri; From f9f3b997742835930078a3ccb86fcfe6aa1d0165 Mon Sep 17 00:00:00 2001 From: Yavor Yanchev Date: Sat, 3 Sep 2016 15:12:41 +0300 Subject: [PATCH 9/9] Addressing PR comments --- .../core/location/AbstractLocation.java | 14 +++--- .../apache/brooklyn/feed/http/HttpFeed.java | 46 +++++++++++-------- .../location/byon/ByonLocationResolver.java | 2 - .../http/executor/HttpExecutorFactory.java | 2 +- .../executor/HttpExecutorFactoryImpl.java | 24 ++++------ .../http/executor/HttpExecutorImplTest.java | 4 +- .../util/http/executor/Credentials.java | 27 ++--------- .../util/http/executor/HttpExecutor.java | 14 +++++- .../util/http/executor/HttpRequest.java | 6 +-- .../util/http/executor/HttpRequestImpl.java | 6 +-- .../util/http/executor/HttpResponseImpl.java | 1 + .../util/http/executor/UsernamePassword.java | 41 +++++++++++++++++ .../apacheclient/HttpExecutorImpl.java | 34 +++++--------- 13 files changed, 122 insertions(+), 99 deletions(-) create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/http/executor/UsernamePassword.java diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java index fa510cad84..ba11c0c682 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java @@ -730,21 +730,21 @@ public void rebind() { } private void loadExtension() { - try { - Map extensions = getConfig(LocationConfigKeys.EXTENSIONS); - if (extensions != null) { - for (Map.Entry extension: extensions.entrySet()) { + Map extensions = getConfig(LocationConfigKeys.EXTENSIONS); + if (extensions != null) { + for (Map.Entry extension: extensions.entrySet()) { + try { Class extensionClassType = new ClassLoaderUtils(this, getManagementContext()).loadClass(extension.getKey()); if (!hasExtension(extensionClassType)) { Object extensionClass = new ClassLoaderUtils(this, getManagementContext()).loadClass(extension.getValue()).newInstance(); addExtension((Class)extensionClassType, extensionClass); } + } catch (Exception e) { + LOG.error("Location extension can not be loaded (rethrowing): {} {} {}", new Object[] {extension.getKey(), extension.getValue(), e}); + throw Exceptions.propagate(e); } } - } catch (Exception e) { - LOG.error("Location extension can not be loaded (rethrowing): {}", new Object[] {e}); - throw Exceptions.propagate(e); } } diff --git a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java index 8480b4c7bb..03962045ed 100644 --- a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java +++ b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java @@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.URI; import java.net.URL; @@ -44,13 +45,14 @@ import org.apache.brooklyn.core.location.Machines; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.http.HttpToolResponse; -import org.apache.brooklyn.util.http.executor.Credentials.BasicAuth; +import org.apache.brooklyn.util.http.executor.UsernamePassword; import org.apache.brooklyn.util.http.executor.HttpConfig; import org.apache.brooklyn.util.http.executor.HttpExecutor; import org.apache.brooklyn.util.http.executor.HttpExecutorFactory; import org.apache.brooklyn.util.http.executor.HttpRequest; import org.apache.brooklyn.util.http.executor.HttpResponse; import org.apache.brooklyn.util.http.executor.apacheclient.HttpExecutorImpl; +import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.time.Duration; import org.apache.http.auth.Credentials; import org.apache.http.auth.UsernamePasswordCredentials; @@ -61,6 +63,7 @@ import com.google.common.base.Optional; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; +import com.google.common.base.Throwables; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -370,9 +373,9 @@ protected void preStart() { public HttpToolResponse call() throws Exception { if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo); - BasicAuth creds = null; + UsernamePassword creds = null; if (pollInfo.credentials.isPresent()) { - creds = org.apache.brooklyn.util.http.executor.Credentials.basic( + creds = new UsernamePassword( pollInfo.credentials.get().getUserPrincipal().getName(), pollInfo.credentials.get().getPassword()); } @@ -404,24 +407,29 @@ protected Poller getPoller() { private HttpToolResponse createHttpToolRespose(HttpResponse response) throws IOException { int responseCode = response.code(); - /* From https://tools.ietf.org/html/rfc4918#section-11.2 - The 422 (Unprocessable Entity) status code means the server - understands the content type of the request entity (hence a - 415(Unsupported Media Type) status code is inappropriate), and the - syntax of the request entity is correct (thus a 400 (Bad Request) - status code is inappropriate) but was unable to process the contained - instructions. */ - if (responseCode == 422) { - throw new IOException(" Unprocessable Entity: " + response.reasonPhrase()); - } Map> headers = (Map>) (Map) response.headers().asMap(); - return new HttpToolResponse(responseCode, - headers, - ByteStreams.toByteArray(response.getContent()), - System.currentTimeMillis(), - 0, - 0); + byte[] content = null; + final long durationMillisOfFirstResponse; + final long durationMillisOfFullContent; + final long startTime = System.currentTimeMillis(); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + durationMillisOfFirstResponse = Duration.sinceUtc(startTime).toMilliseconds(); + try { + ByteStreams.copy(response.getContent(), out); + content = out.toByteArray(); + } catch (IOException e) { + throw Throwables.propagate(e); + } finally { + Streams.closeQuietly(out); + } + durationMillisOfFullContent = Duration.sinceUtc(startTime).toMilliseconds(); + + return new HttpToolResponse(responseCode, headers, content, + startTime, + durationMillisOfFirstResponse, + durationMillisOfFullContent); } } diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java index 05824dd02b..ca478fd872 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java @@ -33,7 +33,6 @@ import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.core.location.AbstractLocationResolver; -import org.apache.brooklyn.core.location.LocationConfigKeys; import org.apache.brooklyn.core.mgmt.internal.LocalLocationManager; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; @@ -109,7 +108,6 @@ protected ConfigBag extractConfig(Map locationFlags, String spec, LocationR MutableMap defaultProps = MutableMap.of(); defaultProps.addIfNotNull("user", user); defaultProps.addIfNotNull("port", port); - defaultProps.addIfNotNull(LocationConfigKeys.EXTENSIONS.getName(), config.get(LocationConfigKeys.EXTENSIONS)); List hostAddresses; diff --git a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java index b6489f4010..2a043a8e3c 100644 --- a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java +++ b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactory.java @@ -21,7 +21,7 @@ import java.util.Map; public interface HttpExecutorFactory { - public static final String HTTP_EXECUTOR_CLASS = "httpExecutorClass"; + public static final String HTTP_EXECUTOR_CLASS_CONFIG = "httpExecutorClass"; public static final String HTTP_EXECUTOR_CLASS_CONFIG_PREFIX = "httpExecutorClass."; diff --git a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java index e9b769a7e7..8d8ed7ac8e 100644 --- a/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java +++ b/core/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutorFactoryImpl.java @@ -23,7 +23,6 @@ import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; -import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.http.executor.apacheclient.HttpExecutorImpl; import org.apache.brooklyn.util.text.StringPredicates; @@ -44,27 +43,24 @@ public HttpExecutorFactoryImpl() { public HttpExecutor getHttpExecutor(Map props) { HttpExecutor httpExecutor; - String httpExecutorClass = (String) props.get(HTTP_EXECUTOR_CLASS); + String httpExecutorClass = (String) props.get(HTTP_EXECUTOR_CLASS_CONFIG); if (httpExecutorClass != null) { + Map httpExecutorProps = MutableMap.of(); Map executorProps = Maps.filterKeys(props, StringPredicates.isStringStartingWith(HTTP_EXECUTOR_CLASS_CONFIG_PREFIX)); if (executorProps.size() > 0) { - Map httpExecutorProps = MutableMap.of(); for (Entry entry: executorProps.entrySet()) { String keyName = Strings.removeFromStart((String)entry.getKey(), HTTP_EXECUTOR_CLASS_CONFIG_PREFIX); - httpExecutorProps.put(keyName, TypeCoercions.coerce(entry.getValue(), String.class)); + httpExecutorProps.put(keyName, entry.getValue()); } - - try { - httpExecutor = (HttpExecutor) new ClassLoaderUtils(getClass()).loadClass(httpExecutorClass).getConstructor(Map.class).newInstance(httpExecutorProps); - } catch (Exception e) { - throw Exceptions.propagate(e); - } - } else { - LOG.error("Missing parameters for: " + HTTP_EXECUTOR_CLASS); - throw Exceptions.propagate(new IllegalArgumentException("Missing parameters for: " + HTTP_EXECUTOR_CLASS)); } + try { + httpExecutor = (HttpExecutor) new ClassLoaderUtils(getClass()).loadClass(httpExecutorClass).getConstructor(Map.class).newInstance(httpExecutorProps); + } catch (Exception e) { + throw Exceptions.propagate(e); + } + } else { - LOG.warn(HTTP_EXECUTOR_CLASS + " parameter not provided. Using the default implementation " + HttpExecutorImpl.class.getName()); + LOG.info(HTTP_EXECUTOR_CLASS_CONFIG + " parameter not provided. Using the default implementation " + HttpExecutorImpl.class.getName()); httpExecutor = HttpExecutorImpl.newInstance(); } diff --git a/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java b/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java index 48d5a7f737..1f18d32a9d 100644 --- a/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/http/executor/HttpExecutorImplTest.java @@ -59,7 +59,7 @@ public void setUp() throws Exception { baseUrl = server.getUrl("/"); } - @AfterMethod + @AfterMethod(alwaysRun=true) public void afterMethod() throws Exception { if (server != null) server.shutdown(); } @@ -159,7 +159,7 @@ public void testHttpPasswordRequest() throws Exception { HttpRequest executorRequest = new HttpRequest.Builder().headers(ImmutableMap.of("RequestHeader", "RequestHeaderValue")) .method("GET") .uri(baseUrl.toURI()) - .credentials(new Credentials.BasicAuth("brooklyn", "apache")) + .credentials(new UsernamePassword("brooklyn", "apache")) .build(); HttpResponse executorResponse = executor.execute(executorRequest); diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java index f46045d59c..a19f2cf2a7 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/Credentials.java @@ -21,29 +21,8 @@ import com.google.common.annotations.Beta; @Beta -public class Credentials { +public interface Credentials { + String getUser(); - private Credentials() {} - - public static BasicAuth basic(String username, String password) { - return new BasicAuth(username, password); - } - - public static class BasicAuth { - private final String username; - private final String password; - - protected BasicAuth(String username, String password) { - this.username = username; - this.password = password; - } - - public String username() { - return username; - } - - public String password() { - return password; - } - } + String getPassword(); } diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java index 26d8b1a519..d070d57c88 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpExecutor.java @@ -24,7 +24,19 @@ * An abstraction for executing HTTP requests, allowing an appropriate implementation to be chosen. */ public interface HttpExecutor { - + /** + * HTTP methods + */ + static final String GET = "GET"; + + static final String HEAD = "HEAD"; + + static final String POST = "POST"; + + static final String PUT = "PUT"; + + static final String DELETE = "DELETE"; + /** * Synchronously send the request, and return its response. * diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java index a2de4b7ac6..e3c6db87ce 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java @@ -41,7 +41,7 @@ public static class Builder { protected String method; protected byte[] body; protected Multimap headers = ArrayListMultimap.create(); - protected Credentials.BasicAuth credentials; + protected Credentials credentials; protected HttpConfig config; public Builder uri(URI val) { @@ -82,7 +82,7 @@ public Builder header(String key, String val) { return this; } - public Builder credentials(@Nullable Credentials.BasicAuth val) { + public Builder credentials(@Nullable Credentials val) { credentials = val; return this; } @@ -120,7 +120,7 @@ public HttpRequest build() { Multimap headers(); @Nullable - Credentials.BasicAuth credentials(); + Credentials credentials(); /** * Additional optional configuration to customize how the call is done. diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java index cf10752ad4..dda08fe703 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequestImpl.java @@ -22,7 +22,7 @@ import java.net.URI; -import org.apache.brooklyn.util.http.executor.Credentials.BasicAuth; +import org.apache.brooklyn.util.http.executor.Credentials; import com.google.common.annotations.Beta; import com.google.common.collect.ArrayListMultimap; @@ -39,7 +39,7 @@ public class HttpRequestImpl implements HttpRequest { protected final String method; protected final byte[] body; protected final Multimap headers; - protected final Credentials.BasicAuth credentials; + protected final Credentials credentials; protected final HttpConfig config; protected HttpRequestImpl(HttpRequest.Builder builder) { @@ -81,7 +81,7 @@ public Multimap headers() { } @Override - public BasicAuth credentials() { + public Credentials credentials() { return credentials; } diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java index a28aa059df..97faeef8c5 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpResponseImpl.java @@ -67,6 +67,7 @@ public Multimap headers() { return headers; } + // TODO In the streaming case could have Content-Length set in the headers. @Override public long getContentLength() { return contentLength; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/UsernamePassword.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/UsernamePassword.java new file mode 100644 index 0000000000..13f61e1e71 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/UsernamePassword.java @@ -0,0 +1,41 @@ +/* + * 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. + */ +package org.apache.brooklyn.util.http.executor; + +import com.google.common.annotations.Beta; + +@Beta +public class UsernamePassword implements Credentials { + private final String username; + private final String password; + + public UsernamePassword(String username, String password) { + this.username = username; + this.password = password; + } + + public String getUser() { + return username; + } + + public String getPassword() { + return password; + } +} + diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java index b2d2ada1a2..7642c901ff 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java @@ -22,7 +22,6 @@ import java.util.Map; import org.apache.brooklyn.util.http.HttpTool; -import org.apache.brooklyn.util.http.HttpTool.HttpClientBuilder; import org.apache.brooklyn.util.http.HttpToolResponse; import org.apache.brooklyn.util.http.executor.HttpConfig; import org.apache.brooklyn.util.http.executor.HttpExecutor; @@ -47,35 +46,24 @@ public class HttpExecutorImpl implements HttpExecutor { .build(); public static HttpExecutorImpl newInstance() { - return newInstance(HttpTool.httpClientBuilder()); + return new HttpExecutorImpl(); } - - /** - * This {@code baseBuilder} is used to construct a new {@link HttpClient} for each call to {@link #execute(HttpRequest)}. - * Callers are strongly discouraged from modifying the baseBuilder after passing it in! - */ - public static HttpExecutorImpl newInstance(HttpTool.HttpClientBuilder baseBuilder) { - return new HttpExecutorImpl(baseBuilder); - } - - private final HttpClientBuilder baseBuilder; /** * A must have constructor. */ public HttpExecutorImpl(Map props) { - this(HttpTool.httpClientBuilder()); + } - protected HttpExecutorImpl(HttpClientBuilder baseBuilder) { - this.baseBuilder = baseBuilder; + public HttpExecutorImpl() { } @Override public HttpResponse execute(HttpRequest request) throws IOException { HttpConfig config = (request.config() != null) ? request.config() : DEFAULT_CONFIG; - Credentials creds = (request.credentials() != null) ? new UsernamePasswordCredentials(request.credentials().username(), request.credentials().password()) : null; - HttpClient httpClient = HttpTool.HttpClientBuilder.fromBuilder(baseBuilder) + Credentials creds = (request.credentials() != null) ? new UsernamePasswordCredentials(request.credentials().getUser(), request.credentials().getPassword()) : null; + HttpClient httpClient = HttpTool.httpClientBuilder() .uri(request.uri()) .credential(Optional.fromNullable(creds)) .laxRedirect(config.laxRedirect()) @@ -85,20 +73,20 @@ public HttpResponse execute(HttpRequest request) throws IOException { HttpToolResponse response; - switch (request.method().toLowerCase()) { - case "get": + switch (request.method().toUpperCase()) { + case HttpExecutor.GET: response = HttpTool.httpGet(httpClient, request.uri(), request.headers()); break; - case "head": + case HttpExecutor.HEAD: response = HttpTool.httpHead(httpClient, request.uri(), request.headers()); break; - case "post": + case HttpExecutor.POST: response = HttpTool.httpPost(httpClient, request.uri(), request.headers(), orEmpty(request.body())); break; - case "put": + case HttpExecutor.PUT: response = HttpTool.httpPut(httpClient, request.uri(), request.headers(), orEmpty(request.body())); break; - case "delete": + case HttpExecutor.DELETE: response = HttpTool.httpDelete(httpClient, request.uri(), request.headers()); break; default: