Support strong typed log entry state (#257)

* Updated LogEntry.State to return a dictionary +semver: breaking
roryprimrose · May 17, 2024 · 7092181 · 7092181
1 parent 677ba26
commit 7092181
Show file tree

Hide file tree

Showing 24 changed files with 271 additions and 130 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -24,13 +24,13 @@ jobs:
         fetch-depth: 0
 
     - name: Install GitVersion
-      uses: gittools/actions/gitversion/setup@v0.10.2
+      uses: gittools/actions/gitversion/setup@v1.1.1
       with:
           versionSpec: '5.x'
 
     - name: Use GitVersion
       id: gitversion # step id used as reference for output values
-      uses: gittools/actions/gitversion/execute@v0.10.2
+      uses: gittools/actions/gitversion/execute@v1.1.1
       with:
         useConfigFile: true
         configFilePath: ./GitVersion.yml
@@ -59,7 +59,7 @@ jobs:
 
     - name: Generate coverage report
       # run: reportgenerator -reports:**/coverage.cobertura.xml -targetdir:Report -reporttypes:HtmlInline_AzurePipelines;Cobertura
-      uses: danielpalme/ReportGenerator-GitHub-Action@5.2.0
+      uses: danielpalme/ReportGenerator-GitHub-Action@5.3.0
       with:
         reports: "**/coverage*cobertura.xml"
         targetdir: "Report"

diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -0,0 +1,29 @@
+<Project>
+	<!-- Central Package Management: https://learn.microsoft.com/en-us/nuget/consume-packages/Central-Package-Management  -->
+	<PropertyGroup>
+		<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
+		<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
+	</PropertyGroup>
+	<ItemGroup Label="Package Versions used by this repository">
+		<!-- Direct dependencies -->
+
+		<PackageVersion Include="Microsoft.Extensions.Logging" Version="7.0.0" />
+		<PackageVersion Include="System.Text.Json" Version="7.0.0" />
+		<PackageVersion Include="Xunit.Abstractions" Version="2.0.3" />
+
+		<PackageVersion Include="NSubstitute" Version="5.1.0" />
+		<PackageVersion Include="ModelBuilder" Version="8.6.0" />
+		<PackageVersion Include="FluentAssertions" Version="6.12.0" />
+		<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
+		<PackageVersion Include="xunit" Version="2.8.0" />
+		<PackageVersion Include="xunit.runner.visualstudio" Version="2.8.0" PrivateAssets="All"
+		                IncludeAssets="runtime; build; native; contentfiles; analyzers; buildtransitive" />
+		<PackageVersion Include="coverlet.msbuild" Version="6.0.2" PrivateAssets="All"
+		                IncludeAssets="runtime; build; native; contentfiles; analyzers; buildtransitive" />
+		<!-- Overrides -->
+	</ItemGroup>
+	<ItemGroup Label="Global Package References, added to every project">
+		<GlobalPackageVersion Include="ReferenceTrimmer" Version="3.1.22" PrivateAssets="All" />
+		<GlobalPackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0" PrivateAssets="All" />
+	</ItemGroup>
+</Project>
diff --git a/Neovolve.Logging.Xunit.UnitTests/CacheLoggerTests.cs b/Neovolve.Logging.Xunit.UnitTests/CacheLoggerTests.cs
@@ -72,7 +72,7 @@ public void CanDisposeWithoutFactory()
             using var sut = new CacheLogger();
 
             // ReSharper disable once AccessToDisposedClosure
-            Action action = () => sut.Dispose();
+            var action = () => sut.Dispose();
 
             action.Should().NotThrow();
         }
@@ -90,6 +90,32 @@ public void DisposeCallsDisposeOnFactory()
             factory.Received().Dispose();
         }
 
+        [Fact]
+        public void EntriesRecordsLogStateInformation()
+        {
+            using var sut = new CacheLogger();
+
+            var first = Guid.NewGuid();
+            var second = Guid.NewGuid().ToString();
+            var third = Environment.TickCount;
+            var format = "This {First} value is not the same as {Second}. We are also interested in {Third}.";
+
+            sut.LogInformation(format,
+                first, second, third);
+
+            var entry = sut.Last!;
+
+            entry.State.Should().NotBeNull();
+
+            var data = ((IEnumerable<KeyValuePair<string, object>>)entry.State).ToList();
+
+            data.Should().NotBeNull();
+            data.Should().ContainKey("First").WhoseValue.Should().Be(first);
+            data.Should().ContainKey("Second").WhoseValue.Should().Be(second);
+            data.Should().ContainKey("Third").WhoseValue.Should().Be(third);
+            data.Should().ContainKey("{OriginalFormat}").WhoseValue.Should().Be(format);
+        }
+
         [Fact]
         public void EntriesReturnsRecordsEntriesInOrder()
         {
@@ -215,7 +241,7 @@ public void LastReturnsLogEntry()
             sut.Last!.EventId.Should().Be(eventId);
             sut.Last.Exception.Should().Be(exception);
             sut.Last.LogLevel.Should().Be(logLevel);
-            sut.Last.State.Should().Be(state);
+            sut.Last.State.Should().ContainKey("State").WhoseValue.Should().Be(state);
             sut.Last.Message.Should().Be(data);
         }
 
