From b531616ebad93bb4bd5c82108562731d64a23078 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 28 Jun 2017 19:18:16 +0200 Subject: [PATCH] Cache PerformanceBar data using RequestStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- config/initializers/1_settings.rb | 2 +- lib/gitlab/performance_bar.rb | 29 +++++++++++++++++++------ spec/lib/gitlab/performance_bar_spec.rb | 20 ++++++++--------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 7d35404119f..55664399111 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -489,7 +489,7 @@ Settings.gitaly['enabled'] = true if Settings.gitaly['enabled'].nil? # Performance bar # Settings['performance_bar'] ||= Settingslogic.new({}) -Settings.performance_bar['allowed_group'] = 'gitlab-org' if Settings.performance_bar['allowed_group'].nil? +Settings.performance_bar['allowed_group'] ||= nil # # Webpack settings diff --git a/lib/gitlab/performance_bar.rb b/lib/gitlab/performance_bar.rb index 2ca2cbeac08..53812633888 100644 --- a/lib/gitlab/performance_bar.rb +++ b/lib/gitlab/performance_bar.rb @@ -5,19 +5,34 @@ module Gitlab end def self.allowed_actor?(actor) - group = allowed_group - return false unless actor&.is_a?(User) && group + return false unless actor.thing&.is_a?(User) && allowed_group - GroupMembersFinder.new(group) - .execute - .where(user_id: actor.id) - .any? + if RequestStore.active? + RequestStore.fetch('performance_bar:user_member_of_allowed_group') do + user_member_of_allowed_group?(actor.thing) + end + else + user_member_of_allowed_group?(actor.thing) + end end def self.allowed_group return nil unless Gitlab.config.performance_bar.allowed_group - Group.by_path(Gitlab.config.performance_bar.allowed_group) + if RequestStore.active? + RequestStore.fetch('performance_bar:allowed_group') do + Group.by_path(Gitlab.config.performance_bar.allowed_group) + end + else + Group.by_path(Gitlab.config.performance_bar.allowed_group) + end + end + + def self.user_member_of_allowed_group?(user) + GroupMembersFinder.new(allowed_group) + .execute + .where(user_id: user.id) + .any? end end end diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb index 0f630c243ad..56cd8813f0d 100644 --- a/spec/lib/gitlab/performance_bar_spec.rb +++ b/spec/lib/gitlab/performance_bar_spec.rb @@ -2,38 +2,38 @@ require 'spec_helper' describe Gitlab::PerformanceBar do describe '.enabled?' do - it 'returns false when given user is nil' do + it 'returns false when given actor is nil' do expect(described_class.enabled?(nil)).to be_falsy end it 'returns false when feature is disabled' do - user = double('user') + actor = double('actor') expect(Feature).to receive(:enabled?) - .with(:gitlab_performance_bar, user).and_return(false) + .with(:gitlab_performance_bar, actor).and_return(false) - expect(described_class.enabled?(user)).to be_falsy + expect(described_class.enabled?(actor)).to be_falsy end it 'returns true when feature is enabled' do - user = double('user') + actor = double('actor') expect(Feature).to receive(:enabled?) - .with(:gitlab_performance_bar, user).and_return(true) + .with(:gitlab_performance_bar, actor).and_return(true) - expect(described_class.enabled?(user)).to be_truthy + expect(described_class.enabled?(actor)).to be_truthy end end describe '.allowed_actor?' do it 'returns false when given actor is not a User' do - actor = double + actor = double('actor', thing: double) expect(described_class.allowed_actor?(actor)).to be_falsy end context 'when given actor is a User' do - let(:actor) { create(:user) } + let(:actor) { double('actor', thing: create(:user)) } before do stub_performance_bar_setting(allowed_group: 'my-group') @@ -56,7 +56,7 @@ describe Gitlab::PerformanceBar do context 'when user is a member of the allowed group' do before do - my_group.add_developer(actor) + my_group.add_developer(actor.thing) end it 'returns true' do