add request headers to trigger functions

[miguel-s]

Contributing to #1461

Adds the node request headers to the request object in Parse trigger functions.

[flovilmart]

Hi @miguel-s , I was pretty sure you would have gone down that road and that's why we didn't implement it already, the req.config should 'per-request' so I believe this would be a better place to put those headers instead of a new parameter everywhere which could be very fragile.

Could you factor the request headers in the Config, or a Config subclass so it's easier to manage in the future?

[codecov]

Codecov Report

Merging #4012 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4012      +/-   ##
==========================================
+ Coverage   90.59%   90.59%   +<.01%     
==========================================
  Files         116      116              
  Lines        7793     7794       +1     
==========================================
+ Hits         7060     7061       +1     
  Misses        733      733

Impacted Files	Coverage Δ
src/triggers.js	90.9% <ø> (+0.5%)	⬆️
src/middlewares.js	97.27% <100%> (+0.01%)	⬆️
src/RestWrite.js	93.1% <0%> (-0.39%)	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	96.29% <0%> (+0.12%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0571c6f...a043aa1. Read the comment docs.

[oallouch]

From the outside, and from someone who doesn't know the implementation, it feels weird not to have the header in req.headers.
Btw, I just needed this feature to check that a Job is called by Google App Engine's cron.yaml using the "X-Appengine-Cron" header.
Thx a lot, @miguel-s !

[flovilmart]

@oallouch, this is already implemented in functions and jobs. Here it’s for the hooks

[oallouch]

Ah ok. Great. Just asking, do you plan to update the docs for things like this (like afterFind in the Guide and before/afterFind in the JS SDK API ref, for instance) ?

[flovilmart]

If you wanna make a PR on the docs, go ahead!

[miguel-s]

@flovilmart just to make sure I understood you correctly, I could add the headers to the config object in the handleParseHeaders function in middlewares.js, would this be an acceptable solution?

[flovilmart]

@miguel-s that would be better as I believe we create a new Config for each request

[oallouch]

I knew you would say that !
I'll try.

[miguel-s]

@flovilmart let me know what you think of the new approach

[flovilmart]

I like it, less intrusive, we'll refactor the Config at one point to be more practical :)

[montymxb]

Now that this is merged in we can go ahead and close out #1461 ?

[flovilmart]

Yes!

…

On Jul 24, 2017, 13:52 -0400, Benjamin Wilson Friedman ***@***.***>, wrote: Now that this is merged in we can go ahead and close out #1461 ? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.