Merge branch 'remove-FF_LEGACY_PERSISTED_NAMES' into 'master'

Remove `FF_LEGACY_PERSISTED_NAMES` feature flag

See merge request gitlab-org/gitlab!35548

lib/feature.rb

@@ -34,26 +34,13 @@ class Feature
     def persisted_names
       return [] unless Gitlab::Database.exists?
 
-      if Gitlab::Utils.to_boolean(ENV['FF_LEGACY_PERSISTED_NAMES'])
-        # To be removed:
-        # This uses a legacy persisted names that are know to work (always)
-        Gitlab::SafeRequestStore[:flipper_persisted_names] ||=
-          begin
-            # We saw on GitLab.com, this database request was called 2300
-            # times/s. Let's cache it for a minute to avoid that load.
-            Gitlab::ProcessMemoryCache.cache_backend.fetch('flipper:persisted_names', expires_in: 1.minute) do
-              FlipperFeature.feature_names
-            end.to_set
-          end
-      else
-        # This loads names of all stored feature flags
-        # and returns a stable Set in the following order:
-        # - Memoized: using Gitlab::SafeRequestStore or @flipper
-        # - L1: using Process cache
-        # - L2: using Redis cache
-        # - DB: using a single SQL query
-        flipper.adapter.features
-      end
+      # This loads names of all stored feature flags
+      # and returns a stable Set in the following order:
+      # - Memoized: using Gitlab::SafeRequestStore or @flipper
+      # - L1: using Process cache
+      # - L2: using Redis cache
+      # - DB: using a single SQL query
+      flipper.adapter.features
     end
 
     def persisted_name?(feature_name)

spec/lib/feature_spec.rb

@@ -21,66 +21,29 @@ RSpec.describe Feature, stub_feature_flags: false do
   end
 
   describe '.persisted_names' do
-    context 'when FF_LEGACY_PERSISTED_NAMES=false' do
-      before do
-        stub_env('FF_LEGACY_PERSISTED_NAMES', 'false')
-      end
-
-      it 'returns the names of the persisted features' do
-        Feature.enable('foo')
-
-        expect(described_class.persisted_names).to contain_exactly('foo')
-      end
-
-      it 'returns an empty Array when no features are presisted' do
-        expect(described_class.persisted_names).to be_empty
-      end
+    it 'returns the names of the persisted features' do
+      Feature.enable('foo')
 
-      it 'caches the feature names when request store is active',
-       :request_store, :use_clean_rails_memory_store_caching do
-        Feature.enable('foo')
-
-        expect(Gitlab::ProcessMemoryCache.cache_backend)
-          .to receive(:fetch)
-          .once
-          .with('flipper/v1/features', expires_in: 1.minute)
-          .and_call_original
+      expect(described_class.persisted_names).to contain_exactly('foo')
+    end
 
-        2.times do
-          expect(described_class.persisted_names).to contain_exactly('foo')
-        end
-      end
+    it 'returns an empty Array when no features are presisted' do
+      expect(described_class.persisted_names).to be_empty
     end
 
-    context 'when FF_LEGACY_PERSISTED_NAMES=true' do
-      before do
-        stub_env('FF_LEGACY_PERSISTED_NAMES', 'true')
-      end
+    it 'caches the feature names when request store is active',
+      :request_store, :use_clean_rails_memory_store_caching do
+      Feature.enable('foo')
 
-      it 'returns the names of the persisted features' do
-        Feature.enable('foo')
+      expect(Gitlab::ProcessMemoryCache.cache_backend)
+        .to receive(:fetch)
+        .once
+        .with('flipper/v1/features', expires_in: 1.minute)
+        .and_call_original
 
+      2.times do
         expect(described_class.persisted_names).to contain_exactly('foo')
       end
-
-      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(Gitlab::ProcessMemoryCache.cache_backend)
-          .to receive(:fetch)
-          .once
-          .with('flipper:persisted_names', expires_in: 1.minute)
-          .and_call_original
-
-        2.times do
-          expect(described_class.persisted_names).to contain_exactly('foo')
-        end
-      end
     end
 
     it 'fetches all flags once in a single query', :request_store do