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

@@ -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.

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 {

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
 	}
 

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)
+	})
+}

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),
 	}
 }

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 {