Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

References are not resolved according to RFC3986 #272

Open
hiddewie opened this issue Oct 14, 2021 · 3 comments · May be fixed by #676
Open

References are not resolved according to RFC3986 #272

hiddewie opened this issue Oct 14, 2021 · 3 comments · May be fixed by #676

Comments

@hiddewie
Copy link
Contributor

hiddewie commented Oct 14, 2021

See the specification here: https://swagger.io/docs/specification/using-ref/.
This points to RFC3986 (https://tools.ietf.org/html/rfc3986) for the structure of JSON references. This RFC is not supported in openapi-diff.

I made an example of a valid reference according to OpenAPI 3, for which OpenAPI diff throws an error. See #271

Example content:

openapi: 3.0.1
info:
  title: Service
  description: test
  version: test
  contact:
    name: test
    url: 'test'
servers:
  - url: 'localhost'
paths:
  /feature:
    get:
      summary: Get feature state
      operationId: feature
      description: Gets feature
      parameters:
        - name: feature
          in: query
          schema:
            $ref: '#/components/schemas/Feature/properties/feature'    # <---- this
          required: true
      responses:
        '200':
          description: Found feature
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Feature'
components:
  schemas:
    Feature:
      type: object
      properties:
        feature:
          type: string
          enum:
            - alpha
            - beta
        value:
          type: boolean
      required:
        - feature
        - value
@timur27
Copy link
Contributor

timur27 commented Dec 26, 2021

hi @hiddewie! Thanks for creating the issue. I did a quick investigation and this is the outcome:

  1. Although this is not the best way to refer the property (below described why), technically the scenario you described is perfectly possible.
  2. RefPointer class has the method resolveRef(components, t, ref), which in fact cuts off the base prefix and in our case returns /Feature/properties/feature.

There are 2 possible solutions I see so far:

  1. Treat such a definition as not supported and implement the proper handling (instead of ref #/components/schemas/Feature/properties/feature doesn't exist we would inform the user with the useful information that the diff checker doesn't support such reference syntax.
  2. Assume that among all the component types the 'nesting' reference is possible only for the property of Schema Object, and implement the solution (shortly: components.getSchemas().get(ref).getProperties().get(refProperty)).

Do you know who I can ping here in order to receive an opinion which direction we prefer to go? Maybe @joschi you could let us know what's your opinion? Thx in advance.

Referring the property of the schema object.

I didn't found any restrictions to use such way ($ref: '#/components/schemas/Feature/properties/feature') of referencing the component's (here schema's) children in the specification. However, as components are there for the user to define reusable things, I'd expect the definition to look like:

openapi: 3.0.1
info:
  title: Service
  description: test
  version: test
  contact:
    name: test
    url: 'test'
servers:
  - url: 'localhost'
paths:
  /feature:
    get:
      summary: Get feature state
      operationId: feature
      description: Gets feature
      parameters:
        - name: feature
          in: query
          schema:
            $ref: '#/components/schemas/reusableProperty'
          required: true
      responses:
        '200':
          description: Found feature
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Feature'
components:
  schemas:
    Feature:
      type: object
      properties:
        feature:
          $ref: #/components/schemas/reusableProperty'
        value:
          type: boolean
      required:
        - feature
        - value
    reusableProperty:
      type: string
      enum:
        - alpha
        - beta

@hiddewie
Copy link
Contributor Author

hiddewie commented Jan 3, 2022

Hi @timur27. Thanks for the reaction.

I know that this is not the best way to refer to properties. However, our users are free to use any valid OpenAPI structure in their documents. Unfortunately that also means that our users can use uncommon ways to make $refs. Other tooling does support this way of referencing other properties, so 'everything works' until OpenAPI-diff is used.

It would be good if openapi-diff should not implement a solution specific to this problem, but rather create a solution that supports any valid value of $ref (it does not have to be a components/schemas/... JSON pointer, it can be something else according to the RFC). As long as it points to a structure that defines a valid OpenAPI schema (or JSON schema from OpenAPI 3.1 onwards).

@thake
Copy link

thake commented Jan 17, 2022

Additional to the use case of @hiddewie, an RFC3986 compliant ref resolving would also support an OpenAPI-Spec that is splitted in multiple (possibly remote) files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants