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

[Langer, Christoph]

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

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

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


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

line 329: grammar: Validate that a FileAlreadyExistsException is thrown

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

line 387: comment should be:
Create a Zip file containing the given entries, using Zip File System and the specified properties map


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.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191025/098bf49c/attachment-0001.html>