diff --git a/app/services/concerns/search/cache.rb b/app/services/concerns/search/cache.rb new file mode 100644 index 0000000000000000000000000000000000000000..ca92ca330b11f90143c467501b9832edb2da3c98 --- /dev/null +++ b/app/services/concerns/search/cache.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Search + class Cache + DEFAULT_EXPIRES_IN = 1.minute + + def self.lookup(...) + new(...).lookup { yield } + end + + attr_reader :cache_key, :expires_in, :enabled + + def initialize(resource:, action:, expires_in: DEFAULT_EXPIRES_IN, cache_key: nil, enabled: true) + @cache_key = cache_key || generate_cache_key(resource, action) + @expires_in = expires_in + @enabled = enabled + end + + def lookup + return yield unless enabled + + Rails.cache.fetch(cache_key, expires_in: expires_in) { yield } + end + + private + + def generate_cache_key(resource, action) + "search_#{resource.class.name.downcase}_#{resource.id}_#{action}" + end + end +end diff --git a/config/feature_flags/gitlab_com_derisk/search_cache_authorizations.yml b/config/feature_flags/gitlab_com_derisk/search_cache_authorizations.yml new file mode 100644 index 0000000000000000000000000000000000000000..66a2558c49d8e9dc128c03247ed822de967ac11e --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/search_cache_authorizations.yml @@ -0,0 +1,9 @@ +--- +name: search_cache_authorizations +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/472011 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/169630 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/499721 +milestone: '17.6' +group: group::global search +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/app/services/ee/search/global_service.rb b/ee/app/services/ee/search/global_service.rb index 213973acdefac0b345f21e1cfdd2a2269bea9d27..38ea490c5b217bfce86dc4b5f2c23075fca439c6 100644 --- a/ee/app/services/ee/search/global_service.rb +++ b/ee/app/services/ee/search/global_service.rb @@ -61,7 +61,14 @@ def elastic_projects if current_user&.can_read_all_resources? :any elsif current_user - current_user.authorized_projects.pluck(:id) # rubocop: disable CodeReuse/ActiveRecord + ::Search::Cache.lookup( + resource: current_user, + action: :authorized_projects, + expires_in: 1.minute, + enabled: ::Feature.enabled?(:search_cache_authorizations, current_user) + ) do + current_user.authorized_projects.pluck_primary_key + end else [] end diff --git a/ee/spec/services/search/global_service_spec.rb b/ee/spec/services/search/global_service_spec.rb index 4f620c55bf501db8ce34f12811c52bc85e3a3d65..01a832d7794f01c8bd60aa4c8fe8d8a9d6cf57b5 100644 --- a/ee/spec/services/search/global_service_spec.rb +++ b/ee/spec/services/search/global_service_spec.rb @@ -517,6 +517,26 @@ it 'returns the projects the user has access to' do expect(elastic_projects).to eq([project.id]) end + + it 'fetches authorized projects with cache' do + expect(::Search::Cache).to receive(:lookup).and_call_original + + expect(elastic_projects).to eq([project.id]) + end + + context 'when search_cache_authorizations feature flag is disabled' do + before do + stub_feature_flags(search_cache_authorizations: false) + end + + it 'fetches authorized projects with cache with enabled: false' do + expect(::Search::Cache).to receive(:lookup) + .with(resource: user, action: anything, expires_in: anything, enabled: false) + .and_call_original + + expect(elastic_projects).to eq([project.id]) + end + end end context 'when there is no user' do diff --git a/spec/services/concerns/search/cache_spec.rb b/spec/services/concerns/search/cache_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0e5ec9ae2ffbed64cda3e7b5c50f18cae0b2152b --- /dev/null +++ b/spec/services/concerns/search/cache_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Search::Cache, feature_category: :global_search do + let_it_be(:resource) { create(:user) } + let(:action) { 'test' } + let(:cache_key) { "search_user_#{resource.id}_#{action}" } + + describe '.lookup' do + it 'uses Rails.cache to fetch the value' do + expect(Rails.cache).to receive(:fetch) + .with(cache_key, expires_in: described_class::DEFAULT_EXPIRES_IN) + .and_call_original + + described_class.lookup(resource: resource, action: action) { 'cached_value' } + end + + context 'when caching is disabled' do + it 'does not use the cache' do + expect(Rails.cache).not_to receive(:fetch) + result = described_class.lookup(resource: resource, action: action, enabled: false) { 'uncached_value' } + expect(result).to eq('uncached_value') + end + end + + context 'with a custom cache key' do + let(:custom_cache_key) { 'my_cache_key' } + + it 'uses the cache key' do + expect(Rails.cache).to receive(:fetch) + .with(custom_cache_key, expires_in: described_class::DEFAULT_EXPIRES_IN) + .and_call_original + + described_class.lookup(resource: resource, action: action, cache_key: custom_cache_key) { 'cached_value' } + end + end + + context 'with a custom expiration' do + let(:custom_expiration) { 5.minutes } + + it 'uses the expiration' do + expect(Rails.cache).to receive(:fetch) + .with(cache_key, expires_in: custom_expiration) + .and_call_original + + described_class.lookup(resource: resource, action: action, expires_in: custom_expiration) { 'cached_value' } + end + end + end +end