diff --git a/okio-testing-support/src/commonMain/kotlin/okio/AbstractFileSystemTest.kt b/okio-testing-support/src/commonMain/kotlin/okio/AbstractFileSystemTest.kt index 3fbe32156f..8601d66265 100644 --- a/okio-testing-support/src/commonMain/kotlin/okio/AbstractFileSystemTest.kt +++ b/okio-testing-support/src/commonMain/kotlin/okio/AbstractFileSystemTest.kt @@ -47,6 +47,7 @@ abstract class AbstractFileSystemTest( ) { val base: Path = temporaryDirectory / "${this::class.simpleName}-${randomToken(16)}" private val isNodeJsFileSystem = fileSystem::class.simpleName?.startsWith("NodeJs") ?: false + private val isWrappingJimFileSystem = this::class.simpleName?.contains("JimFileSystem") ?: false @BeforeTest fun setUp() { @@ -72,13 +73,17 @@ abstract class AbstractFileSystemTest( val cwdString = cwd.toString() val slash = Path.DIRECTORY_SEPARATOR assertTrue(cwdString) { - cwdString.endsWith("okio${slash}okio") || - cwdString.endsWith("${slash}okio-parent-okio-js-legacy-test") || - cwdString.endsWith("${slash}okio-parent-okio-js-ir-test") || - cwdString.endsWith("${slash}okio-parent-okio-nodefilesystem-js-ir-test") || - cwdString.endsWith("${slash}okio-parent-okio-nodefilesystem-js-legacy-test") || - cwdString.contains("/CoreSimulator/Devices/") || // iOS simulator. - cwdString == "/" // Android emulator. + if (isWrappingJimFileSystem) { + cwdString.endsWith("work") + } else { + cwdString.endsWith("okio${slash}okio") || + cwdString.endsWith("${slash}okio-parent-okio-js-legacy-test") || + cwdString.endsWith("${slash}okio-parent-okio-js-ir-test") || + cwdString.endsWith("${slash}okio-parent-okio-nodefilesystem-js-ir-test") || + cwdString.endsWith("${slash}okio-parent-okio-nodefilesystem-js-legacy-test") || + cwdString.contains("/CoreSimulator/Devices/") || // iOS simulator. + cwdString == "/" // Android emulator. + } } } @@ -90,12 +95,19 @@ abstract class AbstractFileSystemTest( } @Test - fun canonicalizeNoSuchFile() { + fun canonicalizeAbsolutePathNoSuchFile() { assertFailsWith { fileSystem.canonicalize(base / "no-such-file") } } + @Test + fun canonicalizeRelativePathNoSuchFile() { + assertFailsWith { + fileSystem.canonicalize("no-such-file".toPath()) + } + } + @Test fun canonicalizeFollowsSymlinkDirectories() { if (!supportsSymlink()) return @@ -260,6 +272,10 @@ abstract class AbstractFileSystemTest( fileSystem.write("a.txt".toPath()) { writeUtf8("hello, world!") } + } else if (isWrappingJimFileSystem) { + fileSystem.write("a.txt".toPath()) { + writeUtf8("hello, world!") + } } val entries = fileSystem.list(".".toPath()) @@ -319,6 +335,10 @@ abstract class AbstractFileSystemTest( fileSystem.write("a.txt".toPath()) { writeUtf8("hello, world!") } + } else if (isWrappingJimFileSystem) { + fileSystem.write("a.txt".toPath()) { + writeUtf8("hello, world!") + } } val entries = fileSystem.listOrNull(".".toPath()) @@ -2156,7 +2176,7 @@ abstract class AbstractFileSystemTest( } @Test - fun symlinkMetadata() { + fun absoluteSymlinkMetadata() { if (!supportsSymlink()) return val target = base / "symlink-target" @@ -2171,6 +2191,22 @@ abstract class AbstractFileSystemTest( assertInRange(sourceMetadata.createdAt, minTime, maxTime) } + @Test + fun relativeSymlinkMetadata() { + if (!supportsSymlink()) return + + val target = "symlink-target".toPath() + val source = base / "symlink-source" + + val minTime = clock.now() + fileSystem.createSymlink(source, target) + val maxTime = clock.now() + + val sourceMetadata = fileSystem.metadata(source) + assertEquals(target, sourceMetadata.symlinkTarget) + assertInRange(sourceMetadata.createdAt, minTime, maxTime) + } + @Test fun createSymlinkSourceAlreadyExists() { if (!supportsSymlink()) return @@ -2211,6 +2247,7 @@ abstract class AbstractFileSystemTest( @Test fun openSymlinkSink() { if (!supportsSymlink()) return + if (isJimFileSystem()) return val target = base / "symlink-target" val source = base / "symlink-source" @@ -2404,7 +2441,6 @@ abstract class AbstractFileSystemTest( if (windowsLimitations) return false return when (fileSystem::class.simpleName) { "JvmSystemFileSystem", - "NioFileSystemWrappingFileSystem", -> false else -> true } @@ -2488,6 +2524,10 @@ abstract class AbstractFileSystemTest( return windowsLimitations && fileSystem::class.simpleName == "JvmSystemFileSystem" } + private fun isJimFileSystem(): Boolean { + return "JimfsFileSystem" in fileSystem.toString() + } + private fun isNodeJsFileSystemOnWindows(): Boolean { return windowsLimitations && fileSystem::class.simpleName == "NodeJsFileSystem" } diff --git a/okio/src/jvmMain/kotlin/okio/NioFileSystemWrappingFileSystem.kt b/okio/src/jvmMain/kotlin/okio/NioFileSystemWrappingFileSystem.kt index 4131e09eb9..8737f8c8ca 100644 --- a/okio/src/jvmMain/kotlin/okio/NioFileSystemWrappingFileSystem.kt +++ b/okio/src/jvmMain/kotlin/okio/NioFileSystemWrappingFileSystem.kt @@ -17,40 +17,40 @@ package okio import java.io.InterruptedIOException import java.nio.channels.FileChannel -import java.nio.file.FileSystem as JavaNioFileSystem -import java.nio.file.Files +import java.nio.file.FileSystem as NioFileSystem import java.nio.file.NoSuchFileException import java.nio.file.Path as NioPath import java.nio.file.StandardCopyOption import java.nio.file.StandardOpenOption import kotlin.io.path.createDirectory +import kotlin.io.path.createSymbolicLinkPointingTo +import kotlin.io.path.deleteExisting import kotlin.io.path.exists import kotlin.io.path.inputStream -import kotlin.io.path.isSymbolicLink import kotlin.io.path.listDirectoryEntries +import kotlin.io.path.moveTo import kotlin.io.path.outputStream -import kotlin.io.path.readSymbolicLink import okio.Path.Companion.toOkioPath /** * A file system that wraps a `java.nio.file.FileSystem` and executes all operations in the context of the wrapped file * system. */ -internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSystem) : NioSystemFileSystem() { - // TODO(Benoit) How do deal with multiple directories? Test with Windows someday. - private val delegateRoot = javaNioFileSystem.rootDirectories.first() - +internal class NioFileSystemWrappingFileSystem(private val nioFileSystem: NioFileSystem) : NioSystemFileSystem() { /** - * On a `java.nio.file.FileSystem`, `java.nio.file.Path` are stateful and hold a reference to the file system they - * got provided from. We need to [resolve][java.nio.file.Path.resolve] all okio paths before doing operations on the - * nio file system in order for things to work properly. + * On a [java.nio.file.FileSystem], paths are stateful and hold a reference to the file system they got provided from. + * Using [getPath][NioFileSystem.getPath], we ask [nioFileSystem] to wrap the [Path]'s value in order to set itself as + * its provider which is needed for operations on the nio file system to work properly. */ - private fun Path.resolve(readSymlink: Boolean = false): NioPath { - val resolved = delegateRoot.resolve(toString()) - return if (readSymlink && resolved.isSymbolicLink()) { - resolved.readSymbolicLink() - } else { - resolved + private fun Path.resolve(): NioPath { + return nioFileSystem.getPath(toString()) + } + + override fun canonicalize(path: Path): Path { + try { + return path.resolve().toRealPath().toOkioPath() + } catch (e: NoSuchFileException) { + throw FileNotFoundException("no such file: $path") } } @@ -74,17 +74,7 @@ internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSys return null } } - val result = entries.mapTo(mutableListOf()) { entry -> - // TODO(Benoit) This whole block can surely be improved. - val path = dir / entry.toOkioPath() - if (dir.isRelative) { - // All `entries` are absolute and resolving them against `dir` won't have any effect. We need to manually - // relativize them back. - nioDir.relativize(path.resolve()).toOkioPath() - } else { - path - } - } + val result = entries.mapTo(mutableListOf()) { entry -> dir / entry.toString() } result.sort() return result } @@ -120,7 +110,7 @@ internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSys override fun source(file: Path): Source { try { - return file.resolve(readSymlink = true).inputStream().source() + return file.resolve().inputStream().source() } catch (e: NoSuchFileException) { throw FileNotFoundException("no such file: $file") } @@ -131,7 +121,7 @@ internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSys if (mustCreate) add(StandardOpenOption.CREATE_NEW) } try { - return file.resolve(readSymlink = true) + return file.resolve() .outputStream(*openOptions.toTypedArray()) .sink() } catch (e: NoSuchFileException) { @@ -166,8 +156,7 @@ internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSys // Note that `java.nio.file.FileSystem` allows atomic moves of a file even if the target is an existing directory. override fun atomicMove(source: Path, target: Path) { try { - Files.move( - source.resolve(), + source.resolve().moveTo( target.resolve(), StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING, @@ -186,7 +175,7 @@ internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSys } val nioPath = path.resolve() try { - Files.delete(nioPath) + nioPath.deleteExisting() } catch (e: NoSuchFileException) { if (mustExist) throw FileNotFoundException("no such file: $path") } catch (e: IOException) { @@ -195,8 +184,8 @@ internal class NioFileSystemWrappingFileSystem(javaNioFileSystem: JavaNioFileSys } override fun createSymlink(source: Path, target: Path) { - Files.createSymbolicLink(source.resolve(), target.resolve()) + source.resolve().createSymbolicLinkPointingTo(target.resolve()) } - override fun toString(): String = "NioFileSystemWrappingFileSystem" + override fun toString() = nioFileSystem::class.simpleName!! } diff --git a/okio/src/jvmTest/kotlin/okio/FileHandleFileSystemTest.kt b/okio/src/jvmTest/kotlin/okio/FileHandleFileSystemTest.kt index 06baf8a060..021ffb4cbb 100644 --- a/okio/src/jvmTest/kotlin/okio/FileHandleFileSystemTest.kt +++ b/okio/src/jvmTest/kotlin/okio/FileHandleFileSystemTest.kt @@ -19,6 +19,7 @@ import com.google.common.jimfs.Configuration import com.google.common.jimfs.Jimfs import java.nio.file.FileSystems import kotlinx.datetime.Clock +import okio.FileHandleFileSystemTest.FileHandleTestingFileSystem import okio.FileSystem.Companion.asOkioFileSystem /** @@ -59,15 +60,17 @@ class FileHandleFileSystemTest : AbstractFileSystemTest( } } -class FileHandleNioFileSystemWrapperFileSystemTest : AbstractFileSystemTest( +class FileHandleNioJimFileSystemWrapperFileSystemTest : AbstractFileSystemTest( clock = Clock.System, - fileSystem = Jimfs - .newFileSystem( - when (Path.DIRECTORY_SEPARATOR == "\\") { - true -> Configuration.windows() - false -> Configuration.unix() - }, - ).asOkioFileSystem(), + fileSystem = FileHandleTestingFileSystem( + Jimfs + .newFileSystem( + when (Path.DIRECTORY_SEPARATOR == "\\") { + true -> Configuration.windows() + false -> Configuration.unix() + }, + ).asOkioFileSystem(), + ), windowsLimitations = false, allowClobberingEmptyDirectories = true, allowAtomicMoveFromFileToDirectory = true, @@ -76,7 +79,9 @@ class FileHandleNioFileSystemWrapperFileSystemTest : AbstractFileSystemTest( class FileHandleNioDefaultFileSystemWrapperFileSystemTest : AbstractFileSystemTest( clock = Clock.System, - fileSystem = FileSystems.getDefault().asOkioFileSystem(), + fileSystem = FileHandleTestingFileSystem( + FileSystems.getDefault().asOkioFileSystem(), + ), windowsLimitations = false, allowClobberingEmptyDirectories = Path.DIRECTORY_SEPARATOR == "\\", allowAtomicMoveFromFileToDirectory = false, diff --git a/okio/src/jvmTest/kotlin/okio/JvmSystemFileSystemTest.kt b/okio/src/jvmTest/kotlin/okio/JvmSystemFileSystemTest.kt index 1463727c0c..fc4b7c0910 100644 --- a/okio/src/jvmTest/kotlin/okio/JvmSystemFileSystemTest.kt +++ b/okio/src/jvmTest/kotlin/okio/JvmSystemFileSystemTest.kt @@ -18,6 +18,7 @@ package okio import com.google.common.jimfs.Configuration import com.google.common.jimfs.Jimfs import java.io.InterruptedIOException +import java.nio.file.FileSystems import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.fail @@ -59,7 +60,7 @@ class JvmSystemFileSystemTest : AbstractFileSystemTest( } } -class NioFileSystemWrappingFileSystemTest : AbstractFileSystemTest( +class NioJimFileSystemWrappingFileSystemTest : AbstractFileSystemTest( clock = Clock.System, fileSystem = Jimfs .newFileSystem( @@ -73,3 +74,13 @@ class NioFileSystemWrappingFileSystemTest : AbstractFileSystemTest( allowAtomicMoveFromFileToDirectory = true, temporaryDirectory = FileSystem.SYSTEM_TEMPORARY_DIRECTORY, ) + +class NioDefaultFileSystemWrappingFileSystemTest : AbstractFileSystemTest( + clock = Clock.System, + fileSystem = FileSystems.getDefault().asOkioFileSystem(), + windowsLimitations = false, + allowClobberingEmptyDirectories = Path.DIRECTORY_SEPARATOR == "\\", + allowAtomicMoveFromFileToDirectory = false, + allowRenameWhenTargetIsOpen = Path.DIRECTORY_SEPARATOR != "\\", + temporaryDirectory = FileSystem.SYSTEM_TEMPORARY_DIRECTORY, +)