From 85b02b94f610ffbdd265ab14ff74da54332903f9 Mon Sep 17 00:00:00 2001 From: Tony Collen Date: Wed, 11 May 2011 17:14:42 -0500 Subject: [PATCH 1/3] Gemfile and gitignore for vim and mac turds --- .gitignore | 2 ++ Gemfile | 4 ++++ 2 files changed, 6 insertions(+) create mode 100644 Gemfile diff --git a/.gitignore b/.gitignore index e43b0f9..e2a8b2e 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ +Gemfile.lock .DS_Store +*.swp diff --git a/Gemfile b/Gemfile new file mode 100644 index 0000000..cb2ab8b --- /dev/null +++ b/Gemfile @@ -0,0 +1,4 @@ +source :rubygems + +gem 'mocha', '0.9.12' +gem 'system_timer', '1.0' From aa19a0229c21c3fab36b38afe271e635f0954dfc Mon Sep 17 00:00:00 2001 From: Tony Collen Date: Wed, 11 May 2011 17:16:24 -0500 Subject: [PATCH 2/3] refactor base, imap and pop fetchers to use a connection method that may be stubbed. create or use a logger object, and expose through a method as well. explicitly catch exceptions and log them to this logger, rather than swallowing them --- lib/fetcher/base.rb | 27 +++++++++++++++++++++++++++ lib/fetcher/imap.rb | 38 +++++++++++++++++++------------------- lib/fetcher/pop.rb | 18 +++++++++--------- test/fetcher_test.rb | 30 +++++++++++++++++++++++++++++- 4 files changed, 84 insertions(+), 29 deletions(-) diff --git a/lib/fetcher/base.rb b/lib/fetcher/base.rb index 5b7ea3e..7656af0 100644 --- a/lib/fetcher/base.rb +++ b/lib/fetcher/base.rb @@ -1,5 +1,9 @@ +require 'logger' + module Fetcher class Base + attr_reader :connection + # Options: # * :server - Server to connect to. # * :username - Username to use when connecting to server. @@ -23,6 +27,7 @@ def initialize(options={}) end instance_eval("@#{opt} = options[:#{opt}]") + @logger = create_logger(options[:logger]) end end @@ -59,5 +64,27 @@ def process_message(message) def handle_bogus_message(message) #:nodoc: raise NotImplementedError, "This method should be overridden by subclass" end + + def handle_exception(ex) + log "Fetcher: Exception: #{ex.message}" + log ex.backtrace.join("\n") + end + + def log(msg) + @logger.info(msg) + end + + def create_logger(provided_logger) + @logger = provided_logger || rails_logger || default_logger + end + + def rails_logger + defined?(Rails) && Rails.logger + end + + def default_logger + ::Logger.new(STDERR) + end + end end diff --git a/lib/fetcher/imap.rb b/lib/fetcher/imap.rb index c0d0c72..7c27a4e 100644 --- a/lib/fetcher/imap.rb +++ b/lib/fetcher/imap.rb @@ -3,7 +3,6 @@ module Fetcher class Imap < Base - PORT = 143 protected @@ -28,61 +27,62 @@ def initialize(options={}) # Open connection and login to server def establish_connection - timeout_call = (RUBY_VERSION < '1.9.0') ? "SystemTimer.timeout_after(15.seconds) do" : "Timeout::timeout(15) do" + timeout_call = (RUBY_VERSION < '1.9.0') ? "SystemTimer.timeout_after(15) do" : "Timeout::timeout(15) do" eval("#{timeout_call} - @connection = Net::IMAP.new(@server, @port, @ssl) + connection = Net::IMAP.new(@server, @port, @ssl) if @use_login - @connection.login(@username, @password) + connection.login(@username, @password) else - @connection.authenticate(@authentication, @username, @password) + connection.authenticate(@authentication, @username, @password) end end") end # Retrieve messages from server def get_messages - @connection.select(@in_folder) - @connection.uid_search(['ALL']).each do |uid| - msg = @connection.uid_fetch(uid,'RFC822').first.attr['RFC822'] + connection.select(@in_folder) + connection.uid_search(['ALL']).each do |uid| + msg = connection.uid_fetch(uid,'RFC822').first.attr['RFC822'] begin process_message(msg) add_to_processed_folder(uid) if @processed_folder - rescue + rescue Exception => ex + handle_exception(ex) handle_bogus_message(msg) end # Mark message as deleted - @connection.uid_store(uid, "+FLAGS", [:Seen, :Deleted]) + connection.uid_store(uid, "+FLAGS", [:Seen, :Deleted]) end end # Store the message for inspection if the receiver errors def handle_bogus_message(message) create_mailbox(@error_folder) - @connection.append(@error_folder, message) + connection.append(@error_folder, message) end # Delete messages and log out def close_connection - @connection.expunge - @connection.logout + connection.expunge + connection.logout begin - @connection.disconnect unless @connection.disconnected? + connection.disconnect unless connection.disconnected? rescue - Rails.logger.info("Fetcher: Remote closed connection before I could disconnect.") + log("Fetcher: Remote closed connection before I could disconnect.") end end def add_to_processed_folder(uid) create_mailbox(@processed_folder) - @connection.uid_copy(uid, @processed_folder) + connection.uid_copy(uid, @processed_folder) end def create_mailbox(mailbox) - unless @connection.list("", mailbox) - @connection.create(mailbox) + unless connection.list("", mailbox) + connection.create(mailbox) end end - + end end diff --git a/lib/fetcher/pop.rb b/lib/fetcher/pop.rb index e165799..cd2d69a 100644 --- a/lib/fetcher/pop.rb +++ b/lib/fetcher/pop.rb @@ -2,7 +2,6 @@ module Fetcher class Pop < Base - protected # Additional Options: @@ -16,18 +15,19 @@ def initialize(options={}) # Open connection and login to server def establish_connection - @connection = Net::POP3.new(@server, @port) - @connection.enable_ssl(OpenSSL::SSL::VERIFY_NONE) if @ssl - @connection.start(@username, @password) + connection = Net::POP3.new(@server, @port) + connection.enable_ssl(OpenSSL::SSL::VERIFY_NONE) if @ssl + connection.start(@username, @password) end # Retrieve messages from server def get_messages - unless @connection.mails.empty? - @connection.each_mail do |msg| + unless connection.mails.empty? + connection.each_mail do |msg| begin process_message(msg.pop) - rescue + rescue Exception => ex + handle_exception(ex) handle_bogus_message(msg.pop) end # Delete message from server @@ -40,10 +40,10 @@ def get_messages def handle_bogus_message(message) # This needs a good solution end - + # Close connection to server def close_connection - @connection.finish + connection.finish end end diff --git a/test/fetcher_test.rb b/test/fetcher_test.rb index 12329ba..795ea6e 100644 --- a/test/fetcher_test.rb +++ b/test/fetcher_test.rb @@ -38,6 +38,34 @@ def test_should_require_password def test_should_require_receiver assert_raise(ArgumentError) { create_fetcher(:receiver => nil) } end + + def test_should_handle_exception_if_receiver_raises_exception_during_receive + @imap_fetcher = Fetcher.create(:type => :imap, :server => 'test.host', + :username => 'name', + :password => 'password', + :receiver => @receiver) + + # most of this stubbing/mocking is to prevent the fetcher from + # actually connecting to a server. + @fake_message = stub('fake-message') + @stub_connection = stub_everything('fake-imap-connection', { + :uid_search => ['123'], + :uid_fetch => [stub('fake-message-piece', :attr => {"RFC822" => @fake_message})] + }) + @imap_fetcher.stubs(:establish_connection).returns(true) + @imap_fetcher.stubs(:connection).returns(@stub_connection) + + # The meat of this test + exception = RuntimeError.new("Explosion") + @receiver.expects(:receive).with(@fake_message).raises(exception) + + # we should handle the exception + @imap_fetcher.expects(:handle_exception).with(exception) + + # but also ensure we handle the bogus message as well + @imap_fetcher.expects(:handle_bogus_message).with(@fake_message) + @imap_fetcher.fetch + end def create_fetcher(options={}) @fetcher = Fetcher::Base.new({:server => 'test.host', :username => 'name', :password => 'password', :receiver => @receiver}.merge(options)) @@ -71,4 +99,4 @@ def test_should_require_type end -# Write tests for sub-classes \ No newline at end of file +# Write tests for sub-classes From 87a8e930662505f874aceb64b8f9d61c253ea0a1 Mon Sep 17 00:00:00 2001 From: Tony Collen Date: Fri, 20 May 2011 15:46:27 -0500 Subject: [PATCH 3/3] use the power of the twiddle-wakka --- Gemfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index cb2ab8b..8873ea2 100644 --- a/Gemfile +++ b/Gemfile @@ -1,4 +1,4 @@ source :rubygems -gem 'mocha', '0.9.12' -gem 'system_timer', '1.0' +gem 'mocha', '~> 0.9.12' +gem 'system_timer', '~> 1.0'