fastify/fast-json-stringify

allOf Schema and additionalProperties result in unexpected result

artem-kliuev opened this issue · 10 comments

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the regression has not already been reported

Last working version

5.11.1

Stopped working in version

5.12.0

Node.js version

21.x

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

14.2

💥 Regression Report

allOf schema stringify doesn't work as expected. See details below.

Steps to Reproduce

Here is example:

import FJS from 'fast-json-stringify';
import util from 'util';

const AJV_CONFIG = { removeAdditional: 'all', allErrors: true, coerceTypes: true, strictTypes: false };

const schema = {
    allOf: [
        {
            type: 'object',
            properties: {
                id: {
                    type: 'integer'
                },
                parent_id: {
                    type: ['null', 'integer'],
                    default: null
                },
                name: {
                    type: 'string'
                }
            },
            required: ['id', 'name']
        },
        {
            type: 'object',
            properties: {
                integrations: {
                    type: 'array',
                    items: {
                        type: 'object',
                        properties: {
                            id: {
                                type: 'integer'
                            },
                            domain: {
                                type: 'string'
                            },
                            is_enabled: {
                                type: 'boolean'
                            }
                        },
                        required: ['id', 'domain', 'is_enabled']
                    }
                }
            },
            required: ['integrations']
        },
        {
            type: 'object',
            properties: {
                parent: {
                    oneOf: [
                        {
                            type: 'object',
                            properties: {
                                id: {
                                    type: 'integer'
                                },
                                name: {
                                    type: 'string'
                                }
                            },
                            required: ['id', 'name']
                        },
                        {
                            type: 'null'
                        }
                    ],
                    default: null
                }
            }
        }
    ]
};

const s = FJS({ additionalProperties: false, ...schema }, { ajv: AJV_CONFIG });

const input = {
    id: 1,
    name: 'Name',
    integrations: [
        {
            id: 1,
            domain: 'example.com',
            is_enabled: 1
        }
    ],
    parent_id: 1,
    parent: {
        id: 1,
        name: 'Name'
    }
};

console.log(util.inspect(JSON.parse(s(input)), false, null, true));

Expected Behavior

Version 5.11.1

Expected result:

{
    id: 1,
    parent_id: 1,
    name: 'Name',
    integrations: [{ id: 1, domain: 'example.com', is_enabled: true }],
    parent: { id: 1, name: 'Name' }
}

Next, if i comment down integrations.domain inside input object i should receive error:

Error: "domain" is required!

Version 5.12.0

Next in version 5.12.0 result will be following:

{
  id: 1,
  name: 'Name',
  integrations: [ { id: 1, domain: 'example.com', is_enabled: 1 } ],
  parent_id: 1,
  parent: { id: 1, name: 'Name' }
}

We can see that integrations.is_enabled has wrong type.

Next, if i comment down integrations.domain inside input object i should receive error, but got this:

{
  id: 1,
  name: 'Name',
  integrations: [ { id: 1, is_enabled: 1 } ],
  parent_id: 1,
  parent: { id: 1, name: 'Name' }
}

No error fired and domain property is missed. Remove additional props is not working as well.

Thank you for attention.
Cheers.

Your example is using allOf but the title tells anyOf.
Which one is the one not working?

@climba03003 yep my bad, i've updated title and details

cc @ivan-tymoshenko it might be a bug in your latest changes

I cannot reproduce your problem.
Can you please double check if your example is correct?

import { mergeSchemas } from '@fastify/merge-json-schemas'
import { build } from 'fast-json-stringify'

const schema = {
  allOf: [
      {
          type: 'object',
          properties: {
              id: {
                  type: 'integer'
              },
              parent_id: {
                  type: ['null', 'integer'],
                  default: null
              },
              name: {
                  type: 'string'
              }
          },
          required: ['id', 'name']
      },
      {
          type: 'object',
          properties: {
              integrations: {
                  type: 'array',
                  items: {
                      type: 'object',
                      properties: {
                          id: {
                              type: 'integer'
                          },
                          domain: {
                              type: 'string'
                          },
                          is_enabled: {
                              type: 'boolean'
                          }
                      },
                      required: ['id', 'domain', 'is_enabled']
                  }
              }
          },
          required: ['integrations']
      },
      {
          type: 'object',
          properties: {
              parent: {
                  oneOf: [
                      {
                          type: 'object',
                          properties: {
                              id: {
                                  type: 'integer'
                              },
                              name: {
                                  type: 'string'
                              }
                          },
                          required: ['id', 'name']
                      },
                      {
                          type: 'null'
                      }
                  ],
                  default: null
              }
          }
      }
  ]
}

const merged = mergeSchemas(schema.allOf)

console.log(JSON.stringify(merged, null, 2))

const input = {
  id: 1,
  name: 'Name',
  integrations: [
      {
          id: 1,
          domain: 'example.com',
          is_enabled: 1
      }
  ],
  parent_id: 1,
  parent: {
      id: 1,
      name: 'Name'
  }
}


const stringify = build(merged)

const result = stringify(input)
console.log(result)

const stringify2 = build(schema)
const result2 = stringify2(input)
console.log(result2)

I see where the problem comes from.
It only appear when you have explicitly provide additionalProperties: false

@climba03003 hmm, yep i've double checked it now and found that

build({ additionalProperties: false, ...schema }

additionalProperties: false is the problem.

@climba03003 yep, seems i should update my codebase then. Thank you for fast response, you're very helpful! Have a good day, guys

It should be a bug that need to fix though.
additionalProperties: false poison the whole schema merging process.

If I understand @climba03003 correctly than this is a bug.
Reopening this issue

@climba03003 Close please If i misunderstood

In the test, it seems like expected result.

https://github.com/fastify/merge-json-schemas/blob/main/test/properties.test.js#L138-L167

It is easy to have additional additionalProperties: false inside the schema.
The current merging not a desirable one for fast-json-stringify.

I would say we should either

  1. Warn the user if they provide exact { additionalProperties: false }.
  2. Only fallback when additionalProperties is object.