Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
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
30 changes: 24 additions & 6 deletions samples/GeekLearning.Email.Samples/Controllers/HomeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,37 @@ public IActionResult Index()

public async Task<IActionResult> SendEmail()
{
var user = new User
// Email addresses have Email, DisplayName, and an enum: AddressTarget (To,Cc,Bcc, or ReplyTo)
List<IEmailAddress> recipients = new List<IEmailAddress>() {
new EmailAddress() { Email = "[email protected]", DisplayName = "Samuel", AddressAs = AddressTarget.To },
new EmailAddress() { Email = "[email protected]", DisplayName = "Bob", AddressAs = AddressTarget.Cc },
new EmailAddress() { Email = "[email protected]", DisplayName="George Jones", AddressAs= AddressTarget.ReplyTo }
};


// example of how to add a simple attachment. Add images, streams, etc as byte arrays, for example:

MimeKit.AttachmentCollection attachments = new MimeKit.AttachmentCollection
{
Email = "[email protected]",
DisplayName = "John Doe"
{ "sample_attachment.txt", System.Text.Encoding.UTF8.GetBytes("This is the content of the file attachment.") }
};

// the Reply-To addresses are simply another list of IEmailAddress objects, here, were are ignoring them as null.
// Also, in the From address, the AddressAs property is ignored, and the From address is positional and always treated as the From.
// Likewise, a From enumeration value in the recipients list is ignored, and will be treated as a To address.
await this.emailSender.SendEmailAsync(new EmailAddress() { Email="[email protected]", DisplayName="Me", AddressAs=AddressTarget.From }, "A simple message","This is a test message", recipients, attachments);


// Here is a second send example. No attachments, but using templates:

recipients.Clear();
recipients.Add(new EmailAddress { Email="[email protected]", DisplayName="George Jones", AddressAs=AddressTarget.To });
var context = new
{
ApplicationName = "Email Sender Sample",
User = user
User = recipients
};

await this.emailSender.SendTemplatedEmailAsync("Invitation", context, user);
await this.emailSender.SendTemplatedEmailAsync("Invitation", context, recipients );

return RedirectToAction("Index");
}
Expand Down
8 changes: 7 additions & 1 deletion src/GeekLearning.Email.InMemory/InMemoryEmailProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ public InMemoryEmailProvider(IEmailProviderOptions options, IInMemoryEmailReposi
{
this.inMemoryEmailRepository = inMemoryEmailRepository;
}


// for compatibility:
public Task SendEmailAsync(IEmailAddress from, IEnumerable<IEmailAddress> recipients, string subject, string text, string html)
{
return this.SendEmailAsync(from, recipients, subject, text, html, null);
}

public Task SendEmailAsync(IEmailAddress from, IEnumerable<IEmailAddress> recipients, string subject, string text, string html, MimeKit.AttachmentCollection attachments)
{
this.inMemoryEmailRepository.Save(new InMemoryEmail
{
Expand Down
3 changes: 2 additions & 1 deletion src/GeekLearning.Email.SendGrid/SendGridEmailProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public async Task SendEmailAsync(
IEnumerable<IEmailAddress> recipients,
string subject,
string text,
string html)
string html,
MimeKit.AttachmentCollection attachments)
{

var client = new SendGridClient(this.apiKey);
Expand Down
29 changes: 27 additions & 2 deletions src/GeekLearning.Email.Smtp/SmtpEmailProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,29 @@ public async Task SendEmailAsync(
IEnumerable<IEmailAddress> recipients,
string subject,
string text,
string html)
string html,
AttachmentCollection attachments)
{
var message = new MimeMessage();
message.From.Add(new MailboxAddress(from.DisplayName, from.Email));
foreach (var recipient in recipients)
{
message.To.Add(new MailboxAddress(recipient.DisplayName, recipient.Email));
InternetAddress address = new MailboxAddress(recipient.DisplayName, recipient.Email);
switch (recipient.AddressAs)
{
case AddressTarget.Cc:
message.Cc.Add(address);
break;
case AddressTarget.Bcc:
message.Bcc.Add(address);
break;
case AddressTarget.ReplyTo:
message.ReplyTo.Add(address);
break;
default:
message.To.Add(address);
break;
}
}

message.Subject = subject;
Expand All @@ -53,6 +69,15 @@ public async Task SendEmailAsync(
TextBody = text,
HtmlBody = html
};

builder.Attachments.Clear();
if (attachments != null)
{
foreach (var attachment in attachments)
{
builder.Attachments.Add(attachment);
}
}

message.Body = builder.ToMessageBody();

Expand Down
6 changes: 5 additions & 1 deletion src/GeekLearning.Email/IEmailAddress.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
namespace GeekLearning.Email
namespace GeekLearning.Email
{
public enum AddressTarget { From, To, Cc, Bcc, ReplyTo}

public interface IEmailAddress
{
string Email { get; }

string DisplayName { get; }

AddressTarget AddressAs { get; }
Copy link
Member

@sandorfr sandorfr Apr 17, 2018

Choose a reason for hiding this comment

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

IEmailAddress is meant to be a simple interface consumer can implement on their on types (or through a facade) so they can use such objects directly.

I think AddressTarget should be an argument for IEmailSender methods overloads, not a part of this interface.

Copy link
Author

Choose a reason for hiding this comment

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

I understand your idea about IEmailAddress, just defining the conventional "name [email protected]" definition. So, what might be an elegant way to link the email user with their 'AddressTarget' attributes? I think I was guessing your thinking when I made the AddressAs property optional. It can be left out, and will default to a 'To:' address insertion. But to avoid the original params implementation (which gets awkward if you want to define 5-10 or more recipients!), and keep the original simplicity of the IEmailAddress contract, maybe there has to have as inputs some kind of User structure, in which the IEmailAddress is itself a property? or overload the class methods? It would be nice to avoid too much of that.

}
}
13 changes: 10 additions & 3 deletions src/GeekLearning.Email/IEmailProvider.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
namespace GeekLearning.Email
namespace GeekLearning.Email
{
using System.Collections.Generic;
using System.Threading.Tasks;

using MimeKit;

public interface IEmailProvider
{
Task SendEmailAsync(IEmailAddress from, IEnumerable<IEmailAddress> recipients, string subject, string bodyText, string bodyHtml);
Task SendEmailAsync(
Copy link
Member

Choose a reason for hiding this comment

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

IEmailProvider should probably use AddressTarget

Copy link
Author

Choose a reason for hiding this comment

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

My idea was to be able to define a complete email transaction in one call. You might want to send to two (or more) different addresses, Cc: another, and Bcc: yet another, and also have a ReplyTo: for the message that is different that the From: address. (some hosting services required the From address to match the authentication user name, which might not be the same as the 'sender' of the immediate message). Using an attribute parameter for the email address implies a single target transaction. Slower if the app wants to send to several targets.

IEmailAddress from,
IEnumerable<IEmailAddress> recipients,
string subject,
string bodyText,
string bodyHtml
AttachmentCollection attachments=null);
}
}

8 changes: 4 additions & 4 deletions src/GeekLearning.Email/IEmailSender.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

public interface IEmailSender
Copy link
Member

Choose a reason for hiding this comment

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

there should be additional overloads using AddressTarget

Copy link
Author

Choose a reason for hiding this comment

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

That's a thought. But there needs to be a solid way to link an AddressTarget to an email address. And how does it look if we have multiple email addresses in the call?

{
Task SendEmailAsync(string subject, string message, params IEmailAddress[] to);
Copy link
Member

Choose a reason for hiding this comment

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

We should not remove the params commodity.

Task SendEmailAsync(string subject, string message, IEnumerable<IEmailAddress> recipients, AttachmentCollection attachments=null);

Task SendEmailAsync(IEmailAddress from, string subject, string message, params IEmailAddress[] to);
Task SendEmailAsync(IEmailAddress from, string subject, string message, IEnumerable<IEmailAddress> recipients, AttachmentCollection attachments=null);

Task SendTemplatedEmailAsync<T>(string templateKey, T context, params IEmailAddress[] to);
Task SendTemplatedEmailAsync<T>(string templateKey, T context, IEnumerable<IEmailAddress> recipients, AttachmentCollection attachments=null);

Task SendTemplatedEmailAsync<T>(IEmailAddress from, string templateKey, T context, params IEmailAddress[] to);
Task SendTemplatedEmailAsync<T>(IEmailAddress from, string templateKey, T context, IEnumerable<IEmailAddress> recipients, AttachmentCollection attachments=null);
}
}
5 changes: 4 additions & 1 deletion src/GeekLearning.Email/Internal/EmailAddress.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ public EmailAddress()
{
}

public EmailAddress(string email, string displayName)
public EmailAddress(string email, string displayName, AddressTarget addressAs=AddressTarget.To)
{
this.Email = email;
this.DisplayName = displayName;
this.AddressAs = addressAs;
}

public string Email { get; set; }

public string DisplayName { get; set; }

public AddressTarget AddressAs { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

we don't want that.

}
}
31 changes: 19 additions & 12 deletions src/GeekLearning.Email/Internal/EmailSender.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using System.Threading.Tasks;
using Templating;
using System.Text.RegularExpressions;

public class EmailSender : IEmailSender
{
Expand Down Expand Up @@ -43,27 +44,28 @@ public EmailSender(
}
}

public Task SendEmailAsync(string subject, string message, params IEmailAddress[] to)
public Task SendEmailAsync(string subject, string message, IEnumerable<IEmailAddress> to, MimeKit.AttachmentCollection attachments)
{
return this.SendEmailAsync(options.DefaultSender, subject, message, to);
return this.SendEmailAsync(options.DefaultSender, subject, message, to, attachments);
}

public Task SendEmailAsync(IEmailAddress from, string subject, string message, params IEmailAddress[] to)
public Task SendEmailAsync(IEmailAddress from, string subject, string message, IEnumerable<IEmailAddress> to, MimeKit.AttachmentCollection attachments)
{
return DoMockupAndSendEmailAsync(
from,
to,
subject,
message,
string.Format("<html><header></header><body>{0}</body></html>", message));
string.Format("<html><header></header><body>{0}</body></html>", message),
attachments);
}

public Task SendTemplatedEmailAsync<T>(string templateKey, T context, params IEmailAddress[] to)
public Task SendTemplatedEmailAsync<T>(string templateKey, T context, IEnumerable<IEmailAddress> to, MimeKit.AttachmentCollection attachments)
{
return this.SendTemplatedEmailAsync(options.DefaultSender, templateKey, context, to);
return this.SendTemplatedEmailAsync(options.DefaultSender, templateKey, context, to, attachments);
}

public async Task SendTemplatedEmailAsync<T>(IEmailAddress from, string templateKey, T context, params IEmailAddress[] to)
public async Task SendTemplatedEmailAsync<T>(IEmailAddress from, string templateKey, T context, IEnumerable<IEmailAddress> to, MimeKit.AttachmentCollection attachments)
{
var subjectTemplate = await this.GetTemplateAsync(templateKey, EmailTemplateType.Subject);
var textTemplate = await this.GetTemplateAsync(templateKey, EmailTemplateType.BodyText);
Expand All @@ -74,7 +76,8 @@ await this.DoMockupAndSendEmailAsync(
to,
subjectTemplate.Apply(context),
textTemplate.Apply(context),
htmlTemplate.Apply(context));
htmlTemplate.Apply(context),
attachments);
}

private Task<ITemplate> GetTemplateAsync(string templateKey, EmailTemplateType templateType)
Expand All @@ -87,7 +90,8 @@ private async Task DoMockupAndSendEmailAsync(
IEnumerable<IEmailAddress> recipients,
string subject,
string text,
string html)
string html,
MimeKit.AttachmentCollection attachments)
{
var finalRecipients = new List<IEmailAddress>();
var mockedUpRecipients = new List<IEmailAddress>();
Expand All @@ -96,12 +100,15 @@ private async Task DoMockupAndSendEmailAsync(
{
foreach (var recipient in recipients)
{
var emailParts = recipient.Email.Split('@');
if (emailParts.Length != 2)

string trimmedEmail = recipient.Email.Trim();
// not sure if it's the most friendly to validate in the sender method. Should be left to caller code, IMO
if (Regex.IsMatch(trimmedEmail, @"\A(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)\Z", RegexOptions.IgnoreCase).Equals(false))
Copy link
Member

Choose a reason for hiding this comment

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

I'm not comfortable with regex especially without a timeout in our codebase. I'm not sure this is the role of the library to validate input altogether. So I'm all in for dropping it entirely. @asiffermann you may have a different opinion.

Copy link
Author

Choose a reason for hiding this comment

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

Yes. And there was a throw in the original implementation when checking the domain part to conditional branch into the mocking sender insertion. Is there a way to check SMTP errors in the SmtpClient and then throw for failures there? Then at least the user code takes responsibility for address validation, but if that isn't done, at least the app will get alerted at the real point of failure.

{
throw new NotSupportedException("Bad recipient email.");
}

var emailParts = trimmedEmail.Split('@');
var domain = emailParts[1];

if (!this.options.Mockup.Exceptions.Emails.Contains(recipient.Email)
Expand Down Expand Up @@ -142,7 +149,7 @@ await this.provider.SendEmailAsync(
finalRecipients,
subject,
text,
html);
html, attachments);
}
}
}
17 changes: 7 additions & 10 deletions tests/GeekLearning.Email.Integration.Test/SendTemplatedTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,14 @@ public async Task SendNotification1(string storeName)
new SendGrid.SendGridEmailProviderType(),
};

var emailSender = new Internal.EmailSender(
providerTypes,
options,
this.storeFixture.Services.GetRequiredService<IStorageFactory>(),
this.storeFixture.Services.GetRequiredService<ITemplateLoaderFactory>());
List<IEmailAddress> address = new List<IEmailAddress>() {
new Internal.EmailAddress(){
DisplayName = "test user",
Email = "[email protected]",
AddressAs = AddressTarget.To}
};

await emailSender.SendTemplatedEmailAsync("Notification1", new { }, new Internal.EmailAddress
{
DisplayName = "test user",
Email = "[email protected]"
});
await emailSender.SendTemplatedEmailAsync("Notification1", new { }, address, null);
}
}
}