Merge branch 'georgekoltsov/add-group-import-state' into 'master'

Add GroupImportState to group imports

See merge request gitlab-org/gitlab!29588

app/models/group.rb

@@ -61,6 +61,8 @@ class Group < Namespace
 
   has_many :import_failures, inverse_of: :group
 
+  has_one :import_state, class_name: 'GroupImportState', inverse_of: :group
+
   has_many :group_deploy_tokens
   has_many :deploy_tokens, through: :group_deploy_tokens
 

app/models/group_import_state.rb

+# frozen_string_literal: true
+
+class GroupImportState < ApplicationRecord
+  self.primary_key = :group_id
+
+  belongs_to :group, inverse_of: :import_state
+
+  validates :group, :status, :jid, presence: true
+
+  state_machine :status, initial: :created do
+    state :created, value: 0
+    state :started, value: 1
+    state :finished, value: 2
+    state :failed, value: -1
+
+    event :start do
+      transition created: :started
+    end
+
+    event :finish do
+      transition started: :finished
+    end
+
+    event :fail_op do
+      transition any => :failed
+    end
+
+    after_transition any => :failed do |state, transition|
+      last_error = transition.args.first
+
+      state.update_column(:last_error, last_error) if last_error
+    end
+  end
+end

app/workers/group_import_worker.rb

@@ -2,14 +2,23 @@
 
 class GroupImportWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
-  include ExceptionBacktrace
 
+  sidekiq_options retry: false
   feature_category :importers
 
   def perform(user_id, group_id)
     current_user = User.find(user_id)
     group = Group.find(group_id)
+    group_import = group.build_import_state(jid: self.jid)
+
+    group_import.start!
 
     ::Groups::ImportExport::ImportService.new(group: group, user: current_user).execute
+
+    group_import.finish!
+  rescue StandardError => e
+    group_import&.fail_op(e.message)
+
+    raise e
   end
 end

db/migrate/20200420104303_add_group_import_states_table.rb

+# frozen_string_literal: true
+
+class AddGroupImportStatesTable < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # rubocop:disable Migration/AddLimitToTextColumns
+  def up
+    with_lock_retries do
+      create_table :group_import_states, id: false do |t|
+        t.references :group, primary_key: true, foreign_key: { to_table: :namespaces, on_delete: :cascade }
+        t.timestamps_with_timezone null: false
+        t.integer :status, limit: 2, null: false, default: 0
+        t.text :jid, null: false, unique: true
+        t.text :last_error
+      end
+    end
+  end
+  # rubocop:enable Migration/AddLimitToTextColumns
+
+  def down
+    drop_table :group_import_states
+  end
+end

db/migrate/20200420104323_add_text_limit_to_group_import_states.rb

+# frozen_string_literal: true
+
+class AddTextLimitToGroupImportStates < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :group_import_states, :jid, 100
+    add_text_limit :group_import_states, :last_error, 255
+  end
+
+  def down
+    remove_text_limit :group_import_states, :jid
+    remove_text_limit :group_import_states, :last_error
+  end
+end

db/structure.sql

@@ -3078,6 +3078,26 @@ CREATE SEQUENCE public.group_group_links_id_seq
 
 ALTER SEQUENCE public.group_group_links_id_seq OWNED BY public.group_group_links.id;
 
