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 Logging to the Dependency Injection SafeGetTypes function #4455

Merged
merged 10 commits into from
Feb 8, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
<AdditionalFiles Include="..\..\stylecop.json" Link="stylecop.json" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\DotNetNuke.Instrumentation\DotNetNuke.Instrumentation.csproj" />
</ItemGroup>

<Import Project="..\..\DNN_Platform.build" />
<Target Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="XCOPY &quot;$(ProjectDir)bin\$(ConfigurationName)\netstandard2.0\DotNetNuke.DependencyInjection*&quot; &quot;$(WebsitePath)\bin&quot; /S /Y" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information
namespace DotNetNuke.DependencyInjection.Extensions
{
using System;
using System.Linq;
using System.Reflection;
using System.Reflection;

using DotNetNuke.Instrumentation;

/// <summary>
/// <see cref="Type"/> specific extensions to be used
/// in Dependency Injection invocations.
/// </summary>
public static class TypeExtensions
{
// There is no logging in this file by design as
// it would create a dependency on the Logging library
// and this library can't have any dependencies on other
// DNN Libraries.

{
// There is no logging in this file by design as
// it would create a dependency on the Logging library
// and this library can't have any dependencies on other
// DNN Libraries.
valadas marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// Safely Get all Types from the assembly. If there
/// is an error while retrieving the types it will
Expand All @@ -26,10 +28,15 @@ public static class TypeExtensions
/// <param name="assembly">
/// The assembly to retrieve all types from.
/// </param>
/// <param name="logger">
/// A optional <see cref="ILog"/> object. This will log any messages from the exception catching to the logs as an Error.
/// </param>
/// <returns>
/// An array of all <see cref="Type"/> in the given <see cref="Assembly"/>.
/// </returns>
public static Type[] SafeGetTypes(this Assembly assembly)
valadas marked this conversation as resolved.
Show resolved Hide resolved
/// </returns>
#pragma warning disable CS3001 // Argument type is not CLS-compliant
public static Type[] SafeGetTypes(this Assembly assembly, ILog logger = null)
#pragma warning restore CS3001 // Argument type is not CLS-compliant
mtrutledge marked this conversation as resolved.
Show resolved Hide resolved
{
Type[] types = null;
try
Expand All @@ -38,13 +45,26 @@ public static Type[] SafeGetTypes(this Assembly assembly)
}
catch (ReflectionTypeLoadException ex)
{
// TODO: We should log the reason of the exception after the API cleanup
// Ensure that Dnn obtains all types that were loaded, ignoring the failure(s)
if (logger != null)
{
// The loaderexceptions will repeat. Need to get distinct messages here.
string distinctLoaderExceptions = string.Join(
Environment.NewLine,
ex.LoaderExceptions.Select(i => i.Message).Distinct().Select(i => $"- LoaderException: {i}"));

logger.Error($"Unable to configure services for {assembly.FullName}, see exception for details {Environment.NewLine}{distinctLoaderExceptions}", ex);
}

// Ensure that Dnn obtains all types that were loaded, ignoring the failure(s)
types = ex.Types.Where(x => x != null).ToArray<Type>();
}
catch (Exception)
catch (Exception ex)
{
// TODO: We should log the reason of the exception after the API cleanup
if (logger != null)
{
logger.Error($"Unable to configure services for {assembly.FullName}, see exception for details", ex);
}

types = new Type[0];
}

Expand Down
20 changes: 17 additions & 3 deletions DNN Platform/DotNetNuke.Web.Mvc/Extensions/StartupExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,31 @@ namespace DotNetNuke.Web.Mvc.Extensions
using System.Text;
using System.Threading.Tasks;

using DotNetNuke.DependencyInjection.Extensions;
using DotNetNuke.DependencyInjection.Extensions;
using DotNetNuke.Instrumentation;
using DotNetNuke.Web.Mvc.Framework.Controllers;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;

/// <summary>
/// Adds DNN MVC Contoller Specific startup extensions to simplify the
/// <see cref="Startup"/> Class.
/// </summary>
public static class StartupExtensions
{
{
private static readonly ILog Logger = LoggerSource.Instance.GetLogger(typeof(StartupExtensions));

/// <summary>
/// Configures all of the <see cref="DnnController"/>'s to be used
/// with the Service Collection for Dependency Injection.
/// </summary>
/// <param name="services">
/// Service Collection used to registering services in the container.
/// </param>
public static void AddMvcControllers(this IServiceCollection services)
{
var controllerTypes = AppDomain.CurrentDomain.GetAssemblies()
.SelectMany(TypeExtensions.SafeGetTypes)
.SelectMany(x => x.SafeGetTypes(Logger))
.Where(x => typeof(IDnnController).IsAssignableFrom(x)
&& x.IsClass
&& !x.IsAbstract);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ namespace DotNetNuke.Web.Extensions
using System;
using System.Linq;

using DotNetNuke.DependencyInjection.Extensions;
using DotNetNuke.DependencyInjection.Extensions;
using DotNetNuke.Instrumentation;
using DotNetNuke.Web.Api;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand All @@ -17,7 +18,9 @@ namespace DotNetNuke.Web.Extensions
/// <see cref="Startup"/> Class.
/// </summary>
internal static class StartupExtensions
{
{
private static readonly ILog Logger = LoggerSource.Instance.GetLogger(typeof(StartupExtensions));

/// <summary>
/// Configures all of the <see cref="DnnApiController"/>'s to be used
/// with the Service Collection for Dependency Injection.
Expand All @@ -28,7 +31,7 @@ internal static class StartupExtensions
public static void AddWebApi(this IServiceCollection services)
{
var controllerTypes = AppDomain.CurrentDomain.GetAssemblies()
.SelectMany(x => x.SafeGetTypes())
.SelectMany(x => x.SafeGetTypes(Logger))
.Where(x => typeof(DnnApiController).IsAssignableFrom(x) &&
x.IsClass &&
!x.IsAbstract);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ private static void ConfigureAllStartupServices(IServiceCollection services)
assembly => assembly.FullName.StartsWith("DotNetNuke", StringComparison.OrdinalIgnoreCase) ? 0 :
assembly.FullName.StartsWith("DNN", StringComparison.OrdinalIgnoreCase) ? 1 : 2)
.ThenBy(assembly => assembly.FullName)
.SelectMany(assembly => assembly.SafeGetTypes().OrderBy(type => type.FullName ?? type.Name))
.SelectMany(assembly => assembly.SafeGetTypes(Logger).OrderBy(type => type.FullName ?? type.Name))
.Where(type => typeof(IDnnStartup).IsAssignableFrom(type) && type.IsClass && !type.IsAbstract);

var startupInstances = startupTypes.Select(CreateInstance).Where(x => x != null);
Expand Down