RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

[Lance Andersen]

Hi Christoph,

Thank you for looking into this.

Overall, I think this is OK.  Not sure there is currently any downside to removing the JAR FS right now outside of keeping the multi-release code separate.

I would suggesting moving the code added to the constructor for checking the releaseVersion/multi-release properties to a method and grouping it with the other methods added for supporting MR jars around line 1396. (keeps it easier to follow and maintain)

Could you also add a comment above the isMultiReleaseJar method to for clarity going forward (I know there was not one there before)

I might change the name of the lookup field in the method makeParentDirs with the addition of the Function lookup on line 126.

The methods in JarFileSystemProvider getPath and uriToPath are slightly different in ZipFileSystemProvider.  That being said, I do not see anything obvious of concern but wanted to point it out.

The test changes look fine 

Again, thanks for your efforts to improve Zip FS

Best
Lance

> On Nov 14, 2019, at 10:23 AM, Langer, Christoph <christoph.langer at sap.com> wrote:
> 
> Hi,
>  
> after exchanging some direct mails with Lance, I came to the conclusion that the current behavior to ignore file system property “multi-release” when “releaseVersion” is explicitly set to null is the right thing to do. So I updated the webrev to reinstate this functionality and added explicit comments as well as augmenting MultiReleaseJarTest.java a little bit to test that “multi-release” is ignored in the right places.
>  
> This is the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234089.1/ <http://cr.openjdk.java.net/~clanger/webrevs/8234089.1/>
>  
> Best regards
> Christoph
>  
>  
> From: Langer, Christoph 
> Sent: Mittwoch, 13. November 2019 17:42
> To: core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>; nio-dev <nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>>
> Subject: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem
>  
> Hi,
>  
> can I please get reviews for this cleanup change in zipfs.
>  
> Bug: https://bugs.openjdk.java.net/browse/JDK-8234089 <https://bugs.openjdk.java.net/browse/JDK-8234089>
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234089.0/ <http://cr.openjdk.java.net/~clanger/webrevs/8234089.0/>
>  
> I figured that JarFileSystemProvider is completely obsolete (please correct me if I’m wrong) and the reasons for having a class JarFileSystem that extends ZipFileSystems are very little in my opinion. I think maintainability is better when the few implementation details of multi release jars live in ZipFileSystem as well. It saves some code. The only possible drawback is that ZipFileSystem:: getInode would have an additional call to a lookup function, that usually is an identity transformation. I would hope however, that runtime impact is negligible.
>  
> I also fix a small bug when property “releaseVersion” is set to null in the env map and multi-release contains a value. In the current implementation it would not consider the “multi-release” value and treat the mr jar as the current runtime version. I had to do a small fix in MultiReleaseJarTest.java to make sure the properties list is cleared in every case.
>  
> The jdk/nio/zipfs tests run well after my change.
>  
> Thanks
> Christoph

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