diff --git a/Changelog.md b/Changelog.md index 86ca0f7da0d332f6191a4f2fc1841933386d9d3f..ad9ba7dcc8e5d30129511de777802fd4ab8d1382 100644 --- a/Changelog.md +++ b/Changelog.md @@ -41,6 +41,7 @@ If so, please delete it since it will prevent the federation from working proper * Fix order of comments across pods [#7436](https://github.com/diaspora/diaspora/pull/7436) * Prevent publisher from closing in preview mode [#7518](https://github.com/diaspora/diaspora/pull/7518) * Increase reshare counter after reshare on mobile [#7520](https://github.com/diaspora/diaspora/pull/7520) +* Reset stuck exports and handle errors [#7535](https://github.com/diaspora/diaspora/pull/7535) ## Features * Add support for mentions in comments to the backend [#6818](https://github.com/diaspora/diaspora/pull/6818) diff --git a/app/models/user.rb b/app/models/user.rb index 3d98b5eb3c070f5c46e52f1666d40f0eb82d5844..9c440402c87fd687da528bef46c4bfefb1ccb494 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -297,18 +297,22 @@ class User < ApplicationRecord mount_uploader :export, ExportedUser def queue_export - update exporting: true + update exporting: true, export: nil, exported_at: nil Workers::ExportUser.perform_async(id) end def perform_export! - export = Tempfile.new([username, '.json.gz'], encoding: 'ascii-8bit') + export = Tempfile.new([username, ".json.gz"], encoding: "ascii-8bit") export.write(compressed_export) && export.close if export.present? update exporting: false, export: export, exported_at: Time.zone.now else update exporting: false end + rescue => error + logger.error "Unexpected error while exporting user '#{username}': #{error.class}: #{error.message}\n" \ + "#{error.backtrace.first(15).join("\n")}" + update exporting: false end def compressed_export @@ -319,12 +323,16 @@ class User < ApplicationRecord mount_uploader :exported_photos_file, ExportedPhotos def queue_export_photos - update exporting_photos: true + update exporting_photos: true, exported_photos_file: nil, exported_photos_at: nil Workers::ExportPhotos.perform_async(id) end def perform_export_photos! PhotoExporter.new(self).perform + rescue => error + logger.error "Unexpected error while exporting photos for '#{username}': #{error.class}: #{error.message}\n" \ + "#{error.backtrace.first(15).join("\n")}" + update exporting_photos: false end ######### Mailer ####################### diff --git a/db/migrate/20170813222333_reset_export_states.rb b/db/migrate/20170813222333_reset_export_states.rb new file mode 100644 index 0000000000000000000000000000000000000000..12a7e5789e87c93c4cbfcca90a0c6871c0648909 --- /dev/null +++ b/db/migrate/20170813222333_reset_export_states.rb @@ -0,0 +1,12 @@ +class ResetExportStates < ActiveRecord::Migration[5.1] + class User < ApplicationRecord + end + + def up + # rubocop:disable Rails/SkipsModelValidations + User.where(exporting: true).update_all(exporting: false, export: nil, exported_at: nil) + User.where(exporting_photos: true) + .update_all(exporting_photos: false, exported_photos_file: nil, exported_photos_at: nil) + # rubocop:enable Rails/SkipsModelValidations + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7032dac829f847d82fa3f222dbb16f4618f327ea..c74969c0e9c6b24130ef2afeafb0abae2ac71a4d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -981,63 +981,84 @@ describe User, :type => :model do describe "queue_export" do it "queues up a job to perform the export" do - user = FactoryGirl.create :user + user = FactoryGirl.create(:user) + user.update export: Tempfile.new([user.username, ".json.gz"]), exported_at: Time.zone.now expect(Workers::ExportUser).to receive(:perform_async).with(user.id) user.queue_export expect(user.exporting).to be_truthy + expect(user.export).not_to be_present + expect(user.exported_at).to be_nil end end describe "perform_export!" do + let(:user) { FactoryGirl.create(:user, exporting: true) } + it "saves a json export to the user" do - user = FactoryGirl.create :user, exporting: true user.perform_export! expect(user.export).to be_present expect(user.exported_at).to be_present expect(user.exporting).to be_falsey - expect(user.export.filename).to match /.json/ + expect(user.export.filename).to match(/.json/) expect(ActiveSupport::Gzip.decompress(user.export.file.read)).to include user.username end it "compresses the result" do - user = FactoryGirl.create :user, exporting: true expect(ActiveSupport::Gzip).to receive :compress user.perform_export! end + + it "resets exporting to false when failing" do + expect_any_instance_of(Diaspora::Exporter).to receive(:execute).and_raise("Unexpected error!") + user.perform_export! + expect(user.exporting).to be_falsey + expect(user.export).not_to be_present + end end describe "queue_export_photos" do it "queues up a job to perform the export photos" do - user = FactoryGirl.create :user + user = FactoryGirl.create(:user) + user.update exported_photos_file: Tempfile.new([user.username, ".zip"]), exported_photos_at: Time.zone.now expect(Workers::ExportPhotos).to receive(:perform_async).with(user.id) user.queue_export_photos expect(user.exporting_photos).to be_truthy + expect(user.exported_photos_file).not_to be_present + expect(user.exported_photos_at).to be_nil end end describe "perform_export_photos!" do + let(:user) { FactoryGirl.create(:user_with_aspect, exporting: true) } + before do - @user = alice - filename = 'button.png' - image = File.join(File.dirname(__FILE__), '..', 'fixtures', filename) - @saved_image = @user.build_post(:photo, :user_file => File.open(image), :to => alice.aspects.first.id) + image = File.join(File.dirname(__FILE__), "..", "fixtures", "button.png") + @saved_image = user.build_post(:photo, user_file: File.open(image), to: user.aspects.first.id) @saved_image.save! end it "saves a zip export to the user" do - @user.perform_export_photos! - expect(@user.exported_photos_file).to be_present - expect(@user.exported_photos_at).to be_present - expect(@user.exporting_photos).to be_falsey - expect(@user.exported_photos_file.filename).to match /.zip/ - expect(Zip::File.open(@user.exported_photos_file.path).entries.count).to eq(1) + user.perform_export_photos! + expect(user.exported_photos_file).to be_present + expect(user.exported_photos_at).to be_present + expect(user.exporting_photos).to be_falsey + expect(user.exported_photos_file.filename).to match(/.zip/) + expect(Zip::File.open(user.exported_photos_file.path).entries.count).to eq(1) end it "does not add empty entries when photo not found" do - File.unlink @user.photos.first.unprocessed_image.path - @user.perform_export_photos! - expect(@user.exported_photos_file.filename).to match /.zip/ - expect(Zip::File.open(@user.exported_photos_file.path).entries.count).to eq(0) + File.unlink user.photos.first.unprocessed_image.path + user.perform_export_photos! + expect(user.exporting_photos).to be_falsey + expect(user.exported_photos_file.filename).to match(/.zip/) + expect(Zip::File.open(user.exported_photos_file.path).entries.count).to eq(0) + end + + it "resets exporting_photos to false when failing" do + expect_any_instance_of(PhotoExporter).to receive(:perform).and_raise("Unexpected error!") + user.perform_export_photos! + expect(user.exporting_photos).to be_falsey + expect(user.exported_photos_file).not_to be_present end end