Commit 5c464b3d authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Disable Flipper memoization without RequestStore

With RequestStore, it is safe to memoize the feature flag values because
they will be cleared on the next request. Without RequestStore, we can
still memoize the Flipper instance but we must not enable memoization of
feature flag values because those would be permanent until the process
is restarted.
parent 55f670f9
...@@ -155,13 +155,13 @@ class Feature ...@@ -155,13 +155,13 @@ class Feature
def flipper def flipper
if Gitlab::SafeRequestStore.active? if Gitlab::SafeRequestStore.active?
Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance(memoize: true)
else else
@flipper ||= build_flipper_instance @flipper ||= build_flipper_instance
end end
end end
def build_flipper_instance def build_flipper_instance(memoize: false)
active_record_adapter = Flipper::Adapters::ActiveRecord.new( active_record_adapter = Flipper::Adapters::ActiveRecord.new(
feature_class: FlipperFeature, feature_class: FlipperFeature,
gate_class: FlipperGate) gate_class: FlipperGate)
...@@ -182,7 +182,7 @@ class Feature ...@@ -182,7 +182,7 @@ class Feature
expires_in: 1.minute) expires_in: 1.minute)
Flipper.new(flipper_adapter).tap do |flip| Flipper.new(flipper_adapter).tap do |flip|
flip.memoize = true flip.memoize = memoize
end end
end end
......
...@@ -102,12 +102,14 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -102,12 +102,14 @@ RSpec.describe Feature, stub_feature_flags: false do
describe '.flipper' do describe '.flipper' do
context 'when request store is inactive' do context 'when request store is inactive' do
it 'memoizes the Flipper instance' do it 'memoizes the Flipper instance but does not not enable Flipper memoization' do
expect(Flipper).to receive(:new).once.and_call_original expect(Flipper).to receive(:new).once.and_call_original
2.times do 2.times do
described_class.send(:flipper) described_class.flipper
end end
expect(described_class.flipper.adapter.memoizing?).to eq(false)
end end
end end
...@@ -115,9 +117,11 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -115,9 +117,11 @@ RSpec.describe Feature, stub_feature_flags: false do
it 'memoizes the Flipper instance' do it 'memoizes the Flipper instance' do
expect(Flipper).to receive(:new).once.and_call_original expect(Flipper).to receive(:new).once.and_call_original
described_class.send(:flipper) described_class.flipper
described_class.instance_variable_set(:@flipper, nil) described_class.instance_variable_set(:@flipper, nil)
described_class.send(:flipper) described_class.flipper
expect(described_class.flipper.adapter.memoizing?).to eq(true)
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment