Make media paths / urls mapping working under a virtual folder.

[jtkech]

This is done by the MediaFileStore which is a tenant singleton. So, as i commented in the media startup ConfigureServices() where we define how to build it.

// To map between paths and public urls we need to take into account the request 'PathBase'
// which may start by e.g a virtual path and which already contains the 'RequestUrlPrefix'.
// We can't do that here because this tenant may be built in the context of another one.
// So, it is done on demand by the 'MediaFileStore' by using an 'IHttpContextAccessor'.

return new MediaFileStore(fileStore, AssetsUrlPrefix, httpContextAccessor);

Note: Normally, in ConfigureServices() the request PathBase doesn't contain yet the prefix (e.g only a virtual path). But a tenant may be built in a request context of another tenant, so here PathBase would contain a prefix and of another tenant. The prefix is added just before executing the pipeline of a given tenant so it is there e.g in a startup Configure().

• In Media.Azure, for the publicUrlBase of the MediaFileStore we use an uri relying on MediaBlobStorageOptions, and no distinction seems to be done between tenants by using RequestUrlPrefix. Maybe wrong or maybe intended to be defined per tenant by the config.

• So right now, for Media.Azure, to build the MediaFileStore i just passed a null IHttpContextAccessor, at least here it works as before.

[jtkech]

Not ready because just saw in MediaFileProvider

    public IImageResolver Get(HttpContext context)
    {
        // Path has already been correctly parsed before here.

        var filePath = _mediaStore.MapPublicUrlToPath(context.Request.PathBase + context.Request.Path.Value);

        return new MediaFileResolver(_mediaStore, filePath);
    }

Here PathBase is taken into account when calling MapPublicUrlToPath() so seems that my above fix is not done in the right place. It doesn't work as soon as we specify e.g an ?height=** in the url => ImageSharp uses the MediaFileProvider and so on. Will see tomorrow.

Update: this is my test that was wrong, so my above changes seem to be okay.

Talking again about Media.Azure, it uses a BlobFileStore through e.g the same MediaFileProvider and MediaFileStore classes. So, the above method which uses the PathBase is still used and will call the following in MediaFileStore (as it is in the dev branch).

    public string MapPublicUrlToPath(string publicUrl)
    {
        if (!publicUrl.StartsWith(_publicUrlBase, StringComparison.OrdinalIgnoreCase))
        {
            throw new ArgumentOutOfRangeException(nameof(publicUrl), "The specified URL is not inside the URL scope of the file store.");
        }

        return publicUrl.Substring(_publicUrlBase.Length);
    }

So, if PathBase is not empty, because of a virtual folder and / or a tenant prefix, here publicUrl will start with this non empty PathBase, and, as i understand, it can't start by _publicUrlBase which here is related to the uri of the BlobFileStore.

So, as said, right now, for Azure.Media, i pass a null IHttpContextAccessor to build the MediaFileStore so that it works as before.

[jtkech]

Fixes #3195

[jtkech]

@sebastienros returning back to this one.

• I think that's ok for the Media module to generate / resolve media urls / paths under a virtual folder.

• For the Media.Azure module, currently we pass the blob URI to the MediaFileStore, so through MapPathToPublicUrl() a generated url in a view will point directly to the blob storage. That's ok but it means that ImageSharp is not used, and that we can't use e.g a resize_url.

• Maybe we could keep the same logic and use ImageSharp which will use the same MediaFileProvider which use an IMediaFileStore which here is a BlobFileStore.

• Also, i don't see the distinction between tenants, unless we have a different config per tenant, or if we explicitly specify a distinct url pointing to the blob store, so without using some of our tag helpers.

• When mapping an url to a path, we remove the PathBase including the prefix because media files are in different locations per tenant. But for Media.Azure, if we want to keep the ImageSharp logic, we would have to keep e.g the prefix to make the distinction, but not a possible virtual folder.

[TFleury]

Throws with Azure

Microsoft.AspNetCore.Http.PathString..ctor(String value)
The path in 'value' must start with '/'.

_publicUrlBase is a full http URI like https://youraccount.blob.core.windows.net/media

[TFleury]

Hi @jtkech,
I tested your PR with blob storage, and for now it throws.
I commented it in you code.

[jtkech]

@TFleury thanks a lot for trying it!

• So, because you can try it, i will rework on it asap. Here the PR is just to make media working under a virtual folder, and let Media.Azure working as before. So, good catch for the leading slash that anyway is necessary for a PathString.

• Then, i don't remember the details but as said in some of my comments, you confirmed me that here the generated url is a full uri and it is not compatible e.g with the usage of our resize_url filter which relies on ImageSharp and so on.

• I also want to make the distinction between tenants. I thought about using the BasePath in BlobStorageOptions but finally not a good idea because coming from the config we don't know if it is a tenant specific value or a global value for all tenants so without any distinction.

• So i think i will just use the tenant prefix as an additional segment to the media path. Hmm, maybe a little breaking change, but not when only using a Default tenant without any prefix.

I will rework on it asap and let you know when it will be ready to be checked. Thanks again.

[jtkech]

