TAPESTRY-2682: Warnings related to service configuration validations should be escalated to exceptions

git-svn-id: https://svn.apache.org/repos/asf/tapestry/tapestry5/branches/5.1-dev@696628 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/IOCMessages.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/IOCMessages.java
index f4b8f8a..be31e1e 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/IOCMessages.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/IOCMessages.java
@@ -165,27 +165,27 @@
         return MESSAGES.format("contribution-method-error", asString(method), cause);
     }
 
-    static String contributionWasNull(String serviceId, ContributionDef def)
+    static String contributionWasNull(String serviceId)
     {
-        return MESSAGES.format("contribution-was-null", serviceId, def);
+        return MESSAGES.format("contribution-was-null", serviceId);
     }
 
-    static String contributionKeyWasNull(String serviceId, ContributionDef def)
+    static String contributionKeyWasNull(String serviceId)
     {
-        return MESSAGES.format("contribution-key-was-null", serviceId, def);
+        return MESSAGES.format("contribution-key-was-null", serviceId);
     }
 
-    static String contributionWrongValueType(String serviceId, ContributionDef def, Class actualClass,
+    static String contributionWrongValueType(String serviceId, Class actualClass,
                                              Class expectedClass)
     {
-        return MESSAGES.format("contribution-wrong-value-type", serviceId, def, actualClass
+        return MESSAGES.format("contribution-wrong-value-type", serviceId, actualClass
                 .getName(), expectedClass.getName());
     }
 
-    static String contributionWrongKeyType(String serviceId, ContributionDef def, Class actualClass,
+    static String contributionWrongKeyType(String serviceId, Class actualClass,
                                            Class expectedClass)
     {
-        return MESSAGES.format("contribution-wrong-key-type", serviceId, def, actualClass.getName(),
+        return MESSAGES.format("contribution-wrong-key-type", serviceId, actualClass.getName(),
                                expectedClass.getName());
     }
 
@@ -199,10 +199,9 @@
         return MESSAGES.format("generic-type-not-supported", type);
     }
 
-    static String contributionDuplicateKey(String serviceId, ContributionDef contributionDef,
-                                           ContributionDef existingDef)
+    static String contributionDuplicateKey(String serviceId, ContributionDef existingDef)
     {
-        return MESSAGES.format("contribution-duplicate-key", serviceId, contributionDef, existingDef);
+        return MESSAGES.format("contribution-duplicate-key", serviceId, existingDef);
     }
 
     static String errorBuildingService(String serviceId, ServiceDef serviceDef, Throwable cause)
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
index 4d9fd5c..dee5e9f 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
@@ -425,10 +425,11 @@
         for (ContributionDef def : contributions)
         {
             MappedConfiguration<K, V> validating = new ValidatingMappedConfigurationWrapper<K, V>(serviceId, def,
-                                                                                                  logger, keyClass,
+                                                                                                  keyClass,
                                                                                                   valueType,
                                                                                                   keyToContribution,
-                                                                                                  configuration, locator);
+                                                                                                  configuration,
+                                                                                                  locator);
 
             if (debug) logger.debug(IOCMessages.invokingMethod(def));
 
@@ -466,11 +467,10 @@
             }
         };
 
+        Configuration<T> validating = new ValidatingConfigurationWrapper<T>(serviceId, valueType,
+                                                                            configuration);
         for (ContributionDef def : contributions)
         {
-            Configuration<T> validating = new ValidatingConfigurationWrapper<T>(serviceId, logger, valueType, def,
-                                                                                configuration);
-
             if (debug) logger.debug(IOCMessages.invokingMethod(def));
 
             def.contribute(module, locator, validating);
@@ -506,11 +506,11 @@
             }
         };
 
