AMBARI-11045 - IllegalStateException: Work already begun on this thread. (tbeerbower)

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariHandlerList.java

@@ -17,23 +17,7 @@
  */
 package org.apache.ambari.server.controller;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
-import javax.inject.Inject;
-import javax.inject.Singleton;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
+import org.apache.ambari.server.api.AmbariPersistFilter;
 import org.apache.ambari.server.orm.entities.ViewEntity;
 import org.apache.ambari.server.orm.entities.ViewInstanceEntity;
 import org.apache.ambari.server.view.ViewContextImpl;
@@ -52,6 +36,20 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.filter.DelegatingFilterProxy;
 
+import javax.inject.Inject;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * An Ambari specific extension of the FailsafeHandlerList that allows for the addition
  * of view instances as handlers.
@@ -77,13 +75,23 @@ public class AmbariHandlerList extends HandlerCollection implements ViewInstance
   @Inject
   SessionManager sessionManager;
 
+  /**
+   * The web app context provider.
+   */
   @Inject
-  DelegatingFilterProxy springSecurityFilter;
+  Provider<WebAppContext> webAppContextProvider;
 
   /**
-   * The Handler factory.
+   * The persistence filter.
    */
-  private final HandlerFactory handlerFactory;
+  @Inject
+  AmbariPersistFilter persistFilter;
+
+  /**
+   * The security filter.
+   */
+  @Inject
+  DelegatingFilterProxy springSecurityFilter;
 
   /**
    * Mapping of view instance entities to handlers.
@@ -108,30 +116,6 @@ public class AmbariHandlerList extends HandlerCollection implements ViewInstance
    */
   public AmbariHandlerList() {
     super(true);
-    this.handlerFactory = new HandlerFactory() {
-      @Override
-      public Handler create(ViewInstanceEntity viewInstanceDefinition, String webApp, String contextPath) {
-
-        WebAppContext context = new WebAppContext(webApp, contextPath);
-
-        context.setClassLoader(viewInstanceDefinition.getViewEntity().getClassLoader());
-        context.setAttribute(ViewContext.CONTEXT_ATTRIBUTE, new ViewContextImpl(viewInstanceDefinition, viewRegistry));
-        context.setSessionHandler(new SharedSessionHandler(sessionManager));
-        context.addFilter(new FilterHolder(springSecurityFilter), "/*", 1);
-
-        return context;
-      }
-    };
-  }
-
-  /**
-   * Construct an AmbariHandlerList with the given handler factory.
-   *
-   * @param handlerFactory  the handler factory.
-   */
-  protected AmbariHandlerList(HandlerFactory handlerFactory) {
-    super(true);
-    this.handlerFactory = handlerFactory;
   }
 
 
