Merge pull request #356 from brandonbloom/nullable-decoding

mattpolzin · web-flow · commit 33a9984b4af0 · 2024-02-24T15:29:22.000-06:00
fix "nullable" decoding
diff --git a/Sources/OpenAPIKit/Schema Object/JSONSchema.swift b/Sources/OpenAPIKit/Schema Object/JSONSchema.swift
@@ -1807,16 +1807,20 @@ extension JSONSchema: Decodable {
 
         if let ref = try? JSONReference<JSONSchema>(from: decoder) {
             let coreContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
-            self = .reference(ref, coreContext)
+            self = .init(warnings: coreContext.warnings, schema: .reference(ref, coreContext))
             return
         }
 
         let container = try decoder.container(keyedBy: SubschemaCodingKeys.self)
 
         if container.contains(.allOf) {
-            var schema: JSONSchema = .all(
-                of: try container.decode([JSONSchema].self, forKey: .allOf),
-                core: try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            let coreContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            var schema: JSONSchema = .init(
+                warnings: coreContext.warnings,
+                schema: .all(
+                    of: try container.decode([JSONSchema].self, forKey: .allOf),
+                    core: coreContext
+                )
             )
             if schema.subschemas.contains(where: { $0.nullable }) {
                 schema = schema.nullableSchemaObject()
@@ -1827,9 +1831,13 @@ extension JSONSchema: Decodable {
         }
 
         if container.contains(.anyOf) {
-            var schema: JSONSchema = .any(
-                of: try container.decode([JSONSchema].self, forKey: .anyOf),
-                core: try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            let coreContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            var schema: JSONSchema = .init(
+                warnings: coreContext.warnings,
+                schema: .any(
+                    of: try container.decode([JSONSchema].self, forKey: .anyOf),
+                    core: coreContext
+                )
             )
             if schema.subschemas.contains(where: { $0.nullable }) {
                 schema = schema.nullableSchemaObject()
@@ -1840,9 +1848,12 @@ extension JSONSchema: Decodable {
         }
 
         if container.contains(.oneOf) {
-            var schema: JSONSchema = .one(
-                of: try container.decode([JSONSchema].self, forKey: .oneOf),
-                core: try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            let coreContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            var schema: JSONSchema = .init(warnings: coreContext.warnings,
+                schema: .one(
+                    of: try container.decode([JSONSchema].self, forKey: .oneOf),
+                    core: coreContext
+                )
             )
             if schema.subschemas.contains(where: { $0.nullable }) {
                 schema = schema.nullableSchemaObject()
@@ -1853,9 +1864,12 @@ extension JSONSchema: Decodable {
         }
 
         if container.contains(.not) {
-            let schema: JSONSchema = .not(
-                try container.decode(JSONSchema.self, forKey: .not),
-                core: try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            let coreContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            let schema: JSONSchema = .init(warnings: coreContext.warnings,
+                schema: .not(
+                    try container.decode(JSONSchema.self, forKey: .not),
+                    core: coreContext
+                )
             )
             
             self = schema
@@ -1915,34 +1929,48 @@ extension JSONSchema: Decodable {
         let value: Schema
         if typeHint == .null {
             let coreContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            _warnings += coreContext.warnings
             value = .null(coreContext)
 
         } else if typeHint == .integer || typeHint == .number || (typeHint == nil && !numericOrIntegerContainer.allKeys.isEmpty) {
             if typeHint == .integer {
-                value = .integer(try CoreContext<JSONTypeFormat.IntegerFormat>(from: decoder),
+                let coreContext = try CoreContext<JSONTypeFormat.IntegerFormat>(from: decoder)
+                _warnings += coreContext.warnings
+                value = .integer(coreContext,
                                 try IntegerContext(from: decoder))
             } else {
-                value = .number(try CoreContext<JSONTypeFormat.NumberFormat>(from: decoder),
+                let coreContext = try CoreContext<JSONTypeFormat.NumberFormat>(from: decoder)
+                _warnings += coreContext.warnings
+                value = .number(coreContext,
                                try NumericContext(from: decoder))
             }
 
         } else if typeHint == .string || (typeHint == nil && !stringContainer.allKeys.isEmpty) {
-            value = .string(try CoreContext<JSONTypeFormat.StringFormat>(from: decoder),
+            let coreContext = try CoreContext<JSONTypeFormat.StringFormat>(from: decoder)
+            _warnings += coreContext.warnings
+            value = .string(coreContext,
                            try StringContext(from: decoder))
 
         } else if typeHint == .array || (typeHint == nil && !arrayContainer.allKeys.isEmpty) {
-            value = .array(try CoreContext<JSONTypeFormat.ArrayFormat>(from: decoder),
+            let coreContext = try CoreContext<JSONTypeFormat.ArrayFormat>(from: decoder)
+            _warnings += coreContext.warnings
+            value = .array(coreContext,
                           try ArrayContext(from: decoder))
 
         } else if typeHint == .object || (typeHint == nil && !objectContainer.allKeys.isEmpty) {
-            value = .object(try CoreContext<JSONTypeFormat.ObjectFormat>(from: decoder),
+            let coreContext = try CoreContext<JSONTypeFormat.ObjectFormat>(from: decoder)
+            _warnings += coreContext.warnings
+            value = .object(coreContext,
                            try ObjectContext(from: decoder))
 
         } else if typeHint == .boolean {
-            value = .boolean(try CoreContext<JSONTypeFormat.BooleanFormat>(from: decoder))
+            let coreContext = try CoreContext<JSONTypeFormat.BooleanFormat>(from: decoder)
+            _warnings += coreContext.warnings
+            value = .boolean(coreContext)
 
         } else {
             let fragmentContext = try CoreContext<JSONTypeFormat.AnyFormat>(from: decoder)
+            _warnings += fragmentContext.warnings
             if fragmentContext.isEmpty && hintContainerCount > 0 {
                 _warnings.append(
                     .underlyingError(
diff --git a/Sources/OpenAPIKit/Schema Object/JSONSchemaContext.swift b/Sources/OpenAPIKit/Schema Object/JSONSchemaContext.swift
@@ -135,7 +135,9 @@ extension JSONSchemaContext {
 
 extension JSONSchema {
     /// The context that applies to all schemas.
-    public struct CoreContext<Format: OpenAPIFormat>: JSONSchemaContext, Equatable {
+    public struct CoreContext<Format: OpenAPIFormat>: JSONSchemaContext, HasWarnings {
+        public let warnings: [OpenAPI.Warning]
+
         public let format: Format
         public let required: Bool // default true
         public let nullable: Bool // default false
@@ -229,6 +231,7 @@ extension JSONSchema {
             vendorExtensions: [String: AnyCodable] = [:],
             _inferred: Bool = false
         ) {
+            self.warnings = []
             self.format = format
             self.required = required
             self.nullable = nullable
@@ -260,6 +263,7 @@ extension JSONSchema {
             examples: [String],
             vendorExtensions: [String: AnyCodable] = [:]
         ) {
+            self.warnings = []
             self.format = format
             self.required = required
             self.nullable = nullable
@@ -278,6 +282,24 @@ extension JSONSchema {
     }
 }
 
+extension JSONSchema.CoreContext: Equatable {
+    public static func == (lhs: JSONSchema.CoreContext<Format>, rhs: JSONSchema.CoreContext<Format>) -> Bool {
+        lhs.format == rhs.format
+        && lhs.required == rhs.required
+        && lhs.nullable == rhs.nullable
+        && lhs._permissions == rhs._permissions
+        && lhs._deprecated == rhs._deprecated
+        && lhs.title == rhs.title
+        && lhs.description == rhs.description
+        && lhs.externalDocs == rhs.externalDocs
+        && lhs.discriminator == rhs.discriminator
+        && lhs.allowedValues == rhs.allowedValues
+        && lhs.defaultValue == rhs.defaultValue
+        && lhs.vendorExtensions == rhs.vendorExtensions
+        && lhs.inferred == rhs.inferred
+    }
+}
+
 // MARK: - Transformations
 
 extension JSONSchema.CoreContext {
@@ -768,6 +790,7 @@ extension JSONSchema {
     // not nested because Context is a generic type
     internal enum ContextCodingKeys: String, CodingKey {
         case type
+        case nullable
         case format
         case title
         case description
@@ -849,12 +872,15 @@ extension JSONSchema.CoreContext: Encodable {
 
 extension JSONSchema.CoreContext: Decodable {
     public init(from decoder: Decoder) throws {
+        var warnings: [OpenAPI.Warning] = []
+
         let container = try decoder.container(keyedBy: JSONSchema.ContextCodingKeys.self)
 
         format = try container.decodeIfPresent(Format.self, forKey: .format) ?? .unspecified
 
-        let nullable = try Self.decodeNullable(from: container)
+        let (nullable, nullableWarnings) = try Self.decodeNullable(from: container)
         self.nullable = nullable
+        warnings += nullableWarnings
 
         // default to `true` at decoding site.
         // It is the responsibility of decoders farther upstream
@@ -913,6 +939,8 @@ extension JSONSchema.CoreContext: Decodable {
         // full JSON Schema.
         vendorExtensions = [:]
         inferred = false
+
+        self.warnings = warnings
     }
 
     /// Support both `enum` and `const` when decoding allowed values for the schema.
@@ -927,14 +955,32 @@ extension JSONSchema.CoreContext: Decodable {
     }
 
     /// Decode whether or not this is a nullable JSONSchema.
-    private static func decodeNullable(from container: KeyedDecodingContainer<JSONSchema.ContextCodingKeys>) throws -> Bool {
-        if let types = try? container.decodeIfPresent([JSONType].self, forKey: .type) {
-            return types.contains(JSONType.null)
+    private static func decodeNullable(from container: KeyedDecodingContainer<JSONSchema.ContextCodingKeys>) throws -> (Bool, [OpenAPI.Warning]) {
+        let nullable: Bool
+        var warnings: [OpenAPI.Warning] = []
+
+        if let _nullable = try? container.decodeIfPresent(Bool.self, forKey: .nullable) {
+            nullable = _nullable
+            warnings.append(
+                .underlyingError(
+                      InconsistencyError(
+                          subjectName: "OpenAPI Schema",
+                          details: "Found 'nullable' property. This property is not supported by OpenAPI v3.1.0. OpenAPIKit has translated it into 'type: [\"null\", ...]'.",
+                          codingPath: container.codingPath
+                      )
+                  )
+            )
+            
         }
-        if let type = try? container.decodeIfPresent(JSONType.self, forKey: .type) {
-            return type == JSONType.null
+        else if let types = try? container.decodeIfPresent([JSONType].self, forKey: .type) {
+            nullable = types.contains(JSONType.null)
+        }
+        else if let type = try? container.decodeIfPresent(JSONType.self, forKey: .type) {
+            nullable = type == JSONType.null
+        } else {
+          nullable = false
         }
-        return false
+        return (nullable, warnings)
     }
 }
 
diff --git a/Tests/OpenAPIKitErrorReportingTests/SchemaErrorTests.swift b/Tests/OpenAPIKitErrorReportingTests/SchemaErrorTests.swift
@@ -48,4 +48,46 @@ final class SchemaErrorTests: XCTestCase {
             ])
         }
     }
+
+    func test_nullablePropertyInsteadOfNullType() throws {
+        let documentYML =
+        """
+        openapi: "3.1.0"
+        info:
+            title: test
+            version: 1.0
+        paths:
+            /hello/world:
+                get:
+                    responses:
+                        '200':
+                            description: hello
+                            content:
+                                'application/json':
+                                    schema:
+                                        type: integer
+                                        nullable: true
+        """
+
+        let document = try testDecoder.decode(OpenAPI.Document.self, from: documentYML)
+        XCTAssertThrowsError(try document.validate()) { error in
+
+            let openAPIError = OpenAPI.Error(from: error)
+
+            XCTAssertEqual(openAPIError.localizedDescription,
+            """
+            Inconsistency encountered when parsing `OpenAPI Schema`: Found 'nullable' property. This property is not supported by OpenAPI v3.1.0. OpenAPIKit has translated it into 'type: ["null", ...]'.. at path: .paths['/hello/world'].get.responses.200.content['application/json'].schema
+            """)
+            XCTAssertEqual(openAPIError.codingPath.map { $0.stringValue }, [
+                "paths",
+                "/hello/world",
+                "get",
+                "responses",
+                "200",
+                "content",
+                "application/json",
+                "schema"
+            ])
+        }
+    }
 }
diff --git a/Tests/OpenAPIKitTests/Schema Object/JSONSchemaTests.swift b/Tests/OpenAPIKitTests/Schema Object/JSONSchemaTests.swift
@@ -1933,7 +1933,8 @@ extension SchemaObjectTests {
 
     func test_decodeBoolean() throws {
         let booleanData = #"{"type": "boolean"}"#.data(using: .utf8)!
-        let nullableBooleanData = #"{"type": ["boolean", "null"]}"#.data(using: .utf8)!
+        let booleanOrNullData = #"{"type": ["boolean", "null"]}"#.data(using: .utf8)!
+        let nullableBooleanData = #"{"type": "boolean", "nullable": true}"#.data(using: .utf8)!
         let readOnlyBooleanData = #"{"type": "boolean", "readOnly": true}"#.data(using: .utf8)!
         let writeOnlyBooleanData = #"{"type": "boolean", "writeOnly": true}"#.data(using: .utf8)!
         let deprecatedBooleanData = #"{"type": "boolean", "deprecated": true}"#.data(using: .utf8)!
@@ -1943,6 +1944,7 @@ extension SchemaObjectTests {
         let discriminatorBooleanData = #"{"type": "boolean", "discriminator": { "propertyName": "hello" }}"#.data(using: .utf8)!
 
         let boolean = try orderUnstableDecode(JSONSchema.self, from: booleanData)
+        let booleanOrNull = try orderUnstableDecode(JSONSchema.self, from: booleanOrNullData)
         let nullableBoolean = try orderUnstableDecode(JSONSchema.self, from: nullableBooleanData)
         let readOnlyBoolean = try orderUnstableDecode(JSONSchema.self, from: readOnlyBooleanData)
         let writeOnlyBoolean = try orderUnstableDecode(JSONSchema.self, from: writeOnlyBooleanData)
@@ -1953,6 +1955,7 @@ extension SchemaObjectTests {
         let discriminatorBoolean = try orderUnstableDecode(JSONSchema.self, from: discriminatorBooleanData)
 
         XCTAssertEqual(boolean, JSONSchema.boolean(.init(format: .generic)))
+        XCTAssertEqual(booleanOrNull, JSONSchema.boolean(.init(format: .generic, nullable: true)))
         XCTAssertEqual(nullableBoolean, JSONSchema.boolean(.init(format: .generic, nullable: true)))
         XCTAssertEqual(readOnlyBoolean, JSONSchema.boolean(.init(format: .generic, permissions: .readOnly)))
         XCTAssertEqual(writeOnlyBoolean, JSONSchema.boolean(.init(format: .generic, permissions: .writeOnly)))
