Skip to content
Permalink
Browse files
lets BrooklynApi.getEntity understand some errors, and add better method
    getEntityOnSuccess which requires a success code from the server.

    triggered by observing downstream usages which use getEntity on results
    without checking them, and thus have an empty TaskSummary following a severe server error
    (the server error is lost and the code typically fails later with no indication of cause)

    also tidy deprecations/warnings, toString
  • Loading branch information
ahgittin committed Feb 27, 2017
1 parent 1203c09 commit 23fdb1226a71ddfe304b48bea2d3db81cadf8c7e
Showing 4 changed files with 256 additions and 29 deletions.
@@ -26,12 +26,32 @@
import java.lang.reflect.Proxy;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Map;

import javax.annotation.Nullable;
import javax.ws.rs.core.Response;

import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import org.apache.brooklyn.rest.api.AccessApi;
import org.apache.brooklyn.rest.api.ActivityApi;
import org.apache.brooklyn.rest.api.ApplicationApi;
import org.apache.brooklyn.rest.api.CatalogApi;
import org.apache.brooklyn.rest.api.EffectorApi;
import org.apache.brooklyn.rest.api.EntityApi;
import org.apache.brooklyn.rest.api.EntityConfigApi;
import org.apache.brooklyn.rest.api.LocationApi;
import org.apache.brooklyn.rest.api.PolicyApi;
import org.apache.brooklyn.rest.api.PolicyConfigApi;
import org.apache.brooklyn.rest.api.ScriptApi;
import org.apache.brooklyn.rest.api.SensorApi;
import org.apache.brooklyn.rest.api.ServerApi;
import org.apache.brooklyn.rest.api.UsageApi;
import org.apache.brooklyn.rest.api.VersionApi;
import org.apache.brooklyn.rest.client.util.http.BuiltResponsePreservingError;
import org.apache.brooklyn.rest.domain.ApiError;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.javalang.AggregateClassLoader;
import org.apache.brooklyn.util.net.Urls;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.Credentials;
import org.apache.http.auth.UsernamePasswordCredentials;
@@ -52,29 +72,10 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import com.google.gson.Gson;

import org.apache.brooklyn.rest.api.AccessApi;
import org.apache.brooklyn.rest.api.ActivityApi;
import org.apache.brooklyn.rest.api.ApplicationApi;
import org.apache.brooklyn.rest.api.CatalogApi;
import org.apache.brooklyn.rest.api.EffectorApi;
import org.apache.brooklyn.rest.api.EntityApi;
import org.apache.brooklyn.rest.api.EntityConfigApi;
import org.apache.brooklyn.rest.api.LocationApi;
import org.apache.brooklyn.rest.api.PolicyApi;
import org.apache.brooklyn.rest.api.PolicyConfigApi;
import org.apache.brooklyn.rest.api.ScriptApi;
import org.apache.brooklyn.rest.api.SensorApi;
import org.apache.brooklyn.rest.api.ServerApi;
import org.apache.brooklyn.rest.api.UsageApi;
import org.apache.brooklyn.rest.api.VersionApi;
import org.apache.brooklyn.rest.client.util.http.BuiltResponsePreservingError;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.javalang.AggregateClassLoader;
import org.apache.brooklyn.util.net.Urls;

import io.swagger.annotations.ApiOperation;

/**
@@ -380,34 +381,112 @@ public AccessApi getAccessApi() {
return proxy(AccessApi.class);
}

/** Extracts an instance of the given type from the response, including JSON strings in there.
* Forgives most errors except for obviously incompatible ones.
* To fail on any server error, use {@link #getEntityOnSuccess(Response, Class)}.
* <p>
* This method will coerce and empty map "{}" to a no-arg contructed instance of the target class.
* This method will also ignore most errors in the response.
* <p>
* It has changed to identify the most obvious errors. */
public static <T> T getEntity(Response response, Class<T> type) {
if (response instanceof ClientResponse) {
ClientResponse<?> clientResponse = (ClientResponse<?>) response;
return clientResponse.getEntity(type);
} else if (response instanceof BuiltResponse) {
// Handle BuiltResponsePreservingError turning objects into Strings
if (response.getEntity() instanceof String && !type.equals(String.class)) {
failSomeErrors(response, type, true);
return new Gson().fromJson(response.getEntity().toString(), type);
}
}
// Last-gasp attempt.
return type.cast(response.getEntity());
}

/** As {@link #getEntity(Response, Class)} but fails if the response is an error of any sort. */
public static <T> T getEntityOnSuccess(Response response, Class<T> type) {
failSomeErrors(response, type, false);
return getEntity(response, type);
}

