MM-54640 Add API to get multiple emojis by name at once (#24651)

* MM-54640 Add API to get multiple emojis by name at once

* Fix status code when too many names are requested

* Address feedback

* Update unit tests

* Fix styling

* Fix more styling

* Fix mismatched i18n id
This commit is contained in:
Harrison Healey 2023-10-17 12:03:28 -04:00 committed by GitHub
parent 77cc356d46
commit 3d0fd16666
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 241 additions and 35 deletions

View File

@ -17,11 +17,13 @@ import (
const (
EmojiMaxAutocompleteItems = 100
GetEmojisByNamesMax = 200
)
func (api *API) InitEmoji() {
api.BaseRoutes.Emojis.Handle("", api.APISessionRequired(createEmoji)).Methods("POST")
api.BaseRoutes.Emojis.Handle("", api.APISessionRequired(getEmojiList)).Methods("GET")
api.BaseRoutes.Emojis.Handle("/names", api.APISessionRequired(getEmojisByNames)).Methods("POST")
api.BaseRoutes.Emojis.Handle("/search", api.APISessionRequired(searchEmojis)).Methods("POST")
api.BaseRoutes.Emojis.Handle("/autocomplete", api.APISessionRequired(autocompleteEmojis)).Methods("GET")
api.BaseRoutes.Emoji.Handle("", api.APISessionRequired(deleteEmoji)).Methods("DELETE")
@ -221,6 +223,11 @@ func getEmojiByName(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if !*c.App.Config().ServiceSettings.EnableCustomEmoji {
c.Err = model.NewAppError("getEmojiByName", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented)
return
}
emoji, err := c.App.GetEmojiByName(c.AppContext, c.Params.EmojiName)
if err != nil {
c.Err = err
@ -232,6 +239,36 @@ func getEmojiByName(c *Context, w http.ResponseWriter, r *http.Request) {
}
}
func getEmojisByNames(c *Context, w http.ResponseWriter, r *http.Request) {
names := model.ArrayFromJSON(r.Body)
if len(names) == 0 {
c.SetInvalidParam("names")
return
}
if !*c.App.Config().ServiceSettings.EnableCustomEmoji {
c.Err = model.NewAppError("getEmojisByNames", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented)
return
}
if len(names) > GetEmojisByNamesMax {
c.Err = model.NewAppError("getEmojisByNames", "api.emoji.get_multiple_by_name_too_many.request_error", map[string]any{
"MaxNames": GetEmojisByNamesMax,
}, "", http.StatusBadRequest)
return
}
emojis, err := c.App.GetMultipleEmojiByName(c.AppContext, names)
if err != nil {
c.Err = err
return
}
if err := json.NewEncoder(w).Encode(emojis); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}
func getEmojiImage(c *Context, w http.ResponseWriter, r *http.Request) {
c.RequireEmojiId()
if c.Err != nil {

View File

@ -280,6 +280,69 @@ func TestGetEmojiList(t *testing.T) {
require.Greater(t, len(listEmoji), 0, "should return more than 0")
}
func TestGetEmojisByNames(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
// Set up some custom emojis
adminClient := th.SystemAdminClient
imageBytes := utils.CreateTestJpeg(t, 10, 10)
emoji1 := &model.Emoji{
CreatorId: th.SystemAdminUser.Id,
Name: model.NewId(),
}
emoji1, _, err := adminClient.CreateEmoji(context.Background(), emoji1, imageBytes, "emoji.jpg")
require.NoError(t, err)
emoji2 := &model.Emoji{
CreatorId: th.SystemAdminUser.Id,
Name: model.NewId(),
}
emoji2, _, err = adminClient.CreateEmoji(context.Background(), emoji2, imageBytes, "emoji.jpg")
require.NoError(t, err)
client := th.Client
t.Run("should return a single emoji", func(t *testing.T) {
emojis, _, err := client.GetEmojisByNames(context.Background(), []string{emoji1.Name})
require.NoError(t, err)
require.Len(t, emojis, 1)
assert.Equal(t, emoji1.Id, emojis[0].Id)
})
t.Run("should return multiple emojis", func(t *testing.T) {
emojis, _, err := client.GetEmojisByNames(context.Background(), []string{emoji1.Name, emoji2.Name})
require.NoError(t, err)
require.Len(t, emojis, 2)
assert.Equal(t, emoji1.Id, emojis[0].Id)
assert.Equal(t, emoji2.Id, emojis[1].Id)
})
t.Run("should ignore non-existent emojis", func(t *testing.T) {
emojis, _, err := client.GetEmojisByNames(context.Background(), []string{emoji1.Name, emoji2.Name, model.NewId()})
require.NoError(t, err)
require.Len(t, emojis, 2)
assert.Equal(t, emoji1.Id, emojis[0].Id)
assert.Equal(t, emoji2.Id, emojis[1].Id)
})
t.Run("should return an error when too many emojis are requested", func(t *testing.T) {
names := make([]string, GetEmojisByNamesMax+1)
for i := 0; i < len(names); i++ {
names[i] = emoji1.Name
}
_, _, err := client.GetEmojisByNames(context.Background(), names)
require.Error(t, err)
})
}
func TestDeleteEmoji(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()

View File

@ -61,6 +61,7 @@ func NewMainHelperWithOptions(options *HelperOptions) *MainHelper {
// Unset environment variables commonly set for development that interfere with tests.
os.Unsetenv("MM_SERVICESETTINGS_SITEURL")
os.Unsetenv("MM_SERVICESETTINGS_LISTENADDRESS")
os.Unsetenv("MM_SERVICESETTINGS_CONNECTIONSECURITY")
os.Unsetenv("MM_SERVICESETTINGS_ENABLEDEVELOPER")
var mainHelper MainHelper

View File

@ -1796,6 +1796,10 @@
"id": "api.emoji.get_image.read.app_error",
"translation": "Unable to read image file for emoji."
},
{
"id": "api.emoji.get_multiple_by_name_too_many.request_error",
"translation": "Unable to get that many emojis by name. Only {{.MaxNames}} emojis can be requested at once."
},
{
"id": "api.emoji.storage.app_error",
"translation": "File storage not configured properly. Please configure for either S3 or local server file storage."

View File

@ -6618,6 +6618,26 @@ func (c *Client4) GetSortedEmojiList(ctx context.Context, page, perPage int, sor
return list, BuildResponse(r), nil
}
// GetEmojisByNames takes an array of custom emoji names and returns an array of those emojis.
func (c *Client4) GetEmojisByNames(ctx context.Context, names []string) ([]*Emoji, *Response, error) {
buf, err := json.Marshal(names)
if err != nil {
return nil, nil, NewAppError("GetEmojisByNames", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
r, err := c.DoAPIPostBytes(ctx, c.emojisRoute()+"/names", buf)
if err != nil {
return nil, BuildResponse(r), err
}
defer closeBody(r)
var list []*Emoji
if err := json.NewDecoder(r.Body).Decode(&list); err != nil {
return nil, nil, NewAppError("GetEmojisByNames", "api.unmarshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
return list, BuildResponse(r), nil
}
// DeleteEmoji delete an custom emoji on the provided emoji id string.
func (c *Client4) DeleteEmoji(ctx context.Context, emojiId string) (*Response, error) {
r, err := c.DoAPIDelete(ctx, c.emojiRoute(emojiId))

View File

@ -264,36 +264,75 @@ describe('Actions.Emojis', () => {
expect(state.entities.emojis.nonExistentEmoji.has(missingName)).toBeTruthy();
});
it('getCustomEmojisByName', async () => {
const testImageData = fs.createReadStream('src/packages/mattermost-redux/test/assets/images/test.png');
describe('getCustomEmojisByName', () => {
test('should be able to request a single emoji', async () => {
const emoji1 = TestHelper.getCustomEmojiMock({name: 'emoji1', id: 'emojiId1'});
nock(Client4.getBaseRoute()).
post('/emoji').
reply(201, {id: TestHelper.generateId(), create_at: 1507918415696, update_at: 1507918415696, delete_at: 0, creator_id: TestHelper.basicUser!.id, name: TestHelper.generateId()});
nock(Client4.getBaseRoute()).
post('/emoji/names', ['emoji1']).
reply(200, [emoji1]);
const {data: created} = await Actions.createCustomEmoji(
{
name: TestHelper.generateId(),
creator_id: TestHelper.basicUser!.id,
},
testImageData,
)(store.dispatch, store.getState) as ActionResult;
await store.dispatch(Actions.getCustomEmojisByName(['emoji1']));
nock(Client4.getBaseRoute()).
get(`/emoji/name/${created.name}`).
reply(200, created);
const state = store.getState();
expect(state.entities.emojis.customEmoji[emoji1.id]).toEqual(emoji1);
});
const missingName = TestHelper.generateId();
test('should be able to request multiple emojis', async () => {
const emoji1 = TestHelper.getCustomEmojiMock({name: 'emoji1', id: 'emojiId1'});
const emoji2 = TestHelper.getCustomEmojiMock({name: 'emoji2', id: 'emojiId2'});
nock(Client4.getBaseRoute()).
get(`/emoji/name/${missingName}`).
reply(404, {message: 'Not found', status_code: 404});
nock(Client4.getBaseRoute()).
post('/emoji/names', ['emoji1', 'emoji2']).
reply(200, [emoji1, emoji2]);
await Actions.getCustomEmojisByName([created.name, missingName])(store.dispatch, store.getState);
await store.dispatch(Actions.getCustomEmojisByName(['emoji1', 'emoji2']));
const state = store.getState();
expect(state.entities.emojis.customEmoji[created.id]).toBeTruthy();
expect(state.entities.emojis.nonExistentEmoji.has(missingName)).toBeTruthy();
const state = store.getState();
expect(state.entities.emojis.customEmoji[emoji1.id]).toEqual(emoji1);
expect(state.entities.emojis.customEmoji[emoji2.id]).toEqual(emoji2);
});
test('should correctly track non-existent emojis', async () => {
const emoji1 = TestHelper.getCustomEmojiMock({name: 'emoji1', id: 'emojiId1'});
nock(Client4.getBaseRoute()).
post('/emoji/names', ['emoji1', 'emoji2']).
reply(200, [emoji1]);
await store.dispatch(Actions.getCustomEmojisByName(['emoji1', 'emoji2']));
const state = store.getState();
expect(state.entities.emojis.customEmoji[emoji1.id]).toEqual(emoji1);
expect(state.entities.emojis.nonExistentEmoji).toEqual(new Set(['emoji2']));
});
test('should be able to request over 200 emojis', async () => {
const emojis = [];
for (let i = 0; i < 500; i++) {
emojis.push(TestHelper.getCustomEmojiMock({name: 'emoji' + i, id: 'emojiId' + i}));
}
const names = emojis.map((emoji) => emoji.name);
nock(Client4.getBaseRoute()).
post('/emoji/names', names.slice(0, 200)).
reply(200, emojis.slice(0, 200));
nock(Client4.getBaseRoute()).
post('/emoji/names', names.slice(200, 400)).
reply(200, emojis.slice(200, 400));
nock(Client4.getBaseRoute()).
post('/emoji/names', names.slice(400, 500)).
reply(200, emojis.slice(400, 500));
await store.dispatch(Actions.getCustomEmojisByName(names));
const state = store.getState();
expect(Object.keys(state.entities.emojis.customEmoji)).toHaveLength(emojis.length);
for (const emoji of emojis) {
expect(state.entities.emojis.customEmoji[emoji.id]).toEqual(emoji);
}
});
});
it('getCustomEmojisInText', async () => {
@ -311,15 +350,11 @@ describe('Actions.Emojis', () => {
testImageData,
)(store.dispatch, store.getState) as ActionResult;
nock(Client4.getBaseRoute()).
get(`/emoji/name/${created.name}`).
reply(200, created);
const missingName = TestHelper.generateId();
nock(Client4.getBaseRoute()).
get(`/emoji/name/${missingName}`).
reply(404, {message: 'Not found', status_code: 404});
post('/emoji/names', [created.name, missingName]).
reply(200, [created]);
await Actions.getCustomEmojisInText(`some text :${created.name}: :${missingName}:`)(store.dispatch, store.getState);

View File

@ -1,12 +1,15 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import type {AnyAction} from 'redux';
import {batchActions} from 'redux-batched-actions';
import type {CustomEmoji} from '@mattermost/types/emojis';
import {EmojiTypes} from 'mattermost-redux/action_types';
import {Client4} from 'mattermost-redux/client';
import {getCustomEmojisByName as selectCustomEmojisByName} from 'mattermost-redux/selectors/entities/emojis';
import type {GetStateFunc, DispatchFunc, ActionFunc, ActionResult} from 'mattermost-redux/types/actions';
import type {GetStateFunc, DispatchFunc, ActionFunc} from 'mattermost-redux/types/actions';
import {parseNeededCustomEmojisFromText} from 'mattermost-redux/utils/emoji_utils';
import {logError} from './errors';
@ -69,16 +72,52 @@ export function getCustomEmojiByName(name: string): ActionFunc {
}
export function getCustomEmojisByName(names: string[]): ActionFunc {
return async (dispatch: DispatchFunc) => {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
if (!names || names.length === 0) {
return {data: true};
}
const promises: Array<Promise<ActionResult|ActionResult[]>> = [];
names.forEach((name) => promises.push(dispatch(getCustomEmojiByName(name))));
// If necessary, split up the list of names into batches based on api4.GetEmojisByNamesMax on the server
const batchSize = 200;
await Promise.all(promises);
return {data: true};
const batches = [];
for (let i = 0; i < names.length; i += batchSize) {
batches.push(names.slice(i, i + batchSize));
}
let results;
try {
results = await Promise.all(batches.map((batch) => {
return Client4.getCustomEmojisByNames(batch);
}));
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
dispatch(logError(error));
return {error};
}
const data = results.flat();
const actions: AnyAction[] = [{
type: EmojiTypes.RECEIVED_CUSTOM_EMOJIS,
data,
}];
if (data.length !== names.length) {
const foundNames = new Set(data.map((emoji) => emoji.name));
for (const name of names) {
if (foundNames.has(name)) {
continue;
}
actions.push({
type: EmojiTypes.CUSTOM_EMOJI_DOES_NOT_EXIST,
data: name,
});
}
}
return dispatch(actions.length > 1 ? batchActions(actions) : actions[0]);
};
}

View File

@ -2749,6 +2749,13 @@ export default class Client4 {
);
};
getCustomEmojisByNames = (names: string[]) => {
return this.doFetch<CustomEmoji[]>(
`${this.getEmojisRoute()}/names`,
{method: 'post', body: JSON.stringify(names)},
);
};
getCustomEmojis = (page = 0, perPage = PER_PAGE_DEFAULT, sort = '') => {
return this.doFetch<CustomEmoji[]>(
`${this.getEmojisRoute()}${buildQueryString({page, per_page: perPage, sort})}`,