Migrate remote_photo_path and cleanup old photo uploads

If the migration contains a new remote_photo_path migrate all photos of
the old person to this path. If the person was local before, cleanup old
uploaded files of the photos.

closes #8314

Changelog.md

@@ -22,6 +22,7 @@
 * Add username to password-reset mail [#8037](https://github.com/diaspora/diaspora/pull/8037)
 * Resend account migration and deletion for closed recipients [#8309](https://github.com/diaspora/diaspora/pull/8309)
 * Add sharing status to hovercards [#8317](https://github.com/diaspora/diaspora/pull/8317)
+* Migrate photo URLs and cleanup old uploaded photos [#8314](https://github.com/diaspora/diaspora/pull/8314)
 
 # 0.7.15.0
 

app/models/account_migration.rb

@@ -90,6 +90,10 @@ class AccountMigration < ApplicationRecord
     old_user && new_user
   end
 
+  def includes_photo_migration?
+    remote_photo_path.present?
+  end
+
   # We need to resend contacts of users of our pod for the remote new person so that the remote pod received this
   # contact information from the authoritative source.
   def dispatch_contacts
@@ -122,6 +126,7 @@ class AccountMigration < ApplicationRecord
   end
 
   def update_all_references
+    update_remote_photo_path if remotely_initiated? && includes_photo_migration?
     update_person_references
     update_user_references if user_changed_id_locally?
   end
@@ -200,6 +205,20 @@ class AccountMigration < ApplicationRecord
       .destroy_all
   end
 
+  def update_remote_photo_path
+    Photo.where(author: old_person)
+         .update_all(remote_photo_path: remote_photo_path) # rubocop:disable Rails/SkipsModelValidations
+    return unless user_left_our_pod?
+
+    Photo.where(author: old_person).find_in_batches do |batch|
+      batch.each do |photo|
+        photo.processed_image = nil
+        photo.unprocessed_image = nil
+        logger.warn "Error cleaning up photo #{photo.id}" unless photo.save
+      end
+    end
+  end
+
   def update_person_references
     logger.debug "Updating references from person id=#{old_person.id} to person id=#{new_person.id}"
     eliminate_person_duplicates

db/migrate/20211027230348_add_remote_photo_path_to_account_migration.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddRemotePhotoPathToAccountMigration < ActiveRecord::Migration[5.2]
+  def change
+    add_column :account_migrations, :remote_photo_path, :text
+  end
+end

lib/diaspora/federation/receive.rb

@@ -31,7 +31,11 @@ module Diaspora
         profile = profile(entity.profile)
         return if AccountMigration.exists?(old_person: old_person, new_person: profile.person)
 
-        AccountMigration.create!(old_person: old_person, new_person: profile.person).tap do |migration|
+        AccountMigration.create!(
+          old_person:        old_person,
+          new_person:        profile.person,
+          remote_photo_path: entity.remote_photo_path
+        ).tap do |migration|
           migration.signature = entity.signature if old_person.local?
           migration.save!
         end

spec/integration/account_migration_spec.rb

@@ -97,9 +97,29 @@ shared_examples_for "migration scenarios initiated locally" do
       []
     end
 
-    inlined_jobs do
+    inlined_jobs { run_migration }
-      run_migration
   end
+
+  it "does not change the remote paths" do
+    photo = FactoryGirl.create(:photo, author: old_person)
+    remote_photo_path = photo.remote_photo_path
+
+    run_migration
+
+    expect(photo.reload.remote_photo_path).to eq(remote_photo_path)
+  end
+end
+
+shared_examples_for "remote photo migration" do
+  it "changes the remote paths of photos of the old person" do
+    old_photo = FactoryGirl.create(:photo, author: old_person)
+    new_photo = FactoryGirl.create(:photo, author: new_person)
+    new_remote_photo_path = old_photo.remote_photo_path
+
+    run_migration
+
+    expect(old_photo.reload.remote_photo_path).to eq("https://diaspora.example.tld/uploads/images/")
+    expect(new_photo.reload.remote_photo_path).to eq(new_remote_photo_path)
   end
 end
 
@@ -125,6 +145,8 @@ describe "account migration" do
       include_examples "every migration scenario"
 
       include_examples "migration scenarios initiated remotely"
+
+      include_examples "remote photo migration"
     end
 
     # this is the case when we're a pod, which was left by a person in favor of remote one
@@ -136,6 +158,8 @@ describe "account migration" do
 
       include_examples "migration scenarios initiated remotely"
 
+      include_examples "remote photo migration"
+
       it_behaves_like "migration scenarios with local old user"
 
       it_behaves_like "deletes all of the user data" do
@@ -167,7 +191,8 @@ describe "account migration" do
         AccountMigration.create!(
           old_person:        old_user.person,
           new_person:        new_user.person,
-          old_private_key: old_user.serialized_private_key
+          old_private_key:   old_user.serialized_private_key,
+          remote_photo_path: "https://diaspora.example.tld/uploads/images/"
         ).perform!
       end
 
@@ -184,7 +209,9 @@ describe "account migration" do
       include_context "with local new user"
 
       def run_migration
-        AccountMigration.create!(old_person: old_user.person, new_person: new_user.person).perform!
+        AccountMigration.create!(old_person:        old_user.person,
+                                 new_person:        new_user.person,
+                                 remote_photo_path: "https://diaspora.example.tld/uploads/images/").perform!
       end
 
       include_examples "every migration scenario"

spec/integration/federation/receive_federation_messages_spec.rb

@@ -55,7 +55,9 @@ describe "Receive federation messages feature" do
         it "receives account migration correctly" do
           run_migration
           expect(AccountMigration.where(old_person: sender.person, new_person: new_user.person)).to exist
-          expect(AccountMigration.find_by(old_person: sender.person, new_person: new_user.person)).to be_performed
+          account_migration = AccountMigration.find_by(old_person: sender.person, new_person: new_user.person)
+          expect(account_migration).to be_performed
+          expect(account_migration.remote_photo_path).to eq("https://diaspora.example.tld/uploads/images/")
         end
 
         it "doesn't run the same migration for the second time" do

spec/models/account_migration_spec.rb

@@ -16,7 +16,9 @@ describe AccountMigration, type: :model do
   let(:old_person) { FactoryGirl.create(:person) }
   let(:new_person) { FactoryGirl.create(:person) }
   let(:account_migration) {
-    AccountMigration.create!(old_person: old_person, new_person: new_person)
+    AccountMigration.create!(old_person:        old_person,
+                             new_person:        new_person,
+                             remote_photo_path: "https://diaspora.example.tld/uploads/images/")
   }
 
   describe "receive" do
@@ -125,6 +127,34 @@ describe AccountMigration, type: :model do
         expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(contact.user, contact)
         account_migration.perform!
       end
+
+      it "cleans up old local photos" do
+        photo = FactoryGirl.create(:photo, author: old_person)
+        photo.processed_image.store!(photo.unprocessed_image)
+        photo.save!
+
+        account_migration.perform!
+
+        updated_photo = photo.reload
+        expect(updated_photo.remote_photo_path).to eq("https://diaspora.example.tld/uploads/images/")
+        expect(updated_photo.processed_image.path).to be_nil
+        expect(updated_photo.unprocessed_image.path).to be_nil
+      end
+
+      it "does nothing if migration doesn't contain a new remote_photo_path" do
+        photo = FactoryGirl.create(:photo, author: old_person)
+        photo.processed_image.store!(photo.unprocessed_image)
+        photo.save!
+
+        remote_photo_path = photo.remote_photo_path
+
+        AccountMigration.create!(old_person: old_person, new_person: new_person).perform!
+
+        updated_photo = photo.reload
+        expect(updated_photo.remote_photo_path).to eq(remote_photo_path)
+        expect(updated_photo.processed_image.path).not_to be_nil
+        expect(updated_photo.unprocessed_image.path).not_to be_nil
+      end
     end
 
     context "with local new and remote old users" do