Re: svn commit: r908676 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java

[Matthieu Bollot-4]

Hi dev,
I've got some problems since yesterday, may be it is related to this
error :
org.ofbiz.widget.screen.ScreenRenderException: Error rendering screen
[component://common/widget/CommonScreens.xml#FindScreenDecorator]:
java.lang.UnsupportedOperationException (null)

that you can find here :
https://ofbiz-vm.apache.org/sfa/control/FindLeads


in logs I've got :
2010-02-12 14:54:45,274 (Finalizer)
[ EntityListIterator.java:546:ERROR]
---- runtime exception report
--------------------------------------------------
Error closing the SQLProcessor in finalize EntityListIterator
Exception: java.lang.NullPointerException
Message: null
---- stack trace
---------------------------------------------------------------
java.lang.NullPointerException
--------------------------------------------------------------------------------


so it may be related to this commit :

Le mercredi 10 février 2010 à 22:10 +0000, [hidden email] a écrit :
> Author: doogie
> Date: Wed Feb 10 22:10:44 2010
> New Revision: 908676

hope it helps,

--
Matthieu BOLLOT
www.nereide.biz

[Adrian Crum]

Actually, that error message was caused by rev 908700.

-Adrian

Matthieu Bollot wrote:

> Hi dev,
> I've got some problems since yesterday, may be it is related to this
> error :
> org.ofbiz.widget.screen.ScreenRenderException: Error rendering screen
> [component://common/widget/CommonScreens.xml#FindScreenDecorator]:
> java.lang.UnsupportedOperationException (null)
>
> that you can find here :
> https://ofbiz-vm.apache.org/sfa/control/FindLeads
>
>
> in logs I've got :
> 2010-02-12 14:54:45,274 (Finalizer)
> [ EntityListIterator.java:546:ERROR]
> ---- runtime exception report
> --------------------------------------------------
> Error closing the SQLProcessor in finalize EntityListIterator
> Exception: java.lang.NullPointerException
> Message: null
> ---- stack trace
> ---------------------------------------------------------------
> java.lang.NullPointerException
> --------------------------------------------------------------------------------
>
>
> so it may be related to this commit :
>
> Le mercredi 10 février 2010 à 22:10 +0000, [hidden email] a écrit :
>> Author: doogie
>> Date: Wed Feb 10 22:10:44 2010
>> New Revision: 908676
>
> hope it helps,
>

[Adam Heath-2]

Adrian Crum wrote:
> Actually, that error message was caused by rev 908700.

How?  That added an Enum converter; I didn't think ofbiz actually used
enums mutch.

[Adam Heath-2]

Adam Heath wrote:
> Adrian Crum wrote:
>> Actually, that error message was caused by rev 908700.
>
> How?  That added an Enum converter; I didn't think ofbiz actually used
> enums mutch.

Ok, I see it; Is this code trying to convert a String to an Enum?
That's not supported.  To make that work, you also need to pass in a
the target class.  Perhaps we should remove the single arg variant of
convert.

[Adrian Crum-2]

--- On Fri, 2/12/10, Adam Heath <[hidden email]> wrote:

> From: Adam Heath <[hidden email]>
> Subject: Re: svn commit: r908676 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
> To: [hidden email]
> Date: Friday, February 12, 2010, 10:16 AM
> Adam Heath wrote:
> > Adrian Crum wrote:
> >> Actually, that error message was caused by rev
> 908700.
> >
> > How?  That added an Enum converter; I didn't
> think ofbiz actually used
> > enums mutch.
>
> Ok, I see it; Is this code trying to convert a String to an
> Enum?
> That's not supported.  To make that work, you also
> need to pass in a
> the target class.  Perhaps we should remove the single
> arg variant of
> convert.

Most likely the calling code defaulted to some other conversion.

[Adrian Crum-2]

--- On Fri, 2/12/10, Adam Heath <[hidden email]> wrote:

> From: Adam Heath <[hidden email]>
> Subject: Re: svn commit: r908676 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
> To: [hidden email]
> Date: Friday, February 12, 2010, 10:16 AM
> Adam Heath wrote:
> > Adrian Crum wrote:
> >> Actually, that error message was caused by rev
> 908700.
> >
> > How?  That added an Enum converter; I didn't
> think ofbiz actually used
> > enums mutch.
>
> Ok, I see it; Is this code trying to convert a String to an
> Enum?
> That's not supported.  To make that work, you also
> need to pass in a
> the target class.  Perhaps we should remove the single
> arg variant of
> convert.

Why would we want to do that? Also, why not extend AbstractConverter like the other classes? The exception goes away if the converter is written like the rest. Also, StringToEnum.convert(String obj) should throw ConversionException.

[Adam Heath-2]

Matthieu Bollot wrote:
> Hi dev,
> I've got some problems since yesterday, may be it is related to this
> error :
> org.ofbiz.widget.screen.ScreenRenderException: Error rendering screen
> [component://common/widget/CommonScreens.xml#FindScreenDecorator]:
> java.lang.UnsupportedOperationException (null)
>
> that you can find here :
> https://ofbiz-vm.apache.org/sfa/control/FindLeads

Fixed in 909718.

[Adam Heath-2]

Adrian Crum wrote:

> --- On Fri, 2/12/10, Adam Heath <[hidden email]> wrote:
>> From: Adam Heath <[hidden email]>
>> Subject: Re: svn commit: r908676 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
>> To: [hidden email]
>> Date: Friday, February 12, 2010, 10:16 AM
>> Adam Heath wrote:
>>> Adrian Crum wrote:
>>>> Actually, that error message was caused by rev
>> 908700.
>>> How?  That added an Enum converter; I didn't
>> think ofbiz actually used
>>> enums mutch.
>> Ok, I see it; Is this code trying to convert a String to an
>> Enum?
>> That's not supported.  To make that work, you also
>> need to pass in a
>> the target class.  Perhaps we should remove the single
>> arg variant of
>> convert.
>
> Why would we want to do that? Also, why not extend
> AbstractConverter like the other classes? The exception
> goes away if the converter is written like the rest. Also,
> StringToEnum.convert(String obj) should throw ConversionException.

I choose not to extend AbstractConverter, because StringToEnum
requires a concrete class, it can't just pick any random class that
happens to extend Enum.  That's why I added convert(targetClass, obj)
variants.

It's also why I throw UnsupportedOperationException, instead of
ConversionException, during convert(obj).  You shouldn't do doing
unknown conversions, when dealing with enums.  In those cases, you
should know that an enum is being requested, and use the other convert
method.

While tracking this one down, I discovered that the url in question
was eventually calling into ObjectType, and the conversion framework,
and was trying to do a String->Object conversion; that would never
work, and is stupid to do.  The bug that I fixed in 909718 made the
StringToEnum converter think it could handle that request.

Also, this commit allows me to possibly remove the
checkExtendsImplements method.


>
>
>
>
>      

[Adrian Crum-2]

--- On Fri, 2/12/10, Adam Heath <[hidden email]> wrote:
> It's also why I throw UnsupportedOperationException,
> instead of
> ConversionException, during convert(obj).  You
> shouldn't do doing
> unknown conversions, when dealing with enums.  In
> those cases, you
> should know that an enum is being requested, and use the
> other convert
> method.

But the client code is checking for ConversionException, and typically takes an alternate path when one is thrown. Throwing UnsupportedOperationException breaks that pattern.

[Adam Heath-2]

Adrian Crum wrote:

> --- On Fri, 2/12/10, Adam Heath <[hidden email]> wrote:
>> It's also why I throw UnsupportedOperationException,
>> instead of
>> ConversionException, during convert(obj).  You
>> shouldn't do doing
>> unknown conversions, when dealing with enums.  In
>> those cases, you
>> should know that an enum is being requested, and use the
>> other convert
>> method.
>
> But the client code is checking for ConversionException, and typically takes an alternate path when one is thrown. Throwing UnsupportedOperationException breaks that pattern.

ConversionException means that there was a problem converting.
Calling convert(Object) on StringToEnum is not a valid call to make,
it's a more serious issue.  It's not that a conversion failed, it's
that a conversion is not even allowed to take place.  You *must* use
the convert(Class, Object) variant.

StringToEnum can handle all enums, but can only create concrete
sub-classes.  It's not like List or Map, which allows any
implementation to be used.