From 8507681a3dc4fdcaa49e06e1dd63a2747603eb4c Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Fri, 2 Aug 2019 01:40:42 -0300 Subject: [PATCH] Support dynamic locales (#141) This tweaks the locale support in a couple of ways. First, we now locate locales in the root `assets/i18n` directory, resolved via `GetBundlePath`. This hardens the plugin against changes the server might make it how it unpacks the plugin, and simplifies the distribution of i18n resources for users. Secondly, we now register all locales found in the `assets/i18n` directory. This means that adding support for new locales simply requires dropping in the file without any need to modify the code or even recompile. --- Makefile | 1 - {server => assets}/i18n/en.json | 0 server/activate.go | 1 - server/i18n.go | 35 +++++----- server/i18n_test.go | 110 ++++++++++++++++++++++++++++++++ server/plugin.go | 8 ++- server/utils.go | 4 +- 7 files changed, 136 insertions(+), 23 deletions(-) rename {server => assets}/i18n/en.json (100%) create mode 100644 server/i18n_test.go diff --git a/Makefile b/Makefile index 68f13fb..09db284 100755 --- a/Makefile +++ b/Makefile @@ -69,7 +69,6 @@ ifneq ($(HAS_SERVER),) cd server && env GOOS=linux GOARCH=amd64 $(GO) build -o dist/plugin-linux-amd64; cd server && env GOOS=darwin GOARCH=amd64 $(GO) build -o dist/plugin-darwin-amd64; cd server && env GOOS=windows GOARCH=amd64 $(GO) build -o dist/plugin-windows-amd64.exe; - cd server && cp -a i18n dist/i18n endif ## Ensures NPM dependencies are installed without having to run this all the time. diff --git a/server/i18n/en.json b/assets/i18n/en.json similarity index 100% rename from server/i18n/en.json rename to assets/i18n/en.json diff --git a/server/activate.go b/server/activate.go index 862185c..5212958 100755 --- a/server/activate.go +++ b/server/activate.go @@ -26,7 +26,6 @@ func (p *Plugin) OnActivate() error { p.router = p.InitAPI() p.ensureBotExists() p.emptyTime = time.Time{}.AddDate(1, 1, 1) - p.supportedLocales = []string{"en"} for _, team := range teams { if err := p.registerCommand(team.Id); err != nil { diff --git a/server/i18n.go b/server/i18n.go index 63c372b..e4937d0 100755 --- a/server/i18n.go +++ b/server/i18n.go @@ -4,42 +4,45 @@ package main import ( - "fmt" "io/ioutil" + "path" "path/filepath" "strings" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/nicksnyder/go-i18n/i18n" + "github.com/pkg/errors" ) -var locales = make(map[string]string) - func (p *Plugin) TranslationsPreInit() error { - - i18nDirectory, found := fileutils.FindDir(*p.ServerConfig.PluginSettings.Directory + "/" + manifest.Id + "/server/dist/i18n/") - if !found { - return fmt.Errorf("unable to find i18n directory") + bundlePath, err := p.API.GetBundlePath() + if err != nil { + return errors.Wrap(err, "unable to find i18n directory") } - files, _ := ioutil.ReadDir(i18nDirectory) + i18nDirectory := path.Join(bundlePath, "assets", "i18n") + files, err := ioutil.ReadDir(i18nDirectory) + if err != nil { + return errors.Wrap(err, "unable to read i18n directory") + } for _, f := range files { - if filepath.Ext(f.Name()) == ".json" { - filename := f.Name() - locales[strings.Split(filename, ".")[0]] = filepath.Join(i18nDirectory, filename) - + filename := f.Name() + if filepath.Ext(filename) == ".json" { if err := i18n.LoadTranslationFile(filepath.Join(i18nDirectory, filename)); err != nil { - return err + p.API.LogError("Failed to load translation file", "filename", filename, "err", err.Error()) + continue } + + p.API.LogDebug("Loaded translation file", "filename", filename) + p.locales[strings.TrimSuffix(filename, filepath.Ext(filename))] = filepath.Join(i18nDirectory, filename) } } return nil } -func GetUserTranslations(locale string) i18n.TranslateFunc { - if _, ok := locales[locale]; !ok { +func (p *Plugin) GetUserTranslations(locale string) i18n.TranslateFunc { + if _, ok := p.locales[locale]; !ok { locale = model.DEFAULT_LOCALE } diff --git a/server/i18n_test.go b/server/i18n_test.go new file mode 100644 index 0000000..1fa7cf6 --- /dev/null +++ b/server/i18n_test.go @@ -0,0 +1,110 @@ +package main + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/mattermost/mattermost-server/plugin/plugintest" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestTranslationsPreInit(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "TestTranslationsPreInit") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + assetsPath := filepath.Join(tmpDir, "assets") + err = os.Mkdir(assetsPath, 0777) + require.NoError(t, err) + + i18nPath := filepath.Join(tmpDir, "assets", "i18n") + + t.Run("failure to get bundle path", func(t *testing.T) { + api := &plugintest.API{} + api.On("GetBundlePath", mock.Anything).Return(tmpDir, nil) + + defer api.AssertExpectations(t) + + p := &Plugin{} + p.API = api + err := p.TranslationsPreInit() + require.EqualError(t, err, fmt.Sprintf("unable to read i18n directory: open %s: no such file or directory", i18nPath)) + }) + + t.Run("failure to read i18n directory", func(t *testing.T) { + api := &plugintest.API{} + api.On("GetBundlePath", mock.Anything).Return(tmpDir, nil) + + defer api.AssertExpectations(t) + + file, err := os.Create(i18nPath) + require.NoError(t, err) + file.Close() + defer os.Remove(file.Name()) + + p := &Plugin{} + p.API = api + err = p.TranslationsPreInit() + require.EqualError(t, err, fmt.Sprintf("unable to read i18n directory: readdirent: invalid argument")) + }) + + t.Run("no i18n files", func(t *testing.T) { + api := &plugintest.API{} + api.On("GetBundlePath", mock.Anything).Return(tmpDir, nil) + + defer api.AssertExpectations(t) + + err := os.Mkdir(i18nPath, 0777) + require.NoError(t, err) + defer os.Remove(i18nPath) + + p := &Plugin{} + p.API = api + err = p.TranslationsPreInit() + require.NoError(t, err) + }) + + t.Run("various i18n files", func(t *testing.T) { + api := &plugintest.API{} + api.On("GetBundlePath", mock.Anything).Return(tmpDir, nil) + api.On("LogError", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe().Run(func(args mock.Arguments) { + t.Helper() + t.Log(args...) + }) + api.On("LogDebug", mock.Anything, mock.Anything, mock.Anything).Maybe().Run(func(args mock.Arguments) { + t.Helper() + t.Log(args...) + }) + + defer api.AssertExpectations(t) + + err := os.Mkdir(i18nPath, 0777) + require.NoError(t, err) + + err = ioutil.WriteFile(filepath.Join(i18nPath, "not-i18n.txt"), []byte{}, 0777) + require.NoError(t, err) + + err = ioutil.WriteFile(filepath.Join(i18nPath, "invalid.json"), []byte{}, 0777) + require.NoError(t, err) + + err = ioutil.WriteFile(filepath.Join(i18nPath, "en.json"), []byte(`[{"id":"id","translation":"translation"}]`), 0777) + require.NoError(t, err) + + err = ioutil.WriteFile(filepath.Join(i18nPath, "es.json"), []byte(`[{"id":"id","translation":"translation2"}]`), 0777) + require.NoError(t, err) + + p := NewPlugin() + p.API = api + err = p.TranslationsPreInit() + require.NoError(t, err) + + require.Equal(t, map[string]string{ + "en": filepath.Join(i18nPath, "en.json"), + "es": filepath.Join(i18nPath, "es.json"), + }, p.locales) + }) +} diff --git a/server/plugin.go b/server/plugin.go index 37d991b..e7ceff0 100755 --- a/server/plugin.go +++ b/server/plugin.go @@ -1,10 +1,11 @@ package main import ( - "github.com/gorilla/mux" "io/ioutil" "time" + "github.com/gorilla/mux" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" ) @@ -26,13 +27,14 @@ type Plugin struct { defaultTime time.Time - supportedLocales []string - readFile func(path string) ([]byte, error) + + locales map[string]string } func NewPlugin() *Plugin { return &Plugin{ readFile: ioutil.ReadFile, + locales: make(map[string]string), } } diff --git a/server/utils.go b/server/utils.go index d938a6a..2ba0d78 100755 --- a/server/utils.go +++ b/server/utils.go @@ -9,13 +9,13 @@ import ( func (p *Plugin) translation(user *model.User) (i18n.TranslateFunc, string) { locale := "en" - for _, l := range p.supportedLocales { + for l := range p.locales { if user.Locale == l { locale = user.Locale break } } - return GetUserTranslations(locale), locale + return p.GetUserTranslations(locale), locale } func (p *Plugin) location(user *model.User) *time.Location {