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, 49 insertions, 101 deletions

diff --git a/container-disc/pom.xml b/container-disc/pom.xml
index 1caf66e29dc..413af786f2c 100644
--- a/container-disc/pom.xml
+++ b/container-disc/pom.xml
@@ -32,6 +32,11 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.assertj</groupId>
+      <artifactId>assertj-core</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
       <groupId>com.yahoo.vespa</groupId>
       <artifactId>config-lib</artifactId>
       <version>${project.version}</version>
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java
index 972f9dd8e18..195aee93246 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java
@@ -5,12 +5,9 @@ import com.yahoo.component.ComponentId;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.container.di.componentgraph.Provider;
 import com.yahoo.container.http.filter.FilterChainRepository;
-import com.yahoo.jdisc.application.BindingRepository;
 import com.yahoo.jdisc.http.ServerConfig;
-import com.yahoo.jdisc.http.filter.RequestFilter;
-import com.yahoo.jdisc.http.filter.ResponseFilter;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilter;
-import com.yahoo.jdisc.http.server.FilterBindings;
+import com.yahoo.jdisc.http.server.jetty.FilterBindings;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -18,28 +15,26 @@ import java.util.List;
 /**
  * Provides filter bindings based on vespa config.
  *
- * @author bakksjo
+ * @author Oyvind Bakksjo
+ * @author bjorncs
  */
 public class FilterBindingsProvider implements Provider<FilterBindings> {
 
-    final BindingRepository<RequestFilter> requestFilters = new BindingRepository<>();
-    final BindingRepository<ResponseFilter> responseFilters = new BindingRepository<>();
+    private final FilterBindings filterBindings;
 
     public FilterBindingsProvider(ComponentId componentId,
                                   ServerConfig config,
                                   FilterChainRepository filterChainRepository,
                                   ComponentRegistry<SecurityRequestFilter> legacyRequestFilters) {
-        ComponentId serverId = componentId.getNamespace();
         try {
-            FilterUtil.setupFilters(
+            this.filterBindings = FilterUtil.setupFilters(
                     componentId,
                     legacyRequestFilters,
                     toFilterSpecs(config.filter()),
-                    filterChainRepository,
-                    requestFilters,
-                    responseFilters);
+                    filterChainRepository);
         } catch (Exception e) {
-            throw new RuntimeException("Invalid config for http server " + serverId, e);
+            throw new RuntimeException(
+                    "Invalid config for http server '" + componentId.getNamespace() + "': " + e.getMessage(), e);
         }
     }
 
@@ -51,10 +46,7 @@ public class FilterBindingsProvider implements Provider<FilterBindings> {
         return outFilters;
     }
 
-    @Override
-    public FilterBindings get() {
-        return new FilterBindings(requestFilters, responseFilters);
-    }
+    @Override public FilterBindings get() { return filterBindings; }
 
     @Override
     public void deconstruct() {}
diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/FilterUtil.java b/container-disc/src/main/java/com/yahoo/container/jdisc/FilterUtil.java
index 24af56788b9..52829d6710e 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/FilterUtil.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/FilterUtil.java
@@ -5,12 +5,11 @@ import com.yahoo.component.ComponentId;
 import com.yahoo.component.ComponentSpecification;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.container.http.filter.FilterChainRepository;
-import com.yahoo.jdisc.application.BindingRepository;
-import com.yahoo.jdisc.application.UriPattern;
 import com.yahoo.jdisc.http.filter.RequestFilter;
 import com.yahoo.jdisc.http.filter.ResponseFilter;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilter;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain;
+import com.yahoo.jdisc.http.server.jetty.FilterBindings;
 
 import java.util.List;
 
@@ -18,18 +17,15 @@ import java.util.List;
  * Helper class to set up filter binding repositories based on config.
  *
  * @author Øyvind Bakksjø
+ * @author bjorncs
  */
 class FilterUtil {
 
     private static final ComponentId SEARCH_SERVER_COMPONENT_ID = ComponentId.fromString("SearchServer");
 
-    private final BindingRepository<RequestFilter> requestFilters;
-    private final BindingRepository<ResponseFilter> responseFilters;
+    private final FilterBindings.Builder builder = new FilterBindings.Builder();
 
-    private FilterUtil(BindingRepository<RequestFilter> requestFilters, BindingRepository<ResponseFilter> responseFilters) {
-        this.requestFilters = requestFilters;
-        this.responseFilters = responseFilters;
-    }
+    private FilterUtil() {}
 
     private void configureFilters(List<FilterSpec> filtersConfig, FilterChainRepository filterChainRepository) {
         for (FilterSpec filterConfig : filtersConfig) {
@@ -37,50 +33,48 @@ class FilterUtil {
             if (filter == null) {
                 throw new RuntimeException("No http filter with id " + filterConfig.getId());
             }
-            addFilter(filter, filterConfig.getBinding());
+            addFilter(filter, filterConfig.getBinding(), filterConfig.getId());
         }
     }
 
-    private void addFilter(Object filter, String binding) {
+    private void addFilter(Object filter, String binding, String filterId) {
         if (filter instanceof RequestFilter && filter instanceof ResponseFilter) {
             throw new RuntimeException("The filter " + filter.getClass().getName() + 
                                        " is unsupported since it's both a RequestFilter and a ResponseFilter.");
         } else if (filter instanceof RequestFilter) {
-            requestFilters.put(new UriPattern(binding), (RequestFilter) filter);
+            builder.addRequestFilter(filterId, binding, (RequestFilter) filter);
         } else if (filter instanceof ResponseFilter) {
-            responseFilters.put(new UriPattern(binding), (ResponseFilter) filter);
+            builder.addResponseFilter(filterId, binding, (ResponseFilter) filter);
         } else {
             throw new RuntimeException("Unknown filter type " + filter.getClass().getName());
         }
     }
 
-    //TVT: remove
+    // TODO(gjoranv): remove
     private void configureLegacyFilters(ComponentId id, ComponentRegistry<SecurityRequestFilter> legacyRequestFilters) {
         ComponentId serverName = id.getNamespace();
         if (SEARCH_SERVER_COMPONENT_ID.equals(serverName) && !legacyRequestFilters.allComponents().isEmpty()) {
-            requestFilters.bind("http://*/*",
-                                SecurityRequestFilterChain.newInstance(legacyRequestFilters.allComponents()));
+            builder.addRequestFilter(
+                    "legacy-filters", "http://*/*", SecurityRequestFilterChain.newInstance(legacyRequestFilters.allComponents()));
         }
     }
 
     /**
      * Populates binding repositories with filters based on config.
-     *
-     * @param requestFilters output argument that will be mutated
-     * @param responseFilters output argument that will be mutated
      */
-    public static void setupFilters(ComponentId componentId,
-                                    ComponentRegistry<SecurityRequestFilter> legacyRequestFilters,
-                                    List<FilterSpec> filtersConfig,
-                                    FilterChainRepository filterChainRepository,
-                                    BindingRepository<RequestFilter> requestFilters,
-                                    BindingRepository<ResponseFilter> responseFilters) {
-        FilterUtil filterUtil = new FilterUtil(requestFilters, responseFilters);
-
-        // TODO: remove
+    public static FilterBindings setupFilters(
+            ComponentId componentId,
+            ComponentRegistry<SecurityRequestFilter> legacyRequestFilters,
+            List<FilterSpec> filtersConfig,
+            FilterChainRepository filterChainRepository) {
+        FilterUtil filterUtil = new FilterUtil();
+
+        // TODO(gjoranv): remove
         filterUtil.configureLegacyFilters(componentId, legacyRequestFilters);
 
         filterUtil.configureFilters(filtersConfig, filterChainRepository);
+
+        return filterUtil.builder.build();
     }
 
     public static class FilterSpec {
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java
index e3039e88525..c233680590d 100644
--- a/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java
+++ b/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java
@@ -5,32 +5,19 @@ import com.yahoo.component.ComponentId;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.container.core.ChainsConfig;
 import com.yahoo.container.http.filter.FilterChainRepository;
-import com.yahoo.jdisc.application.BindingRepository;
-import com.yahoo.jdisc.application.UriPattern;
 import com.yahoo.jdisc.http.ServerConfig;
 import com.yahoo.jdisc.http.filter.RequestFilter;
 import com.yahoo.jdisc.http.filter.ResponseFilter;
-import com.yahoo.jdisc.http.server.FilterBindings;
-import org.hamcrest.Description;
-import org.hamcrest.Matcher;
-import org.hamcrest.TypeSafeMatcher;
+import com.yahoo.jdisc.http.server.jetty.FilterBindings;
 import org.junit.Test;
 
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-
-import static org.hamcrest.CoreMatchers.not;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
 /**
- * @author bakksjo
+ * @author Oyvind Bakksjo
+ * @author bjorncs
  */
 public class FilterBindingsProviderTest {
     final ServerConfig.Builder configBuilder = new ServerConfig.Builder();
@@ -52,9 +39,9 @@ public class FilterBindingsProviderTest {
 
         final FilterBindings filterBindings = provider.get();
 
-        assertNotNull(filterBindings);
-        assertFalse(filterBindings.getRequestFilters().iterator().hasNext());
-        assertFalse(filterBindings.getResponseFilters().iterator().hasNext());
+        assertThat(filterBindings).isNotNull();
+        assertThat(filterBindings.requestFilterIds()).isEmpty();
+        assertThat(filterBindings.responseFilterIds()).isEmpty();
     }
 
     @Test
@@ -105,19 +92,11 @@ public class FilterBindingsProviderTest {
         final FilterBindings filterBindings = provider.get();
 
         // Verify.
-        assertNotNull(filterBindings);
-        assertThat(
-                filterBindings.getRequestFilters(),
-                containsFilters(requestFilter1Instance, requestFilter2Instance));
-        assertThat(
-                filterBindings.getRequestFilters(),
-                not(containsFilters(requestFilter3Instance)));
-        assertThat(
-                filterBindings.getResponseFilters(),
-                containsFilters(responseFilter1Instance, responseFilter3Instance));
-        assertThat(
-                filterBindings.getResponseFilters(),
-                not(containsFilters(responseFilter2Instance)));
+        assertThat(filterBindings).isNotNull();
+        assertThat(filterBindings.requestFilters())
+                .containsExactlyInAnyOrder(requestFilter1Instance, requestFilter2Instance);
+        assertThat(filterBindings.responseFilters())
+                .containsExactlyInAnyOrder(responseFilter1Instance, responseFilter3Instance);
     }
 
     private interface DualRoleFilter extends RequestFilter, ResponseFilter {}
@@ -148,7 +127,7 @@ public class FilterBindingsProviderTest {
                     new ComponentRegistry<>());
             fail("Dual-role filter should not be accepted");
         } catch (RuntimeException e) {
-            assertTrue(e.getMessage().contains("Invalid config"));
+            assertThat(e.getMessage()).contains("Invalid config");
         }
     }
 
@@ -173,30 +152,8 @@ public class FilterBindingsProviderTest {
                     new ComponentRegistry<>());
             fail("Config with unknown filter reference should not be accepted");
         } catch (RuntimeException e) {
-            assertTrue(e.getMessage().contains("Invalid config"));
+            assertThat(e.getMessage()).contains("Invalid config");
         }
     }
 
-    @SafeVarargs
-    @SuppressWarnings("varargs")
-    private static <T> Matcher<? super BindingRepository<T>> containsFilters(
-            final T... requiredInstances) {
-        return new TypeSafeMatcher<>() {
-            private final Set<T> requiredFilterSet = new HashSet<>(Arrays.asList(requiredInstances));
-
-            @Override
-            protected boolean matchesSafely(final BindingRepository<T> actualInstances) {
-                final Set<T> notFoundFilterSet = new HashSet<>(requiredFilterSet);
-                for (final Map.Entry<UriPattern, T> actualEntry : actualInstances) {
-                    notFoundFilterSet.remove(actualEntry.getValue());
-                }
-                return notFoundFilterSet.isEmpty();
-            }
-
-            @Override
-            public void describeTo(final Description description) {
-                description.appendText("BindingRepository containing " + requiredFilterSet);
-            }
-        };
-    }
 }