Skip to content

Commit

Permalink
Apply Guard.ThrowIfNull to more locations, since better than NRE (#893)
Browse files Browse the repository at this point in the history
  • Loading branch information
@snakefoot
snakefoot authored Nov 27, 2022
1 parent 41faea4 commit 2b8583f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 11 deletions.
29 changes: 23 additions & 6 deletions src/NLog.Web.AspNetCore/AspNetExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;
using NLog.Extensions.Logging;
using NLog.Internal;
using NLog.Web.Internal;

namespace NLog.Web
{
Expand All @@ -31,6 +31,7 @@ public static class AspNetExtensions
[Obsolete("Use UseNLog() on IHostBuilder / IWebHostBuilder, or AddNLogWeb() on ILoggingBuilder")]
public static void AddNLogWeb(this IApplicationBuilder app)
{
Guard.ThrowIfNull(app);
app.ApplicationServices.SetupNLogServiceLocator();
}
#endif
Expand Down Expand Up @@ -62,6 +63,8 @@ public static IServiceProvider SetupNLogServiceLocator(this IServiceProvider ser
[Obsolete("Use UseNLog() on IHostBuilder / IWebHostBuilder, or AddNLogWeb() on ILoggingBuilder")]
public static LoggingConfiguration ConfigureNLog(this IHostEnvironment env, string configFileRelativePath)
{
Guard.ThrowIfNull(env);

ConfigurationItemFactory.Default.RegisterItemsFromAssembly(typeof(AspNetExtensions).GetTypeInfo().Assembly);
LogManager.AddHiddenAssembly(typeof(AspNetExtensions).GetTypeInfo().Assembly);
var fileName = System.IO.Path.Combine(env.ContentRootPath, configFileRelativePath);
Expand All @@ -83,7 +86,7 @@ public static LoggingConfiguration ConfigureNLog(this IHostEnvironment env, stri
public static LogFactory ConfigureNLog(this ILoggingBuilder builder, string configFileName)
{
ConfigurationItemFactory.Default.RegisterItemsFromAssembly(typeof(AspNetExtensions).GetTypeInfo().Assembly);
builder.AddNLog();
builder.AddNLogWeb();
return LogManager.LoadConfiguration(configFileName);
}

Expand All @@ -99,7 +102,7 @@ public static LogFactory ConfigureNLog(this ILoggingBuilder builder, string conf
public static LogFactory ConfigureNLog(this ILoggingBuilder builder, LoggingConfiguration configuration)
{
ConfigurationItemFactory.Default.RegisterItemsFromAssembly(typeof(AspNetExtensions).GetTypeInfo().Assembly);
builder.AddNLog();
builder.AddNLogWeb();
LogManager.Configuration = configuration;
return LogManager.LogFactory;
}
Expand All @@ -120,6 +123,8 @@ public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder)
/// <param name="options">Options for registration of the NLog LoggingProvider and enabling features.</param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, NLogAspNetCoreOptions options)
{
Guard.ThrowIfNull(builder);

AddNLogLoggerProvider(builder.Services, null, null, options, (serviceProvider, config, env, opt) =>
{
return CreateNLogLoggerProvider(serviceProvider, config, env, opt);
Expand All @@ -135,6 +140,9 @@ public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, NLogAspNe
/// <param name="factoryBuilder">Initialize NLog LogFactory with NLog LoggingConfiguration.</param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, NLogAspNetCoreOptions options, Func<IServiceProvider, LogFactory> factoryBuilder)
{
Guard.ThrowIfNull(builder);
Guard.ThrowIfNull(factoryBuilder);

AddNLogLoggerProvider(builder.Services, null, null, options, (serviceProvider, config, env, opt) =>
{
config = SetupNLogConfigSettings(serviceProvider, config, LogManager.LogFactory);
Expand Down Expand Up @@ -164,6 +172,8 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, string confi
/// <param name="configFileName">Path to NLog configuration file, e.g. nlog.config. </param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, string configFileName)
{
Guard.ThrowIfNull(builder);

AddNLogLoggerProvider(builder.Services, null, null, null, (serviceProvider, config, env, options) =>
{
var provider = CreateNLogLoggerProvider(serviceProvider, config, env, options);
Expand Down Expand Up @@ -192,7 +202,7 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, LoggingConfi
/// <param name="configuration">Config for NLog</param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, LoggingConfiguration configuration)
{
return AddNLogWeb(builder, configuration, null);
return AddNLogWeb(builder, configuration, NLogAspNetCoreOptions.Default);
}

/// <summary>
Expand All @@ -215,6 +225,8 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, LoggingConfi
/// <param name="options">Options for registration of the NLog LoggingProvider and enabling features.</param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, LoggingConfiguration configuration, NLogAspNetCoreOptions options)
{
Guard.ThrowIfNull(builder);

AddNLogLoggerProvider(builder.Services, null, null, options, (serviceProvider, config, env, opt) =>
{
var logFactory = configuration?.LogFactory ?? LogManager.LogFactory;
Expand Down Expand Up @@ -244,6 +256,9 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, Func<IServic
/// <param name="factoryBuilder">Initialize NLog LogFactory with NLog LoggingConfiguration.</param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, Func<IServiceProvider, LogFactory> factoryBuilder)
{
Guard.ThrowIfNull(builder);
Guard.ThrowIfNull(factoryBuilder);

AddNLogLoggerProvider(builder.Services, null, null, null, (serviceProvider, config, env, options) =>
{
config = SetupNLogConfigSettings(serviceProvider, config, LogManager.LogFactory);
Expand All @@ -263,6 +278,8 @@ public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, Func<ISer
/// <param name="options">Options for registration of the NLog LoggingProvider and enabling features.</param>
public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, LogFactory logFactory, NLogAspNetCoreOptions options)
{
Guard.ThrowIfNull(builder);

AddNLogLoggerProvider(builder.Services, null, null, options, (serviceProvider, config, env, opt) =>
{
logFactory = logFactory ?? LogManager.LogFactory;
Expand All @@ -277,7 +294,7 @@ public static ILoggingBuilder AddNLogWeb(this ILoggingBuilder builder, LogFactor
/// </summary>
public static IWebHostBuilder UseNLog(this IWebHostBuilder builder)
{
return UseNLog(builder, null);
return UseNLog(builder, NLogAspNetCoreOptions.Default);
}

/// <summary>
Expand All @@ -298,7 +315,7 @@ public static IWebHostBuilder UseNLog(this IWebHostBuilder builder, NLogAspNetCo
/// </summary>
public static IHostBuilder UseNLog(this IHostBuilder builder)
{
return UseNLog(builder, null);
return UseNLog(builder, NLogAspNetCoreOptions.Default);
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/NLog.Web.AspNetCore/Internal/Guard.cs → src/Shared/Internal/Guard.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public CallerArgumentExpressionAttribute(string param)
}
#endif

namespace NLog.Internal
namespace NLog.Web.Internal
{
using System;
using System.Runtime.CompilerServices;
Expand All @@ -60,7 +60,7 @@ internal static T ThrowIfNull<T>(
{
if (arg is null)
{
throw new ArgumentNullException(param);
throw new ArgumentNullException(string.IsNullOrEmpty(param) ? typeof(T).Name : param);
}

return arg;
Expand Down
5 changes: 2 additions & 3 deletions tests/NLog.Web.AspNetCore.Tests/AspNetCoreTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,8 @@ public void UseNLogWithNullWebHostBuilderThrowsArgumentNullException()
}
catch (Exception e)
{
Assert.IsType<ArgumentNullException>(e);

Assert.Equal("builder", (e as ArgumentNullException).ParamName);
var argNullException = Assert.IsType<ArgumentNullException>(e);
Assert.Equal("builder", argNullException.ParamName); // Verify CallerArgumentExpressionAttribute works
}
}

Expand Down

0 comments on commit 2b8583f

Please sign in to comment.