Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async actionmailer #6839

Merged
merged 12 commits into from
Jun 26, 2012
2 changes: 2 additions & 0 deletions actionmailer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

* Raise an `ActionView::MissingTemplate` exception when no implicit template could be found. *Damien Mathieu*

* Asynchronously send messages via the Rails Queue *Brian Cardarella*

## Rails 3.2.5 (Jun 1, 2012) ##

* No changes.
Expand Down
39 changes: 39 additions & 0 deletions actionmailer/lib/action_mailer/async.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
require 'delegate'

module ActionMailer::Async
def method_missing(method_name, *args)
if action_methods.include?(method_name.to_s)
QueuedMessage.new(queue, self, method_name, *args)
else
super
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, couldn't we just expect people to say extend ActionMailer::Async in their class? Then the ClassMethods module and this method become unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is better. The extend ActionMailer::Async can be called from the async= setter on Base

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this effect how the QueuedMessage class gets inherited into the parent class?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. You mean the visibility of the constant?


def queue
Rails.queue
end

class QueuedMessage < ::Delegator
attr_reader :queue

def initialize(queue, mailer_class, method_name, *args)
@queue = queue
@mailer_class = mailer_class
@method_name = method_name
@args = args
end

def __getobj__
@actual_message ||= @mailer_class.send(:new, @method_name, *@args).message
end

def run
__getobj__.deliver
end

# Will push the message onto the Queue to be processed
def deliver
@queue << self
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use method_missing instead of ruby's DelegateClass (from the stdlib's "delegate") which copies the public methods and exposes the public API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't I have to explicitly state which methods I want to delegate? This QueuedMessage class is meant to behave in most ways identical to the original message class. this is quicker to delegate those methods to it than listing each one out. Unless I'm mistaken about the behavior of Delegate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no, I just noticed that you don't know to which class you're delegating until runtime, in that case DelegateClass won't do, since you need to know the delegated class when you define the object.

That said, even if also uses method_missing, I would still use Delegator because it handles things like #respond_to_missing?, #public_methods, etc.

Something like

    class QueuedMessage < Delegator
      def __getobj__
        @actual_message ||= @mailer_class.send(:new, @method_name, *@args).message
      end

      def run
        __getobj__.deliver
      end
    end

Then you don't need #method_missing, #actual_message, or #to_sin that implementation :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa, that's way better. Thank you, updated and credited.

end
end
12 changes: 12 additions & 0 deletions actionmailer/lib/action_mailer/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,18 @@ def respond_to?(method, include_private = false) #:nodoc:
super || action_methods.include?(method.to_s)
end

# Will force ActionMailer to push new messages to the queue defined
# in the ActionMailer class when set to true
#
# class WelcomeMailer < ActionMailer::Base
# self.async = true
def async=(truth)
if truth
require 'action_mailer/async'
extend ActionMailer::Async
end
end

protected

def set_payload_for_mail(payload, mail) #:nodoc:
Expand Down
23 changes: 23 additions & 0 deletions actionmailer/test/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
require 'mailers/base_mailer'
require 'mailers/proc_mailer'
require 'mailers/asset_mailer'
require 'mailers/async_mailer'
require 'rails/queueing'

class BaseTest < ActiveSupport::TestCase
def teardown
Expand Down Expand Up @@ -419,6 +421,26 @@ def teardown
assert_equal(1, BaseMailer.deliveries.length)
end

def stub_queue(klass, queue)
Class.new(klass) {
extend Module.new {
define_method :queue do
queue
end
}
}
end

test "delivering message asynchronously" do
testing_queue = Rails::Queueing::TestQueue.new
AsyncMailer.delivery_method = :test
AsyncMailer.deliveries.clear
stub_queue(AsyncMailer, testing_queue).welcome.deliver
assert_equal(0, AsyncMailer.deliveries.length)
testing_queue.drain
assert_equal(1, AsyncMailer.deliveries.length)
end

test "calling deliver, ActionMailer should yield back to mail to let it call :do_delivery on itself" do
mail = Mail::Message.new
mail.expects(:do_delivery).once
Expand All @@ -434,6 +456,7 @@ def teardown
end

test "should raise if missing template in implicit render" do
BaseMailer.deliveries.clear
assert_raises ActionView::MissingTemplate do
BaseMailer.implicit_different_template('missing_template').deliver
end
Expand Down
1 change: 1 addition & 0 deletions actionmailer/test/fixtures/async_mailer/welcome.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Welcome
3 changes: 3 additions & 0 deletions actionmailer/test/mailers/async_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class AsyncMailer < BaseMailer
self.async = true
end
3 changes: 3 additions & 0 deletions railties/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@

* Rails::Plugin has gone. Instead of adding plugins to vendor/plugins use gems or bundler with path or git dependencies. *Santiago Pastorino*

* Set config.action_mailer.async = true to turn on asynchronous
message delivery *Brian Cardarella*


## Rails 3.2.2 (March 1, 2012) ##

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,8 @@ class Application < Rails::Application
# Version of your assets, change this if you want to expire all your assets.
config.assets.version = '1.0'
<% end -%>

# Enable app-wide asynchronous ActionMailer
# config.action_mailer.async = true
end
end