Commit 70f434db authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'ff-rate_limit' into 'master'

Remove rate_limited_service_issues_create flag

See merge request gitlab-org/gitlab!80990
parents 85af504c e2e94cce
......@@ -36,11 +36,6 @@ class Projects::IssuesController < Projects::ApplicationController
before_action :authorize_import_issues!, only: [:import_csv]
before_action :authorize_download_code!, only: [:related_branches]
# Limit the amount of issues created per minute
before_action -> { check_rate_limit!(:issues_create, scope: [@project, @current_user])},
only: [:create],
if: -> { Feature.disabled?('rate_limited_service_issues_create', project, default_enabled: :yaml) }
before_action do
push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml)
push_frontend_feature_flag(:vue_issues_list, project&.group, default_enabled: :yaml)
......
......@@ -36,7 +36,6 @@ module RateLimitedService
def rate_limit!(service)
evaluated_scope = evaluated_scope_for(service)
return if feature_flag_disabled?(evaluated_scope[:project])
if rate_limiter.throttled?(key, **opts.merge(scope: evaluated_scope.values, users_allowlist: users_allowlist))
raise RateLimitedError.new(key: key, rate_limiter: rate_limiter), _('This endpoint has been requested too many times. Try again later.')
......@@ -54,10 +53,6 @@ module RateLimitedService
all[var] = service.public_send(var) # rubocop: disable GitlabSecurity/PublicSend
end
end
def feature_flag_disabled?(project)
Feature.disabled?("rate_limited_service_#{key}", project, default_enabled: :yaml)
end
end
prepended do
......
---
name: rate_limited_service_issues_create
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68526
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342677
milestone: '14.4'
type: development
group: group::project management
default_enabled: false
......@@ -262,8 +262,6 @@ module API
post ':id/issues' do
Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/21140')
check_rate_limit!(:issues_create, scope: current_user) if Feature.disabled?("rate_limited_service_issues_create", user_project, default_enabled: :yaml)
authorize! :create_issue, user_project
issue_params = declared_params(include_missing: false)
......
......@@ -148,34 +148,11 @@ RSpec.describe Gitlab::Email::Handler::CreateIssueHandler do
end
end
context 'rate limiting' do
let(:rate_limited_service_feature_enabled) { nil }
it 'raises a RateLimitedService::RateLimitedError' do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
before do
stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_feature_enabled)
end
context 'when :rate_limited_service Feature is disabled' do
let(:rate_limited_service_feature_enabled) { false }
it 'does not attempt to throttle' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
setup_attachment
receiver.execute
end
end
context 'when :rate_limited_service Feature is enabled' do
let(:rate_limited_service_feature_enabled) { true }
it 'raises a RateLimitedService::RateLimitedError' do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
setup_attachment
expect { receiver.execute }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.'))
end
end
setup_attachment
expect { receiver.execute }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.'))
end
end
......
......@@ -382,7 +382,6 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
subject { 2.times { receiver.execute } }
before do
stub_feature_flags(rate_limited_service_issues_create: true)
stub_application_setting(issues_create_limit: 1)
end
......
......@@ -36,79 +36,28 @@ RSpec.describe RateLimitedService do
subject { described_class::RateLimiterScopedAndKeyed.new(key: key, opts: opts, rate_limiter: rate_limiter) }
describe '#rate_limit!' do
let(:project_with_feature_enabled) { create(:project) }
let(:project_without_feature_enabled) { create(:project) }
let_it_be(:project) { create(:project) }
let_it_be(:current_user) { create(:user) }
let(:project) { nil }
let(:current_user) { create(:user) }
let(:service) { instance_double(Issues::CreateService, project: project, current_user: current_user) }
let(:evaluated_scope) { [project, current_user] }
let(:evaluated_opts) { { scope: evaluated_scope, users_allowlist: %w[support-bot] } }
let(:rate_limited_service_issues_create_feature_enabled) { nil }
before do
stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_issues_create_feature_enabled)
end
shared_examples 'a service that does not attempt to throttle' do
it 'does not attempt to throttle' do
expect(rate_limiter).not_to receive(:throttled?)
context 'when rate limiting is not in effect' do
let(:throttled) { false }
it 'does not raise an exception' do
expect(subject.rate_limit!(service)).to be_nil
end
end
shared_examples 'a service that does attempt to throttle' do
context 'when rate limiting is in effect' do
before do
allow(rate_limiter).to receive(:throttled?).and_return(throttled)
end
context 'when rate limiting is not in effect' do
let(:throttled) { false }
it 'does not raise an exception' do
expect(subject.rate_limit!(service)).to be_nil
end
end
context 'when rate limiting is in effect' do
let(:throttled) { true }
it 'raises a RateLimitedError exception' do
expect { subject.rate_limit!(service) }.to raise_error(described_class::RateLimitedError, 'This endpoint has been requested too many times. Try again later.')
end
allow(rate_limiter).to receive(:throttled?).and_return(true)
end
end
context 'when :rate_limited_service_issues_create feature is globally disabled' do
let(:rate_limited_service_issues_create_feature_enabled) { false }
it_behaves_like 'a service that does not attempt to throttle'
end
context 'when :rate_limited_service_issues_create feature is globally enabled' do
let(:throttled) { nil }
let(:rate_limited_service_issues_create_feature_enabled) { true }
let(:project) { project_without_feature_enabled }
it_behaves_like 'a service that does attempt to throttle'
end
context 'when :rate_limited_service_issues_create feature is enabled for project_with_feature_enabled' do
let(:throttled) { nil }
let(:rate_limited_service_issues_create_feature_enabled) { project_with_feature_enabled }
context 'for project_without_feature_enabled' do
let(:project) { project_without_feature_enabled }
it_behaves_like 'a service that does not attempt to throttle'
end
context 'for project_with_feature_enabled' do
let(:project) { project_with_feature_enabled }
it_behaves_like 'a service that does attempt to throttle'
it 'raises a RateLimitedError exception' do
expect { subject.rate_limit!(service) }.to raise_error(described_class::RateLimitedError, 'This endpoint has been requested too many times. Try again later.')
end
end
end
......
......@@ -335,7 +335,6 @@ RSpec.describe Issues::CreateService do
let(:user) { create(:user) }
before do
stub_feature_flags(rate_limited_service_issues_create: true)
stub_application_setting(issues_create_limit: 1)
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