[MM-16389] Add icon_emoji parameter to webhooks (#11586)

[MM-16389] Add icon_emoji field to posts from incoming webhooks

To be used to substitute the profile picture on posts.

store/sqlstore/emoji_store.go

@@ -19,7 +19,8 @@ const (
 	EMOJI_CACHE_SEC  = 1800 // 30 mins
 )
 
-var emojiCache *utils.Cache = utils.NewLru(EMOJI_CACHE_SIZE)
+var emojiCacheById = utils.NewLru(EMOJI_CACHE_SIZE)
+var emojiIdCacheByName = utils.NewLru(EMOJI_CACHE_SIZE)
 
 type SqlEmojiStore struct {
 	SqlStore
@@ -60,67 +61,32 @@ func (es SqlEmojiStore) Save(emoji *model.Emoji) (*model.Emoji, *model.AppError)
 	if err := es.GetMaster().Insert(emoji); err != nil {
 		return nil, model.NewAppError("SqlEmojiStore.Save", "store.sql_emoji.save.app_error", nil, "id="+emoji.Id+", "+err.Error(), http.StatusInternalServerError)
 	}
+
 	return emoji, nil
 }
 
 func (es SqlEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) {
 	if allowFromCache {
-		if cacheItem, ok := emojiCache.Get(id); ok {
-			if es.metrics != nil {
-				es.metrics.IncrementMemCacheHitCounter("Emoji")
-			}
-			return cacheItem.(*model.Emoji), nil
-		}
-		if es.metrics != nil {
-			es.metrics.IncrementMemCacheMissCounter("Emoji")
-		}
-	} else {
-		if es.metrics != nil {
-			es.metrics.IncrementMemCacheMissCounter("Emoji")
+		if emoji, ok := es.getFromCacheById(id); ok {
+			return emoji, nil
 		}
 	}
 
-	var emoji *model.Emoji
+	return es.getBy("Id", id, allowFromCache)
+}
 
-	if err := es.GetReplica().SelectOne(&emoji,
-		`SELECT
-			*
-		FROM
-			Emoji
-		WHERE
-			Id = :Id
-			AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil {
-		return nil, model.NewAppError("SqlEmojiStore.Get", "store.sql_emoji.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound)
+func (es SqlEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) {
+	if id, ok := model.GetSystemEmojiId(name); ok {
+		return es.Get(id, allowFromCache)
 	}
 
 	if allowFromCache {
-		emojiCache.AddWithExpiresInSecs(id, emoji, EMOJI_CACHE_SEC)
-	}
-
-	return emoji, nil
-}
-
-func (es SqlEmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) {
-
-	var emoji *model.Emoji
-
-	if err := es.GetReplica().SelectOne(&emoji,
-		`SELECT
-			*
-		FROM
-			Emoji
-		WHERE
-			Name = :Name
-			AND DeleteAt = 0`, map[string]interface{}{"Name": name}); err != nil {
-
-		if err == sql.ErrNoRows {
-			return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusNotFound)
+		if emoji, ok := es.getFromCacheByName(name); ok {
+			return emoji, nil
 		}
-
-		return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusInternalServerError)
 	}
 
-	return emoji, nil
+	return es.getBy("Name", name, allowFromCache)
 }
 
 func (es SqlEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, *model.AppError) {
@@ -158,7 +124,7 @@ func (es SqlEmojiStore) GetList(offset, limit int, sort string) ([]*model.Emoji,
 	return emoji, nil
 }
 
-func (es SqlEmojiStore) Delete(id string, time int64) *model.AppError {
+func (es SqlEmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError {
 	if sqlResult, err := es.GetMaster().Exec(
 		`UPDATE
 			Emoji
@@ -167,13 +133,14 @@ func (es SqlEmojiStore) Delete(id string, time int64) *model.AppError {
 			UpdateAt = :UpdateAt
 		WHERE
 			Id = :Id
-			AND DeleteAt = 0`, map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": id}); err != nil {
-		return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.app_error", nil, "id="+id+", err="+err.Error(), http.StatusInternalServerError)
+			AND DeleteAt = 0`, map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": emoji.Id}); err != nil {
+		return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.app_error", nil, "id="+emoji.Id+", err="+err.Error(), http.StatusInternalServerError)
 	} else if rows, _ := sqlResult.RowsAffected(); rows == 0 {
-		return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+id+", err="+err.Error(), http.StatusBadRequest)
+		return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+emoji.Id+", err="+err.Error(), http.StatusBadRequest)
 	}
 
-	emojiCache.Remove(id)
+	es.removeFromCache(emoji)
+
 	return nil
 }
 
@@ -201,3 +168,74 @@ func (es SqlEmojiStore) Search(name string, prefixOnly bool, limit int) ([]*mode
 	}
 	return emojis, nil
 }
