openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][Swift] Decimal-Decoding not working

Open Feuerwerk opened this issue 1 year ago • 2 comments

Description

I have a service providing a list of objects with a property of type string and format number (BigDecimal on the backend side). The generated service description is fine:

"minValue": {
  "type": "string",
  "format": "number"
}

The models generated by OpenAPI-Generator also looks good:

public var minValue: Decimal?

But when my swift5 client tries to consume this service I always get the exception that the returning data is not valid and can't be decoded. I checked the data transmitted from the server and it looks valid to me.

"minValue": "0.12345"

When sending data from the swift client to the server, it will encode the decimal as a number

"value": 0.12345
openapi-generator version

6.1.0

Generation Details

openapi-generator generate -i http://localhost:8081/v3/api-docs -g swift5 -c config.json

{
	"unwrapRequired" : true,
	"projectName" : "ApiClient",
	"podAuthors" : "XXXXX",
	"podHomepage" : "XXXX",
	"podSummary" : "Summary"
}
Suggest a fix

I can fix the problem when I add the following four methods to the generated Extensions.swift

extension KeyedEncodingContainerProtocol {
    mutating func encode(_ value: Decimal, forKey key: Self.Key) throws {
        var mutableValue = value
        let stringValue = NSDecimalString(&mutableValue, Locale(identifier: "en_US"))
        try encode(stringValue, forKey: key)
    }
    
    mutating func encodeIfPresent(_ value: Decimal?, forKey key: Self.Key) throws {
        if let value = value {
            try encode(value, forKey: key)
        }
    }
}

extension KeyedDecodingContainerProtocol {
    func decode(_ type: Decimal.Type, forKey key: Self.Key) throws -> Decimal {
        let stringValue = try decode(String.self, forKey: key)
        guard let decimalValue = Decimal(string: stringValue) else {
            let context = DecodingError.Context(codingPath: [key], debugDescription: "The key \(key) couldn't be converted to a Decimal value")
            throw DecodingError.typeMismatch(type, context)
        }
        return decimalValue
    }
    
    func decodeIfPresent(_ type: Decimal.Type, forKey key: Self.Key) throws -> Decimal? {
        guard let stringValue = try decodeIfPresent(String.self, forKey: key) else {
            return nil
        }
        guard let decimalValue = Decimal(string: stringValue) else {
            let context = DecodingError.Context(codingPath: [key], debugDescription: "The key \(key) couldn't be converted to a Decimal value")
            throw DecodingError.typeMismatch(type, context)
        }
        return decimalValue
    }
}

Feuerwerk avatar Sep 13 '22 13:09 Feuerwerk

@Feuerwerk I'm using 6.0.0 and I'm not running into this issue. Did you try with that version?

otusweb avatar Sep 19 '22 08:09 otusweb

@otusweb I downgraded openapi-generator to 6.0.0 but got exactly the same problem. Add the 4 methods for encoding/decoding also solves the problem.

Feuerwerk avatar Sep 19 '22 09:09 Feuerwerk

@otusweb I downgraded openapi-generator to 6.0.0 but got exactly the same problem. Add the 4 methods for encoding/decoding also solves the problem.

Hi @Feuerwerk. Does this also work if there are attributes like this in the swagger?

"anotherValue": {
  "type": "number",
  "format": "double"
}

Because I guess with your change they would now also be decoded from a string (which it is not) and encoded to strings which will both fail

Jonas1893 avatar Sep 29 '22 06:09 Jonas1893

@Feuerwerk I've looked at my swagger file and I have :

        "Value": {
          "format": "decimal",   <=== important part here
          "description": "the value of the campaign.
Applies to percent, new price, discount campaign type. Other campaign type use valueX and ValueY directly on the campaign",
          "type": "string"
        }

I guess in the swagger file that you sent, there is no indication that you want a decimal, just that it should be a number. Can you edit the service definition to see if that would fix it. On my team, i worked with the backend team to update the swagger file...

otusweb avatar Sep 29 '22 07:09 otusweb

Hi @Jonas1893

Does this also work if there are attributes like this in the swagger? Because I guess with your change they would now also be decoded from a string (which it is not) and encoded to strings which will both fail

