From fb5b57836ece6da2d0136802ca0d08346638b9e2 Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Tue, 3 Nov 2015 14:52:25 -0500 Subject: [PATCH 1/4] Multiple fixes to major performance issues with teams with 50+ users --- api/user.go | 19 +++++- web/react/components/channel_loader.jsx | 2 +- web/react/components/sidebar.jsx | 83 +++++++++---------------- web/react/stores/user_store.jsx | 33 +++++----- web/react/utils/async_client.jsx | 16 +++-- web/react/utils/client.jsx | 5 +- web/react/utils/constants.jsx | 1 + 7 files changed, 82 insertions(+), 77 deletions(-) diff --git a/api/user.go b/api/user.go index 732c6b9a84..42d3a43e75 100644 --- a/api/user.go +++ b/api/user.go @@ -49,7 +49,7 @@ func InitUser(r *mux.Router) { sr.Handle("/newimage", ApiUserRequired(uploadProfileImage)).Methods("POST") sr.Handle("/me", ApiAppHandler(getMe)).Methods("GET") - sr.Handle("/status", ApiUserRequiredActivity(getStatuses, false)).Methods("GET") + sr.Handle("/status", ApiUserRequiredActivity(getStatuses, false)).Methods("POST") sr.Handle("/profiles", ApiUserRequired(getProfiles)).Methods("GET") sr.Handle("/profiles/{id:[A-Za-z0-9]+}", ApiUserRequired(getProfiles)).Methods("GET") sr.Handle("/{id:[A-Za-z0-9]+}", ApiUserRequired(getUser)).Methods("GET") @@ -1483,16 +1483,31 @@ func updateUserNotify(c *Context, w http.ResponseWriter, r *http.Request) { } func getStatuses(c *Context, w http.ResponseWriter, r *http.Request) { + userIds := model.ArrayFromJson(r.Body) + if len(userIds) == 0 { + c.SetInvalidParam("getStatuses", "userIds") + return + } if result := <-Srv.Store.User().GetProfiles(c.Session.TeamId); result.Err != nil { c.Err = result.Err return } else { - profiles := result.Data.(map[string]*model.User) statuses := map[string]string{} for _, profile := range profiles { + found := false + for _, uid := range userIds { + if uid == profile.Id { + found = true + } + } + + if !found { + continue + } + if profile.IsOffline() { statuses[profile.Id] = model.USER_OFFLINE } else if profile.IsAway() { diff --git a/web/react/components/channel_loader.jsx b/web/react/components/channel_loader.jsx index 55b4a55c07..edc13f7abd 100644 --- a/web/react/components/channel_loader.jsx +++ b/web/react/components/channel_loader.jsx @@ -38,7 +38,7 @@ export default class ChannelLoader extends React.Component { /* Set up interval functions */ this.intervalId = setInterval( - function pollStatuses() { + () => { AsyncClient.getStatuses(); }, 30000 diff --git a/web/react/components/sidebar.jsx b/web/react/components/sidebar.jsx index c47919885c..e47ba6edfc 100644 --- a/web/react/components/sidebar.jsx +++ b/web/react/components/sidebar.jsx @@ -100,74 +100,53 @@ export default class Sidebar extends React.Component { } getStateFromStores() { const members = ChannelStore.getAllMembers(); - var teamMemberMap = UserStore.getActiveOnlyProfiles(); - var currentId = ChannelStore.getCurrentId(); - const currentUserId = UserStore.getCurrentId(); + const currentChannelId = ChannelStore.getCurrentId(); - var teammates = []; - for (var id in teamMemberMap) { - if (id === currentUserId) { - continue; - } - teammates.push(teamMemberMap[id]); - } + const channels = Object.assign([], ChannelStore.getAll()); + const publicChannels = channels.filter((channel) => channel.type === Constants.OPEN_CHANNEL); + const privateChannels = channels.filter((channel) => channel.type === Constants.PRIVATE_CHANNEL); + const directChannels = channels.filter((channel) => channel.type === Constants.DM_CHANNEL); const preferences = PreferenceStore.getPreferences(Constants.Preferences.CATEGORY_DIRECT_CHANNEL_SHOW); var visibleDirectChannels = []; - var hiddenDirectChannelCount = 0; - for (var i = 0; i < teammates.length; i++) { - const teammate = teammates[i]; + for (var i = 0; i < directChannels.length; i++) { + const dm = directChannels[i]; + const teammate = Utils.getDirectTeammate(dm.id); - if (teammate.id === currentUserId) { - continue; - } + const member = members[dm.id]; + const msgCount = dm.total_msg_count - member.msg_count; - const channelName = Utils.getDirectChannelName(currentUserId, teammate.id); + // always show a channel if either it is the current one or if it is unread, but it is not currently being left + const forceShow = (currentChannelId === dm.id || msgCount > 0) && !this.isLeaving.get(dm.id); + const preferenceShow = preferences.some((preference) => (preference.name === teammate.id && preference.value !== 'false')); - let forceShow = false; - let channel = ChannelStore.getByName(channelName); + if (preferenceShow || forceShow) { + dm.display_name = Utils.displayUsername(teammate.id); + dm.teammate_id = teammate.id; + dm.status = UserStore.getStatus(teammate.id); - if (channel) { - const member = members[channel.id]; - const msgCount = channel.total_msg_count - member.msg_count; + visibleDirectChannels.push(dm); - // always show a channel if either it is the current one or if it is unread, but it is not currently being left - forceShow = (currentId === channel.id || msgCount > 0) && !this.isLeaving.get(channel.id); - } else { - channel = {}; - channel.fake = true; - channel.name = channelName; - channel.last_post_at = 0; - channel.total_msg_count = 0; - channel.type = 'D'; - } - - channel.display_name = Utils.displayUsername(teammate.id); - channel.teammate_id = teammate.id; - channel.status = UserStore.getStatus(teammate.id); - - if (preferences.some((preference) => (preference.name === teammate.id && preference.value !== 'false'))) { - visibleDirectChannels.push(channel); - } else if (forceShow) { - // make sure that unread direct channels are visible - const preference = PreferenceStore.setPreference(Constants.Preferences.CATEGORY_DIRECT_CHANNEL_SHOW, teammate.id, 'true'); - AsyncClient.savePreferences([preference]); - - visibleDirectChannels.push(channel); - } else { - hiddenDirectChannelCount += 1; + if (forceShow && !preferenceShow) { + // make sure that unread direct channels are visible + const preference = PreferenceStore.setPreference(Constants.Preferences.CATEGORY_DIRECT_CHANNEL_SHOW, teammate.id, 'true'); + AsyncClient.savePreferences([preference]); + } } } + const hiddenDirectChannelCount = UserStore.getActiveOnlyProfileList().length - visibleDirectChannels.length; + visibleDirectChannels.sort(this.sortChannelsByDisplayName); const tutorialPref = PreferenceStore.getPreference(Preferences.TUTORIAL_STEP, UserStore.getCurrentId(), {value: '0'}); return { - activeId: currentId, - channels: ChannelStore.getAll(), + activeId: currentChannelId, members, + publicChannels, + privateChannels, visibleDirectChannels, hiddenDirectChannelCount, showTutorialTip: parseInt(tutorialPref.value, 10) === TutorialSteps.CHANNEL_POPOVER @@ -534,11 +513,9 @@ export default class Sidebar extends React.Component { this.lastUnreadChannel = null; // create elements for all 3 types of channels - const publicChannels = this.state.channels.filter((channel) => channel.type === 'O'); - const publicChannelItems = publicChannels.map(this.createChannelElement); + const publicChannelItems = this.state.publicChannels.map(this.createChannelElement); - const privateChannels = this.state.channels.filter((channel) => channel.type === 'P'); - const privateChannelItems = privateChannels.map(this.createChannelElement); + const privateChannelItems = this.state.privateChannels.map(this.createChannelElement); const directMessageItems = this.state.visibleDirectChannels.map((channel, index, arr) => { return this.createChannelElement(channel, index, arr, this.handleLeaveDirectChannel); diff --git a/web/react/stores/user_store.jsx b/web/react/stores/user_store.jsx index ce80c5ec9f..aedb3dc09d 100644 --- a/web/react/stores/user_store.jsx +++ b/web/react/stores/user_store.jsx @@ -204,12 +204,13 @@ class UserStoreClass extends EventEmitter { } getActiveOnlyProfiles() { - var active = {}; - var current = this.getProfiles(); + const active = {}; + const profiles = this.getProfiles(); + const currentId = this.getCurrentId(); - for (var key in current) { - if (current[key].delete_at === 0) { - active[key] = current[key]; + for (var key in profiles) { + if (profiles[key].delete_at === 0 && profiles[key].id !== currentId) { + active[key] = profiles[key]; } } @@ -219,9 +220,10 @@ class UserStoreClass extends EventEmitter { getActiveOnlyProfileList() { const profileMap = this.getActiveOnlyProfiles(); const profiles = []; + const currentId = this.getCurrentId(); for (const id in profileMap) { - if (profileMap.hasOwnProperty(id)) { + if (profileMap.hasOwnProperty(id) && id !== currentId) { profiles.push(profileMap[id]); } } @@ -235,6 +237,14 @@ class UserStoreClass extends EventEmitter { BrowserStore.setItem('profiles', ps); } + saveProfiles(profiles) { + const currentId = this.getCurrentId(); + if (currentId in profiles) { + delete profiles[currentId]; + } + BrowserStore.setItem('profiles', profiles); + } + setSessions(sessions) { BrowserStore.setItem('sessions', sessions); } @@ -320,15 +330,8 @@ UserStore.dispatchToken = AppDispatcher.register((payload) => { switch (action.type) { case ActionTypes.RECIEVED_PROFILES: - for (var id in action.profiles) { - // profiles can have incomplete data, so don't overwrite current user - if (id === UserStore.getCurrentId()) { - continue; - } - var profile = action.profiles[id]; - UserStore.saveProfile(profile); - UserStore.emitChange(profile.id); - } + UserStore.saveProfiles(action.profiles); + UserStore.emitChange(); break; case ActionTypes.RECIEVED_ME: UserStore.setCurrentUser(action.me); diff --git a/web/react/utils/async_client.jsx b/web/react/utils/async_client.jsx index 75dd35e3f1..0ecd26186a 100644 --- a/web/react/utils/async_client.jsx +++ b/web/react/utils/async_client.jsx @@ -588,13 +588,21 @@ export function getMe() { } export function getStatuses() { - if (isCallInProgress('getStatuses')) { + const directChannels = ChannelStore.getAll().filter((channel) => channel.type === Constants.DM_CHANNEL); + + const teammateIds = []; + for (var i = 0; i < directChannels.length; i++) { + const teammate = utils.getDirectTeammate(directChannels[i].id); + teammateIds.push(teammate.id); + } + + if (isCallInProgress('getStatuses') || teammateIds.length === 0) { return; } callTracker.getStatuses = utils.getTimestamp(); - client.getStatuses( - function getStatusesSuccess(data, textStatus, xhr) { + client.getStatuses(teammateIds, + (data, textStatus, xhr) => { callTracker.getStatuses = 0; if (xhr.status === 304 || !data) { @@ -606,7 +614,7 @@ export function getStatuses() { statuses: data }); }, - function getStatusesFailure(err) { + (err) => { callTracker.getStatuses = 0; dispatchError(err, 'getStatuses'); } diff --git a/web/react/utils/client.jsx b/web/react/utils/client.jsx index 7ce1346f99..003e24d336 100644 --- a/web/react/utils/client.jsx +++ b/web/react/utils/client.jsx @@ -1069,12 +1069,13 @@ export function exportTeam(success, error) { }); } -export function getStatuses(success, error) { +export function getStatuses(ids, success, error) { $.ajax({ url: '/api/v1/users/status', dataType: 'json', contentType: 'application/json', - type: 'GET', + type: 'POST', + data: JSON.stringify(ids), success, error: function onError(xhr, status, err) { var e = handleError('getStatuses', xhr, status, err); diff --git a/web/react/utils/constants.jsx b/web/react/utils/constants.jsx index fd64b1554a..39be577df3 100644 --- a/web/react/utils/constants.jsx +++ b/web/react/utils/constants.jsx @@ -127,6 +127,7 @@ module.exports = { MAX_DMS: 20, DM_CHANNEL: 'D', OPEN_CHANNEL: 'O', + PRIVATE_CHANNEL: 'P', INVITE_TEAM: 'I', OPEN_TEAM: 'O', MAX_POST_LEN: 4000, From a26145ef91197ba374d525c947984ae672cbd94d Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Tue, 3 Nov 2015 15:11:16 -0500 Subject: [PATCH 2/4] Minor changes to statuses client code --- web/react/components/channel_loader.jsx | 9 ++------- web/react/components/sidebar.jsx | 3 +++ web/react/utils/async_client.jsx | 4 +++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/web/react/components/channel_loader.jsx b/web/react/components/channel_loader.jsx index edc13f7abd..4fc115a92e 100644 --- a/web/react/components/channel_loader.jsx +++ b/web/react/components/channel_loader.jsx @@ -30,19 +30,14 @@ export default class ChannelLoader extends React.Component { AsyncClient.getChannels(true, true); AsyncClient.getChannelExtraInfo(true); AsyncClient.findTeams(); - AsyncClient.getStatuses(); AsyncClient.getMyTeam(); + setTimeout(() => AsyncClient.getStatuses(), 3000); // temporary until statuses are reworked a bit /* Perform pending post clean-up */ PostStore.clearPendingPosts(); /* Set up interval functions */ - this.intervalId = setInterval( - () => { - AsyncClient.getStatuses(); - }, - 30000 - ); + this.intervalId = setInterval(() => AsyncClient.getStatuses(), 30000); /* Device tracking setup */ var iOS = (/(iPad|iPhone|iPod)/g).test(navigator.userAgent); diff --git a/web/react/components/sidebar.jsx b/web/react/components/sidebar.jsx index e47ba6edfc..aab9919a48 100644 --- a/web/react/components/sidebar.jsx +++ b/web/react/components/sidebar.jsx @@ -113,6 +113,9 @@ export default class Sidebar extends React.Component { for (var i = 0; i < directChannels.length; i++) { const dm = directChannels[i]; const teammate = Utils.getDirectTeammate(dm.id); + if (!teammate) { + continue; + } const member = members[dm.id]; const msgCount = dm.total_msg_count - member.msg_count; diff --git a/web/react/utils/async_client.jsx b/web/react/utils/async_client.jsx index 0ecd26186a..205c7461ce 100644 --- a/web/react/utils/async_client.jsx +++ b/web/react/utils/async_client.jsx @@ -593,7 +593,9 @@ export function getStatuses() { const teammateIds = []; for (var i = 0; i < directChannels.length; i++) { const teammate = utils.getDirectTeammate(directChannels[i].id); - teammateIds.push(teammate.id); + if (teammate) { + teammateIds.push(teammate.id); + } } if (isCallInProgress('getStatuses') || teammateIds.length === 0) { From a15649e3827025a1222684b7960b4ec19351bc81 Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Tue, 3 Nov 2015 15:27:35 -0500 Subject: [PATCH 3/4] Updated getStatuses unit testgs --- api/user_test.go | 8 +++++++- model/client.go | 4 ++-- model/version.go | 8 ++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/api/user_test.go b/api/user_test.go index 0ad3541bc5..f067182cbc 100644 --- a/api/user_test.go +++ b/api/user_test.go @@ -1020,9 +1020,15 @@ func TestStatuses(t *testing.T) { ruser := Client.Must(Client.CreateUser(&user, "")).Data.(*model.User) store.Must(Srv.Store.User().VerifyEmail(ruser.Id)) + user2 := model.User{TeamId: rteam.Data.(*model.Team).Id, Email: strings.ToLower(model.NewId()) + "corey@test.com", Nickname: "Corey Hulen", Password: "pwd"} + ruser2 := Client.Must(Client.CreateUser(&user2, "")).Data.(*model.User) + store.Must(Srv.Store.User().VerifyEmail(ruser2.Id)) + Client.LoginByEmail(team.Name, user.Email, user.Password) - r1, err := Client.GetStatuses() + userIds := []string{ruser2.Id} + + r1, err := Client.GetStatuses(userIds) if err != nil { t.Fatal(err) } diff --git a/model/client.go b/model/client.go index 19183098e0..d022d37143 100644 --- a/model/client.go +++ b/model/client.go @@ -783,8 +783,8 @@ func (c *Client) ResetPassword(data map[string]string) (*Result, *AppError) { } } -func (c *Client) GetStatuses() (*Result, *AppError) { - if r, err := c.DoApiGet("/users/status", "", ""); err != nil { +func (c *Client) GetStatuses(data []string) (*Result, *AppError) { + if r, err := c.DoApiPost("/users/status", ArrayToJson(data)); err != nil { return nil, err } else { return &Result{r.Header.Get(HEADER_REQUEST_ID), diff --git a/model/version.go b/model/version.go index a61004fdee..5dce70e1db 100644 --- a/model/version.go +++ b/model/version.go @@ -21,9 +21,9 @@ var versions = []string{ } var CurrentVersion string = versions[0] -var BuildNumber = "_BUILD_NUMBER_" -var BuildDate = "_BUILD_DATE_" -var BuildHash = "_BUILD_HASH_" +var BuildNumber = "dev" +var BuildDate = "Tue 3 Nov 2015 20:11:28 UTC" +var BuildHash = "a26145ef91197ba374d525c947984ae672cbd94d" func SplitVersion(version string) (int64, int64, int64) { parts := strings.Split(version, ".") @@ -69,7 +69,7 @@ func GetPreviousVersion(currentVersion string) (int64, int64) { } func IsOfficalBuild() bool { - return BuildNumber != "_BUILD_NUMBER_" + return BuildNumber != "dev" } func IsCurrentVersion(versionToCheck string) bool { From ee66b089349d46695bbe652b381da4be2b67954b Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Tue, 3 Nov 2015 15:38:15 -0500 Subject: [PATCH 4/4] Revert model/version.go --- model/version.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/model/version.go b/model/version.go index 5dce70e1db..a61004fdee 100644 --- a/model/version.go +++ b/model/version.go @@ -21,9 +21,9 @@ var versions = []string{ } var CurrentVersion string = versions[0] -var BuildNumber = "dev" -var BuildDate = "Tue 3 Nov 2015 20:11:28 UTC" -var BuildHash = "a26145ef91197ba374d525c947984ae672cbd94d" +var BuildNumber = "_BUILD_NUMBER_" +var BuildDate = "_BUILD_DATE_" +var BuildHash = "_BUILD_HASH_" func SplitVersion(version string) (int64, int64, int64) { parts := strings.Split(version, ".") @@ -69,7 +69,7 @@ func GetPreviousVersion(currentVersion string) (int64, int64) { } func IsOfficalBuild() bool { - return BuildNumber != "dev" + return BuildNumber != "_BUILD_NUMBER_" } func IsCurrentVersion(versionToCheck string) bool {