Commit 81786aa3 authored by Robert May's avatar Robert May

Limit page number on explore/projects

Limit the maximum page number to 500 on project exploration.
Prior to this change it was possible to iterate infinitely, and
high page numbers are prone to slow speeds due to the project list
being a slow query amplified by OFFSET pagination slowness.

There's a new Prometheus metric available at
`gitlab_page_out_of_bounds` for monitoring the traffic which hits the
page limit.
parent 56a473da
# frozen_string_literal: true
class Explore::ProjectsController < Explore::ApplicationController
include PageLimiter
include ParamsBackwardCompatibility
include RendersMemberAccess
include SortingHelper
include SortingPreference
PAGE_LIMIT = 500
before_action :set_non_archived_param
before_action :set_sorting
limit_pages PAGE_LIMIT
helper_method :max_page_number
def index
@projects = load_projects
......@@ -53,10 +59,14 @@ class Explore::ProjectsController < Explore::ApplicationController
private
# rubocop: disable CodeReuse/ActiveRecord
def load_projects
def load_project_counts
@total_user_projects_count = ProjectsFinder.new(params: { non_public: true }, current_user: current_user).execute
@total_starred_projects_count = ProjectsFinder.new(params: { starred: true }, current_user: current_user).execute
end
# rubocop: disable CodeReuse/ActiveRecord
def load_projects
load_project_counts
projects = ProjectsFinder.new(current_user: current_user, params: params)
.execute
......@@ -80,4 +90,25 @@ class Explore::ProjectsController < Explore::ApplicationController
def sorting_field
Project::SORTING_PREFERENCE_FIELD
end
def max_page_number
PAGE_LIMIT
end
# Overrides the default in the PageLimiter concern
def page_out_of_bounds
load_project_counts
respond_to do |format|
format.html do
render "page_out_of_bounds", status: :bad_request
end
format.json do
render json: {
html: view_to_html_string("explore/projects/page_out_of_bounds")
}, status: :bad_request
end
end
end
end
- @hide_top_links = true
- page_title _("Projects")
- header_title _("Projects"), dashboard_projects_path
= render_dashboard_gold_trial(current_user)
- if current_user
= render 'dashboard/projects_head', project_tab_filter: :explore
- else
= render 'explore/head'
= render 'explore/projects/nav' unless Feature.enabled?(:project_list_filter_bar) && current_user
.nothing-here-block
.svg-content
= image_tag 'illustrations/profile-page/personal-project.svg', size: '75'
.text-content
%h5= _("Maximum page reached")
%p= _("Sorry, you have exceeded the maximum browsable page number. Please use the API to explore further.")
= link_to _("Back to page %{number}") % { number: max_page_number }, request.params.merge(page: max_page_number), class: 'btn btn-inverted'
---
title: Limit page number on explore/projects
merge_request: 22876
author:
type: performance
......@@ -2480,6 +2480,9 @@ msgstr ""
msgid "Average per day: %{average}"
msgstr ""
msgid "Back to page %{number}"
msgstr ""
msgid "Background Color"
msgstr ""
......@@ -11310,6 +11313,9 @@ msgstr ""
msgid "Maximum number of mirrors that can be synchronizing at the same time."
msgstr ""
msgid "Maximum page reached"
msgstr ""
msgid "Maximum push size (MB)"
msgstr ""
......@@ -17152,6 +17158,9 @@ msgstr ""
msgid "Sorry, no projects matched your search"
msgstr ""
msgid "Sorry, you have exceeded the maximum browsable page number. Please use the API to explore further."
msgstr ""
msgid "Sorry, your filter produced no results"
msgstr ""
......
......@@ -59,6 +59,75 @@ describe Explore::ProjectsController do
end
end
shared_examples "blocks high page numbers" do
let(:page_limit) { Explore::ProjectsController::PAGE_LIMIT }
context "page number is too high" do
[:index, :trending, :starred].each do |endpoint|
describe "GET #{endpoint}" do
render_views
before do
get endpoint, params: { page: page_limit + 1 }
end
it { is_expected.to respond_with(:bad_request) }
it { is_expected.to render_template("explore/projects/page_out_of_bounds") }
end
describe "GET #{endpoint}.json" do
render_views
before do
get endpoint, params: { page: page_limit + 1 }, format: :json
end
it { is_expected.to respond_with(:bad_request) }
end
describe "metrics recording" do
after do
get endpoint, params: { page: page_limit + 1 }
end
it "records the interception" do
expect(Gitlab::Metrics).to receive(:counter).with(
:gitlab_page_out_of_bounds,
controller: "explore/projects",
action: endpoint.to_s,
agent: "Rails Testing"
)
end
end
end
end
context "page number is acceptable" do
[:index, :trending, :starred].each do |endpoint|
describe "GET #{endpoint}" do
render_views
before do
get endpoint, params: { page: page_limit }
end
it { is_expected.to respond_with(:success) }
it { is_expected.to render_template("explore/projects/#{endpoint}") }
end
describe "GET #{endpoint}.json" do
render_views
before do
get endpoint, params: { page: page_limit }, format: :json
end
it { is_expected.to respond_with(:success) }
end
end
end
end
context 'when user is signed in' do
let(:user) { create(:user) }
......@@ -67,6 +136,7 @@ describe Explore::ProjectsController do
end
include_examples 'explore projects'
include_examples "blocks high page numbers"
context 'user preference sorting' do
let(:project) { create(:project) }
......@@ -79,6 +149,7 @@ describe Explore::ProjectsController do
context 'when user is not signed in' do
include_examples 'explore projects'
include_examples "blocks high page numbers"
context 'user preference sorting' do
let(:project) { create(:project) }
......@@ -91,4 +162,11 @@ describe Explore::ProjectsController do
end
end
end
describe "#max_page_number" do
subject { controller.send(:max_page_number) }
it { is_expected.to eq(Explore::ProjectsController::PAGE_LIMIT) }
it { is_expected.to be_an(Integer) }
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment