Resend RFR 8231766: Files.copy/move do not honor requested compression method when copying/moving within the same zip file

[Lance Andersen]

Hi Christoph,

Thank you for the review, please see below.

> On Oct 25, 2019, at 4:00 AM, Langer, Christoph <christoph.langer at sap.com> wrote:
> 
> Hi Lance,
>  
> I’ve had a look as well now. Thanks for doing this fix and providing extensive testing ��
:-)
>  
> The change in ZipFileSystem looks correct.

thank you
> I have a few remarks for the test(s):
>  
> UpdateEntryTest:
>  
> testReplaceStoredEntry
>  
> maybe add some comment that we expect the updated entry to have the default compression method of the zip file system (which is DEFLATED for this test case)
done
>  
> Maybe you can fix some minor thing that was already in the test, while touching it:
> line 121: We don't use rc, so either remove it or add @SuppressWarnings("unused") if you think it's important to have for debugging purposes. I'd rather vote for removing :)

OK, done.  I added this  originally  because Intellij complained via the inspect option and  then complained after the change that it was an unused variable.  so I  just removed it per your request.
>  
> lines 163ff: Maybe you can initialize the properties map in one line: Map.of("noCompression", e1.method != ZipEntry.DEFLATED) and use this in place in line 177
> The only drawback is that you wouldn't have a check whether e1.method is in (ZipEntry.DEFLATED, ZipEntry.STORED) but is that needed? 10 fewer lines of code to read/understand ;-)

OK done in a similar way to your suggestion.  
>  
>  
> CopyMoveTests
>  
> line 110: use "expectedCompression == ZipEntry.STORED" instead of targetCompression (remove line 102). I think that makes the code more readable
> This also applies to line 144, 181, 247 and 280/282

OK,  while I think this is a personal style choice, I made the suggested change.
>  
> line 329: grammar: Validate that a FileAlreadyExistsException is thrown
done
>  
> line 338: formatMap printout: I guess this is for debugging and it does only exist in this one place although something like that could be as useful everywhere. I suggest to either remove it here or insert it in each test method, maybe hidden behind some debug switch

Added a DEBUG flag and  also updated the System.out.printf in the createZip method
>  
> line 387: comment should be:
> Create a Zip file containing the given entries, using Zip File System and the specified properties map

Updated the comment

The updated webrev can be found at: http://cr.openjdk.java.net/~lancea/8231766/webrev.02/index.html <http://cr.openjdk.java.net/~lancea/8231766/webrev.02/index.html>

Currently re-running mach5 jdk-tier1 through jdk-tier3 

Best
Lance
>  
>  
> Thanks
> Christoph
>  
> From: nio-dev <nio-dev-bounces at openjdk.java.net> On Behalf Of Brian Burkhalter
> Sent: Donnerstag, 24. Oktober 2019 19:50
> To: Lance Andersen <lance.andersen at oracle.com>
> Cc: nio-dev <nio-dev at openjdk.java.net>
> Subject: Re: Resend RFR 8231766: Files.copy/move do not honor requested compression method when copying/moving within the same zip file
>  
> Hi Lance,
>  
> OK that all sounds good. No need to update the webrev.
>  
> Thanks,
>  
> Brian
> 
> 
> On Oct 24, 2019, at 5:21 AM, Lance Andersen <lance.andersen at oracle.com <mailto:lance.andersen at oracle.com>> wrote:
>  
> Thank you for the review.
> On Oct 23, 2019, at 8:03 PM, Brian Burkhalter <brian.burkhalter at oracle.com <mailto:brian.burkhalter at oracle.com>> wrote:
>  
> Hi Lance,
>  
> Please see comments on the test below listed by line numbers.

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191025/a75ca780/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191025/a75ca780/oracle_sig_logo-0001.gif>