Add NullMediaFileStoreCache to avoid injecting IServiceProvider

[hishamco]

Introduce a Null Object Pattern for IMediaFileStoreCache to avoid injecting IServiceProvider

[Piedone]

I'd wait with this until #15276 is merged.

[MikeAlhayek]

I'd wait with this until #15276 is merged.

I agree this will have to wait.

@hishamco what you did here is much better than what we have today. I am not a fan of the null pattern, can you think of a better way like using feature dependency instead?

[hishamco]

I am not a fan of the null pattern

:)

We could use feature dependency as you said but this is useful for 3rd-party media providers

[MikeAlhayek]

We could use feature dependency as you said but this is useful for 3rd-party media providers

How about following the providers pattern (used in email PR) here too?

[hishamco]

I'm not sure if it's a good idea to use the Provider Model everywhere, that's why I'm suggesting Null Object Pattern

[MikeAlhayek]

I'm not sure if it's a good idea to use the Provider Model everywhere, that's why I'm suggesting Null Object Pattern

I'm not sure if it's a good idea to use the Null Object Pattern everywhere, that's why I'm suggesting Provider Model. 🤣

[hishamco]

No problem bro, let's hear the other's feedback

[hishamco]

@Piedone can we have a look to this again

[Piedone]

Did you test this with AWS and Azure Media?

As for the provider pattern: this PR is about addressing something that's more of an aesthetic issue and I imagine it's only a more serious problem in extreme cases (I certainly haven't encountered any, nor can imagine one, even after working a lot with Media feature set in the last two years or so). The fix prevents potential NREs and DI resolution exceptions and is fine for that IMO.

I don't see any necessity to add any further complexity and would advise anybody against it too. If you'd like to, feel free, of course, but due to this and because I don't see this as a high-priority problem, I won't be able to allocate time to collaborate on it.

[Piedone]

The nullcheck below should be adjusted too.

[hishamco]

@MikeAlhayek while @Piedone doesn't have much time on this, can we use the Null Object Pattern or any other approach instead if using IServiceProvider

[Piedone]

So, any news here? After #15391 (comment) I'm OK to merge this but only if as mentioned under #15391 (review) you confirm that you also tested it with both providers.

[hishamco]

I can continue in this PR but my issue with testing with the underlying providers coz I didn't work with both Azure & S3 storage

[Piedone]

Time to start now, then :).

[hishamco]

While I don't have much experience with Azure & S3 storage media providers I might need to hand over this PR, or let someone to try it

[Piedone]

Let's close this, then.

[hishamco]

IMHO it's better to leave this open, so anyone can test if possible instead of not doing anything at all

[Piedone]

In the last two months, nobody else came forward with an interest in this PR. What do we hope from keeping it open, and for how long?

[hishamco]

While the PR change the pattern nothing related to the media itself, I hope to revise it in the upcoming month