Skip to content

Commit 3d03712

Browse files
authored
fix(projecthub-rest-api): drop page size (#406)
## 📝 Description Removed the `page_size` query parameter due to inconsistent behavior. Pagination now relies on the default `page_size` and the `page` parameter only. Retained only the `x-page` and `x-has-more` response headers, which are sufficient for paginated iteration. Related [task](renderedtext/tasks#7953). ## ✅ Checklist - [x] I have tested this change - [x] ~This change requires documentation update~ - N/A
1 parent 9176101 commit 3d03712

File tree

5 files changed

+53
-61
lines changed

5 files changed

+53
-61
lines changed

projecthub-rest-api/config/config.exs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ config :projecthub, http_port: 4000
1010
config :projecthub,
1111
projecthub_grpc_endpoint: "0.0.0.0:50051",
1212
organization_grpc_endpoint: "0.0.0.0:50051",
13-
rbac_grpc_endpoint: "0.0.0.0:50051"
13+
rbac_grpc_endpoint: "0.0.0.0:50051",
14+
projects_page_size: 500
1415

1516
config :projecthub, :enviroment, config_env()
1617

projecthub-rest-api/config/runtime.exs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,6 @@ if config_env() == :prod do
1717
config :projecthub,
1818
projecthub_grpc_endpoint: System.fetch_env!("INTERNAL_API_URL_PROJECT"),
1919
organization_grpc_endpoint: System.fetch_env!("INTERNAL_API_URL_ORGANIZATION"),
20-
rbac_grpc_endpoint: System.fetch_env!("INTERNAL_API_URL_RBAC")
20+
rbac_grpc_endpoint: System.fetch_env!("INTERNAL_API_URL_RBAC"),
21+
projects_page_size: System.get_env("PROJECTS_PAGE_SIZE", "500") |> String.to_integer()
2122
end

projecthub-rest-api/config/test.exs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ config :junit_formatter,
77
print_report_file: true,
88
include_filename?: true,
99
include_file_line?: true
10+
11+
config :projecthub, :projects_page_size, 2

projecthub-rest-api/lib/projecthub/http_api.ex

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,9 @@ defmodule Projecthub.HttpApi do
4040

4141
get "/api/#{@version}/projects" do
4242
case list_projects(conn) do
43-
{:ok, {projects, page, page_size, total, has_more}} ->
43+
{:ok, {projects, page, has_more}} ->
4444
conn
4545
|> put_resp_header("x-page", Integer.to_string(page))
46-
|> put_resp_header("x-page-size", Integer.to_string(page_size))
47-
|> put_resp_header("x-total-count", Integer.to_string(total))
4846
|> put_resp_header("x-has-more", to_string(has_more))
4947
|> send_resp(200, Poison.encode!(projects))
5048

@@ -694,23 +692,22 @@ defmodule Projecthub.HttpApi do
694692
org_id = conn.assigns.org_id
695693
restricted = Organization.restricted?(org_id)
696694

697-
with {:ok, page} <- parse_int(conn.params, "page", 1, 1_000_000, 1),
698-
{:ok, page_size} <- parse_int(conn.params, "page_size", 1, 500, 500) do
699-
do_list_projects(conn, org_id, restricted, page, page_size)
700-
else
701-
{:error, reason} ->
702-
{:error, reason}
695+
case parse_int(conn.params, "page", 1, 100, 1) do
696+
{:ok, page} -> do_list_projects(conn, org_id, restricted, page)
697+
{:error, reason} -> {:error, reason}
703698
end
704699
end
705700

706-
defp do_list_projects(conn, org_id, restricted, page, page_size) do
701+
defp page_size, do: Application.get_env(:projecthub, :projects_page_size, 500)
702+
703+
defp do_list_projects(conn, org_id, restricted, page) do
707704
req =
708705
InternalApi.Projecthub.ListRequest.new(
709706
metadata: Utils.construct_req_meta(conn),
710707
pagination:
711708
InternalApi.Projecthub.PaginationRequest.new(
712709
page: page,
713-
page_size: page_size
710+
page_size: page_size()
714711
)
715712
)
716713

@@ -728,8 +725,13 @@ defmodule Projecthub.HttpApi do
728725
|> Enum.map(&Map.merge(&1, %{"apiVersion" => @version, "kind" => "Project"}))
729726

730727
total = Map.get(res.pagination || %{}, :total_entries, 0)
731-
has_more = (page - 1) * page_size + length(projects) < total
732-
{:ok, {projects, page, page_size, total, has_more}}
728+
has_more = (page - 1) * page_size() + length(res.projects) < total
729+
730+
if total < (page - 1) * page_size() do
731+
{:ok, {[], page, false}}
732+
else
733+
{:ok, {projects, page, has_more}}
734+
end
733735

734736
:NOT_FOUND ->
735737
{:error, :not_found}

projecthub-rest-api/test/projecthub/http_api_test.exs

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -292,16 +292,15 @@ defmodule Projecthub.HttpApi.Test do
292292
p1 = create("project1", p1_id)
293293
p2 = create("project2", p2_id)
294294
p3 = create("project3", p3_id)
295+
page_size = Application.get_env(:projecthub, :projects_page_size)
295296

296297
FunRegistry.set!(FakeServices.RbacService, :list_accessible_projects, fn _, _ ->
297298
InternalApi.RBAC.ListAccessibleProjectsResponse.new(project_ids: [p1_id, p2_id, p3_id])
298299
end)
299300

300301
FunRegistry.set!(FakeServices.ProjectService, :list, fn req, _ ->
301302
alias InternalApi.Projecthub, as: PH
302-
# Simulate pagination
303303
page = req.pagination.page
304-
page_size = req.pagination.page_size
305304
all_projects = [p1, p2, p3]
306305
projects = Enum.slice(all_projects, (page - 1) * page_size, page_size)
307306

@@ -327,30 +326,50 @@ defmodule Projecthub.HttpApi.Test do
327326
test "returns correct pagination headers for /projects" do
328327
{:ok, response} =
329328
HTTPoison.get(
330-
"http://localhost:#{@port}/api/#{@version}/projects?page=1&page_size=2",
329+
"http://localhost:#{@port}/api/#{@version}/projects?page=1",
331330
@headers
332331
)
333332

334333
assert response.status_code == 200
335334
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-page" and v == "1" end)
336-
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-page-size" and v == "2" end)
337-
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-total-count" and v == "3" end)
338335
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-has-more" and v == "true" end)
339336
projects = Poison.decode!(response.body)
340337
assert length(projects) == 2
338+
339+
{:ok, response2} =
340+
HTTPoison.get(
341+
"http://localhost:#{@port}/api/#{@version}/projects?page=2",
342+
@headers
343+
)
344+
345+
assert response2.status_code == 200
346+
assert response2.headers |> Enum.any?(fn {k, v} -> k == "x-page" and v == "2" end)
347+
assert response2.headers |> Enum.any?(fn {k, v} -> k == "x-has-more" and v == "false" end)
348+
projects2 = Poison.decode!(response2.body)
349+
assert length(projects2) == 1
350+
351+
{:ok, response3} =
352+
HTTPoison.get(
353+
"http://localhost:#{@port}/api/#{@version}/projects?page=3",
354+
@headers
355+
)
356+
357+
assert response3.status_code == 200
358+
assert response3.headers |> Enum.any?(fn {k, v} -> k == "x-page" and v == "3" end)
359+
assert response3.headers |> Enum.any?(fn {k, v} -> k == "x-has-more" and v == "false" end)
360+
projects3 = Poison.decode!(response3.body)
361+
assert Enum.empty?(projects3)
341362
end
342363

343364
test "returns correct pagination headers for /projects when there are no more projects" do
344365
{:ok, response} =
345366
HTTPoison.get(
346-
"http://localhost:#{@port}/api/#{@version}/projects?page=2&page_size=2",
367+
"http://localhost:#{@port}/api/#{@version}/projects?page=2",
347368
@headers
348369
)
349370

350371
assert response.status_code == 200
351372
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-page" and v == "2" end)
352-
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-page-size" and v == "2" end)
353-
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-total-count" and v == "3" end)
354373
assert response.headers |> Enum.any?(fn {k, v} -> k == "x-has-more" and v == "false" end)
355374
projects = Poison.decode!(response.body)
356375
assert length(projects) == 1
@@ -371,7 +390,7 @@ defmodule Projecthub.HttpApi.Test do
371390

372391
{:ok, response} =
373392
HTTPoison.get(
374-
"http://localhost:#{@port}/api/#{@version}/projects?page=10&page_size=2",
393+
"http://localhost:#{@port}/api/#{@version}/projects?page=10",
375394
@headers
376395
)
377396

@@ -401,7 +420,7 @@ defmodule Projecthub.HttpApi.Test do
401420

402421
{:ok, response} =
403422
HTTPoison.get(
404-
"http://localhost:#{@port}/api/#{@version}/projects?page=foo&page_size=bar",
423+
"http://localhost:#{@port}/api/#{@version}/projects?page=foo",
405424
@headers
406425
)
407426

@@ -411,29 +430,18 @@ defmodule Projecthub.HttpApi.Test do
411430
test "returns 400 on negative page" do
412431
{:ok, response} =
413432
HTTPoison.get(
414-
"http://localhost:#{@port}/api/#{@version}/projects?page=-1&page_size=2",
433+
"http://localhost:#{@port}/api/#{@version}/projects?page=-1",
415434
@headers
416435
)
417436

418437
assert response.status_code == 400
419438
assert Poison.decode!(response.body)["message"] =~ "page must be at least 1"
420439
end
421440

422-
test "returns 400 on zero page_size" do
423-
{:ok, response} =
424-
HTTPoison.get(
425-
"http://localhost:#{@port}/api/#{@version}/projects?page=1&page_size=0",
426-
@headers
427-
)
428-
429-
assert response.status_code == 400
430-
assert Poison.decode!(response.body)["message"] =~ "page_size must be at least 1"
431-
end
432-
433441
test "returns 400 on too large page" do
434442
{:ok, response} =
435443
HTTPoison.get(
436-
"http://localhost:#{@port}/api/#{@version}/projects?page=9999&page_size=2",
444+
"http://localhost:#{@port}/api/#{@version}/projects?page=9999",
437445
@headers
438446
)
439447

@@ -443,28 +451,6 @@ defmodule Projecthub.HttpApi.Test do
443451
assert Poison.decode!(response.body)["message"] =~ "page must be at most"
444452
end
445453
end
446-
447-
test "returns 400 on too large page_size" do
448-
{:ok, response} =
449-
HTTPoison.get(
450-
"http://localhost:#{@port}/api/#{@version}/projects?page=1&page_size=9999",
451-
@headers
452-
)
453-
454-
assert response.status_code == 400
455-
assert Poison.decode!(response.body)["message"] =~ "page_size must be at most"
456-
end
457-
458-
test "returns 400 on non-numeric page_size" do
459-
{:ok, response} =
460-
HTTPoison.get(
461-
"http://localhost:#{@port}/api/#{@version}/projects?page=1&page_size=abc",
462-
@headers
463-
)
464-
465-
assert response.status_code == 400
466-
assert Poison.decode!(response.body)["message"] =~ "page_size must be a number"
467-
end
468454
end
469455

470456
describe "GET /api/<version>/projects/:name with authorized user" do
@@ -1828,7 +1814,7 @@ defmodule Projecthub.HttpApi.Test do
18281814

18291815
def create(name, id) do
18301816
alias InternalApi.Projecthub.Project
1831-
alias InternalApi.Projecthub.Project.Spec.{Repository, Visibility, PermissionType}
1817+
alias InternalApi.Projecthub.Project.Spec.{PermissionType, Repository, Visibility}
18321818

18331819
Project.new(
18341820
metadata: Project.Metadata.new(name: name, id: id),

0 commit comments

Comments
 (0)