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

[Roger Riggs]

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
>>>
>>
>