[Bug] False positive: "The discriminator property name used must be defined in this schema"

[liamnichols]

Hello, I'm evaluating this tool for use with our project and have spotted an issue with one of the rules that we can't actually opt out of that produces the following error against my spec:

The discriminator property name used must be defined in this schema

The error appears to be produced by the builtin rule, which I can't seem to opt-out of, but I believe that its a false positive due to the way that we're using allOf references in our polymorphic object definition. I have put together the following schema to reproduce this issue:

Schema

{
  "openapi": "3.0.3",
  "info": {
    "title": "Test",
    "version": "0.0.1"
  },
  "paths": {

  },
  "components": {
    "schemas": {
      "Polymorphic": {
        "anyOf": [
          {
            "$ref": "#/components/schemas/Foo"
          },
          {
            "$ref": "#/components/schemas/Bar"
          }
        ],
        "discriminator": {
          "propertyName": "type",
          "mapping": {
            "foo": "#/components/schemas/Foo",
            "bar": "#/components/schemas/Bar"
          }
        }
      },
      "Foo": {
        "type": "object",
        "required": [
          "type"
        ],
        "properties": {
          "type": {
            "type": "string",
            "enum": ["foo"]
          },
          "foo": {
            "type": "string"
          }
        }
      },
      "Bar": {
        "type": "object",
        "required": [
          "type"
        ],
        "properties": {
          "type": {
            "type": "string",
            "enum": ["bar"]
          },
          "bar": {
            "type": "string"
          }
        }
      }
    }
  }
}

Note 1: This technique follows guidance described in Swaggers docs: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism

Note 2: mapping isn't necessarily related, but I use it in my real use-case so wanted to include it. You can also achieve the same using anyOf and oneOf. I can't remember the reason why we use anyOf but I imagine that we'd need to support both.

When validating the above schema, I get the following output:

$ lint-openapi schema.json

errors

  Message :   The discriminator property name used must be defined in this schema
  Path    :   components.schemas.Polymorphic.discriminator.propertyName
  Line    :   22

I think that I was able to trace it back to the following code:

openapi-validator/packages/validator/src/plugins/validation/oas3/semantic-validators/discriminator.js

Lines 40 to 49 in 668f2e7

	if (!has(properties, propertyName)) {
	messages.addMessage(
	basePath
	.concat([schemaName, 'discriminator', 'propertyName'])
	.join('.'),
	'The discriminator property name used must be defined in this schema',
	'error'
	);
	return;
	}

The check here is relatively simple and is expecting that properties contains type, but it doesn't account for allOf/oneOf/anyOf.

If I can help to patch this, please let me know! I'm not familiar with the project so I'm not sure of the approach that we should be taking to resolve an absolute list of properties. Alternatively I guess that having a way to just disable this rule could also be an option.

Thanks!

[dpopp07]

Hey @liamnichols, thanks for the issue. This does look like a false positive to me and probably needs a fix in the code. You're more than welcome to open a PR to resolve! Otherwise, I will try to patch this next week. I think a better check would be to see if the property exists in properties or, in the case of a oneOf/anyOf, make sure it is present in all child schemas or, in the case of an allOf, make sure it is present in at least one schema. What do you think about that?

[liamnichols]

Thanks @dpopp07, I agree! In fact, I did start having a go at patching it however I got a little stuck since I ran into a secondary issue since the contents of *Of could in fact be a reference to other schemas and the object passed into discriminator.js hadn't been dereferenced 😄

I'm not too familiar with the codebase (or Javascript/Node for that matter) so there might be an easier way to solve it, but I'm not sure... If you have any further suggestions then I can take a look but if its a bit more complex than that then maybe I'll let you take a further look 😅

Alternatively, I was wondering.. Is there a reason that these builtin checks can't be disabled? It would be ideal if it could work, but at the same time, I think it might just be something that would be handy if we could disable should it cause problems with complex schemas

[dpopp07]

@liamnichols Sounds good. To answer your last question, the idea behind the builtin checks is that they are fundamental to the OpenAPI specification and therefore any API definition that breaks these rules is technically not a valid OpenAPI document. That said, I don't think this rule was meant to be that kind of rule.

Since that is also a component of the issue, I'll take a pass at fixing this one and in the process I'll convert it into a Spectral-formatted rule so that it can be configured as well (this is the direction we're planning on going in for all rules, eventually).

I'll be able to work on that this week

[liamnichols]

Thanks @dpopp07, I appreciate it 🚀 Let me know if I can be of any help

[dpopp07]

I opened a PR for this. It grew a little in scope but it resolves this issue

[ibm-devx-sdk]

🎉 This issue has been resolved in version 0.2.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[ibm-devx-sdk]

🎉 This issue has been resolved in version 0.54.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[liamnichols]

Thanks @dpopp07, I'll give this a go shortly! 🚀