diff --git a/app/controllers/image_uploads_controller.rb b/app/controllers/image_uploads_controller.rb index 6c6cf656b..e8f789c69 100644 --- a/app/controllers/image_uploads_controller.rb +++ b/app/controllers/image_uploads_controller.rb @@ -76,7 +76,7 @@ class ImageUploadsController < ApplicationController Array.wrap(images).map do |image| ArticleImageUploader.new.tap do |uploader| uploader.store!(image) - rate_limiter.track_image_uploads + rate_limiter.track_limit_by_action(:image_upload) end end end diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 59cac6ec9..8119a70a2 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -17,7 +17,7 @@ class OrganizationsController < ApplicationController @organization = Organization.new(organization_params) authorize @organization if @organization.save - rate_limiter.track_organization_creation + rate_limiter.track_limit_by_action(:organization_creation) @organization_membership = OrganizationMembership.create!(organization_id: @organization.id, user_id: current_user.id, type_of_user: "admin") flash[:settings_notice] = "Your organization was successfully created and you are an admin." redirect_to "/settings/organization/#{@organization.id}" diff --git a/app/labor/rate_limit_checker.rb b/app/labor/rate_limit_checker.rb index 812b5875e..298641051 100644 --- a/app/labor/rate_limit_checker.rb +++ b/app/labor/rate_limit_checker.rb @@ -52,14 +52,10 @@ class RateLimitChecker result end - def track_image_uploads - expires_in = RETRY_AFTER[:image_upload].seconds - Rails.cache.increment("#{@user.id}_image_upload", 1, expires_in: expires_in) - end - - def track_article_updates - expires_in = RETRY_AFTER[:article_update].seconds - Rails.cache.increment("#{@user.id}_article_update", 1, expires_in: expires_in) + def track_limit_by_action(action) + cache_key = "#{@user.id}_#{action}" + expires_in = RETRY_AFTER[action].seconds + Rails.cache.increment(cache_key, 1, expires_in: expires_in) end def limit_by_email_recipient_address(address) @@ -68,11 +64,6 @@ class RateLimitChecker SiteConfig.rate_limit_email_recipient end - def track_organization_creation - expires_in = RETRY_AFTER[:organization_creation].seconds - Rails.cache.increment("#{@user.id}_organization_creation", 1, expires_in: expires_in) - end - private def check_comment_creation_limit diff --git a/app/services/articles/updater.rb b/app/services/articles/updater.rb index 77438dba6..25d429104 100644 --- a/app/services/articles/updater.rb +++ b/app/services/articles/updater.rb @@ -36,7 +36,7 @@ module Articles article_params[:edited_at] = Time.current if update_edited_at article.update!(article_params) - rate_limiter.track_article_updates + rate_limiter.track_limit_by_action(:article_update) # send notification only the first time an article is published send_notification = article.published && article.saved_change_to_published_at.present? diff --git a/spec/labor/rate_limit_checker_spec.rb b/spec/labor/rate_limit_checker_spec.rb index 9617df73e..6652bb1e0 100644 --- a/spec/labor/rate_limit_checker_spec.rb +++ b/spec/labor/rate_limit_checker_spec.rb @@ -137,25 +137,15 @@ RSpec.describe RateLimitChecker, type: :labor do end end - describe ".track_image_uploads" do - it "calls the cache object correctly" do + describe "#track_limit_by_action" do + it "increments cache for action with retry as expiration" do allow(Rails.cache).to receive(:increment) + action = :image_upload + rate_limit_checker.track_limit_by_action(action) - rate_limit_checker.track_image_uploads - - key = "#{user.id}_image_upload" - expect(Rails.cache).to have_received(:increment).with(key, 1, expires_in: 30.seconds) - end - end - - describe ".track_article_updates" do - it "calls the cache object correctly" do - allow(Rails.cache).to receive(:increment) - - rate_limit_checker.track_article_updates - - key = "#{user.id}_article_update" - expect(Rails.cache).to have_received(:increment).with(key, 1, expires_in: 30.seconds) + key = "#{user.id}_#{action}" + expires_in = described_class::RETRY_AFTER[action] + expect(Rails.cache).to have_received(:increment).with(key, 1, expires_in: expires_in) end end