RFR (JAXP): 8144967: javax.xml.transform.Source and org.xml.sax.InputSource can be empty

[huizhe wang]

Thanks Roger!

Joe

On 12/22/2015 12:07 PM, Roger Riggs wrote:
> Hi Joe,
>
> Thanks for the updates.  Looks fine.
>
> Roger
>
>
> On 12/21/2015 7:11 PM, huizhe wang wrote:
>> Thanks Roger!
>>
>> On 12/21/2015 1:35 PM, Roger Riggs wrote:
>>> Hi Joe,
>>>
>>> A few comments:
>>>
>>> InputSource:
>>
>> I assume you meant javax.xml.transform.Source (there was an error in 
>> Source's javadoc where the return statement referred to it as 
>> InputSource).
>>
>>>  - Should the default method return true? The default method is only 
>>> present
>>>    to allow source compatibility with unknown subtypes. I would 
>>> expect that without
>>>    any specific implementation knowledge it would need to be false.
>>>   All of the JDK provided Sources will implement isEmpty appropriately.
>>
>> Changed to throw UOE now as Joe recommended.
>>
>>>
>>>  - Editorial:  "Empty means that there is no input available from 
>>> this Source".
>>
>> Fixed.
>>>
>>>  - from the definition and implementation it seems that isEmpty will 
>>> be true
>>>    when a stream (streamSource) is positioned just before EOF.  Is 
>>> that intended?
>>
>> Yes. In general, a Source object shall be passed to a processor with 
>> input positioned at the beginning. If for some reason it's not, the 
>> Source object will not handle the situation.
>>>
>>>
>>> stream/StreamSource:
>>>  - Does not need to check both sources;  if the first source has any 
>>> input, return false immediately.
>>
>> Fixed.
>>>
>>>  - the choice to return false when an IOException occurs seems odd;
>>>    if reading throws an exception then it seems unlikely that any 
>>> future read could work;
>>>    and hence there is no more input.
>>
>> It's true no future read would work. But since a Source object can be 
>> used in various situations where "empty" and failure to read are 
>> treated differently by the processor, the isEmpty method needs to 
>> abort its process, return false to indicate it's not empty, thus 
>> allow the processor to handle the error.
>>
>> I added a statement to the spec.
>>
>>>
>>>  - Note also that you probably need separate try/catch handlers for 
>>> the stream vs the reader.
>>>    An exception from reading the inputStream should no preempt the 
>>> reader.
>>
>> As discussed above, in case of error, isEmpty will abort its process 
>> and return false.
>>
>>>
>>> sax/InputSource:
>>>   - ditto the comments above.
>>>
>>> In the test:
>>>  - This test is duplicated:
>>>
>>> + "{new SAXSource(new InputSource(new StringReader("")))},
>>> +  {new SAXSource(new InputSource(new StringReader("")))},
>>
>> Fixed.
>>
>> http://cr.openjdk.java.net/~joehw/jdk9/8144967/webrev/
>>
>> Thanks,
>> Joe
>>
>>>
>>>
>>> Roger
>>>
>>>
>>> On 12/17/2015 7:25 PM, huizhe wang wrote:
>>>> Hi,
>>>>
>>>> Adding a convenient method isEmpty to javax.xml.transform.Source 
>>>> and org.xml.sax.InputSource.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8144967
>>>> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8144967/webrev/
>>>>
>>>> Thanks,
>>>> Joe
>>>>
>>>
>>
>