MBeanClientInterceptor assumes lower-case Attribute names

[Joris Kuipers]

Hi,

I just tried the new JMX access code in 1.2RC1 against an MX4J JMX server using MBeanClientInterceptor and encountered the following problem:
MX4J exposes all MBean attributes for my application MBeans (I used CruiseControl as a test case) as starting with a capital letter; the method getProjects() results in an attribute called 'Projects', while the MBeanClientInterceptor uses BeanUtils.findPropertyForMethod and comes up with a lower-case attribute name so getProjects() results in 'projects'. Obviously this causes a conflict, since the attribute is not present in allowedAttributes with its lower-case name.

Is this something that should be configurable in the MBeanClientInterceptor? Does the JMX spec say anything about default casing of attribute names?

Joris

[Joris Kuipers]

I had a look at the spec, and this seems to be a Spring bug. This is a quote from page 40 of the spec:

Case Sensitivity

All attribute and operation names derived from these design patterns are casesensitive.
For example, this means that the methods getstate and setState
define two attributes, one called state that is read-only, and one called State that is write-only.

So this means that the MBeanClientInterceptor.invoke() method shouldn't just use BeanUtils.findPropertyForMethod().getName() to determine the property-name, since that will always return the lower-case property name.

[robh]

I think this is more of a preference thing. I like lowercase because it is JavaBeans style, but I'll add a setting to the MBeanClientInterceptor that allows for the casing behaviour to be changed.

Rob

[Joris Kuipers]

That's great; just make sure that one of the preferences that can be enabled (should be the default, IMHO) will result in getProjects() and setProjects() becoming 'Projects' and getprojects() and setprojects() becoming 'projects', like the spec describes. It would be ashame if one could only set a preference that will make all attributes either upper- or lower-case, since apparently that's not what the JMX people had in mind when they wrote the specification.

[robh]

The only thing to consider here is that getstate() is not a valid JavaBean property and Spring works on JavaBean style properties, so it won't pick up the method getstate() as an attribute.

I guess we could extend the detection code if there is real need, but I need to think about the impacts of that a little more.

Rob

[Andreas Senft]

Maybe the property detection could be factored out as a strategy, defaulting to the current (JavaBean) behaviour.
So different property styles could be supported as well without breaking existing code.

Just my 2 cents,
Andreas

[Joris Kuipers]

The only thing to consider here is that getstate() is not a valid JavaBean property and Spring works on JavaBean style properties, so it won't pick up the method getstate() as an attribute.

I know that Spring uses JavaBean style properties for configuring and querying beans, but that's simply not how JMX works. I fail to see why we would choose to pretend that it does, instead of just patching the MBeanClientInterceptor.invoke() method. What the code should do is figure out if a method corresponds to a JMX attribute, not if it corresponds to a JavaBean property. As I see it, this doesn't conflict with Spring practices in any way, since JXM Attributes aren't really based on JavaBean properties. When you say that "Spring works on JavaBean style properties", that doesn't mean that it can't support other method to attribute mappings where that's appropiate, right?
I also like JavaBeans properties better than the way JMX chooses to determine its attribute names, but that's really irrelevant in this case.

[Joris Kuipers]

Maybe the property detection could be factored out as a strategy, defaulting to the current (JavaBean) behaviour.
So different property styles could be supported as well without breaking existing code.

As I state in my other mail, I think this would come down to "let's make a default behaviour that ignores the specification and pretends that JMX Attributes are equal to JavaBean properties while we know they're not". If I understand the JMX specification correctly (and please correct me if I don't), there's no need for a pluggable behaviour, and the current behaviour of the MBeanClientInterceptor is just wrong.

With respect to breaking existing code: the MBeanClientInterceptor hasn't been a part of any official release yet, right? And on top of that, the current implementation doesn't work... So I don't think that changing the current behaviour will cause any issues.

[robh]

I agree with you, we just need to be aware that getstate() is not detected as a property by the Java introspector so it won't map to an attribute called state it will map to an operation called getstate.

I have checked in this support and I changed the default to match the spec. It should sync to public CVS in the next few hours.

Rob

[Andreas Senft]

With respect to breaking existing code: the MBeanClientInterceptor hasn't been a part of any official release yet, right? And on top of that, the current implementation doesn't work... So I don't think that changing the current behaviour will cause any issues.

Concerning JMX you are right. I meant to enhance the property detection for spring istelf, not only for the JMX component (which would then could take advantage of this). This way you could specify properties in a non-JavaBean style while Spring could still use them.
Anyway, it was just an idea, no requirement.

Regards,
Andreas