Merge branch 'import-reduce-amount-of-project-find-sql-queries' into 'master'

Do not use `restored_project` to reduce amount of SQL queries

See merge request gitlab-org/gitlab!18005

doc/development/import_export.md

@@ -312,7 +312,7 @@ module Gitlab
     class Importer
       def execute
         if import_file && check_version! && restorers.all?(&:restore) && overwrite_project
-          project_tree.restored_project
+          project
         else
           raise Projects::ImportService::Error.new(@shared.errors.join(', '))
         end

ee/lib/ee/gitlab/import_export/importer.rb

@@ -17,7 +17,7 @@ module EE::Gitlab::ImportExport::Importer
     Gitlab::ImportExport::DesignRepoRestorer.new(
       path_to_bundle: design_repo_path,
       shared: shared,
-      project: project_tree.restored_project
+      project: project
     )
   end
 

ee/lib/ee/gitlab/import_export/project_tree_restorer.rb

@@ -9,11 +9,11 @@ module EE
 
         private
 
-        attr_accessor :project
-
         override :remove_feature_dependent_sub_relations
         def remove_feature_dependent_sub_relations(relation_item)
-          if relation_item.is_a?(Hash) && ::Feature.disabled?(:export_designs, project, default_enabled: true)
+          export_designs_disabled = ::Feature.disabled?(:export_designs, project, default_enabled: true)
+
+          if relation_item.is_a?(Hash) && export_designs_disabled
             relation_item.except!('designs', 'design_versions')
           end
         end

lib/gitlab/import_export/importer.rb

@@ -19,7 +19,7 @@ module Gitlab
 
       def execute
         if import_file && check_version! && restorers.all?(&:restore) && overwrite_project
-          project_tree.restored_project
+          project
         else
           raise Projects::ImportService::Error.new(shared.errors.to_sentence)
         end
@@ -55,32 +55,32 @@ module Gitlab
       end
 
       def avatar_restorer
-        Gitlab::ImportExport::AvatarRestorer.new(project: project_tree.restored_project, shared: shared)
+        Gitlab::ImportExport::AvatarRestorer.new(project: project, shared: shared)
       end
 
       def repo_restorer
         Gitlab::ImportExport::RepoRestorer.new(path_to_bundle: repo_path,
                                                shared: shared,
-                                               project: project_tree.restored_project)
+                                               project: project)
       end
 
       def wiki_restorer
         Gitlab::ImportExport::WikiRestorer.new(path_to_bundle: wiki_repo_path,
                                                shared: shared,
-                                               project: ProjectWiki.new(project_tree.restored_project),
+                                               project: ProjectWiki.new(project),
                                                wiki_enabled: project.wiki_enabled?)
       end
 
       def uploads_restorer
-        Gitlab::ImportExport::UploadsRestorer.new(project: project_tree.restored_project, shared: shared)
+        Gitlab::ImportExport::UploadsRestorer.new(project: project, shared: shared)
       end
 
       def lfs_restorer
-        Gitlab::ImportExport::LfsRestorer.new(project: project_tree.restored_project, shared: shared)
+        Gitlab::ImportExport::LfsRestorer.new(project: project, shared: shared)
       end
 
       def statistics_restorer
-        Gitlab::ImportExport::StatisticsRestorer.new(project: project_tree.restored_project, shared: shared)
+        Gitlab::ImportExport::StatisticsRestorer.new(project: project, shared: shared)
       end
 
       def path_with_namespace
@@ -105,8 +105,6 @@ module Gitlab
       end
 
       def overwrite_project
-        project = project_tree.restored_project
-
         return unless can?(current_user, :admin_namespace, project.namespace)
 
         if overwrite_project?

lib/gitlab/import_export/project_tree_restorer.rb

@@ -6,19 +6,21 @@ module Gitlab
       # Relations which cannot be saved at project level (and have a group assigned)
       GROUP_MODELS = [GroupLabel, Milestone].freeze
 
+      attr_reader :user
+      attr_reader :shared
+      attr_reader :project
+
       def initialize(user:, shared:, project:)
         @path = File.join(shared.export_path, 'project.json')
         @user = user
         @shared = shared
         @project = project
-        @project_id = project.id
         @saved = true
       end
 
       def restore
         begin
-          json = IO.read(@path)
-          @tree_hash = ActiveSupport::JSON.decode(json)
+          @tree_hash = read_tree_hash
         rescue => e
           Rails.logger.error("Import/Export error: #{e.message}") # rubocop:disable Gitlab/RailsLogger
           raise Gitlab::ImportExport::Error.new('Incorrect JSON format')
@@ -30,26 +32,31 @@ module Gitlab
 
         ActiveRecord::Base.uncached do
           ActiveRecord::Base.no_touching do
+            update_project_params
             create_relations
           end
         end
+
+        # ensure that we have latest version of the restore
+        @project.reload # rubocop:disable Cop/ActiveRecordAssociationReload
+
+        true
       rescue => e
         @shared.error(e)
         false
       end
 
-      def restored_project
-        return @project unless @tree_hash
+      private
 
-        @restored_project ||= restore_project
+      def read_tree_hash
+        json = IO.read(@path)
+        ActiveSupport::JSON.decode(json)
       end
 
-      private
-
       def members_mapper
         @members_mapper ||= Gitlab::ImportExport::MembersMapper.new(exported_members: @project_members,
                                                                     user: @user,
-                                                                    project: restored_project)
+                                                                    project: @project)
       end
 
       # A Hash of the imported merge request ID -> imported ID.
@@ -83,12 +90,11 @@ module Gitlab
 
         remove_group_models(relation_hash) if relation_hash.is_a?(Array)
 
-        @saved = false unless restored_project.append_or_update_attribute(relation_key, relation_hash)
+        @saved = false unless @project.append_or_update_attribute(relation_key, relation_hash)
 
         save_id_mappings(relation_key, relation_hash_batch, relation_hash)
 
-        # Restore the project again, extra query that skips holding the AR objects in memory
-        @restored_project = Project.find(@project_id)
+        @project.reset
       end
 
       # Older, serialized CI pipeline exports may only have a
@@ -127,12 +133,10 @@ module Gitlab
         reader.attributes_finder.find_relations_tree(:project)
       end
 
-      def restore_project
+      def update_project_params
         Gitlab::Timeless.timeless(@project) do
           @project.update(project_params)
         end
-
-        @project
       end
 
       def project_params
@@ -245,7 +249,7 @@ module Gitlab
             members_mapper: members_mapper,
             merge_requests_mapping: merge_requests_mapping,
             user: @user,
-            project: @restored_project,
+            project: @project,
             excluded_keys: excluded_keys_for_relation(relation_key))
         end.compact
 

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -520,20 +520,21 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
     end
   end
 
-  describe '#restored_project' do
+  context 'Minimal JSON' do
     let(:project) { create(:project) }
     let(:tree_hash) { { 'visibility_level' => visibility } }
     let(:restorer) { described_class.new(user: nil, shared: shared, project: project) }
 
     before do
-      restorer.instance_variable_set(:@tree_hash, tree_hash)
+      expect(restorer).to receive(:read_tree_hash) { tree_hash }
     end
 
     context 'no group visibility' do
       let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
 
       it 'uses the project visibility' do
-        expect(restorer.restored_project.visibility_level).to eq(visibility)
+        expect(restorer.restore).to eq(true)
+        expect(restorer.project.visibility_level).to eq(visibility)
       end
     end
 
@@ -544,7 +545,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         it 'uses private visibility' do
           stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
 
-          expect(restorer.restored_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+          expect(restorer.restore).to eq(true)
+          expect(restorer.project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
         end
       end
     end
@@ -561,7 +563,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
 
         it 'uses the group visibility' do
-          expect(restorer.restored_project.visibility_level).to eq(group_visibility)
+          expect(restorer.restore).to eq(true)
+          expect(restorer.project.visibility_level).to eq(group_visibility)
         end
       end
 
@@ -570,7 +573,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
 
         it 'uses the project visibility' do
-          expect(restorer.restored_project.visibility_level).to eq(visibility)
+          expect(restorer.restore).to eq(true)
+          expect(restorer.project.visibility_level).to eq(visibility)
         end
       end
 
@@ -579,14 +583,16 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
 
         it 'uses the group visibility' do
-          expect(restorer.restored_project.visibility_level).to eq(group_visibility)
+          expect(restorer.restore).to eq(true)
+          expect(restorer.project.visibility_level).to eq(group_visibility)
         end
 
         context 'with restricted internal visibility' do
           it 'sets private visibility' do
             stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
 
-            expect(restorer.restored_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+            expect(restorer.restore).to eq(true)
+            expect(restorer.project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
           end
         end
       end