Merge branch 'ab/cleanup-schema-checks' into 'master'

Remove checks for specific database schema version

See merge request gitlab-org/gitlab!32310

app/models/event.rb

@@ -76,7 +76,7 @@ class Event < ApplicationRecord
   # Callbacks
   after_create :reset_project_activity
   after_create :set_last_repository_updated_at, if: :push_action?
-  after_create :track_user_interacted_projects
+  after_create ->(event) { UserInteractedProject.track(event) }
 
   # Scopes
   scope :recent, -> { reorder(id: :desc) }
@@ -429,13 +429,6 @@ class Event < ApplicationRecord
       .update_all(last_repository_updated_at: created_at)
   end
 
-  def track_user_interacted_projects
-    # Note the call to .available? is due to earlier migrations
-    # that would otherwise conflict with the call to .track
-    # (because the table does not exist yet).
-    UserInteractedProject.track(self) if UserInteractedProject.available?
-  end
-
   def design_action_names
     {
       created: _('uploaded'),

app/models/internal_id.rb

@@ -25,8 +25,6 @@ class InternalId < ApplicationRecord
 
   validates :usage, presence: true
 
-  REQUIRED_SCHEMA_VERSION = 20180305095250
-
   # Increments #last_value and saves the record
   #
   # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
@@ -63,24 +61,16 @@ class InternalId < ApplicationRecord
 
   class << self
     def track_greatest(subject, scope, usage, new_value, init)
-      return new_value unless available?
-
       InternalIdGenerator.new(subject, scope, usage)
         .track_greatest(init, new_value)
     end
 
     def generate_next(subject, scope, usage, init)
-      # Shortcut if `internal_ids` table is not available (yet)
-      # This can be the case in other (unrelated) migration specs
-      return (init.call(subject) || 0) + 1 unless available?
-
       InternalIdGenerator.new(subject, scope, usage)
         .generate(init)
     end
 
     def reset(subject, scope, usage, value)
-      return false unless available?
-
       InternalIdGenerator.new(subject, scope, usage)
         .reset(value)
     end
@@ -95,20 +85,6 @@ class InternalId < ApplicationRecord
 
       where(filter).delete_all
     end
-
-    def available?
-      return true unless Rails.env.test?
-
-      Gitlab::SafeRequestStore.fetch(:internal_ids_available_flag) do
-        ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION
-      end
-    end
-
-    # Flushes cached information about schema
-    def reset_column_information
-      Gitlab::SafeRequestStore[:internal_ids_available_flag] = nil
-      super
-    end
   end
 
   class InternalIdGenerator

app/models/project.rb

@@ -96,8 +96,7 @@ class Project < ApplicationRecord
   after_create :create_project_feature, unless: :project_feature
 
   after_create :create_ci_cd_settings,
-    unless: :ci_cd_settings,
-    if: proc { ProjectCiCdSetting.available? }
+    unless: :ci_cd_settings
 
   after_create :create_container_expiration_policy,
                unless: :container_expiration_policy

app/models/project_ci_cd_setting.rb

@@ -3,9 +3,6 @@
 class ProjectCiCdSetting < ApplicationRecord
   belongs_to :project, inverse_of: :ci_cd_settings
 
-  # The version of the schema that first introduced this model/table.
-  MINIMUM_SCHEMA_VERSION = 20180403035759
-
   DEFAULT_GIT_DEPTH = 50
 
   before_create :set_default_git_depth
@@ -20,16 +17,6 @@ class ProjectCiCdSetting < ApplicationRecord
 
   default_value_for :forward_deployment_enabled, true
 
-  def self.available?
-    @available ||=
-      ActiveRecord::Migrator.current_version >= MINIMUM_SCHEMA_VERSION
-  end
-
-  def self.reset_column_information
-    @available = nil
-    super
-  end
-
   def forward_deployment_enabled?
     super && ::Feature.enabled?(:forward_deployment_enabled, project, default_enabled: true)
   end

app/models/user_interacted_project.rb

@@ -9,9 +9,6 @@ class UserInteractedProject < ApplicationRecord
 
   CACHE_EXPIRY_TIME = 1.day
 
-  # Schema version required for this model
-  REQUIRED_SCHEMA_VERSION = 20180223120443
-
   class << self
     def track(event)
       # For events without a project, we simply don't care.
@@ -38,17 +35,6 @@ class UserInteractedProject < ApplicationRecord
       end
     end
 
-    # Check if we can safely call .track (table exists)
-    def available?
-      @available_flag ||= ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION # rubocop:disable Gitlab/PredicateMemoization
-    end
-
-    # Flushes cached information about schema
-    def reset_column_information
-      @available_flag = nil
-      super
-    end
-
     private
 
     def cached_exists?(project_id:, user_id:, &block)

spec/models/event_spec.rb

@@ -68,20 +68,13 @@ describe Event do
       end
     end
 
-    describe 'after_create :track_user_interacted_projects' do
+    describe 'after_create UserInteractedProject.track' do
       let(:event) { build(:push_event, project: project, author: project.owner) }
 
       it 'passes event to UserInteractedProject.track' do
-        expect(UserInteractedProject).to receive(:available?).and_return(true)
         expect(UserInteractedProject).to receive(:track).with(event)
         event.save
       end
-
-      it 'does not call UserInteractedProject.track if its not yet available' do
-        expect(UserInteractedProject).to receive(:available?).and_return(false)
-        expect(UserInteractedProject).not_to receive(:track)
-        event.save
-      end
     end
   end
 

spec/models/internal_id_spec.rb

@@ -88,33 +88,6 @@ describe InternalId do
 
       expect(normalized).to eq((0..seq.size - 1).to_a)
     end
-
-    context 'with an insufficient schema version' do
-      before do
-        described_class.reset_column_information
-        # Project factory will also call the current_version
-        expect(ActiveRecord::Migrator).to receive(:current_version).at_least(:once).and_return(InternalId::REQUIRED_SCHEMA_VERSION - 1)
-      end
-
-      let(:init) { double('block') }
-
-      it 'calculates next internal ids on the fly' do
-        val = rand(1..100)
-
-        expect(init).to receive(:call).with(issue).and_return(val)
-        expect(subject).to eq(val + 1)
-      end
-
-      it 'always attempts to generate internal IDs in production mode' do
-        stub_rails_env('production')
-
-        val = rand(1..100)
-        generator = double(generate: val)
-        expect(InternalId::InternalIdGenerator).to receive(:new).and_return(generator)
-
-        expect(subject).to eq(val)
-      end
-    end
   end
 
   describe '.reset' do
@@ -152,20 +125,6 @@ describe InternalId do
         described_class.generate_next(issue, scope, usage, init)
       end
     end
-
-    context 'with an insufficient schema version' do
-      let(:value) { 2 }
-
-      before do
-        described_class.reset_column_information
-        # Project factory will also call the current_version
-        expect(ActiveRecord::Migrator).to receive(:current_version).at_least(:once).and_return(InternalId::REQUIRED_SCHEMA_VERSION - 1)
-      end
-
-      it 'does not reset any of the iids' do
-        expect(subject).to be_falsey
-      end
-    end
   end
 
   describe '.track_greatest' do

spec/models/project_ci_cd_setting_spec.rb

@@ -3,25 +3,6 @@
 require 'spec_helper'
 
 describe ProjectCiCdSetting do
-  describe '.available?' do
-    before do
-      described_class.reset_column_information
-    end
-
-    it 'returns true' do
-      expect(described_class).to be_available
-    end
-
-    it 'memoizes the schema version' do
-      expect(ActiveRecord::Migrator)
-        .to receive(:current_version)
-        .and_call_original
-        .once
-
-      2.times { described_class.available? }
-    end
-  end
-
   describe 'validations' do
     it 'validates default_git_depth is between 0 and 1000 or nil' do
       expect(subject).to validate_numericality_of(:default_git_depth)

spec/models/user_interacted_project_spec.rb

@@ -44,21 +44,6 @@ describe UserInteractedProject do
     end
   end
 
-  describe '.available?' do
-    before do
-      described_class.instance_variable_set('@available_flag', nil)
-    end
-
-    it 'checks schema version and properly caches positive result' do
-      expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION - 1 - rand(1000))
-      expect(described_class.available?).to be_falsey
-      expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION + rand(1000))
-      expect(described_class.available?).to be_truthy
-      expect(ActiveRecord::Migrator).not_to receive(:current_version)
-      expect(described_class.available?).to be_truthy # cached response
-    end
-  end
-
   it { is_expected.to validate_presence_of(:project_id) }
   it { is_expected.to validate_presence_of(:user_id) }
 end