Make Swaggen library work on Linux and split Model/Request/Client

mackoj commented 4 years ago

Sorry it's a big PR 🙇‍♂️

This PR make swagger library work on Linux(with FoundationNetworking when needed) and still work on Darwin.

I did these changes because I needed to have a generated library working on Linux and Alamofire even the version 5 don't support Linux(https://github.com/Alamofire/Alamofire/pull/3098).

This PR will give more control on which part of generated code you want to use:

only Models (no need for Alamofire)
Models + Requests (no need for Alamofire / you can create a simple client if needed on Linux for example)
Models + Request + Client (required Alamofire)

The generated code is usable with or without dynamic library(it will now work with dynamic dependencies in Xcodegen that are imported in multiples frameworks).

This should change nothing at all to most people except the removal of Timeline(which is a class of Alamofire that have bleed into the client).

This should simplify refactoring of any part of the template if needed. For example if we want to migrate out of Alamofire it easier because now there is only one part of the code to focus on to remove it.

I did move code around lot and made multiple SPM targets but tried to reduce the other modifications to the strict minimum.

├── Client
│   ├── APIClient.swift
│   └── RequestBehaviour.swift
├── Models
│   ├── APIModel.swift
│   ├── AnyCodable.swift
│   ├── Coding.swift
│   ├── Enum.swift
│   └── Model.swift
├── Requests
│   ├── API.swift
│   ├── APIRequest.swift
│   ├── APIResponse.swift
│   ├── APIService.swift
│   ├── AnyRequest.swift
│   └── Request.swift
└── SharedCode
    ├── APIClientError.swift
    └── APIResult.swift

Issue

This force me to remove Timeline feature of Alamofire.

Questions

I'm not sure of what should be public in KeyedDecodingContainer if I can have guidance on that @yonaskolb ?

mackoj commented 4 years ago

Should this be in another Template ? The client could be made using https://github.com/swift-server/async-http-client ? We should add support for CI for Linux after this is merged.

mackoj commented 4 years ago

Adding test on podspec in CI could be great too.

#!/usr/bin/env sh

sw_version=$(swift --version | head -n 1 | cut -d ' ' -f 4)
podspec=$(ls *.podspec)
pod lib lint "${podspec}" --no-clean --verbose --swift-version="${sw_version}" --use-libraries --allow-warnings

mackoj commented 4 years ago

@yonaskolb I would love to have your insight on this when you have time. Thanks

mackoj commented 4 years ago

I would love to drop support for Cocoapods and Carthage since SPM is now compatible with iOS projects(and it will slightly simplify the code).

yonaskolb commented 4 years ago

Hi @mackoj! Thanks for your work on this. There seem to be 3 main changes bundle up in this PR

Removing Alamofire from generated code: This is great and something people have asked for for a while.
Get generated template running on Linux: This is facilitated by the above, and perhaps some other small changes. (I haven't gone through everything). This is definitely useful.
Splitting up the generated code into multiple modules: This is quite a large and complicated change. I would urge you to use your own template if you want this sort of change. This can be done using the --template argument

Would you be able to create a PR for 1, then a follow up with a PR for 2? For number 3 I would probably leave that to your own template, but if you want to submit that after as another PR we can review more closely

mackoj commented 4 years ago

Hi @yonaskolb,

Thanks for taking the time to review this. FWIW all the modifications has been done in the template I did not touch the rest of the core part of SwagGen.

1. Removing Alamofire from generated code: This is great and something people have asked for for a while.

I did not remove Alamofire for the moment I split the generated into multiple module that allow me to not use the client part on Linux and other iOS project. In order to fully remove Alamofire we will need to rewrite the client part of the template.

2. Get generated template running on Linux: This is facilitated by the above, and perhaps some other small changes. (I haven't gone through everything). This is definitely useful.

Removing the Timeline object(removed from APIClient/APIResponse/RequestBehaviour) allow me to move Alamofire in the Client folder but force me to add the two functions below in APIRequest in order to make createURLRequest work again.

  private func encodeURLParameters(_ urlRequest: URLRequest, with parameters: [String: Any]?) throws -> URLRequest {
    var urlRequest = urlRequest
    guard let parameters = parameters else { return urlRequest }
    guard let url = urlRequest.url else { throw(APIClientError.missingURL) }

    if var urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: url.resolvingAgainstBaseURL), !parameters.isEmpty {
      var queryItems = urlComponents.queryItems ?? []
      let urlQueryItems = parameters.map { (arg0) -> URLQueryItem in
        let (key, value) = arg0
        return URLQueryItem(name: key, value: value as? String)
      }
      queryItems.append(contentsOf: urlQueryItems)
      urlComponents.queryItems = queryItems
      urlRequest.url = urlComponents.url
      return urlRequest
    }
    throw(APIClientError.urlEncodingError)
  }

  private func encodeFormParam(_ urlRequest: URLRequest, with parameters: [String: Any]?) throws -> URLRequest {
    var urlRequest = urlRequest
    guard let parameters = parameters else { return urlRequest }
    guard let url = urlRequest.url else { throw(APIClientError.missingURL) }

    if urlRequest.value(forHTTPHeaderField: "Content-Type") == nil {
        urlRequest.setValue("application/x-www-form-urlencoded; charset=utf-8", forHTTPHeaderField: "Content-Type")
    }

    if var urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: url.resolvingAgainstBaseURL), !parameters.isEmpty {
      var queryItems = urlComponents.queryItems ?? []
      let urlQueryItems = parameters.map { (arg0) -> URLQueryItem in
        let (key, value) = arg0
        return URLQueryItem(name: key, value: value as? String)
      }
      queryItems.append(contentsOf: urlQueryItems)
      urlComponents.queryItems = queryItems

      urlRequest.httpBody = urlComponents.queryItems?.map { (queryItem) -> String in
        "\(queryItem.name)=\(queryItem.value)"
      }.joined(separator: "&").data(using: .utf8, allowLossyConversion: false)
    }

    throw(APIClientError.urlEncodingError)
  }

3. Splitting up the generated code into multiple modules: This is quite a large and complicated change. I would urge you to use your own template if you want this sort of change. This can be done using the --template argument

The rest of the change are missing public or import. I can do another template but I want to upstream this one because it don't really differ from the original Swift template it is basically the same with some code moved around and isolation of Alamofire in the client folder/target.

Most of the work require the splitting up of generated code in order to make sense and the split is really working only in SPM for the moment.

Thus I don't see how I can split this into smaller PR maybe I can redo a PR without the fixtures then you will be able to better parse the signal from the noise.

mackoj commented 4 years ago

https://github.com/yonaskolb/SwagGen/pull/232

yonaskolb / SwagGen

Make Swaggen library work on Linux and split Model/Request/Client #225

Issue

Questions