Skip to content

Commit

Permalink
search: ignore searchResults if it isn't the active query
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brunnre8 committed Jan 8, 2023
1 parent 8b1a4f7 commit 0ebc3a5
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 46 deletions.
37 changes: 23 additions & 14 deletions client/components/Windows/SearchResults.vue
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,19 @@
<button
ref="loadMoreButton"
:disabled="
store.state.messageSearchInProgress || !store.state.isConnected
!!store.state.messageSearchPendingQuery ||
!store.state.isConnected
"
class="btn"
@click="onShowMoreClick"
>
<span v-if="store.state.messageSearchInProgress">Loading…</span>
<span v-if="store.state.messageSearchPendingQuery">Loading…</span>
<span v-else>Show older messages</span>
</button>
</div>

<div
v-if="store.state.messageSearchInProgress && !offset"
v-if="store.state.messageSearchPendingQuery && !offset"
class="search-status"
>
Searching…
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 () => {
Expand Down
19 changes: 17 additions & 2 deletions client/js/socket-events/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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});
});
11 changes: 6 additions & 5 deletions client/js/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -85,7 +86,7 @@ export type State = {
messageSearchResults: {
results: ClientMessage[];
} | null;
messageSearchInProgress: boolean;
messageSearchPendingQuery: SearchQuery | null;
searchEnabled: boolean;
};

Expand All @@ -111,7 +112,7 @@ const state = () =>
versionDataExpired: false,
serverHasSettings: false,
messageSearchResults: null,
messageSearchInProgress: false,
messageSearchPendingQuery: null,
searchEnabled: false,
} as State);

Expand Down Expand Up @@ -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<State["messageSearchResults"]>): void;
};
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 5 additions & 8 deletions server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down Expand Up @@ -618,15 +618,12 @@ class Client {
}
}

search(query: SearchQuery) {
async search(query: SearchQuery): Promise<SearchResponse> {
if (!this.messageProvider?.isEnabled) {
return Promise.resolve({
return {
...query,
results: [],
target: "",
networkUuid: "",
offset: 0,
searchTerm: query?.searchTerm,
});
};
}

return this.messageProvider.search(query);
Expand Down
9 changes: 2 additions & 7 deletions server/plugins/messageStorage/sqlite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
9 changes: 3 additions & 6 deletions server/plugins/messageStorage/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,9 @@ export type SearchQuery = {
offset: number;
};

export type SearchResponse =
| Omit<SearchQuery, "channelName" | "offset"> & {
results: Message[];
target: string;
offset: number;
};
export type SearchResponse = SearchQuery & {
results: Message[];
};

type SearchFunction = (query: SearchQuery) => Promise<SearchResponse>;

Expand Down
5 changes: 2 additions & 3 deletions server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}) => {
Expand Down
2 changes: 1 addition & 1 deletion server/types/socket-events.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 0ebc3a5

Please sign in to comment.