fix: ignore defined type if anyOf and oneOf are defined

[Uzlopak]

Closes #290

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

lgtm

[ivan-tymoshenko]

We should catch a case when there is a type keyword at the top level or if a type is derived from other keywords and it's not equal to type in oneOf/anyOf/allOf.

Example1:

{
  type: 'object',
  oneOf: [
    { type: 'string' },
    { type: 'object' }
  ]
}

Example2:

{
  required: ['foo', 'bar'],
  oneOf: [
    { type: 'string' },
    { type: 'object' }
  ]
}

Ideally, we should catch this case with a isShemaValid, although it's hard to catch it there.

[Uzlopak]

isnt ajv already validating it?

[ivan-tymoshenko]

It shows a warning. IMHO, it should throw an error. Anyway, we should have tests for cases

[ivan-tymoshenko]

To fix this issue, you should merge (look how we merge allOf subschemas) each oneOf/anyOf option with a parent schema. Otherwise, you will fix one problem but create another.

Complex test:

test('should merge oneOf schema with parent schema', (t) => {
  t.plan(1)

  const schema = {
    properties: {
      bat: { type: 'string' }
    },
    oneOf: [
      {
        type: 'object',
        properties: {
          foo: { type: 'string' },
          bar: { type: 'string' }
        }
      },
      {
        type: 'object',
        properties: {
          foo: { type: 'string' },
          baz: { type: 'string' }
        }
      }
    ],
    required: ['foo']
  }

  const stringify = build(schema)

  t.equal(stringify({ bat: 'bat', foo: 'foo' }), '{"bat":"bat","foo":"foo"}')
  t.throws(() => stringify({ bat: 'bar' })) // foo is required
})

A simple test to show the breaking change:

test('breaking change', (t) => {
  t.plan(1)

  const schema = {
    properties: {
      foo: { type: 'string' }
    },
    oneOf: [{ type: 'object' }]
  }

  const stringify = build(schema)

  t.equal(stringify({ foo: 'foo' }), '{"foo":"foo"}')
})

[ivan-tymoshenko]

If you have some time to deal with it, you can think about merging nested oneOf/anyOf.

test('should merge nested oneOf schemas with parent schema', (t) => {
  t.plan(1)

  const schema = {
    type: 'object',
    oneOf: [
      {
        oneOf: [
          { required: ['test1'] },
          { required: ['test2'] }
        ]
      }
    ],
    required: ['test0']
  }

  const stringify = build(schema)

  t.throws(() => stringify({ test1: 'data' })) // test0 is missing
  t.throws(() => stringify({ test0: 'data' })) // test1 is missing
})

[Uzlopak]

That is basically a total rewrite of anyOf and oneOf logic.

[ivan-tymoshenko]

That is why this issue has been waiting for someone so long)

[ivan-tymoshenko]

I would start by creating a separate module with the mergeSchemas function. Maybe, even a separate npm package. It should deeply merge two or more schemas. And then use it here for merging oneOf/anyOf/allOf cases.

[Uzlopak]

I was under the shower and had the same thought while washing my hair.

We basically should have some schema helper which creates disjunct, union and intersections of two or more schemas.

So if we have an allOf, it should merge the schemas together.

But in case of oneOf we have to check if actually only one schema is matching, so we basically have to call every validation function per schema and check if we have exactly one matching schema and call its corresponding serializer fn.

In case of anyOf, we have to first create a matrix of merged schemas and then have to check which schemas are matching and then call based on the determined matching schemas the correct merged schema from the matrix

E.g. we have three schemas. We need 7 variations. We can store them in an array use the bitwise positions for the lookup. So

0. Error
1. (=001b) schema 1
2. (=010b) schema 2
3. (=011b) schema 1 + schema 2
4. (=100b) schema 3
5. (=101b) schema3 + schema 1
6. (=110b) schema 3 + schema 2
7. (=111b) schema 3 + schema 2 + schema 1

and in the for loop we validate them like

let serializerArrayPos = 0
for (let i = 0, il = schemas.length; i < il; ++i) {
if (validateBySchema(schemas[i]) (
serializerArrayPos += (1 >> i)
}
}

[ivan-tymoshenko]

1. We treat the oneOf keyword as the anyOf. Trying to check that only one schema matches the data is a validation task. It's not necessary to have it here. It's part of another discussion.
2. Not sure that you need disjunct, union, and intersections. You need only a union. If it's an allOf you need to create a union of all schemas (parent schema, child1 schema, child2 schema). If it's a oneOf or anyOf, you need to create a few unions of (parent schema, child1 schema), (parent schema, child2 schema).
3. The mergeSchema should have access to the refResolver, because you might have references inside the schema.
4. When you merge schemas, you should track both current locations: location in the original schema and location in the merged schema. Check fix: merged schemas reference resolving #546.

[Uzlopak]

Not the solution we need.