[GitHub] [couchdb] iilyak commented on a change in pull request #3094: Fix ordering of page_size based pagination for views

[GitBox <gi...@apache.org>]

iilyak commented on a change in pull request #3094:
URL: https://github.com/apache/couchdb/pull/3094#discussion_r480341255



##########
File path: src/chttpd/test/exunit/pagination_test.exs
##########
@@ -1163,7 +1221,153 @@ defmodule Couch.Test.Pagination do
             assert length(result["rows"]) > ctx.page_size
           end)
         end
+
+        test "can retrieve all pages", ctx do
+          [descending_query, limit_query] =
+            paginate_queries(
+              ctx,
+              url:
+                "/#{ctx.db_name}/_design/#{ctx.ddoc_id}/_view/#{ctx.view_name}/queries",
+              query: %{page_size: ctx.page_size, descending: ctx.descending},
+              body: :jiffy.encode(ctx.queries)
+            )
+
+          results = List.flatten(descending_query)
+          assert ctx.n_docs == length(results)
+          expected_key_order = :descending
+          expected_ids_order = :ascending
+          assert expected_key_order == ordering?(results, "key"),
+            "expecing keys in #{expected_key_order} order, got: #{inspect field(results, "key")}"
+          assert expected_ids_order == ordering?(results, "id"),
+            "expecing ids in #{expected_ids_order} order, got: #{inspect field(results, "id")}"
+
+          results = List.flatten(limit_query)
+          expected_length = if ctx.page_size + 2 > ctx.n_docs do
+            ctx.page_size + 1 - 2
+          else
+            ctx.page_size + 1
+          end
+          assert expected_length == length(results)
+
+          {expected_key_order, expected_ids_order} = if ctx.descending do
+            {:descending, :ascending}
+          else
+            {:ascending, :descending}
+          end
+          assert expected_key_order == ordering?(results, "key"),
+            ~s(expecing keys in #{expected_key_order} order, got: #{inspect field(results, "key")})
+          assert expected_ids_order == ordering?(results, "id"),
+            ~s(expecing keys in #{expected_ids_order} order, got: #{inspect field(results, "id")})
+          keys = Enum.map(results, &(Map.get(&1, "key")))
+        end
       end
     end
   end
+
+  for descending <- [false, true] do
+    for n <- [4, 9] do
+      describe "Pagination API (10 docs) : /{db}/_design/{ddoc}/_view/queries?page_size=#{
+                 n
+               }&descending=#{descending} : pages with same key" do
+        @describetag descending: descending
+        @describetag n_docs: 10
+        @describetag page_size: n
+
+        @describetag queries: %{
+                       queries: [
+                         %{
+                           descending: true
+                         },
+                         %{
+                           limit: n + 1,
+                           skip: 2
+                         }
+                       ]
+                     }
+        setup [:with_session, :random_db, :with_view, :with_same_key_docs]
+
+        test "handle same key", ctx do
+          '''
+          make sure the results are first sorted by key and then by id
+          '''
+          [descending_query, limit_query] =
+            paginate_queries(
+              ctx,
+              url:
+                "/#{ctx.db_name}/_design/#{ctx.ddoc_id}/_view/#{ctx.view_name}/queries",
+              query: %{page_size: ctx.page_size, descending: ctx.descending},
+              body: :jiffy.encode(ctx.queries)
+            )
+          IO.puts("descending_query 1: #{inspect descending_query}")
+          aggregate = fn pages ->
+            Enum.reduce(pages, {[], %{}}, fn page, acc ->
+              Enum.reduce(page, acc, fn row, {keys, in_acc} ->
+                id = Map.get(row, "id")
+                key = Map.get(row, "key")
+                {keys ++ [key], Map.update(in_acc, key, [id], &(&1 ++ [id]))}
+              end)
+            end)
+          end
+          {keys, aggregated} = aggregate.(descending_query)
+
+          # keys are sorted in reverse order
+          assert :descending == ordering?(keys),
+            ~s(expecing keys in descending order, got: #{inspect keys})
+
+
+          Enum.each(Map.values(aggregated), fn ids ->
+            # Keys are sorted in reverse order by id
+            assert :descending == ordering?(ids),
+              ~s(expecing ids in descending order, got: #{inspect ids})
+          end)
+
+          {keys, aggregated} = aggregate.(limit_query)
+          {expected_key_order, expected_ids_order} = if ctx.descending do
+            {:descending, :descending}
+          else
+            {:ascending, :ascending}
+          end
+
+          # keys are sorted
+          assert expected_key_order == ordering?(keys) or :equal == ordering?(keys),
+            ~s(expecing keys in #{expected_key_order} order, got: #{inspect keys})
+
+          Enum.each(Map.values(aggregated), fn ids ->
+            # Keys are sorted by id
+            assert expected_ids_order == ordering?(ids) or :equal == ordering?(ids),
+              ~s(expecing ids in #{expected_ids_order} order, got: #{inspect ids})
+          end)
+        end
+      end
+    end
+  end
+
+  defp ordering?(maps, key) do
+    ordering?(field(maps, key))
+  end
+
+  defp ordering?(elements) do
+    ascending = Enum.sort(elements)
+    descending = Enum.reverse(Enum.sort(elements))
+    '''
+    case elements do
+      ^ascending ->
+        :ascending
+      ^descending ->
+        :descending
+      _ ->
+        :unordered
+    end
+    '''
+    case {ascending, descending} do
+      {^elements, ^elements} -> :equal
+      {^elements, _} -> :ascending
+      {_, ^descending} -> :descending
+      _ -> :unordered
+    end
+  end
+
+  defp field(maps, key) do
+    Enum.map(maps, &(Map.get(&1, key)))

Review comment:
       Can you elaborate a little bit?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org