-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Adding Azure and AWS-specific Image Caches of ImageSharp (Lombiq Technologies: OCORE-136) #15028
Changes from 60 commits
1d1d03a
831a55e
f63afd5
b97171c
d92ce82
db5b9b0
930d41d
20beeaa
572412a
254848b
eabc330
60f16e3
d7fb2ec
73aa34d
6b05f4e
57b7c84
9cf574c
bea7554
f22fc72
2f1378c
da23340
9f77673
d33b463
3042c8d
171b686
8609f53
cf3abea
f73dba3
bb7f327
548a8df
e47985c
502cd27
96abf72
564fec1
23af590
aa1330f
d0f41ab
f2c8598
657e660
94b6fca
ce2a097
6d25a32
d44be33
0704d81
f297e99
13f5b37
5dd7d33
3a1d010
5fc658b
acb8b06
f94bea9
14750b1
3a797ab
d6efce8
988e7d2
5bc8efb
00418e7
d40f1ba
daa1657
fc6d725
a91d5b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
namespace OrchardCore.Media.AmazonS3; | ||
|
||
internal static class AmazonS3Constants | ||
{ | ||
internal static class ValidationMessages | ||
{ | ||
public const string BucketNameIsEmpty = "BucketName is required attribute for S3 storage."; | ||
public const string RegionAndServiceUrlAreEmpty = "Region or ServiceURL is a required attribute for S3 storage."; | ||
} | ||
|
||
internal static class AwsCredentialParamNames | ||
{ | ||
public const string SecretKey = "SecretKey"; | ||
public const string AccessKey = "AccessKey"; | ||
} | ||
|
||
internal static class ConfigSections | ||
{ | ||
public const string AmazonS3 = "OrchardCore_Media_AmazonS3"; | ||
public const string AmazonS3ImageSharpCache = "OrchardCore_Media_AmazonS3_ImageSharp_Cache"; | ||
} | ||
} |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
using Fluid; | ||
using OrchardCore.Environment.Shell; | ||
|
||
namespace OrchardCore.Media.AmazonS3.Helpers; | ||
|
||
// This is almost the same as in OrchardCore.Media.Azure but there isn't really a good common place for it. | ||
MikeAlhayek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
internal sealed class OptionsFluidParserHelper<TOptions> where TOptions : class | ||
{ | ||
// Local instance since it can be discarded once the startup is over. | ||
private readonly FluidParser _fluidParser = new(); | ||
private readonly ShellSettings _shellSettings; | ||
|
||
private TemplateContext _templateContext; | ||
|
||
public OptionsFluidParserHelper(ShellSettings shellSettings) | ||
{ | ||
_shellSettings = shellSettings; | ||
} | ||
|
||
public string ParseAndFormat(string template) | ||
{ | ||
var templateContext = GetTemplateContext(); | ||
|
||
// Use Fluid directly as this is transient and cannot invoke _liquidTemplateManager. | ||
var parsedTemplate = _fluidParser.Parse(template); | ||
return parsedTemplate.Render(templateContext, NullEncoder.Default) | ||
.Replace("\r", string.Empty) | ||
.Replace("\n", string.Empty) | ||
.Trim(); | ||
} | ||
|
||
private TemplateContext GetTemplateContext() | ||
{ | ||
if (_templateContext == null) | ||
{ | ||
var templateOptions = new TemplateOptions(); | ||
_templateContext = new TemplateContext(templateOptions); | ||
templateOptions.MemberAccessStrategy.Register<ShellSettings>(); | ||
templateOptions.MemberAccessStrategy.Register<TOptions>(); | ||
_templateContext.SetValue("ShellSettings", _shellSettings); | ||
} | ||
|
||
return _templateContext; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
using Microsoft.Extensions.Options; | ||
using OrchardCore.FileStorage.AmazonS3; | ||
using SixLabors.ImageSharp.Web.Caching.AWS; | ||
|
||
namespace OrchardCore.Media.AmazonS3.Services; | ||
|
||
// Configuration for ImageSharp's own configuration object. We just pass the settings from the Orchard Core | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for such a comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You asked about the Azure version of this previously, why that was necessary in addition to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure that the comment is helpful, answer any questions or adds any value here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What comment would make it clear for you here why we need a second, seemingly unnecessary configuration class? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest removing it. but it's up to you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd keep it really because apparently it's not clear at a first glance. I can't come up with a better comment. |
||
// configuration. | ||
internal sealed class AWSS3StorageCacheOptionsConfiguration : IConfigureOptions<AWSS3StorageCacheOptions> | ||
{ | ||
private readonly AwsImageSharpImageCacheOptions _options; | ||
|
||
public AWSS3StorageCacheOptionsConfiguration(IOptions<AwsImageSharpImageCacheOptions> options) | ||
{ | ||
_options = options.Value; | ||
} | ||
|
||
public void Configure(AWSS3StorageCacheOptions options) | ||
{ | ||
var credentials = _options.AwsOptions.Credentials.GetCredentials(); | ||
|
||
// Only Endpoint or Region is necessary. | ||
options.Endpoint = _options.AwsOptions.DefaultClientConfig.ServiceURL; | ||
options.Region = _options.AwsOptions.Region?.SystemName; | ||
options.BucketName = _options.BucketName; | ||
options.AccessKey = credentials.AccessKey; | ||
options.AccessSecret = credentials.SecretKey; | ||
options.CacheFolder = _options.BasePath; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same as the section on the page: https://docs.orchardcore.net/en/latest/docs/reference/#search-indexing-querying And that looks good so, since while these are tightly related topics, it's not just search (like SQL queries have nothing to do with search, but queries in general do relate to Lucene and Elasticsearch, which are about indexing, and usually, but not necessarily search...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term should be Search. `Indexing and Querying are implementation details for search.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indexing maybe, but Queries is about much more than search. SQL Queries (i.e. the module feature) doesn't have anything to do with search nor indexing, and you can use both Lucene and Elasticsearch querying (e.g. listing all content items with the type Blog Post) without utilizing them for full-text search. That's why I'm saying these three are tightly related, but the whole feature set is neither just search, nor just querying, nor just indexing.