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

[huizhe wang]

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