MM-11272 Added initial post metadata (#9175)

* MM-11272 Added app.PreparePostForClient

* MM-11272 Added app.PreparePostListForClient

* MM-11272 Added EmojiStore.GetMultipleByName

* MM-11272 Added emojis to PreparePostForClient

* MM-11272 Added unit tests for getting reaction counts

* MM-11272 Added unit tests for TestPreparePostForClient

* MM-11272 Added emojis from reactions to Post.Emojis

* MM-11272 Always update post.UpdateAt when reactions change to bust cache

* Fixed merge conflicts

* Moved post metadata-related code into its own file

* Update store mocks

* Fixed typo

* Add missing license headers

* Updated post metadata tests when custom emojis are disabled

* Fix unreliable unit tests

* Fix inconsistent casing in SQL statements

* Fix blank line

* Invalidate store cache after making changes

* Clear post cache synchronously with reactions
Этот коммит содержится в:
Harrison Healey
2018-08-07 16:24:56 -04:00
родитель 2e945e287d
Коммит 48f16b6401
27 изменённых файлов: 902 добавлений и 87 удалений

Просмотреть файл

@@ -18,10 +18,11 @@ func (s *LocalCacheSupplier) handleClusterInvalidateRole(msg *model.ClusterMessa
}
func (s *LocalCacheSupplier) RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().RoleSave(ctx, role, hints...)
if len(role.Id) != 0 {
defer s.doInvalidateCacheCluster(s.roleCache, role.Name)
s.doInvalidateCacheCluster(s.roleCache, role.Name)
}
return s.Next().RoleSave(ctx, role, hints...)
return result
}
func (s *LocalCacheSupplier) RoleGet(ctx context.Context, roleId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
@@ -81,8 +82,10 @@ func (s *LocalCacheSupplier) RoleDelete(ctx context.Context, roleId string, hint
}
func (s *LocalCacheSupplier) RolePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
defer s.roleCache.Purge()
defer s.doClearCacheCluster(s.roleCache)
result := s.Next().RolePermanentDeleteAll(ctx, hints...)
return s.Next().RolePermanentDeleteAll(ctx, hints...)
s.roleCache.Purge()
s.doClearCacheCluster(s.roleCache)
return result
}

Просмотреть файл

