[REQ] [Swift5] POP proposal

[wolfAle]

Hi guys,

I've been using the OpenAPI Generator for Swift 4/5 for a while now, quite happy with your work, thanks!

But after a while I struggled a bit whenever I needed to add some customisation on the generated code.
Mainly the issues I found were related to the fact that the methods are defined as class func.
That makes it hard to:

• override: you would need to define a class SomethingSubclassAPI that inherits from the generated SomethingAPI class and override the method. Then you would need to use the subclass around. Easy to do if that happens at the beginning of the project, much more complicated if you already used a lot the generated class around
• it forces you to pass through the same classes for all the calls, hence the problem above

I then modified the templates to achieve something different.

From this:

open class AlertAPI {
    /**
     Returns the alert
     
     - parameter ifNoneMatch: (header) The value of the Etag header returned in the previous response (optional)
     - parameter completion: completion handler to receive the data and the error objects
     */
    open class func getAlert(ifNoneMatch: String? = nil, completion: @escaping ((_ data: [Alert]?,_ error: Error?) -> Void)) {
        getAlertWithRequestBuilder(ifNoneMatch: ifNoneMatch).execute { (response, error) -> Void in
            completion(response?.body, error)
        }
    }
}

To this:

public protocol AlertAPIProtocol {
  /**
   Returns the alert
   
   - parameter ifNoneMatch: (header) The value of the Etag header returned in the previous response (optional)
   - parameter completion: completion handler to receive the data and the error objects
   */
   static func getAlert(ifNoneMatch: String?, completion: @escaping ((_ data: [Alert]?, _ headers:[String:String]?, _ error: Error?) -> Void))
}

public extension AlertAPIProtocol {

    static func getAlert(ifNoneMatch: String? = nil, completion: @escaping ((_ data: [Alert]?, _ headers:[String:String]?, _ error: Error?) -> Void)) {
        getAlertWithRequestBuilder(ifNoneMatch: ifNoneMatch).execute { (response, error) -> Void in
            completion(response?.body, response?.header, error)
        }
    }
}

This approach works quite well for me and it allows me to:

• conform to the protocol in each class/struct/enum where I need to perform such API request
• override the methods whenever and wherever I need to. I can override in a single case, if needed, or in a subprotocol, or even make conditional override based on conditions through the where operator
• plug different protocols to the same class/struct/enum, composing different APIs by needs

Additionally I have an autogenerated Mock version of the same protocols, that inherits from the original ones, that load content from a json in the bundle. This approach opens to the possibility of conforming to AlertAPIPrototocol in a target and to AlertAPIMockPrototocol in another and being one a subprotocol to the other you can easily use polymorphism.
This is just an example of what this approach can lead to. :-)

I'd love to see this approach implemented in the official OpenAPI Generator for swift, I think it could be useful for a lot of people.
Are you interested on discussing this further?

Thanks,
Alessandro

[frabarbo]

Follow.
it could help me too!

[4brunu]

This looks like something interesting.
Anyone interested in creating a draft PR?

[wolfAle]

Hi @4brunu ,

I already have some templates ready on my side but only for the Combine case, I didn't support all the cases because are out of scope for me right now. Furthermore I'm not a mustache-master and I'm not sure I followed the best practices. :-)
I see that in the meantime a new version of the generator came out with some new logic/syntax in the templates, most likely this will lead to a re-writing of what I did.

I could start from there and then we can iterate to support all the cases, but the main question here is: is that of interested for the OpenAPI community?
Looking at this thread, that's being sitting here for 4 months now, it doesn't look like that. :-)
If we could have additional feedback in here from people interested, it would be great.

Thanks,
Alessandro

[4brunu]

Hey @wolfAle,

I was curious about this, so I made a little draft to see how many changes that would imply, and it seams that protocols have some limitations, like methods with default parameters and around visibility modifiers, but after some changes this draft, kind of "work" for most cases.

Could you please check if this is what you are proposing?
4brunu@956e6b1

I could start from there and then we can iterate to support all the cases, but the main question here is: is that of interested for the OpenAPI community?
Looking at this thread, that's being sitting here for 4 months now, it doesn't look like that. :-)
If we could have additional feedback in here from people interested, it would be great.

This is something that I have been struggling with too.
Getting feedback from the community it's not always easy.
I also have some questions that I would like to get feedback on, like:

