Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension mechanism for FileSystem #1466

Open
swankjesse opened this issue Apr 5, 2024 · 4 comments · May be fixed by #1470
Open

Extension mechanism for FileSystem #1466

swankjesse opened this issue Apr 5, 2024 · 4 comments · May be fixed by #1470

Comments

@swankjesse
Copy link
Collaborator

There’s a bunch of FileSystem features that we don’t implement:

  • ACLs & Permissions
  • Locks
  • Watches
  • Volumes & Capacity

We should make it possible to get at extended features like these, without necessarily having all of these features in the core API.

Users could add an interface like this:

interface FileLocker {
  fun tryAcquire(path: Path): FileLock?

  interface FileLock : Closeable
}

You’d use it like this:

val fileSystem: FileSystem = ...

val locker = fileSystem.extension<FileLocker>()
val lock = locker.tryAcquire(path)
...

Finally you’d attach extensions in a way similar to CoroutineContext:

val fileLocker = MyFileLocker()
val fileSystemWithLocker = FileSystem.SYSTEM + fileLocker
@swankjesse
Copy link
Collaborator Author

It’d also be nice to use this to simplify testing for libraries like SQLite that go direct to the filesystem by default.

val database = fileSystem.openDatabase("data.sqlite".toPath())
fun FileSystem.openDatabase(path: Path): SqliteDatabase {
  extension<SqliteExtension>().open(path)
}

interface SqliteExtension {
  fun open(path: Path): SqliteDatabase
}

/**
 * Returns a file system that includes the extension.
 * 
 * @param inMemory true to return database instances that aren’t durable on disk; appropriate
 *    for use with FakeFileSystem. Otherwise the
 *    databases are written to FileSystem.SYSTEM.
 */
fun applySqlite(
  fileSystem: FileSystem,
  inMemory: Boolean
): FileSystem

swankjesse pushed a commit that referenced this issue Apr 15, 2024
@swankjesse swankjesse linked a pull request Apr 15, 2024 that will close this issue
@swankjesse
Copy link
Collaborator Author

I think we need more stuff to support the chroot use case, where one FileSystem maps paths upon another.

Perhaps something like this?

interface FileSystemExtension {
  fun interface Factory<E : FileSystemExtension> {
    fun create(host: FileSystemExtension.Host): E
  }

  interface Host {
    fun onPathParameter(path: Path, functionName: String, parameterName: String): Path
    fun onPathResult(path: Path, functionName: String): Path
  }
}
fun <E : FileSystemExtension> FileSystem.extend(
  factory: FileSystemExtension.Factory<E>,
): FileSystem

This makes it a bit more annoying to create extensions, but it gives them the stuff they need to do 1:1 path mapping.

Also getting the implementation right is tricky, cause we need to potentially apply multiple layers of mappings.

@swankjesse
Copy link
Collaborator Author

Yep, implementation is beyond tricky; it’s impossible. I can’t create a single instance of an extension because it could need different path transforms depending on which wrapped FileSystem returned it.

@swankjesse
Copy link
Collaborator Author

Some updates on extensions after a discussion with @dellisd ...

  1. It might not be worth the effort to build a fully-capable SQLDelight extension targeting Android. Android’s Context APIs encapsulate the paths to the DBs, and it’s likely simpler to accept this design than to try to get Android apps to load their databases from a FileSystem.

  2. Our Mapper should be prepared for a more sophisticated mapping than what ForwardingFileSystem does. Consider this:

    val fileSystem = FileSystemBuilder()
      .add(
        "/cache".toPath(),
        FileSystem.SYSTEM,
        "/tmp/cache".toPath(),
      )
      .add(
        "/downloads/movies".toPath(),
        FileSystem.SYSTEM,
        "/Volumes/media/movies".toPath(),
        writable = false,
      )
      .add(
        "/downloads".toPath(),
        FileSystem.SYSTEM,
        "/Users/jwilson/Downloads".toPath(),
      )
      .add(
        "/builds".toPath(),
        FakeFileSystem(),
        "/".toPath(),
      )
      .build()

In this listing we target multiple underlying FileSystem which seems quite reasonable. I also added writable = false as a wrinkle to consider that we might reduce capabilities on a mapped FileSystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants