Commit ee734caf authored by Andreas Brandl's avatar Andreas Brandl

Remove explicit check for schema version

This removes now unneeded explicit checks for the
database schema version.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/28249
parent 42893446
...@@ -76,7 +76,7 @@ class Event < ApplicationRecord ...@@ -76,7 +76,7 @@ class Event < ApplicationRecord
# Callbacks # Callbacks
after_create :reset_project_activity after_create :reset_project_activity
after_create :set_last_repository_updated_at, if: :push_action? after_create :set_last_repository_updated_at, if: :push_action?
after_create :track_user_interacted_projects after_create ->(event) { UserInteractedProject.track(event) }
# Scopes # Scopes
scope :recent, -> { reorder(id: :desc) } scope :recent, -> { reorder(id: :desc) }
...@@ -429,13 +429,6 @@ class Event < ApplicationRecord ...@@ -429,13 +429,6 @@ class Event < ApplicationRecord
.update_all(last_repository_updated_at: created_at) .update_all(last_repository_updated_at: created_at)
end 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 def design_action_names
{ {
created: _('uploaded'), created: _('uploaded'),
......
...@@ -25,8 +25,6 @@ class InternalId < ApplicationRecord ...@@ -25,8 +25,6 @@ class InternalId < ApplicationRecord
validates :usage, presence: true validates :usage, presence: true
REQUIRED_SCHEMA_VERSION = 20180305095250
# Increments #last_value and saves the record # Increments #last_value and saves the record
# #
# The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL). # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
...@@ -63,24 +61,16 @@ class InternalId < ApplicationRecord ...@@ -63,24 +61,16 @@ class InternalId < ApplicationRecord
class << self class << self
def track_greatest(subject, scope, usage, new_value, init) def track_greatest(subject, scope, usage, new_value, init)
return new_value unless available?
InternalIdGenerator.new(subject, scope, usage) InternalIdGenerator.new(subject, scope, usage)
.track_greatest(init, new_value) .track_greatest(init, new_value)
end end
def generate_next(subject, scope, usage, init) 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) InternalIdGenerator.new(subject, scope, usage)
.generate(init) .generate(init)
end end
def reset(subject, scope, usage, value) def reset(subject, scope, usage, value)
return false unless available?
InternalIdGenerator.new(subject, scope, usage) InternalIdGenerator.new(subject, scope, usage)
.reset(value) .reset(value)
end end
...@@ -95,20 +85,6 @@ class InternalId < ApplicationRecord ...@@ -95,20 +85,6 @@ class InternalId < ApplicationRecord
where(filter).delete_all where(filter).delete_all
end 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 end
class InternalIdGenerator class InternalIdGenerator
......
...@@ -96,8 +96,7 @@ class Project < ApplicationRecord ...@@ -96,8 +96,7 @@ class Project < ApplicationRecord
after_create :create_project_feature, unless: :project_feature after_create :create_project_feature, unless: :project_feature
after_create :create_ci_cd_settings, after_create :create_ci_cd_settings,
unless: :ci_cd_settings, unless: :ci_cd_settings
if: proc { ProjectCiCdSetting.available? }
after_create :create_container_expiration_policy, after_create :create_container_expiration_policy,
unless: :container_expiration_policy unless: :container_expiration_policy
......
...@@ -3,9 +3,6 @@ ...@@ -3,9 +3,6 @@
class ProjectCiCdSetting < ApplicationRecord class ProjectCiCdSetting < ApplicationRecord
belongs_to :project, inverse_of: :ci_cd_settings 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 DEFAULT_GIT_DEPTH = 50
before_create :set_default_git_depth before_create :set_default_git_depth
...@@ -20,16 +17,6 @@ class ProjectCiCdSetting < ApplicationRecord ...@@ -20,16 +17,6 @@ class ProjectCiCdSetting < ApplicationRecord
default_value_for :forward_deployment_enabled, true 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? def forward_deployment_enabled?
super && ::Feature.enabled?(:forward_deployment_enabled, project, default_enabled: true) super && ::Feature.enabled?(:forward_deployment_enabled, project, default_enabled: true)
end end
......
...@@ -9,9 +9,6 @@ class UserInteractedProject < ApplicationRecord ...@@ -9,9 +9,6 @@ class UserInteractedProject < ApplicationRecord
CACHE_EXPIRY_TIME = 1.day CACHE_EXPIRY_TIME = 1.day
# Schema version required for this model
REQUIRED_SCHEMA_VERSION = 20180223120443
class << self class << self
def track(event) def track(event)
# For events without a project, we simply don't care. # For events without a project, we simply don't care.
...@@ -38,17 +35,6 @@ class UserInteractedProject < ApplicationRecord ...@@ -38,17 +35,6 @@ class UserInteractedProject < ApplicationRecord
end end
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 private
def cached_exists?(project_id:, user_id:, &block) def cached_exists?(project_id:, user_id:, &block)
......
...@@ -68,20 +68,13 @@ describe Event do ...@@ -68,20 +68,13 @@ describe Event do
end end
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) } let(:event) { build(:push_event, project: project, author: project.owner) }
it 'passes event to UserInteractedProject.track' do it 'passes event to UserInteractedProject.track' do
expect(UserInteractedProject).to receive(:available?).and_return(true)
expect(UserInteractedProject).to receive(:track).with(event) expect(UserInteractedProject).to receive(:track).with(event)
event.save event.save
end 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
end end
......
...@@ -88,33 +88,6 @@ describe InternalId do ...@@ -88,33 +88,6 @@ describe InternalId do
expect(normalized).to eq((0..seq.size - 1).to_a) expect(normalized).to eq((0..seq.size - 1).to_a)
end 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 end
describe '.reset' do describe '.reset' do
...@@ -152,20 +125,6 @@ describe InternalId do ...@@ -152,20 +125,6 @@ describe InternalId do
described_class.generate_next(issue, scope, usage, init) described_class.generate_next(issue, scope, usage, init)
end end
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 end
describe '.track_greatest' do describe '.track_greatest' do
......
...@@ -3,25 +3,6 @@ ...@@ -3,25 +3,6 @@
require 'spec_helper' require 'spec_helper'
describe ProjectCiCdSetting do 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 describe 'validations' do
it 'validates default_git_depth is between 0 and 1000 or nil' do it 'validates default_git_depth is between 0 and 1000 or nil' do
expect(subject).to validate_numericality_of(:default_git_depth) expect(subject).to validate_numericality_of(:default_git_depth)
......
...@@ -44,21 +44,6 @@ describe UserInteractedProject do ...@@ -44,21 +44,6 @@ describe UserInteractedProject do
end end
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(:project_id) }
it { is_expected.to validate_presence_of(:user_id) } it { is_expected.to validate_presence_of(:user_id) }
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