Merge pull request #7535 from SuperTux88/unstuck-export

Reset stuck exports and handle errors

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)

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 #######################

db/migrate/20170813222333_reset_export_states.rb

+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

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