• does it steal make sense to support Alamofire, now that we support URLSession? Could we drop support for Alamofire?
• does anyone still uses PromiseKit, or it's in the generator for legacy reasons?

But to try not to get away from the main issue here, I see value in this proposal, but this would need to be behind a flag for the user's to choose between the current approach and this POP approach.
I though it would be a bit easier to support it, but it's nothing that with a bit of work we could support both options.
This being said, I also would like to have more feedback on this to see how much interest there is on this, to see if the effort to support is is worth it.

[wolfAle]

Hi @4brunu ,

yeah, your draft looks ok! Of course with some iterations over it I believe we could further improve the outcome, but that's what I meant and what I've being using in the last year or so. :-)

I'm happy to help you out with the implementation, if you'd like me to, but I'll have a few questions such as:

• what IDE/Tool do you use to edit the mustache files?
• is there a guide or something about the best practices when working with mustache files?

About Alamofire/PromiseKit: my feeling is that they are not needed anymore and it would save a lot of work to deprecate them, but that's not my call really.
Would be removing them from the swift5 folder and maybe add them later (if requested) to another swift5Legacy folder an option?
Or maybe have the Protocol implementation with only URLSession and Combine support in a Swift5Protocol folder and we can then see how are feedback about it?
Just to break down the implementations a bit and don't have a lot of maintenance to do.

I'd be happy if there was a way to invite people to give a feedback on this thread, but I guess there isn't. :-/

[wolfAle]

And, BTW, it's a bit unclear to me what's the relationship between OpenAPI and Swagger right now. I thought that one was the evolution of the other, but I just found out that there's a (quite recent) Swagger version of the generator with the protocol approach:

https://github.com/swagger-api/swagger-codegen/tree/master/modules/swagger-codegen/src/main/resources/swift5-protocol-oriented

🤔

[4brunu]

Here is the reasoning behind the fork.
https://openapi-generator.tech/docs/faq#what-is-the-difference-between-swagger-codegen-and-openapi-generator
https://openapi-generator.tech/docs/fork-qna

Since the fork, the swift generators in swagger-codegen and openapi-generator started to diverge a bit in terms of feature set.
DISCLAIMER: This was a quick list that I created and might be incorrect or outdated.

Features in openapi-generator that are not present in swagger-codegen:

• URLSession
• Combine
• Result
• Swift Package Manager
• XcodeGen
• Hashable models
• oneOf
• vapor (soon)

Features in swagger-codegen that are not present in openapi-generator:

• POP

• what IDE/Tool do you use to edit the mustache files?

This is my personal response, so VSCode or IntelliJ IDEA.

• is there a guide or something about the best practices when working with mustache files?

Not that I'm aware of, I learned it by comparison, but here are the docs.
https://mustache.github.io

Instead of creating a new resources directory to support POP, I would prefer to create if elses the current resources, unless the changes are huge, but from the draft that I created it doesn't seem so.
So I would prefer to avoid resources duplication, but if we come to the conclusion that a lot of things changes, we can think of duplicate just that file, but let's try to avoid that.
And ideally the POP implementation should support all the response types.

Your PR is more than welcome, and if you have questions, or want feedback on something, or need help in something, feel free to ping me 🙂

[4brunu]

By the way, there is a huge PR should be merged soon #9625, and it will change a lot of files, so maybe you might want to wait for that PR to get merged before doing this, to avoid merge conflicts.

[4brunu]

Now that #9625 is merged, I think it's safe to start working on POP implementation 🙂

[4brunu]

Hey @wolfAle,
Did you made any progress with this?
Do you need help?

[wolfAle]

Hi @4brunu ,

honestly, didn't had any chance to work on this :-(
This second part of the year has been pretty busy and I couldn't find the time to proceed further.
But it wasn't forgot, I swear! I was thinking about this a few days ago.

Hopefully I'll find some free time to get to the bottom of it!

Alessandro

[4brunu]

No problem, take your time 🙂 there is no pressure, I was just looking at some issue and remembered this one

[4brunu]

We don't have exactly this implemented, but now in the new swift 6 generator, it's possible to create instances of a class, and use instance methods instead of class methods, which makes it possible to create subclasses of the api files, with custom behaviour.

Give it a try with the flag apiStaticMethod: false