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

OAS 3.0 schemas cannot make non-nullable references nullable #1882

Open
liamnichols opened this issue Jan 23, 2022 · 7 comments
Open

OAS 3.0 schemas cannot make non-nullable references nullable #1882

liamnichols opened this issue Jan 23, 2022 · 7 comments
Labels

Comments

@liamnichols
Copy link

What version of Ajv are you using? Does the issue happen if you use the latest version?

8.9.0

Ajv options object

{allErrors: true}

JSON Schema

{
  "$defs": {
    "Fork": {
      "type": "object",
      "properties": {
        "id": {"type": "string"}
      }
    }
  },
  "type": "object",
  "properties": {
    "id": {"type": "string"},
    "dataFork": {"$ref": "#/$defs/Fork"},
    "resourceFork": {
      "nullable": true,
      "anyOf": [
        {"$ref": "#/$defs/Fork"}
      ]
    }
  }
}

Note: The issue relates to use of OAS 3.0's nullable keyword, but I replicated without using a complete OAS schema by using JSON schema's $defs instead.

Sample data

{
  "id": "ffff",
  "dataFork": {"id": "aaaa"},
  "resourceFork": null
}

Your code

const Ajv = require("ajv")
const ajv = new Ajv({allErrors: true})

const schema = {
  "$defs": {
    "Fork": {
      "type": "object",
      "properties": {
        "id": {"type": "string"}
      }
    }
  },
  "type": "object",
  "properties": {
    "id": {"type": "string"},
    "dataFork": {"$ref": "#/$defs/Fork"},
    "resourceFork": {
      "nullable": true,
      "anyOf": [
        {"$ref": "#/$defs/Fork"}
      ]
    }
  }
}

const validate = ajv.compile(schema)

test({
  "id": "ffff",
  "dataFork": {"id": "aaaa"},
  "resourceFork": null
})

function test(data) {
  const valid = validate(data)
  if (valid) console.log("Valid!")
  else console.log("Invalid: " + ajv.errorsText(validate.errors))
}

RunKit link - https://runkit.com/liamnichols/61ed680062e12c0008e66e2a

Validation result, data AFTER validation, error messages

error: "nullable" cannot be used without "type"

After adding "type": "object":

"Invalid: data/resourceFork must be object, data/resourceFork must match a schema in anyOf"

What results did you expect?

Sample data to validate without any errors

Are you going to resolve the issue?

I dug into the code, but I got stuck and I need guidance on how best to approach the problem.

@liamnichols
Copy link
Author

Hey 👋

We're looking to find a tool that allows us to validate the example payloads of our OAS 3.0.2 schema. We came across openapi-examples-validator, openapi-validator and spectral as good candidates for this however we've ran into a blocker based on the way that we define nullable properties on the object schemas within the spec. After a bit of work, I narrowed the root cause down to some handing within ajv, but I need some guidance since the way that many tools handle this scenario varies and I'm not sure what the "correct" approach is 😅

As a bit of background, I want to reference the following tickets:

In general, when writing OAS 3.0 schemas, its very common for you to define your object schemas in the components -> schemas section and then to reference these schemas across various other objects. The problem comes when an object needs referencing as a property in multiple places, but in some places the object might be nullable. In the example above, the schema defines a File property that has two properties that reference the Fork object: dataFork and resourceFork. While the object is the same in both cases, the resourceFork property can sometimes be null. Because Fork is used in numerous places where it's not nullable, we don't want to set the nullable property to true in the main definition since it would degrade the quality of the overall schema by suggesting that the object might be null when it wouldn't be.

We also don't want to redefine the Fork reference numerous times, especially because the OAS schema is later used by client codebases to generate entities (using openapi-generator and createapi).

From the linked ticket, and the support added to various other tools that we rely on (such as the generators linked above), I have concluded that the appropriate way to express our scenario in OAS 3.0 is to utilise nullable and a *Of array with a single object like in the sample code above.

After trying to understand this project a little more, I have concluded that since the primary focus here is JSON Schema, support for nullable is more of an add-on since the main focus isn't Open API schemas. The current implementation works by mapping nullable references to types: [t, "null"] internally however the problems that I am facing come because {type: ["object", "null"], "anyOf": [{"$ref": "..."}]} isn't a valid JSON schema in the sense that it would't make the referenced schema nullable.

I did however find that once I changed the resourceFork property definition to the following that the ajv worked as expected:

{
  "anyOf": [
    { "$ref": "#/$defs/Fork" },
    { "type": "object", "nullable": true }
  ]
}

It's great that this works, but unfortunately this approach does not play well with other OAS tools (specifically the generators that we rely on) which suggests to me that its not an appropriate way to solve this problem.

It does however give me an idea that maybe ajv could support this approach by looking for objects that define *Of (with 1 object) and nullable and then converting them into a single anyOf array like the above before running validation. Does this sound like something that the tool should do?

If not, I guess I can implement this myself, or maybe even in the OAS specific validator tools that depend on this package, but I wanted to start here before I went anywhere else 🙂

Let me know what you think! If you have a suggestion for how to approach solving the problem, I'd love to try and help out but I am not too familiar with the project so I'll probably need some guidance first.

Thanks!

@epoberezkin
Copy link
Member

nullable is defined as extension of type keyword that should be located in the same schema object, not as extension of any schema. To make it work you need to add type: object keyword as a sibling to nullable.

@liamnichols
Copy link
Author

Thanks @epoberezkin, I didn't know the specifics around that however after adding type: "object", there is another error. In my original comment, I mentioned the following:

After adding "type": "object":

"Invalid: data/resourceFork must be object, data/resourceFork must match a schema in anyOf"

I wanted to write the report using nullable on its own as well because all of the other tools that I have come across have interpreted the schema correctly without "type": "object", but if we put that aside anyway, does this count as a bug?

Here is a more complete OAS example:

openapi: 3.0.3
info:
  version: 1.0.0
  title: Sample API
servers:
  - url: https://cookpad.github.io/global-mobile-hiring
paths:
  /api/foos:
    get:
      responses:
        200:
          description: ''
          content:
            application/json:
              schema:
                type: array
                items: 
                  $ref: '#/components/schemas/Foo'
                
components:
  schemas:
    Foo:
      type: object
      required:
        - nullable_bar
        - non_nullable_bar
      properties:
        nullable_bar:
          type: object
          allOf: 
            - $ref: '#/components/schemas/Bar'
          nullable: true
          example: null
        non_nullable_bar:
          $ref: '#/components/schemas/Bar'
    Bar:
      type: object
      required:
        - id
      properties:
        id: 
          type: integer
          example: 42
      

@Tofandel
Copy link

Tofandel commented Jan 11, 2023

There is a bug that makes it unusable in typescript

interface test {
  test?: string | Array<string>
}

const schema1: JSONSchemaType<test> = {
  type: 'object',
  properties: {
    test: {
      // nullable: true,
      anyOf: [
        {
          type: 'string',
        },
        {
          type: 'array',
          items: {
            type: 'string',
          },
        },
      ],
    },
  },
}

Without nullable it throws a typescript error, with nullable it throws the runtime error "nullable" cannot be used without "type"

@pebo
Copy link

pebo commented May 3, 2023

@liamnichols - This issue is a bit old but if it's still relevant you could try adding nullable: true to the definition of Bar and see it that helps.

I've run into a similar issue (htps://github.com/cdimascio/express-openapi-validator/issues/839) and adding an extra nullable seems to be a possible workaround, i.e.

 Bar:
      type: object
      nullable: true
      required:
        - id
      properties:
        id: 
          type: integer
          example: 42

@liamnichols
Copy link
Author

Thanks @pebo for the suggestion.

The problem that we had with this (in our use case) is that code generators will consider any property with a ref to Bar as nullable, which will then allow us to mistakenly send null values to the API when they should be required in some cases 😬

@marco10024
Copy link

Hello @liamnichols, I had exactly the same problem and I found a solution:

my basic schema which gave me the same error:

            ...
            "value": {
                "nullable": true,
                "anyOf": [
                    {
                        "type": "string"
                    },
                    {
                        "type": "integer"
                    },
                    {
                        "type": "boolean"
                    }
                ]
            },
            ...

I went through an additional diagram and it's ok. well supported by AJV, and in my case I am on Fastify and fastify swagger and everything is ok

    {
        "$id": "FolderProductDataValue",
        "oneOf": [
            {
                "type": "number",
                "nullable": true
            },
            {
                "type": "string",
                "nullable": true
            },
            {
                "type": "boolean",
                "nullable": true
            }
        ]
    }
            ...
            "value": {
                "$ref": "FolderProductDataValue#"
            },
            ...
```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

5 participants