Merge branch 'fix-db-checks' into 'master'

Fix invalid DB checks

See merge request gitlab-org/gitlab!24998

config/initializers/0_license.rb

@@ -10,7 +10,7 @@ Gitlab.ee do
   end
 
   # Needed to run migration
-  if ActiveRecord::Base.connected? && ActiveRecord::Base.connection.table_exists?('licenses')
+  if Gitlab::Database.cached_table_exists?('licenses')
     message = LicenseHelper.license_message(signed_in: true, is_admin: true, in_html: false)
     if ::License.block_changes? && message.present?
       warn "WARNING: #{message}"

config/initializers/load_balancing.rb

@@ -3,7 +3,7 @@
 # We need to run this initializer after migrations are done so it doesn't fail on CI
 
 Gitlab.ee do
-  if ActiveRecord::Base.connected? && ActiveRecord::Base.connection.table_exists?('licenses')
+  if Gitlab::Database.cached_table_exists?('licenses')
     if Gitlab::Database::LoadBalancing.enable?
       Gitlab::Database.disable_prepared_statements
 

ee/app/helpers/license_helper.rb

@@ -18,7 +18,7 @@ module LicenseHelper
     License.current&.maximum_user_count || 0
   end
 
-  def license_message(signed_in: signed_in?, is_admin: current_user&.admin?)
+  def license_message(signed_in: signed_in?, is_admin: current_user&.admin?, in_html: true)
     return unless current_license
     return unless signed_in
     return unless (is_admin && current_license.notify_admins?) || current_license.notify_users?
@@ -48,12 +48,7 @@ module LicenseHelper
       message << 'service.'
     end
 
-    unless is_trial
-      renewal_faq_url = 'https://about.gitlab.com/pricing/licensing-faq/#self-managed-gitlab'
-      renewal_faq_link_start = "<a href='#{renewal_faq_url}' target='_blank'>".html_safe
-      link_end = '</a>'.html_safe
-      message << _('For renewal instructions %{link_start}view our Licensing FAQ.%{link_end}') % { link_start: renewal_faq_link_start, link_end: link_end }
-    end
+    message << renewal_instructions_message(in_html: in_html) unless is_trial
 
     message.join(' ').html_safe
   end
@@ -148,4 +143,16 @@ module LicenseHelper
   def active_user_count
     User.active.count
   end
+
+  def renewal_instructions_message(in_html: true)
+    renewal_faq_url = 'https://about.gitlab.com/pricing/licensing-faq/#self-managed-gitlab'
+
+    renewal_faq_link_start = in_html ? "<a href='#{renewal_faq_url}' target='_blank'>".html_safe : ''
+    link_end = in_html ? '</a>'.html_safe : ''
+
+    message = _('For renewal instructions %{link_start}view our Licensing FAQ.%{link_end}') % { link_start: renewal_faq_link_start, link_end: link_end }
+    message += ' ' + renewal_faq_url unless in_html
+
+    message
+  end
 end

ee/lib/ee/gitlab/import_sources.rb

@@ -12,9 +12,10 @@ module EE
 
       def ee_import_table
         # This method can be called/loaded before the database
-        # has been created. With this guard clause we prevent calling
-        # the License table until the connection has been established
-        return [] unless ActiveRecord::Base.connected? && License.feature_available?(:custom_project_templates)
+        # has been created. With this guard clause we prevent querying
+        # the License table until the table exists
+        return [] unless ::Gitlab::Database.cached_table_exists?('licenses') &&
+          License.feature_available?(:custom_project_templates)
 
         [::Gitlab::ImportSources::ImportSource.new('gitlab_custom_project_template',
                                                    'GitLab custom project template export',

ee/spec/helpers/license_helper_spec.rb

@@ -10,10 +10,7 @@ describe LicenseHelper do
 
   describe '#license_message' do
     context 'license installed' do
-      subject { license_message(signed_in: true, is_admin: false) }
-
       let(:license) { double('License') }
-      let(:faq_link_regex) { /For renewal instructions <a href.*>view our Licensing FAQ\.<\/a>/ }
 
       before do
         allow(License).to receive(:current).and_return(license)
@@ -22,6 +19,11 @@ describe LicenseHelper do
         allow(license).to receive(:remaining_days).and_return(4)
       end
 
+      context 'in HTML' do
+        let(:faq_link_regex) { /For renewal instructions <a href.*>view our Licensing FAQ\.<\/a>/ }
+
+        subject { license_message(signed_in: true, is_admin: false) }
+
         it 'does NOT have a license faq link if license is a trial' do
           allow(license).to receive(:trial?).and_return(true)
 
@@ -35,6 +37,17 @@ describe LicenseHelper do
         end
       end
 
+      context 'not in HTML' do
+        subject { license_message(signed_in: true, is_admin: false, in_html: false) }
+
+        it 'has license faq link if license is not a trial' do
+          allow(license).to receive(:trial?).and_return(false)
+
+          expect(subject).to match(/For renewal instructions view our Licensing FAQ. https:.*/)
+        end
+      end
+    end
+
     context 'no license installed' do
       before do
         allow(License).to receive(:current).and_return(nil)

lib/gitlab/database.rb

@@ -233,7 +233,7 @@ module Gitlab
     end
 
     def self.cached_table_exists?(table_name)
-      connection.schema_cache.data_source_exists?(table_name)
+      exists? && connection.schema_cache.data_source_exists?(table_name)
     end
 
     def self.database_version

lib/gitlab/marginalia.rb

@@ -22,7 +22,7 @@ module Gitlab
 
     def self.set_feature_cache
       # During db:create and db:bootstrap skip feature query as DB is not available yet.
-      return false unless ActiveRecord::Base.connected? && Gitlab::Database.cached_table_exists?('features')
+      return false unless Gitlab::Database.cached_table_exists?('features')
 
       self.enabled = Feature.enabled?(MARGINALIA_FEATURE_FLAG)
     end

spec/lib/gitlab/database_spec.rb

@@ -394,6 +394,12 @@ describe Gitlab::Database do
         expect(described_class.cached_table_exists?(:bogus_table_name)).to be_falsey
       end
     end
+
+    it 'returns false when database does not exist' do
+      expect(ActiveRecord::Base).to receive(:connection) { raise ActiveRecord::NoDatabaseError, 'broken' }
+
+      expect(described_class.cached_table_exists?(:projects)).to be(false)
+    end
   end
 
   describe '.exists?' do