Skip to content

fix "nullable" decoding #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 24, 2024
Merged

fix "nullable" decoding #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2700757
fix "nullable" decoding
brandonbloom Feb 18, 2024
86da5eb
Add a warning when decoding an OpenAPI 3.1 document that uses the nul…
mattpolzin Feb 24, 2024
d35921c
Merge pull request #1 from mattpolzin/nullable-decoding-warning
brandonbloom Feb 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 47 additions & 19 deletions Sources/OpenAPIKit/Schema Object/JSONSchema.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down
62 changes: 54 additions & 8 deletions Sources/OpenAPIKit/Schema Object/JSONSchemaContext.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -229,6 +231,7 @@ extension JSONSchema {
vendorExtensions: [String: AnyCodable] = [:],
_inferred: Bool = false
) {
self.warnings = []
self.format = format
self.required = required
self.nullable = nullable
Expand Down Expand Up @@ -260,6 +263,7 @@ extension JSONSchema {
examples: [String],
vendorExtensions: [String: AnyCodable] = [:]
) {
self.warnings = []
self.format = format
self.required = required
self.nullable = nullable
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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)
}
}

Expand Down
42 changes: 42 additions & 0 deletions Tests/OpenAPIKitErrorReportingTests/SchemaErrorTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
])
}
}
}
5 changes: 4 additions & 1 deletion Tests/OpenAPIKitTests/Schema Object/JSONSchemaTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)!
Expand All @@ -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)
Expand All @@ -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)))
Expand Down