Review for 7157656 (zipfs) SeekableByteChannel to entry in zip file...

[Xueming Shen]

Given the sequential nature of the zip stream, it's a kinda of hard to 
implement the
position(long) method for a "pure" zip byte channel. You don't want to 
jump either
backward or forward N bytes when writing and appending (we don't have a 
"position
mapping" between the compressed and uncompressed data, so you simply can't
"position" to a desired N pos when compressing), and for the same reason 
you don't
want to jump back either when reading, the only "meaningful/doable" 
position(N)
operation is when N> position() for reading, which is basically a "skip 
N - position()
byte" operation.

In order to implement the position(N), for writing, you have to write 
all the data
without compression into a "buffer", you then can "position(N)" on it. 
And only do the
real compression upon closing. For reading, you have to de-compress the 
whole zip
entry into a buffer first, then build a channel on top of this buffer 
for the reading/
positioning. This is how newFileChannel is implemented now.

-Sherman

On 04/17/2012 03:28 AM, Paul Sandoz wrote:
> On Apr 17, 2012, at 11:50 AM, Alan Bateman wrote:
>
>> On 16/04/2012 17:55, Paul Sandoz wrote:
>>> Hi,
>>>
>>> I ain't got permission to publish webrevs yet. So attached is a patch produced by hg export on the jdk tree for:
>>>
>>>    7157656 (zipfs) SeekableByteChannel to entry in zip file always reports its position as 0
>>>
>>> Paul.
>> Sherman wrote this zip provider and I assume will want to review this so I'll leave it to him.
>>
> OK. Managed to push a webrev:
>
>    http://cr.openjdk.java.net/~psandoz/7157656/webrev.0/
>
> proxy issues...
>
>
>> One thing I notice, and nothing to do with your patch, is that the position(long) method is missing an implementation, it shouldn't throw UOE.
> There is a comment:
> // sbc.position(pos) is not supported in current version
> in the test code. I put test checks in place, mainly to be consistent, rather than any foresight on my part.
>
>
>> Also for the append case it looks like read throws UOE whereas it should throw NonReadableChannelException.
>>
>
> OK. What about the methods: position&  truncate?
>
> I will let Sherman comment as appropriate. Sherman, i can fix if you like.
>
> Paul.
>