bug: discriminator compilation fails for discriminators nested in oneOf

[kklash]

Context

Consider an object which can have three different shapes:

{ type: 'noop' }

{
  type: 'request',
  method: 'shout',
  word: 'hello',
}

{
  type: 'request',
  method: 'dance',
  dance_move: 'The Twist',
}

To validate such an object, i first set a discriminator on the type property, and branched based on that. Then, within the type: 'request' branch of oneOf, i specified another discriminator on the method property. However, Schemasafe fails to compile in this scenario, throwing the following error:

[discriminator]: has to be checked for type: \"object\" at #/oneOf/1

Note that we clearly did check the node for type: 'object' before applying the discriminator keyword.

Demo

This test case demonstrates the problem:

const schemasafe = require('@exodus/schemasafe')

describe('schemasafe', () => {
  it('should parse nested discriminators', () => {
    expect(() => {
      schemasafe.parser({
        $schema: 'https://json-schema.org/draft/2020-12/schema',
        type: 'object',
        additionalProperties: false,
        required: ['type'],
        discriminator: { propertyName: 'type' },
        properties: {
          type: {
            enum: ['noop', 'request'],
          },
        },
        oneOf: [
          {
            required: [],
            properties: {
              type: { const: 'noop' },
            },
          },
          {
            required: ['method'],
            discriminator: { propertyName: 'method' },
            properties: {
              type: { const: 'request' },
              method: {
                enum: ['shout', 'dance'],
              },
            },
            oneOf: [
              {
                required: ['dance_move'],
                properties: {
                  method: { const: 'dance' },
                  dance_move: {
                    type: 'string',
                    pattern: '^.*$',
                    maxLength: 100,
                  },
                },
              },
              {
                required: ['word'],
                properties: {
                  method: { const: 'shout' },
                  word: {
                    type: 'string',
                    pattern: '^[a-z]+$',
                    maxLength: 30,
                  },
                },
              },
            ],
          },
        ],
      })
    }).not.toThrow()
  })
})

Compilation fails even if type: 'object' is added to both of the relevant oneOf branches.

diff --git a/test/schema.test.js b/test/schema.test.js
index f3f60a2..241e958 100644
--- a/test/schema.test.js
+++ b/test/schema.test.js
@@ -32,6 +32,7 @@ describe('schemasafe', () => {
             },
             oneOf: [
               {
+                type: 'object',
                 required: ['dance_move'],
                 properties: {
                   method: { const: 'dance' },
@@ -43,6 +44,7 @@ describe('schemasafe', () => {
                 },
               },
               {
+                type: 'object',
                 required: ['word'],
                 properties: {
                   method: { const: 'shout' },

[kklash]

The error seems to originate at this line:

fix(functions.deepEqual(stat.type, ['object']), 'has to be checked for type:', 'object')

I logged the value of stat while running my demo test:

stat: {
    type: null,
    items: 0,
    properties: [ 'method', 'type' ],
    patterns: [],
    required: [ 'method' ],
    fullstring: false,
    dyn: { item: false, items: 0, properties: [Array], patterns: [] },
    unknown: false
}

Seems like stat is not properly kept in sync with the current validation node?

[olistic]

@ChALkeR Can you take a look?

[ChALkeR]

I see the error and a there is a work-around, working on a fix.

[ChALkeR]

There are three typecheck levels here:

1) top-level 2) discriminator oneOf branches 3) discriminator oneOf -> discriminator oneOf branches.

@olistic @kklash https://github.com/ExodusMovement/schemasafe/pull/177 should stop failing now if type: object is added to lvl 2 or level 3 subbranches, in addition or without the lvl 1 check.

Without #177 (i.e. on current releases), removing lvl1 check and leaving only 2/3 or a combo of them will also work.

A future fix #179 is needed to be able to specify only lvl 1 typecheck, it needs some testing to ensure nothing is broken.

---

Explanation:

Each discriminator expects either itself or each of it sub-branches to be typechecked to object.

Nested useless typechecks are ignored, but branches didn't report them as typechecked in the stat/delta info objects.

So, when lvl 1 typecheck was present, the safeguard in the second discriminator didn't see that all the branches are already checked. This happened regardless of if the second discriminator or it subbranches had type: object checks because those were short-circuited due to the presence of the typecheck check on lvl 1 which guaranteed that already and the actual validation logic traced that.

177 just adds the corresponding type info to the stat object in that "short-circuit" codepath, but only if it tries to typecheck again (for now).

179 solves this completely.

[ChALkeR]

Side note: you probably want unevaluatedProperties, not additionalProperties, as the latter one cares only about sibling, not nested properties rules, and will fail on e.g. {type:'request', method: 'dance'} as it only expects type.

[kklash]

Thanks for the tip @ChALkeR! Moving type: object down to the oneOf branches indeed solves the issue. :heavy_check_mark: