Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Commit d14f475

Browse files
kichallakichalla
committed
Fixes #326: Logger from Type name shouldn't include generic parameters
1 parent 6c29db3 commit d14f475

File tree

6 files changed

+246
-16
lines changed

6 files changed

+246
-16
lines changed

‎src/Microsoft.Extensions.Logging.Abstractions/Internal/TypeNameHelper.cs

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Reflection;
7+
8+
namespace Microsoft.Extensions.Logging.Abstractions.Internal
9+
{
10+
public class TypeNameHelper
11+
{
12+
private static readonly Dictionary<Type, string> _builtInTypeNames = new Dictionary<Type, string>
13+
{
14+
{ typeof(bool), "bool" },
15+
{ typeof(byte), "byte" },
16+
{ typeof(char), "char" },
17+
{ typeof(decimal), "decimal" },
18+
{ typeof(double), "double" },
19+
{ typeof(float), "float" },
20+
{ typeof(int), "int" },
21+
{ typeof(long), "long" },
22+
{ typeof(object), "object" },
23+
{ typeof(sbyte), "sbyte" },
24+
{ typeof(short), "short" },
25+
{ typeof(string), "string" },
26+
{ typeof(uint), "uint" },
27+
{ typeof(ulong), "ulong" },
28+
{ typeof(ushort), "ushort" }
29+
};
30+
31+
public static string GetTypeDisplayName(Type type)
32+
{
33+
if (type.GetTypeInfo().IsGenericType)
34+
{
35+
var fullName = type.GetGenericTypeDefinition().FullName;
36+
37+
// Nested types (public or private) have a '+' in their full name
38+
var parts = fullName.Split('+');
39+
40+
// Handle nested generic types
41+
// Examples:
42+
// ConsoleApp.Program+Foo`1+Bar
43+
// ConsoleApp.Program+Foo`1+Bar`1
44+
for (var i = 0; i < parts.Length; i++)
45+
{
46+
var partName = parts[i];
47+
48+
var backTickIndex = partName.IndexOf('`');
49+
if (backTickIndex >= 0)
50+
{
51+
// Since '.' is typically used to filter log messages in a hierarchy kind of scenario,
52+
// do not include any generic type information as part of the name.
53+
// Example:
54+
// Microsoft.AspNet.Mvc -> log level set as Warning
55+
// Microsoft.AspNet.Mvc.ModelBinding -> log level set as Verbose
56+
partName = partName.Substring(0, backTickIndex);
57+
}
58+
59+
parts[i] = partName;
60+
}
61+
62+
return string.Join(".", parts);
63+
}
64+
else if (_builtInTypeNames.ContainsKey(type))
65+
{
66+
return _builtInTypeNames[type];
67+
}
68+
else
69+
{
70+
var fullName = type.FullName;
71+
72+
if (type.IsNested)
73+
{
74+
fullName = fullName.Replace('+', '.');
75+
}
76+
77+
return fullName;
78+
}
79+
}
80+
}
81+
}

‎src/Microsoft.Extensions.Logging.Abstractions/LoggerOfT.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using Microsoft.Extensions.Internal;
5+
using Microsoft.Extensions.Logging.Abstractions.Internal;
66

77
namespace Microsoft.Extensions.Logging
88
{
@@ -26,7 +26,7 @@ public Logger(ILoggerFactory factory)
2626
throw new ArgumentNullException(nameof(factory));
2727
}
2828

29-
_logger = factory.CreateLogger(TypeNameHelper.GetTypeDisplayName(typeof(T), fullName: true));
29+
_logger = factory.CreateLogger(TypeNameHelper.GetTypeDisplayName(typeof(T)));
3030
}
3131

3232
IDisposable ILogger.BeginScopeImpl(object state)

‎src/Microsoft.Extensions.Logging.Abstractions/project.json

-6
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,6 @@
99
"warningsAsErrors": true,
1010
"keyFile": "../../tools/Key.snk"
1111
},
12-
"dependencies": {
13-
"Microsoft.Extensions.TypeNameHelper.Sources": {
14-
"type": "build",
15-
"version": "1.0.0-*"
16-
}
17-
},
1812
"frameworks": {
1913
"net451": {},
2014
"dotnet5.4": {

‎src/Microsoft.Extensions.Logging.Testing/LogValuesAssert.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Linq;
7-
using Microsoft.Extensions.Internal;
87
using Xunit.Sdk;
98

109
namespace Microsoft.Extensions.Logging.Testing
@@ -32,7 +31,7 @@ public static void Contains(
3231
/// <param name="expectedValues">Expected subset of values</param>
3332
/// <param name="actualValues">Actual set of values</param>
3433
/// <param name="comparer">
35-
/// An <see cref="IEqualityComparer{T}"/> to compare the values.
34+
/// An <see cref="IEqualityComparer{T}"/> to compare the values.
3635
/// In case no comparer is provided, a default comparer is used.
3736
/// </param>
3837
public static void Contains(

‎test/Microsoft.Extensions.Logging.Test/LoggerFactoryExtensionsTest.cs

+57-6
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
#if DNX451
4+
using System.Collections.Generic;
55
using Moq;
6-
#endif
76
using Xunit;
87

98
namespace Microsoft.Extensions.Logging.Test
109
{
1110
public class LoggerFactoryExtensionsTest
1211
{
13-
#if DNX451
14-
[Fact]
12+
[Fact]
1513
public void LoggerFactoryCreateOfT_CallsCreateWithCorrectName()
1614
{
1715
// Arrange
@@ -53,12 +51,65 @@ public void LoggerFactoryCreateOfT_TwoGenerics_CallsCreateWithCorrectName()
5351
x => x.Equals("Microsoft.Extensions.Logging.Test.GenericClass<Microsoft.Extensions.Logging.Test.TestType, Microsoft.Extensions.Logging.Test.SecondTestType>"))))
5452
.Returns(new Mock<ILogger>().Object);
5553

56-
var logger = factory.Object.CreateLogger<GenericClass<TestType,SecondTestType>>();
54+
var logger = factory.Object.CreateLogger<GenericClass<TestType, SecondTestType>>();
5755

5856
// Assert
5957
Assert.NotNull(logger);
6058
}
61-
#endif
59+
60+
[Fact]
61+
public void CreatesLoggerName_WithoutGenericTypeArgumentsInformation()
62+
{
63+
// Arrange
64+
var fullName = typeof(GenericClass<string>).GetGenericTypeDefinition().FullName;
65+
var fullNameWithoutBacktick = fullName.Substring(0, fullName.IndexOf('`'));
66+
var testSink = new TestSink();
67+
var factory = new TestLoggerFactory(testSink, enabled: true);
68+
69+
// Act
70+
var logger = factory.CreateLogger<GenericClass<string>>();
71+
logger.LogInformation("test message");
72+
73+
// Assert
74+
Assert.Single(testSink.Writes);
75+
Assert.Equal(fullNameWithoutBacktick, testSink.Writes[0].LoggerName);
76+
}
77+
78+
[Fact]
79+
public void CreatesLoggerName_OnNestedGenericType_CreatesWithoutGenericTypeArgumentsInformation()
80+
{
81+
// Arrange
82+
var fullName = typeof(GenericClass<GenericClass<string>>).GetGenericTypeDefinition().FullName;
83+
var fullNameWithoutBacktick = fullName.Substring(0, fullName.IndexOf('`'));
84+
var testSink = new TestSink();
85+
var factory = new TestLoggerFactory(testSink, enabled: true);
86+
87+
// Act
88+
var logger = factory.CreateLogger<GenericClass<GenericClass<string>>>();
89+
logger.LogInformation("test message");
90+
91+
// Assert
92+
Assert.Single(testSink.Writes);
93+
Assert.Equal(fullNameWithoutBacktick, testSink.Writes[0].LoggerName);
94+
}
95+
96+
[Fact]
97+
public void CreatesLoggerName_OnMultipleTypeArgumentGenericType_CreatesWithoutGenericTypeArgumentsInformation()
98+
{
99+
// Arrange
100+
var fullName = typeof(GenericClass<string, string>).GetGenericTypeDefinition().FullName;
101+
var fullNameWithoutBacktick = fullName.Substring(0, fullName.IndexOf('`'));
102+
var testSink = new TestSink();
103+
var factory = new TestLoggerFactory(testSink, enabled: true);
104+
105+
// Act
106+
var logger = factory.CreateLogger<GenericClass<string, string>>();
107+
logger.LogInformation("test message");
108+
109+
// Assert
110+
Assert.Single(testSink.Writes);
111+
Assert.Equal(fullNameWithoutBacktick, testSink.Writes[0].LoggerName);
112+
}
62113
}
63114

64115
internal class TestType

‎test/Microsoft.Extensions.Logging.Test/TypeNameHelperTest.cs

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using Xunit;
7+
8+
namespace Microsoft.Extensions.Logging.Abstractions.Internal
9+
{
10+
public class TypeNameHelperTest
11+
{
12+
public static TheoryData<Type, string> FullTypeNameData
13+
{
14+
get
15+
{
16+
return new TheoryData<Type, string>
17+
{
18+
// Predefined Types
19+
{ typeof(int), "int" },
20+
{ typeof(List<int>), "System.Collections.Generic.List" },
21+
{ typeof(Dictionary<int, string>), "System.Collections.Generic.Dictionary" },
22+
{ typeof(Dictionary<int, List<string>>), "System.Collections.Generic.Dictionary" },
23+
{ typeof(List<List<string>>), "System.Collections.Generic.List" },
24+
25+
// Classes inside NonGeneric class
26+
{ typeof(A), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.A" },
27+
{ typeof(B<int>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.B" },
28+
{ typeof(C<int, string>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.C" },
29+
{ typeof(C<int, B<string>>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.C" },
30+
{ typeof(B<B<string>>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.B" },
31+
32+
// Classes inside Generic class
33+
{ typeof(Outer<int>.D), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.Outer.D" },
34+
{ typeof(Outer<int>.E<int>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.Outer.E" },
35+
{ typeof(Outer<int>.F<int, string>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.Outer.F" },
36+
{ typeof(Outer<int>.F<int, Outer<int>.E<string>>),"Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.Outer.F" },
37+
{ typeof(Outer<int>.E<Outer<int>.E<string>>), "Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelperTest.Outer.E" }
38+
};
39+
}
40+
}
41+
42+
[Theory]
43+
[MemberData(nameof(FullTypeNameData))]
44+
public void Can_PrettyPrint_FullTypeName(Type type, string expectedTypeName)
45+
{
46+
// Arrange & Act
47+
var displayName = TypeNameHelper.GetTypeDisplayName(type);
48+
49+
// Assert
50+
Assert.Equal(expectedTypeName, displayName);
51+
}
52+
53+
public static TheoryData<Type, string> BuiltInTypesData
54+
{
55+
get
56+
{
57+
return new TheoryData<Type, string>
58+
{
59+
// Predefined Types
60+
{ typeof(bool), "bool" },
61+
{ typeof(byte), "byte" },
62+
{ typeof(char), "char" },
63+
{ typeof(decimal), "decimal" },
64+
{ typeof(double), "double" },
65+
{ typeof(float), "float" },
66+
{ typeof(int), "int" },
67+
{ typeof(long), "long" },
68+
{ typeof(object), "object" },
69+
{ typeof(sbyte), "sbyte" },
70+
{ typeof(short), "short" },
71+
{ typeof(string), "string" },
72+
{ typeof(uint), "uint" },
73+
{ typeof(ulong), "ulong" },
74+
{ typeof(ushort), "ushort" },
75+
};
76+
}
77+
}
78+
79+
[Theory]
80+
[MemberData(nameof(BuiltInTypesData))]
81+
public void ReturnsCommonName_ForBuiltinTypes(Type type, string expectedTypeName)
82+
{
83+
// Arrange & Act
84+
var displayName = TypeNameHelper.GetTypeDisplayName(type);
85+
86+
// Assert
87+
Assert.Equal(expectedTypeName, displayName);
88+
}
89+
90+
private class A { }
91+
92+
private class B<T> { }
93+
94+
private class C<T1, T2> { }
95+
96+
private class Outer<T>
97+
{
98+
public class D { }
99+
100+
public class E<T1> { }
101+
102+
public class F<T1, T2> { }
103+
}
104+
}
105+
}

0 commit comments

Comments
 (0)