Harden account deletion

* Wrap it into a transaction
* Use destroy over delete so dependent destroys get triggered
  and we thus don't fail on the foreign key constraits
* Check if a photos status message actually exists before accessing
  it
* Add missing dependent destroys
This commit is contained in:
Jonne Haß 2014-05-26 23:54:40 +02:00
parent 1e09814b13
commit fc1f249129
7 changed files with 26 additions and 24 deletions

View file

@ -8,7 +8,7 @@ class Aspect < ActiveRecord::Base
has_many :aspect_memberships, :dependent => :destroy has_many :aspect_memberships, :dependent => :destroy
has_many :contacts, :through => :aspect_memberships has_many :contacts, :through => :aspect_memberships
has_many :aspect_visibilities has_many :aspect_visibilities, :dependent => :destroy
has_many :posts, :through => :aspect_visibilities, :source => :shareable, :source_type => 'Post' has_many :posts, :through => :aspect_visibilities, :source => :shareable, :source_type => 'Post'
has_many :photos, :through => :aspect_visibilities, :source => :shareable, :source_type => 'Photo' has_many :photos, :through => :aspect_visibilities, :source => :shareable, :source_type => 'Photo'

View file

@ -11,7 +11,7 @@ class Contact < ActiveRecord::Base
delegate :name, :diaspora_handle, :guid, :first_name, delegate :name, :diaspora_handle, :guid, :first_name,
to: :person, prefix: true to: :person, prefix: true
has_many :aspect_memberships has_many :aspect_memberships, :dependent => :destroy
has_many :aspects, :through => :aspect_memberships has_many :aspects, :through => :aspect_memberships
has_many :share_visibilities, :source => :shareable, :source_type => 'Post' has_many :share_visibilities, :source => :shareable, :source_type => 'Post'
@ -51,7 +51,7 @@ class Contact < ActiveRecord::Base
Notification.where(:target_type => "Person", Notification.where(:target_type => "Person",
:target_id => person_id, :target_id => person_id,
:recipient_id => user_id, :recipient_id => user_id,
:type => "Notifications::StartedSharing").delete_all :type => "Notifications::StartedSharing").destroy_all
end end
def dispatch_request def dispatch_request

View file

@ -22,6 +22,6 @@ class Mention < ActiveRecord::Base
end end
def delete_notification def delete_notification
Notification.where(:target_type => self.class.name, :target_id => self.id).delete_all Notification.where(:target_type => self.class.name, :target_id => self.id).destroy_all
end end
end end

View file

@ -51,7 +51,7 @@ class Photo < ActiveRecord::Base
end end
def clear_empty_status_message def clear_empty_status_message
if self.status_message_guid && self.status_message.text_and_photos_blank? if self.status_message && self.status_message.text_and_photos_blank?
self.status_message.destroy self.status_message.destroy
else else
true true

View file

@ -23,20 +23,22 @@ class AccountDeleter
end end
def perform! def perform!
#person ActiveRecord::Base.transaction do
delete_standard_person_associations #person
remove_conversation_visibilities delete_standard_person_associations
remove_share_visibilities_on_persons_posts remove_conversation_visibilities
delete_contacts_of_me remove_share_visibilities_on_persons_posts
tombstone_person_and_profile delete_contacts_of_me
tombstone_person_and_profile
if self.user if self.user
#user deletion methods #user deletion methods
remove_share_visibilities_on_contacts_posts remove_share_visibilities_on_contacts_posts
delete_standard_user_associations delete_standard_user_associations
disassociate_invitations disassociate_invitations
disconnect_contacts disconnect_contacts
tombstone_user tombstone_user
end
end end
end end
@ -55,13 +57,13 @@ class AccountDeleter
def delete_standard_user_associations def delete_standard_user_associations
normal_ar_user_associates_to_delete.each do |asso| normal_ar_user_associates_to_delete.each do |asso|
self.user.send(asso).each{|model| model.delete} self.user.send(asso).each{|model| model.destroy }
end end
end end
def delete_standard_person_associations def delete_standard_person_associations
normal_ar_person_associates_to_delete.each do |asso| normal_ar_person_associates_to_delete.each do |asso|
self.person.send(asso).delete_all self.person.send(asso).destroy_all
end end
end end

View file

@ -7,9 +7,9 @@ namespace :accounts do
account_delete.perform! account_delete.perform!
end end
puts "OK." puts "OK."
else
puts "No acccount deletions to run."
end end
puts "No acccount deletions to run."
end end
end end

View file

@ -73,7 +73,7 @@ describe AccountDeleter do
it 'removes all standard user associaltions' do it 'removes all standard user associaltions' do
@account_deletion.normal_ar_user_associates_to_delete.each do |asso| @account_deletion.normal_ar_user_associates_to_delete.each do |asso|
association_double = double association_double = double
association_double.should_receive(:delete) association_double.should_receive(:destroy)
bob.should_receive(asso).and_return([association_double]) bob.should_receive(asso).and_return([association_double])
end end
@ -88,7 +88,7 @@ describe AccountDeleter do
it 'removes all standard person associaltions' do it 'removes all standard person associaltions' do
@account_deletion.normal_ar_person_associates_to_delete.each do |asso| @account_deletion.normal_ar_person_associates_to_delete.each do |asso|
association_double = double association_double = double
association_double.should_receive(:delete_all) association_double.should_receive(:destroy_all)
bob.person.should_receive(asso).and_return(association_double) bob.person.should_receive(asso).and_return(association_double)
end end