diff --git a/app/controllers/superadmin/account_types_controller.rb b/app/controllers/superadmin/account_types_controller.rb index a5f2a0de89..89133dce9c 100644 --- a/app/controllers/superadmin/account_types_controller.rb +++ b/app/controllers/superadmin/account_types_controller.rb @@ -18,15 +18,11 @@ class Superadmin::AccountTypesController < Superadmin::SuperadminController end def update - if @account_type.rate_limit + if @account_type.rate_limit.different?(@rate_limit) @account_type.rate_limit.update_attributes!(@rate_limit.rate_limit_attributes) - else - @account_type.rate_limit = @rate_limit - @account_type.save! + ::Resque.enqueue(::Resque::UserJobs::RateLimitsJobs::SyncRedis, @account_type.account_type) end - ::Resque.enqueue(::Resque::UserJobs::RateLimitsJobs::SyncRedis, @account_type.account_type) - render json: @account_type, status: 204 end diff --git a/app/models/carto/rate_limit.rb b/app/models/carto/rate_limit.rb index f54bb9bc3e..14187664ce 100644 --- a/app/models/carto/rate_limit.rb +++ b/app/models/carto/rate_limit.rb @@ -68,5 +68,11 @@ module Carto attrs ||= attributes attrs.with_indifferent_access.slice(*Carto::RateLimit::RATE_LIMIT_ATTRIBUTES) end + + def different?(rate_limit) + rate_limit_attributes.lazy.zip(rate_limit.rate_limit_attributes).any? do |x, y| + x[1].to_array != y[1].to_array + end + end end end diff --git a/spec/models/carto/rate_limit_spec.rb b/spec/models/carto/rate_limit_spec.rb index 9054e3baa6..ff41df97f2 100644 --- a/spec/models/carto/rate_limit_spec.rb +++ b/spec/models/carto/rate_limit_spec.rb @@ -40,7 +40,7 @@ describe Carto::RateLimit do @limits_feature_flag.destroy end - describe '#create' do + describe '#CRUD' do it 'is persisted correctly to database' do rate_limit = Carto::RateLimit.find(@rate_limit.id) @@ -315,5 +315,59 @@ describe Carto::RateLimit do rate_limit_not_saved.save_to_redis(@user) }.to raise_error(ActiveRecord::RecordInvalid) end + + it 'compares the same rate limits attributes' do + @rate_limit.different?(@rate_limit).should eq false + end + + it 'compares two different rate limits attributes' do + @rate_limit2 = Carto::RateLimit.create!(maps_anonymous: Carto::RateLimitValues.new([1, 1, 2]), + maps_static: Carto::RateLimitValues.new([3, 4, 5]), + maps_static_named: Carto::RateLimitValues.new([6, 7, 8]), + maps_dataview: Carto::RateLimitValues.new([9, 10, 11]), + maps_dataview_search: Carto::RateLimitValues.new([12, 13, 14]), + maps_analysis: Carto::RateLimitValues.new([18, 19, 20]), + maps_tile: Carto::RateLimitValues.new([1, 2, 17, 30, 32, 34]), + maps_attributes: Carto::RateLimitValues.new([21, 22, 23]), + maps_named_list: Carto::RateLimitValues.new([24, 25, 26]), + maps_named_create: Carto::RateLimitValues.new([27, 28, 29]), + maps_named_get: Carto::RateLimitValues.new([30, 31, 32]), + maps_named: Carto::RateLimitValues.new([33, 34, 35]), + maps_named_update: Carto::RateLimitValues.new([36, 37, 38]), + maps_named_delete: Carto::RateLimitValues.new([39, 40, 41]), + maps_named_tiles: Carto::RateLimitValues.new([10, 11, 12]), + sql_query: Carto::RateLimitValues.new([13, 14, 15]), + sql_query_format: Carto::RateLimitValues.new([16, 17, 18]), + sql_job_create: Carto::RateLimitValues.new([19, 110, 111]), + sql_job_get: Carto::RateLimitValues.new([6, 7, 8]), + sql_job_delete: Carto::RateLimitValues.new([0, 1, 2])) + + @rate_limit.different?(@rate_limit2).should eq true + end + + it 'compares two different rate limits with same attributes' do + @rate_limit2 = Carto::RateLimit.create!(maps_anonymous: Carto::RateLimitValues.new([0, 1, 2]), + maps_static: Carto::RateLimitValues.new([3, 4, 5]), + maps_static_named: Carto::RateLimitValues.new([6, 7, 8]), + maps_dataview: Carto::RateLimitValues.new([9, 10, 11]), + maps_dataview_search: Carto::RateLimitValues.new([12, 13, 14]), + maps_analysis: Carto::RateLimitValues.new([18, 19, 20]), + maps_tile: Carto::RateLimitValues.new([1, 2, 17, 30, 32, 34]), + maps_attributes: Carto::RateLimitValues.new([21, 22, 23]), + maps_named_list: Carto::RateLimitValues.new([24, 25, 26]), + maps_named_create: Carto::RateLimitValues.new([27, 28, 29]), + maps_named_get: Carto::RateLimitValues.new([30, 31, 32]), + maps_named: Carto::RateLimitValues.new([33, 34, 35]), + maps_named_update: Carto::RateLimitValues.new([36, 37, 38]), + maps_named_delete: Carto::RateLimitValues.new([39, 40, 41]), + maps_named_tiles: Carto::RateLimitValues.new([10, 11, 12]), + sql_query: Carto::RateLimitValues.new([13, 14, 15]), + sql_query_format: Carto::RateLimitValues.new([16, 17, 18]), + sql_job_create: Carto::RateLimitValues.new([19, 110, 111]), + sql_job_get: Carto::RateLimitValues.new([6, 7, 8]), + sql_job_delete: Carto::RateLimitValues.new([0, 1, 2])) + + @rate_limit.different?(@rate_limit2).should eq false + end end end diff --git a/spec/requests/superadmin/account_types_spec.rb b/spec/requests/superadmin/account_types_spec.rb index 6d7296366b..65ad0c7878 100644 --- a/spec/requests/superadmin/account_types_spec.rb +++ b/spec/requests/superadmin/account_types_spec.rb @@ -90,6 +90,25 @@ describe Superadmin::AccountTypesController do response.body[:error].should =~ /ERROR. account_type not found/ end end + + it 'should not sync redis if rate limits do not change' do + ::Resque.expects(:enqueue).never + + current_rate_limits = @account_type.rate_limit.api_attributes + @account_type_param = { + account_type: @account_type.account_type, + rate_limit: current_rate_limits + } + + expect { + put superadmin_account_type_url(@account_type.account_type), + { account_type: @account_type_param }.to_json, + superadmin_headers + + @account_type.reload + @account_type.rate_limit.api_attributes.should eq current_rate_limits + }.to_not raise_error + end end describe '#destroy' do