brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [31/34] brooklyn-server git commit: move common ha filter code to shared class
Date Thu, 18 Feb 2016 15:47:49 GMT
move common ha filter code to shared class

and ensure classes not in scope aren't referenced


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

Branch: refs/heads/master
Commit: 3ab00406bc484ec4d8c667d1a068977a25e2aba6
Parents: 7f047ba
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Thu Feb 18 12:15:08 2016 +0000
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Thu Feb 18 13:28:54 2016 +0000

----------------------------------------------------------------------
 .../rest/filter/HaHotCheckHelperAbstract.java   | 82 ++++++++++++++++++++
 .../rest/filter/HaHotCheckResourceFilter.java   | 72 +++++------------
 .../rest/resources/CatalogResourceTest.java     |  2 +
 .../rest/filter/HaMasterCheckFilter.java        | 35 ++++-----
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |  4 -
 5 files changed, 118 insertions(+), 77 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java
new file mode 100644
index 0000000..7f183a6
--- /dev/null
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java
@@ -0,0 +1,82 @@
+/*
+ * 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.filter;
+
+import java.util.Set;
+
+import javax.ws.rs.core.Response;
+
+import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
+import org.apache.brooklyn.rest.domain.ApiError;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.Beta;
+import com.google.common.collect.ImmutableSet;
+
+@Beta
+public abstract class HaHotCheckHelperAbstract {
+
+    private static final Logger log = LoggerFactory.getLogger(HaHotCheckHelperAbstract.class);
+    
+    public static final String SKIP_CHECK_HEADER = "Brooklyn-Allow-Non-Master-Access";
+
+    private static final Set<ManagementNodeState> HOT_STATES = ImmutableSet.of(
+        ManagementNodeState.MASTER, ManagementNodeState.HOT_STANDBY, ManagementNodeState.HOT_BACKUP);
+
+    /** Returns a string describing the problem if mgmt is null or not running; returns absent
if no problems */
+    public static Maybe<String> getProblemMessageIfServerNotRunning(ManagementContext
mgmt) {
+        if (mgmt==null) return Maybe.of("no management context available");
+        if (!mgmt.isRunning()) return Maybe.of("server no longer running");
+        if (!mgmt.isStartupComplete()) return Maybe.of("server not in required startup-completed
state");
+        return Maybe.absent();
+    }
+    
+    public Maybe<String> getProblemMessageIfServerNotRunning() {
+        return getProblemMessageIfServerNotRunning(mgmt());
+    }
+
+    public Response disallowResponse(String problem, Object info) {
+        log.warn("Disallowing web request as "+problem+": "+info+" (caller should set '"+HaHotCheckHelperAbstract.SKIP_CHECK_HEADER+"'
to force)");
+        return ApiError.builder()
+            .message("This request is only permitted against an active master Brooklyn server")
+            .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse();
+    }
+
+    public boolean isSkipCheckHeaderSet(String headerValueString) {
+        return "true".equalsIgnoreCase(headerValueString);
+    }
+
+    public boolean isHaHotStatus() {
+        ManagementNodeState state = mgmt().getHighAvailabilityManager().getNodeState();
+        return HOT_STATES.contains(state);
+    }
+
+    public abstract ManagementContext mgmt();
+
+    // Maybe there should be a separate state to indicate that we have switched state
+    // but still haven't finished rebinding. (Previously there was a time delay and an
+    // isRebinding check, but introducing RebindManager#isAwaitingInitialRebind() seems cleaner.)
+    public boolean isStateNotYetValid() {
+        return mgmt().getRebindManager().isAwaitingInitialRebind();
+    }
+    
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java
index 13c5cbf..3c9c129 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java
@@ -21,25 +21,20 @@ package org.apache.brooklyn.rest.filter;
 import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
-import java.util.Set;
 
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerRequestFilter;
 import javax.ws.rs.container.ResourceInfo;
 import javax.ws.rs.core.Context;
-import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.ContextResolver;
 import javax.ws.rs.ext.Provider;
 
 import org.apache.brooklyn.api.mgmt.ManagementContext;
-import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
-import org.apache.brooklyn.rest.domain.ApiError;
+import org.apache.brooklyn.rest.util.BrooklynRestResourceUtils;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.text.Strings;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableSet;
 
 /** 
  * Checks that if the method or resource class corresponding to a request
@@ -52,13 +47,8 @@ import com.google.common.collect.ImmutableSet;
  */
 @Provider
 public class HaHotCheckResourceFilter implements ContainerRequestFilter {
-    public static final String SKIP_CHECK_HEADER = "Brooklyn-Allow-Non-Master-Access";
+    public static final String SKIP_CHECK_HEADER = HaHotCheckHelperAbstract.SKIP_CHECK_HEADER;
     
-    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);
-
     // Not quite standards compliant. Should instead be:
     // @Context Providers providers
     // ....
@@ -70,6 +60,12 @@ public class HaHotCheckResourceFilter implements ContainerRequestFilter
{
     @Context
     private ResourceInfo resourceInfo;
     
+    private HaHotCheckHelperAbstract helper = new HaHotCheckHelperAbstract() {
+        public ManagementContext mgmt() {
+            return mgmt.getContext(ManagementContext.class);
+        }
+    };
+    
     public HaHotCheckResourceFilter() {
     }
 
@@ -78,60 +74,44 @@ public class HaHotCheckResourceFilter implements ContainerRequestFilter
{
         this.mgmt = mgmt;
     }
 
-    private ManagementContext mgmt() {
-        return mgmt.getContext(ManagementContext.class);
-    }
-
     @Override
     public void filter(ContainerRequestContext requestContext) throws IOException {
         String problem = lookForProblem(requestContext);
         if (Strings.isNonBlank(problem)) {
-            log.warn("Disallowing web request as "+problem+": "+requestContext.getUriInfo()+"/"+resourceInfo.getResourceMethod()+"
(caller should set '"+SKIP_CHECK_HEADER+"' to force)");
-            requestContext.abortWith(ApiError.builder()
-                .message("This request is only permitted against an active hot Brooklyn server")
-                .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse());
+            requestContext.abortWith(helper.disallowResponse(problem, requestContext.getUriInfo()+"/"+resourceInfo.getResourceMethod()));
         }
     }
 
     private String lookForProblem(ContainerRequestContext requestContext) {
-        if (isSkipCheckHeaderSet(requestContext)) 
+        if (helper.isSkipCheckHeaderSet(requestContext.getHeaderString(SKIP_CHECK_HEADER)))

             return null;
         
         if (!isHaHotStateRequired())
             return null;
         
-        String problem = lookForProblemIfServerNotRunning(mgmt());
-        if (Strings.isNonBlank(problem)) 
-            return problem;
+        Maybe<String> problem = helper.getProblemMessageIfServerNotRunning();
+        if (problem.isPresent()) 
+            return problem.get();
         
-        if (!isHaHotStatus())
+        if (!helper.isHaHotStatus())
             return "server not in required HA hot state";
-        if (isStateNotYetValid())
+        if (helper.isStateNotYetValid())
             return "server not yet completed loading data for required HA hot state";
         
         return null;
     }
-    
+
+    /** @deprecated since 0.9.0 use {@link BrooklynRestResourceUtils#getProblemMessageIfServerNotRunning(ManagementContext)}
*/
     public static String lookForProblemIfServerNotRunning(ManagementContext mgmt) {
-        if (mgmt==null) return "no management context available";
-        if (!mgmt.isRunning()) return "server no longer running";
-        if (!mgmt.isStartupComplete()) return "server not in required startup-completed state";
-        return null;
+        return HaHotCheckHelperAbstract.getProblemMessageIfServerNotRunning(mgmt).orNull();
     }
     
-    // Maybe there should be a separate state to indicate that we have switched state
-    // but still haven't finished rebinding. (Previously there was a time delay and an
-    // isRebinding check, but introducing RebindManager#isAwaitingInitialRebind() seems cleaner.)
-    private boolean isStateNotYetValid() {
-        return mgmt().getRebindManager().isAwaitingInitialRebind();
-    }
-
-    private boolean isHaHotStateRequired() {
+    protected boolean isHaHotStateRequired() {
         // TODO support super annotations
         Method m = resourceInfo.getResourceMethod();
         return getAnnotation(m, HaHotStateRequired.class) != null;
     }
-    
+
     private <T extends Annotation> T getAnnotation(Method m, Class<T> annotation)
{
         T am = m.getAnnotation(annotation);
         if (am != null) {
@@ -146,14 +126,4 @@ public class HaHotCheckResourceFilter implements ContainerRequestFilter
{
         return null;
     }
 
-    private boolean isSkipCheckHeaderSet(ContainerRequestContext requestContext) {
-        return "true".equalsIgnoreCase(requestContext.getHeaderString(SKIP_CHECK_HEADER));
-    }
-
-    private boolean isHaHotStatus() {
-        ManagementNodeState state = mgmt().getHighAvailabilityManager().getNodeState();
-        return HOT_STATES.contains(state);
-    }
-
-
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
index 8d5d017..f3374dc 100644
--- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
+++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
@@ -126,6 +126,7 @@ public class CatalogResourceTest extends BrooklynRestResourceTest {
         // an InterfacesTag should be created for every catalog item
         assertEquals(entityItem.getTags().size(), 1);
         Object tag = entityItem.getTags().iterator().next();
+        @SuppressWarnings("unchecked")
         List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
         List<Class<?>> expectedInterfaces = Reflections.getAllInterfaces(TestEntity.class);
         assertEquals(actualInterfaces.size(), expectedInterfaces.size());
@@ -138,6 +139,7 @@ public class CatalogResourceTest extends BrooklynRestResourceTest {
     }
 
     @Test
+    // osgi may fail in IDE, typically works on CLI though
     public void testRegisterOsgiPolicyTopLevelSyntax() {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java
b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java
index 991d58c..bfb1caf 100644
--- a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java
+++ b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java
@@ -30,18 +30,14 @@ 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;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
-import org.apache.brooklyn.rest.domain.ApiError;
+import org.apache.brooklyn.rest.util.OsgiCompat;
 import org.apache.brooklyn.rest.util.WebResourceUtils;
-import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.guava.Maybe;
 
 import com.google.common.collect.Sets;
-import org.apache.brooklyn.rest.util.OsgiCompat;
 
 /**
  * Checks that for requests that want HA master state, the server is up and in that state.
@@ -52,13 +48,17 @@ import org.apache.brooklyn.rest.util.OsgiCompat;
 // TODO Merge with HaHotCheckResourceFilter so the functionality is available in Karaf
 public class HaMasterCheckFilter implements Filter {
 
-    private static final Logger log = LoggerFactory.getLogger(HaMasterCheckFilter.class);
-    
     private static final Set<String> SAFE_STANDBY_METHODS = Sets.newHashSet("GET",
"HEAD");
 
     protected ServletContext servletContext;
     protected ManagementContext mgmt;
 
+    private HaHotCheckHelperAbstract helper = new HaHotCheckHelperAbstract() {
+        public ManagementContext mgmt() {
+            return mgmt;
+        }
+    };
+
     @Override
     public void init(FilterConfig config) throws ServletException {
         servletContext = config.getServletContext();
@@ -66,15 +66,15 @@ public class HaMasterCheckFilter implements Filter {
     }
 
     private String lookForProblem(ServletRequest request) {
-        if (isSkipCheckHeaderSet(request)) 
+        if (helper.isSkipCheckHeaderSet(((HttpServletRequest)request).getHeader(HaHotCheckHelperAbstract.SKIP_CHECK_HEADER)))

             return null;
         
         if (!isMasterRequiredForRequest(request))
             return null;
         
-        String problem = HaHotCheckResourceFilter.lookForProblemIfServerNotRunning(mgmt);
-        if (Strings.isNonBlank(problem)) 
-            return problem;
+        Maybe<String> problem = helper.getProblemMessageIfServerNotRunning();
+        if (problem.isPresent()) 
+            return problem.get();
         
         if (!isMaster()) 
             return "server not in required HA master state";
@@ -86,10 +86,7 @@ public class HaMasterCheckFilter implements Filter {
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
         String problem = lookForProblem(request);
         if (problem!=null) {
-            log.warn("Disallowing web request as "+problem+": "+request.getParameterMap()+"
(caller should set '"+HaHotCheckResourceFilter.SKIP_CHECK_HEADER+"' to force)");
-            WebResourceUtils.applyJsonResponse(servletContext, ApiError.builder()
-                .message("This request is only permitted against an active master Brooklyn
server")
-                .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse(), (HttpServletResponse)response);
+            WebResourceUtils.applyJsonResponse(servletContext, helper.disallowResponse(problem,
request.getParameterMap()), (HttpServletResponse)response);
         } else {
             chain.doFilter(request, response);
         }
@@ -124,10 +121,4 @@ public class HaMasterCheckFilter implements Filter {
         return true;
     }
 
-    private boolean isSkipCheckHeaderSet(ServletRequest httpRequest) {
-        if (httpRequest instanceof HttpServletRequest)
-            return "true".equalsIgnoreCase(((HttpServletRequest)httpRequest).getHeader(HaHotCheckResourceFilter.SKIP_CHECK_HEADER));
-        return false;
-    }
-
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
index 8d71dcb..d9f0f1a 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -36,7 +36,6 @@ import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
 import org.apache.brooklyn.core.server.BrooklynServiceAttributes;
-import org.apache.brooklyn.rest.apidoc.RestApiResourceScanner;
 import org.apache.brooklyn.rest.filter.BrooklynPropertiesSecurityFilter;
 import org.apache.brooklyn.rest.filter.HaMasterCheckFilter;
 import org.apache.brooklyn.rest.filter.LoggingFilter;
@@ -44,7 +43,6 @@ import org.apache.brooklyn.rest.filter.RequestTaggingFilter;
 import org.apache.brooklyn.rest.security.provider.AnyoneSecurityProvider;
 import org.apache.brooklyn.rest.security.provider.SecurityProvider;
 import org.apache.brooklyn.rest.util.ManagementContextProvider;
-import org.apache.brooklyn.rest.util.OsgiCompat;
 import org.apache.brooklyn.rest.util.ServerStoppingShutdownHandler;
 import org.apache.brooklyn.rest.util.ShutdownHandlerProvider;
 import org.apache.brooklyn.util.core.ResourceUtils;
@@ -68,8 +66,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.io.Files;
 
-import io.swagger.config.ScannerFactory;
-
 /** Convenience and demo for launching programmatically. Also used for automated tests.
  * <p>
  * BrooklynLauncher has a more full-featured CLI way to start, 


Mime
View raw message