How to delete mail after downloading? about mail HOT 35 CLOSED

I would like to request to put the valid examples (maybe some way refactored) into the official readme.

It is hard to search trough this issue every time and only to fetch the valid examples (final implementation).

from mail.

This isn't implemented yet... I know... crazy

I need to put it in. I'll try and get it out for the next version

Mikel

from mail.

Hi Mike, thanks for the work so far. I've just implemented a script that distributes my mail. Now I also like to delete the distributed message. I'd like to help but I would propably do more harm than good. Anyway, do you have any idea on when deleting messages will be implemented?
Thanks very much
Hendrik

from mail.

Hi Mike, thanks for Mail !!!

It is stated in Current Capabilities of Mail, that it has:
Wrappers for File, Net/POP3, Net/SMTP.

Net::POPMail has delete, delete! and deleted? methods, and it states:
"This class represents a message which exists on the POP server. Instances of this class are created by the POP3 class; they should not be directly created by the user. "

How can I access the wrappers for this methods via Mail?

from mail.

Any idea when you are planning on implementing this? I am considering using Mail but the inability to delete messages once they are read is the only thing holding me back.

-JP

from mail.

I was actually just looking through the files/methods to see how to delete an email when I came across this post. So I'd also like to add my support/vote for this feature request.

In any case, thank you (and the contributors) very much for such a great, clean gem!

from mail.

I'm very interested in this too. Like vrmex said, it's stated in the docs that it has Wrappers for Net/POP3 which has delete methods.

If this is a wrapper, delete should be available already, how do we access it?

from mail.

+1 for a solution to this:)

from mail.

This is indeed "crazy".

While there are use-cases that won't mind, if using this library to automate email processing they will have to be deleted/marked as read or otherwise marked as being already processed.

For the time being, I am storing the processed messages ids and then deleting them with another library.

from mail.

I have a delete_all_messages and a delete_message(message) function in my fork. For those interested:

#beta, mostly from spec
def delete_message(letter)
start do |pop|
if letter
File.open("inbox/#{letter.message_id}",'w') do |f|
f.write letter.pop
end
letter.delete
else
puts "Letter is invalid."
end
end
end

#beta, straight from spec
def delete_all_messages()
  start do |pop|
    if pop.mails.empty?
      puts 'No mail.'
    else
      i = 1
      pop.delete_all do |m|
        File.open("inbox/#{i}", 'w') do |f|
          f.write m.pop
        end
        i += 1
      end
    end
  end
end

from mail.

The start method of Mail::POP3 clears all erase flags when deleting, see http://github.com/mikel/mail/blob/master/lib/mail/network/retriever_methods/pop3.rb#L143,so including these functions in that class won't work. I use this for the time being:

    module Mail
      class POP3
        def delete_all_messages()
          start do |pop|
            unless pop.mails.empty?
              pop.delete_all 
              pop.finish
            end
          end
        end
      end
    end

The finish call ensures the messages get deleted before the block returns to the start method.

from mail.

martijn, that code looks nice.

The only bad thing is that it will indiscriminately nuke all messages on the server. For a webservice I am creating, this includes messages that were not successfully processed and even messages that just arrived in the servers but I haven't seen yet.

It would be nice if the Mail gem supported deleting a single message, as they are processed.

from mail.

Hi there spedrosa,

Please do implement. You just need to add code to the lib/network/retriever_methods/pop3.rb... it is fairly simple. The hardest part is writing specs for it.

Mikel

from mail.

I forked and implemented delete_all plus specs. I'll look into delete by message as well, shouldn't be too hard as the code is very well written :)

from mail.

Awesome, I'll merge in shortly

from mail.

Merged and pushed in gem 2.2.5

from mail.

Hmm.. deleting single messages isn't that easy since we have to find the message on the POP server again in order to mark it for deletion.

We could compare the raw_source of the Mail::Message object with each mail on the server and delete the mail if it matches, but that sounds like a bad idea. We could use the UIDL extension if available, but then we'd have to store the UID in the Mail::Message.

I think a good way would be to compare the Message-Id header of both objects, since that should be 100% unique. Any thoughts, anyone?

from mail.

Hi martijn,

Thanks for implementing delete_all, and for working on the single message delete. I think it would be good to store the UIDL with the message, and that using the UIDL to verify which message to delete would be slightly more robust. It's a fairly small string, relatively speaking, and could be useful in other applications anyway (e.g. building a mail client that leaves emails on the server, and only retrieves new ones on update... like what they do with desktop mail clients.)

I guess the main problem is figuring out which email to actually delete between sessions. One possibility to use the server's email id number and message size as a "first-line defense," since we get that information with the LIST command anyway. E.g. (private) @server_message_id and (private) @server_message_size?

(I'm suggesting storing the server size because the current method of calling mail.encoded.size doesn't seem to return the same value as the server's LIST command. That, and it's much cheaper to store and retrieve an integer than it is to encode a Mail object, and count its length.)

Anyway, here's what I'm thinking for this:

1. When Mail downloads an email, it can store the ID and size of the message (@server_message_id and server_message_size), as given by the LIST command.

These should probably be internal values that can't be accessed by users of the class. (Side-Note that the LIST command could also have other info after the size, although it's discouraged; so only the first two columns of data should be stored. RFC 1939 has more details).

2. When a user calls #delete on a particular Mail object, the following is performed:

• Reconnect to the mail server and run LIST, and build a two-column array (or a hash) for the id and size of each message (e.g. "messages_on_server).
• If messages_on_server.length < @server_message_id, then (1) check If messages_on_server[@server_message_id] == @server_message_size, If so, (2) confirm that it's the correct message by checking the UIDL, and if yes, (3) delete the message.
• If messages_on_server[@server_message_id] != @server_message_size, then the email numbering was changed due to other email additions or deletions. An easy way to find the appropriate message would be to check the messages_on_server array for all elements that have a value equal to @server_message_size. These messages are the only candidates that can match the email we're trying to delete. So then, we check each of these message's UIDL's, and once we find the appropriate one, delete it.

It's possible, though, that the server doesn't generate UIDL's for their emails, in which case, the above method would still work, but replacing the UIDL with the Message-ID header. The odds of an email having the same size and sender generated Message-ID header are quite small, so although the Message-ID isn't necessarily unique, it should be robust enough.

As a side note, it would be great if the #delete method worked on an array of Mail objects, like the one returned by Mail.find(). Actually, it may be easier to even write #delete so that it can only be called on an array, since it's much more efficient to delete a list of emails in one server connection, rather than reconnecting multiple times to delete emails separately. Then, if a user wants to delete just one email, they can just do something like:

single_message_as_array = [email]
single_message_as_array.delete

Or,

[email].delete

(Which could be placed in a Mail method for a single delete).

Anyway, those are my thoughts on this. Hopefully this makes sense. I think it'll work, but I'm not a Ruby or email expert. Thanks again for working on this feature, it'll be a great addition and will be very useful!

from mail.

Thanks for thinking with me, M7. I definately would like to implement Mail::Message#delete, but it's a bit tricky since we'd have to verify that the mailbox we're trying to delete from is the same as the message was originally downloaded from. So I'm postponing that for now.

The UIDL + message_size or mesage_id + message_size in case a server doesn't supply UIDs looks like a good idea. I guess there's no harm in a @server_message_uid and @server_message_size variable in the Message class. I'll pick this op in the next couple of days.

from mail.

Hi martijn,

Ah, good point on verifying that the mailbox is the same on delete. I was thinking about this, and I think that using @server_message_id and @server_message_size could also act as a verification, since the odds of two emails on separate servers having the same @server_message_id and @server_message_size is extremely unlikely. Also, for a final confirmation, the UIDL could be used.

Do you think that would work?

from mail.

Awesome thread going here guys.

One thing to keep in mind though, is that Mail itself has to draw the line on cleverness :) Basically, we are a library that other people and applications wrap to get their desired functionality.

