brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [3/6] incubator-brooklyn git commit: cleaned up how non-master filter errors are reported
Date Fri, 20 Mar 2015 17:09:39 GMT
cleaned up how non-master filter errors are reported


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/d04d560a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/d04d560a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/d04d560a

Branch: refs/heads/master
Commit: d04d560aed764c7637c8cba24ff2fd2300558bd8
Parents: e400d77
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Fri Mar 20 14:06:45 2015 +0000
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Fri Mar 20 14:06:45 2015 +0000

----------------------------------------------------------------------
 .../java/brooklyn/rest/domain/ApiError.java     | 48 ++++++++++++++++----
 .../rest/filter/HaHotCheckResourceFilter.java   | 17 ++++---
 .../rest/filter/HaMasterCheckFilter.java        | 18 +++++---
 .../brooklyn/rest/resources/ServerResource.java |  3 +-
 .../brooklyn/rest/util/WebResourceUtils.java    | 26 +++++++++--
 5 files changed, 85 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d04d560a/usage/rest-api/src/main/java/brooklyn/rest/domain/ApiError.java
----------------------------------------------------------------------
diff --git a/usage/rest-api/src/main/java/brooklyn/rest/domain/ApiError.java b/usage/rest-api/src/main/java/brooklyn/rest/domain/ApiError.java
index e2689f4..37f2eee 100644
--- a/usage/rest-api/src/main/java/brooklyn/rest/domain/ApiError.java
+++ b/usage/rest-api/src/main/java/brooklyn/rest/domain/ApiError.java
@@ -71,6 +71,7 @@ public class ApiError {
     public static class Builder {
         private String message;
         private String details;
+        private Integer errorCode;
 
         public Builder message(String message) {
             this.message = checkNotNull(message, "message");
@@ -82,6 +83,15 @@ public class ApiError {
             return this;
         }
 
+        public Builder errorCode(Status errorCode) {
+            return errorCode(errorCode.getStatusCode());
+        }
+        
+        public Builder errorCode(Integer errorCode) {
+            this.errorCode = errorCode;
+            return this;
+        }
+        
         /** as {@link #prefixMessage(String, String)} with default separator of `: ` */
         public Builder prefixMessage(String prefix) {
             return prefixMessage(prefix, ": ");
@@ -97,7 +107,7 @@ public class ApiError {
         }
         
         public ApiError build() {
-            return new ApiError(message, details);
+            return new ApiError(message, details, errorCode);
         }
 
         /** @deprecated since 0.7.0; use {@link #copy(ApiError)} */
@@ -109,7 +119,8 @@ public class ApiError {
         public Builder copy(ApiError error) {
             return this
                     .message(error.message)
-                    .details(error.details);
+                    .details(error.details)
+                    .errorCode(error.error);
         }
         
         public String getMessage() {
@@ -122,15 +133,18 @@ public class ApiError {
     @JsonSerialize(include=Inclusion.NON_EMPTY)
     private final String details;
 
-    public ApiError(String message) {
-        this(message, null);
-    }
+    @JsonSerialize(include=Inclusion.NON_NULL)
+    private final Integer error;
 
+    public ApiError(String message) { this(message, null); }
+    public ApiError(String message, String details) { this(message, details, null); }
     public ApiError(
             @JsonProperty("message") String message,
-            @JsonProperty("details") String details) {
+            @JsonProperty("details") String details,
+            @JsonProperty("error") Integer error) {
         this.message = checkNotNull(message, "message");
         this.details = details != null ? details : "";
+        this.error = error;
     }
 
     public String getMessage() {
@@ -141,18 +155,23 @@ public class ApiError {
         return details;
     }
 
+    public Integer getErrorCode() {
+        return error;
+    }
+    
     @Override
     public boolean equals(Object other) {
         if (this == other) return true;
         if (other == null || getClass() != other.getClass()) return false;
         ApiError that = ApiError.class.cast(other);
         return Objects.equal(this.message, that.message) &&
-                Objects.equal(this.details, that.details);
+                Objects.equal(this.details, that.details) &&
+                Objects.equal(this.error, that.error);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hashCode(message, details);
+        return Objects.hashCode(message, details, error);
     }
 
     @Override
@@ -160,6 +179,7 @@ public class ApiError {
         return Objects.toStringHelper(this)
                 .add("message", message)
                 .add("details", details)
+                .add("error", error)
                 .toString();
     }
 
@@ -167,10 +187,18 @@ public class ApiError {
         return asResponse(Status.BAD_REQUEST, MediaType.APPLICATION_JSON_TYPE);
     }
 
-    public Response asResponse(Status status, MediaType type) {
-        return Response.status(status)
+    public Response asResponse(Status defaultStatus, MediaType type) {
+        return Response.status(error!=null ? error : defaultStatus!=null ? defaultStatus.getStatusCode()
: Status.INTERNAL_SERVER_ERROR.getStatusCode())
             .type(type)
             .entity(this)
             .build();
     }
+    
+    public Response asResponse(MediaType type) {
+        return asResponse(null, type);
+    }
+    
+    public Response asJsonResponse() {
+        return asResponse(MediaType.APPLICATION_JSON_TYPE);
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d04d560a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
index 5367439..c219de2 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
@@ -24,12 +24,15 @@ import java.util.Set;
 
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Context;
-import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import brooklyn.entity.rebind.RebindManagerImpl.RebindTracker;
 import brooklyn.management.ManagementContext;
 import brooklyn.management.ha.ManagementNodeState;
+import brooklyn.rest.domain.ApiError;
 import brooklyn.util.time.Duration;
 
 import com.google.common.collect.ImmutableSet;
@@ -41,6 +44,9 @@ import com.sun.jersey.spi.container.ResourceFilter;
 import com.sun.jersey.spi.container.ResourceFilterFactory;
 
 public class HaHotCheckResourceFilter implements ResourceFilterFactory {
+    
+    private static final Logger log = LoggerFactory.getLogger(HaHotCheckResourceFilter.class);
+    
     private static final Set<ManagementNodeState> HOT_STATES = ImmutableSet.of(
             ManagementNodeState.MASTER, ManagementNodeState.HOT_STANDBY, ManagementNodeState.HOT_BACKUP);
     private static final long STATE_CHANGE_SETTLE_OFFSET = Duration.seconds(10).toMilliseconds();
@@ -71,11 +77,10 @@ public class HaHotCheckResourceFilter implements ResourceFilterFactory
{
         @Override
         public ContainerRequest filter(ContainerRequest request) {
             if (!isStateLoaded() && isUnsafe(request)) {
-                Response response = Response.status(Response.Status.FORBIDDEN)
-                        .type(MediaType.APPLICATION_JSON)
-                        .entity("{\"error\":403,\"message\":\"Requests should be made to
the master Brooklyn server\"}")
-                        .build();
-                throw new WebApplicationException(response);
+                log.warn("Disallowed request to standby brooklyn: "+request+"/"+am+" (caller
should set '"+HaMasterCheckFilter.SKIP_CHECK_HEADER+"' to force)");
+                throw new WebApplicationException(ApiError.builder()
+                    .message("This request is not permitted against a standby Brooklyn server")
+                    .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse());
             }
             return request;
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d04d560a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
index 03d75e0..bfb2cf4 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
@@ -24,11 +24,13 @@ import java.util.Set;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
+import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.core.Response;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,6 +38,8 @@ import org.slf4j.LoggerFactory;
 import brooklyn.config.BrooklynServiceAttributes;
 import brooklyn.management.ManagementContext;
 import brooklyn.management.ha.ManagementNodeState;
+import brooklyn.rest.domain.ApiError;
+import brooklyn.rest.util.WebResourceUtils;
 
 import com.google.common.collect.Sets;
 
@@ -51,22 +55,22 @@ public class HaMasterCheckFilter implements Filter {
     public static final String SKIP_CHECK_HEADER = "Brooklyn-Allow-Non-Master-Access";
     private static final Set<String> SAFE_STANDBY_METHODS = Sets.newHashSet("GET",
"HEAD");
 
+    protected ServletContext servletContext;
     protected ManagementContext mgmt;
 
     @Override
     public void init(FilterConfig config) throws ServletException {
-        mgmt = (ManagementContext) config.getServletContext().getAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT);
+        servletContext = config.getServletContext();
+        mgmt = (ManagementContext) servletContext.getAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT);
     }
 
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
         if (!isMaster() && !isRequestAllowedForNonMaster(request)) {
-            log.warn("Disallowed request to non-HA master: "+request+"/"+request.getParameterMap()+"
(caller should set '"+SKIP_CHECK_HEADER+"' to force)");
-            HttpServletResponse httpResponse = (HttpServletResponse) response;
-            httpResponse.setStatus(HttpServletResponse.SC_FORBIDDEN);
-            httpResponse.setContentType("application/json");
-            httpResponse.setCharacterEncoding("UTF-8");
-            httpResponse.getWriter().write("{\"error\":403,\"message\":\"Requests should
be made to the master Brooklyn server\"}");
+            log.warn("Disallowed request to non-HA-master brooklyn: "+request+"/"+request.getParameterMap()+"
(caller should set '"+SKIP_CHECK_HEADER+"' to force)");
+            WebResourceUtils.applyJsonResponse(servletContext, ApiError.builder()
+                .message("This request is only permitted against a master Brooklyn server")
+                .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse(), (HttpServletResponse)response);
         } else {
             chain.doFilter(request, response);
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d04d560a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
index 0714c4b..e44437f 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
@@ -102,7 +102,8 @@ public class ServerResource extends AbstractBrooklynRestResource implements
Serv
         log.info("REST call to shutdown server, stopAppsFirst="+stopAppsFirst+", delayForHttpReturn="+shutdownTimeoutRaw);
 
         if (stopAppsFirst && !isMaster()) {
-            throw WebResourceUtils.serverError("Not allowed to stop all apps when server
is not master");
+            log.warn("REST call to shutdown non-master server while stopping apps is disallowed");
+            throw WebResourceUtils.forbidden("Not allowed to stop all apps when server is
not master");
         }
         final Duration shutdownTimeout = parseDuration(shutdownTimeoutRaw, Duration.of(20,
TimeUnit.SECONDS));
         Duration requestTimeout = parseDuration(requestTimeoutRaw, Duration.of(20, TimeUnit.SECONDS));

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d04d560a/usage/rest-server/src/main/java/brooklyn/rest/util/WebResourceUtils.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/util/WebResourceUtils.java b/usage/rest-server/src/main/java/brooklyn/rest/util/WebResourceUtils.java
index 2a4f5e2..fcc2c5c 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/util/WebResourceUtils.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/util/WebResourceUtils.java
@@ -18,8 +18,11 @@
  */
 package brooklyn.rest.util;
 
+import java.io.IOException;
 import java.util.Map;
 
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
@@ -30,11 +33,13 @@ import org.slf4j.LoggerFactory;
 
 import brooklyn.catalog.internal.CatalogUtils;
 import brooklyn.rest.domain.ApiError;
+import brooklyn.rest.util.json.BrooklynJacksonJsonProvider;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.net.Urls;
 import brooklyn.util.text.StringEscapes.JavaStringEscapes;
 
 import com.google.common.collect.ImmutableMap;
+import com.sun.jersey.spi.container.ContainerResponse;
 
 public class WebResourceUtils {
 
@@ -47,9 +52,9 @@ public class WebResourceUtils {
             log.debug("responding {} {} ({})",
                     new Object[]{status.getStatusCode(), status.getReasonPhrase(), msg});
         }
-        throw new WebApplicationException(Response.status(status)
-                .type(MediaType.APPLICATION_JSON_TYPE)
-                .entity(ApiError.builder().message(msg).build()).build());
+        ApiError apiError = ApiError.builder().message(msg).errorCode(status).build();
+        // including a Throwable is the only way to include a message with the WebApplicationException
- ugly!
+        throw new WebApplicationException(new Throwable(apiError.toString()), apiError.asJsonResponse());
     }
 
     /** @throws WebApplicationException With code 500 internal server error */
@@ -67,6 +72,11 @@ public class WebResourceUtils {
         return throwWebApplicationException(Response.Status.UNAUTHORIZED, format, args);
     }
 
+    /** @throws WebApplicationException With code 403 forbidden */
+    public static WebApplicationException forbidden(String format, Object... args) {
+        return throwWebApplicationException(Response.Status.FORBIDDEN, format, args);
+    }
+
     /** @throws WebApplicationException With code 404 not found */
     public static WebApplicationException notFound(String format, Object... args) {
         return throwWebApplicationException(Response.Status.NOT_FOUND, format, args);
@@ -139,4 +149,14 @@ public class WebResourceUtils {
             return Urls.encode(versionedId);
         }
     }
+
+    /** Sets the {@link HttpServletResponse} target (last argument) from the given source
{@link Response};
+     * useful in filters where we might have a {@link Response} and need to set up an {@link
HttpServletResponse}. 
+     * Similar to {@link ContainerResponse#setResponse(Response)}; nothing like that seems
to be available for {@link HttpServletResponse}. */
+    public static void applyJsonResponse(ServletContext servletContext, Response source,
HttpServletResponse target) throws IOException {
+        target.setStatus(source.getStatus());
+        target.setContentType(MediaType.APPLICATION_JSON);
+        target.setCharacterEncoding("UTF-8");
+        target.getWriter().write(BrooklynJacksonJsonProvider.findAnyObjectMapper(servletContext,
null).writeValueAsString(source.getEntity()));
+    }
 }


Mime
View raw message