Skip to content

Support queries endpoints#1222

Merged
jiangphcn merged 1 commit intoapache:masterfrom
cloudant:issue-820-add-queries
Mar 22, 2018
Merged

Support queries endpoints#1222
jiangphcn merged 1 commit intoapache:masterfrom
cloudant:issue-820-add-queries

Conversation

@jiangphcn
Copy link
Copy Markdown
Contributor

@jiangphcn jiangphcn commented Mar 14, 2018

Overview

Allow to POST to /{db}/_all_docs/queries and specify a queries list in the body, the same way you can do

  • for design documents /{db}/_design_docs/queries
  • for local documents /{db}/_local_docs/queries
  • and for a regular MapReduce view /{db}/_design/{ddoc}/_view/{viewname}/queries
curl -u foo:bar -H "Content-Type: application/json" -X POST -d @multiple_queries.json http://localhost:15984/db/_all_docs/queries
{"results":[
{"total_rows":30,"rows":[
{"id":"point2","key":"point2","value":{"rev":"1-d942f0ce01647aa0f46518b213b5628e"}},
{"id":"point3","key":"point3","value":{"rev":"1-721fead6e6c8d811a225d5a62d08dfd0"}}
]},
{"total_rows":30,"offset":2,"rows":[
{"id":"point11","key":"point11","value":{"rev":"1-ffb6c2ae737918f8f911e1fce668333e"}},
{"id":"point12","key":"point12","value":{"rev":"1-c32d315f69ce72128244a60d0cbe4628"}},
{"id":"point13","key":"point13","value":{"rev":"1-a9b955ca17f553398cd877629e803c5d"}},
{"id":"point14","key":"point14","value":{"rev":"1-2f2aa0db2a8c35754e7ab2308940f60c"}},
{"id":"point15","key":"point15","value":{"rev":"1-247191505d8fa6b4b9dc426321987b17"}}
]}
]}

multiple_queries.json looks like

    {
        "queries": [
            {
                "keys": [
                    "point2",
                    "point3"
                ]
            },
            {
                "limit": 5,
                "skip": 2
            }
        ]
    }

Testing recommendations

make check skip_deps+=couch_epi apps=chttpd tests=all_test_

Compiled test/chttpd_db_test.erl
    Running test function(s):
      chttpd_db_test:all_db_test_/0
======================== EUnit ========================
chttpd db tests
Application crypto was left running!
  chttpd_db_test:90: should_return_ok_true_on_bulk_update...[0.082 s] ok
  chttpd_db_test:117: should_accept_live_as_an_alias_for_continuous...[0.051 s] ok
  chttpd_db_test:132: should_return_404_for_delete_att_on_notadoc...[0.006 s] ok
  chttpd_db_test:154: should_return_409_for_del_att_without_rev...[0.072 s] ok
  chttpd_db_test:172: should_return_200_for_del_att_with_rev...[0.061 s] ok
  chttpd_db_test:193: should_return_409_for_put_att_nonexistent_rev...[0.016 s] ok
  chttpd_db_test:208: should_return_update_seq_when_set_on_all_docs...[0.095 s] ok
  chttpd_db_test:222: should_not_return_update_seq_when_unset_on_all_docs...[0.080 s] ok
  chttpd_db_test:236: should_return_correct_id_on_doc_copy...[0.091 s] ok
  chttpd_db_test:267: should_return_400_for_bad_engine...[0.003 s] ok
  chttpd_db_test:279: should_succeed_on_all_docs_with_queries_keys...[0.250 s] ok
  chttpd_db_test:293: should_succeed_on_all_docs_with_queries_limit_skip...[0.266 s] ok
  chttpd_db_test:308: should_succeed_on_all_docs_with_multiple_queries...[0.259 s] ok
  chttpd_db_test:326: should_succeed_on_design_docs_with_queries_keys...[0.246 s] ok
  chttpd_db_test:341: should_succeed_on_design_docs_with_queries_limit_skip...[0.267 s] ok
  chttpd_db_test:356: should_succeed_on_design_docs_with_multiple_queries...[0.253 s] ok
  chttpd_db_test:374: should_succeed_on_local_docs_with_queries_keys...[0.237 s] ok
  chttpd_db_test:389: should_succeed_on_local_docs_with_queries_limit_skip...[0.239 s] ok
  chttpd_db_test:403: should_succeed_on_local_docs_with_multiple_queries...[0.253 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  [done in 5.480 s]
=======================================================
  All 19 tests passed.

make check skip_deps+=couch_epi apps=chttpd tests=all_view_test_

Compiled src/chttpd_db.erl
    Running test function(s):
      chttpd_view_test:all_view_test_/0
======================== EUnit ========================
chttpd view tests
Application crypto was left running!
  chttpd_view_test:74: should_succeed_on_view_with_queries_keys...[0.510 s] ok
  chttpd_view_test:90: should_succeed_on_view_with_queries_limit_skip...[0.282 s] ok
  chttpd_view_test:107: should_succeed_on_view_with_multiple_queries...[0.280 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  [done in 1.650 s]
=======================================================
  All 3 tests passed.

Related Issues or Pull Requests

#820
#1032
#1143
apache/couchdb-documentation#241

Checklist

@jiangphcn jiangphcn force-pushed the issue-820-add-queries branch from 7308961 to 01448da Compare March 15, 2018 04:04
@jiangphcn jiangphcn changed the title [WIP] Support queries for endpoints Support queries for endpoints Mar 15, 2018
@jiangphcn jiangphcn force-pushed the issue-820-add-queries branch from 01448da to c0802dc Compare March 16, 2018 03:57
@jiangphcn jiangphcn changed the title Support queries for endpoints Support queries endpoints Mar 16, 2018
@jiangphcn
Copy link
Copy Markdown
Contributor Author

Could somebody please help review this PR? Thanks

Copy link
Copy Markdown
Member

@eiri eiri left a comment

Choose a reason for hiding this comment

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

A couple of nit-picks but overall looks good to me. I'd rather see those tests in JS suite, they are about API testing, not unit testing, but I get that your are following bad precedence in chttpd_db_test.erl. The view bits, however, need to go into own suite, something like chttpd_view_test.erl, it's not good to lump everything together.

Comment thread src/chttpd/src/chttpd_db.erl Outdated
path_parts=[_, OP, <<"queries">>]}=Req, Db) when ?IS_ALL_DOCS(OP) ->
Props = chttpd:json_body_obj(Req),
case couch_mrview_util:get_view_queries(Props) of
Queries when Queries =/= undefined ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change order here to make undefined match first, you can avoid a repetitive guard that way.

Comment thread src/chttpd/src/chttpd_db.erl Outdated
<<"POST body must include `queries` parameter.">>})
end;

db_req(#httpd{path_parts=[_, _OP, <<"queries">>]}=Req,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's rather weird you are using guard on underscored var, change _OP to a proper variable name.

Comment thread src/chttpd/src/chttpd_view.erl Outdated
chttpd:validate_ctype(Req, "application/json"),
Props = couch_httpd:json_body_obj(Req),
case couch_mrview_util:get_view_queries(Props) of
Queries when Queries =/= undefined ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here, switch the match order.

Comment thread src/chttpd/test/chttpd_db_test.erl Outdated
fun should_succeed_on_local_docs_with_queries_keys/1,
fun should_succeed_on_local_docs_with_queries_limit_skip/1,
fun should_succeed_on_local_docs_with_multiple_queries/1,
fun should_succeed_on_view_with_queries_keys/1,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Those three tests doesn't belong to chttpd_db_test.erl, they are not testing chttpd db functionality, but views functionality.

@jiangphcn
Copy link
Copy Markdown
Contributor Author

Thanks @eiri for your review. I already addressed your comments in
fe6c197. Could you please check and approve if applicable? And then I can squash and merge them.

-define(DESTHEADER1, {"Destination", "foo%E5%95%8Abar"}).
-define(DESTHEADER2, {"Destination", "foo%2Fbar%23baz%3Fpow%3Afiz"}).


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove empty line change.

Copy link
Copy Markdown
Member

@eiri eiri left a comment

Choose a reason for hiding this comment

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

lgtm. Don't forget to remove that accidental empty line and squash before merging.

  - Add _all_docs/queries endpoint,
  - Add _design_docs/queries endpoint,
  - Add _local_docs/queries endpoint, and
  - Add _design/{ddoc}/_view/{viewname}/queries endpoint

Fixes apache#820
@jiangphcn jiangphcn force-pushed the issue-820-add-queries branch from fe6c197 to 36ecf92 Compare March 22, 2018 01:15
@jiangphcn
Copy link
Copy Markdown
Contributor Author

Thanks @eiri. Removed empty line and squashed commits. Passed CI. Merged it.

@jiangphcn jiangphcn merged commit 0a477b5 into apache:master Mar 22, 2018
@jiangphcn jiangphcn deleted the issue-820-add-queries branch March 22, 2018 02:03
@eiri eiri mentioned this pull request Mar 22, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants