fastify / fluent-json-schema

A fluent API to generate JSON schemas
MIT License
495 stars 58 forks source link

Object type ignored when nested properties use oneOf and raw #233

Open giovanni-bertoncelli opened 11 months ago

giovanni-bertoncelli commented 11 months ago

Prerequisites

Fastify version

4.23.2

Plugin version

4.2.1

Node.js version

20.x

Operating system

macOS

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

13.4

Description

When I'm creating a schema like this:

const schema = S.object().prop(
  "content",
  S.object()
    .raw({ type: "object", discriminator: { propertyName: "type" } })
    .oneOf([schema1, schema2]),
);

The type: object is omitted in the resulting JSON Schema both event if I use S.object() and raw type object. This can throw errors for example when using Ajv in strict mode. I do not know if the problem it's caused by the oneOf or the raw clause.

Steps to Reproduce

You can see a full reproduction example here: https://codesandbox.io/p/sandbox/infallible-cloud-4kw5xm. Run npm start.

Expected Behavior

The resulting JSON schema should not be this:

{
  '$schema': 'http://json-schema.org/draft-07/schema#',
  type: 'object',
  properties: { content: { discriminator: [Object], oneOf: [Array] } }
}

but this:

{
  '$schema': 'http://json-schema.org/draft-07/schema#',
  type: 'object',
  properties: { content: { type: 'object', discriminator: [Object], oneOf: [Array] } }
}

It seems that this issue is not present on schemas root level oneOf/raw.

giovanni-bertoncelli commented 10 months ago

@aboutlo @mcollina anyhing on this?

aboutlo commented 10 months ago

Hey @giovanni-bertoncelli thank you for raising this. Can you try to write the tests for isolating the issue and if there is the issue propose a PR? You can start from here by adding a test covering your use case: https://github.com/fastify/fluent-json-schema/blob/c962de0ab7b2f1a0a8e95c31ecd7adce77eae133/src/utils.test.js#L6

giovanni-bertoncelli commented 10 months ago

@aboutlo I have no clear idea on how to add this test case. I think this may be come from the oneOf part of that schema, but I do not know what specifically to test since I can't see any test about setComposeType util

globalexport commented 8 months ago

I can confirm this behavior in my project.

Node: v20.11.0 fastify: v4.25.2 fastify-plugin: v4.5.1 fluent-json-schema: v4.2.1

import fp from 'fastify-plugin'
import S from 'fluent-json-schema'

export default fp((fastify, opts, next) => {
  const addSchema = (schema) => {
    fastify.addSchema(schema)
    return schema
  }

  // this doesn't work due to missing necessary type: 'object' for data

  // addSchema(
  //   S.object()
  //     .additionalProperties(true)
  //     .id('JobPublished')
  //     .prop(
  //       'data',
  //       S.object()
  //         .required()
  //         .oneOf([
  //           S.ifThen(S.object().prop('key', S.integer()), S.required(['key'])),
  //           S.ifThen(S.object().prop('keys', S.array()), S.required(['keys']))
  //         ])
  //     )
  // )

  // this works as expected:
  addSchema({
    $schema: 'http://json-schema.org/draft-07/schema#',
    type: 'object',
    additionalProperties: true,
    $id: 'JobPublished',
    properties: {
      data: {
        type: 'object', // <--- this is missing
        oneOf: [
          {
            if: {
              properties: {
                key: {
                  type: 'integer'
                }
              }
            },
            then: {
              required: ['key']
            }
          },
          {
            if: {
              properties: {
                keys: {
                  type: 'array'
                }
              }
            },
            then: {
              required: ['keys']
            }
          }
        ]
      }
    },
    required: ['data']
  })

  next()
})

strict mode: missing type "object" for keyword "properties" at "JobPublished#/properties/data/oneOf/0/if" (strictTypes) strict mode: missing type "object" for keyword "required" at "JobPublished#/properties/data/oneOf/0/then" (strictTypes) strict mode: missing type "object" for keyword "properties" at "JobPublished#/properties/data/oneOf/1/if" (strictTypes) strict mode: missing type "object" for keyword "required" at "JobPublished#/properties/data/oneOf/1/then" (strictTypes)

globalexport commented 8 months ago

Forgot to mention that the change of behaviour occurred after updating fastify, fastify-plugin and fluent-json-schema from v3 to v4.

mcollina commented 8 months ago

Thanks, would you like to send a PR?

giovanni-bertoncelli commented 8 months ago

@mcollina any suggestions on where to start?

Uzlopak commented 8 months ago

https://github.com/fastify/fluent-json-schema/blob/ec486485c18cdc35e9cd41ec55e252d5c0402e5a/src/ObjectSchema.js#L305

changing it to const type = attributes.type fixes your issue. But it can not be THE solution, as you would need to drill down the combinedKeywords and determine what values type can be.

it('should set type to the potential types based on the combinedKeyword oneOf', () => {
    const schema = S.object()
      .additionalProperties(true)
      .id('JobPublished')
      .prop(
        'data',
        S
          .required()
          .oneOf([
            S.integer(),
            S.array(),
          ])
      ).valueOf()
    expect(schema.properties.data.type).toEqual(['integer', 'array'])
  })
Uzlopak commented 8 months ago

@giovanni-bertoncelli @globalexport

Proposed PR #237 . Review and give comments please.

globalexport commented 8 months ago

Hi @Uzlopak!

In my case, the fix is resulting in:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "additionalProperties": true,
  "$id": "JobPublished",
  "properties": {
    "data": {
      "type": [
        "object",
        null
      ],
      "oneOf": [
        {
          "if": {
            "properties": {
              "key": {
                "type": "integer"
              }
            }
          },
          "then": {
            "required": [
              "key"
            ]
          }
        },
        {
          "if": {
            "properties": {
              "keys": {
                "type": "array"
              }
            }
          },
          "then": {
            "required": [
              "keys"
            ]
          }
        }
      ]
    }
  },
  "required": [
    "data"
  ]
}

This is giving me the following schema error:

Error: schema is invalid: data/properties/data/type must be equal to one of the allowed values, data/properties/data/type/1 must be equal to one of the allowed values, data/properties/data/type must match a schema in anyOf
      at Ajv.validateSchema (/Users/me/projects/test/node_modules/.pnpm/ajv@8.12.0/node_modules/ajv/dist/core.js:266:23)
      at Ajv._addSchema (/Users/me/projects/test/node_modules/.pnpm/ajv@8.12.0/node_modules/ajv/dist/core.js:460:18)
      at Ajv.addSchema (/Users/me/projects/test/node_modules/.pnpm/ajv@8.12.0/node_modules/ajv/dist/core.js:234:34)
      at new ValidatorCompiler (/Users/me/projects/test/node_modules/.pnpm/@fastify+ajv-compiler@3.5.0/node_modules/@fastify/ajv-compiler/lib/validator-compiler.js:37:16)
      at buildCompilerFromPool (/Users/me/projects/test/node_modules/.pnpm/@fastify+ajv-compiler@3.5.0/node_modules/@fastify/ajv-compiler/index.js:32:22)
      at SchemaController.setupValidator (/Users/me/projects/test/node_modules/.pnpm/fastify@4.25.2/node_modules/fastify/lib/schema-controller.js:112:56)
      at Boot.<anonymous> (/Users/me/projects/test/node_modules/.pnpm/fastify@4.25.2/node_modules/fastify/lib/route.js:394:32)
      at Object.onceWrapper (node:events:632:28)
      at Boot.emit (node:events:530:35)
      at /Users/me/projects/test/node_modules/.pnpm/avvio@8.2.1/node_modules/avvio/boot.js:160:12
globalexport commented 8 months ago

I created some examples here:

Invalid schema: https://www.jsonschemavalidator.net/s/WwSDDh5d

Valid: https://www.jsonschemavalidator.net/s/TtIL1q1K https://www.jsonschemavalidator.net/s/7kqlWmB9