Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Discriminator Support#10

Merged
kean merged 4 commits intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom
PhilipTrauner:discriminatorPhilipTrauner/CreateAPI:discriminatorCopy head branch name to clipboard
Feb 3, 2022
Merged

Discriminator Support#10
kean merged 4 commits intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom
PhilipTrauner:discriminatorPhilipTrauner/CreateAPI:discriminatorCopy head branch name to clipboard

Conversation

@PhilipTrauner
Copy link
Contributor

Adds initial support for decoding oneOf entities with a discriminator attribute.

Using the existing property type name machinery when constructing the discriminator is still outstanding.
Would really appreciate some help with that, because I struggled to find a clean way to call into one of the name canonicalization functions.

Generated Code
public struct A: Codable {
    public var kind: String

    public init(kind: String) {
        self.kind = kind
    }
}

public struct B: Codable {
    public var kind: String

    public init(kind: String) {
        self.kind = kind
    }
}

public struct C: Codable {
    public var kind: String

    public init(kind: String) {
        self.kind = kind
    }
}

public struct Container: Codable {
    public var content: Content

    public enum Content: Codable {
        case a(A)
        case b(B)
        case c(C)

        public init(from decoder: Decoder) throws {

            struct Discriminator: Decodable {
                let kind: String
            }

            let container = try decoder.singleValueContainer()

            switch try? container.decode(Discriminator.self) {
            case .some(let inner) where inner.kind == "a":
                self = .a(try container.decode(A.self))
            case .some(let inner) where inner.kind == "b":
                self = .b(try container.decode(B.self))
            case .some(let inner) where inner.kind == "c":
                self = .c(try container.decode(C.self))
            default:
                throw DecodingError.dataCorruptedError(in: container, debugDescription: "Failed to initialize `oneOf`")
            }
        }

        public func encode(to encoder: Encoder) throws {
            var container = encoder.singleValueContainer()
            switch self {
            case .a(let value): try container.encode(value)
            case .b(let value): try container.encode(value)
            case .c(let value): try container.encode(value)
            }
        }
    }

    public init(content: Content) {
        self.content = content
    }
}

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
    }
}
Schema
openapi: "3.0.0"
info:
  version: 1.0.0
  title: Discriminator

paths:
  /container:
    get:
      responses:
        "200":
          description: ""
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Container"
components:
  schemas:
    A:
      type: object
      required:
        - kind
      properties:
        kind:
          type: string
    B:
      type: object
      required:
        - kind
      properties:
        kind:
          type: string
    C:
      type: object
      required:
        - kind
      properties:
        kind:
          type: string

    Container:
      type: object
      required:
        - content
      properties:
        content:
          oneOf:
            - $ref: "#/components/schemas/A"
            - $ref: "#/components/schemas/B"
            - $ref: "#/components/schemas/C"
          discriminator:
            propertyName: kind
            mapping:
              a: "#/components/schemas/A"
              b: "#/components/schemas/B"
              c: "#/components/schemas/C"

if let innerMapping = discriminator.mapping {

// TODO: Use type name machinery instead
mapping = innerMapping.mapValues { value in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should work:

mapping = innerMapping.mapValues { value in
    guard let key = OpenAPI.ComponentKey(rawValue: value) else {
        throw GeneratorError("TBD") // This should never happen
    }
    // This takes care of options.rename.
    // It _doesn't_ take options.entities.include / exclude into the account.
    // Ideally CreateAPI should throw an error (or warning if --strict is disabled) if
    // the type is exlucded (neat-picking).
    guard let name = getTypeName(for: key) else {
        // TODO:
    }
    return .userDefined(name: name)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guard let key = OpenAPI.ComponentKey(rawValue: value) else {
    throw GeneratorError("TBD") // This should never happen
}

The rawValue: constructor enforces that the provided value matches this regex: ^[a-zA-Z0-9\.\-_]+$.
At this stage the mapping values are still prefixed with "#/components/schemas/", which does not conform to said regex.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I just tested it and yes, the string value should be the component name (without "#/components/schemas/").

String(value.dropFirst("#/components/schemas/".count))

That's a nice approach for removing the prefix. I would've usually used replacingOccurrences, but this is much better.

@kean
Copy link
Member

kean commented Jan 28, 2022

I suggest to cleanup the generated code a bit. I think this should be functionally equivalent:

switch try container.decode(Discriminator.self) {
case "a": self = .a(try container.decode(A.self))
// ...
}

@kean
Copy link
Member

kean commented Jan 29, 2022

I fixed the test target in main. I need to restructure the package a bit. I'll do it once this PR is merged so you don't have to move anything / resolve conflicts.

@PhilipTrauner PhilipTrauner marked this pull request as ready for review February 3, 2022 10:03
@PhilipTrauner PhilipTrauner changed the title Discriminator Draft Discriminator Support Feb 3, 2022
@kean
Copy link
Member

kean commented Feb 3, 2022

Thanks, @PhilipTrauner. Merging it.

I'll address the following later today and add the expected test output to the repo:

// TODO: Find it automatically
var projectPath = ("~/Developer/CreateAPI/" as NSString).expandingTildeInPath

It's needed for "snapshot" testing. The idea is that when you add a new test, and it can't find the expected files in the repo, it generates them for you to commit.

@kean kean merged commit f40ae92 into CreateAPI:main Feb 3, 2022
@PhilipTrauner PhilipTrauner deleted the discriminator branch February 3, 2022 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.