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

MM-10188: expect io.Reader in FileBackend.WriteFile (#8765)

Просмотр исходного кода 
This is a reworked set of changes originally from @josephGuo to begin
reducing the duplicated memory required when uploading files.
Этот коммит содержится в:
Jesse Hallam
2018-05-10 18:16:33 -04:00
коммит произвёл Christopher Speller
родитель 7fa1c6c4ba
Коммит 2b27e12445
10 изменённых файлов: 95 добавлений и 51 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

2
api/emoji_test.go
Просмотреть файл

@@ -272,7 +272,7 @@ func TestDeleteEmoji(t *testing.T) {
func createTestEmoji(t *testing.T, a *app.App, emoji *model.Emoji, imageData []byte) *model.Emoji { func createTestEmoji(t *testing.T, a *app.App, emoji *model.Emoji, imageData []byte) *model.Emoji {
emoji = store.Must(a.Srv.Store.Emoji().Save(emoji)).(*model.Emoji) emoji = store.Must(a.Srv.Store.Emoji().Save(emoji)).(*model.Emoji)
if err := a.WriteFile(imageData, "emoji/"+emoji.Id+"/image"); err != nil { if _, err := a.WriteFile(bytes.NewReader(imageData), "emoji/"+emoji.Id+"/image"); err != nil {
store.Must(a.Srv.Store.Emoji().Delete(emoji.Id, time.Now().Unix())) store.Must(a.Srv.Store.Emoji().Delete(emoji.Id, time.Now().Unix()))
t.Fatalf("failed to write image: %v", err.Error()) t.Fatalf("failed to write image: %v", err.Error())
} }

7
app/emoji.go
Просмотреть файл

@@ -98,7 +98,7 @@ func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *mode
if err := gif.EncodeAll(newbuf, resized_gif); err != nil { if err := gif.EncodeAll(newbuf, resized_gif); err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest) return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest)
} }
if err := a.WriteFile(newbuf.Bytes(), getEmojiImagePath(id)); err != nil { if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil {
return err return err
} }
} }
@@ -110,14 +110,15 @@ func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *mode
if err := png.Encode(newbuf, resized_image); err != nil { if err := png.Encode(newbuf, resized_image); err != nil {
return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest) return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest)
} }
if err := a.WriteFile(newbuf.Bytes(), getEmojiImagePath(id)); err != nil { if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil {
return err return err
} }
} }
} }
} }
return a.WriteFile(buf.Bytes(), getEmojiImagePath(id)) _, appErr := a.WriteFile(buf, getEmojiImagePath(id))
return appErr
} }
func (a *App) DeleteEmoji(emoji *model.Emoji) *model.AppError { func (a *App) DeleteEmoji(emoji *model.Emoji) *model.AppError {

13
app/file.go
Просмотреть файл

@@ -78,12 +78,13 @@ func (a *App) MoveFile(oldPath, newPath string) *model.AppError {
return backend.MoveFile(oldPath, newPath) return backend.MoveFile(oldPath, newPath)
} }
func (a *App) WriteFile(f []byte, path string) *model.AppError { func (a *App) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
backend, err := a.FileBackend() backend, err := a.FileBackend()
if err != nil { if err != nil {
return err return 0, err
} }
return backend.WriteFile(f, path)
return backend.WriteFile(fr, path)
} }
func (a *App) RemoveFile(path string) *model.AppError { func (a *App) RemoveFile(path string) *model.AppError {
@@ -414,7 +415,7 @@ func (a *App) DoUploadFile(now time.Time, rawTeamId string, rawChannelId string,
info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg" info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg"
} }
if err := a.WriteFile(data, info.Path); err != nil { if _, err := a.WriteFile(bytes.NewReader(data), info.Path); err != nil {
return nil, err return nil, err
} }
@@ -531,7 +532,7 @@ func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string, widt
return return
} }
if err := a.WriteFile(buf.Bytes(), thumbnailPath); err != nil { if _, err := a.WriteFile(buf, thumbnailPath); err != nil {
mlog.Error(fmt.Sprintf("Unable to upload thumbnail path=%v err=%v", thumbnailPath, err)) mlog.Error(fmt.Sprintf("Unable to upload thumbnail path=%v err=%v", thumbnailPath, err))
return return
} }
@@ -553,7 +554,7 @@ func (a *App) generatePreviewImage(img image.Image, previewPath string, width in
return return
} }
if err := a.WriteFile(buf.Bytes(), previewPath); err != nil { if _, err := a.WriteFile(buf, previewPath); err != nil {
mlog.Error(fmt.Sprintf("Unable to upload preview err=%v", err), mlog.String("path", previewPath)) mlog.Error(fmt.Sprintf("Unable to upload preview err=%v", err), mlog.String("path", previewPath))
return return
} }

2
app/team.go
Просмотреть файл

@@ -1001,7 +1001,7 @@ func (a *App) SetTeamIconFromFile(teamId string, file multipart.File) *model.App
path := "teams/" + teamId + "/teamIcon.png" path := "teams/" + teamId + "/teamIcon.png"
if err := a.WriteFile(buf.Bytes(), path); err != nil { if _, err := a.WriteFile(buf, path); err != nil {
return model.NewAppError("SetTeamIcon", "api.team.set_team_icon.write_file.app_error", nil, "", http.StatusInternalServerError) return model.NewAppError("SetTeamIcon", "api.team.set_team_icon.write_file.app_error", nil, "", http.StatusInternalServerError)
} }

4
app/user.go
Просмотреть файл

@@ -754,7 +754,7 @@ func (a *App) GetProfileImage(user *model.User) ([]byte, bool, *model.AppError)
} }
if user.LastPictureUpdate == 0 { if user.LastPictureUpdate == 0 {
if err := a.WriteFile(img, path); err != nil { if _, err := a.WriteFile(bytes.NewReader(img), path); err != nil {
return nil, false, err return nil, false, err
} }
} }
@@ -810,7 +810,7 @@ func (a *App) SetProfileImageFromFile(userId string, file multipart.File) *model
path := "users/" + userId + "/profile.png" path := "users/" + userId + "/profile.png"
if err := a.WriteFile(buf.Bytes(), path); err != nil { if _, err := a.WriteFile(buf, path); err != nil {
return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.upload_profile.app_error", nil, "", http.StatusInternalServerError) return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.upload_profile.app_error", nil, "", http.StatusInternalServerError)
} }

3
utils/file_backend.go
Просмотреть файл

@@ -4,6 +4,7 @@
package utils package utils
import ( import (
"io"
"net/http" "net/http"
"github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/model"
@@ -15,7 +16,7 @@ type FileBackend interface {
ReadFile(path string) ([]byte, *model.AppError) ReadFile(path string) ([]byte, *model.AppError)
CopyFile(oldPath, newPath string) *model.AppError CopyFile(oldPath, newPath string) *model.AppError
MoveFile(oldPath, newPath string) *model.AppError MoveFile(oldPath, newPath string) *model.AppError
WriteFile(f []byte, path string) *model.AppError WriteFile(fr io.Reader, path string) (int64, *model.AppError)
RemoveFile(path string) *model.AppError RemoveFile(path string) *model.AppError
ListDirectory(path string) (*[]string, *model.AppError) ListDirectory(path string) (*[]string, *model.AppError)

28
utils/file_backend_local.go
Просмотреть файл

@@ -4,6 +4,8 @@
package utils package utils
import ( import (
"bytes"
"io"
"io/ioutil" "io/ioutil"
"net/http" "net/http"
"os" "os"
@@ -22,8 +24,8 @@ type LocalFileBackend struct {
} }
func (b *LocalFileBackend) TestConnection() *model.AppError { func (b *LocalFileBackend) TestConnection() *model.AppError {
f := []byte("testingwrite") f := bytes.NewReader([]byte("testingwrite"))
if err := writeFileLocally(f, filepath.Join(b.directory, TEST_FILE_PATH)); err != nil { if _, err := writeFileLocally(f, filepath.Join(b.directory, TEST_FILE_PATH)); err != nil {
return model.NewAppError("TestFileConnection", "Don't have permissions to write to local path specified or other error.", nil, err.Error(), http.StatusInternalServerError) return model.NewAppError("TestFileConnection", "Don't have permissions to write to local path specified or other error.", nil, err.Error(), http.StatusInternalServerError)
} }
os.Remove(filepath.Join(b.directory, TEST_FILE_PATH)) os.Remove(filepath.Join(b.directory, TEST_FILE_PATH))
@@ -58,21 +60,25 @@ func (b *LocalFileBackend) MoveFile(oldPath, newPath string) *model.AppError {
return nil return nil
} }
func (b *LocalFileBackend) WriteFile(f []byte, path string) *model.AppError { func (b *LocalFileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
return writeFileLocally(f, filepath.Join(b.directory, path)) return writeFileLocally(fr, filepath.Join(b.directory, path))
} }
func writeFileLocally(f []byte, path string) *model.AppError { func writeFileLocally(fr io.Reader, path string) (int64, *model.AppError) {
if err := os.MkdirAll(filepath.Dir(path), 0774); err != nil { if err := os.MkdirAll(filepath.Dir(path), 0774); err != nil {
directory, _ := filepath.Abs(filepath.Dir(path)) directory, _ := filepath.Abs(filepath.Dir(path))
return model.NewAppError("WriteFile", "api.file.write_file_locally.create_dir.app_error", nil, "directory="+directory+", err="+err.Error(), http.StatusInternalServerError) return 0, model.NewAppError("WriteFile", "api.file.write_file_locally.create_dir.app_error", nil, "directory="+directory+", err="+err.Error(), http.StatusInternalServerError)
} }
fw, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
if err := ioutil.WriteFile(path, f, 0644); err != nil { if err != nil {
return model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError) return 0, model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError)
} }
defer fw.Close()
return nil written, err := io.Copy(fw, fr)
if err != nil {
return written, model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError)
}
return written, nil
} }
func (b *LocalFileBackend) RemoveFile(path string) *model.AppError { func (b *LocalFileBackend) RemoveFile(path string) *model.AppError {

14
utils/file_backend_s3.go
Просмотреть файл

@@ -4,7 +4,7 @@
package utils package utils
import ( import (
"bytes" "io"
"io/ioutil" "io/ioutil"
"net/http" "net/http"
"os" "os"
@@ -136,10 +136,10 @@ func (b *S3FileBackend) MoveFile(oldPath, newPath string) *model.AppError {
return nil return nil
} }
func (b *S3FileBackend) WriteFile(f []byte, path string) *model.AppError { func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
s3Clnt, err := b.s3New() s3Clnt, err := b.s3New()
if err != nil { if err != nil {
return model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError) return 0, model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
} }
var contentType string var contentType string
@@ -150,12 +150,12 @@ func (b *S3FileBackend) WriteFile(f []byte, path string) *model.AppError {
} }
options := s3PutOptions(b.encrypt, contentType) options := s3PutOptions(b.encrypt, contentType)
written, err := s3Clnt.PutObject(b.bucket, path, fr, -1, options)
if _, err = s3Clnt.PutObject(b.bucket, path, bytes.NewReader(f), -1, options); err != nil { if err != nil {
return model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError) return written, model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
} }
return nil return written, nil
} }
func (b *S3FileBackend) RemoveFile(path string) *model.AppError { func (b *S3FileBackend) RemoveFile(path string) *model.AppError {

67
utils/file_backend_test.go
Просмотреть файл

@@ -4,6 +4,7 @@
package utils package utils
import ( import (
"bytes"
"fmt" "fmt"
"io/ioutil" "io/ioutil"
"os" "os"
@@ -95,7 +96,9 @@ func (s *FileBackendTestSuite) TestReadWriteFile() {
b := []byte("test") b := []byte("test")
path := "tests/" + model.NewId() path := "tests/" + model.NewId()
s.Nil(s.backend.WriteFile(b, path)) written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path) defer s.backend.RemoveFile(path)
read, err := s.backend.ReadFile(path) read, err := s.backend.ReadFile(path)
@@ -109,7 +112,9 @@ func (s *FileBackendTestSuite) TestReadWriteFileImage() {
b := []byte("testimage") b := []byte("testimage")
path := "tests/" + model.NewId() + ".png" path := "tests/" + model.NewId() + ".png"
s.Nil(s.backend.WriteFile(b, path)) written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path) defer s.backend.RemoveFile(path)
read, err := s.backend.ReadFile(path) read, err := s.backend.ReadFile(path)
@@ -124,8 +129,9 @@ func (s *FileBackendTestSuite) TestCopyFile() {
path1 := "tests/" + model.NewId() path1 := "tests/" + model.NewId()
path2 := "tests/" + model.NewId() path2 := "tests/" + model.NewId()
err := s.backend.WriteFile(b, path1) written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err) s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path1) defer s.backend.RemoveFile(path1)
err = s.backend.CopyFile(path1, path2) err = s.backend.CopyFile(path1, path2)
@@ -144,8 +150,9 @@ func (s *FileBackendTestSuite) TestCopyFileToDirectoryThatDoesntExist() {
path1 := "tests/" + model.NewId() path1 := "tests/" + model.NewId()
path2 := "tests/newdirectory/" + model.NewId() path2 := "tests/newdirectory/" + model.NewId()
err := s.backend.WriteFile(b, path1) written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err) s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path1) defer s.backend.RemoveFile(path1)
err = s.backend.CopyFile(path1, path2) err = s.backend.CopyFile(path1, path2)
@@ -164,13 +171,15 @@ func (s *FileBackendTestSuite) TestMoveFile() {
path1 := "tests/" + model.NewId() path1 := "tests/" + model.NewId()
path2 := "tests/" + model.NewId() path2 := "tests/" + model.NewId()
s.Nil(s.backend.WriteFile(b, path1)) written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path1) defer s.backend.RemoveFile(path1)
s.Nil(s.backend.MoveFile(path1, path2)) s.Nil(s.backend.MoveFile(path1, path2))
defer s.backend.RemoveFile(path2) defer s.backend.RemoveFile(path2)
_, err := s.backend.ReadFile(path1) _, err = s.backend.ReadFile(path1)
s.Error(err) s.Error(err)
_, err = s.backend.ReadFile(path2) _, err = s.backend.ReadFile(path2)
@@ -181,15 +190,26 @@ func (s *FileBackendTestSuite) TestRemoveFile() {
b := []byte("test") b := []byte("test")
path := "tests/" + model.NewId() path := "tests/" + model.NewId()
s.Nil(s.backend.WriteFile(b, path)) written, err := s.backend.WriteFile(bytes.NewReader(b), path)
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
s.Nil(s.backend.RemoveFile(path)) s.Nil(s.backend.RemoveFile(path))
_, err := s.backend.ReadFile(path) _, err = s.backend.ReadFile(path)
s.Error(err) s.Error(err)
s.Nil(s.backend.WriteFile(b, "tests2/foo")) written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/foo")
s.Nil(s.backend.WriteFile(b, "tests2/bar")) s.Nil(err)
s.Nil(s.backend.WriteFile(b, "tests2/asdf")) s.EqualValues(len(b), written, "expected given number of bytes to have been written")
written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/bar")
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/asdf")
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
s.Nil(s.backend.RemoveDirectory("tests2")) s.Nil(s.backend.RemoveDirectory("tests2"))
} }
@@ -198,9 +218,14 @@ func (s *FileBackendTestSuite) TestListDirectory() {
path1 := "19700101/" + model.NewId() path1 := "19700101/" + model.NewId()
path2 := "19800101/" + model.NewId() path2 := "19800101/" + model.NewId()
s.Nil(s.backend.WriteFile(b, path1)) written, err := s.backend.WriteFile(bytes.NewReader(b), path1)
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path1) defer s.backend.RemoveFile(path1)
s.Nil(s.backend.WriteFile(b, path2))
written, err = s.backend.WriteFile(bytes.NewReader(b), path2)
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
defer s.backend.RemoveFile(path2) defer s.backend.RemoveFile(path2)
paths, err := s.backend.ListDirectory("") paths, err := s.backend.ListDirectory("")
@@ -222,13 +247,21 @@ func (s *FileBackendTestSuite) TestListDirectory() {
func (s *FileBackendTestSuite) TestRemoveDirectory() { func (s *FileBackendTestSuite) TestRemoveDirectory() {
b := []byte("test") b := []byte("test")
s.Nil(s.backend.WriteFile(b, "tests2/foo")) written, err := s.backend.WriteFile(bytes.NewReader(b), "tests2/foo")
s.Nil(s.backend.WriteFile(b, "tests2/bar")) s.Nil(err)
s.Nil(s.backend.WriteFile(b, "tests2/aaa")) s.EqualValues(len(b), written, "expected given number of bytes to have been written")
written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/bar")
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/aaa")
s.Nil(err)
s.EqualValues(len(b), written, "expected given number of bytes to have been written")
s.Nil(s.backend.RemoveDirectory("tests2")) s.Nil(s.backend.RemoveDirectory("tests2"))
_, err := s.backend.ReadFile("tests2/foo") _, err = s.backend.ReadFile("tests2/foo")
s.Error(err) s.Error(err)
_, err = s.backend.ReadFile("tests2/bar") _, err = s.backend.ReadFile("tests2/bar")
s.Error(err) s.Error(err)

6
utils/mail_test.go
Просмотреть файл

@@ -150,8 +150,10 @@ func TestSendMailUsingConfig(t *testing.T) {
filePath2 := fmt.Sprintf("test2/%s", fileName) filePath2 := fmt.Sprintf("test2/%s", fileName)
fileContents1 := []byte("hello world") fileContents1 := []byte("hello world")
fileContents2 := []byte("foo bar") fileContents2 := []byte("foo bar")
assert.Nil(t, fileBackend.WriteFile(fileContents1, filePath1)) _, err := fileBackend.WriteFile(bytes.NewReader(fileContents1), filePath1)
assert.Nil(t, fileBackend.WriteFile(fileContents2, filePath2)) assert.Nil(t, err)
_, err := fileBackend.WriteFile(bytes.NewReader(fileContents2), filePath2)
assert.Nil(t, err)
defer fileBackend.RemoveFile(filePath1) defer fileBackend.RemoveFile(filePath1)
defer fileBackend.RemoveFile(filePath2) defer fileBackend.RemoveFile(filePath2)