Skip to content

Commit

Permalink
Merge pull request square#864 from square/jwilson.1230.fnfe
Browse files Browse the repository at this point in the history
Specialize FileNotFoundException
  • Loading branch information
swankjesse authored Jan 2, 2021
2 parents 7027351 + 1381fcc commit 6f98625
Show file tree
Hide file tree
Showing 18 changed files with 112 additions and 53 deletions.
16 changes: 9 additions & 7 deletions okio-testing/src/commonMain/kotlin/okio/FakeFilesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class FakeFilesystem(
val canonicalPath = workingDirectory / path

if (canonicalPath !in elements) {
throw IOException("no such file: $path")
throw FileNotFoundException("no such file: $path")
}

return canonicalPath
Expand Down Expand Up @@ -138,7 +138,7 @@ class FakeFilesystem(

override fun source(file: Path): Source {
val canonicalPath = workingDirectory / file
val element = elements[canonicalPath] ?: throw IOException("no such file: $file")
val element = elements[canonicalPath] ?: throw FileNotFoundException("no such file: $file")

if (element !is File) {
throw IOException("not a file: $file")
Expand Down Expand Up @@ -221,7 +221,7 @@ class FakeFilesystem(
}

val removed = elements.remove(canonicalSource)
?: throw IOException("source doesn't exist: $source")
?: throw FileNotFoundException("source doesn't exist: $source")
elements[canonicalTarget] = removed
}

Expand All @@ -239,7 +239,7 @@ class FakeFilesystem(
}

if (elements.remove(canonicalPath) == null) {
throw IOException("no such file: $path")
throw FileNotFoundException("no such file: $path")
}
}

Expand All @@ -256,14 +256,16 @@ class FakeFilesystem(
val element = elements[path]
if (element is Directory) return element

// If the path is a root, create it on demand.
if (element == null && path.isRoot) {
// If the path is a root, create a directory for it on demand.
if (path.isRoot) {
val root = Directory(createdAt = clock.now())
elements[path] = root
return root
}

throw IOException("path is not a directory: $path")
if (element == null) throw FileNotFoundException("no such directory: $path")

throw IOException("not a directory: $path")
}

private sealed class Element(
Expand Down
2 changes: 1 addition & 1 deletion okio/src/appleMain/kotlin/okio/posixVariant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal actual fun PosixSystemFilesystem.variantMetadataOrNull(path: Path): Fil
val stat = alloc<stat>()
if (platform.posix.lstat(path.toString(), stat.ptr) != 0) {
if (errno == ENOENT) return null
throw IOException(errnoString(errno))
throw errnoToIOException(errno)
}
return@memScoped FileMetadata(
isRegularFile = stat.st_mode.toInt() and S_IFMT == S_IFREG,
Expand Down
2 changes: 2 additions & 0 deletions okio/src/commonMain/kotlin/okio/-Platform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,5 @@ expect open class IOException(message: String?, cause: Throwable?) : Exception {
}

expect open class EOFException(message: String? = null) : IOException

expect class FileNotFoundException(message: String? = null) : IOException
2 changes: 1 addition & 1 deletion okio/src/commonMain/kotlin/okio/Filesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ abstract class Filesystem {
*/
@Throws(IOException::class)
fun metadata(path: Path): FileMetadata {
return metadataOrNull(path) ?: throw IOException("no such file: $path")
return metadataOrNull(path) ?: throw FileNotFoundException("no such file: $path")
}

/**
Expand Down
36 changes: 26 additions & 10 deletions okio/src/commonTest/kotlin/okio/AbstractFilesystemTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ abstract class AbstractFilesystemTest(

@Test
fun canonicalizeNoSuchFile() {
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.canonicalize(base / "no-such-file")
}
}
Expand All @@ -78,14 +78,23 @@ abstract class AbstractFilesystemTest(

@Test
fun listNoSuchDirectory() {
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.list(base / "no-such-directory")
}
}

@Test
fun fileSourceNoSuchDirectory() {
fun listFile() {
val target = base / "list"
target.writeUtf8("hello, world!")
assertFailsWith<IOException> {
filesystem.list(target)
}
}

@Test
fun fileSourceNoSuchDirectory() {
assertFailsWith<FileNotFoundException> {
filesystem.source(base / "no-such-directory" / "file")
}
}
Expand Down Expand Up @@ -166,7 +175,7 @@ abstract class AbstractFilesystemTest(

@Test
fun fileSinkNoSuchDirectory() {
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.sink(base / "no-such-directory" / "file")
}
}
Expand Down Expand Up @@ -275,7 +284,7 @@ abstract class AbstractFilesystemTest(
fun atomicMoveSourceDoesNotExist() {
val source = base / "source"
val target = base / "target"
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.atomicMove(source, target)
}
}
Expand Down Expand Up @@ -317,7 +326,7 @@ abstract class AbstractFilesystemTest(
fun copySourceDoesNotExist() {
val source = base / "source"
val target = base / "target"
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.copy(source, target)
}
assertFalse(target in filesystem.list(base))
Expand Down Expand Up @@ -353,8 +362,15 @@ abstract class AbstractFilesystemTest(
@Test
fun deleteFailsOnNoSuchFile() {
val path = base / "no-such-file"
assertFailsWith<IOException> {
filesystem.delete(path)
// TODO(jwilson): fix Windows to throw FileNotFoundException on deleting an absent file.
if (windowsLimitations) {
assertFailsWith<IOException> {
filesystem.delete(path)
}
} else {
assertFailsWith<FileNotFoundException> {
filesystem.delete(path)
}
}
}

Expand Down Expand Up @@ -387,7 +403,7 @@ abstract class AbstractFilesystemTest(
@Test
fun deleteRecursivelyFailsOnNoSuchFile() {
val path = base / "no-such-file"
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.deleteRecursively(path)
}
}
Expand Down Expand Up @@ -460,7 +476,7 @@ abstract class AbstractFilesystemTest(
@Test
fun absentMetadata() {
val path = base / "no-such-file"
assertFailsWith<IOException> {
assertFailsWith<FileNotFoundException> {
filesystem.metadata(path)
}
}
Expand Down
23 changes: 15 additions & 8 deletions okio/src/jsMain/kotlin/okio/NodeJsSystemFilesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ internal object NodeJsSystemFilesystem : Filesystem() {
val canonicalPath = realpathSync(path.toString())
return canonicalPath.toString().toPath()
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

Expand Down Expand Up @@ -81,7 +81,7 @@ internal object NodeJsSystemFilesystem : Filesystem() {
opendir.closeSync()
}
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

Expand All @@ -90,7 +90,7 @@ internal object NodeJsSystemFilesystem : Filesystem() {
val fd = openSync(file.toString(), flags = "r")
return FileSource(fd)
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

Expand All @@ -99,7 +99,7 @@ internal object NodeJsSystemFilesystem : Filesystem() {
val fd = openSync(file.toString(), flags = "w")
return FileSink(fd)
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

Expand All @@ -108,23 +108,23 @@ internal object NodeJsSystemFilesystem : Filesystem() {
val fd = openSync(file.toString(), flags = "a")
return FileSink(fd)
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

override fun createDirectory(dir: Path) {
try {
mkdirSync(dir.toString())
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

override fun atomicMove(source: Path, target: Path) {
try {
renameSync(source.toString(), target.toString())
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

Expand All @@ -143,7 +143,14 @@ internal object NodeJsSystemFilesystem : Filesystem() {
try {
rmdirSync(path.toString())
} catch (e: Throwable) {
throw IOException(e.message)
throw e.toIOException()
}
}

private fun Throwable.toIOException(): IOException {
return when (errorCode) {
"ENOENT" -> FileNotFoundException(message)
else -> IOException(message)
}
}

Expand Down
2 changes: 2 additions & 0 deletions okio/src/jvmMain/kotlin/okio/-Platform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,5 @@ internal actual inline fun <R> synchronized(lock: Any, block: () -> R): R {
actual typealias IOException = java.io.IOException

actual typealias EOFException = java.io.EOFException

actual typealias FileNotFoundException = java.io.FileNotFoundException
17 changes: 13 additions & 4 deletions okio/src/jvmMain/kotlin/okio/JvmSystemFilesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ package okio
internal open class JvmSystemFilesystem : Filesystem() {
override fun canonicalize(path: Path): Path {
val canonicalFile = path.toFile().canonicalFile
if (!canonicalFile.exists()) throw IOException("no such file")
if (!canonicalFile.exists()) throw FileNotFoundException("no such file")
return canonicalFile.toOkioPath()
}

Expand Down Expand Up @@ -56,7 +56,12 @@ internal open class JvmSystemFilesystem : Filesystem() {
}

override fun list(dir: Path): List<Path> {
val entries = dir.toFile().list() ?: throw IOException("failed to list $dir")
val file = dir.toFile()
val entries = file.list()
if (entries == null) {
if (!file.exists()) throw FileNotFoundException("no such file $dir")
throw IOException("failed to list $dir")
}
return entries.map { dir / it }
}

Expand All @@ -82,8 +87,12 @@ internal open class JvmSystemFilesystem : Filesystem() {
}

override fun delete(path: Path) {
val deleted = path.toFile().delete()
if (!deleted) throw IOException("failed to delete $path")
val file = path.toFile()
val deleted = file.delete()
if (!deleted) {
if (!file.exists()) throw FileNotFoundException("no such file $path")
else throw IOException("failed to delete $path")
}
}

override fun toString() = "JvmSystemFilesystem"
Expand Down
2 changes: 2 additions & 0 deletions okio/src/jvmMain/kotlin/okio/NioSystemFilesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ internal class NioSystemFilesystem : JvmSystemFilesystem() {
override fun atomicMove(source: Path, target: Path) {
try {
Files.move(source.toNioPath(), target.toNioPath(), ATOMIC_MOVE, REPLACE_EXISTING)
} catch (e: NoSuchFileException) {
throw FileNotFoundException(e.message)
} catch (e: UnsupportedOperationException) {
throw IOException("atomic move not supported")
}
Expand Down
2 changes: 1 addition & 1 deletion okio/src/linuxX64Main/kotlin/okio/posixVariant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal actual fun PosixSystemFilesystem.variantMetadataOrNull(path: Path): Fil
val stat = alloc<stat>()
if (platform.posix.lstat(path.toString(), stat.ptr) != 0) {
if (errno == ENOENT) return null
throw IOException(errnoString(errno))
throw errnoToIOException(errno)
}
return@memScoped FileMetadata(
isRegularFile = stat.st_mode.toInt() and S_IFMT == S_IFREG,
Expand Down
12 changes: 7 additions & 5 deletions okio/src/mingwX64Main/kotlin/okio/posixVariant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal actual fun PosixSystemFilesystem.variantDelete(path: Path) {
if (rmdir(pathString) == 0) return
}

throw IOException(errnoString(EACCES))
throw errnoToIOException(EACCES)
}

@ExperimentalFilesystem
Expand All @@ -60,10 +60,12 @@ internal actual fun PosixSystemFilesystem.variantMkdir(dir: Path): Int {
internal actual fun PosixSystemFilesystem.variantCanonicalize(path: Path): Path {
// Note that _fullpath() returns normally if the file doesn't exist.
val fullpath = _fullpath(null, path.toString(), PATH_MAX)
?: throw IOException(errnoString(errno))
?: throw errnoToIOException(errno)
try {
val pathString = Buffer().writeNullTerminated(fullpath).readUtf8()
if (platform.posix.access(pathString, 0) != 0 && errno == ENOENT) throw IOException("no such file")
if (platform.posix.access(pathString, 0) != 0 && errno == ENOENT) {
throw FileNotFoundException("no such file")
}
return pathString.toPath()
} finally {
free(fullpath)
Expand All @@ -76,7 +78,7 @@ internal actual fun PosixSystemFilesystem.variantMetadataOrNull(path: Path): Fil
val stat = alloc<_stat64>()
if (_stat64(path.toString(), stat.ptr) != 0) {
if (errno == ENOENT) return null
throw IOException(errnoString(errno))
throw errnoToIOException(errno)
}
return@memScoped FileMetadata(
isRegularFile = stat.st_mode.toInt() and S_IFMT == S_IFREG,
Expand All @@ -92,6 +94,6 @@ internal actual fun PosixSystemFilesystem.variantMetadataOrNull(path: Path): Fil
@ExperimentalFilesystem
internal actual fun PosixSystemFilesystem.variantMove(source: Path, target: Path) {
if (MoveFileExA(source.toString(), target.toString(), MOVEFILE_REPLACE_EXISTING) == 0) {
throw IOException(lastErrorString())
throw lastErrorToIOException()
}
}
12 changes: 11 additions & 1 deletion okio/src/mingwX64Main/kotlin/okio/windows.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,25 @@ package okio
import kotlinx.cinterop.ByteVarOf
import kotlinx.cinterop.allocArray
import kotlinx.cinterop.memScoped
import platform.windows.DWORD
import platform.windows.ERROR_FILE_NOT_FOUND
import platform.windows.ERROR_PATH_NOT_FOUND
import platform.windows.FORMAT_MESSAGE_FROM_SYSTEM
import platform.windows.FORMAT_MESSAGE_IGNORE_INSERTS
import platform.windows.FormatMessageA
import platform.windows.GetLastError
import platform.windows.LANG_NEUTRAL
import platform.windows.SUBLANG_DEFAULT

internal fun lastErrorString(): String {
internal fun lastErrorToIOException(): IOException {
val lastError = GetLastError()
return when (lastError.toInt()) {
ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND -> FileNotFoundException(lastErrorString(lastError))
else -> IOException(lastErrorString(lastError))
}
}

internal fun lastErrorString(lastError: DWORD): String {
memScoped {
val messageMaxSize = 2048
val message = allocArray<ByteVarOf<Byte>>(messageMaxSize)
Expand Down
Loading

0 comments on commit 6f98625

Please sign in to comment.