Migrate FileInfo store to Sync by default (#10837)

store/sqlstore/file_info_store.go

@@ -61,50 +61,42 @@ func (fs SqlFileInfoStore) CreateIndexesIfNotExists() {
 	fs.CreateIndexIfNotExists("idx_fileinfo_postid_at", "FileInfo", "PostId")
 }
 
-func (fs SqlFileInfoStore) Save(info *model.FileInfo) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		info.PreSave()
-		if result.Err = info.IsValid(); result.Err != nil {
-			return
-		}
+func (fs SqlFileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, *model.AppError) {
+	info.PreSave()
+	if err := info.IsValid(); err != nil {
+		return nil, err
+	}
 
-		if err := fs.GetMaster().Insert(info); err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.Save", "store.sql_file_info.save.app_error", nil, err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = info
-		}
-	})
+	if err := fs.GetMaster().Insert(info); err != nil {
+		return nil, model.NewAppError("SqlFileInfoStore.Save", "store.sql_file_info.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+	return info, nil
 }
 
-func (fs SqlFileInfoStore) Get(id string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		info := &model.FileInfo{}
+func (fs SqlFileInfoStore) Get(id string) (*model.FileInfo, *model.AppError) {
+	info := &model.FileInfo{}
 
-		if err := fs.GetReplica().SelectOne(info,
-			`SELECT
-				*
-			FROM
-				FileInfo
-			WHERE
-				Id = :Id
-				AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil {
-			if err == sql.ErrNoRows {
-				result.Err = model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound)
-			} else {
-				result.Err = model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
-			}
-		} else {
-			result.Data = info
+	if err := fs.GetReplica().SelectOne(info,
+		`SELECT
+			*
+		FROM
+			FileInfo
+		WHERE
+			Id = :Id
+			AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil {
+		if err == sql.ErrNoRows {
+			return nil, model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound)
 		}
-	})
+		return nil, model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+	}
+	return info, nil
 }
 
-func (fs SqlFileInfoStore) GetByPath(path string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		info := &model.FileInfo{}
+func (fs SqlFileInfoStore) GetByPath(path string) (*model.FileInfo, *model.AppError) {
+	info := &model.FileInfo{}
 
-		if err := fs.GetReplica().SelectOne(info,
-			`SELECT
+	if err := fs.GetReplica().SelectOne(info,
+		`SELECT
 				*
 			FROM
 				FileInfo
@@ -112,11 +104,9 @@ func (fs SqlFileInfoStore) GetByPath(path string) store.StoreChannel {
 				Path = :Path
 				AND DeleteAt = 0
 			LIMIT 1`, map[string]interface{}{"Path": path}); err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.GetByPath", "store.sql_file_info.get_by_path.app_error", nil, "path="+path+", "+err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = info
-		}
-	})
+		return nil, model.NewAppError("SqlFileInfoStore.GetByPath", "store.sql_file_info.get_by_path.app_error", nil, "path="+path+", "+err.Error(), http.StatusInternalServerError)
+	}
+	return info, nil
 }
 
 func (fs SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) {
@@ -126,37 +116,34 @@ func (fs SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) {
 	}
 }
 
-func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		if allowFromCache {
-			if cacheItem, ok := fileInfoCache.Get(postId); ok {
-				if fs.metrics != nil {
-					fs.metrics.IncrementMemCacheHitCounter("File Info Cache")
-				}
-
-				result.Data = cacheItem.([]*model.FileInfo)
-				return
-			} else {
-				if fs.metrics != nil {
-					fs.metrics.IncrementMemCacheMissCounter("File Info Cache")
-				}
-			}
-		} else {
+func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) ([]*model.FileInfo, *model.AppError) {
+	if allowFromCache {
+		if cacheItem, ok := fileInfoCache.Get(postId); ok {
 			if fs.metrics != nil {
-				fs.metrics.IncrementMemCacheMissCounter("File Info Cache")
+				fs.metrics.IncrementMemCacheHitCounter("File Info Cache")
 			}
+
+			return cacheItem.([]*model.FileInfo), nil
 		}
-
-		var infos []*model.FileInfo
-
-		dbmap := fs.GetReplica()
-
-		if readFromMaster {
-			dbmap = fs.GetMaster()
+		if fs.metrics != nil {
+			fs.metrics.IncrementMemCacheMissCounter("File Info Cache")
 		}
+	} else {
+		if fs.metrics != nil {
+			fs.metrics.IncrementMemCacheMissCounter("File Info Cache")
+		}
+	}
 
-		if _, err := dbmap.Select(&infos,
-			`SELECT
+	var infos []*model.FileInfo
+
+	dbmap := fs.GetReplica()
+
+	if readFromMaster {
+		dbmap = fs.GetMaster()
+	}
+
+	if _, err := dbmap.Select(&infos,
+		`SELECT
 				*
 			FROM
 				FileInfo
@@ -165,26 +152,23 @@ func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowF
 				AND DeleteAt = 0
 			ORDER BY
 				CreateAt`, map[string]interface{}{"PostId": postId}); err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.GetForPost",
-				"store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error(), http.StatusInternalServerError)
-		} else {
-			if len(infos) > 0 {
-				fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC)
-			}
+		return nil, model.NewAppError("SqlFileInfoStore.GetForPost",
+			"store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error(), http.StatusInternalServerError)
+	}
+	if len(infos) > 0 {
+		fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC)
+	}
 
-			result.Data = infos
-		}
-	})
+	return infos, nil
 }
 
