Skip to content

Commit

Permalink
Merge pull request square#1275 from square/bquenaudon.2023-06-19.symlink
Browse files Browse the repository at this point in the history 
Support symlink for NioFSWrappingFS
  • Loading branch information
@oldergod
oldergod authored Jun 21, 2023
2 parents 06317a3 + a681001 commit bc00d92
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 55 deletions.
60 changes: 50 additions & 10 deletions okio-testing-support/src/commonMain/kotlin/okio/AbstractFileSystemTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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.
}
}
}

Expand All @@ -90,12 +95,19 @@ abstract class AbstractFileSystemTest(
}

@Test
fun canonicalizeNoSuchFile() {
fun canonicalizeAbsolutePathNoSuchFile() {
assertFailsWith<FileNotFoundException> {
fileSystem.canonicalize(base / "no-such-file")
}
}

@Test
fun canonicalizeRelativePathNoSuchFile() {
assertFailsWith<FileNotFoundException> {
fileSystem.canonicalize("no-such-file".toPath())
}
}

@Test
fun canonicalizeFollowsSymlinkDirectories() {
if (!supportsSymlink()) return
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -2156,7 +2176,7 @@ abstract class AbstractFileSystemTest(
}

@Test
fun symlinkMetadata() {
fun absoluteSymlinkMetadata() {
if (!supportsSymlink()) return

val target = base / "symlink-target"
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -2404,7 +2441,6 @@ abstract class AbstractFileSystemTest(
if (windowsLimitations) return false
return when (fileSystem::class.simpleName) {
"JvmSystemFileSystem",
"NioFileSystemWrappingFileSystem",
-> false
else -> true
}
Expand Down Expand Up @@ -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"
}
Expand Down
59 changes: 24 additions & 35 deletions okio/src/jvmMain/kotlin/okio/NioFileSystemWrappingFileSystem.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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!!
}
23 changes: 14 additions & 9 deletions okio/src/jvmTest/kotlin/okio/FileHandleFileSystemTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
13 changes: 12 additions & 1 deletion okio/src/jvmTest/kotlin/okio/JvmSystemFileSystemTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -59,7 +60,7 @@ class JvmSystemFileSystemTest : AbstractFileSystemTest(
}
}

class NioFileSystemWrappingFileSystemTest : AbstractFileSystemTest(
class NioJimFileSystemWrappingFileSystemTest : AbstractFileSystemTest(
clock = Clock.System,
fileSystem = Jimfs
.newFileSystem(
Expand All @@ -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,
)

0 comments on commit bc00d92

Please sign in to comment.