I can toy with that.  I started off by manipulating the generated byte
code, but it struck me as being invasive, per my comment on the issue.
But, there is a ton of value in knowing which part of the property path
failed as well.

Thinking a bit more, it's probably not all that invasive anyway.  An NPE
is going to get thrown.  All we be doing is wrapping it with a
meaningful message.

-- 
Kevin Menard
Servprise International, Inc.
Remote reboot & power control for your network
www.servprise.com                  +1 508.892.3823 x308


> -----Original Message-----
> From: Howard Lewis Ship [mailto:[EMAIL PROTECTED]
> Sent: Monday, February 25, 2008 3:08 PM
> To: [email protected]
> Subject: Re: svn commit: r630376 - in
> /tapestry/tapestry5/trunk/tapestry-core/src:
> main/java/org/apache/tapestry/corelib/base/
> main/resources/org/apache/tapestry/corelib/base/
> test/java/org/apache/tapestry/corelib/base/
> 
> I think an eventual improvement to this would be to identify the null
> property value.  This would involve adding additional generated code
> to check for nulls and throw the NPE there, where the property name is
> known.
> 
> On Fri, Feb 22, 2008 at 6:17 PM,  <[EMAIL PROTECTED]> wrote:
> > Author: kmenard
> >  Date: Fri Feb 22 18:17:09 2008
> >  New Revision: 630376
> >
> >  URL: http://svn.apache.org/viewvc?rev=630376&view=rev
> >  Log:
> >  Fixed TAPESTRY-2182: NullPointerExceptions, due to reading nested
> properties that do not suppress null values, do not indicate
> problematic expression for AbstractPropertyOutput derivatives.
> >
> >  Added:
> >     tapestry/tapestry5/trunk/tapestry-
> core/src/main/java/org/apache/tapestry/corelib/base/BaseMessages.java
> >     tapestry/tapestry5/trunk/tapestry-
> core/src/main/resources/org/apache/tapestry/corelib/base/
> >     tapestry/tapestry5/trunk/tapestry-
>
core/src/main/resources/org/apache/tapestry/corelib/base/BaseStrings.pr
> operties
> >     tapestry/tapestry5/trunk/tapestry-
>
core/src/test/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> putTest.java
> >  Modified:
> >     tapestry/tapestry5/trunk/tapestry-
>
core/src/main/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> put.java
> >
> >  Modified: tapestry/tapestry5/trunk/tapestry-
>
core/src/main/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> put.java
> >  URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-
>
core/src/main/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> put.java?rev=630376&r1=630375&r2=630376&view=diff
> >
>
=======================================================================
> =======
> >  --- tapestry/tapestry5/trunk/tapestry-
>
core/src/main/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> put.java (original)
> >  +++ tapestry/tapestry5/trunk/tapestry-
>
core/src/main/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> put.java Fri Feb 22 18:17:09 2008
> >  @@ -37,7 +37,7 @@
> >   *
> >   * @see BeanBlockSource
> >   */
> >  -public class AbstractPropertyOutput
> >  +public abstract class AbstractPropertyOutput
> >   {
> >      /**
> >       * Model for property displayed by the cell.
> >  @@ -130,11 +130,18 @@
> >          return false;
> >      }
> >
> >  -    private Object readPropertyForObject()
> >  +    Object readPropertyForObject()
> >      {
> >          PropertyConduit conduit = _model.getConduit();
> >
> >  -        return conduit == null ? null : conduit.get(_object);
> >  +        try
> >  +        {
> >  +            return conduit == null ? null : conduit.get(_object);
> >  +        }
> >  +        catch (final NullPointerException ex)
> >  +        {
> >  +            throw new
>
NullPointerException(BaseMessages.nullValueInPath(_model.getPropertyNam
> e()));
> >  +        }
> >      }
> >
> >      private Messages getOverrideMessages()
> >  @@ -159,4 +166,10 @@
> >          }
> >      }
> >
> >  +    // Used for testing.
> >  +    void inject(final PropertyModel model, final Object object)
> >  +    {
> >  +        _model = model;
> >  +        _object = object;
> >  +    }
> >   }
> >
> >  Added: tapestry/tapestry5/trunk/tapestry-
> core/src/main/java/org/apache/tapestry/corelib/base/BaseMessages.java
> >  URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-
>
core/src/main/java/org/apache/tapestry/corelib/base/BaseMessages.java?r
> ev=630376&view=auto
> >
>
=======================================================================
> =======
> >  --- tapestry/tapestry5/trunk/tapestry-
> core/src/main/java/org/apache/tapestry/corelib/base/BaseMessages.java
> (added)
> >  +++ tapestry/tapestry5/trunk/tapestry-
> core/src/main/java/org/apache/tapestry/corelib/base/BaseMessages.java
> Fri Feb 22 18:17:09 2008
> >  @@ -0,0 +1,28 @@
> >  +// Copyright 2008 The Apache Software Foundation
> >  +//
> >  +// Licensed under the Apache License, Version 2.0 (the "License");
> >  +// you may not use this file except in compliance with the
License.
> >  +// You may obtain a copy of the License at
> >  +//
> >  +//     http://www.apache.org/licenses/LICENSE-2.0
> >  +//
> >  +// Unless required by applicable law or agreed to in writing,
> software
> >  +// distributed under the License is distributed on an "AS IS"
> BASIS,
> >  +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> >  +// See the License for the specific language governing permissions
> and
> >  +// limitations under the License.
> >  +
> >  +package org.apache.tapestry.corelib.base;
> >  +
> >  +import org.apache.tapestry.ioc.Messages;
> >  +import org.apache.tapestry.ioc.internal.util.MessagesImpl;
> >  +
> >  +public final class BaseMessages
> >  +{
> >  +    private static final Messages MESSAGES =
> MessagesImpl.forClass(BaseMessages.class);
> >  +
> >  +    public static String nullValueInPath(final String path)
> >  +    {
> >  +        return MESSAGES.format("null-value-in-path", path);
> >  +    }
> >  +}
> >
> >  Added: tapestry/tapestry5/trunk/tapestry-
>
core/src/main/resources/org/apache/tapestry/corelib/base/BaseStrings.pr
> operties
> >  URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-
>
core/src/main/resources/org/apache/tapestry/corelib/base/BaseStrings.pr
> operties?rev=630376&view=auto
> >
>
=======================================================================
> =======
> >  --- tapestry/tapestry5/trunk/tapestry-
>
core/src/main/resources/org/apache/tapestry/corelib/base/BaseStrings.pr
> operties (added)
> >  +++ tapestry/tapestry5/trunk/tapestry-
>
core/src/main/resources/org/apache/tapestry/corelib/base/BaseStrings.pr
> operties Fri Feb 22 18:17:09 2008
> >  @@ -0,0 +1,15 @@
> >  +# Copyright 2008 The Apache Software Foundation
> >  +#
> >  +# Licensed under the Apache License, Version 2.0 (the "License");
> >  +# you may not use this file except in compliance with the License.
> >  +# You may obtain a copy of the License at
> >  +#
> >  +#     http://www.apache.org/licenses/LICENSE-2.0
> >  +#
> >  +# Unless required by applicable law or agreed to in writing,
> software
> >  +# distributed under the License is distributed on an "AS IS"
BASIS,
> >  +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> >  +# See the License for the specific language governing permissions
> and
> >  +# limitations under the License.
> >  +
> >  +null-value-in-path=Property '%s' contains a null value in the
path.
> >  \ No newline at end of file
> >
> >  Added: tapestry/tapestry5/trunk/tapestry-
>
core/src/test/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> putTest.java
> >  URL:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-
>
core/src/test/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> putTest.java?rev=630376&view=auto
> >
>
=======================================================================
> =======
> >  --- tapestry/tapestry5/trunk/tapestry-
>
core/src/test/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> putTest.java (added)
> >  +++ tapestry/tapestry5/trunk/tapestry-
>
core/src/test/java/org/apache/tapestry/corelib/base/AbstractPropertyOut
> putTest.java Fri Feb 22 18:17:09 2008
> >  @@ -0,0 +1,57 @@
> >  +// Copyright 2008 The Apache Software Foundation
> >  +//
> >  +// Licensed under the Apache License, Version 2.0 (the "License");
> >  +// you may not use this file except in compliance with the
License.
> >  +// You may obtain a copy of the License at
> >  +//
> >  +//     http://www.apache.org/licenses/LICENSE-2.0
> >  +//
> >  +// Unless required by applicable law or agreed to in writing,
> software
> >  +// distributed under the License is distributed on an "AS IS"
> BASIS,
> >  +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> >  +// See the License for the specific language governing permissions
> and
> >  +// limitations under the License.
> >  +
> >  +package org.apache.tapestry.corelib.base;
> >  +
> >  +import org.testng.annotations.Test;
> >  +import org.apache.tapestry.beaneditor.PropertyModel;
> >  +import org.apache.tapestry.internal.test.InternalBaseTestCase;
> >  +import org.apache.tapestry.PropertyConduit;
> >  +
> >  +public class AbstractPropertyOutputTest extends
> InternalBaseTestCase
> >  +{
> >  +    private final AbstractPropertyOutput propertyOutputFixture =
> new AbstractPropertyOutput()
> >  +    {
> >  +    };
> >  +
> >  +    @Test
> >  +    // Tests TAPESTRY-2182.
> >  +    public void test_null_pointer_exception_message()
> >  +    {
> >  +        final PropertyConduit conduit = mockPropertyConduit();
> >  +        final PropertyModel model = mockPropertyModel();
> >  +        final Object object = new Object();
> >  +
> >  +        propertyOutputFixture.inject(model, object);
> >  +
> >  +        expect(model.getConduit()).andReturn(conduit);
> >  +        expect(conduit.get(object)).andThrow(new
> NullPointerException());
> >  +
> expect(model.getPropertyName()).andReturn("wilma.occupation.address");
> >  +
> >  +        replay();
> >  +
> >  +        try
> >  +        {
> >  +            propertyOutputFixture.readPropertyForObject();
> >  +
> >  +            fail("Expected a NullPointerException to be thrown.");
> >  +        }
> >  +        catch (final NullPointerException ex)
> >  +        {
> >  +            assertEquals(ex.getMessage(), "Property
> 'wilma.occupation.address' contains a null value in the path.");
> >  +        }
> >  +
> >  +        verify();
> >  +    }
> >  +}
> >
> >
> >
> 
> 
> 
> --
> Howard M. Lewis Ship
> 
> Creator Apache Tapestry and Apache HiveMind
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]


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

Reply via email to