Skip to content

Commit 2517f8b

Browse files
committed
more wip on batching
1 parent 3cab26c commit 2517f8b

File tree

6 files changed

+78
-25
lines changed

6 files changed

+78
-25
lines changed

packages/app/cypress/e2e/specs_list_latest_runs.cy.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ describe('ACI - Latest runs and Average duration', { viewportWidth: 1200, viewpo
149149
beforeEach(() => {
150150
cy.loginUser()
151151

152-
cy.remoteGraphQLIntercept(async (obj) => {
152+
cy.remoteGraphQLInterceptBatched(async (obj) => {
153153
if (obj.result.data && 'cloudSpecByPath' in obj.result.data) {
154154
obj.result.data.cloudSpecByPath = {
155155
__typename: 'CloudProjectSpecNotFound',

packages/app/src/specs/SpecsList.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ const collapsible = computed(() => {
436436
})
437437
const treeSpecList = computed(() => collapsible.value.tree.filter(((item) => !item.hidden.value)))
438438
439-
const { containerProps, list, wrapperProps, scrollTo } = useVirtualList(treeSpecList, { itemHeight: 40, overscan: 10 })
439+
const { containerProps, list, wrapperProps, scrollTo } = useVirtualList(treeSpecList, { itemHeight: 40, overscan: 2 })
440440
441441
const scrollbarOffset = ref(0)
442442

packages/data-context/src/sources/CloudDataSource.ts

+50-22
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import crypto from 'crypto'
99

1010
import type { DataContext } from '..'
1111
import getenv from 'getenv'
12-
import { print, DocumentNode, ExecutionResult, GraphQLResolveInfo, OperationTypeNode } from 'graphql'
12+
import { print, DocumentNode, ExecutionResult, GraphQLResolveInfo, OperationTypeNode, visit, OperationDefinitionNode } from 'graphql'
1313
import {
1414
createClient,
1515
dedupExchange,
@@ -50,6 +50,7 @@ export interface CloudExecuteQuery {
5050
}
5151

5252
export interface CloudExecuteRemote extends CloudExecuteQuery {
53+
shouldBatch?: boolean
5354
operationType?: OperationTypeNode
5455
requestPolicy?: RequestPolicy
5556
onUpdatedResult?: (data: any) => any
@@ -81,14 +82,9 @@ export class CloudDataSource {
8182

8283
constructor (private params: CloudDataSourceParams) {
8384
this.#cloudUrqlClient = this.reset()
84-
this.#batchExecutor = createBatchingExecutor(({ document, variables }) => {
85-
debug(`Executing remote dashboard request %s, %j`, print(document), variables)
86-
87-
return this.#cloudUrqlClient.query(document, variables ?? {}, {
88-
...this.#additionalHeaders,
89-
requestPolicy: 'network-only',
90-
}).toPromise()
91-
})
85+
this.#batchExecutor = createBatchingExecutor((config) => {
86+
return this.#executeQuery(namedExecutionDocument(config.document), config.variables)
87+
}, { maxBatchSize: 20 })
9288

9389
this.#batchExecutorBatcher = this.#makeBatchExecutionBatcher()
9490
}
@@ -213,7 +209,11 @@ export class CloudDataSource {
213209
return loading
214210
}
215211

216-
loading = this.#batchExecutorBatcher.load(config).then(this.#formatWithErrors)
212+
const query = config.shouldBatch
213+
? this.#batchExecutorBatcher.load(config)
214+
: this.#executeQuery(config.operationDoc, config.operationVariables)
215+
216+
loading = query.then(this.#formatWithErrors)
217217
.then((op) => {
218218
this.#pendingPromises.delete(stableKey)
219219

@@ -235,6 +235,12 @@ export class CloudDataSource {
235235
return loading
236236
}
237237

238+
#executeQuery (operationDoc: DocumentNode, operationVariables: object = {}) {
239+
debug(`Executing remote dashboard request %s, %j`, print(operationDoc), operationVariables)
240+
241+
return this.#cloudUrqlClient.query(operationDoc, operationVariables, { requestPolicy: 'network-only' }).toPromise()
242+
}
243+
238244
isResolving (config: CloudExecuteQuery) {
239245
const stableKey = this.#hashRemoteRequest(config)
240246

@@ -330,18 +336,6 @@ export class CloudDataSource {
330336
*/
331337
#makeBatchExecutionBatcher () {
332338
return new DataLoader<CloudExecuteRemote, any>(async (toBatch) => {
333-
const first = toBatch[0]
334-
335-
// If we only have a single entry, we can just hit the query directly,
336-
// without rewriting anything - this makes the queries simpler in most cases in the app
337-
if (toBatch.length === 1 && first) {
338-
return [this.#cloudUrqlClient.query(first.operation, first.operationVariables ?? {}, {
339-
...this.#additionalHeaders,
340-
requestPolicy: 'network-only',
341-
}).toPromise()]
342-
}
343-
344-
// Otherwise run this through batchExecutor:
345339
return Promise.allSettled(toBatch.map((b) => {
346340
return this.#batchExecutor({
347341
operationType: 'query',
@@ -358,3 +352,37 @@ export class CloudDataSource {
358352
return val instanceof Error ? val : new Error(val)
359353
}
360354
}
355+
356+
/**
357+
* Adds "batchExecutionQuery" to the query that we generate from the batch loader,
358+
* useful to key off of in the tests.
359+
*/
360+
function namedExecutionDocument (document: DocumentNode) {
361+
let hasReplaced = false
362+
363+
return visit(document, {
364+
enter () {
365+
if (hasReplaced) {
366+
return false
367+
}
368+
369+
return
370+
},
371+
OperationDefinition (op) {
372+
if (op.name) {
373+
return op
374+
}
375+
376+
hasReplaced = true
377+
const namedOperationNode: OperationDefinitionNode = {
378+
...op,
379+
name: {
380+
kind: 'Name',
381+
value: 'batchTestRunnerExecutionQuery',
382+
},
383+
}
384+
385+
return namedOperationNode
386+
},
387+
})
388+
}

packages/data-context/src/sources/RemoteRequestDataSource.ts

+4
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ interface MaybeLoadRemoteFetchable extends CloudExecuteQuery {
1919
shouldFetch?: boolean
2020
remoteQueryField: string
2121
isMutation: boolean
22+
shouldBatch?: boolean
2223
}
2324

2425
interface OperationDefinition {
@@ -65,6 +66,7 @@ export class RemoteRequestDataSource {
6566
remoteQueryField,
6667
shouldFetch: true,
6768
isMutation: true,
69+
shouldBatch: true,
6870
})
6971
}
7072

@@ -157,6 +159,7 @@ export class RemoteRequestDataSource {
157159
operationHash,
158160
operationVariables,
159161
requestPolicy: 'network-only',
162+
shouldBatch: params.shouldBatch,
160163
}))
161164
.then((result) => {
162165
const toPushDefinition = this.#operationRegistryPushToFrontend.get(operationHash)
@@ -292,6 +295,7 @@ export class RemoteRequestDataSource {
292295
shouldFetch: shouldEagerFetch,
293296
remoteQueryField: fieldConfig.remoteQueryField,
294297
isMutation: false,
298+
shouldBatch: true,
295299
})
296300
})
297301
}

packages/frontend-shared/cypress/e2e/e2ePluginSetup.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ async function makeE2ETasks () {
105105
let ctx: DataContext
106106
let testState: Record<string, any> = {}
107107
let remoteGraphQLIntercept: RemoteGraphQLInterceptor | undefined
108+
let remoteGraphQLInterceptBatchHandler: RemoteGraphQLInterceptor | undefined
108109
let scaffoldedProjects = new Set<string>()
109110

110111
const cachedCwd = process.cwd()
@@ -182,6 +183,7 @@ async function makeE2ETasks () {
182183
sinon.reset()
183184
sinon.restore()
184185
remoteGraphQLIntercept = undefined
186+
remoteGraphQLInterceptBatchHandler = undefined
185187

186188
const fetchApi = ctx.util.fetch
187189

@@ -213,7 +215,11 @@ async function makeE2ETasks () {
213215

214216
operationCount[operationName ?? 'unknown']++
215217

216-
if (remoteGraphQLIntercept) {
218+
if (operationName === 'batchTestRunnerExecutionQuery' && remoteGraphQLInterceptBatchHandler) {
219+
// const toSettle = _.mapValues(result.data, (key, val) => {
220+
// /^_(?:\d+)_(.*?)%/.test(key)
221+
// })
222+
} else if (remoteGraphQLIntercept) {
217223
try {
218224
result = await remoteGraphQLIntercept({
219225
operationName,
@@ -278,6 +284,11 @@ async function makeE2ETasks () {
278284

279285
return null
280286
},
287+
__internal_remoteGraphQLInterceptBatched (fn: string) {
288+
remoteGraphQLInterceptBatchHandler = new Function('console', 'obj', 'testState', `return (${fn})(obj, testState)`).bind(null, console) as RemoteGraphQLInterceptor
289+
290+
return null
291+
},
281292
async __internal_addProject (opts: InternalAddProjectOpts) {
282293
if (!scaffoldedProjects.has(opts.projectName)) {
283294
await __internal_scaffoldProject(opts.projectName)

packages/frontend-shared/cypress/e2e/support/e2eSupport.ts

+10
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ declare global {
133133
* Gives the ability to intercept the remote GraphQL request & respond accordingly
134134
*/
135135
remoteGraphQLIntercept: typeof remoteGraphQLIntercept
136+
/**
137+
* Gives the ability to intercept the remote GraphQL request & respond accordingly
138+
*/
139+
remoteGraphQLInterceptBatched: typeof remoteGraphQLInterceptBatched
136140
/**
137141
* Removes the sinon spy'ing on the remote GraphQL fake requests
138142
*/
@@ -444,6 +448,12 @@ function remoteGraphQLIntercept (fn: RemoteGraphQLInterceptor) {
444448
})
445449
}
446450

451+
function remoteGraphQLInterceptBatched (fn: RemoteGraphQLInterceptor) {
452+
return logInternal('remoteGraphQLInterceptBatched', () => {
453+
return taskInternal('__internal_remoteGraphQLInterceptBatched', fn.toString())
454+
})
455+
}
456+
447457
type Resolved<V> = V extends Promise<infer U> ? U : V
448458

449459
/**

0 commit comments

Comments
 (0)