aspnet/Localization

Append ApplicationName and RelativePath doesn't make sense for types from class library in ResourceManagerStringLocalizerFactory

Closed this issue · 12 comments

See:
https://github.com/aspnet/Localization/blob/dev/src/Microsoft.Extensions.Localization/ResourceManagerStringLocalizerFactory.cs#L72

It is useful to append _applicationEnvironment.ApplicationName, _resourcesRelativePath to baseName only for types from the main application assembly. It makes impossible to use this factory for resources shared in class library assemblies.

Eilon commented

If the type name being requested doesn't start with the app name then we shouldn't use any fancy prefix logic. This way the application's configuration for localization won't interfere with class libraries doing their own localization.

this is kind of a bad bug, only solution at the moment is to fork ResourceManagerStringLocalizerFactory and change the logic, wondering why this is backlog, thought it would be fixed in rc2 but backlog suggests it won't even be fixed in RTM

For example if I have a class library with a FooController like this:

    using Microsoft.AspNetCore.Mvc;
    using Microsoft.Extensions.Localization;
    namespace WebLib.Controllers
    {
        public class FooController : Controller
        {
            public FooController(
                IStringLocalizerFactory localizerFactory
                )
            {
                _localizer = localizerFactory.Create("MyResources.Controllers.FooController", "WebLib"); ;
            }

            private readonly IStringLocalizer _localizer;

            public IActionResult Index()
            {
                ViewData["Title"] = _localizer["Foo"];
                ViewData["Message"] = _localizer["Greetings ya'll"];

                return View();
            }
        }
    }
and in the project.json I embed the MyResources folder which contains Controllers.FooController.fr.resx

"buildOptions": {
    "embed": [ "Views/**", "MyResources/**" ]
},

It works if the main web app is configured like this:

services.AddLocalization();

but as soon as the web app decides to use this configuration:

services.AddLocalization(options => options.ResourcesPath = "Resources" );

It breaks the localization in FooController even though I'm using the factory and asking for exactly the correct name of my resource

and a further note, it is currently not possible in a class library to get a working IStringLocalizer like this:

    using Microsoft.AspNetCore.Mvc;
    using Microsoft.Extensions.Localization;

    namespace WebLib.Controllers
    {
        public class OtherController : Controller
        {
            public OtherController(
                IStringLocalizer<OtherController> localizer
                )
            {
                _localizer = localizer;
            }

            private readonly IStringLocalizer _localizer;

            public IActionResult Index()
            {
                ViewData["Title"] = _localizer["Otherwise"];
                ViewData["Message"] = _localizer["this is the other one"];

                return View();
            }
        }
   }

seems like with this approach there should at least be a convention where to put my resx files and how to name them so they work, but currently it just cannot work at all, so at the moment it is not possible to localize a controller that lives in a class library using IStringLocalizer, unless you fork the factory and change the logic

If it is not intended to work and we are supposed to do localization differently in a class library then it should be documented as such

so I implemented a PatchedResourceManagerStringLocalizerFactory with changes to get both broken examples working. To use:

IStringLocalizer<OtherController> 

requires a convention to add the resx file as embedded in the root of the class library named without the root namepace like Controllers.OtherController.fr.resx

I'm not sure if this is the same way the team would fix this but here is my full implementation:

        using System;
    using System.Collections.Concurrent;
    using System.IO;
    using System.Reflection;
    using System.Resources;
    using Microsoft.AspNetCore.Hosting;
    using Microsoft.Extensions.Options;

    namespace Microsoft.Extensions.Localization
    {
        /// <summary>
        /// An <see cref="IStringLocalizerFactory"/> that creates instances of <see cref="ResourceManagerStringLocalizer"/>.
        /// </summary>
        public class PatchedResourceManagerStringLocalizerFactory : IStringLocalizerFactory
        {
            private readonly IResourceNamesCache _resourceNamesCache = new ResourceNamesCache();
            private readonly ConcurrentDictionary<string, ResourceManagerStringLocalizer> _localizerCache =
                new ConcurrentDictionary<string, ResourceManagerStringLocalizer>();
            private readonly IHostingEnvironment _hostingEnvironment;
            private readonly string _resourcesRelativePath;

            /// <summary>
            /// Creates a new <see cref="ResourceManagerStringLocalizer"/>.
            /// </summary>
            /// <param name="hostingEnvironment">The <see cref="IHostingEnvironment"/>.</param>
            /// <param name="localizationOptions">The <see cref="IOptions{LocalizationOptions}"/>.</param>
            public PatchedResourceManagerStringLocalizerFactory(
                IHostingEnvironment hostingEnvironment,
                IOptions<LocalizationOptions> localizationOptions)
            {
                if (hostingEnvironment == null)
                {
                    throw new ArgumentNullException(nameof(hostingEnvironment));
                }

                if (localizationOptions == null)
                {
                    throw new ArgumentNullException(nameof(localizationOptions));
                }

                _hostingEnvironment = hostingEnvironment;
                _resourcesRelativePath = localizationOptions.Value.ResourcesPath ?? string.Empty;
                if (!string.IsNullOrEmpty(_resourcesRelativePath))
                {
                    _resourcesRelativePath = _resourcesRelativePath.Replace(Path.AltDirectorySeparatorChar, '.')
                        .Replace(Path.DirectorySeparatorChar, '.') + ".";
                }
            }

            /// <summary>
            /// Creates a <see cref="ResourceManagerStringLocalizer"/> using the <see cref="Assembly"/> and
            /// <see cref="Type.FullName"/> of the specified <see cref="Type"/>.
            /// </summary>
            /// <param name="resourceSource">The <see cref="Type"/>.</param>
            /// <returns>The <see cref="ResourceManagerStringLocalizer"/>.</returns>
            public IStringLocalizer Create(Type resourceSource)
            {
                if (resourceSource == null)
                {
                    throw new ArgumentNullException(nameof(resourceSource));
                }

                var typeInfo = resourceSource.GetTypeInfo();
                var assembly = typeInfo.Assembly;

                // Re-root the base name if a resources path is set
                string baseName;
                if (assembly.FullName.StartsWith(_hostingEnvironment.ApplicationName))
                {
                    baseName = string.IsNullOrEmpty(_resourcesRelativePath)
                    ? typeInfo.FullName
                    : _hostingEnvironment.ApplicationName + "." + _resourcesRelativePath
                        + TrimPrefix(typeInfo.FullName, _hostingEnvironment.ApplicationName + ".");
                }
                else
                {
                    var appName = TrimOnFirstComma(assembly.FullName); 
                    baseName = appName + "." +  TrimPrefix(typeInfo.FullName, appName + ".");
                }

                return _localizerCache.GetOrAdd(baseName, _ =>
                        new ResourceManagerStringLocalizer(
                            new ResourceManager(baseName, assembly),
                            assembly,
                            baseName,
                            _resourceNamesCache)
                            );
            }

            /// <summary>
            /// Creates a <see cref="ResourceManagerStringLocalizer"/>.
            /// </summary>
            /// <param name="baseName">The base name of the resource to load strings from.</param>
            /// <param name="location">The location to load resources from.</param>
            /// <returns>The <see cref="ResourceManagerStringLocalizer"/>.</returns>
            public IStringLocalizer Create(string baseName, string location)
            {
                if (baseName == null)
                {
                    throw new ArgumentNullException(nameof(baseName));
                }

                location = location ?? _hostingEnvironment.ApplicationName;

                if(location.StartsWith(_hostingEnvironment.ApplicationName))
                {
                    baseName = location + "." + _resourcesRelativePath + TrimPrefix(baseName, location + ".");
                }
                else 
                {
                    baseName = location + "." + TrimPrefix(baseName, location + ".");
                }

                return _localizerCache.GetOrAdd($"B={baseName},L={location}", _ =>
                {
                    var assembly = Assembly.Load(new AssemblyName(location));
                    return new ResourceManagerStringLocalizer(
                        new ResourceManager(baseName, assembly),
                        assembly,
                        baseName,
                        _resourceNamesCache);
                });
            }

            private static string TrimPrefix(string name, string prefix)
            {
                if (name.StartsWith(prefix, StringComparison.Ordinal))
                {
                    return name.Substring(prefix.Length);
                }
                return name;
            }

            private static string TrimOnFirstComma(string name)
            {     
                return name.Substring(0,name.IndexOf(","));    
            }
        }
    }

