Skip to content

Conversation

@klimov-paul
Copy link
Member

Here is my proposal for #31: email component

Here I have created an abstraction layer for the email mailer and message.
I have created ‘Vendor*’ classes, which should help to created wrappers over existing email libraries.
By default solution uses SwiftMailer, but you should be able to switch it to any other library such as Zend/Email without much effots.

This solution of course need some adjustments, feel free to propose ones.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling 358e013 on klimov-paul:email-swift into c3ba7f6 on yiisoft:master.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why just a number? Isn't an array of errors better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Particular libraries may have a problem with that.

@qiangxue
Copy link
Member

@klimov-paul I'm still reviewing your PR. Here are my initial thoughts about it:

  1. The design seems to me emphasizes too much in supporting alternative mail solutions with the architecture BaseMailer -> VendorMailer -> swift\Mailer, and BaseMessage -> VendorMessage -> swift\Message. But most developers I believe only use either SwiftMail or ZendMail, or perhaps they just need a solution and won't care whether it is SwiftMail or ZendMail behind. For this reason, I think perhaps we should use this architecture MailerInterface -> Mailer, MessageInterface -> Message, where Mailer and Message are implemented using SwiftMail (or ZendMail, but not both).
  2. Besides Mailer and Message, we also need these two features: message composition (creating message content from a view), and mail queue.

@yiisoft/core-developers Could you help review this PR too? The mail component should be a core app component, I think.

@klimov-paul
Copy link
Member Author

There is no need to haste with this one – we have enough time, I think. I have only meant this PR is a little scratch at the moment, but that is because its idea requires consideration.

Some architecture clarifications:

  1. ‘BaseMailer’ and ‘BaseMessage’ were created to support ‘Factory’ feature for the email message objects. Developer may need to adjust some default values for specific message properties, for example: ‘encoding’. I think this is necessary, because some mail libraries do not introduce a solid ‘Mailer’ component, while packing everything inside the message.

  2. It is unlikely developer will switch Swift in favor to Zend or vice versa. Indeed we can choose some specific library and just leave it there. The main goal of the created class hierarchy is the support of uncommon mail solutions like using Amazon SES or some web service. Such solutions can also come in ready-to-go libraries, which can be wrapped in the same way as Swift or Zend.

  3. Email content composition from patterns indeed is a common functionality. But I can not see a universal solution for it. It has 2 major problems (sections):
    – storage for the email templates: it could be files, database and so on. Also, do not forget that templates should support i18n in some way.
    – pattern language: we can use plain PHP to compose the actual message content same way we compose the view files. But developer may want to use something like ‘Smarty’ for the better syntax.
    I am not sure Yii should handle this – it could be leaved up to developer choice. Still, If we are agree framework must provide the message patterns functionality, I will attempt to compose one.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.42%) when pulling 94f4b09 on klimov-paul:email-swift into 380bb5c on yiisoft:master.

@qiangxue
Copy link
Member

1, 2). I mainly want to remove VendorMessage and VendorMailer. I don't quite like relying magic methods to delegate methods because it has many side effects. I'd rather explicitly implement the required interface methods, even though it involves some redundant code (such as methods wrapping vendor class methods).

3). What I meant is mainly about composing mail content via view template files. We don't need to consider templates in database, just like we don't need to consider normal view templates stored in database. Should we really care about that, it probably can be solved by developing a new View class, which is independent of the mailer feature, as long as the new view class follows the same interface.

@klimov-paul
Copy link
Member Author

I mainly want to remove VendorMessage and VendorMailer. I don't quite like relying magic methods to delegate methods because it has many side effects. I'd rather explicitly implement the required interface methods, even though it involves some redundant code (such as methods wrapping vendor class methods).

The problem is I don’t know which methods should compose the email message interface. It is too complicated. Email has a lot of possible headers, in common situations only a few of them are needed, but it could be situations, which require specific headers to be set. Also there are such complex problems, like embedded images, which creation and insertion in the message content is not a trivial.
Different email libraries may do such things in very different ways, I am unsure the common interface could be created for them all.

What I meant is mainly about composing mail content via view template files. We don't need to consider templates in database, just like we don't need to consider normal view templates stored in database. Should we really care about that, it probably can be solved by developing a new View class, which is independent of the mailer feature, as long as the new view class follows the same interface.

Agreed. I’ll see what I can do.

@klimov-paul
Copy link
Member Author

The problem is I don’t know which methods should compose the email message interface.

I mean I can create a email interfaces and email classes, which implement them, wrapping the Swift. But in this case, I am afraid, these interfaces would be bound to the Swift interfaces, for ill or good.

@qiangxue
Copy link
Member

I think the situation is a bit like our MemCache class which has a method getMemcache() returning the underlying implementation object so that users can directly access it to use advanced features. For us, we mainly need to define an interface supporting most common needs (methods in your BaseMessage is a good starting point, I think). This interface certainly shouldn't cover all use cases, but it should clearly define the common behavior we want.

@klimov-paul
Copy link
Member Author

What I meant is mainly about composing mail content via view template files. We don't need to consider templates in database, just like we don't need to consider normal view templates stored in database. Should we really care about that, it probably can be solved by developing a new View class, which is independent of the mailer feature, as long as the new view class follows the same interface.

It can not! Yii View class does not resolve the view name. ‘\yii\base\View’ only renders the files given by explicit file name. It does not convert view short name like ‘contact/body’ into actual file name. This means, if I would bind email message to the View component, I have to resolved view name inside the Message class itself. So if I want to store email templates inside the database, replacing the View component would not help me – I have to override the Message class.

@psihius
Copy link

psihius commented Oct 23, 2013

@klimov-paul regarding to the mail headers - just add methods for the common headers, like: From, Reply-To, Return-Path. For the other custom headers just make a method "addHeader($name, $content)" and be done with it.

As to the API and vendorMail - I looks a little bit too overcomplicated, but you and @qiangxue are handling it well :) I think that the real think is when people use Amazon services or some other could service for e-mail instead of SwiftMail with SMTP.

@klimov-paul
Copy link
Member Author

Migrated to #1049

@klimov-paul klimov-paul deleted the email-swift branch December 26, 2013 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants