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
- Warn the user if they provide exact
{ additionalProperties: false }
. - Only fallback when
additionalProperties
isobject
.