From 10f35a91bcdc0c9c37aea1a493793b37040040cc Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Sep 2021 16:44:52 -0600 Subject: [PATCH 1/7] Adds 'undefined' Map case This allows input types to differentiate between fields that were defined as null or undefined --- Sources/GraphQL/Map/Map.swift | 32 +++- Sources/GraphQL/Map/MapSerialization.swift | 6 +- Sources/GraphQL/Utilities/ValueFromAST.swift | 5 + .../GraphQLTests/InputTests/InputTests.swift | 159 ++++++++++++++++++ 4 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 Tests/GraphQLTests/InputTests/InputTests.swift diff --git a/Sources/GraphQL/Map/Map.swift b/Sources/GraphQL/Map/Map.swift index 8ea1690c..f80f75db 100644 --- a/Sources/GraphQL/Map/Map.swift +++ b/Sources/GraphQL/Map/Map.swift @@ -11,6 +11,7 @@ public enum MapError : Error { // MARK: Map public enum Map { + case undefined case null case bool(Bool) case number(Number) @@ -165,6 +166,13 @@ extension Map { // MARK: is extension Map { + public var isUndefined: Bool { + if case .undefined = self { + return true + } + return false + } + public var isNull: Bool { if case .null = self { return true @@ -206,6 +214,8 @@ extension Map { extension Map { public var typeDescription: String { switch self { + case .undefined: + return "undefined" case .null: return "null" case .bool: @@ -259,6 +269,9 @@ extension Map { } switch self { + case .undefined: + return false + case .null: return false @@ -337,6 +350,9 @@ extension Map { } switch self { + case .undefined: + return "undefined" + case .null: return "null" @@ -645,6 +661,8 @@ extension Map : Codable { var container = encoder.singleValueContainer() switch self { + case .undefined: + fatalError("undefined values should have been excluded from encoding") case .null: try container.encodeNil() case let .bool(value): @@ -660,8 +678,10 @@ extension Map : Codable { // Instead decode as a keyed container (like normal Dictionary) in the order of our OrderedDictionary var container = encoder.container(keyedBy: _DictionaryCodingKey.self) for (key, value) in dictionary { - let codingKey = _DictionaryCodingKey(stringValue: key)! - try container.encode(value, forKey: codingKey) + if !value.isUndefined { + let codingKey = _DictionaryCodingKey(stringValue: key)! + try container.encode(value, forKey: codingKey) + } } } } @@ -711,6 +731,8 @@ public func == (lhs: Map, rhs: Map) -> Bool { extension Map : Hashable { public func hash(into hasher: inout Hasher) { switch self { + case .undefined: + hasher.combine(0) case .null: hasher.combine(0) case let .bool(value): @@ -837,6 +859,8 @@ extension Map { func serialize(map: Map) -> String { switch map { + case .undefined: + return "undefined" case .null: return "null" case let .bool(value): @@ -893,6 +917,10 @@ extension Map { } for (key, value) in dictionary.sorted(by: {$0.0 < $1.0}) { + guard !value.isUndefined else { + continue // Do not serialize undefined values + } + if debug { string += "\n" string += indent() diff --git a/Sources/GraphQL/Map/MapSerialization.swift b/Sources/GraphQL/Map/MapSerialization.swift index b3ddda51..5a2ea022 100644 --- a/Sources/GraphQL/Map/MapSerialization.swift +++ b/Sources/GraphQL/Map/MapSerialization.swift @@ -34,6 +34,8 @@ public struct MapSerialization { static func object(with map: Map) throws -> NSObject { switch map { + case .undefined: + fatalError("undefined values should have been excluded from serialization") case .null: return NSNull() case let .bool(value): @@ -48,7 +50,9 @@ public struct MapSerialization { // Coerce to an unordered dictionary var unorderedDictionary: [String: NSObject] = [:] for (key, value) in dictionary { - try unorderedDictionary[key] = object(with: value) + if !value.isUndefined { + try unorderedDictionary[key] = object(with: value) + } } return unorderedDictionary as NSDictionary } diff --git a/Sources/GraphQL/Utilities/ValueFromAST.swift b/Sources/GraphQL/Utilities/ValueFromAST.swift index 65474fc0..4ced06ba 100644 --- a/Sources/GraphQL/Utilities/ValueFromAST.swift +++ b/Sources/GraphQL/Utilities/ValueFromAST.swift @@ -70,6 +70,11 @@ func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: var obj = obj let field = fields[fieldName] let fieldAST = fieldASTs[fieldName] + guard fieldAST != nil else { + obj[fieldName] = .undefined + return obj + } + var fieldValue = try valueFromAST( valueAST: fieldAST?.value, type: field!.type, diff --git a/Tests/GraphQLTests/InputTests/InputTests.swift b/Tests/GraphQLTests/InputTests/InputTests.swift new file mode 100644 index 00000000..641cb2cd --- /dev/null +++ b/Tests/GraphQLTests/InputTests/InputTests.swift @@ -0,0 +1,159 @@ +import XCTest +import NIO +@testable import GraphQL + +fileprivate struct Echo { + let field1: String? + let field2: String? +} + +fileprivate let EchoInputType = try! GraphQLInputObjectType( + name: "EchoInput", + fields: [ + "field1": InputObjectField( + type: GraphQLString + ), + "field2": InputObjectField( + type: GraphQLString + ), + ] +) + +fileprivate let EchoOutputType = try! GraphQLObjectType( + name: "Echo", + description: "", + fields: [ + "field1": GraphQLField( + type: GraphQLString + ), + "field2": GraphQLField( + type: GraphQLString + ), + ], + isTypeOf: { source, _, _ in + source is Echo + } +) + +class InputTests : XCTestCase { + + let schema = try! GraphQLSchema( + query: try! GraphQLObjectType( + name: "Query", + fields: [ + "echo": GraphQLField( + type: EchoOutputType, + args: [ + "input": GraphQLArgument( + type: EchoInputType + ) + ], + resolve: { _, arguments, _, _ in + let input = arguments["input"] + print(input["field2"]) + return Echo( + field1: input["field1"].string, + field2: input["field2"].string + ) + } + ), + ] + ), + types: [EchoInputType, EchoOutputType] + ) + + func testBasic() throws { + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1", + field2: "value2", + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": "value2", + ] + ]) + ) + } + + func testIncludedNull() throws { + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1", + field2: null, + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": nil, + ] + ]) + ) + } + + func testImpliedNull() throws { + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1" + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": nil, + ] + ]) + ) + } +} From dbf92a5c3968cb1483e0ff5ad47f54528cad0a57 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Sep 2021 21:40:14 -0600 Subject: [PATCH 2/7] Parsed Maps include null/undefined values --- Sources/GraphQL/Execution/Values.swift | 20 ++++--- Sources/GraphQL/Map/Map.swift | 12 ++--- Sources/GraphQL/Utilities/ValueFromAST.swift | 56 ++++++++++---------- 3 files changed, 45 insertions(+), 43 deletions(-) diff --git a/Sources/GraphQL/Execution/Values.swift b/Sources/GraphQL/Execution/Values.swift index 3fc34bc7..f703e20c 100644 --- a/Sources/GraphQL/Execution/Values.swift +++ b/Sources/GraphQL/Execution/Values.swift @@ -35,16 +35,20 @@ func getArgumentValues(argDefs: [GraphQLArgumentDefinition], argASTs: [Argument] return try argDefs.reduce([:]) { result, argDef in var result = result let name = argDef.name - let valueAST = argASTMap[name]?.value + let argAST = argASTMap[name] + + if let argAST = argAST { + let valueAST = argAST.value - let value = try valueFromAST( - valueAST: valueAST, - type: argDef.type, - variables: variableValues - ) ?? argDef.defaultValue + let value = try valueFromAST( + valueAST: valueAST, + type: argDef.type, + variables: variableValues + ) - if let value = value { result[name] = value + } else { + result[name] = .null } return result @@ -75,7 +79,7 @@ func getVariableValue(schema: GraphQLSchema, definitionAST: VariableDefinition, if errors.isEmpty { if input == .null { if let defaultValue = definitionAST.defaultValue { - return try valueFromAST(valueAST: defaultValue, type: inputType)! + return try valueFromAST(valueAST: defaultValue, type: inputType) } else if !(inputType is GraphQLNonNull) { return .null diff --git a/Sources/GraphQL/Map/Map.swift b/Sources/GraphQL/Map/Map.swift index f80f75db..f185889b 100644 --- a/Sources/GraphQL/Map/Map.swift +++ b/Sources/GraphQL/Map/Map.swift @@ -915,12 +915,12 @@ extension Map { if debug { indentLevel += 1 } + + let filtered = dictionary.filter({ item in + !item.value.isUndefined + }) - for (key, value) in dictionary.sorted(by: {$0.0 < $1.0}) { - guard !value.isUndefined else { - continue // Do not serialize undefined values - } - + for (key, value) in filtered.sorted(by: {$0.0 < $1.0}) { if debug { string += "\n" string += indent() @@ -929,7 +929,7 @@ extension Map { string += escape(key) + ":" + serialize(map: value) } - if index != dictionary.count - 1 { + if index != filtered.count - 1 { if debug { string += ", " } else { diff --git a/Sources/GraphQL/Utilities/ValueFromAST.swift b/Sources/GraphQL/Utilities/ValueFromAST.swift index 4ced06ba..5d98ea73 100644 --- a/Sources/GraphQL/Utilities/ValueFromAST.swift +++ b/Sources/GraphQL/Utilities/ValueFromAST.swift @@ -17,7 +17,7 @@ import OrderedCollections * | Enum Value | .string | * */ -func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: Map] = [:]) throws -> Map? { +func valueFromAST(valueAST: Value, type: GraphQLInputType, variables: [String: Map] = [:]) throws -> Map { if let nonNullType = type as? GraphQLNonNull { // Note: we're not checking that the result of valueFromAST is non-null. // We're assuming that this query has been validated and the value used @@ -25,10 +25,6 @@ func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: return try valueFromAST(valueAST: valueAST, type: nonNullType.ofType as! GraphQLInputType, variables: variables) } - guard let valueAST = valueAST else { - return nil - } - if let variable = valueAST as? Variable { let variableName = variable.name.value @@ -38,7 +34,11 @@ func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: // Note: we're not doing any checking that this variable is correct. We're // assuming that this query has been validated and the variable usage here // is of the correct type. - return variables[variableName] + if let variable = variables[variableName] { + return variable + } else { + return .null + } } if let list = type as? GraphQLList { @@ -50,41 +50,43 @@ func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: valueAST: $0, type: itemType as! GraphQLInputType, variables: variables - )! + ) })) } - return try [valueFromAST(valueAST: valueAST, type: itemType as! GraphQLInputType, variables: variables)!] + return try [valueFromAST(valueAST: valueAST, type: itemType as! GraphQLInputType, variables: variables)] } if let objectType = type as? GraphQLInputObjectType { guard let objectValue = valueAST as? ObjectValue else { - return nil + throw GraphQLError(message: "Must be object type") } let fields = objectType.fields - let fieldASTs = objectValue.fields.keyMap({ $0.name.value }) - return try .dictionary(fields.keys.reduce([:] as OrderedDictionary) { obj, fieldName in + return try .dictionary(fields.keys.reduce(OrderedDictionary()) { obj, fieldName in var obj = obj - let field = fields[fieldName] + let field = fields[fieldName]! let fieldAST = fieldASTs[fieldName] - guard fieldAST != nil else { - obj[fieldName] = .undefined - return obj - } - - var fieldValue = try valueFromAST( - valueAST: fieldAST?.value, - type: field!.type, - variables: variables - ) + if let fieldAST = fieldAST { + let fieldValue = try valueFromAST( + valueAST: fieldAST.value, + type: field.type, + variables: variables + ) - if fieldValue == .null { - fieldValue = field.flatMap({ $0.defaultValue.map({ .string($0) }) }) + if fieldValue == .null { + if let defaultValue = field.defaultValue { + obj[fieldName] = .string(defaultValue) + } else { + obj[fieldName] = .null + } + } else { + obj[fieldName] = fieldValue + } } else { - obj[fieldName] = fieldValue + obj[fieldName] = .undefined } return obj @@ -97,9 +99,5 @@ func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: let parsed = try type.parseLiteral(valueAST: valueAST) - guard parsed != .null else { - return nil - } - return parsed } From 707f61f4802f8120829318a0d888dd9fb7abb06e Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 20 Sep 2021 10:15:56 -0600 Subject: [PATCH 3/7] Fixes default values and adds tests --- Sources/GraphQL/Execution/Values.swift | 2 +- Sources/GraphQL/Type/Definition.swift | 11 +- Sources/GraphQL/Utilities/ValueFromAST.swift | 15 +- .../GraphQLTests/InputTests/InputTests.swift | 387 +++++++++++++++--- 4 files changed, 339 insertions(+), 76 deletions(-) diff --git a/Sources/GraphQL/Execution/Values.swift b/Sources/GraphQL/Execution/Values.swift index f703e20c..29eb15e2 100644 --- a/Sources/GraphQL/Execution/Values.swift +++ b/Sources/GraphQL/Execution/Values.swift @@ -152,7 +152,7 @@ func coerceValue(type: GraphQLInputType, value: Map) throws -> Map? { var fieldValue = try coerceValue(type: field!.type, value: value[fieldName] ?? .null) if fieldValue == .null { - fieldValue = field.flatMap({ $0.defaultValue.map({ .string($0) }) }) + fieldValue = field.flatMap({ $0.defaultValue }) } else { objCopy[fieldName] = fieldValue } diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index aa735b38..b4157889 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1,3 +1,4 @@ +import OrderedCollections import Foundation import NIO @@ -1274,12 +1275,12 @@ func defineInputObjectFieldMap( public struct InputObjectField { public let type: GraphQLInputType - public let defaultValue: String? + public let defaultValue: Map? public let description: String? public init(type: GraphQLInputType, defaultValue: Map? = nil, description: String? = nil) { self.type = type - self.defaultValue = defaultValue?.description + self.defaultValue = defaultValue self.description = description } } @@ -1290,13 +1291,13 @@ public final class InputObjectFieldDefinition { public let name: String public internal(set) var type: GraphQLInputType public let description: String? - public let defaultValue: String? + public let defaultValue: Map? init( name: String, type: GraphQLInputType, description: String? = nil, - defaultValue: String? = nil + defaultValue: Map? = nil ) { self.name = name self.type = type @@ -1351,7 +1352,7 @@ extension InputObjectFieldDefinition : KeySubscriptable { } } -public typealias InputObjectFieldDefinitionMap = [String: InputObjectFieldDefinition] +public typealias InputObjectFieldDefinitionMap = OrderedDictionary /** * List Modifier diff --git a/Sources/GraphQL/Utilities/ValueFromAST.swift b/Sources/GraphQL/Utilities/ValueFromAST.swift index 5d98ea73..35aa641a 100644 --- a/Sources/GraphQL/Utilities/ValueFromAST.swift +++ b/Sources/GraphQL/Utilities/ValueFromAST.swift @@ -75,18 +75,13 @@ func valueFromAST(valueAST: Value, type: GraphQLInputType, variables: [String: M type: field.type, variables: variables ) - - if fieldValue == .null { - if let defaultValue = field.defaultValue { - obj[fieldName] = .string(defaultValue) - } else { - obj[fieldName] = .null - } + obj[fieldName] = fieldValue + } else { + if let defaultValue = field.defaultValue { + obj[fieldName] = defaultValue } else { - obj[fieldName] = fieldValue + obj[fieldName] = .undefined } - } else { - obj[fieldName] = .undefined } return obj diff --git a/Tests/GraphQLTests/InputTests/InputTests.swift b/Tests/GraphQLTests/InputTests/InputTests.swift index 641cb2cd..68fc4ebd 100644 --- a/Tests/GraphQLTests/InputTests/InputTests.swift +++ b/Tests/GraphQLTests/InputTests/InputTests.swift @@ -2,69 +2,85 @@ import XCTest import NIO @testable import GraphQL -fileprivate struct Echo { - let field1: String? - let field2: String? -} - -fileprivate let EchoInputType = try! GraphQLInputObjectType( - name: "EchoInput", - fields: [ - "field1": InputObjectField( - type: GraphQLString - ), - "field2": InputObjectField( - type: GraphQLString - ), - ] -) - -fileprivate let EchoOutputType = try! GraphQLObjectType( - name: "Echo", - description: "", - fields: [ - "field1": GraphQLField( - type: GraphQLString - ), - "field2": GraphQLField( - type: GraphQLString - ), - ], - isTypeOf: { source, _, _ in - source is Echo - } -) class InputTests : XCTestCase { - let schema = try! GraphQLSchema( - query: try! GraphQLObjectType( - name: "Query", + func testInputParsing() throws { + struct Echo : Codable { + let field1: String? + let field2: String? + + init(field1: String?, field2: String?) { + self.field1 = field1 + self.field2 = field2 + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + XCTAssertTrue(container.contains(.field1)) + field1 = try container.decodeIfPresent(String.self, forKey: .field1) + XCTAssertTrue(container.contains(.field2)) + field2 = try container.decodeIfPresent(String.self, forKey: .field2) + } + } + + struct EchoArgs : Codable { + let input: Echo + } + + let EchoInputType = try! GraphQLInputObjectType( + name: "EchoInput", fields: [ - "echo": GraphQLField( - type: EchoOutputType, - args: [ - "input": GraphQLArgument( - type: EchoInputType - ) - ], - resolve: { _, arguments, _, _ in - let input = arguments["input"] - print(input["field2"]) - return Echo( - field1: input["field1"].string, - field2: input["field2"].string - ) - } + "field1": InputObjectField( + type: GraphQLString + ), + "field2": InputObjectField( + type: GraphQLString ), ] - ), - types: [EchoInputType, EchoOutputType] - ) - - func testBasic() throws { - let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + ) + + let EchoOutputType = try! GraphQLObjectType( + name: "Echo", + description: "", + fields: [ + "field1": GraphQLField( + type: GraphQLString + ), + "field2": GraphQLField( + type: GraphQLString + ), + ], + isTypeOf: { source, _, _ in + source is Echo + } + ) + + let schema = try! GraphQLSchema( + query: try! GraphQLObjectType( + name: "Query", + fields: [ + "echo": GraphQLField( + type: EchoOutputType, + args: [ + "input": GraphQLArgument( + type: EchoInputType + ) + ], + resolve: { _, arguments, _, _ in + let args = try MapDecoder().decode(EchoArgs.self, from: arguments) + return Echo( + field1: args.input.field1, + field2: args.input.field2 + ) + } + ), + ] + ), + types: [EchoInputType, EchoOutputType] + ) + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) defer { XCTAssertNoThrow(try group.syncShutdownGracefully()) } @@ -94,9 +110,82 @@ class InputTests : XCTestCase { ) } - func testIncludedNull() throws { - let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + func testInputParsingDefinedNull() throws { + struct Echo : Codable { + let field1: String? + let field2: String? + + init(field1: String?, field2: String?) { + self.field1 = field1 + self.field2 = field2 + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + XCTAssertTrue(container.contains(.field1)) + field1 = try container.decodeIfPresent(String.self, forKey: .field1) + XCTAssertTrue(container.contains(.field2)) + field2 = try container.decodeIfPresent(String.self, forKey: .field2) + } + } + + struct EchoArgs : Codable { + let input: Echo + } + + let EchoInputType = try! GraphQLInputObjectType( + name: "EchoInput", + fields: [ + "field1": InputObjectField( + type: GraphQLString + ), + "field2": InputObjectField( + type: GraphQLString + ), + ] + ) + + let EchoOutputType = try! GraphQLObjectType( + name: "Echo", + description: "", + fields: [ + "field1": GraphQLField( + type: GraphQLString + ), + "field2": GraphQLField( + type: GraphQLString + ), + ], + isTypeOf: { source, _, _ in + source is Echo + } + ) + let schema = try! GraphQLSchema( + query: try! GraphQLObjectType( + name: "Query", + fields: [ + "echo": GraphQLField( + type: EchoOutputType, + args: [ + "input": GraphQLArgument( + type: EchoInputType + ) + ], + resolve: { _, arguments, _, _ in + let args = try MapDecoder().decode(EchoArgs.self, from: arguments) + return Echo( + field1: args.input.field1, + field2: args.input.field2 + ) + } + ), + ] + ), + types: [EchoInputType, EchoOutputType] + ) + + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) defer { XCTAssertNoThrow(try group.syncShutdownGracefully()) } @@ -126,9 +215,82 @@ class InputTests : XCTestCase { ) } - func testImpliedNull() throws { - let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + func testInputParsingUndefined() throws { + struct Echo : Codable { + let field1: String? + let field2: String? + + init(field1: String?, field2: String?) { + self.field1 = field1 + self.field2 = field2 + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + XCTAssertTrue(container.contains(.field1)) + field1 = try container.decodeIfPresent(String.self, forKey: .field1) + XCTAssertFalse(container.contains(.field2)) // Container should not include field2, since it is undefined + field2 = try container.decodeIfPresent(String.self, forKey: .field2) + } + } + + struct EchoArgs : Codable { + let input: Echo + } + + let EchoInputType = try! GraphQLInputObjectType( + name: "EchoInput", + fields: [ + "field1": InputObjectField( + type: GraphQLString + ), + "field2": InputObjectField( + type: GraphQLString + ), + ] + ) + + let EchoOutputType = try! GraphQLObjectType( + name: "Echo", + description: "", + fields: [ + "field1": GraphQLField( + type: GraphQLString + ), + "field2": GraphQLField( + type: GraphQLString + ), + ], + isTypeOf: { source, _, _ in + source is Echo + } + ) + let schema = try! GraphQLSchema( + query: try! GraphQLObjectType( + name: "Query", + fields: [ + "echo": GraphQLField( + type: EchoOutputType, + args: [ + "input": GraphQLArgument( + type: EchoInputType + ) + ], + resolve: { _, arguments, _, _ in + let args = try MapDecoder().decode(EchoArgs.self, from: arguments) + return Echo( + field1: args.input.field1, + field2: args.input.field2 + ) + } + ), + ] + ), + types: [EchoInputType, EchoOutputType] + ) + + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) defer { XCTAssertNoThrow(try group.syncShutdownGracefully()) } @@ -156,4 +318,109 @@ class InputTests : XCTestCase { ]) ) } + + func testInputParsingUndefinedWithDefault() throws { + struct Echo : Codable { + let field1: String? + let field2: String? + + init(field1: String?, field2: String?) { + self.field1 = field1 + self.field2 = field2 + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + XCTAssertTrue(container.contains(.field1)) + field1 = try container.decodeIfPresent(String.self, forKey: .field1) + XCTAssertTrue(container.contains(.field2)) // default value should be used + field2 = try container.decodeIfPresent(String.self, forKey: .field2) + } + } + + struct EchoArgs : Codable { + let input: Echo + } + + let EchoInputType = try! GraphQLInputObjectType( + name: "EchoInput", + fields: [ + "field1": InputObjectField( + type: GraphQLString + ), + "field2": InputObjectField( + type: GraphQLString, + defaultValue: .string("value2") + ), + ] + ) + + let EchoOutputType = try! GraphQLObjectType( + name: "Echo", + description: "", + fields: [ + "field1": GraphQLField( + type: GraphQLString + ), + "field2": GraphQLField( + type: GraphQLString + ), + ], + isTypeOf: { source, _, _ in + source is Echo + } + ) + + let schema = try! GraphQLSchema( + query: try! GraphQLObjectType( + name: "Query", + fields: [ + "echo": GraphQLField( + type: EchoOutputType, + args: [ + "input": GraphQLArgument( + type: EchoInputType + ) + ], + resolve: { _, arguments, _, _ in + let args = try MapDecoder().decode(EchoArgs.self, from: arguments) + return Echo( + field1: args.input.field1, + field2: args.input.field2 + ) + } + ), + ] + ), + types: [EchoInputType, EchoOutputType] + ) + + let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1" + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": "value2", + ] + ]) + ) + } } From 2dd8a863b7ed8987d5f9ba94bab018f0a4f029fb Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 20 Sep 2021 10:26:47 -0600 Subject: [PATCH 4/7] Adds map tests to ensure encoding/decoding --- Sources/GraphQL/Type/Definition.swift | 2 +- Tests/GraphQLTests/MapTests/MapTests.swift | 108 ++++++++++++++++++++- 2 files changed, 105 insertions(+), 5 deletions(-) diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index b4157889..0de3ab27 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1352,7 +1352,7 @@ extension InputObjectFieldDefinition : KeySubscriptable { } } -public typealias InputObjectFieldDefinitionMap = OrderedDictionary +public typealias InputObjectFieldDefinitionMap = [String: InputObjectFieldDefinition] /** * List Modifier diff --git a/Tests/GraphQLTests/MapTests/MapTests.swift b/Tests/GraphQLTests/MapTests/MapTests.swift index 5110b29e..444d59bc 100644 --- a/Tests/GraphQLTests/MapTests/MapTests.swift +++ b/Tests/GraphQLTests/MapTests/MapTests.swift @@ -8,7 +8,6 @@ class MapTests: XCTestCase { XCTAssertEqual(try Map.bool(false).boolValue(), false) XCTAssertEqual(try Map.bool(true).boolValue(), true) XCTAssertEqual(try Map.string("Hello world").stringValue(), "Hello world") - } func testOptionalConversion() { @@ -36,16 +35,117 @@ class MapTests: XCTestCase { [ "first": .number(1), "second": .number(4), - "third": .number(9) + "third": .number(9), + "fourth": .null, + "fifth": .undefined ] ) - XCTAssertEqual(map.dictionary?.count, 3) + XCTAssertEqual(map.dictionary?.count, 5) let dictionary = try map.dictionaryValue() - XCTAssertEqual(dictionary.count, 3) + XCTAssertEqual(dictionary.count, 5) XCTAssertEqual(try dictionary["first"]?.intValue(), 1) XCTAssertEqual(try dictionary["second"]?.intValue(), 4) XCTAssertEqual(try dictionary["third"]?.intValue(), 9) + XCTAssertEqual(dictionary["fourth"]?.isNull, true) + XCTAssertEqual(dictionary["fifth"]?.isUndefined, true) + } + + // Ensure that default decoding preserves undefined becoming nil + func testNilAndUndefinedDecodeToNilByDefault() throws { + struct DecodableTest : Codable { + let first: Int? + let second: Int? + let third: Int? + let fourth: Int? + } + + let map = Map.dictionary( + [ + "first": .number(1), + "second": .null, + "third": .undefined + // fourth not included + ] + ) + + let decodable = try MapDecoder().decode(DecodableTest.self, from: map) + XCTAssertEqual(decodable.first, 1) + XCTAssertEqual(decodable.second, nil) + XCTAssertEqual(decodable.third, nil) + XCTAssertEqual(decodable.fourth, nil) + } + + // Ensure that, if custom decoding is defined, provided nulls and unset values can be differentiated. + // This should match JSON in that values set to `null` should be 'contained' by the container, but + // values expected by the result that are undefined or not present should not be. + func testNilAndUndefinedDecoding() throws { + struct DecodableTest : Codable { + let first: Int? + let second: Int? + let third: Int? + let fourth: Int? + + init( + first: Int?, + second: Int?, + third: Int?, + fourth: Int? + ) { + self.first = first + self.second = second + self.third = third + self.fourth = fourth + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + + XCTAssertTrue(container.contains(.first)) + // Null value should be contained, but decode to nil + XCTAssertTrue(container.contains(.second)) + // Undefined value should not be contained + XCTAssertFalse(container.contains(.third)) + // Missing value should operate the same as undefined + XCTAssertFalse(container.contains(.fourth)) + + first = try container.decodeIfPresent(Int.self, forKey: .first) + second = try container.decodeIfPresent(Int.self, forKey: .second) + third = try container.decodeIfPresent(Int.self, forKey: .third) + fourth = try container.decodeIfPresent(Int.self, forKey: .fourth) + } + } + + let map = Map.dictionary( + [ + "first": .number(1), + "second": .null, + "third": .undefined + // fourth not included + ] + ) + + _ = try MapDecoder().decode(DecodableTest.self, from: map) + } + + // Ensure that map encoding includes defined nulls, but skips undefined values + func testMapEncoding() throws { + let map = Map.dictionary( + [ + "first": .number(1), + "second": .null, + "third": .undefined + ] + ) + + let data = try JSONEncoder().encode(map) + let json = String(data: data, encoding: .utf8) + XCTAssertEqual( + json, + """ + {"first":1,"second":null} + """ + ) } } From 9f6320429ddb34fc9a11a7d9b67a1a633b2a82c4 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 20 Sep 2021 10:41:30 -0600 Subject: [PATCH 5/7] Cleanup and documentation --- Sources/GraphQL/Utilities/ValueFromAST.swift | 9 ++++----- Tests/GraphQLTests/InputTests/InputTests.swift | 4 ++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Sources/GraphQL/Utilities/ValueFromAST.swift b/Sources/GraphQL/Utilities/ValueFromAST.swift index 35aa641a..811722a6 100644 --- a/Sources/GraphQL/Utilities/ValueFromAST.swift +++ b/Sources/GraphQL/Utilities/ValueFromAST.swift @@ -68,8 +68,7 @@ func valueFromAST(valueAST: Value, type: GraphQLInputType, variables: [String: M return try .dictionary(fields.keys.reduce(OrderedDictionary()) { obj, fieldName in var obj = obj let field = fields[fieldName]! - let fieldAST = fieldASTs[fieldName] - if let fieldAST = fieldAST { + if let fieldAST = fieldASTs[fieldName] { let fieldValue = try valueFromAST( valueAST: fieldAST.value, type: field.type, @@ -77,6 +76,7 @@ func valueFromAST(valueAST: Value, type: GraphQLInputType, variables: [String: M ) obj[fieldName] = fieldValue } else { + // If AST doesn't contain field, it is undefined if let defaultValue = field.defaultValue { obj[fieldName] = defaultValue } else { @@ -92,7 +92,6 @@ func valueFromAST(valueAST: Value, type: GraphQLInputType, variables: [String: M throw GraphQLError(message: "Must be leaf type") } - let parsed = try type.parseLiteral(valueAST: valueAST) - - return parsed + // If we've made it this far, it should be a literal + return try type.parseLiteral(valueAST: valueAST) } diff --git a/Tests/GraphQLTests/InputTests/InputTests.swift b/Tests/GraphQLTests/InputTests/InputTests.swift index 68fc4ebd..c7d9f805 100644 --- a/Tests/GraphQLTests/InputTests/InputTests.swift +++ b/Tests/GraphQLTests/InputTests/InputTests.swift @@ -5,6 +5,7 @@ import NIO class InputTests : XCTestCase { + // Test that input objects parse as expected from non-null literals func testInputParsing() throws { struct Echo : Codable { let field1: String? @@ -110,6 +111,7 @@ class InputTests : XCTestCase { ) } + // Test that inputs parse as expected when null literals are present func testInputParsingDefinedNull() throws { struct Echo : Codable { let field1: String? @@ -215,6 +217,7 @@ class InputTests : XCTestCase { ) } + // Test that input objects parse as expected when there are missing fields with no default func testInputParsingUndefined() throws { struct Echo : Codable { let field1: String? @@ -319,6 +322,7 @@ class InputTests : XCTestCase { ) } + // Test that input objects parse as expected when there are missing fields with defaults func testInputParsingUndefinedWithDefault() throws { struct Echo : Codable { let field1: String? From 42bc0b08bee0e66c27bcbdb64b2a78b69a1ba8c6 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Tue, 21 Sep 2021 14:20:00 -0600 Subject: [PATCH 6/7] Removes unused import --- Sources/GraphQL/Type/Definition.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index 0de3ab27..57649903 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1,4 +1,3 @@ -import OrderedCollections import Foundation import NIO From 0a47a82ab23c17c5b15a5c248a94bd74670a670f Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Tue, 21 Sep 2021 16:26:44 -0600 Subject: [PATCH 7/7] Deletes ubuntu-16.04 from CI b/c it's deprecated For more details, see: https://github.com/actions/virtual-environments/issues/3287 --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 993616f2..3f37c124 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [macos-10.15, macos-latest, ubuntu-16.04, ubuntu-18.04, ubuntu-20.04] + os: [macos-10.15, macos-latest, ubuntu-18.04, ubuntu-20.04] steps: - uses: actions/checkout@v2 - name: Set code coverage path