Skip to content

Commit

Permalink
fix: Serial Execution Strategy
Browse files Browse the repository at this point in the history
Updated test mocks to properly expose non-serial behavior.
Updated SerialFieldExecutionStrategy to actually be serial.
  • Loading branch information
Jairon Terrero committed Apr 11, 2023
1 parent 7c910f2 commit f0812cd
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 28 deletions.
41 changes: 23 additions & 18 deletions Sources/GraphQL/Execution/Execute.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,24 +118,29 @@ public struct SerialFieldExecutionStrategy: QueryFieldExecutionStrategy,
path: IndexPath,
fields: OrderedDictionary<String, [Field]>
) throws -> Future<OrderedDictionary<String, Any>> {
var results = OrderedDictionary<String, Future<Any>>()

try fields.forEach { field in
let fieldASTs = field.value
let fieldPath = path.appending(field.key)

let result = try resolveField(
exeContext: exeContext,
parentType: parentType,
source: sourceValue,
fieldASTs: fieldASTs,
path: fieldPath
)

results[field.key] = result.map { $0 ?? Map.null }
}

return results.flatten(on: exeContext.eventLoopGroup)
var results = OrderedDictionary<String, Any>()

return fields
.reduce(exeContext.eventLoopGroup.next().makeSucceededVoidFuture()) { prev, field in
// We use ``flatSubmit`` here to avoid a stack overflow issue with EventLoopFutures.
// See: https://github.com/apple/swift-nio/issues/970
exeContext.eventLoopGroup.next().flatSubmit {
prev.tryFlatMap {
let fieldASTs = field.value
let fieldPath = path.appending(field.key)

return try resolveField(
exeContext: exeContext,
parentType: parentType,
source: sourceValue,
fieldASTs: fieldASTs,
path: fieldPath
).map { result in
results[field.key] = result ?? Map.null
}
}
}
}.map { results }
}
}

Expand Down
28 changes: 28 additions & 0 deletions Sources/GraphQL/Utilities/NIO+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,31 @@ public protocol FutureType {
extension Future: FutureType {
public typealias Expectation = Value
}

// Copied from https://github.com/vapor/async-kit/blob/e2f741640364c1d271405da637029ea6a33f754e/Sources/AsyncKit/EventLoopFuture/Future%2BTry.swift
// in order to avoid full package dependency.
public extension EventLoopFuture {
func tryFlatMap<NewValue>(
file _: StaticString = #file, line _: UInt = #line,
_ callback: @escaping (Value) throws -> EventLoopFuture<NewValue>
) -> EventLoopFuture<NewValue> {
/// When the current `EventLoopFuture<Value>` is fulfilled, run the provided callback,
/// which will provide a new `EventLoopFuture`.
///
/// This allows you to dynamically dispatch new asynchronous tasks as phases in a
/// longer series of processing steps. Note that you can use the results of the
/// current `EventLoopFuture<Value>` when determining how to dispatch the next operation.
///
/// The key difference between this method and the regular `flatMap` is error handling.
///
/// With `tryFlatMap`, the provided callback _may_ throw Errors, causing the returned `EventLoopFuture<Value>`
/// to report failure immediately after the completion of the original `EventLoopFuture`.
return flatMap { [eventLoop] value in
do {
return try callback(value)
} catch {
return eventLoop.makeFailedFuture(error)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Dispatch
@testable import GraphQL
import NIO
import NIOConcurrencyHelpers
import XCTest

class FieldExecutionStrategyTests: XCTestCase {
Expand All @@ -15,15 +16,10 @@ class FieldExecutionStrategyTests: XCTestCase {
"sleep": GraphQLField(
type: GraphQLString,
resolve: { _, _, _, eventLoopGroup, _ in
let group = DispatchGroup()
group.enter()

DispatchQueue.global().asyncAfter(wallDeadline: .now() + 0.1) {
group.leave()
eventLoopGroup.next().makeSucceededVoidFuture().map {
Thread.sleep(forTimeInterval: 0.1)
return "z"
}

group.wait()
return eventLoopGroup.next().makeSucceededFuture("z")
}
),
"bang": GraphQLField(
Expand Down Expand Up @@ -251,7 +247,7 @@ class FieldExecutionStrategyTests: XCTestCase {
eventLoopGroup: eventLoopGroup
).wait())
XCTAssertEqual(result.value, multiExpected)
// XCTAssertEqualWithAccuracy(1.0, result.seconds, accuracy: 0.5)
// XCTAssertEqualWithAccuracy(1.0, result.seconds, accuracy: 0.5)
}

func testSerialFieldExecutionStrategyWithMultipleFieldErrors() throws {
Expand Down Expand Up @@ -300,7 +296,7 @@ class FieldExecutionStrategyTests: XCTestCase {
eventLoopGroup: eventLoopGroup
).wait())
XCTAssertEqual(result.value, multiExpected)
// XCTAssertEqualWithAccuracy(0.1, result.seconds, accuracy: 0.25)
// XCTAssertEqualWithAccuracy(0.1, result.seconds, accuracy: 0.25)
}

func testConcurrentDispatchFieldExecutionStrategyWithMultipleFieldErrors() throws {
Expand Down

0 comments on commit f0812cd

Please sign in to comment.