Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unstable order when reading spec from .json file #12

Closed
PhilipTrauner opened this issue Jan 28, 2022 · 6 comments
Closed

Unstable order when reading spec from .json file #12

PhilipTrauner opened this issue Jan 28, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@PhilipTrauner
Copy link
Contributor

https://github.com/kean/CreateAPI/blob/87ba65d28a657ef7b94a758697cbf68d6075dbb9/Sources/CreateAPI/Generate.swift#L159-L160

In addition to not being thread-safe, JSONDecoder also does not appear to guarantee any explicit order of object keys.

Spec
{
  "openapi": "3.0.0",
  "info": {
    "version": "1.0.0",
    "title": "Order"
  },
  "paths": {
    "/container": {
      "get": {
        "responses": {
          "200": {
            "description": "",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Container"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Container": {
        "type": "object",
        "required": null,
        "properties": {
          "propertyOne": {
            "type": "string"
          },
          "propertyTwo": {
            "type": "string"
          },
          "propertyThree": {
            "type": "string"
          },
          "propertyFour": {
            "type": "string"
          }
        }
      }
    }
  }
}
Output
// Generated by Create API
// https://github.com/kean/CreateAPI
//
// swiftlint:disable all

import Foundation

public struct Container: Codable {
    public var propertyFour: String?
    public var propertyThree: String?
    public var propertyOne: String?
    public var propertyTwo: String?

    public init(propertyFour: String? = nil, propertyThree: String? = nil, propertyOne: String? = nil, propertyTwo: String? = nil) {
        self.propertyFour = propertyFour
        self.propertyThree = propertyThree
        self.propertyOne = propertyOne
        self.propertyTwo = propertyTwo
    }
}

struct StringCodingKey: CodingKey, ExpressibleByStringLiteral {
    private let string: String
    private var int: Int?

    var stringValue: String { return string }

    init(string: String) {
        self.string = string
    }

    init?(stringValue: String) {
        self.string = stringValue
    }

    var intValue: Int? { return int }

    init?(intValue: Int) {
        self.string = String(describing: intValue)
        self.int = intValue
    }

    init(stringLiteral value: String) {
        self.string = value
    }
}

The easiest workaround is to convert the .json spec to a .yaml spec beforehand.

@kean
Copy link
Member

kean commented Jan 28, 2022

Unfortunately, that seems to be the case. It's recommended to use YAML. OpenAPIKit uses ordered dictionaries to preserve the order of the keys when it can (YAML).

To convert, you can use Swagger Editor. See File / Convert and save as JSON.

There is also a CreateAPI option to sort properties alphabetically which is disabled by default:

entities:
  isSortingPropertiesAlphabetically: true

@kean kean added the enhancement New feature or request label Jan 28, 2022
@PhilipTrauner
Copy link
Contributor Author

PhilipTrauner commented Jan 29, 2022

The primary objective of this revision is to bring YAML into compliance with JSON as an official subset.

YAML 1.2 Specification (2009)

It should be perfectly valid to use YAMLDecoder for json documents.
This of course hinges on how closely Yams follows the spec.

I didn't run into any issues with the OpenAPI specs I tried (two small internal ones), but that feels like too small of a sample size to conclusively rule out any nasty edge cases.

@PhilipTrauner
Copy link
Contributor Author

If retaining order is important for your use-case, I recommend the Yams and FineJSON libraries for YAML and JSON respectively. Also keep in mind that JSON is entirely valid YAML and therefore you will likely get good results from Yams decoding of JSON as well (it just won't encode as valid JSON).

OpenAPIKit Documentation

Not sure if "you will likely get good results" is good enough, but I'd argue that entity generation with JSON as input should currently be considered broken, as its behavior is unexpected for anyone unfamiliar with the JSON specification.

Would you be interested in a PR that switches the JSON parser over to Yams and adds tests?
It wouldn't be necessary to check in any JSON specs, as decoding a YAML spec with Yams and consequently encoding it with JSONEncoder does retain order.

@kean
Copy link
Member

kean commented Feb 3, 2022

should currently be considered broke

As in the ordering, or are there more issues?

Would you be interested in a PR that switches the JSON parser over to Yams and adds tests?

Yeah, absolutely. Let's give it a try. If it resolved the ordering issue, that would be fantastic.

I have a private repo with 1KK lines of specs, but they are all YAML. I'll give you access in case you need it. It's not perfectly structured. It was initially part of this project, so I just copied it and symlinked the sources for now.

But I think adding a JSON+YAML test to this repo should be enough.

@liamnichols
Copy link
Member

liamnichols commented Apr 6, 2022

I had a go at doing this, but I found a bug in Yams that is a slight blocker (it caused one of the specs that decodes a json configuration to fail):

Other than the empty string/optional issue though, Yams has been pretty good at handling our 31,500 line OpenAPI schema in JSON format, we've just been renaming it to .yml (so that CreateAPI uses Yams and preserves the ordering) but not converting it from JSON.

@liamnichols
Copy link
Member

Fixed in 0.0.4! Thanks 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants