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 diff --git a/Sources/GraphQL/Execution/Values.swift b/Sources/GraphQL/Execution/Values.swift index 3fc34bc7..29eb15e2 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 @@ -148,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/Map/Map.swift b/Sources/GraphQL/Map/Map.swift index 8ea1690c..f185889b 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): @@ -891,8 +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}) { + for (key, value) in filtered.sorted(by: {$0.0 < $1.0}) { if debug { string += "\n" string += indent() @@ -901,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/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/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index aa735b38..57649903 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1274,12 +1274,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 +1290,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 diff --git a/Sources/GraphQL/Utilities/ValueFromAST.swift b/Sources/GraphQL/Utilities/ValueFromAST.swift index 65474fc0..811722a6 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,36 +50,38 @@ 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 fieldAST = fieldASTs[fieldName] - var fieldValue = try valueFromAST( - valueAST: fieldAST?.value, - type: field!.type, - variables: variables - ) - - if fieldValue == .null { - fieldValue = field.flatMap({ $0.defaultValue.map({ .string($0) }) }) - } else { + let field = fields[fieldName]! + if let fieldAST = fieldASTs[fieldName] { + let fieldValue = try valueFromAST( + valueAST: fieldAST.value, + type: field.type, + variables: variables + ) obj[fieldName] = fieldValue + } else { + // If AST doesn't contain field, it is undefined + if let defaultValue = field.defaultValue { + obj[fieldName] = defaultValue + } else { + obj[fieldName] = .undefined + } } return obj @@ -90,11 +92,6 @@ func valueFromAST(valueAST: Value?, type: GraphQLInputType, variables: [String: throw GraphQLError(message: "Must be leaf type") } - let parsed = try type.parseLiteral(valueAST: valueAST) - - guard parsed != .null else { - return nil - } - - 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 new file mode 100644 index 00000000..c7d9f805 --- /dev/null +++ b/Tests/GraphQLTests/InputTests/InputTests.swift @@ -0,0 +1,430 @@ +import XCTest +import NIO +@testable import GraphQL + + +class InputTests : XCTestCase { + + // Test that input objects parse as expected from non-null literals + 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: [ + "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()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1", + field2: "value2", + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": "value2", + ] + ]) + ) + } + + // Test that inputs parse as expected when null literals are present + 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()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1", + field2: null, + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": nil, + ] + ]) + ) + } + + // Test that input objects parse as expected when there are missing fields with no default + 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()) + } + + XCTAssertEqual( + try graphql( + schema: schema, + request: """ + { + echo(input:{ + field1: "value1" + }) { + field1 + field2 + } + } + """, + eventLoopGroup: group + ).wait(), + GraphQLResult(data: [ + "echo": [ + "field1": "value1", + "field2": nil, + ] + ]) + ) + } + + // Test that input objects parse as expected when there are missing fields with defaults + 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", + ] + ]) + ) + } +} 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} + """ + ) } }