Encode startkey/endkey for all_docs (#2538)

* Encode startkey/endkey for all_docs

Encodes the startkey/endkey so that if a startkey is not binary it will return the expected results.

3 files changed, 60 insertions, 16 deletions

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 108881f72..2e3af2064 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -926,13 +926,13 @@ all_docs_view_opts(Args) ->
         EKey -> EKey
     end,
     StartKeyOpts = case StartKey of
-        <<_/binary>> -> [{start_key, StartKey}];
-        undefined -> []
+        undefined -> [];
+        _ -> [{start_key, fabric2_util:encode_all_doc_key(StartKey)}]
     end,
     EndKeyOpts = case {EndKey, Args#mrargs.inclusive_end} of
-        {<<_/binary>>, false} -> [{end_key_gt, EndKey}];
-        {<<_/binary>>, true} -> [{end_key, EndKey}];
-        {undefined, _} -> []
+        {undefined, _} -> [];
+        {_, false} -> [{end_key_gt, fabric2_util:encode_all_doc_key(EndKey)}];
+        {_, true} -> [{end_key, fabric2_util:encode_all_doc_key(EndKey)}]
     end,
     [
         {dir, Args#mrargs.direction},
diff --git a/src/fabric/src/fabric2_util.erl b/src/fabric/src/fabric2_util.erl
index 4e2e2d76b..2a940659e 100644
--- a/src/fabric/src/fabric2_util.erl
+++ b/src/fabric/src/fabric2_util.erl
@@ -33,7 +33,9 @@
     get_value/3,
     to_hex/1,
     from_hex/1,
-    uuid/0
+    uuid/0,
+
+    encode_all_doc_key/1
 ]).
 
 
@@ -235,3 +237,12 @@ hex_to_nibble(N) ->
 
 uuid() ->
     to_hex(crypto:strong_rand_bytes(16)).
+
+
+encode_all_doc_key(null) -> <<>>;
+encode_all_doc_key(true) -> <<>>;
+encode_all_doc_key(false) -> <<>>;
+encode_all_doc_key(N) when is_number(N) -> <<>>;
+encode_all_doc_key(B) when is_binary(B) -> B;
+encode_all_doc_key(L) when is_list(L) -> <<255>>;
+encode_all_doc_key({O}) when is_list(O) -> <<255>>.
diff --git a/test/elixir/test/all_docs_test.exs b/test/elixir/test/all_docs_test.exs
index 7e154eb0f..e3bca0f35 100644
--- a/test/elixir/test/all_docs_test.exs
+++ b/test/elixir/test/all_docs_test.exs
@@ -41,11 +41,9 @@ defmodule AllDocsTest do
     assert resp["total_rows"] == length(rows)
 
     # Check _all_docs offset
-    retry_until(fn ->
-      resp = Couch.get("/#{db_name}/_all_docs", query: %{:startkey => "\"2\""}).body
-      assert resp["offset"] == :null
-      assert Enum.at(resp["rows"], 0)["key"] == "2"
-    end)
+    resp = Couch.get("/#{db_name}/_all_docs", query: %{:startkey => "\"2\""}).body
+    assert resp["offset"] == :null
+    assert Enum.at(resp["rows"], 0)["key"] == "2"
 
     # Confirm that queries may assume raw collation
     resp =
@@ -73,11 +71,9 @@ defmodule AllDocsTest do
     changes = Couch.get("/#{db_name}/_changes").body["results"]
     assert length(changes) == 4
 
-    retry_until(fn ->
-      deleted = Enum.filter(changes, fn row -> row["deleted"] end)
-      assert length(deleted) == 1
-      assert hd(deleted)["id"] == "1"
-    end)
+    deleted = Enum.filter(changes, fn row -> row["deleted"] end)
+    assert length(deleted) == 1
+    assert hd(deleted)["id"] == "1"
 
     # (remember old seq)
     orig_doc = Enum.find(changes, fn row -> row["id"] == "3" end)
@@ -187,4 +183,41 @@ defmodule AllDocsTest do
 
     assert length(rows) == 1
   end
+
+  @tag :with_db
+  test "all_docs ordering", context do
+    db_name = context[:db_name]
+
+    docs = [
+      %{:_id => "a"},
+      %{:_id => "m"},
+      %{:_id => "z"}
+    ]
+
+    resp = Couch.post("/#{db_name}/_bulk_docs", body: %{:docs => docs})
+    Enum.each(resp.body, &assert(&1["ok"]))
+
+    resp = Couch.get("/#{db_name}/_all_docs", query: %{:startkey => false}).body
+    rows = resp["rows"]
+    assert length(rows) === 3
+    assert get_ids(resp) == ["a", "m", "z"]
+
+    resp = Couch.get("/#{db_name}/_all_docs", query: %{:startkey => 0}).body
+    rows = resp["rows"]
+    assert length(rows) === 3
+    assert get_ids(resp) == ["a", "m", "z"]
+
+    resp = Couch.get("/#{db_name}/_all_docs", query: %{:startkey => "[1,2]"}).body
+    rows = resp["rows"]
+    assert length(rows) === 0
+
+    resp = Couch.get("/#{db_name}/_all_docs", query: %{:end_key => 0}).body
+    rows = resp["rows"]
+    assert length(rows) === 0
+  end
+
+  defp get_ids(resp) do
+    %{"rows" => rows} = resp
+    Enum.map(rows, fn row -> row["id"] end)
+  end
 end