Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

[WIP] Use Accept header for Content-Type of response #27

Closed

Conversation

winniehell
Copy link
Contributor

Current behavior is that server decides what MIME types are acceptable and which of those are preferred. Shouldn't this really be the business of the client and the Accept-header?

If Swagger definition and Accept-header of client agree on image/jpeg, but an object should be sent, application/json is used anyway. In such a situation, I would rather prefer 406 Not Acceptable.

This influences how I would proceed on #23.

@winniehell
Copy link
Contributor Author

In other words: what is the reason to have hard-coded MIME types in the code rather than relying on produces and Accept?

@winniehell
Copy link
Contributor Author

@BigstickCarpet: Do you already have any opinion on that? I think that this is a prerequisite for #23. So I'd rather not continue work on that.

@JamesMessinger
Copy link
Member

Yeah, you're right. The setContentType() method will need some intelligent logic where it chooses the best content type based on the supported and excluded params as well as req.accepts()

@winniehell
Copy link
Contributor Author

Thank you for the feedback! I'll try to come up with some intelligent logic then... 😄

@winniehell winniehell changed the title Let client decide on content type? Use Accept header for Content-Type of response Feb 1, 2016
@winniehell
Copy link
Contributor Author

Currently, I don't agree that the excluded parameter in setContentType() is correct. I opened a new issued #33 to keep this pull request clean.

@winniehell winniehell force-pushed the use-accept-for-response branch 3 times, most recently from d9611ec to 9ec0385 Compare February 29, 2016 23:50
@winniehell winniehell changed the title Use Accept header for Content-Type of response [WIP] Use Accept header for Content-Type of response Mar 1, 2016
@winniehell winniehell force-pushed the use-accept-for-response branch from 9b90333 to 745aa7d Compare March 4, 2016 22:37
@winniehell winniehell force-pushed the use-accept-for-response branch from 745aa7d to 103e9c2 Compare March 5, 2016 12:23
.omit(function (header, name) {
return !_.isObject(header) ||
!_.includes(['string', 'number', 'integer', 'boolean', 'array'], header.type) ||
!_.isString(name)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon

Fixed. However I don't agree with this rule. 😉 See for example https://github.com/feross/standard

@winniehell winniehell force-pushed the use-accept-for-response branch from 6e6f371 to 25f34a5 Compare March 6, 2016 11:29
@winniehell winniehell force-pushed the use-accept-for-response branch from dd8b90b to 6f6b591 Compare March 12, 2016 19:40
@winniehell
Copy link
Contributor Author

I won't finish this as we have decided not to use swagger.

@winniehell winniehell closed this Aug 24, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants