You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2015/05/29 12:36:08 UTC
[08/13] incubator-brooklyn git commit: improve rest client handling
of unclosed "response" objects
improve rest client handling of unclosed "response" objects
prevents the error that the client has not been closed,
also tests for this, and simplifies rest client test framework
(which had some complexities which are not needed);
a few other misc cleanups and code review also
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/dbcc40a7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/dbcc40a7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/dbcc40a7
Branch: refs/heads/master
Commit: dbcc40a7c26bb74f4d3fba6929bc5bd5e97c38ac
Parents: dbd0cdc
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 12:41:18 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 13:08:18 2015 +0100
----------------------------------------------------------------------
usage/rest-client/pom.xml | 36 ++--------
.../java/brooklyn/rest/client/BrooklynApi.java | 37 ++++++++--
.../util/http/BuiltResponsePreservingError.java | 76 ++++++++++++++++++++
.../ApplicationResourceIntegrationTest.java | 19 ++---
.../rest/client/BrooklynApiRestClientTest.java | 43 ++++++++---
.../provider/DelegatingSecurityProvider.java | 1 -
6 files changed, 151 insertions(+), 61 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/pom.xml
----------------------------------------------------------------------
diff --git a/usage/rest-client/pom.xml b/usage/rest-client/pom.xml
index 70c48e2..af001aa 100644
--- a/usage/rest-client/pom.xml
+++ b/usage/rest-client/pom.xml
@@ -118,6 +118,13 @@
</dependency>
<dependency>
<groupId>org.apache.brooklyn</groupId>
+ <artifactId>brooklyn-core</artifactId>
+ <version>${project.version}</version>
+ <classifier>tests</classifier>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.brooklyn</groupId>
<artifactId>brooklyn-rest-server</artifactId>
<version>${project.version}</version>
<scope>test</scope>
@@ -143,33 +150,4 @@
</dependency>
</dependencies>
- <build>
-
- <plugins>
- <plugin>
- <artifactId>maven-dependency-plugin</artifactId>
- <executions>
- <execution>
- <id>prep-server</id>
- <phase>pre-integration-test</phase>
- <goals>
- <goal>unpack</goal>
- </goals>
- <configuration>
- <artifactItems>
- <artifactItem>
- <groupId>${project.groupId}</groupId>
- <artifactId>brooklyn-rest-server</artifactId>
- <version>${brooklyn.version}</version>
- <overWrite>true</overWrite>
- <outputDirectory>target/test-rest-server</outputDirectory>
- </artifactItem>
- </artifactItems>
- </configuration>
- </execution>
- </executions>
- </plugin>
- </plugins>
- </build>
-
</project>
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java b/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
index c813864..9765ecf 100644
--- a/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
+++ b/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
@@ -20,6 +20,10 @@ package brooklyn.rest.client;
import static com.google.common.base.Preconditions.checkNotNull;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
import java.net.MalformedURLException;
import java.net.URL;
@@ -52,6 +56,8 @@ import brooklyn.rest.api.SensorApi;
import brooklyn.rest.api.ServerApi;
import brooklyn.rest.api.UsageApi;
import brooklyn.rest.api.VersionApi;
+import brooklyn.util.exceptions.Exceptions;
+import brooklyn.util.http.BuiltResponsePreservingError;
/**
* @author Adam Lowe
@@ -105,8 +111,29 @@ public class BrooklynApi {
this.clientExecutor = checkNotNull(clientExecutor, "clientExecutor");
}
+ @SuppressWarnings("unchecked")
private <T> T proxy(Class<T> clazz) {
- return ProxyFactory.create(clazz, target, clientExecutor);
+ final T result0 = ProxyFactory.create(clazz, target, clientExecutor);
+ return (T) Proxy.newProxyInstance(clazz.getClassLoader(), new Class<?>[] { clazz }, new InvocationHandler() {
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+ Object result1;
+ try {
+ result1 = method.invoke(result0, args);
+ } catch (Throwable e) {
+ if (e instanceof InvocationTargetException) {
+ // throw the original exception
+ e = ((InvocationTargetException)e).getTargetException();
+ }
+ throw Exceptions.propagate(e);
+ }
+ if (result1 instanceof Response) {
+ // wrap the original response so it self-closes
+ result1 = BuiltResponsePreservingError.copyResponseAndClose((Response) result1);
+ }
+ return result1;
+ }
+ });
}
public ActivityApi getActivityApi() {
@@ -169,21 +196,19 @@ public class BrooklynApi {
return proxy(AccessApi.class);
}
- @SuppressWarnings({ "unchecked", "rawtypes" })
public static <T> T getEntity(Response response, Class<T> type) {
if (!(response instanceof ClientResponse)) {
throw new IllegalArgumentException("Response should be instance of ClientResponse, is: " + response.getClass());
}
- ClientResponse clientResponse = (ClientResponse) response;
+ ClientResponse<?> clientResponse = (ClientResponse<?>) response;
return (T) clientResponse.getEntity(type);
}
- @SuppressWarnings({ "unchecked", "rawtypes" })
- public static <T> T getEntityGeneric(Response response, GenericType type) {
+ public static <T> T getEntityGeneric(Response response, GenericType<T> type) {
if (!(response instanceof ClientResponse)) {
throw new IllegalArgumentException("Response should be instance of ClientResponse, is: " + response.getClass());
}
- ClientResponse clientResponse = (ClientResponse) response;
+ ClientResponse<?> clientResponse = (ClientResponse<?>) response;
return (T) clientResponse.getEntity(type);
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java b/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java
new file mode 100644
index 0000000..1dffb58
--- /dev/null
+++ b/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java
@@ -0,0 +1,76 @@
+/*
+ * 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 brooklyn.util.http;
+
+import java.lang.annotation.Annotation;
+
+import javax.ws.rs.core.Response;
+
+import org.jboss.resteasy.core.Headers;
+import org.jboss.resteasy.specimpl.BuiltResponse;
+
+import brooklyn.util.exceptions.Exceptions;
+
+/**
+ * Allows wrapping a {@link Response} with the stream fully read and closed so that the client can be re-used.
+ * <p>
+ * The entity may be stored as a string as type info is not available when it is deserialized,
+ * and that's a relatively convenient common format.
+ *
+ * TODO It would be nice to support other parsing, storing the byte array.
+ */
+public class BuiltResponsePreservingError extends BuiltResponse {
+
+ private Throwable error;
+
+ public BuiltResponsePreservingError(int status, Headers<Object> headers, Object entity, Annotation[] annotations, Throwable error) {
+ super(status, headers, entity, annotations);
+ this.error = error;
+ }
+
+ @SuppressWarnings("deprecation")
+ public static Response copyResponseAndClose(Response source) {
+ int status = -1;
+ Headers<Object> headers = new Headers<Object>();
+ Object entity = null;
+ try {
+ status = source.getStatus();
+ headers.putAll(source.getHeaders());
+ if (source instanceof org.jboss.resteasy.client.ClientResponse) {
+ // ClientResponse requires strong type info, which we don't yet have
+ entity = ((org.jboss.resteasy.client.ClientResponse<?>)source).getEntity(String.class);
+ } else {
+ entity = source.getEntity();
+ }
+ return new BuiltResponsePreservingError(status, headers, entity, new Annotation[0], null);
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ return new BuiltResponsePreservingError(status, headers, entity, new Annotation[0], e);
+ } finally {
+ source.close();
+ }
+ }
+
+ @Override
+ public Object getEntity() {
+ if (error!=null) Exceptions.propagate(error);
+ return super.getEntity();
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java b/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
index e303571..a55aa45 100644
--- a/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
+++ b/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
@@ -27,7 +27,6 @@ import java.util.Collection;
import javax.ws.rs.core.Response;
import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.webapp.WebAppContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.Assert;
@@ -35,7 +34,6 @@ import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
-import brooklyn.config.BrooklynServiceAttributes;
import brooklyn.entity.Application;
import brooklyn.entity.basic.Lifecycle;
import brooklyn.entity.basic.StartableApplication;
@@ -87,16 +85,8 @@ public class ApplicationResourceIntegrationTest {
@BeforeClass(groups = "Integration")
public void setUp() throws Exception {
- WebAppContext context;
-
- // running in source mode; need to use special classpath
- context = new WebAppContext("src/test/webapp", "/");
- context.setExtraClasspath("./target/test-rest-server/");
- context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, getManagementContext());
-
Server server = BrooklynRestApiLauncher.launcher()
- .managementContext(manager)
- .customContext(context)
+ .managementContext(getManagementContext())
.start();
api = new BrooklynApi("http://localhost:" + server.getConnectors()[0].getPort() + "/");
@@ -179,13 +169,12 @@ public class ApplicationResourceIntegrationTest {
} catch (Exception failure) {
// expected -- it will be a ClientResponseFailure but that class is deprecated so catching all
// and asserting contains the word 404
- Assert.assertTrue(failure.toString().indexOf("404") >= 0);
+ Assert.assertTrue(failure.toString().indexOf("404") >= 0, "wrong failure, got: "+failure);
}
}});
} catch (Exception failure) {
- // expected -- it will be a ClientResponseFailure but that class is deprecated so catching all
- // and asserting contains the word 404
- Assert.assertTrue(failure.toString().indexOf("404") >= 0);
+ // expected -- as above
+ Assert.assertTrue(failure.toString().indexOf("404") >= 0, "wrong failure, got: "+failure);
}
assertEquals(getManagementContext().getApplications().size(), size - 1);
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java b/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
index 9e5fbb3..7a320e7 100644
--- a/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
+++ b/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
@@ -21,15 +21,16 @@ package brooklyn.rest.client;
import java.util.List;
import java.util.Map;
+import javax.ws.rs.core.Response;
+
import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.webapp.WebAppContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
-import brooklyn.config.BrooklynServiceAttributes;
import brooklyn.entity.Application;
import brooklyn.entity.basic.Entities;
import brooklyn.entity.basic.StartableApplication;
@@ -41,6 +42,8 @@ import brooklyn.rest.BrooklynRestApiLauncherTest;
import brooklyn.rest.domain.ApplicationSummary;
import brooklyn.rest.domain.CatalogLocationSummary;
import brooklyn.rest.security.provider.TestSecurityProvider;
+import brooklyn.test.HttpTestUtils;
+import brooklyn.test.entity.TestEntity;
@Test
public class BrooklynApiRestClientTest {
@@ -63,17 +66,9 @@ public class BrooklynApiRestClientTest {
@BeforeClass
public void setUp() throws Exception {
- WebAppContext context;
-
- // running in source mode; need to use special classpath
- context = new WebAppContext("src/test/webapp", "/");
- context.setExtraClasspath("./target/test-rest-server/");
- context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, getManagementContext());
-
Server server = BrooklynRestApiLauncher.launcher()
.managementContext(manager)
.securityProvider(TestSecurityProvider.class)
- .customContext(context)
.start();
api = new BrooklynApi("http://localhost:" + server.getConnectors()[0].getPort() + "/",
@@ -107,5 +102,33 @@ public class BrooklynApiRestClientTest {
List<ApplicationSummary> apps = api.getApplicationApi().list(null);
log.info("apps are: "+apps);
}
+
+ public void testApplicationApiCreate() throws Exception {
+ Response r1 = api.getApplicationApi().createFromYaml("name: test-1234\n"
+ + "services: [ { type: "+TestEntity.class.getName()+" } ]");
+ HttpTestUtils.assertHealthyStatusCode(r1.getStatus());
+ log.info("creation result: "+r1.getEntity());
+ List<ApplicationSummary> apps = api.getApplicationApi().list(null);
+ log.info("apps with test: "+apps);
+ Assert.assertTrue(apps.toString().contains("test-1234"), "should have had test-1234 as an app; instead: "+apps);
+ }
+
+ public void testApplicationApiHandledError() throws Exception {
+ Response r1 = api.getApplicationApi().createFromYaml("name: test");
+ Assert.assertTrue(r1.getStatus()/100 != 2, "needed an unhealthy status, not "+r1.getStatus());
+ Object entity = r1.getEntity();
+ Assert.assertTrue(entity.toString().indexOf("Unrecognized application blueprint format: no services defined")>=0,
+ "Missing expected text in response: "+entity.toString());
+ }
+ public void testApplicationApiThrownError() throws Exception {
+ try {
+ ApplicationSummary summary = api.getApplicationApi().get("test-5678");
+ Assert.fail("Should have thrown, not given: "+summary);
+ } catch (Exception e) {
+ e.printStackTrace();
+ Assert.assertTrue(e.toString().toLowerCase().contains("not found"),
+ "Missing expected text in response: "+e.toString());
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java b/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
index f4307ac..2291adb 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
@@ -106,7 +106,6 @@ public class DelegatingSecurityProvider implements SecurityProvider {
}
} catch (Exception e) {
log.warn("REST unable to instantiate security provider " + className + "; all logins are being disallowed", e);
- e.printStackTrace();
delegate = new BlackholeSecurityProvider();
}
return delegate;