nickva commented on a change in pull request #1195: Add support for bulk get
with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232722844
##########
File path: src/chttpd/src/chttpd_db.erl
##########
@@ -481,18 +482,62 @@ db_req(#httpd{method='POST', path_parts=[_,
<<"_bulk_get">>]}=Req, Db) ->
} = bulk_get_parse_doc_query(Req),
Options = [{user_ctx, Req#httpd.user_ctx} | Options0],
- {ok, Resp} = start_json_response(Req, 200),
- send_chunk(Resp, <<"{\"results\": [">>),
-
- lists:foldl(fun(Doc, Sep) ->
- {DocId, Results, Options1} = bulk_get_open_doc_revs(Db, Doc,
- Options),
- bulk_get_send_docs_json(Resp, DocId, Results, Options1, Sep),
- <<",">>
- end, <<"">>, Docs),
-
- send_chunk(Resp, <<"]}">>),
- end_json_response(Resp)
+ % Decide whether to use multipart response or application/json
+ AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
+ AcceptRelatedMp =
MochiReq:accepts_content_type("multipart/related"),
+ AcceptMp = AcceptMixedMp orelse AcceptRelatedMp,
Review comment:
I think the difference is that with existing open_revs logic it's ok if it
is inconsistent because it is already release, but I am worried about changing
the default behavior of what _bulk_gets if there is no `Accept` header or
`Accept` header contains `*/*` (that is accept anything).
In those cases I think `_bulk_get` will start returning mp content instead
of json as before. Try for example sending a request before the PR to _bulk_get
without an Accept header and after the PR. For me, it looked like the default
changed and that might be unexpected (even if technically correct from the HTTP
request / response point).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