-func (fs SqlFileInfoStore) GetForUser(userId string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		var infos []*model.FileInfo
+func (fs SqlFileInfoStore) GetForUser(userId string) ([]*model.FileInfo, *model.AppError) {
+	var infos []*model.FileInfo
 
-		dbmap := fs.GetReplica()
+	dbmap := fs.GetReplica()
 
-		if _, err := dbmap.Select(&infos,
-			`SELECT
+	if _, err := dbmap.Select(&infos,
+		`SELECT
 				*
 			FROM
 				FileInfo
@@ -193,18 +177,15 @@ func (fs SqlFileInfoStore) GetForUser(userId string) store.StoreChannel {
 				AND DeleteAt = 0
 			ORDER BY
 				CreateAt`, map[string]interface{}{"CreatorId": userId}); err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.GetForPost",
-				"store.sql_file_info.get_for_user_id.app_error", nil, "creator_id="+userId+", "+err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = infos
-		}
-	})
+		return nil, model.NewAppError("SqlFileInfoStore.GetForPost",
+			"store.sql_file_info.get_for_user_id.app_error", nil, "creator_id="+userId+", "+err.Error(), http.StatusInternalServerError)
+	}
+	return infos, nil
 }
 
-func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		sqlResult, err := fs.GetMaster().Exec(
-			`UPDATE
+func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) *model.AppError {
+	sqlResult, err := fs.GetMaster().Exec(
+		`UPDATE
 					FileInfo
 				SET
 					PostId = :PostId
@@ -212,94 +193,80 @@ func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) store.
 					Id = :Id
 					AND PostId = ''
 					AND CreatorId = :CreatorId`, map[string]interface{}{"PostId": postId, "Id": fileId, "CreatorId": creatorId})
-		if err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.AttachToPost",
-				"store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError)
-			return
-		}
+	if err != nil {
+		return model.NewAppError("SqlFileInfoStore.AttachToPost",
+			"store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError)
+	}
 
-		count, err := sqlResult.RowsAffected()
-		if err != nil {
-			// RowsAffected should never fail with the MySQL or Postgres drivers
-			result.Err = model.NewAppError("SqlFileInfoStore.AttachToPost",
-				"store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError)
-		} else if count == 0 {
-			// Could not attach the file to the post
-			result.Err = model.NewAppError("SqlFileInfoStore.AttachToPost",
-				"store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId, http.StatusBadRequest)
-		}
-	})
+	count, err := sqlResult.RowsAffected()
+	if err != nil {
+		// RowsAffected should never fail with the MySQL or Postgres drivers
+		return model.NewAppError("SqlFileInfoStore.AttachToPost",
+			"store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError)
+	} else if count == 0 {
+		// Could not attach the file to the post
+		return model.NewAppError("SqlFileInfoStore.AttachToPost",
+			"store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId, http.StatusBadRequest)
+	}
+	return nil
 }
 
-func (fs SqlFileInfoStore) DeleteForPost(postId string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		if _, err := fs.GetMaster().Exec(
-			`UPDATE
+func (fs SqlFileInfoStore) DeleteForPost(postId string) (string, *model.AppError) {
+	if _, err := fs.GetMaster().Exec(
+		`UPDATE
 				FileInfo
 			SET
 				DeleteAt = :DeleteAt
 			WHERE
 				PostId = :PostId`, map[string]interface{}{"DeleteAt": model.GetMillis(), "PostId": postId}); err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.DeleteForPost",
-				"store.sql_file_info.delete_for_post.app_error", nil, "post_id="+postId+", err="+err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = postId
-		}
-	})
+		return "", model.NewAppError("SqlFileInfoStore.DeleteForPost",
+			"store.sql_file_info.delete_for_post.app_error", nil, "post_id="+postId+", err="+err.Error(), http.StatusInternalServerError)
+	}
+	return postId, nil
 }
 
-func (fs SqlFileInfoStore) PermanentDelete(fileId string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		if _, err := fs.GetMaster().Exec(
-			`DELETE FROM
+func (fs SqlFileInfoStore) PermanentDelete(fileId string) *model.AppError {
+	if _, err := fs.GetMaster().Exec(
+		`DELETE FROM
 				FileInfo
 			WHERE
 				Id = :FileId`, map[string]interface{}{"FileId": fileId}); err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.PermanentDelete",
-				"store.sql_file_info.permanent_delete.app_error", nil, "file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError)
-		}
-	})
+		return model.NewAppError("SqlFileInfoStore.PermanentDelete",
+			"store.sql_file_info.permanent_delete.app_error", nil, "file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError)
+	}
+	return nil
 }
 
-func (s SqlFileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		var query string
-		if s.DriverName() == "postgres" {
-			query = "DELETE from FileInfo WHERE Id = any (array (SELECT Id FROM FileInfo WHERE CreateAt < :EndTime LIMIT :Limit))"
-		} else {
-			query = "DELETE from FileInfo WHERE CreateAt < :EndTime LIMIT :Limit"
-		}
+func (s SqlFileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) {
+	var query string
+	if s.DriverName() == "postgres" {
+		query = "DELETE from FileInfo WHERE Id = any (array (SELECT Id FROM FileInfo WHERE CreateAt < :EndTime LIMIT :Limit))"
+	} else {
+		query = "DELETE from FileInfo WHERE CreateAt < :EndTime LIMIT :Limit"
+	}
 
-		sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"EndTime": endTime, "Limit": limit})
-		if err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
-		} else {
-			rowsAffected, err1 := sqlResult.RowsAffected()
-			if err1 != nil {
-				result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
-				result.Data = int64(0)
-			} else {
-				result.Data = rowsAffected
-			}
-		}
-	})
+	sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"EndTime": endTime, "Limit": limit})
+	if err != nil {
+		return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
+	}
+	rowsAffected, err1 := sqlResult.RowsAffected()
+	if err1 != nil {
+		return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
+	}
+	return rowsAffected, nil
 }
 
