Skip to content

Commit

Permalink
Fixing file info caching issue with Aurora for master (#5477)
Browse files Browse the repository at this point in the history
  • Loading branch information
@coreyhulen @crspeller
coreyhulen authored and crspeller committed Feb 20, 2017
1 parent dd4d844 commit 274b9b6
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 17 deletions.
7 changes: 5 additions & 2 deletions api/post.go
View file
Expand Up @@ -421,13 +421,16 @@ func getFileInfosForPost(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

if infos, err := app.GetFileInfosForPost(postId); err != nil {
if infos, err := app.GetFileInfosForPost(postId, false); err != nil {
c.Err = err
return
} else if HandleEtag(model.GetEtagForFileInfos(infos), "Get File Infos For Post", w, r) {
return
} else {
w.Header().Set("Cache-Control", "max-age=2592000, public")
if len(infos) > 0 {
w.Header().Set("Cache-Control", "max-age=2592000, public")
}

w.Header().Set(model.HEADER_ETAG_SERVER, model.GetEtagForFileInfos(infos))
w.Write([]byte(model.FileInfosToJson(infos)))
}
Expand Down
2 changes: 1 addition & 1 deletion api/post_test.go
View file
Expand Up @@ -136,7 +136,7 @@ func TestCreatePost(t *testing.T) {
} else if rpost9 := resp.Data.(*model.Post); len(rpost9.FileIds) != 3 {
t.Fatal("post should have 3 files")
} else {
infos := store.Must(app.Srv.Store.FileInfo().GetForPost(rpost9.Id, true)).([]*model.FileInfo)
infos := store.Must(app.Srv.Store.FileInfo().GetForPost(rpost9.Id, true, true)).([]*model.FileInfo)

if len(infos) != 3 {
t.Fatal("should've attached all 3 files to post")
Expand Down
2 changes: 1 addition & 1 deletion app/file.go
View file
Expand Up @@ -319,7 +319,7 @@ func MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
return []*model.FileInfo{}
} else if newPost := result.Data.(*model.PostList).Posts[post.Id]; len(newPost.Filenames) != len(post.Filenames) {
// Another thread has already created FileInfos for this post, so just return those
if result := <-Srv.Store.FileInfo().GetForPost(post.Id, false); result.Err != nil {
if result := <-Srv.Store.FileInfo().GetForPost(post.Id, true, false); result.Err != nil {
l4g.Error(utils.T("api.file.migrate_filenames_to_file_infos.get_post_file_infos_again.app_error"), post.Id, result.Err)
return []*model.FileInfo{}
} else {
Expand Down
10 changes: 7 additions & 3 deletions app/notification.go
View file
Expand Up @@ -26,7 +26,11 @@ import (

func SendNotifications(post *model.Post, team *model.Team, channel *model.Channel, sender *model.User) ([]string, *model.AppError) {
pchan := Srv.Store.User().GetAllProfilesInChannel(channel.Id, true)
fchan := Srv.Store.FileInfo().GetForPost(post.Id, true)
var fchan store.StoreChannel

if len(post.FileIds) != 0 {
fchan = Srv.Store.FileInfo().GetForPost(post.Id, true, true)
}

var profileMap map[string]*model.User
if result := <-pchan; result.Err != nil {
Expand Down Expand Up @@ -268,7 +272,7 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe
message.Add("sender_name", senderUsername)
message.Add("team_id", team.Id)

if len(post.FileIds) != 0 {
if len(post.FileIds) != 0 && fchan != nil {
message.Add("otherFile", "true")

var infos []*model.FileInfo
Expand Down Expand Up @@ -410,7 +414,7 @@ func GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFun

// extract the filenames from their paths and determine what type of files are attached
var infos []*model.FileInfo
if result := <-Srv.Store.FileInfo().GetForPost(post.Id, true); result.Err != nil {
if result := <-Srv.Store.FileInfo().GetForPost(post.Id, true, true); result.Err != nil {
l4g.Warn(utils.T("api.post.get_message_for_notification.get_files.error"), post.Id, result.Err)
} else {
infos = result.Data.([]*model.FileInfo)
Expand Down
4 changes: 2 additions & 2 deletions app/post.go
View file
Expand Up @@ -463,9 +463,9 @@ func SearchPostsInTeam(terms string, userId string, teamId string, isOrSearch bo
return posts, nil
}

func GetFileInfosForPost(postId string) ([]*model.FileInfo, *model.AppError) {
func GetFileInfosForPost(postId string, readFromMaster bool) ([]*model.FileInfo, *model.AppError) {
pchan := Srv.Store.Post().GetSingle(postId)
fchan := Srv.Store.FileInfo().GetForPost(postId, true)
fchan := Srv.Store.FileInfo().GetForPost(postId, readFromMaster, true)

var infos []*model.FileInfo
if result := <-fchan; result.Err != nil {
Expand Down
15 changes: 12 additions & 3 deletions store/sql_file_info_store.go
View file
Expand Up @@ -143,7 +143,7 @@ func (s SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) {
fileInfoCache.Remove(postId)
}

func (fs SqlFileInfoStore) GetForPost(postId string, allowFromCache bool) StoreChannel {
func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) StoreChannel {
storeChannel := make(StoreChannel, 1)

go func() {
Expand Down Expand Up @@ -174,7 +174,13 @@ func (fs SqlFileInfoStore) GetForPost(postId string, allowFromCache bool) StoreC

var infos []*model.FileInfo

if _, err := fs.GetReplica().Select(&infos,
dbmap := fs.GetReplica()

if readFromMaster {
dbmap = fs.GetMaster()
}

if _, err := dbmap.Select(&infos,
`SELECT
*
FROM
Expand All @@ -187,7 +193,10 @@ func (fs SqlFileInfoStore) GetForPost(postId string, allowFromCache bool) StoreC
result.Err = model.NewLocAppError("SqlFileInfoStore.GetForPost",
"store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error())
} else {
fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC)
if len(infos) > 0 {
fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC)
}

result.Data = infos
}

Expand Down
14 changes: 10 additions & 4 deletions store/sql_file_info_store_test.go
View file
Expand Up @@ -114,13 +114,19 @@ func TestFileInfoGetForPost(t *testing.T) {
infos[i] = Must(store.FileInfo().Save(info)).(*model.FileInfo)
}

if result := <-store.FileInfo().GetForPost(postId, false); result.Err != nil {
if result := <-store.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")
}

if result := <-store.FileInfo().GetForPost(postId, true); result.Err != nil {
if result := <-store.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")
}

if result := <-store.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")
Expand Down Expand Up @@ -163,7 +169,7 @@ func TestFileInfoAttachToPost(t *testing.T) {
info2 = Must(store.FileInfo().Get(info2.Id)).(*model.FileInfo)
}

if result := <-store.FileInfo().GetForPost(postId, false); result.Err != nil {
if result := <-store.FileInfo().GetForPost(postId, true, false); result.Err != nil {
t.Fatal(result.Err)
} else if infos := result.Data.([]*model.FileInfo); len(infos) != 2 {
t.Fatal("should've returned exactly 2 file infos")
Expand Down Expand Up @@ -208,7 +214,7 @@ func TestFileInfoDeleteForPost(t *testing.T) {
t.Fatal(result.Err)
}

if infos := Must(store.FileInfo().GetForPost(postId, false)).([]*model.FileInfo); len(infos) != 0 {
if infos := Must(store.FileInfo().GetForPost(postId, true, false)).([]*model.FileInfo); len(infos) != 0 {
t.Fatal("shouldn't have returned any file infos")
}
}
2 changes: 1 addition & 1 deletion store/store.go
View file
Expand Up @@ -326,7 +326,7 @@ type FileInfoStore interface {
Save(info *model.FileInfo) StoreChannel
Get(id string) StoreChannel
GetByPath(path string) StoreChannel
GetForPost(postId string, allowFromCache bool) StoreChannel
GetForPost(postId string, readFromMaster bool, allowFromCache bool) StoreChannel
InvalidateFileInfosForPostCache(postId string)
AttachToPost(fileId string, postId string) StoreChannel
DeleteForPost(postId string) StoreChannel
Expand Down

0 comments on commit 274b9b6

Please sign in to comment.