@@ -246,7 +272,7 @@ public void LogCachesLogMessage(LogLevel logLevel)
             entry.EventId.Should().Be(eventId);
             entry.Exception.Should().Be(exception);
             entry.LogLevel.Should().Be(logLevel);
-            entry.State.Should().Be(state);
+            sut.Last!.State.Should().ContainKey("State").WhoseValue.Should().Be(state);
             entry.Message.Should().Be(data);
         }
 
@@ -478,7 +504,7 @@ public void LogThrowsExceptionWithNullFormatter()
             using var sut = new CacheLogger(logger);
 
             // ReSharper disable once AccessToDisposedClosure
-            Action action = () => sut.Log(logLevel, eventId, state, exception, null!);
+            var action = () => sut.Log(logLevel, eventId, state, exception, null!);
 
             action.Should().Throw<ArgumentNullException>();
         }

diff --git a/Neovolve.Logging.Xunit.UnitTests/FilterLoggerTests.cs b/Neovolve.Logging.Xunit.UnitTests/FilterLoggerTests.cs
@@ -49,7 +49,7 @@ public void LogThrowsExceptionWithNullFormatter()
 
             var sut = new IsEnabledWrapper();
 
-            Action action = () => sut.Log(LogLevel.Critical, default, state, exception, null!);
+            var action = () => sut.Log(LogLevel.Critical, default, state, exception, null!);
 
             action.Should().Throw<ArgumentNullException>();
         }

diff --git a/Neovolve.Logging.Xunit.UnitTests/Formatters.cs b/Neovolve.Logging.Xunit.UnitTests/Formatters.cs
@@ -1,8 +1,8 @@
-using System;
-using Microsoft.Extensions.Logging;
-
-namespace Neovolve.Logging.Xunit.UnitTests
+namespace Neovolve.Logging.Xunit.UnitTests
 {
+    using System;
+    using Microsoft.Extensions.Logging;
+
     internal static class Formatters
     {
         // This an example message formatter.

diff --git a/Neovolve.Logging.Xunit.UnitTests/LogEntryTests.cs b/Neovolve.Logging.Xunit.UnitTests/LogEntryTests.cs
@@ -1,6 +1,7 @@
 namespace Neovolve.Logging.Xunit.UnitTests
 {
     using System;
+    using System.Collections.Generic;
     using FluentAssertions;
     using global::Xunit;
     using Microsoft.Extensions.Logging;
@@ -16,16 +17,70 @@ public void ReturnsConstructorValuesInProperties()
             var state = Guid.NewGuid().ToString();
             var exception = new ArgumentNullException(Guid.NewGuid().ToString(), Guid.NewGuid().ToString());
             var message = Guid.NewGuid().ToString();
-            var scopes = new object[] {Guid.NewGuid().ToString()};
+            var scopes = new object[] { Guid.NewGuid().ToString() };
 
             var sut = new LogEntry(level, eventId, state, exception, message, scopes);
 
             sut.EventId.Should().Be(eventId);
             sut.Exception.Should().Be(exception);
-            sut.State.Should().Be(state);
             sut.LogLevel.Should().Be(level);
             sut.Message.Should().Be(message);
             sut.Scopes.Should().BeEquivalentTo(scopes);
         }
+
+        [Fact]
+        public void StateReturnsEmptyWhenStateValueIsNull()
+        {
+            var level = LogLevel.Error;
+            var eventId = Model.Create<EventId>();
+            object? state = null;
+            var exception = new ArgumentNullException(Guid.NewGuid().ToString(), Guid.NewGuid().ToString());
+            var message = Guid.NewGuid().ToString();
+            var scopes = new object[] { Guid.NewGuid().ToString() };
+
+            var sut = new LogEntry(level, eventId, state, exception, message, scopes);
+
+            sut.State.Should().BeEmpty();
+        }
+
+        [Fact]
+        public void StateReturnsStateAsEntryWhenNotExpectedType()
+        {
+            var level = LogLevel.Error;
+            var eventId = Model.Create<EventId>();
+            object? state = Guid.NewGuid().ToString();
+            var exception = new ArgumentNullException(Guid.NewGuid().ToString(), Guid.NewGuid().ToString());
+            var message = Guid.NewGuid().ToString();
+            var scopes = new object[] { Guid.NewGuid().ToString() };
+
+            var sut = new LogEntry(level, eventId, state, exception, message, scopes);
+
+            sut.State.Should().ContainKey("State").WhoseValue.Should().Be(state);
+        }
+
+        [Fact]
+        public void StateReturnsStateAsStrongTypeWhenExpectedType()
+        {
+            var level = LogLevel.Error;
+            var eventId = Model.Create<EventId>();
+            var first = Guid.NewGuid();
+            var second = Guid.NewGuid().ToString();
+            var third = Environment.TickCount;
+            IReadOnlyList<KeyValuePair<string, object?>> state = new List<KeyValuePair<string, object?>>
+            {
+                new("First", first),
+                new("Second", second),
+                new("Third", third)
+            };
+            var exception = new ArgumentNullException(Guid.NewGuid().ToString(), Guid.NewGuid().ToString());
+            var message = Guid.NewGuid().ToString();
+            var scopes = new object[] { Guid.NewGuid().ToString() };
+
+            var sut = new LogEntry(level, eventId, state, exception, message, scopes);
+
+            sut.State.Should().ContainKey("First").WhoseValue.Should().Be(first);
+            sut.State.Should().ContainKey("Second").WhoseValue.Should().Be(second);
+            sut.State.Should().ContainKey("Third").WhoseValue.Should().Be(third);
+        }
     }
 }
diff --git a/Neovolve.Logging.Xunit.UnitTests/LogFactoryTests.cs b/Neovolve.Logging.Xunit.UnitTests/LogFactoryTests.cs
@@ -27,7 +27,7 @@ public void CreateReturnsFactory()
         [Fact]
         public void CreateThrowsExceptionWithNullOutput()
         {
-            Action action = () =>
+            var action = () =>
             {
                 using (LogFactory.Create(null!))
                 {

diff --git a/Neovolve.Logging.Xunit.UnitTests/LoggerExtensionsTests.cs b/Neovolve.Logging.Xunit.UnitTests/LoggerExtensionsTests.cs
@@ -5,7 +5,7 @@
     using global::Xunit;
     using Microsoft.Extensions.Logging;
     using NSubstitute;
-    using LoggerExtensions = Xunit.LoggerExtensions;
+    using LoggerExtensions = LoggerExtensions;
 
     public class LoggerExtensionsTests
     {

diff --git a/Neovolve.Logging.Xunit.UnitTests/LoggerFactoryExtensionsTests.cs b/Neovolve.Logging.Xunit.UnitTests/LoggerFactoryExtensionsTests.cs
@@ -26,7 +26,7 @@ public void AddXunitThrowsExceptionWithNullFactory()
         {
             var output = Substitute.For<ITestOutputHelper>();
 
-            var sut = (ILoggerFactory) null!;
+            var sut = (ILoggerFactory)null!;
 
             Action action = () => sut.AddXunit(output);
 

diff --git a/Neovolve.Logging.Xunit.UnitTests/LoggingBuilderExtensionsTests.cs b/Neovolve.Logging.Xunit.UnitTests/LoggingBuilderExtensionsTests.cs
@@ -54,7 +54,7 @@ public void AddXunitThrowsExceptionWithNullBuilder()
         {
             ILoggingBuilder builder = null!;
 
-            Action action = () => builder.AddXunit(_output);
+            var action = () => builder.AddXunit(_output);
 
             action.Should().Throw<ArgumentNullException>();
         }
@@ -64,7 +64,7 @@ public void AddXunitThrowsExceptionWithNullOutput()
         {
             var builder = Substitute.For<ILoggingBuilder>();
 
-            Action action = () => builder.AddXunit(null!);
+            var action = () => builder.AddXunit(null!);
 
             action.Should().Throw<ArgumentNullException>();
         }

diff --git a/Neovolve.Logging.Xunit.UnitTests/Neovolve.Logging.Xunit.UnitTests.csproj b/Neovolve.Logging.Xunit.UnitTests/Neovolve.Logging.Xunit.UnitTests.csproj
@@ -1,35 +1,25 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
-  <PropertyGroup>
-    <TargetFrameworks>net6.0</TargetFrameworks>
-    <IsPackable>False</IsPackable>
-    <CodeAnalysisRuleSet>..\Solution Items\UnitTest.ruleset</CodeAnalysisRuleSet>
-    <LangVersion>latest</LangVersion>
-    <Nullable>enable</Nullable>
-  </PropertyGroup>
+	<PropertyGroup>
+		<TargetFrameworks>net8.0</TargetFrameworks>
+		<IsPackable>False</IsPackable>
+		<CodeAnalysisRuleSet>..\Solution Items\UnitTest.ruleset</CodeAnalysisRuleSet>
+		<LangVersion>latest</LangVersion>
+		<Nullable>enable</Nullable>
+	</PropertyGroup>
 
-  <ItemGroup>
-    <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
-    <PackageReference Include="xunit" Version="2.8.0" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
-    <PackageReference Include="ModelBuilder" Version="8.6.0" />
-    <PackageReference Include="NSubstitute" Version="5.1.0" />
-    <PackageReference Include="FluentAssertions" Version="6.12.0" />
-    <PackageReference Include="coverlet.msbuild" Version="6.0.2">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
-  </ItemGroup>
+	<ItemGroup>
+		<PackageReference Include="Microsoft.NET.Test.Sdk" />
+		<PackageReference Include="xunit" />
+		<PackageReference Include="xunit.runner.visualstudio" />
+		<PackageReference Include="ModelBuilder" />
+		<PackageReference Include="NSubstitute" />
+		<PackageReference Include="FluentAssertions" />
+		<PackageReference Include="coverlet.msbuild" />
+	</ItemGroup>
 
-  <ItemGroup>
-    <ProjectReference Include="..\Neovolve.Logging.Xunit\Neovolve.Logging.Xunit.csproj" />
-  </ItemGroup>
+	<ItemGroup>
+		<ProjectReference Include="..\Neovolve.Logging.Xunit\Neovolve.Logging.Xunit.csproj" />
+	</ItemGroup>
 
-</Project>
+</Project>
diff --git a/Neovolve.Logging.Xunit.UnitTests/ScopeScenarioTests.cs b/Neovolve.Logging.Xunit.UnitTests/ScopeScenarioTests.cs
@@ -80,11 +80,11 @@ public void TestOutputWritesMessagesUsingScopesWithoutState(string? scopeState)
             Logger.LogTrace("Writing trace message");
             Logger.LogWarning("Writing warning message");
 
-            using (Logger.BeginScope((object) scopeState!))
+            using (Logger.BeginScope((object)scopeState!))
             {
                 Logger.LogInformation("Inside first scope");
 
-                using (Logger.BeginScope((object) scopeState!))
+                using (Logger.BeginScope((object)scopeState!))
                 {
                     Logger.LogInformation("Inside second scope");
                 }

diff --git a/Neovolve.Logging.Xunit.UnitTests/StructuredData.cs b/Neovolve.Logging.Xunit.UnitTests/StructuredData.cs
@@ -14,5 +14,6 @@ public class StructuredData
 
     public string LastName { get; set; } = string.Empty;
 }
+
 // ReSharper restore once ClassNeverInstantiated.Local
-// ReSharper restore UnusedMember.Local
+// ReSharper restore UnusedMember.Local
diff --git a/Neovolve.Logging.Xunit.UnitTests/TestOutputLoggerTests.cs b/Neovolve.Logging.Xunit.UnitTests/TestOutputLoggerTests.cs
@@ -31,7 +31,7 @@ public void BeginScopeReturnsInstance()
 
             actual.Should().NotBeNull();
 
-            Action action = () => actual.Dispose();
+            var action = () => actual.Dispose();
 
             action.Should().NotThrow();
         }
@@ -73,7 +73,7 @@ public void LogIgnoresTestBoundaryFailure()
         {
             // This test should not fail the test runner
             var categoryName = Guid.NewGuid().ToString();
-            var config = new LoggingConfig {IgnoreTestBoundaryException = true};
+            var config = new LoggingConfig { IgnoreTestBoundaryException = true };
 
             var sut = new TestOutputLogger(categoryName, _output, config);
 
@@ -179,7 +179,7 @@ public void LogWritesMessageUsingSpecifiedLineFormatter()
             string Formatter(string logState, Exception? error) => message;
 
             var formatter = Substitute.For<ILogFormatter>();
-            var config = new LoggingConfig {Formatter = formatter};
+            var config = new LoggingConfig { Formatter = formatter };
 
             formatter.Format(0, categoryName, logLevel, eventId, message, exception).Returns(expected);
 
@@ -235,7 +235,7 @@ public void WriteLogIgnoresExceptionWhenIgnoreTestBoundaryExceptionEnabled()
 
             var sut = new TestOutputLogger(categoryName, output, config);
 
-            Action action = () => sut.LogInformation(message);
+            var action = () => sut.LogInformation(message);
 
             action.Should().NotThrow();
         }
@@ -256,7 +256,7 @@ public void WriteLogThrowsExceptionWhenIgnoreTestBoundaryExceptionDisabled()
 
             var sut = new TestOutputLogger(categoryName, output, config);
 
-            Action action = () => sut.LogInformation(message);
+            var action = () => sut.LogInformation(message);
 
             action.Should().Throw<InvalidOperationException>();
         }