RFR (JAXP) 8176405 : Catalog circular reference check did not work in certain scenarios

[Roger Riggs]

Looks good.

Thanks, Roger


On 3/23/2017 4:37 PM, huizhe wang wrote:
>
> On 3/23/2017 11:08 AM, Roger Riggs wrote:
>> Hi Joe,
>>
>>  - javax.xml.catalog.GroupEntry.set(catalog):183
>>    - rename the method to setCatalog(catalog) to be a bit more 
>> expressive.
>
> Done.
>>
>>  - GroupEntry: 460:461;  correct the @param for "catalogId" -> 
>> "catalogURI"
>>    Check all the @param catalogId -> catalogURI in the file
>>    and there are extra blank lines between the @param tags
>
> Fixed.
>>
>> - CatalogImpl.java: line 432: the @param name "path" should be "uri" 
>> to match the method signature
>
> Fixed, and plus a couple of methods where @param was missing.
>>
>> - CatalogMessages.properties:
>>   Is it intentional that the OtherError JAXP09000002: Unexpected error
>>   has the same number as FormatFailed: JAXP09000002?
>
> Fixed, JAXP09000003 as it should be.
>>
>>   And should CircularReference = JAXP0901001 be JAXP09030004?
>
> It's parsing error but also a restriction imposed by the impl. I moved 
> it to it's own category "Implementation restriction".
>>
>> - CatalogTest: Line 574:  The Assert at 573 prints the expected and 
>> actual.
>>   Adding a reason as a third arg makes it easier to debug.
>>   The println at 574 will only be printed if the test succeeds, so is 
>> of little value
>
> Changed that to assertTrue(msg.contains(expectedMsgId)). It's safer, 
> in case the test is run on a non-English system.
>
> Updated:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8176405
> Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8176405/webrev/
>
> Thanks,
> Joe
>
>>
>> Regards, Roger
>>
> On 3/17/2017 1:22 PM, huizhe wang wrote:
>> Hi,
>>
>> Please review a fix to the circular reference check that incorrectly 
>> treated duplicate entries as circular.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176405
>> Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8176405/webrev/
>>
>> Thanks,
>> Joe
>>