Skip to content

Commit 3cb735e

Browse files
author
Jairon Terrero
committed
fix: Serial Execution Strategy
Updated test mocks to properly expose non-serial behavior. Updated SerialFieldExecutionStrategy to actually be serial.
1 parent 7c910f2 commit 3cb735e

File tree

3 files changed

+61
-28
lines changed

3 files changed

+61
-28
lines changed

Sources/GraphQL/Execution/Execute.swift

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -118,24 +118,30 @@ public struct SerialFieldExecutionStrategy: QueryFieldExecutionStrategy,
118118
path: IndexPath,
119119
fields: OrderedDictionary<String, [Field]>
120120
) throws -> Future<OrderedDictionary<String, Any>> {
121-
var results = OrderedDictionary<String, Future<Any>>()
122-
123-
try fields.forEach { field in
124-
let fieldASTs = field.value
125-
let fieldPath = path.appending(field.key)
126-
127-
let result = try resolveField(
128-
exeContext: exeContext,
129-
parentType: parentType,
130-
source: sourceValue,
131-
fieldASTs: fieldASTs,
132-
path: fieldPath
133-
)
134-
135-
results[field.key] = result.map { $0 ?? Map.null }
136-
}
137-
138-
return results.flatten(on: exeContext.eventLoopGroup)
121+
var results = OrderedDictionary<String, Any>()
122+
123+
return fields.reduce(exeContext.eventLoopGroup.next().makeSucceededFuture(results), { prev, field in
124+
// We use ``flatSubmit`` here to avoid a stack overflow issue with EventLoopFutures.
125+
// See: https://github.com/apple/swift-nio/issues/970
126+
exeContext.eventLoopGroup.next().flatSubmit {
127+
prev.tryFlatMap { results in
128+
var results = results
129+
let fieldASTs = field.value
130+
let fieldPath = path.appending(field.key)
131+
132+
return try resolveField(
133+
exeContext: exeContext,
134+
parentType: parentType,
135+
source: sourceValue,
136+
fieldASTs: fieldASTs,
137+
path: fieldPath
138+
).map { result in
139+
results[field.key] = result ?? Map.null
140+
return results
141+
}
142+
}
143+
}
144+
})
139145
}
140146
}
141147

Sources/GraphQL/Utilities/NIO+Extensions.swift

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,32 @@ public protocol FutureType {
101101
extension Future: FutureType {
102102
public typealias Expectation = Value
103103
}
104+
105+
106+
// Copied from https://github.com/vapor/async-kit/blob/e2f741640364c1d271405da637029ea6a33f754e/Sources/AsyncKit/EventLoopFuture/Future%2BTry.swift
107+
// in order to avoid full package dependency.
108+
extension EventLoopFuture {
109+
public func tryFlatMap<NewValue>(
110+
file _: StaticString = #file, line _: UInt = #line,
111+
_ callback: @escaping (Value) throws -> EventLoopFuture<NewValue>
112+
) -> EventLoopFuture<NewValue> {
113+
/// When the current `EventLoopFuture<Value>` is fulfilled, run the provided callback,
114+
/// which will provide a new `EventLoopFuture`.
115+
///
116+
/// This allows you to dynamically dispatch new asynchronous tasks as phases in a
117+
/// longer series of processing steps. Note that you can use the results of the
118+
/// current `EventLoopFuture<Value>` when determining how to dispatch the next operation.
119+
///
120+
/// The key difference between this method and the regular `flatMap` is error handling.
121+
///
122+
/// With `tryFlatMap`, the provided callback _may_ throw Errors, causing the returned `EventLoopFuture<Value>`
123+
/// to report failure immediately after the completion of the original `EventLoopFuture`.
124+
return self.flatMap() { [eventLoop] value in
125+
do {
126+
return try callback(value)
127+
} catch {
128+
return eventLoop.makeFailedFuture(error)
129+
}
130+
}
131+
}
132+
}

Tests/GraphQLTests/FieldExecutionStrategyTests/FieldExecutionStrategyTests.swift

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import Dispatch
22
@testable import GraphQL
33
import NIO
4+
import NIOConcurrencyHelpers
45
import XCTest
56

67
class FieldExecutionStrategyTests: XCTestCase {
@@ -15,15 +16,12 @@ class FieldExecutionStrategyTests: XCTestCase {
1516
"sleep": GraphQLField(
1617
type: GraphQLString,
1718
resolve: { _, _, _, eventLoopGroup, _ in
18-
let group = DispatchGroup()
19-
group.enter()
20-
21-
DispatchQueue.global().asyncAfter(wallDeadline: .now() + 0.1) {
22-
group.leave()
19+
let promise = eventLoopGroup.next().makePromise(of: Optional<Any>.self)
20+
Task {
21+
try await Task.sleep(nanoseconds: UInt64(1E8))
22+
promise.succeed("z")
2323
}
24-
25-
group.wait()
26-
return eventLoopGroup.next().makeSucceededFuture("z")
24+
return promise.futureResult
2725
}
2826
),
2927
"bang": GraphQLField(
@@ -251,7 +249,7 @@ class FieldExecutionStrategyTests: XCTestCase {
251249
eventLoopGroup: eventLoopGroup
252250
).wait())
253251
XCTAssertEqual(result.value, multiExpected)
254-
// XCTAssertEqualWithAccuracy(1.0, result.seconds, accuracy: 0.5)
252+
// XCTAssertEqualWithAccuracy(1.0, result.seconds, accuracy: 0.5)
255253
}
256254

257255
func testSerialFieldExecutionStrategyWithMultipleFieldErrors() throws {
@@ -300,7 +298,7 @@ class FieldExecutionStrategyTests: XCTestCase {
300298
eventLoopGroup: eventLoopGroup
301299
).wait())
302300
XCTAssertEqual(result.value, multiExpected)
303-
// XCTAssertEqualWithAccuracy(0.1, result.seconds, accuracy: 0.25)
301+
// XCTAssertEqualWithAccuracy(0.1, result.seconds, accuracy: 0.25)
304302
}
305303

306304
func testConcurrentDispatchFieldExecutionStrategyWithMultipleFieldErrors() throws {

0 commit comments

Comments
 (0)