Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strict contract responses #99

Closed
ghost opened this issue Oct 29, 2019 · 10 comments
Closed

Strict contract responses #99

ghost opened this issue Oct 29, 2019 · 10 comments
Labels
enhancement New feature or request

Comments

@ghost
Copy link

ghost commented Oct 29, 2019

Is there a configuration that I'm missing that strips out any fields that are not defined in the response schema?

@ghost ghost changed the title Strict contract response Strict contract responses Oct 29, 2019
@cdimascio
Copy link
Owner

thanks for the issue @plastic-abubakr.
currently, we do not strip out fields not defined in the response schema, however its certainly worth consideration.
can you describe your use case, the desired behavior, and an example. thanks!

@cdimascio
Copy link
Owner

prior to a recent fix, responses were allowing additional properties to pass validation. we could consider making this option again. is that what you are looking for?

@cdimascio
Copy link
Owner

@plastic-abubakr ^

@ghost
Copy link
Author

ghost commented Oct 31, 2019

@cdimascio here is my use case:

Lets say my response schema is define to return array of *Roles:

[
  {
    "guid": "actor",
    "label": "Actor"
  },
  {
    "guid": "producer",
    "label": "Producer"
  }
]

Roles data model has some additional properties defined, for example an id for each row. Problem is, I can not send the database query result as it is, and have to manually filter out any additional fields that are not supposed to be sent to the client.

Right now, if i don't filter out the results, it will send out the following response to the client:

[
  {
   "id": 1,
    "guid": "actor",
    "label": "Actor",
    "active": true
  },
  {
   "id": 2,
    "guid": "producer",
    "label": "Producer",
    "active": true
  }
]

Where as the openapi schema is defiend to send only guid and label. I feel it should strip out extra fields which are not defined in response schema

@cdimascio
Copy link
Owner

Got it. This makes sense. I'll add support for this.

@cdimascio cdimascio added the enhancement New feature or request label Oct 31, 2019
@richdouglasevans
Copy link
Contributor

By all means add support for this: I understand the issue and the motivation 👍

This does expand the scope of this library though, a library which has validator in the project name. My expectation of a validator is that it will validate: if you add support for transformation also the library will continue to validate… but, I don't expect a validator to transform.

Transformation of either the input or the output to conform to some schema is a distinct concern. Even if the transformation feature is optional — and I expect it'll be off by default — adding this feature dilutes the focus of this library. It'd be helpful for some set of users, undoubtedly, but it also adds weight that's not directly relevant to what this library is about: validation.

Perhaps there's a distinct library that could be developed — express-openapi-transformer — that does transformation only? An application might compose these to achieve the effect of both validation and transformation.

Like I say, do add support for this if you're so minded 👍 😃

@comino
Copy link
Collaborator

comino commented Nov 1, 2019

@richdouglasevans I tend to disagree. The widely used JSON validation library "joi" also decided to support "stripUnknown" to strip unknown fields from a payload. Users like me would end up in adding Joi just to implement a sanitizer to request and response. That would mean I have to maintain Joi model definitions along with the openAPI definition.

Personally I would like to see this feature ( but would disable it by default).

cdimascio added a commit that referenced this issue Nov 2, 2019
#99 option to remove additional props from responses
@cdimascio
Copy link
Owner

cdimascio commented Nov 2, 2019

in v2.14.1 you can get the behavior you are looking for with the following option:

validateResponses: {
   removeAdditional: 'failing'
 }

This will validate responses, but remove any additionalProperties that would not pass schema validation.

@ghost
Copy link
Author

ghost commented Nov 6, 2019

This does not seem to be working

in v2.14.1 you can get the behavior you are looking for with the following option:

validateResponses: {
   removeAdditional: 'failing'
 }

This will validate responses, but remove any additionalProperties that would not pass schema validation.

@cdimascio
Copy link
Owner

@plastic-abubakr I have a couple of tests to demonstrate this functionality. Perhaps have a quick look to see if your look similar. I believe this should work now, the caveat is that additionalProperties: false must be used in addition to the removeAdditional: 'failing'

Here are some examples in the test suite

  1. Configure the validator to remove additional
    https://github.com/cdimascio/express-openapi-validator/blob/master/test/response.validation.options.spec.ts#L19

  2. Specify not to include additional props
    https://github.com/cdimascio/express-openapi-validator/blob/master/test/resources/response.validation.yaml#L157

  3. Test to show the additional prop is removed
    https://github.com/cdimascio/express-openapi-validator/blob/master/test/response.validation.options.spec.ts#L94

Hopefully, you're missing something, if not please pass along the details and i'll look into

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants