mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

Reducing the filestore dependencies from the rest of the source code (#16816)

Просмотр исходного кода 
* Reducing the filestore dependencies from the rest of the source code

* Making more generic config conversion to FileBackendSettings

* Fixing usage of the NewFileBackend function

* Fixing more usages of the NewFileBackend function

* Fix some linter errors

* Fix more linter errors

* Fixing some unit tests

* Fixing linter problem

* Addressing PR review comments

* Simplifing the CopyFile for tests

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
Jesús Espino
2021-02-15 10:09:28 +01:00
коммит произвёл GitHub
родитель a246104d04
Коммит a3de71fba4
15 изменённых файлов: 231 добавлений и 144 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

45
services/filesstore/filesstore.go
Просмотреть файл

@@ -8,8 +8,11 @@ import (
"time"
"github.com/pkg/errors"
)
"github.com/mattermost/mattermost-server/v5/model"
const (
driverS3 = "amazons3"
driverLocal = "local"
)
type ReadCloseSeeker interface {
@@ -35,17 +38,45 @@ type FileBackend interface {
RemoveDirectory(path string) error
}
func NewFileBackend(settings *model.FileSettings, enableComplianceFeatures bool) (FileBackend, error) {
switch *settings.DriverName {
case model.IMAGE_DRIVER_S3:
backend, err := NewS3FileBackend(settings, enableComplianceFeatures)
type FileBackendSettings struct {
DriverName string
Directory string
AmazonS3AccessKeyId string
AmazonS3SecretAccessKey string
AmazonS3Bucket string
AmazonS3PathPrefix string
AmazonS3Region string
AmazonS3Endpoint string
AmazonS3SSL bool
AmazonS3SignV2 bool
AmazonS3SSE bool
AmazonS3Trace bool
}
func (settings *FileBackendSettings) CheckMandatoryS3Fields() error {
if settings.AmazonS3Bucket == "" {
return errors.New("missing s3 bucket settings")
}
// if S3 endpoint is not set call the set defaults to set that
if settings.AmazonS3Endpoint == "" {
settings.AmazonS3Endpoint = "s3.amazonaws.com"
}
return nil
}
func NewFileBackend(settings FileBackendSettings) (FileBackend, error) {
switch settings.DriverName {
case driverS3:
backend, err := NewS3FileBackend(settings)
if err != nil {
return nil, errors.Wrap(err, "unable to connect to the s3 backend")
}
return backend, nil
case model.IMAGE_DRIVER_LOCAL:
case driverLocal:
return &LocalFileBackend{
directory: *settings.Directory,
directory: settings.Directory,
}, nil
}
return nil, errors.New("no valid filestorage driver found")

99
services/filesstore/filesstore_test.go
Просмотреть файл

@@ -14,16 +14,19 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/xtgo/uuid"
"github.com/mattermost/mattermost-server/v5/mlog"
"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/utils"
)
func randomString() string {
return uuid.NewRandom().String()
}
type FileBackendTestSuite struct {
suite.Suite
settings model.FileSettings
settings FileBackendSettings
backend FileBackend
}
@@ -42,9 +45,9 @@ func TestLocalFileBackendTestSuite(t *testing.T) {
defer os.RemoveAll(dir)
suite.Run(t, &FileBackendTestSuite{
settings: model.FileSettings{
DriverName: model.NewString(model.IMAGE_DRIVER_LOCAL),
Directory: &dir,
settings: FileBackendSettings{
DriverName: driverLocal,
Directory: dir,
},
})
}
@@ -71,24 +74,22 @@ func runBackendTest(t *testing.T, encrypt bool) {
s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port)
suite.Run(t, &FileBackendTestSuite{
settings: model.FileSettings{
DriverName: model.NewString(model.IMAGE_DRIVER_S3),
AmazonS3AccessKeyId: model.NewString(model.MINIO_ACCESS_KEY),
AmazonS3SecretAccessKey: model.NewString(model.MINIO_SECRET_KEY),
AmazonS3Bucket: model.NewString(model.MINIO_BUCKET),
AmazonS3Region: model.NewString(""),
AmazonS3Endpoint: model.NewString(s3Endpoint),
AmazonS3PathPrefix: model.NewString(""),
AmazonS3SSL: model.NewBool(false),
AmazonS3SSE: model.NewBool(encrypt),
settings: FileBackendSettings{
DriverName: driverS3,
AmazonS3AccessKeyId: "minioaccesskey",
AmazonS3SecretAccessKey: "miniosecretkey",
AmazonS3Bucket: "mattermost-test",
AmazonS3Region: "",
AmazonS3Endpoint: s3Endpoint,
AmazonS3PathPrefix: "",
AmazonS3SSL: false,
AmazonS3SSE: encrypt,
},
})
}
func (s *FileBackendTestSuite) SetupTest() {
utils.TranslationsPreInit()
backend, err := NewFileBackend(&s.settings, true)
backend, err := NewFileBackend(s.settings)
require.NoError(s.T(), err)
s.backend = backend
@@ -102,7 +103,7 @@ func (s *FileBackendTestSuite) TestConnection() {
func (s *FileBackendTestSuite) TestReadWriteFile() {
b := []byte("test")
path := "tests/" + model.NewId()
path := "tests/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
@@ -118,7 +119,7 @@ func (s *FileBackendTestSuite) TestReadWriteFile() {
func (s *FileBackendTestSuite) TestReadWriteFileImage() {
b := []byte("testimage")
path := "tests/" + model.NewId() + ".png"
path := "tests/" + randomString() + ".png"
written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
@@ -134,7 +135,7 @@ func (s *FileBackendTestSuite) TestReadWriteFileImage() {
func (s *FileBackendTestSuite) TestFileExists() {
b := []byte("testimage")
path := "tests/" + model.NewId() + ".png"
path := "tests/" + randomString() + ".png"
_, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
@@ -151,8 +152,8 @@ func (s *FileBackendTestSuite) TestFileExists() {
func (s *FileBackendTestSuite) TestCopyFile() {
b := []byte("test")
path1 := "tests/" + model.NewId()
path2 := "tests/" + model.NewId()
path1 := "tests/" + randomString()
path2 := "tests/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err)
@@ -175,8 +176,8 @@ func (s *FileBackendTestSuite) TestCopyFile() {
func (s *FileBackendTestSuite) TestCopyFileToDirectoryThatDoesntExist() {
b := []byte("test")
path1 := "tests/" + model.NewId()
path2 := "tests/newdirectory/" + model.NewId()
path1 := "tests/" + randomString()
path2 := "tests/newdirectory/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err)
@@ -196,8 +197,8 @@ func (s *FileBackendTestSuite) TestCopyFileToDirectoryThatDoesntExist() {
func (s *FileBackendTestSuite) TestMoveFile() {
b := []byte("test")
path1 := "tests/" + model.NewId()
path2 := "tests/" + model.NewId()
path1 := "tests/" + randomString()
path2 := "tests/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err)
@@ -218,7 +219,7 @@ func (s *FileBackendTestSuite) TestMoveFile() {
func (s *FileBackendTestSuite) TestRemoveFile() {
b := []byte("test")
path := "tests/" + model.NewId()
path := "tests/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
@@ -245,8 +246,8 @@ func (s *FileBackendTestSuite) TestRemoveFile() {
func (s *FileBackendTestSuite) TestListDirectory() {
b := []byte("test")
path1 := "19700101/" + model.NewId()
path2 := "19800101/" + model.NewId()
path1 := "19700101/" + randomString()
path2 := "19800101/" + randomString()
paths, err := s.backend.ListDirectory("19700101")
s.Nil(err)
@@ -316,7 +317,7 @@ func (s *FileBackendTestSuite) TestRemoveDirectory() {
func (s *FileBackendTestSuite) TestAppendFile() {
s.Run("should fail if target file is missing", func() {
path := "tests/" + model.NewId()
path := "tests/" + randomString()
b := make([]byte, 1024)
written, err := s.backend.AppendFile(bytes.NewReader(b), path)
s.Error(err)
@@ -330,7 +331,7 @@ func (s *FileBackendTestSuite) TestAppendFile() {
for i := range b {
b[i] = 'A'
}
path := "tests/" + model.NewId()
path := "tests/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
@@ -376,7 +377,7 @@ func (s *FileBackendTestSuite) TestFileSize() {
s.Run("valid file", func() {
data := make([]byte, rand.Intn(1024*1024)+1)
path := "tests/" + model.NewId()
path := "tests/" + randomString()
written, err := s.backend.WriteFile(bytes.NewReader(data), path)
s.Nil(err)
@@ -397,7 +398,7 @@ func (s *FileBackendTestSuite) TestFileModTime() {
})
s.Run("valid file", func() {
path := "tests/" + model.NewId()
path := "tests/" + randomString()
data := []byte("some data")
written, err := s.backend.WriteFile(bytes.NewReader(data), path)
@@ -412,7 +413,7 @@ func (s *FileBackendTestSuite) TestFileModTime() {
// We wait 1 second so that the times will differ enough to be testable.
time.Sleep(1 * time.Second)
path2 := "tests/" + model.NewId()
path2 := "tests/" + randomString()
written, err = s.backend.WriteFile(bytes.NewReader(data), path2)
s.Nil(err)
s.EqualValues(len(data), written)
@@ -426,27 +427,25 @@ func (s *FileBackendTestSuite) TestFileModTime() {
}
func BenchmarkS3WriteFile(b *testing.B) {
utils.TranslationsPreInit()
settings := &model.FileSettings{
DriverName: model.NewString(model.IMAGE_DRIVER_S3),
AmazonS3AccessKeyId: model.NewString(model.MINIO_ACCESS_KEY),
AmazonS3SecretAccessKey: model.NewString(model.MINIO_SECRET_KEY),
AmazonS3Bucket: model.NewString(model.MINIO_BUCKET),
AmazonS3Region: model.NewString(""),
AmazonS3Endpoint: model.NewString("localhost:9000"),
AmazonS3PathPrefix: model.NewString(""),
AmazonS3SSL: model.NewBool(false),
AmazonS3SSE: model.NewBool(false),
settings := FileBackendSettings{
DriverName: driverS3,
AmazonS3AccessKeyId: "minioaccesskey",
AmazonS3SecretAccessKey: "miniosecretkey",
AmazonS3Bucket: "mattermost-test",
AmazonS3Region: "",
AmazonS3Endpoint: "localhost:9000",
AmazonS3PathPrefix: "",
AmazonS3SSL: false,
AmazonS3SSE: false,
}
backend, err := NewFileBackend(settings, true)
backend, err := NewFileBackend(settings)
require.NoError(b, err)
// This is needed to create the bucket if it doesn't exist.
require.NoError(b, backend.TestConnection())
path := "tests/" + model.NewId()
path := "tests/" + randomString()
size := 1 * 1024 * 1024
data := make([]byte, size)

48
services/filesstore/localstore.go
Просмотреть файл

@@ -14,7 +14,6 @@ import (
"github.com/pkg/errors"
"github.com/mattermost/mattermost-server/v5/mlog"
"github.com/mattermost/mattermost-server/v5/utils"
)
const (
@@ -25,6 +24,51 @@ type LocalFileBackend struct {
directory string
}
// copyFile will copy a file from src path to dst path.
// Overwrites any existing files at dst.
// Permissions are copied from file at src to the new file at dst.
func copyFile(src, dst string) (err error) {
in, err := os.Open(src)
if err != nil {
return
}
defer in.Close()
if err = os.MkdirAll(filepath.Dir(dst), os.ModePerm); err != nil {
return
}
out, err := os.Create(dst)
if err != nil {
return
}
defer func() {
if e := out.Close(); e != nil {
err = e
}
}()
_, err = io.Copy(out, in)
if err != nil {
return
}
err = out.Sync()
if err != nil {
return
}
stat, err := os.Stat(src)
if err != nil {
return
}
err = os.Chmod(dst, stat.Mode())
if err != nil {
return
}
return
}
func (b *LocalFileBackend) TestConnection() error {
f := bytes.NewReader([]byte("testingwrite"))
if _, err := writeFileLocally(f, filepath.Join(b.directory, TestFilePath)); err != nil {
@@ -81,7 +125,7 @@ func (b *LocalFileBackend) FileModTime(path string) (time.Time, error) {
}
func (b *LocalFileBackend) CopyFile(oldPath, newPath string) error {
if err := utils.CopyFile(filepath.Join(b.directory, oldPath), filepath.Join(b.directory, newPath)); err != nil {
if err := copyFile(filepath.Join(b.directory, oldPath), filepath.Join(b.directory, newPath)); err != nil {
return errors.Wrapf(err, "unable to copy file from %s to %s", oldPath, newPath)
}
return nil

62
services/filesstore/s3store.go
Просмотреть файл

@@ -18,7 +18,6 @@ import (
"github.com/pkg/errors"
"github.com/mattermost/mattermost-server/v5/mlog"
"github.com/mattermost/mattermost-server/v5/model"
)
// S3FileBackend contains all necessary information to communicate with
@@ -42,19 +41,37 @@ const (
bucketNotFound = "NoSuchBucket"
)
var (
imageExtensions = map[string]bool{".jpg": true, ".jpeg": true, ".gif": true, ".bmp": true, ".png": true, ".tiff": true, "tif": true}
imageMimeTypes = map[string]string{".jpg": "image/jpeg", ".jpeg": "image/jpeg", ".gif": "image/gif", ".bmp": "image/bmp", ".png": "image/png", ".tiff": "image/tiff", ".tif": "image/tif"}
)
func isFileExtImage(ext string) bool {
ext = strings.ToLower(ext)
return imageExtensions[ext]
}
func getImageMimeType(ext string) string {
ext = strings.ToLower(ext)
if imageMimeTypes[ext] == "" {
return "image"
}
return imageMimeTypes[ext]
}
// NewS3FileBackend returns an instance of an S3FileBackend.
func NewS3FileBackend(settings *model.FileSettings, enableComplianceFeatures bool) (*S3FileBackend, error) {
func NewS3FileBackend(settings FileBackendSettings) (*S3FileBackend, error) {
backend := &S3FileBackend{
endpoint: *settings.AmazonS3Endpoint,
accessKey: *settings.AmazonS3AccessKeyId,
secretKey: *settings.AmazonS3SecretAccessKey,
secure: settings.AmazonS3SSL == nil || *settings.AmazonS3SSL,
signV2: settings.AmazonS3SignV2 != nil && *settings.AmazonS3SignV2,
region: *settings.AmazonS3Region,
bucket: *settings.AmazonS3Bucket,
pathPrefix: *settings.AmazonS3PathPrefix,
encrypt: settings.AmazonS3SSE != nil && *settings.AmazonS3SSE && enableComplianceFeatures,
trace: settings.AmazonS3Trace != nil && *settings.AmazonS3Trace,
endpoint: settings.AmazonS3Endpoint,
accessKey: settings.AmazonS3AccessKeyId,
secretKey: settings.AmazonS3SecretAccessKey,
secure: settings.AmazonS3SSL,
signV2: settings.AmazonS3SignV2,
region: settings.AmazonS3Region,
bucket: settings.AmazonS3Bucket,
pathPrefix: settings.AmazonS3PathPrefix,
encrypt: settings.AmazonS3SSE,
trace: settings.AmazonS3Trace,
}
cli, err := backend.s3New()
if err != nil {
@@ -264,8 +281,8 @@ func (b *S3FileBackend) MoveFile(oldPath, newPath string) error {
func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, error) {
var contentType string
path = filepath.Join(b.pathPrefix, path)
if ext := filepath.Ext(path); model.IsFileExtImage(ext) {
contentType = model.GetImageMimeType(ext)
if ext := filepath.Ext(path); isFileExtImage(ext) {
contentType = getImageMimeType(ext)
} else {
contentType = "binary/octet-stream"
}
@@ -286,8 +303,8 @@ func (b *S3FileBackend) AppendFile(fr io.Reader, path string) (int64, error) {
}
var contentType string
if ext := filepath.Ext(fp); model.IsFileExtImage(ext) {
contentType = model.GetImageMimeType(ext)
if ext := filepath.Ext(fp); isFileExtImage(ext) {
contentType = getImageMimeType(ext)
} else {
contentType = "binary/octet-stream"
}
@@ -406,16 +423,3 @@ func s3PutOptions(encrypted bool, contentType string) s3.PutObjectOptions {
return options
}
func CheckMandatoryS3Fields(settings *model.FileSettings) error {
if settings.AmazonS3Bucket == nil || *settings.AmazonS3Bucket == "" {
return errors.New("missing s3 bucket settings")
}
// if S3 endpoint is not set call the set defaults to set that
if settings.AmazonS3Endpoint == nil || *settings.AmazonS3Endpoint == "" {
settings.SetDefaults(true)
}
return nil
}

18
services/filesstore/s3store_test.go
Просмотреть файл

@@ -7,24 +7,22 @@ import (
"testing"
"github.com/stretchr/testify/require"
"github.com/mattermost/mattermost-server/v5/model"
)
func TestCheckMandatoryS3Fields(t *testing.T) {
cfg := model.FileSettings{}
cfg := FileBackendSettings{}
err := CheckMandatoryS3Fields(&cfg)
err := cfg.CheckMandatoryS3Fields()
require.Error(t, err)
require.Equal(t, err.Error(), "missing s3 bucket settings", "should've failed with missing s3 bucket")
cfg.AmazonS3Bucket = model.NewString("test-mm")
err = CheckMandatoryS3Fields(&cfg)
cfg.AmazonS3Bucket = "test-mm"
err = cfg.CheckMandatoryS3Fields()
require.NoError(t, err)
cfg.AmazonS3Endpoint = model.NewString("")
err = CheckMandatoryS3Fields(&cfg)
cfg.AmazonS3Endpoint = ""
err = cfg.CheckMandatoryS3Fields()
require.NoError(t, err)
require.Equal(t, *cfg.AmazonS3Endpoint, "s3.amazonaws.com", "should've set the endpoint to the default")
require.Equal(t, "s3.amazonaws.com", cfg.AmazonS3Endpoint, "should've set the endpoint to the default")
}

2
services/mailservice/mail.go
Просмотреть файл

@@ -295,7 +295,7 @@ func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComp
defer c.Quit()
defer c.Close()
fileBackend, nErr := filesstore.NewFileBackend(&config.FileSettings, enableComplianceFeatures)
fileBackend, nErr := filesstore.NewFileBackend(config.FileSettings.ToFileBackendSettings(enableComplianceFeatures))
if nErr != nil {
return errors.Wrap(nErr, "unable to initialize file backend")
}

5
services/mailservice/mail_test.go
Просмотреть файл

@@ -225,7 +225,7 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) {
//Delete all the messages before check the sample email
DeleteMailBox("test2@example.com")
fileBackend, err := filesstore.NewFileBackend(&cfg.FileSettings, true)
fileBackend, err := filesstore.NewFileBackend(cfg.FileSettings.ToFileBackendSettings(true))
assert.NoError(t, err)
// create two files with the same name that will both be attached to the email
@@ -398,7 +398,8 @@ func TestSendMail(t *testing.T) {
DriverName: model.NewString(model.IMAGE_DRIVER_LOCAL),
Directory: &dir,
}
mockBackend, err := filesstore.NewFileBackend(&settings, true)
settings.SetDefaults(true)
mockBackend, err := filesstore.NewFileBackend(settings.ToFileBackendSettings(true))
require.NoError(t, err)
mocm := &mockMailer{}