Issues

ZF-1322: Zend_Log_Writer_Mail to send log messages to an email address

Description


class Zend_Log_Writer_Mail
    extends Zend_Log_Writer_Mock {
    
    private $mail = null;
    
    public function __construct(Zend_Mail $mail) {
        $this->mail = $mail;
    }
    
    public function shutdown() {
        if (!$this->shutdown) {
            try {
                if (count($this->events)) {
                    $body = $this->mail->getBodyText();
                    foreach ($this->events as $i => $event) {
                        $body.= $this->_formatter->format($event);
                    }
                    $this->mail->setBodyText($body);
                    $this->mail->send();
                }
            } catch (Exception $e) {
                // loggen ?
                throw $e;
            }
        }
        parent::shutdown();
    }
    
}

Comments

Zend_Log related issue, assigned to Mike.
Must previously be accepted by the devteam for inclusion.

Changing to 'Unassigned'

Personally, I would move the count() out of the try block and have it as the first if (). Then clear the events after the e-mail gets sent. That way if someone called shutdown and then added more events they would get sent as well.

Also, maybe add optional header and footers would be nice too. That way you can add more then just a log of events. My need would be to make a table with other descriptions. I'll try to add a screenshot of what I typically e-mail. I'll also try to write an update to this patch for these features.

Example of what e-mails look like from my error logs. If the optional header and footer is added, I can turn the list of events into a HTML table.

Ok, here is what I've put together. Changes since the original patch:

  • Removed shutdown check, rather checks for events
  • Removed the try/catch block, Zend_Mail will not throw exceptions anyway
  • Changed mail property access to protected (better support for extending the class)
  • Changed $mail to $_mail (seems to be the "Zend" way for naming protected properties)
  • Added support to add a header and footer
  • Added support to change body type to HTML
  • Added header, footer, and html properties as public, shutdown() can deal with improper values

Also, I removed my thought about multiple shutdowns, it's unlikely to happen anyway.


class Zend_Log_Writer_Mail extends Zend_Log_Writer_Mock
{
    protected $_mail;
    
    public $header;
    public $footer;
    public $html;
    
    public function __construct(Zend_Mail $mail, $header = '', $footer = '', $html = false)
    {
        $this->_mail = $mail;
        $this->header = $header;
        $this->footer = $footer;
        $this->html   = $html;
    }
    
    public function shutdown()
    {
        // Check for events
        if (!empty($this->events)) {
            
            // Start body, add previous content
            $body  = ($this->html ? $this->_mail->getBodyHtml(true) : $this->_mail->getBodyText(true));
            
            // Add header, cast as string in case non-string set
            $body .= (string) $this->header;
            
            // Add each event and apply the format
            foreach ($this->events as $event) {
                $body .= $this->_formatter->format($event);
            }
            
            // Add footer, cast as string in case non-string set
            $body .= (string) $this->footer;
            
            // Determin e-mail type and add body
            if ($this->html) {
                $this->_mail->setBodyHtml($body);
            } else {
                $this->_mail->setBodyText($body);
            }
            
            // Send e-mail
            $this->_mail->send();
        }
        
        // Call parent, will set shutdown to true
        parent::shutdown();
    }
}

Should you may be a little more careful with the "html" data member?

For example, if I set the "html" data member to some random object, won't that cause the setBodyHtml() call to barf somewhere along the way?

Maybe do an is_scalar() check before that "if ($this->html)" check in shutdown()? That or cast it to a string as you do with "header" and "footer".

Though, personally, I'd tend to prefer getHtml() and setHtml() instead of public data members.

Ah, nevermind about my previous comment. It's a non-issue due to the ternary expression for setting $body above.

I had thought about that but decided it shouldn't be a big deal. If like you say, the html property is set to an object, the if statement should just count it as true. The value is not actually used anywhere.

I also like to force getters and setters but it also seems to be the "Zend" way to keep things rather open. After all, PHP is supposed to be very flexible with their data types. This is an example of why it really doesn't matter. It's the users fault if they set html to anything other than boolean. [End rant] :)

Eh, personally, I tend to agree with making "header", "footer", and "html" public data members. I'm by no means a Jedi Master on Zend Framework's preferences when it comes to these types of decisions, but for the sake of a clear interface, I'd prefer to see the getters and setters rather than have willy nilly access to the public data members.

Quick, rough stat from trunk: out of 1,081 *.php files, I see that only 86 have public data members, which would lead me to believe that this isn't a common practice. A large portion of those 86 come from Zend_Form and Zend_Service.

Again, this would just be my personal preference for the sake of a clear interface.

Actually, I'm glad they don't use public members throughout the framework. I like how you added the stats in there. :) I am very new to ZF and just started to work with it. The log was the first thing I am working on and this is what was missing.

Also, digging into HTML in e-mail a bit more, I wonder if setting a HTML body and text body would be a nice feature. Then set the $_mail->setType() to Zend_Mime::MULTIPART_ALTERNATIVE. That way the e-mail client can choose which to use. Of course the HTML would have to be striped for the text body. Or is this getting into more than is worth? :D Will attach a new patch with this feature. Are these patches close to what you were thinking?

Also, I see that this issue tracker ticket was opened in May 2007, while I proposed Zend_Log_Writer_Mail in January 2008.

http://framework.zend.com/wiki/display/…

Maybe there's some overlap here; just wanted to point out the parallel thinking going on in this area.

Changes since my last patch:

  • Removed html data member
  • Added feature to auto add html body and text body

New file uploaded, changes since first file:

  • Changed E-mail Template to Zend_Layout
  • Removed Dependency of Zend_Log_Writer_Mock

What is the diff in functionality between this patch and the proposal? I'd like to coordinate efforts, if possible.

Nothing from what I can tell, just coded a little differently. The latest proposal skeleton class has the same features, and more. At least between the patch that I created, Zend_Log_Writer_Mail-Layout.txt.

Hey Wil,

Good question! I'll summarize the main differences below. My proposal:

1) ...allows for two formatters -- one for the plaintext entries, and one for the HTML entries. This allows you to end the HTML entries with, say, a "
" or any other appropriate line ending, rather than forcing the user into ending with line break tags.

2) ...allows for the message subject to be appended with the summary of errors per-error level, if the caller provided "subject prepend text."

3) ...catches Zend_Mail exceptions and re-throws them at the proper level of abstraction

Originally, the class in this big report extended Zend_Log_Writer_Mock, which I did not agree with. As in the revised version of this bug report, it, too, now extends Zend_Log_Writer_abstract.

I also never actually saw the second file attached to this bug report until today, so I didn't notice some of those similar decisions. There is quite a bit of overlap between the patches here and my proposal, though personally, I tend to think that mine's a bit more "ready for primetime" as the proposal is now "Ready for Recommendation."

Let me know if you'd like any further feedback or thoughts. Thanks for taking the time to consider these two options!

Reassigning to Brian, since he has a proposal here: http://framework.zend.com/wiki/display/…

Wrote unit tests with 100% code coverage as well as reference guide documentation on 1/1/2009.

Sent along to Matthew Weier O'Phinney for review and/or committal to the Standard Incubator.

Zend_Log_Writer_Mail, a test class for it, and documentation were all committed to the Standard Incubator as of 9:55 AM ET, 1/7/2009. See r13532.

Zend_Log_Writer_Mail, its unit tests, and its documentation additions were promoted to trunk on 1/14/2009 by Matthew Weier O'Phinney.