oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

Properly create models for allOf schemas with properties additionally defined

Open DouglasDwyer opened this issue 3 years ago • 9 comments

The types generator does not properly create structure types for models of the following format. The properties defined directly on the model are not included in the output struct:

SessionIdentity:
  required:
  - websocket_url
  type: object
  allOf:
     - $ref: '#/components/schemas/SessionData'
  properties:
    websocket_url:
      type: string
      description: websocket url

This is because models that contain an allOf are constructed by merging the properties of all base types together. However, properties which are defined on the actual model itself are not considered. This PR addresses the problem by including the actual schema instance during allOf merging.

DouglasDwyer avatar Aug 10 '22 15:08 DouglasDwyer

I'm not sure your approach is valid. If anything, I think the properties would end up being those you describe on the base schema, overriding the results of allOf. I could be mistaken there though.

chanced avatar Aug 10 '22 16:08 chanced

@chanced Thanks for your feedback. Currently, the issue that we are facing with this is that the generated model only contains the fields of the inherited types (in the case of the example, that would be SessionData). The changes in this PR add the properties on the schema itself, as well as all of its allOf schemas, to the list of schemas to merge together.

Specifically, when it comes time to generate a "merged" schema containing all properties, I begin by creating a copy of the original schema. I set its allOf property to nil, to avoid an infinite egress. Then I iterate over the original schema's allOfs, merging them together with the copied schema, to create a merged result.

The result is a struct with the allOf schema properties and the schema properties themselves merged together.

DouglasDwyer avatar Aug 10 '22 16:08 DouglasDwyer

My point is is that nested objects don't get merged in JSON schema without being in either a oneOf or anyOf. I'm just not 100% certain which would take precedent. I'm fairly certain the base schema's properties would though.

To achieve what you are after, use the schema:

SessionIdentity:
  required:
  - websocket_url
  type: object
  allOf:
     - $ref: '#/components/schemas/SessionData'
     - type: object
        properties:
          websocket_url:
            type: string
            description: websocket url

chanced avatar Aug 10 '22 16:08 chanced

@chanced I see - your concern is with regard to the OpenAPI standard itself? Unfortunately, I couldn't find any relevant place in the standard where the behavior for mixing properties and allOf is defined. However, both the official Swagger generator and the general OpenAPI generator properly handle this case, by generating a struct with both sets of properties. Plus, according to this StackOverflow answer and the associated source, the given schema with properties should behave the same as the schema example that you gave.

I agree that placing the properties inside of an allOf tag would be a workaround for this issue. However, wouldn't it be better for the behavior to match that of OpenAPI generator? Thanks.

DouglasDwyer avatar Aug 10 '22 17:08 DouglasDwyer

Ah, I may be mistaken regarding properties. If so, I apologize.

chanced avatar Aug 10 '22 18:08 chanced

@DouglasDwyer I was completely wrong about the merging of properties. I apologize for adding noise to your pull request.

I meant to mention this earlier and forgot.

chanced avatar Aug 12 '22 16:08 chanced

I wanted to lend some support for this pull-request as I too ran into the same issue. I cloned this pull-request and tried it on my full REST service OpenAPI 3.1 spec and it produced identical results as the previous merged allOf schema.

With this PR Schema

User:
  required:
    - id
    - firstName
  allOf:
    - type: object
      properties:
        id:
          type: string
    - $ref: '#/components/schemas/UserProperties'

Produces the same result as

User:
  allOf:
    - type: object
      properties:
        id:
          type: string
    - $ref: '#/components/schemas/UserProperties'
    - type: object
      required:
        - id
        - firstName

This PR resolves #931

Swiftwork avatar Jan 21 '23 20:01 Swiftwork

Any update on this? I am running into this issue with the following spec of an external service:

UsersPaginated:
            description: List of users with pagination information
            type: object
            allOf:
                - $ref: '#/components/schemas/PageInfo'
            properties:
                items:
                    description: >-
                            The list of users
                    type: array
                    items:
                        $ref: '#/components/schemas/User'

Generated object:

// UsersPaginated defines model for UsersPaginated.
type UsersPaginated struct {
	// CurrentPage the number of the current page
	CurrentPage *int `json:"currentPage,omitempty"`

	// ItemsPerPage the number of items per page
	ItemsPerPage *int `json:"itemsPerPage,omitempty"`

	// SortBy the field that sorts the results
	SortBy *string `json:"sortBy,omitempty"`

	// SortOrder the sort order (asc or desc)
	SortOrder *string `json:"sortOrder,omitempty"`

	// TotalItems the total number of items
	TotalItems *int `json:"totalItems,omitempty"`

	// TotalPages the total number of pages
	TotalPages *int `json:"totalPages,omitempty"`
}

If I use https://editor.swagger.io/ the items field is detected, so it doesn't seem to be an issue with the spec declaration. Also, if I use openapi-generator-cli It generates the code with the items field.

befc avatar Feb 23 '24 11:02 befc