RFR 8024707: TRANSFORMEREXCEPTION : ITEM() RETURNS NULL WITH NODE LIST OF LENGTH =1 IN JAXP

[huizhe wang]

Aleksej,

The change looks good.

Thanks,
Joe

On 9/28/2013 7:54 AM, Aleksej Efimov wrote:
> Joe,
> Thank you for a suggestion. But I modified it a little bit:
>       public Node item(int index) {
>           if (m_iter != null) {
> -            int node;
> +            int node = 0;
>               int count = m_cachedNodes.size();
>   
>               if (count > index) {
>                   node = m_cachedNodes.elementAt(index);
>                   return m_dtm.getNode(node);
>               } else if (m_last == -1) {
> -                while (((node = m_iter.next()) != DTMAxisIterator.END)
> -                           && count <= index) {
> +                while (count <= index
> +                        && ((node = m_iter.next()) != DTMAxisIterator.END)) {
>                       m_cachedNodes.addElement(node);
>                       count++;
>                   }
> We need a value assigned to 'node' to avoid compilation error with 
> uninitialized 'node' variable. But this value doesn't mean nothing 
> (let it be 0) because for the first while loop iteration the second 
> "&&" condition will be always executed (we can reach the while loop 
> only when "count<=index" - the outer if-else condition). The test 
> passes with this new version.
> As the result - new webrev: 
> http://cr.openjdk.java.net/~aefimov/8024707/webrev.01/ 
> <http://cr.openjdk.java.net/%7Eaefimov/8024707/webrev.01/>
>
> Best Regards,
> Aleksej
>
> On 09/26/2013 10:46 PM, huizhe wang wrote:
>> Hi Aleksej,
>>
>> From your tests, it appears that the getLength() method was 
>> implemented correctly, and that the bug is not in the call to 
>> m_iter.next(), but it should not be called again once the count is > 
>> index.  So that fix would be to move the conditions around such that:
>>     public Node item(int index) {
>>         if (m_iter != null) {
>>             int node;
>>             int count = m_cachedNodes.size();
>>
>>             if (count > index) {
>>                 node = m_cachedNodes.elementAt(index);
>>                 return m_dtm.getNode(node);
>>             } else if (m_last == -1) {
>> -                while (((node = m_iter.next()) != DTMAxisIterator.END)
>> -                           && count <= index) {
>> +                while (count <= index
>> +                           && ((node = m_iter.next()) != 
>> DTMAxisIterator.END)) {
>>                     m_cachedNodes.addElement(node);
>>                     count++;
>>                 }
>>                 if (node == DTMAxisIterator.END) {
>>                     m_last = count;
>>                 } else {
>>                     return m_dtm.getNode(node);
>>                 }
>>             }
>>         }
>>         return null;
>>     }
>>
>> Thanks,
>> Joe
>>
>>
>> On 9/25/2013 4:10 AM, Aleksej Efimov wrote:
>>> Hi Joe,
>>> Your suggestion about getLength() brings to mine attention the 
>>> following behavior of unmodified JDK:
>>> If we slightly modify a TestFunc class [1] in such manner:
>>>
>>>     public static Node test( NodeList list ) {
>>>                  Node ret = list.item(0);
>>>                  System.err.println(list.getLength());
>>>                  return ret;
>>>     }
>>>
>>> And add more elements to in.xml:
>>> <input1><seq-elem1>inp1_1</seq-elem1><seq-elem1>inp1_2</seq-elem1><seq-elem1>inp1_3</seq-elem1></input1>
>>>
>>> The test will fails:
>>>
>>>     2
>>>     Transformation completed. Result:<?xml version="1.0"
>>>     encoding="UTF-8"?>inp1_2
>>>     Exception in thread "main" java.lang.RuntimeException: Incorrect
>>>     transformation result
>>>         at XSLT.main(XSLT.java:49)
>>>
>>> As you can see the length of 2 is incorrect value and the 'inp1_1' 
>>> element is ignored. But If we do another one change to test function 
>>> - first get the length() and then get the item:
>>>
>>>     public static Node test( NodeList list ) {
>>>     	    System.err.println(list.getLength());
>>>                  Node ret = list.item(0);
>>>                  return ret;
>>>     }
>>>
>>> The test will pass:
>>>
>>>     3
>>>     Transformation completed. Result:<?xml version="1.0"
>>>     encoding="UTF-8"?>inp1_1
>>>
>>> This behavior tells that item() method incorrectly caches the nodes 
>>> (The method called first - do the caching). But the caching of nodes 
>>> is done in correct way by getLength() function, but its according to 
>>> the test results. Currently, I'm trying to understand why it's 
>>> working in such way in a view of source code.
>>>
>>> -Aleksej
>>>
>>>
>>>
>>> [1] 
>>> http://cr.openjdk.java.net/~aefimov/8024707/jdk/test/javax/xml/jaxp/parsers/8024707/TestFunc.java 
>>> <http://cr.openjdk.java.net/%7Eaefimov/8024707/jdk/test/javax/xml/jaxp/parsers/8024707/TestFunc.java>
>>>
>>> On 09/20/2013 01:19 AM, huizhe wang wrote:
>>>> Hi Aleksej,
>>>>
>>>> Looks like the getLength() method has the same problem.
>>>>
>>>> Joe
>>>>
>>>> On 9/17/2013 5:01 AM, Aleksej Efimov wrote:
>>>>> Hi Everyone,
>>>>>
>>>>> There is a bug [1] in JAXP about transformation of one-item sized 
>>>>> node list:
>>>>> When the length of nodelist which is passed to a XSLT extension 
>>>>> function is 1, the node gotten from the node list becomes null.
>>>>> New test illustrates this issue [2]. Full webrev with proposed fix 
>>>>> can be found here: [3].
>>>>>
>>>>> [1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8024707
>>>>> [2] 
>>>>> http://cr.openjdk.java.net/~aefimov/8024707/raw_files/new/jdk/test/javax/xml/jaxp/parsers/8024707/XSLT.java
>>>>> [3] http://cr.openjdk.java.net/~aefimov/8024707/
>>>>>
>>>>> Best regards,
>>>>> Aleksej
>>>>
>>>
>>
>