Skip to content

Resolve: Better way to resolve allOf #208

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

SOHELAHMED7
Copy link
Contributor

@SOHELAHMED7 SOHELAHMED7 commented Sep 11, 2024

Fixes cebe/yii2-openapi#165

Also part of php-openapi/yii2-openapi#10

@cebe

At this moment allOf wiil be only resolved for OpenAPI entities classes inheriting from SpecBaseObject.
If this pull request is approved and merged I will create similar pull request for Callbacks, Responses and Paths

@SOHELAHMED7
Copy link
Contributor Author

SOHELAHMED7 commented Sep 14, 2024

Requirements:

Simple example of resolving a property:

Schema:

openapi: 3.0.3

info:
  title: Resolve property
  version: 1.0.0

components:
  schemas:
    User:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: integer
        name:
          type: string
    Post:
      type: object
      properties:
        id:
          type: integer
        content:
          type: string
        user:
          $ref: '#/components/schemas/User'

paths:
  '/':
    get:
      responses:
        '200':
          description: OK

After resolving the references:

components:
  schemas:
    User:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: integer
        name:
          type: string
    Post:
      type: object
      properties:
        id:
          type: integer
        content:
          type: string
        user:
          type: object
          required:
            - id
            - name
          properties:
            id:
              type: integer
            name:
              type: string

In the same manner allOf with reference and custom OpenAPI extension with the schema:

    Post:
      type: object
      properties:
        id:
          type: integer
        content:
          type: string
        user:
          allOf:
            - $ref: '#/components/schemas/User'
            - x-faker: false

must be resolved as:

    Post:
      type: object
      properties:
        id:
          type: integer
        content:
          type: string
        user:
          type: object
          required:
            - id
            - name
          properties:
            id:
              type: integer
            name:
              type: string
          x-faker: false

So we can access x-faker in same way as we use type, required and properties of User component schema:

/** @var \cebe\openapi\spec\Schema $user A property (similar to "content" and "id") of a Post component schema */
$user->type # 'object'
$user->required # [0 => 'id', 1 => 'name']
$user->{'x-faker'} # `false`

At this moment references are resolved but not allOf:

Array &0 (
    'type' => 'object'
    'properties' => Array &1 (
        'id' => Array &2 (
            'type' => 'integer'
        )
        'content' => Array &3 (
            'type' => 'string'
        )
        'user' => Array &4 (
            'allOf' => Array &5 (
                0 => Array &6 (
                    'required' => Array &7 (
                        0 => 'id'
                        1 => 'name'
                    )
                    'type' => 'object'
                    'properties' => Array &8 (
                        'id' => Array &9 (
                            'type' => 'integer'
                        )
                        'name' => Array &10 (
                            'type' => 'string'
                        )
                    )
                )
                1 => Array &11 (
                    'x-faker' => false
                )
            )
        )
    )
)

@SOHELAHMED7
Copy link
Contributor Author

If a duplicate property is found

components:
  schemas:
    User:
      type: object
      required:
        - id
        - name # <--------------------------------------------------------------
      properties:
        id:
          type: integer
        name: # <--------------------------------------------------------------
          type: string
          maxLength: 10 # <--------------------------------------------------------------
    Pet:
      type: object
      required:
        - id2
        - name # <--------------------------------------------------------------
      properties:
        id2:
          type: integer
        name: # <--------------------------------------------------------------
          type: string
          maxLength: 12 # <--------------------------------------------------------------
    Post:
      type: object
      properties:
        id:
          type: integer
        content:
          type: string
        user:
          allOf:
            - $ref: '#/components/schemas/User'
            - $ref: '#/components/schemas/Pet'
            - x-faker: true

then property from the last component schema will be considered:

Post:
  type: object
  properties:
    id:
      type: integer
    content:
      type: string
    user:
      type: object
      required:
        - id
        - name # <--------------------------------------------------------------
        - id2
      properties:
        id:
          type: integer
        name: # <--------------------------------------------------------------
          type: string
          maxLength: 12 # <--------------------------------------------------------------
        id2:
          type: integer
      x-faker: true

@SOHELAHMED7 SOHELAHMED7 changed the title Draft: Better way to resolve allOf Resolve: Better way to resolve allOf Sep 23, 2024
@SOHELAHMED7 SOHELAHMED7 marked this pull request as ready for review September 23, 2024 14:15
@simPod
Copy link
Contributor

simPod commented Sep 23, 2024

You'll have better chance in https://github.com/DEVizzent/cebe-php-openapi since this repo is abandoned.

Copy link
Owner

@cebe cebe left a comment

Choose a reason for hiding this comment

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

Not sure about the recursive call. This can be a significant performance problem. I'd keep the resolveAllOf part on the Schema only and call it if needed.

@cebe
Copy link
Owner

cebe commented Nov 15, 2024

@SOHELAHMED7 can you explain more about why you implemented the recursive resolution of allOf? allOf is a property of Schema and it only make sense to call it on a schema object. I'd expect the resolveAllOf() function inside of Schema and see it applied to sub-schemas only.

@SOHELAHMED7
Copy link
Contributor Author

SOHELAHMED7 commented Nov 16, 2024

can you explain more about why you implemented the recursive resolution of allOf?

I think allOf and "reference" are very similar. "reference" can be nested so as allOf. allOf can have "reference". I have added a test case for this.. Also resolving allOf will require resolving "reference".

So I implemented resolving allOf similar to how "reference" are resolved.

SOHELAHMED7 and others added 3 commits November 18, 2024 17:54
Co-authored-by: Carsten Brandt <mail@cebe.cc>
Co-authored-by: Carsten Brandt <mail@cebe.cc>
Co-authored-by: Carsten Brandt <mail@cebe.cc>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better way to resolve allOf
3 participants