Cache/reuse jar filesystems.

Use Fs.fromUrl() instead of Paths.get().
robolectric · Dec 14, 2018 · dde6253 · dde6253
1 parent 5ffaf54
commit dde6253
Show file tree

Hide file tree

Showing 9 changed files with 143 additions and 28 deletions.
diff --git a/resources/src/main/java/org/robolectric/res/Fs.java b/resources/src/main/java/org/robolectric/res/Fs.java
@@ -9,18 +9,28 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.nio.file.FileStore;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.PathMatcher;
 import java.nio.file.Paths;
+import java.nio.file.WatchService;
+import java.nio.file.attribute.UserPrincipalLookupService;
+import java.nio.file.spi.FileSystemProvider;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 import org.robolectric.util.Util;
 
 @SuppressWarnings({"NewApi", "AndroidJdkLibsChecker"})
 abstract public class Fs {
 
+  private static final Map<Path, FsWrapper> ZIP_FILESYSTEMS = new HashMap<>();
+
   /**
    * @deprecated Use {@link File#toPath()} instead.
    */
@@ -30,25 +40,32 @@ public static Path newFile(File file) {
   }
 
   /**
-   * @deprecated Use {@link Paths#get(String, String...)} instead.
+   * @deprecated Use {@link #fromUrl(String)} instead.
    */
   @Deprecated
   public static Path fileFromPath(String path) {
-    return Paths.get(path);
+    return Fs.fromUrl(path);
   }
 
   public static FileSystem forJar(URL url) {
     return forJar(Paths.get(toUri(url)));
   }
 
-  public static FileSystem forJar(Path file) {
+  public static FileSystem forJar(Path jarFile) {
     try {
-      return FileSystems.newFileSystem(file, null);
+      return getJarFs(jarFile);
     } catch (IOException e) {
       throw new RuntimeException(e);
     }
   }
 
+  /**
+   * Use this method instead of {@link Paths#get(String, String...)} or {@link Paths#get(URI)}.
+   *
+   * Supports "file:path", "jar:file:jarfile.jar!/path", and plain old paths.
+   *
+   * For JAR files, automatically open and cache filesystems.
+   */
   public static Path fromUrl(String urlString) {
     if (urlString.startsWith("file:") || urlString.startsWith("jar:")) {
       URL url;
@@ -74,7 +91,7 @@ public static Path fromUrl(URL url) {
         case "jar":
           String[] parts = url.getPath().split("!", 0);
           Path jarFile = Paths.get(new URI(parts[0]).toURL().getFile());
-          FileSystem fs = FileSystems.newFileSystem(jarFile, null);
+          FileSystem fs = getJarFs(jarFile);
           return fs.getPath(parts[1].substring(1));
         default:
           throw new IllegalArgumentException("unsupported fs type for '" + url + "'");
@@ -146,4 +163,106 @@ public static String externalize(Path path) {
       return path.toUri().toString();
     }
   }
+
+  /**
+   * Returns a reference-counted Jar FileSystem, possibly one that was previously returned.
+   */
+  synchronized private static FileSystem getJarFs(Path jarFile) throws IOException {
+    Path key = jarFile.toAbsolutePath();
+
+    FsWrapper fs = ZIP_FILESYSTEMS.get(key);
+    if (fs == null) {
+
+      fs = new FsWrapper(FileSystems.newFileSystem(key, null), key);
+      ZIP_FILESYSTEMS.put(key, fs);
+    }
+
+    fs.incrRefCount();
+
+    return fs;
+  }
+
+  @SuppressWarnings("NewApi")
+  private static class FsWrapper extends FileSystem {
+    private final FileSystem delegate;
+    private final Path jarFile;
+
+    private int refCount;
+
+    public FsWrapper(FileSystem delegate, Path jarFile) {
+      this.delegate = delegate;
+      this.jarFile = jarFile;
+    }
+
+    synchronized void incrRefCount() {
+      refCount++;
+    }
+
+    synchronized void decrRefCount() throws IOException {
+      if (refCount-- == 0) {
+        ZIP_FILESYSTEMS.remove(jarFile);
+        delegate.close();
+      }
+    }
+
+    @Override
+    public FileSystemProvider provider() {
+      return delegate.provider();
+    }
+
+    @Override
+    public void close() throws IOException {
+      decrRefCount();
+    }
+
+    @Override
+    public boolean isOpen() {
+      return delegate.isOpen();
+    }
+
+    @Override
+    public boolean isReadOnly() {
+      return delegate.isReadOnly();
+    }
+
+    @Override
+    public String getSeparator() {
+      return delegate.getSeparator();
+    }
+
+    @Override
+    public Iterable<Path> getRootDirectories() {
+      return delegate.getRootDirectories();
+    }
+
+    @Override
+    public Iterable<FileStore> getFileStores() {
+      return delegate.getFileStores();
+    }
+
+    @Override
+    public Set<String> supportedFileAttributeViews() {
+      return delegate.supportedFileAttributeViews();
+    }
+
+    @Override
+    public Path getPath(String first, String... more) {
+      return delegate.getPath(first, more);
+    }
+
+    @Override
+    public PathMatcher getPathMatcher(String syntaxAndPattern) {
+      return delegate.getPathMatcher(syntaxAndPattern);
+    }
+
+    @Override
+    public UserPrincipalLookupService getUserPrincipalLookupService() {
+      return delegate.getUserPrincipalLookupService();
+    }
+
+    @Override
+    public WatchService newWatchService() throws IOException {
+      return delegate.newWatchService();
+    }
+  }
 }
diff --git a/resources/src/main/java/org/robolectric/res/android/CppAssetManager.java b/resources/src/main/java/org/robolectric/res/android/CppAssetManager.java
@@ -1769,10 +1769,10 @@ public List<AssetPath> getAssetPaths() {
         Path path;
         switch (asset_path.type) {
           case kFileTypeDirectory:
-            path = Paths.get(asset_path.path.string());
+            path = Fs.fromUrl(asset_path.path.string());
             break;
           case kFileTypeRegular:
-            path = Paths.get(asset_path.path.string());
+            path = Fs.fromUrl(asset_path.path.string());
             break;
           default:
             throw new IllegalStateException("Unsupported type " + asset_path.type + " for + "

diff --git a/resources/src/main/java/org/robolectric/res/android/CppAssetManager2.java b/resources/src/main/java/org/robolectric/res/android/CppAssetManager2.java
@@ -16,7 +16,6 @@
 import static org.robolectric.res.android.Util.isTruthy;
 
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -26,6 +25,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import org.robolectric.res.Fs;
 import org.robolectric.res.android.AssetDir.FileInfo;
 import org.robolectric.res.android.CppApkAssets.ForEachFileCallback;
 import org.robolectric.res.android.CppAssetManager.FileType;
@@ -1658,7 +1658,7 @@ public boolean SetTo(final Theme o) {
   public List<AssetPath> getAssetPaths() {
     ArrayList<AssetPath> assetPaths = new ArrayList<>(apk_assets_.size());
     for (CppApkAssets apkAssets : apk_assets_) {
-      Path path = Paths.get(apkAssets.GetPath());
+      Path path = Fs.fromUrl(apkAssets.GetPath());
       assetPaths.add(new AssetPath(path, apkAssets.GetLoadedArsc().IsSystem()));
     }
     return assetPaths;

diff --git a/robolectric/src/main/java/org/robolectric/internal/ManifestIdentifier.java b/robolectric/src/main/java/org/robolectric/internal/ManifestIdentifier.java
@@ -1,7 +1,6 @@
 package org.robolectric.internal;
 
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -50,9 +49,9 @@ public ManifestIdentifier(Path manifestFile, Path resDir, Path assetDir, String
       for (Path libraryDir : libraryDirs) {
         libraries.add(new ManifestIdentifier(
             null,
-            libraryDir.resolve(Paths.get(Config.DEFAULT_MANIFEST_NAME)),
-            libraryDir.resolve(Paths.get(Config.DEFAULT_RES_FOLDER)),
-            libraryDir.resolve(Paths.get(Config.DEFAULT_ASSET_FOLDER)),
+            libraryDir.resolve(Config.DEFAULT_MANIFEST_NAME),
+            libraryDir.resolve(Config.DEFAULT_RES_FOLDER),
+            libraryDir.resolve(Config.DEFAULT_ASSET_FOLDER),
             null));
       }
     }

diff --git a/robolectric/src/test/java/org/robolectric/ManifestFactoryTest.java b/robolectric/src/test/java/org/robolectric/ManifestFactoryTest.java
@@ -3,7 +3,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 
-import java.net.URI;
+import java.net.URL;
 import java.nio.file.Paths;
 import java.util.Properties;
 import org.junit.Test;
@@ -14,6 +14,7 @@
 import org.robolectric.internal.ManifestFactory;
 import org.robolectric.internal.ManifestIdentifier;
 import org.robolectric.manifest.AndroidManifest;
+import org.robolectric.res.Fs;
 
 @RunWith(JUnit4.class)
 public class ManifestFactoryTest {
@@ -76,9 +77,8 @@ protected Properties getBuildSystemApiProperties() {
     ManifestFactory manifestFactory = testRunner.getManifestFactory(config);
     assertThat(manifestFactory).isInstanceOf(DefaultManifestFactory.class);
     ManifestIdentifier manifestIdentifier = manifestFactory.identify(config);
-    URI expectedUri = getClass().getClassLoader().getResource("TestAndroidManifest.xml").toURI();
-    assertThat(manifestIdentifier.getManifestFile())
-            .isEqualTo(Paths.get(expectedUri));
+    URL expectedUrl = getClass().getClassLoader().getResource("TestAndroidManifest.xml");
+    assertThat(manifestIdentifier.getManifestFile()).isEqualTo(Fs.fromUrl(expectedUrl));
     assertThat(manifestIdentifier.getResDir()).isEqualTo(Paths.get("/path/to/merged-resources"));
     assertThat(manifestIdentifier.getAssetDir()).isEqualTo(Paths.get("/path/to/merged-assets"));
     assertThat(manifestIdentifier.getLibraries()).isEmpty();

diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowTypefaceTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowTypefaceTest.java
@@ -8,7 +8,6 @@
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import java.io.File;
-import java.nio.file.Paths;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -22,7 +21,7 @@ public class ShadowTypefaceTest {
   @Before
   public void setup() throws Exception {
     fontFile =
-        TestUtil.resourcesBaseDir().resolve(Paths.get("assets/myFont.ttf")).toFile();
+        TestUtil.resourcesBaseDir().resolve("assets/myFont.ttf").toFile();
   }
 
   @Test

diff --git a/shadows/framework/src/main/java/org/robolectric/android/XmlResourceParserImpl.java b/shadows/framework/src/main/java/org/robolectric/android/XmlResourceParserImpl.java
@@ -10,10 +10,10 @@
 import java.io.InputStream;
 import java.io.Reader;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
 import org.robolectric.res.AttributeResource;
+import org.robolectric.res.Fs;
 import org.robolectric.res.ResName;
 import org.robolectric.res.ResourceTable;
 import org.robolectric.res.StringResources;
@@ -77,7 +77,7 @@ public XmlResourceParserImpl(
       String packageName,
       String applicationPackageName,
       ResourceTable resourceTable) {
-    this(document, Paths.get(fileName), packageName, applicationPackageName, resourceTable);
+    this(document, Fs.fromUrl(fileName), packageName, applicationPackageName, resourceTable);
   }
 
   public XmlResourceParserImpl(

diff --git a/shadows/framework/src/main/java/org/robolectric/shadows/ShadowLegacyAssetManager.java b/shadows/framework/src/main/java/org/robolectric/shadows/ShadowLegacyAssetManager.java
@@ -37,7 +37,6 @@
 import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -389,7 +388,7 @@ protected final AssetFileDescriptor openFd(String fileName) throws IOException {
 
   private Path findAssetFile(String fileName) throws IOException {
     for (Path assetDir : getAllAssetDirs()) {
-      Path assetFile = assetDir.resolve(Paths.get(fileName));
+      Path assetFile = assetDir.resolve(fileName);
       if (Files.exists(assetFile)) {
         return assetFile;
       }
@@ -446,7 +445,7 @@ protected final String[] list(String path) throws IOException {
       if (path.isEmpty()) {
         file = assetsDir;
       } else {
-        file = assetsDir.resolve(Paths.get(path));
+        file = assetsDir.resolve(path);
       }
 
       if (Files.isDirectory(file)) {
@@ -530,7 +529,7 @@ protected Number openXmlAssetNative(int cookie, String fileName) throws FileNotF
   @Implementation
   protected final XmlResourceParser openXmlResourceParser(int cookie, String fileName)
       throws IOException {
-    XmlBlock xmlBlock = XmlBlock.create(Paths.get(fileName), resourceTable.getPackageName());
+    XmlBlock xmlBlock = XmlBlock.create(Fs.fromUrl(fileName), resourceTable.getPackageName());
     if (xmlBlock == null) {
       throw new Resources.NotFoundException(fileName);
     }
@@ -602,7 +601,7 @@ private XmlResourceParser getXmlResourceParser(ResourceTable resourceProvider, X
 
   @HiddenApi @Implementation
   public int addAssetPath(String path) {
-    assetDirs.add(Paths.get(path));
+    assetDirs.add(Fs.fromUrl(path));
     return 1;
   }
 
@@ -622,7 +621,7 @@ public void setApkAssets(Object apkAssetsObject, Object invalidateCachesObject)
     boolean invalidateCaches = (boolean) invalidateCachesObject;
 
     for (ApkAssets apkAsset : apkAssets) {
-      assetDirs.add(Paths.get(apkAsset.getAssetPath()));
+      assetDirs.add(Fs.fromUrl(apkAsset.getAssetPath()));
     }
     directlyOn(realObject, AssetManager.class).setApkAssets(apkAssets, invalidateCaches);
   }

diff --git a/shadows/framework/src/main/java/org/robolectric/shadows/ShadowTypeface.java b/shadows/framework/src/main/java/org/robolectric/shadows/ShadowTypeface.java
@@ -19,7 +19,6 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.function.Predicate;
 import org.robolectric.annotation.HiddenApi;
 import org.robolectric.annotation.Implementation;
 import org.robolectric.annotation.Implements;