-func (s SqlFileInfoStore) PermanentDeleteByUser(userId string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		query := "DELETE from FileInfo WHERE CreatorId = :CreatorId"
+func (s SqlFileInfoStore) PermanentDeleteByUser(userId string) (int64, *model.AppError) {
+	query := "DELETE from FileInfo WHERE CreatorId = :CreatorId"
 
-		sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"CreatorId": userId})
-		if err != nil {
-			result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
-		} else {
-			rowsAffected, err1 := sqlResult.RowsAffected()
-			if err1 != nil {
-				result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
-				result.Data = int64(0)
-			} else {
-				result.Data = rowsAffected
-			}
-		}
-	})
+	sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"CreatorId": userId})
+	if err != nil {
+		return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
+	}
+
+	rowsAffected, err1 := sqlResult.RowsAffected()
+	if err1 != nil {
+		return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError)
+	}
+	return rowsAffected, nil
 }

store/store.go

@@ -476,17 +476,17 @@ type StatusStore interface {
 }
 
 type FileInfoStore interface {
-	Save(info *model.FileInfo) StoreChannel
-	Get(id string) StoreChannel
-	GetByPath(path string) StoreChannel
-	GetForPost(postId string, readFromMaster bool, allowFromCache bool) StoreChannel
-	GetForUser(userId string) StoreChannel
+	Save(info *model.FileInfo) (*model.FileInfo, *model.AppError)
+	Get(id string) (*model.FileInfo, *model.AppError)
+	GetByPath(path string) (*model.FileInfo, *model.AppError)
+	GetForPost(postId string, readFromMaster bool, allowFromCache bool) ([]*model.FileInfo, *model.AppError)
+	GetForUser(userId string) ([]*model.FileInfo, *model.AppError)
 	InvalidateFileInfosForPostCache(postId string)
-	AttachToPost(fileId string, postId string, creatorId string) StoreChannel
-	DeleteForPost(postId string) StoreChannel
-	PermanentDelete(fileId string) StoreChannel
-	PermanentDeleteBatch(endTime int64, limit int64) StoreChannel
-	PermanentDeleteByUser(userId string) StoreChannel
+	AttachToPost(fileId string, postId string, creatorId string) *model.AppError
+	DeleteForPost(postId string) (string, *model.AppError)
+	PermanentDelete(fileId string) *model.AppError
+	PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError)
+	PermanentDeleteByUser(userId string) (int64, *model.AppError)
 	ClearCaches()
 }
 

