[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: [PING] Re: RFR 8129215: com.sun.jmx.mbeanserver.Introspector may provide results inconsistent wi
From:       Jaroslav Bachorik <jaroslav.bachorik () oracle ! com>
Date:       2015-07-31 13:17:56
Message-ID: 55BB7584.2050700 () oracle ! com
[Download RAW message or body]

On 31.7.2015 15:03, Daniel Fuchs wrote:
> Hi Jaroslav,
>
> Thanks for the ping, I hadn't realized you were waiting :-)
>
> Since this is limited to complex composite attribute names
> I believe it is the right thing. However - it would be good
> to verify (maybe you already did it?) that it passes the JCK.

actually, JCK tests are failing without this change (one of the comments 
in the issue)

>
> One small suggestion for the SimpleIntrospectorTest: I find
> the logic of the checkGetter difficult to follow - I believe it
> would be easier for future maintainers if that method had a
> parameter called  "nullExpected" rather than "reverse".

ok.

Thanks!

-JB-

>
> Otherwise, this looks good!
>
> best regards,
>
> -- daniel
>
>
> On 31/07/15 13:56, Jaroslav Bachorik wrote:
>> On 19.6.2015 18:20, Jaroslav Bachorik wrote:
>>> On 19.6.2015 15:50, Daniel Fuchs wrote:
>>>> Hi guys,
>>>>
>>>> I believe there's a difference between Attribute names, and navigation
>>>> into complex attributes.
>>>>
>>>> At the MBean level attribute names are case sensitive.
>>>> At Attribute level (= within an attribute) I believe it follows
>>>> the Bean patterns (like for instance the mapping of complex
>>>> MXBean attribute types to CompositeData).
>>>> A property of an Attribute (as in MemoryUsage.used) is not itself
>>>> an Attribute - but a bean property.
>>>>
>>>> A careful reading of the JMX 1.4 spec should hopefully make it possible
>>>> to validate that ;-)
>>>
>>> Also, some JCK tests are relying on the Attribute properties being
>>> treated as JavaBean properties. These were, in fact, the ones triggering
>>> this issue when run without j.b.Introspector being available.
>>>
>>> So, if I understand it correctly, with my fix the implementation will be
>>> in agreement with the spec again even when j.b.Introspector is not
>>> available.
>>
>> Could we get this wrapped, please?
>> The change is not introducing any new behaviour - it just makes the
>> SimpleIntrospector (being used when j.b.Introspector is not available)
>> to behave accordingly to the spec.
>>
>> -JB-
>>
>>>
>>> -JB-
>>>
>>>>
>>>> cheers,
>>>>
>>>> -- daniel
>>>>
>>>> On 19/06/15 14:05, Jaroslav Bachorik wrote:
>>>>> On 19.6.2015 13:44, Alan Bateman wrote:
>>>>>>
>>>>>> On 19/06/2015 12:38, Jaroslav Bachorik wrote:
>>>>>>>
>>>>>>> Both the j.b.Introspector.getReadMethod() and the
>>>>>>> SimpleIntrospector.getReadMethod() are used only from
>>>>>>> c.s.j.m.Introspector.elementFromComplex() method and only to resolve
>>>>>>> the getter for a complex attribute.
>>>>>>>
>>>>>>> So, any change in the SimpleIntrospector will not affect the rest of
>>>>>>> the JMX system.
>>>>>>>
>>>>>>> Given this statement in the Monitoring javadocs
>>>>>>> (https://docs.oracle.com/javase/8/docs/api/javax/management/monitor/package-summary.html)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> "If the above rules do not produce a value, and if introspection, as
>>>>>>> if by calling Introspector.getBeanInfo, for the class of v
>>>>>>> (v.getClass()) identifies a property with the name e, then x is the
>>>>>>> result of reading the property value."
>>>>>>>
>>>>>>> I would strongly propose to adjust the SimpleIntrospector to closely
>>>>>>> follow the j.b.Introspector functionality.
>>>>>>>
>>>>>> I added the SimpleIntrospector and also adjusted the above wording in
>>>>>> the monitor package description to make it possible to have JMX in a
>>>>>> compact Profile of Java SE that didn't have java.beans.
>>>>>>
>>>>>> The intention was that the SimpleIntrospector works like the beans
>>>>>> Introspector. So I agree this is the right thing to do but we should
>>>>>> create a bug for the issue that Daniel brings up.
>>>>>
>>>>> I'm not sure there is an issue here. The monitoring api attribute
>>>>> names
>>>>> seem to be governed by different rules than the MBean attribute names.
>>>>> This change relates to the monitoring api only and brings the
>>>>> implementation up to the what is specified in the javadoc.
>>>>>
>>>>> -JB-
>>>>>
>>>>>>
>>>>>> -Alan.
>>>>>
>>>>
>>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]