Skip to content

Conversation

@m-nikhil
Copy link

@m-nikhil m-nikhil commented Mar 25, 2020

Fixes #655
Fixes #860
Related to pull request #760

Changes proposed in this pull request:

Problem Statement:

  • In the case of multiple content_types in spec, response validation assumes incorrect content_type and tries to json_load.

Expected Scenario:

  • In the case of multiple content_types in spec, the content_type will be set in the response by the user; else the default application/json will be used.

Solution:

  • In is_json_schema_compatible function, the logic rely on self.mimetype which defaults to application/json always.
  • The above causes the function to always return True.
  • The is_json_schema_compatible must depend on the content_type set by the user.
  • The changes made pass content_type to the function and operates the logic on content_type instead of self.mimetype, so returns the correct boolean.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.291% when pulling 00a008e on m-nikhil:master into 96bdcb0 on zalando:master.

@m-nikhil
Copy link
Author

@hjacobs @dtkav Can you guys please take a look.

@m-nikhil m-nikhil marked this pull request as draft June 28, 2020 12:38
@m-nikhil m-nikhil marked this pull request as ready for review September 5, 2020 16:03
@uniqueg
Copy link

uniqueg commented Sep 19, 2020

Would be very interested to have this merged, or have the issue of dealing with different content types addressed anyhow

@robguinness-snyk
Copy link

Any updates on this?

vmarkovtsev added a commit to athenianco/especifico that referenced this pull request Apr 3, 2022
I am fixing the mime type hell in the current codebase.
vmarkovtsev added a commit to athenianco/especifico that referenced this pull request Apr 3, 2022
@vmarkovtsev
Copy link
Contributor

Ported as athenianco#10

@m-nikhil
Copy link
Author

m-nikhil commented Apr 3, 2022

Bumping it up!
Adding current maintainers, @RobbeSneyders , @Ruwann

@RobbeSneyders
Copy link
Member

Thanks @m-nikhil.
Could you rebase on and target this to the v2 branch? We'll cherrypick it to main for v3 later.

@m-nikhil m-nikhil changed the base branch from master to v2 April 15, 2022 07:21
@m-nikhil
Copy link
Author

Thanks @m-nikhil. Could you rebase on and target this to the v2 branch? We'll cherrypick it to main for v3 later.
@RobbeSneyders , do let me know if it looks good.

@RobbeSneyders
Copy link
Member

Thanks @m-nikhil

Two remarks:

@RobbeSneyders
Copy link
Member

Outdated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

6 participants