fix: Allow setting type: "object" when using Boolean JSON Schema combination keywords (allOf, anyOf, oneOf, not)

[beninato8]

This PR removes hasCombiningKeywords, and instead always sets type to attributes.type. It also adds test cases for the expected behavior.

---

I am using fastify fluent-json-schema for API validation. I noticed an issue where every time the API is started, my logs have multiple warnings related to missing schema properties.

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

These are coming from Ajv, which has the default option of logging issues that violate the rules for strictTypes.

My schema responsible for violating strictTypes is like this (see the allOf inside parent group in FluentSchema.integration.test.js):

const schema = S.object()
  .prop('parent', S.object()
    .prop('name', S.string().enum(['foo', 'bar']).required())
    .prop('index', S.number().required())
    .allOf([
      S.ifThen(
        S.object().prop('name', S.const('foo')),
        S.object().prop('index', S.const(0))
      ),
      S.ifThen(
        S.object().prop('name', S.const('bar')),
        S.object().prop('index', S.const(1))
      )
    ])
  ).required()
  .valueOf()

I know I can silence these warnings if I pass { strictTypes: false } as Ajv options when initializing fastify, but I wanted to see if I could fix the warning instead of modifying the default options.

I found that the issue is due to the output from fluent-json-schema missing an expected type: "object" under the parent property.
However, I was unable to get it to show up - S.object().allOf(), S.raw({ schemaWithTypeObject }), and S.allOf().raw({ type: "object" }) all were missing the expected type field.

{
  $schema: 'http://json-schema.org/draft-07/schema#',
  type: 'object',
  properties: {
    parent: {
      type: 'object', // this was always missing
      allOf: [
        ...
      ]
    }
  }
}

This behavior happens due to explicitly setting type to undefined if there are any combining keywords.

const hasCombiningKeywords = (attributes) =>
  attributes.allOf || attributes.anyOf || attributes.oneOf || attributes.not

const type = hasCombiningKeywords(attributes)
  ? undefined
  : attributes.type

After looking through the commit history, it seems like this behavior has existed since beginning, aside from being moved around files.

• Nov 13, 2018 (added support for anyOf) 50a357a
• Nov 13, 2018 (extract to hasCombiningKeywords) c94156e
• Nov 13, 2018 (move hasCombiningKeywords to utils) 0ab3fb2
• Feb 1, 2019 (removed hasCombiningKeywords usage from FluentSchema) f6975b1
• Feb 1, 2019 (initial commit for ObjectSchema uses hasCombiningKeywords, copied from FluentSchema) 88ac13b

I'm not sure what the original reason was for implementing it this way, but I believe that the current behavior should be changed, as combining keywords should be able to specify a type.

For example, the docs for json-schema have this:

{
  "type": "number",
  "oneOf": [
    { "multipleOf": 5 },
    { "multipleOf": 3 }
  ]
}

And here are some examples from Ajv:

This simple JSON Schema is valid, but it violates strictTypes:

{
  properties: {
    foo: {type: "number"},
    bar: {type: "string"}
  }
  required: ["foo", "bar"]
}

This is a very common mistake that even people experienced with JSON Schema often make - the problem here is that any value that is not an object would be valid against this schema - this is rarely intentional.

To fix it, "type" keyword has to be added:

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

You do not necessarily have to have "type" keyword in the same schema object; as long as there is "type" keyword applying to the same part of data instance in the same schema document, not via "$ref", it will be ok:

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

Both "properties" and "required" need type: "object" to satisfy strictTypes - it is sufficient to have it once in the parent schema, without repeating it in each schema.

After looking into solving this issue, I found #233. It looks like previous attempt to fix it was closed (#237) which used type unions.

This PR is not trying to determine the types of everything inside of a schema combination; instead, it just fixes the current behavior of fluent-json-schema which prevents the creation of a valid schema, even if S.raw is used.

As seen in the tests added to FluentSchema.test.js, any usage of an operator like S.allOf() will remain the same - the only time that type is included is if it is specified like S.object().allOf(). This allows the creation of schemas that pass Ajv strict mode, which requires type: "object".

While type is not a requirement for any of these boolean operators, it should still be an option if desired, instead of always being forced to be undefined.

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

This PR closes #233