@sebastienros @TFleury

• So, in one of my 2 last commits i simplified the code. I thought i was not possible to get the request PathBase in the media startup because a tenant can be built by another tenant, but in fact i get it when the service is resolved, so that's ok.

• So, with this commit we still fix Media urls when under a virtual folder, and Media.Azure really works as before. So the 1st test would be to use 5941027, not the last commit.

• Then, in the following commit the suggestion for Media.Azure is to init the MediaFileStore in the same way so that urls / paths mapping is exactly the same and ImageSharp is not bypassed (this for image resizing and local cache). Then this is ImageSharp that will get the blob file because it uses the same MediaFileProvider which uses the same MediaFileStore that uses an IFileStore which here is our BlobFileStore.

• Finally, to make the BlobFileStore tenant aware i used shell settings RequestUrlPrefix to init the BlobStorageOptions.BasePath. But not sure it is okay to do this. So, for Media.Azure you can try my last commit, to check the previous point, and for this last point you would have to create a new tenant.

Maybe Media.Azure is not intended to be used with ImageSharp, but at least i think we need to make the BlobFileStore tenant aware.

[TFleury]

@jtkech
5941027 works with Azure Media Storage.

After 2405fcc it fails, because (for now) we don't have to be tenant aware, we use raw blob URIs (media are directly served from Azure)

[TFleury]

The next step could be branch the pipeline at 'media' path, move UseImageSharp in that branch and replace UseStaticFiles by a custom middleware that serves files from FileSystemStore or BlobFileStore.

Pros :

• Only 'media' requests will pass through UseImageSharp
• Ability to use resize_url when using Azure storage

Cons :

• All media will be served from Orchard (performance ?)
• MediaBlobStorageOptions.PublicHostName will be obsolete.

[jtkech]

@TFleury thanks.

5941027 works with Azure Media Storage.

Good to know.

After 2405fcc it fails, ...

Okay.

... because (for now) we don't have to be tenant aware, ...

So, how do you make the distinction between tenants. Do you mean the same media files are shared across tenants? Or am i missing something?

... we use raw blob URIs (media are directly served from Azure).

Yes i understand but the idea was to get all images through the BlobFileStore.

Only 'media' requests will pass through UseImageSharp

Oh, i thought it was already the case but anyway this is the case with non image media files that would be also stored on azure, as you said, we would need an UseStaticFiles() with a custom file provider.

Ability to use resize_url when using Azure storage

Yes, this was one of the goals.

All media will be served from Orchard (performance ?)

Yes, i agree.

MediaBlobStorageOptions.PublicHostName will be obsolete.

Yes, i saw this, the idea being that all images would be served through the BlobFileStore.

Sorry to have bothered you, i will revert to 5941027 and if people ask for this, because it is not so obvious, i will open a new PR and take the time to try it with a real blob storage account.

So, my 2 last questions are, how do you use it as it is if you have more than one tenant, do they share the same media files? Finally, how do you configure https://youraccount.blob.core.windows.net/media, is it just your blob uri or do you use MediaBlobStorageOptions.BasePath = /media?

[TFleury]

So, how do you make the distinction between tenants. Do you mean the same media files are shared across tenants? Or am i missing something?

In my case blob storage is configured for each tenant, so each tenant have its own.
But if you have a global blob storage configuration, you will share files across tenants, that's right.

I had not seen the things that way, and now I understand why you want to make the BlobFileStore tenant aware.

For the case of the URL https://youraccount.blob.core.windows.net/media, media is the container name (ContainerName property of MediaBlobStorageOptions).

If you use BasePath option, you will have https://[account_name].blob.core.windows.net/[container]/[base_path]/[media_path]

For those who wants to share the same AzureStorageAccount for multiple tenants, options are to have container or different 'BasePath' per tenant.

[jtkech]

@TFleury thanks for the infos.

In my case blob storage is configured for each tenant, so each tenant have its own.

Thanks for confirming this.

But if you have a global blob storage configuration, you will share files across tenants, that's right.

Okay.

I had not seen the things that way, and now I understand why you want to make the BlobFileStore tenant aware.

Yes ;)

If you use BasePath option, you will have https://[account_name].blob.core.windows.net/[container]/[base_path]/[media_path]

Okay, this is what i tried to do automatically by setting BasePath to /{tenantUrlPrefix} if the prefix is not null. So, i don't understand why images was not served by the BlobFileStore e.g with the default tenant where the uri would just be https://[account_name].blob.core.windows.net/[container]/[media_path]

Anyway, because as you said there are pros and cons, i removed the changes related to azure. We will do another PR if people want to use the ImageSharp middleware for medias on azure. And for multi tenants, make sense to have different blob storage configs per tenant.

[jtkech]

@sebastienros so, for infos i think this PR is ready. There is only one file changed to make Media paths / urls mapping working under a virtual folder. And as said above, if people want to use the ImageSharp middleware for medias on azure, we will do another PR.

[jtkech]

I think this one is ready to be merged.

• Beyond what we plan for media / media azure, this PR has been reduced to only make media paths / urls mapping working as it is but also under a virtual folder, only one file updated.