+        OrderedConfiguration<T> validating = new ValidatingOrderedConfigurationWrapper<T>(serviceId,
+                                                                                          valueType, configuration);
+
         for (ContributionDef def : contributions)
         {
-            OrderedConfiguration<T> validating = new ValidatingOrderedConfigurationWrapper<T>(serviceId, def, logger,
-                                                                                              valueType, configuration);
-
             if (debug) logger.debug(IOCMessages.invokingMethod(def));
 
             def.contribute(module, locator, validating);
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapper.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapper.java
index 70dc78f..ed39e62 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapper.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapper.java
@@ -15,9 +15,7 @@
 package org.apache.tapestry5.ioc.internal;
 
 import org.apache.tapestry5.ioc.Configuration;
-import org.apache.tapestry5.ioc.def.ContributionDef;
 import org.apache.tapestry5.ioc.internal.util.Defense;
-import org.slf4j.Logger;
 
 /**
  * Performs some validation before delegating to another Configuration.
@@ -26,45 +24,31 @@
 {
     private final String serviceId;
 
-    private final ContributionDef contributionDef;
-
-    private final Logger logger;
-
     private final Configuration<T> delegate;
 
     private final Class expectedType;
 
     // Need a strategy for determing the right order for this mass of parameters!
 
-    public ValidatingConfigurationWrapper(String serviceId, Logger logger, Class expectedType,
-                                          ContributionDef contributionDef, Configuration<T> delegate)
+    public ValidatingConfigurationWrapper(String serviceId, Class expectedType, Configuration<T> delegate)
     {
         this.serviceId = serviceId;
-        this.logger = logger;
         this.expectedType = expectedType;
-        this.contributionDef = contributionDef;
         this.delegate = delegate;
     }
 
     public void add(T object)
     {
         if (object == null)
-        {
-            logger.warn(IOCMessages.contributionWasNull(serviceId, contributionDef));
-            return;
-        }
+            throw new NullPointerException(IOCMessages.contributionWasNull(serviceId));
 
         // Sure, we say it is type T ... but is it really?
 
         if (!expectedType.isInstance(object))
-        {
-            logger.warn(IOCMessages.contributionWrongValueType(
+            throw new IllegalArgumentException(IOCMessages.contributionWrongValueType(
                     serviceId,
-                    contributionDef,
                     object.getClass(),
                     expectedType));
-            return;
-        }
 
         delegate.add(object);
     }
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapper.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapper.java
index 4965d7c..9bef8a7 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapper.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapper.java
@@ -17,7 +17,6 @@
 import org.apache.tapestry5.ioc.MappedConfiguration;
 import org.apache.tapestry5.ioc.ObjectLocator;
 import org.apache.tapestry5.ioc.def.ContributionDef;
-import org.slf4j.Logger;
 
 import java.util.Map;
 
@@ -37,8 +36,6 @@
 
     private final ContributionDef contributionDef;
 
-    private final Logger logger;
-
     private final Class<K> expectedKeyType;
 
     private final Class<V> expectedValueType;
@@ -50,13 +47,12 @@
     private final ObjectLocator locator;
 
     public ValidatingMappedConfigurationWrapper(String serviceId, ContributionDef contributionDef,
-                                                Logger logger, Class<K> expectedKeyType, Class<V> expectedValueType,
+                                                Class<K> expectedKeyType, Class<V> expectedValueType,
                                                 Map<K, ContributionDef> keyToContributor,
                                                 MappedConfiguration<K, V> delegate, ObjectLocator locator)
     {
         this.serviceId = serviceId;
         this.contributionDef = contributionDef;
-        this.logger = logger;
         this.expectedKeyType = expectedKeyType;
         this.expectedValueType = expectedValueType;
         this.keyToContributor = keyToContributor;
@@ -67,42 +63,25 @@
     public void add(K key, V value)
     {
         if (key == null)
-        {
-            logger.warn(IOCMessages.contributionKeyWasNull(serviceId, contributionDef));
-            return;
-        }
+            throw new NullPointerException(IOCMessages.contributionKeyWasNull(serviceId));
 
         if (!expectedKeyType.isInstance(key))
-        {
-            logger.warn(IOCMessages.contributionWrongKeyType(serviceId, contributionDef, key
-                    .getClass(), expectedKeyType));
-            return;
-        }
+            throw new IllegalArgumentException(
+                    IOCMessages.contributionWrongKeyType(serviceId, key
+                            .getClass(), expectedKeyType));
 
         if (value == null)
-        {
-            logger.warn(IOCMessages.contributionWasNull(serviceId, contributionDef));
-            return;
-        }
+            throw new NullPointerException(IOCMessages.contributionWasNull(serviceId));
 
 
         if (!expectedValueType.isInstance(value))
-        {
-            logger.warn(IOCMessages.contributionWrongValueType(serviceId, contributionDef, value
+            throw new IllegalArgumentException(IOCMessages.contributionWrongValueType(serviceId, value
                     .getClass(), expectedValueType));
-            return;
-        }
 
         ContributionDef existing = keyToContributor.get(key);
 
         if (existing != null)
-        {
-            logger.warn(IOCMessages.contributionDuplicateKey(
-                    serviceId,
-                    contributionDef,
-                    existing));
-            return;
-        }
+            throw new IllegalArgumentException(IOCMessages.contributionDuplicateKey(serviceId, existing));
 
         delegate.add(key, value);
 
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapper.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapper.java
index 3fbd768..33d237d 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapper.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapper.java
@@ -15,9 +15,7 @@
 package org.apache.tapestry5.ioc.internal;
 
 import org.apache.tapestry5.ioc.OrderedConfiguration;
-import org.apache.tapestry5.ioc.def.ContributionDef;
 import org.apache.tapestry5.ioc.internal.util.Defense;
-import org.slf4j.Logger;
 
 /**
  * Implements validation of values provided to an {@link org.apache.tapestry5.ioc.OrderedConfiguration}. If you provide
@@ -31,27 +29,22 @@
 {
     private final String serviceId;
 
-    private final ContributionDef contributionDef;
-
-    private final Logger logger;
-
     private final Class expectedType;
 
     private final OrderedConfiguration<T> delegate;
 
-    public ValidatingOrderedConfigurationWrapper(String serviceId, ContributionDef contributionDef,
-                                                 Logger logger, Class expectedType, OrderedConfiguration<T> delegate)
+    public ValidatingOrderedConfigurationWrapper(String serviceId, Class expectedType, OrderedConfiguration<T> delegate)
     {
         this.serviceId = serviceId;
-        this.contributionDef = contributionDef;
-        this.logger = logger;
         this.expectedType = expectedType;
         this.delegate = delegate;
     }
 
     public void add(String id, T object, String... constraints)
     {
-        delegate.add(id, validVersionOf(object), constraints);
+        checkValid(object);
+
+        delegate.add(id, object, constraints);
     }
 
     public void addInstance(String id, Class<? extends T> clazz, String... constraints)
@@ -60,17 +53,15 @@
 
         if (!expectedType.isAssignableFrom(clazz))
             throw new IllegalArgumentException(IOCMessages.wrongContributionClass(clazz, serviceId, expectedType));
-        
+
         delegate.addInstance(id, clazz, constraints);
     }
 
-    private T validVersionOf(T object)
+    private void checkValid(T object)
     {
-        if (object == null || expectedType.isInstance(object)) return object;
+        if (object == null || expectedType.isInstance(object)) return;
 
-        logger.warn(IOCMessages.contributionWrongValueType(serviceId, contributionDef, object
+        throw new IllegalArgumentException(IOCMessages.contributionWrongValueType(serviceId, object
                 .getClass(), expectedType));
-
-        return null;
     }
 }
diff --git a/tapestry-ioc/src/main/resources/org/apache/tapestry5/ioc/internal/IOCStrings.properties b/tapestry-ioc/src/main/resources/org/apache/tapestry5/ioc/internal/IOCStrings.properties
index 6e7f464..f4468b8 100644
--- a/tapestry-ioc/src/main/resources/org/apache/tapestry5/ioc/internal/IOCStrings.properties
+++ b/tapestry-ioc/src/main/resources/org/apache/tapestry5/ioc/internal/IOCStrings.properties
@@ -53,14 +53,13 @@
   Configuration, OrderedConfiguration or MappedConfiguration. This parameter is how the method \
   make contributions into the service's configuration. The method has been ignored.
 contribution-method-error=Error invoking service contribution method %s: %s
-contribution-was-null=Service contribution (to service '%s', by %s) was null and has been ignored.
-contribution-key-was-null=Key for service contribution (to service '%s', by %s) was null and has been ignored.
-contribution-wrong-value-type=Service contribution (to service '%s', by %s) was an instance of %s, \
-  but %s was expected. The contribution has been ignored.
-contribution-wrong-key-type=Key for service contribution (to service '%s', by %s) was an instance of %s, \
-  but %s was expected. The contribution has been ignored.
-contribution-duplicate-key=Service contribution (to service '%s', by %s) conflicts with \
-  existing contribution (by %s) and has been ignored.
+contribution-was-null=Service contribution (to service '%s') was null.
+contribution-key-was-null=Key for service contribution (to service '%s') was null.
+contribution-wrong-value-type=Service contribution (to service '%s') was an instance of %s, \
+  which is not assignable to the configuration type %s.
+contribution-wrong-key-type=Key for service contribution (to service '%s') was an instance of %s, \
+  but the expected key type was %s.
+contribution-duplicate-key=Service contribution (to service '%s') conflicts with existing contribution (by %s).
 generic-type-not-supported=Generic type '%s' is not supported. Only simple parameterized lists are \
   supported.
 error-building-service=Error building service proxy for service '%s' (at %s): %s
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapperTest.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapperTest.java
index a4058cf..007de4b 100644
--- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapperTest.java
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingConfigurationWrapperTest.java
@@ -28,8 +28,6 @@
     @Test
     public void valid_contribution()
     {
-        ContributionDef def = mockContributionDef();
-        Logger logger = mockLogger();
         Configuration configuration = mockConfiguration();
         Runnable value = mockRunnable();
 
@@ -37,8 +35,8 @@
 
         replay();
 
-        Configuration wrapper = new ValidatingConfigurationWrapper("foo.Bar", logger,
-                                                                   Runnable.class, def, configuration);
+        Configuration wrapper = new ValidatingConfigurationWrapper("foo.Bar",
+                                                                   Runnable.class, configuration);
 
         wrapper.add(value);
 
@@ -57,8 +55,8 @@
 
         replay();
 
-        Configuration wrapper = new ValidatingConfigurationWrapper("foo.Bar", logger,
-                                                                   Map.class, def, configuration);
+        Configuration wrapper = new ValidatingConfigurationWrapper("foo.Bar",
+                                                                   Map.class, configuration);
 
         wrapper.addInstance(HashMap.class);
 
@@ -69,19 +67,22 @@
     @Test
     public void null_contribution()
     {
-        Logger logger = mockLogger();
         Configuration configuration = mockConfiguration();
-        ContributionDef def = new ContributionDefImpl("Bar", findMethod("contributeUnorderedNull"),
-                                                      getClassFactory());
-
-        logger.warn(IOCMessages.contributionWasNull("Bar", def));
 
         replay();
 
-        Configuration wrapper = new ValidatingConfigurationWrapper("Bar", logger, Runnable.class,
-                                                                   def, configuration);
+        Configuration wrapper = new ValidatingConfigurationWrapper("Bar", Runnable.class,
+                                                                   configuration);
 
-        wrapper.add(null);
+        try
+        {
+            wrapper.add(null);
+            unreachable();
+        }
+        catch (NullPointerException ex)
+        {
+            assertEquals(ex.getMessage(), "Service contribution (to service 'Bar') was null.");
+        }
 
         verify();
     }
@@ -90,20 +91,23 @@
     @Test
     public void wrong_type_of_contribution()
     {
-        Logger logger = mockLogger();
         Configuration configuration = mockConfiguration();
-        ContributionDef def = new ContributionDefImpl("Bar", findMethod("contributeUnorderedNull"),
-                                                      getClassFactory());
-
-        logger.warn(IOCMessages
-                .contributionWrongValueType("Bar", def, String.class, Runnable.class));
 
         replay();
 
-        Configuration wrapper = new ValidatingConfigurationWrapper("Bar", logger, Runnable.class,
-                                                                   def, configuration);
+        Configuration wrapper = new ValidatingConfigurationWrapper("Bar", Runnable.class,
+                                                                   configuration);
 
-        wrapper.add("runnable");
+        try
+        {
+            wrapper.add("runnable");
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertEquals(ex.getMessage(),
+                         "Service contribution (to service 'Bar') was an instance of java.lang.String, which is not assignable to the configuration type java.lang.Runnable.");
+        }
 
         verify();
     }
@@ -111,15 +115,12 @@
     @Test
     public void wrong_class_contributed()
     {
-        Logger logger = mockLogger();
         Configuration configuration = mockConfiguration();
-        ContributionDef def = new ContributionDefImpl("Bar", findMethod("contributeWrongType"),
-                                                      getClassFactory());
 
         replay();
 
-        Configuration wrapper = new ValidatingConfigurationWrapper("Bar", logger, Runnable.class,
-                                                                   def, configuration);
+        Configuration wrapper = new ValidatingConfigurationWrapper("Bar", Runnable.class,
+                                                                   configuration);
 
         try
         {
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapperTest.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapperTest.java
index 63a143c..06e06e3 100644
--- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapperTest.java
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingMappedConfigurationWrapperTest.java
@@ -45,7 +45,7 @@
         replay();
 
         MappedConfiguration<Class, Runnable> wrapper = new ValidatingMappedConfigurationWrapper<Class, Runnable>(
-                SERVICE_ID, def, logger, Class.class, Runnable.class, keyToContribution, delegate, locator);
+                SERVICE_ID, def, Class.class, Runnable.class, keyToContribution, delegate, locator);
 
         wrapper.add(key, value);
 
@@ -59,7 +59,6 @@
     {
         ContributionDef def1 = newContributionDef("contributionPlaceholder1");
         ContributionDef def2 = newContributionDef("contributionPlaceholder2");
-        Logger logger = mockLogger();
         Map<Class, ContributionDef> keyToContribution = newMap();
         ObjectLocator locator = mockObjectLocator();
 
@@ -70,14 +69,21 @@
         Class key = Integer.class;
         Runnable value = mockRunnable();
 
-        logger.warn(IOCMessages.contributionDuplicateKey(SERVICE_ID, def2, def1));
-
         replay();
 
         MappedConfiguration<Class, Runnable> wrapper = new ValidatingMappedConfigurationWrapper<Class, Runnable>(
-                SERVICE_ID, def2, logger, Class.class, Runnable.class, keyToContribution, delegate, locator);
+                SERVICE_ID, def2, Class.class, Runnable.class, keyToContribution, delegate, locator);
 
-        wrapper.add(key, value);
+        try
+        {
+            wrapper.add(key, value);
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertMessageContains(ex,
+                                  "Service contribution (to service 'Baz') conflicts with existing contribution");
+        }
 
         verify();
 
@@ -88,20 +94,25 @@
     public void null_key()
     {
         ContributionDef def = newContributionDef("contributionPlaceholder1");
-        Logger logger = mockLogger();
         Map<Class, ContributionDef> keyToContribution = newMap();
         MappedConfiguration<Class, Runnable> delegate = mockMappedConfiguration();
         Runnable value = mockRunnable();
         ObjectLocator locator = mockObjectLocator();
 
-        logger.warn(IOCMessages.contributionKeyWasNull(SERVICE_ID, def));
-
         replay();
 
         MappedConfiguration<Class, Runnable> wrapper = new ValidatingMappedConfigurationWrapper<Class, Runnable>(
-                SERVICE_ID, def, logger, Class.class, Runnable.class, keyToContribution, delegate, locator);
+                SERVICE_ID, def, Class.class, Runnable.class, keyToContribution, delegate, locator);
 
-        wrapper.add(null, value);
+        try
+        {
+            wrapper.add(null, value);
+            unreachable();
+        }
+        catch (NullPointerException ex)
+        {
+            assertEquals(ex.getMessage(), "Key for service contribution (to service 'Baz') was null.");
+        }
 
         verify();
     }
@@ -111,22 +122,27 @@
     public void wrong_key_type()
     {
         ContributionDef def = newContributionDef("contributionPlaceholder1");
-        Logger logger = mockLogger();
         Map<?, ContributionDef> keyToContribution = newMap();
         MappedConfiguration delegate = mockMappedConfiguration();
         Runnable value = mockRunnable();
         ObjectLocator locator = mockObjectLocator();
 
-        logger.warn(IOCMessages
-                .contributionWrongKeyType(SERVICE_ID, def, String.class, Class.class));
-
         replay();
 
         MappedConfiguration wrapper = new ValidatingMappedConfigurationWrapper(SERVICE_ID, def,
-                                                                               logger, Class.class, Runnable.class,
+                                                                               Class.class, Runnable.class,
                                                                                keyToContribution, delegate, locator);
 
-        wrapper.add("java.util.List", value);
+        try
+        {
+            wrapper.add("java.util.List", value);
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertEquals(ex.getMessage(),
+                         "Key for service contribution (to service 'Baz') was an instance of java.lang.String, but the expected key type was java.lang.Class.");
+        }
 
         verify();
     }
@@ -136,24 +152,27 @@
     public void wrong_value_type()
     {
         ContributionDef def = newContributionDef("contributionPlaceholder1");
-        Logger logger = mockLogger();
         Map<?, ContributionDef> keyToContribution = newMap();
         MappedConfiguration delegate = mockMappedConfiguration();
         ObjectLocator locator = mockObjectLocator();
 
-        logger.warn(IOCMessages.contributionWrongValueType(
-                SERVICE_ID,
-                def,
-                String.class,
-                Runnable.class));
 
         replay();
 
         MappedConfiguration wrapper = new ValidatingMappedConfigurationWrapper(SERVICE_ID, def,
-                                                                               logger, Class.class, Runnable.class,
+                                                                               Class.class, Runnable.class,
                                                                                keyToContribution, delegate, locator);
 
-        wrapper.add(List.class, "do something");
+        try
+        {
+            wrapper.add(List.class, "do something");
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertEquals(ex.getMessage(),
+                         "Service contribution (to service 'Baz') was an instance of java.lang.String, which is not assignable to the configuration type java.lang.Runnable.");
+        }
 
         verify();
     }
@@ -162,22 +181,26 @@
     public void null_value()
     {
         ContributionDef def = newContributionDef("contributionPlaceholder1");
-        Logger logger = mockLogger();
         Map<Class, ContributionDef> keyToContribution = newMap();
         MappedConfiguration<Class, Runnable> delegate = mockMappedConfiguration();
         ObjectLocator locator = mockObjectLocator();
-                                                                                                   
-        logger.warn(IOCMessages.contributionWasNull(SERVICE_ID, def));
 
         replay();
 
         MappedConfiguration<Class, Runnable> wrapper = new ValidatingMappedConfigurationWrapper<Class, Runnable>(
-                SERVICE_ID, def, logger, Class.class, Runnable.class, keyToContribution, delegate, locator);
+                SERVICE_ID, def, Class.class, Runnable.class, keyToContribution, delegate, locator);
 
-        wrapper.add(Integer.class, null);
+        try
+        {
+            wrapper.add(Integer.class, null);
+            unreachable();
+        }
+        catch (NullPointerException ex)
+        {
+            assertEquals(ex.getMessage(), "Service contribution (to service 'Baz') was null.");
+        }
 
         verify();
-
     }
 
     private ContributionDef newContributionDef(String methodName)
@@ -194,5 +217,4 @@
     {
 
     }
-
 }
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapperTest.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapperTest.java
index 99644b9..780ab2c 100644
--- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapperTest.java
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/ValidatingOrderedConfigurationWrapperTest.java
@@ -15,11 +15,8 @@
 package org.apache.tapestry5.ioc.internal;
 
 import org.apache.tapestry5.ioc.OrderedConfiguration;
-import org.apache.tapestry5.ioc.def.ContributionDef;
-import org.slf4j.Logger;
 import org.testng.annotations.Test;
 
-import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
@@ -29,8 +26,6 @@
     @Test
     public void valid_type_long_form()
     {
-        ContributionDef def = mockContributionDef();
-        Logger logger = mockLogger();
         OrderedConfiguration<Runnable> configuration = mockOrderedConfiguration();
         Runnable contribution = mockRunnable();
 
@@ -39,7 +34,7 @@
         replay();
 
         OrderedConfiguration<Runnable> wrapper = new ValidatingOrderedConfigurationWrapper<Runnable>(
-                "Service", def, logger, Runnable.class, configuration);
+                "Service", Runnable.class, configuration);
 
         wrapper.add("id", contribution, "after:pre", "before:post");
 
@@ -49,8 +44,6 @@
     @Test
     public void contribute_valid_class()
     {
-        ContributionDef def = mockContributionDef();
-        Logger logger = mockLogger();
         OrderedConfiguration<Map> configuration = mockOrderedConfiguration();
 
         configuration.addInstance("id", HashMap.class, "after:pre", "before:post");
@@ -58,7 +51,7 @@
         replay();
 
         OrderedConfiguration<Map> wrapper = new ValidatingOrderedConfigurationWrapper<Map>(
-                "Service", def, logger, Map.class, configuration);
+                "Service", Map.class, configuration);
 
         wrapper.addInstance("id", HashMap.class, "after:pre", "before:post");
 
@@ -68,14 +61,12 @@
     @Test
     public void contribute_invalid_class()
     {
-        ContributionDef def = mockContributionDef();
-        Logger logger = mockLogger();
         OrderedConfiguration<Object> configuration = mockOrderedConfiguration();
 
         replay();
 
         OrderedConfiguration<Object> wrapper = new ValidatingOrderedConfigurationWrapper<Object>(
-                "Service", def, logger, Map.class, configuration);
+                "Service", Map.class, configuration);
 
         try
         {
@@ -96,8 +87,6 @@
     @Test
     public void valid_type_short_form()
     {
-        ContributionDef def = mockContributionDef();
-        Logger logger = mockLogger();
         OrderedConfiguration<Runnable> configuration = mockOrderedConfiguration();
         Runnable contribution = mockRunnable();
 
@@ -106,7 +95,7 @@
         replay();
 
         OrderedConfiguration<Runnable> wrapper = new ValidatingOrderedConfigurationWrapper<Runnable>(
-                "Service", def, logger, Runnable.class, configuration);
+                "Service", Runnable.class, configuration);
 
         wrapper.add("id", contribution);
 
@@ -116,8 +105,6 @@
     @Test
     public void null_object_passed_through()
     {
-        ContributionDef def = mockContributionDef();
-        Logger logger = mockLogger();
         OrderedConfiguration<Runnable> configuration = mockOrderedConfiguration();
 
         configuration.add("id", null);
@@ -125,7 +112,7 @@
         replay();
 
         OrderedConfiguration<Runnable> wrapper = new ValidatingOrderedConfigurationWrapper<Runnable>(
-                "Service", def, logger, Runnable.class, configuration);
+                "Service", Runnable.class, configuration);
 
         wrapper.add("id", null);
 
@@ -136,32 +123,24 @@
     @Test
     public void incorrect_contribution_type_is_passed_through_as_null()
     {
-        Method method = findMethod("contributeBarneyService");
-
-        ContributionDef def = new ContributionDefImpl("Service", method, getClassFactory());
-        Logger log = mockLogger();
         OrderedConfiguration<Runnable> configuration = mockOrderedConfiguration();
 
-        log.warn(IOCMessages.contributionWrongValueType(
-                "Service",
-                def,
-                String.class,
-                Runnable.class));
-
-        configuration.add("id", null);
-
         replay();
 
-        OrderedConfiguration wrapper = new ValidatingOrderedConfigurationWrapper("Service", def,
-                                                                                 log, Runnable.class, configuration);
+        OrderedConfiguration wrapper = new ValidatingOrderedConfigurationWrapper("Service",
+                                                                                 Runnable.class, configuration);
 
-        wrapper.add("id", "string");
+        try
+        {
+            wrapper.add("id", "string");
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertEquals(ex.getMessage(),
+                         "Service contribution (to service 'Service') was an instance of java.lang.String, which is not assignable to the configuration type java.lang.Runnable.");
+        }
 
         verify();
     }
-
-    public void contributeBarneyService(OrderedConfiguration<Runnable> configuration)
-    {
-
-    }
 }