move account deletion out of a tranaction and into a job

app/controllers/users_controller.rb

@@ -71,7 +71,8 @@ class UsersController < ApplicationController
   end
 
   def destroy
-    current_user.destroy
+    Resque.enqueue(Job::DeleteAccount, current_user.id)
+    current_user.lock_access!
     sign_out current_user
     flash[:notice] = I18n.t 'users.destroy'
     redirect_to root_path

app/models/jobs/delete_account.rb

@@ -0,0 +1,15 @@
+#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+
+module Job
+  class DeleteAccount < Base
+    @queue = :delete_account
+    def self.perform_delegate(user_id)
+      user = User.find(user_id)
+      user.remove_all_traces
+      user.destroy
+    end
+  end
+end

app/models/user.rb

@@ -13,7 +13,8 @@ class User < ActiveRecord::Base
 
   devise :invitable, :database_authenticatable, :registerable,
          :recoverable, :rememberable, :trackable, :validatable,
-         :timeoutable, :token_authenticatable
+         :timeoutable, :token_authenticatable, :lockable,
+         :lock_strategy => :none, :unlock_strategy => :none
 
   before_validation :strip_and_downcase_username
   before_validation :set_current_language, :on => :create
@@ -39,11 +40,11 @@ class User < ActiveRecord::Base
   has_many :services
   has_many :user_preferences
 
-  before_destroy :disconnect_everyone, :remove_mentions, :remove_person
   before_save do
     person.save if person && person.changed?
   end
 
+
   attr_accessible :getting_started, :password, :password_confirmation, :language, :disable_mail
 
   def update_user_preferences(pref_hash)
@@ -317,7 +318,11 @@ class User < ActiveRecord::Base
     AppConfig[:admins].present? && AppConfig[:admins].include?(self.username)
   end
 
-  protected
+  def remove_all_traces
+    disconnect_everyone
+    remove_mentions
+    remove_person
+  end
 
   def remove_person
     self.person.destroy
@@ -325,11 +330,11 @@ class User < ActiveRecord::Base
 
   def disconnect_everyone
     self.contacts.each do |contact|
-      unless contact.person.owner.nil?
+      if contact.person.remote?
+        self.disconnect(contact)
+      else
         contact.person.owner.disconnected_by(self.person)
         remove_contact(contact, :force => true)
-      else
-        self.disconnect(contact)
       end
     end
     self.aspects.destroy_all

config/locales/diaspora/en.yml

@@ -704,7 +704,7 @@ en:
       private_message: "...you receive a private message?"
       liked: "...someone likes your post?"
       change: "Change"
-    destroy: "Account successfully closed."
+    destroy: "Your account has been locked.  It may take 20 minutes for us to finish closing your account.  Thank you for trying Diaspora."
     getting_started:
       welcome: "Welcome to Diaspora!"
       signup_steps: "Finish your sign up by completing these three steps:"

db/migrate/20110603181015_lockable_users.rb

@@ -0,0 +1,9 @@
+class LockableUsers < ActiveRecord::Migration
+  def self.up
+    add_column :users, :locked_at, :datetime
+  end
+
+  def self.down
+    remove_column :users, :locked_at
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20110527135552) do
+ActiveRecord::Schema.define(:version => 20110603181015) do
 
   create_table "aspect_memberships", :force => true do |t|
     t.integer  "aspect_id",  :null => false
@@ -369,6 +369,7 @@ ActiveRecord::Schema.define(:version => 20110527135552) do
     t.integer  "invited_by_id"
     t.string   "invited_by_type"
     t.string   "authentication_token",   :limit => 30
+    t.datetime "locked_at"
   end
 
   add_index "users", ["authentication_token"], :name => "index_users_on_authentication_token", :unique => true

spec/controllers/users_controller_spec.rb

@@ -145,4 +145,15 @@ describe UsersController do
       response.should redirect_to new_user_session_path
     end
   end
+
+  describe '#destroy' do
+    it 'enqueues a delete job' do
+      Resque.should_receive(:enqueue).with(Job::DeleteAccount, alice.id)
+      delete :destroy
+    end
+    it 'locks the user out' do
+      delete :destroy
+      alice.reload.access_locked?.should be_true
+    end
+  end
 end

spec/models/jobs/delete_account_spec.rb

@@ -0,0 +1,30 @@
+#   Copyright (c) 2011, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+require 'spec_helper'
+
+describe Job::DeleteAccount do
+  describe '#perform' do
+    it 'calls remove_all_traces' do
+      stub_find_for(bob)
+      bob.should_receive(:remove_all_traces)
+      Job::DeleteAccount.perform(bob.id)
+    end
+
+    it 'calls destroy' do
+      stub_find_for(bob)
+      bob.should_receive(:destroy)
+      Job::DeleteAccount.perform(bob.id)
+    end
+    def stub_find_for model
+      model.class.stub!(:find) do |id, conditions|
+        if id == model.id
+          model
+        else
+          model.class.find_by_id(id)
+        end
+      end
+    end
+  end
+end

spec/models/user_spec.rb

@@ -226,7 +226,7 @@ describe User do
     it "returns false if the users are already connected" do
       alice.can_add?(bob.person).should be_false
     end
-    
+
     it "returns false if the user has already sent a request to that person" do
       alice.share_with(eve.person, alice.aspects.first)
       alice.reload
@@ -374,71 +374,68 @@ describe User do
     end
   end
 
-  describe 'account removal' do
-    it 'should disconnect everyone' do
-      alice.should_receive(:disconnect_everyone)
-      alice.destroy
+  describe 'account deletion' do
+    describe '#remove_all_traces' do
+      it 'should disconnect everyone' do
+        alice.should_receive(:disconnect_everyone)
+        alice.remove_all_traces
+      end
+
+
+      it 'should remove mentions' do
+        alice.should_receive(:remove_mentions)
+        alice.remove_all_traces
+      end
+
+      it 'should remove person' do
+        alice.should_receive(:remove_person)
+        alice.remove_all_traces
+      end
+
+      it 'should remove all aspects' do
+        lambda {
+          alice.remove_all_traces
+        }.should change{ alice.aspects(true).count }.by(-1)
+      end
     end
 
-    it 'removes invitations from the user' do
-      alice.invite_user alice.aspects.first.id, 'email', 'blah@blah.blah'
-      lambda {
-        alice.destroy
-      }.should change {alice.invitations_from_me(true).count }.by(-1)
-    end
+    describe '#destroy' do
+      it 'removes invitations from the user' do
+        alice.invite_user alice.aspects.first.id, 'email', 'blah@blah.blah'
+        lambda {
+          alice.destroy
+        }.should change {alice.invitations_from_me(true).count }.by(-1)
+      end
 
-    it 'removes invitations to the user' do
-      Invitation.create(:sender_id => eve.id, :recipient_id => alice.id, :aspect_id => eve.aspects.first.id)
-      lambda {
-        alice.destroy
-      }.should change {alice.invitations_to_me(true).count }.by(-1)
-    end
+      it 'removes invitations to the user' do
+        Invitation.create(:sender_id => eve.id, :recipient_id => alice.id, :aspect_id => eve.aspects.first.id)
+        lambda {
+          alice.destroy
+        }.should change {alice.invitations_to_me(true).count }.by(-1)
+      end
 
