Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit e70cef8

Browse files
[Backport 5.2] alter function using apollo client to achieve pagination (#59837)
Search contexts: paginate repo names (#58685) A customer reported that search contexts with over a thousand repos were failing when trying to create them. The root cause is that the webapp was fetching repositories to validate them, but was not using pagination, and instead tried to get them all in one go. The backend limits the number of results in a page to 1000, even if the client requests more. Now, the web app pages through results. (cherry picked from commit 4397287) Co-authored-by: Jason Hawk Harris <[email protected]>
1 parent 3c0a8b4 commit e70cef8

File tree

3 files changed

+66
-28
lines changed

3 files changed

+66
-28
lines changed

client/web/src/enterprise/searchContexts/SearchContextForm.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, { useCallback, useMemo, useState } from 'react'
22

33
import classNames from 'classnames'
44
import { useNavigate } from 'react-router-dom'
5-
import { type Observable, of, throwError } from 'rxjs'
5+
import { type Observable, of, throwError, from } from 'rxjs'
66
import { catchError, map, startWith, switchMap, tap } from 'rxjs/operators'
77

88
import { SyntaxHighlightedSearchQuery, LazyQueryInput } from '@sourcegraph/branded'
@@ -221,7 +221,7 @@ export const SearchContextForm: React.FunctionComponent<React.PropsWithChildren<
221221
return of({ type: 'repositories', repositories: [] } as RepositoriesParseResult)
222222
}
223223

224-
return fetchRepositoriesByNames(repositoryNames).pipe(
224+
return from(fetchRepositoriesByNames(repositoryNames)).pipe(
225225
map(repositories => {
226226
const repositoryNameToID = new Map(repositories.map(({ id, name }) => [name, id]))
227227
const errors: Error[] = []
Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,48 @@
1-
import type { Observable } from 'rxjs'
2-
import { map } from 'rxjs/operators'
3-
41
import { dataOrThrowErrors, gql } from '@sourcegraph/http-client'
2+
import type { GraphQLResult } from '@sourcegraph/http-client'
53

64
import { requestGraphQL } from '../../backend/graphql'
7-
import type { RepositoriesByNamesResult, RepositoriesByNamesVariables } from '../../graphql-operations'
5+
import type { InputMaybe, RepositoriesByNamesResult, RepositoriesByNamesVariables } from '../../graphql-operations'
6+
7+
const query = gql`
8+
query RepositoriesByNames($names: [String!]!, $first: Int!, $after: String) {
9+
repositories(names: $names, first: $first, after: $after) {
10+
nodes {
11+
id
12+
name
13+
}
14+
pageInfo {
15+
endCursor
16+
hasNextPage
17+
}
18+
}
19+
}
20+
`
821

9-
export function fetchRepositoriesByNames(
22+
export async function fetchRepositoriesByNames(
1023
names: string[]
11-
): Observable<RepositoriesByNamesResult['repositories']['nodes']> {
24+
): Promise<RepositoriesByNamesResult['repositories']['nodes']> {
25+
let repos: RepositoriesByNamesResult['repositories']['nodes'] = []
1226
const first = names.length
13-
return requestGraphQL<RepositoriesByNamesResult, RepositoriesByNamesVariables>(
14-
gql`
15-
query RepositoriesByNames($names: [String!]!, $first: Int!) {
16-
repositories(names: $names, first: $first) {
17-
nodes {
18-
id
19-
name
20-
}
21-
}
22-
}
23-
`,
24-
{
27+
let after: InputMaybe<string> = null
28+
29+
while (true) {
30+
const result: GraphQLResult<RepositoriesByNamesResult> = await requestGraphQL<
31+
RepositoriesByNamesResult,
32+
RepositoriesByNamesVariables
33+
>(query, {
2534
names,
2635
first,
36+
after,
37+
}).toPromise()
38+
39+
const data: RepositoriesByNamesResult = dataOrThrowErrors(result)
40+
41+
repos = repos.concat(data.repositories.nodes)
42+
if (!data.repositories.pageInfo.hasNextPage) {
43+
break
2744
}
28-
).pipe(
29-
map(dataOrThrowErrors),
30-
map(data => data.repositories.nodes)
31-
)
45+
after = data.repositories.pageInfo.endCursor
46+
}
47+
return repos
3248
}

client/web/src/integration/search-contexts.test.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,19 @@ describe('Search contexts', () => {
142142
test('Create static search context', async () => {
143143
testContext.overrideGraphQL({
144144
...testContextForSearchContexts,
145-
RepositoriesByNames: ({ names }) => ({
146-
repositories: { nodes: names.map((name, index) => ({ id: `index-${index}`, name })) },
145+
RepositoriesByNames: ({ names, first, after }) => ({
146+
repositories: {
147+
nodes: names.map((name, index) => ({ id: `index-${index}`, name })),
148+
pageInfo: {
149+
endCursor: null,
150+
hasNextPage: false,
151+
},
152+
},
153+
variables: {
154+
names,
155+
first,
156+
after,
157+
},
147158
}),
148159
CreateSearchContext: ({ searchContext, repositories }) => ({
149160
createSearchContext: {
@@ -291,8 +302,19 @@ describe('Search contexts', () => {
291302
test('Edit search context', async () => {
292303
testContext.overrideGraphQL({
293304
...testContextForSearchContexts,
294-
RepositoriesByNames: ({ names }) => ({
295-
repositories: { nodes: names.map((name, index) => ({ id: `index-${index}`, name })) },
305+
RepositoriesByNames: ({ names, first, after }) => ({
306+
repositories: {
307+
nodes: names.map((name, index) => ({ id: `index-${index}`, name })),
308+
pageInfo: {
309+
endCursor: null,
310+
hasNextPage: false,
311+
},
312+
},
313+
variables: {
314+
names,
315+
first,
316+
after,
317+
},
296318
}),
297319
UpdateSearchContext: ({ id, searchContext, repositories }) => ({
298320
updateSearchContext: {

0 commit comments

Comments
 (0)