@@ -18,10 +18,11 @@ func (s *LocalCacheSupplier) handleClusterInvalidateScheme(msg *model.ClusterMes
}
func (s *LocalCacheSupplier) SchemeSave(ctx context.Context, scheme *model.Scheme, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemeSave(ctx, scheme, hints...)
if len(scheme.Id) != 0 {
defer s.doInvalidateCacheCluster(s.schemeCache, scheme.Id)
s.doInvalidateCacheCluster(s.schemeCache, scheme.Id)
}
return s.Next().SchemeSave(ctx, scheme, hints...)
return result
}
func (s *LocalCacheSupplier) SchemeGet(ctx context.Context, schemeId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
@@ -41,10 +42,12 @@ func (s *LocalCacheSupplier) SchemeGetByName(ctx context.Context, schemeName str
}
func (s *LocalCacheSupplier) SchemeDelete(ctx context.Context, schemeId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
defer s.doInvalidateCacheCluster(s.schemeCache, schemeId)
defer s.doClearCacheCluster(s.roleCache)
result := s.Next().SchemeDelete(ctx, schemeId, hints...)
return s.Next().SchemeDelete(ctx, schemeId, hints...)
s.doInvalidateCacheCluster(s.schemeCache, schemeId)
s.doClearCacheCluster(s.roleCache)
return result
}
func (s *LocalCacheSupplier) SchemeGetAllPage(ctx context.Context, scope string, offset int, limit int, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {

Просмотреть файл

@@ -13,6 +13,7 @@ import (
func (s *RedisSupplier) RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
key := buildRedisKeyForRoleName(role.Name)
result := s.Next().RoleSave(ctx, role, hints...)
defer func() {
if err := s.client.Del(key).Err(); err != nil {
@@ -20,7 +21,7 @@ func (s *RedisSupplier) RoleSave(ctx context.Context, role *model.Role, hints ..
}
}()
return s.Next().RoleSave(ctx, role, hints...)
return result
}
func (s *RedisSupplier) RoleGet(ctx context.Context, roleId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
@@ -86,6 +87,7 @@ func (s *RedisSupplier) RoleGetByNames(ctx context.Context, roleNames []string,
}
func (s *RedisSupplier) RoleDelete(ctx context.Context, roleId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
// XXXXXX Shouldn't this call Role
result := s.Next().RoleGet(ctx, roleId, hints...)
if result.Err == nil {
@@ -103,17 +105,17 @@ func (s *RedisSupplier) RoleDelete(ctx context.Context, roleId string, hints ...
}
func (s *RedisSupplier) RolePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
defer func() {
if keys, err := s.client.Keys("roles:*").Result(); err != nil {
mlog.Error("Redis encountered an error on read: " + err.Error())
} else {
if err := s.client.Del(keys...).Err(); err != nil {
mlog.Error("Redis encountered an error on delete: " + err.Error())
}
}
}()
result := s.Next().RolePermanentDeleteAll(ctx, hints...)
return s.Next().RolePermanentDeleteAll(ctx, hints...)
if keys, err := s.client.Keys("roles:*").Result(); err != nil {
mlog.Error("Redis encountered an error on read: " + err.Error())
} else {
if err := s.client.Del(keys...).Err(); err != nil {
mlog.Error("Redis encountered an error on delete: " + err.Error())
}
}
return result
}
func buildRedisKeyForRoleName(roleName string) string {

Просмотреть файл

@@ -10,31 +10,37 @@ import (
)
func (s *RedisSupplier) SchemeSave(ctx context.Context, scheme *model.Scheme, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemeSave(ctx, scheme, hints...)
// TODO: Redis caching.
return s.Next().SchemeSave(ctx, scheme, hints...)
return result
}
func (s *RedisSupplier) SchemeGet(ctx context.Context, schemeId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemeGet(ctx, schemeId, hints...)
// TODO: Redis caching.
return s.Next().SchemeGet(ctx, schemeId, hints...)
return result
}
func (s *RedisSupplier) SchemeGetByName(ctx context.Context, schemeName string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemeGetByName(ctx, schemeName, hints...)
// TODO: Redis caching.
return s.Next().SchemeGetByName(ctx, schemeName, hints...)
return result
}
func (s *RedisSupplier) SchemeDelete(ctx context.Context, schemeId string, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemeDelete(ctx, schemeId, hints...)
// TODO: Redis caching.
return s.Next().SchemeDelete(ctx, schemeId, hints...)
return result
}
func (s *RedisSupplier) SchemeGetAllPage(ctx context.Context, scope string, offset int, limit int, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemeGetAllPage(ctx, scope, offset, limit, hints...)
// TODO: Redis caching.
return s.Next().SchemeGetAllPage(ctx, scope, offset, limit, hints...)
return result
}
func (s *RedisSupplier) SchemePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *LayeredStoreSupplierResult {
result := s.Next().SchemePermanentDeleteAll(ctx, hints...)
// TODO: Redis caching.
return s.Next().SchemePermanentDeleteAll(ctx, hints...)
return result
}

Просмотреть файл

@@ -5,6 +5,7 @@ package sqlstore
import (
"database/sql"
"fmt"
"net/http"
"github.com/mattermost/mattermost-server/einterfaces"
@@ -128,6 +129,27 @@ func (es SqlEmojiStore) GetByName(name string) store.StoreChannel {
})
}
func (es SqlEmojiStore) GetMultipleByName(names []string) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
keys, params := MapStringsToQueryParams(names, "Emoji")
var emojis []*model.Emoji
if _, err := es.GetReplica().Select(&emojis,
`SELECT
*
FROM
Emoji
WHERE
Name IN `+keys+`
AND DeleteAt = 0`, params); err != nil {
result.Err = model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, fmt.Sprintf("names=%v, %v", names, err.Error()), http.StatusInternalServerError)
} else {
result.Data = emojis
}
})
}
func (es SqlEmojiStore) GetList(offset, limit int, sort string) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
var emoji []*model.Emoji
@@ -151,7 +173,7 @@ func (es SqlEmojiStore) GetList(offset, limit int, sort string) store.StoreChann
func (es SqlEmojiStore) Delete(id string, time int64) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
if sqlResult, err := es.GetMaster().Exec(
`Update
`UPDATE
Emoji
SET
DeleteAt = :DeleteAt,

Просмотреть файл

@@ -4,7 +4,6 @@
package sqlstore
import (
"bytes"
"fmt"
"net/http"
"regexp"
@@ -1144,19 +1143,9 @@ func (s *SqlPostStore) GetPostsCreatedAt(channelId string, time int64) store.Sto
func (s *SqlPostStore) GetPostsByIds(postIds []string) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
keys := bytes.Buffer{}
params := make(map[string]interface{})
for i, postId := range postIds {
if keys.Len() > 0 {
keys.WriteString(",")
}
keys, params := MapStringsToQueryParams(postIds, "Post")
key := "Post" + strconv.Itoa(i)
keys.WriteString(":" + key)
params[key] = postId
}
query := `SELECT * FROM Posts WHERE Id in (` + keys.String() + `) ORDER BY CreateAt DESC`
query := `SELECT * FROM Posts WHERE Id IN ` + keys + ` ORDER BY CreateAt DESC`
var posts []*model.Post
_, err := s.GetReplica().Select(&posts, query, params)

Просмотреть файл

@@ -192,22 +192,18 @@ func deleteReactionAndUpdatePost(transaction *gorp.Transaction, reaction *model.
}
const (
// Set HasReactions = true if and only if the post has reactions, update UpdateAt only if HasReactions changes
UPDATE_POST_HAS_REACTIONS_ON_DELETE_QUERY = `UPDATE
Posts
SET
UpdateAt = (CASE
WHEN HasReactions != (SELECT count(0) > 0 FROM Reactions WHERE PostId = :PostId) THEN :UpdateAt
ELSE UpdateAt
END),
UpdateAt = :UpdateAt,
HasReactions = (SELECT count(0) > 0 FROM Reactions WHERE PostId = :PostId)
WHERE
Id = :PostId`
)
func updatePostForReactionsOnDelete(transaction *gorp.Transaction, postId string) error {
_, err := transaction.Exec(UPDATE_POST_HAS_REACTIONS_ON_DELETE_QUERY, map[string]interface{}{"PostId": postId, "UpdateAt": model.GetMillis()})
updateAt := model.GetMillis()
_, err := transaction.Exec(UPDATE_POST_HAS_REACTIONS_ON_DELETE_QUERY, map[string]interface{}{"PostId": postId, "UpdateAt": updateAt})
return err
}
@@ -219,7 +215,7 @@ func updatePostForReactionsOnInsert(transaction *gorp.Transaction, postId string
HasReactions = True,
UpdateAt = :UpdateAt
WHERE
Id = :PostId AND HasReactions = False`,
Id = :PostId`,
map[string]interface{}{"PostId": postId, "UpdateAt": model.GetMillis()})
return err

28
store/sqlstore/utils.go Обычный файл
Просмотреть файл

@@ -0,0 +1,28 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
package sqlstore
import (
"bytes"
"fmt"
"strconv"
)
// Converts a list of strings into a list of query parameters and a named parameter map that can
// be used as part of a SQL query.
func MapStringsToQueryParams(list []string, paramPrefix string) (string, map[string]interface{}) {
keys := bytes.Buffer{}
params := make(map[string]interface{})
for i, entry := range list {
if keys.Len() > 0 {
keys.WriteString(",")
}
key := paramPrefix + strconv.Itoa(i)
keys.WriteString(":" + key)
params[key] = entry
}
return fmt.Sprintf("(%v)", keys.String()), params
}

32
store/sqlstore/utils_test.go Обычный файл
Просмотреть файл

@@ -0,0 +1,32 @@
package sqlstore
import (
"testing"
)
func TestMapStringsToQueryParams(t *testing.T) {
t.Run("one item", func(t *testing.T) {
input := []string{"apple"}
keys, params := MapStringsToQueryParams(input, "Fruit")
if len(params) != 1 || params["Fruit0"] != "apple" {
t.Fatal("returned incorrect params", params)
} else if keys != "(:Fruit0)" {
t.Fatal("returned incorrect query", keys)
}
})
t.Run("multiple items", func(t *testing.T) {
input := []string{"carrot", "tomato", "potato"}
keys, params := MapStringsToQueryParams(input, "Vegetable")
if len(params) != 3 || params["Vegetable0"] != "carrot" ||
params["Vegetable1"] != "tomato" || params["Vegetable2"] != "potato" {
t.Fatal("returned incorrect params", params)
} else if keys != "(:Vegetable0,:Vegetable1,:Vegetable2)" {
t.Fatal("returned incorrect query", keys)
}
})
}

Просмотреть файл

@@ -427,6 +427,7 @@ type EmojiStore interface {
Save(emoji *model.Emoji) StoreChannel
Get(id string, allowFromCache bool) StoreChannel
GetByName(name string) StoreChannel
GetMultipleByName(names []string) StoreChannel
GetList(offset, limit int, sort string) StoreChannel
Delete(id string, time int64) StoreChannel
Search(name string, prefixOnly bool, limit int) StoreChannel

Просмотреть файл

@@ -17,6 +17,7 @@ func TestEmojiStore(t *testing.T, ss store.Store) {
t.Run("EmojiSaveDelete", func(t *testing.T) { testEmojiSaveDelete(t, ss) })
t.Run("EmojiGet", func(t *testing.T) { testEmojiGet(t, ss) })
t.Run("EmojiGetByName", func(t *testing.T) { testEmojiGetByName(t, ss) })
t.Run("EmojiGetMultipleByName", func(t *testing.T) { testEmojiGetMultipleByName(t, ss) })
t.Run("EmojiGetList", func(t *testing.T) { testEmojiGetList(t, ss) })
t.Run("EmojiSearch", func(t *testing.T) { testEmojiSearch(t, ss) })
}
@@ -132,6 +133,64 @@ func testEmojiGetByName(t *testing.T, ss store.Store) {
}
}
func testEmojiGetMultipleByName(t *testing.T, ss store.Store) {
emojis := []model.Emoji{
{
CreatorId: model.NewId(),
Name: model.NewId(),
},
{
CreatorId: model.NewId(),
Name: model.NewId(),
},
{
CreatorId: model.NewId(),
Name: model.NewId(),
},
}
for i, emoji := range emojis {
emojis[i] = *store.Must(ss.Emoji().Save(&emoji)).(*model.Emoji)
}
defer func() {
for _, emoji := range emojis {
store.Must(ss.Emoji().Delete(emoji.Id, time.Now().Unix()))
}
}()
t.Run("one emoji", func(t *testing.T) {
if result := <-ss.Emoji().GetMultipleByName([]string{emojis[0].Name}); result.Err != nil {
t.Fatal("could not get emoji", result.Err)
} else if received := result.Data.([]*model.Emoji); len(received) != 1 || *received[0] != emojis[0] {
t.Fatal("got incorrect emoji")
}
})
t.Run("multiple emojis", func(t *testing.T) {
if result := <-ss.Emoji().GetMultipleByName([]string{emojis[0].Name, emojis[1].Name, emojis[2].Name}); result.Err != nil {
t.Fatal("could not get emojis", result.Err)
} else if received := result.Data.([]*model.Emoji); len(received) != 3 {
t.Fatal("got incorrect emojis")
}
})
t.Run("one nonexistent emoji", func(t *testing.T) {
if result := <-ss.Emoji().GetMultipleByName([]string{"ab"}); result.Err != nil {
t.Fatal("could not get emoji", result.Err)
} else if received := result.Data.([]*model.Emoji); len(received) != 0 {
t.Fatal("got incorrect emoji")
}
})
t.Run("multiple emojis with nonexistent names", func(t *testing.T) {
if result := <-ss.Emoji().GetMultipleByName([]string{emojis[0].Name, emojis[1].Name, emojis[2].Name, "abcd", "1234"}); result.Err != nil {
t.Fatal("could not get emojis", result.Err)
} else if received := result.Data.([]*model.Emoji); len(received) != 3 {
t.Fatal("got incorrect emojis")
}
})
}
func testEmojiGetList(t *testing.T, ss store.Store) {
emojis := []model.Emoji{
{

Просмотреть файл

@@ -77,6 +77,22 @@ func (_m *EmojiStore) GetList(offset int, limit int, sort string) store.StoreCha
return r0
}
// GetMultipleByName provides a mock function with given fields: names
func (_m *EmojiStore) GetMultipleByName(names []string) store.StoreChannel {
ret := _m.Called(names)
var r0 store.StoreChannel
if rf, ok := ret.Get(0).(func([]string) store.StoreChannel); ok {
r0 = rf(names)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(store.StoreChannel)
}
}
return r0
}
// Save provides a mock function with given fields: emoji
func (_m *EmojiStore) Save(emoji *model.Emoji) store.StoreChannel {
ret := _m.Called(emoji)

Просмотреть файл

@@ -61,8 +61,8 @@ func testReactionSave(t *testing.T, ss store.Store) {
t.Fatal(result.Err)
}
if postList := store.Must(ss.Post().Get(reaction2.PostId)).(*model.PostList); postList.Posts[post.Id].UpdateAt != secondUpdateAt {
t.Fatal("shouldn't mark as updated when HasReactions hasn't changed")
if postList := store.Must(ss.Post().Get(reaction2.PostId)).(*model.PostList); postList.Posts[post.Id].UpdateAt == secondUpdateAt {
t.Fatal("should've marked post as updated even if HasReactions doesn't change")
}
// different post
@@ -123,7 +123,7 @@ func testReactionDelete(t *testing.T, ss store.Store) {
if postList := store.Must(ss.Post().Get(post.Id)).(*model.PostList); postList.Posts[post.Id].HasReactions {
t.Fatal("should've set HasReactions = false on post")
} else if postList.Posts[post.Id].UpdateAt == firstUpdateAt {
t.Fatal("shouldn't mark as updated when HasReactions has changed after deleting reactions")
t.Fatal("should mark post as updated after deleting reactions")
}
}