Amazon S3 Media Storage support #11738

neglectedvalue · 2022-05-21T21:30:16Z

Hi, long time ago I've created module to support AWS S3 bucket for Orchard media storage and since OrchardCore was in my opinion Azure based I did not think about any contributions, but recently @Piedone contacted with me and suggested to create a PR.
Basically nothing complex, module for Azure Media support was done nicely and understandable so I've made mine as much equal as possible.

dnfadmin · 2022-05-21T21:30:28Z

All CLA requirements met.

hishamco · 2022-05-22T07:27:46Z

I was planned to make this for Orchard Core Contrib (OCC), but it's good to see someone push a PR to implement it

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Manifest.cs

Piedone

Awesome, thank you!

src/OrchardCore.Cms.Web/appsettings.json

src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AmazonWebServiceResponseExtensions.cs

src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsDirectory.cs

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/OrchardCore.Media.AmazonS3.csproj

Co-authored-by: Zoltán Lehóczky <[email protected]>

….Media.AmazonS3.csproj Co-authored-by: Zoltán Lehóczky <[email protected]>

Co-authored-by: Zoltán Lehóczky <[email protected]>

…ions.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

…ptionsExtension.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

Co-authored-by: Zoltán Lehóczky <[email protected]>

…ile.

src/docs/reference/modules/Media.Azure/README.md

Piedone · 2022-05-24T16:07:41Z

Please ask for a review (upper right corner) if you're done addressing all the feedback.

neglectedvalue · 2022-05-24T16:12:27Z

Yes, sorry always forgetting about it.

neglectedvalue · 2022-05-24T17:54:59Z

I've looked at Azure Media module and I want to transfer some things into Amazon Media : templating for BucketName/ BasePath and ModularEvent so it would be possible to create S3 bucket automatically. Hope I'll finish it today.

src/OrchardCore.Cms.Web/appsettings.json

src/docs/reference/README.md

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs

src/docs/reference/modules/Media.Azure/README.md

src/docs/reference/modules/Media.Amazon/README.md

src/OrchardCore.Cms.Web/appsettings.json

src/docs/reference/modules/Media.Amazon/README.md

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsExtension.cs

Piedone

Apart from these and #11738 (comment) looks good.

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/CreateMediaS3BucketEvent.cs

Piedone · 2022-05-25T19:53:34Z

Oh, and also address #11738 (comment), please.

…ptionsConfiguration.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

…S3BucketEvent.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

src/docs/reference/modules/Media.AmazonS3/README.md

Co-authored-by: Zoltán Lehóczky <[email protected]>

Piedone

Anything you'd like to add, @hishamco @jtkech?

sebastienros · 2022-05-26T18:22:25Z

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs

+
+            options.BucketName = template
+                .Render(templateContext, NullEncoder.Default)
+                .Replace("\r", String.Empty).Replace("\n", String.Empty);


Why not just trim. If there are space in the middle of the string they should be intentional. I could understand that having a new line in a fluid template might be hardly noticeable though.

Can we use Environment.NewLine instead?

Instead of "\n"? So result will be smth like :
options.BucketName = template .Render(templateContext, NullEncoder.Default) .Replace("\r", String.Empty) .Replace(System.Environment.NewLine, String.Empty) .Trim();

Environment.NewLine insstead of \r\n

options.BucketName = template .Render(templateContext, NullEncoder.Default) .Replace(Environment.NewLine, String.Empty)

So guys, should i rollback my latest commit? :)

I think it is safer, but keep the trim

I would have let the replace of \r and \n.

Is there anything wrong with Environment.NewLine?

We already used quite often https://github.com/OrchardCMS/OrchardCore/search?q=NewLine

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs

hishamco · 2022-05-27T18:10:43Z

Just a note to @agriffard please add it as a contributor once the PR is merged

…rces

Piedone · 2022-05-30T19:38:30Z

@jtkech anything you'd like to add or would you approve, please?

jtkech · 2022-05-30T20:00:43Z

Related to the comments I did, that's okay for me.
Didn't look at all things, but LGTM so I will approve.

Piedone · 2022-05-30T20:03:35Z

Congrats @neglectedvalue and thank you for this contribution!

neglectedvalue added 2 commits May 21, 2022 23:58

Amazon S3 Media Storage feature impl

a2bcc94

Amazon S3 Media Storage coment update

6741b4c

jtkech reviewed May 22, 2022

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Manifest.cs Outdated Show resolved Hide resolved

neglectedvalue added 2 commits May 23, 2022 00:09

Redundant Feature attribute was removed from Manifest and Startup

d43d612

Manifest changes

c1f3427

Piedone requested changes May 23, 2022

View reviewed changes

neglectedvalue and others added 10 commits May 23, 2022 21:59

Update src/OrchardCore.Cms.Web/appsettings.json

857128f

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsDirectory.cs

6f2f4cb

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsFile.cs

b446d71

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/OrchardCore…

d88aafe

….Media.AmazonS3.csproj Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsFile.cs

