Runtime: IFileInfo async methods

[synercoder]

Background and Motivation

I am currently working on a hobby (web) project, and for testing I have a bunch of images I server stashed in a folder. The images are served using:

app.UseStaticFiles(new StaticFileOptions()
{
    RequestPath = "/images",
    FileProvider = new PhysicalFileProvider(@"C:\path\to\my\images\folder")
});

This works fine of course, but I would like to have the images served from the cloud if I ever put this project on the web.

To this end, I wanted to see if I could replace the PhysicalFileProvider with a custom creation of my own, something in the line of: AzureBlobFileProvider. So I dived into IFileProvider and saw that it only had synchronous methods.

Proposed API

namespace Microsoft.Extensions.FileProviders
{
    public interface IFileProvider
    {
-       IDirectoryContents GetDirectoryContents(string subpath);
+       Task<IDirectoryContents> GetDirectoryContentsAsync(string subpath);
+       Task<IDirectoryContents> GetDirectoryContentsAsync(string subpath, System.Threading.CancellationToken cancellationToken);
-       IFileInfo GetFileInfo(string subpath);
+       Task<IFileInfo> GetFileInfoAsync(string subpath);
+       Task<IFileInfo> GetFileInfoAsync(string subpath, System.Threading.CancellationToken cancellationToken);
    }

    public interface IFileInfo
    {
-       Stream CreateReadStream();
+       Task<Stream> CreateReadStreamAsync();
+       Task<Stream> CreateReadStreamAsync(System.Threading.CancellationToken cancellationToken);
    }
}

And perhaps?

namespace Microsoft.Extensions.FileProviders
{
-    public interface IDirectoryContents : IEnumerable<IFileInfo>, IEnumerable
+    public interface IDirectoryContents : IAsyncEnumerable<IFileInfo>
}

Usage Examples

The code in the UseStaticFiles extension should be needed to adapted to use the new API surface.

Alternative design

For my idea

As stated in the background and motivation, I would like to have the option to access other "filesystems", some filesystems like Azure Blob Storage resides on the web. Therefor it is best practice to use async methods. Unfortunately this is in the currently API-surface not possible.

The Azure library however offers some sync methods, but I would like to avoid those if possible.

For the API-proposal

Instead of removing the currently sync methods, the async methods could be an addition to the current interfaces. This would make this a non-breaking change, except for IDirectoryContents since this would mean adding another interface & thus changing the signature.

Risks

The risks with this API-proposal is like I mentioned before that this would entail a breaking change. How much of a breaking change depends of course on whether the sync methods will be replaced or if they stay and async will be added.

[scalablecory]

One complication here is that local filesystem APIs on Windows do not have async support. Generally we try to avoid "fake async" methods, because they create a false promise of scalability. I think Linux has at least some async support on very recent kernels, though.

[synercoder]

@scalablecory I get your point, but let me make a counter-point:

IFileInfo says nothing about windows. IFileInfo is about a file-system abstract, not about a specific implementation. The fact that a specific implementation doesn't support async, doesn't mean that the abstract should be sync. As I mentioned, my idea for an AzureBlobFileProvider would be an async implementation, that would mean that this implementation doesn't "fake async". So if multiple implementations (linux?), to work correctly/better, would need async, why should the windows argument trump all other implementation arguments?

[KalleOlaviNiemitalo]

Could have CancellationToken parameters as well.

[synercoder]

@KalleOlaviNiemitalo Agreed, updated proposal

[ethanshea-ms]

Why not make the change purely additive? There's no reason for this to be breaking.

[scalablecory]

why should the windows argument trump all other implementation arguments?

It doesn't. I'm not shutting your API down, just adding more context. I agree async methods make sense for services which do support it.

Why not make the change purely additive? There's no reason for this to be breaking.

Indeed, we would generally not remove APIs from an interface.

Adding to an interface can be done using Default Interface Methods. This will only work if the package is targeting .NET Core though, not .NET Standard. I'm not sure what this one targets.

The only way we could use DIMs here is to wrap the sync methods by default, which isn't great. An all new interface like IAsyncFileInfo may be better.

[davidfowl]

I think the reasonable thing to do here would be to introduce an IAsyncFileInfo that consumers can use. The API suggested here is a non-starter because of how breaking it is. On top of that there’s reactionary work to consume the new API in static files that would need to happen

[synercoder]

Alright, in the case of the IAsyncFileInfo... how would that relate to, for example, the UseStaticFiles? Because if that one (and other current uses) still uses the old IFileInfo what would be the point of the new interface, if that is not used by the ecosystem? Because starting to use the new interface would also be a breaking change.

My whole point with this api suggestion would be to have the option of async in the current eco system.

So adding the async methods to the interface with default interface methods is a good option, if the consumers of the current interface also start using the async methods.

Else I can write a AzureBlobFileProvider that only implements the async methods, and throws a NotImplementedException on the sync methods. Than when I plug that provider into the UseStaticFiles method the app will crash with NotImplementedException.

[davidfowl]

My whole point with this api suggestion would be to have the option of async in the current eco system.

You can't make a breaking change like that, it's too impactful (especially when the other providers don't need it).

Because starting to use the new interface would also be a breaking change.

It's not a binary breaking change. It's new behavior libraries can opt-into.

With IAsyncFileInfo consumers need to check for the interface and light up the async behavior.

Else I can write a AzureBlobFileProvider that only implements the async methods, and throws a NotImplementedException on the sync methods. Than when I plug that provider into the UseStaticFiles method the app will crash with NotImplementedException.

Sure, you should just implement both.

Let me propose a crazier non breaking alternative. You can implement the async IO today by delaying it until the stream is opened. It does mean you don't get not found failures until the file is opened though.

[synercoder]

I would love to hear more about your crazy idea. :) Do you have some documentation or a blog post about delaying async?

Also with the IAsyncFileInfo, you mean like:

if ( provider is IAsyncFileProvider asyncProvider)
{
    // Async Stuff
}