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

EncodeIfPresent Doesn't Honor If An Optional is Present for URLEncodedFormEncoder #3870

Open
phoney opened this issue May 15, 2024 · 1 comment

Comments

@phoney
Copy link

phoney commented May 15, 2024

What did you do?

I expect that encodeIfPresent() when used with URLEncodedFormEncoder will NOT emit an optional value that is nil. I expect that a simple encode() WILL include an optional value, even if it's nil, based on the nilEncoding value. I believe that's how JSONEncoder works but I don't think that's how URLEncodedFormEncoder works.

I have these two unit tests that I expect to pass. The JSONEncoder test passes but the URLEncodedFormEncoder test doesn't pass.

    private struct OptionalEncodableStruct2: Encodable {
        let one = "one"
        let two: String? = nil
        let three: String? = nil

        func encode(to encoder: any Encoder) throws {
            var container = encoder.container(keyedBy: CodingKeys.self)

            try container.encode(one, forKey: .one)
            try container.encode(two, forKey: .two)
            try container.encodeIfPresent(three, forKey: .three)
        }

        private enum CodingKeys: String, CodingKey {
            case one, two, three
        }
    }

    func testThatEncodeIfPresentDoesntEncodeNil() {
        // Given
        let encoder = URLEncodedFormEncoder(nilEncoding: .null)

        // When
        let result = Result<String, Error> { try encoder.encode(OptionalEncodableStruct2()) }

        // Then
        // XCTAssertEqual(result.success, "one=one")   // Cannot convert to string...
        switch result {
        case .success(let value):
            // two should be emitted, three should not be emitted
            XCTAssertEqual(value, "one=one&two=null")
            // XCTAssertEqual failed: ("one=one&three=null&two=null") is not equal to ("one=one&two=null").null
            // XCTAssertEqual failed: ("one=one&three=&two=") is not equal to ("one=one&two=") .dropValue
            // XCTAssertEqual failed: ("one=one") is not equal to ("one=one&two=null") .dropKey
        case .failure(let error):
            print(error)
            XCTFail()
        }
    }

    func testThatEncodeIfPresentDoesntEncodeNilJSONEncoder() throws {
        // Given
        let encoder = JSONEncoder()

        // When
        let result = try encoder.encode(OptionalEncodableStruct2())
        let value = String(data: result, encoding: .utf8) ?? ""

        // Then
        // two should be emitted, three should not be emitted
        XCTAssertEqual(value, "{\"one\":\"one\",\"two\":null}")
    }

What did you expect to happen?

I expect that when using encodeIfPresent() with URLEncodedFormEncoder that nil values will not be emitted.
I expect that encode() works differently from encodeIfPresent() for nil values.
I expect that encode() will emil a nil value based on the nilEncoding value.
I expect that encodeIfPresent() will only emit a value based on the nilEncoding value if the optional is not nil.

I expect that URLEncodedFormEncoder will handle encodeIfPresent() in the same way as JSONEncoder.

What happened instead?

A nil value is emitted both when using encode() and encodeIfPresent() for nilEncoding of .null and .dropValue.
A nil value is not emitted both when using encode() and encodeIfPresent() for nilEncoding of .dropKey.

I have a case where my Encodable struct has several optional values. I want one of them to not be present if it's nil and I want the other to be present even if it's nil. I realize there's no way to manage this with the simple compiler-emitted encoding but I should be able to manage this by use of encode() and encodeIfPresent(). The only way to currently do that is to only call encodeIfPresent() based on whether the object is nil in my encode(to:) method.

Like

    if let three {
        try container.encodeIfPresent(three, forKey: .three)
    }

but it should work like JSONEncoder.

This issue is in some way related to this other issue #3778

Alamofire Environment

Alamofire Version: 5.9.1
CocoaPods 1.15.2
Xcode Version: Version 15.4 (15F31d)
Swift Version: Swift 5
Platform(s) Running Alamofire: iOS 17.5
macOS Version Running Xcode: 14.4.1 (23E224)

Demo Project

I think my unit tests above should demonstrate the issue.

@jshier
Copy link
Contributor

jshier commented May 18, 2024

This behavior is intentional, in order to give users control over their nil encoding while still using the compiler's synthesized conformance. Changing it now would drop every Optional value of every user of URLEncodedFormEncoder, which would be a breaking change.

That said, we could probably add yet another configuration flag to control that behavior. If you'd like to add something in a PR I can review.

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

No branches or pull requests

2 participants