MM-56906: Remove redundant calls on team switch (#30771)
On page load, we load ALL channels and channel members from all teams. But then, on team_switch, we would again load channels and channel members from that team. This was redundant and mainly kept because previously the websocket events were considered unreliable. Now with reliable websockets, and client-side pings, we can detect broken connections faster and recover without loss. Additionally, the getAllChannelMembers call would page through all responses on the client side. This was inefficient and incur extra latency. To optimize for this, we introduce server-side streaming of the full response if page is set to -1. This optimizes the intial response as well. https://mattermost.atlassian.net/browse/MM-56906 ```release-note Optimize team switch operation by removing calls to get channels and channel members. ``` Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
883711c72d
Коммит
4803892492
@@ -34,6 +34,7 @@ import {
|
||||
getChannelMemberCountsByGroup,
|
||||
fetchAllMyChannelMembers,
|
||||
fetchAllMyTeamsChannels,
|
||||
fetchChannelsAndMembers,
|
||||
} from 'mattermost-redux/actions/channels';
|
||||
import {getCloudSubscription} from 'mattermost-redux/actions/cloud';
|
||||
import {clearErrors, logError} from 'mattermost-redux/actions/errors';
|
||||
@@ -882,6 +883,7 @@ async function handleTeamAddedEvent(msg) {
|
||||
await dispatch(TeamActions.getMyTeamMembers());
|
||||
const state = getState();
|
||||
await dispatch(TeamActions.getMyTeamUnreads(isCollapsedThreadsEnabled(state)));
|
||||
await dispatch(fetchChannelsAndMembers(msg.data.team_id));
|
||||
const license = getLicense(state);
|
||||
if (license.Cloud === 'true') {
|
||||
dispatch(getTeamsUsage());
|
||||
|
||||
@@ -30,14 +30,6 @@ export function initializeTeam(team: Team): ActionFuncAsync<Team> {
|
||||
const currentUser = getCurrentUser(state);
|
||||
LocalStorageStore.setPreviousTeamId(currentUser.id, team.id);
|
||||
|
||||
try {
|
||||
await dispatch(fetchChannelsAndMembers(team.id));
|
||||
} catch (error) {
|
||||
forceLogoutIfNecessary(error as ServerError, dispatch, getState);
|
||||
dispatch(logError(error as ServerError));
|
||||
return {error: error as ServerError};
|
||||
}
|
||||
|
||||
const enabledUserStatuses = getIsUserStatusesConfigEnabled(state);
|
||||
if (enabledUserStatuses) {
|
||||
dispatch(addVisibleUsersInCurrentChannelAndSelfToStatusPoll());
|
||||
@@ -99,6 +91,14 @@ export function joinTeam(teamname: string, joinedOnFirstLoad: boolean): ActionFu
|
||||
|
||||
await dispatch(initializeTeam(team));
|
||||
|
||||
try {
|
||||
await dispatch(fetchChannelsAndMembers(team.id));
|
||||
} catch (error) {
|
||||
forceLogoutIfNecessary(error as ServerError, dispatch, getState);
|
||||
dispatch(logError(error as ServerError));
|
||||
return {error: error as ServerError};
|
||||
}
|
||||
|
||||
return {data: team};
|
||||
}
|
||||
throw addUserToTeamResult.error;
|
||||
|
||||
@@ -59,7 +59,6 @@ function TeamController(props: Props) {
|
||||
DesktopApp.reactAppInitialized();
|
||||
async function fetchAllChannels() {
|
||||
await props.fetchAllMyTeamsChannels();
|
||||
|
||||
setInitialChannelsLoaded(true);
|
||||
}
|
||||
|
||||
|
||||
@@ -2099,14 +2099,13 @@ describe('Actions.Channels', () => {
|
||||
});
|
||||
|
||||
nock(Client4.getBaseRoute()).get(
|
||||
'/users/some-user-id/channel_members?page=0&per_page=200').
|
||||
reply(200, [...Array(200).keys()].map((index) => ({channel_id: `channel-${index}`, user_id: 'some-user-id'})));
|
||||
nock(Client4.getBaseRoute()).get(
|
||||
'/users/some-user-id/channel_members?page=1&per_page=200').
|
||||
reply(200, [...Array(200).keys()].map((index) => ({channel_id: `channel-${index + 200}`, user_id: 'some-user-id'})));
|
||||
nock(Client4.getBaseRoute()).get(
|
||||
'/users/some-user-id/channel_members?page=2&per_page=200').
|
||||
reply(200, [...Array(100).keys()].map((index) => ({channel_id: `channel-${index + 400}`, user_id: 'some-user-id'})));
|
||||
'/users/some-user-id/channel_members?page=-1&per_page=60').
|
||||
reply(200, [...Array(500).keys()].map((index) => (
|
||||
{
|
||||
channel_id: `channel-${index}`,
|
||||
user_id: 'some-user-id',
|
||||
roles: 'channel_user',
|
||||
})));
|
||||
|
||||
await store.dispatch(Actions.fetchAllMyChannelMembers());
|
||||
expect(Object.keys(store.getState().entities.channels.myMembers).length).toBe(500);
|
||||
|
||||
@@ -427,8 +427,8 @@ export function getChannelTimezones(channelId: string): ActionFuncAsync<string[]
|
||||
|
||||
export function fetchChannelsAndMembers(teamId: string): ActionFuncAsync<{channels: ServerChannel[]; channelMembers: ChannelMembership[]}> {
|
||||
return async (dispatch, getState) => {
|
||||
let channels;
|
||||
let channelMembers;
|
||||
let channels: ServerChannel[] = [];
|
||||
let channelMembers: ChannelMembership[] = [];
|
||||
try {
|
||||
[channels, channelMembers] = await Promise.all([
|
||||
Client4.getMyChannels(teamId),
|
||||
@@ -471,32 +471,33 @@ export function fetchAllMyChannelMembers(): ActionFuncAsync {
|
||||
const state = getState();
|
||||
const {currentUserId} = state.entities.users;
|
||||
|
||||
let channelsMembers: ChannelMembership[] = [];
|
||||
let hasMoreMembers = true;
|
||||
let page = 0;
|
||||
let channelMembers: ChannelMembership[] = [];
|
||||
try {
|
||||
while (hasMoreMembers) {
|
||||
// Expected to disable since we don't have number of pages, so we can't use Promise.all
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
const data = await Client4.getAllChannelsMembers(currentUserId, page, 200);
|
||||
channelsMembers = [...channelsMembers, ...data];
|
||||
if (data.length < 200) {
|
||||
hasMoreMembers = false;
|
||||
}
|
||||
page++;
|
||||
}
|
||||
// The server exposes a streaming API if page is set to -1
|
||||
// We don't need to paginate through the responses, and thefore pageSize doesn't matter
|
||||
channelMembers = await Client4.getAllChannelsMembers(currentUserId, -1);
|
||||
} catch (error) {
|
||||
forceLogoutIfNecessary(error, dispatch, getState);
|
||||
dispatch(logError(error));
|
||||
return {error};
|
||||
}
|
||||
|
||||
const roles = new Set<string>();
|
||||
for (const member of channelMembers) {
|
||||
for (const role of member.roles.split(' ')) {
|
||||
roles.add(role);
|
||||
}
|
||||
}
|
||||
if (roles.size > 0) {
|
||||
dispatch(loadRolesIfNeeded(roles));
|
||||
}
|
||||
|
||||
dispatch({
|
||||
type: ChannelTypes.RECEIVED_MY_CHANNEL_MEMBERS,
|
||||
data: channelsMembers,
|
||||
data: channelMembers,
|
||||
currentUserId,
|
||||
});
|
||||
return {data: channelsMembers};
|
||||
return {data: channelMembers};
|
||||
};
|
||||
}
|
||||
|
||||
@@ -512,7 +513,7 @@ export function fetchAllMyTeamsChannels(): ActionFuncAsync {
|
||||
}
|
||||
|
||||
dispatch({
|
||||
type: ChannelTypes.RECEIVED_ALL_CHANNELS,
|
||||
type: ChannelTypes.RECEIVED_CHANNELS,
|
||||
data: channels,
|
||||
});
|
||||
return {data: channels};
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
import nock from 'nock';
|
||||
|
||||
import Client4, {ClientError, HEADER_X_VERSION_ID} from './client4';
|
||||
import {buildQueryString} from './helpers';
|
||||
import type {TelemetryHandler} from './telemetry';
|
||||
|
||||
describe('Client4', () => {
|
||||
@@ -40,6 +41,34 @@ describe('Client4', () => {
|
||||
|
||||
expect(client.serverVersion).toEqual('5.3.0.5.3.0.abc123');
|
||||
});
|
||||
|
||||
test('should parse NDJSON responses correctly', async () => {
|
||||
const client = new Client4();
|
||||
client.setUrl('http://mattermost.example.com');
|
||||
|
||||
const userId = 'dummy-user-id';
|
||||
const page = -1; // Special value to trigger NDJSON response
|
||||
|
||||
// Sample NDJSON data with multiple channel memberships on separate lines
|
||||
const ndjsonData = '{"user_id":"dummy-user-id","channel_id":"channel1","roles":"channel_user"}\n' +
|
||||
'{"user_id":"dummy-user-id","channel_id":"channel2","roles":"channel_user channel_admin"}\n' +
|
||||
'{"user_id":"dummy-user-id","channel_id":"channel3","roles":"channel_user"}';
|
||||
|
||||
// Create a mock endpoint for getAllChannelsMembers that returns NDJSON data
|
||||
nock(client.getBaseRoute()).
|
||||
get(`/users/${userId}/channel_members${buildQueryString({page, per_page: 60})}`).
|
||||
reply(200, ndjsonData, {'Content-Type': 'application/x-ndjson'});
|
||||
|
||||
// Call the getAllChannelsMembers method which will use our implementation for NDJSON
|
||||
const result = await client.getAllChannelsMembers(userId, page);
|
||||
|
||||
// Verify the response was parsed as an array of objects
|
||||
expect(Array.isArray(result)).toBe(true);
|
||||
expect(result).toHaveLength(3);
|
||||
expect(result[0]).toEqual({user_id: 'dummy-user-id', channel_id: 'channel1', roles: 'channel_user'});
|
||||
expect(result[1]).toEqual({user_id: 'dummy-user-id', channel_id: 'channel2', roles: 'channel_user channel_admin'});
|
||||
expect(result[2]).toEqual({user_id: 'dummy-user-id', channel_id: 'channel3', roles: 'channel_user'});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -4189,8 +4189,13 @@ export default class Client4 {
|
||||
|
||||
let data;
|
||||
try {
|
||||
if (headers.get('Content-Type') === 'application/json') {
|
||||
const contentType = headers.get('Content-Type');
|
||||
if (contentType === 'application/json') {
|
||||
data = await response.json();
|
||||
} else if (contentType === 'application/x-ndjson') {
|
||||
const text = await response.text();
|
||||
const objects = text.trim().split('\n');
|
||||
data = objects.map((obj) => JSON.parse(obj));
|
||||
} else {
|
||||
data = await response.text();
|
||||
}
|
||||
|
||||
Ссылка в новой задаче
Block a user