Skip to content

Commit

Permalink
- Removed OpenExeConfiguration, which could lead to errors.
Browse files Browse the repository at this point in the history 
- try-catch for reading SmtpSection.
- MailSettings should not be public (only in 4.3.2)
- Improve checking of "From" and overriding
- Improve test cases.
  • Loading branch information
@304NotModified
304NotModified committed Apr 28, 2016
1 parent f5ee3cf commit 59d9952
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 87 deletions.
82 changes: 54 additions & 28 deletions src/NLog/Targets/MailTarget.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ public class MailTarget : TargetWithLayoutHeaderAndFooter
{
private const string RequiredPropertyIsEmptyFormat = "After the processing of the MailTarget's '{0}' property it appears to be empty. The email message will not be sent.";

private Layout _from;

/// <summary>
/// Initializes a new instance of the <see cref="MailTarget" /> class.
/// </summary>
Expand All @@ -116,18 +118,31 @@ public MailTarget()
/// Gets the mailSettings/smtp configuration from app.config in cases when we need those configuration.
/// E.g when UseSystemNetMailSettings is enabled and we need to read the From attribute from system.net/mailSettings/smtp
/// </summary>
public SmtpSection MailSettings
/// <remarks>Internal for mocking</remarks>
internal SmtpSection SmtpSection
{
get
{
if (null == _currentailSettings)
if (_currentailSettings == null)
{
_currentailSettings = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None).GetSection("system.net/mailSettings/smtp") as SmtpSection;
try
{
_currentailSettings = System.Configuration.ConfigurationManager.GetSection("system.net/mailSettings/smtp") as SmtpSection;
}
catch (Exception ex)
{
InternalLogger.Warn(ex, "reading 'From' from .config failed.");

if (ex.MustBeRethrown())
{
throw;
}
_currentailSettings = new SmtpSection();
}
}

return _currentailSettings;
}

set { _currentailSettings = value; }
}
#endif
Expand All @@ -136,8 +151,30 @@ public SmtpSection MailSettings
/// Gets or sets sender's email address (e.g. joe@domain.com).
/// </summary>
/// <docgen category='Message Options' order='10' />
[RequiredParameter]
public Layout From { get; set; }
public Layout From
{
get
{
#if !__ANDROID__ && !__IOS__
if (UseSystemNetMailSettings)
{
// In contrary to other settings, System.Net.Mail.SmtpClient doesn't read the 'From' attribute from the system.net/mailSettings/smtp section in the config file.
// Thus, when UseSystemNetMailSettings is enabled we have to read the configuration section of system.net/mailSettings/smtp to initialize the 'From' address.
// It will do so only if the 'From' attribute in system.net/mailSettings/smtp is not empty.

//only use from config when not set in current
if (_from == null)
{
var from = SmtpSection.From;
if (from == null) return null;
return from;
}
}
#endif
return _from;
}
set { _from = value; }
}

/// <summary>
/// Gets or sets recipients' email addresses separated by semicolons (e.g. john@domain.com;jane@domain.com).
Expand Down Expand Up @@ -317,20 +354,7 @@ protected override void InitializeTarget()
base.InitializeTarget();
}

/// <summary>
/// In contrary to other settings, System.Net.Mail.SmtpClient doesn't read the 'From' attribute from the system.net/mailSettings/smtp section in the config file.
/// Thus, when UseSystemNetMailSettings is enabled we have to read the configuration section of system.net/mailSettings/smtp to initialize the 'From' address.
/// It will do so only if the 'From' attribute in system.net/mailSettings/smtp is not empty.
/// </summary>
private void ConfigureFrom()
{
#if !__ANDROID__ && !__IOS__
if (!string.IsNullOrEmpty(MailSettings.From))
{
From = MailSettings.From;
}
#endif
}


/// <summary>
/// Create mail and send with SMTP
Expand Down Expand Up @@ -360,10 +384,6 @@ private void ProcessSingleMailMessage([NotNull] List<AsyncLogEventInfo> events)
{
ConfigureMailClient(lastEvent, client);
}
else
{
ConfigureFrom();
}

InternalLogger.Debug("Sending mail to {0} using {1}:{2} (ssl={3})", msg.To, client.Host, client.Port, client.EnableSsl);
InternalLogger.Trace(" Subject: '{0}'", msg.Subject);
Expand Down Expand Up @@ -440,6 +460,7 @@ private StringBuilder CreateBodyBuffer(IEnumerable<AsyncLogEventInfo> events, Lo
/// </summary>
/// <param name="lastEvent">last event for username/password</param>
/// <param name="client">client to set properties on</param>
/// <remarks>Configure not at <see cref="InitializeTarget"/>, as the properties could have layout renderers.</remarks>
internal void ConfigureMailClient(LogEventInfo lastEvent, ISmtpClient client)
{
CheckRequiredParameters();
Expand Down Expand Up @@ -523,14 +544,17 @@ private void CheckRequiredParameters()
{
if (!this.UseSystemNetMailSettings && this.SmtpServer == null && this.DeliveryMethod == SmtpDeliveryMethod.Network)
{
throw new NLogConfigurationException(
string.Format("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=Network. The email message will not be sent.", "SmtpServer"));
throw new NLogConfigurationException("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=Network. The email message will not be sent.", "SmtpServer");
}

if (!this.UseSystemNetMailSettings && string.IsNullOrEmpty(this.PickupDirectoryLocation) && this.DeliveryMethod == SmtpDeliveryMethod.SpecifiedPickupDirectory)
{
throw new NLogConfigurationException(
string.Format("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=SpecifiedPickupDirectory. The email message will not be sent.", "PickupDirectoryLocation"));
throw new NLogConfigurationException("The MailTarget's '{0}' properties are not set - but needed because useSystemNetMailSettings=false and DeliveryMethod=SpecifiedPickupDirectory. The email message will not be sent.", "PickupDirectoryLocation");
}

if (this.From == null)
{
throw new NLogConfigurationException(RequiredPropertyIsEmptyFormat, "From");
}
}

Expand Down Expand Up @@ -568,6 +592,8 @@ private static void AppendLayout(StringBuilder sb, LogEventInfo logEvent, Layout
sb.Append(layout.Render(logEvent));
}



/// <summary>
/// Create the mailmessage with the addresses, properties and body.
/// </summary>
Expand Down
99 changes: 40 additions & 59 deletions tests/NLog.UnitTests/Targets/MailTargetTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
// THE POSSIBILITY OF SUCH DAMAGE.
//

using System.Net.Configuration;

#if !SILVERLIGHT

Expand All @@ -51,6 +52,11 @@ namespace NLog.UnitTests.Targets

public class MailTargetTests : NLogTestBase
{
public MailTargetTests()
{
LogManager.ThrowExceptions = true;
}

[Fact]
public void SimpleEmailTest()
{
Expand Down Expand Up @@ -270,6 +276,7 @@ public void PerMessageServer()
[Fact]
public void ErrorHandlingTest()
{
LogManager.ThrowExceptions = false;
var mmt = new MockMailTarget
{
From = "foo@bar.com",
Expand Down Expand Up @@ -441,7 +448,7 @@ public void NoReplaceNewlinesWithBreakInHtmlMail()

var messageSent = mmt.CreatedMocks[0].MessagesSent[0];
Assert.True(messageSent.IsBodyHtml);
var lines = messageSent.Body.Split(new[] {Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
var lines = messageSent.Body.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
Assert.True(lines.Length == 3);
}

Expand Down Expand Up @@ -541,8 +548,6 @@ public void MailTarget_WithValidToAndEmptyBcc_SendsMail()

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));

Assert.Null(exceptions[0]);
Assert.Equal(1, mmt.CreatedMocks.Count);
Assert.Equal(1, mmt.CreatedMocks[0].MessagesSent.Count);
}
Expand All @@ -560,12 +565,9 @@ public void MailTarget_WithEmptyTo_ThrowsNLogRuntimeException()
Body = "${level} ${logger} ${message}",
};
mmt.Initialize(null);

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));
Assert.Throws<NLogRuntimeException>(() => mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add)));

Assert.NotNull(exceptions[0]);
Assert.IsType<NLogRuntimeException>(exceptions[0]);
}

[Fact]
Expand All @@ -583,10 +585,7 @@ public void MailTarget_WithEmptyFrom_ThrowsNLogRuntimeException()
mmt.Initialize(null);

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));

Assert.NotNull(exceptions[0]);
Assert.IsType<NLogRuntimeException>(exceptions[0]);
Assert.Throws<NLogRuntimeException>(() => mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add)));
}

[Fact]
Expand All @@ -604,10 +603,7 @@ public void MailTarget_WithEmptySmtpServer_ThrowsNLogRuntimeException()
mmt.Initialize(null);

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add));

Assert.NotNull(exceptions[0]);
Assert.IsType<NLogRuntimeException>(exceptions[0]);
Assert.Throws<NLogRuntimeException>(() => mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Info, "MyLogger", "log message 1").WithContinuation(exceptions.Add)));
}

[Fact]
Expand All @@ -633,7 +629,8 @@ public void MailTargetInitialize_WithoutSpecifiedFrom_ThrowsConfigException()
Subject = "Hello from NLog",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}"
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = false
};
Assert.Throws<NLogConfigurationException>(() => mmt.Initialize(null));
}
Expand All @@ -650,7 +647,7 @@ public void MailTargetInitialize_WithoutSpecifiedSmtpServer_should_not_ThrowsCon
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = true
};

mmt.Initialize(null);
}

[Fact]
Expand Down Expand Up @@ -729,7 +726,7 @@ public void MailTarget_UseSystemNetMailSettings_True()

Assert.Equal(mmt.SmtpClientPickUpDirectory, inConfigVal);
}

[Fact]
public void MailTarget_UseSystemNetMailSettings_True_WithVirtualPath()
{
Expand All @@ -745,7 +742,7 @@ public void MailTarget_UseSystemNetMailSettings_True_WithVirtualPath()
DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory
};
mmt.ConfigureMailClient();

Assert.NotEqual(inConfigVal, mmt.SmtpClientPickUpDirectory);
var separator = Path.DirectorySeparatorChar;
Assert.Contains(string.Format("{0}App_Data{0}Mail", separator), mmt.SmtpClientPickUpDirectory);
Expand All @@ -754,84 +751,67 @@ public void MailTarget_UseSystemNetMailSettings_True_WithVirtualPath()
#if !__ANDROID__ && !__IOS__

[Fact]
public void MailTarget_UseSystemNetMailSettings_True_ReadFromFromConfigFile()
public void MailTarget_UseSystemNetMailSettings_True_ReadFromFromConfigFile_dontoverride()
{
var configuration = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
var section = configuration.GetSection("system.net/mailSettings/smtp") as System.Net.Configuration.SmtpSection;
section.From = "foo@bar.com";

var mmt = new MockMailTarget()
var mmt = new MailTarget()
{
From = "foo@foo.com",
From = "nlog@foo.com",
To = "bar@bar.com",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = true,
MailSettings = section
SmtpSection = new SmtpSection { From = "config@foo.com" }
};
mmt.Initialize(null);
mmt.ConfigureMailClient();
Assert.Equal("'nlog@foo.com'", mmt.From.ToString());

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Debug, "MyLogger", "log message").WithContinuation(exceptions.Add));
mmt.Initialize(null);

Assert.Equal("'foo@bar.com'", mmt.From.ToString());
Assert.Equal("'nlog@foo.com'", mmt.From.ToString());
}

[Fact]
public void MailTarget_UseSystemNetMailSettings_False_ReadFromFromConfigFile()
public void MailTarget_UseSystemNetMailSettings_True_ReadFromFromConfigFile()
{
var configuration = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
var section = configuration.GetSection("system.net/mailSettings/smtp") as System.Net.Configuration.SmtpSection;
section.From = "foo@bar.com";

var mmt = new MockMailTarget()
var mmt = new MailTarget()
{
From = "foo@foo.com",
From = null,
To = "bar@bar.com",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = false,
MailSettings = section
UseSystemNetMailSettings = true,
SmtpSection = new SmtpSection { From = "config@foo.com" }
};
mmt.Initialize(null);
mmt.ConfigureMailClient();
Assert.Equal("'config@foo.com'", mmt.From.ToString());

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Debug, "MyLogger", "log message").WithContinuation(exceptions.Add));
mmt.Initialize(null);

Assert.Equal("'foo@foo.com'", mmt.From.ToString());
Assert.Equal("'config@foo.com'", mmt.From.ToString());
}

[Fact]
public void MailTarget_UseSystemNetMailSettings_True_KeepOverridenFiles()
public void MailTarget_UseSystemNetMailSettings_False_ReadFromFromConfigFile()
{
var configuration = System.Configuration.ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
var section = configuration.GetSection("system.net/mailSettings/smtp") as System.Net.Configuration.SmtpSection;
section.From = null;

var mmt = new MockMailTarget()
var mmt = new MailTarget()
{
From = "foo@foo.com",
From = null,
To = "bar@bar.com",
SmtpServer = "server1",
SmtpPort = 27,
Body = "${level} ${logger} ${message}",
UseSystemNetMailSettings = true,
MailSettings = section
UseSystemNetMailSettings = false,
SmtpSection = new SmtpSection { From = "config@foo.com" }
};
Assert.Null(mmt.From);

mmt.Initialize(null);
mmt.ConfigureMailClient();

var exceptions = new List<Exception>();
mmt.WriteAsyncLogEvent(new LogEventInfo(LogLevel.Debug, "MyLogger", "log message").WithContinuation(exceptions.Add));
Assert.Throws <NLogConfigurationException>(() => mmt.Initialize(null));

Assert.Equal("'foo@foo.com'", mmt.From.ToString());
}

#endif

[Fact]
Expand Down Expand Up @@ -925,6 +905,7 @@ internal override ISmtpClient CreateSmtpClient()
return client;
}


public void ConfigureMailClient()
{
if (UseSystemNetMailSettings) return;
Expand Down

0 comments on commit 59d9952

Please sign in to comment.