Skip to content

Commit

Permalink
Make ResourceFileSystem.read lazy (#1348)
Browse files Browse the repository at this point in the history 
* Make ResourceFileSystem.read lazy

Closes: #1347

* Spotless
  • Loading branch information
@squarejesse
squarejesse authored Sep 18, 2023
1 parent 1a30fb6 commit ac2f1af
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 50 deletions.
98 changes: 48 additions & 50 deletions okio/src/jvmMain/kotlin/okio/internal/ResourceFileSystem.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import okio.Path.Companion.toOkioPath
import okio.Path.Companion.toPath
import okio.Sink
import okio.Source
import okio.source

/**
* A file system exposing Java classpath resources. It is equivalent to the files returned by
Expand All @@ -41,8 +42,9 @@ import okio.Source
* This file system is read-only.
*/
internal class ResourceFileSystem internal constructor(
classLoader: ClassLoader,
private val classLoader: ClassLoader,
indexEagerly: Boolean,
private val systemFileSystem: FileSystem = SYSTEM,
) : FileSystem() {
private val roots: List<Pair<FileSystem, Path>> by lazy { classLoader.toClasspathRoots() }

Expand Down Expand Up @@ -122,14 +124,10 @@ internal class ResourceFileSystem internal constructor(

override fun source(file: Path): Source {
if (!keepPath(file)) throw FileNotFoundException("file not found: $file")
val relativePath = file.toRelativePath()
for ((fileSystem, base) in roots) {
try {
return fileSystem.source(base / relativePath)
} catch (_: FileNotFoundException) {
}
}
throw FileNotFoundException("file not found: $file")
// Make sure we have a path that doesn't start with '/'.
val relativePath = ROOT.resolve(file).relativeTo(ROOT)
return classLoader.getResourceAsStream(relativePath.toString())?.source()
?: throw FileNotFoundException("file not found: $file")
}

override fun sink(file: Path, mustCreate: Boolean): Sink {
Expand Down Expand Up @@ -157,6 +155,47 @@ internal class ResourceFileSystem internal constructor(
return canonicalThis.relativeTo(ROOT).toString()
}

/**
* Returns a search path of classpath roots. Each element contains a file system to use, and
* the base directory of that file system to search from.
*/
private fun ClassLoader.toClasspathRoots(): List<Pair<FileSystem, Path>> {
// We'd like to build this upon an API like ClassLoader.getURLs() but unfortunately that
// API exists only on URLClassLoader (and that isn't the default class loader implementation).
//
// The closest we have is `ClassLoader.getResources("")`. It returns all classpath roots that
// are directories but none that are .jar files. To mitigate that we also search for all
// `META-INF/MANIFEST.MF` files, hastily assuming that every .jar file will have such an
// entry.
//
// Classpath entries that aren't directories and don't have a META-INF/MANIFEST.MF file will
// not be visible in this file system.
return getResources("").toList().mapNotNull { it.toFileRoot() } +
getResources("META-INF/MANIFEST.MF").toList().mapNotNull { it.toJarRoot() }
}

private fun URL.toFileRoot(): Pair<FileSystem, Path>? {
if (protocol != "file") return null // Ignore unexpected URLs.
return systemFileSystem to File(toURI()).toOkioPath()
}

private fun URL.toJarRoot(): Pair<FileSystem, Path>? {
val urlString = toString()
if (!urlString.startsWith("jar:file:")) return null // Ignore unexpected URLs.

// Given a URL like `jar:file:/tmp/foo.jar!/META-INF/MANIFEST.MF`, get the path to the archive
// file, like `/tmp/foo.jar`.
val suffixStart = urlString.lastIndexOf("!")
if (suffixStart == -1) return null
val path = File(URI.create(urlString.substring("jar:".length, suffixStart))).toOkioPath()
val zip = openZip(
zipPath = path,
fileSystem = systemFileSystem,
predicate = { entry -> keepPath(entry.canonicalPath) },
)
return zip to ROOT
}

private companion object {
val ROOT = "/".toPath()

Expand All @@ -165,47 +204,6 @@ internal class ResourceFileSystem internal constructor(
return ROOT / (toString().removePrefix(prefix).replace('\\', '/'))
}

/**
* Returns a search path of classpath roots. Each element contains a file system to use, and
* the base directory of that file system to search from.
*/
fun ClassLoader.toClasspathRoots(): List<Pair<FileSystem, Path>> {
// We'd like to build this upon an API like ClassLoader.getURLs() but unfortunately that
// API exists only on URLClassLoader (and that isn't the default class loader implementation).
//
// The closest we have is `ClassLoader.getResources("")`. It returns all classpath roots that
// are directories but none that are .jar files. To mitigate that we also search for all
// `META-INF/MANIFEST.MF` files, hastily assuming that every .jar file will have such an
// entry.
//
// Classpath entries that aren't directories and don't have a META-INF/MANIFEST.MF file will
// not be visible in this file system.
return getResources("").toList().mapNotNull { it.toFileRoot() } +
getResources("META-INF/MANIFEST.MF").toList().mapNotNull { it.toJarRoot() }
}

fun URL.toFileRoot(): Pair<FileSystem, Path>? {
if (protocol != "file") return null // Ignore unexpected URLs.
return SYSTEM to File(toURI()).toOkioPath()
}

fun URL.toJarRoot(): Pair<FileSystem, Path>? {
val urlString = toString()
if (!urlString.startsWith("jar:file:")) return null // Ignore unexpected URLs.

// Given a URL like `jar:file:/tmp/foo.jar!/META-INF/MANIFEST.MF`, get the path to the archive
// file, like `/tmp/foo.jar`.
val suffixStart = urlString.lastIndexOf("!")
if (suffixStart == -1) return null
val path = File(URI.create(urlString.substring("jar:".length, suffixStart))).toOkioPath()
val zip = openZip(
zipPath = path,
fileSystem = SYSTEM,
predicate = { entry -> keepPath(entry.canonicalPath) },
)
return zip to ROOT
}

private fun keepPath(path: Path) = !path.name.endsWith(".class", ignoreCase = true)
}
}
77 changes: 77 additions & 0 deletions okio/src/jvmTest/kotlin/okio/internal/ResourceFileSystemTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import kotlin.test.assertFailsWith
import kotlin.test.fail
import okio.BufferedSource
import okio.ByteString
import okio.FileNotFoundException
import okio.FileSystem
import okio.ForwardingFileSystem
import okio.IOException
import okio.Path
import okio.Path.Companion.toPath
Expand All @@ -48,6 +50,7 @@ class ResourceFileSystemTest {
assertThat(metadata.isDirectory).isFalse()

val content = fileSystem.read(path) { readUtf8() }
assertThat(fileSystem.metadata(path).isRegularFile).isTrue()

assertThat(content).isEqualTo("a")
}
Expand All @@ -63,6 +66,7 @@ class ResourceFileSystemTest {
assertThat(metadata.isDirectory).isFalse()

val content = fileSystem.read(path) { readUtf8() }
assertThat(fileSystem.metadata(path).isRegularFile).isTrue()

assertThat(content).isEqualTo("b/b")
}
Expand All @@ -81,9 +85,15 @@ class ResourceFileSystemTest {

assertThat(resourceFileSystem.read("hello.txt".toPath()) { readUtf8() })
.isEqualTo("Hello World")
assertThat(
resourceFileSystem.metadata("hello.txt".toPath()).isRegularFile,
).isTrue()

assertThat(resourceFileSystem.read("directory/subdirectory/child.txt".toPath()) { readUtf8() })
.isEqualTo("Another file!")
assertThat(
resourceFileSystem.metadata("directory/subdirectory/child.txt".toPath()).isRegularFile,
).isTrue()

assertThat(resourceFileSystem.list("/".toPath()))
.hasSameElementsAs(listOf("/META-INF".toPath(), "/hello.txt".toPath(), "/directory".toPath()))
Expand Down Expand Up @@ -126,6 +136,10 @@ class ResourceFileSystemTest {
assertThat(resourceFileSystem.read("/colors/blue.txt".toPath()) { readUtf8() })
.isEqualTo("The sky is blue")

assertThat(resourceFileSystem.metadata("/colors/red.txt".toPath()).isRegularFile).isTrue()
assertThat(resourceFileSystem.metadata("/colors/green.txt".toPath()).isRegularFile).isTrue()
assertThat(resourceFileSystem.metadata("/colors/blue.txt".toPath()).isRegularFile).isTrue()

assertThat(resourceFileSystem.list("/".toPath()))
.hasSameElementsAs(listOf("/META-INF".toPath(), "/colors".toPath()))
assertThat(resourceFileSystem.list("/colors".toPath())).hasSameElementsAs(
Expand Down Expand Up @@ -169,6 +183,10 @@ class ResourceFileSystemTest {
assertThat(resourceFileSystem.read("/colors/blue.txt".toPath()) { readUtf8() })
.isEqualTo("The sky is blue")

assertThat(resourceFileSystem.metadata("/colors/red.txt".toPath()).isRegularFile).isTrue()
assertThat(resourceFileSystem.metadata("/colors/green.txt".toPath()).isRegularFile).isTrue()
assertThat(resourceFileSystem.metadata("/colors/blue.txt".toPath()).isRegularFile).isTrue()

assertThat(resourceFileSystem.list("/".toPath()))
.hasSameElementsAs(listOf("/colors".toPath()))
assertThat(resourceFileSystem.list("/colors".toPath())).hasSameElementsAs(
Expand Down Expand Up @@ -213,6 +231,10 @@ class ResourceFileSystemTest {
assertThat(resourceFileSystem.read("/colors/blue.txt".toPath()) { readUtf8() })
.isEqualTo("The sky is blue")

assertThat(resourceFileSystem.metadata("/colors/red.txt".toPath()).isRegularFile).isTrue()
assertThat(resourceFileSystem.metadata("/colors/green.txt".toPath()).isRegularFile).isTrue()
assertThat(resourceFileSystem.metadata("/colors/blue.txt".toPath()).isRegularFile).isTrue()

assertThat(resourceFileSystem.list("/".toPath()))
.hasSameElementsAs(listOf("/META-INF".toPath(), "/colors".toPath()))
assertThat(resourceFileSystem.list("/colors".toPath())).hasSameElementsAs(
Expand Down Expand Up @@ -292,6 +314,9 @@ class ResourceFileSystemTest {
assertThat(resourceFileSystem.list("/com/example/project".toPath())).isEmpty()
assertThat(resourceFileSystem.metadataOrNull("/com/example/project/Hello.class".toPath()))
.isNull()
assertFailsWith<FileNotFoundException> {
resourceFileSystem.source("/com/example/project/Hello.class".toPath())
}
}

@Test
Expand Down Expand Up @@ -364,6 +389,57 @@ class ResourceFileSystemTest {
).hasMessage("finding a resource")
}

/** Confirm we can read individual files without triggering indexing. */
@Test
fun testSourceDoesntTriggerIndexing() {
val processedPaths = mutableSetOf<Path>()
val recordingFileSystem = object : ForwardingFileSystem(FileSystem.SYSTEM) {
override fun onPathParameter(path: Path, functionName: String, parameterName: String): Path {
processedPaths += path
return super.onPathParameter(path, functionName, parameterName)
}
}

val zipAPath = ZipBuilder(base)
.addEntry("colors/red.txt", "Apples are red")
.addEntry("META-INF/MANIFEST.MF", "Manifest-Version: 1.0\n")
.build()
val zipBPath = ZipBuilder(base)
.addEntry("colors/blue.txt", "The sky is blue")
.addEntry("META-INF/MANIFEST.MF", "Manifest-Version: 1.0\n")
.build()
val resourceFileSystem = ResourceFileSystem(
classLoader = URLClassLoader(
arrayOf(
zipAPath.toFile().toURI().toURL(),
zipBPath.toFile().toURI().toURL(),
),
null,
),
indexEagerly = false,
systemFileSystem = recordingFileSystem,
)

// Reading paths with source() or read() doesn't index zips.
assertThat(
resourceFileSystem.read("/colors/red.txt".toPath()) { readUtf8() },
).isEqualTo("Apples are red")
assertThat(
resourceFileSystem.read("/colors/blue.txt".toPath()) { readUtf8() },
).isEqualTo("The sky is blue")
assertThat(processedPaths).isEmpty()

// Calling list() does though.
assertThat(resourceFileSystem.list("/colors".toPath())).containsExactlyInAnyOrder(
"/colors/red.txt".toPath(),
"/colors/blue.txt".toPath(),
)
assertThat(processedPaths).containsExactlyInAnyOrder(
zipAPath,
zipBPath,
)
}

/**
* Our resource file system uses [URLClassLoader] internally, which means we need to go back and
* forth between [File], [URL], and [URI] models for component paths. This is a big hazard for
Expand All @@ -382,6 +458,7 @@ class ResourceFileSystemTest {

assertThat(resourceFileSystem.read("hello.txt".toPath()) { readUtf8() })
.isEqualTo("Hello World")
assertThat(resourceFileSystem.metadata("hello.txt".toPath())).isNotNull()
}

@Test
Expand Down

0 comments on commit ac2f1af

Please sign in to comment.