https://kotlinlang.org logo
Join Slack
Powered by
Are these acceptable implementations of `Path.canR...
# squarelibraries
m
Are these acceptable implementations of 
Path.canRead()
and 
Path.canWrite()
when using okio? (note: some extension functions like 
Path.isDirectory()
are not shown for brevity (see thread).
Copy code
fun Path.canRead(): Boolean {
    return try {
        if (this.isDirectory()) {
            this.listFiles()
        } else {
            FileSystem.SYSTEM.source(this).close() // Attempt to open for a file
        }
        true
    } catch (e: Exception) {
        println("path not readable: $this")
        false
    }
}

fun Path.canWrite(): Boolean {
    return try {
        if (this.isDirectory()) {
            val tempFile = this / currentEpochMillis().toString()
            try {
                FileSystem.SYSTEM.sink(tempFile).close()
            } finally {
                tempFile.delete() // Cleanup after test
            }
        } else {
            FileSystem.SYSTEM.sink(this).close()
        }
        true
    } catch (e: Exception) {
        println("path not writable: $this")
        false
    }
}
j
I don’t like any function that hides FileSystem.SYSTEM on you
The second problem is that these functions are racy, so much so as it’s not worth calling them
Much simpler is to attempt to read and recover if that fails
m
Ok thanks
The motivation here is that, on Android, I have some startup logic that attempts to work out where to put various files. Sometimes `filesDir`or 
externalFilesDir
can’t be used for some reason, so it determines that before proceeding with unpacking zips etc. So, as you suggest, I could just proceed with the unpacking first, and then should a problem arise, fallback to other install locations.
And just to be clear, I’m not calling these methods before attempting a read/write etc. Apart from when inspecting the file system (mentioned earlier) I also use it for when reporting issues from the end user. How about if the 
FileSystem
was passed in: 
fun File.canRead(fs: FileSystem): Boolean
?
j
Sure
If you make a decision about where to write files, the operation could still fail due to inadequate free space
The only true way to know if an operation will succeed is to attempt it
m
yes, exception handling is still necessary, for sure. When deciding where to unpack zips, things like canWrite() and space availability on each candidate are all taken into account. That’s not to say that space will never be an issue. So it makes sense to minimize the chances of running out of space before proceeding. Why unpack 100s of MBs into filesDir() when you can determine upfront that it’s never going to succeed?
j
I don’t think it works to ask the file system if a write will succeed without attempting it. Seemingly simple things like “how many bytes are free” get difficult because the file system might be compressed, have quotas, do copy-on-write, optimize small files, allocate fixed-size blocks, etc
You can do what you like here, but it’s worth considering how many bytes of storage an empty file consumes on the disk
m
All good food for thought, much appreciated 🙏
Rather unrelated but… when accessing an Android asset using 
AssetManager.ACCESS_RANDOM
presumably it doesn’t make sense to use 
Source.buffer()
? I see there’s no 
RandomAccessSource
and so the only way I can see is to use a 
BufferedSource
with skip?
j
Consider using 
AssetManager.asFileSystem()
with the 
okio-assetfilesystem
dep https://github.com/square/okio/tree/master/okio-assetfilesystem
m
Thanks Jesse that looks like a really neat solution, although looking at the implementation, 
accessMode
is just always being used as default (
AssetManager.ACCESS_STREAMING
). Wouldn’t 
AssetManager.ACCESS_RANDOM
be useful for accessing individual files within a large zip file stored in assets, for example?
j
I don’t expect it to matter in practice? I guess I’d have to read the implementation code to see what kinds of decisions those modes end up impacting
m
I ran some very unscientific tests on my real-world app, and it doesn’t seem to provide any benefit on my Pixel 7a device (zip files in assets are up to 10MB containing a binary item and manifest XML item). Maybe older devices are different, I don’t know yet.
BTW, have you thought about having a 
FileSystem.rootedAt(directory: Path): FileSystem
extension function, which essentially just maps all incoming and outgoing paths? Often, when processing files in a directory (possibly with nested directories), you don’t care where it is located in a filesystem. Otherwise you have to pass around both the directory and 
FileSystem
.
j
Doing file path mapping is something I wanna implement. It’s a bit tricky to do securely due to symlinks
(If we implement something like chroot, it shouldn’t be possible to escape the jail)
👍 1
m
Maybe there’s also value in being able to escape the jail. I remember back in the Amiga days, some tool that allowed a ZIP file to be mounted as a directory (not just a volume), so had a parent etc. Maybe it was even possible that all ZIP files were auto-mounted like this, can’t remember.
So you could imagine a 
FileSystem
that auto-resolves ZIP files within it.
Also I wonder about a DSL:
Copy code
buildDirectory(rootDirectory) { // or maybe buildFileSystem()
    file("foo.txt") { sink ->
        ...
    }
    directory("bar") {
        file("baz.txt") { sink ->
            ...
        }
        zipFile("qux.zip") { zipOutputStream ->

        }
    }
}
j
👍🏻
I sketched out something like this to write a 
.zip
file https://github.com/square/okio/issues/1442#issuecomment-1962387496
m
Nice, I guess it could all be combined:
Copy code
buildDirectory(rootDirectory) { // or maybe buildFileSystem()
    file("foo.txt") { sink ->
        ...
    }
    directory("bar") {
        file("baz.txt") { sink ->
            ...
        }
        zipFile("qux.zip") { zipOutputStream ->
            file("foo2.txt") { sink ->
                ...
            }
            directory("bar2") {
                file("baz2.txt") { sink ->
                    ...
                }
            }
        }
    }
}
One thing I’ve noticed with multiple filesystems (e.g. extracting a zip file in assets involves three different filesystems) is how easy it is to accidentally use the wrong 
FileSystem
with a 
Path
. Have you looked into having a class that wraps that pair? Perhaps:
Copy code
class UniversalPath(val path: Path, private val fileSystem: FileSystem) {
    fun exists(): Boolean ...
    fun listPaths(): List<UniversalPath> ...
    fun incarcerate(): FileSystem // naming subject to change
}
j
Absolutely not!
I’ve got strong opinions here
I guess you could consider a Path+FileSystem as a FileService
But at that point you’re probably better off also folding in the encoder and decoder for the file’s contents
m
Well yes, I was thinking of 
UniversalPath
more as an extension of 
FileSystem
than 
Path
so in that sense, a service. Maybe it’s a naming problem. 
RootedFileSystem
instead, perhaps. I’m not seeing a huge difference between a 
FileSystem
that exposes the contents of a zip, with a 
FileSystem
(or 
UniversalPath
) that exposes the contents of a directory represented by a 
Path
. Going the value object way, you could have a 
Path
+ 
FileSystemIdentifier
. Where the latter could be encoded in the value string: 
assets:foo/bar.zip:qux/baz.xml
though I suppose this could end up conflicting with some platform-specific paths.
j
I think the abstraction I want there is content-specific.
Copy code
interface ScreenshotStore {
  fun write(id: ScreenshotId, content: Screenshot)
  fun list(): List<ScreenshotId>
  fun read(id: ScreenshotId): Screenshot
}
Ie. don’t abstract further over FileSystems, instead abstract over what you’re using the file system for
m
Yes, that makes sense anyway. It’s easy to let `Path`s and `FileSystem`s get into parts of application code where they don’t belong. I suppose, in this case, the implementation of 
ScreenshotStore
would be relatively trivial using the existing APIs, although you probably would still have implementations that have to deal with the three-filesystem situation mentioned earlier. No biggie I guess.
👍🏻 1
I suppose the reason there is no support for Android’s 
DocumentFile
: https://developer.android.com/reference/androidx/documentfile/provider/DocumentFile (which is more along the lines of 
UniversalPath
mentioned earlier) is because that is a more application-focussed interface. i.e. something that could be directly used in the implementation of 
ScreenshotStore
whereas it is not suitable to implement a 
FileSystem
because it only works on a given file/directory and so is not easily used to give results for an arbitrary 
Path
. Now I’m wondering how best to migrate away from 
DocumentFile
to a KMP alternative. At the moment, it looks like I need to write it to application cache storage and then use okio on that.
j
Hmm… repo is archived!
😞 1
m
I wonder what is the proposed solution for how an app handles an incoming content uri that represents a ZIP file. In Android, you would 
openInputStream()
and then use 
ZipInputStream
to inspect the contents to see if it’s a ZIP file supported by the app. That sort of inspection isn’t really possible with okio (unless I’ve missed something) because 
FileSystem.openZip()
works with a 
Path
not a 
Source
. So that means having to first write the 
Source
to local cache just so that we have a 
Path
.
j
Agreed
Similarly you can’t resolve https:// URLs with the FileSystem API. Both are out of scope.
It’s 90% focused on being a simple, testable, multiplatform API for local file I/O. Reading zips and resources as a read-only file system is a secondary feature cause it’s a pretty natural way to access that stuff
m
Reading a Source of a zip file as a read-only file system would also fit within the scope, no?
j
Yes
But it’s deliberately not doing other things that could map to file systems.
If you wanted to use this to do FTP you could, but it wouldn’t expose features like timeouts appropriately
We don’t include latency / distributed systems capabilities, which is either a limitation or a simplification, depending on what you wanna do
We also don’t even bother with ACLs
More details here on this tracking bug: https://github.com/square/okio/issues/1466
m
The sqlite database thing there reminded me of this feature request to open a read-only database from assets: https://issuetracker.google.com/issues/134928363
6 Views
Open in Slack
PreviousNext
Powered by