Skip to content
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.

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

Incorrect decoding of type: null from Yaml #364

Closed
brandonbloom opened this issue Mar 31, 2024 · 5 comments · Fixed by #366
Closed

Incorrect decoding of type: null from Yaml #364

brandonbloom opened this issue Mar 31, 2024 · 5 comments · Fixed by #366

Comments

@brandonbloom
Copy link
Contributor

This test exists:

    func test_decodeNullType() throws {
        let nullTypeData = """
        {
          "type": "null"
        }
        """.data(using: .utf8)!

        let decoded = try orderUnstableDecode(JSONSchema.self, from: nullTypeData)

        XCTAssertEqual(decoded, .null())
    }

However, adding a similar test for Yaml highlights the bug:

diff --git a/Tests/OpenAPIKitTests/Schema Object/SchemaObjectYamsTests.swift b/Tests/OpenAPIKitTests/Schema Object/SchemaObjectYamsTests.swift
index 12c7c10c..1d32f5d7 100644
--- a/Tests/OpenAPIKitTests/Schema Object/SchemaObjectYamsTests.swift   
+++ b/Tests/OpenAPIKitTests/Schema Object/SchemaObjectYamsTests.swift   
@@ -16,6 +16,20 @@ import OpenAPIKit
 import Yams
 
 final class SchemaObjectYamsTests: XCTestCase {
+    func test_nullTypeDecode() throws {
+        let nullString =
+        """
+        type: 'null'
+        """
+
+        let null = try YAMLDecoder().decode(JSONSchema.self, from: nullString)
+
+        XCTAssertEqual(
+            null,
+            JSONSchema.null()
+        )
+    }
+
     func test_floatingPointWholeNumberIntegerDecode() throws {
         let integerString =
         """

In short, nil is returned, when JSONType.null is expected.

@brandonbloom
Copy link
Contributor Author

I've been looking at this a little more and have narrowed the problem down to decodeIfPresent for JSONType. This is used in a few places (decodeNullable and decodeTypes) and behaves incorrectly for type: null and type: 'null'. Interestingly, container.contains(.type) ? container.decode(JSONType.self, forKey: .type) : nil works as expected.

brandonbloom added a commit to brandonbloom/OpenAPIKit that referenced this issue Mar 31, 2024
@mattpolzin
Copy link
Owner

So this is actually related to a Yams bug that has been fixed as of version 5.1.0 of Yams. I had purposefully avoided writing a test for Yams decoding similar to the one you wrote because I knew it would not pass. Now that Yams 5.1.0 is released, it can be added (and passes): #365

Interestingly, the Yams fix also only works for Swift releases later than 5.4, it appears. A bit surprising. That means that I can't bump the locked version of Yams for the project prior to OpenAPIKit 4.0.0.

Here's a little more background: apple/swift-openapi-generator#286 (comment). In that comment, you can focus on the second paragraph where I explain and link to a Yams bug that describes the behavior you've described in the OP for this issue.

@mattpolzin
Copy link
Owner

I see you came to the same conclusion as I was typing up my response!

@mattpolzin
Copy link
Owner

Your PR looks good to me if you rebase it against release/4_0.

brandonbloom added a commit to brandonbloom/OpenAPIKit that referenced this issue Apr 1, 2024
@brandonbloom
Copy link
Contributor Author

Rebase: Done.

brandonbloom added a commit to brandonbloom/OpenAPIKit that referenced this issue Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants