Skip to content
This repository has been archived by the owner on Nov 6, 2018. It is now read-only.

Suggest to make NotFoundFileInfo and NoopChangeToken public #145

Closed
PrimeObjects opened this issue Nov 24, 2015 · 17 comments
Closed

Suggest to make NotFoundFileInfo and NoopChangeToken public #145

PrimeObjects opened this issue Nov 24, 2015 · 17 comments
Assignees
Milestone

Comments

@PrimeObjects
Copy link

No description provided.

@Eilon
Copy link
Member

Eilon commented Nov 25, 2015

Can you clarify the reason for wanting them public? Are you writing a custom file system provider? Or something else?

@PrimeObjects
Copy link
Author

Hi Eilon,

I'm writing a ViewFileProvider, I hope this FileProvider can serve MVC by using view files saved in the database.

I need to return NoopChangeToken and NotFoundFileInfo like what RazorEmbeddedFileProvider did.
I want to use the one defined in .NET in my class directly.
e.g.

public IChangeToken Watch(string pattern)
        {
            return NoopChangeToken.Singleton;
        }

Thanks
Gary

@Eilon
Copy link
Member

Eilon commented Nov 25, 2015

Ah, got it. So just for code re-use, which sounds fine. Just wanted to make sure it wasn't for checking equality, which would be wrong (e.g. checking if (foo.GetType() == typeof(NoopChangeToken)) and making some decision based on that.

@PrimeObjects
Copy link
Author

thank you for reminding. I cloned the two classes into my namespace for now. if you decide to make the two classes public I should be able use them directly.

Thanks again.
Gary

@muratg muratg added this to the Backlog milestone Dec 8, 2015
@mgrosperrin
Copy link
Contributor

Hi,
maybe we could provide a factory to create empty implementation of these types (IFileInfo, IDirectoryContents and IChangeToken). This would protect against comparing the type (except if people do scary things like foo.GetType() == factory.GetXXX().GetType()) and allow reuse of the types.
I can provide you a PR to prototype it if needed (I just need a name for the project hosting the factory 😄).

@pranavkm
Copy link
Contributor

@PrimeObjects all of these packages are in the Microsoft.AspNet.FileProviders.Sources package. You should be able to reference them and use that.

@mgrosperrin
Copy link
Contributor

@pranavkm I have tried to do this for my composite file provider, but the Microsoft.AspNet.FileProviders.Sources package has not been published in the RC1 wave. Will it be published as part of RC2 (it is in the nightly) ?

@pranavkm
Copy link
Contributor

We don't publish any of the Sources packages to NuGet.org. You'd have to pick it up from aspnetmaster.

@Driedas
Copy link
Contributor

Driedas commented Mar 1, 2016

I second this suggestion, my use case is a custom IFileProvider that internally wraps the PhysicalFileProvider to provide a virtual file system, returning files based on other criteria than just the file path. If all criteria fails to find the requested file, I would like to return NotFoundFileInfo directly. Returning null just yields a NullReferenceException in Microsoft.AspNet.StaticFiles.StaticFileContext.LookupFileInfo()

@MiloSmith
Copy link

I agree with this, I am using SaasKit and have a tenant context. My file provider takes that context into account and servers up files based on the tenant. I had to copy several classes into my code from the source, and that is a pretty ugly solution. Because of this, I get the impression that you guys don't want us making new file providers. Is that the correct? Am I missing something here?

@pranavkm
Copy link
Contributor

@Eilon - thoughts on publishing the NoOp* types from Sources into Abstractions?

@Eilon Eilon modified the milestones: 1.0.1, Backlog May 24, 2016
@Eilon
Copy link
Member

Eilon commented May 24, 2016

In 1.0.1. No time in this milestone.

@Driedas
Copy link
Contributor

Driedas commented May 24, 2016

I understand you guys are all swamped at the moment to get to RTM, so would you accept a pull request with this change?

@Eilon
Copy link
Member

Eilon commented May 25, 2016

@Driedas we would certainly consider it, though of course it's difficult to guarantee anything 😄

@Driedas
Copy link
Contributor

Driedas commented May 25, 2016

@Eilon implemented suggestion by @pranavkm in PR #195

@Eilon Eilon modified the milestones: 1.0.0, 1.0.1 May 26, 2016
@Eilon
Copy link
Member

Eilon commented May 26, 2016

Assigning to @pranavkm because he's taking care of the PR.

@pranavkm
Copy link
Contributor

Fixed in f282bce

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

No branches or pull requests

7 participants