From cadc9e11e4c5456bae97d8ba4031ea9e72edf7fb Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Wed, 21 Dec 2016 12:45:21 -0500 Subject: Remove race between multiple autocomplete requests (#4860) --- .../components/suggestion/at_mention_provider.jsx | 11 ++++++++- .../suggestion/channel_mention_provider.jsx | 9 ++++++- webapp/components/suggestion/provider.jsx | 28 ++++++++++++++++++++++ .../suggestion/search_channel_provider.jsx | 9 ++++++- .../components/suggestion/search_user_provider.jsx | 9 ++++++- webapp/components/suggestion/suggestion_box.jsx | 8 ------- .../suggestion/switch_channel_provider.jsx | 9 ++++++- webapp/stores/suggestion_store.jsx | 17 +++++++------ 8 files changed, 80 insertions(+), 20 deletions(-) create mode 100644 webapp/components/suggestion/provider.jsx diff --git a/webapp/components/suggestion/at_mention_provider.jsx b/webapp/components/suggestion/at_mention_provider.jsx index 9a5e103ad..551f3a72b 100644 --- a/webapp/components/suggestion/at_mention_provider.jsx +++ b/webapp/components/suggestion/at_mention_provider.jsx @@ -2,6 +2,7 @@ // See License.txt for license information. import Suggestion from './suggestion.jsx'; +import Provider from './provider.jsx'; import ChannelStore from 'stores/channel_store.jsx'; @@ -100,8 +101,10 @@ class AtMentionSuggestion extends Suggestion { } } -export default class AtMentionProvider { +export default class AtMentionProvider extends Provider { constructor(channelId) { + super(); + this.channelId = channelId; } @@ -110,10 +113,16 @@ export default class AtMentionProvider { if (captured) { const prefix = captured[1]; + this.startNewRequest(prefix); + autocompleteUsersInChannel( prefix, this.channelId, (data) => { + if (this.shouldCancelDispatch(prefix)) { + return; + } + const members = data.in_channel; for (const id of Object.keys(members)) { members[id].type = Constants.MENTION_MEMBERS; diff --git a/webapp/components/suggestion/channel_mention_provider.jsx b/webapp/components/suggestion/channel_mention_provider.jsx index 7c781a2f7..63e6944ac 100644 --- a/webapp/components/suggestion/channel_mention_provider.jsx +++ b/webapp/components/suggestion/channel_mention_provider.jsx @@ -2,6 +2,7 @@ // See License.txt for license information. import Suggestion from './suggestion.jsx'; +import Provider from './provider.jsx'; import {autocompleteChannels} from 'actions/channel_actions.jsx'; @@ -49,15 +50,21 @@ class ChannelMentionSuggestion extends Suggestion { } } -export default class ChannelMentionProvider { +export default class ChannelMentionProvider extends Provider { handlePretextChanged(suggestionId, pretext) { const captured = (/(^|\s)(~([^~]*))$/i).exec(pretext.toLowerCase()); if (captured) { const prefix = captured[3]; + this.startNewRequest(prefix); + autocompleteChannels( prefix, (data) => { + if (this.shouldCancelDispatch(prefix)) { + return; + } + const channels = data; // Wrap channels in an outer object to avoid overwriting the 'type' property. diff --git a/webapp/components/suggestion/provider.jsx b/webapp/components/suggestion/provider.jsx new file mode 100644 index 000000000..db34dcebf --- /dev/null +++ b/webapp/components/suggestion/provider.jsx @@ -0,0 +1,28 @@ +// Copyright (c) 2016 Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +export default class Provider { + constructor() { + this.latestPrefix = ''; + this.latestComplete = true; + } + + handlePretextChanged(suggestionId, pretext) { // eslint-disable-line no-unused-vars + // NO-OP for inherited classes to override + } + + startNewRequest(prefix) { + this.latestPrefix = prefix; + this.latestComplete = false; + } + + shouldCancelDispatch(prefix) { + if (prefix === this.latestPrefix) { + this.latestComplete = true; + } else if (this.latestComplete) { + return true; + } + + return false; + } +} diff --git a/webapp/components/suggestion/search_channel_provider.jsx b/webapp/components/suggestion/search_channel_provider.jsx index 283113fb9..8965e7a76 100644 --- a/webapp/components/suggestion/search_channel_provider.jsx +++ b/webapp/components/suggestion/search_channel_provider.jsx @@ -2,6 +2,7 @@ // See License.txt for license information. import Suggestion from './suggestion.jsx'; +import Provider from './provider.jsx'; import {autocompleteChannels} from 'actions/channel_actions.jsx'; @@ -32,15 +33,21 @@ class SearchChannelSuggestion extends Suggestion { } } -export default class SearchChannelProvider { +export default class SearchChannelProvider extends Provider { handlePretextChanged(suggestionId, pretext) { const captured = (/\b(?:in|channel):\s*(\S*)$/i).exec(pretext.toLowerCase()); if (captured) { const channelPrefix = captured[1]; + this.startNewRequest(channelPrefix); + autocompleteChannels( channelPrefix, (data) => { + if (this.shouldCancelDispatch(channelPrefix)) { + return; + } + const publicChannels = data; const localChannels = ChannelStore.getAll(); diff --git a/webapp/components/suggestion/search_user_provider.jsx b/webapp/components/suggestion/search_user_provider.jsx index 6fce8edcf..bff59ace8 100644 --- a/webapp/components/suggestion/search_user_provider.jsx +++ b/webapp/components/suggestion/search_user_provider.jsx @@ -2,6 +2,7 @@ // See License.txt for license information. import Suggestion from './suggestion.jsx'; +import Provider from './provider.jsx'; import {autocompleteUsersInTeam} from 'actions/user_actions.jsx'; @@ -56,15 +57,21 @@ class SearchUserSuggestion extends Suggestion { } } -export default class SearchUserProvider { +export default class SearchUserProvider extends Provider { handlePretextChanged(suggestionId, pretext) { const captured = (/\bfrom:\s*(\S*)$/i).exec(pretext.toLowerCase()); if (captured) { const usernamePrefix = captured[1]; + this.startNewRequest(usernamePrefix); + autocompleteUsersInTeam( usernamePrefix, (data) => { + if (this.shouldCancelDispatch(usernamePrefix)) { + return; + } + const users = data.in_team; const mentions = users.map((user) => user.username); diff --git a/webapp/components/suggestion/suggestion_box.jsx b/webapp/components/suggestion/suggestion_box.jsx index 2fb44a340..b3f9843c6 100644 --- a/webapp/components/suggestion/suggestion_box.jsx +++ b/webapp/components/suggestion/suggestion_box.jsx @@ -38,14 +38,6 @@ export default class SuggestionBox extends React.Component { SuggestionStore.addPretextChangedListener(this.suggestionId, this.handlePretextChanged); } - componentWillReceiveProps(nextProps) { - // Clear any suggestions when the SuggestionBox is cleared - if (nextProps.value === '' && this.props.value !== nextProps.value) { - // TODO - Find a better way to not "dispatch during dispatch" - setTimeout(() => GlobalActions.emitClearSuggestions(this.suggestionId), 1); - } - } - componentWillUnmount() { SuggestionStore.removeCompleteWordListener(this.suggestionId, this.handleCompleteWord); SuggestionStore.removePretextChangedListener(this.suggestionId, this.handlePretextChanged); diff --git a/webapp/components/suggestion/switch_channel_provider.jsx b/webapp/components/suggestion/switch_channel_provider.jsx index bf9e7c646..35ce4d578 100644 --- a/webapp/components/suggestion/switch_channel_provider.jsx +++ b/webapp/components/suggestion/switch_channel_provider.jsx @@ -2,6 +2,7 @@ // See License.txt for license information. import Suggestion from './suggestion.jsx'; +import Provider from './provider.jsx'; import ChannelStore from 'stores/channel_store.jsx'; import UserStore from 'stores/user_store.jsx'; @@ -58,15 +59,21 @@ class SwitchChannelSuggestion extends Suggestion { } } -export default class SwitchChannelProvider { +export default class SwitchChannelProvider extends Provider { handlePretextChanged(suggestionId, channelPrefix) { if (channelPrefix) { + this.startNewRequest(channelPrefix); + const allChannels = ChannelStore.getAll(); const channels = []; autocompleteUsers( channelPrefix, (users) => { + if (this.shouldCancelDispatch(channelPrefix)) { + return; + } + const currentId = UserStore.getCurrentId(); for (const id of Object.keys(allChannels)) { diff --git a/webapp/stores/suggestion_store.jsx b/webapp/stores/suggestion_store.jsx index 2dbef0490..a0cd88370 100644 --- a/webapp/stores/suggestion_store.jsx +++ b/webapp/stores/suggestion_store.jsx @@ -121,11 +121,6 @@ class SuggestionStore extends EventEmitter { } addSuggestions(id, terms, items, component, matchedPretext) { - if (!this.getPretext(id).endsWith(matchedPretext)) { - // These suggestions are out of date since the pretext has changed - return; - } - const suggestion = this.getSuggestions(id); suggestion.terms.push(...terms); @@ -222,6 +217,11 @@ class SuggestionStore extends EventEmitter { suggestion.selection = suggestion.terms[selectionIndex]; } + checkIfPretextMatches(id, matchedPretext) { + const pretext = this.getPretext(id) || ''; + return pretext.endsWith(matchedPretext); + } + handleEventPayload(payload) { const {type, id, ...other} = payload.action; @@ -241,9 +241,12 @@ class SuggestionStore extends EventEmitter { this.emitSuggestionsChanged(id); break; case ActionTypes.SUGGESTION_RECEIVED_SUGGESTIONS: - this.clearSuggestions(id); + if (!this.checkIfPretextMatches(id, other.matchedPretext)) { + // These suggestions are out of date since the pretext has changed + return; + } - // ensure the matched pretext hasn't changed so that we don't receive suggestions for outdated pretext + this.clearSuggestions(id); this.addSuggestions(id, other.terms, other.items, other.component, other.matchedPretext); this.ensureSelectionExists(id); -- cgit v1.2.3-1-g7c22