From 0eaacc266eab53b5408bccb7a25d79e941bbfcae Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Tue, 29 Nov 2022 23:09:57 -0500 Subject: [PATCH] i18n: Fix bug where package-level variables are not translated. Change i18n.T() to load translations if they have not yet been loaded. Added new integration tests to test help output translation. Kubernetes-commit: c0dea5e31af856ed96b8257b5caa952161c8a05b --- pkg/cmd/cmd.go | 7 -- pkg/util/i18n/i18n.go | 67 ++++++++++++++++++- pkg/util/i18n/i18n_test.go | 131 +++++++++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 88e3a583..8313179b 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -331,13 +331,6 @@ func NewKubectlCommand(o KubectlOptions) *cobra.Command { f := cmdutil.NewFactory(matchVersionKubeConfigFlags) - // Sending in 'nil' for the getLanguageFn() results in using - // the LANG environment variable. - // - // TODO: Consider adding a flag or file preference for setting - // the language, instead of just loading from the LANG env. variable. - i18n.LoadTranslations("kubectl", nil) - // Proxy command is incompatible with CommandHeaderRoundTripper, so // clear the WrapConfigFn before running proxy command. proxyCmd := proxy.NewCmdProxy(f, o.IOStreams) diff --git a/pkg/util/i18n/i18n.go b/pkg/util/i18n/i18n.go index 4aebe318..d850b283 100644 --- a/pkg/util/i18n/i18n.go +++ b/pkg/util/i18n/i18n.go @@ -24,8 +24,10 @@ import ( "fmt" "os" "strings" + "sync" + + "github.com/chai2010/gettext-go" - gettext "github.com/chai2010/gettext-go" "k8s.io/klog/v2" ) @@ -52,6 +54,56 @@ var knownTranslations = map[string][]string{ }, } +var ( + lazyLoadTranslationsOnce sync.Once + LoadTranslationsFunc = func() error { + return LoadTranslations("kubectl", nil) + } + translationsLoaded bool +) + +// SetLoadTranslationsFunc sets the function called to lazy load translations. +// It must be called in an init() func that occurs BEFORE any i18n.T() calls are made by any package. You can +// accomplish this by creating a separate package containing your init() func, and then importing that package BEFORE +// any other packages that call i18n.T(). +// +// Example Usage: +// +// package myi18n +// +// import "k8s.io/kubectl/pkg/util/i18n" +// +// func init() { +// if err := i18n.SetLoadTranslationsFunc(loadCustomTranslations); err != nil { +// panic(err) +// } +// } +// +// func loadCustomTranslations() error { +// // Load your custom translations here... +// } +// +// And then in your main or root command package, import your custom package like this: +// +// import ( +// // Other imports that don't need i18n... +// _ "example.com/myapp/myi18n" +// // Other imports that do need i18n... +// ) +func SetLoadTranslationsFunc(f func() error) error { + if translationsLoaded { + return errors.New("translations have already been loaded") + } + LoadTranslationsFunc = func() error { + if err := f(); err != nil { + return err + } + translationsLoaded = true + return nil + } + return nil +} + func loadSystemLanguage() string { // Implements the following locale priority order: LC_ALL, LC_MESSAGES, LANG // Similarly to: https://www.gnu.org/software/gettext/manual/html_node/Locale-Environment-Variables.html @@ -128,13 +180,26 @@ func LoadTranslations(root string, getLanguageFn func() string) error { gettext.BindLocale(gettext.New("k8s", root+".zip", buf.Bytes())) gettext.SetDomain("k8s") gettext.SetLanguage(langStr) + translationsLoaded = true return nil } +func lazyLoadTranslations() { + lazyLoadTranslationsOnce.Do(func() { + if translationsLoaded { + return + } + if err := LoadTranslationsFunc(); err != nil { + klog.Warning("Failed to load translations") + } + }) +} + // T translates a string, possibly substituting arguments into it along // the way. If len(args) is > 0, args1 is assumed to be the plural value // and plural translation is used. func T(defaultValue string, args ...int) string { + lazyLoadTranslations() if len(args) == 0 { return gettext.PGettext("", defaultValue) } diff --git a/pkg/util/i18n/i18n_test.go b/pkg/util/i18n/i18n_test.go index eff6188d..3098e6db 100644 --- a/pkg/util/i18n/i18n_test.go +++ b/pkg/util/i18n/i18n_test.go @@ -18,7 +18,10 @@ package i18n import ( "os" + "sync" "testing" + + "github.com/chai2010/gettext-go" ) var knownTestLocale = "en_US.UTF-8" @@ -155,3 +158,131 @@ func TestTranslationUsingEnvVar(t *testing.T) { }) } } + +// resetLazyLoading allows multiple tests to test translation lazy loading by resetting the state +func resetLazyLoading() { + translationsLoaded = false + lazyLoadTranslationsOnce = sync.Once{} +} + +func TestLazyLoadTranslationFuncIsCalled(t *testing.T) { + resetLazyLoading() + + timesCalled := 0 + err := SetLoadTranslationsFunc(func() error { + timesCalled++ + return LoadTranslations("test", func() string { return "en_US" }) + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if translationsLoaded { + t.Errorf("expected translationsLoaded to be false, but it was true") + } + + // Translation should succeed and use the lazy loaded translations + result := T("test_string") + if result != "baz" { + t.Errorf("expected: %s, saw: %s", "baz", result) + } + if timesCalled != 1 { + t.Errorf("expected LoadTranslationsFunc to have been called 1 time, but it was called %d times", timesCalled) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } + + // Call T() again, and timesCalled should remain 1 + T("test_string") + if timesCalled != 1 { + t.Errorf("expected LoadTranslationsFunc to have been called 1 time, but it was called %d times", timesCalled) + } +} + +func TestLazyLoadTranslationFuncOnlyCalledIfTranslationsNotLoaded(t *testing.T) { + resetLazyLoading() + + // Set a custom translations func + timesCalled := 0 + err := SetLoadTranslationsFunc(func() error { + timesCalled++ + return LoadTranslations("test", func() string { return "en_US" }) + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if translationsLoaded { + t.Errorf("expected translationsLoaded to be false, but it was true") + } + + // Explicitly load translations before lazy loading can occur + err = LoadTranslations("test", func() string { return "default" }) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } + + // Translation should succeed, and use the explicitly loaded translations, not the lazy loaded ones + result := T("test_string") + if result != "foo" { + t.Errorf("expected: %s, saw: %s", "foo", result) + } + if timesCalled != 0 { + t.Errorf("expected LoadTranslationsFunc to have not been called, but it was called %d times", timesCalled) + } +} + +func TestSetCustomLoadTranslationsFunc(t *testing.T) { + resetLazyLoading() + + // Set a custom translations func that loads translations from a directory + err := SetLoadTranslationsFunc(func() error { + gettext.BindLocale(gettext.New("k8s", "./translations/test")) + gettext.SetDomain("k8s") + gettext.SetLanguage("en_US") + return nil + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if translationsLoaded { + t.Errorf("expected translationsLoaded to be false, but it was true") + } + + // Translation should succeed + result := T("test_string") + if result != "baz" { + t.Errorf("expected: %s, saw: %s", "baz", result) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } +} + +func TestSetCustomLoadTranslationsFuncAfterTranslationsLoadedShouldFail(t *testing.T) { + resetLazyLoading() + + // Explicitly load translations + err := LoadTranslations("test", func() string { return "en_US" }) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } + + // This should fail because translations have already been loaded, and the custom function should not be called. + timesCalled := 0 + err = SetLoadTranslationsFunc(func() error { + timesCalled++ + return nil + }) + if err == nil { + t.Errorf("expected error, but it did not occur") + } + if timesCalled != 0 { + t.Errorf("expected LoadTranslationsFunc to have not been called, but it was called %d times", timesCalled) + } +}