/** This fails if it is clearly an ApiError response which the caller did not want.
* To fail on any error (probably better), use */
private static <T> void failSomeErrors(Response response, Class<?> type, boolean onlyIfItLooksLikeApiError) {
if (response.getStatus()<400) {
// not an error
return;
}
if (onlyIfItLooksLikeApiError && type.isAssignableFrom(ApiError.class) && !Map.class.isAssignableFrom(type)) {
// if user wanted a map or an ApiError, don't fail (for legacy compatibility)
return;
}

Object obj = new Gson().fromJson(response.getEntity().toString(), Object.class);

if (onlyIfItLooksLikeApiError && !(obj instanceof Map)) {
// only handle maps
return;
}

@SuppressWarnings("rawtypes")
Map m = (Map)obj;
Object error = m.get("error");
if (onlyIfItLooksLikeApiError && (error==null || new Integer(0).equals(error) || "".equals(error))) {
// error should be non-zero for "ApiError"
return;
}

Object message = m.get("message");
if (message==null) message = m.get("detail");

throw new IllegalArgumentException("Server error "+response.getStatus()+" cannot be converted to "+type.getName()+
(message!=null ? ": "+message : ""));
}

/** As {@link #getEntity(Response, Class)} */
public static <T> T getEntity(Response response, GenericType<T> type) {
if (response instanceof ClientResponse) {
ClientResponse<?> clientResponse = (ClientResponse<?>) response;
return clientResponse.getEntity(type);
} else if (response instanceof BuiltResponse) {
// Handle BuiltResponsePreservingError turning objects into Strings
if (response.getEntity() instanceof String) {
failSomeErrors(response, type.getType(), true);
return new Gson().fromJson(response.getEntity().toString(), type.getGenericType());
}
}
// Last-gasp attempt.
return type.getType().cast(response.getEntity());
}


/** As {@link #getEntity(Response, GenericType)} but fails if the response is an error of any sort. */
public static <T> T getEntityOnSuccess(Response response, GenericType<T> type) {
failSomeErrors(response, type.getType(), false);
return getEntity(response, type);
}

/** As {@link #getEntity(Response, Class)} */
@SuppressWarnings("unchecked")
public static <T> T getEntity(Response response, javax.ws.rs.core.GenericType<T> type) {
if (response instanceof BuiltResponse) {
// Handle BuiltResponsePreservingError turning objects into Strings
if (response.getEntity() instanceof String) {
failSomeErrors(response, type.getRawType(), true);
return new Gson().fromJson(response.getEntity().toString(), type.getType());
}
}
return (T) getEntity(response, type.getRawType());
}


/** As {@link #getEntity(Response, javax.ws.rs.core.GenericType)} but fails if the response is an error of any sort. */
public static <T> T getEntityOnSuccess(Response response, javax.ws.rs.core.GenericType<T> type) {
failSomeErrors(response, type.getRawType(), false);
return getEntity(response, type);
}

/**
* @deprecated since 0.8.0-incubating. Use {@link #getEntity(Response, GenericType)} instead.
*/
@@ -103,7 +103,7 @@ public static Status waitForAppStatus(final BrooklynApi api, final String applic
final AtomicReference<Status> appStatus = new AtomicReference<>(Status.UNKNOWN);
final boolean shortcutOnError = !Status.ERROR.equals(desiredStatus) && !Status.UNKNOWN.equals(desiredStatus);
LOG.info("Waiting " + timeout + " from " + new Date() + " for application " + application + " to be " + desiredStatus);
boolean finalAppStatusKnown = Repeater.create("Waiting for application " + application + " status to be " + desiredStatus)
Repeater.create("Waiting for application " + application + " status to be " + desiredStatus)
.every(pollPeriod)
.limitTimeTo(timeout)
.rethrowExceptionImmediately()
@@ -23,7 +23,6 @@
import javax.ws.rs.core.Response;

import org.apache.brooklyn.util.exceptions.Exceptions;
import org.jboss.resteasy.client.core.BaseClientResponse;
import org.jboss.resteasy.core.Headers;
import org.jboss.resteasy.specimpl.BuiltResponse;

@@ -51,8 +50,8 @@ public static <T> Response copyResponseAndClose(Response source, Class<T> type)
Object entity = null;
try {
status = source.getStatus();
if (source instanceof BaseClientResponse)
headers.putAll(((BaseClientResponse<?>)source).getMetadata());
if (source instanceof org.jboss.resteasy.client.core.BaseClientResponse)
headers.putAll(((org.jboss.resteasy.client.core.BaseClientResponse<?>)source).getMetadata());
if (source instanceof org.jboss.resteasy.client.ClientResponse) {
entity = ((org.jboss.resteasy.client.ClientResponse<?>)source).getEntity(type);
} else {
@@ -63,8 +62,8 @@ public static <T> Response copyResponseAndClose(Response source, Class<T> type)
Exceptions.propagateIfFatal(e);
return new BuiltResponsePreservingError(status, headers, entity, new Annotation[0], e);
} finally {
if (source instanceof BaseClientResponse)
((BaseClientResponse<?>)source).close();
if (source instanceof org.jboss.resteasy.client.core.BaseClientResponse)
((org.jboss.resteasy.client.core.BaseClientResponse<?>)source).close();
}
}

@@ -76,4 +75,10 @@ public Object getEntity() {
return super.getEntity();
}

@Override
public String toString() {
return "brooklyn-rest-client:BuiltResponse["+error+":"+getStatus()+
(!isClosed() && error==null ? ":"+getEntity() : "")+"]";
}

}
@@ -0,0 +1,143 @@
/*
* 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.rest.client;

import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import org.apache.brooklyn.rest.domain.ApiError;
import org.apache.brooklyn.rest.domain.TaskSummary;
import org.apache.brooklyn.test.Asserts;
import org.jboss.resteasy.specimpl.BuiltResponse;
import org.testng.Assert;
import org.testng.annotations.Test;

public class BrooklynApiGetEntityTest {

@Test(expectedExceptions=NullPointerException.class)
public void testGetEntityDisallowsNull() {
Assert.assertNull( BrooklynApi.getEntity(null, TaskSummary.class) );
}

@Test(expectedExceptions=Exception.class)
public void testGetEntityFailsOnNonJson() {
BrooklynApi.getEntity(
new BuiltResponse(200, null, "I'm a string not JSON", null),
TaskSummary.class);
}

@Test
public void testGetEntityAllowsEmptyMaps() {
BrooklynApi.getEntity(
new BuiltResponse(200, null, "{}", null),
TaskSummary.class);
}

@Test
public void testGetEntityIgnoresExtraFields() {
BrooklynApi.getEntity(
new BuiltResponse(200, null, "{ foo: \"This should cause an error\" }", null),
TaskSummary.class);
}

@Test
public void testGetEntityAllowsNull() {
BrooklynApi.getEntity(
new BuiltResponse(200, null, null, null),
TaskSummary.class);
}

@Test
public void testGetEntityIgnoresErrorResponseCodeInBuiltResponse() {
BrooklynApi.getEntity(
new BuiltResponse(400, null, "{}", null),
TaskSummary.class);
}

@Test
public void testGetEntityIgnoresErrorResponseCodeInBasicResponse() {
BrooklynApi.getEntity(
newBadRequestResponse("{}"),
TaskSummary.class);
}

@Test(expectedExceptions=Exception.class)
public void testGetEntityFailsIfLooksLikeApiError() {
BrooklynApi.getEntity(
newBadRequestResponse("{ error: 400 }"),
TaskSummary.class);
}

@Test
public void testGetEntityFailsWithMessageIfLooksLikeApiErrorWithMessage() {
try {
BrooklynApi.getEntity(
newBadRequestResponse("{ error: 400, message: \"Foo\" }"),
TaskSummary.class);
Asserts.shouldHaveFailedPreviously();
} catch (Exception e) {
Asserts.expectedFailureContains(e, "Foo");
}
}

@Test
public void testGetEntitySucceedsIfLooksLikeApiErrorWhenWantingApiError() {
BrooklynApi.getEntity(
newBadRequestResponse("{ error: 400 }"),
ApiError.class);
}

@Test(expectedExceptions=Exception.class)
public void testGetSuccessfulEntityFailsOnAnyError() {
BrooklynApi.getEntityOnSuccess(
newBadRequestResponse("{ error: 400 }"),
ApiError.class);
}

@Test
public void testGetEntitySucceedsIfNoErrorCodeWhenApiError() {
BrooklynApi.getEntity(
newJsonResponse(Status.OK, "{ error: 400 }"),
TaskSummary.class);
}

@Test
public void testGetSuccessfulEntitySucceedsOnNoErrorApiError() {
BrooklynApi.getEntityOnSuccess(
newJsonResponse(Status.OK, "{ error: 400 }"),
ApiError.class);
}

public Response newBadRequestResponse(Object entity) {
return newJsonResponse(Status.BAD_REQUEST, entity);
}

public static Response newJsonResponse(Status status, Object entity) {
return newResponse(status, MediaType.APPLICATION_JSON_TYPE, entity);
}

public static Response newResponse(Status status, MediaType type, Object entity) {
return Response.status(status)
.type(type)
.entity(entity)
.build();
}

}

0 comments on commit 23fdb12

Please sign in to comment.