jmx-dev RFR 8139870: sun.management.LazyCompositeData.isTypeMatched() fails for composite types with items of ArrayType

[Jaroslav Bachorik]

On 19.10.2015 16:31, Jaroslav Bachorik wrote:
> On 19.10.2015 16:28, Daniel Fuchs wrote:
>> On 19/10/15 15:36, Jaroslav Bachorik wrote:
>>> On 19.10.2015 14:40, Daniel Fuchs wrote:
>>>> Hi Jaroslav,
>>>>
>>>> This - and the cleanup - look good to me, but it would
>>>> be nicer if it was accompanied by a unit test :-)
>>>
>>> This time with test -
>>> http://cr.openjdk.java.net/~jbachorik/8139870/webrev.01
>>
>> Thanks for the test Jaroslav!
>>
>>> Testing *only* the correctness of the newly introduced functionality
>>> (eg. the test fails on older JDKs and passes on the build with the fix
>>> in place). Providing full coverage for LazyCompositeData is probably out
>>> of the scope of this change.
>>
>> Yes, I agree. This is fine.
>> BTW - did you verify that the JCK still passes?
>> Otherwise you might need to challenge it...
>
> Not yet. I really hope it will pass - otherwise it would mean that the
> incorrect behaviour has been baked in :/

Ran JCK, just to be sure. It did pass, as expected (I am changing the 
internal sun.management implementation and not a Java SE API, afterall).

-JB-

>
> -JB-
>
>>
>> best regards,
>>
>> -- daniel
>>
>>>
>>> -JB-
>>>
>>>>
>>>> best regards,
>>>>
>>>> -- daniel
>>>>
>>>> On 19/10/15 13:37, Jaroslav Bachorik wrote:
>>>>> Please, review the following change
>>>>>
>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8139870
>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8139870/webrev.00
>>>>>
>>>>> sun.management.LazyCompositeData.isTypeMatched() method is used to
>>>>> compare two composite types with the backward compatibility in mind.
>>>>> The
>>>>> idea is that when we have two types (type1, type2) type1 is matched by
>>>>> type2 when and only when type2 contains all the items of type1 and
>>>>> their
>>>>> types are in turn matching the item types from type1, recursively.
>>>>>
>>>>> However, this method fails to account for ArrayType type and
>>>>> instead of
>>>>> calling isTypeMatched() recursively on the array type it performs
>>>>> plain
>>>>> Object.equals(). This will cause problems when one tries to safely
>>>>> evolve (only adding items) composite types referred through ArrayType
>>>>> items.
>>>>>
>>>>> The patch adds the missing functionality.
>>>>>
>>>>> In http://cr.openjdk.java.net/~jbachorik/8139870/webrev.00/cleanup
>>>>> there
>>>>> is a followup webrev of code warnings cleanup for
>>>>> s.m.LazyComponentData.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -JB-
>>>>
>>>
>>
>