yonaskolb / SwagGen

OpenAPI/Swagger 3.0 Parser and Swift code generator
MIT License
625 stars 146 forks source link

Allow weakening Alamofire's validation #299

Closed marcelvoss closed 2 years ago

marcelvoss commented 2 years ago

This allows weakening Alamofire's validation, in order to e.g. broaden the accepted status codes for empty responses by Alamofire. It does so by basically mirroring Alamofire's responseData API and allowing an integrator to force Alamofire to be less strict.

Why? We finally wanted to upgrade our networking layer(s) to use Alamofire 5 only to realize that it is more strict about status code validation. Unfortunately, we have a couple of APIs that do not strictly conform to REST standards by e.g. returning an empty body within a DELETE endpoint and return a 200. This is differs from how the standard is defined. Long-term we want to upgrade our APIs to fix that but short-term, we can't. Therefore, we want to allow the APIClient to take a a set of status code values, hand it down to Alamofire's response methods and be able to slightly tweak it, so a 200 can be valid for an empty body. See here.

This change is only additive and does not break any existing integrations of generated code.

This was also determined to be valid by Alamofire 4.x. Therefore, this is breaking all of our integrations.

Usage when integrated could look something like this:

productCatalogClient.makeRequest(request, emptyResponseCodes: [200, 204, 205]) { [weak self] response in
      switch response.result {
                 // ...
       }
}
yonaskolb commented 2 years ago

Hi @marcelvoss. Thanks for the PR. I hesitate to leak Alamofire's APIs out of the library (there may already be some cases of this now, I haven't checked). One aim is to actual remove Alamofire as a dependency, and a move like this makes that harder. I suggest modifying the template and checking it into your project and using that instead of SwagGen's default one. What do you think?

marcelvoss commented 2 years ago

Hi @yonaskolb, thanks for the response. We've done that as well now, having realized that we reached a point where it makes sense to have our own template any way. Will close this PR.