Migrate unsubscribe keys to the database.

This should reduce a lot of the keys in redis.

app/controllers/email_controller.rb

@@ -10,7 +10,7 @@ class EmailController < ApplicationController
   end
 
   def unsubscribe
-    @user = User.find_by_temporary_key(params[:key])
+    @user = DigestUnsubscribeKey.user_for_key(params[:key])
 
     # Don't allow the use of a key while logged in as a different user
     if current_user.present? && (@user != current_user)
@@ -28,7 +28,7 @@ class EmailController < ApplicationController
   end
 
   def resubscribe
-    @user = User.find_by_temporary_key(params[:key])
+    @user = DigestUnsubscribeKey.user_for_key(params[:key])
     raise Discourse::NotFound unless @user.present?
     @user.update_column(:email_digests, true)
   end

app/jobs/scheduled/clean_up_digest_keys.rb

@@ -0,0 +1,13 @@
+module Jobs
+
+  class CleanUpDigestKeys < Jobs::Scheduled
+    every 1.day
+
+    def execute(args)
+      DigestUnsubscribeKey.where('created_at < ?', 2.months.ago).delete_all
+    end
+
+  end
+
+end
+

app/mailers/user_notifications.rb

@@ -69,6 +69,7 @@ class UserNotifications < ActionMailer::Base
 
       @featured_topics, @new_topics = @featured_topics[0..4], @featured_topics[5..-1]
       @markdown_linker = MarkdownLinker.new(Discourse.base_url)
+      @unsubscribe_key = DigestUnsubscribeKey.create_key_for(@user)
 
       build_email user.email,
                   from_alias: I18n.t('user_notifications.digest.from', site_name: SiteSetting.title),

app/models/digest_unsubscribe_key.rb

@@ -0,0 +1,19 @@
+class DigestUnsubscribeKey < ActiveRecord::Base
+  belongs_to :user
+
+  before_create :generate_random_key
+
+  def self.create_key_for(user)
+    DigestUnsubscribeKey.create(user_id: user.id).key
+  end
+
+  def self.user_for_key(key)
+    where(key: key).first.try(:user)
+  end
+
+  private
+
+    def generate_random_key
+      self.key = SecureRandom.hex(32)
+    end
+end

app/models/user.rb

@@ -164,14 +164,6 @@ class User < ActiveRecord::Base
     name.titleize
   end
 
-  # Find a user by temporary key, nil if not found or key is invalid
-  def self.find_by_temporary_key(key)
-    user_id = $redis.get("temporary_key:#{key}")
-    if user_id.present?
-      find_by(id: user_id.to_i)
-    end
-  end
-
   def self.find_by_username_or_email(username_or_email)
     if username_or_email.include?('@')
       find_by_email(username_or_email)
@@ -203,13 +195,6 @@ class User < ActiveRecord::Base
     save
   end
 
-  # Use a temporary key to find this user, store it in redis with an expiry
-  def temporary_key
-    key = SecureRandom.hex(32)
-    $redis.setex "temporary_key:#{key}", 2.months, id.to_s
-    key
-  end
-
   def created_topic_count
     stat = user_stat || create_user_stat
     stat.topic_count

app/views/user_notifications/digest.html.erb

@@ -73,6 +73,6 @@
 <div class='footer'>
   <%=raw(t :'user_notifications.digest.unsubscribe',
            site_link: html_site_link,
-           unsubscribe_link: link_to(t('user_notifications.digest.click_here'), email_unsubscribe_url(host: Discourse.base_url, key: @user.temporary_key)))  %>
+           unsubscribe_link: link_to(t('user_notifications.digest.click_here'), email_unsubscribe_url(host: Discourse.base_url, key: @unsubscribe_key)))  %>
 </div>
 

app/views/user_notifications/digest.text.erb

@@ -33,6 +33,6 @@
 
 <%=raw(t :'user_notifications.digest.unsubscribe',
      site_link: site_link,
-     unsubscribe_link: raw(@markdown_linker.create(t('user_notifications.digest.click_here'), email_unsubscribe_url(key: @user.temporary_key, only_path: true))))  %>
+     unsubscribe_link: raw(@markdown_linker.create(t('user_notifications.digest.click_here'), email_unsubscribe_url(key: @unsubscribe_key, only_path: true))))  %>
 
 <%= raw(@markdown_linker.references) %>

db/migrate/20150213174159_create_digest_unsubscribe_keys.rb

