Merge branch 'ajk-214978-designs-iids' into 'master'

Add IID to designs

See merge request gitlab-org/gitlab!46596

app/models/alert_management/alert.rb

@@ -34,7 +34,7 @@ module AlertManagement
     has_many :ordered_notes, -> { fresh }, as: :noteable, class_name: 'Note'
     has_many :user_mentions, class_name: 'AlertManagement::AlertUserMention', foreign_key: :alert_management_alert_id
 
-    has_internal_id :iid, scope: :project, init: ->(s) { s.project.alert_management_alerts.maximum(:iid) }
+    has_internal_id :iid, scope: :project
 
     sha_attribute :fingerprint
 

app/models/ci/pipeline.rb

@@ -42,9 +42,16 @@ module Ci
     belongs_to :external_pull_request
     belongs_to :ci_ref, class_name: 'Ci::Ref', foreign_key: :ci_ref_id, inverse_of: :pipelines
 
-    has_internal_id :iid, scope: :project, presence: false, track_if: -> { !importing? }, ensure_if: -> { !importing? }, init: ->(s) do
-      s&.project&.all_pipelines&.maximum(:iid) || s&.project&.all_pipelines&.count
-    end
+    has_internal_id :iid, scope: :project, presence: false,
+      track_if: -> { !importing? },
+      ensure_if: -> { !importing? },
+      init: ->(pipeline, scope) do
+        if pipeline
+          pipeline.project&.all_pipelines&.maximum(:iid) || pipeline.project&.all_pipelines&.count
+        elsif scope
+          ::Ci::Pipeline.where(**scope).maximum(:iid)
+        end
+      end
 
     has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline
     has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline

app/models/concerns/atomic_internal_id.rb

@@ -27,16 +27,42 @@ module AtomicInternalId
   extend ActiveSupport::Concern
 
   class_methods do
-    def has_internal_id(column, scope:, init:, ensure_if: nil, track_if: nil, presence: true, backfill: false) # rubocop:disable Naming/PredicateName
-      # We require init here to retain the ability to recalculate in the absence of a
-      # InternalId record (we may delete records in `internal_ids` for example).
-      raise "has_internal_id requires a init block, none given." unless init
+    def has_internal_id( # rubocop:disable Naming/PredicateName
+      column, scope:, init: :not_given, ensure_if: nil, track_if: nil,
+      presence: true, backfill: false, hook_names: :create)
+      raise "has_internal_id init must not be nil if given." if init.nil?
       raise "has_internal_id needs to be defined on association." unless self.reflect_on_association(scope)
 
-      before_validation :"track_#{scope}_#{column}!", on: :create, if: track_if
-      before_validation :"ensure_#{scope}_#{column}!", on: :create, if: ensure_if
+      init = infer_init(scope) if init == :not_given
+      before_validation :"track_#{scope}_#{column}!", on: hook_names, if: track_if
+      before_validation :"ensure_#{scope}_#{column}!", on: hook_names, if: ensure_if
       validates column, presence: presence
 
+      define_singleton_internal_id_methods(scope, column, init)
+      define_instance_internal_id_methods(scope, column, init, backfill)
+    end
+
+    private
+
+    def infer_init(scope)
+      case scope
+      when :project
+        AtomicInternalId.project_init(self)
+      when :group
+        AtomicInternalId.group_init(self)
+      else
+        # We require init here to retain the ability to recalculate in the absence of a
+        # InternalId record (we may delete records in `internal_ids` for example).
+        raise "has_internal_id - cannot infer init for scope: #{scope}"
+      end
+    end
+
+    # Defines instance methods:
+    #   - ensure_{scope}_{column}!
+    #   - track_{scope}_{column}!
+    #   - reset_{scope}_{column}
+    #   - {column}=
+    def define_instance_internal_id_methods(scope, column, init, backfill)
       define_method("ensure_#{scope}_#{column}!") do
         return if backfill && self.class.where(column => nil).exists?
 
@@ -103,19 +129,95 @@ module AtomicInternalId
         read_attribute(column)
       end
     end
+
+    # Defines class methods:
+    #
+    # - with_{scope}_{column}_supply
+    #   This method can be used to allocate a block of IID values during
+    #   bulk operations (importing/copying, etc). This can be more efficient
+    #   than creating instances one-by-one.
+    #
+    #   Pass in a block that receives a `Supply` instance. To allocate a new
+    #   IID value, call `Supply#next_value`.
+    #
+    #   Example:
+    #
+    #   MyClass.with_project_iid_supply(project) do |supply|
+    #     attributes = MyClass.where(project: project).find_each do |record|
+    #       record.attributes.merge(iid: supply.next_value)
+    #     end
+    #
+    #     bulk_insert(attributes)
+    #   end
+    def define_singleton_internal_id_methods(scope, column, init)
+      define_singleton_method("with_#{scope}_#{column}_supply") do |scope_value, &block|
+        subject = find_by(scope => scope_value) || self
+        scope_attrs = ::AtomicInternalId.scope_attrs(scope_value)
+        usage = ::AtomicInternalId.scope_usage(self)
+
+        generator = InternalId::InternalIdGenerator.new(subject, scope_attrs, usage, init)
+
+        generator.with_lock do
+          supply = Supply.new(generator.record.last_value)
+          block.call(supply)
+        ensure
+          generator.track_greatest(supply.current_value) if supply
+        end
+      end
+    end
+  end
+
+  def self.scope_attrs(scope_value)
+    { scope_value.class.table_name.singularize.to_sym => scope_value } if scope_value
   end
 
   def internal_id_scope_attrs(scope)
     scope_value = internal_id_read_scope(scope)
 
-    { scope_value.class.table_name.singularize.to_sym => scope_value } if scope_value
+    ::AtomicInternalId.scope_attrs(scope_value)
   end
 
   def internal_id_scope_usage
-    self.class.table_name.to_sym
+    ::AtomicInternalId.scope_usage(self.class)
+  end
+
+  def self.scope_usage(including_class)
+    including_class.table_name.to_sym
+  end
+
+  def self.project_init(klass, column_name = :iid)
+    ->(instance, scope) do
+      if instance
+        klass.where(project_id: instance.project_id).maximum(column_name)
+      elsif scope.present?
+        klass.where(**scope).maximum(column_name)
+      end
+    end
+  end
+
+  def self.group_init(klass, column_name = :iid)
+    ->(instance, scope) do
+      if instance
+        klass.where(group_id: instance.group_id).maximum(column_name)
+      elsif scope.present?
+        klass.where(group: scope[:namespace]).maximum(column_name)
+      end
+    end
   end
 
   def internal_id_read_scope(scope)
     association(scope).reader
   end
+
+  class Supply
+    attr_reader :current_value
+
+    def initialize(start_value)
+      @current_value = start_value
+    end
+
+    def next_value
+      @current_value += 1
+    end
+  end
 end

app/models/concerns/enums/internal_id.rb

@@ -14,7 +14,8 @@ module Enums
         operations_feature_flags: 6,
         operations_user_lists: 7,
         alert_management_alerts: 8,
-        sprints: 9 # iterations
+        sprints: 9, # iterations
+        design_management_designs: 10
       }
     end
   end

app/models/deployment.rb

@@ -21,9 +21,7 @@ class Deployment < ApplicationRecord
 
   has_one :deployment_cluster
 
-  has_internal_id :iid, scope: :project, track_if: -> { !importing? }, init: ->(s) do
-    Deployment.where(project: s.project).maximum(:iid) if s&.project
-  end
+  has_internal_id :iid, scope: :project, track_if: -> { !importing? }
 
   validates :sha, presence: true
   validates :ref, presence: true

app/models/design_management/design.rb

@@ -2,6 +2,7 @@
 
 module DesignManagement
   class Design < ApplicationRecord
+    include AtomicInternalId
     include Importable
     include Noteable
     include Gitlab::FileTypeDetection
@@ -26,6 +27,10 @@ module DesignManagement
 
     has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
+    has_internal_id :iid, scope: :project, presence: true,
+      hook_names: %i[create update], # Deal with old records
+      track_if: -> { !importing? }
+
     validates :project, :filename, presence: true
     validates :issue, presence: true, unless: :importing?
     validates :filename, uniqueness: { scope: :issue_id }, length: { maximum: 255 }

app/models/internal_id.rb

@@ -61,13 +61,13 @@ class InternalId < ApplicationRecord
 
   class << self
     def track_greatest(subject, scope, usage, new_value, init)
-      InternalIdGenerator.new(subject, scope, usage)
-        .track_greatest(init, new_value)
+      InternalIdGenerator.new(subject, scope, usage, init)
+        .track_greatest(new_value)
     end
 
     def generate_next(subject, scope, usage, init)
-      InternalIdGenerator.new(subject, scope, usage)
-        .generate(init)
+      InternalIdGenerator.new(subject, scope, usage, init)
+        .generate
     end
 
     def reset(subject, scope, usage, value)
@@ -99,15 +99,18 @@ class InternalId < ApplicationRecord
     # 4) In the absence of a record in the internal_ids table, one will be created
     #    and last_value will be calculated on the fly.
     #
-    # subject: The instance we're generating an internal id for. Gets passed to init if called.
+    # subject: The instance or class we're generating an internal id for.
     # scope: Attributes that define the scope for id generation.
+    #        Valid keys are `project/project_id` and `namespace/namespace_id`.
     # usage: Symbol to define the usage of the internal id, see InternalId.usages
-    attr_reader :subject, :scope, :scope_attrs, :usage
+    # init: Proc that accepts the subject and the scope and returns Integer|NilClass
+    attr_reader :subject, :scope, :scope_attrs, :usage, :init
 
-    def initialize(subject, scope, usage)
+    def initialize(subject, scope, usage, init = nil)
       @subject = subject
       @scope = scope
       @usage = usage
+      @init = init
 
       raise ArgumentError, 'Scope is not well-defined, need at least one column for scope (given: 0)' if scope.empty?
 
@@ -119,13 +122,13 @@ class InternalId < ApplicationRecord
     # Generates next internal id and returns it
     # init: Block that gets called to initialize InternalId record if not present
     #       Make sure to not throw exceptions in the absence of records (if this is expected).
-    def generate(init)
+    def generate
       subject.transaction do
         # Create a record in internal_ids if one does not yet exist
         # and increment its last value
         #
         # Note this will acquire a ROW SHARE lock on the InternalId record
-        (lookup || create_record(init)).increment_and_save!
+        record.increment_and_save!
       end
     end
 
@@ -148,12 +151,20 @@ class InternalId < ApplicationRecord
     # and set its new_value if it is higher than the current last_value
     #
     # Note this will acquire a ROW SHARE lock on the InternalId record
-    def track_greatest(init, new_value)
+    def track_greatest(new_value)
       subject.transaction do
-        (lookup || create_record(init)).track_greatest_and_save!(new_value)
+        record.track_greatest_and_save!(new_value)
       end
     end
 
+    def record
+      @record ||= (lookup || create_record)
+    end
+
+    def with_lock(&block)
+      record.with_lock(&block)
+    end
+
     private
 
     # Retrieve InternalId record for (project, usage) combination, if it exists
@@ -171,12 +182,16 @@ class InternalId < ApplicationRecord
     # was faster in doing this, we'll realize once we hit the unique key constraint
     # violation. We can safely roll-back the nested transaction and perform
     # a lookup instead to retrieve the record.
-    def create_record(init)
+    def create_record
+      raise ArgumentError, 'Cannot initialize without init!' unless init
+
+      instance = subject.is_a?(::Class) ? nil : subject
+
       subject.transaction(requires_new: true) do
         InternalId.create!(
           **scope,
           usage: usage_value,
-          last_value: init.call(subject) || 0
+          last_value: init.call(instance, scope) || 0
         )
       end
     rescue ActiveRecord::RecordNotUnique

app/models/issue.rb

@@ -48,7 +48,7 @@ class Issue < ApplicationRecord
   belongs_to :moved_to, class_name: 'Issue'
   has_one :moved_from, class_name: 'Issue', foreign_key: :moved_to_id
 
-  has_internal_id :iid, scope: :project, track_if: -> { !importing? }, init: ->(s) { s&.project&.issues&.maximum(:iid) }
+  has_internal_id :iid, scope: :project, track_if: -> { !importing? }
 
   has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 

app/models/iteration.rb

@@ -17,8 +17,8 @@ class Iteration < ApplicationRecord
   belongs_to :project
   belongs_to :group
 
-  has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.iterations&.maximum(:iid) }
-  has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.iterations&.maximum(:iid) }
+  has_internal_id :iid, scope: :project
+  has_internal_id :iid, scope: :group
 
   validates :start_date, presence: true
   validates :due_date, presence: true

app/models/merge_request.rb

@@ -41,7 +41,14 @@ class MergeRequest < ApplicationRecord
   belongs_to :merge_user, class_name: "User"
   belongs_to :iteration, foreign_key: 'sprint_id'
 
-  has_internal_id :iid, scope: :target_project, track_if: -> { !importing? }, init: ->(s) { s&.target_project&.merge_requests&.maximum(:iid) }
+  has_internal_id :iid, scope: :target_project, track_if: -> { !importing? },
+    init: ->(mr, scope) do
+      if mr
+        mr.target_project&.merge_requests&.maximum(:iid)
+      elsif scope[:project]
+        where(target_project: scope[:project]).maximum(:iid)
+      end
+    end
 
   has_many :merge_request_diffs
   has_many :merge_request_context_commits, inverse_of: :merge_request

app/models/milestone.rb

@@ -12,8 +12,8 @@ class Milestone < ApplicationRecord
   has_many :milestone_releases
   has_many :releases, through: :milestone_releases
 
-  has_internal_id :iid, scope: :project, track_if: -> { !importing? }, init: ->(s) { s&.project&.milestones&.maximum(:iid) }
-  has_internal_id :iid, scope: :group, track_if: -> { !importing? }, init: ->(s) { s&.group&.milestones&.maximum(:iid) }
+  has_internal_id :iid, scope: :project, track_if: -> { !importing? }
+  has_internal_id :iid, scope: :group, track_if: -> { !importing? }
 
   has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 

app/models/operations/feature_flag.rb

@@ -13,7 +13,7 @@ module Operations
 
     belongs_to :project
 
-    has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.operations_feature_flags&.maximum(:iid) }
+    has_internal_id :iid, scope: :project
 
     default_value_for :active, true
 

app/models/operations/feature_flags/user_list.rb

@@ -13,7 +13,7 @@ module Operations
       has_many :strategy_user_lists
       has_many :strategies, through: :strategy_user_lists
 
-      has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.operations_feature_flags_user_lists&.maximum(:iid) }, presence: true
+      has_internal_id :iid, scope: :project, presence: true
 
       validates :project, presence: true
       validates :name,

app/services/design_management/copy_design_collection/copy_service.rb

@@ -172,20 +172,26 @@ module DesignManagement
       def copy_designs!
         design_attributes = attributes_config[:design_attributes]
 
-        new_rows = designs.map do |design|
-          design.attributes.slice(*design_attributes).merge(
-            issue_id: target_issue.id,
-            project_id: target_project.id
+        ::DesignManagement::Design.with_project_iid_supply(target_project) do |supply|
+          new_rows = designs.each_with_index.map do |design, i|
+            design.attributes.slice(*design_attributes).merge(
+              issue_id: target_issue.id,
+              project_id: target_project.id,
+              iid: supply.next_value
+            )
+          end
+
+          # TODO Replace `Gitlab::Database.bulk_insert` with `BulkInsertSafe`
+          # once https://gitlab.com/gitlab-org/gitlab/-/issues/247718 is fixed.
+          # When this is fixed, we can remove the call to
+          # `with_project_iid_supply` above, since the objects will be instantiated
+          # and callbacks (including `ensure_project_iid!`) will fire.
+          ::Gitlab::Database.bulk_insert( # rubocop:disable Gitlab/BulkInsert
+            DesignManagement::Design.table_name,
+            new_rows,
+            return_ids: true
           )
         end
-
-        # TODO Replace `Gitlab::Database.bulk_insert` with `BulkInsertSafe`
-        # once https://gitlab.com/gitlab-org/gitlab/-/issues/247718 is fixed.
-        ::Gitlab::Database.bulk_insert( # rubocop:disable Gitlab/BulkInsert
-          DesignManagement::Design.table_name,
-          new_rows,
-          return_ids: true
-        )
       end
 
       def copy_versions!

changelogs/unreleased/ajk-214978-designs-iids.yml

+---
+title: Add iid column to design_management_designs
+merge_request: 46596
+author:
+type: added

db/migrate/20201030200115_add_iid_to_design_management_design.rb

+# frozen_string_literal: true
+
+class AddIidToDesignManagementDesign < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :design_management_designs, :iid, :integer
+  end
+end

db/migrate/20201030200539_add_index_on_design_management_designs_iid_project_id.rb

+# frozen_string_literal: true
+
+class AddIndexOnDesignManagementDesignsIidProjectId < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_design_management_designs_on_iid_and_project_id'
+
+  def up
+    add_concurrent_index :design_management_designs, [:project_id, :iid],
+      name: INDEX_NAME,
+      unique: true
+  end
+
+  def down
+    remove_concurrent_index_by_name :design_management_designs, INDEX_NAME
+  end
+end

db/post_migrate/20201030203854_backfill_design_iids.rb

+# frozen_string_literal: true
+
+class BackfillDesignIids < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Designs < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'design_management_designs'
+  end
+
+  def up
+    backfill = ::Gitlab::BackgroundMigration::BackfillDesignInternalIds.new(Designs)
+
+    Designs.select(:project_id).distinct.each_batch(of: 100, column: :project_id) do |relation|
+      backfill.perform(relation)
+    end
+  end
+
+  def down
+    # NOOP
+  end
+end

db/post_migrate/20201102152554_add_not_null_check_on_iid_on_design_manangement_designs.rb

+# frozen_string_literal: true
+
+class AddNotNullCheckOnIidOnDesignManangementDesigns < ActiveRecord::Migration[6.0]
+  include ::Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_not_null_constraint(:design_management_designs, :iid)
+  end
+
+  def down
+    remove_not_null_constraint(:design_management_designs, :iid)
+  end
+end

db/schema_migrations/20201030200115

+bef50f2417b9676c89aea838f7b9c85fb88af9f52c197d8eb4613a9c91bc7741
\ No newline at end of file

db/schema_migrations/20201030200539

+2f6c7efc1716d02dd40adb08bd09b9f1e63e4248619678c0562f4b8d581e6065
\ No newline at end of file

db/schema_migrations/20201030203854

+3937235469c8fb1f2b0af9cdf38933db5ae61552d1a9050755cec5f7c16ebb66
\ No newline at end of file

db/schema_migrations/20201102152554

+7ec73c06ccc4c9f618e0455d0a7aae3b591bf52b5ddb1b3f1678d2fd50b9fd5e
\ No newline at end of file

db/structure.sql

@@ -11650,7 +11650,9 @@ CREATE TABLE design_management_designs (
     issue_id integer,
     filename character varying NOT NULL,
     relative_position integer,
-    CONSTRAINT check_07155e2715 CHECK ((char_length((filename)::text) <= 255))
+    iid integer,
+    CONSTRAINT check_07155e2715 CHECK ((char_length((filename)::text) <= 255)),
+    CONSTRAINT check_cfb92df01a CHECK ((iid IS NOT NULL))
 );
 
 CREATE SEQUENCE design_management_designs_id_seq
@@ -20592,6 +20594,8 @@ CREATE INDEX index_description_versions_on_merge_request_id ON description_versi
 
 CREATE INDEX index_design_management_designs_issue_id_relative_position_id ON design_management_designs USING btree (issue_id, relative_position, id);
 
+CREATE UNIQUE INDEX index_design_management_designs_on_iid_and_project_id ON design_management_designs USING btree (project_id, iid);
+
 CREATE UNIQUE INDEX index_design_management_designs_on_issue_id_and_filename ON design_management_designs USING btree (issue_id, filename);
 
 CREATE INDEX index_design_management_designs_on_project_id ON design_management_designs USING btree (project_id);

ee/app/models/ee/epic.rb

@@ -51,7 +51,7 @@ module EE
       has_many :children, class_name: "Epic", foreign_key: :parent_id
       has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
-      has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.epics&.maximum(:iid) }
+      has_internal_id :iid, scope: :group
 
       has_many :epic_issues
       has_many :issues, through: :epic_issues

ee/app/models/requirements_management/requirement.rb

@@ -23,7 +23,7 @@ module RequirementsManagement
 
     has_many :test_reports, inverse_of: :requirement
 
-    has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.requirements&.maximum(:iid) }
+    has_internal_id :iid, scope: :project
 
     validates :author, :project, :title, presence: true
 

ee/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb

@@ -106,7 +106,7 @@ RSpec.describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMent
       let(:design_user_mentions) { table(:design_user_mentions) }
 
       let(:project) { projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) }
-      let!(:design) { designs.create!(filename: 'test.png', project_id: project.id) }
+      let!(:design) { designs.create!(filename: 'test.png', project_id: project.id, iid: 1000) }
 
       let!(:note1) { notes.create!(noteable_id: design.id, noteable_type: 'DesignManagement::Design', project_id: project.id, author_id: author.id, note: description_mentions) }
       let!(:note2) { notes.create!(noteable_id: design.id, noteable_type: 'DesignManagement::Design', project_id: project.id, author_id: author.id, note: 'sample note') }

lib/gitlab/background_migration/backfill_design_internal_ids.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Backfill design.iid for a range of projects
+    class BackfillDesignInternalIds
+      # See app/models/internal_id
+      # This is a direct copy of the application code with the following changes:
+      # - usage enum is hard-coded to the value for design_management_designs
+      # - init is not passed around, but ignored
+      class InternalId < ActiveRecord::Base
+        def self.track_greatest(subject, scope, new_value)
+          InternalIdGenerator.new(subject, scope).track_greatest(new_value)
+        end
+
+        # Increments #last_value with new_value if it is greater than the current,
+        # and saves the record
+        #
+        # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
+        # As such, the increment is atomic and safe to be called concurrently.
+        def track_greatest_and_save!(new_value)
+          update_and_save { self.last_value = [last_value || 0, new_value].max }
+        end
+
+        private
+
+        def update_and_save(&block)
+          lock!
+          yield
+          # update_and_save_counter.increment(usage: usage, changed: last_value_changed?)
+          save!
+          last_value
+        end
+      end
+
+      # See app/models/internal_id
+      class InternalIdGenerator
+        attr_reader :subject, :scope, :scope_attrs
+
+        def initialize(subject, scope)
+          @subject = subject
+          @scope = scope
+
+          raise ArgumentError, 'Scope is not well-defined, need at least one column for scope (given: 0)' if scope.empty?
+        end
+
+        # Create a record in internal_ids if one does not yet exist
+        # and set its new_value if it is higher than the current last_value
+        #
+        # Note this will acquire a ROW SHARE lock on the InternalId record
+        def track_greatest(new_value)
+          subject.transaction do
+            record.track_greatest_and_save!(new_value)
+          end
+        end
+
+        def record
+          @record ||= (lookup || create_record)
+        end
+
+        def lookup
+          InternalId.find_by(**scope, usage: usage_value)
+        end
+
+        def usage_value
+          10 # see Enums::InternalId - this is the value for design_management_designs
+        end
+
+        # Create InternalId record for (scope, usage) combination, if it doesn't exist
+        #
+        # We blindly insert without synchronization. If another process
+        # was faster in doing this, we'll realize once we hit the unique key constraint
+        # violation. We can safely roll-back the nested transaction and perform
+        # a lookup instead to retrieve the record.
+        def create_record
+          subject.transaction(requires_new: true) do
+            InternalId.create!(
+              **scope,
+              usage: usage_value,
+              last_value: 0
+            )
+          end
+        rescue ActiveRecord::RecordNotUnique
+          lookup
+        end
+      end
+
+      attr_reader :design_class
+
+      def initialize(design_class)
+        @design_class = design_class
+      end
+
+      def perform(relation)
+        start_id, end_id = relation.pluck("min(project_id), max(project_id)").flatten
+        table = 'design_management_designs'
+
+        ActiveRecord::Base.connection.execute <<~SQL
+          WITH
+          starting_iids(project_id, iid) as (
+            SELECT project_id, MAX(COALESCE(iid, 0))
+            FROM #{table}
+            WHERE project_id BETWEEN #{start_id} AND #{end_id}
+            GROUP BY project_id
+          ),
+          with_calculated_iid(id, iid) as (
+            SELECT design.id,
+                   init.iid + ROW_NUMBER() OVER (PARTITION BY design.project_id ORDER BY design.id ASC)
+              FROM #{table} as design, starting_iids as init
+              WHERE design.project_id BETWEEN #{start_id} AND #{end_id}
+                AND design.iid IS NULL
+                AND init.project_id = design.project_id
+          )
+
+          UPDATE #{table}
+             SET iid = with_calculated_iid.iid
+            FROM with_calculated_iid
+           WHERE #{table}.id = with_calculated_iid.id
+        SQL
+
+        # track the new greatest IID value
+        relation.each do |design|
+          current_max = design_class.where(project_id: design.project_id).maximum(:iid)
+          scope = { project_id: design.project_id }
+          InternalId.track_greatest(design, scope, current_max)
+        end
+      end
+    end
+  end
+end

lib/gitlab/design_management/copy_design_collection_model_attributes.yml

@@ -29,6 +29,7 @@ ignore_design_attributes:
   - id
   - issue_id
   - project_id
+  - iid
 
 ignore_version_attributes:
   - id

spec/factories/design_management/designs.rb

 # frozen_string_literal: true
 
 FactoryBot.define do
-  factory :design, class: 'DesignManagement::Design' do
+  factory :design, traits: [:has_internal_id], class: 'DesignManagement::Design' do
     issue { association(:issue) }
     project { issue&.project || association(:project) }
     sequence(:filename) { |n| "homescreen-#{n}.jpg" }

spec/fixtures/lib/gitlab/import_export/designs/project.json

@@ -98,6 +98,7 @@
          "designs":[
             {
                "id":38,
+               "iid": 1,
                "project_id":30,
                "issue_id":469,
                "filename":"chirrido3.jpg",
@@ -107,6 +108,7 @@
             },
             {
                "id":39,
+               "iid": 2,
                "project_id":30,
                "issue_id":469,
                "filename":"jonathan_richman.jpg",
@@ -116,6 +118,7 @@
             },
             {
                "id":40,
+               "iid": 3,
                "project_id":30,
                "issue_id":469,
                "filename":"mariavontrap.jpeg",
@@ -137,6 +140,7 @@
                      "event":0,
                      "design":{
                         "id":38,
+                       "iid": 1,
                         "project_id":30,
                         "issue_id":469,
                         "filename":"chirrido3.jpg"
@@ -156,6 +160,7 @@
                      "event":1,
                      "design":{
                         "id":38,
+                       "iid": 1,
                         "project_id":30,
                         "issue_id":469,
                         "filename":"chirrido3.jpg"
@@ -167,6 +172,7 @@
                      "event":0,
                      "design":{
                         "id":39,
+                        "iid": 2,
                         "project_id":30,
                         "issue_id":469,
                         "filename":"jonathan_richman.jpg"
@@ -186,6 +192,7 @@
                      "event":1,
                      "design":{
                         "id":38,
+                       "iid": 1,
                         "project_id":30,
                         "issue_id":469,
                         "filename":"chirrido3.jpg"
@@ -197,6 +204,7 @@
                      "event":2,
                      "design":{
                         "id":39,
+                       "iid": 2,
                         "project_id":30,
                         "issue_id":469,
                         "filename":"jonathan_richman.jpg"
@@ -208,6 +216,7 @@
                      "event":0,
                      "design":{
                         "id":40,
+                        "iid": 3,
                         "project_id":30,
                         "issue_id":469,
                         "filename":"mariavontrap.jpeg"

spec/lib/gitlab/background_migration/backfill_design_internal_ids_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillDesignInternalIds, :migration, schema: 20201030203854 do
+  subject { described_class.new(designs) }
+
+  let_it_be(:namespaces)     { table(:namespaces) }
+  let_it_be(:projects)       { table(:projects) }
+  let_it_be(:designs)        { table(:design_management_designs) }
+
+  let(:namespace) { namespaces.create!(name: 'foo', path: 'foo') }
+  let(:project) { projects.create!(namespace_id: namespace.id) }
+  let(:project_2) { projects.create!(namespace_id: namespace.id) }
+
+  def create_design!(proj = project)
+    designs.create!(project_id: proj.id, filename: generate(:filename))
+  end
+
+  def migrate!
+    relation = designs.where(project_id: [project.id, project_2.id]).select(:project_id).distinct
+
+    subject.perform(relation)
+  end
+
+  it 'backfills the iid for designs' do
+    3.times { create_design! }
+
+    expect do
+      migrate!
+    end.to change { designs.pluck(:iid) }.from(contain_exactly(nil, nil, nil)).to(contain_exactly(1, 2, 3))
+  end
+
+  it 'scopes IIDs and handles range and starting-point correctly' do
+    create_design!.update!(iid: 10)
+    create_design!.update!(iid: 12)
+    create_design!(project_2).update!(iid: 7)
+    project_3 = projects.create!(namespace_id: namespace.id)
+
+    2.times { create_design! }
+    2.times { create_design!(project_2) }
+    2.times { create_design!(project_3) }
+
+    migrate!
+
+    expect(designs.where(project_id: project.id).pluck(:iid)).to contain_exactly(10, 12, 13, 14)
+    expect(designs.where(project_id: project_2.id).pluck(:iid)).to contain_exactly(7, 8, 9)
+    expect(designs.where(project_id: project_3.id).pluck(:iid)).to contain_exactly(nil, nil)
+  end
+
+  it 'updates the internal ID records' do
+    design = create_design!
+    2.times { create_design! }
+    design.update!(iid: 10)
+    scope = { project_id: project.id }
+    usage = :design_management_designs
+    init = ->(_d, _s) { 0 }
+
+    ::InternalId.track_greatest(design, scope, usage, 10, init)
+
+    migrate!
+
+    next_iid = ::InternalId.generate_next(design, scope, usage, init)
+
+    expect(designs.pluck(:iid)).to contain_exactly(10, 11, 12)
+    expect(design.reload.iid).to eq(10)
+    expect(next_iid).to eq(13)
+  end
+end

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -1680,7 +1680,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
 
         has_internal_id :iid,
           scope: :project,
-          init: ->(s) { s&.project&.issues&.maximum(:iid) },
+          init: ->(s, _scope) { s&.project&.issues&.maximum(:iid) },
           backfill: true,
           presence: false
       end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -774,6 +774,7 @@ ExternalPullRequest:
 - target_sha
 DesignManagement::Design:
 - id
+- iid
 - project_id
 - filename
 - relative_position

spec/models/concerns/atomic_internal_id_spec.rb

@@ -86,4 +86,20 @@ RSpec.describe AtomicInternalId do
       expect { subject }.to change { milestone.iid }.from(nil).to(iid.to_i)
     end
   end
+
+  describe '.with_project_iid_supply' do
+    let(:iid) { 100 }
+
+    it 'wraps generate and track_greatest in a concurrency-safe lock' do
+      expect_next_instance_of(InternalId::InternalIdGenerator) do |g|
+        expect(g).to receive(:with_lock).and_call_original
+        expect(g.record).to receive(:last_value).and_return(iid)
+        expect(g).to receive(:track_greatest).with(iid + 4)
+      end
+
+      ::Milestone.with_project_iid_supply(milestone.project) do |supply|
+        4.times { supply.next_value }
+      end
+    end
+  end
 end

spec/models/design_management/design_spec.rb

@@ -11,6 +11,14 @@ RSpec.describe DesignManagement::Design do
   let_it_be(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) }
   let_it_be(:deleted_design) { create(:design, :with_versions, deleted: true) }
 
+  it_behaves_like 'AtomicInternalId', validate_presence: true do
+    let(:internal_id_attribute) { :iid }
+    let(:instance) { build(:design, issue: issue) }
+    let(:scope) { :project }
+    let(:scope_attrs) { { project: instance.project } }
+    let(:usage) { :design_management_designs }
+  end
+
   it_behaves_like 'a class that supports relative positioning' do
     let_it_be(:relative_parent) { create(:issue) }
 

spec/models/internal_id_spec.rb

@@ -6,8 +6,9 @@ RSpec.describe InternalId do
   let(:project) { create(:project) }
   let(:usage) { :issues }
   let(:issue) { build(:issue, project: project) }
+  let(:id_subject) { issue }
   let(:scope) { { project: project } }
-  let(:init) { ->(s) { s.project.issues.size } }
+  let(:init) { ->(issue, scope) { issue&.project&.issues&.size || Issue.where(**scope).count } }
 
   it_behaves_like 'having unique enum values'
 
@@ -39,7 +40,7 @@ RSpec.describe InternalId do
   end
 
   describe '.generate_next' do
-    subject { described_class.generate_next(issue, scope, usage, init) }
+    subject { described_class.generate_next(id_subject, scope, usage, init) }
 
     context 'in the absence of a record' do
       it 'creates a record if not yet present' do
@@ -88,6 +89,14 @@ RSpec.describe InternalId do
 
       expect(normalized).to eq((0..seq.size - 1).to_a)
     end
+
+    context 'there are no instances to pass in' do
+      let(:id_subject) { Issue }
+
+      it 'accepts classes instead' do
+        expect(subject).to eq(1)
+      end
+    end
   end
 
   describe '.reset' do
@@ -130,7 +139,7 @@ RSpec.describe InternalId do
   describe '.track_greatest' do
     let(:value) { 9001 }
 
-    subject { described_class.track_greatest(issue, scope, usage, value, init) }
+    subject { described_class.track_greatest(id_subject, scope, usage, value, init) }
 
     context 'in the absence of a record' do
       it 'creates a record if not yet present' do
@@ -166,6 +175,14 @@ RSpec.describe InternalId do
         expect(subject).to eq 10_001
       end
     end
+
+    context 'there are no instances to pass in' do
+      let(:id_subject) { Issue }
+
+      it 'accepts classes instead' do
+        expect(subject).to eq(value)
+      end
+    end
   end
 
   describe '#increment_and_save!' do

spec/services/design_management/copy_design_collection/copy_service_spec.rb

@@ -68,6 +68,31 @@ RSpec.describe DesignManagement::CopyDesignCollection::CopyService, :clean_gitla
             include_examples 'service error', message: 'Target design collection already has designs'
           end
 
+          context 'when target project already has designs' do
+            let!(:issue_x) { create(:issue, project: target_issue.project) }
+            let!(:existing) { create(:design, issue: issue_x, project: target_issue.project) }
+
+            let(:new_designs) do
+              target_issue.reset
+              target_issue.designs.where.not(id: existing.id)
+            end
+
+            it 'sets IIDs for new designs above existing ones' do
+              subject
+
+              expect(new_designs).to all(have_attributes(iid: (be > existing.iid)))
+            end
+
+            it 'does not allow for IID collisions' do
+              subject
+              create(:design, issue: issue_x, project: target_issue.project)
+
+              design_iids = target_issue.project.designs.map(&:id)
+
+              expect(design_iids).to match_array(design_iids.uniq)
+            end
+          end
+
           include_examples 'service success'
 
           it 'creates a design repository for the target project' do
@@ -162,9 +187,7 @@ RSpec.describe DesignManagement::CopyDesignCollection::CopyService, :clean_gitla
           it 'copies the Git repository data', :aggregate_failures do
             subject
 
-            commit_shas = target_repository.commits('master', limit: 99).map(&:id)
-
-            expect(commit_shas).to include(*target_issue.design_versions.ordered.pluck(:sha))
+            expect(commits_on_master(limit: 99)).to include(*target_issue.design_versions.ordered.pluck(:sha))
           end
 
           it 'creates a master branch if none previously existed' do
@@ -212,9 +235,7 @@ RSpec.describe DesignManagement::CopyDesignCollection::CopyService, :clean_gitla
               issue_2 = create(:issue, project: target_issue.project)
               create(:design, :with_file, issue: issue_2, project: target_issue.project)
 
-              expect { subject }.not_to change {
-                expect(target_repository.commits('master', limit: 10).size).to eq(1)
-              }
+              expect { subject }.not_to change { commits_on_master }
             end
 
             it 'sets the design collection copy state' do
@@ -223,6 +244,10 @@ RSpec.describe DesignManagement::CopyDesignCollection::CopyService, :clean_gitla
               expect(target_issue.design_collection).to be_copy_error
             end
           end
+
+          def commits_on_master(limit: 10)
+            target_repository.commits('master', limit: limit).map(&:id)
+          end
         end
       end
     end

spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb

@@ -76,6 +76,26 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
       end
     end
 
+    describe 'supply of internal ids' do
+      let(:scope_value) { scope_attrs.each_value.first }
+      let(:method_name) { :"with_#{scope}_#{internal_id_attribute}_supply" }
+
+      it 'provides a persistent supply of IID values, sensitive to the current state' do
+        iid = rand(1..1000)
+        write_internal_id(iid)
+        instance.public_send(:"track_#{scope}_#{internal_id_attribute}!")
+
+        # Allocate 3 IID values
+        described_class.public_send(method_name, scope_value) do |supply|
+          3.times { supply.next_value }
+        end
+
+        current_value = described_class.public_send(method_name, scope_value, &:current_value)
+
+        expect(current_value).to eq(iid + 3)
+      end
+    end
+
     describe "#reset_scope_internal_id_attribute" do
       it 'rewinds the allocated IID' do
         expect { ensure_scope_attribute! }.not_to raise_error