From 0ebc3a574c42185c818ca8795a56d8eb58a20f4e Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 26 Nov 2022 17:14:09 +0100 Subject: [PATCH] search: ignore searchResults if it isn't the active query Prior to this, the search is still racy but one tends to notice this only when the DB is large or network is involved. The user can initiate a search, get bored, navigate to another chan issue a different search. Now however, the results of the first search come back in and hilarity ensues as we are now confused with the state. To avoid this, keep track of the last search done and any result that comes in that isn't equal to the active query is garbage and can be dropped. --- client/components/Windows/SearchResults.vue | 37 +++++++++++++-------- client/js/socket-events/search.ts | 19 +++++++++-- client/js/store.ts | 11 +++--- server/client.ts | 13 +++----- server/plugins/messageStorage/sqlite.ts | 9 ++--- server/plugins/messageStorage/types.d.ts | 9 ++--- server/server.ts | 5 ++- server/types/socket-events.d.ts | 2 +- 8 files changed, 59 insertions(+), 46 deletions(-) diff --git a/client/components/Windows/SearchResults.vue b/client/components/Windows/SearchResults.vue index c29d51e9..bc0c153d 100644 --- a/client/components/Windows/SearchResults.vue +++ b/client/components/Windows/SearchResults.vue @@ -33,18 +33,19 @@
Searching… @@ -105,6 +106,7 @@ import type {ClientMessage} from "../../js/types"; import {useStore} from "../../js/store"; import {useRoute, useRouter} from "vue-router"; import {switchToChannel} from "../../js/router"; +import {SearchQuery} from "../../../server/plugins/messageStorage/types"; export default defineComponent({ name: "SearchResults", @@ -187,37 +189,44 @@ export default defineComponent({ const clearSearchState = () => { offset.value = 0; - store.commit("messageSearchInProgress", false); store.commit("messageSearchResults", null); + store.commit("messageSearchPendingQuery", null); }; const doSearch = () => { + if (!network.value || !channel.value) { + return; + } + clearSearchState(); // this is a new search, so we need to clear anything before that - socket.emit("search", { - networkUuid: network.value?.uuid, - channelName: channel.value?.name, + const query: SearchQuery = { + networkUuid: network.value.uuid, + channelName: channel.value.name, searchTerm: String(route.query.q || ""), offset: offset.value, - }); + }; + store.commit("messageSearchPendingQuery", query); + socket.emit("search", query); }; const onShowMoreClick = () => { - if (!chat.value) { + if (!chat.value || !network.value || !channel.value) { return; } offset.value += 100; - store.commit("messageSearchInProgress", true); oldScrollTop.value = chat.value.scrollTop; oldChatHeight.value = chat.value.scrollHeight; - socket.emit("search", { - networkUuid: network.value?.uuid, - channelName: channel.value?.name, + const query: SearchQuery = { + networkUuid: network.value.uuid, + channelName: channel.value.name, searchTerm: String(route.query.q || ""), offset: offset.value, - }); + }; + store.commit("messageSearchPendingQuery", query); + socket.emit("search", query); }; const jumpToBottom = async () => { diff --git a/client/js/socket-events/search.ts b/client/js/socket-events/search.ts index 7553e990..b83891e0 100644 --- a/client/js/socket-events/search.ts +++ b/client/js/socket-events/search.ts @@ -2,12 +2,27 @@ import socket from "../socket"; import {store} from "../store"; socket.on("search:results", (response) => { - store.commit("messageSearchInProgress", false); + const pendingQuery = store.state.messageSearchPendingQuery; + + if ( + !pendingQuery || + pendingQuery.channelName !== response.channelName || + pendingQuery.networkUuid !== response.networkUuid || + pendingQuery.offset !== response.offset || + pendingQuery.searchTerm !== response.searchTerm + ) { + // This is a response from a search that we are not interested in. + // The user may have entered a different search while one was still in flight. + // We can simply drop it on the floor. + return; + } + + store.commit("messageSearchPendingQuery", null); if (store.state.messageSearchResults) { store.commit("addMessageSearchResults", response); return; } - store.commit("messageSearchResults", response); + store.commit("messageSearchResults", {results: response.results}); }); diff --git a/client/js/store.ts b/client/js/store.ts index c7f724f9..d536389f 100644 --- a/client/js/store.ts +++ b/client/js/store.ts @@ -15,6 +15,7 @@ import type { import type {InjectionKey} from "vue"; import {SettingsState} from "./settings"; +import {SearchQuery} from "../../server/plugins/messageStorage/types"; const appName = document.title; @@ -85,7 +86,7 @@ export type State = { messageSearchResults: { results: ClientMessage[]; } | null; - messageSearchInProgress: boolean; + messageSearchPendingQuery: SearchQuery | null; searchEnabled: boolean; }; @@ -111,7 +112,7 @@ const state = () => versionDataExpired: false, serverHasSettings: false, messageSearchResults: null, - messageSearchInProgress: false, + messageSearchPendingQuery: null, searchEnabled: false, } as State); @@ -260,7 +261,7 @@ type Mutations = { versionStatus(state: State, payload: State["versionStatus"]): void; versionDataExpired(state: State, payload: State["versionDataExpired"]): void; serverHasSettings(state: State, value: State["serverHasSettings"]): void; - messageSearchInProgress(state: State, value: State["messageSearchInProgress"]): void; + messageSearchPendingQuery(state: State, value: State["messageSearchPendingQuery"]): void; messageSearchResults(state: State, value: State["messageSearchResults"]): void; addMessageSearchResults(state: State, value: NonNullable): void; }; @@ -338,8 +339,8 @@ const mutations: Mutations = { serverHasSettings(state, value) { state.serverHasSettings = value; }, - messageSearchInProgress(state, value) { - state.messageSearchInProgress = value; + messageSearchPendingQuery(state, value) { + state.messageSearchPendingQuery = value; }, messageSearchResults(state, value) { state.messageSearchResults = value; diff --git a/server/client.ts b/server/client.ts index cac35532..9612de19 100644 --- a/server/client.ts +++ b/server/client.ts @@ -17,7 +17,7 @@ import SqliteMessageStorage from "./plugins/messageStorage/sqlite"; import TextFileMessageStorage from "./plugins/messageStorage/text"; import Network, {IgnoreListItem, NetworkWithIrcFramework} from "./models/network"; import ClientManager from "./clientManager"; -import {MessageStorage, SearchQuery} from "./plugins/messageStorage/types"; +import {MessageStorage, SearchQuery, SearchResponse} from "./plugins/messageStorage/types"; type OrderItem = Chan["id"] | Network["uuid"]; type Order = OrderItem[]; @@ -618,15 +618,12 @@ class Client { } } - search(query: SearchQuery) { + async search(query: SearchQuery): Promise { if (!this.messageProvider?.isEnabled) { - return Promise.resolve({ + return { + ...query, results: [], - target: "", - networkUuid: "", - offset: 0, - searchTerm: query?.searchTerm, - }); + }; } return this.messageProvider.search(query); diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index fb7d9c3e..9c5ec985 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -259,15 +259,10 @@ class SqliteMessageStorage implements ISqliteMessageStorage { params.push(query.offset); const rows = await this.serialize_fetchall(select, ...params); - const response: SearchResponse = { - searchTerm: query.searchTerm, - target: query.channelName, - networkUuid: query.networkUuid, - offset: query.offset, + return { + ...query, results: parseSearchRowsToMessages(query.offset, rows).reverse(), }; - - return response; } canProvideMessages() { diff --git a/server/plugins/messageStorage/types.d.ts b/server/plugins/messageStorage/types.d.ts index 6a038822..68c6c271 100644 --- a/server/plugins/messageStorage/types.d.ts +++ b/server/plugins/messageStorage/types.d.ts @@ -29,12 +29,9 @@ export type SearchQuery = { offset: number; }; -export type SearchResponse = - | Omit & { - results: Message[]; - target: string; - offset: number; - }; +export type SearchResponse = SearchQuery & { + results: Message[]; +}; type SearchFunction = (query: SearchQuery) => Promise; diff --git a/server/server.ts b/server/server.ts index ee103997..a0a616a8 100644 --- a/server/server.ts +++ b/server/server.ts @@ -760,9 +760,8 @@ function initializeClient( }); socket.on("search", async (query) => { - await client.search(query).then((results) => { - socket.emit("search:results", results); - }); + const results = await client.search(query); + socket.emit("search:results", results); }); socket.on("mute:change", ({target, setMutedTo}) => { diff --git a/server/types/socket-events.d.ts b/server/types/socket-events.d.ts index ccfe1e37..3c7df130 100644 --- a/server/types/socket-events.d.ts +++ b/server/types/socket-events.d.ts @@ -107,7 +107,7 @@ interface ServerToClientEvents { token: string; }) => void; - "search:results": (response: {results: ClientMessage[]}) => void; + "search:results": (response: SearchResponse) => void; quit: (args: {network: string}) => void;