PhysicalFileProvider.GetFileInfo does not return info for directories, even though return type has an IsDirectory property

[olivier-spinelli]

A directory exists but when calling GetFileInfo to obtain an "abstract" IFIleInfo it does not.
However when a directory is returned by the enumeration of a IDirectoryContents, a IFileInfo is correctly returned (with both Exists and IsDirectory sets to true).

This means that the code that handles a IFileInfo HAS to be aware of how it has been created/obtained.

[Test]
public void standard_physical_file_provider_is_full_of_surprises()
{
   using( var pStd = new PhysicalFileProvider( LocalTestHelper.WorldFolder ) )
   {
       var fCA = pStd.GetDirectoryContents( "TestGitRepository" );
       fCA.Exists.Should().BeTrue( "Yes, this is a directory..." );

        var fA = pStd.GetFileInfo( "TestGitRepository" );

        fA.IsDirectory.Should().BeFalse( "What??? This IS a directory!" );
        fA.Exists.Should().BeFalse( "And it DOES exist !" );
   }
}

I suspect that this oddity is due to the FileSystemInfo being abstract: the current code uses a new FileInfo for its PhysicalFileInfo returned object, totally missing the IsDirectory possibility.

Any chance for this to be fixed? Or should I go with my own SimplePhysicalFileProvider?

[natemcmaster]

This is an odd behavior. We will take a look at it in triage. My guess is that changing the return type to include directories may cause issues because this behavior has been this way since ASP.NET Core 1.0.

To triage team: if we don't change, we could consider making PhysicalFileProvider.GetFileInfo virtual so those wishing to change the behavior don't have to reimplement the entire interface. cref https://github.com/aspnet/FileSystem/blob/dev/src/FS.Physical/PhysicalFileProvider.cs#L129

[olivier-spinelli]

Thanks to consider this. The actual complexity of the PhysicalFileProvider is in the Watch implementation that relies on the public PhysicalFilesWatcher that has no dependency on the PhysicalFileProvider concrete class. It is quite easy to implement a new PhysicalFilesWatcher that behaves correclty regarding this.

In my opinion, the "virtual way" will be a trap into which a lot of future users will fall: the "fixed with virtual implementation" will be copy-pasted in each and every projects that will demand correct behavior. Or another package will be used by the community.

I see 3 options here:

• A fixed PhysicalFileProvider and a new PhysicalFileProviderLegacy
• A new PhysicalFileProvider2 (no idea better name comes into my mind) and an Obsolete ('warn' for x months, and later 'error') attribute on the PhysicalFileProvider in this package.
• A new bool legacyFileInfoBehavior = false in the ctors. (setting it to true by default will not solve the issue on the long term). (This bool will be definitely removed later.)

Any thoughts? Other ideas?

[muratg]

Moving out to 3.0.0 because it's not clear what can be accomplished in 2.2.

[muratg]

I don't expect we'll make further changes here.