Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add send_args to Log::Dispatch::Email and subclasses #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions lib/Log/Dispatch/Email.pm
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use Log::Dispatch::Output;
use base qw( Log::Dispatch::Output );

use Devel::GlobalDestruction qw( in_global_destruction );
use Params::Validate qw(validate SCALAR ARRAYREF BOOLEAN);
use Params::Validate qw(validate SCALAR ARRAYREF HASHREF BOOLEAN);
Params::Validate::validation_options( allow_extra => 1 );

# need to untaint this value
Expand All @@ -31,6 +31,10 @@ sub new {
type => SCALAR,
optional => 1
},
send_args => {
type => SCALAR | ARRAYREF | HASHREF,
optional => 1
},
buffered => {
type => BOOLEAN,
default => 1
Expand All @@ -43,8 +47,9 @@ sub new {
$self->_basic_init(%p);

$self->{subject} = $p{subject} || "$0: log email";
$self->{to} = ref $p{to} ? $p{to} : [ $p{to} ];
$self->{from} = $p{from};
$self->{to} = ref $p{to} ? $p{to} : [ $p{to} ];
$self->{from} = $p{from};
$self->{send_args} = $p{send_args};

# Default to buffered for obvious reasons!
$self->{buffered} = $p{buffered};
Expand Down Expand Up @@ -154,6 +159,11 @@ addresses. Required.
A string containing an email address. This is optional and may not
work with all mail sending methods.

=item * send_args ($, \@ or \%)
Copy link
Member

Choose a reason for hiding this comment

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

Is there any email sending module where a scalar would make sense?

Copy link
Author

Choose a reason for hiding this comment

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

I haven't seen any, but if we disallow them, we remove the possibility of implementing a subclass of Log::Dispatch::Email that uses a scalar.


This stores any options used to send email, such as smtp host, port, etc.
These are different for each subclass.

=item * buffered (0 or 1)

This determines whether the object sends one email per message it is
Expand Down
33 changes: 28 additions & 5 deletions lib/Log/Dispatch/Email/MIMELite.pm
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,27 @@ sub send_email {

$mail{From} = $self->{from} if defined $self->{from};

local $?;
unless ( MIME::Lite->new(%mail)->send ) {
warn "Error sending mail with MIME::Lite";
}
warn "Error sending mail with MIME::Lite"
unless do {
local $?;
if ( defined $self->{send_args} && @{ $self->{send_args} } > 0 ) {
my @args = @{ $self->{send_args} };
if ( @args >= 3 ) {
MIME::Lite->new(%mail)
->send( $args[0], $args[1], @args[ 2 .. $#args ] );
}
elsif ( @args == 2 ) {
MIME::Lite->new(%mail)->send( $args[0], $args[1] );
}
else {
MIME::Lite->new(%mail)->send( $args[0] );
}
}
else {
MIME::Lite->new(%mail)->send;
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This all seems like a very complicated way of writing this:

MIME::Lite->new(%mail)->send( @{ self->{send_args} || [] } )

There's no reason to care about the number of args since this code doesn't actually do anything with them. It just passes them along.

Copy link
Author

Choose a reason for hiding this comment

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

Good point.
In my testing I was having trouble with the different ways to call MIME::Lite->new, and left that code there.
I will create a new pull request without this.

Copy link
Member

Choose a reason for hiding this comment

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

There's no need to create a new PR. Just force push to your fork and this PR will be updated.


}

1;
Expand All @@ -46,7 +63,8 @@ __END__
'Email::MIMELite',
min_level => 'emerg',
to => [qw( [email protected] [email protected] )],
subject => 'Big error!'
subject => 'Big error!',
send_args => [ 'smtp', 'smtp.example.org', { AuthUser => 'john', AuthPass => 'secret' } ]
]
],
);
Expand All @@ -58,4 +76,9 @@ __END__
This is a subclass of L<Log::Dispatch::Email> that implements the
send_email method using the L<MIME::Lite> module.

=head1 CHANGING HOW MAIL IS SENT

To change how mail is sent, set send_args to according to what
L<< MIME::Lite->send >> expects.

=cut
18 changes: 16 additions & 2 deletions lib/Log/Dispatch/Email/MailSend.pm
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ sub send_email {

local $?;
eval {
my $fh = $msg->open
my $fh = $msg->open( @{ $self->{send_args} } )
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this will die in a very ugly way when $self->{send_args} is undef.

Copy link
Author

Choose a reason for hiding this comment

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

It works for me. Mail::Send will create a Mail::Mailer object with no attributes, which seems to be ok.

Copy link
Member

Choose a reason for hiding this comment

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

$> perl -Mstrict -Mwarnings -E 'my $foo = undef; say @{$foo}'
Can't use an undefined value as an ARRAY reference at -e line 1.

Copy link
Author

Choose a reason for hiding this comment

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

I tested it and there was no warning.
Try this:
perl -Mstrict -Mwarnings -E 'my $foo = undef; mysay(@{$foo}); sub mysay { say(@_); }'

That said, of course I can add a safeguard if you think its an issue.

or die "Cannot open handle to mail program";

$fh->print( $p{message} )
Expand Down Expand Up @@ -54,7 +54,8 @@ __END__
'Email::MailSend',
min_level => 'emerg',
to => [qw( [email protected] [email protected] )],
subject => 'Big error!'
subject => 'Big error!',
send_args => [ 'smtp', Server => 'mail.example.org' ],
]
],
);
Expand All @@ -68,13 +69,26 @@ method using the L<Mail::Send> module.

=head1 CHANGING HOW MAIL IS SENT

There are two ways to change how mail is sent:

=over 4

=item 1

Since L<Mail::Send> is a subclass of L<Mail::Mailer>, you can change
how mail is sent from this module by simply C<use>ing L<Mail::Mailer>
in your code before mail is sent. For example, to send mail via smtp,
you could do:

use Mail::Mailer 'smtp', Server => 'foo.example.com';

=item 2

Set send_args to the same arguments as
the constructor of L<Mail::Mailer> expects.

=back

For more details, see the L<Mail::Mailer> docs.

=cut
11 changes: 10 additions & 1 deletion lib/Log/Dispatch/Email/MailSendmail.pm
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ sub send_email {
From => $self->{from} || '[email protected]',
);

# merge options from %{send_args}
%mail = ( %mail, %{ $self->{send_args} } ) if defined $self->{send_args};

local $?;
unless ( Mail::Sendmail::sendmail(%mail) ) {
warn "Error sending mail: $Mail::Sendmail::error";
Expand All @@ -46,7 +49,8 @@ __END__
'Email::MailSendmail',
min_level => 'emerg',
to => [qw( [email protected] [email protected] )],
subject => 'Big error!'
subject => 'Big error!',
send_args => { smtp => '127.0.0.1', retries => 10, delay => 5, debug => 0 }
]
],
);
Expand All @@ -58,4 +62,9 @@ __END__
This is a subclass of L<Log::Dispatch::Email> that implements the
send_email method using the L<Mail::Sendmail> module.

=head1 CHANGING HOW MAIL IS SENT

To change how mail is sent, set send_args to a hash reference just
like L<< %Mail::Sendmail::mailcfg >>.

=cut