Make it possible to generate media size limit dynamically

[sarahelsaig]

Is your feature request related to a problem?

The use case is to generate media upload size limit based on external factors. For example to allot a certain amount of total disk space per tenant.

Describe the solution you'd like

We are doing something similar in Lombiq.Hosting.Tenants.MediaStorageManagement package, but it uses a somewhat fragile workaround. So adding a provider interface to MediaSizeLimitAttribute would be prefarable.

Describe alternatives you've considered

Currently we are using a secondary filter meant to overwrite the result of MediaSizeLimitAttribute when applicable. It's not ideal.

[sebastienros]

Each tenant should have its own options instance, so each tenant can have different values. And since it's an options pattern it can be changed on startup (even dynamically I believe).

Not sure it requires a new type of service for that. That's what IConfigureOptions is for. And it would be a dangerous precedent that could lead to adding interfaces for more options...

[sarahelsaig]

The problem with IConfigureOptions is that it's sync-only while the calculation logic we depend on is async. I've tried to create an IAsyncConfigureOptions<MediaOptions> but I get the exception

The type 'OrchardCore.Media.MediaOptions' must be convertible to 'Microsoft.Extensions.Options.IAsyncOptions' in order to use it as parameter 'TOptions' in the generic interface 'Microsoft.Extensions.Options.IAsyncConfigureOptions'

Would it be fine to alter MediaOptions and make it inherit from IAsyncOptions? Or is there a reason why that's not possible?

[sebastienros]

Make it inherit from IAsyncOption then. But maybe even better to just obsolete IAsyncOption and remove its usage so we can do it without change option classes. I looked at the PR and I don't understand why it's necessary.

[sebastienros]

Based on this issue this could break. I am thinking about fixing it.

[sarahelsaig]

Another problem, if I try to implement our code in IConfigureOptions or IAsyncConfigureOptions I get an infinite resolution loop and the program freezes. I think it's because IMediaFileStore (used to calculate the current disk usage) indirectly depends on IOptions<MediaOptions>.

It doesn't look like the options pattern is very suitable for more complex configuration.

[sebastienros]

Check #18793

[github-actions]

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

[sarahelsaig]

@sebastienros The PR you linked looks good, but I don't think it's helpful here. My problem is the MediaOptions have to be already configured to get the disk usage from the media file store, which is necessary if I want to configure the max size dynamically

If we must stick with the configuration pattern, then MaxFileSize should be moved into a separate options file, e.g. MediaLimitOptions.MaxFileSize, so it's not blocker for initializing the media file store. Is that acceptable?