one more thing I noticed that I thought I should mention, maybe it is normal/intended but seemed a little weird to me. When I plug in my PatchedResourceManagerStringLocalizerFactory and I sett a breakpoint in the else of this method:

        public IStringLocalizer Create(Type resourceSource)
        {
            if (resourceSource == null)
            {
                throw new ArgumentNullException(nameof(resourceSource));
            }

            var typeInfo = resourceSource.GetTypeInfo();
            var assembly = typeInfo.Assembly;

            // Re-root the base name if a resources path is set
            string baseName;
            if (assembly.FullName.StartsWith(_hostingEnvironment.ApplicationName))
            {
                baseName = string.IsNullOrEmpty(_resourcesRelativePath)
                ? typeInfo.FullName
                : _hostingEnvironment.ApplicationName + "." + _resourcesRelativePath
                    + TrimPrefix(typeInfo.FullName, _hostingEnvironment.ApplicationName + ".");
            }
            else
            {
                //breakpoint here
                var appName = TrimOnFirstComma(assembly.FullName); 
                baseName = appName + "." +  TrimPrefix(typeInfo.FullName, appName + ".");
            }

            return _localizerCache.GetOrAdd(baseName, _ =>
                    new ResourceManagerStringLocalizer(
                        new ResourceManager(baseName, assembly),
                        assembly,
                        baseName,
                        _resourceNamesCache)
                        );


        }

if I set that breakpoint before starting the app in the debugger, I notice 2 times that method and else clause are invoked and the type being passed in is CultureInfo. I'm not sure where the calling code that does that is, I don't think it is part of my application code. After startup I don't see that happening anymore, just the 2 times at initial startup

maybe there is a good reason for that, but it seemed unexpected to me so thought I should mention it

figured out that the 2 times when CultureInfo is passed in as the type is happening because I'm using the language selector partial view from the Localization.StarterWeb sample

when I remove that from my _Layout I no longer see those invocations. But looking at the partial view I don't see where

IStringLocalizer<CultureInfo> 

is being requested, maybe it is being requested implicitly somehow

@using Microsoft.AspNetCore.Builder
@using Microsoft.AspNetCore.Http.Features
@using Microsoft.AspNetCore.Localization
@using Microsoft.AspNetCore.Mvc.Localization
@using Microsoft.Extensions.Options
@inject IViewLocalizer Localizer
@inject IOptions<RequestLocalizationOptions> LocOptions

@{
    var requestCulture = Context.Features.Get<IRequestCultureFeature>();
    var cultureItems = LocOptions.Value.SupportedUICultures
        .Select(c => new SelectListItem { Value = c.Name, Text = c.DisplayName })
        .ToList();
}

<div title="@Localizer["Request culture provider:"] @requestCulture?.Provider?.GetType().Name">
    <form id="selectLanguage" asp-controller="Home" asp-action="SetLanguage" asp-route-returnUrl="@Context.Request.Path" method="post" class="form-horizontal" role="form">
    @Localizer["Language:"] <select name="culture" asp-for="@requestCulture.RequestCulture.UICulture.Name" asp-items="cultureItems"></select>
    </form>
</div>

If the fixes in my PatchedResourcManagerStringLocalizerFactory look correct, I would be glad to submit a PR on ResourceManagerStringLocalizerFactory

ProTip! Annotate your fenced code blocks with the language you're using, so it's easier to parse the code.

Or even better; create a gist and link to it. Right now, it's looks like a wall of text.

Example:

​```csharp
public static void Main() { }
​```

@khellang thanks for the tip, I'm kind of a noob when it comes to the finer points of markdown. I've wrapped my code as you suggested mostly, but wasn't sure whether the razor code is csharp so left that one alone. for me the formatting looked fine in the web browser even before wrapping like that so I was not aware it looked like a wall of text for anyone

for anyone else blocked by this bug or if you want to be able to localize class library web components by dropping resx files into the main web app please take a look at my new project here:
https://github.com/joeaudette/cloudscribe.Web.Localization

Eilon commented

@ryanbrandenburg please make sure we have appropriate test coverage in this area as well.

This is not a scenario which the current system supports, however we did a little refactoring of ResourceManagerStringLocalizerFactory to make it easier to subclass and implement this behavior yourself. An example of what that might look like can be found at aspnet/Entropy#159.

@joeaudette this is essentially what you had done, but now you shouldn't have to duplicate any code, just subclass ResourceManagerStringLocalizerFactory and override GetResourcePrefix to suit your needs.