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/03/05 22:55:18 UTC

svn commit: r514880 - in /tapestry/tapestry5: tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ tapestry-core/trunk/src/main/resources/org/apache/tapestry/internal/services/ tapestry-core/trunk/src/test/conf/ tapestry-ioc/trunk/s...

Author: hlship
Date: Mon Mar  5 13:55:16 2007
New Revision: 514880

URL: http://svn.apache.org/viewvc?view=rev&rev=514880
Log:
TAPESTRY-1291: Make CaseInsensitiveMap threadsafe as long as it isn't being modified (it was completely non-threadsafe before)
TAPESTRY-1291: On an error inside OneShotServiceCreator, log the error and reset the lock flag (in case we try to instantiate the service again, later).
TAPESTRY-1308: Log exceptions as well as rendering the exception report page.
Split up the tapestry-core test suite into a few named tests.

Modified:
    tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/DefaultRequestExceptionHandler.java
    tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/InternalModule.java
    tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java
    tapestry/tapestry5/tapestry-core/trunk/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties
    tapestry/tapestry5/tapestry-core/trunk/src/test/conf/testng.xml
    tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
    tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/IOCMessages.java
    tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
    tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/OneShotServiceCreator.java
    tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/util/CaseInsensitiveMap.java
    tapestry/tapestry5/tapestry-ioc/trunk/src/main/resources/org/apache/tapestry/ioc/internal/IOCStrings.properties
    tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImplTest.java
    tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/OneShotServiceCreatorTest.java

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/DefaultRequestExceptionHandler.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/DefaultRequestExceptionHandler.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/DefaultRequestExceptionHandler.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/DefaultRequestExceptionHandler.java Mon Mar  5 13:55:16 2007
@@ -12,47 +12,51 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry.internal.services;
-
-import java.io.IOException;
-
-import org.apache.tapestry.internal.structure.Page;
-import org.apache.tapestry.services.ExceptionReporter;
-import org.apache.tapestry.services.RequestExceptionHandler;
-import org.apache.tapestry.services.Response;
-
-/**
- * Default implementation of {@link RequestExceptionHandler} that displays the standard
- * ExceptionReport page. The page must implement the {@link ExceptionReporter} interface.
- * 
- * 
- */
-public class DefaultRequestExceptionHandler implements RequestExceptionHandler
-{
-    private final RequestPageCache _pageCache;
-
-    private final PageResponseRenderer _renderer;
-
-    private final Response _response;
-
-    public DefaultRequestExceptionHandler(RequestPageCache pageCache,
-            PageResponseRenderer renderer, Response response)
-    {
-        _pageCache = pageCache;
-        _renderer = renderer;
-        _response = response;
-    }
-
-    public void handleRequestException(Throwable exception) throws IOException
-    {
-        Page page = _pageCache.get("ExceptionReport");
-
-        ExceptionReporter rootComponent = (ExceptionReporter) page.getRootComponent();
-
-        // Let the page set up for the new exception.
-
-        rootComponent.reportException(exception);
-
-        _renderer.renderPageResponse(page, _response);
-    }
-}
+package org.apache.tapestry.internal.services;
+
+import java.io.IOException;
+
+import org.apache.commons.logging.Log;
+import org.apache.tapestry.internal.structure.Page;
+import org.apache.tapestry.services.ExceptionReporter;
+import org.apache.tapestry.services.RequestExceptionHandler;
+import org.apache.tapestry.services.Response;
+
+/**
+ * Default implementation of {@link RequestExceptionHandler} that displays the standard
+ * ExceptionReport page. The page must implement the {@link ExceptionReporter} interface.
+ */
+public class DefaultRequestExceptionHandler implements RequestExceptionHandler
+{
+    private final RequestPageCache _pageCache;
+
+    private final PageResponseRenderer _renderer;
+
+    private final Response _response;
+
+    private final Log _log;
+
+    public DefaultRequestExceptionHandler(RequestPageCache pageCache,
+            PageResponseRenderer renderer, Response response, Log log)
+    {
+        _pageCache = pageCache;
+        _renderer = renderer;
+        _response = response;
+        _log = log;
+    }
+
+    public void handleRequestException(Throwable exception) throws IOException
+    {
+        _log.error(ServicesMessages.requestException(exception), exception);
+
+        Page page = _pageCache.get("ExceptionReport");
+
+        ExceptionReporter rootComponent = (ExceptionReporter) page.getRootComponent();
+
+        // Let the page set up for the new exception.
+
+        rootComponent.reportException(exception);
+
+        _renderer.renderPageResponse(page, _response);
+    }
+}

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/InternalModule.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/InternalModule.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/InternalModule.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/InternalModule.java Mon Mar  5 13:55:16 2007
@@ -542,11 +542,12 @@
         configuration.add("StringLiteral", stringFactory);
     }
 
-    public RequestExceptionHandler buildDefaultRequestExceptionHandler(
-            @InjectService("PageResponseRenderer")
-            PageResponseRenderer renderer)
+    public RequestExceptionHandler buildDefaultRequestExceptionHandler(Log log,
+
+    @InjectService("PageResponseRenderer")
+    PageResponseRenderer renderer)
     {
-        return new DefaultRequestExceptionHandler(_pageCache, renderer, _response);
+        return new DefaultRequestExceptionHandler(_pageCache, renderer, _response, log);
     }
 
     /** Service used to create links for components and pages. */

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java Mon Mar  5 13:55:16 2007
@@ -156,7 +156,6 @@
         return MESSAGES.format("non-private-fields", className, InternalUtils.joinSorted(names));
     }
 
-
     static String compTypeConflict(String embeddedId, String templateType, String modelType)
     {
         return MESSAGES.format("comp-type-conflict", embeddedId, templateType, modelType);
@@ -328,7 +327,6 @@
                 .getClass()), InternalUtils.joinSorted(classNames));
     }
 
