swift-openapi-generator icon indicating copy to clipboard operation
swift-openapi-generator copied to clipboard

Recursive types not supported

Open scsinke opened this issue 1 year ago • 2 comments

Currently the package does not support recursive types. Should this be possible? Maybe I'm doing something wrong. otherwise, I would propose to add this constraint in the docs. I have this use case due to Server Driven UI. This results in components or actions that can be embedded in each other.

Errors:

  • Value type 'x' cannot have a stored property that recursively contains it
  • Value type 'x' has infinite size

scsinke avatar Jun 16 '23 10:06 scsinke

Thanks for filing the issue, we'd certainly accept a contribution of support of recursive types.

Could you please add a snippet of OpenAPI that uses it in YAML, and how you imagine the generated code would change? That'll help us discuss the desired solution.

czechboy0 avatar Jun 16 '23 10:06 czechboy0

We are looking at migrating from our own hand-rolled code generation to this library, and for some of our APIs we are also encountering this issue.

Here's an example schema with recursion (involving only one model):

Recursion:
  properties:
    recursion:
      "$ref": "#/components/schemas/Recursion"
  type: object

In our existing code generator we break out of the dependency cycles by wrapping the property in a Box:

    public final class Box<Wrapped> {
        public var value: Wrapped
        public init(_ value: Wrapped) {
            self.value = value
        }
    }

    extension Box: Encodable where Wrapped: Encodable {
        public func encode(to encoder: Encoder) throws {
            try value.encode(to: encoder)
        }
    }

    extension Box: Decodable where Wrapped: Decodable {
        public convenience init(from decoder: Decoder) throws {
            let value = try Wrapped(from: decoder)
            self.init(value)
        }
    }

    public struct Recursion: Codable, Hashable {
        public let recursion: Box<Recursion>?
    }

knellr avatar Jun 16 '23 16:06 knellr

I don't know if this can solve the issue. But maybe we can make use of the indirect keyword? https://www.hackingwithswift.com/example-code/language/what-are-indirect-enums

scsinke avatar Jun 21 '23 07:06 scsinke

Seems that indirect can only be used on enums, not structs.

Before we consider the explicit Box approach, I'd like to see if there's another recommended way in Swift to solve this.

czechboy0 avatar Jun 21 '23 07:06 czechboy0

Hereby a simplified spec we use. For the full openAPI spec we use. check this link.

{
  "openapi": "3.0.2",
  "components": {
    "schemas": {
      "Action": {
        "anyOf": [
          {
            "$ref": "#/components/schemas/PromptAction"
          }
        ]
      },
      "PromptAction": {
        "type": "object",
        "properties": {
          "type": {
            "type": "string",
            "enum": [
              "PROMPT"
            ]
          },
          "prompt": {
            "$ref": "#/components/schemas/Prompt"
          }
        },
        "required": [
          "type",
          "prompt"
        ]
      },
      "Prompt": {
        "anyOf": [
          {
            "$ref": "#/components/schemas/DialogPrompt"
          }
        ]
      },
      "DialogPrompt": {
        "type": "object",
        "properties": {
          "type": {
            "type": "string",
            "enum": [
              "DIALOG"
            ]
          },
          "title": {
            "type": "string"
          },
          "message": {
            "type": "string"
          },
          "confirmButtonTitle": {
            "type": "string"
          },
          "cancelButonTitle": {
            "type": "string"
          },
          "confirmAction": {
            "$ref": "#/components/schemas/Action"
          }
        },
        "required": [
          "type",
          "title",
          "message",
          "confirmButtonTitle",
          "cancelButonTitle",
          "confirmAction"
        ]
      }
    }
  }
}

scsinke avatar Jun 21 '23 09:06 scsinke

I think there are three options as far as I can tell.

  • Make it a class. But this does make it a bit less swifty because we're not using structs
class Recursive {
    let value: Recursive
}
  • Make an indirect enum.
enum Recursive<Element> {
  indirect case value(Recursive<Element>)
}
  • Use the box as stated above. So it can be used inside a struct. This can be a bit simplified by maybe adding a property wrapper.

scsinke avatar Jun 21 '23 17:06 scsinke

Yeah, that sounds about right. Ideally, we could constrain this as much as possible, to:

  • direct nesting
  • the property must be optional

And we could use a property wrapper.

We already have the concept of a TypeUsage that represents a type being wrapped in an optional, an array, etc. I guess we could think of an "indirection box" as just another type of wrapping here, make it Sendable, Codable, etc.

It's important we limit how many places in the generator have to make explicit decisions based on this.

Might be worth for someone to prototype this and see what else might get hit.

czechboy0 avatar Jun 21 '23 17:06 czechboy0

@czechboy0 If you can give me some pointers on how and where to start prototyping. Will try to play around with it.

scsinke avatar Jun 26 '23 15:06 scsinke

Hi @scsinke, sure!

TypeUsage is defined here: https://github.com/apple/swift-openapi-generator/blob/6b11135cccfb0846809f434cf2ad95134b65e945/Sources/_OpenAPIGeneratorCore/Translator/TypeAssignment/TypeUsage.swift#L43

The need for boxing could be represented as another case in the type usage internal enum.

The need for it would be calculated for the property schema, probably somewhere around https://github.com/apple/swift-openapi-generator/blob/6b11135cccfb0846809f434cf2ad95134b65e945/Sources/_OpenAPIGeneratorCore/Translator/TypeAssignment/TypeAssigner.swift#L234, if the parent type and the property type are the same.

Then, we'd probably need to adjust the struct generation logic to include the extra boxing when requested: https://github.com/apple/swift-openapi-generator/blob/6b11135cccfb0846809f434cf2ad95134b65e945/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStructBlueprint.swift#L21

Would be great if the box could be represented as a property wrapper somehow.

But that's just one idea, you'll see what looks good when you're prototyping it, feel free to tag me on a PR with a proof of concept, and we can discuss more.

czechboy0 avatar Jun 26 '23 16:06 czechboy0

Another way to implement this would be:

  • The type that needs to be "boxed" (aka is part of a cycle) would have a slightly different implementation under the hood: it'd still be a struct publicly, but internally would hold a class and use CoW. So the adopter would not know whether it's a plain old struct, or this "boxed" struct, but it'd break the cycle and allow for both direct and transitive recursive types.
  • Deciding which type needs to be boxed is a separate algorithm, ideally we want to box as few types as needed to break all the cycles in a single document, but an algorithm that boxes all types (func needsBoxing(...) -> Bool { true }) would be an okay starting point, and try to make it more efficient from there.

If someone wants to pick this up, it could make sense to PR these bits independently, e.g. the algorithm first, then the changes to the generator.

czechboy0 avatar Jul 25 '23 12:07 czechboy0

This also blocks generating the App Store Connect OpenAPI document: https://developer.apple.com/sample-code/app-store-connect/app-store-connect-openapi-specification.zip

The schema DiagnosticLogCallStackNode is recursive there.

czechboy0 avatar Sep 26 '23 07:09 czechboy0

We have lots of cycles in the API we want to use, so fixing this would be crucial for us. Thanks in advance.

herrernst avatar Oct 05 '23 18:10 herrernst

Thanks @herrernst for letting us know. Without going into any confidential specifics, what's your use case of recursive schemas? We've seen a representation of a file system hierarchy, which is inherently recursive, and in general graph representations. What's yours?

czechboy0 avatar Oct 05 '23 19:10 czechboy0

Also very interested in this. We also have the use-case of server-driven UI.

yanniks avatar Oct 07 '23 09:10 yanniks

Thanks @herrernst for letting us know. Without going into any confidential specifics, what's your use case of recursive schemas? We've seen a representation of a file system hierarchy, which is inherently recursive, and in general graph representations. What's yours?

Just basic things like a Person having a partner that is also a Person or having friends that are Persons.

herrernst avatar Oct 08 '23 12:10 herrernst

In our use case, we have a data type which represents a polymorphic wrapper which encapsulates nested data types of the same wrapper. In short, we have Content which contains an array of Content.

For example:

[
    {
        "type": "box",
        "value": [ // Polymorphic envelope
            {
                "type": "text",
                "value": [ // Polymorphic envelope
                    {
                        "text": "hello world"
                    }
                ]
            }
        ]
    }
]

(We're also anxiously awaiting recursive types to be supported)

MattNewberry avatar Oct 10 '23 17:10 MattNewberry

Thanks for the info, everyone, helps as I'm working on the design for this.

czechboy0 avatar Oct 10 '23 17:10 czechboy0

Landed in main, will get released in 0.3.1.

czechboy0 avatar Oct 19 '23 14:10 czechboy0

Shipped in https://github.com/apple/swift-openapi-generator/releases/tag/0.3.1.

czechboy0 avatar Oct 19 '23 15:10 czechboy0

Thank you, I can confirm it's working for my schema!

herrernst avatar Oct 20 '23 13:10 herrernst

Glad to hear that. Just FYI for anyone else following up, I did find a scenario under which the cycle detector doesn't work correctly in a complex graph and am investigating it. If you hit an issue where the generated code doesn't compile, complaining about "infinite size" structs, please let me know here.

czechboy0 avatar Oct 20 '23 14:10 czechboy0

Here's the fix: https://github.com/apple/swift-openapi-generator/pull/335

czechboy0 avatar Oct 20 '23 22:10 czechboy0