Check existence of export object and report status to user

Now that the project export status in the database may be updated before
the upload has transferred to object storage, we should inform the user
that the export is still not available. We do this when the user
attempts to request the actual download since the existence check
requires a HEAD request to ensure the file is present.

app/controllers/groups_controller.rb

@@ -181,8 +181,13 @@ class GroupsController < Groups::ApplicationController
   end
 
   def download_export
-    if @group.export_file_exists?
+    db_status, object_status = @group.export_file_exists_and_stored?
+
+    if db_status && object_status
       send_upload(@group.export_file, attachment: @group.export_file.filename)
+    elsif db_status && !object_status
+      redirect_to edit_group_path(@group),
+        alert: _('The file containing the export is not available yet; it may still be transferring. Please try again later.')
     else
       redirect_to edit_group_path(@group),
         alert: _('Group export link has expired. Please generate a new export from your group settings.')

app/controllers/projects_controller.rb

@@ -225,8 +225,15 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def download_export
-    if @project.export_file_exists?
+    db_status, object_status = @project.export_file_exists_and_stored?
+
+    if db_status && object_status
       send_upload(@project.export_file, attachment: @project.export_file.filename)
+    elsif db_status && !object_status
+      redirect_to(
+        edit_project_path(@project, anchor: 'js-export-project'),
+        alert: _("The file containing the export is not available yet; it may still be transferring. Please try again later.")
+      )
     else
       redirect_to(
         edit_project_path(@project, anchor: 'js-export-project'),

app/models/group.rb

@@ -652,6 +652,15 @@ class Group < Namespace
     import_export_upload&.export_file
   end
 
+  # This method returns two values: whether a record of the export file exists in the
+  # database and whether the object is actually stored. The latter requires a HEAD request
+  # if object storage is used.
+  def export_file_exists_and_stored?
+    file = export_file&.file
+
+    [file.present?, !!file&.exists?]
+  end
+
   def adjourned_deletion?
     false
   end

app/models/import_export_upload.rb

@@ -14,8 +14,10 @@ class ImportExportUpload < ApplicationRecord
   # This causes CarrierWave v1 and v3 (but not v2) to upload the file to
   # object storage *after* the database entry has been committed to the
   # database. This avoids idling in a transaction.
-  skip_callback :save, :after, :store_export_file!
-  set_callback :commit, :after, :store_export_file!
+  if Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_STORE_EXPORT_FILE_AFTER_COMMIT', true))
+    skip_callback :save, :after, :store_export_file!
+    set_callback :commit, :after, :store_export_file!
+  end
 
   scope :updated_before, ->(date) { where('updated_at < ?', date) }
   scope :with_export_file, -> { where.not(export_file: nil) }

app/models/project.rb

@@ -2000,6 +2000,15 @@ class Project < ApplicationRecord
     import_export_upload&.export_file
   end
 
+  # This method returns two values: whether a record of the export file exists in the
+  # database and whether the object is actually stored. The latter requires a HEAD request
+  # if object storage is used.
+  def export_file_exists_and_stored?
+    file = export_file&.file
+
+    [file.present?, !!file&.exists?]
+  end
+
   def full_path_slug
     Gitlab::Utils.slugify(full_path.to_s)
   end

lib/api/group_export.rb

@@ -22,8 +22,12 @@ module API
       get ':id/export/download' do
         check_rate_limit! :group_download_export, [current_user, user_group]
 
-        if user_group.export_file_exists?
+        db_status, object_status = user_group.export_file_exists_and_stored?
+
+        if db_status && object_status
           present_carrierwave_file!(user_group.export_file)
+        elsif db_status && !object_status
+          render_api_error!('The group export file is not available yet', 404)
         else
           render_api_error!('404 Not found or has expired', 404)
         end

lib/api/project_export.rb

@@ -29,8 +29,12 @@ module API
       get ':id/export/download' do
         check_rate_limit! :project_download_export, [current_user, user_project]
 
-        if user_project.export_file_exists?
+        db_status, object_status = user_project.export_file_exists_and_stored?
+
+        if db_status && object_status
           present_carrierwave_file!(user_project.export_file)
+        elsif db_status && !object_status
+          render_api_error!('The project export file is not available yet', 404)
         else
           render_api_error!('404 Not found or has expired', 404)
         end

locale/gitlab.pot

@@ -32320,6 +32320,9 @@ msgstr ""
 msgid "The errors we encountered were:"
 msgstr ""
 
+msgid "The file containing the export is not available yet; it may still be transferring. Please try again later."
+msgstr ""
+
 msgid "The file has been successfully created."
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -1026,14 +1026,13 @@ RSpec.describe GroupsController, factory_default: :keep do
 
   describe 'GET #download_export' do
     let(:admin) { create(:admin) }
+    let(:export_file) { fixture_file_upload('spec/fixtures/group_export.tar.gz') }
 
     before do
       enable_admin_mode!(admin)
     end
 
     context 'when there is a file available to download' do
-      let(:export_file) { fixture_file_upload('spec/fixtures/group_export.tar.gz') }
-
       before do
         sign_in(admin)
         create(:import_export_upload, group: group, export_file: export_file)
@@ -1046,6 +1045,22 @@ RSpec.describe GroupsController, factory_default: :keep do
       end
     end
 
+    context 'when the file is no longer present on disk' do
+      before do
+        sign_in(admin)
+
+        create(:import_export_upload, group: group, export_file: export_file)
+        group.export_file.file.delete
+      end
+
+      it 'returns not found' do
+        get :download_export, params: { id: group.to_param }
+
+        expect(flash[:alert]).to include('file containing the export is not available yet')
+        expect(response).to redirect_to(edit_group_path(group))
+      end
+    end
+
     context 'when there is no file available to download' do
       before do
         sign_in(admin)

spec/controllers/projects_controller_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe ProjectsController do
   include ProjectForksHelper
   using RSpec::Parameterized::TableSyntax
 
-  let_it_be(:project, reload: true) { create(:project, service_desk_enabled: false) }
+  let_it_be(:project, reload: true) { create(:project, :with_export, service_desk_enabled: false) }
   let_it_be(:public_project) { create(:project, :public) }
   let_it_be(:user) { create(:user) }
 
@@ -1349,7 +1349,7 @@ RSpec.describe ProjectsController do
       end
     end
 
-    describe '#download_export' do
+    describe '#download_export', :clean_gitlab_redis_cache do
       let(:action) { :download_export }
 
       context 'object storage enabled' do
@@ -1361,6 +1361,17 @@ RSpec.describe ProjectsController do
           end
         end
 
+        context 'when project export file is absent' do
+          it 'alerts the user and returns 302' do
+            project.export_file.file.delete
+
+            get action, params: { namespace_id: project.namespace, id: project }
+
+            expect(flash[:alert]).to include('file containing the export is not available yet')
+            expect(response).to have_gitlab_http_status(:found)
+          end
+        end
+
         context 'when project export is disabled' do
           before do
             stub_application_setting(project_export_enabled?: false)

spec/factories/groups.rb

@@ -58,6 +58,13 @@ FactoryBot.define do
       shared_runners_enabled { false }
     end
 
+    trait :with_export do
+      after(:create) do |group, _evaluator|
+        export_file = fixture_file_upload('spec/fixtures/group_export.tar.gz')
+        create(:import_export_upload, group: group, export_file: export_file)
+      end
+    end
+
     trait :allow_descendants_override_disabled_shared_runners do
       allow_descendants_override_disabled_shared_runners { true }
     end

spec/models/group_spec.rb

@@ -2613,4 +2613,28 @@ RSpec.describe Group do
       expect(group.activity_path).to eq(expected_path)
     end
   end
+
+  describe '#export_file_exists_and_stored?' do
+    it 'returns false and false' do
+      expect(group.export_file_exists_and_stored?).to eq([false, false])
+    end
+
+    context 'with export' do
+      let(:group) { create(:group, :with_export) }
+
+      it 'returns true and true' do
+        expect(group.export_file_exists_and_stored?).to eq([true, true])
+      end
+
+      context 'when object file does not exist' do
+        before do
+          group.export_file.file.delete
+        end
+
+        it 'returns true and false' do
+          expect(group.reload.export_file_exists_and_stored?).to eq([true, false])
+        end
+      end
+    end
+  end
 end

spec/models/project_spec.rb

@@ -4346,6 +4346,32 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#export_file_exists_and_stored?' do
+    let(:project) { create(:project) }
+
+    it 'returns false and false' do
+      expect(project.export_file_exists_and_stored?).to eq([false, false])
+    end
+
+    context 'with export' do
+      let(:project) { create(:project, :with_export) }
+
+      it 'returns true and true' do
+        expect(project.export_file_exists_and_stored?).to eq([true, true])
+      end
+
+      context 'when object file does not exist' do
+        before do
+          project.export_file.file.delete
+        end
+
+        it 'returns true and false' do
+          expect(project.export_file_exists_and_stored?).to eq([true, false])
+        end
+      end
+    end
+  end
+
   describe '#forks_count' do
     it 'returns the number of forks' do
       project = build(:project)

spec/requests/api/group_export_spec.rb

@@ -64,6 +64,23 @@ RSpec.describe API::GroupExport do
             expect(response).to have_gitlab_http_status(:not_found)
           end
         end
+
+        context 'when object is not present' do
+          let(:other_group) { create(:group, :with_export) }
+          let(:other_download_path) { "/groups/#{other_group.id}/export/download" }
+
+          before do
+            other_group.add_owner(user)
+            other_group.export_file.file.delete
+          end
+
+          it 'returns 404' do
+            get api(other_download_path, user)
+
+            expect(response).to have_gitlab_http_status(:not_found)
+            expect(json_response['message']).to eq('The group export file is not available yet')
+          end
+        end
       end
 
       context 'when export file does not exist' do

spec/requests/api/project_export_spec.rb

@@ -196,6 +196,19 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do
         end
       end
 
+      context 'when export object is not present' do
+        before do
+          project_after_export.export_file.file.delete
+        end
+
+        it 'returns 404' do
+          get api(download_path_export_action, user)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(json_response['message']).to eq('The project export file is not available yet')
+        end
+      end
+
       context 'when upload complete' do
         before do
           project_after_export.remove_exports