Open j-a-m-l opened 9 years ago
You're absolutely right, the salt needs to be externally configurable.
There are some other architectural changes which I've had in mind from using this system in production with tens of thousands of emails daily. I'm open to discuss these observations I have if you're open to implementing some of them.
Ok, so what are your ideas? I think that your experience would be very valuable, so I could develop some of them, specially the ones that are useful to my engine.
Here are some thoughts (sorry it turned a bit long):
When you have a few tens of thousands of emails to send daily, enqueueing them for sending means taking these records from the list of subscribers and inserting corresponding rows in queued_mails
, going through the ActiveRecord
class QueuedMail
, creating objects, executing callbacks, etc. As one can imagine, this is slow.
However, I don't see an easy way to avoid it except to use a totally different type of queue, which would probably mean a total redesign of this library.
I am fine to live with the current situation, since QueuedMail
s are deleted after sending, so the table never grows huge. It's just slow.
If you have any ideas on how to speed that, though, I'd be open to discussion and implementations.
After sending, each email goes into the finished_mails
table via the FinishedMail
model. This table constantly grows. It is currently used for some simple "open" tracking and for unsubscribe purposes.
It also did contain the full interpolated unique body text, but at one point I added an option to not store these texts as the table quickly grew pretty huge.
This is not enough, though, and I'd like the option to not save any records in the finished_mails
table at all.
I think it is possible by:
message_key
when the option is set (just interpolate it to a blank string).mailing_list_id
and email_key
interpolation variables. They do not need an existing record in finished_mails
. Even email_key
would be enough, but having the option to also pass the mailing_list_id
in the unsubscribe URL gives the opportunity for one-click unsubscribe from a certain mailing list.mail_campaign_id
, plus optionally email_key
. Of course, without am actual FinishedMail
record there will be no built-in protecttion against the same people opening the same email more than once. Theoretically, opens might exceed sent emails count. I am fine with that, though, as it will be the case only when the option to not store finished_mails
in the DB is set. One can also implement this protection in their application code, having a campaign ID and an email key. Moreover–open tracking in real life is not precise anyway since some email clients do not show images by default, or prefetch and cache them for all their clients (e.g. Gmail). The main purpose of the gem is not keeping these stats.Not storing finished mails in the DB will mean no constantly and quickly growing tables even if one sends lots of emails daily. This is my top priority.
There are some other options which could also be made configurable (e.g. Smailer::BOUNCES_PREFIX
and some other salts). These are of low importance, though.
Lots of the code can be refactored, simplified and generally improved. I'm also open to PRs related to that if you feel in the mood for it :)
I share some of your observations. I'll be recolecting new observations and creating issues about them.
Currently the
Smailer::Modesl::MailKey.generate
method is using MD5 with a fixed string and the email as salt.I think that at least the string should be configurable. What about reading that string from the configuration?
I also think that it could admit a block, so it's much more customizable.
What do you think? I would be pleased to implement it.