store/storetest/file_info_store.go

@@ -32,36 +32,30 @@ func testFileInfoSaveGet(t *testing.T, ss store.Store) {
 		Path:      "file.txt",
 	}
 
-	if result := <-ss.FileInfo().Save(info); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.(*model.FileInfo); len(returned.Id) == 0 {
-		t.Fatal("should've assigned an id to FileInfo")
-	} else {
-		info = returned
-	}
+	info, err := ss.FileInfo().Save(info)
+	require.Nil(t, err)
+	require.NotEqual(t, len(info.Id), 0)
+
 	defer func() {
-		<-ss.FileInfo().PermanentDelete(info.Id)
+		ss.FileInfo().PermanentDelete(info.Id)
 	}()
 
-	if result := <-ss.FileInfo().Get(info.Id); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.(*model.FileInfo); returned.Id != info.Id {
-		t.Log(info)
-		t.Log(returned)
-		t.Fatal("should've returned correct FileInfo")
-	}
+	rinfo, err := ss.FileInfo().Get(info.Id)
+	require.Nil(t, err)
+	require.Equal(t, info.Id, rinfo.Id)
 
-	info2 := store.Must(ss.FileInfo().Save(&model.FileInfo{
+	info2, err := ss.FileInfo().Save(&model.FileInfo{
 		CreatorId: model.NewId(),
 		Path:      "file.txt",
 		DeleteAt:  123,
-	})).(*model.FileInfo)
+	})
+	require.Nil(t, err)
+
+	_, err = ss.FileInfo().Get(info2.Id)
+	assert.NotNil(t, err)
 
-	if result := <-ss.FileInfo().Get(info2.Id); result.Err == nil {
-		t.Fatal("shouldn't have gotten deleted file")
-	}
 	defer func() {
-		<-ss.FileInfo().PermanentDelete(info2.Id)
+		ss.FileInfo().PermanentDelete(info2.Id)
 	}()
 }
 
@@ -71,36 +65,29 @@ func testFileInfoSaveGetByPath(t *testing.T, ss store.Store) {
 		Path:      fmt.Sprintf("%v/file.txt", model.NewId()),
 	}
 
-	if result := <-ss.FileInfo().Save(info); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.(*model.FileInfo); len(returned.Id) == 0 {
-		t.Fatal("should've assigned an id to FileInfo")
-	} else {
-		info = returned
-	}
+	info, err := ss.FileInfo().Save(info)
+	require.Nil(t, err)
+	assert.NotEqual(t, len(info.Id), 0)
 	defer func() {
-		<-ss.FileInfo().PermanentDelete(info.Id)
+		ss.FileInfo().PermanentDelete(info.Id)
 	}()
 
-	if result := <-ss.FileInfo().GetByPath(info.Path); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.(*model.FileInfo); returned.Id != info.Id {
-		t.Log(info)
-		t.Log(returned)
-		t.Fatal("should've returned correct FileInfo")
-	}
+	rinfo, err := ss.FileInfo().GetByPath(info.Path)
+	require.Nil(t, err)
+	assert.Equal(t, info.Id, rinfo.Id)
 
-	info2 := store.Must(ss.FileInfo().Save(&model.FileInfo{
+	info2, err := ss.FileInfo().Save(&model.FileInfo{
 		CreatorId: model.NewId(),
 		Path:      "file.txt",
 		DeleteAt:  123,
-	})).(*model.FileInfo)
+	})
+	require.Nil(t, err)
+
+	_, err = ss.FileInfo().GetByPath(info2.Id)
+	assert.NotNil(t, err)
 
-	if result := <-ss.FileInfo().GetByPath(info2.Id); result.Err == nil {
-		t.Fatal("shouldn't have gotten deleted file")
-	}
 	defer func() {
-		<-ss.FileInfo().PermanentDelete(info2.Id)
+		ss.FileInfo().PermanentDelete(info2.Id)
 	}()
 }
 
@@ -133,29 +120,25 @@ func testFileInfoGetForPost(t *testing.T, ss store.Store) {
 	}
 
 	for i, info := range infos {
-		infos[i] = store.Must(ss.FileInfo().Save(info)).(*model.FileInfo)
+		newInfo, err := ss.FileInfo().Save(info)
+		require.Nil(t, err)
+		infos[i] = newInfo
 		defer func(id string) {
-			<-ss.FileInfo().PermanentDelete(id)
-		}(infos[i].Id)
+			ss.FileInfo().PermanentDelete(id)
+		}(newInfo.Id)
 	}
 
-	if result := <-ss.FileInfo().GetForPost(postId, true, false); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 {
-		t.Fatal("should've returned exactly 2 file infos")
-	}
+	postInfos, err := ss.FileInfo().GetForPost(postId, true, false)
+	require.Nil(t, err)
+	assert.Len(t, postInfos, 2)
 
-	if result := <-ss.FileInfo().GetForPost(postId, false, false); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 {
-		t.Fatal("should've returned exactly 2 file infos")
-	}
+	postInfos, err = ss.FileInfo().GetForPost(postId, false, false)
+	require.Nil(t, err)
+	assert.Len(t, postInfos, 2)
 
-	if result := <-ss.FileInfo().GetForPost(postId, true, true); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 {
-		t.Fatal("should've returned exactly 2 file infos")
-	}
+	postInfos, err = ss.FileInfo().GetForPost(postId, true, true)
+	require.Nil(t, err)
+	assert.Len(t, postInfos, 2)
 }
 
 func testFileInfoGetForUser(t *testing.T, ss store.Store) {
@@ -187,23 +170,21 @@ func testFileInfoGetForUser(t *testing.T, ss store.Store) {
 	}
 
 	for i, info := range infos {
-		infos[i] = store.Must(ss.FileInfo().Save(info)).(*model.FileInfo)
+		newInfo, err := ss.FileInfo().Save(info)
+		require.Nil(t, err)
+		infos[i] = newInfo
 		defer func(id string) {
-			<-ss.FileInfo().PermanentDelete(id)
-		}(infos[i].Id)
+			ss.FileInfo().PermanentDelete(id)
+		}(newInfo.Id)
 	}
 
-	if result := <-ss.FileInfo().GetForUser(userId); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.([]*model.FileInfo); len(returned) != 3 {
-		t.Fatal("should've returned exactly 3 file infos")
-	}
+	userPosts, err := ss.FileInfo().GetForUser(userId)
+	require.Nil(t, err)
+	assert.Len(t, userPosts, 3)
 
-	if result := <-ss.FileInfo().GetForUser(userId2); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if returned := result.Data.([]*model.FileInfo); len(returned) != 1 {
-		t.Fatal("should've returned exactly 1 file infos")
-	}
+	userPosts, err = ss.FileInfo().GetForUser(userId2)
+	require.Nil(t, err)
+	assert.Len(t, userPosts, 1)
 }
 
 func testFileInfoAttachToPost(t *testing.T, ss store.Store) {
@@ -211,28 +192,28 @@ func testFileInfoAttachToPost(t *testing.T, ss store.Store) {
 		userId := model.NewId()
 		postId := model.NewId()
 
-		info1 := store.Must(ss.FileInfo().Save(&model.FileInfo{
+		info1, err := ss.FileInfo().Save(&model.FileInfo{
 			CreatorId: userId,
 			Path:      "file.txt",
-		})).(*model.FileInfo)
-		info2 := store.Must(ss.FileInfo().Save(&model.FileInfo{
+		})
+		require.Nil(t, err)
+		info2, err := ss.FileInfo().Save(&model.FileInfo{
 			CreatorId: userId,
 			Path:      "file2.txt",
-		})).(*model.FileInfo)
+		})
+		require.Nil(t, err)
 
 		require.Equal(t, "", info1.PostId)
 		require.Equal(t, "", info2.PostId)
 
-		result := <-ss.FileInfo().AttachToPost(info1.Id, postId, userId)
-		assert.Nil(t, result.Err)
+		err = ss.FileInfo().AttachToPost(info1.Id, postId, userId)
+		assert.Nil(t, err)
 
-		result = <-ss.FileInfo().AttachToPost(info2.Id, postId, userId)
-		assert.Nil(t, result.Err)
+		err = ss.FileInfo().AttachToPost(info2.Id, postId, userId)
+		assert.Nil(t, err)
 
-		result = <-ss.FileInfo().GetForPost(postId, true, false)
-		assert.Nil(t, result.Err)
-
-		data := result.Data.([]*model.FileInfo)
+		data, err := ss.FileInfo().GetForPost(postId, true, false)
+		assert.Nil(t, err)
 
 		assert.Len(t, data, 2)
 		assert.True(t, data[0].Id == info1.Id || data[0].Id == info2.Id)
@@ -243,34 +224,35 @@ func testFileInfoAttachToPost(t *testing.T, ss store.Store) {
 		userId := model.NewId()
 		postId := model.NewId()
 
-		info := store.Must(ss.FileInfo().Save(&model.FileInfo{
+		info, err := ss.FileInfo().Save(&model.FileInfo{
 			CreatorId: userId,
 			Path:      "file.txt",
-		})).(*model.FileInfo)
+		})
+		require.Nil(t, err)
 
 		require.Equal(t, "", info.PostId)
 
-		result := <-ss.FileInfo().AttachToPost(info.Id, model.NewId(), userId)
-		assert.Nil(t, result.Err)
+		err = ss.FileInfo().AttachToPost(info.Id, model.NewId(), userId)
+		assert.Nil(t, err)
 
-		result = <-ss.FileInfo().AttachToPost(info.Id, postId, userId)
-		assert.NotNil(t, result.Err)
+		err = ss.FileInfo().AttachToPost(info.Id, postId, userId)
+		assert.NotNil(t, err)
 	})
 
 	t.Run("should not attach files owned from a different user", func(t *testing.T) {
 		userId := model.NewId()
 		postId := model.NewId()
 
-		info := store.Must(ss.FileInfo().Save(&model.FileInfo{
+		info, err := ss.FileInfo().Save(&model.FileInfo{
 			CreatorId: model.NewId(),
 			Path:      "file.txt",
-		})).(*model.FileInfo)
+		})
+		require.Nil(t, err)
 
 		require.Equal(t, "", info.PostId)
 
-		result := <-ss.FileInfo().AttachToPost(info.Id, postId, userId)
-
-		assert.NotNil(t, result.Err)
+		err = ss.FileInfo().AttachToPost(info.Id, postId, userId)
+		assert.NotNil(t, err)
 	})
 }
 
@@ -303,83 +285,84 @@ func testFileInfoDeleteForPost(t *testing.T, ss store.Store) {
 	}
 
 	for i, info := range infos {
-		infos[i] = store.Must(ss.FileInfo().Save(info)).(*model.FileInfo)
+		newInfo, err := ss.FileInfo().Save(info)
+		require.Nil(t, err)
+		infos[i] = newInfo
 		defer func(id string) {
-			<-ss.FileInfo().PermanentDelete(id)
-		}(infos[i].Id)
+			ss.FileInfo().PermanentDelete(id)
+		}(newInfo.Id)
 	}
 
-	if result := <-ss.FileInfo().DeleteForPost(postId); result.Err != nil {
-		t.Fatal(result.Err)
-	}
+	_, err := ss.FileInfo().DeleteForPost(postId)
+	require.Nil(t, err)
 
