Reject multiple conflicting values of `partition`

If a user specifies different values for `partition` parameters in the
query string or request body the behavior can be surprising on which one
ends up being used. This change provides the user with an error that
indicates the condition rather than leaving the user to figure out why
queries are returning unexpected data.

2 files changed, 34 insertions, 4 deletions

diff --git a/src/dreyfus_httpd.erl b/src/dreyfus_httpd.erl
index f9cdb7ce3..012bfff9c 100644
--- a/src/dreyfus_httpd.erl
+++ b/src/dreyfus_httpd.erl
@@ -189,8 +189,15 @@ parse_index_params(#httpd{method='GET'}=Req, Db) ->
         chttpd:qs(Req)),
     parse_index_params(IndexParams, Db);
 parse_index_params(#httpd{method='POST'}=Req, Db) ->
-    IndexParams = lists:flatmap(fun({K, V}) -> parse_json_index_param(K, V) end,
-        element(1, chttpd:json_body_obj(Req))),
+    {JsonBody} = chttpd:json_body_obj(Req),
+    QSEntry = case chttpd:qs_value(Req, "partition") of
+        undefined -> [];
+        StrVal -> [{<<"partition">>, ?l2b(StrVal)}]
+    end,
+    IndexParams = lists:flatmap(fun({K, V}) ->
+        parse_json_index_param(K, V)
+    end, QSEntry ++ [JsonBody]),
+    ensure_unique_partition(IndexParams),
     parse_index_params(IndexParams, Db);
 parse_index_params(IndexParams, Db) ->
     DefaultLimit = case fabric_util:is_partitioned(Db) of
@@ -424,6 +431,19 @@ parse_non_negative_int_param(Name, Val, Prop, Default) ->
     end.
 
 
+ensure_unique_partition(IndexParams) ->
+    Partitions = lists:filter(fun({Key, _Val}) ->
+        Key == partition
+    end, IndexParams),
+    case length(length:usort(Partitions)) > 1 of
+        true ->
+            Msg = <<"Multiple conflicting values for `partition` provided">>,
+            throw({bad_request, Msg});
+        false ->
+            ok
+    end.
+
+
 validate_search_restrictions(Db, DDoc, Args) ->
     #index_query_args{
         q = Query,
diff --git a/test/elixir/test/partition_search_test.exs b/test/elixir/test/partition_search_test.exs
index f0d32bd17..052a41ad1 100644
--- a/test/elixir/test/partition_search_test.exs
+++ b/test/elixir/test/partition_search_test.exs
@@ -21,7 +21,7 @@ defmodule PartitionSearchTest do
       }
     end
 
-    resp = Couch.post("/#{db_name}/_bulk_docs", body: %{:docs => docs} )
+    resp = Couch.post("/#{db_name}/_bulk_docs", body: %{:docs => docs}, query: %{w: 3})
     assert resp.status_code == 201
   end
 
@@ -126,7 +126,7 @@ defmodule PartitionSearchTest do
     create_ddoc(db_name)
 
     url = "/#{db_name}/_partition/foo/_design/library/_search/books"
-    resp = Couch.post(url, body: %{:q => "some:field", :limit => 1, :partition=> "true"})
+    resp = Couch.post(url, body: %{:q => "some:field", :limit => 1})
     assert resp.status_code == 200
   end
 
@@ -206,4 +206,14 @@ defmodule PartitionSearchTest do
     assert resp.status_code == 400
   end
 
+  @tag :with_partitioned_db
+  test "rejects conflicting partition values", context do
+    db_name = context[:db_name]
+    create_search_docs(db_name)
+    create_ddoc(db_name)
+
+    url = "/#{db_name}/_partition/foo/_design/library/_search/books"
+    resp = Couch.post(url, body: %{q: "some:field", partition: "bar"})
+    assert resp.status_code == 400
+  end
 end