Turn on Cat-File cache by default

The feature flag has been introduced an was turned off by default,
now the it will default to be turned on. That change would still allow
users to turn this feature off by leveraging the Rails console by
running:

`Feature.disable("gitaly_catfile-cache")`

Another option is to manage the number of items the LRU cache will
contain, by updating the `config.toml` for Gitaly. This would be the
`catfile_cache_size`:
https://gitlab.com/gitlab-org/gitaly/blob/0dcb5c579e63754f557aef91a4fa7a00e5b8b127/config.toml.example#L27

Closes: https://gitlab.com/gitlab-org/gitaly/issues/1712

6 files changed, 43 insertions, 26 deletions

diff --git a/lib/feature/gitaly.rb b/lib/feature/gitaly.rb
index 33868e49f14..d7a8f8a0b9e 100644
--- a/lib/feature/gitaly.rb
+++ b/lib/feature/gitaly.rb
@@ -1,24 +1,30 @@
+# frozen_string_literal: true
+
+require 'set'
+
 class Feature
   class Gitaly
+    # Server feature flags should use '_' to separate words.
+    # CATFILE_CACHE sets an incorrect example
     CATFILE_CACHE = 'catfile-cache'.freeze
 
-    # Server feature flags should use '_' to separate words.
     SERVER_FEATURE_FLAGS = [CATFILE_CACHE].freeze
+    DEFAULT_ON_FLAGS = Set.new([CATFILE_CACHE]).freeze
 
     class << self
       def enabled?(feature_flag)
-        Feature::FlipperFeature.table_exists? && Feature.enabled?("gitaly_#{feature_flag}")
+        return false unless Feature::FlipperFeature.table_exists?
+
+        default_on = DEFAULT_ON_FLAGS.include?(feature_flag)
+        Feature.enabled?("gitaly_#{feature_flag}", default_enabled: default_on)
       rescue ActiveRecord::NoDatabaseError
         false
       end
 
       def server_feature_flags
-        @server_feature_flags ||=
-          begin
-            SERVER_FEATURE_FLAGS.map do |f|
-              ["gitaly-feature-#{f.tr('_', '-')}", enabled?(f).to_s]
-            end.to_h
-          end
+        SERVER_FEATURE_FLAGS.map do |f|
+          ["gitaly-feature-#{f.tr('_', '-')}", enabled?(f).to_s]
+        end.to_h
       end
     end
   end
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index f3b0e65400b..47976389af6 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -240,7 +240,6 @@ module Gitlab
       Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid
     end
 
-
     def self.token(storage)
       params = Gitlab.config.repositories.storages[storage]
       raise "storage not found: #{storage.inspect}" if params.nil?
diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb
index 78ef6bfc0ec..7d1206e551b 100644
--- a/lib/gitlab/gitaly_client/storage_settings.rb
+++ b/lib/gitlab/gitaly_client/storage_settings.rb
@@ -34,7 +34,7 @@ module Gitlab
       def self.disk_access_denied?
         return false if rugged_enabled?
 
-        !temporarily_allowed?(ALLOW_KEY) && GitalyClient.feature_enabled?(DISK_ACCESS_DENIED_FLAG)
+        !temporarily_allowed?(ALLOW_KEY) && Feature::Gitaly.enabled?(DISK_ACCESS_DENIED_FLAG)
       rescue
         false # Err on the side of caution, don't break gitlab for people
       end
diff --git a/spec/lib/feature/gitaly_spec.rb b/spec/lib/feature/gitaly_spec.rb
index 12923e4ec41..0e24a927d4c 100644
--- a/spec/lib/feature/gitaly_spec.rb
+++ b/spec/lib/feature/gitaly_spec.rb
@@ -1,32 +1,40 @@
 require 'spec_helper'
 
 describe Feature::Gitaly do
-  let(:feature_flag) { "mepmep" }
+  let(:feature_flag) { "mep_mep" }
+
+  before do
+    stub_const("#{described_class}::SERVER_FEATURE_FLAGS", [feature_flag])
+  end
 
   describe ".enabled?" do
     context 'when the gate is closed' do
       before do
-        allow(Feature).to receive(:enabled?).with("gitaly_mepmep").and_return(false)
+        stub_feature_flags(gitaly_mep_mep: false)
       end
 
       it 'returns false' do
         expect(described_class.enabled?(feature_flag)).to be(false)
       end
     end
-  end
 
-  describe ".server_feature_flags" do
-    before do
-      stub_const("#{described_class}::SERVER_FEATURE_FLAGS", [feature_flag])
-      allow(Feature).to receive(:enabled?).with("gitaly_mepmep").and_return(false)
+    context 'when the flag defaults to on' do
+      it 'returns true' do
+        expect(described_class.enabled?(feature_flag)).to be(true)
+      end
     end
+  end
 
-    subject { described_class.server_feature_flags }
+  describe ".server_feature_flags" do
+    context 'when one flag is disabled' do
+      before do
+        stub_feature_flags(gitaly_mep_mep: false)
+      end
 
-    it { is_expected.to be_a(Hash) }
+      subject { described_class.server_feature_flags }
 
-    context 'when one flag is disabled' do
-      it { is_expected.to eq("gitaly-feature-mepmep" => "false") }
+      it { is_expected.to be_a(Hash) }
+      it { is_expected.to eq("gitaly-feature-mep-mep" => "false") }
     end
   end
 end
diff --git a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb
index bb10be2a4dc..f2f53982b09 100644
--- a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb
@@ -28,12 +28,16 @@ describe Gitlab::GitalyClient::StorageSettings do
   end
 
   describe '.disk_access_denied?' do
-    it 'return false when Rugged is enabled', :enable_rugged do
-      expect(described_class.disk_access_denied?).to be_falsey
+    context 'when Rugged is enabled', :enable_rugged do
+      it 'returns false' do
+        expect(described_class.disk_access_denied?).to be_falsey
+      end
     end
 
-    it 'returns true' do
-      expect(described_class.disk_access_denied?).to be_truthy
+    context 'when Rugged is disabled' do
+      it 'returns true' do
+        expect(described_class.disk_access_denied?).to be_truthy
+      end
     end
   end
 end
diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb
index 33648292037..100e439ef44 100644
--- a/spec/support/helpers/cycle_analytics_helpers.rb
+++ b/spec/support/helpers/cycle_analytics_helpers.rb
@@ -10,7 +10,7 @@ module CycleAnalyticsHelpers
     repository = project.repository
     oldrev = repository.commit(branch_name)&.sha || Gitlab::Git::BLANK_SHA
 
-    if Timecop.frozen? && Gitlab::GitalyClient.feature_enabled?(:operation_user_commit_files)
+    if Timecop.frozen?
       mock_gitaly_multi_action_dates(repository, commit_time)
     end