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

[API Proposal]: Implement IDirectoryContents on PhysicalDirectoryInfo at MS.Extensions.FileProviders.Physical #86354

Closed
vpenades opened this issue May 16, 2023 · 4 comments · Fixed by #91024
Labels
api-approved API was approved in API review, it can be implemented area-System.IO
Milestone

Comments

@vpenades
Copy link

Background and motivation

Microsoft.Extensions.FileProviders.Abstractions provides a basic set of interfaces that allow accessing the file system.

The default implementation, Microsoft.Extensions.FileProviders.Physical , wraps FileInfo and DirectoryInfo , and the context is initialized using a PhysicalFileProvider, which, as I understand it, might be used at the directory root, or any directory that can be considered the "local root"

The problem:

PhysicalFileProvider implements IDirectoryContents which gives the list of IFileInfo items contained in the root directory, which are represented by both files and directories , the latter being implemented by a PhysicalDirectoryInfo , which implements IFileInfo, but does not implement IDirectoryContents , and as a consequence of this, it's not possible to traverse the file system tree beyond the 1st directory level provided by PhysicalFileProvider

API Proposal

namespace Microsoft.Extensions.FileProviders.Physical
{
    public class PhysicalDirectoryInfo : IFileInfo, IDirectoryContents  
    {
        // IDirectoryContents implementation
    }

API Usage

class Utils
{
    public static void RecursivelyTraverseTree(IDirectoryContents contents)
    {
        foreach (var entry in contents)
        {
            if (entry.IsDirectory)
            {
                if (entry is IDirectoryContents childContents)
                {
                    RecursivelyTraverseTree(childContents);
                }                
            }
        }           
    }
}

Alternative Designs

Alternatively, PhysicalDirectoryInfo could implement IFileProvider instead of IDirectoryContents because the latter can be retrieved from the former.

Anyway, I am open to proposals to know how to traverse a file system tree based on IFileInfo abstractions, without taking assumptions about the underlaying implementation.

Risks

No response

@vpenades vpenades added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 16, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 16, 2023
@ghost
Copy link

ghost commented May 16, 2023

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Microsoft.Extensions.FileProviders.Abstractions provides a basic set of interfaces that allow accessing the file system.

The default implementation, Microsoft.Extensions.FileProviders.Physical , wraps FileInfo and DirectoryInfo , and the context is initialized using a PhysicalFileProvider, which, as I understand it, might be used at the directory root, or any directory that can be considered the "local root"

The problem:

PhysicalFileProvider implements IDirectoryContents which gives the list of IFileInfo items contained in the root directory, which are represented by both files and directories , the latter being implemented by a PhysicalDirectoryInfo , which implements IFileInfo, but does not implement IDirectoryContents , and as a consequence of this, it's not possible to traverse the file system tree beyond the 1st directory level provided by PhysicalFileProvider

API Proposal

namespace Microsoft.Extensions.FileProviders.Physical
{
    public class PhysicalDirectoryInfo : IFileInfo, IDirectoryContents  
    {
        // IDirectoryContents implementation
    }

API Usage

class Utils
{
    public static void RecursivelyTraverseTree(IDirectoryContents contents)
    {
        foreach (var entry in contents)
        {
            if (entry.IsDirectory)
            {
                if (entry is IDirectoryContents childContents)
                {
                    RecursivelyTraverseTree(childContents);
                }                
            }
        }           
    }
}

Alternative Designs

Alternatively, PhysicalDirectoryInfo could implement IFileProvider instead of IDirectoryContents because the latter can be retrieved from the former.

Anyway, I am open to proposals to know how to traverse a file system tree based on IFileInfo abstractions, without taking assumptions about the underlaying implementation.

Risks

No response

Author: vpenades
Assignees: -
Labels:

api-suggestion, area-System.IO

Milestone: -

@Jozkee Jozkee added this to the Future milestone Jul 21, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 21, 2023
@Jozkee Jozkee added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jul 21, 2023
@Jozkee
Copy link
Member

Jozkee commented Jul 21, 2023

As a workaround, you could just create a PhysicalDirectoryContents using PhysicalDirectoryInfo.PhysicalPath, no?

@Jozkee Jozkee removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jul 21, 2023
@vpenades
Copy link
Author

As a workaround, you could just create a PhysicalDirectoryContents using PhysicalDirectoryInfo.PhysicalPath, no?

No, the point of using the interfaces is to have an abstraction layer that does not implicitly assume there's a physical path. In fact I want to create virtualized file systems, or file system from within archives, where PhysicalPath would throw an exception.

@Jozkee Jozkee added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 21, 2023
@bartonjs
Copy link
Member

bartonjs commented Aug 3, 2023

Video

Looks good as proposed

namespace Microsoft.Extensions.FileProviders.Physical
{
    public partial class PhysicalDirectoryInfo : IFileInfo
+       , IDirectoryContents  
    {
        // IDirectoryContents implementation
    }

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Aug 3, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 23, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 31, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.IO
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants