Prepare FilterBindings to allow default filter chains and metrics

Track and expose filter ids from FilterBindings.
Add builder to simplify construction for FilterBindings in provider and unit tests.
Move FilterBindings to com.yahoo.jdisc.http.server.jetty package.
Encapsulate binding matching in FilterBindings.

4 files changed, 70 insertions, 104 deletions

diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java
index 0b94829f720..a978e42f7cb 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java
@@ -7,7 +7,6 @@ import com.yahoo.jdisc.AbstractResource;
 import com.yahoo.jdisc.Request;
 import com.yahoo.jdisc.ResourceReference;
 import com.yahoo.jdisc.Response;
-import com.yahoo.jdisc.application.BindingRepository;
 import com.yahoo.jdisc.handler.AbstractRequestHandler;
 import com.yahoo.jdisc.handler.CompletionHandler;
 import com.yahoo.jdisc.handler.ContentChannel;
@@ -24,7 +23,6 @@ import com.yahoo.jdisc.http.filter.ResponseHeaderFilter;
 import com.yahoo.jdisc.http.filter.chain.RequestFilterChain;
 import com.yahoo.jdisc.http.filter.chain.ResponseFilterChain;
 import com.yahoo.jdisc.http.guiceModules.ConnectorFactoryRegistryModule;
-import com.yahoo.jdisc.http.server.FilterBindings;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
@@ -53,12 +51,12 @@ import static org.mockito.Mockito.when;
 public class FilterTestCase {
     @Test
     public void requireThatRequestFilterIsNotRunOnUnboundPath() throws Exception {
-        final RequestFilter filter = mock(RequestFilterMockBase.class);
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        requestFilters.bind("http://*/filtered/*", filter);
-        final BindingRepository<ResponseFilter> responseFilters = null;
+        RequestFilterMockBase filter = mock(RequestFilterMockBase.class);
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/filtered/*", filter)
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html");
 
@@ -71,11 +69,11 @@ public class FilterTestCase {
     @Test
     public void requireThatRequestFilterIsRunOnBoundPath() throws Exception {
         final RequestFilter filter = mock(RequestFilterMockBase.class);
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        requestFilters.bind("http://*/filtered/*", filter);
-        final BindingRepository<ResponseFilter> responseFilters = null;
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/filtered/*", filter)
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/filtered/status.html");
 
@@ -88,11 +86,11 @@ public class FilterTestCase {
     @Test
     public void requireThatRequestFilterChangesAreSeenByRequestHandler() throws Exception {
         final RequestFilter filter = new HeaderRequestFilter("foo", "bar");
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        requestFilters.bind("http://*/*", filter);
-        final BindingRepository<ResponseFilter> responseFilters = null;
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/*", filter)
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("status.html");
 
@@ -104,11 +102,11 @@ public class FilterTestCase {
 
     @Test
     public void requireThatRequestFilterCanRespond() throws Exception {
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        requestFilters.bind("http://*/*", new RespondForbiddenFilter());
-        final BindingRepository<ResponseFilter> responseFilters = null;
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/*", new RespondForbiddenFilter())
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html").expectStatusCode(is(Response.Status.FORBIDDEN));
 
@@ -121,11 +119,11 @@ public class FilterTestCase {
     public void requireThatFilterCanHaveNullCompletionHandler() throws Exception {
         final int responseStatus = Response.Status.OK;
         final String responseMessage = "Excellent";
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        requestFilters.bind("http://*/*", new NullCompletionHandlerFilter(responseStatus, responseMessage));
-        final BindingRepository<ResponseFilter> responseFilters = null;
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/*", new NullCompletionHandlerFilter(responseStatus, responseMessage))
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html")
                 .expectStatusCode(is(responseStatus))
@@ -138,11 +136,11 @@ public class FilterTestCase {
 
     @Test
     public void requireThatRequestFilterExecutionIsExceptionSafe() throws Exception {
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        final BindingRepository<ResponseFilter> responseFilters = null;
-        requestFilters.bind("http://*/*", new ThrowingRequestFilter());
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/*", new ThrowingRequestFilter())
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html").expectStatusCode(is(Response.Status.INTERNAL_SERVER_ERROR));
 
@@ -154,11 +152,11 @@ public class FilterTestCase {
     @Test
     public void requireThatResponseFilterIsNotRunOnUnboundPath() throws Exception {
         final ResponseFilter filter = mock(ResponseFilterMockBase.class);
-        final BindingRepository<RequestFilter> requestFilters = null;
-        final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
-        responseFilters.bind("http://*/filtered/*", filter);
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addResponseFilter("my-response-filter", "http://*/filtered/*", filter)
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html");
 
@@ -171,11 +169,11 @@ public class FilterTestCase {
     @Test
     public void requireThatResponseFilterIsRunOnBoundPath() throws Exception {
         final ResponseFilter filter = mock(ResponseFilterMockBase.class);
-        final BindingRepository<RequestFilter> requestFilters = null;
-        final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
-        responseFilters.bind("http://*/filtered/*", filter);
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addResponseFilter("my-response-filter", "http://*/filtered/*", filter)
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/filtered/status.html");
 
@@ -187,11 +185,11 @@ public class FilterTestCase {
 
     @Test
     public void requireThatResponseFilterChangesAreWrittenToResponse() throws Exception {
-        final BindingRepository<RequestFilter> requestFilters = null;
-        final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
-        responseFilters.bind("http://*/*", new HeaderResponseFilter("foo", "bar"));
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addResponseFilter("my-response-filter", "http://*/*", new HeaderResponseFilter("foo", "bar"))
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html")
                 .expectHeader("foo", is("bar"));
@@ -203,11 +201,11 @@ public class FilterTestCase {
 
     @Test
     public void requireThatResponseFilterExecutionIsExceptionSafe() throws Exception {
-        final BindingRepository<RequestFilter> requestFilters = null;
-        final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
-        responseFilters.bind("http://*/*", new ThrowingResponseFilter());
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addResponseFilter("my-response-filter", "http://*/*", new ThrowingResponseFilter())
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html").expectStatusCode(is(Response.Status.INTERNAL_SERVER_ERROR));
 
@@ -218,15 +216,15 @@ public class FilterTestCase {
 
     @Test
     public void requireThatRequestFilterAndResponseFilterCanBindToSamePath() throws Exception {
-        final String uriPattern = "http://*/*";
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
         final RequestFilter requestFilter = mock(RequestFilterMockBase.class);
-        requestFilters.bind(uriPattern, requestFilter);
-        final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
         final ResponseFilter responseFilter = mock(ResponseFilterMockBase.class);
-        responseFilters.bind(uriPattern, responseFilter);
+        final String uriPattern = "http://*/*";
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", uriPattern, requestFilter)
+                .addResponseFilter("my-response-filter", uriPattern, responseFilter)
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html");
 
@@ -239,12 +237,12 @@ public class FilterTestCase {
 
     @Test
     public void requireThatResponseFromRequestFilterGoesThroughResponseFilter() throws Exception {
-        final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-        requestFilters.bind("http://*/*", new RespondForbiddenFilter());
-        final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
-        responseFilters.bind("http://*/*", new HeaderResponseFilter("foo", "bar"));
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/*", new RespondForbiddenFilter())
+                .addResponseFilter("my-response-filter", "http://*/*", new HeaderResponseFilter("foo", "bar"))
+                .build();
         final MyRequestHandler requestHandler = new MyRequestHandler();
-        final TestDriver testDriver = newDriver(requestHandler, requestFilters, responseFilters);
+        final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
         testDriver.client().get("/status.html")
                 .expectStatusCode(is(Response.Status.FORBIDDEN))
@@ -384,26 +382,20 @@ public class FilterTestCase {
 
     private static TestDriver newDriver(
             final MyRequestHandler requestHandler,
-            final BindingRepository<RequestFilter> requestFilters,
-            final BindingRepository<ResponseFilter> responseFilters)
+            FilterBindings filterBindings)
             throws IOException {
         return TestDriver.newInstance(
                 JettyHttpServer.class,
                 requestHandler,
-                newFilterModule(requestFilters, responseFilters));
+                newFilterModule(filterBindings));
     }
 
-    private static com.google.inject.Module newFilterModule(
-            final BindingRepository<RequestFilter> requestFilters,
-            final BindingRepository<ResponseFilter> responseFilters) {
+    private static com.google.inject.Module newFilterModule(FilterBindings filterBindings) {
         return Modules.combine(
                 new AbstractModule() {
                     @Override
                     protected void configure() {
-                        bind(FilterBindings.class).toInstance(
-                                new FilterBindings(
-                                        requestFilters != null ? requestFilters : EMPTY_REQUEST_FILTER_REPOSITORY,
-                                        responseFilters != null ? responseFilters : EMPTY_RESPONSE_FILTER_REPOSITORY));
+                        bind(FilterBindings.class).toInstance(filterBindings);
                         bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder()));
                         bind(ConnectorConfig.class).toInstance(new ConnectorConfig(new ConnectorConfig.Builder()));
                         bind(ServletPathsConfig.class).toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder()));
@@ -412,9 +404,6 @@ public class FilterTestCase {
                 new ConnectorFactoryRegistryModule());
     }
 
-    private static final BindingRepository<RequestFilter> EMPTY_REQUEST_FILTER_REPOSITORY = new BindingRepository<>();
-    private static final BindingRepository<ResponseFilter> EMPTY_RESPONSE_FILTER_REPOSITORY = new BindingRepository<>();
-
     private static abstract class RequestFilterMockBase extends AbstractResource implements RequestFilter {}
     private static abstract class ResponseFilterMockBase extends AbstractResource implements ResponseFilter {}
 
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java
index a239dc1ae23..63307dab5ff 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java
@@ -4,11 +4,9 @@ package com.yahoo.jdisc.http.server.jetty;
 import com.google.inject.AbstractModule;
 import com.google.inject.Module;
 import com.google.inject.util.Modules;
-import com.yahoo.jdisc.application.BindingRepository;
 import com.yahoo.jdisc.http.ServerConfig;
 import com.yahoo.jdisc.http.ServletPathsConfig;
 import com.yahoo.jdisc.http.guiceModules.ConnectorFactoryRegistryModule;
-import com.yahoo.jdisc.http.server.FilterBindings;
 import com.yahoo.jdisc.test.ServerProviderConformanceTest;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpVersion;
@@ -770,9 +768,7 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest {
                         @Override
                         protected void configure() {
                             bind(FilterBindings.class)
-                                    .toInstance(new FilterBindings(
-                                            new BindingRepository<>(),
-                                            new BindingRepository<>()));
+                                    .toInstance(new FilterBindings.Builder().build());
                             bind(ServerConfig.class)
                                     .toInstance(new ServerConfig(new ServerConfig.Builder()));
                             bind(ServletPathsConfig.class)
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java
index 255e42fb886..2ff19794ea3 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java
@@ -4,16 +4,12 @@ package com.yahoo.jdisc.http.server.jetty;
 import com.google.inject.AbstractModule;
 import com.google.inject.Module;
 import com.google.inject.util.Modules;
-import com.yahoo.jdisc.application.BindingRepository;
 import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.http.ConnectorConfig;
 import com.yahoo.jdisc.http.ServerConfig;
 import com.yahoo.jdisc.http.ServletPathsConfig;
-import com.yahoo.jdisc.http.filter.RequestFilter;
-import com.yahoo.jdisc.http.filter.ResponseFilter;
 import com.yahoo.jdisc.http.guiceModules.ConnectorFactoryRegistryModule;
 import com.yahoo.jdisc.http.guiceModules.ServletModule;
-import com.yahoo.jdisc.http.server.FilterBindings;
 
 import java.io.IOException;
 import java.nio.file.Path;
@@ -83,10 +79,7 @@ public class TestDrivers {
                         bind(ServletPathsConfig.class).toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder()));
                         bind(ServerConfig.class).toInstance(new ServerConfig(serverConfig));
                         bind(ConnectorConfig.class).toInstance(new ConnectorConfig(connectorConfigBuilder));
-                        bind(FilterBindings.class).toInstance(
-                                new FilterBindings(
-                                        new BindingRepository<>(),
-                                        new BindingRepository<>()));
+                        bind(FilterBindings.class).toInstance(new FilterBindings.Builder().build());
                     }
                 },
                 new ConnectorFactoryRegistryModule(connectorConfigBuilder),
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java
index 5ad76c6348f..272d6fbb66c 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java
@@ -7,13 +7,12 @@ import com.google.inject.util.Modules;
 import com.yahoo.jdisc.AbstractResource;
 import com.yahoo.jdisc.Request;
 import com.yahoo.jdisc.Response;
-import com.yahoo.jdisc.application.BindingRepository;
 import com.yahoo.jdisc.handler.ContentChannel;
 import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.HttpRequest;
 import com.yahoo.jdisc.http.filter.RequestFilter;
 import com.yahoo.jdisc.http.filter.ResponseFilter;
-import com.yahoo.jdisc.http.server.FilterBindings;
+import com.yahoo.jdisc.http.server.jetty.FilterBindings;
 import com.yahoo.jdisc.http.server.jetty.FilterInvoker;
 import com.yahoo.jdisc.http.server.jetty.SimpleHttpClient.ResponseValidator;
 import com.yahoo.jdisc.http.server.jetty.TestDriver;
@@ -79,21 +78,27 @@ public class JDiscFilterForServletTest extends ServletTestBase {
     }
 
     private TestDriver requestFilterTestDriver() throws IOException {
-        return TestDrivers.newInstance(dummyRequestHandler, bindings(requestFilters(), noBindings()));
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addRequestFilter("my-request-filter", "http://*/*", new TestRequestFilter())
+                .build();
+        return TestDrivers.newInstance(dummyRequestHandler, bindings(filterBindings));
     }
 
     private TestDriver responseFilterTestDriver() throws IOException {
-        return TestDrivers.newInstance(dummyRequestHandler, bindings(noBindings(), responseFilters()));
+        FilterBindings filterBindings = new FilterBindings.Builder()
+                .addResponseFilter("my-response-filter", "http://*/*", new TestResponseFilter())
+                .build();
+        return TestDrivers.newInstance(dummyRequestHandler, bindings(filterBindings));
     }
 
-    private Module bindings(BindingRepository<RequestFilter> requestFilters,
-                            BindingRepository<ResponseFilter> responseFilters) {
 
+
+    private Module bindings(FilterBindings filterBindings) {
         return Modules.combine(
                 new AbstractModule() {
                     @Override
                     protected void configure() {
-                        bind(FilterBindings.class).toInstance(new FilterBindings(requestFilters, responseFilters));
+                        bind(FilterBindings.class).toInstance(filterBindings);
                         bind(FilterInvoker.class).toInstance(new FilterInvoker() {
                             @Override
                             public HttpServletRequest invokeRequestFilterChain(
@@ -122,23 +127,6 @@ public class JDiscFilterForServletTest extends ServletTestBase {
                 guiceModule());
     }
 
-    private BindingRepository<RequestFilter> requestFilters() {
-        BindingRepository<RequestFilter> repository = new BindingRepository<>();
-        repository.bind("http://*/*" , new TestRequestFilter());
-        return repository;
-    }
-
-    private BindingRepository<ResponseFilter> responseFilters() {
-        BindingRepository<ResponseFilter> repository = new BindingRepository<>();
-        repository.bind("http://*/*" , new TestResponseFilter());
-        return repository;
-    }
-
-    private <T> BindingRepository<T> noBindings() {
-        return new BindingRepository<>();
-    }
-
-
     static class TestRequestFilter extends AbstractResource implements RequestFilter {
         static final String simpleName = TestRequestFilter.class.getSimpleName();
         static final String responseContent = "Rejected by " + simpleName;