Skip to content
This repository was archived by the owner on May 25, 2021. It is now read-only.

2966 before response hook#150

Merged
asfgit merged 5 commits intoapache:masterfrom
cloudant:2966-before_response_hook
Mar 15, 2016
Merged

2966 before response hook#150
asfgit merged 5 commits intoapache:masterfrom
cloudant:2966-before_response_hook

Conversation

@iilyak
Copy link
Contributor

@iilyak iilyak commented Mar 10, 2016

Implement before_response and before_serve_file EPI hooks.

These hooks are useful in following cases:

  • to inject vendor specific headers
  • to inject vendor keys into json objects returned to client
  • to override return code

This is a replacement PR for #133

before_response(Req0, Code0, Headers0, Args0) ->
chttpd_plugin:before_response(Req0, Code0, Headers0, Args0).

respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
Copy link
Member

Choose a reason for hiding this comment

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

Does it worth to isolate MochiReq here and now while we still have a lot of places where we use it directly? I just quite don't like such kind of generic functions and it seems that this doesn't gives any positive gain now, but one more function in call chain (and traceback).

Copy link
Member

Choose a reason for hiding this comment

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

Ok, after thinking about the ways how to make the same changes without it, I ended with callbacks. Now sure which one is better here. So ok.

@kxepal
Copy link
Member

kxepal commented Mar 14, 2016

+1, but I think we can live without respond_.

++ ExtraHeaders,
ResponseHeaders1 = chttpd_cors:headers(Req, ResponseHeaders),
{ok, MochiReq:serve_file(RelativePath, DocumentRoot, ResponseHeaders1)}.
serve_file(Req0, RelativePath0, DocumentRoot0, ExtraHeaders) ->
Copy link
Member

Choose a reason for hiding this comment

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

Why is every line in this function double-spaced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eiri: fixed

@jaydoane
Copy link
Contributor

+1

1 similar comment
@eiri
Copy link
Member

eiri commented Mar 14, 2016

+1

Following functions were introduced:

 - add_headers/2
 - basic_headers/2
 - basic_headers_no_cors/2

COUCHDB-2966
It is much easier to write epi plugins if the object
it receives is not encoded yet.
So we pass unencoded JSON object to chttpd_plugin:before_response

COUCHDB-2966
@iilyak iilyak force-pushed the 2966-before_response_hook branch from 41fdff3 to 3be6796 Compare March 15, 2016 17:16
@asfgit asfgit merged commit 3be6796 into apache:master Mar 15, 2016
asfgit pushed a commit that referenced this pull request Mar 15, 2016
This closes #150

Signed-off-by: ILYA Khlopotov <iilyak@ca.ibm.com>
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.

6 participants