You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by kxepal <gi...@git.apache.org> on 2015/04/23 13:32:07 UTC

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

GitHub user kxepal opened a pull request:

    https://github.com/apache/couchdb-chttpd/pull/33

    Implement /db/_bulk_get endpoint

    Based on RCouch implementation by @benoitc.
    
    COUCHDB-2310

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kxepal/couchdb-chttpd 2310-bulk_get

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-chttpd/pull/33.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #33
    
----
commit ced8a8d77fa100ea83f1a8f87bccb6ea17799af9
Author: Alexander Shorin <kx...@apache.org>
Date:   2015-04-22T18:30:47Z

    Implement /db/_bulk_get endpoint
    
    Based on RCouch implementation by @benoitc.
    
    COUCHDB-2310

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98408064
  
    @nolanlawson hm..fine then (: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95671034
  
    Ah okay, that makes sense. If `_bulk_get` exactly implements the Couchbase version, then that makes life easier for PouchDB, as well as allowing Couchbase Mobile to plug into CouchDB. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39495379
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter is not acceptable">>,
    +            throw({bad_request, Msg});
    +        Key when Key =:= "rev"; Key =:= "atts_since" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter should be applied
    +                    on per document basis in payload">>,
    +            throw({bad_request, Msg});
    +        _ ->
    +            parse_doc_query({Key, Value}, Args)
    +        end
    +    end, #doc_query_args{}, chttpd:qs(Req)).
    +
    +bulk_get_open_doc_revs({Props}, Db, Options) ->
    +    case parse_field(<<"id">>, couch_util:get_value(<<"id">>, Props)) of
    +    {error, {DocId, Error, Reason}} ->
    +        {DocId, {error, {null, Error, Reason}}, Options};
    +
    +    {ok, undefined} ->
    +        {null, {error, {null, bad_request, <<"document id missed">>}}, Options};
    +
    +    {ok, DocId} ->
    +        RevStr = couch_util:get_value(<<"rev">>, Props),
    +        case parse_field(<<"rev">>, RevStr) of
    +        {error, {RevStr, Error, Reason}} ->
    +            {DocId, {error, {RevStr, Error, Reason}}, Options};
    +
    +        {ok, Rev} ->
    +            Revs = case Rev of undefined -> all; _ -> [Rev] end,
    +            AttsSinceStr = couch_util:get_value(<<"atts_since">>, Props),
    +
    +            case parse_field(<<"atts_since">>, AttsSinceStr) of
    +            {error, {BadAttsSinceRev, Error, Reason}} ->
    +                {DocId, {error, {BadAttsSinceRev, Error, Reason}}, Options};
    +
    +            {ok, AttsSince} ->
    +                Options1 = case AttsSince of
    +                    [] ->
    +                        Options;
    +                    RevList ->
    +                        [{atts_since, RevList}, attachments | Options]
    +                end,
    +
    +                case fabric:open_revs(Db, DocId, Revs, Options1) of
    +                {ok, []} ->
    +                    {DocId,
    +                     {error, {RevStr, <<"not_found">>, <<"missing">>}},
    +                     Options1};
    +                Results ->
    +                    {DocId, Results, Options1}
    +                end
    +            end
    +        end
    +    end.
    +
    +
    +parse_field(<<"id">>, undefined) ->
    +    {ok, undefined};
    +parse_field(<<"id">>, Value) ->
    +    case catch couch_doc:validate_docid(Value) of
    --- End diff --
    
    try/catch here is more readable, this is the old form of 'catch'.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by janl <gi...@git.apache.org>.
Github user janl commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-106022822
  
    @kxepal what @nolanlawson explained here: https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98407533



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by willholley <gi...@git.apache.org>.
Github user willholley commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-139565250
  
    in my tests, bulk_get doesn't return the list of revisions correctly:
    
    
    	$ curl http://localhost:15984/willtest -XPUT 
    	{"ok":true}
    
    	$ curl http://localhost:15984/willtest -XPOST -H 'Content-Type:application/json' --data-binary '{"_id":"foo"}'
    	{"ok":true,"id":"foo","rev":"1-967a00dff5e02add41819138abb3284d"}
    
    	$ curl 'http://localhost:15984/willtest/foo?revs=true
    	{"_id":"foo","_rev":"1-967a00dff5e02add41819138abb3284d","_revisions":{"start":1,"ids":["967a00dff5e02add41819138abb3284d"]}}
    
    	$ curl http://localhost:15984/willtest -XPOST -H 'Content-Type:application/json' --data-binary '{"_id":"foo","_rev":"1-967a00dff5e02add41819138abb3284d","generation":2}'
    	{"ok":true,"id":"foo","rev":"2-0cb4bbdce165620d03a15eff82d8712a"}
    
    	$ curl 'http://localhost:15984/willtest/foo?revs=true'
    	{"_id":"foo","_rev":"2-0cb4bbdce165620d03a15eff82d8712a","generation":2,"_revisions":{"start":2,"ids":["0cb4bbdce165620d03a15eff82d8712a","967a00dff5e02add41819138abb3284d"]}}
    
    	$ curl 'http://localhost:15984/willtest/_bulk_get' -H 'Content-Type:application/json' --data-binary '{"docs":[{"id":"foo","rev":"2-0cb4bbdce165620d03a15eff82d8712a"}],"revs":true}' -H 'Accept: application/json'
    	{"results": [{"id": "foo", "docs": [{"ok":{"_id":"foo","_rev":"2-0cb4bbdce165620d03a15eff82d8712a","generation":2}}]}]}
    
    In that last response, I'd expect a "_revisions" field similar to the explicit request for /foo?revs=true.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-135569008
  
    pong @robertkowalski 
    Sorry, no updates so far. And not sure what causes issue for PouchDB since the failed tests are not related to these changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98389923
  
    > `revs=true` on other hand need to specify explicitly and this option applied on all the docs specified in payload.
    
    I think you mean "no need to specify"? It seems implicit.
    
    > No, `open_revs` explicitly banned to prevent confusion
    
    OK, fine by me. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/couchdb-chttpd/pull/33


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-106021935
  
    @janl what the test you ran?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98391707
  
    I don't see you requiring `revs` anywhere in your example above. `rev` yes, `revs` no...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98407945
  
    @nolanlawson 
    
    > Shouldn't application/json be the expected default?
    
    I think it should. Good point!
    
    > curl -X POST "127.0.0.1:15984/mydb/_bulk_get" -H 'content-type:application/json' --data-binary '{"revs": true, "docs":[{"id":"foo","rev":"1-967a00dff5e02add41819138abb3284d"}]}'
    
    `revs` need to be passed as query parameter, not in payload. In you request it just being ignored.
    
    > Also: it seems impossible to specify attachments anywhere (keep in mind that PouchDB has not implemented atts_since yet). This seems to be a blocker bug that would make us unable to use your implementation.
    
    Well, this is blocker bug for PouchDB to sync with Couchbase as well as they don't declare attachments query parameter as the one that accepted for /_bulk_get endpoint. I can make it work (thought, I wonder why it don't - didn't check yet), but that would be deviation from the original spec.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by willholley <gi...@git.apache.org>.
Github user willholley commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-140180282
  
    I have a PouchDB PR with all tests now passing against this implementation. I imagine there are some optimisations that could be done in terms of minimising individual requests to cluster nodes but as a first cut this looks good.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98390306
  
    >> revs=true on other hand need to specify explicitly and this option applied on all the docs specified in payload.
    
    > I think you mean "no need to specify"? It seems implicit.
    
    No, I mean, that you need to specify it in order to have `_revisions` field in the response. Couchbase also requires to have it specified explicitly since the default value for this param for them is false. See example above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-131155474
  
    @nolanlawson I eventually forgot about and left it behind. Let me recall the problems and undone work. Thanks for pinging!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39497586
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    --- End diff --
    
    Oh, I think I can just reuse function signature instead having case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39550307
  
    --- Diff: src/chttpd_db.erl ---
    @@ -449,6 +449,45 @@ db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req,
     db_req(#httpd{path_parts=[_,<<"_bulk_docs">>]}=Req, _Db) ->
         send_method_not_allowed(Req, "POST");
     
    +
    +db_req(#httpd{method='POST',
    +              path_parts=[_, <<"_bulk_get">>],
    +              mochi_req=MochiReq}=Req,
    +       Db) ->
    +    couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
    +    couch_httpd:validate_ctype(Req, "application/json"),
    +    {JsonProps} = chttpd:json_body_obj(Req),
    +    case couch_util:get_value(<<"docs">>, JsonProps) of
    +    undefined ->
    +        throw({bad_request, <<"Missing JSON list of 'docs'.">>});
    +    Docs ->
    +        #doc_query_args{
    +            options = Options
    +        } = bulk_get_parse_doc_query(Req),
    +
    +        AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
    +        AcceptRelatedMp = MochiReq:accepts_content_type("multipart/related"),
    +
    +        case AcceptMixedMp orelse AcceptRelatedMp of
    +        true ->
    +            throw({not_acceptable,
    --- End diff --
    
    Let's remove it, it's the wrong http semantic and it's best not to have speculative / non-working code (we have enough of that by accident!)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-140161373
  
    @willholley Actually, we all here should use http://developer.couchbase.com/mobile/develop/references/sync-gateway/rest-api/database/post-bulk-get/index.html as the base to maintain compatibility - that's the whole point of /_bulk_get for now. The only exception is JSON response which not covered by Couchbase spec where we can act more or less freely.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-140348091
  
    +1 after addressing comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-106041063
  
    @janl @nolanlawson need your help with:
    
    ```
    $ npm link ../express-pouchdb
    
    npm ERR! Error: EACCES: permission denied, symlink '/home/kxepal/tmp/pouchdb/express-pouchdb' -> '/usr/lib64/node_modules/express-pouchdb'
    npm ERR!     at Error (native)
    npm ERR!  { [Error: EACCES: permission denied, symlink '/home/kxepal/tmp/pouchdb/express-pouchdb' -> '/usr/lib64/node_modules/express-pouchdb']
    npm ERR!   errno: -13,
    npm ERR!   code: 'EACCES',
    npm ERR!   syscall: 'symlink',
    npm ERR!   path: '/home/kxepal/tmp/pouchdb/express-pouchdb',
    npm ERR!   dest: '/usr/lib64/node_modules/express-pouchdb' }
    npm ERR! 
    npm ERR! Please try running this command again as root/Administrator.
    ```
    
    I do not want to let npm install anything in my system bypassing system package manager. How can I tell `npm link` to use local directory, not `/usr/lib64/...`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95555025
  
    cc @janl @rnewson @nolanlawson @iilyak @dholth


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98407533
  
    To test PouchDB Server with the new `_bulk_get` API, just run:
    
    ```
    hub clone pouchdb/pouchdb-server
    hub clone pouchdb/express-pouchdb --branch 196
    hub clone pouchdb/pouchdb --branch 3424
    # start up pouchdb server
    cd pouchdb-server
    npm link ../express-pouchdb
    npm link ../pouchdb
    ./bin/pouchdb-server -p 6984
    # run the tests from pouchdb against it
    cd ../pouchdb
    COUCH_HOST=http://127.0.0.1:6984 npm run dev
    # then open up the tests in a browser, or for node.js:
    COUCH_HOST=http://127.0.0.1:6984 npm test
    ```
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39550551
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    --- End diff --
    
    I was thinking of a function that hand clauses like;
    
    ```
    validate_query_param("open_revs" = Key) ->
        throw_bad_query(Key);
    validate_query_param("new_edits" = Key) ->
        throw_bad_query(Key);
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98403278
  
    Also: it seems impossible to specify `attachments` anywhere (keep in mind that PouchDB has not implemented `atts_since` yet). This seems to be a blocker bug that would make us unable to use your implementation.
    
    As a suggestion, can we specify `attachments=true` in the same way as `revs`, ala:
    
        curl -X POST "127.0.0.1:15984/mydb/_bulk_get?attachments=true" -H 'content-type:application/json' -H 'accept:application/json' --data-binary '{"revs": true, "docs":[{"id":"foo","rev":"1-967a00dff5e02add41819138abb3284d"}]}'
    
    Specifying it per-doc would also be okay, but it's wasteful from our perspective, because we will set it to `true` for every doc.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by willholley <gi...@git.apache.org>.
Github user willholley commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-140140779
  
    ah - ignore the above. PouchDB is wrong here (if we're basing this on rcouch). The CouchDB implementation works as expected through the clustered interface.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95675070
  
    @nolanlawson yes, it will, but for now there is no multipart support, so it still does not completely.  Multipart support postponed to the next iteration after https://github.com/apache/couchdb-couch/pull/19 merge - I need it to avoid very stupid code duplicity for response generation.
    
    So with this PR we could make CouchDB, PouchDB and RCouch agree on `/_bulk_get` API. What is also good for start (:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-131152149
  
    @kxepal @robertkowalski What is the status of this? Been meaning to implement this in PouchDB/PouchDB Server, but I was hoping this could get merged first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-141205964
  
    @rnewson I fixed all your comments + made a bit cleanup. All changes are done in own commits for simpler review.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-141245687
  
    nice! +1 after squash.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95578515
  
    mmm, iirc the plan was to get /_bulk_get and automatically deprecate it in favour of /_bulk_revs. But during deprecation, there are some API bits which are need to be fixed in sync with Couchbase team if we want to maintain compatibility.
    
    Making /_bulk_revs and /_bulk_docs synonyms isn't possible during completely different duties and responses. Technically, we can make GET /_bulk_docs to work as POST /_bulk_get, but I fear no http client is expected such legal, but uncommon GET method usage.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by janl <gi...@git.apache.org>.
Github user janl commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-105253198
  
    This seems to have to do with the *eventual* nature of a cluster :)
    
    Now this is failing, a little later than the previous one:
    
    ```
      test.views.js-local-http
    
    
      3 passing (16s)
      1 failing
    
      1) test.all_docs.js-http Testing deleting in changes:
         Uncaught AssertionError: expected [ Array(4) ] to include '1'
          at db.changes.complete (/private/tmp/pouchdb/tests/integration/test.all_docs.js:150:38)
          at Changes.<anonymous> (/private/tmp/pouchdb/lib/changes.js:28:7)
          at Changes.emit (events.js:95:17)
          at /private/tmp/pouchdb/lib/changes.js:21:12
          at /private/tmp/pouchdb/lib/utils.js:342:11
          at /private/tmp/pouchdb/node_modules/argsarray/index.js:14:18
          at /private/tmp/pouchdb/lib/changes.js:80:5
          at tryCatch1 (/private/tmp/pouchdb/node_modules/bluebird/js/main/util.js:63:19)
          at Promise$_callHandler [as _callHandler] (/private/tmp/pouchdb/node_modules/bluebird/js/main/promise.js:695:13)
          at Promise$_settlePromiseFromHandler [as _settlePromiseFromHandler] (/private/tmp/pouchdb/node_modules/bluebird/js/main/promise.js:711:18)
          at Promise$_settlePromiseAt [as _settlePromiseAt] (/private/tmp/pouchdb/node_modules/bluebird/js/main/promise.js:868:14)
          at Promise$_settlePromises [as _settlePromises] (/private/tmp/pouchdb/node_modules/bluebird/js/main/promise.js:1006:14)
          at Async$_consumeFunctionBuffer [as _consumeFunctionBuffer] (/private/tmp/pouchdb/node_modules/bluebird/js/main/async.js:74:12)
          at Async$consumeFunctionBuffer (/private/tmp/pouchdb/node_modules/bluebird/js/main/async.js:37:14)
          at process._tickCallback (node.js:442:13)
    
    ```
    
    Logs include a bunch of these:
    
    ```
    2015-05-25 17:34:36.701 [warning] node1@127.0.0.1 <0.5018.0> write quorum (2) failed for updated:testdb
    2015-05-25 17:34:36.700 [warning] node2@127.0.0.1 <0.5759.0> write quorum (2) failed for updated:testdb
    2015-05-25 17:34:36.700 [warning] node3@127.0.0.1 <0.5746.0> write quorum (2) failed for updated:testdb
    2015-05-25 17:34:36.700 [warning] node3@127.0.0.1 <0.5746.0> write quorum (2) failed for updated:testdb
    ```
    
    Full logs here: https://gist.github.com/janl/0f80037cc6803860e84d


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95554589
  
    Example:
    ```
    $ echo '{"docs": [{"id": "good-doc"}, \
                      {"id": "_bad-doc"}, \
                      {"id": "non-existed-doc"}, \
                      {"id": "good-doc", "rev": "1-badrev"}, \
                      {"_id": "typo"}, \
                      {"id": "good-doc", "atts_since": ["bad-atts-since"]}, \
                      {"id": "good-doc", "atts_since": "wtf"}]}'\
     | http 'http://localhost:15984/test/_bulk_get?revs=true'
    
    HTTP/1.1 200 OK
    Cache-Control: must-revalidate
    Content-Type: application/json
    Date: Thu, 23 Apr 2015 11:30:50 GMT
    Server: CouchDB/42c9047 (Erlang OTP/17)
    Transfer-Encoding: chunked
    X-Couch-Request-ID: c063fcc5
    X-CouchDB-Body-Time: 0
    
    {
        "results": [
            {
                "docs": [
                    {
                        "ok": {
                            "_id": "good-doc",
                            "_rev": "1-967a00dff5e02add41819138abb3284d",
                            "_revisions": {
                                "ids": [
                                    "967a00dff5e02add41819138abb3284d"
                                ],
                                "start": 1
                            }
                        }
                    }
                ],
                "id": "good-doc"
            },
            {
                "docs": [
                    {
                        "error": {
                            "error": "bad_request",
                            "id": "_bad-doc",
                            "reason": "Only reserved document ids may start with underscore.",
                            "rev": null
                        }
                    }
                ],
                "id": "_bad-doc"
            },
            {
                "docs": [
                    {
                        "error": {
                            "error": "not_found",
                            "id": "non-existed-doc",
                            "reason": "missing",
                            "rev": "undefined"
                        }
                    }
                ],
                "id": "non-existed-doc"
            },
            {
                "docs": [
                    {
                        "error": {
                            "error": "not_found",
                            "id": "good-doc",
                            "reason": "missing",
                            "rev": "1-badrev"
                        }
                    }
                ],
                "id": "good-doc"
            },
            {
                "docs": [
                    {
                        "error": {
                            "error": "bad_request",
                            "id": null,
                            "reason": "document id missed",
                            "rev": null
                        }
                    }
                ],
                "id": null
            },
            {
                "docs": [
                    {
                        "error": {
                            "error": "bad_request",
                            "id": "good-doc",
                            "reason": "Invalid rev format",
                            "rev": "bad-atts-since"
                        }
                    }
                ],
                "id": "good-doc"
            },
            {
                "docs": [
                    {
                        "error": {
                            "error": "bad_request",
                            "id": "good-doc",
                            "reason": "att_since value must be array of revs.",
                            "rev": "wtf"
                        }
                    }
                ],
                "id": "good-doc"
            }
        ]
    }
    
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98387637
  
    @nolanlawson 
    
    1) Yes. Couchbase spec assumes and actually there is no point of having atts_since without returning attachments back.
    
    2) `revs=true` on other hand need to specify explicitly and this option applied on all the docs specified in payload.
    
    3) No, `open_revs` [explicitly banned](https://github.com/apache/couchdb-chttpd/pull/33/files#diff-eb6d44751e9360ac262be41e9f69fab2R1532) to prevent confusion. According Couchbase implementation there is the following logic: if no `rev` field specified for doc in payload, the server acts as if `open_revs=all` was specified. Otherwise, it returns a specific revision which you'd put in this field. That's a design flaw, imho, since to emulate `open_revs` for multiple revisions, you have to put the same document into payload multiple times per each conflict revision you want to get. So your example is correct, unfortunately.
    
    For the last one:  
    - git clone couchdb
    - cd couchdb
    - ./configure
    - cd src/chttpd
    - curl -O https://github.com/apache/couchdb-chttpd/pull/33.patch
    - git apply 33.patch
    - cd ../../
    - make
    - dev/run
    
    alternatively, instead of patching, you can edit rebar.config.script in order to point chttpd to my fork and PR feature branch, but it required more typing and I may not guess your favourite editor (: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by willholley <gi...@git.apache.org>.
Github user willholley commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-137404884
  
    it looks like this needs rebasing against master @kxepal 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39495298
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    --- End diff --
    
    let's pull this into its own validation function using simple clauses.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-135567878
  
    ping @kxepal 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98386177
  
    Questions:
    
    1) Are attachments assumed if `atts_since` is provided? Seems more intuitive to me that you would need to individually specify `attachments=true` like a normal GET.
    
    2) Is `revs=true` assumed for each result? Seems so, if the `_revisions` object is being returned.
    
    2) Is `open_revs` supported? Can a single `docs` list contain more than one rev? E.g. can I do:
    
    ```json
    {"docs": [{"id": "my-doc", "open_revs": ["1-x", "1-y", "1-z"]}] }
    ```
    
    Or do I have to do:
    
    ```json
    {"docs": [
      {"id": "my-doc", "rev": "1-x"},
      {"id": "my-doc", "rev": "1-y"},
      {"id": "my-doc", "rev": "1-z"}
    ]
    ```
    
    ?
    
    Also possibly the more important question: how can I build CouchDB with this branch so I can start testing against it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98395137
  
    @nolanlawson As for PouchDB test suite issue, @chewbranca work is not merge yet. Thought, you might be interested to try 2657-fix-cassim-fabric-calls branch of cassim repository and run test suite again - it passed original issue, but hits more ones and I'm in doubt to say whom to blame since PouchDB starts fail for expected revs in changes feed. 
    
    Look forward for you PR! Just drop a reference here onto it, I'll join to yours testing party (:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39497411
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter is not acceptable">>,
    +            throw({bad_request, Msg});
    +        Key when Key =:= "rev"; Key =:= "atts_since" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter should be applied
    +                    on per document basis in payload">>,
    +            throw({bad_request, Msg});
    +        _ ->
    +            parse_doc_query({Key, Value}, Args)
    +        end
    +    end, #doc_query_args{}, chttpd:qs(Req)).
    +
    +bulk_get_open_doc_revs({Props}, Db, Options) ->
    +    case parse_field(<<"id">>, couch_util:get_value(<<"id">>, Props)) of
    +    {error, {DocId, Error, Reason}} ->
    +        {DocId, {error, {null, Error, Reason}}, Options};
    +
    +    {ok, undefined} ->
    +        {null, {error, {null, bad_request, <<"document id missed">>}}, Options};
    +
    +    {ok, DocId} ->
    +        RevStr = couch_util:get_value(<<"rev">>, Props),
    +        case parse_field(<<"rev">>, RevStr) of
    +        {error, {RevStr, Error, Reason}} ->
    +            {DocId, {error, {RevStr, Error, Reason}}, Options};
    +
    +        {ok, Rev} ->
    +            Revs = case Rev of undefined -> all; _ -> [Rev] end,
    +            AttsSinceStr = couch_util:get_value(<<"atts_since">>, Props),
    +
    +            case parse_field(<<"atts_since">>, AttsSinceStr) of
    +            {error, {BadAttsSinceRev, Error, Reason}} ->
    +                {DocId, {error, {BadAttsSinceRev, Error, Reason}}, Options};
    +
    +            {ok, AttsSince} ->
    +                Options1 = case AttsSince of
    +                    [] ->
    +                        Options;
    +                    RevList ->
    +                        [{atts_since, RevList}, attachments | Options]
    +                end,
    +
    +                case fabric:open_revs(Db, DocId, Revs, Options1) of
    +                {ok, []} ->
    +                    {DocId,
    +                     {error, {RevStr, <<"not_found">>, <<"missing">>}},
    +                     Options1};
    +                Results ->
    +                    {DocId, Results, Options1}
    +                end
    +            end
    +        end
    +    end.
    +
    +
    +parse_field(<<"id">>, undefined) ->
    +    {ok, undefined};
    +parse_field(<<"id">>, Value) ->
    +    case catch couch_doc:validate_docid(Value) of
    --- End diff --
    
    Ok, good, though I found case catch to be more shorter and simpler.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39591526
  
    --- Diff: src/chttpd_db.erl ---
    @@ -449,6 +449,45 @@ db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req,
     db_req(#httpd{path_parts=[_,<<"_bulk_docs">>]}=Req, _Db) ->
         send_method_not_allowed(Req, "POST");
     
    +
    +db_req(#httpd{method='POST',
    +              path_parts=[_, <<"_bulk_get">>],
    +              mochi_req=MochiReq}=Req,
    +       Db) ->
    +    couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
    +    couch_httpd:validate_ctype(Req, "application/json"),
    +    {JsonProps} = chttpd:json_body_obj(Req),
    +    case couch_util:get_value(<<"docs">>, JsonProps) of
    +    undefined ->
    +        throw({bad_request, <<"Missing JSON list of 'docs'.">>});
    +    Docs ->
    +        #doc_query_args{
    +            options = Options
    +        } = bulk_get_parse_doc_query(Req),
    +
    +        AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
    +        AcceptRelatedMp = MochiReq:accepts_content_type("multipart/related"),
    +
    +        case AcceptMixedMp orelse AcceptRelatedMp of
    +        true ->
    +            throw({not_acceptable,
    --- End diff --
    
    Ok.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39591539
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    --- End diff --
    
    mmm...that's better. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95557755
  
    Please note that Couchbase proposal has a design flaw: you may request only one or all revisions, but you cannot follow open_revs semantic completely when document has, say, 10 conflicts, 7 target is already know and you need to transfer only missing 3 - you'll have to fetch all 10 conflicts and filter them on client side. This is what will be need fixed when we'll migrate to /_bulk_revs.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39495330
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter is not acceptable">>,
    +            throw({bad_request, Msg});
    +        Key when Key =:= "rev"; Key =:= "atts_since" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter should be applied
    +                    on per document basis in payload">>,
    +            throw({bad_request, Msg});
    +        _ ->
    +            parse_doc_query({Key, Value}, Args)
    +        end
    +    end, #doc_query_args{}, chttpd:qs(Req)).
    +
    +bulk_get_open_doc_revs({Props}, Db, Options) ->
    +    case parse_field(<<"id">>, couch_util:get_value(<<"id">>, Props)) of
    +    {error, {DocId, Error, Reason}} ->
    --- End diff --
    
    indentation is wrong (four spaces please, the unindented form from couchdb original is deprecated)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-135588193
  
    Yeah, CouchDB master is currently failing our build for unrelated reasons; I've tracked it here: https://github.com/pouchdb/pouchdb/issues/4209


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-140336820
  
    I'd like this to land for 2.0 but haven't been focusing attention here. Is PouchDB satisified with the api and functionality?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by janl <gi...@git.apache.org>.
Github user janl commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95649095
  
    In addition, I think once this lands and alleviate the most egregious replication performance issues, we should all look at figuring out a completely new endpoint (e.g. `/_stream`) that has more unified semantics. But one thing at a time, let’s get this moving first :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98394660
  
    >  | http 'http://localhost:15984/test/_bulk_get?revs=true'
    
    (;


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-106049749
  
    @kxepal I have never seen that error before. But if `npm link` is not working, you can always explicitly link it like so:
    
    ```
    rm -fr node_modules/express-pouchdb
    ln -s ../../express-pouchdb node_modules/express-pouchdb
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-106976630
  
    @kxepal ping me if you need help, in fact npm is the most friendly package manager I ever used :)
    
    I can probably also help with node-gyo issues...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98407398
  
    OK, I have an implementation that works in both PouchDB and PouchDB Server. It needs to be tested against @kxepal's API somehow, though. When I try to run this branch against the PouchDB test suite, i see various errors, including [this one](https://gist.github.com/nolanlawson/efa5f362184af4aea3df). I have no idea what needs to be done to fix it; I haven't been following the cassim repo or any other fix. :)
    
    Pull requests:
     * https://github.com/pouchdb/pouchdb/pull/3783
     * https://github.com/pouchdb/express-pouchdb/pull/208



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39495390
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter is not acceptable">>,
    +            throw({bad_request, Msg});
    +        Key when Key =:= "rev"; Key =:= "atts_since" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter should be applied
    +                    on per document basis in payload">>,
    +            throw({bad_request, Msg});
    +        _ ->
    +            parse_doc_query({Key, Value}, Args)
    +        end
    +    end, #doc_query_args{}, chttpd:qs(Req)).
    +
    +bulk_get_open_doc_revs({Props}, Db, Options) ->
    +    case parse_field(<<"id">>, couch_util:get_value(<<"id">>, Props)) of
    +    {error, {DocId, Error, Reason}} ->
    +        {DocId, {error, {null, Error, Reason}}, Options};
    +
    +    {ok, undefined} ->
    +        {null, {error, {null, bad_request, <<"document id missed">>}}, Options};
    +
    +    {ok, DocId} ->
    +        RevStr = couch_util:get_value(<<"rev">>, Props),
    +        case parse_field(<<"rev">>, RevStr) of
    +        {error, {RevStr, Error, Reason}} ->
    +            {DocId, {error, {RevStr, Error, Reason}}, Options};
    +
    +        {ok, Rev} ->
    +            Revs = case Rev of undefined -> all; _ -> [Rev] end,
    +            AttsSinceStr = couch_util:get_value(<<"atts_since">>, Props),
    +
    +            case parse_field(<<"atts_since">>, AttsSinceStr) of
    +            {error, {BadAttsSinceRev, Error, Reason}} ->
    +                {DocId, {error, {BadAttsSinceRev, Error, Reason}}, Options};
    +
    +            {ok, AttsSince} ->
    +                Options1 = case AttsSince of
    +                    [] ->
    +                        Options;
    +                    RevList ->
    +                        [{atts_since, RevList}, attachments | Options]
    +                end,
    +
    +                case fabric:open_revs(Db, DocId, Revs, Options1) of
    +                {ok, []} ->
    +                    {DocId,
    +                     {error, {RevStr, <<"not_found">>, <<"missing">>}},
    +                     Options1};
    +                Results ->
    +                    {DocId, Results, Options1}
    +                end
    +            end
    +        end
    +    end.
    +
    +
    +parse_field(<<"id">>, undefined) ->
    +    {ok, undefined};
    +parse_field(<<"id">>, Value) ->
    +    case catch couch_doc:validate_docid(Value) of
    +        {Error, Reason} ->  {error, {Value, Error, Reason}};
    +        ok -> {ok, Value}
    +    end;
    +parse_field(<<"rev">>, undefined) ->
    +    {ok, undefined};
    +parse_field(<<"rev">>, Value) ->
    +    case catch couch_doc:parse_rev(Value) of
    --- End diff --
    
    ditto


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-95577933
  
    Looks great! I thought the original plan was to have `_bulk_revs` and `_bulk_docs` be synonymous, though? It might be confusing to introduce two slightly different APIs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-137405230
  
    @willholley done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39497223
  
    --- Diff: src/chttpd_db.erl ---
    @@ -449,6 +449,45 @@ db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req,
     db_req(#httpd{path_parts=[_,<<"_bulk_docs">>]}=Req, _Db) ->
         send_method_not_allowed(Req, "POST");
     
    +
    +db_req(#httpd{method='POST',
    +              path_parts=[_, <<"_bulk_get">>],
    +              mochi_req=MochiReq}=Req,
    +       Db) ->
    +    couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
    +    couch_httpd:validate_ctype(Req, "application/json"),
    +    {JsonProps} = chttpd:json_body_obj(Req),
    +    case couch_util:get_value(<<"docs">>, JsonProps) of
    +    undefined ->
    +        throw({bad_request, <<"Missing JSON list of 'docs'.">>});
    +    Docs ->
    +        #doc_query_args{
    +            options = Options
    +        } = bulk_get_parse_doc_query(Req),
    +
    +        AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
    +        AcceptRelatedMp = MochiReq:accepts_content_type("multipart/related"),
    +
    +        case AcceptMixedMp orelse AcceptRelatedMp of
    +        true ->
    +            throw({not_acceptable,
    --- End diff --
    
    It's left as reserved place for TODO: to have compatible with Couchbase on this endpoint, we need to support multipart responses as they don't use JSON ones. I didn't finished that part, suddenly ):


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by willholley <gi...@git.apache.org>.
Github user willholley commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-140141043
  
    to elaborate, PouchDB is adding query parameters (revs=true, attachments=true) to the request body instead of the query string.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39495399
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter is not acceptable">>,
    +            throw({bad_request, Msg});
    +        Key when Key =:= "rev"; Key =:= "atts_since" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter should be applied
    +                    on per document basis in payload">>,
    +            throw({bad_request, Msg});
    +        _ ->
    +            parse_doc_query({Key, Value}, Args)
    +        end
    +    end, #doc_query_args{}, chttpd:qs(Req)).
    +
    +bulk_get_open_doc_revs({Props}, Db, Options) ->
    +    case parse_field(<<"id">>, couch_util:get_value(<<"id">>, Props)) of
    +    {error, {DocId, Error, Reason}} ->
    +        {DocId, {error, {null, Error, Reason}}, Options};
    +
    +    {ok, undefined} ->
    +        {null, {error, {null, bad_request, <<"document id missed">>}}, Options};
    +
    +    {ok, DocId} ->
    +        RevStr = couch_util:get_value(<<"rev">>, Props),
    +        case parse_field(<<"rev">>, RevStr) of
    +        {error, {RevStr, Error, Reason}} ->
    +            {DocId, {error, {RevStr, Error, Reason}}, Options};
    +
    +        {ok, Rev} ->
    +            Revs = case Rev of undefined -> all; _ -> [Rev] end,
    +            AttsSinceStr = couch_util:get_value(<<"atts_since">>, Props),
    +
    +            case parse_field(<<"atts_since">>, AttsSinceStr) of
    +            {error, {BadAttsSinceRev, Error, Reason}} ->
    +                {DocId, {error, {BadAttsSinceRev, Error, Reason}}, Options};
    +
    +            {ok, AttsSince} ->
    +                Options1 = case AttsSince of
    +                    [] ->
    +                        Options;
    +                    RevList ->
    +                        [{atts_since, RevList}, attachments | Options]
    +                end,
    +
    +                case fabric:open_revs(Db, DocId, Revs, Options1) of
    +                {ok, []} ->
    +                    {DocId,
    +                     {error, {RevStr, <<"not_found">>, <<"missing">>}},
    +                     Options1};
    +                Results ->
    +                    {DocId, Results, Options1}
    +                end
    +            end
    +        end
    +    end.
    +
    +
    +parse_field(<<"id">>, undefined) ->
    +    {ok, undefined};
    +parse_field(<<"id">>, Value) ->
    +    case catch couch_doc:validate_docid(Value) of
    +        {Error, Reason} ->  {error, {Value, Error, Reason}};
    +        ok -> {ok, Value}
    +    end;
    +parse_field(<<"rev">>, undefined) ->
    +    {ok, undefined};
    +parse_field(<<"rev">>, Value) ->
    +    case catch couch_doc:parse_rev(Value) of
    +        {bad_request=Error, Reason} ->  {error, {Value, Error, Reason}};
    +        Rev -> {ok, Rev}
    +    end;
    +parse_field(<<"atts_since">>, undefined) ->
    +    {ok, []};
    +parse_field(<<"atts_since">>, []) ->
    +    {ok, []};
    +parse_field(<<"atts_since">>, Value) when is_list(Value) ->
    +    RevList = lists:foldl(fun
    +        (RevStr, Acc) when is_list(Acc) ->
    +            case catch couch_doc:parse_rev(RevStr) of
    --- End diff --
    
    ditto


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-106053906
  
    @nolanlawson thanks, that works, but now I recall that node-gyp tried to build broken v8 for my host. *sigh* it doesn't simply to build pouchdb /: Downloading Ububuntu ISO image now...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39497299
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    --- End diff --
    
    Like this:
    ```
    bulk_get_parse_doc_query(Req) ->
        lists:foldl(fun({Key,Value}, Args) ->
            case Key of
                "open_revs"  -> bad_query_param(not_acceptable, Key);
                "new_edits"  -> bad_query_param(not_acceptable, Key);
                "w"          -> bad_query_param(not_acceptable, Key);
                "rev"        -> bad_query_param(should_be_inlined, Key);
                "atts_since" -> bad_query_param(should_be_inlined, Key);
                _            -> parse_doc_query({Key, Value}, Args)
            end
        end, #doc_query_args{}, chttpd:qs(Req)).
    
    bad_query_param(Error, Key) when is_list(Key) ->
        bad_query_param(Error, ?l2b(Key));
    bad_query_param(not_acceptable, Key) ->
        Msg = <<"\"", Key/binary, "\" query parameter is not acceptable">>,
        throw({bad_request, Msg});
    bad_query_param(should_be_inlined, Key) ->
        Msg = <<"\"", Key/binary, "\" query parameter should be applied
                 on per document basis in payload">>,
        throw({bad_request, Msg}).
    ```
    ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39497381
  
    --- Diff: src/chttpd_db.erl ---
    @@ -1536,6 +1576,135 @@ set_namespace(<<"_design_docs">>, Args) ->
     set_namespace(NS, #mrargs{extra = Extra} = Args) ->
         Args#mrargs{extra = [{namespace, NS} | Extra]}.
     
    +
    +%% /db/_bulk_get stuff
    +
    +bulk_get_parse_doc_query(Req) ->
    +    lists:foldl(fun({Key,Value}, Args) ->
    +        case Key of
    +        Key when Key =:= "open_revs"; Key =:= "new_edits"; Key =:= "w" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter is not acceptable">>,
    +            throw({bad_request, Msg});
    +        Key when Key =:= "rev"; Key =:= "atts_since" ->
    +            BKey = ?l2b(Key),
    +            Msg = <<"\"", BKey/binary, "\" query parameter should be applied
    +                    on per document basis in payload">>,
    +            throw({bad_request, Msg});
    +        _ ->
    +            parse_doc_query({Key, Value}, Args)
    +        end
    +    end, #doc_query_args{}, chttpd:qs(Req)).
    +
    +bulk_get_open_doc_revs({Props}, Db, Options) ->
    +    case parse_field(<<"id">>, couch_util:get_value(<<"id">>, Props)) of
    +    {error, {DocId, Error, Reason}} ->
    --- End diff --
    
    eh, that was a "hack" to fit 80 chars margin (:  Ok, will try to find a way to break deeply nested cases. That's not good to have them anyway.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#discussion_r39495209
  
    --- Diff: src/chttpd_db.erl ---
    @@ -449,6 +449,45 @@ db_req(#httpd{method='POST',path_parts=[_,<<"_bulk_docs">>], user_ctx=Ctx}=Req,
     db_req(#httpd{path_parts=[_,<<"_bulk_docs">>]}=Req, _Db) ->
         send_method_not_allowed(Req, "POST");
     
    +
    +db_req(#httpd{method='POST',
    +              path_parts=[_, <<"_bulk_get">>],
    +              mochi_req=MochiReq}=Req,
    +       Db) ->
    +    couch_stats:increment_counter([couchdb, httpd, bulk_requests]),
    +    couch_httpd:validate_ctype(Req, "application/json"),
    +    {JsonProps} = chttpd:json_body_obj(Req),
    +    case couch_util:get_value(<<"docs">>, JsonProps) of
    +    undefined ->
    +        throw({bad_request, <<"Missing JSON list of 'docs'.">>});
    +    Docs ->
    +        #doc_query_args{
    +            options = Options
    +        } = bulk_get_parse_doc_query(Req),
    +
    +        AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
    +        AcceptRelatedMp = MochiReq:accepts_content_type("multipart/related"),
    +
    +        case AcceptMixedMp orelse AcceptRelatedMp of
    +        true ->
    +            throw({not_acceptable,
    --- End diff --
    
    why do this? If application/json is acceptable (which we don't even test for here), we can send the response, it doesn't matter if the request would accept other types.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98403217
  
    One slightly awkward thing I noticed: if you don't specify any `Accept` header at all, then you get an error:
    
        {"error":"not_acceptable","reason":"Only application/json response is acceptable."}
    
    Shouldn't `application/json` be the expected default?
    
    To reproduce:
    
        curl -X POST "127.0.0.1:15984/mydb/_bulk_get" -H 'content-type:application/json' --data-binary '{"revs": true, "docs":[{"id":"foo","rev":"1-967a00dff5e02add41819138abb3284d"}]}'
    
    Whereas this works:
    
        curl -X POST "127.0.0.1:15984/mydb/_bulk_get" -H 'content-type:application/json' -H 'accept:application/json' --data-binary '{"revs": true, "docs":[{"id":"foo","rev":"1-967a00dff5e02add41819138abb3284d"}]}'


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by janl <gi...@git.apache.org>.
Github user janl commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-105252240
  
    The referenced PR that broke PouchDB tests against CouchDB master have been merged.
    
    Running the PouchDB tests with the linked repos as @nolanlawson explained now gives me this error:
    
    ```
     test.views.js-local-http
    
    
      1 passing (4s)
      1 failing
    
      1) test.all_docs.js-http Testing all docs:
    
          correct number of results
          + expected - actual
    
          +4
          -3
          
          at /private/tmp/pouchdb/tests/integration/test.all_docs.js:33:36
          at /private/tmp/pouchdb/lib/utils.js:360:11
          at process._tickCallback (node.js:442:13)
    
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98394888
  
    Ohhhhhh, didn't see that. :)
    
    BTW I managed to build everything, but unfortunately forgot that the tests are not currently passing against CouchDB master. So even with PouchDB master and CouchDB master, I can't get the tests to pass, so unfortunately I will just have to wing it.
    
    I have a strawman implementation done; I will submit it as a PR to both pouchdb and express-pouchdb today, and hopefully somebody with a better setup than mine can compare whether my implementation matches yours...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-chttpd pull request: Implement /db/_bulk_get endpoint

Posted by nolanlawson <gi...@git.apache.org>.
Github user nolanlawson commented on the pull request:

    https://github.com/apache/couchdb-chttpd/pull/33#issuecomment-98408019
  
    Attachment replication with Couchbase is already broken because they only accept `multipart/mixed` and we only accept base64. :) `multipart/mixed` is on our roadmap though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---