Add code to support running migrations in milestone order

In order to support running of migrations in milestone order, this
commit adds some of the necessary classes and tests to make this work.

.rubocop.yml

@@ -119,6 +119,12 @@ Lint/EmptyFile:
     - 'ee/db/embedding/seeds.rb'
     - 'ee/db/geo/seeds.rb'
 
+# This file has a lot of these, and how we name classes here is essential for how we
+# implement migration versions
+Naming/ClassAndModuleCamelCase:
+  Exclude:
+    - 'lib/gitlab/database/migration.rb'
+
 # This cop checks whether some constant value isn't a
 # mutable literal (e.g. array or hash).
 Style/MutableConstant:

gems/gitlab-utils/lib/gitlab/version_info.rb

@@ -7,10 +7,22 @@ class VersionInfo
     attr_reader :major, :minor, :patch
 
     VERSION_REGEX = /(\d+)\.(\d+)\.(\d+)/
+    MILESTONE_REGEX = /\A(\d+)\.(\d+)\z/
     # To mitigate ReDoS, limit the length of the version string we're
     # willing to check
     MAX_VERSION_LENGTH = 128
 
+    InvalidMilestoneError = Class.new(StandardError)
+
+    def self.parse_from_milestone(str)
+      raise InvalidMilestoneError if str.length > MAX_VERSION_LENGTH
+
+      m = MILESTONE_REGEX.match(str)
+      raise InvalidMilestoneError if m.nil?
+
+      VersionInfo.new(m[1].to_i, m[2].to_i)
+    end
+
     def self.parse(str, parse_suffix: false)
       return str if str.is_a?(self)
 

gems/gitlab-utils/spec/gitlab/version_info_spec.rb

@@ -130,6 +130,40 @@
     end
   end
 
+  describe '.parse_from_milestone' do
+    subject(:milestone) { described_class.parse_from_milestone(milestone_str) }
+
+    context 'when the milestone string is valid' do
+      let(:milestone_str) { '14.7' }
+
+      it "creates a #{described_class.class} with patch version zero" do
+        expect(milestone.major).to eq 14
+        expect(milestone.minor).to eq 7
+        expect(milestone.patch).to eq 0
+      end
+    end
+
+    context 'when the milestone string is not valid' do
+      let(:milestone_str) { 'foo' }
+
+      it 'raises InvalidMilestoneError' do
+        expect do
+          milestone
+        end.to raise_error "#{described_class}::InvalidMilestoneError".constantize
+      end
+    end
+
+    context 'when the milestone string is too long' do
+      let(:milestone_str) { 'a' * 129 }
+
+      it 'raises InvalidMilestoneError' do
+        expect do
+          milestone
+        end.to raise_error "#{described_class}::InvalidMilestoneError".constantize
+      end
+    end
+  end
+
   describe '.to_s' do
     it { expect(@v1_0_0.to_s).to eq("1.0.0") }
     it { expect(@v1_0_1_rc1.to_s).to eq("1.0.1-rc1") }

lib/gitlab/database/migration.rb

@@ -34,7 +34,7 @@ def enable_lock_retries?
       # to indicate backwards-compatible or otherwise minor changes (e.g. a Rails version bump).
       # However, this hasn't been strictly formalized yet.
 
-      class V1_0 < ActiveRecord::Migration[6.1] # rubocop:disable Naming/ClassAndModuleCamelCase
+      class V1_0 < ActiveRecord::Migration[6.1]
         include LockRetriesConcern
         include Gitlab::Database::MigrationHelpers::V2
         include Gitlab::Database::MigrationHelpers::AnnounceDatabase
@@ -47,11 +47,11 @@ class MigrationRecord < ActiveRecord::Base
         end
       end
 
-      class V2_0 < V1_0 # rubocop:disable Naming/ClassAndModuleCamelCase
+      class V2_0 < V1_0
         include Gitlab::Database::MigrationHelpers::RestrictGitlabSchema
       end
 
-      class V2_1 < V2_0 # rubocop:disable Naming/ClassAndModuleCamelCase
+      class V2_1 < V2_0
         include Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables
         include Gitlab::Database::Migrations::RunnerBackoff::MigrationHelpers
       end

lib/gitlab/database/migrations/milestone_mixin.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Database
+    module Migrations
+      module MilestoneMixin
+        extend ActiveSupport::Concern
+        include Gitlab::ClassAttributes
+
+        MilestoneNotSetError = Class.new(StandardError)
+
+        class_methods do
+          def milestone(milestone_str = nil)
+            if milestone_str.present?
+              set_class_attribute(:migration_milestone, milestone_str)
+            else
+              get_class_attribute(:migration_milestone)
+            end
+          end
+        end
+
+        def initialize(name = class_name, version = nil, type = nil)
+          raise MilestoneNotSetError, "Milestone is not set for #{self.class.name}" if milestone.nil?
+
+          super(name, version)
+          @version = Gitlab::Database::Migrations::Version.new(version, milestone, type)
+        end
+
+        def milestone # rubocop:disable Lint/DuplicateMethods
+          @milestone ||= self.class.milestone
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/migrations/version.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Database
+    module Migrations
+      class Version
+        InvalidTypeError = Class.new(StandardError)
+
+        include Comparable
+
+        TYPE_VALUES = {
+          regular: 0,
+          post: 1
+        }.freeze
+
+        attr_reader :timestamp, :milestone, :type_value
+
+        def initialize(timestamp, milestone, type)
+          @timestamp = timestamp
+          @milestone = milestone
+          self.type = type
+        end
+
+        def type
+          TYPE_VALUES.key(@type_value)
+        end
+
+        def type=(value)
+          @type_value = TYPE_VALUES.fetch(value.to_sym) { raise InvalidTypeError }
+        end
+
+        def regular?
+          @type_value == TYPE_VALUES[:regular]
+        end
+
+        def post_deployment?
+          @type_value == TYPE_VALUES[:post]
+        end
+
+        def <=>(other)
+          return 1 unless other.is_a?(self.class)
+
+          return milestone <=> other.milestone if milestone != other.milestone
+
+          return @type_value <=> other.type_value if @type_value != other.type_value
+
+          @timestamp <=> other.timestamp
+        end
+
+        def to_s
+          @timestamp.to_s
+        end
+
+        def to_i
+          @timestamp.to_i
+        end
+
+        def coerce(_other)
+          [-1, timestamp.to_i]
+        end
+
+        def eql?(other)
+          (self <=> other) == 0
+        end
+
+        def ==(other)
+          eql?(other)
+        end
+
+        def hash
+          [timestamp, milestone, @type_value].hash
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/database/migrations/milestone_mixin_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Database::Migrations::MilestoneMixin, feature_category: :database do
+  let(:migration_no_mixin) do
+    Class.new(Gitlab::Database::Migration[2.1]) do
+      def change
+        # no-op here to make rubocop happy
+      end
+    end
+  end
+
+  let(:migration_mixin) do
+    Class.new(Gitlab::Database::Migration[2.1]) do
+      include Gitlab::Database::Migrations::MilestoneMixin
+    end
+  end
+
+  let(:migration_mixin_version) do
+    Class.new(Gitlab::Database::Migration[2.1]) do
+      include Gitlab::Database::Migrations::MilestoneMixin
+      milestone '16.4'
+    end
+  end
+
+  context 'when the mixin is not included' do
+    it 'does not raise an error' do
+      expect { migration_no_mixin.new(4, 4) }.not_to raise_error
+    end
+  end
+
+  context 'when the mixin is included' do
+    context 'when a milestone is not specified' do
+      it "raises MilestoneNotSetError" do
+        expect { migration_mixin.new(4, 4, :regular) }.to raise_error(
+          "#{described_class}::MilestoneNotSetError".constantize
+        )
+      end
+    end
+
+    context 'when a milestone is specified' do
+      it "does not raise an error" do
+        expect { migration_mixin_version.new(4, 4, :regular) }.not_to raise_error
+      end
+    end
+  end
+end

spec/lib/gitlab/database/migrations/version_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Database::Migrations::Version, feature_category: :database do
+  let(:test_versions) do
+    [
+      4,
+      5,
+      described_class.new(6, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+      7,
+      described_class.new(8, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+      described_class.new(9, Gitlab::VersionInfo.parse_from_milestone('10.4'), :regular),
+      described_class.new(10, Gitlab::VersionInfo.parse_from_milestone('10.3'), :post),
+      described_class.new(11, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular)
+    ]
+  end
+
+  describe "#<=>" do
+    it 'sorts by existence of milestone, then by milestone, then by type, then by timestamp when sorted by version' do
+      expect(test_versions.sort.map(&:to_i)).to eq [4, 5, 7, 6, 8, 11, 10, 9]
+    end
+  end
+
+  describe 'initialize' do
+    context 'when the type is :post or :regular' do
+      it 'does not raise an error' do
+        expect { described_class.new(4, 4, :regular) }.not_to raise_error
+        expect { described_class.new(4, 4, :post) }.not_to raise_error
+      end
+    end
+
+    context 'when the type is anything else' do
+      it 'does not raise an error' do
+        expect { described_class.new(4, 4, 'foo') }.to raise_error("#{described_class}::InvalidTypeError".constantize)
+      end
+    end
+  end
+
+  describe 'eql?' do
+    where(:version1, :version2, :expected_equality) do
+      [
+        [
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+          true
+        ],
+        [
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.4'), :regular),
+          false
+        ],
+        [
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :post),
+          false
+        ],
+        [
+          described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+          described_class.new(5, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular),
+          false
+        ]
+      ]
+    end
+
+    with_them do
+      it 'correctly evaluates deep equality' do
+        expect(version1.eql?(version2)).to eq(expected_equality)
+      end
+
+      it 'correctly evaluates deep equality using ==' do
+        expect(version1 == version2).to eq(expected_equality)
+      end
+    end
+  end
+
+  describe 'type' do
+    subject { described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), migration_type) }
+
+    context 'when the migration is regular' do
+      let(:migration_type) { :regular }
+
+      it 'correctly identifies the migration type' do
+        expect(subject.type).to eq(:regular)
+        expect(subject.regular?).to eq(true)
+        expect(subject.post_deployment?).to eq(false)
+      end
+    end
+
+    context 'when the migration is post_deployment' do
+      let(:migration_type) { :post }
+
+      it 'correctly identifies the migration type' do
+        expect(subject.type).to eq(:post)
+        expect(subject.regular?).to eq(false)
+        expect(subject.post_deployment?).to eq(true)
+      end
+    end
+  end
+
+  describe 'to_s' do
+    subject { described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular) }
+
+    it 'returns the given timestamp value as a string' do
+      expect(subject.to_s).to eql('4')
+    end
+  end
+
+  describe 'hash' do
+    subject { described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular) }
+
+    let(:expected_hash) { subject.hash }
+
+    it 'deterministically returns a hash of the timestamp, milestone, and type value' do
+      3.times do
+        expect(subject.hash).to eq(expected_hash)
+      end
+    end
+  end
+end