Skip to content

Commit

Permalink
Add key range check for _all_docs
Browse files Browse the repository at this point in the history
Using key and start/end_key in a different order will produce
different responses when querying `_all_docs`. To reduce confusion,
add a key range check for `_all_docs`.

- If direction=fwd, start_key > end_key throws an error
- If direction=rev, start_key < end_key throws an error
- Otherwise, return relevant responses
  • Loading branch information
jiahuili430 committed Apr 26, 2023
1 parent e4bc2b5 commit f42b978
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 37 deletions.
70 changes: 34 additions & 36 deletions src/chttpd/test/eunit/chttpd_view_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
<<"error">> := <<"query_parse_error">>,
<<"reason">> := <<"`keys` is incompatible with `key`, `start_key` and `end_key`">>
}).
-define(ERROR_KEY_RANGE, #{
<<"error">> := <<"query_parse_error">>,
<<"reason">> :=
<<"No rows can match your key range, reverse your ",
"start_key and end_key or set descending=true">>
}).

% seconds
-define(TIMEOUT, 60).
Expand Down Expand Up @@ -163,26 +169,17 @@ t_view_with_multiple_queries(Db) ->
?assertEqual(200, Code).

t_view_with_key_and_start_key(Db) ->
{Code1, Res1} = req(get, url(Db, "_design/ddoc/_view/map", "key=\"a\"&startkey=\"b\"")),
{Code2, Res2} = req(get, url(Db, "_design/ddoc/_view/map", "startkey=\"b\"&key=\"a\"")),
?assertMatch(
#{
<<"error">> := <<"query_parse_error">>,
<<"reason">> :=
<<"No rows can match your key range, reverse your start_key and end_key or set descending=true">>
},
Res1
),
?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2),
?assertEqual(400, Code1),
?assertEqual(200, Code2).
test_helper_key_and_start_key(Db, "_design/ddoc/_view/map").

t_all_docs_with_key_and_start_key(Db) ->
{Code1, Res1} = req(get, url(Db, "_all_docs", "key=\"a\"&startkey=\"b\"")),
{Code2, Res2} = req(get, url(Db, "_all_docs", "startkey=\"b\"&key=\"a\"")),
?assertMatch(#{<<"rows">> := []}, Res1),
test_helper_key_and_start_key(Db, "_all_docs").

test_helper_key_and_start_key(Db, Path) ->
{Code1, Res1} = req(get, url(Db, Path, "key=\"a\"&startkey=\"b\"")),
{Code2, Res2} = req(get, url(Db, Path, "startkey=\"b\"&key=\"a\"")),
?assertMatch(?ERROR_KEY_RANGE, Res1),
?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2),
?assertEqual(200, Code1),
?assertEqual(400, Code1),
?assertEqual(200, Code2).

t_view_with_key_and_end_key(Db) ->
Expand All @@ -200,31 +197,32 @@ test_helper_key_and_end_key(Db, Path) ->
?assertEqual(200, Code2).

t_view_with_single_keys_and_start_key(Db) ->
{Code, Res} = req(get, url(Db, "_design/ddoc/_view/map?keys=[\"a\"]&startkey=\"b\"")),
?assertMatch(
#{
<<"error">> := <<"query_parse_error">>,
<<"reason">> :=
<<"No rows can match your key range, reverse your start_key and end_key or set descending=true">>
},
Res
),
?assertEqual(400, Code).
test_helper_single_keys_and_start_key(Db, "_design/ddoc/_view/map").

t_all_docs_with_single_keys_and_start_key(Db) ->
{Code, Res} = req(get, url(Db, "_all_docs?keys=[\"a\"]&startkey=\"b\"")),
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res),
?assertEqual(400, Code).
test_helper_single_keys_and_start_key(Db, "_all_docs").

test_helper_single_keys_and_start_key(Db, Path) ->
{Code1, Res1} = req(get, url(Db, Path, "keys=[\"a\"]&startkey=\"b\"")),
{Code2, Res2} = req(get, url(Db, Path, "startkey=\"b\"&keys=[\"a\"]")),
?assertMatch(?ERROR_KEY_RANGE, Res1),
?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2),
?assertEqual(400, Code1),
?assertEqual(200, Code2).

t_view_with_keys_and_start_key(Db) ->
{Code, Res} = req(get, url(Db, "_design/ddoc/_view/map", "keys=[\"a\",\"b\"]&start_key=\"b\"")),
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res),
?assertEqual(400, Code).
test_helper_keys_and_start_key(Db, "_design/ddoc/_view/map").

t_all_docs_with_keys_and_start_key(Db) ->
{Code, Res} = req(get, url(Db, "_all_docs", "keys=[\"a\",\"b\"]&start_key=\"b\"")),
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res),
?assertEqual(400, Code).
test_helper_keys_and_start_key(Db, "_all_docs").

test_helper_keys_and_start_key(Db, Path) ->
{Code1, Res1} = req(get, url(Db, Path, "keys=[\"a\",\"b\"]&start_key=\"b\"")),
{Code2, Res2} = req(get, url(Db, Path, "startkey=\"b\"&keys=[\"a\",\"b\"]")),
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res1),
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res2),
?assertEqual(400, Code1),
?assertEqual(400, Code2).

t_view_with_key_non_existent_docs(Db) ->
{Code, Res} = req(get, url(Db, "_design/ddoc/_view/map", "key=\"not_exist\"")),
Expand Down
19 changes: 19 additions & 0 deletions src/couch_mrview/src/couch_mrview_util.erl
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,25 @@ validate_args(Args) ->
ok
end,

case {Args#mrargs.direction, Args#mrargs.start_key, Args#mrargs.end_key} of
{_, undefined, _} ->
ok;
{_, _, undefined} ->
ok;
{fwd, SK, EK} when SK > EK ->
mrverror(
<<"No rows can match your key range, reverse your ",
"start_key and end_key or set descending=true">>
);
{rev, SK, EK} when SK < EK ->
mrverror(
<<"No rows can match your key range, reverse your ",
"start_key and end_key or set descending=true">>
);
_ ->
ok
end,

case Args#mrargs.keys of
Keys when is_list(Keys) -> ok;
undefined -> ok;
Expand Down
2 changes: 1 addition & 1 deletion src/docs/src/api/ddoc/views.rst
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ multi-element ``keys``. For single-element ``keys``, treat it as a ``key``.
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?key="a"'
{"rows":[{"key":null,"value":1}]}
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?keys="[\"a\"]"'
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?keys=\["a"\]'
{"rows":[{"key":null,"value":1}]}
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?keys=\["a","b"\]'
Expand Down

0 comments on commit f42b978

Please sign in to comment.