-	if infos := store.Must(ss.FileInfo().GetForPost(postId, true, false)).([]*model.FileInfo); len(infos) != 0 {
-		t.Fatal("shouldn't have returned any file infos")
-	}
+	infos, err = ss.FileInfo().GetForPost(postId, true, false)
+	require.Nil(t, err)
+	assert.Len(t, infos, 0)
 }
 
 func testFileInfoPermanentDelete(t *testing.T, ss store.Store) {
-	info := store.Must(ss.FileInfo().Save(&model.FileInfo{
+	info, err := ss.FileInfo().Save(&model.FileInfo{
 		PostId:    model.NewId(),
 		CreatorId: model.NewId(),
 		Path:      "file.txt",
-	})).(*model.FileInfo)
+	})
+	require.Nil(t, err)
 
-	if result := <-ss.FileInfo().PermanentDelete(info.Id); result.Err != nil {
-		t.Fatal(result.Err)
-	}
+	err = ss.FileInfo().PermanentDelete(info.Id)
+	require.Nil(t, err)
 }
 
 func testFileInfoPermanentDeleteBatch(t *testing.T, ss store.Store) {
 	postId := model.NewId()
 
-	store.Must(ss.FileInfo().Save(&model.FileInfo{
+	_, err := ss.FileInfo().Save(&model.FileInfo{
 		PostId:    postId,
 		CreatorId: model.NewId(),
 		Path:      "file.txt",
 		CreateAt:  1000,
-	}))
+	})
+	require.Nil(t, err)
 
-	store.Must(ss.FileInfo().Save(&model.FileInfo{
+	_, err = ss.FileInfo().Save(&model.FileInfo{
 		PostId:    postId,
 		CreatorId: model.NewId(),
 		Path:      "file.txt",
 		CreateAt:  1200,
-	}))
+	})
+	require.Nil(t, err)
 
-	store.Must(ss.FileInfo().Save(&model.FileInfo{
+	_, err = ss.FileInfo().Save(&model.FileInfo{
 		PostId:    postId,
 		CreatorId: model.NewId(),
 		Path:      "file.txt",
 		CreateAt:  2000,
-	}))
+	})
+	require.Nil(t, err)
 
-	if result := <-ss.FileInfo().GetForPost(postId, true, false); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if len(result.Data.([]*model.FileInfo)) != 3 {
-		t.Fatal("Expected 3 fileInfos")
-	}
+	postFiles, err := ss.FileInfo().GetForPost(postId, true, false)
+	require.Nil(t, err)
+	assert.Len(t, postFiles, 3)
 
-	store.Must(ss.FileInfo().PermanentDeleteBatch(1500, 1000))
+	_, err = ss.FileInfo().PermanentDeleteBatch(1500, 1000)
+	require.Nil(t, err)
 
-	if result := <-ss.FileInfo().GetForPost(postId, true, false); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if len(result.Data.([]*model.FileInfo)) != 1 {
-		t.Fatal("Expected 3 fileInfos")
-	}
+	postFiles, err = ss.FileInfo().GetForPost(postId, true, false)
+	require.Nil(t, err)
+	assert.Len(t, postFiles, 1)
 }
 
 func testFileInfoPermanentDeleteByUser(t *testing.T, ss store.Store) {
 	userId := model.NewId()
 	postId := model.NewId()
 
-	store.Must(ss.FileInfo().Save(&model.FileInfo{
+	_, err := ss.FileInfo().Save(&model.FileInfo{
 		PostId:    postId,
 		CreatorId: userId,
 		Path:      "file.txt",
-	}))
+	})
+	require.Nil(t, err)
 
-	if result := <-ss.FileInfo().PermanentDeleteByUser(userId); result.Err != nil {
-		t.Fatal(result.Err)
-	}
+	_, err = ss.FileInfo().PermanentDeleteByUser(userId)
+	require.Nil(t, err)
 }

store/storetest/mocks/FileInfoStore.go

@@ -6,7 +6,6 @@ package mocks
 
 import mock "github.com/stretchr/testify/mock"
 import model "github.com/mattermost/mattermost-server/model"
-import store "github.com/mattermost/mattermost-server/store"
 
 // FileInfoStore is an autogenerated mock type for the FileInfoStore type
 type FileInfoStore struct {
@@ -14,15 +13,15 @@ type FileInfoStore struct {
 }
 
 // AttachToPost provides a mock function with given fields: fileId, postId, creatorId
-func (_m *FileInfoStore) AttachToPost(fileId string, postId string, creatorId string) store.StoreChannel {
+func (_m *FileInfoStore) AttachToPost(fileId string, postId string, creatorId string) *model.AppError {
 	ret := _m.Called(fileId, postId, creatorId)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, string, string) store.StoreChannel); ok {
+	var r0 *model.AppError
+	if rf, ok := ret.Get(0).(func(string, string, string) *model.AppError); ok {
 		r0 = rf(fileId, postId, creatorId)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.AppError)
 		}
 	}
 
@@ -35,83 +34,126 @@ func (_m *FileInfoStore) ClearCaches() {
 }
 
 // DeleteForPost provides a mock function with given fields: postId
-func (_m *FileInfoStore) DeleteForPost(postId string) store.StoreChannel {
+func (_m *FileInfoStore) DeleteForPost(postId string) (string, *model.AppError) {
 	ret := _m.Called(postId)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 string
+	if rf, ok := ret.Get(0).(func(string) string); ok {
 		r0 = rf(postId)
 	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+		r0 = ret.Get(0).(string)
+	}
+
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(postId)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
 		}
 	}
 
-	return r0
+	return r0, r1
 }
 
 // Get provides a mock function with given fields: id
