summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorILYA Khlopotov <iilyak@apache.org>2020-10-22 08:58:47 -0700
committerILYA Khlopotov <iilyak@apache.org>2020-10-22 09:16:41 -0700
commitc0efba32fa1ced66b5baf63ef1ab0dc6188974c5 (patch)
tree5fcc8e2c60e7a16b390dfa3961ef267a5fb7b5ed
parent39aa7423b2e79d9859f8626b1678636dba738c9f (diff)
downloadcouchdb-c0efba32fa1ced66b5baf63ef1ab0dc6188974c5.tar.gz
Fix semantics of total_rows
The `total_rows` field suppose to be a number of documents in the database/view. See https://docs.couchdb.org/en/stable/api/ddoc/views.html. When new pagination API was introduced the meaning of `total_rows` field was changed to number of rows in the query results. The PR reverts this accidental change.
-rw-r--r--src/chttpd/test/exunit/pagination_test.exs56
-rw-r--r--src/couch_views/src/couch_views_http.erl17
2 files changed, 23 insertions, 50 deletions
diff --git a/src/chttpd/test/exunit/pagination_test.exs b/src/chttpd/test/exunit/pagination_test.exs
index 6544017df..4e0a5d6fe 100644
--- a/src/chttpd/test/exunit/pagination_test.exs
+++ b/src/chttpd/test/exunit/pagination_test.exs
@@ -255,7 +255,7 @@ defmodule Couch.Test.Pagination do
@describetag descending: descending
setup [:with_session, :random_db, :with_docs]
- test "total_rows matches the length of rows array", ctx do
+ test "total_rows matches the number of docs", ctx do
resp =
Couch.Session.get(ctx.session, "/#{ctx.db_name}/_all_docs",
query: %{descending: ctx.descending}
@@ -263,7 +263,7 @@ defmodule Couch.Test.Pagination do
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
body = resp.body
- assert body["total_rows"] == length(body["rows"])
+ assert body["total_rows"] == ctx.n_docs
end
test "the rows are correctly sorted", ctx do
@@ -371,7 +371,7 @@ defmodule Couch.Test.Pagination do
@describetag page_size: 4
setup [:with_session, :random_db, :with_view, :with_docs]
- test "total_rows matches the length of rows array", ctx do
+ test "total_rows matches the number of documents in view", ctx do
resp =
Couch.Session.get(
ctx.session,
@@ -381,7 +381,7 @@ defmodule Couch.Test.Pagination do
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
body = resp.body
- assert body["total_rows"] == length(body["rows"])
+ assert body["total_rows"] == ctx.n_docs
end
end
@@ -593,14 +593,9 @@ defmodule Couch.Test.Pagination do
assert Map.has_key?(body, "next")
end
- test "total_rows matches the length of rows array", ctx do
+ test "total_rows matches the number of documents", ctx do
body = ctx.response
- assert body["total_rows"] == length(body["rows"])
- end
-
- test "total_rows matches the requested page_size", ctx do
- body = ctx.response
- assert body["total_rows"] == ctx.page_size
+ assert body["total_rows"] == ctx.n_docs
end
test "can use 'next' bookmark to get remaining results", ctx do
@@ -613,8 +608,8 @@ defmodule Couch.Test.Pagination do
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
body = resp.body
- assert body["total_rows"] == length(body["rows"])
- assert body["total_rows"] <= ctx.page_size
+ assert body["total_rows"] == ctx.n_docs
+ assert length(body["rows"]) <= ctx.page_size
end
end
@@ -721,7 +716,7 @@ defmodule Couch.Test.Pagination do
test "final page doesn't include 'next' bookmark", ctx do
assert not Map.has_key?(ctx.response, "next")
- assert ctx.response["total_rows"] == rem(ctx.n_docs, ctx.page_size)
+ assert length(ctx.response["rows"]) == rem(ctx.n_docs, ctx.page_size)
end
test "each but last page has page_size rows", ctx do
@@ -768,14 +763,9 @@ defmodule Couch.Test.Pagination do
assert not Map.has_key?(body, "next")
end
- test "total_rows matches the length of rows array", ctx do
- body = ctx.response
- assert body["total_rows"] == length(body["rows"])
- end
-
- test "total_rows less than the requested page_size", ctx do
+ test "total_rows matches the number of documents", ctx do
body = ctx.response
- assert body["total_rows"] <= ctx.page_size
+ assert body["total_rows"] == ctx.n_docs
end
end
end
@@ -950,7 +940,7 @@ defmodule Couch.Test.Pagination do
assert not Map.has_key?(resp.body, "previous")
end
- test "total_rows matches the length of rows array", ctx do
+ test "total_rows matches the number of documents in view", ctx do
resp =
Couch.Session.get(
ctx.session,
@@ -960,19 +950,7 @@ defmodule Couch.Test.Pagination do
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
body = resp.body
- assert body["total_rows"] == length(body["rows"])
- end
-
- test "total_rows matches the requested page_size", ctx do
- resp =
- Couch.Session.get(
- ctx.session,
- "/#{ctx.db_name}/_design/#{ctx.ddoc_id}/_view/#{ctx.view_name}",
- query: %{page_size: ctx.page_size, descending: ctx.descending}
- )
-
- assert resp.status_code == 200, "got error #{inspect(resp.body)}"
- assert resp.body["total_rows"] == ctx.page_size
+ assert body["total_rows"] == ctx.n_docs
end
test "can use 'next' bookmark to get remaining results", ctx do
@@ -994,8 +972,8 @@ defmodule Couch.Test.Pagination do
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
body = resp.body
- assert body["total_rows"] == length(body["rows"])
- assert body["total_rows"] <= ctx.page_size
+ assert body["total_rows"] == ctx.n_docs
+ assert length(body["rows"]) <= ctx.page_size
end
test "can use 'previous' bookmark", ctx do
@@ -1065,7 +1043,7 @@ defmodule Couch.Test.Pagination do
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
body = resp.body
- assert body["total_rows"] == length(body["rows"])
+ assert body["total_rows"] == ctx.n_docs
end
test "total_rows less than the requested page_size", ctx do
@@ -1077,7 +1055,7 @@ defmodule Couch.Test.Pagination do
)
assert resp.status_code == 200, "got error #{inspect(resp.body)}"
- assert resp.body["total_rows"] <= ctx.page_size
+ assert length(resp.body["rows"]) <= ctx.page_size
end
end
end
diff --git a/src/couch_views/src/couch_views_http.erl b/src/couch_views/src/couch_views_http.erl
index e21acfb9f..769d8c36c 100644
--- a/src/couch_views/src/couch_views_http.erl
+++ b/src/couch_views/src/couch_views_http.erl
@@ -95,9 +95,8 @@ paginated_cb({meta, Meta}, #vacc{}=VAcc) ->
case MetaData of
{_Key, undefined} ->
Acc;
- {total, _Value} ->
- %% We set total_rows elsewere
- Acc;
+ {total, Value} ->
+ maps:put(total_rows, Value, Acc);
{Key, Value} ->
maps:put(list_to_binary(atom_to_list(Key)), Value, Acc)
end
@@ -129,14 +128,12 @@ do_paginated(PageSize, QueriesArgs, KeyFun, Fun) when is_list(QueriesArgs) ->
Result0 = maybe_add_next_bookmark(
OriginalLimit, PageSize, Args, Meta, Items, KeyFun),
Result = maybe_add_previous_bookmark(Args, Result0, KeyFun),
- #{total_rows := Total} = Result,
- {Limit - Total, [Result | Acc]};
+ {Limit - length(maps:get(rows, Result)), [Result | Acc]};
false ->
Bookmark = bookmark_encode(Args0),
Result = #{
rows => [],
- next => Bookmark,
- total_rows => 0
+ next => Bookmark
},
{Limit, [Result | Acc]}
end
@@ -152,8 +149,7 @@ maybe_add_next_bookmark(OriginalLimit, PageSize, Args0, Response, Items, KeyFun)
case check_completion(OriginalLimit, RequestedLimit, Items) of
{Rows, nil} ->
maps:merge(Response, #{
- rows => Rows,
- total_rows => length(Rows)
+ rows => Rows
});
{Rows, Next} ->
{FirstId, FirstKey} = first_key(KeyFun, Rows),
@@ -169,8 +165,7 @@ maybe_add_next_bookmark(OriginalLimit, PageSize, Args0, Response, Items, KeyFun)
Bookmark = bookmark_encode(Args),
maps:merge(Response, #{
rows => Rows,
- next => Bookmark,
- total_rows => length(Rows)
+ next => Bookmark
})
end.