JERSEY-2739: "NameBinding" results in a deployment failure

 - Ignoring name binding when @PreMatching is present

Change-Id: Ib441dbb02379ec25c7e2aadf5e59d3a07a01ee85
Signed-off-by: Michal Gajdos <[email protected]>

Author: Michal Gajdos

core-server/src/main/java/org/glassfish/jersey/server/ApplicationHandler.java

@@ -780,7 +780,7 @@ public boolean apply(final ContractProvider input) {
      * parameter.
      *
      * @param all                     Collection of all filters to be processed.
-     * @param preMatching             Collection into which pre-matching filters should be added.
+     * @param preMatchingFilters      Collection into which pre-matching filters should be added.
      * @param componentBag            Component bag
      * @param applicationNameBindings Collection of name binding annotations attached to the JAX-RS application.
      * @param inverseNameBoundMap     Inverse name bound map into which the name bound providers should be inserted. The keys
@@ -789,7 +789,7 @@ public boolean apply(final ContractProvider input) {
      */
     private static <T> MultivaluedMap<Class<? extends Annotation>, RankedProvider<T>> filterNameBound(
             final Iterable<RankedProvider<T>> all,
-            final Collection<RankedProvider<ContainerRequestFilter>> preMatching,
+            final Collection<RankedProvider<ContainerRequestFilter>> preMatchingFilters,
             final ComponentBag componentBag,
             final Collection<Class<? extends Annotation>> applicationNameBindings,
             final MultivaluedMap<RankedProvider<T>, Class<? extends Annotation>> inverseNameBoundMap) {
@@ -811,23 +811,30 @@ private static <T> MultivaluedMap<Class<? extends Annotation>, RankedProvider<T>
                 model = ComponentBag.modelFor(providerClass);
             }
 
-            if (preMatching != null && providerClass.getAnnotation(PreMatching.class) != null) {
+            final boolean preMatching = providerClass.getAnnotation(PreMatching.class) != null;
+            if (preMatching && preMatchingFilters != null) {
                 it.remove();
-                preMatching.add(new RankedProvider<>((ContainerRequestFilter) provider.getProvider(),
+                preMatchingFilters.add(new RankedProvider<>((ContainerRequestFilter) provider.getProvider(),
                         model.getPriority(ContainerRequestFilter.class)));
             }
 
             boolean nameBound = model.isNameBound();
-            if (nameBound && !applicationNameBindings.isEmpty() && applicationNameBindings.containsAll(model.getNameBindings())) {
+            if (nameBound
+                    && !applicationNameBindings.isEmpty()
+                    && applicationNameBindings.containsAll(model.getNameBindings())) {
                 // override the name-bound flag
                 nameBound = false;
             }
 
             if (nameBound) { // not application-bound
-                it.remove();
-                for (final Class<? extends Annotation> binding : model.getNameBindings()) {
-                    result.add(binding, provider);
-                    inverseNameBoundMap.add(provider, binding);
+                if (!preMatching) {
+                    it.remove();
+                    for (final Class<? extends Annotation> binding : model.getNameBindings()) {
+                        result.add(binding, provider);
+                        inverseNameBoundMap.add(provider, binding);
+                    }
+                } else {
+                    LOGGER.warning(LocalizationMessages.PREMATCHING_ALSO_NAME_BOUND(providerClass));
                 }
             }
         }

core-server/src/main/resources/org/glassfish/jersey/server/internal/localization.properties

@@ -164,6 +164,7 @@ non.pub.sub.res.method=A sub-resource method, {0}, MUST be public scoped otherwi
 param.null="{0}" parameter is null.
 # {0} = parameter name; {1} = parameter type; {2} = field description
 parameter.unresolvable=Parameter {0} of type {1} from {2} is not resolvable to a concrete type.
+prematching.also.name.bound=@PreMatching provider, {0}, also annotated with a name binding annotation. Name binding will be ignored.
 # {0} = exception class name; {1} = exception message
 property.value.tostring.throws.exception=[{0} thrown from property value toString(): {1}]
 rc.not.modifiable=The resource configuration is not modifiable in this context.

tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/server/NameBindingTest.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 2013-2014 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013-2015 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -47,37 +47,47 @@
 import java.lang.annotation.Target;
 import java.util.Set;
 
+import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
+import javax.ws.rs.HeaderParam;
 import javax.ws.rs.NameBinding;
 import javax.ws.rs.Path;
 import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ContainerRequestFilter;
 import javax.ws.rs.container.ContainerResponseContext;
 import javax.ws.rs.container.ContainerResponseFilter;
+import javax.ws.rs.container.PreMatching;
 import javax.ws.rs.core.Application;
 import javax.ws.rs.core.Response;
 
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
 
 import org.junit.Test;
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 
 import jersey.repackaged.com.google.common.collect.Sets;
 
 /**
- * @author Miroslav Fuksa (miroslav.fuksa at oracle.com)
+ * Test-suite ensuring the correct functionality of name binding.
  *
+ * @author Miroslav Fuksa
+ * @author Michal Gajdos (michal.gajdos at oracle.com)
  */
 public class NameBindingTest extends JerseyTest {
+
     @Override
     protected Application configure() {
         return new ResourceConfig(Resource.class, FooResource.class, BarResource.class, FooBarResource.class, FooFilter.class,
-                BarFilter.class, FooBarFilter.class);
+                BarFilter.class, FooBarFilter.class, PreMatchingFooFilter.class);
     }
 
     @Path("resource")
     public static class Resource {
+
         @GET
         public String noBinding() {
             return "noBinding";
@@ -105,35 +115,53 @@ public String foobar() {
             return "foobar";
         }
 
+        @GET
+        @Path("preMatchingNameBinding")
+        public String preMatchingNameBinding(@HeaderParam("header") @DefaultValue("bar") final String header) {
+            return header;
+        }
     }
 
     @NameBinding
     @Target({ElementType.TYPE, ElementType.METHOD})
     @Retention(value = RetentionPolicy.RUNTIME)
     public static @interface FooBinding {
+
     }
 
     @NameBinding
     @Target({ElementType.TYPE, ElementType.METHOD})
     @Retention(value = RetentionPolicy.RUNTIME)
     public static @interface BarBinding {
-    }
 
+    }
 
     @FooBinding
     public static class FooFilter implements ContainerResponseFilter {
 
         @Override
-        public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException {
+        public void filter(final ContainerRequestContext requestContext, final ContainerResponseContext responseContext) throws
+                IOException {
             responseContext.getHeaders().add(this.getClass().getSimpleName(), "called");
         }
     }
 
+    @PreMatching
+    @FooBinding
+    public static class PreMatchingFooFilter implements ContainerRequestFilter {
+
+        @Override
+        public void filter(final ContainerRequestContext requestContext) throws IOException {
+            requestContext.getHeaders().putSingle("header", "foo");
+        }
+    }
+
     @BarBinding
     public static class BarFilter implements ContainerResponseFilter {
 
         @Override
-        public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException {
+        public void filter(final ContainerRequestContext requestContext, final ContainerResponseContext responseContext) throws
+                IOException {
             responseContext.getHeaders().add(this.getClass().getSimpleName(), "called");
         }
     }
@@ -143,12 +171,12 @@ public void filter(ContainerRequestContext requestContext, ContainerResponseCont
     public static class FooBarFilter implements ContainerResponseFilter {
 
         @Override
-        public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException {
+        public void filter(final ContainerRequestContext requestContext, final ContainerResponseContext responseContext) throws
+                IOException {
             responseContext.getHeaders().add(this.getClass().getSimpleName(), "called");
         }
     }
 
-
     private static final Set<Class<?>> FILTERS = initialize();
 
     private static Set<Class<?>> initialize() {
@@ -159,22 +187,20 @@ private static Set<Class<?>> initialize() {
         return set;
     }
 
-
-    private void checkCalled(Response response, Class<?>... filtersThatShouldBeCalled) {
-        Set<Class<?>> positiveFilters = Sets.newHashSet(filtersThatShouldBeCalled);
-        for (Class<?> filter : FILTERS) {
+    private void checkCalled(final Response response, final Class<?>... filtersThatShouldBeCalled) {
+        final Set<Class<?>> positiveFilters = Sets.newHashSet(filtersThatShouldBeCalled);
+        for (final Class<?> filter : FILTERS) {
             if (positiveFilters.contains(filter)) {
-                assertEquals("Filter '" + filter.getSimpleName() + "' should be called.",
-                        "called", response.getHeaders().getFirst(filter.getSimpleName()));
+                assertEquals("Filter '" + filter.getSimpleName() + "' should be called.", "called", response.getHeaders()
+                        .getFirst(filter.getSimpleName()));
             } else {
-                assertNull("Filter '" + filter.getSimpleName() + "' should not be called.",
-                        response.getHeaders().get(filter.getSimpleName()));
+                assertNull("Filter '" + filter.getSimpleName() + "' should not be called.", response.getHeaders().get(filter
+                        .getSimpleName()));
             }
         }
     }
 
-
-    private Response _getResponse(String path) {
+    private Response _getResponse(final String path) {
         final Response response = target().path(path).request().get();
         assertEquals(200, response.getStatus());
         return response;
@@ -190,6 +216,18 @@ public void testResourceFooBinding() {
         checkCalled(_getResponse("resource/foo"), FooFilter.class);
     }
 
+    /**
+     * Reproducer for JERSEY-2739. Name bound annotation on a pre-matching filter should be ignored and the filter should be
+     * invoked for each resource method (globally).
+     */
+    @Test
+    public void preMatchingNameBinding() {
+        final Response response = _getResponse("resource/preMatchingNameBinding");
+
+        // Request filter - applied, even when the filter is name bound and the resource method is not.
+        assertThat("Name binding on a @PreMatching filter not ignored.", response.readEntity(String.class), is("foo"));
+    }
+
     @Test
     public void testResourceBarBinding() {
         checkCalled(_getResponse("resource/bar"), BarFilter.class);
@@ -200,10 +238,10 @@ public void testResourceFooBarBinding() {
         checkCalled(_getResponse("resource/foobar"), FooFilter.class, BarFilter.class, FooBarFilter.class);
     }
 
-
     @Path("foo-resource")
     @FooBinding
     public static class FooResource extends Resource {
+
     }
 
     @Test
@@ -229,6 +267,7 @@ public void testFooResourceFooBarBinding() {
     @Path("bar-resource")
     @BarBinding
     public static class BarResource extends Resource {
+
     }
 
     @Test
@@ -251,11 +290,11 @@ public void testBarResourceFooBarBinding() {
         checkCalled(_getResponse("bar-resource/foobar"), BarFilter.class, FooFilter.class, FooBarFilter.class);
     }
 
-
     @Path("foobar-resource")
     @BarBinding
     @FooBinding
     public static class FooBarResource extends Resource {
+
     }
 
     @Test
@@ -278,5 +317,4 @@ public void testFooBarResourceFooBarBinding() {
         checkCalled(_getResponse("foobar-resource/foobar"), BarFilter.class, FooFilter.class, FooBarFilter.class);
     }
 
-
 }