You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@myfaces.apache.org by Mike Kienenberger <mk...@gmail.com> on 2007/02/27 21:04:00 UTC
Re: MYFACES-1545 / MYFACES-1532
On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
> Some other question to Mike. You have just closed MYFACES-1545. I think,
> it's something very similar to MYFACES-1532, I've created 13.02.2007. I
> have asked about this in both of DEV and USERS lists, but nobody has
> answered, so I'm still not sure, whether it's really a bug, or just a
> feature. (There are some other related issues too, however they aren't
> linked to this one.) Would you please confirm this or close 1532 too?
Paul, please use a new thread when asking unrelated questions.
I closed MYFACES-1545 for the reasons listed in the issue, after
searching the mailing lists for relevent posts by the reporter. There
were none. Custom converters on UISelectOne items are quite common,
and I've yet to have an issue with them.
It sounds like there could be a bug in h:selectBooleanCheckbox that
causes the problem you described in MYFACES-1532. I've never used a
converter on this component, and it could very well be as you
described, so I'm going to leave that open, especially since you
provided example code. I vaguely recall issues with the
convertBoolean converter in the sandbox in the distant past, so it
wouldn't be at all surprising.
Again, though, you have an unrelated second issue described in the
same issue. I'd recommend voiding that part and opening another
issue (although if it's a spec issue, it'll just be closed out of hand
as those are outside of the scope of MyFaces to handle).
Re: MYFACES-1545 / MYFACES-1532
Posted by Paul Iov <pa...@voller-ernst.de>.
Sure. Unfortunately i have deadline of one unrelated project right now.
It can take a little bit time to figure it out clearly (I've already
seen some mistakes in my code). I'll try to post it as soon as
possible... probably tomorrow. Is it OK?
Regards,
paul
Martin Marinschek schrieb:
> Can you post your suggested changes again, and interweave the existing
> code, so that I can look at it step by step and see what your changes
> would mean?
>
> regards,
>
> Martin
>
> On 3/1/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> Hi Martin,
>>
>> I've checked this with the latest nightly build 1.1.6.
>> 1. getValue() behavior seems to be correct now (at least without
>> conversion) :)
>> 2. conversion (and validation?) issue is still present :(
>>
>> my test case:
>> bean
>> private boolean boolBoolean = true;
>> private String strBoolean = "true";
>> private Date datDate = new Date();
>> private String strDate = "01.01.2007";
>> (with all correct get/set)
>>
>> page
>> <h:selectBooleanCheckbox id="myBoolBoolean"
>> value="#{MyBean.boolBoolean}"/>
>> --OK
>>
>> <h:selectBooleanCheckbox id="myStrBoolean"
>> value="#{MyBean.strBoolean}"/>
>> --fails with Expected submitted value of type Boolean for Component:
>> ...myStrBoolean
>>
>> <h:selectBooleanCheckbox id="myStrBoolean"
>> value="#{MyBean.strBoolean}"
>> converter="MyBooleanConverter"/>
>> --fails with Expected submitted value..., converter not called
>>
>> <t:inputCalendar value="#{MyBean.datDate}"/>
>> --OK
>>
>> <t:inputCalendar value="#{MyBean.strDate}"/>
>> --fails with Expected submitted value...
>>
>> <t:inputCalendar value="#{MyBean.strDate}"
>> converter="MyDateConverter" />
>> --fails with Expected submitted value..., converter not called
>>
>> "Expected submitted value..." is thrown in all cases during initial
>> rendering, so it's not possible to check behavior on submit etc.
>>
>> Regards,
>> paul
>>
>>
>> Martin Marinschek schrieb:
>> > Hi Paul,
>> >
>> > I couldn't follow all your arguments in this rather long mail, but
>> > here a short thought for you to consider - it might help you
>> > determining if you've found a bug or not:
>> >
>> > the submitted value should always be used for rendering, if it is not
>> > 'null'. If it is null, the getValue() method of the component should
>> > be called (I've just found an issue with that, posted to the
>> > dev-list). getValue() should in fact return a local value, if it is
>> > set (a submitted value gets to be a local value after conversion and
>> > validation), if it is not set, the value-binding should be evaluated
>> > (and the backing-bean value used). Does any of the behaviour you see
>> > in this component contradict this?
>> >
>> > regards,
>> >
>> > Martin
>> >
>> > On 3/1/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> >> Well, I've reviewed the sources related to this issue again before
>> >> posting some comments/code to JIRA and can state now, that it's
>> really
>> >> obscure. At least to me :( I think it's better to figure out my
>> doubts
>> >> about this converter stuff again. Perhaps someone can just point
>> out my
>> >> mistake or else confirm the global problem with converter handling in
>> >> current code.
>> >>
>> >> Few things, that are clear:
>> >> 1. Description of value attribute says:
>> >> "The initial value of this component. This value is generally set
>> as a
>> >> value-binding in the form #{myBean.myProperty}, where myProperty
>> can be
>> >> any data-type of Java (also user-defined data-types), if a
>> converter for
>> >> this data-type exists. Special cases...[irrelevant]"
>> >> This is true for many components, but I'm referencing here and
>> below the
>> >> h:selectBooleanCheckbox since it was the start point for me.
>> >>
>> >> 2. In most cases it not works but produce *Expected submitted
>> value of
>> >> type* ... *for Component* ...if value attribute is bound to a
>> property
>> >> of any type excepts expected one (boolean in case of
>> >> h:selectBooleanCheckbox). This happens during initial rendering of
>> >> component, so there is no submitted value. The only value existing at
>> >> this time is the value of bound bean property.
>> >>
>> >> 3. Neither provided custom converter nor standard one eventually
>> >> registered for expected type is called to try to convert this bean
>> value
>> >> into expected type.
>> >>
>> >> After hours studying of source code I have grave doubts about
>> >> correctness of how the rendering of components is handled generally
>> >> because the problem lays IMHO in two very common utility classes:
>> >>
>> >> org.apache.myfaces.shared_impl.renderkit.RendererUtils
>> >> The static methods of this class are mostly used to accomplish the
>> >> 'standard' tasks such as retrieving of component's value, so if I'm
>> >> right, this problem affects wide range of html components and
>> >> automatically theirs descendants. At least the components dealing
>> with
>> >> Boolean and Date as value should be affected. I'm not sure if this
>> >> utility method's hierarchy (since both of getBooleanValue() and
>> >> getDateValue() relays on getObjectValue() ) is the right place to
>> >> implement handling of custom converters, but I have a questions
>> >> regarding this hierarchy too since it seems to be a part of problem.
>> >> Why should the getObjectValue() call the .getSubmittedValue() and not
>> >> the .getValue() method of EditableValueHolder? This causes that all
>> >> affected components becomes null/false/empty as value during initial
>> >> rendering regardless of how the underlying bean's properties are
>> >> initialized since there is no SUBMITTED values at this time and the
>> >> method always returns null.
>> >>
>> >> org.apache.myfaces.shared_impl.taglib.UIComponentTagUtils
>> >> Probably THE right place to implement converters... but why should
>> the
>> >> renderkit use own utility class above to accomplish all of GETs
>> and the
>> >> SETs are grouped here? IMHO it should be the common task, since about
>> >> any component can provide custom converter and GETs as well SETs
>> should
>> >> use it in some consistent way.
>> >>
>> >> I've tried first to implement usage of custom/standard converter only
>> >> for getBoolean() but then I have realized that it should be done
>> >> consistent for all standard types. That's why I'm not providing
>> any path
>> >> right now (and the code in past posting is just an idea of how it
>> can be
>> >> handled). The key questions that I'd like to ask first are:
>> >> Where it should be done? (RendererUtils or UIComponentTagUtils)
>> >> How exactly should it work?
>> >>
>> >> The common definition from spec(?) is beautiful, but how to
>> implement it
>> >> really consistent? Let's say, X is the expected by component type
>> and Y
>> >> is the type effectively provided by underlying value binding
>> property.
>> >> If X and Y are not the same type, there are many different conversion
>> >> cases.
>> >>
>> >> a. component retrieves the value from value-binding ((Y)object or
>> >> null -> X)
>> >>
>> >> b. component sets submitted value to value binding (X -> Y)
>> >>
>> >> c. component retrieves submitted value from request (String -> X)
>> >> d. component renders retrieved value (X -> String)
>> >>
>> >> Which should be covered by custom converter and how, and which not?
>> >>
>> >> First of all, for c and d component have to know itself how to render
>> >> own type X = how to convert submitted String back to X. Let's name
>> this
>> >> knowledge 'own conversion [String <-> X]'. Generally it is not
>> affected
>> >> by any extra conversion, but I guess there are cases when it should
>> >> be done!
>> >>
>> >> If no custom converter provided, we can still try to obtain standard
>> >> one, capable to convert [String <-> X] and to use
>> >> - a: getAsObject( (String)value )
>> >> - b: getAsString(X)
>> >> It won't work, if underlying bean property is not String, but then we
>> >> can pass throw the exception and the message of this exception
>> will be
>> >> absolutely clear to user.
>> >>
>> >> If custom converter provided, but it's unable to cast type
>> returned by
>> >> getAsObject() to our X type, we have two possibility:
>> >> 1) we can rely on our own 'knowledge' about how to convert [String
>> <->
>> >> X] and suppose that getAsString() of custom converter returns
>> something
>> >> compatible. Then we can use:
>> >> - a: getAsString() + own conversion String -> X
>> >> - b: own conversion X->String + getAsObject()
>> >>
>> >> 2) we can try to obtain standard converter [String <-> X] and use it
>> >> instead of own 'knowledge' like:
>> >> - a: customConv.getAsString() + standardConv.getAsObject()
>> >> - b: standardConv.getAsString() + customConv.getAsObject()
>> >>
>> >> The case when custom converter provided and return type of
>> getAsObject()
>> >> is compatible with our X seems to be clear, but in fact it's a most
>> >> complicated one. There are two reason to provide custom converter for
>> >> standard type. Either the value should be rendered by non-textual
>> >> component (i.e. Boolean + checkBox and user going to implement own
>> >> tristate logic, like true|false|unknown) or some data manipulation
>> >> should be done (i.e. Date parsing/formating). I'm not sure how
>> this case
>> >> should be handled to preserve maximum flexibility.
>> >>
>> >> Just an idea: it should be different for both component kinds and
>> >> probably, we must ignore own rendering behavior as well.
>> >> For textual components:
>> >> - a and *c*: getAsObject()
>> >> - b and *d*: getAsString()
>> >>
>> >> For non-textual components (checkBox):
>> >> - a,b and *c*: getAsString() + getAsObject()
>> >> - *d*: getAsString()
>> >>
>> >> Well, the problem seems to be really complex. I think that
>> providing any
>> >> 'light' path for some little part of this problem is not a good idea.
>> >> I'm also not familiar enough with JSF myself to state weather this
>> >> contradict spec or is just a minor issue, which should be worked
>> around
>> >> some way until new better version i.e. 2.0 is released. I've just
>> tried
>> >> to use this framework's feature and discovered a totally
>> inconsistence.
>> >> I can provide test cases as well as check this against RI or
>> figure it
>> >> out clearly in JIRA etc.but that's exactly why I'd like to ask first,
>> >> weather it makes any sense.
>> >>
>> >> best regards,
>> >> paul
>> >>
>> >>
>> >> Mike Kienenberger schrieb:
>> >> > Paul, in my own case, I wasn't reading myfaces email in depth
>> the week
>> >> > you posted this. Sometimes things just fall through the cracks
>> >> > because people are busy with other things.
>> >> >
>> >> > Sometimes the issue is so obscure that few people really know
>> how to
>> >> > respond to it.
>> >> >
>> >> > In both cases, the best thing to do is
>> >> >
>> >> > 1) prove there's a bug. Either test the same code using the
>> JSF RI
>> >> > or another version of MyFaces. Or show that it contradicts the
>> JSF
>> >> > Specs. It's best to provide a simplified example demonstrating
>> the
>> >> > problem -- this would be easy in your particular issue. The
>> easier
>> >> > you make it for someone else to look at the problem, the more
>> likely
>> >> > someone will do so.
>> >> >
>> >> > 2) Provide a patch (like it looks like you're doing above) in JIRA.
>> >> > If no one has done anything with it, and the bug is proven and the
>> >> > patch is easily reviewed, feel free to ping the list after a
>> week of
>> >> > no action. Repeat weekly until someone either rejects or approves
>> >> > it, or otherwise responds :-) Again, having a test case or example
>> >> > makes it easier to test your patch to verify it should be applied.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> >> >>
>> >> >> Thank you for answer, Mike.
>> >> >>
>> >> >> I'm still not sure, weather the second part is really an
>> >> unrelated spec
>> >> >> issue because it could be just the consequence of the first one.
>> >> >> So I've decide to describe both together thinking, it can help
>> >> >> during code
>> >> >> review since it belongs to very common part of core implementation
>> >> >> (org.apache.myfaces.shared.renderkit.RendererUtils). If my
>> >> >> suggestion is wrong, the second part can be just ignored.
>> Please see
>> >> >> also my
>> >> >> comment to MYFACES-1452, which was the start point of my
>> >> >> 'investigation' :)
>> >> >>
>> >> >> I've tried to solve the problem myself, but it was just a
>> partially
>> >> >> solution for me (respectively to second part of MYFACES-1532
>> >> >> description).
>> >> >> Since no one has answered in mailing list, I've thinking my
>> >> >> suggestion is
>> >> >> just wrong and haven't provide this as path. But if I'm right, the
>> >> same
>> >> >> should affect other standard types too (i.e. Date etc.)
>> >> >>
>> >> >> Anyway, here's my code. This method is intended to replace
>> existing
>> >> >> getBooleanValue() in
>> >> >> org.apache.myfaces.shared.renderkit.RendererUtils, which is
>> >> >> called from
>> >> >>
>> >>
>> org.apache.myfaces.shared.renderkit.html.HtmlCheckboxRendererBase.encodeEnd().
>>
>> >>
>> >> >>
>> >> >> public static Boolean
>> >> >> getBooleanValueForCheckbox(FacesContext facesContext,
>> >> >> UIComponent component) {
>> >> >> Object value = getObjectValue(component);
>> >> >>
>> >> >> // This call relays on the implementation of getObjectValue()!!!
>> >> >> // If uiComponent is not an instance of ValueHolder, the
>> >> >> // IllegalArgumentException should be already thrown.
>> >> >> Converter converter = ((ValueHolder) component).getConverter();
>> >> >>
>> >> >> if (null == value || value instanceof Boolean) {
>> >> >> return (Boolean) value;
>> >> >> }
>> >> >>
>> >> >> if (null == converter && value instanceof String) {
>> >> >> // it's still correct to convert String into Boolean, because
>> >> >> // Boolean provides constructor Boolean(String x)!
>> >> >> return new Boolean(value.toString());
>> >> >> }
>> >> >>
>> >> >> // If component provides no custom converter, we
>> >> >> // must try to obtain a standard one from Application
>> >> >> if (null == converter) {
>> >> >> try {
>> >> >> converter = facesContext.getApplication().createConverter(
>> >> >> value.getClass());
>> >> >> } catch (FacesException ex) {
>> >> >> throw new IllegalArgumentException("Component : "
>> >> >> + getPathToComponent(component)
>> >> >> + " expects value of type Boolean.\n"
>> >> >> + "Neither standard converter for "
>> >> >> + value.getClass().getName()
>> >> >> + " nor custom converter provided.");
>> >> >> }
>> >> >> }
>> >> >>
>> >> >> if (null != converter) {
>> >> >> try {
>> >> >>
>> >> >> /* Try to convert it into String. That is!
>> >> >> * The getAsObject() provides the conversion of string value's
>> >> >> * representation into custom user type. We are expecting a
>> Boolean,
>> >> >> * but the value itself is not a Boolean, so we need to get String
>> >> >> first.
>> >> >> */
>> >> >> String strValue = converter.getAsString(facesContext,
>> >> >> component, value);
>> >> >>
>> >> >> boolean warn = (null == strValue);
>> >> >> warn = warn
>> >> >> || !(strValue.equalsIgnoreCase(Boolean.TRUE.toString()) ||
>> strValue
>> >> >> .equalsIgnoreCase(Boolean.TRUE.toString()));
>> >> >>
>> >> >> if (warn) {
>> >> >> /* Note that this situation is not an error! We have some
>> >> >> * converter and it got back some string, and no exception has
>> been
>> >> >> * thrown!
>> >> >> */
>> >> >> log.warn("Component "
>> >> >> + getPathToComponent(component)
>> >> >> + " expects value of type Boolean. The value was converted with "
>> >> >> + converter.getClass().getName()
>> >> >> + " into "
>> >> >> + strValue
>> >> >> + ", what is neither true nor false! false assumed.");
>> >> >> }
>> >> >>
>> >> >> return new Boolean(strValue);
>> >> >>
>> >> >> } catch (ConverterException ex) {
>> >> >> throw new IllegalArgumentException("Component : "
>> >> >> + getPathToComponent(component)
>> >> >> + " expects value of type Boolean.\n"
>> >> >> + "Unable to convert " + value.getClass().getName()
>> >> >> + " into Boolean.");
>> >> >> }
>> >> >>
>> >> >> } else {
>> >> >> throw new IllegalArgumentException("Component "
>> >> >> + getPathToComponent(component)
>> >> >> + " expects value of type Boolean but "
>> >> >> + value.getClass().getName() + " was found.");
>> >> >> }
>> >> >>
>> >> >> }
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >> regards,
>> >> >> paul
>> >> >>
>> >> >> Mike Kienenberger schrieb:
>> >> >> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> >> >>
>> >> >> Some other question to Mike. You have just closed MYFACES-1545. I
>> >> think,
>> >> >> it's something very similar to MYFACES-1532, I've created
>> >> 13.02.2007. I
>> >> >> have asked about this in both of DEV and USERS lists, but
>> nobody has
>> >> >> answered, so I'm still not sure, whether it's really a bug, or
>> >> just a
>> >> >> feature. (There are some other related issues too, however they
>> >> aren't
>> >> >> linked to this one.) Would you please confirm this or close 1532
>> >> too?
>> >> >>
>> >> >> Paul, please use a new thread when asking unrelated questions.
>> >> >>
>> >> >> I closed MYFACES-1545 for the reasons listed in the issue, after
>> >> >> searching the mailing lists for relevent posts by the reporter.
>> >> There
>> >> >> were none. Custom converters on UISelectOne items are quite
>> common,
>> >> >> and I've yet to have an issue with them.
>> >> >>
>> >> >> It sounds like there could be a bug in h:selectBooleanCheckbox
>> that
>> >> >> causes the problem you described in MYFACES-1532. I've never
>> >> used a
>> >> >> converter on this component, and it could very well be as you
>> >> >> described, so I'm going to leave that open, especially since you
>> >> >> provided example code. I vaguely recall issues with the
>> >> >> convertBoolean converter in the sandbox in the distant past,
>> so it
>> >> >> wouldn't be at all surprising.
>> >> >>
>> >> >> Again, though, you have an unrelated second issue described
>> in the
>> >> >> same issue. I'd recommend voiding that part and opening another
>> >> >> issue (although if it's a spec issue, it'll just be closed out of
>> >> hand
>> >> >> as those are outside of the scope of MyFaces to handle).
>> >> >>
>> >> >>
>> >> >>
>> >> >
>> >>
>> >>
>> >
>> >
>>
>>
>
>
Re: MYFACES-1545 / MYFACES-1532
Posted by Paul Iov <pa...@voller-ernst.de>.
Martin Marinschek schrieb:
> Can you post your suggested changes again, and interweave the existing
> code, so that I can look at it step by step and see what your changes
> would mean?
>
> regards,
>
> Martin
>
Hi Martin,
here is a 'little' peace of code for RendererUtils :) It implements,
what I've talking about in my early posting. The
getBooleanValueForCheckbox() method is intended to replace existing
getBooleanValue(), which is currently called from renderer in
RenderUtils however wit one more parameter, so this call should be
changed too (HtmlCheckboxRendererBase line 62).
It is something like proof-of-concept. I've not implement yet the
controversy case when custom converter provides the same type as our
'expected' (see comments in code) since IMHO it should be really
clarified first. The String2Boolean() can be probably implemented some
smarter way too. But I think, this approach can be used in
UIComponentTagUtils as well, since it should work in both way.
Regards,
paul
/**
* Envelope for results of obtaining converter.
*/
public static class FoundConverter{
private int converterType = 0; //int due to Java 1.42
compatibility! (Use Enum in 5+ ?)
private Converter converter = null;
private Class customClass = null;
/**
* @return found conwerter if type > 0, otherweis null.
*/
public final Converter getConverter() {
return converter;
}
public final void setConverter(Converter converter) {
this.converter = converter;
}
/**
* @return -1 not required;<br>
* 0 not found;<br>
* 1 custom;<br>
* 2 standard for object's class<br>
* 3 standard for target class (last choice!)
*/
public final int getConverterType() {
return converterType;
}
public final void setConverterType(int converterType) {
this.converterType = converterType;
}
public final Class getCustomClass() {
return customClass;
}
public final void setCustomClass(Class customClass) {
this.customClass = customClass;
}
}
/**
* @param value - object, usually returned by getValue() or
getSubmittedValue()<br>
* @param targetClass - class to which value should be converted<br>
* @param facesContext - Faces context<br>
* @param component - component<br>
* @param probe - string to be used for determining return type of
getAsObject() of custom converter<br>
*
* @return an instance of envelope class FoundConverter, containing
evtl. found converter and
* information on type of this converter.
*
* Obtains apropriative converter in order:<br>
* custom converter,<br>
* standard converter registered for value's class,<br>
* standard converter registered for target class.<br>
*
* Returns no converter if:<br>
* - no custom converter provided but value is already of suitable
type;<br>
* - no custom converter provided and value is null.<br>
* If custom converter provided, tries also to call getAsObject()
with probe
* as argument to determine custom type.
*
*
* @throws IllegalArgumentException if value is not null, not of
suitable type and no converter can be found.
*/
public static FoundConverter obtainConverter(Object value, Class
targetClass,
FacesContext facesContext,
UIComponent component,
String probe) throws
IllegalArgumentException{
FoundConverter result = new FoundConverter();//result envelope
//--- check if component is a ValueHolder ---
if (! (component instanceof ValueHolder))
throw new IllegalArgumentException("Component : " +
getPathToComponent(component) + "is not a ValueHolder");
Converter converter = ((ValueHolder) component).getConverter();
//--- custom converter found ---
if (converter != null){
try{
//--- try to determine, which type returns getAsObject()
of this converter
Object x = converter.getAsObject(facesContext,
component, probe);
result.setCustomClass(x.getClass());
}
catch(Exception ex){
log.warn("Unable to determine object type returned by
custom converter: "+
converter.getClass().getName()+
" with probe string \""+probe+"\"");
}
result.setConverterType(1);//custom converter found
result.setConverter(converter);
return result;
}
//--- NO custom converter ---
if(value == null){
result.setConverterType(0);//not found, undefined for null
return result;
}
//--- value not null but already of the same type ---
if ( targetClass.isAssignableFrom(value.getClass()) ){
result.setConverterType(-1); //not found, not required!
return result;
}
/*
* If component provides no custom converter, we
* cann still try to obtain a standard one from Application
*/
try {
//--- try first for type of value... ---
converter = facesContext.getApplication()
.createConverter(value.getClass());
result.setConverterType(2);//standard for value's type
result.setConverter(converter);
return result;
} catch (FacesException ex) {
//--- ...then for target type ---
try{
converter = facesContext.getApplication()
.createConverter(targetClass);
result.setConverterType(3);//standard for target type
result.setConverter(converter);
return result;
}
catch(FacesException ex2){
throw new IllegalArgumentException("Component : "
+ getPathToComponent(component)+
" expects value of type "+ targetClass.getName() +
" but " + value.getClass().getName() + " was
passed and"+
" no suitable converter found.");
}
}
}
/**
* @param str - string to convert into Boolean
* @return Boolean
*
* null-safe conversion of strings like "true","yes", "on" etc. to
Boolean.
*/
public static Boolean String2Boolean(String str){
if(null != str &&
(
str.equalsIgnoreCase("true") ||
str.equalsIgnoreCase("yes") ||
str.equalsIgnoreCase("on") ||
str.equalsIgnoreCase("1")
)
)
return Boolean.TRUE;
else
return Boolean.FALSE;
}
public static Boolean getBooleanValueForCheckbox(FacesContext
facesContext,
UIComponent component) {
/*
* The value is returned in order: getSubmittedValue(), getValue()
*/
Object value = getObjectValue(component);
FoundConverter xConverter =
obtainConverter(value,Boolean.class,facesContext,component,"true");
String valueClassName = (null != value) ?
value.getClass().getName() : "null";
String strExceptionMsg = "Component: " +
getPathToComponent(component)+
" expects value of type Boolean, but "+
valueClassName + " passed.";
try{
switch (xConverter.getConverterType()){
case -1:
return (Boolean)value;
case 0:
log.warn(strExceptionMsg + " FALSE assumed since no
converter provided.");
return Boolean.FALSE; // Boolean(null) -> false
/*
* Custom converter(1) as well as standard converter for
value's type(2)
* provide conversion [String <-> custom-type] so if we
are going
* to use it, we have to call getAsString() and then
* try to convert that String into our expected type
*/
case 1:
//The conversion logic in case when custom converter
produce the same
//type of 'object' as our expected type, that I've
describe in my posting
//early, can be implemented here... i.e.:
//
//if(xConverter.getCustomClass() != null){
// if
(Boolean.class.isAssignableFrom(xConverter.getCustomClass())){
// ...
// }
//}
case 2:
String xString =
xConverter.getConverter().getAsString(facesContext, component, value);
return String2Boolean(xString);
/*
* Standard converter for target type(3) provide
conversion [String <-> our-expected-type]
* so we can get value.toString() first and then call
getAsObject()
*/
case 3:
String yString = value.toString();
return
(Boolean)xConverter.getConverter().getAsObject(facesContext, component,
yString);
default:
throw new IllegalArgumentException(strExceptionMsg +
" No suitable converter available.");
}//case
}
catch(ConverterException ex){
throw new IllegalArgumentException(strExceptionMsg +
" Conversion with " +
xConverter.getConverter().getClass().getName()+
" failed."
);
}
}
//----------------------- END OF CODE --------------------------------
Re: MYFACES-1545 / MYFACES-1532
Posted by Martin Marinschek <ma...@gmail.com>.
Can you post your suggested changes again, and interweave the existing
code, so that I can look at it step by step and see what your changes
would mean?
regards,
Martin
On 3/1/07, Paul Iov <pa...@voller-ernst.de> wrote:
> Hi Martin,
>
> I've checked this with the latest nightly build 1.1.6.
> 1. getValue() behavior seems to be correct now (at least without
> conversion) :)
> 2. conversion (and validation?) issue is still present :(
>
> my test case:
> bean
> private boolean boolBoolean = true;
> private String strBoolean = "true";
> private Date datDate = new Date();
> private String strDate = "01.01.2007";
> (with all correct get/set)
>
> page
> <h:selectBooleanCheckbox id="myBoolBoolean"
> value="#{MyBean.boolBoolean}"/>
> --OK
>
> <h:selectBooleanCheckbox id="myStrBoolean" value="#{MyBean.strBoolean}"/>
> --fails with Expected submitted value of type Boolean for Component:
> ...myStrBoolean
>
> <h:selectBooleanCheckbox id="myStrBoolean" value="#{MyBean.strBoolean}"
> converter="MyBooleanConverter"/>
> --fails with Expected submitted value..., converter not called
>
> <t:inputCalendar value="#{MyBean.datDate}"/>
> --OK
>
> <t:inputCalendar value="#{MyBean.strDate}"/>
> --fails with Expected submitted value...
>
> <t:inputCalendar value="#{MyBean.strDate}"
> converter="MyDateConverter" />
> --fails with Expected submitted value..., converter not called
>
> "Expected submitted value..." is thrown in all cases during initial
> rendering, so it's not possible to check behavior on submit etc.
>
> Regards,
> paul
>
>
> Martin Marinschek schrieb:
> > Hi Paul,
> >
> > I couldn't follow all your arguments in this rather long mail, but
> > here a short thought for you to consider - it might help you
> > determining if you've found a bug or not:
> >
> > the submitted value should always be used for rendering, if it is not
> > 'null'. If it is null, the getValue() method of the component should
> > be called (I've just found an issue with that, posted to the
> > dev-list). getValue() should in fact return a local value, if it is
> > set (a submitted value gets to be a local value after conversion and
> > validation), if it is not set, the value-binding should be evaluated
> > (and the backing-bean value used). Does any of the behaviour you see
> > in this component contradict this?
> >
> > regards,
> >
> > Martin
> >
> > On 3/1/07, Paul Iov <pa...@voller-ernst.de> wrote:
> >> Well, I've reviewed the sources related to this issue again before
> >> posting some comments/code to JIRA and can state now, that it's really
> >> obscure. At least to me :( I think it's better to figure out my doubts
> >> about this converter stuff again. Perhaps someone can just point out my
> >> mistake or else confirm the global problem with converter handling in
> >> current code.
> >>
> >> Few things, that are clear:
> >> 1. Description of value attribute says:
> >> "The initial value of this component. This value is generally set as a
> >> value-binding in the form #{myBean.myProperty}, where myProperty can be
> >> any data-type of Java (also user-defined data-types), if a converter for
> >> this data-type exists. Special cases...[irrelevant]"
> >> This is true for many components, but I'm referencing here and below the
> >> h:selectBooleanCheckbox since it was the start point for me.
> >>
> >> 2. In most cases it not works but produce *Expected submitted value of
> >> type* ... *for Component* ...if value attribute is bound to a property
> >> of any type excepts expected one (boolean in case of
> >> h:selectBooleanCheckbox). This happens during initial rendering of
> >> component, so there is no submitted value. The only value existing at
> >> this time is the value of bound bean property.
> >>
> >> 3. Neither provided custom converter nor standard one eventually
> >> registered for expected type is called to try to convert this bean value
> >> into expected type.
> >>
> >> After hours studying of source code I have grave doubts about
> >> correctness of how the rendering of components is handled generally
> >> because the problem lays IMHO in two very common utility classes:
> >>
> >> org.apache.myfaces.shared_impl.renderkit.RendererUtils
> >> The static methods of this class are mostly used to accomplish the
> >> 'standard' tasks such as retrieving of component's value, so if I'm
> >> right, this problem affects wide range of html components and
> >> automatically theirs descendants. At least the components dealing with
> >> Boolean and Date as value should be affected. I'm not sure if this
> >> utility method's hierarchy (since both of getBooleanValue() and
> >> getDateValue() relays on getObjectValue() ) is the right place to
> >> implement handling of custom converters, but I have a questions
> >> regarding this hierarchy too since it seems to be a part of problem.
> >> Why should the getObjectValue() call the .getSubmittedValue() and not
> >> the .getValue() method of EditableValueHolder? This causes that all
> >> affected components becomes null/false/empty as value during initial
> >> rendering regardless of how the underlying bean's properties are
> >> initialized since there is no SUBMITTED values at this time and the
> >> method always returns null.
> >>
> >> org.apache.myfaces.shared_impl.taglib.UIComponentTagUtils
> >> Probably THE right place to implement converters... but why should the
> >> renderkit use own utility class above to accomplish all of GETs and the
> >> SETs are grouped here? IMHO it should be the common task, since about
> >> any component can provide custom converter and GETs as well SETs should
> >> use it in some consistent way.
> >>
> >> I've tried first to implement usage of custom/standard converter only
> >> for getBoolean() but then I have realized that it should be done
> >> consistent for all standard types. That's why I'm not providing any path
> >> right now (and the code in past posting is just an idea of how it can be
> >> handled). The key questions that I'd like to ask first are:
> >> Where it should be done? (RendererUtils or UIComponentTagUtils)
> >> How exactly should it work?
> >>
> >> The common definition from spec(?) is beautiful, but how to implement it
> >> really consistent? Let's say, X is the expected by component type and Y
> >> is the type effectively provided by underlying value binding property.
> >> If X and Y are not the same type, there are many different conversion
> >> cases.
> >>
> >> a. component retrieves the value from value-binding ((Y)object or
> >> null -> X)
> >>
> >> b. component sets submitted value to value binding (X -> Y)
> >>
> >> c. component retrieves submitted value from request (String -> X)
> >> d. component renders retrieved value (X -> String)
> >>
> >> Which should be covered by custom converter and how, and which not?
> >>
> >> First of all, for c and d component have to know itself how to render
> >> own type X = how to convert submitted String back to X. Let's name this
> >> knowledge 'own conversion [String <-> X]'. Generally it is not affected
> >> by any extra conversion, but I guess there are cases when it should
> >> be done!
> >>
> >> If no custom converter provided, we can still try to obtain standard
> >> one, capable to convert [String <-> X] and to use
> >> - a: getAsObject( (String)value )
> >> - b: getAsString(X)
> >> It won't work, if underlying bean property is not String, but then we
> >> can pass throw the exception and the message of this exception will be
> >> absolutely clear to user.
> >>
> >> If custom converter provided, but it's unable to cast type returned by
> >> getAsObject() to our X type, we have two possibility:
> >> 1) we can rely on our own 'knowledge' about how to convert [String <->
> >> X] and suppose that getAsString() of custom converter returns something
> >> compatible. Then we can use:
> >> - a: getAsString() + own conversion String -> X
> >> - b: own conversion X->String + getAsObject()
> >>
> >> 2) we can try to obtain standard converter [String <-> X] and use it
> >> instead of own 'knowledge' like:
> >> - a: customConv.getAsString() + standardConv.getAsObject()
> >> - b: standardConv.getAsString() + customConv.getAsObject()
> >>
> >> The case when custom converter provided and return type of getAsObject()
> >> is compatible with our X seems to be clear, but in fact it's a most
> >> complicated one. There are two reason to provide custom converter for
> >> standard type. Either the value should be rendered by non-textual
> >> component (i.e. Boolean + checkBox and user going to implement own
> >> tristate logic, like true|false|unknown) or some data manipulation
> >> should be done (i.e. Date parsing/formating). I'm not sure how this case
> >> should be handled to preserve maximum flexibility.
> >>
> >> Just an idea: it should be different for both component kinds and
> >> probably, we must ignore own rendering behavior as well.
> >> For textual components:
> >> - a and *c*: getAsObject()
> >> - b and *d*: getAsString()
> >>
> >> For non-textual components (checkBox):
> >> - a,b and *c*: getAsString() + getAsObject()
> >> - *d*: getAsString()
> >>
> >> Well, the problem seems to be really complex. I think that providing any
> >> 'light' path for some little part of this problem is not a good idea.
> >> I'm also not familiar enough with JSF myself to state weather this
> >> contradict spec or is just a minor issue, which should be worked around
> >> some way until new better version i.e. 2.0 is released. I've just tried
> >> to use this framework's feature and discovered a totally inconsistence.
> >> I can provide test cases as well as check this against RI or figure it
> >> out clearly in JIRA etc.but that's exactly why I'd like to ask first,
> >> weather it makes any sense.
> >>
> >> best regards,
> >> paul
> >>
> >>
> >> Mike Kienenberger schrieb:
> >> > Paul, in my own case, I wasn't reading myfaces email in depth the week
> >> > you posted this. Sometimes things just fall through the cracks
> >> > because people are busy with other things.
> >> >
> >> > Sometimes the issue is so obscure that few people really know how to
> >> > respond to it.
> >> >
> >> > In both cases, the best thing to do is
> >> >
> >> > 1) prove there's a bug. Either test the same code using the JSF RI
> >> > or another version of MyFaces. Or show that it contradicts the JSF
> >> > Specs. It's best to provide a simplified example demonstrating the
> >> > problem -- this would be easy in your particular issue. The easier
> >> > you make it for someone else to look at the problem, the more likely
> >> > someone will do so.
> >> >
> >> > 2) Provide a patch (like it looks like you're doing above) in JIRA.
> >> > If no one has done anything with it, and the bug is proven and the
> >> > patch is easily reviewed, feel free to ping the list after a week of
> >> > no action. Repeat weekly until someone either rejects or approves
> >> > it, or otherwise responds :-) Again, having a test case or example
> >> > makes it easier to test your patch to verify it should be applied.
> >> >
> >> >
> >> >
> >> >
> >> > On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
> >> >>
> >> >> Thank you for answer, Mike.
> >> >>
> >> >> I'm still not sure, weather the second part is really an
> >> unrelated spec
> >> >> issue because it could be just the consequence of the first one.
> >> >> So I've decide to describe both together thinking, it can help
> >> >> during code
> >> >> review since it belongs to very common part of core implementation
> >> >> (org.apache.myfaces.shared.renderkit.RendererUtils). If my
> >> >> suggestion is wrong, the second part can be just ignored. Please see
> >> >> also my
> >> >> comment to MYFACES-1452, which was the start point of my
> >> >> 'investigation' :)
> >> >>
> >> >> I've tried to solve the problem myself, but it was just a partially
> >> >> solution for me (respectively to second part of MYFACES-1532
> >> >> description).
> >> >> Since no one has answered in mailing list, I've thinking my
> >> >> suggestion is
> >> >> just wrong and haven't provide this as path. But if I'm right, the
> >> same
> >> >> should affect other standard types too (i.e. Date etc.)
> >> >>
> >> >> Anyway, here's my code. This method is intended to replace existing
> >> >> getBooleanValue() in
> >> >> org.apache.myfaces.shared.renderkit.RendererUtils, which is
> >> >> called from
> >> >>
> >> org.apache.myfaces.shared.renderkit.html.HtmlCheckboxRendererBase.encodeEnd().
> >>
> >> >>
> >> >> public static Boolean
> >> >> getBooleanValueForCheckbox(FacesContext facesContext,
> >> >> UIComponent component) {
> >> >> Object value = getObjectValue(component);
> >> >>
> >> >> // This call relays on the implementation of getObjectValue()!!!
> >> >> // If uiComponent is not an instance of ValueHolder, the
> >> >> // IllegalArgumentException should be already thrown.
> >> >> Converter converter = ((ValueHolder) component).getConverter();
> >> >>
> >> >> if (null == value || value instanceof Boolean) {
> >> >> return (Boolean) value;
> >> >> }
> >> >>
> >> >> if (null == converter && value instanceof String) {
> >> >> // it's still correct to convert String into Boolean, because
> >> >> // Boolean provides constructor Boolean(String x)!
> >> >> return new Boolean(value.toString());
> >> >> }
> >> >>
> >> >> // If component provides no custom converter, we
> >> >> // must try to obtain a standard one from Application
> >> >> if (null == converter) {
> >> >> try {
> >> >> converter = facesContext.getApplication().createConverter(
> >> >> value.getClass());
> >> >> } catch (FacesException ex) {
> >> >> throw new IllegalArgumentException("Component : "
> >> >> + getPathToComponent(component)
> >> >> + " expects value of type Boolean.\n"
> >> >> + "Neither standard converter for "
> >> >> + value.getClass().getName()
> >> >> + " nor custom converter provided.");
> >> >> }
> >> >> }
> >> >>
> >> >> if (null != converter) {
> >> >> try {
> >> >>
> >> >> /* Try to convert it into String. That is!
> >> >> * The getAsObject() provides the conversion of string value's
> >> >> * representation into custom user type. We are expecting a Boolean,
> >> >> * but the value itself is not a Boolean, so we need to get String
> >> >> first.
> >> >> */
> >> >> String strValue = converter.getAsString(facesContext,
> >> >> component, value);
> >> >>
> >> >> boolean warn = (null == strValue);
> >> >> warn = warn
> >> >> || !(strValue.equalsIgnoreCase(Boolean.TRUE.toString()) || strValue
> >> >> .equalsIgnoreCase(Boolean.TRUE.toString()));
> >> >>
> >> >> if (warn) {
> >> >> /* Note that this situation is not an error! We have some
> >> >> * converter and it got back some string, and no exception has been
> >> >> * thrown!
> >> >> */
> >> >> log.warn("Component "
> >> >> + getPathToComponent(component)
> >> >> + " expects value of type Boolean. The value was converted with "
> >> >> + converter.getClass().getName()
> >> >> + " into "
> >> >> + strValue
> >> >> + ", what is neither true nor false! false assumed.");
> >> >> }
> >> >>
> >> >> return new Boolean(strValue);
> >> >>
> >> >> } catch (ConverterException ex) {
> >> >> throw new IllegalArgumentException("Component : "
> >> >> + getPathToComponent(component)
> >> >> + " expects value of type Boolean.\n"
> >> >> + "Unable to convert " + value.getClass().getName()
> >> >> + " into Boolean.");
> >> >> }
> >> >>
> >> >> } else {
> >> >> throw new IllegalArgumentException("Component "
> >> >> + getPathToComponent(component)
> >> >> + " expects value of type Boolean but "
> >> >> + value.getClass().getName() + " was found.");
> >> >> }
> >> >>
> >> >> }
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> regards,
> >> >> paul
> >> >>
> >> >> Mike Kienenberger schrieb:
> >> >> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
> >> >>
> >> >> Some other question to Mike. You have just closed MYFACES-1545. I
> >> think,
> >> >> it's something very similar to MYFACES-1532, I've created
> >> 13.02.2007. I
> >> >> have asked about this in both of DEV and USERS lists, but nobody has
> >> >> answered, so I'm still not sure, whether it's really a bug, or
> >> just a
> >> >> feature. (There are some other related issues too, however they
> >> aren't
> >> >> linked to this one.) Would you please confirm this or close 1532
> >> too?
> >> >>
> >> >> Paul, please use a new thread when asking unrelated questions.
> >> >>
> >> >> I closed MYFACES-1545 for the reasons listed in the issue, after
> >> >> searching the mailing lists for relevent posts by the reporter.
> >> There
> >> >> were none. Custom converters on UISelectOne items are quite common,
> >> >> and I've yet to have an issue with them.
> >> >>
> >> >> It sounds like there could be a bug in h:selectBooleanCheckbox that
> >> >> causes the problem you described in MYFACES-1532. I've never
> >> used a
> >> >> converter on this component, and it could very well be as you
> >> >> described, so I'm going to leave that open, especially since you
> >> >> provided example code. I vaguely recall issues with the
> >> >> convertBoolean converter in the sandbox in the distant past, so it
> >> >> wouldn't be at all surprising.
> >> >>
> >> >> Again, though, you have an unrelated second issue described in the
> >> >> same issue. I'd recommend voiding that part and opening another
> >> >> issue (although if it's a spec issue, it'll just be closed out of
> >> hand
> >> >> as those are outside of the scope of MyFaces to handle).
> >> >>
> >> >>
> >> >>
> >> >
> >>
> >>
> >
> >
>
>
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: MYFACES-1545 / MYFACES-1532
Posted by Paul Iov <pa...@voller-ernst.de>.
Hi Martin,
I've checked this with the latest nightly build 1.1.6.
1. getValue() behavior seems to be correct now (at least without
conversion) :)
2. conversion (and validation?) issue is still present :(
my test case:
bean
private boolean boolBoolean = true;
private String strBoolean = "true";
private Date datDate = new Date();
private String strDate = "01.01.2007";
(with all correct get/set)
page
<h:selectBooleanCheckbox id="myBoolBoolean"
value="#{MyBean.boolBoolean}"/>
--OK
<h:selectBooleanCheckbox id="myStrBoolean" value="#{MyBean.strBoolean}"/>
--fails with Expected submitted value of type Boolean for Component:
...myStrBoolean
<h:selectBooleanCheckbox id="myStrBoolean" value="#{MyBean.strBoolean}"
converter="MyBooleanConverter"/>
--fails with Expected submitted value..., converter not called
<t:inputCalendar value="#{MyBean.datDate}"/>
--OK
<t:inputCalendar value="#{MyBean.strDate}"/>
--fails with Expected submitted value...
<t:inputCalendar value="#{MyBean.strDate}"
converter="MyDateConverter" />
--fails with Expected submitted value..., converter not called
"Expected submitted value..." is thrown in all cases during initial
rendering, so it's not possible to check behavior on submit etc.
Regards,
paul
Martin Marinschek schrieb:
> Hi Paul,
>
> I couldn't follow all your arguments in this rather long mail, but
> here a short thought for you to consider - it might help you
> determining if you've found a bug or not:
>
> the submitted value should always be used for rendering, if it is not
> 'null'. If it is null, the getValue() method of the component should
> be called (I've just found an issue with that, posted to the
> dev-list). getValue() should in fact return a local value, if it is
> set (a submitted value gets to be a local value after conversion and
> validation), if it is not set, the value-binding should be evaluated
> (and the backing-bean value used). Does any of the behaviour you see
> in this component contradict this?
>
> regards,
>
> Martin
>
> On 3/1/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> Well, I've reviewed the sources related to this issue again before
>> posting some comments/code to JIRA and can state now, that it's really
>> obscure. At least to me :( I think it's better to figure out my doubts
>> about this converter stuff again. Perhaps someone can just point out my
>> mistake or else confirm the global problem with converter handling in
>> current code.
>>
>> Few things, that are clear:
>> 1. Description of value attribute says:
>> "The initial value of this component. This value is generally set as a
>> value-binding in the form #{myBean.myProperty}, where myProperty can be
>> any data-type of Java (also user-defined data-types), if a converter for
>> this data-type exists. Special cases...[irrelevant]"
>> This is true for many components, but I'm referencing here and below the
>> h:selectBooleanCheckbox since it was the start point for me.
>>
>> 2. In most cases it not works but produce *Expected submitted value of
>> type* ... *for Component* ...if value attribute is bound to a property
>> of any type excepts expected one (boolean in case of
>> h:selectBooleanCheckbox). This happens during initial rendering of
>> component, so there is no submitted value. The only value existing at
>> this time is the value of bound bean property.
>>
>> 3. Neither provided custom converter nor standard one eventually
>> registered for expected type is called to try to convert this bean value
>> into expected type.
>>
>> After hours studying of source code I have grave doubts about
>> correctness of how the rendering of components is handled generally
>> because the problem lays IMHO in two very common utility classes:
>>
>> org.apache.myfaces.shared_impl.renderkit.RendererUtils
>> The static methods of this class are mostly used to accomplish the
>> 'standard' tasks such as retrieving of component's value, so if I'm
>> right, this problem affects wide range of html components and
>> automatically theirs descendants. At least the components dealing with
>> Boolean and Date as value should be affected. I'm not sure if this
>> utility method's hierarchy (since both of getBooleanValue() and
>> getDateValue() relays on getObjectValue() ) is the right place to
>> implement handling of custom converters, but I have a questions
>> regarding this hierarchy too since it seems to be a part of problem.
>> Why should the getObjectValue() call the .getSubmittedValue() and not
>> the .getValue() method of EditableValueHolder? This causes that all
>> affected components becomes null/false/empty as value during initial
>> rendering regardless of how the underlying bean's properties are
>> initialized since there is no SUBMITTED values at this time and the
>> method always returns null.
>>
>> org.apache.myfaces.shared_impl.taglib.UIComponentTagUtils
>> Probably THE right place to implement converters... but why should the
>> renderkit use own utility class above to accomplish all of GETs and the
>> SETs are grouped here? IMHO it should be the common task, since about
>> any component can provide custom converter and GETs as well SETs should
>> use it in some consistent way.
>>
>> I've tried first to implement usage of custom/standard converter only
>> for getBoolean() but then I have realized that it should be done
>> consistent for all standard types. That's why I'm not providing any path
>> right now (and the code in past posting is just an idea of how it can be
>> handled). The key questions that I'd like to ask first are:
>> Where it should be done? (RendererUtils or UIComponentTagUtils)
>> How exactly should it work?
>>
>> The common definition from spec(?) is beautiful, but how to implement it
>> really consistent? Let's say, X is the expected by component type and Y
>> is the type effectively provided by underlying value binding property.
>> If X and Y are not the same type, there are many different conversion
>> cases.
>>
>> a. component retrieves the value from value-binding ((Y)object or
>> null -> X)
>>
>> b. component sets submitted value to value binding (X -> Y)
>>
>> c. component retrieves submitted value from request (String -> X)
>> d. component renders retrieved value (X -> String)
>>
>> Which should be covered by custom converter and how, and which not?
>>
>> First of all, for c and d component have to know itself how to render
>> own type X = how to convert submitted String back to X. Let's name this
>> knowledge 'own conversion [String <-> X]'. Generally it is not affected
>> by any extra conversion, but I guess there are cases when it should
>> be done!
>>
>> If no custom converter provided, we can still try to obtain standard
>> one, capable to convert [String <-> X] and to use
>> - a: getAsObject( (String)value )
>> - b: getAsString(X)
>> It won't work, if underlying bean property is not String, but then we
>> can pass throw the exception and the message of this exception will be
>> absolutely clear to user.
>>
>> If custom converter provided, but it's unable to cast type returned by
>> getAsObject() to our X type, we have two possibility:
>> 1) we can rely on our own 'knowledge' about how to convert [String <->
>> X] and suppose that getAsString() of custom converter returns something
>> compatible. Then we can use:
>> - a: getAsString() + own conversion String -> X
>> - b: own conversion X->String + getAsObject()
>>
>> 2) we can try to obtain standard converter [String <-> X] and use it
>> instead of own 'knowledge' like:
>> - a: customConv.getAsString() + standardConv.getAsObject()
>> - b: standardConv.getAsString() + customConv.getAsObject()
>>
>> The case when custom converter provided and return type of getAsObject()
>> is compatible with our X seems to be clear, but in fact it's a most
>> complicated one. There are two reason to provide custom converter for
>> standard type. Either the value should be rendered by non-textual
>> component (i.e. Boolean + checkBox and user going to implement own
>> tristate logic, like true|false|unknown) or some data manipulation
>> should be done (i.e. Date parsing/formating). I'm not sure how this case
>> should be handled to preserve maximum flexibility.
>>
>> Just an idea: it should be different for both component kinds and
>> probably, we must ignore own rendering behavior as well.
>> For textual components:
>> - a and *c*: getAsObject()
>> - b and *d*: getAsString()
>>
>> For non-textual components (checkBox):
>> - a,b and *c*: getAsString() + getAsObject()
>> - *d*: getAsString()
>>
>> Well, the problem seems to be really complex. I think that providing any
>> 'light' path for some little part of this problem is not a good idea.
>> I'm also not familiar enough with JSF myself to state weather this
>> contradict spec or is just a minor issue, which should be worked around
>> some way until new better version i.e. 2.0 is released. I've just tried
>> to use this framework's feature and discovered a totally inconsistence.
>> I can provide test cases as well as check this against RI or figure it
>> out clearly in JIRA etc.but that's exactly why I'd like to ask first,
>> weather it makes any sense.
>>
>> best regards,
>> paul
>>
>>
>> Mike Kienenberger schrieb:
>> > Paul, in my own case, I wasn't reading myfaces email in depth the week
>> > you posted this. Sometimes things just fall through the cracks
>> > because people are busy with other things.
>> >
>> > Sometimes the issue is so obscure that few people really know how to
>> > respond to it.
>> >
>> > In both cases, the best thing to do is
>> >
>> > 1) prove there's a bug. Either test the same code using the JSF RI
>> > or another version of MyFaces. Or show that it contradicts the JSF
>> > Specs. It's best to provide a simplified example demonstrating the
>> > problem -- this would be easy in your particular issue. The easier
>> > you make it for someone else to look at the problem, the more likely
>> > someone will do so.
>> >
>> > 2) Provide a patch (like it looks like you're doing above) in JIRA.
>> > If no one has done anything with it, and the bug is proven and the
>> > patch is easily reviewed, feel free to ping the list after a week of
>> > no action. Repeat weekly until someone either rejects or approves
>> > it, or otherwise responds :-) Again, having a test case or example
>> > makes it easier to test your patch to verify it should be applied.
>> >
>> >
>> >
>> >
>> > On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> >>
>> >> Thank you for answer, Mike.
>> >>
>> >> I'm still not sure, weather the second part is really an
>> unrelated spec
>> >> issue because it could be just the consequence of the first one.
>> >> So I've decide to describe both together thinking, it can help
>> >> during code
>> >> review since it belongs to very common part of core implementation
>> >> (org.apache.myfaces.shared.renderkit.RendererUtils). If my
>> >> suggestion is wrong, the second part can be just ignored. Please see
>> >> also my
>> >> comment to MYFACES-1452, which was the start point of my
>> >> 'investigation' :)
>> >>
>> >> I've tried to solve the problem myself, but it was just a partially
>> >> solution for me (respectively to second part of MYFACES-1532
>> >> description).
>> >> Since no one has answered in mailing list, I've thinking my
>> >> suggestion is
>> >> just wrong and haven't provide this as path. But if I'm right, the
>> same
>> >> should affect other standard types too (i.e. Date etc.)
>> >>
>> >> Anyway, here's my code. This method is intended to replace existing
>> >> getBooleanValue() in
>> >> org.apache.myfaces.shared.renderkit.RendererUtils, which is
>> >> called from
>> >>
>> org.apache.myfaces.shared.renderkit.html.HtmlCheckboxRendererBase.encodeEnd().
>>
>> >>
>> >> public static Boolean
>> >> getBooleanValueForCheckbox(FacesContext facesContext,
>> >> UIComponent component) {
>> >> Object value = getObjectValue(component);
>> >>
>> >> // This call relays on the implementation of getObjectValue()!!!
>> >> // If uiComponent is not an instance of ValueHolder, the
>> >> // IllegalArgumentException should be already thrown.
>> >> Converter converter = ((ValueHolder) component).getConverter();
>> >>
>> >> if (null == value || value instanceof Boolean) {
>> >> return (Boolean) value;
>> >> }
>> >>
>> >> if (null == converter && value instanceof String) {
>> >> // it's still correct to convert String into Boolean, because
>> >> // Boolean provides constructor Boolean(String x)!
>> >> return new Boolean(value.toString());
>> >> }
>> >>
>> >> // If component provides no custom converter, we
>> >> // must try to obtain a standard one from Application
>> >> if (null == converter) {
>> >> try {
>> >> converter = facesContext.getApplication().createConverter(
>> >> value.getClass());
>> >> } catch (FacesException ex) {
>> >> throw new IllegalArgumentException("Component : "
>> >> + getPathToComponent(component)
>> >> + " expects value of type Boolean.\n"
>> >> + "Neither standard converter for "
>> >> + value.getClass().getName()
>> >> + " nor custom converter provided.");
>> >> }
>> >> }
>> >>
>> >> if (null != converter) {
>> >> try {
>> >>
>> >> /* Try to convert it into String. That is!
>> >> * The getAsObject() provides the conversion of string value's
>> >> * representation into custom user type. We are expecting a Boolean,
>> >> * but the value itself is not a Boolean, so we need to get String
>> >> first.
>> >> */
>> >> String strValue = converter.getAsString(facesContext,
>> >> component, value);
>> >>
>> >> boolean warn = (null == strValue);
>> >> warn = warn
>> >> || !(strValue.equalsIgnoreCase(Boolean.TRUE.toString()) || strValue
>> >> .equalsIgnoreCase(Boolean.TRUE.toString()));
>> >>
>> >> if (warn) {
>> >> /* Note that this situation is not an error! We have some
>> >> * converter and it got back some string, and no exception has been
>> >> * thrown!
>> >> */
>> >> log.warn("Component "
>> >> + getPathToComponent(component)
>> >> + " expects value of type Boolean. The value was converted with "
>> >> + converter.getClass().getName()
>> >> + " into "
>> >> + strValue
>> >> + ", what is neither true nor false! false assumed.");
>> >> }
>> >>
>> >> return new Boolean(strValue);
>> >>
>> >> } catch (ConverterException ex) {
>> >> throw new IllegalArgumentException("Component : "
>> >> + getPathToComponent(component)
>> >> + " expects value of type Boolean.\n"
>> >> + "Unable to convert " + value.getClass().getName()
>> >> + " into Boolean.");
>> >> }
>> >>
>> >> } else {
>> >> throw new IllegalArgumentException("Component "
>> >> + getPathToComponent(component)
>> >> + " expects value of type Boolean but "
>> >> + value.getClass().getName() + " was found.");
>> >> }
>> >>
>> >> }
>> >>
>> >>
>> >>
>> >>
>> >> regards,
>> >> paul
>> >>
>> >> Mike Kienenberger schrieb:
>> >> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>> >>
>> >> Some other question to Mike. You have just closed MYFACES-1545. I
>> think,
>> >> it's something very similar to MYFACES-1532, I've created
>> 13.02.2007. I
>> >> have asked about this in both of DEV and USERS lists, but nobody has
>> >> answered, so I'm still not sure, whether it's really a bug, or
>> just a
>> >> feature. (There are some other related issues too, however they
>> aren't
>> >> linked to this one.) Would you please confirm this or close 1532
>> too?
>> >>
>> >> Paul, please use a new thread when asking unrelated questions.
>> >>
>> >> I closed MYFACES-1545 for the reasons listed in the issue, after
>> >> searching the mailing lists for relevent posts by the reporter.
>> There
>> >> were none. Custom converters on UISelectOne items are quite common,
>> >> and I've yet to have an issue with them.
>> >>
>> >> It sounds like there could be a bug in h:selectBooleanCheckbox that
>> >> causes the problem you described in MYFACES-1532. I've never
>> used a
>> >> converter on this component, and it could very well be as you
>> >> described, so I'm going to leave that open, especially since you
>> >> provided example code. I vaguely recall issues with the
>> >> convertBoolean converter in the sandbox in the distant past, so it
>> >> wouldn't be at all surprising.
>> >>
>> >> Again, though, you have an unrelated second issue described in the
>> >> same issue. I'd recommend voiding that part and opening another
>> >> issue (although if it's a spec issue, it'll just be closed out of
>> hand
>> >> as those are outside of the scope of MyFaces to handle).
>> >>
>> >>
>> >>
>> >
>>
>>
>
>
Re: MYFACES-1545 / MYFACES-1532
Posted by Martin Marinschek <ma...@gmail.com>.
Hi Paul,
I couldn't follow all your arguments in this rather long mail, but
here a short thought for you to consider - it might help you
determining if you've found a bug or not:
the submitted value should always be used for rendering, if it is not
'null'. If it is null, the getValue() method of the component should
be called (I've just found an issue with that, posted to the
dev-list). getValue() should in fact return a local value, if it is
set (a submitted value gets to be a local value after conversion and
validation), if it is not set, the value-binding should be evaluated
(and the backing-bean value used). Does any of the behaviour you see
in this component contradict this?
regards,
Martin
On 3/1/07, Paul Iov <pa...@voller-ernst.de> wrote:
> Well, I've reviewed the sources related to this issue again before
> posting some comments/code to JIRA and can state now, that it's really
> obscure. At least to me :( I think it's better to figure out my doubts
> about this converter stuff again. Perhaps someone can just point out my
> mistake or else confirm the global problem with converter handling in
> current code.
>
> Few things, that are clear:
> 1. Description of value attribute says:
> "The initial value of this component. This value is generally set as a
> value-binding in the form #{myBean.myProperty}, where myProperty can be
> any data-type of Java (also user-defined data-types), if a converter for
> this data-type exists. Special cases...[irrelevant]"
> This is true for many components, but I'm referencing here and below the
> h:selectBooleanCheckbox since it was the start point for me.
>
> 2. In most cases it not works but produce *Expected submitted value of
> type* ... *for Component* ...if value attribute is bound to a property
> of any type excepts expected one (boolean in case of
> h:selectBooleanCheckbox). This happens during initial rendering of
> component, so there is no submitted value. The only value existing at
> this time is the value of bound bean property.
>
> 3. Neither provided custom converter nor standard one eventually
> registered for expected type is called to try to convert this bean value
> into expected type.
>
> After hours studying of source code I have grave doubts about
> correctness of how the rendering of components is handled generally
> because the problem lays IMHO in two very common utility classes:
>
> org.apache.myfaces.shared_impl.renderkit.RendererUtils
> The static methods of this class are mostly used to accomplish the
> 'standard' tasks such as retrieving of component's value, so if I'm
> right, this problem affects wide range of html components and
> automatically theirs descendants. At least the components dealing with
> Boolean and Date as value should be affected. I'm not sure if this
> utility method's hierarchy (since both of getBooleanValue() and
> getDateValue() relays on getObjectValue() ) is the right place to
> implement handling of custom converters, but I have a questions
> regarding this hierarchy too since it seems to be a part of problem.
> Why should the getObjectValue() call the .getSubmittedValue() and not
> the .getValue() method of EditableValueHolder? This causes that all
> affected components becomes null/false/empty as value during initial
> rendering regardless of how the underlying bean's properties are
> initialized since there is no SUBMITTED values at this time and the
> method always returns null.
>
> org.apache.myfaces.shared_impl.taglib.UIComponentTagUtils
> Probably THE right place to implement converters... but why should the
> renderkit use own utility class above to accomplish all of GETs and the
> SETs are grouped here? IMHO it should be the common task, since about
> any component can provide custom converter and GETs as well SETs should
> use it in some consistent way.
>
> I've tried first to implement usage of custom/standard converter only
> for getBoolean() but then I have realized that it should be done
> consistent for all standard types. That's why I'm not providing any path
> right now (and the code in past posting is just an idea of how it can be
> handled). The key questions that I'd like to ask first are:
> Where it should be done? (RendererUtils or UIComponentTagUtils)
> How exactly should it work?
>
> The common definition from spec(?) is beautiful, but how to implement it
> really consistent? Let's say, X is the expected by component type and Y
> is the type effectively provided by underlying value binding property.
> If X and Y are not the same type, there are many different conversion cases.
>
> a. component retrieves the value from value-binding ((Y)object or null -> X)
>
> b. component sets submitted value to value binding (X -> Y)
>
> c. component retrieves submitted value from request (String -> X)
> d. component renders retrieved value (X -> String)
>
> Which should be covered by custom converter and how, and which not?
>
> First of all, for c and d component have to know itself how to render
> own type X = how to convert submitted String back to X. Let's name this
> knowledge 'own conversion [String <-> X]'. Generally it is not affected
> by any extra conversion, but I guess there are cases when it should be done!
>
> If no custom converter provided, we can still try to obtain standard
> one, capable to convert [String <-> X] and to use
> - a: getAsObject( (String)value )
> - b: getAsString(X)
> It won't work, if underlying bean property is not String, but then we
> can pass throw the exception and the message of this exception will be
> absolutely clear to user.
>
> If custom converter provided, but it's unable to cast type returned by
> getAsObject() to our X type, we have two possibility:
> 1) we can rely on our own 'knowledge' about how to convert [String <->
> X] and suppose that getAsString() of custom converter returns something
> compatible. Then we can use:
> - a: getAsString() + own conversion String -> X
> - b: own conversion X->String + getAsObject()
>
> 2) we can try to obtain standard converter [String <-> X] and use it
> instead of own 'knowledge' like:
> - a: customConv.getAsString() + standardConv.getAsObject()
> - b: standardConv.getAsString() + customConv.getAsObject()
>
> The case when custom converter provided and return type of getAsObject()
> is compatible with our X seems to be clear, but in fact it's a most
> complicated one. There are two reason to provide custom converter for
> standard type. Either the value should be rendered by non-textual
> component (i.e. Boolean + checkBox and user going to implement own
> tristate logic, like true|false|unknown) or some data manipulation
> should be done (i.e. Date parsing/formating). I'm not sure how this case
> should be handled to preserve maximum flexibility.
>
> Just an idea: it should be different for both component kinds and
> probably, we must ignore own rendering behavior as well.
> For textual components:
> - a and *c*: getAsObject()
> - b and *d*: getAsString()
>
> For non-textual components (checkBox):
> - a,b and *c*: getAsString() + getAsObject()
> - *d*: getAsString()
>
> Well, the problem seems to be really complex. I think that providing any
> 'light' path for some little part of this problem is not a good idea.
> I'm also not familiar enough with JSF myself to state weather this
> contradict spec or is just a minor issue, which should be worked around
> some way until new better version i.e. 2.0 is released. I've just tried
> to use this framework's feature and discovered a totally inconsistence.
> I can provide test cases as well as check this against RI or figure it
> out clearly in JIRA etc.but that's exactly why I'd like to ask first,
> weather it makes any sense.
>
> best regards,
> paul
>
>
> Mike Kienenberger schrieb:
> > Paul, in my own case, I wasn't reading myfaces email in depth the week
> > you posted this. Sometimes things just fall through the cracks
> > because people are busy with other things.
> >
> > Sometimes the issue is so obscure that few people really know how to
> > respond to it.
> >
> > In both cases, the best thing to do is
> >
> > 1) prove there's a bug. Either test the same code using the JSF RI
> > or another version of MyFaces. Or show that it contradicts the JSF
> > Specs. It's best to provide a simplified example demonstrating the
> > problem -- this would be easy in your particular issue. The easier
> > you make it for someone else to look at the problem, the more likely
> > someone will do so.
> >
> > 2) Provide a patch (like it looks like you're doing above) in JIRA.
> > If no one has done anything with it, and the bug is proven and the
> > patch is easily reviewed, feel free to ping the list after a week of
> > no action. Repeat weekly until someone either rejects or approves
> > it, or otherwise responds :-) Again, having a test case or example
> > makes it easier to test your patch to verify it should be applied.
> >
> >
> >
> >
> > On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
> >>
> >> Thank you for answer, Mike.
> >>
> >> I'm still not sure, weather the second part is really an unrelated spec
> >> issue because it could be just the consequence of the first one.
> >> So I've decide to describe both together thinking, it can help
> >> during code
> >> review since it belongs to very common part of core implementation
> >> (org.apache.myfaces.shared.renderkit.RendererUtils). If my
> >> suggestion is wrong, the second part can be just ignored. Please see
> >> also my
> >> comment to MYFACES-1452, which was the start point of my
> >> 'investigation' :)
> >>
> >> I've tried to solve the problem myself, but it was just a partially
> >> solution for me (respectively to second part of MYFACES-1532
> >> description).
> >> Since no one has answered in mailing list, I've thinking my
> >> suggestion is
> >> just wrong and haven't provide this as path. But if I'm right, the same
> >> should affect other standard types too (i.e. Date etc.)
> >>
> >> Anyway, here's my code. This method is intended to replace existing
> >> getBooleanValue() in
> >> org.apache.myfaces.shared.renderkit.RendererUtils, which is
> >> called from
> >> org.apache.myfaces.shared.renderkit.html.HtmlCheckboxRendererBase.encodeEnd().
> >>
> >> public static Boolean
> >> getBooleanValueForCheckbox(FacesContext facesContext,
> >> UIComponent component) {
> >> Object value = getObjectValue(component);
> >>
> >> // This call relays on the implementation of getObjectValue()!!!
> >> // If uiComponent is not an instance of ValueHolder, the
> >> // IllegalArgumentException should be already thrown.
> >> Converter converter = ((ValueHolder) component).getConverter();
> >>
> >> if (null == value || value instanceof Boolean) {
> >> return (Boolean) value;
> >> }
> >>
> >> if (null == converter && value instanceof String) {
> >> // it's still correct to convert String into Boolean, because
> >> // Boolean provides constructor Boolean(String x)!
> >> return new Boolean(value.toString());
> >> }
> >>
> >> // If component provides no custom converter, we
> >> // must try to obtain a standard one from Application
> >> if (null == converter) {
> >> try {
> >> converter = facesContext.getApplication().createConverter(
> >> value.getClass());
> >> } catch (FacesException ex) {
> >> throw new IllegalArgumentException("Component : "
> >> + getPathToComponent(component)
> >> + " expects value of type Boolean.\n"
> >> + "Neither standard converter for "
> >> + value.getClass().getName()
> >> + " nor custom converter provided.");
> >> }
> >> }
> >>
> >> if (null != converter) {
> >> try {
> >>
> >> /* Try to convert it into String. That is!
> >> * The getAsObject() provides the conversion of string value's
> >> * representation into custom user type. We are expecting a Boolean,
> >> * but the value itself is not a Boolean, so we need to get String
> >> first.
> >> */
> >> String strValue = converter.getAsString(facesContext,
> >> component, value);
> >>
> >> boolean warn = (null == strValue);
> >> warn = warn
> >> || !(strValue.equalsIgnoreCase(Boolean.TRUE.toString()) || strValue
> >> .equalsIgnoreCase(Boolean.TRUE.toString()));
> >>
> >> if (warn) {
> >> /* Note that this situation is not an error! We have some
> >> * converter and it got back some string, and no exception has been
> >> * thrown!
> >> */
> >> log.warn("Component "
> >> + getPathToComponent(component)
> >> + " expects value of type Boolean. The value was converted with "
> >> + converter.getClass().getName()
> >> + " into "
> >> + strValue
> >> + ", what is neither true nor false! false assumed.");
> >> }
> >>
> >> return new Boolean(strValue);
> >>
> >> } catch (ConverterException ex) {
> >> throw new IllegalArgumentException("Component : "
> >> + getPathToComponent(component)
> >> + " expects value of type Boolean.\n"
> >> + "Unable to convert " + value.getClass().getName()
> >> + " into Boolean.");
> >> }
> >>
> >> } else {
> >> throw new IllegalArgumentException("Component "
> >> + getPathToComponent(component)
> >> + " expects value of type Boolean but "
> >> + value.getClass().getName() + " was found.");
> >> }
> >>
> >> }
> >>
> >>
> >>
> >>
> >> regards,
> >> paul
> >>
> >> Mike Kienenberger schrieb:
> >> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
> >>
> >> Some other question to Mike. You have just closed MYFACES-1545. I think,
> >> it's something very similar to MYFACES-1532, I've created 13.02.2007. I
> >> have asked about this in both of DEV and USERS lists, but nobody has
> >> answered, so I'm still not sure, whether it's really a bug, or just a
> >> feature. (There are some other related issues too, however they aren't
> >> linked to this one.) Would you please confirm this or close 1532 too?
> >>
> >> Paul, please use a new thread when asking unrelated questions.
> >>
> >> I closed MYFACES-1545 for the reasons listed in the issue, after
> >> searching the mailing lists for relevent posts by the reporter. There
> >> were none. Custom converters on UISelectOne items are quite common,
> >> and I've yet to have an issue with them.
> >>
> >> It sounds like there could be a bug in h:selectBooleanCheckbox that
> >> causes the problem you described in MYFACES-1532. I've never used a
> >> converter on this component, and it could very well be as you
> >> described, so I'm going to leave that open, especially since you
> >> provided example code. I vaguely recall issues with the
> >> convertBoolean converter in the sandbox in the distant past, so it
> >> wouldn't be at all surprising.
> >>
> >> Again, though, you have an unrelated second issue described in the
> >> same issue. I'd recommend voiding that part and opening another
> >> issue (although if it's a spec issue, it'll just be closed out of hand
> >> as those are outside of the scope of MyFaces to handle).
> >>
> >>
> >>
> >
>
>
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: MYFACES-1545 / MYFACES-1532
Posted by Paul Iov <pa...@voller-ernst.de>.
Well, I've reviewed the sources related to this issue again before
posting some comments/code to JIRA and can state now, that it's really
obscure. At least to me :( I think it's better to figure out my doubts
about this converter stuff again. Perhaps someone can just point out my
mistake or else confirm the global problem with converter handling in
current code.
Few things, that are clear:
1. Description of value attribute says:
"The initial value of this component. This value is generally set as a
value-binding in the form #{myBean.myProperty}, where myProperty can be
any data-type of Java (also user-defined data-types), if a converter for
this data-type exists. Special cases...[irrelevant]"
This is true for many components, but I'm referencing here and below the
h:selectBooleanCheckbox since it was the start point for me.
2. In most cases it not works but produce *Expected submitted value of
type* ... *for Component* ...if value attribute is bound to a property
of any type excepts expected one (boolean in case of
h:selectBooleanCheckbox). This happens during initial rendering of
component, so there is no submitted value. The only value existing at
this time is the value of bound bean property.
3. Neither provided custom converter nor standard one eventually
registered for expected type is called to try to convert this bean value
into expected type.
After hours studying of source code I have grave doubts about
correctness of how the rendering of components is handled generally
because the problem lays IMHO in two very common utility classes:
org.apache.myfaces.shared_impl.renderkit.RendererUtils
The static methods of this class are mostly used to accomplish the
'standard' tasks such as retrieving of component's value, so if I'm
right, this problem affects wide range of html components and
automatically theirs descendants. At least the components dealing with
Boolean and Date as value should be affected. I'm not sure if this
utility method's hierarchy (since both of getBooleanValue() and
getDateValue() relays on getObjectValue() ) is the right place to
implement handling of custom converters, but I have a questions
regarding this hierarchy too since it seems to be a part of problem.
Why should the getObjectValue() call the .getSubmittedValue() and not
the .getValue() method of EditableValueHolder? This causes that all
affected components becomes null/false/empty as value during initial
rendering regardless of how the underlying bean's properties are
initialized since there is no SUBMITTED values at this time and the
method always returns null.
org.apache.myfaces.shared_impl.taglib.UIComponentTagUtils
Probably THE right place to implement converters... but why should the
renderkit use own utility class above to accomplish all of GETs and the
SETs are grouped here? IMHO it should be the common task, since about
any component can provide custom converter and GETs as well SETs should
use it in some consistent way.
I've tried first to implement usage of custom/standard converter only
for getBoolean() but then I have realized that it should be done
consistent for all standard types. That's why I'm not providing any path
right now (and the code in past posting is just an idea of how it can be
handled). The key questions that I'd like to ask first are:
Where it should be done? (RendererUtils or UIComponentTagUtils)
How exactly should it work?
The common definition from spec(?) is beautiful, but how to implement it
really consistent? Let's say, X is the expected by component type and Y
is the type effectively provided by underlying value binding property.
If X and Y are not the same type, there are many different conversion cases.
a. component retrieves the value from value-binding ((Y)object or null -> X)
b. component sets submitted value to value binding (X -> Y)
c. component retrieves submitted value from request (String -> X)
d. component renders retrieved value (X -> String)
Which should be covered by custom converter and how, and which not?
First of all, for c and d component have to know itself how to render
own type X = how to convert submitted String back to X. Let's name this
knowledge 'own conversion [String <-> X]'. Generally it is not affected
by any extra conversion, but I guess there are cases when it should be done!
If no custom converter provided, we can still try to obtain standard
one, capable to convert [String <-> X] and to use
- a: getAsObject( (String)value )
- b: getAsString(X)
It won't work, if underlying bean property is not String, but then we
can pass throw the exception and the message of this exception will be
absolutely clear to user.
If custom converter provided, but it's unable to cast type returned by
getAsObject() to our X type, we have two possibility:
1) we can rely on our own 'knowledge' about how to convert [String <->
X] and suppose that getAsString() of custom converter returns something
compatible. Then we can use:
- a: getAsString() + own conversion String -> X
- b: own conversion X->String + getAsObject()
2) we can try to obtain standard converter [String <-> X] and use it
instead of own 'knowledge' like:
- a: customConv.getAsString() + standardConv.getAsObject()
- b: standardConv.getAsString() + customConv.getAsObject()
The case when custom converter provided and return type of getAsObject()
is compatible with our X seems to be clear, but in fact it's a most
complicated one. There are two reason to provide custom converter for
standard type. Either the value should be rendered by non-textual
component (i.e. Boolean + checkBox and user going to implement own
tristate logic, like true|false|unknown) or some data manipulation
should be done (i.e. Date parsing/formating). I'm not sure how this case
should be handled to preserve maximum flexibility.
Just an idea: it should be different for both component kinds and
probably, we must ignore own rendering behavior as well.
For textual components:
- a and *c*: getAsObject()
- b and *d*: getAsString()
For non-textual components (checkBox):
- a,b and *c*: getAsString() + getAsObject()
- *d*: getAsString()
Well, the problem seems to be really complex. I think that providing any
'light' path for some little part of this problem is not a good idea.
I'm also not familiar enough with JSF myself to state weather this
contradict spec or is just a minor issue, which should be worked around
some way until new better version i.e. 2.0 is released. I've just tried
to use this framework's feature and discovered a totally inconsistence.
I can provide test cases as well as check this against RI or figure it
out clearly in JIRA etc.but that's exactly why I'd like to ask first,
weather it makes any sense.
best regards,
paul
Mike Kienenberger schrieb:
> Paul, in my own case, I wasn't reading myfaces email in depth the week
> you posted this. Sometimes things just fall through the cracks
> because people are busy with other things.
>
> Sometimes the issue is so obscure that few people really know how to
> respond to it.
>
> In both cases, the best thing to do is
>
> 1) prove there's a bug. Either test the same code using the JSF RI
> or another version of MyFaces. Or show that it contradicts the JSF
> Specs. It's best to provide a simplified example demonstrating the
> problem -- this would be easy in your particular issue. The easier
> you make it for someone else to look at the problem, the more likely
> someone will do so.
>
> 2) Provide a patch (like it looks like you're doing above) in JIRA.
> If no one has done anything with it, and the bug is proven and the
> patch is easily reviewed, feel free to ping the list after a week of
> no action. Repeat weekly until someone either rejects or approves
> it, or otherwise responds :-) Again, having a test case or example
> makes it easier to test your patch to verify it should be applied.
>
>
>
>
> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>>
>> Thank you for answer, Mike.
>>
>> I'm still not sure, weather the second part is really an unrelated spec
>> issue because it could be just the consequence of the first one.
>> So I've decide to describe both together thinking, it can help
>> during code
>> review since it belongs to very common part of core implementation
>> (org.apache.myfaces.shared.renderkit.RendererUtils). If my
>> suggestion is wrong, the second part can be just ignored. Please see
>> also my
>> comment to MYFACES-1452, which was the start point of my
>> 'investigation' :)
>>
>> I've tried to solve the problem myself, but it was just a partially
>> solution for me (respectively to second part of MYFACES-1532
>> description).
>> Since no one has answered in mailing list, I've thinking my
>> suggestion is
>> just wrong and haven't provide this as path. But if I'm right, the same
>> should affect other standard types too (i.e. Date etc.)
>>
>> Anyway, here's my code. This method is intended to replace existing
>> getBooleanValue() in
>> org.apache.myfaces.shared.renderkit.RendererUtils, which is
>> called from
>> org.apache.myfaces.shared.renderkit.html.HtmlCheckboxRendererBase.encodeEnd().
>>
>> public static Boolean
>> getBooleanValueForCheckbox(FacesContext facesContext,
>> UIComponent component) {
>> Object value = getObjectValue(component);
>>
>> // This call relays on the implementation of getObjectValue()!!!
>> // If uiComponent is not an instance of ValueHolder, the
>> // IllegalArgumentException should be already thrown.
>> Converter converter = ((ValueHolder) component).getConverter();
>>
>> if (null == value || value instanceof Boolean) {
>> return (Boolean) value;
>> }
>>
>> if (null == converter && value instanceof String) {
>> // it's still correct to convert String into Boolean, because
>> // Boolean provides constructor Boolean(String x)!
>> return new Boolean(value.toString());
>> }
>>
>> // If component provides no custom converter, we
>> // must try to obtain a standard one from Application
>> if (null == converter) {
>> try {
>> converter = facesContext.getApplication().createConverter(
>> value.getClass());
>> } catch (FacesException ex) {
>> throw new IllegalArgumentException("Component : "
>> + getPathToComponent(component)
>> + " expects value of type Boolean.\n"
>> + "Neither standard converter for "
>> + value.getClass().getName()
>> + " nor custom converter provided.");
>> }
>> }
>>
>> if (null != converter) {
>> try {
>>
>> /* Try to convert it into String. That is!
>> * The getAsObject() provides the conversion of string value's
>> * representation into custom user type. We are expecting a Boolean,
>> * but the value itself is not a Boolean, so we need to get String
>> first.
>> */
>> String strValue = converter.getAsString(facesContext,
>> component, value);
>>
>> boolean warn = (null == strValue);
>> warn = warn
>> || !(strValue.equalsIgnoreCase(Boolean.TRUE.toString()) || strValue
>> .equalsIgnoreCase(Boolean.TRUE.toString()));
>>
>> if (warn) {
>> /* Note that this situation is not an error! We have some
>> * converter and it got back some string, and no exception has been
>> * thrown!
>> */
>> log.warn("Component "
>> + getPathToComponent(component)
>> + " expects value of type Boolean. The value was converted with "
>> + converter.getClass().getName()
>> + " into "
>> + strValue
>> + ", what is neither true nor false! false assumed.");
>> }
>>
>> return new Boolean(strValue);
>>
>> } catch (ConverterException ex) {
>> throw new IllegalArgumentException("Component : "
>> + getPathToComponent(component)
>> + " expects value of type Boolean.\n"
>> + "Unable to convert " + value.getClass().getName()
>> + " into Boolean.");
>> }
>>
>> } else {
>> throw new IllegalArgumentException("Component "
>> + getPathToComponent(component)
>> + " expects value of type Boolean but "
>> + value.getClass().getName() + " was found.");
>> }
>>
>> }
>>
>>
>>
>>
>> regards,
>> paul
>>
>> Mike Kienenberger schrieb:
>> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>>
>> Some other question to Mike. You have just closed MYFACES-1545. I think,
>> it's something very similar to MYFACES-1532, I've created 13.02.2007. I
>> have asked about this in both of DEV and USERS lists, but nobody has
>> answered, so I'm still not sure, whether it's really a bug, or just a
>> feature. (There are some other related issues too, however they aren't
>> linked to this one.) Would you please confirm this or close 1532 too?
>>
>> Paul, please use a new thread when asking unrelated questions.
>>
>> I closed MYFACES-1545 for the reasons listed in the issue, after
>> searching the mailing lists for relevent posts by the reporter. There
>> were none. Custom converters on UISelectOne items are quite common,
>> and I've yet to have an issue with them.
>>
>> It sounds like there could be a bug in h:selectBooleanCheckbox that
>> causes the problem you described in MYFACES-1532. I've never used a
>> converter on this component, and it could very well be as you
>> described, so I'm going to leave that open, especially since you
>> provided example code. I vaguely recall issues with the
>> convertBoolean converter in the sandbox in the distant past, so it
>> wouldn't be at all surprising.
>>
>> Again, though, you have an unrelated second issue described in the
>> same issue. I'd recommend voiding that part and opening another
>> issue (although if it's a spec issue, it'll just be closed out of hand
>> as those are outside of the scope of MyFaces to handle).
>>
>>
>>
>
Re: MYFACES-1545 / MYFACES-1532
Posted by Mike Kienenberger <mk...@gmail.com>.
Paul, in my own case, I wasn't reading myfaces email in depth the week
you posted this. Sometimes things just fall through the cracks
because people are busy with other things.
Sometimes the issue is so obscure that few people really know how to
respond to it.
In both cases, the best thing to do is
1) prove there's a bug. Either test the same code using the JSF RI
or another version of MyFaces. Or show that it contradicts the JSF
Specs. It's best to provide a simplified example demonstrating the
problem -- this would be easy in your particular issue. The easier
you make it for someone else to look at the problem, the more likely
someone will do so.
2) Provide a patch (like it looks like you're doing above) in JIRA.
If no one has done anything with it, and the bug is proven and the
patch is easily reviewed, feel free to ping the list after a week of
no action. Repeat weekly until someone either rejects or approves
it, or otherwise responds :-) Again, having a test case or example
makes it easier to test your patch to verify it should be applied.
On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>
> Thank you for answer, Mike.
>
> I'm still not sure, weather the second part is really an unrelated spec
> issue because it could be just the consequence of the first one.
> So I've decide to describe both together thinking, it can help during code
> review since it belongs to very common part of core implementation
> (org.apache.myfaces.shared.renderkit.RendererUtils). If my
> suggestion is wrong, the second part can be just ignored. Please see also my
> comment to MYFACES-1452, which was the start point of my 'investigation' :)
>
> I've tried to solve the problem myself, but it was just a partially
> solution for me (respectively to second part of MYFACES-1532 description).
> Since no one has answered in mailing list, I've thinking my suggestion is
> just wrong and haven't provide this as path. But if I'm right, the same
> should affect other standard types too (i.e. Date etc.)
>
> Anyway, here's my code. This method is intended to replace existing
> getBooleanValue() in
> org.apache.myfaces.shared.renderkit.RendererUtils, which is
> called from
> org.apache.myfaces.shared.renderkit.html.HtmlCheckboxRendererBase.encodeEnd().
> public static Boolean
> getBooleanValueForCheckbox(FacesContext facesContext,
> UIComponent component) {
> Object value = getObjectValue(component);
>
> // This call relays on the implementation of getObjectValue()!!!
> // If uiComponent is not an instance of ValueHolder, the
> // IllegalArgumentException should be already thrown.
> Converter converter = ((ValueHolder) component).getConverter();
>
> if (null == value || value instanceof Boolean) {
> return (Boolean) value;
> }
>
> if (null == converter && value instanceof String) {
> // it's still correct to convert String into Boolean, because
> // Boolean provides constructor Boolean(String x)!
> return new Boolean(value.toString());
> }
>
> // If component provides no custom converter, we
> // must try to obtain a standard one from Application
> if (null == converter) {
> try {
> converter = facesContext.getApplication().createConverter(
> value.getClass());
> } catch (FacesException ex) {
> throw new IllegalArgumentException("Component : "
> + getPathToComponent(component)
> + " expects value of type Boolean.\n"
> + "Neither standard converter for "
> + value.getClass().getName()
> + " nor custom converter provided.");
> }
> }
>
> if (null != converter) {
> try {
>
> /* Try to convert it into String. That is!
> * The getAsObject() provides the conversion of string value's
> * representation into custom user type. We are expecting a Boolean,
> * but the value itself is not a Boolean, so we need to get String first.
> */
> String strValue = converter.getAsString(facesContext,
> component, value);
>
> boolean warn = (null == strValue);
> warn = warn
> || !(strValue.equalsIgnoreCase(Boolean.TRUE.toString()) || strValue
> .equalsIgnoreCase(Boolean.TRUE.toString()));
>
> if (warn) {
> /* Note that this situation is not an error! We have some
> * converter and it got back some string, and no exception has been
> * thrown!
> */
> log.warn("Component "
> + getPathToComponent(component)
> + " expects value of type Boolean. The value was converted with "
> + converter.getClass().getName()
> + " into "
> + strValue
> + ", what is neither true nor false! false assumed.");
> }
>
> return new Boolean(strValue);
>
> } catch (ConverterException ex) {
> throw new IllegalArgumentException("Component : "
> + getPathToComponent(component)
> + " expects value of type Boolean.\n"
> + "Unable to convert " + value.getClass().getName()
> + " into Boolean.");
> }
>
> } else {
> throw new IllegalArgumentException("Component "
> + getPathToComponent(component)
> + " expects value of type Boolean but "
> + value.getClass().getName() + " was found.");
> }
>
> }
>
>
>
>
> regards,
> paul
>
> Mike Kienenberger schrieb:
> On 2/27/07, Paul Iov <pa...@voller-ernst.de> wrote:
>
> Some other question to Mike. You have just closed MYFACES-1545. I think,
> it's something very similar to MYFACES-1532, I've created 13.02.2007. I
> have asked about this in both of DEV and USERS lists, but nobody has
> answered, so I'm still not sure, whether it's really a bug, or just a
> feature. (There are some other related issues too, however they aren't
> linked to this one.) Would you please confirm this or close 1532 too?
>
> Paul, please use a new thread when asking unrelated questions.
>
> I closed MYFACES-1545 for the reasons listed in the issue, after
> searching the mailing lists for relevent posts by the reporter. There
> were none. Custom converters on UISelectOne items are quite common,
> and I've yet to have an issue with them.
>
> It sounds like there could be a bug in h:selectBooleanCheckbox that
> causes the problem you described in MYFACES-1532. I've never used a
> converter on this component, and it could very well be as you
> described, so I'm going to leave that open, especially since you
> provided example code. I vaguely recall issues with the
> convertBoolean converter in the sandbox in the distant past, so it
> wouldn't be at all surprising.
>
> Again, though, you have an unrelated second issue described in the
> same issue. I'd recommend voiding that part and opening another
> issue (although if it's a spec issue, it'll just be closed out of hand
> as those are outside of the scope of MyFaces to handle).
>
>
>