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

Add FileProviderImageProvider and WebRootImageProvider #243

Conversation

ronaldbarendse
Copy link
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This adds a FileProviderImageProvider and WebRootImageProvider, following the pattern mentioned in umbraco/Umbraco-CMS#12185 (comment). This is basically a simplified version of PR #207, but these IImageProviders are now included as additional, non-default implementations.

This would allow users to easily switch from the default PhysicalFileSystemProvider to the WebRootImageProvider by using:

services.AddImageSharp()
    .RemoveProvider<PhysicalFileSystemProvider>() // or .ClearProviders()
    .AddProvider<WebRootImageProvider>();

/// Initializes a new instance of the <see cref="FileProviderImageResolver"/> class.
/// </summary>
/// <param name="fileInfo">The file info.</param>
public FileProviderImageResolver(IFileInfo fileInfo) => this.fileInfo = fileInfo;
Copy link
Contributor Author

@ronaldbarendse ronaldbarendse Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also do a null check?

Suggested change
public FileProviderImageResolver(IFileInfo fileInfo) => this.fileInfo = fileInfo;
public FileProviderImageResolver(IFileInfo fileInfo) => this.fileInfo = fileInfo ?? throw new ArgumentNullException(nameof(fileInfo));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't bother. The default implementation is smart enough to only call this type when the IFileInfo is not null. If someone doesn't do this with their custom code then it's up to them.

@codecov
Copy link

codecov bot commented Apr 2, 2022

Codecov Report

Merging #243 (9d34c2c) into main (249be0d) will decrease coverage by 0%.
The diff coverage is 0%.

@@         Coverage Diff         @@
##           main   #243   +/-   ##
===================================
- Coverage    85%    84%   -1%     
===================================
  Files        72     75    +3     
  Lines      2020   2036   +16     
  Branches    291    294    +3     
===================================
  Hits       1724   1724           
- Misses      212    228   +16     
  Partials     84     84           
Flag Coverage Δ
unittests 84% <0%> (-1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...geSharp.Web/Providers/FileProviderImageProvider.cs 0% <0%> (ø)
...c/ImageSharp.Web/Providers/WebRootImageProvider.cs 0% <0%> (ø)
...geSharp.Web/Resolvers/FileProviderImageResolver.cs 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 249be0d...9d34c2c. Read the comment docs.

@JimBobSquarePants JimBobSquarePants merged commit 8892d76 into SixLabors:main Apr 4, 2022
@ronaldbarendse ronaldbarendse deleted the feature/fileproviderimageprovider branch April 4, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants