On Feb 17, 2008 2:10 PM,  <[EMAIL PROTECTED]> wrote:
> Author: kmenard
> Date: Sun Feb 17 14:10:29 2008
> New Revision: 628562
>
> URL: http://svn.apache.org/viewvc?rev=628562&view=rev
> Log:
> Fixed TAPESTRY-1978: When supplying an empty parameter binding, indicate 
> problem parameter in error message.
>
> Modified:
>     
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/BindingSourceImpl.java
>     
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/PageElementFactoryImpl.java
>     
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java
>     
> tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties
>     
> tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/BindingSourceImplTest.java
>
> Modified: 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/BindingSourceImpl.java
> URL: 
> http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/BindingSourceImpl.java?rev=628562&r1=628561&r2=628562&view=diff
> ==============================================================================
> --- 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/BindingSourceImpl.java
>  (original)
> +++ 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/BindingSourceImpl.java
>  Sun Feb 17 14:10:29 2008
> @@ -47,7 +47,16 @@
>          notNull(container, "container");
>          notNull(component, "component");
>          notBlank(defaultPrefix, "defaultPrefix");
> -        notBlank(expression, "expression");
> +
> +        try
> +        {
> +            notBlank(expression, "expression");
> +        }
> +        catch (final Exception ex)
> +        {
> +            throw new 
> TapestryException(ServicesMessages.emptyBinding(description), location, ex);
> +        }
> +

Please use the method InternalUtils.isBlank() rather than the Defense
call.  It's what Defense itself calls and its much more reasonable to
check a boolean than throw/catch an exception.

>          // Location might be null
>
>          String subexpression = expression;
>
> Modified: 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/PageElementFactoryImpl.java
> URL: 
> http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/PageElementFactoryImpl.java?rev=628562&r1=628561&r2=628562&view=diff
> ==============================================================================
> --- 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/PageElementFactoryImpl.java
>  (original)
> +++ 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/PageElementFactoryImpl.java
>  Sun Feb 17 14:10:29 2008
> @@ -302,7 +302,7 @@
>              return new AttributeExpansionBinding(provider, location);
>          }
>
> -        return _bindingSource.newBinding("parameter " + parameterName, 
> loadingComponentResources,
> +        return _bindingSource.newBinding(parameterName, 
> loadingComponentResources,
>                                           embeddedComponentResources, 
> defaultBindingPrefix, expression, location);
>      }
>  }


I'm trying to remember the value of using a "parameter " prefix here
... it may have been to do with expansions and perhaps inherited
parameters.

>
> Modified: 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java
> URL: 
> http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java?rev=628562&r1=628561&r2=628562&view=diff
> ==============================================================================
> --- 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java
>  (original)
> +++ 
> tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ServicesMessages.java
>  Sun Feb 17 14:10:29 2008
> @@ -404,4 +404,9 @@
>          return MESSAGES.format("no-translator-for-type", 
> ClassFabUtils.toJavaClassName(valueType),
>                                 InternalUtils.joinSorted(typeNames));
>      }
> +
> +    static String emptyBinding(String parameterName)
> +    {
> +        return MESSAGES.format("parameter-binding-must-not-be-empty", 
> parameterName);
> +    }
>  }
>
> Modified: 
> tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties
> URL: 
> http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties?rev=628562&r1=628561&r2=628562&view=diff
> ==============================================================================
> --- 
> tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties
>  (original)
> +++ 
> tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry/internal/services/ServicesStrings.properties
>  Sun Feb 17 14:10:29 2008
> @@ -91,4 +91,5 @@
>    Try increasing the hard limit (symbol tapestry.page-pool.hard-limit) to 
> allow additional instances to be created, \
>    or increasing the soft wait (symbol tapestry.page-pool.soft-wait) to trade 
> away some throughput for more efficient use of page instances.
>  unknown-null-field-strategy-name=Unrecognized name '%s' locating a null 
> field strategy.  Available strategies: %s.
> -no-translator-for-type=No translator is defined for type %s.  Registered 
> types: %s.
> \ No newline at end of file
> +no-translator-for-type=No translator is defined for type %s.  Registered 
> types: %s.
> +parameter-binding-must-not-be-empty=Parameter '%s' must have a non-empty 
> binding.
> \ No newline at end of file

I would have phrased this as something like "The expression for
parameter '%s' was blank."

>
> Modified: 
> tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/BindingSourceImplTest.java
> URL: 
> http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/BindingSourceImplTest.java?rev=628562&r1=628561&r2=628562&view=diff
> ==============================================================================
> --- 
> tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/BindingSourceImplTest.java
>  (original)
> +++ 
> tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/BindingSourceImplTest.java
>  Sun Feb 17 14:10:29 2008
> @@ -176,4 +176,37 @@
>
>          verify();
>      }
> +
> +    @Test
> +    public void empty_parameter_binding()
> +    {
> +        BindingFactory factory = mockBindingFactory();
> +        ComponentResources container = mockComponentResources();
> +        ComponentResources component = mockComponentResources();
> +        Location l = mockLocation();
> +
> +        String defaultPrefix = "def";
> +        String description = "wilma";
> +        String expression = "";
> +
> +        replay();
> +
> +        Map<String, BindingFactory> map = newMap();
> +
> +        map.put(defaultPrefix, factory);
> +
> +        BindingSource source = new BindingSourceImpl(map);
> +
> +        try
> +        {
> +            source.newBinding(description, container, component, 
> defaultPrefix, expression, l);
> +        }
> +        catch (TapestryException ex)
> +        {
> +            assertEquals(ex.getMessage(), "Parameter 'wilma' must have a 
> non-empty binding.");
> +            assertSame(ex.getLocation(), l);
> +        }
> +
> +        verify();
> +    }
>  }
>
>
>

Nice to see some fresh blood making commits!

-- 
Howard M. Lewis Ship

Creator Apache Tapestry and Apache HiveMind

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to