harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Deven You <devyo...@gmail.com>
Subject Re: [classlib][nio_cahr] Issue about CharsetEncdoer.flush() does not follow the spec and RI5 also doesn't follow the spec
Date Fri, 23 Jul 2010 05:52:32 GMT
I have updated the HARMONY-6594
https://issues.apache.org/jira/browse/HARMONY-6594, anyone would like to
take a look and try this fix, Thanks a lot!

2010/7/23 Deven You <devyoudw@gmail.com>

>
>
> 2010/7/22 Deven You <devyoudw@gmail.com>
>
> I have summarized our harmony unit test results between RI5 and RI6,
>> Test case    Test expected (Follow spec5?)        RI5(Follow spec5?)
>>                        RI6(Follow spec6?)
>> CharsetEncoder sequence
>> [1]                 No exception(x)
>>  Throws illegaStateException (yes)     the same as RI5 (yes)
>>             new created Encoder -> flush()
>> [2]                 No exception(yes)                              Throws
>> illegaStateException (x)         the same as RI5 (yes)
>>        reset -> encode(,,,) -> reset -> flush
>> [3]                Throw IlegalStateException(?)          Not throw
>> illegaSateException(?)       the same as RI5 (?)
>>     encode(in) -> flush()
>> [4]                Throw IlegalStateException(yes)      Throw
>> IlegalStateException (yes)    Not throw illegaSateException(yes)
>>  flush -> flush
>>
>> [1]org.apache.harmony.nio_char.tests.java.nio.charset.ASCIICharsetEncoderTest.testInternalState_Flushed()
>> (//init -> flushed unit)
>> [2]org.apache.harmony.nio_char.tests.java.nio.charset.ASCIICharsetEncoderTest.testInternalState_Flushed()
>> () (//encoding -> flushed unit)
>>
>  sorry, //encoding -> flushed unit should be //reset - > flushed
>
>>
>> [3]org.apache.harmony.nio_char.tests.java.nio.charset.ASCIICharsetEncoderTest.testInternalState_from_Encode()
>> [43]tests.api.java.nio.charset.CharsetEncoderTest.testFlushIllegalState()
>>
>>
>> For [2], whether follow the spec depending on how we treat the state of
>> after calling the encode(CharBuffer), from my point, this state should be
>> FULSH, so harmony trunk follows spec5, RI5 doesn't and RI6 also follows the
>> spec6 since spec6's change.
>>
>> From above, the logic of flush with our harmony is not clear,  I think we
>> need decision which logic we should choose, RI or spec?
>> 2010/7/21 Deven You <devyoudw@gmail.com>
>>
>>
>>>
>>> 2010/7/21 Tim Ellison <t.p.ellison@gmail.com>
>>>
>>> On 21/Jul/2010 06:24, Deven You wrote:
>>>> > See https://issues.apache.org/jira/browse/HARMONY-6594
>>>> > <https://issues.apache.org/jira/browse/HARMONY-6594>
>>>> > In Java5 Spec, the flush() method should always be invoked after
>>>> reset() or the
>>>> > three-argument encode<http://
>>>> ../../../java/nio/charset/CharsetEncoder.html#encode(java.nio.CharBuffer,+java.nio.ByteBuffer,+boolean)>
>>>> >  method with a value of true for the endOfInput parameter, otherwise,
>>>> an
>>>> > IllegalStateException will be throwed. Harmony's implementation does
>>>> not
>>>> > implement this logic, when an encoder is created and followed by
>>>> calling its
>>>> > flush() method,  flush() should throw IllegalStateException. I have
>>>> fix
>>>> > previous case with
>>>> > HARMONY-6594<https://issues.apache.org/jira/browse/HARMONY-6594>.
>>>>
>>>> I wonder if we shouldn't just take a pragmatic view on this one.
>>>>
>>>> I can see why flush() wouldn't make sense during an encoding until all
>>>> the data has been received, but I see no reason why the flush() of a
>>>> newly created encoder should fail, and I'd be very surprised if anyone
>>>> actually relies on this behavior.
>>>>
>>>> >  However I checked that RI5 also  does not completely follow the spec.
>>>>  For
>>>> > the invocation sequence: reset -> encode with 3 arguments -> reset
 ->
>>>> > flush, RI5 throw IlegalStateException against the spec.
>>>> >
>>>> (seeorg.apache.harmony.nio_char.tests.java.nio.charset.ASCIICharsetEncoderTest.testInternalState_Flushed()
>>>> > )
>>>>
>>>> Same as above, the reset() should put the encoder back to its starting
>>>> state -- whereupon I'd expect a flush() to do nothing.
>>>>
>>>> > and sequence: encode(Charbuffer) -> flush(), RI5 doesn't throw
>>>> > IlegalStateException against the spec. (after encode(Charbuffer), the
>>>> > encoder should be in FLUSH state)
>>>> >  (see
>>>> seeorg.apache.harmony.nio_char.tests.java.nio.charset.ASCIICharsetEncoderTest.testInternalState_from_Encode)
>>>>
>>>> Again, despite the spec, the behavior you observe of flush()-ing twice
>>>> not being an error sounds reasonable to me.  In this case I definitely
>>>> think we should follow the behavior rather than the spec. as it is more
>>>> likely to occur in normal usage.
>>>>
>>>> > Further Investigation shows from Java6 Spec, this behavior is changed,
>>>> it
>>>> > says  flush() will throw IllegalStateException if the previous step
of
>>>> the
>>>> > current encoding operation was an invocation neither of the
>>>> > flush<http://
>>>> ../../../java/nio/charset/CharsetEncoder.html#flush(java.nio.ByteBuffer)>
>>>> >  method nor of the three-argument
>>>> > encode<http://
>>>> ../../../java/nio/charset/CharsetEncoder.html#encode(java.nio.CharBuffer,+java.nio.ByteBuffer,+boolean)>
>>>> >  method with a value of true for the endOfInput parameter. And
>>>> actually, RI5
>>>> > follows the java6 spec rather than java5!
>>>>
>>>> I'm guessing the spec was updated to reflect the desired behavior -- so
>>>> I'd go for matching the Java 6 behavior in both Harmony 5 and Harmony 6
>>>> streams.
>>>>
>>>> > So now I am confused if we should modify our harmony trunk
>>>> CharsetEncoder to
>>>> > comply with the java5 spec or in other hand modify it to comply with
>>>> RI5 and
>>>> > java6 spec for above 2 cases? Anyone could give me some suggestions
>>>> for this
>>>> > point?
>>>>
>>>> What does Java 6 do for flush()-ing a
>>>>  - newly created encoder?
>>>>
>>> private static ByteBuffer bytes = ByteBuffer.allocate(8192);
>>>      private static CharsetEncoder encoder;
>>>
>>> public static void main(String[] args) {
>>> String encoding = AccessController
>>>  .doPrivileged(new PriviAction<String>(
>>> "file.encoding", "ISO8859_1")); //$NON-NLS-1$ //$NON-NLS-2$
>>>  encoder = Charset.forName(encoding).newEncoder();
>>> encoder.onMalformedInput(CodingErrorAction.REPLACE);
>>>  encoder.onUnmappableCharacter(CodingErrorAction.REPLACE);
>>>  encoder.flush(bytes);
>>>  System.out.println("should not reach here");
>>>
>>> }
>>> RI6:
>>> Exception in thread "main" java.lang.IllegalStateException: Current state
>>> = RESET, new state = FLUSHED
>>> at
>>> java.nio.charset.CharsetEncoder.throwIllegalStateException(CharsetEncoder.java:951)
>>>  at java.nio.charset.CharsetEncoder.flush(CharsetEncoder.java:640)
>>> at
>>> ydw.arena7.luni.test.TestCharsetEncoderFlush.main(TestCharsetEncoderFlush.java:25)
>>>
>>>  - a reset encoder?
>>>>
>>>
>>>  org.apache.harmony.nio_char.tests.java.nio.charset.ASCIICharsetEncoderTest.testInternalState_Flushed()
>>> covers this usage, RI6 also return IlegalStateException.
>>>
>>> I also think RI's behavior is strange. My concern is if we do not comply
>>> with RI, Many CharsetEncoder cases will have different results between
>>> harmony and RI.
>>>
>>>>
>>>> Regards,
>>>> Tim
>>>>
>>>
>>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message