tapestry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hls...@apache.org
Subject svn commit: r596866 - in /tapestry/tapestry5/trunk/tapestry-ioc/src: main/java/org/apache/tapestry/ioc/annotations/ main/java/org/apache/tapestry/ioc/internal/ main/java/org/apache/tapestry/ioc/internal/util/ main/resources/org/apache/tapestry/ioc/inte...
Date Tue, 20 Nov 2007 22:58:09 GMT
Author: hlship
Date: Tue Nov 20 14:58:08 2007
New Revision: 596866

URL: http://svn.apache.org/viewvc?rev=596866&view=rev
Log:
TAPESTRY-1863: Tapestry should verify that marker annotations have retention type runtime

Added:
    tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/NotRetainedRuntime.java
Modified:
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/annotations/Marker.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ServiceBinderImpl.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/InternalUtils.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/UtilMessages.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/resources/org/apache/tapestry/ioc/internal/util/UtilStrings.properties
    tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/InternalUtilsTest.java

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/annotations/Marker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/annotations/Marker.java?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/annotations/Marker.java
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/annotations/Marker.java
Tue Nov 20 14:58:08 2007
@@ -24,13 +24,15 @@
 import java.lang.annotation.Target;
 
 /**
- * Used to define a {@linkplain ServiceDef#getMarkers() marker annotation} for a service
+ * Used to define one or more {@linkplain ServiceDef#getMarkers() marker annotations} for
a service
  * implementation. This allows for injection based on the combination of type and marker
interface.
  * These marker interfaces should not have any values. The mere presence of the marker annotation
is
  * all that is needed.
  * <p/>
- * When applied to a module class, this sets the default marker for all services within the
module
- * (whereas the normal default marker is null).
+ * When applied to a module class, this sets the default markers for all services within
the module.  Markers
+ * are additive, so a Marker annotation on the implementation class and/or specified with
+ * {@link org.apache.tapestry.ioc.ServiceBindingOptions#withMarker(Class[])} will accumulate;
a service may
+ * have any number of markers.  Generally one or two is enough.
  */
 @Target(
         {TYPE, METHOD})
@@ -41,5 +43,5 @@
     /**
      * The type of annotation (which will be present at the injection point).
      */
-    Class value();
+    Class[] value();
 }

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
Tue Nov 20 14:58:08 2007
@@ -86,8 +86,8 @@
 
     /**
      * @param builderClass the class that is responsible for building services, etc.
-     * @param logger
-     * @param classFactory TODO
+     * @param logger       based on the class name of the module
+     * @param classFactory factory used to create new classes at runtime or locate method
line numbers for error reporting
      */
     public DefaultModuleDefImpl(Class<?> builderClass, Logger logger, ClassFactory
classFactory)
     {
@@ -97,7 +97,11 @@
 
         Marker annotation = builderClass.getAnnotation(Marker.class);
 
-        if (annotation != null) _defaultMarkers.addAll(Arrays.asList(annotation.value()));
+        if (annotation != null)
+        {
+            InternalUtils.validateMarkerAnnotations(annotation.value());
+            _defaultMarkers.addAll(Arrays.asList(annotation.value()));
+        }
 
         grind();
         bind();
@@ -140,8 +144,7 @@
             {
                 int result = o1.getName().compareTo(o2.getName());
 
-                if (result == 0)
-                    result = o2.getParameterTypes().length - o1.getParameterTypes().length;
+                if (result == 0) result = o2.getParameterTypes().length - o1.getParameterTypes().length;
 
                 return result;
             }
@@ -180,8 +183,7 @@
         String serviceId = stripMethodPrefix(method, CONTRIBUTE_METHOD_NAME_PREFIX);
 
         Class returnType = method.getReturnType();
-        if (!returnType.equals(void.class))
-            _logger.warn(IOCMessages.contributionWrongReturnType(method));
+        if (!returnType.equals(void.class)) _logger.warn(IOCMessages.contributionWrongReturnType(method));
 
         ConfigurationType type = null;
 
@@ -245,11 +247,9 @@
 
         // TODO: Validate constraints here?
 
-        String[] patterns = match == null ? new String[]
-                {decoratorId} : match.value();
+        String[] patterns = match == null ? new String[]{decoratorId} : match.value();
 
-        DecoratorDef def = new DecoratorDefImpl(decoratorId, method, patterns, constraints,
-                                                _classFactory);
+        DecoratorDef def = new DecoratorDefImpl(decoratorId, method, patterns, constraints,
_classFactory);
 
         _decoratorDefs.put(decoratorId, def);
     }
@@ -315,8 +315,7 @@
         Set<Class> markers = newSet(_defaultMarkers);
         markers.addAll(extractMarkers(method));
 
-        ServiceDefImpl serviceDef = new ServiceDefImpl(returnType, serviceId, markers, scope,
-                                                       eagerLoad, source);
+        ServiceDefImpl serviceDef = new ServiceDefImpl(returnType, serviceId, markers, scope,
eagerLoad, source);
 
         addServiceDef(serviceDef);
     }
@@ -381,9 +380,7 @@
 
             if (!Modifier.isStatic(bindMethod.getModifiers()))
             {
-                _logger.error(IOCMessages.bindMethodMustBeStatic(InternalUtils.asString(
-                        bindMethod,
-                        _classFactory)));
+                _logger.error(IOCMessages.bindMethodMustBeStatic(InternalUtils.asString(bindMethod,
_classFactory)));
 
                 return;
             }

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ServiceBinderImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ServiceBinderImpl.java?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ServiceBinderImpl.java
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/ServiceBinderImpl.java
Tue Nov 20 14:58:08 2007
@@ -41,8 +41,7 @@
 
     private final Set<Class> _defaultMarkers;
 
-    public ServiceBinderImpl(ServiceDefAccumulator accumulator, ClassFactory classFactory,
-                             Set<Class> defaultMarkers)
+    public ServiceBinderImpl(ServiceDefAccumulator accumulator, ClassFactory classFactory,
Set<Class> defaultMarkers)
     {
         _accumulator = accumulator;
         _classFactory = classFactory;
@@ -91,8 +90,7 @@
         Set<Class> markers = CollectionFactory.newSet(_defaultMarkers);
         markers.addAll(_markers);
 
-        ServiceDef serviceDef = new ServiceDefImpl(_serviceInterface, _serviceId, markers,
_scope,
-                                                   _eagerLoad, source);
+        ServiceDef serviceDef = new ServiceDefImpl(_serviceInterface, _serviceId, markers,
_scope, _eagerLoad, source);
 
         _accumulator.addServiceDef(serviceDef);
 
@@ -108,9 +106,8 @@
     {
         Constructor result = InternalUtils.findAutobuildConstructor(_serviceImplementation);
 
-        if (result == null)
-            throw new RuntimeException(IOCMessages
-                    .noConstructor(_serviceImplementation, _serviceId));
+        if (result == null) throw new RuntimeException(IOCMessages
+                .noConstructor(_serviceImplementation, _serviceId));
 
         return result;
     }
@@ -120,8 +117,7 @@
         return bind(implementationClass, implementationClass);
     }
 
-    public <T> ServiceBindingOptions bind(Class<T> serviceInterface,
-                                          Class<? extends T> serviceImplementation)
+    public <T> ServiceBindingOptions bind(Class<T> serviceInterface, Class<?
extends T> serviceImplementation)
     {
         notNull(serviceInterface, "serviceIterface");
         notNull(serviceImplementation, "serviceImplementation");
@@ -144,7 +140,11 @@
 
         Marker marker = serviceImplementation.getAnnotation(Marker.class);
 
-        if (marker != null) _markers.addAll(Arrays.asList(marker.value()));
+        if (marker != null)
+        {
+            InternalUtils.validateMarkerAnnotations(marker.value());
+            _markers.addAll(Arrays.asList(marker.value()));
+        }
 
         return this;
     }
@@ -183,6 +183,8 @@
     public <T extends Annotation> ServiceBindingOptions withMarker(Class<T>...
marker)
     {
         _lock.check();
+
+        InternalUtils.validateMarkerAnnotations(marker);
 
         _markers.addAll(Arrays.asList(marker));
 

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/InternalUtils.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/InternalUtils.java?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/InternalUtils.java
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/InternalUtils.java
Tue Nov 20 14:58:08 2007
@@ -25,6 +25,8 @@
 import org.apache.tapestry.ioc.services.ClassFactory;
 
 import java.lang.annotation.Annotation;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -158,8 +160,7 @@
      * @param annotationClass to match
      * @return the annotation instance, if found, or null otherwise
      */
-    public static <T extends Annotation> T findAnnotation(Annotation[] annotations,
-                                                          Class<T> annotationClass)
+    public static <T extends Annotation> T findAnnotation(Annotation[] annotations,
Class<T> annotationClass)
     {
         for (Annotation a : annotations)
         {
@@ -170,9 +171,8 @@
     }
 
     @SuppressWarnings("unchecked")
-    private static Object calculateParameterValue(Class parameterType,
-                                                  final Annotation[] parameterAnnotations,
ObjectLocator locator,
-                                                  Map<Class, Object> parameterDefaults)
+    private static Object calculateParameterValue(Class parameterType, final Annotation[]
parameterAnnotations,
+                                                  ObjectLocator locator, Map<Class, Object>
parameterDefaults)
     {
         AnnotationProvider provider = new AnnotationProvider()
         {
@@ -220,8 +220,7 @@
         return calculateParameters(locator, parameterDefaults, parameterTypes, annotations);
     }
 
-    public static Object[] calculateParametersForConstructor(Constructor constructor,
-                                                             ObjectLocator locator,
+    public static Object[] calculateParametersForConstructor(Constructor constructor, ObjectLocator
locator,
                                                              Map<Class, Object> parameterDefaults)
     {
         Class[] parameterTypes = constructor.getParameterTypes();
@@ -230,9 +229,8 @@
         return calculateParameters(locator, parameterDefaults, parameterTypes, annotations);
     }
 
-    public static Object[] calculateParameters(ObjectLocator locator,
-                                               Map<Class, Object> parameterDefaults,
Class[] parameterTypes,
-                                               Annotation[][] parameterAnnotations)
+    public static Object[] calculateParameters(ObjectLocator locator, Map<Class, Object>
parameterDefaults,
+                                               Class[] parameterTypes, Annotation[][] parameterAnnotations)
     {
         int parameterCount = parameterTypes.length;
 
@@ -240,11 +238,8 @@
 
         for (int i = 0; i < parameterCount; i++)
         {
-            parameters[i] = calculateParameterValue(
-                    parameterTypes[i],
-                    parameterAnnotations[i],
-                    locator,
-                    parameterDefaults);
+            parameters[i] = calculateParameterValue(parameterTypes[i], parameterAnnotations[i],
locator,
+                                                    parameterDefaults);
         }
 
         return parameters;
@@ -487,5 +482,24 @@
         }
 
         list.add(value);
+    }
+
+    /**
+     * Validates that the marker annotation class had a retention policy of runtime.
+     *
+     * @param markerClass the marker annotation class
+     */
+    public static void validateMarkerAnnotation(Class markerClass)
+    {
+        Retention policy = (Retention) markerClass.getAnnotation(Retention.class);
+
+        if (policy != null && policy.value() == RetentionPolicy.RUNTIME) return;
+
+        throw new IllegalArgumentException(UtilMessages.badMarkerAnnotation(markerClass));
+    }
+
+    public static void validateMarkerAnnotations(Class[] markerClasses)
+    {
+        for (Class markerClass : markerClasses) validateMarkerAnnotation(markerClass);
     }
 }

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/UtilMessages.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/UtilMessages.java?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/UtilMessages.java
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry/ioc/internal/util/UtilMessages.java
Tue Nov 20 14:58:08 2007
@@ -58,4 +58,9 @@
     {
         return MESSAGES.format("bad-cast", parameterName, parameterValue, type.getName());
     }
+
+    static String badMarkerAnnotation(Class annotationClass)
+    {
+        return MESSAGES.format("bad-marker-annotation", annotationClass.getName());
+    }
 }

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/resources/org/apache/tapestry/ioc/internal/util/UtilStrings.properties
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/resources/org/apache/tapestry/ioc/internal/util/UtilStrings.properties?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/resources/org/apache/tapestry/ioc/internal/util/UtilStrings.properties
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/resources/org/apache/tapestry/ioc/internal/util/UtilStrings.properties
Tue Nov 20 14:58:08 2007
@@ -1,17 +1,17 @@
-# Copyright 2006 The Apache Software Foundation
-#
-# Licensed 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.
-
+# Copyright 2006 The Apache Software Foundation
+#
+# Licensed 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.
+
 dependency-cycle=Unable to add '%s' as a dependency of '%s', as that forms a dependency cycle
('%<s' depends on itself via '%1$s'). The dependency has been ignored.
 duplicate-orderer=Could not add object with duplicate id '%s'.  The duplicate object has
been ignored.
 constraint-format=Could not parse ordering constraint '%s' (for '%s'). The constraint has
been ignored.
@@ -19,3 +19,4 @@
 parameter-was-null=Parameter %s was null.
 parameter-was-blank=Parameter %s was null or contained only whitespace.
 bad-cast=Parameter %s (%s) is not assignable to type %s.
+bad-marker-annotation=Marker annotation class %s is not valid because it is not visible at
runtime. Add a @RetentionPolicy(RUNTIME) to the class.

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/InternalUtilsTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/InternalUtilsTest.java?rev=596866&r1=596865&r2=596866&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/InternalUtilsTest.java
(original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/InternalUtilsTest.java
Tue Nov 20 14:58:08 2007
@@ -16,6 +16,7 @@
 
 import org.apache.tapestry.ioc.Locatable;
 import org.apache.tapestry.ioc.Location;
+import org.apache.tapestry.ioc.annotations.Inject;
 import static org.apache.tapestry.ioc.internal.util.CollectionFactory.newMap;
 import static org.apache.tapestry.ioc.internal.util.InternalUtils.toList;
 import org.apache.tapestry.ioc.test.IOCTestCase;
@@ -77,8 +78,7 @@
     @Test
     public void array_size_when_non_null()
     {
-        Object[] array =
-                {1, 2, 3};
+        Object[] array = {1, 2, 3};
 
         assertEquals(InternalUtils.size(array), 3);
     }
@@ -92,12 +92,8 @@
     @DataProvider(name = "memberPrefixData")
     public Object[][] memberPrefixData()
     {
-        return new Object[][]
-                {
-                        {"simple", "simple"},
-                        {"_name", "name"},
-                        {"$name", "name"},
-                        {"$_$__$__$_$___$_$_$_$$name$", "name$"}};
+        return new Object[][]{{"simple", "simple"}, {"_name", "name"}, {"$name", "name"},
+                              {"$_$__$__$_$___$_$_$_$$name$", "name$"}};
     }
 
     @Test
@@ -156,13 +152,7 @@
     @DataProvider(name = "capitalize_inputs")
     public Object[][] capitalize_inputs()
     {
-        return new Object[][]
-                {
-                        {"hello", "Hello"},
-                        {"Goodbye", "Goodbye"},
-                        {"", ""},
-                        {"a", "A"},
-                        {"A", "A"}};
+        return new Object[][]{{"hello", "Hello"}, {"Goodbye", "Goodbye"}, {"", ""}, {"a",
"A"}, {"A", "A"}};
     }
 
     @Test
@@ -293,5 +283,27 @@
         InternalUtils.addToMapList(map, "fred", 2);
 
         assertEquals(map.get("fred"), Arrays.asList(1, 2));
+    }
+
+    // Test the check for runtime annotations. This is all well and good, we actually don't
have a proper test
+    // that this code is used (ideally we should have tests for @Marker on a module, on a
service impl, and passed
+    // to ServiceBindingOptions.withMarker(), to prove that those are wired for checks.
+
+    @Test
+    public void validate_marker_annotation()
+    {
+        InternalUtils.validateMarkerAnnotation(Inject.class);
+
+
+        try
+        {
+            InternalUtils.validateMarkerAnnotations(new Class[]{Inject.class, NotRetainedRuntime.class});
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertEquals(ex.getMessage(),
+                         "Marker annotation class org.apache.tapestry.ioc.internal.util.NotRetainedRuntime
is not valid because it is not visible at runtime. Add a @RetentionPolicy(RUNTIME) to the
class.");
+        }
     }
 }

Added: tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/NotRetainedRuntime.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/NotRetainedRuntime.java?rev=596866&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/NotRetainedRuntime.java
(added)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry/ioc/internal/util/NotRetainedRuntime.java
Tue Nov 20 14:58:08 2007
@@ -0,0 +1,27 @@
+// Copyright 2007 The Apache Software Foundation
+//
+// Licensed 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.tapestry.ioc.internal.util;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+
+/**
+ * Used for testing; this annotation is NOT retained at runtime.
+ */
+@Target({ElementType.PARAMETER, ElementType.FIELD})
+public @interface NotRetainedRuntime
+{
+}



Mime
View raw message