Support mapping keyword when discriminator is used

[mefellows]

avj is the library we use to parse the JSON schema. It currently does not support the mapping keyword, making it cumbersome for users who follow this style.

There is a PR for AVJ to add this support, however it has not yet been accepted: https://github.com/ajv-validator/ajv/pull/2262/files.

Other possible solutions might include transforming the OAS before to (somehow) address this.

[mefellows]

There was a response from the maintainer of ajv the other day (essentially brushing aside the request, from what I can see, and ignoring the bit about us happy to create a PR if they are open to it - presumably they are not). I have responded because I wasn’t entirely satisfied with the reasoning. This being said, there may be some light at the end of the tunnel.

It looks like an uphill battle though getting a change into ajv, so I think plan B (or beyond) is needed.

I’ve done a small spike locally, there are a couple of options that might work.

1. Remove the mapping (or indeed, the discriminator) prior to uploading to PactFlow
2. Remove the mapping but modify the components/references in the oneOf clauses to match the implicit discriminator

Option 1

This is fairly straightforward, and involves just walking the OAS tree and deleting any mapping you find, and then writing the file back out.
I need to review why this works, but my guess is that where there is no ambiguity in the types, the validator is clever enough to tease apart the differences. And even with the discriminator, mapping is not needed because the validator can see the types.

I'd be keen to see if anybody can try this on a real project to see if this works. If so, we could look to add that onto our backlog to update the core library here: https://github.com/pactflow/swagger-mock-validator/ (we'd also welcome submission of a PR, as there is already a visitor pattern in that library that walks the OAS tree and manipulates it)

Option 2

i.e. given the following response schema:

      schema:
        oneOf:
          - $ref: '#/components/schemas/CatObject'
          - $ref: '#/components/schemas/DogObject'
        discriminator:
          propertyName: petType
          mapping: 
            Cat: '#/components/schemas/CatObject'
            Dog: '#/components/schemas/DogObject'

This translation would be converted to:

      schema:
        oneOf:
          - $ref: '#/components/schemas/Cat'
          - $ref: '#/components/schemas/Dog'
        discriminator:
          propertyName: petType

(and the Cat and Dog schemeas would be moved to components).

The mapping is no longer needed, because the implicit schema aligns.
I started testing (2) first actually, but realised that in many cases the mapping isn’t actually needed, so didn’t complete this. But I think you get the idea. The library used below would be easy enough to do this manipulation, I think.

const SwaggerParser = require("@apidevtools/swagger-parser");
const fs = require("fs");

(async () => {
  try {
    // 1. Parse the document
    const api = await SwaggerParser.validate("./inheritance.oas.yml");

    console.log("Rewriting API name: %s, Version: %s", api.info.title, api.info.version);

    // 2. Iterate the top level endpoints in the API
    Object.keys(api.paths).forEach((path) => {
      const methods = [ "get", "post", "put", "patch", "delete", "head", "options" ];
      methods.forEach((method) => {
        // TODO: look at request bodies
        // TODO: handle $ref and not just inline schemas

        // 3. iterate the combinations of resources
        Object.keys(api.paths[path]?.[method]?.responses || {}).forEach(
          (status) => {
            const response = api.paths[path]?.[method]?.responses[status];
            Object.keys(response.content).forEach((mediaType) => {
              if (mediaType) {
                delete response.content[mediaType]?.schema?.discriminator?.mapping;
              }
            });
          }
        );
      });
    });

    // 4. Write out the new file
    const output = await SwaggerParser.bundle(api);
    fs.writeFileSync("inheritance.oas.json", JSON.stringify(output));
  } catch (err) {
    console.error(err);
  }
})();

And some test files:

npm init -f 
npm i @apidevtools/swagger-parser
node index.js // this should write out the inheritance.oas.json file

To test it out:

# original OAS with mapping, should fail
npx @pactflow/swagger-mock-validator@latest inheritance.oas-before.json inheritance.pact.json

# updated file, should pass
npx @pactflow/swagger-mock-validator@latest inheritance.oas-processed.json inheritance.pact.json 

inheritance.pact.json
inheritance.oas-processed.json
inheritance.oas-before.json

[brokenmass]

Instead of actually modifying your files you can simply “patch” ajv discriminator keyword to , at least, ignore mapping

const discriminatorKeyword = ajv.getKeyword(“discriminator”)
const originalCode = discriminatorKeyword.code;

discriminatorKeyword.code = cxt => {
delete cxt.schema.mapping
return originalCode(cxt)
}

[mefellows]

Ah, does that work? I had considered the use of https://www.npmjs.com/package/patch-package to do something like that - but this looks even simpler and possibly we could incorporate it into our package. Thanks!

Are you aware of any other side effects of doing this?

[brokenmass]

I’ve a library at work that converts decorated typescript classes into openapi (for rest interface) and protobuf + jsonschema (for grpc interface) and where necessary we generate oneofs with discriminator and mapping. with those 4 lines of “patch” we are able to output a schema as complete as possible and still use ajv for validation.

No, I don’t envision any side effect. As you are just removing an options field that ajv doesn’t use anyway.

mid you wanted you could even remove the internal discriminator implementation and replace it with your own use the “removeKeyword”/“addKeyword” method.

Obviously it would be much better if Evgeny at ajv accepted either pr 2262 (yours) or 2263 (a pr to allow to disable the hard fail if mapping is defined)

[mefellows]

Indeed it would be nice, thanks for sharing the above, this will no doubt be useful for others.