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

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

Open
2 tasks done
giovanni-bertoncelli opened this issue Nov 17, 2023 · 11 comments
Open
2 tasks done

Comments

@giovanni-bertoncelli
Copy link

Prerequisites

  • I have written a descriptive issue title
  • 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.

@giovanni-bertoncelli
Copy link
Author

@aboutlo @mcollina anyhing on this?

@aboutlo
Copy link
Collaborator

aboutlo commented Nov 27, 2023

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:

describe('setRaw', () => {

@giovanni-bertoncelli
Copy link
Author

@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
Copy link

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
Copy link

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

@mcollina
Copy link
Member

Thanks, would you like to send a PR?

@giovanni-bertoncelli
Copy link
Author

@mcollina any suggestions on where to start?

@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 26, 2024

const type = hasCombiningKeywords(attributes)

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
Copy link
Contributor

Uzlopak commented Jan 26, 2024

@giovanni-bertoncelli
@globalexport

Proposed PR #237 . Review and give comments please.

@globalexport
Copy link

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
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants