[Discussion] ResourceManagerWithCultureStringLocalizer class and WithCulture interface member marked Obsolete and will be removed

[ryanbrandenburg]

The ResourceManagerWithCultureStringLocalizer class and WithCulture interface member are often sources of confusion for users of Localization, especially if they want to create their own IStringLocalizer implementation. These items give the user the impression that we expect an IStringLocalizer instance to be "per-language, per-resource", when actually they should only be "per-resource", with the language searched for determined by the CultureInfo.CurrentUICulture at execution time. To remove this source of confusion and to avoid APIs which we don't want users to use we will be obsoleting these in 3.0.0-preview3, and they will be removed in a future release (4.0 or above).

For context, see here.

---

This is the discussion issue for aspnet/Announcements#346.

[hishamco]

and they will be removed in a future release (4.0 or above).

Why we don't remove them in 3.0 while it's a major release?!!!

[ryanbrandenburg]

@hishamco our process is generally to Obsolete in a major release then remove in the next major after that.

[hishamco]

Got it .. thanks

[joakimriedel]

We're using WithCulture to specify the culture used for the localizer used in an e-mail template, when sending e-mails triggered by certain events. If I understand you correctly @ryanbrandenburg, this usage will be Obsolete.

        protected IStringLocalizer GetLocalizerForCulture<T>(string culture)
        {
            var type = typeof(T);
            var localizer = _localizerFactory.Create(type);

            var cultureInfo = String.IsNullOrEmpty(culture) ? CultureInfo.CurrentCulture : new CultureInfo(culture);
            return localizer.WithCulture(cultureInfo);
        }

        public EmailMessage GenerateDailyDigestMessage(DailyDigest digest)
        {
            var localizer = GetLocalizerForCulture<DailyDigestTemplate>(digest.User.Culture);
            var template = new DailyDigestTemplate(localizer, digest.User.FirstName);

            return new EmailMessage(template, digest.User);
        }

In this example, GenerateDailyDigestMessage will be called multiple times for different users, each having (potentially) a different culture.

Please advise what would be a suggested implementation in 3.0.

Changing the thread's locale multiple times in a loop seems a bit wrong to me.

[ryanbrandenburg]

It sounds like you already have an idea, but our proximal suggestion for that scenario is something like:

        private CultureInfo _originalCulture;

        protected void SetCurrentCulture(string culture)
        {
            _originalCulture = CultureInfo.CurrentCulture;
            var cultureInfo = String.IsNullOrEmpty(culture) ? CultureInfo.CurrentCulture : new CultureInfo(culture);
            CultureInfo.CurrentCulture = cultureInfo;
            CultureInfo.CurrentUICulture = cultureInfo;
        }

        protected void ResetCurrentCulture()
        {
            CultureInfo.CurrentCulture = _originalCulture;
            CultureInfo.CurrentUICulture = _originalCulture;
        }

        public EmailMessage GenerateDailyDigestMessage(DailyDigest digest)
        {
            try
            {
                SetCurrentCulture(digest.User.Culture);
                var localizer = _localizerFactory.Create(type);
                var template = new DailyDigestTemplate(localizer, digest.User.FirstName);
                return new EmailMessage(template, digest.User);
            }
            finally
            {
                ResetCurrentCulture();
            }
        }

That code hasn't been tested or anything, but it should be relatively close.

My more structural suggestion would be that the CurrentCulture and CurrentUICulture be set off of User.Culture when the users context is loaded (and reset when it's disposed). That way you don't have to repeat this pattern everywhere you localize in your system. If you're working inside of an MVC context that's basically what happens behind the scenes anyway, but given the implied context here I assume this is more of a "CRON job" kind of task.

[joakimriedel]

Thanks @ryanbrandenburg.

As you've guessed, this is part of a background task (IHostedService) where there are 20+ templates for e-mail and text messaging sent to thousands of users all on different locales.

I was mostly concerned about performance, concurrency or other issues that could arise from rapidly changing the current culture on the thread. Since that does not seem to be an issue, I will go with your suggestion.

Took the liberty to redesign it for the using pattern, which would pair nicely with the new C# 8.0 using directive. https://github.com/jcemoller/culture-switcher/blob/master/src/CultureSwitcher.cs

Thanks for the early notification, gives us time to adapt our systems in time for 3.0.

[ryanbrandenburg]

Looks great! Yes, this is a great candidate for a using/IDisposable structure. Glad we could help!

[martincostello]

Similar to the above, we were using WithCulture() to create a localizer for specific cultures for usage in tests. It was neat and clean.

It could also be used to pull a single string for a specific language out for inclusion in a page rendered in another language, for example in a language picker where a string in the language of what you want to switch the UI to is needed.

Now we need to wrap all such tests in try-catch blocks to change the culture of the current thread and then swap it back again, which is much less tidy.

[martincostello]

We also have a unit test that we use to validate that translations have been applied to our resource files. This is now much less trivial to achieve.

foreach (string language in SupportedOtherLanguages)
{
    var culture = new CultureInfo(language);
    var specificLocalizer = localizer.WithCulture(language);

    foreach (string resource in resourceNames)
    {
        string otherText = specificLocalizer[resource].Value;
        string englishText = baseLocalizer[resource].Value;

        otherText.ShouldNotBe(
            englishText,
            $"The value of resource '{resource}' is the same in English and {culture.EnglishName}. Does it exist in the appropriate .resx file?");
    }
}

[ryanbrandenburg]

RE try-catch: Something similar to the IDisposable pattern @jcemoller used cleans that up a lot. With regards to loading multiple languages depending on how many you needed you could either cache the result before changing languages or write a ResourceManagerStringLocalizer with behavior similar to https://github.com/aspnet/Extensions/blob/master/src/Localization/Localization/src/ResourceManagerWithCultureStringLocalizer.cs.

[dradovic]

From an API design point of view, I would expect that an IStringLocalizerFactory is able to create an IStringLocalizer which is bound to the correct CultureInfo.

Why not add a parameter there? Something like:

    IStringLocalizer Create(Type resourceSource, CultureInfo culture);

The default for culture then of course should be CurrentUICulture. (Needless to say that such a defaulting can easily be achieved by having a IStringLocalizerFactory extension method that only has a resourceSource parameter).

[hishamco]

@ryanbrandenburg I think we need to close this while it's already done

[ryanbrandenburg]

There is a bot that closes Discussion issues after a certain period, there's no need to close it outside of that framework.

[anna-git]

To generate documents, emails and for so many other usecases, we need to be able to specify the culture in the localizers. The idea of reading the culture statically within the thread is non testable, not a solution for these so common use cases, and feels somewhat dirty. Why not keep the nice mechanism that the method GetStringSafely() provided within ResourceManagerWithCultureStringLocalizer, eventually calling ResourceManager.GetString(name, culture) which seems totally made for the purpose. I understand the false "per-language, per-resource" impression with WithCulture function returning a new instance, then why not make it a void method, which would just change the _culture field value?
If I understood correctly, the solution for those common usecases is to copy the obsolete class and to keep it locally in the project. It seems like a regression, something that is gonna be missing again from the framework.

[skuami]

Like many others, I also need a StringLocalizer for email generation in a specific language.

I totaly agree with @anna-git - the suggested solutions with switching the current culture is kind of dirty. It also needs way more code to write. But on the other hand I do understand the reason to remove this method from the IStringLocalizer interface.

My suggestion for a cleaner solution would be to make an overload of the Create method with a CultureInfo parameter to the IStringLocalizerFactory.

This is my old implementation:

public StepResult(IStringLocalizer<SharedResources> localizer)
{
    _localizer = localizer;
    _localizerEnglish = localizer.WithCulture(new CultureInfo("en"));
}

And this would be the new implementation:

public StepResult(IStringLocalizer<SharedResources> localizer, IStringLocalizerFactory factory)
{
    _localizer = localizer;
    _localizerEnglish = factory.Create(typeof(SharedResources), new CultureInfo("en"));
}

[dradovic]

Exactly what I wrote too.

[FilipVanBouwel]

I'm also totally not a fan of having this method removed. Changing CurrentUICulture and reverting it every time is very annoying and makes it pretty much not unit-testable.

[skuami]

@dradovic

Exactly what I wrote too.

Sorry for the double post.
You're right. I've just read your code and there it is not obvious you meant the IStringLocalizerFactory but you describe it in the text very well.

[joshfriend]

The process of obsoleting this API somehow broke the whole localization system for SharedResoruce dotnet/sdk#4033

Is there an alternative I can use?

Also, I really strongly dislike the suggestion of modifying Culture.CurrentUICulture. It's a very messy way to work with localizations and it is extremely prone to someone forgetting to set the culture back to the old value. It also does not work because of the above issue. IStringLocalizer.WithCulture made total sense to me and was exactly what I wanted. I need to get strings for several languages at once and getting a "scoped" localizer was perfect.

Basically I am doing exactly what was described earlier in this comment: #7756 (comment)

[Cybrosys]

Hello,

I just migrated one of our projects over to .NET Core 3.1 and was working through the warnings. I then noticed that IStringLocalizer.WithCulture had been marked as Obsolete; with vague references to CurrentCulture and CurrentUICulture.

Are we expected to change the current thread's culture at runtime when requesting a specific localization?

We are in several scenarios reading the default localization as well as the user's requested localization. One of those scenarios are when we're returning an error. The error structure contains the neutral error message (In English) and the localized error message (In the user's requested language).

I see above that others also retrieve multiple localizations at the same time.

So how would you replace code similar to this:

return BadRequest(new ErrorDto
{
    Error = _localizer.WithCulture("en")["The error message"],
    LocalizedError = _localizer["The error message"],
});

What I think I will do, for now, is create a decorator for IStringLocalizer which is returned from an extension method on IStringLocalizer. That decorator will switch the thread's current culture back and forth.

[maxbeaudoin]

Temporarily forcing the thread's culture is a gross hack at best. Always been.

[iozcelik]

I think ".WithCulture" is usefull. Changing culture everytime is not the good solution and it is untestable.

[the-avid-engineer]

WithCulture should remain as a way to override the thread's culture. Maybe a better name, such as WithCultureOverride? I like hishamco's original idea of making it an extension method.

[dradovic]

I still fail to see why it's not the responsibility of the factory to create a "immutable" localizer with the correct culture instead of working with a side-effect.

Can please one of the proponents of WithCulture explain?

[the-avid-engineer]

I don’t have a strong opinion there.. I think the factory being responsible is a good solution

[BrightSoul]

Can please one of the proponents of WithCulture explain?

Sometimes I do not want a localizer for the current culture. Instead, I want to be able to get multiple localizers for lots of different cultures because I'm in a loop and I'm sending localized e-mails.
I'm using this in production.

The hack proposed by @ryanbrandenburg would indeed work but it would be just that - a hack.

are often sources of confusion for users

If something's confusing, then the solution is to explain it better, not to remove it altogether. There are people who are relying on this functionality. I hope Microsoft re-thinks this decision.

Moreover, the Obsolete description is not helpful at all.

This method is obsolete. Use `CurrentCulture` and `CurrentUICulture` instead.

Suddenly, I'm asked to understand how string localizers work internally and deal with their black magic. This is confusing.
If the solution is to use the overload of IStringLocalizerFactory.Create accepting a CultureInfo, it should mention that.

[dradovic]

@BrightSoul of course we sometimes want a localizer in a different culture. That's not a question. By

Can please one of the proponents of WithCulture explain?

I meant, WithCulture vs. the factory pattern in order to get a localizer in a different culture.

[BrightSoul]

I meant, WithCulture vs. the factory pattern in order to get a localizer in a different culture.

In my opinion, either way is fine but I tend to prefer WithCulture because:

• People won't have to change their source code;
• The Obsolete message is poorly written and it currently points to the wrong direction. If it mentioned the factory pattern approach, it would ease the transition but that's not the case;
• I don't find it confusing at all. With* methods are widely known and used across all languages.

[rubo]

You're killing a good API. Judging by this thread, there's even no replacement for the functionality cut by this removal.
The suggested workaround of temporarily changing the current culture is awful for many reasons and is not a real solution.

This move doesn't really seem justified and the community opinion is kind of neglected. What a pity.

[joshfriend]

there's even no replacement for the functionality cut by this removal

I tried the "recommended" path hinted at by the docs and several other things for DAYS with no luck, eventually resorting to hard-coding translations of my text in code. I was using an online translation management service for my resx files, but am no longer able to do that because all my text is jumbled inside a few .cs files. It's an awful mess.

Also a reminder that "deprecation" means planning to remove something at a later date, but instead .NET 3.1 broke the API entirely.

[ivann14]

I tried the "recommended" path hinted at by the docs and several other things for DAYS with no luck, eventually resorting to hard-coding translations of my text in code. I was using an online translation management service for my resx files, but am no longer able to do that because all my text is jumbled inside a few .cs files. It's an awful mess.

Exactly my case... I used the approach mentioned above with any luck whatsoever... Can you at least fix it so that your workaround works?!

[odinnou]

Hello, I would liked to migrate an app with ~35k lines of codes from .NET Core 3.1 to .NET 5.0 and currently the only one reason preventing me from doing this is : ResourceManagerWithCultureStringLocalizer class and WithCulture interface member removed

It's very frustrating :(. I prefer stick in .NET Core 3.1 and use an obsolete method instead of use untestable hack

[hishamco]

No worry @odinnou you can move to .NET 5.0, if you did something with WithCulture you can easily use something similar to https://github.com/aspnet/Localization/blob/master/src/Microsoft.Extensions.Localization/ResourceManagerWithCultureStringLocalizer.cs, because WithCulture is nothing but localizer with specified culture (other than current culture)

[valdisiljuconoks]

@rubo exactly, this API change reminds me of usage of static ambient context dependency (thread's current culture) which leads to other nasty side-effects..

[ghost]

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!