diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-05-09 14:46:50 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-05-09 14:46:50 +0000 |
commit | dccf5f19f911eaf8d32e37e9f369a46d9e7ec9bd (patch) | |
tree | bf85d85d79b885b90f20ed587fc7ea77387095c3 | |
parent | 2aa084a484784bc72e85dbaa82189a66a913e352 (diff) | |
parent | 3531ea096f730b8533df259ac2f6cbed738965ed (diff) | |
download | gitlab-ce-dccf5f19f911eaf8d32e37e9f369a46d9e7ec9bd.tar.gz |
Merge branch 'tc-cache-trackable-attributes' into 'master'
Limit User's trackable attributes to update at most once/hour
Closes #22068
See merge request !11053
-rw-r--r-- | app/models/user.rb | 11 | ||||
-rw-r--r-- | changelogs/unreleased/tc-cache-trackable-attributes.yml | 4 | ||||
-rw-r--r-- | spec/features/groups/members/sorting_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 29 |
4 files changed, 46 insertions, 2 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index 4e5f94683b8..77b2b12ee0b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,6 +40,17 @@ class User < ActiveRecord::Base devise :lockable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable + # Override Devise::Models::Trackable#update_tracked_fields! + # to limit database writes to at most once every hour + def update_tracked_fields!(request) + update_tracked_fields(request) + + lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i) + return unless lease.try_obtain + + save(validate: false) + end + attr_accessor :force_random_password # Virtual attribute for authenticating by either username or email diff --git a/changelogs/unreleased/tc-cache-trackable-attributes.yml b/changelogs/unreleased/tc-cache-trackable-attributes.yml new file mode 100644 index 00000000000..4a2cf50893a --- /dev/null +++ b/changelogs/unreleased/tc-cache-trackable-attributes.yml @@ -0,0 +1,4 @@ +--- +title: "Limit User's trackable attributes, like `current_sign_in_at`, to update at most once/hour" +merge_request: 11053 +author: diff --git a/spec/features/groups/members/sorting_spec.rb b/spec/features/groups/members/sorting_spec.rb index 608aedd3471..902d3f789ff 100644 --- a/spec/features/groups/members/sorting_spec.rb +++ b/spec/features/groups/members/sorting_spec.rb @@ -68,7 +68,7 @@ feature 'Groups > Members > Sorting', feature: true do expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Name, descending') end - scenario 'sorts by recent sign in' do + scenario 'sorts by recent sign in', :redis do visit_members_list(sort: :recent_sign_in) expect(first_member).to include(owner.name) @@ -76,7 +76,7 @@ feature 'Groups > Members > Sorting', feature: true do expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in') end - scenario 'sorts by oldest sign in' do + scenario 'sorts by oldest sign in', :redis do visit_members_list(sort: :oldest_sign_in) expect(first_member).to include(developer.name) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 63e71f5ff2f..c7ddd17872b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -344,6 +344,35 @@ describe User, models: true do end end + describe '#update_tracked_fields!', :redis do + let(:request) { OpenStruct.new(remote_ip: "127.0.0.1") } + let(:user) { create(:user) } + + it 'writes trackable attributes' do + expect do + user.update_tracked_fields!(request) + end.to change { user.reload.current_sign_in_at } + end + + it 'does not write trackable attributes when called a second time within the hour' do + user.update_tracked_fields!(request) + + expect do + user.update_tracked_fields!(request) + end.not_to change { user.reload.current_sign_in_at } + end + + it 'writes trackable attributes for a different user' do + user2 = create(:user) + + user.update_tracked_fields!(request) + + expect do + user2.update_tracked_fields!(request) + end.to change { user2.reload.current_sign_in_at } + end + end + shared_context 'user keys' do let(:user) { create(:user) } let!(:key) { create(:key, user: user) } |