Allow all params to be passed via body for POST view

This change should allow users to supply all params in POST
that can be supplied for GET now. This way we could avoid the ?key="foo"
things that would probably cause a lot of pain for users.

Author: Bessenyei Balázs Donát

src/chttpd/src/chttpd_view.erl

@@ -39,9 +39,15 @@ multi_query_view(Req, Db, DDoc, ViewName, Queries) ->
     {ok, Resp1} = chttpd:send_delayed_chunk(VAcc2#vacc.resp, "\r\n]}"),
     chttpd:end_delayed_json_response(Resp1).
 
+design_doc_post_view(Req, Props, Db, DDoc, ViewName, Keys) ->
+    Args = couch_mrview_http:parse_body_and_query(Req, Props, Keys),
+    fabric_query_view(Db, Req, DDoc, ViewName, Args).
 
 design_doc_view(Req, Db, DDoc, ViewName, Keys) ->
     Args = couch_mrview_http:parse_params(Req, Keys),
+    fabric_query_view(Db, Req, DDoc, ViewName, Args).
+
+fabric_query_view(Db, Req, DDoc, ViewName, Args) ->
     Max = chttpd:chunked_response_buffer_size(),
     VAcc = #vacc{db=Db, req=Req, threshold=Max},
     Options = [{user_ctx, Req#httpd.user_ctx}],
@@ -89,16 +95,9 @@ handle_view_req(#httpd{method='POST',
     chttpd:validate_ctype(Req, "application/json"),
     Props = couch_httpd:json_body_obj(Req),
     assert_no_queries_param(couch_mrview_util:get_view_queries(Props)),
-    case couch_mrview_util:get_view_keys(Props) of
-        Keys when is_list(Keys) ->
-            couch_stats:increment_counter([couchdb, httpd, view_reads]),
-            design_doc_view(Req, Db, DDoc, ViewName, Keys);
-        _ ->
-            throw({
-                bad_request,
-                "POST body must contain an array called `keys`"
-            })
-    end;
+    Keys = couch_mrview_util:get_view_keys(Props),
+    couch_stats:increment_counter([couchdb, httpd, view_reads]),
+    design_doc_post_view(Req, Props, Db, DDoc, ViewName, Keys);
 
 handle_view_req(Req, _Db, _DDoc) ->
     chttpd:send_method_not_allowed(Req, "GET,POST,HEAD").

src/couch_mrview/src/couch_mrview_http.erl

@@ -29,6 +29,7 @@
     parse_int/1,
     parse_pos_int/1,
     prepend_val/1,
+    parse_body_and_query/3,
     parse_params/2,
     parse_params/3,
     parse_params/4,
@@ -453,12 +454,21 @@ parse_params(Props, Keys, Args) ->
 
 parse_params(Props, Keys, #mrargs{}=Args0, Options) ->
     IsDecoded = lists:member(decoded, Options),
-    % group_level set to undefined to detect if explicitly set by user
-    Args1 = Args0#mrargs{keys=Keys, group=undefined, group_level=undefined},
+    Args1 = case lists:member(keep_group_level, Options) of
+        true ->
+            Args0;
+        _ ->
+            % group_level set to undefined to detect if explicitly set by user
+            Args0#mrargs{keys=Keys, group=undefined, group_level=undefined}
+    end,
     lists:foldl(fun({K, V}, Acc) ->
         parse_param(K, V, Acc, IsDecoded)
     end, Args1, Props).
 
+parse_body_and_query(Req, {Props}, Keys) ->
+    Args = #mrargs{keys=Keys, group=undefined, group_level=undefined},
+    BodyArgs = parse_params(Props, Keys, Args, [decoded]),
+    parse_params(chttpd:qs(Req), Keys, BodyArgs, [keep_group_level]).
 
 parse_param(Key, Val, Args, IsDecoded) when is_binary(Key) ->
     parse_param(binary_to_list(Key), Val, Args, IsDecoded);

src/couch_mrview/src/couch_mrview_util.erl

@@ -1152,7 +1152,6 @@ extract_view_reduce({red, {N, _Lang, #mrview{reduce_funs=Reds}}, _Ref}) ->
 get_view_keys({Props}) ->
     case couch_util:get_value(<<"keys">>, Props) of
         undefined ->
-            couch_log:debug("POST with no keys member.", []),
             undefined;
         Keys when is_list(Keys) ->
             Keys;

test/elixir/test/view_test.exs

@@ -0,0 +1,143 @@
+defmodule ViewTest do
+  use CouchTestCase
+
+  @moduletag :view
+
+  @moduledoc """
+  Test CouchDB /{db}/_design/{ddoc}/_view/{view}
+  """
+
+  setup_all do
+    db_name = random_db_name()
+    {:ok, _} = create_db(db_name)
+    on_exit(fn -> delete_db(db_name) end)
+
+    {:ok, _} = create_doc(
+      db_name,
+      %{
+        _id: "foo",
+        bar: "baz"
+      }
+    )
+
+    {:ok, _} = create_doc(
+      db_name,
+      %{
+        _id: "foo2",
+        bar: "baz2"
+      }
+    )
+
+    map_fun = """
+      function(doc) {
+        emit(doc._id, doc.bar);
+      }
+    """
+
+
+    body = %{
+      :docs => [
+        %{
+          _id: "_design/map",
+          views: %{
+            some: %{
+              map: map_fun
+            }
+          }
+        }
+      ]
+    }
+
+    resp = Couch.post("/#{db_name}/_bulk_docs", body: body)
+    Enum.each(resp.body, &assert(&1["ok"]))
+
+    {:ok, [db_name: db_name]}
+  end
+
+  test "GET with no parameters", context do
+    resp = Couch.get(
+      "/#{context[:db_name]}/_design/map/_view/some"
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 2
+  end
+
+  test "GET with one key", context do
+    resp = Couch.get(
+      "/#{context[:db_name]}/_design/map/_view/some",
+      query: %{
+        :key => "\"foo\"",
+      }
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 1
+  end
+
+  test "GET with multiple keys", context do
+    resp = Couch.get(
+      "/#{context[:db_name]}/_design/map/_view/some",
+      query: %{
+        :keys => "[\"foo\", \"foo2\"]",
+      }
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 2
+  end
+
+  test "POST with empty body", context do
+    resp = Couch.post(
+      "/#{context[:db_name]}/_design/map/_view/some",
+      body: %{}
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 2
+  end
+
+  test "POST with keys and limit", context do
+    resp = Couch.post(
+      "/#{context[:db_name]}/_design/map/_view/some",
+      body: %{
+        :keys => ["foo", "foo2"],
+        :limit => 1
+      }
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 1
+  end
+
+  test "POST with query parameter and JSON body", context do
+    resp = Couch.post(
+      "/#{context[:db_name]}/_design/map/_view/some",
+      query: %{
+        :limit => 1
+      },
+      body: %{
+        :keys => ["foo", "foo2"]
+      }
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 1
+  end
+
+  test "POST edge case with colliding parameters - query takes precedence", context do
+    resp = Couch.post(
+      "/#{context[:db_name]}/_design/map/_view/some",
+      query: %{
+        :limit => 1
+      },
+      body: %{
+        :keys => ["foo", "foo2"],
+        :limit => 2
+      }
+    )
+
+    assert resp.status_code == 200
+    assert length(Map.get(resp, :body)["rows"]) == 1
+  end
+end