fast-json-stringify icon indicating copy to clipboard operation
fast-json-stringify copied to clipboard

oneOf/anyOf doesn't seem to work with objects

Open bluenote10 opened this issue 4 years ago • 17 comments

🐛 Bug Report

oneOf/anyOf doesn't seem to work with objects.

To Reproduce

The following schema models a union type, i.e. in TypeScript terms:

type Foo = { foo: string, baz: string };
type Bar = { bar: string, baz: string };
type FooBar = Foo | Bar;
const fastJson = require("fast-json-stringify");

const schemaFoo = {
  properties: {
    foo: { type: "string" },
    baz: { type: "string" },
  },
  required: ["foo", "baz"],
};

const schemaBar = {
  properties: {
    bar: { type: "string" },
    baz: { type: "string" },
  },
  required: ["bar", "baz"],
};

const stringify = fastJson({
  type: "object",
  anyOf: [schemaFoo, schemaBar],
});

console.log(stringify({ foo: "foo", baz: "baz" }));
console.log(stringify({ bar: "bar", baz: "baz" }));

Output:

{}
{}

Expected behavior

Since both objects match the schema, both should be stringified.

Your Environment

  • node version: v14.16.0
  • fastify version: 3.13.0
  • fast-json-stringify: 2.4.2
  • os: Ubuntu 18.04

bluenote10 avatar Mar 06 '21 22:03 bluenote10

Would you mind trying to add type: 'object' to both schemas?

mcollina avatar Mar 07 '21 01:03 mcollina

Yes I did that as well before, but it didn't make a difference.

bluenote10 avatar Mar 07 '21 07:03 bluenote10

this schema works:

const stringify = fastJson({
  anyOf: [schemaFoo, schemaBar]
})

Defining the type beside the anyOf would conflict when your data could be an object or a string like:

const stringify = fastJson({
  anyOf: [schemaFoo, schemaBar, { type: 'string' }]
})

Eomm avatar Mar 07 '21 18:03 Eomm

Good catch, it makes certainly more sense without specifying type in the top level schema. I'm not sure where I got that from. So perhaps there is no need to support that.

I just did a quick check and ran

{
  "type": "object",
  "anyOf": [
    {
      "properties": {
        "foo": { "type": "string" }
      }
    },
    {
      "properties": {
        "bar": { "type": "string" }
      }
    }
  ]
}

through the online JSON schema validator, which considers it valid against all schema drafts, but probably cannot verify such semantic violations anyway.

bluenote10 avatar Mar 07 '21 18:03 bluenote10

I think this is a bug in this module: we should throw if we cannot deal with this ambiguity. Better of, we could handle it if it wasn't really hard.

The schema is valid JSON schema (we validate it on creation).

mcollina avatar Mar 07 '21 21:03 mcollina

Would you like to send a PR?

mcollina avatar Mar 07 '21 21:03 mcollina

Sure, I can have a look, but it may take a bit until I get to it.

bluenote10 avatar Mar 07 '21 21:03 bluenote10

same issue on my side, trying to serialize

const schema = {
  type: 'array',
  items: {
    anyOf: [
      {
        type: 'object',
        required: [ 'id', 'createdAt', 'author', 'type', 'body' ],
        properties: {
          id: { type: 'string' },
          createdAt: { type: 'string', format: 'date-time' },
          author: {
            type: 'object',
            required: [ '_id', 'fullName' ],
            properties: { _id: { type: 'string' }, fullName: { type: 'string' } },
            additionalProperties: false
          },
          type: { type: 'string', enum: [ 'Text' ] },
          body: { type: 'string' }
        },
        additionalProperties: false
      },
      {
        type: 'object',
        required: [ 'id', 'createdAt', 'author', 'type', 'status' ],
        properties: {
          id: { type: 'string' },
          createdAt: { type: 'string', format: 'date-time' },
          author: {
            type: 'object',
            required: [ '_id', 'fullName' ],
            properties: { _id: { type: 'string' }, fullName: { type: 'string' } },
            additionalProperties: false
          },
          type: { type: 'string', enum: [ 'StatusTransition' ] },
          status: {
            type: 'string',
          }
        },
        additionalProperties: false
      }
    ]
  }
}

And I get :

{
  "messages": [
    null
  ]
}

Update: if I add type: "object" to anyOf I get

{
  "messages": [
    {}
  ]
}

Update 2: Full example to test: https://gist.github.com/stalniy/05f3940d5a55d80a38557edd8ffbfe14

stalniy avatar Jul 16 '21 16:07 stalniy

is there any support for discriminated union types?

stalniy avatar Jul 16 '21 17:07 stalniy

is there any support for discriminated union types?

As a general rule, I suggest running the debugMode whenever there is some issue to understand what is going on.

Debugging a bit your gist with this feature I found:

  • if you change the schema removing the type, the serializer function output would be fine. this is a bug in FJS and it would be great if you would like to fix it (ignore the type when there is the anyOf keyword)
  items: {
-    type: "object",
    anyOf: [
  • looking deeply at the schema compared to the input object, the ids field must be strings instead of integer

With these two changes, your code will works 👍🏽

Eomm avatar Jul 17 '21 08:07 Eomm

Are there any known workarounds at the moment for using the discriminator keyword? When setting strict to true, it requires a type of object at the top level, so the current state is between a rock and hard place:

  1. Remove the top-level type, set it on each oneOf schema instead, and disable strict mode type checks.
  2. Keep the top-level type but always get an empty object back.

Actually, it looks like going with option 1 allows arbitrary non-objects to pass validation and fall through, so that's no good :( It seems like the only thing that works at the moment is modifying the schema before it's used as an output schema in Fastify and setting the relevant property to an empty object.

ghost avatar Jan 10 '22 09:01 ghost

discriminator keyword?

No: it is a OpenAPI keyword and fast-json-stringify requires a JSON Schema Draft 7

Eomm avatar Jan 14 '22 17:01 Eomm

discriminator keyword?

No: it is a OpenAPI keyword and fast-json-stringify requires a JSON Schema Draft 7

Sorry, to clarify, it's not that I need support for the discriminator keyword specifically, but rather that keyword uses oneOf, and since the discriminator keyword requires type set at the top level, it hits this bug where it simply returns an empty object in serialization. Bumping the type down into the oneOf subschemas fixes the serialization part, but breaks validating things properly, but then ajv throws warnings/errors by default since the top-level type keyword is missing.

ghost avatar Jan 14 '22 19:01 ghost

@stalniy Here is another implementation based on AJV JTD Serializer that should ensure better soundness regarding the validation of the schema cc @epoberezkin

const Ajv = require("ajv/dist/jtd");

const ajv = new Ajv(); // options can be passed, e.g. {allErrors: true}

const schemaFoo = {
  properties: {
    id: { type: "string" },
    createdAt: { type: "timestamp" },
    author: {
      properties: { _id: { type: "string" }, fullName: { type: "string" } },
    },
    type: { enum: ["Test"] },
    body: { type: "string" },
  },
};

const schemaBar = {
  properties: {
    id: { type: "string" },
    createdAt: { type: "timestamp" },
    author: {
      properties: { _id: { type: "string" }, fullName: { type: "string" } },
    },
    type: { enum: ["StatusTransition"] },
    status: {
      type: "string",
    },
  },
};

const schema = {
  elements: {
    metadata: {
      union: [schemaFoo, schemaBar],
    },
  },
};

// const dataFoo = [
//   {
//     type: "Test",
//     body: "bla",
//     id: "1",
//     createdAt: new Date(),
//     author: {
//       _id: "2",
//       fullName: "John Doe",
//     },
//   },
// ];

const dataBar = [
  {
    type: "StatusTransition",
    status: "bla",
    id: "1",
    createdAt: new Date(),
    author: {
      _id: "2",
      fullName: "John Doe",
    },
  },
];

const validate = ajv.compile(schema);
const serialize = ajv.compileSerializer(schema);

function validateAndSerialize(data) {
  const valid = validate(data);

  if (valid) {
    console.log(serialize(data));
  } else {
    console.log("invalid", validate.errors);
  }
}

for (const data of [
  // dataFoo,
  dataBar,
]) {
  validateAndSerialize(data);
}

gfortaine avatar Jan 22 '23 14:01 gfortaine

I seem to have ran in this issue with a GeoJSON structure in which I had a oneOf of Polygon or MultiPolygon schema on a geometry property.

It scrambled the body of the request, altering the first coordinates of the geometry. Shall I add more info here or rather open a new issue?

thom4parisot avatar Aug 07 '23 15:08 thom4parisot

This issue needs somebody to lead its implementation, anyOf and oneOf have likely the same bugs.

mcollina avatar Aug 08 '23 07:08 mcollina