Add key range check for _all_docs

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

Author: jiahuili430

src/chttpd/test/eunit/chttpd_view_test.erl

@@ -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).
@@ -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) ->
@@ -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\"")),

src/couch_mrview/src/couch_mrview_util.erl

@@ -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;

src/docs/src/api/ddoc/views.rst

@@ -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"\]'