Merge pull request #120 from sillsdev/BL13266_StrictInitialization

Throw if the library is used before being initialized (BL-13266)

CHANGELOG.md

@@ -16,128 +16,132 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Changed
+
+-   BREAKING CHANGE: If no `LocalizationManager`s have been created, but the client asks for a string to be localized, an `InvalidOperationException` is thrown. This is to prevent an invalid state where language IDs get mapped incorrectly at the beginning and then never get updated which can cause us to fail to return properly localized strings when requested (see BL-13245). This is a breaking change because it may cause existing code to throw an exception. The fix is to ensure that a LocalizationManager is created before calling any localization methods. Or, to maintain existing behavior, set `LocalizationManager.StrictInitializationMode` to false.
+
 ## [7.0.0] - 2023-11-03
 
 ### Added
 
-- `LocalizationManager.Create` methods without `TranslationMemory kind` parameter
+-   `LocalizationManager.Create` methods without `TranslationMemory kind` parameter
 
 ### Fixed
 
-- `LocalizationManager.Create("es"` loads `es-ES` if it is the best match (previously, this resulted in a dialog making the user choose)
+-   `LocalizationManager.Create("es"` loads `es-ES` if it is the best match (previously, this resulted in a dialog making the user choose)
 
 ### Deprecated
 
-- `LocalizationManager.Create` methods with `TranslationMemory kind` parameter
+-   `LocalizationManager.Create` methods with `TranslationMemory kind` parameter
 
 ## [6.0.0] - 2022-11-21
 
 ### Changed
 
-- Added cleanUpTmx parameter to LocalizationManager.DeleteOldTranslationFiles to allow for cleanup of old TMX files.
+-   Added cleanUpTmx parameter to LocalizationManager.DeleteOldTranslationFiles to allow for cleanup of old TMX files.
 
 ### Removed
 
-- TMX-based localization no longer supported
-- LocalizationManager.GetTranslationFileNameForLanguage is no longer public.
+-   TMX-based localization no longer supported
+-   LocalizationManager.GetTranslationFileNameForLanguage is no longer public.
 
 ## [5.0.0] - 2022-07-08
 
 ### Added
 
-- option `LocalizationManager.ThrowIfManagerDisposed` to not throw if LM disposed (BL-9904)
-- XliffBody.TransUnitsUnordered for where you just need to enumerate all of them.
-- (Made public) XliffBody.AddTransUnit and .RemoveTransUnit for where you need to modify.
-- XliffBody.TransUnitsForXml. This is necessarily public to support (backwards-compatible)
+-   option `LocalizationManager.ThrowIfManagerDisposed` to not throw if LM disposed (BL-9904)
+-   XliffBody.TransUnitsUnordered for where you just need to enumerate all of them.
+-   (Made public) XliffBody.AddTransUnit and .RemoveTransUnit for where you need to modify.
+-   XliffBody.TransUnitsForXml. This is necessarily public to support (backwards-compatible)
     serialization and deserialization in XML, but is not intended for any other purpose.
 
 ### Changed
 
-- Scanning resources for strings no longer rethrows unexpected exceptions.  It now writes the
-  exception (and stack trace) using a (conditional) Console.WriteLine and a Debug.WriteLine.
-  Rethrowing the exception leads to creating a zero-length xliff file which causes another
-  exception.  Swallowing the exception allows the scanning process to continue and complete.
-  The old behavior has been an endless source of periodic instability in using L10NSharp over
-  the years.
-- remove progress dialog when initializating Xliff localization managers (BL-11157)
-- Made string retrieval operations on Xliff-based LocalizationManagers thread-safe
-- Added ILocalizationManager parameter to StringsLocalizedHandler
-- It's long been a convention that xliff file names are module.lang.xlf (e.g., Bloom.fr.xlf)
-or else kept in language-code folders (.../en/Bloom.xlf) if UseLanguageCodeFolders is set.
-With the latest changes, this is required: the language name indicated in these ways in the file
-name must match the language declared in the target-language attribute, or at least match the
-first element of the target-language (e.g., a file with target-languge es-ES may be stored in
-file like Bloom.es.xlf or .../es/Bloom.xlf).
-- Progress dialogs are no longer shown when initializing XLIFF-based LocalizationManagers.
-- Made it possible for caller to specify the file extension of the "original" executable file
-when constructing an XLIFF-based LocalizationManager.
-- Changed the way the "original" attribute is set in XLIFF files. It used to be based on the
-Name, but changed it to use Id instead.
-- Added optional owner parameter to methods that show dialog boxes so that they can be
-displayed centered on a parent window (and not appear off-screen).
+-   Scanning resources for strings no longer rethrows unexpected exceptions. It now writes the
+    exception (and stack trace) using a (conditional) Console.WriteLine and a Debug.WriteLine.
+    Rethrowing the exception leads to creating a zero-length xliff file which causes another
+    exception. Swallowing the exception allows the scanning process to continue and complete.
+    The old behavior has been an endless source of periodic instability in using L10NSharp over
+    the years.
+-   remove progress dialog when initializating Xliff localization managers (BL-11157)
+-   Made string retrieval operations on Xliff-based LocalizationManagers thread-safe
+-   Added ILocalizationManager parameter to StringsLocalizedHandler
+-   It's long been a convention that xliff file names are module.lang.xlf (e.g., Bloom.fr.xlf)
+    or else kept in language-code folders (.../en/Bloom.xlf) if UseLanguageCodeFolders is set.
+    With the latest changes, this is required: the language name indicated in these ways in the file
+    name must match the language declared in the target-language attribute, or at least match the
+    first element of the target-language (e.g., a file with target-languge es-ES may be stored in
+    file like Bloom.es.xlf or .../es/Bloom.xlf).
+-   Progress dialogs are no longer shown when initializing XLIFF-based LocalizationManagers.
+-   Made it possible for caller to specify the file extension of the "original" executable file
+    when constructing an XLIFF-based LocalizationManager.
+-   Changed the way the "original" attribute is set in XLIFF files. It used to be based on the
+    Name, but changed it to use Id instead.
+-   Added optional owner parameter to methods that show dialog boxes so that they can be
+    displayed centered on a parent window (and not appear off-screen).
 
 ### Removed
 
-- XliffBody.TransUnits, as there is no good way to make this thread-safe for all the ways
+-   XliffBody.TransUnits, as there is no good way to make this thread-safe for all the ways
     it could be used, such as adding items to the list. (See Added for replacements.)
 
 ## [4.1.0] - 2021-03-04
 
 ### Changed
 
-- Add `ExtractXliff` tool as nuget package
-- Add `CheckOrFixXliff` tool as nuget package
-- Added version of LocalizationManager.Create to allow "custom" localization methods
-- Added -m switch to ExtractXliff command-line to allow caller to pass additional string-localization methods
+-   Add `ExtractXliff` tool as nuget package
+-   Add `CheckOrFixXliff` tool as nuget package
+-   Added version of LocalizationManager.Create to allow "custom" localization methods
+-   Added -m switch to ExtractXliff command-line to allow caller to pass additional string-localization methods
 
 ## [4.0.3] - 2020-01-21
 
 ### Changed
 
-- Add build number to AssemblyFileVersion
+-   Add build number to AssemblyFileVersion
 
 ## [4.0.2] - 2019-07-09
 
 ### Fixed
 
-- If translator returns an unmodified source string, don't substitute the English language name for the vernacular name.
+-   If translator returns an unmodified source string, don't substitute the English language name for the vernacular name.
 
 ## [4.0.1] - 2019-07-08
 
 ### Added
 
-- create symbol nuget package
+-   create symbol nuget package
 
 ### Fixed
 
-- Find TMX files in `Generated` and `User Modified` directories
+-   Find TMX files in `Generated` and `User Modified` directories
 
-- Don't ask Bing translator to translate language names: https://github.com/sillsdev/l10nsharp/issues/66.
-  Also don't display the name a second time in parentheses if English and native name are identical.
+-   Don't ask Bing translator to translate language names: https://github.com/sillsdev/l10nsharp/issues/66.
+    Also don't display the name a second time in parentheses if English and native name are identical.
 
 ## [4.0.0] - 2019-05-16
 
 ### Changed
 
