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

Pass the Express request object as context #25

Closed
wants to merge 3 commits into from

Conversation

VegetarianOrc
Copy link

Allows access to params and other portions of the request object.

@colthreepv
Copy link

mhm such a functionality change would definitely need some documentation.

What's the use case?
An API endpoint that checks some parts of the body only if some params is there?

Seems quite a niche from here

@VegetarianOrc
Copy link
Author

Sorry about the lack of detail. I'm implementing a JSON API compliant API and there are some requirements in the specification that require members of the body to match the server's endpoint.

This is one in particular that applies to updating resources:

A server MUST return 409 Conflict when processing a PATCH request in which the resource object's type and id do not match the server's endpoint.

This example from the spec shows the desired behavior with an endpoint like /articles/:id:

PATCH /articles/1 HTTP/1.1
Content-Type: application/vnd.api+json
Accept: application/vnd.api+json

{
  "data": {
    "type": "articles",
    "id": "1",
    "attributes": {
      "title": "To TDD or Not",
      "text": "TLDR; It's complicated... but check your test coverage regardless."
    }
  }
}

Here type and id must match the params in the server endpoint. By passing the entire request object this allows you to create validations that can verify this for you like:

{
 data: {
  id: Joi.number().valid(Joi.ref('$params.id'))
 }
}

References can still reach the body of the request by referencing the body in the in the request object: Joi.ref('$body.my_value').

Alex Mazzeo added 2 commits February 16, 2016 18:11
@colthreepv
Copy link

I appreciate this input.. I think it could end behind a flag.
I was hoping to merge 0.5 before further develop, but Andrew is missing.

If I can get npm permissions I will be back with some more activity

@VegetarianOrc
Copy link
Author

Sounds good. I'm happy to rework it as needed.

@AndrewKeig
Copy link
Owner

Sorry guys very busy week, will leave this with @colthreepv, please go ahead with 0.5 merge, many thanks.

@colthreepv
Copy link

Hi @amazzeo I brought your idea and commits in a new branch context-request

I put this functionality behind the flag contextRequest, it can be turn on globally, or selectively for a specific route.

Please review the branch and drop some feedback.

Also on flag naming
Hardest task

@VegetarianOrc
Copy link
Author

Sorry for the delay in getting back to you! I think that works great, thanks for including it!

@skelet00r
Copy link

Is this functionality going to be put into the master branch? The code seems to be stagnating in @colthreepv 's branch.

@colthreepv
Copy link

Ouch.. quite some dust on this PR!
Sorry for absence, will work on this promptly 👷

colthreepv pushed a commit that referenced this pull request Jun 29, 2016
@colthreepv
Copy link

merged and released 1.0.0

@colthreepv colthreepv closed this Jun 29, 2016
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.

4 participants