Board teamless file path (#4577)

* Updated upload and donwload code

* Removed archived file handling as we no longer have cloud limits

* Fixed server lint

* Restored unused

* CI

* Added new tests

* Fixed integration tests

* Added Path column for personal server's FileInfo table

* Removed sophesticated, elegant debug logs

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

server/api/api.go

@@ -227,7 +227,7 @@ func jsonStringResponse(w http.ResponseWriter, code int, message string) { //nol
 	fmt.Fprint(w, message)
 }
 
-func jsonBytesResponse(w http.ResponseWriter, code int, json []byte) {
+func jsonBytesResponse(w http.ResponseWriter, code int, json []byte) { //nolint:unparam
 	setResponseHeader(w, "Content-Type", "application/json")
 	w.WriteHeader(code)
 	_, _ = w.Write(json)

server/api/files.go

@@ -123,37 +123,12 @@ func (a *API) handleServeFile(w http.ResponseWriter, r *http.Request) {
 	auditRec.AddMeta("teamID", board.TeamID)
 	auditRec.AddMeta("filename", filename)
 
-	fileInfo, err := a.app.GetFileInfo(filename)
+	fileInfo, fileReader, err := a.app.GetFile(board.TeamID, boardID, filename)
 	if err != nil && !model.IsErrNotFound(err) {
 		a.errorResponse(w, r, err)
 		return
 	}
 
-	if fileInfo != nil && fileInfo.Archived {
-		fileMetadata := map[string]interface{}{
-			"archived":  true,
-			"name":      fileInfo.Name,
-			"size":      fileInfo.Size,
-			"extension": fileInfo.Extension,
-		}
-
-		data, jsonErr := json.Marshal(fileMetadata)
-		if jsonErr != nil {
-			a.logger.Error("failed to marshal archived file metadata", mlog.String("filename", filename), mlog.Err(jsonErr))
-			a.errorResponse(w, r, jsonErr)
-			return
-		}
-
-		jsonBytesResponse(w, http.StatusBadRequest, data)
-		return
-	}
-
-	fileReader, err := a.app.GetFileReader(board.TeamID, boardID, filename)
-	if err != nil && !errors.Is(err, app.ErrFileNotFound) {
-		a.errorResponse(w, r, err)
-		return
-	}
-
 	if errors.Is(err, app.ErrFileNotFound) && board.ChannelID != "" {
 		// prior to moving from workspaces to teams, the filepath was constructed from
 		// workspaceID, which is the channel ID in plugin mode.

server/app/files.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/mattermost/focalboard/server/model"
 	mmModel "github.com/mattermost/mattermost-server/v6/model"
 
 	"github.com/mattermost/focalboard/server/utils"
@@ -28,7 +29,7 @@ func (a *App) SaveFile(reader io.Reader, teamID, rootID, filename string) (strin
 
 	createdFilename := utils.NewID(utils.IDTypeNone)
 	fullFilename := fmt.Sprintf(`%s%s`, createdFilename, fileExtension)
-	filePath := filepath.Join(teamID, rootID, fullFilename)
+	filePath := filepath.Join(utils.GetBaseFilePath(), fullFilename)
 
 	fileSize, appErr := a.filesBackend.WriteFile(reader, filePath)
 	if appErr != nil {
@@ -45,7 +46,7 @@ func (a *App) SaveFile(reader io.Reader, teamID, rootID, filename string) (strin
 		CreateAt:        now,
 		UpdateAt:        now,
 		DeleteAt:        0,
-		Path:            emptyString,
+		Path:            filePath,
 		ThumbnailPath:   emptyString,
 		PreviewPath:     emptyString,
 		Name:            filename,
@@ -59,6 +60,7 @@ func (a *App) SaveFile(reader io.Reader, teamID, rootID, filename string) (strin
 		Content:         "",
 		RemoteId:        nil,
 	}
+
 	err := a.store.SaveFileInfo(fileInfo)
 	if err != nil {
 		return "", err
@@ -77,6 +79,7 @@ func (a *App) GetFileInfo(filename string) (*mmModel.FileInfo, error) {
 	// will be the fileinfo id.
 	parts := strings.Split(filename, ".")
 	fileInfoID := parts[0][1:]
+
 	fileInfo, err := a.store.GetFileInfo(fileInfoID)
 	if err != nil {
 		return nil, err
@@ -85,6 +88,40 @@ func (a *App) GetFileInfo(filename string) (*mmModel.FileInfo, error) {
 	return fileInfo, nil
 }
 
+func (a *App) GetFile(teamID, rootID, fileName string) (*mmModel.FileInfo, filestore.ReadCloseSeeker, error) {
+	fileInfo, err := a.GetFileInfo(fileName)
+	if err != nil && !model.IsErrNotFound(err) {
+		a.logger.Error("111")
+		return nil, nil, err
+	}
+
+	var filePath string
+
+	if fileInfo != nil && fileInfo.Path != "" {
+		filePath = fileInfo.Path
+	} else {
+		filePath = filepath.Join(teamID, rootID, fileName)
+	}
+
+	exists, err := a.filesBackend.FileExists(filePath)
+	if err != nil {
+		a.logger.Error(fmt.Sprintf("GetFile: Failed to check if file exists as path. Path: %s, error: %e", filePath, err))
+		return nil, nil, err
+	}
+
+	if !exists {
+		return nil, nil, ErrFileNotFound
+	}
+
+	reader, err := a.filesBackend.Reader(filePath)
+	if err != nil {
+		a.logger.Error(fmt.Sprintf("GetFile: Failed to get file reader of existing file at path: %s, error: %e", filePath, err))
+		return nil, nil, err
+	}
+
+	return fileInfo, reader, nil
+}
+
 func (a *App) GetFileReader(teamID, rootID, filename string) (filestore.ReadCloseSeeker, error) {
 	filePath := filepath.Join(teamID, rootID, filename)
 	exists, err := a.filesBackend.FileExists(filePath)

server/app/files_test.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
@@ -195,8 +196,8 @@ func TestSaveFile(t *testing.T) {
 
 		writeFileFunc := func(reader io.Reader, path string) int64 {
 			paths := strings.Split(path, string(os.PathSeparator))
-			assert.Equal(t, "1", paths[0])
-			assert.Equal(t, testBoardID, paths[1])
+			assert.Equal(t, "boards", paths[0])
+			assert.Equal(t, time.Now().Format("20060102"), paths[1])
 			fileName = paths[2]
 			return int64(10)
 		}
@@ -219,8 +220,8 @@ func TestSaveFile(t *testing.T) {
 
 		writeFileFunc := func(reader io.Reader, path string) int64 {
 			paths := strings.Split(path, string(os.PathSeparator))
-			assert.Equal(t, "1", paths[0])
-			assert.Equal(t, "test-board-id", paths[1])
+			assert.Equal(t, "boards", paths[0])
+			assert.Equal(t, time.Now().Format("20060102"), paths[1])
 			assert.Equal(t, "jpg", strings.Split(paths[2], ".")[1])
 			return int64(10)
 		}
@@ -243,8 +244,8 @@ func TestSaveFile(t *testing.T) {
 
 		writeFileFunc := func(reader io.Reader, path string) int64 {
 			paths := strings.Split(path, string(os.PathSeparator))
-			assert.Equal(t, "1", paths[0])
-			assert.Equal(t, "test-board-id", paths[1])
+			assert.Equal(t, "boards", paths[0])
+			assert.Equal(t, time.Now().Format("20060102"), paths[1])
 			assert.Equal(t, "jpg", strings.Split(paths[2], ".")[1])
 			return int64(10)
 		}
@@ -304,3 +305,80 @@ func TestGetFileInfo(t *testing.T) {
 		assert.Nil(t, fetchedFileInfo)
 	})
 }
+
+func TestGetFile(t *testing.T) {
+	th, _ := SetupTestHelper(t)
+
+	t.Run("when FileInfo exists", func(t *testing.T) {
+		th.Store.EXPECT().GetFileInfo("fileInfoID").Return(&mmModel.FileInfo{
+			Id:   "fileInfoID",
+			Path: "/path/to/file/fileName.txt",
+		}, nil)
+
+		mockedFileBackend := &mocks.FileBackend{}
+		th.App.filesBackend = mockedFileBackend
+		mockedReadCloseSeek := &mocks.ReadCloseSeeker{}
+		readerFunc := func(path string) filestore.ReadCloseSeeker {
+			return mockedReadCloseSeek
+		}
+
+		readerErrorFunc := func(path string) error {
+			return nil
+		}
+		mockedFileBackend.On("Reader", "/path/to/file/fileName.txt").Return(readerFunc, readerErrorFunc)
+		mockedFileBackend.On("FileExists", "/path/to/file/fileName.txt").Return(true, nil)
+
+		fileInfo, seeker, err := th.App.GetFile("teamID", "boardID", "7fileInfoID.txt")
+		assert.NoError(t, err)
+		assert.NotNil(t, fileInfo)
+		assert.NotNil(t, seeker)
+	})
+
+	t.Run("when FileInfo doesn't exist", func(t *testing.T) {
+		th.Store.EXPECT().GetFileInfo("fileInfoID").Return(nil, nil)
+
+		mockedFileBackend := &mocks.FileBackend{}
+		th.App.filesBackend = mockedFileBackend
+		mockedReadCloseSeek := &mocks.ReadCloseSeeker{}
+		readerFunc := func(path string) filestore.ReadCloseSeeker {
+			return mockedReadCloseSeek
+		}
+
+		readerErrorFunc := func(path string) error {
+			return nil
+		}
+
+		mockedFileBackend.On("Reader", "teamID/boardID/7fileInfoID.txt").Return(readerFunc, readerErrorFunc)
+		mockedFileBackend.On("FileExists", "teamID/boardID/7fileInfoID.txt").Return(true, nil)
+
+		fileInfo, seeker, err := th.App.GetFile("teamID", "boardID", "7fileInfoID.txt")
+		assert.NoError(t, err)
+		assert.Nil(t, fileInfo)
+		assert.NotNil(t, seeker)
+	})
+
+	t.Run("when FileInfo exists but FileInfo.Path is not set", func(t *testing.T) {
+		th.Store.EXPECT().GetFileInfo("fileInfoID").Return(&mmModel.FileInfo{
+			Id:   "fileInfoID",
+			Path: "",
+		}, nil)
+
+		mockedFileBackend := &mocks.FileBackend{}
+		th.App.filesBackend = mockedFileBackend
+		mockedReadCloseSeek := &mocks.ReadCloseSeeker{}
+		readerFunc := func(path string) filestore.ReadCloseSeeker {
+			return mockedReadCloseSeek
+		}
+
+		readerErrorFunc := func(path string) error {
+			return nil
+		}
+		mockedFileBackend.On("Reader", "teamID/boardID/7fileInfoID.txt").Return(readerFunc, readerErrorFunc)
+		mockedFileBackend.On("FileExists", "teamID/boardID/7fileInfoID.txt").Return(true, nil)
+
+		fileInfo, seeker, err := th.App.GetFile("teamID", "boardID", "7fileInfoID.txt")
+		assert.NoError(t, err)
+		assert.NotNil(t, fileInfo)
+		assert.NotNil(t, seeker)
+	})
+}

server/client/client.go

@@ -380,6 +380,8 @@ func (c *Client) GetCards(boardID string, page int, perPage int) ([]*model.Card,
 		return nil, BuildErrorResponse(r, err)
 	}
 
+	defer closeBody(r)
+
 	var cards []*model.Card
 	if err := json.NewDecoder(r.Body).Decode(&cards); err != nil {
 		return nil, BuildErrorResponse(r, err)
@@ -398,6 +400,8 @@ func (c *Client) PatchCard(cardID string, cardPatch *model.CardPatch, disableNot
 		return nil, BuildErrorResponse(r, err)
 	}
 
+	defer closeBody(r)
+
 	var cardNew *model.Card
 	if err := json.NewDecoder(r.Body).Decode(&cardNew); err != nil {
 		return nil, BuildErrorResponse(r, err)
@@ -412,6 +416,8 @@ func (c *Client) GetCard(cardID string) (*model.Card, *Response) {
 		return nil, BuildErrorResponse(r, err)
 	}
 
+	defer closeBody(r)
+
 	var card *model.Card
 	if err := json.NewDecoder(r.Body).Decode(&card); err != nil {
 		return nil, BuildErrorResponse(r, err)
@@ -450,6 +456,7 @@ func (c *Client) DeleteCategory(teamID, categoryID string) *Response {
 		return BuildErrorResponse(r, err)
 	}
 
+	defer closeBody(r)
 	return BuildResponse(r)
 }
 
@@ -1049,6 +1056,7 @@ func (c *Client) HideBoard(teamID, categoryID, boardID string) *Response {
 		return BuildErrorResponse(r, err)
 	}
 
+	defer closeBody(r)
 	return BuildResponse(r)
 }
 
@@ -1058,5 +1066,6 @@ func (c *Client) UnhideBoard(teamID, categoryID, boardID string) *Response {
 		return BuildErrorResponse(r, err)
 	}
 
+	defer closeBody(r)
 	return BuildResponse(r)
 }

server/services/store/sqlstore/file.go

@@ -22,6 +22,7 @@ func (s *SQLStore) saveFileInfo(db sq.BaseRunner, fileInfo *mmModel.FileInfo) er
 			"extension",
 			"size",
 			"delete_at",
+			"path",
 			"archived",
 		).
 		Values(
@@ -31,6 +32,7 @@ func (s *SQLStore) saveFileInfo(db sq.BaseRunner, fileInfo *mmModel.FileInfo) er
 			fileInfo.Extension,
 			fileInfo.Size,
 			fileInfo.DeleteAt,
+			fileInfo.Path,
 			false,
 		)
 
@@ -57,6 +59,7 @@ func (s *SQLStore) getFileInfo(db sq.BaseRunner, id string) (*mmModel.FileInfo,
 			"extension",
 			"size",
 			"archived",
+			"path",
 		).
 		From(s.tablePrefix + "file_info").
 		Where(sq.Eq{"Id": id})
@@ -73,6 +76,7 @@ func (s *SQLStore) getFileInfo(db sq.BaseRunner, id string) (*mmModel.FileInfo,
 		&fileInfo.Extension,
 		&fileInfo.Size,
 		&fileInfo.Archived,
+		&fileInfo.Path,
 	)
 
 	if err != nil {

server/services/store/sqlstore/migrations/000039_add_path_to_file_info.down.sql

@@ -0,0 +1 @@
+SELECT 1;

server/services/store/sqlstore/migrations/000039_add_path_to_file_info.up.sql

@@ -0,0 +1 @@
+{{ addColumnIfNeeded "file_info" "path" "varchar(512)" "" }}

server/utils/utils.go

@@ -2,6 +2,7 @@ package utils
 
 import (
 	"encoding/json"
+	"path"
 	"reflect"
 	"time"
 
@@ -120,3 +121,7 @@ func DedupeStringArr(arr []string) []string {
 
 	return dedupedArr
 }
+
+func GetBaseFilePath() string {
+	return path.Join("boards", time.Now().Format("20060102"))
+}