Fix bug where object schemas representing an `allOf` type with more than one schema but only one property were inferred as the wrong type by liamnichols · Pull Request #181 · CreateAPI/CreateAPI

liamnichols · Feb 21, 2023

Background

It's a bit of a mouthful, but we ran into an issue with a very specific schema definition:

components:
  schemas:
    A:
      type: object
      required:
      - kind
      properties:
        kind:
          type: string
    Four:
      allOf:
      - $ref: "#/components/schemas/A"
      - type: object

The above schema describes two types, type A, which has a single property called kind which is a String and type Four, which is actually identical to A.

It's a unique situation, but when we attempt to use Four in a polymorphic schema that uses a discriminator, it's type is incorrectly inferred as demonstrated in 0d04732:

CreateAPI/Tests/Support/Specs/discriminator.yaml

Lines 81 to 98 in 0d04732

    
           AnotherContainer: 
        
             type: object 
        
             required: 
        
               - content 
        
             properties: 
        
               content: 
        
                 oneOf: 
        
                   - $ref: "#/components/schemas/One" 
        
                   - $ref: "#/components/schemas/Two" 
        
                   - $ref: "#/components/schemas/Three" 
        
                   - $ref: "#/components/schemas/Four" 
        
                 discriminator: 
        
                   propertyName: kind 
        
                   mapping: 
        
                     one: "#/components/schemas/One" 
        
                     two: "#/components/schemas/Two" 
        
                     three: "#/components/schemas/Three" 
        
                     four: "#/components/schemas/Four"

CreateAPI/Tests/Support/Snapshots/discriminator/Sources/Entities/AnotherContainer.swift

Lines 6 to 35 in 0d04732

    
           public struct AnotherContainer: Codable { 
        
               public var content: Content 
        
               public enum Content: Codable { 
        
                   case a(A) 
        
                   case three(Three) 
        
                   case string(String) 
        
                   public init(from decoder: Decoder) throws { 
        
                       struct Discriminator: Decodable { 
        
                           let kind: String 
        
                       } 
        
                       let container = try decoder.singleValueContainer() 
        
                       let discriminatorValue = try container.decode(Discriminator.self).kind 
        
                       switch discriminatorValue { 
        
                       case "one": self = .a(try container.decode(A.self)) 
        
                       case "two": self = .a(try container.decode(A.self)) 
        
                       case "three": self = .three(try container.decode(Three.self)) 
        
                       case "four": self = .string(try container.decode(String.self)) 
        
                       default: 
        
                           throw DecodingError.dataCorruptedError( 
        
                               in: container, 
        
                               debugDescription: "Discriminator value '\(discriminatorValue)' does not match any expected values (one, two, three, four)." 
        
                           ) 
        
                       } 
        
                   }

Instead of the AnotherContainer type containing a four(Four) case, it ends up with string(String), which is absolutely not what we want.

We traced it back to this code:

CreateAPI/Sources/CreateAPI/Generator/Generator+Schemas.swift

Lines 494 to 503 in da8730b

    
           // TODO: Improve this and adopt for other types (see Zoom spec) 
        
           if properties.count == 1 { 
        
               var property = properties[0] 
        
               if let nested = property.nested as? EntityDeclaration, nested.name.rawValue == "Object" { 
        
                   nested.name = name 
        
                   property.nested = nested 
        
                   property.type = .userDefined(name: name) 
        
               } 
        
               return TypealiasDeclaration(name: name, type: property.type, nested: property.nested) 
        
           }

Changes

It's not entirely clear what the special Zoom code was trying to achieve, but when running CreateAPI against the Zoom API spec again, it turns out that the presence of this code makes no difference to the output. For that reason, I can only assume that it's now redundant and given that its presence causes a bug in other usage, we've decided to remove it.

If we do for some reason realise that we need this code back, it could always return as a vendor extension, but realistically I don't think that will happen. I'd also rather look into the exact motivation and figure out how to support that in other ways anyway.

After removing this code, I am able to generate the expected representation of AnotherContainer which is committed as part of this Pull Request.

Note: I noticed an unrelated bug when adding this snapshot configuration where AnyJSON has been generated when it shouldn't have been. It has been reported separately as #182

… property

…ix issue with disrciminator types

liamnichols added 2 commits February 21, 2023 16:25

reproduce issue with discriminator type when object only has a single…

0d04732

… property

Remove edge-case workaround code for Zoom spec that is redundant to f…

419f0d1

…ix issue with disrciminator types

liamnichols self-assigned this Feb 21, 2023

liamnichols mentioned this pull request Feb 21, 2023

AnyJSON type included unexpectedly #182

Open

liamnichols marked this pull request as ready for review February 21, 2023 15:47

liamnichols merged commit 0ba6538 into main Feb 22, 2023

liamnichols deleted the fix-bug-with-discriminator branch February 22, 2023 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fix bug where object schemas representing an `allOf` type with more than one schema but only one property were inferred as the wrong type#181

liamnichols commented Feb 21, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	AnotherContainer:
	type: object
	required:
	- content
	properties:
	content:
	oneOf:
	- $ref: "#/components/schemas/One"
	- $ref: "#/components/schemas/Two"
	- $ref: "#/components/schemas/Three"
	- $ref: "#/components/schemas/Four"
	discriminator:
	propertyName: kind
	mapping:
	one: "#/components/schemas/One"
	two: "#/components/schemas/Two"
	three: "#/components/schemas/Three"
	four: "#/components/schemas/Four"

	public struct AnotherContainer: Codable {
	public var content: Content

	public enum Content: Codable {
	case a(A)
	case three(Three)
	case string(String)

	public init(from decoder: Decoder) throws {

	struct Discriminator: Decodable {
	let kind: String
	}

	let container = try decoder.singleValueContainer()
	let discriminatorValue = try container.decode(Discriminator.self).kind

	switch discriminatorValue {
	case "one": self = .a(try container.decode(A.self))
	case "two": self = .a(try container.decode(A.self))
	case "three": self = .three(try container.decode(Three.self))
	case "four": self = .string(try container.decode(String.self))

	default:
	throw DecodingError.dataCorruptedError(
	in: container,
	debugDescription: "Discriminator value '\(discriminatorValue)' does not match any expected values (one, two, three, four)."
	)
	}
	}

	// TODO: Improve this and adopt for other types (see Zoom spec)
	if properties.count == 1 {
	var property = properties[0]
	if let nested = property.nested as? EntityDeclaration, nested.name.rawValue == "Object" {
	nested.name = name
	property.nested = nested
	property.type = .userDefined(name: name)
	}
	return TypealiasDeclaration(name: name, type: property.type, nested: property.nested)
	}

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

Comments

Conversation

liamnichols commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

liamnichols commented Feb 21, 2023 •

edited

Loading