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 all 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
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ public interface ILocalizationManager
/// </summary>
/// <param name="culture">The <see cref="CultureInfo"/>.</param>
/// <returns>A <see cref="CultureDictionary"/> for the specified culture.</returns>
CultureDictionary GetDictionary(CultureInfo culture);
Task<CultureDictionary> GetDictionaryAsync(CultureInfo culture);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ public interface ITranslationProvider
/// </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);
Task LoadTranslationsAsync(string cultureName, CultureDictionary dictionary);
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ public LocalizationManager(
}

/// <inheritdoc />
public CultureDictionary GetDictionary(CultureInfo culture)
public async Task<CultureDictionary> GetDictionaryAsync(CultureInfo culture)
{
var cachedDictionary = _cache.GetOrCreate(CacheKeyPrefix + culture.Name, k => new Lazy<CultureDictionary>(() =>
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)

{
var rule = _defaultPluralRule;

Expand All @@ -50,12 +50,12 @@ public CultureDictionary GetDictionary(CultureInfo culture)
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)
{
translationProvider.LoadTranslations(culture.Name, dictionary);
await translationProvider.LoadTranslationsAsync(culture.Name, dictionary);
}

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

return cachedDictionary.Value;
return cachedDictionary;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Linq.Async" />
<PackageReference Include="ZString" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,21 @@ public PoFilesTranslationsProvider(ILocalizationFileLocationProvider poFileLocat
}

/// <inheritdocs />
public void LoadTranslations(string cultureName, CultureDictionary dictionary)
public async Task LoadTranslationsAsync(string cultureName, CultureDictionary dictionary)
{
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)
{
using var stream = fileInfo.CreateReadStream();
using var reader = new StreamReader(stream);
dictionary.MergeTranslations(_parser.Parse(reader));
dictionary.MergeTranslations(await _parser.ParseAsync(reader).ToListAsync());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ static PoParser()
/// <param name="reader">The <see cref="TextReader"/>.</param>
/// <returns>A list of culture records.</returns>
#pragma warning disable CA1822 // Mark members as static
public IEnumerable<CultureDictionaryRecord> Parse(TextReader reader)
public async IAsyncEnumerable<CultureDictionaryRecord> ParseAsync(TextReader reader)
#pragma warning restore CA1822 // Mark members as static
{
var entryBuilder = new DictionaryRecordBuilder();
string line;
while ((line = reader.ReadLine()) != null)
while ((line = await reader.ReadLineAsync()) != null)
{
(var context, var content) = ParseLine(line);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ public virtual LocalizedString this[string name]
public virtual IEnumerable<LocalizedString> GetAllStrings(bool includeParentCultures)
{
var culture = CultureInfo.CurrentUICulture;
var localizedStrings = includeParentCultures
? GetAllStringsFromCultureHierarchyAsync(culture)
: GetAllStringsAsync(culture);

return includeParentCultures
? GetAllStringsFromCultureHierarchy(culture)
: GetAllStrings(culture);
return localizedStrings.ToEnumerable();
}

/// <inheritdocs />
Expand Down Expand Up @@ -106,45 +107,45 @@ public virtual (LocalizedString, object[]) GetTranslation(string name, params ob
}
}

private IEnumerable<LocalizedString> GetAllStrings(CultureInfo culture)
private async IAsyncEnumerable<LocalizedString> GetAllStringsAsync(CultureInfo culture)
{
var dictionary = _localizationManager.GetDictionary(culture);
var dictionary = await _localizationManager.GetDictionaryAsync(culture);

foreach (var translation in dictionary.Translations)
{
yield return new LocalizedString(translation.Key, translation.Value.FirstOrDefault());
}
}

private List<LocalizedString> GetAllStringsFromCultureHierarchy(CultureInfo culture)
private async IAsyncEnumerable<LocalizedString> GetAllStringsFromCultureHierarchyAsync(CultureInfo culture)
{
var currentCulture = culture;
var allLocalizedStrings = new List<LocalizedString>();
var resourcesNames = new HashSet<string>();

do
{
var localizedStrings = GetAllStrings(currentCulture);
var localizedStrings = await GetAllStringsAsync(currentCulture).ToListAsync();

if (localizedStrings != null)
{
foreach (var localizedString in localizedStrings)
{
if (!allLocalizedStrings.Any(ls => ls.Name == localizedString.Name))
if (!resourcesNames.Contains(localizedString.Name))
{
allLocalizedStrings.Add(localizedString);
resourcesNames.Add(localizedString.Name);

yield return localizedString;
}
}
}

currentCulture = currentCulture.Parent;
} while (currentCulture != currentCulture.Parent);

return allLocalizedStrings;
}

protected string GetTranslation(string[] pluralForms, CultureInfo culture, int? count)
hishamco marked this conversation as resolved.
Show resolved Hide resolved
{
var dictionary = _localizationManager.GetDictionary(culture);
var dictionary = _localizationManager.GetDictionaryAsync(culture).GetAwaiter().GetResult();

var pluralForm = count.HasValue ? dictionary.PluralRule(count.Value) : 0;

Expand Down Expand Up @@ -187,7 +188,7 @@ protected string GetTranslation(string name, string context, CultureInfo culture

string ExtractTranslation()
{
var dictionary = _localizationManager.GetDictionary(culture);
var dictionary = _localizationManager.GetDictionaryAsync(culture).GetAwaiter().GetResult();

if (dictionary != null)
{
Expand Down
52 changes: 26 additions & 26 deletions test/OrchardCore.Tests/Localization/LocalizationManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,57 +24,57 @@ public LocalizationManagerTests()
_memoryCache = new MemoryCache(new MemoryCacheOptions());
}

[Fact]
public void GetDictionaryReturnsDictionaryWithPluralRuleAndCultureIfNoTranslationsExists()
{
_translationProvider.Setup(o => o.LoadTranslations(
It.Is<string>(culture => culture == "cs"),
It.IsAny<CultureDictionary>())
);
[Fact]
public async Task GetDictionaryReturnsDictionaryWithPluralRuleAndCultureIfNoTranslationsExists()
{
_translationProvider.Setup(o => o.LoadTranslationsAsync(
It.Is<string>(culture => culture == "cs"),
It.IsAny<CultureDictionary>())
);

var manager = new LocalizationManager(new[] { _pluralRuleProvider.Object }, new[] { _translationProvider.Object }, _memoryCache);

var dictionary = manager.GetDictionary(CultureInfo.GetCultureInfo("cs"));
var dictionary = await manager.GetDictionaryAsync(CultureInfo.GetCultureInfo("cs"));

Assert.Equal("cs", dictionary.CultureName);
Assert.Equal(PluralizationRule.Czech, dictionary.PluralRule);
}

[Fact]
public void GetDictionaryReturnsDictionaryWithTranslationsFromProvider()
{
var dictionaryRecord = new CultureDictionaryRecord("ball", "míč", "míče", "míčů");
_translationProvider
.Setup(o => o.LoadTranslations(It.Is<string>(culture => culture == "cs"), It.IsAny<CultureDictionary>()))
.Callback<string, CultureDictionary>((culture, dictioanry) => dictioanry.MergeTranslations(new[] { dictionaryRecord }));
[Fact]
public async Task GetDictionaryReturnsDictionaryWithTranslationsFromProvider()
{
var dictionaryRecord = new CultureDictionaryRecord("ball", "míč", "míče", "míčů");
_translationProvider
.Setup(o => o.LoadTranslationsAsync(It.Is<string>(culture => culture == "cs"), It.IsAny<CultureDictionary>()))
.Callback<string, CultureDictionary>((culture, dictioanry) => dictioanry.MergeTranslations(new[] { dictionaryRecord }));

var manager = new LocalizationManager([_pluralRuleProvider.Object], [_translationProvider.Object], _memoryCache);

var dictionary = manager.GetDictionary(CultureInfo.GetCultureInfo("cs"));
var key = new CultureDictionaryRecordKey { MessageId = "ball" };
var dictionary = await manager.GetDictionaryAsync(CultureInfo.GetCultureInfo("cs"));
var key = new CultureDictionaryRecordKey { MessageId = "ball" };

dictionary.Translations.TryGetValue(key, out var translations);

Assert.Equal(translations, dictionaryRecord.Translations);
}

[Fact]
public void GetDictionarySelectsPluralRuleFromProviderWithHigherPriority()
{
PluralizationRuleDelegate csPluralRuleOverride = n => ((n == 1) ? 0 : (n >= 2 && n <= 4) ? 1 : 0);
[Fact]
public async Task GetDictionarySelectsPluralRuleFromProviderWithHigherPriority()
{
PluralizationRuleDelegate csPluralRuleOverride = n => ((n == 1) ? 0 : (n >= 2 && n <= 4) ? 1 : 0);

var highPriorityRuleProvider = new Mock<IPluralRuleProvider>();
highPriorityRuleProvider.SetupGet(o => o.Order).Returns(-1);
highPriorityRuleProvider.Setup(o => o.TryGetRule(It.Is<CultureInfo>(culture => culture.Name == "cs"), out csPluralRuleOverride)).Returns(true);

_translationProvider.Setup(o => o.LoadTranslations(
It.Is<string>(culture => culture == "cs"),
It.IsAny<CultureDictionary>())
);
_translationProvider.Setup(o => o.LoadTranslationsAsync(
It.Is<string>(culture => culture == "cs"),
It.IsAny<CultureDictionary>())
);

var manager = new LocalizationManager([_pluralRuleProvider.Object, highPriorityRuleProvider.Object], [_translationProvider.Object], _memoryCache);

var dictionary = manager.GetDictionary(CultureInfo.GetCultureInfo("cs"));
var dictionary = await manager.GetDictionaryAsync(CultureInfo.GetCultureInfo("cs"));

Assert.Equal(dictionary.PluralRule, csPluralRuleOverride);
}
Expand Down
Loading
Loading