efc8f33

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsStorageOpt…

e2600f6

…ions.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsStorageOpt…

35cd290

…ions.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageO…

13dbfa7

…ptionsExtension.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs

3fa2206

Co-authored-by: Zoltán Lehóczky <[email protected]>

Changes after Piedone review

2ad2279

neglectedvalue requested a review from agriffard as a code owner May 23, 2022 22:11

MediaConfigurationException was added. Small corrections to the doc f…

9c90f5c

…ile.

hishamco reviewed May 24, 2022

View reviewed changes

src/docs/reference/modules/Media.Azure/README.md Outdated Show resolved Hide resolved

Documentation update.

4b4100f

neglectedvalue requested a review from Piedone May 24, 2022 16:10

Piedone requested changes May 24, 2022

View reviewed changes

Piedone reviewed May 24, 2022

View reviewed changes

src/OrchardCore.Cms.Web/appsettings.json Outdated Show resolved Hide resolved

Piedone reviewed May 24, 2022

View reviewed changes

src/docs/reference/modules/Media.Amazon/README.md Outdated Show resolved Hide resolved

jtkech reviewed May 24, 2022

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsExtension.cs Outdated Show resolved Hide resolved

Piedone requested changes May 25, 2022

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs Outdated Show resolved Hide resolved

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/CreateMediaS3BucketEvent.cs Outdated Show resolved Hide resolved

neglectedvalue and others added 4 commits May 25, 2022 22:55

Update src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageO…

cbfe4d6

…ptionsConfiguration.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

Update src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/CreateMedia…

501932a

…S3BucketEvent.cs Co-authored-by: Zoltán Lehóczky <[email protected]>

ACL and bucket configuration documentation

a42fd5a

Section naming fix

d92cd07

neglectedvalue requested review from jtkech and Piedone May 25, 2022 20:42

Piedone requested changes May 25, 2022

View reviewed changes

src/docs/reference/modules/Media.AmazonS3/README.md Outdated Show resolved Hide resolved

Update src/docs/reference/modules/Media.AmazonS3/README.md

f492d1d

Co-authored-by: Zoltán Lehóczky <[email protected]>

Piedone approved these changes May 25, 2022

View reviewed changes

neglectedvalue requested a review from hishamco May 26, 2022 10:59

sebastienros reviewed May 26, 2022

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs Outdated Show resolved Hide resolved

sebastienros approved these changes May 26, 2022

View reviewed changes

neglectedvalue added 2 commits May 26, 2022 23:03

sebastienros and hishamco remarks fix

a77cf06

Changes after discussion.

fd78ac4

hishamco requested changes May 27, 2022

View reviewed changes

Trim removed, NewLine styling

dc24e5a

neglectedvalue requested a review from hishamco May 27, 2022 15:00

hishamco reviewed May 27, 2022

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs Show resolved Hide resolved

hishamco approved these changes May 27, 2022

View reviewed changes

Typo in logger param, NewLine changed to support different config sou…

b464a5d

…rces

jtkech approved these changes May 30, 2022

View reviewed changes

Piedone merged commit 187327c into OrchardCMS:main May 30, 2022

neglectedvalue mentioned this pull request May 30, 2022

Contribute to Orchard Core neglectedvalue/OrchardCore.Media.Amazon.S3#7

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amazon S3 Media Storage support #11738

Amazon S3 Media Storage support #11738

neglectedvalue commented May 21, 2022

dnfadmin commented May 21, 2022 •

edited

Loading

hishamco commented May 22, 2022

Piedone left a comment

Piedone commented May 24, 2022

neglectedvalue commented May 24, 2022

neglectedvalue commented May 24, 2022

Piedone left a comment

Piedone commented May 25, 2022

Piedone left a comment

sebastienros May 26, 2022

hishamco May 26, 2022

neglectedvalue May 26, 2022 •

edited

Loading

hishamco May 26, 2022

hishamco May 26, 2022

neglectedvalue May 26, 2022

jtkech May 26, 2022

neglectedvalue May 26, 2022

hishamco May 27, 2022

hishamco May 27, 2022

hishamco commented May 27, 2022

Piedone commented May 30, 2022

jtkech commented May 30, 2022

Piedone commented May 30, 2022

Amazon S3 Media Storage support #11738

Amazon S3 Media Storage support #11738

Conversation

neglectedvalue commented May 21, 2022

dnfadmin commented May 21, 2022 • edited Loading

hishamco commented May 22, 2022

Piedone left a comment

Choose a reason for hiding this comment

Piedone commented May 24, 2022

neglectedvalue commented May 24, 2022

neglectedvalue commented May 24, 2022

Piedone left a comment

Choose a reason for hiding this comment

Piedone commented May 25, 2022

Piedone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neglectedvalue May 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hishamco commented May 27, 2022

Piedone commented May 30, 2022

jtkech commented May 30, 2022

Piedone commented May 30, 2022

dnfadmin commented May 21, 2022 •

edited

Loading

neglectedvalue May 26, 2022 •

edited

Loading