-
     static String undefinedTapestryAttribute(String elementName, String attributeName,
             String allowedAttributeName)
     {
@@ -383,5 +381,10 @@
                 propertyName,
                 clazz.getName(),
                 propertyExpression);
+    }
+
+    static String requestException(Throwable cause)
+    {
+        return MESSAGES.format("request-exception", cause);
     }
 }

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties Mon Mar  5 13:55:16 2007
@@ -76,3 +76,4 @@
 method-not-found=No public method '%s' in class %s (within property expression '%s').
 no-such-property=Class %s does not contain a property named '%s' (within property expression '%s').
 write-only-property=Property '%s' of class %s (within property expression '%s') is not readable (it has no read accessor method).
+request-exception=Processing of request failed with uncaught exception: %s
\ No newline at end of file

Modified: tapestry/tapestry5/tapestry-core/trunk/src/test/conf/testng.xml
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/test/conf/testng.xml?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/test/conf/testng.xml (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/test/conf/testng.xml Mon Mar  5 13:55:16 2007
@@ -17,14 +17,35 @@
 
 <suite name="Tapestry Core" parallel="false" thread-count="10" annotations="1.5" verbose="2">
   <parameter name="tapestry.integration-webapp" value="src/test/app1"/>
-  <test name="Tapestry Core">
+  <test name="Integration">
     <packages>
-      <package name="org.apache.tapestry.test.pagelevel"/>
-      <package name="org.apache.tapestry.integration.pagelevel"/>
+      <package name="org.apache.tapestry.integration.pagelevel"/>
       <package name="org.apache.tapestry.integration"/>
-      <package name="org.apache.tapestry"/>
+    </packages>
+  </test>
+  <test name="Page Level Integration">
+    <packages>
+      <package name="org.apache.tapestry.test.pagelevel"/>
+    </packages>
+  </test>
+  <test name="Components">
+    <packages>
       <package name="org.apache.tapestry.corelib.components"/>
+    </packages>
+  </test>
+  <test name="Public">
+    <packages>
+      <package name="org.apache.tapestry"/>
       <package name="org.apache.tapestry.dom"/>
+      <package name="org.apache.tapestry.services"/>
+      <package name="org.apache.tapestry.util"/>
+      <package name="org.apache.tapestry.runtime"/>
+      <package name="org.apache.tapestry.translator"/>
+      <package name="org.apache.tapestry.validator"/>
+    </packages>
+  </test>
+  <test name="Internals">
+    <packages>
       <package name="org.apache.internal"/>
       <package name="org.apache.tapestry.internal"/>
       <package name="org.apache.tapestry.internal.beaneditor"/>
@@ -32,13 +53,8 @@
       <package name="org.apache.tapestry.internal.services"/>
       <package name="org.apache.tapestry.internal.structure"/>
       <package name="org.apache.tapestry.internal.util"/>
-      <package name="org.apache.tapestry.services"/>
-      <package name="org.apache.tapestry.util"/>
-      <package name="org.apache.tapestry.runtime"/>
       <package name="org.apache.tapestry.internal.bindings"/>
       <package name="org.apache.tapestry.internal.model"/>
-      <package name="org.apache.tapestry.translator"/>
-      <package name="org.apache.tapestry.validator"/>
     </packages>
   </test>
 </suite>

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImpl.java Mon Mar  5 13:55:16 2007
@@ -12,8 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry.ioc.internal;
-
+package org.apache.tapestry.ioc.internal;
+
 import static org.apache.tapestry.ioc.IOCUtilities.toQualifiedId;
 import static org.apache.tapestry.ioc.internal.ConfigurationType.MAPPED;
 import static org.apache.tapestry.ioc.internal.ConfigurationType.ORDERED;
@@ -50,316 +50,320 @@
 import org.apache.tapestry.ioc.def.DecoratorDef;
 import org.apache.tapestry.ioc.def.ModuleDef;
 import org.apache.tapestry.ioc.def.ServiceDef;
-
-/**
- * Starting from the Class for a module builder, identifies all the services (service builder
- * methods), decorators (service decorator methods) and (not yet implemented) contributions (service
- * contributor methods).
- * 
- * 
- */
-public class DefaultModuleDefImpl implements ModuleDef
-{
-    /** The prefix used to identify service builder methods. */
-    private static final String BUILD_METHOD_NAME_PREFIX = "build";
-
-    /** The prefix used to identify service decorator methods. */
-    private static final String DECORATE_METHOD_NAME_PREFIX = "decorate";
-
-    /** The prefix used to identify service contribution methods. */
-    private static final String CONTRIBUTE_METHOD_NAME_PREFIX = "contribute";
-
-    private final Class _builderClass;
-
-    private final Log _log;
-
-    /** Keyed on fully qualified service id. */
-    private final Map<String, ServiceDef> _serviceDefs = newCaseInsensitiveMap();
-
-    /** Keyed on fully qualified decorator id. */
-    private final Map<String, DecoratorDef> _decoratorDefs = newCaseInsensitiveMap();
-
-    private final Set<ContributionDef> _contributionDefs = newSet();
-
-    private final String _moduleId;
-
-    private final static Map<Class, ConfigurationType> PARAMETER_TYPE_TO_CONFIGURATION_TYPE = newMap();
-
-    static
-    {
-        PARAMETER_TYPE_TO_CONFIGURATION_TYPE.put(Configuration.class, UNORDERED);
-        PARAMETER_TYPE_TO_CONFIGURATION_TYPE.put(OrderedConfiguration.class, ORDERED);
-        PARAMETER_TYPE_TO_CONFIGURATION_TYPE.put(MappedConfiguration.class, MAPPED);
-    }
-
-    /**
-     * @param builderClass
-     *            the class that is responsible for building services, etc.
-     * @param log
-     */
-    public DefaultModuleDefImpl(Class builderClass, Log log)
-    {
-        _builderClass = builderClass;
-        _log = log;
-
-        _moduleId = extractModuleId();
-
-        grind();
-    }
-
-    public Class getBuilderClass()
-    {
-        return _builderClass;
-    }
-
-    public Set<String> getServiceIds()
-    {
-        return _serviceDefs.keySet();
-    }
-
-    public ServiceDef getServiceDef(String serviceId)
-    {
-        return _serviceDefs.get(serviceId);
-    }
-
-    public String getModuleId()
-    {
-        return _moduleId;
-    }
-
-    private String extractModuleId()
-    {
-        Id id = getAnnotation(_builderClass, Id.class);
-
-        if (id != null)
-            return id.value();
-
-        String className = _builderClass.getName();
-
-        // Don't try to do this with classes in the default package. Then again, you should
-        // never put classes in the default package!
-
-        int lastdot = className.lastIndexOf('.');
-
-        return className.substring(0, lastdot);
-    }
-
-    // This appears useless, but it's really about some kind of ambiguity in Class, which seems
-    // to think getAnnotation() returns Object, not <? extends Annotation>. This may be a bug
-    // in Eclipse's Java compiler.
-
-    private <T extends Annotation> T getAnnotation(AnnotatedElement element, Class<T> annotationType)
-    {
-        return element.getAnnotation(annotationType);
-    }
-
-    private void grind()
-    {
-        Method[] methods = _builderClass.getMethods();
-
-        Comparator<Method> c = new Comparator<Method>()
-        {
-            // By name, ascending, then by parameter count, descending.
-
-            public int compare(Method o1, Method o2)
-            {
-                int result = o1.getName().compareTo(o2.getName());
-
-                if (result == 0)
-                    result = o2.getParameterTypes().length - o1.getParameterTypes().length;
-
-                return result;
-            }
-
-        };
-
-        Arrays.sort(methods, c);
-
-        for (Method m : methods)
-        {
-            String name = m.getName();
-
-            if (name.startsWith(BUILD_METHOD_NAME_PREFIX))
-            {
-                addServiceDef(m);
-                continue;
-            }
-
-            if (name.startsWith(DECORATE_METHOD_NAME_PREFIX))
-            {
-                addDecoratorDef(m);
-                continue;
-            }
-
-            if (name.startsWith(CONTRIBUTE_METHOD_NAME_PREFIX))
-            {
-                addContributionDef(m);
-                continue;
-            }
-
-        }
-    }
-
-    private void addContributionDef(Method method)
-    {
-        String serviceId = stripMethodPrefix(method, CONTRIBUTE_METHOD_NAME_PREFIX);
-
-        Class returnType = method.getReturnType();
-        if (!returnType.equals(void.class))
-            _log.warn(IOCMessages.contributionWrongReturnType(method));
-
-        Contribute contribute = method.getAnnotation(Contribute.class);
-        if (contribute != null)
-            serviceId = contribute.value();
-
-        ConfigurationType type = null;
-
-        for (Class parameterType : method.getParameterTypes())
-        {
-            ConfigurationType thisParameter = PARAMETER_TYPE_TO_CONFIGURATION_TYPE
-                    .get(parameterType);
-
-            if (thisParameter != null)
-            {
-                if (type != null)
-                {
-                    _log.warn(IOCMessages.tooManyContributionParameters(method));
-                    return;
-                }
-
-                type = thisParameter;
-            }
-        }
-
-        if (type == null)
-        {
-            _log.warn(IOCMessages.noContributionParameter(method));
-            return;
-        }
-
-        // Any other parameters will be validated and worked out at runtime, when we invoke the
-        // service contribution method.
-
-        String qualifiedId = IOCUtilities.toQualifiedId(_moduleId, serviceId);
-
-        ContributionDef def = new ContributionDefImpl(qualifiedId, method);
-
-        _contributionDefs.add(def);
-    }
-
-    private void addDecoratorDef(Method method)
-    {
-        // TODO: methods just named "decorate"
-
-        String simpleDecoratorId = stripMethodPrefix(method, DECORATE_METHOD_NAME_PREFIX);
-        String id = _moduleId + "." + simpleDecoratorId;
-
-        // TODO: Check for duplicates
-
-        Class returnType = method.getReturnType();
-
-        if (returnType.isPrimitive() || returnType.isArray())
-        {
-            _log.warn(decoratorMethodWrongReturnType(method), null);
-            return;
-        }
-
-        if (!methodContainsObjectParameter(method))
-        {
-            _log.warn(IOCMessages.decoratorMethodNeedsDelegateParameter(method), null);
-            return;
-        }
-
-        // TODO: Check that at least one parameter is type java.lang.Object,
-        // since that's how the delegate is passed in.
-
-        Order orderAnnotation = method.getAnnotation(Order.class);
-        Match match = method.getAnnotation(Match.class);
-
-        String[] constraints = orderAnnotation != null ? orderAnnotation.value() : null;
-
-        // TODO: Validate constraints here?
-
-        String[] patterns = match == null ? new String[]
-        { simpleDecoratorId } : match.value();
-
-        // Qualify any unqualified match patterns with the decorator's module id.
-
-        for (int i = 0; i < patterns.length; i++)
-            patterns[i] = toQualifiedId(_moduleId, patterns[i]);
-
-        DecoratorDef def = new DecoratorDefImpl(id, method, patterns, constraints);
-
-        _decoratorDefs.put(id, def);
-    }
-
-    private boolean methodContainsObjectParameter(Method method)
-    {
-        for (Class parameterType : method.getParameterTypes())
-        {
-            // TODO: But what if the type Object parameter has an injection?
-            // We should skip it and look for a different parameter.
-
-            if (parameterType.equals(Object.class))
-                return true;
-        }
-
-        return false;
-    }
-
-    private String stripMethodPrefix(Method method, String prefix)
-    {
-        return method.getName().substring(prefix.length());
-    }
-
-    /** Invoked for public methods that have the proper prefix. */
-    private void addServiceDef(Method method)
-    {
-        // TODO: Methods named just "build"
-        String serviceId = _moduleId + "." + stripMethodPrefix(method, BUILD_METHOD_NAME_PREFIX);
-
-        ServiceDef existing = _serviceDefs.get(serviceId);
-        if (existing != null)
-        {
-            _log.warn(buildMethodConflict(method, existing.toString()), null);
-            return;
-        }
-
-        // Any number of parameters is fine, we'll adapt. Eventually we have to check
-        // that we can satisfy the parameters requested. Thrown exceptions of the method
-        // will be caught and wrapped, so we don't need to check those. But we do need a proper
-        // return type.
-
-        Class returnType = method.getReturnType();
-
-        if (!returnType.isInterface())
-        {
-            _log.warn(buildMethodWrongReturnType(method), null);
-            return;
-        }
-
-        String lifecycle = extractLifecycle(method);
-        boolean isPrivate = method.isAnnotationPresent(Private.class);
-        boolean eagerLoad = method.isAnnotationPresent(EagerLoad.class);
-
-        _serviceDefs.put(serviceId, new ServiceDefImpl(serviceId, lifecycle, method, isPrivate,
-                eagerLoad));
-    }
-
-    private String extractLifecycle(Method method)
-    {
-        Lifecycle lifecycle = method.getAnnotation(Lifecycle.class);
-
-        return lifecycle != null ? lifecycle.value() : IOCConstants.DEFAULT_LIFECYCLE;
-    }
-
-    public Set<DecoratorDef> getDecoratorDefs()
-    {
-        return newSet(_decoratorDefs.values());
-    }
-
-    public Set<ContributionDef> getContributionDefs()
-    {
-        return _contributionDefs;
-    }
-}
+import org.apache.tapestry.ioc.internal.util.InternalUtils;
+
+/**
+ * Starting from the Class for a module builder, identifies all the services (service builder
+ * methods), decorators (service decorator methods) and (not yet implemented) contributions (service
+ * contributor methods).
+ */
+public class DefaultModuleDefImpl implements ModuleDef
+{
+    /** The prefix used to identify service builder methods. */
+    private static final String BUILD_METHOD_NAME_PREFIX = "build";
+
+    /** The prefix used to identify service decorator methods. */
+    private static final String DECORATE_METHOD_NAME_PREFIX = "decorate";
+
+    /** The prefix used to identify service contribution methods. */
+    private static final String CONTRIBUTE_METHOD_NAME_PREFIX = "contribute";
+
+    private final Class _builderClass;
+
+    private final Log _log;
+
+    /** Keyed on fully qualified service id. */
+    private final Map<String, ServiceDef> _serviceDefs = newCaseInsensitiveMap();
+
+    /** Keyed on fully qualified decorator id. */
+    private final Map<String, DecoratorDef> _decoratorDefs = newCaseInsensitiveMap();
+
+    private final Set<ContributionDef> _contributionDefs = newSet();
+
+    private final String _moduleId;
+
+    private final static Map<Class, ConfigurationType> PARAMETER_TYPE_TO_CONFIGURATION_TYPE = newMap();
+
+    static
+    {
+        PARAMETER_TYPE_TO_CONFIGURATION_TYPE.put(Configuration.class, UNORDERED);
+        PARAMETER_TYPE_TO_CONFIGURATION_TYPE.put(OrderedConfiguration.class, ORDERED);
+        PARAMETER_TYPE_TO_CONFIGURATION_TYPE.put(MappedConfiguration.class, MAPPED);
+    }
+
+    /**
+     * @param builderClass
+     *            the class that is responsible for building services, etc.
+     * @param log
+     */
+    public DefaultModuleDefImpl(Class builderClass, Log log)
+    {
+        _builderClass = builderClass;
+        _log = log;
+
+        _moduleId = extractModuleId();
+
+        grind();
+    }
+
+    /** Identified the module builder class and a list of service ids within the module. */
+    @Override
+    public String toString()
+    {
+        return String.format("ModuleDef[%s %s]", _builderClass.getName(), InternalUtils
+                .joinSorted(_serviceDefs.keySet()));
+    }
+
+    public Class getBuilderClass()
+    {
+        return _builderClass;
+    }
+
+    public Set<String> getServiceIds()
+    {
+        return _serviceDefs.keySet();
+    }
+
+    public ServiceDef getServiceDef(String serviceId)
+    {
+        return _serviceDefs.get(serviceId);
+    }
+
+    public String getModuleId()
+    {
+        return _moduleId;
+    }
+
+    private String extractModuleId()
+    {
+        Id id = getAnnotation(_builderClass, Id.class);
+
+        if (id != null) return id.value();
+
+        String className = _builderClass.getName();
+
+        // Don't try to do this with classes in the default package. Then again, you should
+        // never put classes in the default package!
+
+        int lastdot = className.lastIndexOf('.');
+
+        return className.substring(0, lastdot);
+    }
+
+    // This appears useless, but it's really about some kind of ambiguity in Class, which seems
+    // to think getAnnotation() returns Object, not <? extends Annotation>. This may be a bug
+    // in Eclipse's Java compiler.
+
+    private <T extends Annotation> T getAnnotation(AnnotatedElement element, Class<T> annotationType)
+    {
+        return element.getAnnotation(annotationType);
+    }
+
+    private void grind()
+    {
+        Method[] methods = _builderClass.getMethods();
+
+        Comparator<Method> c = new Comparator<Method>()
+        {
+            // By name, ascending, then by parameter count, descending.
+
+            public int compare(Method o1, Method o2)
+            {
+                int result = o1.getName().compareTo(o2.getName());
+
+                if (result == 0)
+                    result = o2.getParameterTypes().length - o1.getParameterTypes().length;
+
+                return result;
+            }
+
+        };
+
+        Arrays.sort(methods, c);
+
+        for (Method m : methods)
+        {
+            String name = m.getName();
+
+            if (name.startsWith(BUILD_METHOD_NAME_PREFIX))
+            {
+                addServiceDef(m);
+                continue;
+            }
+
+            if (name.startsWith(DECORATE_METHOD_NAME_PREFIX))
+            {
+                addDecoratorDef(m);
+                continue;
+            }
+
+            if (name.startsWith(CONTRIBUTE_METHOD_NAME_PREFIX))
+            {
+                addContributionDef(m);
+                continue;
+            }
+
+        }
+    }
+
+    private void addContributionDef(Method method)
+    {
+        String serviceId = stripMethodPrefix(method, CONTRIBUTE_METHOD_NAME_PREFIX);
+
+        Class returnType = method.getReturnType();
+        if (!returnType.equals(void.class))
+            _log.warn(IOCMessages.contributionWrongReturnType(method));
+
+        Contribute contribute = method.getAnnotation(Contribute.class);
+        if (contribute != null) serviceId = contribute.value();
+
+        ConfigurationType type = null;
+
+        for (Class parameterType : method.getParameterTypes())
+        {
+            ConfigurationType thisParameter = PARAMETER_TYPE_TO_CONFIGURATION_TYPE
+                    .get(parameterType);
+
+            if (thisParameter != null)
+            {
+                if (type != null)
+                {
+                    _log.warn(IOCMessages.tooManyContributionParameters(method));
+                    return;
+                }
+
+                type = thisParameter;
+            }
+        }
+
+        if (type == null)
+        {
+            _log.warn(IOCMessages.noContributionParameter(method));
+            return;
+        }
+
+        // Any other parameters will be validated and worked out at runtime, when we invoke the
+        // service contribution method.
+
+        String qualifiedId = IOCUtilities.toQualifiedId(_moduleId, serviceId);
+
+        ContributionDef def = new ContributionDefImpl(qualifiedId, method);
+
+        _contributionDefs.add(def);
+    }
+
+    private void addDecoratorDef(Method method)
+    {
+        // TODO: methods just named "decorate"
+
+        String simpleDecoratorId = stripMethodPrefix(method, DECORATE_METHOD_NAME_PREFIX);
+        String id = _moduleId + "." + simpleDecoratorId;
+
+        // TODO: Check for duplicates
+
+        Class returnType = method.getReturnType();
+
+        if (returnType.isPrimitive() || returnType.isArray())
+        {
+            _log.warn(decoratorMethodWrongReturnType(method), null);
+            return;
+        }
+
+        if (!methodContainsObjectParameter(method))
+        {
+            _log.warn(IOCMessages.decoratorMethodNeedsDelegateParameter(method), null);
+            return;
+        }
+
+        // TODO: Check that at least one parameter is type java.lang.Object,
+        // since that's how the delegate is passed in.
+
+        Order orderAnnotation = method.getAnnotation(Order.class);
+        Match match = method.getAnnotation(Match.class);
+
+        String[] constraints = orderAnnotation != null ? orderAnnotation.value() : null;
+
+        // TODO: Validate constraints here?
+
+        String[] patterns = match == null ? new String[]
+        { simpleDecoratorId } : match.value();
+
+        // Qualify any unqualified match patterns with the decorator's module id.
+
+        for (int i = 0; i < patterns.length; i++)
+            patterns[i] = toQualifiedId(_moduleId, patterns[i]);
+
+        DecoratorDef def = new DecoratorDefImpl(id, method, patterns, constraints);
+
+        _decoratorDefs.put(id, def);
+    }
+
+    private boolean methodContainsObjectParameter(Method method)
+    {
+        for (Class parameterType : method.getParameterTypes())
+        {
+            // TODO: But what if the type Object parameter has an injection?
+            // We should skip it and look for a different parameter.
+
+            if (parameterType.equals(Object.class)) return true;
+        }
+
+        return false;
+    }
+
+    private String stripMethodPrefix(Method method, String prefix)
+    {
+        return method.getName().substring(prefix.length());
+    }
+
+    /** Invoked for public methods that have the proper prefix. */
+    private void addServiceDef(Method method)
+    {
+        // TODO: Methods named just "build"
+        String serviceId = _moduleId + "." + stripMethodPrefix(method, BUILD_METHOD_NAME_PREFIX);
+
+        ServiceDef existing = _serviceDefs.get(serviceId);
+        if (existing != null)
+        {
+            _log.warn(buildMethodConflict(method, existing.toString()), null);
+            return;
+        }
+
+        // Any number of parameters is fine, we'll adapt. Eventually we have to check
+        // that we can satisfy the parameters requested. Thrown exceptions of the method
+        // will be caught and wrapped, so we don't need to check those. But we do need a proper
+        // return type.
+
+        Class returnType = method.getReturnType();
+
+        if (!returnType.isInterface())
+        {
+            _log.warn(buildMethodWrongReturnType(method), null);
+            return;
+        }
+
+        String lifecycle = extractLifecycle(method);
+        boolean isPrivate = method.isAnnotationPresent(Private.class);
+        boolean eagerLoad = method.isAnnotationPresent(EagerLoad.class);
+
+        _serviceDefs.put(serviceId, new ServiceDefImpl(serviceId, lifecycle, method, isPrivate,
+                eagerLoad));
+    }
+
+    private String extractLifecycle(Method method)
+    {
+        Lifecycle lifecycle = method.getAnnotation(Lifecycle.class);
+
+        return lifecycle != null ? lifecycle.value() : IOCConstants.DEFAULT_LIFECYCLE;
+    }
+
+    public Set<DecoratorDef> getDecoratorDefs()
+    {
+        return newSet(_decoratorDefs.values());
+    }
+
+    public Set<ContributionDef> getContributionDefs()
+    {
+        return _contributionDefs;
+    }
+}

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/IOCMessages.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/IOCMessages.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/IOCMessages.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/IOCMessages.java Mon Mar  5 13:55:16 2007
@@ -288,4 +288,9 @@
     {
         return MESSAGES.format("constructed-configuration", result);
     }
+
+    static String serviceConstructionFailed(ServiceDef serviceDef, Throwable cause)
+    {
+        return MESSAGES.format("service-construction-failed", serviceDef.getServiceId(), cause);
+    }
 }

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/ModuleImpl.java Mon Mar  5 13:55:16 2007
@@ -71,7 +71,7 @@
 
     // Set to true when invoking the module constructor. Used to
     // detect endless loops caused by irresponsible dependencies in
-    // the constructor.  Guarded by MUTEX.
+    // the constructor. Guarded by MUTEX.
     private boolean _insideConstructor;
 
     public ModuleImpl(InternalRegistry registry, ModuleDef moduleDef, Log log)
@@ -234,7 +234,7 @@
 
             // Add a wrapper that makes sure that it only gets created once.
 
-            creator = new OneShotServiceCreator(def, creator);
+            creator = new OneShotServiceCreator(def, creator, log);
 
             return createProxy(resources, creator, def.isEagerLoad());
         }

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/OneShotServiceCreator.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/OneShotServiceCreator.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/OneShotServiceCreator.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/internal/OneShotServiceCreator.java Mon Mar  5 13:55:16 2007
@@ -14,6 +14,7 @@
 
 package org.apache.tapestry.ioc.internal;
 
+import org.apache.commons.logging.Log;
 import org.apache.tapestry.ioc.ObjectCreator;
 import org.apache.tapestry.ioc.def.ServiceDef;
 
@@ -29,12 +30,15 @@
 
     private final ObjectCreator _delegate;
 
+    private final Log _log;
+
     private boolean _locked;
 
-    public OneShotServiceCreator(ServiceDef serviceDef, ObjectCreator delegate)
+    public OneShotServiceCreator(ServiceDef serviceDef, ObjectCreator delegate, Log log)
     {
         _serviceDef = serviceDef;
         _delegate = delegate;
+        _log = log;
     }
 
     /**
@@ -46,9 +50,25 @@
         if (_locked)
             throw new IllegalStateException(IOCMessages.recursiveServiceBuild(_serviceDef));
 
+        // Set the lock, to ensure that recursive service construction fails.
+
         _locked = true;
 
-        return _delegate.createObject();
-    }
+        try
+        {
+            return _delegate.createObject();
+        }
+        catch (RuntimeException ex)
+        {
+            _log.error(IOCMessages.serviceConstructionFailed(_serviceDef, ex), ex);
+
+            // Release the lock on failure; the service is now in an unknown state, but we may
+            // be able to continue from here.
 
+            _locked = false;
+
+            throw ex;
+        }
+
+    }
 }

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/util/CaseInsensitiveMap.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/util/CaseInsensitiveMap.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/util/CaseInsensitiveMap.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/main/java/org/apache/tapestry/ioc/util/CaseInsensitiveMap.java Mon Mar  5 13:55:16 2007
@@ -128,8 +128,7 @@
 
             if (_current < 0) throw new NoSuchElementException();
 
-            _cursor = _current;
-            removeAtCursor();
+            new Position(_current, true).remove();
 
             _expectedModCount = _modCount;
         }
@@ -168,11 +167,9 @@
 
             Map.Entry e = (Map.Entry) o;
 
-            select(e.getKey());
+            Position position = select(e.getKey());
 
-            if (!_found) return false;
-
-            return _entries[_cursor].valueMatches(e.getValue());
+            return position.isFound() ? position.entry().valueMatches(e.getValue()) : false;
         }
 
         @Override
@@ -182,15 +179,122 @@
 
             Map.Entry e = (Map.Entry) o;
 
-            select(e.getKey());
+            Position position = select(e.getKey());
+
+            if (position.isFound() && position.entry().valueMatches(e.getValue()))
+            {
+                position.remove();
+                return true;
+            }
+
+            return false;
+        }
+
+    }
+
+    private class Position
+    {
+        private final int _cursor;
+
+        private final boolean _found;
+
+        Position(int cursor, boolean found)
+        {
+            _cursor = cursor;
+            _found = found;
+        }
+
+        boolean isFound()
+        {
+            return _found;
+        }
+
+        CIMEntry<V> entry()
+        {
+            return _entries[_cursor];
+        }
+
+        V get()
+        {
+            return _found ? _entries[_cursor]._value : null;
+        }
+
+        V remove()
+        {
+            if (!_found) return null;
+
+            V result = _entries[_cursor]._value;
+
+            // Remove the entry by shifting everything else down.
+
+            System.arraycopy(_entries, _cursor + 1, _entries, _cursor, _size - _cursor - 1);
+
+            // We shifted down, leaving one (now duplicate) entry behind.
+
+            _entries[--_size] = null;
+
+            // A structural change for sure
+
+            _modCount++;
+
+            return result;
+        }
+
+        @SuppressWarnings("unchecked")
+        V put(String key, int hashCode, V newValue)
+        {
+            if (_found)
+            {
+                CIMEntry<V> e = _entries[_cursor];
+
+                V result = e._value;
+
+                // Not a structural change, so no change to _modCount
+
+                // Update the key (to maintain case). By definition, the hash code
+                // will not change.
+
+                e._key = key;
+                e._value = newValue;
+
+                return result;
+            }
+
+            // Not found, we're going to add it.
+
+            int newSize = _size + 1;
+
+            if (newSize == _entries.length)
+            {
+                // Time to expand!
+
+                int newCapacity = (_size * 3) / 2 + 1;
+
+                CIMEntry<V>[] newEntries = new CIMEntry[newCapacity];
+
+                System.arraycopy(_entries, 0, newEntries, 0, _cursor);
+
+                System.arraycopy(_entries, _cursor, newEntries, _cursor + 1, _size - _cursor);
+
+                _entries = newEntries;
+            }
+            else
+            {
+                // Open up a space for the new entry
+
+                System.arraycopy(_entries, _cursor, _entries, _cursor + 1, _size - _cursor);
+            }
+
+            CIMEntry<V> newEntry = new CIMEntry<V>(key, hashCode, newValue);
+            _entries[_cursor] = newEntry;
 
-            if (!_found) return false;
+            _size++;
 
-            if (!_entries[_cursor].valueMatches(e.getValue())) return false;
+            // This is definately a structural change
 
-            removeAtCursor();
+            _modCount++;
 
-            return true;
+            return null;
         }
 
     }
@@ -207,10 +311,6 @@
 
     private transient Set<Map.Entry<String, V>> _entrySet;
 
-    private transient int _cursor;
-
-    private transient boolean _found;
-
     public CaseInsensitiveMap()
     {
         this(DEFAULT_SIZE);
@@ -260,107 +360,25 @@
     {
         int hashCode = caseInsenitiveHashCode(key);
 
-        select(key, hashCode);
-
-        if (_found)
-        {
-            CIMEntry<V> e = _entries[_cursor];
-
-            V result = e._value;
-
-            // Not a structural change, so no change to _modCount
-
-            // Update the key (to maintain case). By definition, the hash code
-            // will not change.
-
-            e._key = key;
-            e._value = value;
-
-            return result;
-        }
-
-        // Not found, we're going to add it.
-
-        int newSize = _size + 1;
-
-        if (newSize == _entries.length)
-        {
-            // Time to expand!
-
-            int newCapacity = (_size * 3) / 2 + 1;
-
-            CIMEntry<V>[] newEntries = new CIMEntry[newCapacity];
-
-            System.arraycopy(_entries, 0, newEntries, 0, _cursor);
-
-            System.arraycopy(_entries, _cursor, newEntries, _cursor + 1, _size - _cursor);
-
-            _entries = newEntries;
-        }
-        else
-        {
-            // Open up a space for the new entry
-
-            System.arraycopy(_entries, _cursor, _entries, _cursor + 1, _size - _cursor);
-        }
-
-        CIMEntry<V> newEntry = new CIMEntry<V>(key, hashCode, value);
-        _entries[_cursor] = newEntry;
-
-        _size++;
-
-        // This is definately a structural change
-
-        _modCount++;
-
-        return null;
+        return select(key, hashCode).put(key, hashCode, value);
     }
 
     @Override
     public boolean containsKey(Object key)
     {
-        select(key);
-
-        return _found;
+        return select(key).isFound();
     }
 
     @Override
     public V get(Object key)
     {
-        select(key);
-
-        if (_found) return _entries[_cursor]._value;
-
-        return null;
+        return select(key).get();
     }
 
     @Override
     public V remove(Object key)
     {
-        select(key);
-
-        if (!_found) return null;
-
-        V result = _entries[_cursor]._value;
-
-        removeAtCursor();
-
-        return result;
-    }
-
-    private void removeAtCursor()
-    {
-        // Remove the entry by shifting everything else down.
-
-        System.arraycopy(_entries, _cursor + 1, _entries, _cursor, _size - _cursor - 1);
-
-        // We shifted down, leaving one (now duplicate) entry behind.
-
-        _entries[--_size] = null;
-
-        // A structural change for sure
-
-        _modCount++;
+        return select(key).remove();
     }
 
     @SuppressWarnings("unchecked")
@@ -372,91 +390,90 @@
         return _entrySet;
     }
 
-    private void select(Object key)
+    private Position select(Object key)
     {
         if (key == null || key instanceof String)
         {
             String keyString = (String) key;
-            select(keyString, caseInsenitiveHashCode(keyString));
-        }
-        else
-        {
-            _found = false;
+            return select(keyString, caseInsenitiveHashCode(keyString));
         }
+
+        return new Position(0, false);
     }
 
     /**
      * Searches the elements for the index of the indicated key and (case insensitive) hash code.
      * Sets the _cursor and _found attributes.
      */
-    private void select(String key, int hashCode)
+    private Position select(String key, int hashCode)
     {
+        if (_size == 0) return new Position(0, false);
 
         int low = 0;
         int high = _size - 1;
 
-        _cursor = 0;
-        _found = false;
-
-        if (_size == 0) return;
+        int cursor = 0;
 
         while (low <= high)
         {
-            _cursor = (low + high) >> 1;
+            cursor = (low + high) >> 1;
 
-            CIMEntry e = _entries[_cursor];
+            CIMEntry e = _entries[cursor];
 
             if (e._hashCode < hashCode)
             {
-                low = _cursor + 1;
+                low = cursor + 1;
                 continue;
             }
 
             if (e._hashCode > hashCode)
             {
-                high = _cursor - 1;
+                high = cursor - 1;
                 continue;
             }
 
-            tuneCursor(key, hashCode);
-            return;
+            return tunePosition(key, hashCode, cursor);
         }
 
-        _cursor = low;
+        return new Position(low, false);
     }
 
     /**
-     * find() has located a matching hashCode, but there's an outlying possibility that multiple
+     * select() has located a matching hashCode, but there's an outlying possibility that multiple
      * keys share the same hashCode. Backup the cursor until we get to locate the initial hashCode
      * match, then march forward until the key is located, or the hashCode stops matching.
      * 
      * @param key
      * @param hashCode
      */
-    private void tuneCursor(String key, int hashCode)
+    private Position tunePosition(String key, int hashCode, int cursor)
     {
-        while (_cursor > 0)
+        boolean found = false;
+
+        while (cursor > 0)
         {
-            if (_entries[_cursor - 1]._hashCode != hashCode) break;
+            if (_entries[cursor - 1]._hashCode != hashCode) break;
 
-            _cursor--;
+            cursor--;
         }
 
         while (true)
         {
-            if (_entries[_cursor].matches(key))
+            if (_entries[cursor].matches(key))
             {
-                _found = true;
-                return;
+                found = true;
+                break;
             }
 
             // Advance to the next entry.
 
-            _cursor++;
+            cursor++;
 
             // If out of entries,
-            if (_cursor >= _size || _entries[_cursor]._hashCode != hashCode) return;
+            if (cursor >= _size || _entries[cursor]._hashCode != hashCode) break;
         }
+
+        return new Position(cursor, found);
     }
 
     static int caseInsenitiveHashCode(String input)

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/main/resources/org/apache/tapestry/ioc/internal/IOCStrings.properties
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/main/resources/org/apache/tapestry/ioc/internal/IOCStrings.properties?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/main/resources/org/apache/tapestry/ioc/internal/IOCStrings.properties (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/main/resources/org/apache/tapestry/ioc/internal/IOCStrings.properties Mon Mar  5 13:55:16 2007
@@ -74,4 +74,5 @@
 recursive-module-constructor=The constructor for module '%s' (class %s) is recursive: it depends on itself in some way. \
   The constructor, %s, is in some way is triggering a service builder, decorator or contribution method within the class.
 registry-shutdown=Proxy for service %s is no longer active because the IOC Registry has been shut down.
-constructed-configuration=Constructed configuration: %s
\ No newline at end of file
+constructed-configuration=Constructed configuration: %s
+service-construction-failed=Construction of service %s failed: %s
\ No newline at end of file

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImplTest.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImplTest.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/DefaultModuleDefImplTest.java Mon Mar  5 13:55:16 2007
@@ -78,6 +78,8 @@
 
         ModuleDef md = new DefaultModuleDefImpl(SimpleModule.class, log);
 
+        assertEquals(md.toString(), "ModuleDef[" + className + " ioc.Barney, ioc.Fred, ioc.Wilma]");
+
         Set<String> ids = md.getServiceIds();
 
         assertEquals(ids.size(), 3);

Modified: tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/OneShotServiceCreatorTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/OneShotServiceCreatorTest.java?view=diff&rev=514880&r1=514879&r2=514880
==============================================================================
--- tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/OneShotServiceCreatorTest.java (original)
+++ tapestry/tapestry5/tapestry-ioc/trunk/src/test/java/org/apache/tapestry/ioc/internal/OneShotServiceCreatorTest.java Mon Mar  5 13:55:16 2007
@@ -16,15 +16,18 @@
 
 import java.lang.reflect.Method;
 
+import org.apache.commons.logging.Log;
 import org.apache.tapestry.ioc.ObjectCreator;
 import org.apache.tapestry.ioc.def.ServiceDef;
 import org.testng.annotations.Test;
 
 public class OneShotServiceCreatorTest extends IOCInternalTestCase
 {
+
     @Test
     public void ensure_only_called_once() throws Exception
     {
+        Log log = newLog();
         Method method = getClass().getMethod("buildMyService");
 
         ObjectCreator delegate = newObjectCreator();
@@ -36,7 +39,7 @@
 
         replay();
 
-        ObjectCreator oneShot = new OneShotServiceCreator(def, delegate);
+        ObjectCreator oneShot = new OneShotServiceCreator(def, delegate, log);
 
         assertSame(oneShot.createObject(), service);
 
@@ -54,6 +57,51 @@
                             + ".buildMyService() for references to another service that is itself dependent on service 'foo.Bar'.");
         }
 
+        verify();
+    }
+
+    @Test
+    public void reporting_of_construction_failure() throws Exception
+    {
+        RuntimeException failure = new RuntimeException("Just cranky.");
+        Log log = newLog();
+        Method method = getClass().getMethod("buildMyService");
+
+        ObjectCreator delegate = newObjectCreator();
+        Object service = new Object();
+
+        ServiceDef def = new ServiceDefImpl("foo.Bar", "singleton", method, false, false);
+
+        expect(delegate.createObject()).andThrow(failure);
+
+        log.error("Construction of service foo.Bar failed: Just cranky.", failure);
+
+        replay();
+
+        ObjectCreator oneShot = new OneShotServiceCreator(def, delegate, log);
+
+        try
+        {
+            oneShot.createObject();
+            unreachable();
+        }
+        catch (RuntimeException ex)
+        {
+            assertSame(ex, failure);
+        }
+
+        verify();
+
+        // Now test that the locked flag is not set and that the object may still be created.
+
+        train_createObject(delegate, service);
+
+        replay();
+
+        assertSame(service, oneShot.createObject());
+
+        verify();
+
     }
 
     /** Fake service builder method. */
@@ -61,4 +109,5 @@
     {
         return null;
     }
+
 }