-    it 'should remove mentions' do
-      alice.should_receive(:remove_mentions)
-      alice.destroy
-    end
-
-    it 'should remove person' do
-      alice.should_receive(:remove_person)
-      alice.destroy
-    end
-
-    it 'should remove all aspects' do
-      lambda {
-        alice.destroy
-      }.should change{ alice.aspects(true).count }.by(-1)
-    end
-
-    it 'removes all contacts' do
-      lambda {
-        alice.destroy
-      }.should change {
-        alice.contacts.count
-      }.by(-1)
-    end
-
-    it 'removes all service connections' do
-      Services::Facebook.create(:access_token => 'what', :user_id => alice.id)
-      lambda {
-        alice.destroy
-      }.should change {
-        alice.services.count
-      }.by(-1)
+      it 'removes all service connections' do
+        Services::Facebook.create(:access_token => 'what', :user_id => alice.id)
+        lambda {
+          alice.destroy
+        }.should change {
+          alice.services.count
+        }.by(-1)
+      end
     end
 
     describe '#remove_person' do
       it 'should remove the person object' do
         person = alice.person
-        alice.destroy
+        alice.remove_person
         person.reload
-        person.should be nil
+        person.should be_nil
       end
 
       it 'should remove the posts' do
         message = alice.post(:status_message, :text => "hi", :to => alice.aspects.first.id)
         alice.reload
-        alice.destroy
+        alice.remove_person
         proc { message.reload }.should raise_error ActiveRecord::RecordNotFound
       end
     end
@@ -449,41 +446,50 @@ describe User do
         sm =  Factory(:status_message)
         mention  = Mention.create(:person => person, :post=> sm)
         alice.reload
-        alice.destroy
+        alice.remove_mentions
         proc { mention.reload }.should raise_error ActiveRecord::RecordNotFound
       end
     end
 
     describe '#disconnect_everyone' do
       it 'has no error on a local friend who has deleted his account' do
-        alice.destroy
+        Job::DeleteAccount.perform(alice.id)
         lambda {
-          bob.destroy
+          bob.disconnect_everyone
         }.should_not raise_error
       end
 
       it 'has no error when the user has sent local requests' do
         alice.share_with(eve.person, alice.aspects.first)
         lambda {
-          alice.destroy
+          alice.disconnect_everyone
         }.should_not raise_error
       end
 
-      it 'should send retractions to remote poeple' do
+      it 'sends retractions to remote poeple' do
         person = eve.person
         eve.delete
+        person.owner_id = nil
         person.save
         alice.contacts.create(:person => person, :aspects => [alice.aspects.first])
 
         alice.should_receive(:disconnect).once
-        alice.destroy
+        alice.disconnect_everyone
       end
 
-      it 'should disconnect local people' do
+      it 'disconnects local people' do
         lambda {
-          alice.destroy
+          alice.remove_all_traces
         }.should change{bob.reload.contacts.count}.by(-1)
       end
+
+      it 'removes all contacts' do
+        lambda {
+          alice.disconnect_everyone
+        }.should change {
+          alice.contacts.count
+        }.by(-1)
+      end
     end
   end
 
@@ -519,7 +525,7 @@ describe User do
 
     describe "#add_contact_to_aspect" do
       it 'adds the contact to the aspect' do
-        lambda { 
+        lambda {
           alice.add_contact_to_aspect(@contact, @aspect1)
         }.should change(@aspect1.contacts, :count).by(1)
       end
@@ -557,7 +563,7 @@ describe User do
       end
     end
   end
-  
+
   context 'likes' do
     before do
       @message = alice.post(:status_message, :text => "cool", :to => alice.aspects.first)
@@ -565,24 +571,24 @@ describe User do
       @like = alice.like(true, :post => @message)
       @dislike = bob.like(false, :post => @message)
     end
-    
+
     describe '#like_for' do
       it 'returns the correct like' do
         alice.like_for(@message).should == @like
         bob.like_for(@message).should == @dislike
       end
-      
+
       it "returns nil if there's no like" do
         alice.like_for(@message2).should be_nil
       end
     end
-  
+
     describe '#liked?' do
       it "returns true if there's a like" do
         alice.liked?(@message).should be_true
         bob.liked?(@message).should be_true
       end
-    
+
       it "returns false if there's no like" do
         alice.liked?(@message2).should be_false
       end