https://pact.io logo
Join Slack
Powered by
Hello! :wave: My team is working on a proof of con...
# pactflow
f
Hello! 👋 My team is working on a proof of concept of BiDirectional Contract Testing using PactFlow. Our aim is to assess its possibilities / limitations and we’ve stumbled across a pretty big limitation for us. PactFlow defines a list of requirements for allowed usage of the 
discriminator
keyword in the provider’s OpenAPI, one of which is that the use of 
discriminatorMapping
is not supported. Using such mapping is our preferred approach to polymorphism, based not only on the default behaviour of our openAPI generation library, but also on some internal tools working with OpenAPIs. If I understood the docs correctly, PactFlow’s approach is to instead define, in all subschemas, the discriminator field as required const of given value. • Is the support for 
discriminatorMapping
‘on the roadmap’, or should I interpret it as a by-design limitation? • In the PactFlow’s suggested approach, can the const values be different than the schema names? In the example, the value is 
Dog
, just like the schema - could it be 
ANIMAL_DOG
for example? Or maybe in general - does PactFlow support discriminator values that are different from the schema names?
CC: @Jakub Małyjasiak
m
Hi Filip, I understand. Let me provide you some background to it all
There is a public tracking issue to go with our internal feature request for this problem. The TLDR of it: 
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. I understand the frustration. When we implemented the OAS 3.1 upgrade earlier this year, the scenario came up and we had investigated several paths to address it. The biggest blocker we have is lack of support in a validation library we use (
ajv
) which narrows the possible solutions to fixing this to dodgey workarounds. There are a few issues and this PR to attempt it. There was a response from the maintainer of ajv the recently 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 for you. 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 (converts an explicit mapping to an implicit mapping) 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. My ask to you - if you could see if this works in your case, we could look to add that onto our backlog to update the core library here: https://github.com/pactflow/swagger-mock-validator/ (you could also submit 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:
Copy code
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:
Copy code
schema:
        oneOf:
          - $ref: '#/components/schemas/Cat'
          - $ref: '#/components/schemas/Dog'
        discriminator:
          propertyName: petType
(and the 
Cat
and 
Dog
schemes 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
Here is the JS spike I whipped together:
Copy code
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:
Copy code
npm init -f 
npm i @apidevtools/swagger-parser
node index.js // this should write out the inheritance.oas.json file
To test it out:
Copy code
# original OAS with mapping, should fail
npx @pactflow/swagger-mock-validator@latest inheritance.oas.yml inheritance.pact.json

# updated file, should pass
npx @pactflow/swagger-mock-validator@latest inheritance.oas.json inheritance.pact.json
inheritance.oas.jsoninheritance.pact.jsoninheritance.oas.yml
I’ve updated our tracking ticket with this info
f
Hi Matt, thanks a lot for detailed answer. I’ll check all the info / links / code you’ve provided and get back to you.
👍 1
Hi Matt. The complexity I have not mentioned on purpose makes this situation a bit more complicated for us. The root of all evil lays here: https://bitbucket.org/atlassian/swagger-request-validator/issues/771/discriminator-is-not-working-with-oneof We use this library and to my big surprise, I’ve found this issue. basically they are not supporting 
oneOf
+ 
discriminator
- which is the default approach to polymorphism according to OpenAPI spec.. But hope is there - a PR was suggested 4 days ago that could introduce this much needed support. As a workaround, we are currently using a weird approach, where we $ref to the base class schema. Anonymised, simplified example:
Copy code
"items": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/BaseItem"
            }
          },
And we define discriminator inside BaseItem schema instead:
Copy code
"BaseItem": {
        "required": [
          "itemType"
        ],
        "itemType": "object",
        "properties": {
          "itemType": {
            "type": "string",
            "enum": [
              "T1",
              "T2",
              "T3"
            ]
          }
        },
        "discriminator": {
          "propertyName": "itemType",
          "mapping": {
            "T1": "#/components/schemas/Item1",
            "T2": "#/components/schemas/Item2",
            "T3": "#/components/schemas/Item3"
          }
        }
}
This approach proven to work with the validator, but is also not ‘liked’ by other tools - and I have no idea how PactFlow would react to this if we removed the 
discriminatorMapping
. For now, the validator issue is our main blocker. When it’s resolved (luckily looks like it gained traction), I will come back to the topic, go back to 
oneOf
approach and try your suggestion to run a script and preprocess the document before sending to PactFlow - it looks promising 🤞
m
So we currently use a forked version of that same validator under the hood in BDCT. We took a few different design paths and decisions (e.g. proper support for oneOf and anyOf, as well as discriminator etc.). You can actually try it directly here: https://github.com/pactflow/swagger-mock-validator/ We’ve noticed some significant performance issues as the OAS grows, and a team member has spiked improving it - it’s orders of magnitudes faster.
👀 1
2 Views
Open in Slack
PreviousNext
Powered by