fluent-json-schema icon indicating copy to clipboard operation
fluent-json-schema copied to clipboard

Object type ignored when nested properties use oneOf and raw

Open ghost opened this issue 2 years ago • 11 comments

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched existing issues to ensure the bug has not already been reported

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.

ghost avatar Nov 17 '23 17:11 ghost

@aboutlo @mcollina anyhing on this?

ghost avatar Nov 27 '23 10:11 ghost

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

aboutlo avatar Nov 27 '23 11:11 aboutlo

@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

ghost avatar Nov 27 '23 14:11 ghost

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 avatar Jan 26 '24 17:01 globalexport

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

globalexport avatar Jan 26 '24 17:01 globalexport

Thanks, would you like to send a PR?

mcollina avatar Jan 26 '24 18:01 mcollina

@mcollina any suggestions on where to start?

ghost avatar Jan 26 '24 21:01 ghost

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 avatar Jan 26 '24 21:01 Uzlopak

@giovanni-bertoncelli @globalexport

Proposed PR #237 . Review and give comments please.

Uzlopak avatar Jan 26 '24 22:01 Uzlopak

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/[email protected]/node_modules/ajv/dist/core.js:266:23)
      at Ajv._addSchema (/Users/me/projects/test/node_modules/.pnpm/[email protected]/node_modules/ajv/dist/core.js:460:18)
      at Ajv.addSchema (/Users/me/projects/test/node_modules/.pnpm/[email protected]/node_modules/ajv/dist/core.js:234:34)
      at new ValidatorCompiler (/Users/me/projects/test/node_modules/.pnpm/@[email protected]/node_modules/@fastify/ajv-compiler/lib/validator-compiler.js:37:16)
      at buildCompilerFromPool (/Users/me/projects/test/node_modules/.pnpm/@[email protected]/node_modules/@fastify/ajv-compiler/index.js:32:22)
      at SchemaController.setupValidator (/Users/me/projects/test/node_modules/.pnpm/[email protected]/node_modules/fastify/lib/schema-controller.js:112:56)
      at Boot.<anonymous> (/Users/me/projects/test/node_modules/.pnpm/[email protected]/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/[email protected]/node_modules/avvio/boot.js:160:12

globalexport avatar Jan 29 '24 09:01 globalexport

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

globalexport avatar Jan 29 '24 09:01 globalexport