Merge branch '1756-set-iid-via-api' into 'master'

Resolve "Allow issue's Internal ID (`iid`) to be set when creating via the API"

Closes #1756

See merge request gitlab-org/gitlab-ce!20626

2 files changed, 46 insertions, 7 deletions

diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb
index 4fef615e6e3..5e39676b24b 100644
--- a/app/models/concerns/atomic_internal_id.rb
+++ b/app/models/concerns/atomic_internal_id.rb
@@ -35,16 +35,21 @@ module AtomicInternalId
 
       define_method("ensure_#{scope}_#{column}!") do
         scope_value = association(scope).reader
+        value = read_attribute(column)
 
-        if read_attribute(column).blank? && scope_value
-          scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value }
-          usage = self.class.table_name.to_sym
+        return value unless scope_value
 
-          new_iid = InternalId.generate_next(self, scope_attrs, usage, init)
-          write_attribute(column, new_iid)
+        scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value }
+        usage = self.class.table_name.to_sym
+
+        if value.present?
+          InternalId.track_greatest(self, scope_attrs, usage, value, init)
+        else
+          value = InternalId.generate_next(self, scope_attrs, usage, init)
+          write_attribute(column, value)
         end
 
-        read_attribute(column)
+        value
       end
     end
   end
diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb
index f50f28deffe..e5d0f94073c 100644
--- a/app/models/internal_id.rb
+++ b/app/models/internal_id.rb
@@ -1,6 +1,9 @@
 # An InternalId is a strictly monotone sequence of integers
 # generated for a given scope and usage.
 #
+# The monotone sequence may be broken if an ID is explicitly provided
+# to `.track_greatest_and_save!` or `#track_greatest`.
+#
 # For example, issues use their project to scope internal ids:
 # In that sense, scope is "project" and usage is "issues".
 # Generated internal ids for an issue are unique per project.
@@ -25,13 +28,34 @@ class InternalId < ActiveRecord::Base
   # 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 increment_and_save!
+    update_and_save { self.last_value = (last_value || 0) + 1 }
+  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!
-    self.last_value = (last_value || 0) + 1
+    yield
     save!
     last_value
   end
 
   class << self
+    def track_greatest(subject, scope, usage, new_value, init)
+      return new_value unless available?
+
+      InternalIdGenerator.new(subject, scope, usage, init).track_greatest(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
@@ -94,6 +118,16 @@ class InternalId < ActiveRecord::Base
       end
     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
+        (lookup || create_record).track_greatest_and_save!(new_value)
+      end
+    end
+
     private
 
     # Retrieve InternalId record for (project, usage) combination, if it exists