Rewind IID on Ci::Pipelines

If no pipeline is created we currently have IID gap.
This is due to fact that we generate IID not on save,
but rather ahead of time. This results, us,
losing IIDs.

3 files changed, 130 insertions, 6 deletions

diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb
index ff2382838ae..0ed4e146caa 100644
--- a/spec/models/internal_id_spec.rb
+++ b/spec/models/internal_id_spec.rb
@@ -107,6 +107,57 @@ describe InternalId do
     end
   end
 
+  describe '.reset' do
+    subject { described_class.reset(issue, scope, usage, value) }
+
+    context 'in the absence of a record' do
+      let(:value) { 2 }
+
+      it 'does not revert back the value' do
+        expect { subject }.not_to change { described_class.count }
+        expect(subject).to be_falsey
+      end
+    end
+
+    context 'when valid iid is used to reset' do
+      let!(:value) { generate_next }
+
+      context 'and iid is a latest one' do
+        it 'does rewind and next generated value is the same' do
+          expect(subject).to be_truthy
+          expect(generate_next).to eq(value)
+        end
+      end
+
+      context 'and iid is not a latest one' do
+        it 'does not rewind' do
+          generate_next
+
+          expect(subject).to be_falsey
+          expect(generate_next).to be > value
+        end
+      end
+
+      def generate_next
+        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).twice.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
     let(:value) { 9001 }
     subject { described_class.track_greatest(issue, scope, usage, value, init) }
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 6f8a76e9d56..8a80652b3d8 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -25,7 +25,8 @@ describe Ci::CreatePipelineService do
       merge_request: nil,
       push_options: nil,
       source_sha: nil,
-      target_sha: nil)
+      target_sha: nil,
+      save_on_errors: true)
       params = { ref: ref,
                  before: '00000000',
                  after: after,
@@ -36,7 +37,7 @@ describe Ci::CreatePipelineService do
                  target_sha: target_sha }
 
       described_class.new(project, user, params).execute(
-        source, trigger_request: trigger_request, merge_request: merge_request)
+        source, save_on_errors: save_on_errors, trigger_request: trigger_request, merge_request: merge_request)
     end
     # rubocop:enable Metrics/ParameterLists
 
@@ -57,6 +58,7 @@ describe Ci::CreatePipelineService do
         expect(pipeline).to eq(project.ci_pipelines.last)
         expect(pipeline).to have_attributes(user: user)
         expect(pipeline).to have_attributes(status: 'pending')
+        expect(pipeline.iid).not_to be_nil
         expect(pipeline.repository_source?).to be true
         expect(pipeline.builds.first).to be_kind_of(Ci::Build)
       end
@@ -446,6 +448,43 @@ describe Ci::CreatePipelineService do
         expect(Ci::Build.all).to be_empty
         expect(Ci::Pipeline.count).to eq(0)
       end
+
+      describe '#iid' do
+        let(:internal_id) do
+          InternalId.find_by(project_id: project.id, usage: :ci_pipelines)
+        end
+
+        before do
+          expect_any_instance_of(Ci::Pipeline).to receive(:ensure_project_iid!)
+            .and_call_original
+        end
+
+        context 'when ci_pipeline_rewind_iid is enabled' do
+          before do
+            stub_feature_flags(ci_pipeline_rewind_iid: true)
+          end
+
+          it 'rewinds iid' do
+            result = execute_service
+
+            expect(result).not_to be_persisted
+            expect(internal_id.last_value).to eq(0)
+          end
+        end
+
+        context 'when ci_pipeline_rewind_iid is disabled' do
+          before do
+            stub_feature_flags(ci_pipeline_rewind_iid: false)
+          end
+
+          it 'does not rewind iid' do
+            result = execute_service
+
+            expect(result).not_to be_persisted
+            expect(internal_id.last_value).to eq(1)
+          end
+        end
+      end
     end
 
     context 'with manual actions' do
diff --git a/spec/support/shared_examples/models/atomic_internal_id_spec.rb b/spec/support/shared_examples/models/atomic_internal_id_spec.rb
index c659be8f13a..a248f60d23e 100644
--- a/spec/support/shared_examples/models/atomic_internal_id_spec.rb
+++ b/spec/support/shared_examples/models/atomic_internal_id_spec.rb
@@ -52,20 +52,20 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true|
 
         expect(InternalId).to receive(:generate_next).with(instance, scope_attrs, usage, any_args).and_return(iid)
         subject
-        expect(instance.public_send(internal_id_attribute)).to eq(iid)
+        expect(read_internal_id).to eq(iid)
       end
 
       it 'does not overwrite an existing internal id' do
-        instance.public_send("#{internal_id_attribute}=", 4711)
+        write_internal_id(4711)
 
-        expect { subject }.not_to change { instance.public_send(internal_id_attribute) }
+        expect { subject }.not_to change { read_internal_id }
       end
 
       context 'when the instance has an internal ID set' do
         let(:internal_id) { 9001 }
 
         it 'calls InternalId.update_last_value and sets the `last_value` to that of the instance' do
-          instance.send("#{internal_id_attribute}=", internal_id)
+          write_internal_id(internal_id)
 
           expect(InternalId)
             .to receive(:track_greatest)
@@ -75,5 +75,39 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true|
         end
       end
     end
+
+    describe "#reset_scope_internal_id_attribute" do
+      it 'rewinds the allocated IID' do
+        expect { ensure_scope_attribute! }.not_to raise_error
+        expect(read_internal_id).not_to be_nil
+
+        expect(reset_scope_attribute).to be_nil
+        expect(read_internal_id).to be_nil
+      end
+
+      it 'allocates the same IID' do
+        internal_id = ensure_scope_attribute!
+        reset_scope_attribute
+        expect(read_internal_id).to be_nil
+
+        expect(ensure_scope_attribute!).to eq(internal_id)
+      end
+    end
+
+    def ensure_scope_attribute!
+      instance.public_send(:"ensure_#{scope}_#{internal_id_attribute}!")
+    end
+
+    def reset_scope_attribute
+      instance.public_send(:"reset_#{scope}_#{internal_id_attribute}")
+    end
+
+    def read_internal_id
+      instance.public_send(internal_id_attribute)
+    end
+
+    def write_internal_id(value)
+      instance.public_send(:"#{internal_id_attribute}=", value)
+    end
   end
 end