Inconsistency with primitive wrappers and prop-editors

[sebersole]

Currently the JBoss property editors for the primitive wrapper types never allow a null representation. This seems very inconsistent to me, as developers tend to choose java.lang.Boolean over boolean because they want/allow null values.

The needed change is pretty simple. Assuming that we decide "", "null", and "(String)null" all represent valid null values, just change (BooleanEditor as example):

public void setAsText(final String text)
{
 Object newValue = Boolean.valueOf(text);
 setValue(newValue);
}

to :

protected static final String NULL = "null";
public void setAsText(final String text) {
 if ( isNull( text ) ) {
 setValue( null );
 }
 else {
 setValue( Boolean.valueOf( text ) );
 }
}
protected boolean isNull(final String text) {
 if ( text == null ) {
 return true;
 }
 else {
 final String trimmed = text.trim().toLowerCase();
 return trimmed.size() == 0 || NULL.equals( trimmed );
 }
}

However, I am not sure whether there is existing code which exposes java.lang.Boolean attributes and relies on this behavior of never getting null values. Not sure why they expose the wrapper type anyway though if that is the case, but that's a different topic :)

As of right now, the implication of choosing to expose MBean's with a type-safe interface with any of these primitive wrappers means that the MBean will currently never get back null values.

[sebersole]

of course trimmed.size() should be timmed.length() ;)

[starksm64]

If we are going to go in this direction we should have a consistent ability to represent null for all types. Arguably, one has to deal with nulls unless primatives are used.

My concern is that there is a layer where a property editor is being used to convert a string to an invocation payload value where the signature actually is a primative type. The wrapper is only being used because its required by the reflection/invocation layer.

The general way we treat such evolutionary changes is to allow for the new behavior to be disable via system properties, with the default behavior being the new, more useful one.

[sebersole]

So then should I revert back to exposing everything as String in the
HibernateMBean?

If you point me in the right direction, I'll take a look at this as it irks me that this is such unintuative behavior....

[adrian.brock]

See: http://jira.jboss.com/jira/browse/JBAS-2084
Can you give it a try to make sure it solves your problem.

I only did it for the primitive types with a system property to disable it, although
I can't see why it would break existing code?

I'm loath to add it to the other editors until the PropertyEditor usage is better defined.

Another example is the microcontainer where there is a difference between null and the empty
string.

<property name="aString"/> <!-- the empty string -->
<property name="aString"></property> <!-- the empty string -->
<property name="aString"><value/></property> <!-- the empty string -->
<property name="aString"><value></value></property> <!-- the empty string -->

<property name="aString"><null/></property> <!-- a null string -->

[adrian.brock]

And of course a string that contains the characters 'n' 'u' 'l' 'l'

<property name="aString">null</property> <!-- new String("null"); -->

[sebersole]

Ah dude, I was willing to do the work ;)

I was just refering to pointing me in the direction of the "layer" that might be problematic so that I could verify that the changes would not screw it.

Anyway, thanks. I'll test it out.

[adrian.brock]

Ah dude, I was willing to do the work ;)

I was just refering to pointing me in the direction of the "layer" that might be problematic so that I could verify that the changes would not screw it.

Anyway, thanks. I'll test it out.

Sometimes (when the approach is agreed) it is easier to JFDI than explain it :-)

[sebersole]

I modified the inspectMBean.jsp used in the jmx-console to also handle null boolean values for the java.lang.Boolean wrapper. The changes you made seem to be working :)

So two things:
1) There was some discussion on the dev list as to what the label should be for this particular (null) radio box. I went with 'Null' as that was the only option expressed in that discussion.
2) Currently I just duplicated the lookup of the 'org.jboss.util.property.disablenull' property in the JSP since it is not exposed from PropertyEditors. Is that OK? Or should I expose it from PropertyEditors?

[starksm64]

1) fine.
2) I would expose it from the PropertyEditors class.