@@ -241,8 +225,19 @@ public class AmbariHandlerList extends HandlerCollection implements ViewInstance
    */
   private Handler getHandler(ViewInstanceEntity viewInstanceDefinition)
       throws SystemException {
-    ViewEntity viewDefinition = viewInstanceDefinition.getViewEntity();
-    return handlerFactory.create(viewInstanceDefinition, viewDefinition.getArchive(), viewInstanceDefinition.getContextPath());
+
+    ViewEntity    viewDefinition = viewInstanceDefinition.getViewEntity();
+    WebAppContext webAppContext  = webAppContextProvider.get();
+
+    webAppContext.setWar(viewDefinition.getArchive());
+    webAppContext.setContextPath(viewInstanceDefinition.getContextPath());
+    webAppContext.setClassLoader(viewInstanceDefinition.getViewEntity().getClassLoader());
+    webAppContext.setAttribute(ViewContext.CONTEXT_ATTRIBUTE, new ViewContextImpl(viewInstanceDefinition, viewRegistry));
+    webAppContext.setSessionHandler(new SharedSessionHandler(sessionManager));
+    webAppContext.addFilter(new FilterHolder(persistFilter), "/*", 1);
+    webAppContext.addFilter(new FilterHolder(springSecurityFilter), "/*", 1);
+
+    return webAppContext;
   }
 
   /**
@@ -259,24 +254,6 @@ public class AmbariHandlerList extends HandlerCollection implements ViewInstance
   }
 
 
-  // ----- inner interface : HandlerFactory ----------------------------------
-
-  /**
-   * Factory for creating Handler instances.
-   */
-  protected interface HandlerFactory {
-    /**
-     * Create a Handler.
-     *
-     * @param webApp       the web app archive
-     * @param contextPath  the context path
-     *
-     * @return a new Handler instance
-     */
-    public Handler create(ViewInstanceEntity viewInstanceDefinition, String webApp, String contextPath);
-  }
-
-
   // ----- inner class : SharedSessionHandler --------------------------------
 
   /**

ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java

@@ -18,25 +18,30 @@
 
 package org.apache.ambari.server.controller;
 
+import org.apache.ambari.server.api.AmbariPersistFilter;
 import org.apache.ambari.server.orm.entities.ViewEntity;
 import org.apache.ambari.server.orm.entities.ViewInstanceEntity;
 import org.apache.ambari.server.orm.entities.ViewInstanceEntityTest;
 import org.apache.ambari.server.view.ViewRegistry;
+import org.easymock.Capture;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.servlet.FilterHolder;
+import org.eclipse.jetty.webapp.WebAppContext;
 import org.junit.Assert;
 import org.junit.Test;
+import org.springframework.web.filter.DelegatingFilterProxy;
 
-import javax.servlet.ServletException;
+import javax.inject.Provider;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 
+import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.eq;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
@@ -45,27 +50,31 @@ import static org.easymock.EasyMock.verify;
  * AmbariHandlerList tests.
  */
 public class AmbariHandlerListTest {
+
+  private final AmbariPersistFilter persistFilter = createNiceMock(AmbariPersistFilter.class);
+  private final DelegatingFilterProxy springSecurityFilter = createNiceMock(DelegatingFilterProxy.class);
+
+
   @Test
   public void testAddViewInstance() throws Exception {
 
     ViewInstanceEntity viewInstanceEntity = ViewInstanceEntityTest.getViewInstanceEntity();
 
-    final Handler handler = createNiceMock(Handler.class);
+    final WebAppContext handler = createNiceMock(WebAppContext.class);
     Server server = createNiceMock(Server.class);
 
     expect(handler.getServer()).andReturn(server);
     handler.setServer(null);
 
-    replay(handler, server);
+    Capture<FilterHolder> persistFilterCapture = new Capture<FilterHolder>();
+    Capture<FilterHolder> securityFilterCapture = new Capture<FilterHolder>();
+
+    handler.addFilter(capture(persistFilterCapture), eq("/*"), eq(1));
+    handler.addFilter(capture(securityFilterCapture), eq("/*"), eq(1));
 
-    AmbariHandlerList.HandlerFactory handlerFactory = new AmbariHandlerList.HandlerFactory() {
-      @Override
-      public Handler create(ViewInstanceEntity viewInstanceDefinition, String webApp, String contextPath) {
-        return handler;
-      }
-    };
+    replay(handler, server);
 
-    AmbariHandlerList handlerList = new AmbariHandlerList(handlerFactory);
+    AmbariHandlerList handlerList = getAmbariHandlerList(handler);
 
     handlerList.addViewInstance(viewInstanceEntity);
 
@@ -73,6 +82,9 @@ public class AmbariHandlerListTest {
 
     Assert.assertTrue(handlers.contains(handler));
 
+    Assert.assertEquals(persistFilter, persistFilterCapture.getValue().getFilter());
+    Assert.assertEquals(springSecurityFilter, securityFilterCapture.getValue().getFilter());
+
     verify(handler, server);
   }
 
@@ -80,7 +92,7 @@ public class AmbariHandlerListTest {
   public void testRemoveViewInstance() throws Exception {
     ViewInstanceEntity viewInstanceEntity = ViewInstanceEntityTest.getViewInstanceEntity();
 
-    final Handler handler = createNiceMock(Handler.class);
+    final WebAppContext handler = createNiceMock(WebAppContext.class);
     Server server = createNiceMock(Server.class);
 
     expect(handler.getServer()).andReturn(server);
@@ -88,14 +100,7 @@ public class AmbariHandlerListTest {
 
     replay(handler, server);
 
-    AmbariHandlerList.HandlerFactory handlerFactory = new AmbariHandlerList.HandlerFactory() {
-      @Override
-      public Handler create(ViewInstanceEntity viewInstanceDefinition, String webApp, String contextPath) {
-        return handler;
-      }
-    };
-
-    AmbariHandlerList handlerList = new AmbariHandlerList(handlerFactory);
+    AmbariHandlerList handlerList = getAmbariHandlerList(handler);
 
     handlerList.addViewInstance(viewInstanceEntity);
 
@@ -115,8 +120,7 @@ public class AmbariHandlerListTest {
 
   @Test
   public void testHandle() throws Exception {
-    TestHandler handler = new TestHandler();
-    AmbariHandlerList.HandlerFactory handlerFactory = createNiceMock(AmbariHandlerList.HandlerFactory.class);
+    final WebAppContext handler = createNiceMock(WebAppContext.class);
     ViewRegistry viewRegistry = createNiceMock(ViewRegistry.class);
     ViewEntity viewEntity = createNiceMock(ViewEntity.class);
     ClassLoader classLoader = createNiceMock(ClassLoader.class);
@@ -129,41 +133,44 @@ public class AmbariHandlerListTest {
     expect(viewRegistry.getDefinition("TEST", "1.0.0")).andReturn(viewEntity).anyTimes();
     expect(viewEntity.getClassLoader()).andReturn(classLoader).anyTimes();
 
-    replay(viewRegistry, viewEntity);
+    expect(handler.isStarted()).andReturn(true).anyTimes();
+    handler.handle("/api/v1/views/TEST/versions/1.0.0/instances/INSTANCE_1/resources/test",
+        baseRequest, request, response);
+
+    replay(handler, viewRegistry, viewEntity);
 
-    AmbariHandlerList handlerList = new AmbariHandlerList(handlerFactory);
+    AmbariHandlerList handlerList = getAmbariHandlerList(handler);
     handlerList.viewRegistry = viewRegistry;
 
-    handlerList.addHandler(handler);
     handlerList.start();
+    handlerList.addHandler(handler);
     handlerList.handle("/api/v1/views/TEST/versions/1.0.0/instances/INSTANCE_1/resources/test",
         baseRequest, request, response);
 
-    Assert.assertEquals("/api/v1/views/TEST/versions/1.0.0/instances/INSTANCE_1/resources/test", handler.getTarget());
-    Assert.assertEquals(classLoader, handler.getClassLoader());
-
-    verify(viewRegistry, viewEntity);
+    verify(handler, viewRegistry, viewEntity);
   }
 
-  private static class TestHandler extends AbstractHandler {
+  private AmbariHandlerList getAmbariHandlerList(final WebAppContext handler) {
 
-    private ClassLoader classLoader = null;
-    private String target = null;
+    AmbariHandlerList handlerList = new AmbariHandlerList();
 
-    @Override
-    public void handle(String target, Request request,
-                       HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
-        throws IOException, ServletException {
-      this.target = target;
-      classLoader = Thread.currentThread().getContextClassLoader();
-    }
+    handlerList.webAppContextProvider = new HandlerProvider(handler);
+    handlerList.persistFilter = persistFilter;
+    handlerList.springSecurityFilter = springSecurityFilter;
 
-    public ClassLoader getClassLoader() {
-      return classLoader;
+    return handlerList;
+  }
+
+  private static class HandlerProvider implements Provider<WebAppContext> {
+    private final WebAppContext context;
+
+    private HandlerProvider(WebAppContext context) {
+      this.context = context;
     }
 
-    public String getTarget() {
-      return target;
+    @Override
+    public WebAppContext get() {
+      return context;
     }
   }
 }