-
Notifications
You must be signed in to change notification settings - Fork 143
Fixed IMAP support, added specs and functional test, refactored code. #46
Changes from 12 commits
c66f740
2d1fbd9
6ea375e
9f2095f
317f3a1
903904d
56295fc
237d591
6126ff9
7b848b9
92ca65c
7ea6cd1
657d63f
3266b6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ doc | |
pkg | ||
*.rbc | ||
.bundle | ||
Gemfile.lock | ||
|
||
## PROJECT::SPECIFIC | ||
spec/test-maildir |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
require 'net/imap' | ||
|
||
module Mailman | ||
module Receiver | ||
# Receives messages using IMAP, and passes them to a {MessageProcessor}. | ||
class IMAP | ||
|
||
# @return [Net::IMAP] the IMAP connection | ||
attr_reader :connection | ||
|
||
# @param [Hash] options the receiver options | ||
# @option options [MessageProcessor] :processor the processor to pass new | ||
# messages to | ||
# @option options [String] :server the server to connect to | ||
# @option options [Integer] :port the port to connect to | ||
# @option options [String] :username the username to authenticate with | ||
# @option options [String] :password the password to authenticate with | ||
def initialize(options) | ||
@processor, @username, @password, @server, @filter, @port = nil, nil, nil, nil, ["NEW"], 143 | ||
|
||
@processor = options[:processor] if options.has_key? :processor | ||
@username = options[:username] if options.has_key? :username | ||
@password = options[:password] if options.has_key? :password | ||
@filter = options[:filter] if options.has_key? :filter | ||
@port = options[:port] if options.has_key? :port | ||
@connection = Net::IMAP.new(options[:server], @port) | ||
end | ||
|
||
# Connects to the IMAP server. | ||
def connect | ||
@connection.login(@username, @password) | ||
@connection.examine("INBOX") | ||
end | ||
|
||
# Disconnects from the IMAP server. | ||
def disconnect | ||
@connection.logout | ||
@connection.disconnected? ? true : @connection.disconnect rescue nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about this? @connection.disconnect unless @connection.disconnected? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just copying the specs for POP3, I need to return
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that's fine then. |
||
end | ||
|
||
# Iterates through new messages, passing them to the processor, and | ||
# deleting them. | ||
def get_messages | ||
@connection.search(@filter).each do |message| | ||
puts "PROCESSING MESSAGE #{message}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use the logger, using sentence case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I'll correct this |
||
body = @connection.fetch(message,"RFC822")[0].attr["RFC822"] | ||
@processor.process(body) | ||
@connection.store(message,"+FLAGS",[Net::IMAP::DELETED]) | ||
end | ||
# Clears messages that have the Deleted flag set | ||
@connection.expunge | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about moving messages to a different folder instead of deleting them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, that's going to be the next feature. I'm looking to replace our existing IMAP processor with Mailman, which currently moves processed emails to a new folder. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's what I would like to implement: https://github.com/fatfreecrm/fat_free_crm/blob/master/lib/fat_free_crm/dropbox.rb#L146-164 I'm not sure if / how Mailman handles unmatched emails? We use a |
||
end | ||
|
||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '/spec_helper')) | ||
|
||
describe Mailman::Receiver::IMAP do | ||
|
||
before do | ||
@processor = mock('Message Processor', :process => true) | ||
@receiver_options = { :username => 'user', | ||
:password => 'pass', | ||
:server => 'example.com', | ||
:processor => @processor } | ||
@receiver = Mailman::Receiver::IMAP.new(@receiver_options) | ||
end | ||
|
||
describe 'connection' do | ||
|
||
it 'should connect to a IMAP server' do | ||
@receiver.connect.should be_true | ||
end | ||
|
||
it 'should disconnect from a IMAP server' do | ||
@receiver.connect | ||
@receiver.disconnect.should be_true | ||
end | ||
|
||
end | ||
|
||
describe 'message reception' do | ||
before do | ||
@receiver.connect | ||
end | ||
|
||
it 'should get messages and process them' do | ||
@processor.should_receive(:process).twice.with(/test/) | ||
@receiver.get_messages | ||
end | ||
|
||
it 'should delete the messages after processing' do | ||
@receiver.get_messages | ||
@receiver.connection.search(:all).should be_empty | ||
end | ||
|
||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# From https://github.com/mikel/mail/blob/master/spec/spec_helper.rb#L192 | ||
|
||
class MockIMAPFetchData | ||
attr_reader :attr, :number | ||
|
||
def initialize(rfc822, number) | ||
@attr = { 'RFC822' => rfc822 } | ||
@number = number | ||
end | ||
end | ||
|
||
class MockIMAP | ||
@@connection = false | ||
@@mailbox = nil | ||
@@marked_for_deletion = [] | ||
|
||
def self.examples | ||
@@examples | ||
end | ||
|
||
def initialize | ||
@@examples = [] | ||
2.times do |i| | ||
@@examples << MockIMAPFetchData.new("To: test@example.com\r\nFrom: chunky@bacon.com\r\nSubject: Hello!\r\n\r\nemail message\r\ntest#{i.to_s}", i) | ||
end | ||
end | ||
|
||
def login(user, password) | ||
@@connection = true | ||
end | ||
|
||
def disconnect | ||
@@connection = false | ||
end | ||
|
||
def logout | ||
@@connection = false | ||
end | ||
|
||
def select(mailbox) | ||
@@mailbox = mailbox | ||
end | ||
|
||
def examine(mailbox) | ||
select(mailbox) | ||
end | ||
|
||
def uid_search(keys, charset=nil) | ||
[*(0..@@examples.size - 1)] | ||
end | ||
alias :search :uid_search | ||
|
||
def uid_fetch(set, attr) | ||
[@@examples[set]] | ||
end | ||
alias :fetch :uid_fetch | ||
|
||
def uid_store(set, attr, flags) | ||
if attr == "+FLAGS" && flags.include?(Net::IMAP::DELETED) | ||
@@marked_for_deletion << set | ||
end | ||
end | ||
alias :store :uid_store | ||
|
||
|
||
def expunge | ||
@@marked_for_deletion.reverse.each do |i| # start with highest index first | ||
@@examples.delete_at(i) | ||
end | ||
@@marked_for_deletion = [] | ||
end | ||
|
||
def self.mailbox; @@mailbox end # test only | ||
|
||
def self.disconnected?; @@connection == false end | ||
def disconnected?; @@connection == false end | ||
|
||
end | ||
|
||
require 'net/imap' | ||
class Net::IMAP | ||
def self.new(*args) | ||
MockIMAP.new | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 19-25 can be refactored significantly:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I think he might have copied an older version of
lib/mailman/receiver/pop3.rb
. Will do that