Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ITranslationProvider.LoadTranslationsAsync() #15886

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the other changes in the PR prove to be suitable, then ILocalizationFileLocationProvider can be made async too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why, it just retrieve a set of locations

Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
using System;
using System.Globalization;
using System.Threading.Tasks;

namespace OrchardCore.Localization
namespace OrchardCore.Localization;

/// <summary>
/// Contract to manage the localization.
/// </summary>
public interface ILocalizationManager
{
/// <summary>
/// Contract to manage the localization.
/// Retrieves a dictionary for a specified culture.
/// </summary>
/// <param name="culture">The <see cref="CultureInfo"/>.</param>
/// <returns>A <see cref="CultureDictionary"/> for the specified culture.</returns>
[Obsolete("This method has been deprecated, please use GetDictionaryAsync instead.")]
CultureDictionary GetDictionary(CultureInfo culture) => GetDictionaryAsync(culture).GetAwaiter().GetResult();

/// <summary>
/// Retrieves a dictionary for a specified culture.
/// </summary>
public interface ILocalizationManager
{
/// <summary>
/// Retrieves a dictionary for a specified culture.
/// </summary>
/// <param name="culture">The <see cref="CultureInfo"/>.</param>
/// <returns>A <see cref="CultureDictionary"/> for the specified culture.</returns>
CultureDictionary GetDictionary(CultureInfo culture);
}
/// <param name="culture">The <see cref="CultureInfo"/>.</param>
/// <returns>A <see cref="CultureDictionary"/> for the specified culture.</returns>
Task<CultureDictionary> GetDictionaryAsync(CultureInfo culture);
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
using System;
using System.Threading.Tasks;
using Microsoft.Extensions.Localization;

namespace OrchardCore.Localization
namespace OrchardCore.Localization;

/// <summary>
/// Contract that extends <see cref="IStringLocalizer"/> to support pluralization.
/// </summary>
public interface IPluralStringLocalizer : IStringLocalizer
{
/// <summary>
/// Contract that extends <see cref="IStringLocalizer"/> to support pluralization.
/// Gets the localized strings.
/// </summary>
/// <param name="name">The resource name.</param>
/// <param name="arguments">Optional parameters that can be used inside the resource key.</param>
/// <returns>A list of localized strings including the plural forms.</returns>
[Obsolete("This method is deprecated, please use GetTranslationAsync instead.")]
(LocalizedString, object[]) GetTranslation(string name, params object[] arguments)
=> GetTranslationAsync(name, arguments).GetAwaiter().GetResult();

/// <summary>
/// Gets the localized strings.
/// </summary>
public interface IPluralStringLocalizer : IStringLocalizer
{
/// <summary>
/// Gets the localized strings.
/// </summary>
/// <param name="name">The resource name.</param>
/// <param name="arguments">Optional parameters that can be used inside the resource key.</param>
/// <returns>A list of localized strings including the plural forms.</returns>
(LocalizedString, object[]) GetTranslation(string name, params object[] arguments);
}
/// <param name="name">The resource name.</param>
/// <param name="arguments">Optional parameters that can be used inside the resource key.</param>
/// <returns>A list of localized strings including the plural forms.</returns>
Task<(LocalizedString, object[])> GetTranslationAsync(string name, params object[] arguments);
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
namespace OrchardCore.Localization
using System;
using System.Threading.Tasks;

namespace OrchardCore.Localization;

/// <summary>
/// Contract to provide a translations.
/// </summary>
public interface ITranslationProvider
{
/// <summary>
/// Contract to provide a translations.
/// Loads translations from a certain source for a specific culture.
/// </summary>
public interface ITranslationProvider
{
/// <summary>
/// Loads translations from a certain source for a specific culture.
/// </summary>
/// <param name="cultureName">The culture name.</param>
/// <param name="dictionary">The <see cref="CultureDictionary"/> that will contains all loaded translations.</param>
void LoadTranslations(string cultureName, CultureDictionary dictionary);
}
/// <param name="cultureName">The culture name.</param>
/// <param name="dictionary">The <see cref="CultureDictionary"/> that will contains all loaded translations.</param>
[Obsolete("This method has been deprecated, please use LoadTranslationsAsync instead.")]
void LoadTranslations(string cultureName, CultureDictionary dictionary)
=> LoadTranslationsAsync(cultureName, dictionary).GetAwaiter().GetResult();

/// <summary>
/// Loads translations from a certain source for a specific culture.
/// </summary>
/// <param name="cultureName">The culture name.</param>
/// <param name="dictionary">The <see cref="CultureDictionary"/> that will contains all loaded translations.</param>
Task LoadTranslationsAsync(string cultureName, CultureDictionary dictionary);
}
Original file line number Diff line number Diff line change
@@ -1,66 +1,64 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Caching.Memory;

namespace OrchardCore.Localization
namespace OrchardCore.Localization;

/// <summary>
/// Represents a manager that manage the localization resources.
/// </summary>
public class LocalizationManager : ILocalizationManager
{
private const string CacheKeyPrefix = "CultureDictionary-";

private static readonly PluralizationRuleDelegate _defaultPluralRule = n => (n != 1 ? 1 : 0);

private readonly IList<IPluralRuleProvider> _pluralRuleProviders;
private readonly IEnumerable<ITranslationProvider> _translationProviders;
private readonly IMemoryCache _cache;

/// <summary>
/// Represents a manager that manage the localization resources.
/// Creates a new instance of <see cref="LocalizationManager"/>.
/// </summary>
public class LocalizationManager : ILocalizationManager
/// <param name="pluralRuleProviders">A list of <see cref="IPluralRuleProvider"/>s.</param>
/// <param name="translationProviders">The list of available <see cref="ITranslationProvider"/>.</param>
/// <param name="cache">The <see cref="IMemoryCache"/>.</param>
public LocalizationManager(
IEnumerable<IPluralRuleProvider> pluralRuleProviders,
IEnumerable<ITranslationProvider> translationProviders,
IMemoryCache cache)
{
private const string CacheKeyPrefix = "CultureDictionary-";

private static readonly PluralizationRuleDelegate _defaultPluralRule = n => (n != 1 ? 1 : 0);

private readonly IList<IPluralRuleProvider> _pluralRuleProviders;
private readonly IEnumerable<ITranslationProvider> _translationProviders;
private readonly IMemoryCache _cache;
_pluralRuleProviders = pluralRuleProviders.OrderBy(o => o.Order).ToArray();
_translationProviders = translationProviders;
_cache = cache;
}

/// <summary>
/// Creates a new instance of <see cref="LocalizationManager"/>.
/// </summary>
/// <param name="pluralRuleProviders">A list of <see cref="IPluralRuleProvider"/>s.</param>
/// <param name="translationProviders">The list of available <see cref="ITranslationProvider"/>.</param>
/// <param name="cache">The <see cref="IMemoryCache"/>.</param>
public LocalizationManager(
IEnumerable<IPluralRuleProvider> pluralRuleProviders,
IEnumerable<ITranslationProvider> translationProviders,
IMemoryCache cache)
/// <inheritdoc />
public async Task<CultureDictionary> GetDictionaryAsync(CultureInfo culture)
{
var cachedDictionary = await _cache.GetOrCreateAsync(CacheKeyPrefix + culture.Name, async (e) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sicne we don't want entries to be revoked here (we don't, right? even on memory pressure), we can use a keyed Dictionary and remove the prefix on the key (because there can't be conflicts anymore). I did it recently, I am sure you will recall.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use ConccurentDictionary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a ConcurrentDictionary actually since a culture could be read while another one is being added to it. But still a keyed service so we don't have to use IMemoryCache. We should only use it when we need expiration rules which we don't want here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a comment in the code to remember why we need a ConcurrentDictionary (I will forget myself)

{
_pluralRuleProviders = pluralRuleProviders.OrderBy(o => o.Order).ToArray();
_translationProviders = translationProviders;
_cache = cache;
}
var rule = _defaultPluralRule;

/// <inheritdoc />
public CultureDictionary GetDictionary(CultureInfo culture)
{
var cachedDictionary = _cache.GetOrCreate(CacheKeyPrefix + culture.Name, k => new Lazy<CultureDictionary>(() =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Lazyness here is important as it prevents r-entrancy. Without it multiple threads (concurrent requests?) can get into the body and run. The MemoryCache is not blocking on the factory body. With the Lazy, all concurrent requests can potentially create a Lazy instance, but only one will be added to the cache and executed.

foreach (var provider in _pluralRuleProviders)
{
var rule = _defaultPluralRule;

foreach (var provider in _pluralRuleProviders)
if (provider.TryGetRule(culture, out rule))
{
if (provider.TryGetRule(culture, out rule))
{
break;
}
break;
}
}

var dictionary = new CultureDictionary(culture.Name, rule ?? _defaultPluralRule);
foreach (var translationProvider in _translationProviders)
{
translationProvider.LoadTranslations(culture.Name, dictionary);
}
var dictionary = new CultureDictionary(culture.Name, rule ?? _defaultPluralRule);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LoadTranslationsAsync should be move to a private AwaitedLoadtranslationAsync method with the async/await keywords such that we don't need it in the GetDictionaryAsync.

foreach (var translationProvider in _translationProviders)
{
await translationProvider.LoadTranslationsAsync(culture.Name, dictionary);
}

return dictionary;
}, LazyThreadSafetyMode.ExecutionAndPublication));
return dictionary;
});

return cachedDictionary.Value;
}
return cachedDictionary;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
<FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Linq.Async" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's necessary to convert from IAsyncEnumerable to Enumberable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still builds if I remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really!! I got 4 errors, how the IAsyncEnumerable extension methods be there without referencing a proper package?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno :).


<ItemGroup>
<ProjectReference Include="..\OrchardCore.Localization.Abstractions\OrchardCore.Localization.Abstractions.csproj" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,43 +1,44 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Extensions.FileProviders;

namespace OrchardCore.Localization.PortableObject
namespace OrchardCore.Localization.PortableObject;

/// <summary>
/// Represents a provider that provides a translations for .po files.
/// </summary>
public class PoFilesTranslationsProvider : ITranslationProvider
{
private readonly ILocalizationFileLocationProvider _poFilesLocationProvider;
private readonly PoParser _parser;

/// <summary>
/// Represents a provider that provides a translations for .po files.
/// Creates a new instance of <see cref="PoFilesTranslationsProvider"/>.
/// </summary>
public class PoFilesTranslationsProvider : ITranslationProvider
/// <param name="poFileLocationProvider">The <see cref="ILocalizationFileLocationProvider"/>.</param>
public PoFilesTranslationsProvider(ILocalizationFileLocationProvider poFileLocationProvider)
{
private readonly ILocalizationFileLocationProvider _poFilesLocationProvider;
private readonly PoParser _parser;

/// <summary>
/// Creates a new instance of <see cref="PoFilesTranslationsProvider"/>.
/// </summary>
/// <param name="poFileLocationProvider">The <see cref="ILocalizationFileLocationProvider"/>.</param>
public PoFilesTranslationsProvider(ILocalizationFileLocationProvider poFileLocationProvider)
{
_poFilesLocationProvider = poFileLocationProvider;
_parser = new PoParser();
}
_poFilesLocationProvider = poFileLocationProvider;
_parser = new PoParser();
}

/// <inheritdocs />
public void LoadTranslations(string cultureName, CultureDictionary dictionary)
/// <inheritdocs />
public async Task LoadTranslationsAsync(string cultureName, CultureDictionary dictionary)
{
foreach (var fileInfo in _poFilesLocationProvider.GetLocations(cultureName))
{
foreach (var fileInfo in _poFilesLocationProvider.GetLocations(cultureName))
{
LoadFileToDictionary(fileInfo, dictionary);
}
await LoadFileToDictionaryAsync(fileInfo, dictionary);
}
}

private void LoadFileToDictionary(IFileInfo fileInfo, CultureDictionary dictionary)
private async Task LoadFileToDictionaryAsync(IFileInfo fileInfo, CultureDictionary dictionary)
{
if (fileInfo.Exists && !fileInfo.IsDirectory)
{
if (fileInfo.Exists && !fileInfo.IsDirectory)
{
using var stream = fileInfo.CreateReadStream();
using var reader = new StreamReader(stream);
dictionary.MergeTranslations(_parser.Parse(reader));
}
using var stream = fileInfo.CreateReadStream();
using var reader = new StreamReader(stream);
dictionary.MergeTranslations(await _parser.ParseAsync(reader).ToListAsync());
}
}
}
Loading