add support for email backends

More discussion of this idea has emerged here. There are still some details to sort out, but on the basis of the broad detail from that proposal, I'll call the idea accepted.

Make django.core.mail a package and add backend functionality

Demo backend: App Engine

Demo backend: django-mailer

This is looking good as a first cut. Some quick comments:

• The patch seems to have a lot of noise in it, especially with regards to a testenv backend that appears to have been deleted.
• The 'test' backend should probably be named after how it works, rather than where it is used. 'locmem' would be consistent with the cache framework.
• I'm not sure I'm keen on shipping a log based backend. There isn't any precedent in Django for shipping code that uses the Python logging framework. Writing to stdout in a backend called 'console' would suffice.
• I'd like to see a 'file' backend as one of the defaults we ship. It's not actually useful in itself (except maybe as a toy way to logging 500 errors to disk), but it would verify that backend settings were flexible, as you would need to provide a path name into which email messages could be written. One file per connection session would also allow for testing of the open()/close() API.
• The locmem backend should be self contained - that is, it should hold it's own collection of email messages, not rely on the (external) test framework to provide it. The test framework can set up an alias for django.core.mail.outbox -> connection.outbox.
• I can't see a use case for making TEST_EMAIL_BACKEND configurable. The only backend that the test framework can assert against easily is the 'locmem'/'test' backend.
• The get_connection loader should assume that an backend module will provide a class called EmailBackend?. As currently implemented, the class name is an option for the end developer for 3rd party backends.
• It caught my eye that you've moved EmailMessage? to it's own module; that's fine as long as the import in init.py imports EmailMessage? was intended to maintain the namespace, and not just as a convenience for mail_admins et al. At some point, I will do a double check to ensure that all the publicly documented symbols are still available under django.mail. Feel free to do the same double check yourself :-)
• Error handling: The errors returned when fail_silently=False will be inconsistent depending on the backend. I haven't given this much thought; I'm not sure what the right approach is here.
• I'd steer away from modifying existing tests (e.g., test_client). Adding to existing tests is fine, but don't change the existing test cases. Yes, the API will eventually be deprecated, but in the interim, having at test case using the historical API is good protection against regression.
• Speaking of deprecation - deprecation warnings for SMTPConnection. On first use, this shortcut should raise a DeprecationWarning?. See line 445 of django.contrib.admin.sites for an example. Docs should also indicate deprecated status of SMTPConnection.
• I appreciate that it's difficult to test the backends, the importing process really isn't the part that needs to be tested. For the purposes of testing the backends themselves, just instantiate connections directly.
• Some behaviours that could be easily tested:
  • Importing a custom backend (provide a another dummy backend implementation in the test suite)
  • That the dummy backend returns N when told to send N messages
  • That the console backend outputs a message to console
  • That the locmem backend populates outbox.
  • That the file backend writes files in the right location
  • That a second locmem connection doesn't corrupt the outbox of the first.
  • That a second file connection doesn't corrupt the outbox of the first

It's good that you can override send_messages (plural), but most backends will probably only need a send_message (singular) function which gets called repeatedly. How about making the BaseEmailBackend?.send_messages() function simply call send_message()? Maybe it could even try to call open() and close() if those functions are defined?

Regarding fail_silently, couldn't this be moved out? If an exception is raised by send_messages it simply gets ignored.

Regarding the log backend: This can be useful on App Engine and other systems which don't allow for writing to the console.

Andi, thanks a lot for working on this!

Replying to wkornewald:

It's good that you can override send_messages (plural), but most backends will probably only need a send_message (singular) function which gets called repeatedly. How about making the BaseEmailBackend?.send_messages() function simply call send_message()? Maybe it could even try to call open() and close() if those functions are defined?

I think that this would complicate things. The connection handling needs to be implemented in both methods then as both methods need to know the current connection state (if any) for doing their job right.

Regarding fail_silently, couldn't this be moved out? If an exception is raised by send_messages it simply gets ignored.

This would change the current behavior and available keyword arguments to a lot of functions and there are cases where it's good to know when mail delivery has failed.