-- Allow to select translation memory (TMX or XLIFF). This changed a few APIs.
-  To create a `LocalizationManager` you now pass a `TranslationMemory` parameter
-  (cf. [migration](https://github.com/sillsdev/l10nsharp/wiki/Migration) guide):
+-   Allow to select translation memory (TMX or XLIFF). This changed a few APIs.
+    To create a `LocalizationManager` you now pass a `TranslationMemory` parameter
+    (cf. [migration](https://github.com/sillsdev/l10nsharp/wiki/Migration) guide):
 
-      LocalizationManager.Create(TranslationMemory.XLiff, lang, "SampleApp", "SampleApp",
-        Application.ProductVersion, directoryOfInstalledXliffFiles, "MyCompany/L10NSharpSample",
-        icon, "sample@example.com", "SampleApp");
+        LocalizationManager.Create(TranslationMemory.XLiff, lang, "SampleApp", "SampleApp",
+          Application.ProductVersion, directoryOfInstalledXliffFiles, "MyCompany/L10NSharpSample",
+          icon, "sample@example.com", "SampleApp");
 
-- Nuget package is now called `L10NSharp` instead of `L10NSharp.xliff` or `L10NSharp.tmx`
+-   Nuget package is now called `L10NSharp` instead of `L10NSharp.xliff` or `L10NSharp.tmx`
 
 ## [3.1.1] - 2019-04-26
 
 ### Fixed
 
-- Create .exe for `CheckOrFixXliff` and `ExtractXliff` instead of .dll
+-   Create .exe for `CheckOrFixXliff` and `ExtractXliff` instead of .dll
 
 ## [3.1.0] - 2019-04-16
 
 ### Changed
 
-- Create nuget package
-- Strong-name assembly
+-   Create nuget package
+-   Strong-name assembly

src/L10NSharp/LocalizationManager.cs

@@ -839,5 +839,14 @@ internal static void ClearLoadedManagers()
 		/// the localization managers.
 		/// </summary>
 		public static bool ThrowIfManagerDisposed = true;
+
+		/// <summary>
+		/// True (default) to throw if we try to get a localized string before creating any localization managers.
+		/// This is to prevent an invalid state where language IDs get mapped incorrectly at the beginning and
+		/// then never get updated which can cause us to fail to return properly localized strings when requested (see BL-13245).
+		/// The fix is to ensure that a LocalizationManager is created before calling any localization methods.
+		/// Or, to maintain prior behavior, set this to false.
+		/// </summary>
+		public static bool StrictInitializationMode = true;
 	}
 }

src/L10NSharp/LocalizationManagerInternal.cs

@@ -665,6 +665,12 @@ internal static string MapToExistingLanguageIfPossible(string langId)
 		/// ------------------------------------------------------------------------------------
 		public static bool GetIsStringAvailableForLangId(string id, string langId)
 		{
+			if (LocalizationManager.StrictInitializationMode)
+			{
+				if (LoadedManagers.Count == 0)
+					throw new InvalidOperationException("You must create at least one LocalizationManager before trying to localize any strings.");
+			}
+
 			if (string.IsNullOrEmpty(langId) || string.IsNullOrEmpty(id))
 				return false;
 
@@ -675,6 +681,12 @@ public static bool GetIsStringAvailableForLangId(string id, string langId)
 		/// ------------------------------------------------------------------------------------
 		internal static string GetStringFromAnyLocalizationManager(string stringId)
 		{
+			if (LocalizationManager.StrictInitializationMode)
+			{
+				if (LoadedManagers.Count == 0)
+					throw new InvalidOperationException("You must create at least one LocalizationManager before trying to localize any strings.");
+			}
+
 			// This will enforce that the text to localize is just returned to the caller
 			// when the default language id is the same as the current UI language id.
 			if (LocalizationManager.UILanguageId == LocalizationManager.kDefaultLang)

src/L10NSharpTests/LocalizationManagerTestsBase.cs

@@ -372,10 +372,8 @@ public void GetUiLanguages_FindsAll()
 				SetupManager(folder);
 				var cultures = new List<L10NCultureInfo>(LocalizationManager.GetUILanguages(true));
 				Assert.AreEqual(4, cultures.Count);
-				Assert.AreEqual("ar", cultures[0].IetfLanguageTag);		// Arabic
-				Assert.AreEqual("en", cultures[1].IetfLanguageTag);		// English
-				Assert.AreEqual("fr", cultures[2].IetfLanguageTag);		// French
-				Assert.AreEqual("es", cultures[3].IetfLanguageTag);		// Spanish
+				CollectionAssert.AreEquivalent(new[] { "ar", "en", "fr", "es" }, cultures.Select(c => c.IetfLanguageTag).ToArray());
+				Assert.That(cultures, Is.Ordered.By("DisplayName"));
 			}
 		}
 
@@ -390,10 +388,8 @@ public void GetUiLanguages_FindsAllWithFolders()
 					SetupManager(folder);
 					var cultures = new List<L10NCultureInfo>(LocalizationManager.GetUILanguages(true));
 					Assert.AreEqual(4, cultures.Count);
-					Assert.AreEqual("ar", cultures[0].IetfLanguageTag);		// Arabic
-					Assert.AreEqual("en", cultures[1].IetfLanguageTag);		// English
-					Assert.AreEqual("fr", cultures[2].IetfLanguageTag);		// French
-					Assert.AreEqual("es", cultures[3].IetfLanguageTag);		// Spanish
+					CollectionAssert.AreEquivalent(new[] { "ar", "en", "fr", "es" }, cultures.Select(c => c.IetfLanguageTag).ToArray());
+					Assert.That(cultures, Is.Ordered.By("DisplayName"));
 				}
 			}
 			finally

src/L10NSharpTests/LocalizationManagerTests_NoManagersLoaded.cs

@@ -2,6 +2,8 @@
 // This software is licensed under the MIT License (http://opensource.org/licenses/MIT)
 
 using NUnit.Framework;
+using System;
+using System.Threading;
 
 namespace L10NSharp.Tests
 {
@@ -24,25 +26,105 @@ public void Setup()
 		[Test]
 		public void GetDynamicString_NoManagerLoaded_EnglishNotNull_ReturnsEnglishString()
 		{
-			Assert.That(
-				LocalizationManager.GetDynamicString("Glom", "prefix.data", "data"),
-				Is.EqualTo("data"));
+			try
+			{
+				LocalizationManager.UILanguageId = "en";
+				Assert.That(
+					LocalizationManager.GetDynamicString("Glom", "prefix.data", "data"),
+					Is.EqualTo("data"));
+			}
+			finally
+			{
+				// reset
+				LocalizationManager.UILanguageId = null;
+			}
 		}
 
-		[Test]
-		public void GetDynamicString_NoManagerLoaded_EnglishNull_ReturnsId()
+		[TestCase(null)]
+		[TestCase("en")]
+		[TestCase("es")]
+		public void GetDynamicString_NoManagerLoaded_EnglishNull_ReturnsId(string uiLanguageId)
 		{
-			Assert.That(
-				LocalizationManager.GetDynamicString("Glom", "prefix.data", null),
-				Is.EqualTo("prefix.data"));
+			try
+			{
+				LocalizationManager.UILanguageId = uiLanguageId;
+				Assert.That(
+					LocalizationManager.GetDynamicString("Glom", "prefix.data", null),
+					Is.EqualTo("prefix.data"));
+			}
+			finally
+			{
+				// reset
+				LocalizationManager.UILanguageId = null;
+			}
 		}
 
-		[Test]
-		public void GetDynamicStringOrEnglish_NoManagerLoaded_NonEnglish_ReturnsId()
+		[TestCase(null)]
+		[TestCase("en")]
+		[TestCase("es")]
+		public void GetDynamicStringOrEnglish_NoManagerLoaded_NonEnglish_ReturnsId(string uiLanguageId)
 		{
-			Assert.That(
-				LocalizationManager.GetDynamicStringOrEnglish("Glom", "prefix.data", "data", "no comment", "es"),
-				Is.EqualTo("prefix.data"));
+			try
+			{
+				LocalizationManager.UILanguageId = uiLanguageId;
+				Assert.That(
+					LocalizationManager.GetDynamicStringOrEnglish("Glom", "prefix.data", "data", "no comment", "es"),
+					Is.EqualTo("prefix.data"));
+			}
+			finally
+			{
+				// reset
+				LocalizationManager.UILanguageId = null;
+			}
+		}
+
+		[TestCase("en")]
+		[TestCase("en-US")]
+		[TestCase("es")]
+		[TestCase("es-ES")]
+		[TestCase("es-MX")]
+		public void GetString_NoManagerLoaded_StrictInitializationModeTrue_Throws(string cultureName)
+		{
+			System.Globalization.CultureInfo previousCurrentCulture = null;
+			try
+			{
+				previousCurrentCulture = Thread.CurrentThread.CurrentUICulture;
+
+				Thread.CurrentThread.CurrentUICulture = new System.Globalization.CultureInfo(cultureName);
+
+				// default is true
+				Assert.Throws<InvalidOperationException>(() => LocalizationManager.GetString("prefix.id", "data"));
+
+			}
+			finally
+			{
+				Thread.CurrentThread.CurrentUICulture = previousCurrentCulture;
+				LocalizationManager.UILanguageId = null;
+			}
+		}
+
+		[TestCase("en")]
+		[TestCase("en-US")]
+		[TestCase("es")]
+		[TestCase("es-ES")]
+		[TestCase("es-MX")]
+		public void GetString_NoManagerLoaded_StrictInitializationModeFalse_DoesNotThrow(string cultureName)
+		{
+			System.Globalization.CultureInfo previousCurrentCulture = null;
+			try
+			{
+				previousCurrentCulture = Thread.CurrentThread.CurrentUICulture;
+
+				LocalizationManager.StrictInitializationMode = false;
+				Assert.DoesNotThrow(() => LocalizationManager.GetString("prefix.id", "data"));
+			}
+			finally
+			{
+				Thread.CurrentThread.CurrentUICulture = previousCurrentCulture;
+				LocalizationManager.UILanguageId = null;
+
+				LocalizationManager.StrictInitializationMode = true;
+			}
 		}
 	}
 }

src/L10NSharpTests/XLiffLocalizationManagerTests.cs

@@ -29,6 +29,7 @@ internal override ILocalizationManagerInternal<XLiffDocument> CreateLocalization
 				directoryForGeneratedDefaultFile, directoryOfUserModifiedXliffFiles, additionalGetStringMethodInfo,
 				namespaceBeginnings);
 			Assert.That(manager.OriginalExecutableFile, Is.EqualTo(appId + ".dll"));
+			LocalizationManagerInternal<XLiffDocument>.LoadedManagers.Add("myAppId", manager);
 			return manager;
 		}