+
+// getBy returns one active (not deleted) emoji, found by any one column (what/key).
+func (es SqlEmojiStore) getBy(what string, key interface{}, addToCache bool) (*model.Emoji, *model.AppError) {
+	var emoji *model.Emoji
+
+	err := es.GetReplica().SelectOne(&emoji,
+		`SELECT
+			*
+		FROM
+			Emoji
+		WHERE
+			`+what+` = :Key
+			AND DeleteAt = 0`, map[string]interface{}{"Key": key})
+	if err != nil {
+		var status int
+		if err == sql.ErrNoRows {
+			status = http.StatusNotFound
+		} else {
+			status = http.StatusInternalServerError
+		}
+		return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get.app_error", nil, "key="+fmt.Sprintf("%v", key)+", "+err.Error(), status)
+	}
+
+	if addToCache {
+		es.addToCache(emoji)
+	}
+
+	return emoji, nil
+}
+
+func (es SqlEmojiStore) addToCache(emoji *model.Emoji) {
+	emojiCacheById.AddWithExpiresInSecs(emoji.Id, emoji, EMOJI_CACHE_SEC)
+	emojiIdCacheByName.AddWithExpiresInSecs(emoji.Name, emoji.Id, EMOJI_CACHE_SEC)
+}
+
+func (es SqlEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) {
+	if cacheItem, ok := emojiCacheById.Get(id); ok {
+		es.incrementMemCacheHitCounter("Emoji")
+		return cacheItem.(*model.Emoji), true
+	}
+	es.incrementMemCacheMissCounter("Emoji")
+	return nil, false
+}
+
+func (es SqlEmojiStore) getFromCacheByName(name string) (*model.Emoji, bool) {
+	if id, ok := emojiIdCacheByName.Get(name); ok {
+		return es.getFromCacheById(id.(string))
+	}
+
+	es.incrementMemCacheMissCounter("Emoji")
+	return nil, false
+}
+
+func (es SqlEmojiStore) incrementMemCacheHitCounter(cache string) {
+	if es.metrics == nil {
+		return
+	}
+	es.metrics.IncrementMemCacheHitCounter(cache)
+}
+
+func (es SqlEmojiStore) incrementMemCacheMissCounter(cache string) {
+	if es.metrics == nil {
+		return
+	}
+	es.metrics.IncrementMemCacheMissCounter(cache)
+}
+
+func (es SqlEmojiStore) removeFromCache(emoji *model.Emoji) {
+	emojiCacheById.Remove(emoji.Id)
+	emojiIdCacheByName.Remove(emoji.Name)
+}

store/store.go

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

store/storetest/emoji_store.go

@@ -21,6 +21,7 @@ func TestEmojiStore(t *testing.T, ss store.Store) {
 	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) })
+	t.Run("EmojiCaching", func(t *testing.T) { testEmojiCaching(t, ss) })
 }
 
 func testEmojiSaveDelete(t *testing.T, ss store.Store) {
@@ -45,7 +46,7 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) {
 		t.Fatal("shouldn't be able to save emoji with duplicate name")
 	}
 
-	if err := ss.Emoji().Delete(emoji1.Id, time.Now().Unix()); err != nil {
+	if err := ss.Emoji().Delete(emoji1, time.Now().Unix()); err != nil {
 		t.Fatal(err)
 	}
 
@@ -53,7 +54,7 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) {
 		t.Fatal("should be able to save emoji with duplicate name now that original has been deleted", err)
 	}
 
-	if err := ss.Emoji().Delete(emoji2.Id, time.Now().Unix()+1); err != nil {
+	if err := ss.Emoji().Delete(&emoji2, time.Now().Unix()+1); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -81,7 +82,7 @@ func testEmojiGet(t *testing.T, ss store.Store) {
 	}
 	defer func() {
 		for _, emoji := range emojis {
-			err := ss.Emoji().Delete(emoji.Id, time.Now().Unix())
+			err := ss.Emoji().Delete(&emoji, time.Now().Unix())
 			require.Nil(t, err)
 		}
 	}()
@@ -97,12 +98,61 @@ func testEmojiGet(t *testing.T, ss store.Store) {
 			t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err)
 		}
 	}
+}
 
-	for _, emoji := range emojis {
-		if _, err := ss.Emoji().Get(emoji.Id, true); err != nil {
-			t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err)
+func testEmojiCaching(t *testing.T, ss store.Store) {
+	emojis := make([]*model.Emoji, 3)
+	for i := range emojis {
+		emojis[i] = &model.Emoji{
+			CreatorId: model.NewId(),
+			Name:      model.NewId(),
 		}
 	}
+
+	for _, emoji := range emojis {
+		_, err := ss.Emoji().Save(emoji)
+		require.Nil(t, err)
+	}
+	defer func() {
+		for _, emoji := range emojis {
+			err := ss.Emoji().Delete(emoji, time.Now().Unix())
+			require.Nil(t, err)
+		}
+	}()
+
+	var retrievedEmoji *model.Emoji
+	var cachedEmoji *model.Emoji
+	var err *model.AppError
+
+	for _, emoji := range emojis {
+		cachedEmoji, err = ss.Emoji().Get(emoji.Id, true)
+		assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id)
+
+		retrievedEmoji, err = ss.Emoji().Get(emoji.Id, false)
+		if assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) {
+			assert.Falsef(t, retrievedEmoji == cachedEmoji, "should not be the same as cached with id %v", emoji.Id)
+		}
+
+		retrievedEmoji, err = ss.Emoji().Get(emoji.Id, true)
+		if assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) {
+			assert.Truef(t, retrievedEmoji == cachedEmoji, "should be the cached emoji with id %v", emoji.Id)
+		}
+
+		retrievedEmoji, err = ss.Emoji().GetByName(emoji.Name, false)
+		if assert.Nilf(t, err, "should be able to retrieve emoji with name %v", emoji.Name) {
+			assert.Falsef(t, retrievedEmoji == cachedEmoji, "should not be the same as cached with name %v", emoji.Name)
+		}
+
+		retrievedEmoji, _ = ss.Emoji().GetByName(emoji.Name, true)
+		if assert.Nilf(t, err, "should be able to retrieve emoji with name %v", emoji.Name) {
+			assert.Truef(t, retrievedEmoji == cachedEmoji, "should be the cached emoji with name %v", emoji.Name)
+		}
+	}
+
+	_, err = ss.Emoji().Get(model.NewId(), false)
+	assert.NotNilf(t, err, "should not retrieve emoji with unsaved ID")
+	_, err = ss.Emoji().GetByName(model.NewId(), false)
+	assert.NotNilf(t, err, "should not retrieve emoji with unsaved name")
 }
 
 func testEmojiGetByName(t *testing.T, ss store.Store) {
@@ -128,13 +178,13 @@ func testEmojiGetByName(t *testing.T, ss store.Store) {
 	}
 	defer func() {
 		for _, emoji := range emojis {
-			err := ss.Emoji().Delete(emoji.Id, time.Now().Unix())
+			err := ss.Emoji().Delete(&emoji, time.Now().Unix())
 			require.Nil(t, err)
 		}
 	}()
 
 	for _, emoji := range emojis {
-		if _, err := ss.Emoji().GetByName(emoji.Name); err != nil {
+		if _, err := ss.Emoji().GetByName(emoji.Name, true); err != nil {
 			t.Fatalf("failed to get emoji with name %v: %v", emoji.Name, err)
 		}
 	}
@@ -163,7 +213,7 @@ func testEmojiGetMultipleByName(t *testing.T, ss store.Store) {
 	}
 	defer func() {
 		for _, emoji := range emojis {
-			err := ss.Emoji().Delete(emoji.Id, time.Now().Unix())
+			err := ss.Emoji().Delete(&emoji, time.Now().Unix())
 			require.Nil(t, err)
 		}
 	}()
@@ -224,7 +274,7 @@ func testEmojiGetList(t *testing.T, ss store.Store) {
 	}
 	defer func() {
 		for _, emoji := range emojis {
-			err := ss.Emoji().Delete(emoji.Id, time.Now().Unix())
+			err := ss.Emoji().Delete(&emoji, time.Now().Unix())
 			require.Nil(t, err)
 		}
 	}()
@@ -290,7 +340,7 @@ func testEmojiSearch(t *testing.T, ss store.Store) {
 	}
 	defer func() {
 		for _, emoji := range emojis {
-			err := ss.Emoji().Delete(emoji.Id, time.Now().Unix())
+			err := ss.Emoji().Delete(&emoji, time.Now().Unix())
 			require.Nil(t, err)
 		}
 	}()

store/storetest/mocks/EmojiStore.go

@@ -12,13 +12,13 @@ type EmojiStore struct {
 	mock.Mock
 }
 
-// Delete provides a mock function with given fields: id, time
-func (_m *EmojiStore) Delete(id string, time int64) *model.AppError {
-	ret := _m.Called(id, time)
+// Delete provides a mock function with given fields: emoji, time
+func (_m *EmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError {
+	ret := _m.Called(emoji, time)
 
 	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string, int64) *model.AppError); ok {
-		r0 = rf(id, time)
+	if rf, ok := ret.Get(0).(func(*model.Emoji, int64) *model.AppError); ok {
+		r0 = rf(emoji, time)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*model.AppError)
@@ -53,13 +53,13 @@ func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model.
 	return r0, r1
 }
 
-// GetByName provides a mock function with given fields: name
-func (_m *EmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) {
-	ret := _m.Called(name)
+// GetByName provides a mock function with given fields: name, allowFromCache
+func (_m *EmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) {
+	ret := _m.Called(name, allowFromCache)
 
 	var r0 *model.Emoji
-	if rf, ok := ret.Get(0).(func(string) *model.Emoji); ok {
-		r0 = rf(name)
+	if rf, ok := ret.Get(0).(func(string, bool) *model.Emoji); ok {
+		r0 = rf(name, allowFromCache)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*model.Emoji)
@@ -67,8 +67,8 @@ func (_m *EmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) {
 	}
 
 	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
-		r1 = rf(name)
+	if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok {
+		r1 = rf(name, allowFromCache)
 	} else {
 		if ret.Get(1) != nil {
 			r1 = ret.Get(1).(*model.AppError)