open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

feat(core): reduce unifieid search reveal interval

Browse Source 
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
pull/63003/head
Peter Ringelmann 2 weeks ago
parent 39b315180f
commit 3cff8a9128
2 changed files with 51 additions and 82 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 58
      core/src/services/UnifiedSearchController.ts
  2. 75
      core/src/tests/services/UnifiedSearchController.spec.ts

58
core/src/services/UnifiedSearchController.ts
Unescape View File

@ -23,7 +23,7 @@ export interface CategorySearchParams {
extraQueries?: object
}
export const REVEAL_INTERVAL_MS = 1500
export const REVEAL_INTERVAL_MS = 1000
/**
* Results fetched per category per page. Sized for the detail view (which shows the
@ -32,9 +32,10 @@ export const REVEAL_INTERVAL_MS = 1500
export const PAGE_SIZE = 10
/**
* Whether a category has anything for the user to look at. Blocked is deliberately
* withheld, failed carries no entries, and a loading category keeps its previous page up
* (stale-while-revalidate) so it stays visible through a refetch.
* Whether a category has anything for the user to look at. Blocked is deliberately withheld
* and failed carries no entries. Loading counts because paging keeps the pages already
* fetched on screen while the next one is in flight; a new query has no entries to show, so
* it reads as not visible until results actually land.
*
* Exported so the one definition also serves the Vue-side test doubles; the controller is
* the only place that decides category-level visibility.
@ -74,17 +75,12 @@ export class UnifiedSearchController {
*/
async search(query: string, categories: string[], params?: Record<string, CategorySearchParams>): Promise<void> {
this.cancelPendingRequests()
// Stale-while-revalidate: keep the previous page on screen while the new search is in
// flight, so refining a query swaps results in place instead of flashing an empty panel.
// Each recurring category is reseeded with its prior entries below; dropped ones vanish.
const previous = this.searchStates
// A new query hides everything the last one produced. Carrying results over would only
// let them shift under the user once the real ones land, and the results are about to
// differ anyway. So each search is a clean slate: empty screen, then a fresh ordered
// reveal from priority order. Nothing is on screen, so nothing can be displaced.
this.searchStates = {}
// Prune rather than clear: survivors keep the slots they already hold, so refining a query
// never re-sorts rendered results back to priority order. A category the new search
// dropped is reseeded invisible if it ever returns, so it re-enters at the bottom. This
// cannot cover the window while the states below are still being reseeded one at a time;
// getRevealOrder() does that.
this.revealOrder = this.revealOrder.filter((category) => categories.includes(category))
this.revealOrder = []
this.searchGeneration++
const generation = this.searchGeneration
this.query = query
@ -92,14 +88,7 @@ export class UnifiedSearchController {
this.startRevealTimer()
await Promise.allSettled(categories.map((category) => {
const prev = previous[category]
// Only entries that were actually on screen seed the stale view. A blocked or failed
// category's entries were fetched but never rendered, so they must not carry over
// (and must not let the category skip the ordered reveal).
const staleEntries = prev && isCategoryVisible(prev) ? prev.entries : []
return this.searchCategory(category, generation, categories, staleEntries)
}))
await Promise.allSettled(categories.map((category) => this.searchCategory(category, generation, categories)))
}
/**
@ -165,17 +154,18 @@ export class UnifiedSearchController {
/**
* The ids of the categories currently on screen, in display order.
*
* Append-only, so a category never moves up into a slot another one already occupies: a
* result that arrives late renders below what the user is already reading, however high
* its priority. Read this rather than the snapshot's key order, which is the priority
* order and an input to blocking, not a rendering order.
* Append-only within a search, so a category never moves up into a slot another one already
* occupies: a result that arrives late renders below what the user is already reading,
* however high its priority. A new query starts over from priority order, since it clears
* the screen first and so has nothing to displace. Read this rather than the snapshot's key
* order, which is the priority order and an input to blocking, not a rendering order.
*
* Every id is indexable in the same snapshot, so a caller can map without guarding.
* Only ever names categories the current snapshot holds, so a caller can map without guarding.
*
* @return visible category ids, top to bottom
*/
getRevealOrder(): string[] {
return this.revealOrder.filter((category) => category in this.searchStates)
return [...this.revealOrder]
}
dispose(): void {
@ -196,13 +186,10 @@ export class UnifiedSearchController {
category: string,
generation: number,
categories: string[],
staleEntries: unknown[] = [],
): Promise<void> {
// Seed with the prior page (stale-while-revalidate) so it stays visible under the
// spinner until the fresh page replaces it. Empty on a first search.
this.patchStates({ [category]: {
status: 'loading',
entries: staleEntries,
entries: [],
cursor: null,
hasMore: false,
loadMoreFailed: false,
@ -227,12 +214,9 @@ export class UnifiedSearchController {
const { entries, cursor, isPaginated } = response.data.ocs.data
// Decide blocked vs loaded once, here at settle. Reconcile only promotes after this
// (never re-blocks), so this is the only place a category becomes blocked. A category
// that carried stale results skips blocking: it is already on screen, so blocking it
// would blink it off until its predecessors clear. Ordered reveal is only for the
// first paint, when nothing is shown yet.
// (never re-blocks), so this is the only place a category becomes blocked.
this.patchStates({ [category]: {
status: (staleEntries.length === 0 && this.shouldBlockCategory(category, categories)) ? 'blocked' : 'loaded',
status: this.shouldBlockCategory(category, categories) ? 'blocked' : 'loaded',
entries,
cursor,
hasMore: this.hasMorePages(isPaginated, cursor),

75
core/src/tests/services/UnifiedSearchController.spec.ts
Unescape View File

@ -315,30 +315,29 @@ describe('UnifiedSearchController', () => {
expect(searchController.getRevealOrder()).toEqual(['talk', 'deck'])
})
it('keeps reveal positions across a refined query', async () => {
it('restarts the reveal order from priority on a new query', async () => {
const first = mockProviders(['files', 'talk'])
const searchController = new UnifiedSearchController()
searchController.search('old', ['files', 'talk'])
// talk gets on screen first, so the session order is talk before files.
// talk got on screen first, so this query renders talk above files.
first.talk.resolve(['Old talk'])
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
first.files.resolve(['Old files'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['talk', 'files'])
// Refining must not re-sort what is already on screen back to priority order.
// Both categories stay rendered throughout (stale-while-revalidate), so moving
// them would be a displacement with identical content.
// A new query hides everything: the results are about to be different, so there is
// nothing on screen to protect and the next paint starts from priority order again.
const second = mockProviders(['files', 'talk'])
searchController.search('new', ['files', 'talk'])
expect(searchController.getRevealOrder()).toEqual(['talk', 'files'])
expect(searchController.getRevealOrder()).toEqual([])
second.files.resolve(['New files'])
second.talk.resolve(['New talk'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['talk', 'files'])
expect(searchController.getRevealOrder()).toEqual(['files', 'talk'])
})
it('releases a slot when a category loses its results, and appends it again if it returns', async () => {
@ -360,17 +359,17 @@ describe('UnifiedSearchController', () => {
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['talk'])
// It has results again on the next query, so it comes back as a fresh reveal:
// at the end, not back at its old priority slot.
// The next query is a clean slate, so it comes back in preferred order rather than
// staying demoted for the rest of the session.
const third = mockProviders(['files', 'talk'])
searchController.search('c', ['files', 'talk'])
third.files.resolve(['Files c'])
third.talk.resolve(['Talk c'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['talk', 'files'])
expect(searchController.getRevealOrder()).toEqual(['files', 'talk'])
})
it('re-appends a category that left the search entirely instead of reclaiming its old slot', async () => {
it('recovers preferred order after a provider filter round trip', async () => {
const first = mockProviders(['files', 'talk'])
const searchController = new UnifiedSearchController()
@ -380,24 +379,22 @@ describe('UnifiedSearchController', () => {
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['files', 'talk'])
// A provider filter narrows the search: files leaves the category list altogether,
// which is a different exit from losing its results (that one goes through
// syncRevealOrder; this one goes through the prune in search()).
// A provider filter narrows the search: files leaves the category list altogether.
const second = mockProviders(['talk'])
searchController.search('foo', ['talk'])
second.talk.resolve(['Talk result'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['talk'])
// The filter comes off. talk never left the screen, so files has to come back below
// it: reclaiming slot 0 would shove a rendered group down.
// The filter comes off. Each search stands on its own, so files is back on top
// instead of being stuck below talk until the popover closes.
const third = mockProviders(['files', 'talk'])
searchController.search('foo', ['files', 'talk'])
third.files.resolve(['Files result'])
third.talk.resolve(['Talk result'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getRevealOrder()).toEqual(['talk', 'files'])
expect(searchController.getRevealOrder()).toEqual(['files', 'talk'])
})
it('never hands out a category the snapshot cannot index, part-way through a search', async () => {
@ -443,21 +440,20 @@ describe('UnifiedSearchController', () => {
first.talk.resolve(['Talk result'])
await vi.advanceTimersByTimeAsync(0)
// A narrower search replaces the first. The dropped categories must
// not linger in the snapshot, nor in the display order: the view maps the
// order straight onto the snapshot and would hit a missing category.
// A narrower search replaces the first. The dropped categories must not linger in
// the snapshot, and nothing from the previous query stays on screen.
mockProviders(['files'])
searchController.search('second', ['files'])
expect(searchController.getSnapshot()).toEqual({
files: { status: 'loading', entries: ['Files result'], cursor: null, hasMore: false, loadMoreFailed: false },
files: loading,
})
expect(searchController.getRevealOrder()).toEqual(['files'])
expect(searchController.getRevealOrder()).toEqual([])
})
})
describe('stale-while-revalidate', () => {
it('keeps the previous results visible while a refetch is in flight', async () => {
describe('changing the query', () => {
it('drops the previous results as soon as the query changes', async () => {
const first = mockProviders(['files'])
const searchController = new UnifiedSearchController()
@ -465,19 +461,18 @@ describe('UnifiedSearchController', () => {
first.files.resolve(['Old result'])
await vi.advanceTimersByTimeAsync(0)
// A refined query starts a new search. The prior entries must stay on screen
// (status loading, entries kept) so the panel does not flash empty mid-request.
// The new query is about to return different results, so keeping the old ones up
// would only let them shift under the user once the real ones land. Hide, then show.
const second = mockProviders(['files'])
searchController.search('new', ['files'])
expect(searchController.getSnapshot().files).toEqual({
status: 'loading',
entries: ['Old result'],
entries: [],
cursor: null,
hasMore: false,
loadMoreFailed: false,
})
// The fresh page replaces them once it lands.
second.files.resolve(['New result'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getSnapshot().files).toEqual({
@ -489,7 +484,7 @@ describe('UnifiedSearchController', () => {
})
})
it('settles a refetched category that carried results straight to loaded, never blocked', async () => {
it('puts every category back through the ordered reveal on a new query', async () => {
const first = mockProviders(['files', 'talk'])
const searchController = new UnifiedSearchController()
@ -499,23 +494,15 @@ describe('UnifiedSearchController', () => {
first.talk.resolve(['Old talk'])
await vi.advanceTimersByTimeAsync(0)
// Refine. talk (lower priority) comes back before files this time. It already had
// results, so it must not drop into blocked (which excludes it from the rendered
// set and blinks it off screen); it stays visible by settling straight to loaded.
// Refine. talk comes back first this time. Nothing is on screen to protect any more,
// so it takes its turn in the queue again instead of skipping the reveal.
const second = mockProviders(['files', 'talk'])
searchController.search('new', ['files', 'talk'])
second.talk.resolve(['New talk'])
await vi.advanceTimersByTimeAsync(0)
expect(searchController.getSnapshot().talk.status).toBe('loaded')
// files is still fetching; its stale page stays up meanwhile.
expect(searchController.getSnapshot().files).toEqual({
status: 'loading',
entries: ['Old files'],
cursor: null,
hasMore: false,
loadMoreFailed: false,
})
expect(searchController.getSnapshot().talk.status).toBe('blocked')
expect(searchController.getRevealOrder()).toEqual([])
})
})
@ -1070,10 +1057,8 @@ describe('UnifiedSearchController', () => {
const searchController = new UnifiedSearchController()
searchController.search('first', ['files', 'talk'])
// The first search spends its window on talk, then stands the timer down. talk
// comes back empty so it carries no stale results into the second search, which
// would otherwise settle it straight to loaded and never block it.
first.talk.resolve([])
// The first search spends its window on talk, then stands the timer down.
first.talk.resolve(['First talk'])
await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS)
expect(searchController.getSnapshot().talk.status).toBe('loaded')
expect(vi.getTimerCount()).toBe(0)

Write Preview
Loading…
Cancel
Save