Al least from my understanding of the OpenAPI Spec 3.0 type number / format double results in a Double datatype not in a Decimal datatype. I also tried it with openapi-generator (6.2.0) and the datamodel of my Swift client now has the following property:

public var value: Double?

And in this case the workaround (solution?) I suggested for Decimal would not interfere with Double.

Feuerwerk avatar Sep 29 '22 08:09 Feuerwerk

Hi @otusweb

        "Value": {
          "format": "decimal",   <=== important part here
          "description": "the value of the campaign.
Applies to percent, new price, discount campaign type. Other campaign type use valueX and ValueY directly on the campaign",
          "type": "string"
        }

I guess in the swagger file that you sent, there is no indication that you want a decimal, just that it should be a number. Can you edit the service definition to see if that would fix it. On my team, i worked with the backend team to update the swagger file...

Sadly "decimal" is not specified in OpenAPI Spec 3.0 but due to https://github.com/OpenAPITools/openapi-generator/pull/7808 i thought type string / format number is the way to go. So I changed my server side as you suggested to type string / format decimal and generated my Swift 5 client (now using OpenAPI Generator 6.2.0) but exactly the same code is generated on client side no additional support for decimal is generated. I also gave it a test and the same exception is thrown.

The Apples Swift JsonParser can't handle Decimal from a string without additional support I think.

Feuerwerk avatar Sep 29 '22 08:09 Feuerwerk

Hi @Jonas1893

Does this also work if there are attributes like this in the swagger? Because I guess with your change they would now also be decoded from a string (which it is not) and encoded to strings which will both fail

Al least from my understanding of the OpenAPI Spec 3.0 type number / format double results in a Double datatype not in a Decimal datatype.

Exactly. In the example I described the json response from the server will not be

"minValue": "0.12345"

like in your example but

"anotherValue": 0.12345

And because of your extension decoder would now try to decode from a String, fail and throw the exception, no? Maybe I am overlooking something

Jonas1893 avatar Sep 29 '22 09:09 Jonas1893

@Jonas1893 That are different datatypes (Double vs. Decimal) and they can / will be treated differently by Apples Json-Decoder / -Encoder.

The code I suggested only extend encoding / decoding for swift type Decimal. But if the data model contains the swift type Double (like in your example) the code I suggested simply will not run and therefore will not interfere and cause problems.

Feuerwerk avatar Sep 29 '22 09:09 Feuerwerk

Understood, thx for clarifying

Jonas1893 avatar Sep 29 '22 09:09 Jonas1893

@otusweb

I'm pretty sure that @wing328 who added support for Decimal in Swift-Clients in OpenAPI Generator (type string / format number) did some testing so why didn't he get errors and you don't get errors too but I'm keep getting exceptions? According to this discussion it seems Apple changed something in the Xcode SDK. I'm currently using 14.0.1, what version are you using?

Feuerwerk avatar Sep 29 '22 10:09 Feuerwerk

@Feuerwerk On our end, here is the json spec for a decimal number:

        "PriceIncVat": {
          "format": "decimal",
          "type": "string"
        },

and the data returned by the server for that value: "PriceIncVat": 1.0000

I'm running 6.0.0 and still on Xcode 13, but I'm assuming that the version of Xcode should not have much impact. Maybe iOS version. I've tested on iOS 15 and lower, but not 16 yet. I don't know how the internals work though, just that this combo works for us. Can you get your server to return the decimal number without the quotes?

otusweb avatar Sep 30 '22 07:09 otusweb

@otusweb Ok, that will explain why it's working for you. You signal in your service description that the server will send a string that should be interpreted as a decimal. Therefore OpenAPI Generator will generate a data model with the property PriceIncVat of type Decimal. But your server will send the data actually as a number. On Swift-Client side the received data will be parsed by Apples JsonDecoder which has a default implementation for Decimal and expect a number in that case (not a string). It simply works by accident for you. But you may run into a precision problem, because according to this the buildin JsonDecoder will decode value first as a double and then call Decimal(double: myValue) to convert it into a Decimal. And this could mean that you will loose precision. This is especially dangerous for currency values like in your case.

Feuerwerk avatar Sep 30 '22 09:09 Feuerwerk

@Feuerwerk thanks for pointing that out. Sounds like a larger issue for us then :-)

otusweb avatar Sep 30 '22 14:09 otusweb