swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
17.01k stars 6.03k forks source link

[Swift3] Decoder might be a little bit more liberal #5746

Closed rdalverny closed 6 years ago

rdalverny commented 7 years ago
Description

It happens that some server implementations send booleans or integers as strings, yet the client library expects a boolean or an integer (this is somewhat messy, agreed - in my case).

At this point, on such case, a generated Swift3 client will fatalError("Source \(source) is not convertible to type \(clazz): Maybe swagger file is insufficient") (in swift3/Models.mustache#L92).

Well, the server implementations could be more up to spec, although that's not always the case. The client could just accept only strings and later do the conversion.

However, having the client library cast the server response in the correct type at once would be nice (the Java client library does this, as it relies on gson). More, this also allows the server implementations to stay as they are, or to get up to spec, without having to update the client code.

Swagger-codegen version

I am using the HEAD revision at this point.

Steps to reproduce
Related issues

I am not sure, I'm new to this (Swift and Swagger). However:

Suggest a Fix

I am not sure this is the best way to fix this, or even if there would not be a better, more generic way in Swift. However the following comes to work on my side (apart from fixing the server implementations of course). Would using a specialized library be better (hkellaway/Gloss, ikesyo/Himotoki or other - at this point, I really have no idea)?

--- a/modules/swagger-codegen/src/main/resources/swift3/Models.mustache
+++ b/modules/swagger-codegen/src/main/resources/swift3/Models.mustache
@@ -73,11 +73,20 @@ class Decoders {
             return (source as! NSNumber).int32Value as! T;
         }
         if T.self is Int64.Type && source is NSNumber {
-            return source.int64Value as! T;
+            return (source as! NSNumber).int64Value as! T;
+        }
+        if T.self is Int32.Type && source is String {
+            return (source as! NSString).intValue as! T;
+        }
+        if T.self is Int64.Type && source is String {
+            return (source as! NSString).intValue as! T;
         }
         if T.self is UUID.Type && source is String {
             return UUID(uuidString: source as! String) as! T
         }
+        if T.self is Bool.Type && source is String {
+            return (source as! NSString).boolValue as! T;
+        }
         if source is T {
             return source as! T
         }
ePaul commented 7 years ago

I don't like adapting to a server who doesn't follow its own specification. JSON has just 5 types (or 7 if you count null, and integer separately from number) ... and still people succeed to use the wrong ones :frowning_face:.

If at all, that kind of behavior should only be available as a (non-default) option.

rdalverny commented 7 years ago

I agree it's far from beautiful but:

ePaul commented 7 years ago

The problem with Postel's principle (the part "be liberal in what you accept") is that people start to rely on the behavior. If data consumers start to accept strings instead of other values, then data producers will start to send strings where numbers are meant (without anyone noting that they are doing wrong things). And then other consumers break, so those need to be adapted too ... and so on.

This is why I would like to have this feature protected with some option, so it is not in widespread usage, but just used where we know it is needed (like your example).

rdalverny commented 7 years ago

Fully agree. Here's a possibility: https://github.com/rdalverny/swagger-codegen/commit/209c7c50ab506d19cd12f567d8d1dc335dc41a5d. Shall I push it as a PR or do you have any feedback before at this point (I'm not sure I did it all right regarding the option addition)?

ePaul commented 7 years ago

Looks fine from the distance (I don't know Swift enough to be able to judge about that, and didn't load the Java part in the IDE).