The problem with message_id is that it CAN change on each connection, reading through the RFC (http://tools.ietf.org/html/rfc1081) will give you some ideas on this.

The other thing is that there is already a POP3 library in Ruby, Net::POP3, we should just wrap this and try not to reinvent the wheel. Good RDocs on the dangers should be in the mail code, advising the implementor to make sure they are within a single POP3 session to delete messages, this could be done within a block or the like.

Anyway, just a heads up, try not to get too clever and data integrity is important.

Mikel

from mail.

Yeah that's why I dedided not to look at a delete function on the Message class (at first), since that would require too much cleverness. I think a delete function on the retrieval method should be ok though, this gives the implementor full control. I'll have to read up on UIDL to see if it's unique in the same way as IMAP's uids.

By the way when I was referring to the message id I did not mean the ID on a pop3 protocol level (since those can change indeed), but the Message-Id header of the actual mail message. I know it seems cumbersome, but at the moment I think that's the only way to reliably identify a message (along with maybe the UIDL). I'll read up on the RFC's some more, though.

from mail.

Yes, you can assume that the Message-ID in the header of the email is unique across all email messages...

This is a basic requirement of RFC-2822.

Mikel

from mail.

Hey guys,

Actually, I'm still referring to the message_id that comes through with the LIST command. :) I wasn't taking for granted that it could change between connections though...

It looks like Net::POP3 only has a delete_all or delete-within-the-download-block style deletion. I'm not sure if this will be helpful in some use-cases. For example, what about users who want to download the email, close the connection, process it via a separate thread or whatever, and then only delete it if everything's okay? The way Net::POP3 does it, they'd be forced to add in their processing code within the block, which could be something that takes a while to do and would force the server to keep the POP connection open, and a long running process could cause a timeout.

I'm not saying we should reinvent the wheel, but Net::POP3 may not have the best implementation. We can make this better. :)

Part of the issue I'm seeing with relying mainly on the Message-ID header is that it'll take a long time to delete a message, since (unless I'm mistaken, and please correct me if I'm wrong) Mail would have to retrieve all of the messages/headers again to retrieve the Message-ID header before deleting. If we use the @server_message_id and @server_message_size to at least narrow down the list of messages that we have to look at, the speed would drastically improve.

For instance, what happens if a user has 1000 emails on their server, and only wants to delete three messages? Using @server_message_size, we can narrow down the number of emails we have to check to only those messages that are the same size as the ones we're trying to delete. This could turn out to be exactly three messages, or maybe a few more, but it'll be much less than 1000.

Anyway, I'm not sure if you'll agree, but here's what I'm thinking for the #delete method. It's a rough algorithm, and everything is placed in one function and it's not very DRY, but hopefully it make sense:

Notes

• As Mikel mentioned, the user should be warned not to change servers before deleting the appropriate messages. The following algorithm will still work if the user changed servers, but the deletions will fail (we can signal the user to this with an error or something?).
• As I mentioned before, I think this method should take an array of messages, like the one returned from #find. I'm not sure if it's possible to call messages.delete on that array, or if it would be better to implement it as Mail::delete(messages).
• For single message deletions, the user can call message.delete which would convert the single message to an array and then call Mail::delete(messages).
• Side-Note: In the case of a single message being deleted, we can do an internal check to see if the input is_a? Array, and if not, it can be converted to an array with messages = [messages] (assuming that the input variable was called messages).

Rough Algorithm

When a user requests to delete a message array (where each message has the UIDL, server_message_id and server_message_size stored inside), we:

def delete(message array)
  # This is in pseudocode since I don't know how to do some of this stuff
- Connect to the server
- Issue a UIDL command, which returns the server_message_id and UIDL of 
  each message on the server, or an '-ERR' if the command is invalid. The UIDL
  command is optional according to the spec, so the error condition should be 
  checked.
- If UIDL command result is not '-ERR'
  - Place the result in a hash, with the UIDLs as the key
  - foreach message in array
    - Check for an entry that matches the message's UIDL in the hash, this
      will return the message's server_message_id (which is required by the DELE
      command).
    - if we got a result from the hash lookup (meaning that the message was 
      still on the server)
    - Issue a "DELE message_id" to delete the message
    - endif 
  - end_foreach
- else # UIDL command returned '-ERR'
  - Issue a LIST command, which returns the server_message_id and the
    server_message_size. We'll use this for a trivial rejection/validation to find 
    the appropriate message to delete, instead of retrieving each message in 
    full to find the UIDL/Message-ID. Note that this command could also list other 
    information after the server_message_size column, so only the first two columns
    should be parsed out.
  - Place the LIST command result in an array
  - if array[@server_message_id] == @server_message_size
    - Get the UIDL/Message-ID-Header for current message
    - If UIDL/Message-ID-Header == @UIDL
      - message_to_delete = @server_message_id
  - end
  - if message_to_delete == 0 # or nil, or another error value
    # We haven't found the correct message to delete yet. Check for other 
    messages that could be the one we're looking for. We do this by checking the 
    size of the messages on the server, and then verifying it by ensuring that the 
    UIDL or Message-ID header matches the email we're trying to delete.
    - foreach entry in the array
      - if current element (the message size) equals @server_message_size
        - Get the UIDL/Message-ID-Header for the current message
        - If UIDL/Message-ID-Header == @UIDL
          - message_to_delete = current_message_id # (from array via foreach)
        - endif
    - end_foreach
  - endif
  - if message_to_delete == 0
    - The message no longer exists on the server, so 'continue' to the next 
      message to delete
  - else
    - Issue "DELE message_to_delete"
  - endif 
- endif
- Close the connection
  end
  

I'm still kind of new to Ruby, so any feedback would be appreciated. Please let me know if you'd me to clarify anything.

from mail.

Mikel said:
"The other thing is that there is already a POP3 library in Ruby, Net::POP3", but there is only a #delete_all, so I wonder why the Net::POP3 programmers did not implement a #message.delete, did they see something that we are not seeing?, just wondering why...?

from mail.

Well the delete is in Net::POPMail, which makes sense considering all of the above.

from mail.

Hey all,

Okay, I've implemented some new delete functionality that appears to be working well now, thank God! :)

Mikel other base contributors: Please see my pull request.

There's now two ways to retrieved messages: using #find_and_delete, or by setting the :delete_after_find option to true when calling #find. Only the messages returned by #find or #find_and_delete will be deleted. :)

The #find_and_delete method takes the same arguments as #find, but marks the message(s) for deletion after it retrieves each one. The marked messages are then deleted once the session is closed. This method just calls #find with :delete_after_find set to true, so you can call either one. I just thought it'd be good for people who prefer to explicitly show that "calling this method will delete retrieved messages."

Anyway, I hope you all find this useful. This is my first Open Source contribution, so I'm excited to get it out there! :)

Any feedback would be greatly appreciated.

Thanks
Michael

from mail.

Also, just a few notes, for those who are interested:

1. This code shouldn't break anything. The default for a regular call to #find is to have :delete_after_find set to false; so without any code changes, your apps will continue to run as they were running before this update.

2. This new functionality uses Ruby's Net::POP3 stuff, as suggested by Mikel. :)

3. There was no need to store the UIDL or anything like that, because the message is deleted during the same session, as suggested before. So it's nice and clean.

Usage Examples

Without block:

emails = Mail.find(:what => :first, :count => 10, :delete_after_find => true, :order => :asc)

or,

emails = Mail.find_and_delete(:what => :first, :count => 10, :order => :asc)

With block:

Mail.find(:what => first, :count => 10, :find_and_delete => true, :order => :asc) do |message|
    # Do stuff.
end

or,

Mail.find_and_delete(:what => :first, :count => 10, :order => :asc) do |message|
    # Do stuff
end

In each of these cases, the retrieved messages will be deleted from the server, and the other messages will be left alone.

I hope this helps,
Michael

from mail.

Cool, looks like a smart solution. I do see one possible issue with it though. In the find_and_delete method, the message always gets deleted after yielding the block. It should be possible to abort the deletion from inside the block in case of a processing failure (database error, whatever).

from mail.

Awesome work.

I agree with martijn, can you fix this up? Then we'll work on merging it in.

Mikel

from mail.

Thanks guys! That's a good point Martijn; I'll try and add that in this evening... if not tonight, then tomorrow evening.

Sorry for the delay. I hope this doesn't inconvenience anyone.

Thanks again,
Michael

from mail.

Also, how does this look (for usage):

To abort all pending deletions:

Mail.find(:what => first, :count => 10, :find_and_delete => true, :order => :asc) do |message|
    # Do stuff...
    if error_encountered
        message.abort_all_deletions
    end
end

...Or do you guys think that it should be #Mail.abort_all_deletions? I'm just thinking that calling Mail (which is somewhat stateless) from within a block like that may feel a bit weird. People will get it either way, but I think calling it on an instantiated message (which is being downloaded along with other messages) may make a bit more sense.

I could do it both ways though, I suppose.

For aborting the deletion of a single message:

Mail.find(:what => first, :count => 10, :find_and_delete => true, :order => :asc) do |message|
    # Do stuff...
    if error_encountered
        message.abort_deletion
    end
end

Here's some other options (to be called once an error is encountered):

message.abort_delete
message.cancel_deletion
message.cancel_delete
message.skip_deletion
message.skip_delete
message.do_not_delete
message.delete = false
message.mark_for_delete = false

Any preference or suggestions?

Thanks,
Michael

from mail.

Alright, I've added in a #skip_deletion method, as requested. A message's deletion can only be skipped/aborted from within a #find or #find_and_delete block though. Here's an example:

Mail.find(:what => first, :count => 10, :find_and_delete => true, :order => :asc) do |message|
    # Do stuff...
    rescue Exception => e
        # Handle exception...
        message.skip_deletion
    end
end

I chose "skip" instead of "abort" because I think "skip" makes it more obvious that it's only being done for one message. But I can change it if you guys prefer.

Any thoughts, suggestions or feedback on this? Thanks again for the great suggestions and feedback so far! :)

Cheers,
Michael

from mail.

This is now merged and pushed to master.

Thanks for all your work!

from mail.

You're most welcome! And thank you, very much, for all of your great work on this gem! :)

from mail.