feat: explicity set additionalProperties to false if set to true and …

[YOU54F]

…opts.additionalPropertiesInResponse is false

cc @vwong / @mefellows

./scripts/build.sh
./bin/swagger-mock-validator.mjs  docs/swagger.json docs/pact.json

output

Mock file "docs/pact.json" is not compatible with spec file "docs/swagger.json"
1 error(s)
        response.body.incompatible: 1
0 warning(s)
{
  warnings: [],
  errors: [
    {
      code: 'response.body.incompatible',
      message: 'Response body is incompatible with the response body schema in the spec file: must NOT have additional properties - someField',
      mockDetails: {
        interactionDescription: 'A get request to get a pet 1845563262948980200',
        interactionState: 'A pet 1845563262948980200 exists',
        location: '[root].interactions[0].response.body',
        mockFile: 'docs/pact.json',
        value: { someField: 'some string' }
      },
      source: 'spec-mock-validation',
      specDetails: {
        location: '[root].paths./pet/{petId}.get.responses.200.schema.additionalProperties',
        pathMethod: 'get',
        pathName: '/pet/{petId}',
        specFile: 'docs/swagger.json',
        value: false
      },
      type: 'error'
    }
  ]
}

Error: Mock file "docs/pact.json" is not compatible with spec file "docs/swagger.json"
    at _callee$ (file:///Users/yousaf.nabi/dev/pactflow/swagger-mock-validator/dist/cli.js:3395:36)
    at tryCatch (file:///Users/yousaf.nabi/dev/pactflow/swagger-mock-validator/dist/swagger-mock-validator-1ab1abfe.js:100:17)
    at Generator.<anonymous> (file:///Users/yousaf.nabi/dev/pactflow/swagger-mock-validator/dist/swagger-mock-validator-1ab1abfe.js:181:22)
    at Generator.next (file:///Users/yousaf.nabi/dev/pactflow/swagger-mock-validator/dist/swagger-mock-validator-1ab1abfe.js:125:21)
    at asyncGeneratorStep (file:///Users/yousaf.nabi/dev/pactflow/swagger-mock-validator/dist/swagger-mock-validator-1ab1abfe.js:371:24)
    at _next (file:///Users/yousaf.nabi/dev/pactflow/swagger-mock-validator/dist/swagger-mock-validator-1ab1abfe.js:390:9)

This doesn't fail, against the master build. The https://petstore.swagger.io/v2/swagger.json has been downloaded and modified, to explicitly add additionalProperties: true in the Pet schema

[mefellows]

I think this makes sense.

I can't think of any unintended side effects of this, with the notable exception being that any customer that currently has this enabled will potentially have their contracts break.

This is a manageable thing though, and we can release comms/update docs as needed.

[vwong]

Looks ok to me. Only thing is, the files in dist gets built automatically on npm release, so you don't have to commit that. I don't like that we have the compiled code in the repo, but it's something I haven't got around to fixing..

[YOU54F]

Only thing is, the files in dist gets built automatically on npm release, so you don't have to commit that. I don't like that we have the compiled code in the repo, but it's something I haven't got around to fixing..

Hmmm. yeah I did think that, but thought that wasn't the yak to be shaving right now.

I would also agree that the dist folder isn't relevant in here, we distribute as an npm package that can be executed easily with npx and users can build from source anyway

[YOU54F]

Done, I've removed the dist folder see #41 and also fixed up the bash script that we use for build and release as it was swallowing errors (no -e flag) #40

will pull changes into here and then sort tests out

[mefellows]

There is one final case that’s not currently considered in the release above - where additionalProperties is set to an schema.

For example, given this definition:

 Pet:
    type: object
    required:
      - name
      - photoUrls
    additionalProperties:
      type: string
    properties:
      id:
        type: integer
        format: int64
      category:
        $ref: '#/definitions/Category'
      name:
        type: string
        example: doggie

this body would be allowed:

"body": {
  "someField": "some string"
}

this body would not (foo is a number, not a string):

"body": {
  "someField": "some string",
  "foo": 1
}

Clearly, in some cases, this behaviour could lead to dangerous outcomes. In other cases, it could make complete sense - for example, an API that responds with dynamic keys.

This scenario is hard to police, because you can also put in a strict subschema that specifies specifically what fields are and are not allowed.

So for now, I think it’s the right decision to leave this as is, but I wanted to document it against the issue so we have a trace of it.

[mefellows]

I've created an example project that shows how to effectively allow additionalProperties: true if this is truly needed.