Skip to content

Add support for JSON Patch, JSON API and CSP report #161

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

Merged
merged 3 commits into from
Aug 13, 2019
Merged

Add support for JSON Patch, JSON API and CSP report #161

merged 3 commits into from
Aug 13, 2019

Conversation

simonbrunel
Copy link
Contributor

@simonbrunel simonbrunel commented Aug 4, 2019

Similar to koajs/bodyparser/pull/8, adds support for JSON Patch, JSON API and CSP report:

Extends #160 (though this PR doesn't introduce new option)

Tests added:

JSON media types
      √ should decode body as JSON object for type application/json
      √ should decode body as JSON object for type application/json-patch+json
      √ should decode body as JSON object for type application/vnd.api+json
      √ should decode body as JSON array for type application/json
      √ should decode body as JSON array for type application/json-patch+json
      √ should decode body as JSON array for type application/vnd.api+json

index.js Outdated
@@ -70,7 +70,7 @@ function requestbody(opts) {
// only parse the body on specifically chosen methods
if (opts.parsedMethods.includes(ctx.method.toUpperCase())) {
try {
if (opts.json && ctx.is('json')) {
if (opts.json && ctx.is('json', 'application/json-patch+json')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking we should just borrow the list if JSON content types from koa-bodyparser https://github.com/koajs/bodyparser/blob/master/index.js#L45-L50

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it make sense to use +json instead of ['application/json-patch+json', 'application/vnd.api+json'] so it would support any media types that represent a JSON document (see Structured Syntax Name Suffixes)?

if (opts.json && ctx.is('json', '+json', 'application/csp-report'))

Or should this lib only handle a few explicit types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MarkHerhold I just pushed a commit that adds support for the same types as in bodyparser (PR description updated).

- application/json-patch+json
- application/vnd.api+json
- application/csp-report
@simonbrunel simonbrunel changed the title Add support for JSON Patch (application/json-patch+json) Add support for JSON Patch, JSON API and CSP report Aug 6, 2019
@simonbrunel
Copy link
Contributor Author

@MarkHerhold anything else I can do to get it merged?

@MarkHerhold
Copy link
Contributor

Oh, sorry. Thanks for putting this back on my radar. Reviewing now...

@MarkHerhold
Copy link
Contributor

Looks great, thanks @simonbrunel and @sedenardi

@MarkHerhold
Copy link
Contributor

MarkHerhold commented Aug 13, 2019

published as 4.1.1

@simonbrunel simonbrunel deleted the feat/json-patch branch August 13, 2019 20:21
@simonbrunel
Copy link
Contributor Author

@MarkHerhold Thanks, will test 4.1.1 asap.

@sedenardi
Copy link

Everything looks good. Thanks again @MarkHerhold for getting to this so quickly.

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

Successfully merging this pull request may close these issues.

3 participants