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

OneOf between arrays of object with nullable fields that are null breaks the serialization #246

Closed
bgatellier opened this issue Jul 30, 2020 · 3 comments · Fixed by #500
Closed
Labels
bug Confirmed bug good first issue Good for newcomers

Comments

@bgatellier
Copy link

You have already researched for similiar issues?

Yes, and mine is somewhat similar to this one : #175

What are you trying to achieve or the steps to reproduce?

Context:

I am using fastify with default options.
I am using a schema to validate and serialize the output of my API endpoint.
This schema includes a oneOf between two arrays of objects (see below for details).
The objects contains properties that are nullable(as of OpenAPI 3.0).

Error :

The error happens when the nullable fields are null.
In that case, the array of the API response is a list of null instead of objects with null properties

Validation Schema (simplified, because there are way more properties in the arrays):

{
  "type": "object",
  "properties": {
    "data": {
      "oneOf": [
        {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "id": { "type": "integer", "minimum": 1 }
            },
            "additionalProperties": false,
            "required": [
              "id"
            ]
          }
        },
        {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "job": { "type": "string", "nullable": true }
            },
            "additionalProperties": false,
            "required": [
              "job"
            ]
          }
        }
      ]
    }
  },
  "required": ["data"],
  "additionalProperties": false
}

Output data BEFORE the validation and serialization steps:

{
    "data": [
        {
            "job": null,
        }
    ]
}

What was the result you received?

{
    "data": [
        null
    ]
}

What did you expect?

{
    "data": [
        {
            "job": null,
        }
    ]
}

Context

  • node version: 12
  • fastify version: 2.13.0
  • os: Mac
  • any other relevant information: validation works properly, as I reproduced it with Ajv (example below). It seems that there is a problem with the serialization.
const Ajv = require("ajv")

const ajv = new Ajv({
  coerceTypes: true,
  nullable: true,
  removeAdditional: true,
  useDefaults: true
})

const validate = ajv.compile({
  "type": "object",
  "properties": {
    "data": {
      "oneOf": [
        {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "id": { "type": "integer", "minimum": 1 }
            },
            "additionalProperties": false,
            "required": [
              "id"
            ]
          }
        },
        {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "job": { "type": "string", "nullable": true }
            },
            "additionalProperties": false,
            "required": [
              "job"
            ]
          }
        }
      ]
    }
  },
  "required": ["data"],
  "additionalProperties": false
})

const isValid = validate({
  "data": [
    {
      "job": null
    }
  ]
})

console.log(isValid) // true
@bgatellier bgatellier changed the title OneOf between arays of object with nullable fields that are null breaks the serialization OneOf between arrays of object with nullable fields that are null breaks the serialization Jul 30, 2020
@mcollina
Copy link
Member

This looks like a bug in fast-json-stringify. I'm moving this there.

Would you like to send a PR?

@mcollina mcollina transferred this issue from fastify/help Jul 30, 2020
@bgatellier
Copy link
Author

Thanks for the quick answer.

I do not have time to submit a PR at the moment.

I hope I could do that later, but I can't promise anyhting :/

@mcollina mcollina added bug Confirmed bug good first issue Good for newcomers labels Jul 30, 2020
@jkirkpatrick24
Copy link

I'm not sure this is actually a fast-json-stringify bug. Support for the nullable property in ajv isn't enable by default. You need to explicitly turn it on as an option (as you have done in your example).

If you provide that option to fast-json-stringify when it is initialized, the above schema will work.

For example, this test passes:

// build is fast-json-stringify
test('can configure the nullable property', (t) => {
  t.plan(2)

  const schema = {
    type: 'object',
    properties: {
      data: {
        oneOf: [
          {
            type: 'array',
            items: {
              type: 'object',
              properties: {
                job: { type: 'string', nullable: true }
              },
              additionalProperties: false,
              required: [
                'job'
              ]
            }
          },
          {
            type: 'array',
            items: {
              type: 'object',
              properties: {
                id: { type: 'integer', minimum: 1 }
              },
              additionalProperties: false,
              required: [
                'id'
              ]
            }
          }
        ]
      }
    },
    required: ['data'],
    additionalProperties: false
  }
  const stringifyWithNullable = build(schema, { ajv: { nullable: true } })
  const stringify = build(schema)

  const input = {
    data: [
      {
        job: null
      }
    ]
  }

  t.is(stringify(input), JSON.stringify({ data: null }))
  t.is(stringifyWithNullable(input), JSON.stringify({ data: [{ job: null }] }))
})

Is fastify meant to support the nullable property by default? The validation documentation also shows marking a property as nullable by setting the type property like this:

{ type: ['string', 'null' ]}

So I think the solution here is to just provide the necessary AJV options, but I may be missing something. Hope thats helpful 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants