Forum OpenACS Development: Re: Design Issues in acs-mail-lite

Collapse
Posted by Malte Sussdorff on
Revert the change in notifications. It was premature to switch to using complex_send. Complex send is not compatible with ::send and making it such will be complicated. If you want to keep it though, make sure to add a switch that only uses the new complex_send method if SendmailBin is set to SMTP, otherwise we cannot be sure our users actually have the required SMTP server running.

At the moment we have three different designs for handling incoming e-mail, all of them working to a certain degree. We should best come up with one generic solution and until this can be achieved without rewriting tons of code, we should offer the alternatives in different packages (notifications::load_qmail_mail_queue, acs-mail-lite::load_mail_dir and (currently) acs-mail-lite::load_emails). Not to mention that notifications incoming is a rewrite of acs-mail-lite::load_mail_dir incoming and a lot of procedures have been copy pasted. So a generic cleanup in notifications is in order anyway.

We have two fundamentally different approaches to sending out e-mail, all in one package. Unifying both might be a little bit hard, but it should be our goal to have one single send procedure to handle all the options and maybe span it out into seperate sub procedures.

Here are some fundamental issues with this:

- Complex_send at the moment relies on smtp::sendmessage. As this implies, it does not use the acs-mail-lite::deliver procedure which is the one supporting both SMTP as well as the sendmail binary. Complex send at the moment only supports SMTP sending.

- Complex send does not send the correct MAIL FROM parameter which is set to the bounce address. The way it is done with ns_smtp is not supported by smtp::sendmessage. It does support a different reply to address, but this is not something you want to set to the bounce address.

- Complex_send is still undergoing constant development to add new switches which are useful in sending out e-mail. Though these could be passed (a lot of the time) as extraheaders, you have to know th RFC to pass the correct extra headers, much easier to use a TCL flag.

- Until you support incoming e-mail, you cannot test outgoing e-mail. Therefore writing test procedures has to include the correct setup for handling incoming e-mails. As the handling of incoming email is not unified, we should start there. I would probably just write a testcase sending e-mails to "mailto:acs-mail-lite-test@yourserver.com"; and then have a callback for acs-mail-lite::incoming_email to deal with these test e-mails. Though I'm not entirely sure how I would test how exactly to write these tests, so if someone is willing to sit down with me to write some tests I can make sure I write up how to setup the environment for these tests.

- Complex send could use some cleaning up, as some code parts should probably go into seperate procedures. As an example the whole code for getting the members of a list of groups should probaly folded in group::get_members which would take a list of party_ids instead of a group_id. And maybe someone else will find other things to clean up as well.

- Complex send supports sending e-mails from multiple servers. So if you put your e-mail into a queue, you could have multiple webservers in your cluster take and process the e-mail instead of only one (as with acs-mail-lite::send). This definitely needs some more testing, as we so far only tested it with the main webserver and a seperate server dedicated to sending out e-mails and server static webpages, so not in a real cluster environment as you know it from OpenACS.

Taking out complex_send from acs-mail-lite is a piece of cake as it already comes in it's own -procs.* files. So if it is deemed that the whole complex_send* stuff should be tested a lot more before put into acs-core, this is the time to split it off in HEAD. If this is prefered by the OCT I would in the same time move the incoming e-mail procedures (the new ones) over to this package and then acs-mail-lite could stay juvenile as it was before this functionality was added. It would make dealing with notifications a lot easier, as you could just check if complex_send is installed and if yes, then use it. I would then go and get rid of the notifications incoming e-mail handling if complex send is installed.

Maybe this should have been done right from the beginning, but at that point acs-mail-lite was not core, so I think noone thought about it (and to be honest, I did not think we would continue to make so many changes after the incoming e-mail system was announced and discussed at https://openacs.org/forums/message-view?message_id=316225)