-func (_m *FileInfoStore) Get(id string) store.StoreChannel {
+func (_m *FileInfoStore) Get(id string) (*model.FileInfo, *model.AppError) {
 	ret := _m.Called(id)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 *model.FileInfo
+	if rf, ok := ret.Get(0).(func(string) *model.FileInfo); ok {
 		r0 = rf(id)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.FileInfo)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(id)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetByPath provides a mock function with given fields: path
-func (_m *FileInfoStore) GetByPath(path string) store.StoreChannel {
+func (_m *FileInfoStore) GetByPath(path string) (*model.FileInfo, *model.AppError) {
 	ret := _m.Called(path)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 *model.FileInfo
+	if rf, ok := ret.Get(0).(func(string) *model.FileInfo); ok {
 		r0 = rf(path)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.FileInfo)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(path)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetForPost provides a mock function with given fields: postId, readFromMaster, allowFromCache
-func (_m *FileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) store.StoreChannel {
+func (_m *FileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) ([]*model.FileInfo, *model.AppError) {
 	ret := _m.Called(postId, readFromMaster, allowFromCache)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, bool, bool) store.StoreChannel); ok {
+	var r0 []*model.FileInfo
+	if rf, ok := ret.Get(0).(func(string, bool, bool) []*model.FileInfo); ok {
 		r0 = rf(postId, readFromMaster, allowFromCache)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.FileInfo)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, bool, bool) *model.AppError); ok {
+		r1 = rf(postId, readFromMaster, allowFromCache)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetForUser provides a mock function with given fields: userId
-func (_m *FileInfoStore) GetForUser(userId string) store.StoreChannel {
+func (_m *FileInfoStore) GetForUser(userId string) ([]*model.FileInfo, *model.AppError) {
 	ret := _m.Called(userId)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 []*model.FileInfo
+	if rf, ok := ret.Get(0).(func(string) []*model.FileInfo); ok {
 		r0 = rf(userId)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.FileInfo)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(userId)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // InvalidateFileInfosForPostCache provides a mock function with given fields: postId
@@ -120,15 +162,15 @@ func (_m *FileInfoStore) InvalidateFileInfosForPostCache(postId string) {
 }
 
 // PermanentDelete provides a mock function with given fields: fileId
-func (_m *FileInfoStore) PermanentDelete(fileId string) store.StoreChannel {
+func (_m *FileInfoStore) PermanentDelete(fileId string) *model.AppError {
 	ret := _m.Called(fileId)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 *model.AppError
+	if rf, ok := ret.Get(0).(func(string) *model.AppError); ok {
 		r0 = rf(fileId)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.AppError)
 		}
 	}
 
@@ -136,49 +178,72 @@ func (_m *FileInfoStore) PermanentDelete(fileId string) store.StoreChannel {
 }
 
 // PermanentDeleteBatch provides a mock function with given fields: endTime, limit
-func (_m *FileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) store.StoreChannel {
+func (_m *FileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) {
 	ret := _m.Called(endTime, limit)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(int64, int64) store.StoreChannel); ok {
+	var r0 int64
+	if rf, ok := ret.Get(0).(func(int64, int64) int64); ok {
 		r0 = rf(endTime, limit)
 	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+		r0 = ret.Get(0).(int64)
+	}
+
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(int64, int64) *model.AppError); ok {
+		r1 = rf(endTime, limit)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
 		}
 	}
 
-	return r0
+	return r0, r1
 }
 
 // PermanentDeleteByUser provides a mock function with given fields: userId
-func (_m *FileInfoStore) PermanentDeleteByUser(userId string) store.StoreChannel {
+func (_m *FileInfoStore) PermanentDeleteByUser(userId string) (int64, *model.AppError) {
 	ret := _m.Called(userId)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 int64
+	if rf, ok := ret.Get(0).(func(string) int64); ok {
 		r0 = rf(userId)
 	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+		r0 = ret.Get(0).(int64)
+	}
+
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(userId)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
 		}
 	}
 
-	return r0
+	return r0, r1
 }
 
 // Save provides a mock function with given fields: info
-func (_m *FileInfoStore) Save(info *model.FileInfo) store.StoreChannel {
+func (_m *FileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, *model.AppError) {
 	ret := _m.Called(info)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(*model.FileInfo) store.StoreChannel); ok {
+	var r0 *model.FileInfo
+	if rf, ok := ret.Get(0).(func(*model.FileInfo) *model.FileInfo); ok {
 		r0 = rf(info)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.FileInfo)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(*model.FileInfo) *model.AppError); ok {
+		r1 = rf(info)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }