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

Bundler and linter fail when discriminator value is component name #1877

Open
tobiaspatton-s4 opened this issue Feb 1, 2025 · 5 comments
Open
Labels
p3 Type: Bug Something isn't working

Comments

@tobiaspatton-s4
Copy link

tobiaspatton-s4 commented Feb 1, 2025

Describe the bug

The OAS 3.1 spec states that the value of a discriminator mapping can be be a string that is the name of a component schema in the file:

The mapping entry maps a specific property value to either a different schema component name, or to a schema identified by a URI

And the spec provides examples like:

discriminator:
propertyName: petType
mapping:
dog: Dog

However when such a OAS schema is sent to the redocly cli linter or bundler it results in an error: "Can't resolve $ref"

To Reproduce
Pass this file to redocly lint or redocly bundle

openapi: "3.1.0"
info:
  title: pets
  version: 1.0.0
  license:
    name: Apache 2.0
    identifier: Apache-2.0

servers:
  - url: http://not.real.com/pets.yaml

paths:
  /get-pets:
    get:
      description: get pets
      summary: get pets
      operationId: get_pets
      responses:
        200:
          description: ok
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Animals"
        400:
          description: not ok

components:
  schemas:
    Animals:
      type: object
      properties:
        elements:
          type: array
          items:
            oneOf:
              - $ref: "#/components/schemas/Cat"
              - $ref: "#/components/schemas/Dog"
            discriminator:
              propertyName: species
              mapping:
                dog: Dog
                cat: Cat

    Dog:
      type: object
      required:
        - species
      properties:
        species:
          type: string
          const: dog
        rating:
          type: number

    Cat:
      type: object
      required:
        - species
      properties:
        species:
          type: string
          const: cat
        tuxedo:
          type: boolean

  securitySchemes:
    Bearer:
      type: http
      scheme: bearer
      description: JWT auth0 token (without the word 'Bearer')
      bearerFormat: JWT

security:
  - Bearer: []

Expected behavior

The linter should not report errors.

Logs

OpenAPI description

Redocly Version(s)

1.28.0

Node.js Version(s)

v20.18.1

OS, environment

Mac OS 14.7.2

Additional context

The linter and bundler can be made to pass by changing the file:

            discriminator:
              propertyName: species
              mapping:
                dog: "#/components/schemas/Dog"
                cat: "#/components/schemas/Cat"
@tobiaspatton-s4 tobiaspatton-s4 added the Type: Bug Something isn't working label Feb 1, 2025
@jeremyfiel
Copy link
Contributor

related to #1602

This particular comment has a good breakdown of each type of reference.
#1602 (comment)

The bug described here is directly related to this.

Explicit Mapping - Value:
If the mapping section is provided and the value DOES NOT being with ./ then it will be inferred as a name for a schema found under the Component section of the spec

From the updated 3.1.1 spec.

The mapping entry maps a specific property value to either a different schema component name, or to a schema identified by a URI. When using implicit or explicit schema component names, inline oneOf or anyOf subschemas are not considered. The behavior of a mapping value that is both a valid schema name and a valid relative URI reference is implementation-defined, but it is RECOMMENDED that it be treated as a schema name.

Here the discriminating value of dog will map to the schema #/components/schemas/Dog, rather than the default (implicit) value of #/components/schemas/dog. If the discriminating value does not match an implicit or explicit mapping, no schema can be determined and validation SHOULD fail.

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
        mapping:
          dog: Dog
    Cat:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Cat`
          properties:
            name:
              type: string
    Dog:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Dog`
          properties:
            bark:
              type: string
    Lizard:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Lizard`
          properties:
            lovesRocks:
              type: boolean

@jeremyfiel
Copy link
Contributor

OK, narrowed this down to the rule no-unresolved-refs which is incorrectly reporting the error.

DiscriminatorMapping(mapping, { report, resolve, location }) {
for (const mappingName of Object.keys(mapping)) {
const resolved = resolve({ $ref: mapping[mappingName] });
if (resolved.node !== undefined) return;

The discriminator mapping itself is working correctly, this can be verified by this example

openapi: 3.1.0
info:
  title: pets
  version: 1.0.0
  license:
    name: Apache 2.0
    identifier: Apache-2.0

servers:
  - url: http://not.real.com/pets.yaml

paths:
  /get-pets:
    get:
      description: get pets
      summary: get pets
      operationId: get_pets
      responses:
        200:
          description: ok
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Animals"
              examples: 
                valid_test: 
                  value: 
                    elements:
                      - species: dog
                        rating: 1
                invalid_test: 
                  value: 
                    elements:
                      - species: dog
                        rating: true
        400:
          description: not ok

components:
  schemas:
    Animals:
      type: object
      properties:
        elements:
          type: array
          items:
            oneOf:
              - $ref: "#/components/schemas/Cat"
              - $ref: "#/components/schemas/Dog"
            discriminator:
              propertyName: species
              mapping:
                dog: Dog
                cat: Cat

    Dog:
      type: object
      required:
        - species
      properties:
        species:
          const: dog
        rating:
          type: number

    Cat:
      type: object
      required:
        - species
      properties:
        species:
          const: cat
        tuxedo:
          type: boolean

  securitySchemes:
    Bearer:
      type: http
      scheme: bearer
      description: JWT auth0 token (without the word 'Bearer')
      bearerFormat: JWT

security:
  - Bearer: []

The expected error is from the invalid_test example.

Example value must conform to the schema: `rating` property type must be number.

@jeremyfiel
Copy link
Contributor

played with a few changes but nothing fixed just yet. Seems this is where it's failing to resolve

const resolvedRef = resolvedRefMap.get(refId);

other changes i've made,

from this:

DiscriminatorMapping(mapping, { report, resolve, location }) {
for (const mappingName of Object.keys(mapping)) {
const resolved = resolve({ $ref: mapping[mappingName] });

to this:

DiscriminatorMapping(mapping, { report, resolve, location }) {
      for (const mappingName in mapping) {
        const resolved = resolve({ $ref: `${mapping[mappingName]}` });

added this test to the no-unresolved-refs.test.ts

it('should not report on discriminator mapping explicit value', async () => {
    const document = parseYamlToDocument(
      outdent`
        openapi: 3.1.0
        info:
          title: pets
          version: 1.0.0
        servers:
          - url: http://not.real.com/pets.yaml
        paths:
          /get-pets:
            get:
              description: get pets
              responses:
                200:
                  description: ok
                  content:
                    application/json:
                      schema:
                        $ref: "#/components/schemas/Animals"
                      examples: 
                        valid_test: 
                          value: 
                            elements:
                              - species: dog
                                rating: 1
                        invalid_test: 
                          value: 
                            elements:
                              - species: dog
                                rating: true
                400:
                  description: not ok
        components:
          schemas:
            Animals:
              type: object
              properties:
                elements:
                  type: array
                  items:
                    oneOf:
                      - $ref: "#/components/schemas/Cat"
                      - $ref: "#/components/schemas/Dog"
                    discriminator:
                      propertyName: species
                      mapping:
                        dog: Dog
                        cat: Cat
            Dog:
              type: object
              required:
                - species
              properties:
                species:
                  const: dog
                rating:
                  type: number
            Cat:
              type: object
              required:
                - species
              properties:
                species:
                  const: cat
                tuxedo:
                  type: boolean

      `,
      path.join(__dirname, 'foobar.yaml')
    );

    const results = await lintDocument({
      externalRefResolver: new BaseResolver(),
      document,
      config: await makeConfig({
        rules: {
          'no-unresolved-refs': 'error',
        },
      }),
    });

    expect(replaceSourceWithRef(results, __dirname)).toMatchInlineSnapshot(``);
  });

@jeremyfiel
Copy link
Contributor

jeremyfiel commented Feb 1, 2025

export function isMappingRef(mapping: string) {
// TODO: proper detection of mapping refs
return (
mapping.startsWith('#') ||
mapping.startsWith('https://') ||
mapping.startsWith('http://') ||
mapping.startsWith('./') ||
mapping.startsWith('../') ||
mapping.indexOf('/') > -1
);

This is tangentially related and could be another method to resolve these mapping refs rather than the resolvedRefMap method.

@tatomyr
Copy link
Contributor

tatomyr commented Feb 3, 2025

Looks like a bug indeed. Until it's fixed, you can try working around it with a custom decorator to prepend the schema names in DiscriminatorMapping with #/components/schemas/ or something similar.

@tatomyr tatomyr added the p3 label Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3 Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants