diff options
Diffstat (limited to 'spec/lib/feature_spec.rb')
| -rw-r--r-- | spec/lib/feature_spec.rb | 1512 |
1 files changed, 783 insertions, 729 deletions
diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 7860d85457a..64c249770b7 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -5,1069 +5,1123 @@ require 'spec_helper' RSpec.describe Feature, :clean_gitlab_redis_feature_flag, stub_feature_flags: false, feature_category: :shared do include StubVersion - before do - # reset Flipper AR-engine - Feature.reset - skip_feature_flags_yaml_validation - end + # Pick a long-lasting real feature flag to test that we can check feature flags in the load balancer + let(:load_balancer_test_feature_flag) { :require_email_verification } - describe '.current_request' do - it 'returns a FlipperRequest with a flipper_id' do - flipper_request = described_class.current_request + where(:bypass_load_balancer) { [true, false] } - expect(flipper_request.flipper_id).to include("FlipperRequest:") + with_them do + def wrap_all_methods_with_flag_check(lb, flag) + lb.methods(false).each do |meth| + allow(lb).to receive(meth).and_wrap_original do |m, *args, **kwargs, &block| + Feature.enabled?(flag) + m.call(*args, **kwargs, &block) + end + end end - - context 'when request store is inactive' do - it 'does not cache flipper_id' do - previous_id = described_class.current_request.flipper_id - - expect(described_class.current_request.flipper_id).not_to eq(previous_id) + before do + if bypass_load_balancer + stub_env(Feature::BypassLoadBalancer::FLAG, 'true') + wrap_all_methods_with_flag_check(ApplicationRecord.load_balancer, load_balancer_test_feature_flag) end + + # reset Flipper AR-engine + Feature.reset + skip_feature_flags_yaml_validation end - context 'when request store is active', :request_store do - it 'caches flipper_id when request store is active' do - previous_id = described_class.current_request.flipper_id + describe '.current_request' do + it 'returns a FlipperRequest with a flipper_id' do + flipper_request = described_class.current_request - expect(described_class.current_request.flipper_id).to eq(previous_id) + expect(flipper_request.flipper_id).to include("FlipperRequest:") end - it 'returns a new flipper_id when request ends' do - previous_id = described_class.current_request.flipper_id - - RequestStore.end! + context 'when request store is inactive' do + it 'does not cache flipper_id' do + previous_id = described_class.current_request.flipper_id - expect(described_class.current_request.flipper_id).not_to eq(previous_id) + expect(described_class.current_request.flipper_id).not_to eq(previous_id) + end end - end - end - describe '.get' do - let(:feature) { double(:feature) } - let(:key) { 'my_feature' } + context 'when request store is active', :request_store do + it 'caches flipper_id when request store is active' do + previous_id = described_class.current_request.flipper_id - it 'returns the Flipper feature' do - expect_any_instance_of(Flipper::DSL).to receive(:feature).with(key) - .and_return(feature) + expect(described_class.current_request.flipper_id).to eq(previous_id) + end - expect(described_class.get(key)).to eq(feature) - end - end + it 'returns a new flipper_id when request ends' do + previous_id = described_class.current_request.flipper_id - describe '.persisted_names' do - it 'returns the names of the persisted features' do - Feature.enable('foo') + RequestStore.end! - expect(described_class.persisted_names).to contain_exactly('foo') + expect(described_class.current_request.flipper_id).not_to eq(previous_id) + end + end end - it 'returns an empty Array when no features are presisted' do - expect(described_class.persisted_names).to be_empty - end + describe '.gitlab_instance' do + it 'returns a FlipperGitlabInstance with a flipper_id' do + flipper_request = described_class.gitlab_instance - it 'caches the feature names when request store is active', - :request_store, :use_clean_rails_memory_store_caching do - Feature.enable('foo') + expect(flipper_request.flipper_id).to include("FlipperGitlabInstance:") + end - expect(Gitlab::ProcessMemoryCache.cache_backend) - .to receive(:fetch) - .once - .with('flipper/v1/features', { expires_in: 1.minute }) - .and_call_original + it 'caches flipper_id' do + previous_id = described_class.gitlab_instance.flipper_id - 2.times do - expect(described_class.persisted_names).to contain_exactly('foo') + expect(described_class.gitlab_instance.flipper_id).to eq(previous_id) end end - it 'fetches all flags once in a single query', :request_store do - Feature.enable('foo1') - Feature.enable('foo2') - - queries = ActiveRecord::QueryRecorder.new(skip_cached: false) do - expect(described_class.persisted_names).to contain_exactly('foo1', 'foo2') + describe '.get' do + let(:feature) { double(:feature) } + let(:key) { 'my_feature' } - RequestStore.clear! + it 'returns the Flipper feature' do + expect_any_instance_of(Flipper::DSL).to receive(:feature).with(key) + .and_return(feature) - expect(described_class.persisted_names).to contain_exactly('foo1', 'foo2') + expect(described_class.get(key)).to eq(feature) end - - expect(queries.count).to eq(1) end - end - describe '.persisted_name?' do - context 'when the feature is persisted' do - it 'returns true when feature name is a string' do + describe '.persisted_names' do + it 'returns the names of the persisted features' do Feature.enable('foo') - expect(described_class.persisted_name?('foo')).to eq(true) + expect(described_class.persisted_names).to contain_exactly('foo') end - it 'returns true when feature name is a symbol' do + it 'returns an empty Array when no features are presisted' do + expect(described_class.persisted_names).to be_empty + end + + it 'caches the feature names when request store is active', + :request_store, :use_clean_rails_memory_store_caching do Feature.enable('foo') - expect(described_class.persisted_name?(:foo)).to eq(true) - end - end + expect(Gitlab::ProcessMemoryCache.cache_backend) + .to receive(:fetch) + .once + .with('flipper/v1/features', { expires_in: 1.minute }) + .and_call_original - context 'when the feature is not persisted' do - it 'returns false when feature name is a string' do - expect(described_class.persisted_name?('foo')).to eq(false) + 2.times do + expect(described_class.persisted_names).to contain_exactly('foo') + end end - it 'returns false when feature name is a symbol' do - expect(described_class.persisted_name?(:bar)).to eq(false) - end - end - end + it 'fetches all flags once in a single query', :request_store do + Feature.enable('foo1') + Feature.enable('foo2') - describe '.all' do - let(:features) { Set.new } + queries = ActiveRecord::QueryRecorder.new(skip_cached: false) do + expect(described_class.persisted_names).to contain_exactly('foo1', 'foo2') - it 'returns the Flipper features as an array' do - expect_any_instance_of(Flipper::DSL).to receive(:features) - .and_return(features) + RequestStore.clear! - expect(described_class.all).to eq(features.to_a) + expect(described_class.persisted_names).to contain_exactly('foo1', 'foo2') + end + + expect(queries.count).to eq(1) + end end - end - describe '.flipper' do - context 'when request store is inactive' do - it 'memoizes the Flipper instance but does not not enable Flipper memoization' do - expect(Flipper).to receive(:new).once.and_call_original + describe '.persisted_name?' do + context 'when the feature is persisted' do + it 'returns true when feature name is a string' do + Feature.enable('foo') - 2.times do - described_class.flipper + expect(described_class.persisted_name?('foo')).to eq(true) end - expect(described_class.flipper.adapter.memoizing?).to eq(false) - end - end + it 'returns true when feature name is a symbol' do + Feature.enable('foo') - context 'when request store is active', :request_store do - it 'memoizes the Flipper instance' do - expect(Flipper).to receive(:new).once.and_call_original + expect(described_class.persisted_name?(:foo)).to eq(true) + end + end - described_class.flipper - described_class.instance_variable_set(:@flipper, nil) - described_class.flipper + context 'when the feature is not persisted' do + it 'returns false when feature name is a string' do + expect(described_class.persisted_name?('foo')).to eq(false) + end - expect(described_class.flipper.adapter.memoizing?).to eq(true) + it 'returns false when feature name is a symbol' do + expect(described_class.persisted_name?(:bar)).to eq(false) + end end end - end - describe '.enabled?' do - before do - allow(Feature).to receive(:log_feature_flag_states?).and_return(false) + describe '.all' do + let(:features) { Set.new } - stub_feature_flag_definition(:disabled_feature_flag) - stub_feature_flag_definition(:enabled_feature_flag, default_enabled: true) - end + it 'returns the Flipper features as an array' do + expect_any_instance_of(Flipper::DSL).to receive(:features) + .and_return(features) - context 'when using redis cache', :use_clean_rails_redis_caching do - it 'does not make recursive feature-flag calls' do - expect(described_class).to receive(:enabled?).once.and_call_original - described_class.enabled?(:disabled_feature_flag) + expect(described_class.all).to eq(features.to_a) end end - context 'when self-recursive' do - before do - allow(Feature).to receive(:with_feature).and_wrap_original do |original, name, &block| - original.call(name) do |ff| - Feature.enabled?(name) - block.call(ff) + describe '.flipper' do + context 'when request store is inactive' do + it 'memoizes the Flipper instance but does not not enable Flipper memoization' do + expect(Flipper).to receive(:new).once.and_call_original + + 2.times do + described_class.flipper end + + expect(described_class.flipper.adapter.memoizing?).to eq(false) end end - it 'returns the default value' do - expect(described_class.enabled?(:enabled_feature_flag)).to eq true - end + context 'when request store is active', :request_store do + it 'memoizes the Flipper instance' do + expect(Flipper).to receive(:new).once.and_call_original - it 'detects self recursion' do - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) - .with(have_attributes(message: 'self recursion'), { stack: [:enabled_feature_flag] }) + described_class.flipper + described_class.instance_variable_set(:@flipper, nil) + described_class.flipper - described_class.enabled?(:enabled_feature_flag) + expect(described_class.flipper.adapter.memoizing?).to eq(true) + end end end - context 'when deeply recursive' do + describe '.enabled?' do before do - allow(Feature).to receive(:with_feature).and_wrap_original do |original, name, &block| - original.call(name) do |ff| - Feature.enabled?(:"deeper_#{name}", type: :undefined, default_enabled_if_undefined: true) - block.call(ff) - end + allow(Feature).to receive(:log_feature_flag_states?).and_return(false) + + stub_feature_flag_definition(:disabled_feature_flag) + stub_feature_flag_definition(:enabled_feature_flag, default_enabled: true) + end + + context 'when using redis cache', :use_clean_rails_redis_caching do + it 'does not make recursive feature-flag calls' do + expect(described_class).to receive(:enabled?).once.and_call_original + described_class.enabled?(:disabled_feature_flag) end end - it 'detects deep recursion' do - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) - .with(have_attributes(message: 'deep recursion'), stack: have_attributes(size: be > 10)) + context 'when self-recursive' do + before do + allow(Feature).to receive(:with_feature).and_wrap_original do |original, name, &block| + original.call(name) do |ff| + Feature.enabled?(name) + block.call(ff) + end + end + end - described_class.enabled?(:enabled_feature_flag) - end - end + it 'returns the default value' do + expect(described_class.enabled?(:enabled_feature_flag)).to eq true + end - it 'returns false (and tracks / raises exception for dev) for undefined feature' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + it 'detects self recursion' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with(have_attributes(message: 'self recursion'), { stack: [:enabled_feature_flag] }) - expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey - end + described_class.enabled?(:enabled_feature_flag) + end + end - it 'returns false for undefined feature with default_enabled_if_undefined: false' do - expect(described_class.enabled?(:some_random_feature_flag, default_enabled_if_undefined: false)).to be_falsey - end + context 'when deeply recursive' do + before do + allow(Feature).to receive(:with_feature).and_wrap_original do |original, name, &block| + original.call(name) do |ff| + Feature.enabled?(:"deeper_#{name}", type: :undefined, default_enabled_if_undefined: true) + block.call(ff) + end + end + end - it 'returns true for undefined feature with default_enabled_if_undefined: true' do - expect(described_class.enabled?(:some_random_feature_flag, default_enabled_if_undefined: true)).to be_truthy - end + it 'detects deep recursion' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with(have_attributes(message: 'deep recursion'), stack: have_attributes(size: be > 10)) - it 'returns false for existing disabled feature in the database' do - described_class.disable(:disabled_feature_flag) + described_class.enabled?(:enabled_feature_flag) + end + end - expect(described_class.enabled?(:disabled_feature_flag)).to be_falsey - end + it 'returns false (and tracks / raises exception for dev) for undefined feature' do + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) - it 'returns true for existing enabled feature in the database' do - described_class.enable(:enabled_feature_flag) + expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey + end - expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy - end + it 'returns false for undefined feature with default_enabled_if_undefined: false' do + expect(described_class.enabled?(:some_random_feature_flag, default_enabled_if_undefined: false)).to be_falsey + end - it { expect(described_class.send(:l1_cache_backend)).to eq(Gitlab::ProcessMemoryCache.cache_backend) } - it { expect(described_class.send(:l2_cache_backend)).to eq(Gitlab::Redis::FeatureFlag.cache_store) } + it 'returns true for undefined feature with default_enabled_if_undefined: true' do + expect(described_class.enabled?(:some_random_feature_flag, default_enabled_if_undefined: true)).to be_truthy + end - it 'caches the status in L1 and L2 caches', - :request_store, :use_clean_rails_memory_store_caching do - described_class.enable(:disabled_feature_flag) - flipper_key = "flipper/v1/feature/disabled_feature_flag" + it 'returns false for existing disabled feature in the database' do + described_class.disable(:disabled_feature_flag) - expect(described_class.send(:l2_cache_backend)) - .to receive(:fetch) - .once - .with(flipper_key, { expires_in: 1.hour }) - .and_call_original + expect(described_class.enabled?(:disabled_feature_flag)).to be_falsey + end - expect(described_class.send(:l1_cache_backend)) - .to receive(:fetch) - .once - .with(flipper_key, { expires_in: 1.minute }) - .and_call_original + it 'returns true for existing enabled feature in the database' do + described_class.enable(:enabled_feature_flag) - 2.times do - expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy + expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy end - end - it 'returns the default value when the database does not exist' do - fake_default = double('fake default') - expect(ActiveRecord::Base).to receive(:connection) { raise ActiveRecord::NoDatabaseError, "No database" } + it { expect(described_class.send(:l1_cache_backend)).to eq(Gitlab::ProcessMemoryCache.cache_backend) } + it { expect(described_class.send(:l2_cache_backend)).to eq(Gitlab::Redis::FeatureFlag.cache_store) } - expect(described_class.enabled?(:a_feature, default_enabled_if_undefined: fake_default)).to eq(fake_default) - end + it 'caches the status in L1 and L2 caches', + :request_store, :use_clean_rails_memory_store_caching do + described_class.enable(:disabled_feature_flag) + flipper_key = "flipper/v1/feature/disabled_feature_flag" - context 'logging is enabled', :request_store do - before do - allow(Feature).to receive(:log_feature_flag_states?).and_call_original + expect(described_class.send(:l2_cache_backend)) + .to receive(:fetch) + .once + .with(flipper_key, { expires_in: 1.hour }) + .and_call_original - stub_feature_flag_definition(:enabled_feature_flag, log_state_changes: true) + expect(described_class.send(:l1_cache_backend)) + .to receive(:fetch) + .once + .with(flipper_key, { expires_in: 1.minute }) + .and_call_original - described_class.enable(:feature_flag_state_logs) - described_class.enable(:enabled_feature_flag) - described_class.enabled?(:enabled_feature_flag) + 2.times do + expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy + end end - it 'does not log feature_flag_state_logs' do - expect(described_class.logged_states).not_to have_key("feature_flag_state_logs") - end + it 'returns the default value when the database does not exist' do + fake_default = double('fake default') - it 'logs other feature flags' do - expect(described_class.logged_states).to have_key(:enabled_feature_flag) - expect(described_class.logged_states[:enabled_feature_flag]).to be_truthy - end - end + base_class = Feature::BypassLoadBalancer.enabled? ? Feature::BypassLoadBalancer::FlipperRecord : ActiveRecord::Base + expect(base_class).to receive(:connection) { raise ActiveRecord::NoDatabaseError, "No database" } - context 'cached feature flag', :request_store do - before do - described_class.send(:flipper).memoize = false - described_class.enabled?(:disabled_feature_flag) + expect(described_class.enabled?(:a_feature, default_enabled_if_undefined: fake_default)).to eq(fake_default) end - it 'caches the status in L1 cache for the first minute' do - expect do - expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original - expect(described_class.send(:l2_cache_backend)).not_to receive(:fetch) - expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy - end.not_to exceed_query_limit(0) - end + context 'logging is enabled', :request_store do + before do + allow(Feature).to receive(:log_feature_flag_states?).and_call_original - it 'caches the status in L2 cache after 2 minutes' do - travel_to 2.minutes.from_now do - expect do - expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original - expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original - expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy - end.not_to exceed_query_limit(0) + stub_feature_flag_definition(:enabled_feature_flag, log_state_changes: true) + + described_class.enable(:feature_flag_state_logs) + described_class.enable(:enabled_feature_flag) + described_class.enabled?(:enabled_feature_flag) + end + + it 'does not log feature_flag_state_logs' do + expect(described_class.logged_states).not_to have_key("feature_flag_state_logs") + end + + it 'logs other feature flags' do + expect(described_class.logged_states).to have_key(:enabled_feature_flag) + expect(described_class.logged_states[:enabled_feature_flag]).to be_truthy end end - it 'fetches the status after an hour' do - travel_to 61.minutes.from_now do + context 'cached feature flag', :request_store do + before do + described_class.send(:flipper).memoize = false + described_class.enabled?(:disabled_feature_flag) + end + + it 'caches the status in L1 cache for the first minute' do expect do expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original - expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.send(:l2_cache_backend)).not_to receive(:fetch) expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy - end.not_to exceed_query_limit(1) + end.not_to exceed_query_limit(0) end - end - end - - context 'with current_request actor' do - context 'when request store is inactive' do - it 'returns the approximate percentage set' do - number_of_times = 1_000 - percentage = 50 - described_class.enable_percentage_of_actors(:enabled_feature_flag, percentage) - gate_values = Array.new(number_of_times) do - described_class.enabled?(:enabled_feature_flag, described_class.current_request) + it 'caches the status in L2 cache after 2 minutes' do + travel_to 2.minutes.from_now do + expect do + expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy + end.not_to exceed_query_limit(0) end + end - margin_of_error = 0.05 * number_of_times - expected_size = number_of_times * percentage / 100 - expect(gate_values.count { |v| v }).to be_within(margin_of_error).of(expected_size) + it 'fetches the status after an hour' do + travel_to 61.minutes.from_now do + expect do + expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.enabled?(:disabled_feature_flag)).to be_truthy + end.not_to exceed_query_limit(1) + end end end - context 'when request store is active', :request_store do - it 'always returns the same gate value' do - described_class.enable_percentage_of_actors(:enabled_feature_flag, 50) + [:current_request, :request, described_class.current_request].each do |thing| + context "with #{thing} actor" do + context 'when request store is inactive' do + it 'returns the approximate percentage set' do + number_of_times = 1_000 + percentage = 50 + described_class.enable_percentage_of_actors(:enabled_feature_flag, percentage) - previous_gate_value = described_class.enabled?(:enabled_feature_flag, described_class.current_request) + gate_values = Array.new(number_of_times) do + described_class.enabled?(:enabled_feature_flag, thing) + end - 1_000.times do - expect(described_class.enabled?(:enabled_feature_flag, described_class.current_request)).to eq(previous_gate_value) + margin_of_error = 0.05 * number_of_times + expected_size = number_of_times * percentage / 100 + expect(gate_values.count { |v| v }).to be_within(margin_of_error).of(expected_size) + end end - end - end - end - context 'with a group member' do - let(:key) { :awesome_feature } - let(:guinea_pigs) { create_list(:user, 3) } + context 'when request store is active', :request_store do + it 'always returns the same gate value' do + described_class.enable_percentage_of_actors(:enabled_feature_flag, 50) - before do - described_class.reset - stub_feature_flag_definition(key) - Flipper.unregister_groups - Flipper.register(:guinea_pigs) do |actor| - guinea_pigs.include?(actor.thing) + previous_gate_value = described_class.enabled?(:enabled_feature_flag, thing) + + 1_000.times do + expect(described_class.enabled?(:enabled_feature_flag, thing)).to eq(previous_gate_value) + end + end + end end - described_class.enable(key, described_class.group(:guinea_pigs)) end - it 'is true for all group members' do - expect(described_class.enabled?(key, guinea_pigs[0])).to be_truthy - expect(described_class.enabled?(key, guinea_pigs[1])).to be_truthy - expect(described_class.enabled?(key, guinea_pigs[2])).to be_truthy - end + context 'with gitlab_instance actor' do + it 'always returns the same gate value' do + described_class.enable(:enabled_feature_flag, described_class.gitlab_instance) - it 'is false for any other actor' do - expect(described_class.enabled?(key, create(:user))).to be_falsey + expect(described_class.enabled?(:enabled_feature_flag, described_class.gitlab_instance)).to be_truthy + end end - end - - context 'with an individual actor' do - let(:actor) { stub_feature_flag_gate('CustomActor:5') } - let(:another_actor) { stub_feature_flag_gate('CustomActor:10') } - before do - described_class.enable(:enabled_feature_flag, actor) - end + context 'with :instance actor' do + it 'always returns the same gate value' do + described_class.enable(:enabled_feature_flag, :instance) - it 'returns true when same actor is informed' do - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be_truthy + expect(described_class.enabled?(:enabled_feature_flag, :instance)).to be_truthy + end end - it 'returns false when different actor is informed' do - expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be_falsey - end + context 'with a group member' do + let(:key) { :awesome_feature } + let(:guinea_pigs) { create_list(:user, 3) } - it 'returns false when no actor is informed' do - expect(described_class.enabled?(:enabled_feature_flag)).to be_falsey - end - end + before do + described_class.reset + stub_feature_flag_definition(key) + Flipper.unregister_groups + Flipper.register(:guinea_pigs) do |actor| + guinea_pigs.include?(actor.thing) + end + described_class.enable(key, described_class.group(:guinea_pigs)) + end - context 'with invalid actor' do - let(:actor) { double('invalid actor') } + it 'is true for all group members' do + expect(described_class.enabled?(key, guinea_pigs[0])).to be_truthy + expect(described_class.enabled?(key, guinea_pigs[1])).to be_truthy + expect(described_class.enabled?(key, guinea_pigs[2])).to be_truthy + end - context 'when is dev_or_test_env' do - it 'does raise exception' do - expect { described_class.enabled?(:enabled_feature_flag, actor) } - .to raise_error /needs to include `FeatureGate` or implement `flipper_id`/ + it 'is false for any other actor' do + expect(described_class.enabled?(key, create(:user))).to be_falsey end end - end - context 'validates usage of feature flag with YAML definition' do - let(:definition) do - Feature::Definition.new('development/my_feature_flag.yml', - name: 'my_feature_flag', - type: 'development', - default_enabled: default_enabled - ).tap(&:validate!) - end + context 'with an individual actor' do + let(:actor) { stub_feature_flag_gate('CustomActor:5') } + let(:another_actor) { stub_feature_flag_gate('CustomActor:10') } - let(:default_enabled) { false } + before do + described_class.enable(:enabled_feature_flag, actor) + end - before do - stub_env('LAZILY_CREATE_FEATURE_FLAG', '0') + it 'returns true when same actor is informed' do + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be_truthy + end - allow(Feature::Definition).to receive(:valid_usage!).and_call_original - allow(Feature::Definition).to receive(:definitions) do - { definition.key => definition } + it 'returns false when different actor is informed' do + expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be_falsey end - end - it 'when usage is correct' do - expect { described_class.enabled?(:my_feature_flag) }.not_to raise_error + it 'returns false when no actor is informed' do + expect(described_class.enabled?(:enabled_feature_flag)).to be_falsey + end end - it 'when invalid type is used' do - expect { described_class.enabled?(:my_feature_flag, type: :ops) } - .to raise_error(/The `type:` of/) - end + context 'with invalid actor' do + let(:actor) { double('invalid actor') } - context 'when default_enabled: is false in the YAML definition' do - it 'reads the default from the YAML definition' do - expect(described_class.enabled?(:my_feature_flag)).to eq(default_enabled) + context 'when is dev_or_test_env' do + it 'does raise exception' do + expect { described_class.enabled?(:enabled_feature_flag, actor) } + .to raise_error /needs to include `FeatureGate` or implement `flipper_id`/ + end end end - context 'when default_enabled: is true in the YAML definition' do - let(:default_enabled) { true } - - it 'reads the default from the YAML definition' do - expect(described_class.enabled?(:my_feature_flag)).to eq(true) + context 'validates usage of feature flag with YAML definition' do + let(:definition) do + Feature::Definition.new('development/my_feature_flag.yml', + name: 'my_feature_flag', + type: 'development', + default_enabled: default_enabled + ).tap(&:validate!) end - context 'and feature has been disabled' do - before do - described_class.disable(:my_feature_flag) - end + let(:default_enabled) { false } - it 'is not enabled' do - expect(described_class.enabled?(:my_feature_flag)).to eq(false) + before do + stub_env('LAZILY_CREATE_FEATURE_FLAG', '0') + lb_ff_definition = Feature::Definition.get(load_balancer_test_feature_flag) + allow(Feature::Definition).to receive(:valid_usage!).and_call_original + allow(Feature::Definition).to receive(:definitions) do + { definition.key => definition, lb_ff_definition.key => lb_ff_definition } end end - context 'with a cached value and the YAML definition is changed thereafter' do - before do - described_class.enabled?(:my_feature_flag) + it 'when usage is correct' do + expect { described_class.enabled?(:my_feature_flag) }.not_to raise_error + end + + it 'when invalid type is used' do + expect { described_class.enabled?(:my_feature_flag, type: :ops) } + .to raise_error(/The `type:` of/) + end + + context 'when default_enabled: is false in the YAML definition' do + it 'reads the default from the YAML definition' do + expect(described_class.enabled?(:my_feature_flag)).to eq(default_enabled) end + end - it 'reads new default value' do - allow(definition).to receive(:default_enabled).and_return(true) + context 'when default_enabled: is true in the YAML definition' do + let(:default_enabled) { true } + it 'reads the default from the YAML definition' do expect(described_class.enabled?(:my_feature_flag)).to eq(true) end - end - context 'when YAML definition does not exist for an optional type' do - let(:optional_type) { described_class::Shared::TYPES.find { |name, attrs| attrs[:optional] }.first } + context 'and feature has been disabled' do + before do + described_class.disable(:my_feature_flag) + end - context 'when in dev or test environment' do - it 'raises an error for dev' do - expect { described_class.enabled?(:non_existent_flag, type: optional_type) } - .to raise_error( - Feature::InvalidFeatureFlagError, - "The feature flag YAML definition for 'non_existent_flag' does not exist") + it 'is not enabled' do + expect(described_class.enabled?(:my_feature_flag)).to eq(false) end end - context 'when in production' do + context 'with a cached value and the YAML definition is changed thereafter' do before do - allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) + described_class.enabled?(:my_feature_flag) end - context 'when database exists' do - before do - allow(ApplicationRecord.database).to receive(:exists?).and_return(true) - end + it 'reads new default value' do + allow(definition).to receive(:default_enabled).and_return(true) - it 'checks the persisted status and returns false' do - expect(described_class).to receive(:with_feature).with(:non_existent_flag).and_call_original + expect(described_class.enabled?(:my_feature_flag)).to eq(true) + end + end + + context 'when YAML definition does not exist for an optional type' do + let(:optional_type) { described_class::Shared::TYPES.find { |name, attrs| attrs[:optional] }.first } - expect(described_class.enabled?(:non_existent_flag, type: optional_type)).to eq(false) + context 'when in dev or test environment' do + it 'raises an error for dev' do + expect { described_class.enabled?(:non_existent_flag, type: optional_type) } + .to raise_error( + Feature::InvalidFeatureFlagError, + "The feature flag YAML definition for 'non_existent_flag' does not exist") end end - context 'when database does not exist' do + context 'when in production' do before do - allow(ApplicationRecord.database).to receive(:exists?).and_return(false) + allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) + end + + context 'when database exists' do + before do + allow(ApplicationRecord.database).to receive(:exists?).and_return(true) + end + + it 'checks the persisted status and returns false' do + expect(described_class).to receive(:with_feature).with(:non_existent_flag).and_call_original + + expect(described_class.enabled?(:non_existent_flag, type: optional_type)).to eq(false) + end end - it 'returns false without checking the status in the database' do - expect(described_class).not_to receive(:get) + context 'when database does not exist' do + before do + allow(ApplicationRecord.database).to receive(:exists?).and_return(false) + end + + it 'returns false without checking the status in the database' do + expect(described_class).not_to receive(:get) - expect(described_class.enabled?(:non_existent_flag, type: optional_type)).to eq(false) + expect(described_class.enabled?(:non_existent_flag, type: optional_type)).to eq(false) + end end end end end end end - end - - describe '.disable?' do - it 'returns true (and tracks / raises exception for dev) for undefined feature' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) - - expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy - end - it 'returns true for undefined feature with default_enabled_if_undefined: false' do - expect(described_class.disabled?(:some_random_feature_flag, default_enabled_if_undefined: false)).to be_truthy - end - - it 'returns false for undefined feature with default_enabled_if_undefined: true' do - expect(described_class.disabled?(:some_random_feature_flag, default_enabled_if_undefined: true)).to be_falsey - end + describe '.disable?' do + it 'returns true (and tracks / raises exception for dev) for undefined feature' do + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) - it 'returns true for existing disabled feature in the database' do - stub_feature_flag_definition(:disabled_feature_flag) - described_class.disable(:disabled_feature_flag) + expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy + end - expect(described_class.disabled?(:disabled_feature_flag)).to be_truthy - end + it 'returns true for undefined feature with default_enabled_if_undefined: false' do + expect(described_class.disabled?(:some_random_feature_flag, default_enabled_if_undefined: false)).to be_truthy + end - it 'returns false for existing enabled feature in the database' do - stub_feature_flag_definition(:enabled_feature_flag) - described_class.enable(:enabled_feature_flag) + it 'returns false for undefined feature with default_enabled_if_undefined: true' do + expect(described_class.disabled?(:some_random_feature_flag, default_enabled_if_undefined: true)).to be_falsey + end - expect(described_class.disabled?(:enabled_feature_flag)).to be_falsey - end - end + it 'returns true for existing disabled feature in the database' do + stub_feature_flag_definition(:disabled_feature_flag) + described_class.disable(:disabled_feature_flag) - shared_examples_for 'logging' do - let(:expected_action) {} - let(:expected_extra) {} + expect(described_class.disabled?(:disabled_feature_flag)).to be_truthy + end - it 'logs the event' do - expect(Feature.logger).to receive(:info).at_least(:once).with(key: key, action: expected_action, **expected_extra) + it 'returns false for existing enabled feature in the database' do + stub_feature_flag_definition(:enabled_feature_flag) + described_class.enable(:enabled_feature_flag) - subject + expect(described_class.disabled?(:enabled_feature_flag)).to be_falsey + end end - end - describe '.enable' do - subject { described_class.enable(key, thing) } + shared_examples_for 'logging' do + let(:expected_action) {} + let(:expected_extra) {} - let(:key) { :awesome_feature } - let(:thing) { true } + it 'logs the event' do + expect(Feature.logger).to receive(:info).at_least(:once).with(key: key, action: expected_action, **expected_extra) - it_behaves_like 'logging' do - let(:expected_action) { :enable } - let(:expected_extra) { { "extra.thing" => "true" } } + subject + end end - # This is documented to return true, modify doc/administration/feature_flags.md if it changes - it 'returns true' do - expect(subject).to be true - end + describe '.enable' do + subject { described_class.enable(key, thing) } - context 'when thing is an actor' do - let(:thing) { create(:user) } + let(:key) { :awesome_feature } + let(:thing) { true } it_behaves_like 'logging' do - let(:expected_action) { eq(:enable) | eq(:remove_opt_out) } - let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } + let(:expected_action) { :enable } + let(:expected_extra) { { "extra.thing" => "true" } } end - end - end - describe '.disable' do - subject { described_class.disable(key, thing) } + # This is documented to return true, modify doc/administration/feature_flags.md if it changes + it 'returns true' do + expect(subject).to be true + end - let(:key) { :awesome_feature } - let(:thing) { false } + context 'when thing is an actor' do + let(:thing) { create(:user) } - it_behaves_like 'logging' do - let(:expected_action) { :disable } - let(:expected_extra) { { "extra.thing" => "false" } } + it_behaves_like 'logging' do + let(:expected_action) { eq(:enable) | eq(:remove_opt_out) } + let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } + end + end end - # This is documented to return true, modify doc/administration/feature_flags.md if it changes - it 'returns true' do - expect(subject).to be true - end + describe '.disable' do + subject { described_class.disable(key, thing) } - context 'when thing is an actor' do - let(:thing) { create(:user) } - let(:flag_opts) { {} } + let(:key) { :awesome_feature } + let(:thing) { false } it_behaves_like 'logging' do let(:expected_action) { :disable } - let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } + let(:expected_extra) { { "extra.thing" => "false" } } end - before do - stub_feature_flag_definition(key, flag_opts) + # This is documented to return true, modify doc/administration/feature_flags.md if it changes + it 'returns true' do + expect(subject).to be true end - context 'when the feature flag was enabled for this actor' do - before do - described_class.enable(key, thing) - end + context 'when thing is an actor' do + let(:thing) { create(:user) } + let(:flag_opts) { {} } - it 'marks this thing as disabled' do - expect { subject }.to change { thing_enabled? }.from(true).to(false) + it_behaves_like 'logging' do + let(:expected_action) { :disable } + let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } end - it 'does not change the global value' do - expect { subject }.not_to change { described_class.enabled?(key) }.from(false) + before do + stub_feature_flag_definition(key, flag_opts) end - it 'is possible to re-enable the feature' do - subject + context 'when the feature flag was enabled for this actor' do + before do + described_class.enable(key, thing) + end - expect { described_class.enable(key, thing) } - .to change { thing_enabled? }.from(false).to(true) - end - end + it 'marks this thing as disabled' do + expect { subject }.to change { thing_enabled? }.from(true).to(false) + end - context 'when the feature flag is enabled globally' do - before do - described_class.enable(key) - end + it 'does not change the global value' do + expect { subject }.not_to change { described_class.enabled?(key) }.from(false) + end + + it 'is possible to re-enable the feature' do + subject - it 'does not mark this thing as disabled' do - expect { subject }.not_to change { thing_enabled? }.from(true) + expect { described_class.enable(key, thing) } + .to change { thing_enabled? }.from(false).to(true) + end end - it 'does not change the global value' do - expect { subject }.not_to change { described_class.enabled?(key) }.from(true) + context 'when the feature flag is enabled globally' do + before do + described_class.enable(key) + end + + it 'does not mark this thing as disabled' do + expect { subject }.not_to change { thing_enabled? }.from(true) + end + + it 'does not change the global value' do + expect { subject }.not_to change { described_class.enabled?(key) }.from(true) + end end end end - end - describe 'opt_out' do - subject { described_class.opt_out(key, thing) } + describe 'opt_out' do + subject { described_class.opt_out(key, thing) } - let(:key) { :awesome_feature } + let(:key) { :awesome_feature } - before do - stub_feature_flag_definition(key) - described_class.enable(key) - end + before do + stub_feature_flag_definition(key) + described_class.enable(key) + end - context 'when thing is an actor' do - let_it_be(:thing) { create(:project) } + context 'when thing is an actor' do + let_it_be(:thing) { create(:project) } - it 'marks this thing as disabled' do - expect { subject }.to change { thing_enabled? }.from(true).to(false) - end + it 'marks this thing as disabled' do + expect { subject }.to change { thing_enabled? }.from(true).to(false) + end - it 'does not change the global value' do - expect { subject }.not_to change { described_class.enabled?(key) }.from(true) - end + it 'does not change the global value' do + expect { subject }.not_to change { described_class.enabled?(key) }.from(true) + end - it_behaves_like 'logging' do - let(:expected_action) { eq(:opt_out) } - let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } - end + it_behaves_like 'logging' do + let(:expected_action) { eq(:opt_out) } + let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } + end - it 'stores the opt-out information as a gate' do - subject + it 'stores the opt-out information as a gate' do + subject - flag = described_class.get(key) + flag = described_class.get(key) - expect(flag.actors_value).to include(include(thing.flipper_id)) - expect(flag.actors_value).not_to include(thing.flipper_id) + expect(flag.actors_value).to include(include(thing.flipper_id)) + expect(flag.actors_value).not_to include(thing.flipper_id) + end end - end - context 'when thing is a group' do - let(:thing) { Feature.group(:guinea_pigs) } - let(:guinea_pigs) { create_list(:user, 3) } + context 'when thing is a group' do + let(:thing) { Feature.group(:guinea_pigs) } + let(:guinea_pigs) { create_list(:user, 3) } - before do - Feature.reset - Flipper.unregister_groups - Flipper.register(:guinea_pigs) do |actor| - guinea_pigs.include?(actor.thing) + before do + Feature.reset + Flipper.unregister_groups + Flipper.register(:guinea_pigs) do |actor| + guinea_pigs.include?(actor.thing) + end end - end - it 'has no effect' do - expect { subject }.not_to change { described_class.enabled?(key, guinea_pigs.first) }.from(true) + it 'has no effect' do + expect { subject }.not_to change { described_class.enabled?(key, guinea_pigs.first) }.from(true) + end end end - end - describe 'remove_opt_out' do - subject { described_class.remove_opt_out(key, thing) } + describe 'remove_opt_out' do + subject { described_class.remove_opt_out(key, thing) } - let(:key) { :awesome_feature } + let(:key) { :awesome_feature } - before do - stub_feature_flag_definition(key) - described_class.enable(key) - described_class.opt_out(key, thing) - end + before do + stub_feature_flag_definition(key) + described_class.enable(key) + described_class.opt_out(key, thing) + end - context 'when thing is an actor' do - let_it_be(:thing) { create(:project) } + context 'when thing is an actor' do + let_it_be(:thing) { create(:project) } - it 're-enables this thing' do - expect { subject }.to change { thing_enabled? }.from(false).to(true) - end + it 're-enables this thing' do + expect { subject }.to change { thing_enabled? }.from(false).to(true) + end - it 'does not change the global value' do - expect { subject }.not_to change { described_class.enabled?(key) }.from(true) - end + it 'does not change the global value' do + expect { subject }.not_to change { described_class.enabled?(key) }.from(true) + end - it_behaves_like 'logging' do - let(:expected_action) { eq(:remove_opt_out) } - let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } - end + it_behaves_like 'logging' do + let(:expected_action) { eq(:remove_opt_out) } + let(:expected_extra) { { "extra.thing" => thing.flipper_id.to_s } } + end - it 'removes the opt-out information' do - subject + it 'removes the opt-out information' do + subject - flag = described_class.get(key) + flag = described_class.get(key) - expect(flag.actors_value).to be_empty + expect(flag.actors_value).to be_empty + end end - end - context 'when thing is a group' do - let(:thing) { Feature.group(:guinea_pigs) } - let(:guinea_pigs) { create_list(:user, 3) } + context 'when thing is a group' do + let(:thing) { Feature.group(:guinea_pigs) } + let(:guinea_pigs) { create_list(:user, 3) } - before do - Feature.reset - Flipper.unregister_groups - Flipper.register(:guinea_pigs) do |actor| - guinea_pigs.include?(actor.thing) + before do + Feature.reset + Flipper.unregister_groups + Flipper.register(:guinea_pigs) do |actor| + guinea_pigs.include?(actor.thing) + end end - end - it 'has no effect' do - expect { subject }.not_to change { described_class.enabled?(key, guinea_pigs.first) }.from(true) + it 'has no effect' do + expect { subject }.not_to change { described_class.enabled?(key, guinea_pigs.first) }.from(true) + end end end - end - describe '.enable_percentage_of_time' do - subject { described_class.enable_percentage_of_time(key, percentage) } + describe '.enable_percentage_of_time' do + subject { described_class.enable_percentage_of_time(key, percentage) } - let(:key) { :awesome_feature } - let(:percentage) { 50 } - - it_behaves_like 'logging' do - let(:expected_action) { :enable_percentage_of_time } - let(:expected_extra) { { "extra.percentage" => percentage.to_s } } - end + let(:key) { :awesome_feature } + let(:percentage) { 50 } - context 'when the flag is on' do - before do - described_class.enable(key) + it_behaves_like 'logging' do + let(:expected_action) { :enable_percentage_of_time } + let(:expected_extra) { { "extra.percentage" => percentage.to_s } } end - it 'fails with InvalidOperation' do - expect { subject }.to raise_error(described_class::InvalidOperation) + context 'when the flag is on' do + before do + described_class.enable(key) + end + + it 'fails with InvalidOperation' do + expect { subject }.to raise_error(described_class::InvalidOperation) + end end end - end - describe '.disable_percentage_of_time' do - subject { described_class.disable_percentage_of_time(key) } + describe '.disable_percentage_of_time' do + subject { described_class.disable_percentage_of_time(key) } - let(:key) { :awesome_feature } + let(:key) { :awesome_feature } - it_behaves_like 'logging' do - let(:expected_action) { :disable_percentage_of_time } - let(:expected_extra) { {} } + it_behaves_like 'logging' do + let(:expected_action) { :disable_percentage_of_time } + let(:expected_extra) { {} } + end end - end - - describe '.enable_percentage_of_actors' do - subject { described_class.enable_percentage_of_actors(key, percentage) } - let(:key) { :awesome_feature } - let(:percentage) { 50 } + describe '.enable_percentage_of_actors' do + subject { described_class.enable_percentage_of_actors(key, percentage) } - it_behaves_like 'logging' do - let(:expected_action) { :enable_percentage_of_actors } - let(:expected_extra) { { "extra.percentage" => percentage.to_s } } - end + let(:key) { :awesome_feature } + let(:percentage) { 50 } - context 'when the flag is on' do - before do - described_class.enable(key) + it_behaves_like 'logging' do + let(:expected_action) { :enable_percentage_of_actors } + let(:expected_extra) { { "extra.percentage" => percentage.to_s } } end - it 'fails with InvalidOperation' do - expect { subject }.to raise_error(described_class::InvalidOperation) + context 'when the flag is on' do + before do + described_class.enable(key) + end + + it 'fails with InvalidOperation' do + expect { subject }.to raise_error(described_class::InvalidOperation) + end end end - end - describe '.disable_percentage_of_actors' do - subject { described_class.disable_percentage_of_actors(key) } + describe '.disable_percentage_of_actors' do + subject { described_class.disable_percentage_of_actors(key) } - let(:key) { :awesome_feature } + let(:key) { :awesome_feature } - it_behaves_like 'logging' do - let(:expected_action) { :disable_percentage_of_actors } - let(:expected_extra) { {} } + it_behaves_like 'logging' do + let(:expected_action) { :disable_percentage_of_actors } + let(:expected_extra) { {} } + end end - end - describe '.remove' do - subject { described_class.remove(key) } - - let(:key) { :awesome_feature } - let(:actor) { create(:user) } - - before do - described_class.enable(key) - end + describe '.remove' do + subject { described_class.remove(key) } - it_behaves_like 'logging' do - let(:expected_action) { :remove } - let(:expected_extra) { {} } - end + let(:key) { :awesome_feature } + let(:actor) { create(:user) } - context 'for a non-persisted feature' do - it 'returns nil' do - expect(described_class.remove(:non_persisted_feature_flag)).to be_nil + before do + described_class.enable(key) end - it 'returns true, and cleans up' do - expect(subject).to be_truthy - expect(described_class.persisted_names).not_to include(key) + it_behaves_like 'logging' do + let(:expected_action) { :remove } + let(:expected_extra) { {} } end - end - end - - describe '.log_feature_flag_states?' do - let(:log_state_changes) { false } - let(:milestone) { "0.0" } - let(:flag_name) { :some_flag } - let(:flag_type) { 'development' } - before do - Feature.enable(:feature_flag_state_logs) - Feature.enable(:some_flag) - - allow(Feature).to receive(:log_feature_flag_states?).and_return(false) - allow(Feature).to receive(:log_feature_flag_states?).with(:feature_flag_state_logs).and_call_original - allow(Feature).to receive(:log_feature_flag_states?).with(:some_flag).and_call_original + context 'for a non-persisted feature' do + it 'returns nil' do + expect(described_class.remove(:non_persisted_feature_flag)).to be_nil + end - stub_feature_flag_definition(flag_name, - type: flag_type, - milestone: milestone, - log_state_changes: log_state_changes) + it 'returns true, and cleans up' do + expect(subject).to be_truthy + expect(described_class.persisted_names).not_to include(key) + end + end end - subject { described_class.log_feature_flag_states?(flag_name) } + describe '.log_feature_flag_states?' do + let(:log_state_changes) { false } + let(:milestone) { "0.0" } + let(:flag_name) { :some_flag } + let(:flag_type) { 'development' } - context 'when flag is feature_flag_state_logs' do - let(:milestone) { "14.6" } - let(:flag_name) { :feature_flag_state_logs } - let(:flag_type) { 'ops' } - let(:log_state_changes) { true } + before do + Feature.enable(:feature_flag_state_logs) + Feature.enable(:some_flag) - it { is_expected.to be_falsey } - end + allow(Feature).to receive(:log_feature_flag_states?).and_return(false) + allow(Feature).to receive(:log_feature_flag_states?).with(:feature_flag_state_logs).and_call_original + allow(Feature).to receive(:log_feature_flag_states?).with(:some_flag).and_call_original - context 'when flag is old' do - it { is_expected.to be_falsey } - end + stub_feature_flag_definition(flag_name, + type: flag_type, + milestone: milestone, + log_state_changes: log_state_changes) + end - context 'when flag is old while log_state_changes is not present ' do - let(:log_state_changes) { nil } + subject { described_class.log_feature_flag_states?(flag_name) } - it { is_expected.to be_falsey } - end + context 'when flag is feature_flag_state_logs' do + let(:milestone) { "14.6" } + let(:flag_name) { :feature_flag_state_logs } + let(:flag_type) { 'ops' } + let(:log_state_changes) { true } - context 'when flag is old but log_state_changes is true' do - let(:log_state_changes) { true } + it { is_expected.to be_falsey } + end - it { is_expected.to be_truthy } - end + context 'when flag is old' do + it { is_expected.to be_falsey } + end - context 'when flag is new and not feature_flag_state_logs' do - let(:milestone) { "14.6" } + context 'when flag is old while log_state_changes is not present ' do + let(:log_state_changes) { nil } - before do - stub_version('14.5.123', 'deadbeef') + it { is_expected.to be_falsey } end - it { is_expected.to be_truthy } - end + context 'when flag is old but log_state_changes is true' do + let(:log_state_changes) { true } - context 'when milestone is nil' do - let(:milestone) { nil } + it { is_expected.to be_truthy } + end - it { is_expected.to be_falsey } - end - end + context 'when flag is new and not feature_flag_state_logs' do + let(:milestone) { "14.6" } - context 'caching with stale reads from the database', :use_clean_rails_redis_caching, :request_store, :aggregate_failures do - let(:actor) { stub_feature_flag_gate('CustomActor:5') } - let(:another_actor) { stub_feature_flag_gate('CustomActor:10') } + before do + stub_version('14.5.123', 'deadbeef') + end - # This is a bit unpleasant. For these tests we want to simulate stale reads - # from the database (due to database load balancing). A simple way to do - # that is to stub the response on the adapter Flipper uses for reading from - # the database. However, there isn't a convenient API for this. We know that - # the ActiveRecord adapter is always at the 'bottom' of the chain, so we can - # find it that way. - let(:active_record_adapter) do - adapter = described_class.flipper + it { is_expected.to be_truthy } + end - loop do - break adapter unless adapter.instance_variable_get(:@adapter) + context 'when milestone is nil' do + let(:milestone) { nil } - adapter = adapter.instance_variable_get(:@adapter) + it { is_expected.to be_falsey } end end - before do - stub_feature_flag_definition(:enabled_feature_flag) - end + context 'caching with stale reads from the database', :use_clean_rails_redis_caching, :request_store, :aggregate_failures do + let(:actor) { stub_feature_flag_gate('CustomActor:5') } + let(:another_actor) { stub_feature_flag_gate('CustomActor:10') } - it 'gives the correct value when enabling for an additional actor' do - described_class.enable(:enabled_feature_flag, actor) - initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) + # This is a bit unpleasant. For these tests we want to simulate stale reads + # from the database (due to database load balancing). A simple way to do + # that is to stub the response on the adapter Flipper uses for reading from + # the database. However, there isn't a convenient API for this. We know that + # the ActiveRecord adapter is always at the 'bottom' of the chain, so we can + # find it that way. + let(:active_record_adapter) do + adapter = described_class.flipper - # This should only be enabled for `actor` - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(false) - expect(described_class.enabled?(:enabled_feature_flag)).to be(false) + loop do + break adapter unless adapter.instance_variable_get(:@adapter) - # Enable for `another_actor` and simulate a stale read - described_class.enable(:enabled_feature_flag, another_actor) - allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) + adapter = adapter.instance_variable_get(:@adapter) + end + end - # Should read from the cache and be enabled for both of these actors - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag)).to be(false) - end + before do + stub_feature_flag_definition(:enabled_feature_flag) + end - it 'gives the correct value when enabling for percentage of time' do - described_class.enable_percentage_of_time(:enabled_feature_flag, 10) - initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) + it 'gives the correct value when enabling for an additional actor' do + described_class.enable(:enabled_feature_flag, actor) + initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) - # Test against `gate_values` directly as otherwise it would be non-determistic - expect(described_class.get(:enabled_feature_flag).gate_values.percentage_of_time).to eq(10) + # This should only be enabled for `actor` + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(false) + expect(described_class.enabled?(:enabled_feature_flag)).to be(false) - # Enable 50% of time and simulate a stale read - described_class.enable_percentage_of_time(:enabled_feature_flag, 50) - allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) + # Enable for `another_actor` and simulate a stale read + described_class.enable(:enabled_feature_flag, another_actor) + allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) - # Should read from the cache and be enabled 50% of the time - expect(described_class.get(:enabled_feature_flag).gate_values.percentage_of_time).to eq(50) - end + # Should read from the cache and be enabled for both of these actors + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag)).to be(false) + end - it 'gives the correct value when disabling the flag' do - described_class.enable(:enabled_feature_flag, actor) - described_class.enable(:enabled_feature_flag, another_actor) - initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) + it 'gives the correct value when enabling for percentage of time' do + described_class.enable_percentage_of_time(:enabled_feature_flag, 10) + initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) - # This be enabled for `actor` and `another_actor` - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag)).to be(false) + # Test against `gate_values` directly as otherwise it would be non-determistic + expect(described_class.get(:enabled_feature_flag).gate_values.percentage_of_time).to eq(10) - # Disable for `another_actor` and simulate a stale read - described_class.disable(:enabled_feature_flag, another_actor) - allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) + # Enable 50% of time and simulate a stale read + described_class.enable_percentage_of_time(:enabled_feature_flag, 50) + allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) - # Should read from the cache and be enabled only for `actor` - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(false) - expect(described_class.enabled?(:enabled_feature_flag)).to be(false) - end + # Should read from the cache and be enabled 50% of the time + expect(described_class.get(:enabled_feature_flag).gate_values.percentage_of_time).to eq(50) + end - it 'gives the correct value when deleting the flag' do - described_class.enable(:enabled_feature_flag, actor) - initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) + it 'gives the correct value when disabling the flag' do + described_class.enable(:enabled_feature_flag, actor) + described_class.enable(:enabled_feature_flag, another_actor) + initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) - # This should only be enabled for `actor` - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) - expect(described_class.enabled?(:enabled_feature_flag)).to be(false) + # This be enabled for `actor` and `another_actor` + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag)).to be(false) - # Remove and simulate a stale read - described_class.remove(:enabled_feature_flag) - allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) + # Disable for `another_actor` and simulate a stale read + described_class.disable(:enabled_feature_flag, another_actor) + allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) - # Should read from the cache and be disabled everywhere - expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(false) - expect(described_class.enabled?(:enabled_feature_flag)).to be(false) - end - end + # Should read from the cache and be enabled only for `actor` + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be(false) + expect(described_class.enabled?(:enabled_feature_flag)).to be(false) + end + + it 'gives the correct value when deleting the flag' do + described_class.enable(:enabled_feature_flag, actor) + initial_gate_values = active_record_adapter.get(described_class.get(:enabled_feature_flag)) - describe Feature::Target do - describe '#targets' do - let(:project) { create(:project) } - let(:group) { create(:group) } - let(:user_name) { project.first_owner.username } + # This should only be enabled for `actor` + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(true) + expect(described_class.enabled?(:enabled_feature_flag)).to be(false) - subject do - described_class.new( - user: user_name, - project: project.full_path, - group: group.full_path, - repository: project.repository.full_path - ) - end + # Remove and simulate a stale read + described_class.remove(:enabled_feature_flag) + allow(active_record_adapter).to receive(:get).once.and_return(initial_gate_values) - it 'returns all found targets' do - expect(subject.targets).to be_an(Array) - expect(subject.targets).to eq([project.first_owner, project, group, project.repository]) + # Should read from the cache and be disabled everywhere + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be(false) + expect(described_class.enabled?(:enabled_feature_flag)).to be(false) end + end - context 'when repository target works with different types of repositories' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, :wiki_repo, group: group) } - let_it_be(:project_in_user_namespace) { create(:project, namespace: create(:user).namespace) } - let(:personal_snippet) { create(:personal_snippet) } - let(:project_snippet) { create(:project_snippet, project: project) } - - let(:targets) do - [ - project, - project.wiki, - project_in_user_namespace, - personal_snippet, - project_snippet - ] - end + describe Feature::Target do + describe '#targets' do + let(:project) { create(:project) } + let(:group) { create(:group) } + let(:user_name) { project.first_owner.username } subject do described_class.new( - repository: targets.map { |t| t.repository.full_path }.join(",") + user: user_name, + project: project.full_path, + group: group.full_path, + repository: project.repository.full_path ) end it 'returns all found targets' do expect(subject.targets).to be_an(Array) - expect(subject.targets).to eq(targets.map(&:repository)) + expect(subject.targets).to eq([project.first_owner, project, group, project.repository]) + end + + context 'when repository target works with different types of repositories' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :wiki_repo, group: group) } + let_it_be(:project_in_user_namespace) { create(:project, namespace: create(:user).namespace) } + let(:personal_snippet) { create(:personal_snippet) } + let(:project_snippet) { create(:project_snippet, project: project) } + + let(:targets) do + [ + project, + project.wiki, + project_in_user_namespace, + personal_snippet, + project_snippet + ] + end + + subject do + described_class.new( + repository: targets.map { |t| t.repository.full_path }.join(",") + ) + end + + it 'returns all found targets' do + expect(subject.targets).to be_an(Array) + expect(subject.targets).to eq(targets.map(&:repository)) + end end end end |