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

IUploadedFile: Improve the public API and document #472

Closed
HeyJoel opened this issue Dec 21, 2021 · 1 comment
Closed

IUploadedFile: Improve the public API and document #472

HeyJoel opened this issue Dec 21, 2021 · 1 comment

Comments

@HeyJoel
Copy link
Member

HeyJoel commented Dec 21, 2021

Document and make the API more discoverable

Ref #471 it's not particularly intuitive to add an asset file, we should improve the API here and document it. XML comments on the API also need to ref the method for creating file sources.

Should IUploadedFile be disposable?

The aim is for the callee to handle disposing, but what if open is never called? If there is an unmanaged stream in there then it will never get disposed.

However implementing IDisposable gives the impression that the container stream would be disposed? Perhaps we should dispose if open is never called in instances that hold a stream.

If we can remove the requirement for fileLength then maybe we don't need the stream. Perhaps make it nullable - supply it if we have it? Would make the embedded resource provider simpler, although then we wouldn't be able to do the exists check first? That's kinda nice to be able to return null if it doesn't exist.

Maybe it's the StreamFileSource that is wrong, and we should be passing in a function rather than instance of stream, so it's up to the constructor to ensure it is dealt with accordingly.

Properties

Looking into it more, I think MimeType/FileLength should be optional (and it probably already is, just not indicated as such).

Renaming

It's not always an "uploaded" file, so I think we should change it to IFileSource instead to match other implementations.

@HeyJoel HeyJoel added this to the 0.10 milestone Dec 21, 2021
HeyJoel added a commit that referenced this issue Feb 16, 2022
- `IUploadedFile` has been renamed `IFileSource`
- `FormFileUploadedFile` has been renamed `FormFileSource`
- `IFormFileUploadedFileFactory` has been removed and a FormFileSource can be created by simply calling the constructor `new FormFileSource(formFile)`.
- `IUploadedFile` no longer requires a file size
- `IStreamFileSource` now requires a factory delegate for creating the stream instead of wrapping an existing stream.
@HeyJoel
Copy link
Member Author

HeyJoel commented Feb 16, 2022

Fixed, will be release in v0.10

Breaking changes:

  • IUploadedFile has been renamed IFileSource
  • FormFileUploadedFile has been renamed FormFileSource
  • IFormFileUploadedFileFactory has been removed and a FormFileSource can be created by simply calling the constructor new FormFileSource(formFile).
  • IUploadedFile no longer requires a file size
  • IStreamFileSource now requires a factory delegate for creating the stream instead of wrapping an existing stream.

@HeyJoel HeyJoel closed this as completed Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant