You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2007/11/20 23:58:09 UTC
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...
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
+{
+}