Regarding the log backend: This can be useful on App Engine and other systems which don't allow for writing to the console.

I don't see a real need for App Engine. The SDK already logs messages to the console and if someone still wants to log in production environment it'd be a single additional line in a custom App Engine backend. For other systems I'm not sure. However, writing such a custom backend would be trivial.

Andi, thanks a lot for working on this!

Replying to andialbrecht:

Replying to wkornewald:

It's good that you can override send_messages (plural), but most backends will probably only need a send_message (singular) function which gets called repeatedly. How about making the BaseEmailBackend?.send_messages() function simply call send_message()? Maybe it could even try to call open() and close() if those functions are defined?

I think that this would complicate things. The connection handling needs to be implemented in both methods then as both methods need to know the current connection state (if any) for doing their job right.

I don't understand. What do you mean with "both methods"? To clarify, I think there are a few places with unnecessary code duplication:

Most backends will handle messages one-by-one, so you have to repeat the looping code in send_messages in each backend.

Backends with open() and close() methods have to manage their connection, manually, so you'll call open() and close() in every backend, but IMHO they could as well be called, automatically, so you don't have to repeat the connection management code (e.g., in send_messages) in every backend.

Every backend has to re-implement the handling of fail_silently in send_messages.

So, with something like the following you could get rid of the whole send_messages() implementation and simplify the _send(), open(), and close() functions in almost all backends to the minimum required code (at least we fully abstract the fail_silently code):

class BaseEmailBackend(object):
    def __init__(self, fail_silently=False, **kwds):
        self.fail_silently = fail_silently

    def open(self):
        pass

    def close(self):
        pass

    def send_messages(self, email_messages):
        num_sent = 0
        for email_message in email_messages:
            if self.safely_send_message(email_message):
                num_sent += 1
        return num_sent

    def connect_and_send_messages(self, email_messages):
        try:
            self.open()
        except:
            if not self.fail_silently:
                raise
            return
        num_sent = self.send_messages(email_messages)
        try:
            self.close()
        except:
            if not self.fail_silently:
                raise
        return num_sent

    def send_message(self, email_message):
        raise NotImplementedError

    def safely_send_message(self, email_message):
        if not email_message.recipients():
            return False
        try:
            self.send_message(email_message)
        except:
            if not self.fail_silently:
                raise
            return False
        return True

For example, now the default email backend's _send() function can be reduced from this:

def _send(self, email_message):
    if not email_message.recipients():
        return False
    try:
        self.connection.sendmail(email_message.from_email,
                                 email_message.recipients(),
                                 email_message.message().as_string())
    except:
        if not self.fail_silently:
            raise
        return False
    return True

To this:

def send_message(self, email_message):
    self.connection.sendmail(email_message.from_email,
                             email_message.recipients(),
                             email_message.message().as_string())

IOW, all functions should only implement their core behavior and expect pre-checked data instead of adding repetitive management and validation code to each backend.

Replying to wkornewald:

Sorry, if I didn't make this clear enough. Your proposal exposes a new send_message() method. That method needs to be documented so that backend authors know what they have to change to make their backend work. And if it's documented client code may use it and therefore it needs to implement connection management too. IMHO there should be only one method in backends to send emails. I agree that having send_messages() only means implementing connection management (if any), exception handling and data checking for each backend. OTOH this allows backends good control over those aspects - think of a logging backend for development where it might be good to know that a message without recipients was generated somewhere in the application or a backend that has to take care about with thread-safety.

Regarding "connection": I think that's a term we should be careful with. There's only one backend (smtp) that actually has a "connection". Other backends may have a totally different concept of "connection", if any at all. For me connection handling looks very backend-specific and should be fully implemented in the backend.

If someone needs more control he can jump to the "advanced" section of the documentation and override at the desired API level. For example, if the default connection management code doesn't fit you can just override that. I don't see your concern of losing control, here. Just make the whole API public and documented. The point for me is that most backends don't need that level of control and thus should be quick and easy to implement. Am I missing something? Well, since this is just a small and rarely needed API I won't try to convince you any further. Better we finally get your code integrated and concentrate on app engine db support. ;)

Replying to russellm: Russell, thanks for having a look at the patch and for the comments. I've uploaded a new version of the patch with the following changes:

The noise about testenv is now removed, I've mistakenly uploaded a patch that was a bit out-dated, sorry...

'test' backend is renamed to 'locmem' and the logging is replaced by a console backend as you've suggested. Additionally I've added a 'file' backend that works on a per "connection" basis. The destination directory can be specified by settings.EMAIL_FILE_PATH or a file_path keyword argument when calling get_connection().

The file backend has proved that a backend needs full control in send_messages(). The file backend needs to acquire a thread lock before it opens the file stream and releases it after closing. If it just would acquire the lock when writing a message to the stream, the stream may be closed in a different thread while iterating over the messages in another. Thread-safety is an issue with SMTP backend too. In real world the chance that the problem occurs seems to be quite low as it can only happen when a connection is shared between EmailMessages? that are send in different threads. However this little script illustrates the problem:

import thread
from django.core.mail import EmailMessage
from django.core.mail import SMTPConnection

def break_it():
    conn = SMTPConnection()
    for i in range(10):
        msg = EmailMessage("Subject %d" % i, "Content", "from@example.com",
                           ["to@example.com"], connection=conn)
        thread.start_new_thread(msg.send, tuple())

if __name__ == '__main__': break_it()

The locmem backend is now self contained, but I ran in some troubles setting the alias. That would require much more patching of django.core.mail to always return the same connection instance when running test suites. Now the locmem backend first appends the message to it's own outbox and then checks if mail has an "outbox" attribute. If so, it stores the messages there, too. BTW, it's mentioned in topics/testing.txt that it's ok to manually empty mail.outbox by just assigning an empty list. So this behavior wouldn't break existing tests.

TEST_EMAIL_BACKEND is removed again. I first thought it would be useful to test email backends. But it's much easier by just instantiating them directly... :)

The get_connection loader now behaves like the cache loader and expects a module with a class called EmailBackend?.

Regarding django.core.mail namespace: The imports in init.py are to provide the same namespace as before. Crossing my fingers that your double check has the same results as mine :)

Error handling: I think it's good to treat this the same as with database backends and not to modify the original exceptions. For example the smtp backend may raise various exceptions with useful information where advanced applications may want to handle them differently (assuming that those applications depend on the smtp backend).

Reverted the modification in existing unittest. Thanks for pointing at this, modifying existing tests isn't a good idea at all :)

SMTPConnection is marked as deprecated in the docs and upon initialization a DeprecationWarning? is raised.

Additional test are implemented as suggested.

Ok - this is getting pretty close to ready for trunk.

Some minor comments:

• EMAIL_FILE_PATH isn't documented as a setting (it's mentioned in the docs for the file backend, but not in the settings docs)
• Use the .. deprecated:: 1.2 tag to annotate deprecated features
• I think the filenames produced by the file backend would benefit from being a little more humane. I can see why you've used a hash, but a filename that exposed the actual datestamp plus thread id and/or count would be a lot easier for a human to use - e.g. ,YYYYMMDD-HHMMSS-THREAD.log, which would yield 20090901-100324-2.log, would sort easily under normal a normal ls.
• Also - Django provides django.utils.hashcompat to abstract the md5 import issue. This ceases to be an issue if we move away from hash-based file naming, but I thought I'd mention it anyway.
• file is a reserved word in Python, so using file as a module name isn't a good idea. c.f. the cache backend, which names the backend 'filebased'. This is why cache.BACKENDS is a dictionary, not just a list/tuple.

I have two slightly larger comments - both related to the interaction with the test system.

Firstly, I can see your point about the testing outbox. In retrospect, you may have had the right approach in your first patch - i.e., making the locmem backend a simple interface to a common mail.outbox shared by all locmem instances. However, instead of relying on the test system to create mail.outbox, the locmem backend should check for the outbox when the backend is constructed, and create the outbox if it doesn't exist. The test system can then simply instantiate the locmem backend, and flush mail.outbox when a new test starts. This will necessitate a slight change to the documentation about mail.outbox, since it is no longer a pure testing artefact.

Secondly, I noticed an interesting potential anomaly. During test conditions, the test system will always capture mail that is sent using get_connection(), as well as mail that is sent using the deprecated mail.SMTPConnection. However, users can still manually instantiate smtp.EmailBackend(), and that mail _won't_ be captured.

I'm uncertain how much I'm concerned about this. If you're digging into internals, unusual behaviour should probably be expected. However, it could be interpreted as a change from the way mail.SMTPConnection has historically worked - i.e., that "mail" can always be tested.

At the very least, this limitation should be documented.

However, it may be possible to work around the problem. For example, we could add a 'backend' argument to get_connection(). backend would assume the value of settings.EMAIL_BACKEND by default, but you could manually call get_connection('smtp') to get an instance of the SMTP backend. This would become the preferred means of instantiating a particular email backend connection. Then, under test conditions, get_connection('smtp') would actually return 'locmem' backend.

This approach has some similarity with the cache backend - django.core.cache.get_cache() accepts a uri, which is used to produce an actual cache backend. django.core.cache.cache is an instantiation of the cache specified in settings.

I'd be interested in hearing opinions on whether this is worth the effort, and any alternative approaches.

The locmem backend was changed to automatically create mail.outbox if it doesn't exist and to store the messages just there. The per backend outbox was removed again to not store the messages in two places and make mail.outbox the default destination for messages. But I think the test environment still needs to create mail.outbox to not break tests that verify that no mail was sent - think of a Profile model that has a "don't send me any e-mail" flag, it would check that mail.outbox is an empty list and ideally never calls send_messages or intantiates the backend during the test.

The file backend module was renamed to filebased and the mail.BACKENDS turned into a dictionary to still provide the handy "file" shortcut. The filenames are now more human readable. One file per connection is created. The filename format is "YYYYMMDD-HHMMSS-INSTANCE_ID.log". EMAIL_FILE_PATH is documented in the settings docs.

I've added the "backend" parameter to get_connection. I think it's an good idea to make this the default way to instantiate backends and it would allow custom backends to wrap existing backends very easily. For example a email backend that stores messages in the database before sending them over SMTP doesn't have to deal with internals then; it would even be possible to make this configurable without re-implementing the backend initialization.

Regarding the issue with directly instantiating backends when running test suites: I think it's ok to be not to restrictive here. IMO it's enough to patch all documented methods in django.core.mail to return the locmem backend, i.e. get_connection and for backwards compatibility SMTPConnection. The docs don't mention how to import a specific backend directly. ATM the EMAIL_BACKEND option and the get_connection() method as the preferred way to get a backend connection (added in this changeset) are documented and I think it should be enough to setup the test environment only for these.

RC1 of email backend patch

I've just uploaded my RC1 for this ticket. It's almost exactly the same as Andi's most recent patch, with a few minor cleanups here and there, and some polishing on the docs. Feedback welcome; I intend to land this in the next couple of days.

This generally looks great. A couple minor points of criticism:

• I'd prefer EMAIL_BACKEND to always be a full path (e.g. django.core.email.backends.smtp instead of just smtp). I regret the decision in other places in Django to treat built-in backends "specially", and everyone's gotta look up valid options in the docs anyway. It'd make the code (slightly) simpler, too.
• I'd be nice to add a backend argument to send_mail and friends. That'd make sending mail using different backends nicer.
• The console backend probably shouldn't hardcode sys.stdout; seems it should be possible to direct it to a different stream (sys.stderr, a fake output stream, etc).

None of these are big problems. The first should be addressed (one way or the other) before merging to avoid moving APIs, but the other two could be addressed post-merge if needed.

(In [11709]) Fixed #10355 -- Added an API for pluggable e-mail backends.

Thanks to Andi Albrecht for his work on this patch, and to everyone else that contributed during design and development.