+CREATE TABLE public.group_import_states (
+    group_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    status smallint DEFAULT 0 NOT NULL,
+    jid text NOT NULL,
+    last_error text,
+    CONSTRAINT check_87b58f6b30 CHECK ((char_length(last_error) <= 255)),
+    CONSTRAINT check_96558fff96 CHECK ((char_length(jid) <= 100))
+);
+
+CREATE SEQUENCE public.group_import_states_group_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.group_import_states_group_id_seq OWNED BY public.group_import_states.group_id;
+
 CREATE TABLE public.historical_data (
     id integer NOT NULL,
     date date NOT NULL,
@@ -7354,6 +7374,8 @@ ALTER TABLE ONLY public.group_deploy_tokens ALTER COLUMN id SET DEFAULT nextval(
 
 ALTER TABLE ONLY public.group_group_links ALTER COLUMN id SET DEFAULT nextval('public.group_group_links_id_seq'::regclass);
 
+ALTER TABLE ONLY public.group_import_states ALTER COLUMN group_id SET DEFAULT nextval('public.group_import_states_group_id_seq'::regclass);
+
 ALTER TABLE ONLY public.historical_data ALTER COLUMN id SET DEFAULT nextval('public.historical_data_id_seq'::regclass);
 
 ALTER TABLE ONLY public.identities ALTER COLUMN id SET DEFAULT nextval('public.identities_id_seq'::regclass);
@@ -8115,6 +8137,9 @@ ALTER TABLE ONLY public.group_deploy_tokens
 ALTER TABLE ONLY public.group_group_links
     ADD CONSTRAINT group_group_links_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.group_import_states
+    ADD CONSTRAINT group_import_states_pkey PRIMARY KEY (group_id);
+
 ALTER TABLE ONLY public.historical_data
     ADD CONSTRAINT historical_data_pkey PRIMARY KEY (id);
 
@@ -9493,6 +9518,8 @@ CREATE UNIQUE INDEX index_group_group_links_on_shared_group_and_shared_with_grou
 
 CREATE INDEX index_group_group_links_on_shared_with_group_id ON public.group_group_links USING btree (shared_with_group_id);
 
+CREATE INDEX index_group_import_states_on_group_id ON public.group_import_states USING btree (group_id);
+
 CREATE INDEX index_identities_on_saml_provider_id ON public.identities USING btree (saml_provider_id) WHERE (saml_provider_id IS NOT NULL);
 
 CREATE INDEX index_identities_on_user_id ON public.identities USING btree (user_id);
@@ -11440,6 +11467,9 @@ ALTER TABLE ONLY public.resource_state_events
 ALTER TABLE ONLY public.merge_request_diff_commits
     ADD CONSTRAINT fk_rails_316aaceda3 FOREIGN KEY (merge_request_diff_id) REFERENCES public.merge_request_diffs(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.group_import_states
+    ADD CONSTRAINT fk_rails_31c3e0503a FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.zoom_meetings
     ADD CONSTRAINT fk_rails_3263f29616 FOREIGN KEY (issue_id) REFERENCES public.issues(id) ON DELETE CASCADE;
 
@@ -13421,6 +13451,8 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200416120128
 20200416120354
 20200417044453
+20200420104303
+20200420104323
 20200420162730
 20200420172113
 20200420172752

spec/workers/group_import_worker_spec.rb

@@ -8,12 +8,51 @@ describe GroupImportWorker do
 
   subject { described_class.new }
 
+  before do
+    allow_next_instance_of(described_class) do |job|
+      allow(job).to receive(:jid).and_return(SecureRandom.hex(8))
+    end
+  end
+
   describe '#perform' do
     context 'when it succeeds' do
+      before do
+        expect_next_instance_of(::Groups::ImportExport::ImportService) do |service|
+          expect(service).to receive(:execute)
+        end
+      end
+
       it 'calls the ImportService' do
-        expect_any_instance_of(::Groups::ImportExport::ImportService).to receive(:execute)
+        subject.perform(user.id, group.id)
+      end
+
+      context 'import state' do
+        it 'creates group import' do
+          expect(group.import_state).to be_nil
 
           subject.perform(user.id, group.id)
+          import_state = group.reload.import_state
+
+          expect(import_state).to be_instance_of(GroupImportState)
+          expect(import_state.status_name).to eq(:finished)
+          expect(import_state.jid).not_to be_empty
+        end
+
+        it 'sets the group import status to started' do
+          expect_next_instance_of(GroupImportState) do |import|
+            expect(import).to receive(:start!).and_call_original
+          end
+
+          subject.perform(user.id, group.id)
+        end
+
+        it 'sets the group import status to finished' do
+          expect_next_instance_of(GroupImportState) do |import|
+            expect(import).to receive(:finish!).and_call_original
+          end
+
+          subject.perform(user.id, group.id)
+        end
       end
     end
 
@@ -24,6 +63,22 @@ describe GroupImportWorker do
         expect { subject.perform(non_existing_record_id, group.id) }.to raise_exception(ActiveRecord::RecordNotFound)
         expect { subject.perform(user.id, non_existing_record_id) }.to raise_exception(ActiveRecord::RecordNotFound)
       end
+
+      context 'import state' do
+        before do
+          expect_next_instance_of(::Groups::ImportExport::ImportService) do |service|
+            expect(service).to receive(:execute).and_raise(Gitlab::ImportExport::Error)
+          end
+        end
+
+        it 'sets the group import status to failed' do
+          expect_next_instance_of(GroupImportState) do |import|
+            expect(import).to receive(:fail_op).and_call_original
+          end
+
+          expect { subject.perform(user.id, group.id) }.to raise_exception(Gitlab::ImportExport::Error)
+        end
+      end
     end
   end
 end