From a36d22d72b1d38124ae8400194059b0cf5bbe0d7 Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Mon, 16 Oct 2017 23:00:21 +0200
Subject: [PATCH] Handle duplicate account migrations
closes #7641
---
Changelog.md | 1 +
lib/diaspora/federation/receive.rb | 11 ++++++----
.../receive_federation_messages_spec.rb | 9 ++++----
spec/lib/diaspora/federation/receive_spec.rb | 21 +++++++++++++++++++
4 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/Changelog.md b/Changelog.md
index 5787dc9ca6..b6ce3f0666 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -38,6 +38,7 @@ after you've upgraded.
* Cleanup relayables where the signature is missing [#7637](https://github.com/diaspora/diaspora/pull/7637)
* Avoid page to jump to top after a post deletion [#7638](https://github.com/diaspora/diaspora/pull/7638)
* Handle duplicate account deletions [#7639](https://github.com/diaspora/diaspora/pull/7639)
+* Handle duplicate account migrations [#7641](https://github.com/diaspora/diaspora/pull/7641)
## Features
* Ask for confirmation when leaving a submittable comment field [#7530](https://github.com/diaspora/diaspora/pull/7530)
diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb
index 13e235de11..d4dcea34dd 100644
--- a/lib/diaspora/federation/receive.rb
+++ b/lib/diaspora/federation/receive.rb
@@ -18,11 +18,14 @@ module Diaspora
end
def self.account_migration(entity)
+ old_person = author_of(entity)
profile = profile(entity.profile)
- AccountMigration.create!(
- old_person: Person.by_account_identifier(entity.author),
- new_person: profile.person
- )
+ return if AccountMigration.where(old_person: old_person, new_person: profile.person).exists?
+ AccountMigration.create!(old_person: old_person, new_person: profile.person)
+ rescue => e # rubocop:disable Lint/RescueWithoutErrorClass
+ raise e unless AccountMigration.where(old_person: old_person, new_person: profile.person).exists?
+ logger.warn "ignoring error on receive #{entity}: #{e.class}: #{e.message}"
+ nil
end
def self.comment(entity)
diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb
index 5ab47dbcc2..c9a67aeb49 100644
--- a/spec/integration/federation/receive_federation_messages_spec.rb
+++ b/spec/integration/federation/receive_federation_messages_spec.rb
@@ -58,11 +58,12 @@ describe "Receive federation messages feature" do
expect(AccountMigration.find_by(old_person: sender.person, new_person: new_user.person)).to be_performed
end
- it "doesn't accept the same migration for the second time" do
+ it "doesn't run the same migration for the second time" do
run_migration
- expect {
- run_migration
- }.to raise_error(ActiveRecord::RecordInvalid)
+ expect_any_instance_of(AccountMigration).not_to receive(:perform!)
+ run_migration
+ expect(AccountMigration.where(old_person: sender.person, new_person: new_user.person).count).to eq(1)
+ expect(AccountMigration.find_by(old_person: sender.person, new_person: new_user.person)).to be_performed
end
it "doesn't accept second migration for the same sender" do
diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb
index 1fd76b08d1..2c2e7e1106 100644
--- a/spec/lib/diaspora/federation/receive_spec.rb
+++ b/spec/lib/diaspora/federation/receive_spec.rb
@@ -47,6 +47,27 @@ describe Diaspora::Federation::Receive do
expect(AccountMigration.exists?(old_person: sender, new_person: new_person)).to be_truthy
end
+
+ it "ignores duplicate the account migrations" do
+ AccountMigration.create(old_person: sender, new_person: new_person)
+
+ expect(AccountMigration).not_to receive(:create!)
+
+ expect(Diaspora::Federation::Receive.account_migration(account_migration_entity)).to be_nil
+
+ expect(AccountMigration.exists?(old_person: sender, new_person: new_person)).to be_truthy
+ end
+
+ it "handles race conditions on parallel receive" do
+ expect(AccountMigration).to receive(:create!) do
+ AccountMigration.create(old_person: sender, new_person: new_person)
+ raise "Some database error"
+ end
+
+ expect(Diaspora::Federation::Receive.account_migration(account_migration_entity)).to be_nil
+
+ expect(AccountMigration.exists?(old_person: sender, new_person: new_person)).to be_truthy
+ end
end
describe ".comment" do
--
GitLab