@@ -0,0 +1,46 @@
+class CreateDigestUnsubscribeKeys < ActiveRecord::Migration
+  def up
+    create_table :digest_unsubscribe_keys, id: false do |t|
+      t.string :key, limit: 64, null: false
+      t.integer :user_id, null: false
+      t.timestamps
+    end
+    execute "ALTER TABLE digest_unsubscribe_keys ADD PRIMARY KEY (key)"
+    add_index :digest_unsubscribe_keys, :created_at
+
+    migrate_redis_keys
+  end
+
+  # It is slightly odd to migrate from redis to postgres; I imagine a lot
+  # could fail, so if anything does we just rescue
+  def migrate_redis_keys
+    return if Rails.env.test?
+
+    temp_keys = $redis.keys('temporary_key:*')
+    if temp_keys.present?
+      temp_keys.map! do |key|
+        user_id = $redis.get(key).to_i
+        ttl = $redis.ttl(key).to_i
+
+        if ttl > 0
+          ttl = "'#{ttl.seconds.ago.strftime('%Y-%m-%d %H:%M:%S')}'"
+        else
+          ttl = "CURRENT_TIMESTAMP"
+        end
+        $redis.del(key)
+        key.gsub!('temporary_key:', '')
+        user_id ? "('#{key}', #{user_id}, #{ttl}, #{ttl})" : nil
+      end
+      temp_keys.compact!
+      if temp_keys.present?
+        execute "INSERT INTO digest_unsubscribe_keys (key, user_id, created_at, updated_at) VALUES #{temp_keys.join(', ')}"
+      end
+    end
+  rescue
+    # If anything goes wrong, continue with other migrations
+  end
+
+  def down
+    drop_table :digest_unsubscribe_keys
+  end
+end

spec/controllers/email_controller_spec.rb

@@ -22,10 +22,11 @@ describe EmailController do
   context '.resubscribe' do
 
     let(:user) { Fabricate(:user, email_digests: false) }
+    let(:key) { DigestUnsubscribeKey.create_key_for(user) }
 
     context 'with a valid key' do
       before do
-        get :resubscribe, key: user.temporary_key
+        get :resubscribe, key: key
         user.reload
       end
 
@@ -39,10 +40,11 @@ describe EmailController do
   context '.unsubscribe' do
 
     let(:user) { Fabricate(:user) }
+    let(:key) { DigestUnsubscribeKey.create_key_for(user) }
 
     context 'with a valid key' do
       before do
-        get :unsubscribe, key: user.temporary_key
+        get :unsubscribe, key: key
         user.reload
       end
 
@@ -69,7 +71,7 @@ describe EmailController do
       let!(:logged_in_user) { log_in(:coding_horror) }
 
       before do
-        get :unsubscribe, key: user.temporary_key
+        get :unsubscribe, key: key
         user.reload
       end
 
@@ -87,7 +89,7 @@ describe EmailController do
 
       before do
         log_in_user(user)
-        get :unsubscribe, key: user.temporary_key
+        get :unsubscribe, key: DigestUnsubscribeKey.create_key_for(user)
         user.reload
       end
 

spec/jobs/clean_up_digest_keys.rb

@@ -0,0 +1,13 @@
+module Jobs
+
+  class CleanUpDigestKeys < Jobs::Scheduled
+    every 1.day
+
+    def execute(args)
+      # Remove unused digest keys
+      DigestUnsubscribeKey.where('created_at < ?', 2.months.ago).delete_all
+    end
+  end
+
+end
+

spec/models/digest_unsubscribe_key_spec.rb

@@ -0,0 +1,31 @@
+require 'spec_helper'
+require_dependency 'digest_unsubscribe_key'
+
+describe DigestUnsubscribeKey do
+
+  it { is_expected.to belong_to :user }
+
+  describe 'key' do
+
+    let(:user) { Fabricate(:user) }
+    let!(:key) { DigestUnsubscribeKey.create_key_for(user) }
+
+    it 'has a temporary key' do
+      expect(key).to be_present
+    end
+
+    describe '#user_for_key' do
+
+      it 'can be used to find the user' do
+        expect(DigestUnsubscribeKey.user_for_key(key)).to eq(user)
+      end
+
+      it 'returns nil with an invalid key' do
+        expect(DigestUnsubscribeKey.user_for_key('asdfasdf')).to be_blank
+      end
+
+    end
+
+  end
+
+end

spec/models/user_spec.rb

@@ -384,29 +384,6 @@ describe User do
     end
   end
 
-  describe 'temporary_key' do
-
-    let(:user) { Fabricate(:user) }
-    let!(:temporary_key) { user.temporary_key}
-
-    it 'has a temporary key' do
-      expect(temporary_key).to be_present
-    end
-
-    describe 'User#find_by_temporary_key' do
-
-      it 'can be used to find the user' do
-        expect(User.find_by_temporary_key(temporary_key)).to eq(user)
-      end
-
-      it 'returns nil with an invalid key' do
-        expect(User.find_by_temporary_key('asdfasdf')).to be_blank
-      end
-
-    end
-
-  end
-
   describe 'email_hash' do
     before do
       @user = Fabricate(:user)