Merge pull request #599 from ckadluba/issue-542-disable-hierarchical-props

ckadluba · web-flow · commit 1ad392bd0872 · 2024-11-20T16:29:21.000Z
Implemented #542: Column option ResolveHierarchicalPropertyName to force non-hierarchical handling
diff --git a/Build.ps1 b/Build.ps1
@@ -71,13 +71,17 @@ try
         {
             Push-Location "$testProjectPath"
 
-            echo "build: Testing project in $testProjectPath"
-            & dotnet test -c Release --collect "XPlat Code Coverage"
-            if ($LASTEXITCODE -ne 0)
+            # Run tests for different targets in sequence to avoid database tests
+            # to fail because of concurrency problems
+            foreach ($tfm in @( "net462", "net472", "net8.0" ))
             {
-                exit 2
+                echo "build: Testing project in $testProjectPath for target $tfm"
+                & dotnet test -c Release --collect "XPlat Code Coverage" --framework "$tfm" --results-directory "./TestResults/$tfm"
+                if ($LASTEXITCODE -ne 0)
+                {
+                    exit 2
+                }
             }
-
         }
         finally
         {
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,3 +1,6 @@
+# 8.1.0
+* Implemented #542: Column option `ResolveHierarchicalPropertyName` to force non-hierarchical handling
+
 # 8.0.1
 * Refactoring and performance optimizations in batched and audit sink
 * Create perftest result on release
diff --git a/README.md b/README.md
@@ -339,6 +339,7 @@ Each Standard Column in the `ColumnOptions.Store` list and any custom columns yo
 
 * `ColumnName`
 * `PropertyName`
+* `ResolveHierarchicalPropertyName`
 * `DataType`
 * `AllowNull`
 * `DataLength`
@@ -352,7 +353,11 @@ Any valid SQL column name can be used. Standard Columns have default names assig
 
 The optional name of a Serilog property to use as the value for a custom column. If not provided, the property used is the one that has the same name as the specified ColumnName. It applies only to custom columns defined in `AdditionalColumns` and is ignored for standard columns.
 
-PropertyName can contain a simple property name like `SomeProperty` but it can also be used to hierachically reference sub-properties with expressions like `SomeProperty.SomeSubProperty.SomeThirdLevelProperty`. This can be used to easily bind additional columns to specific sub-properties following the paradigm of structured logging. Please be aware that collections are not supported. This means expressions like `SomeProperty.SomeArray[2]` will not work.
+PropertyName can contain a simple property name like `SomeProperty` but it can also be used to hierarchically reference sub-properties with expressions like `SomeProperty.SomeSubProperty.SomeThirdLevelProperty`. This can be used to easily bind additional columns to specific sub-properties following the paradigm of structured logging. Please be aware that collections are not supported. This means expressions like `SomeProperty.SomeArray[2]` will not work. Hierarchical property resolution can be disabled using `ResolveHierarchicalPropertyName` in case you need property names containing dots which should not be treated as hierarchical.
+
+### ResolveHierarchicalPropertyName
+
+Controls whether hierarchical sub-property expressions in `PropertyName` are evaluated (see above). The default is `true`. If set to `false` any value is treated as a simple property name and no hierarchical sub-property binding is done.
 
 ### DataType
 
@@ -385,7 +390,7 @@ Numeric types use the default precision and scale. For numeric types, you are re
 
 ### AllowNull
 
-Determines whether or not the column can store SQL `NULL` values. The default is true. Some of the other features like `PrimaryKey` have related restrictions, and some of the Standard Columns impose restrictions (for example, the `Id` column never allows nulls).
+Determines whether the column can store SQL `NULL` values. The default is `true`. Some of the other features like `PrimaryKey` have related restrictions, and some of the Standard Columns impose restrictions (for example, the `Id` column never allows nulls).
 
 ### DataLength
 
diff --git a/sample/WorkerServiceDemo/Worker.cs b/sample/WorkerServiceDemo/Worker.cs
@@ -27,6 +27,12 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
             };
             _logger.LogInformation("{@Structured} {@Scalar}", structured, "Scalar Value");
 
+
+            // Logging a property with dots in its name to AdditionalColumn3
+            // but treat it as unstructured according to configuration in AdditionalColumns in appsettings.json
+            _logger.LogInformation("Non-structured property with dot-name to AdditionalColumn3 {@NonstructuredProperty.WithNameContainingDots.Name}",
+                new Random().Next().ToString());
+
             while (!stoppingToken.IsCancellationRequested)
             {
                 _logger.LogInformation("Worker running at: {time}. CustomProperty1: {CustomProperty1}",
diff --git a/sample/WorkerServiceDemo/appsettings.json b/sample/WorkerServiceDemo/appsettings.json
@@ -27,7 +27,7 @@
               "columnName": "Timestamp",
               "convertToUtc": false
             },
-            "additionalColumns": [
+            "customColumns": [
               {
                 "columnName": "AdditionalColumn1",
                 "propertyName": "CustomProperty1",
@@ -37,6 +37,12 @@
                 "columnName": "AdditionalColumn2",
                 "propertyName": "Structured.Name",
                 "dataType": "12"
+              },
+              {
+                "columnName": "AdditionalColumn3",
+                "propertyName": "NonstructuredProperty.WithNameContainingDots.Name",
+                "resolveHierarchicalPropertyName": false,
+                "dataType": "12"
               }
             ]
           },
@@ -63,11 +69,22 @@
               "columnName": "Timestamp",
               "convertToUtc": false
             },
-            "additionalColumns": [
+            "customColumns": [
               {
                 "columnName": "AdditionalColumn1",
                 "propertyName": "CustomProperty1",
                 "dataType": "12"
+              },
+              {
+                "columnName": "AdditionalColumn2",
+                "propertyName": "Structured.Name",
+                "dataType": "12"
+              },
+              {
+                "columnName": "AdditionalColumn3",
+                "propertyName": "NonstructuredProperty.WithNameContainingDots.Name",
+                "resolveHierarchicalPropertyName": false,
+                "dataType": "12"
               }
             ]
           }
diff --git a/src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/ColumnCollection.cs b/src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/ColumnCollection.cs
@@ -1,11 +1,11 @@
 ﻿// Copyright 2015 Serilog Contributors
-// 
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -30,6 +30,12 @@ protected override object GetElementKey(ConfigurationElement element)
         {
             return ((ColumnConfig)element)?.ColumnName;
         }
+
+        // For testing
+        internal void Add(ColumnConfig config)
+        {
+            BaseAdd(config);
+        }
     }
 }
 
diff --git a/src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/ColumnConfig.cs b/src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/ColumnConfig.cs
@@ -1,11 +1,11 @@
 // Copyright 2015 Serilog Contributors
-// 
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -48,6 +48,13 @@ public virtual string PropertyName
             set { this[nameof(PropertyName)] = value; }
         }
 
+        [ConfigurationProperty("ResolveHierarchicalPropertyName")]
+        public virtual string ResolveHierarchicalPropertyName
+        {
+            get { return (string)this[nameof(ResolveHierarchicalPropertyName)]; }
+            set { this[nameof(ResolveHierarchicalPropertyName)] = value; }
+        }
+
         [ConfigurationProperty("DataType")]
         public string DataType
         {
@@ -85,6 +92,8 @@ internal SqlColumn AsSqlColumn()
 
             SetProperty.IfProvidedNotEmpty<string>(this, nameof(PropertyName), (val) => sqlColumn.PropertyName = val);
 
+            SetProperty.IfProvided<bool>(this, nameof(ResolveHierarchicalPropertyName), (val) => sqlColumn.ResolveHierarchicalPropertyName = val);
+
             SetProperty.IfProvidedNotEmpty<string>(this, nameof(DataType), (val) => sqlColumn.SetDataTypeFromConfigString(val));
 
             SetProperty.IfProvided<int>(this, nameof(DataLength), (val) => sqlColumn.DataLength = val);
diff --git a/src/Serilog.Sinks.MSSqlServer/Serilog.Sinks.MSSqlServer.csproj b/src/Serilog.Sinks.MSSqlServer/Serilog.Sinks.MSSqlServer.csproj
@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <Description>A Serilog sink that writes events to Microsoft SQL Server and Azure SQL</Description>
-    <VersionPrefix>8.0.1</VersionPrefix>
+    <VersionPrefix>8.1.0</VersionPrefix>
     <EnablePackageValidation>true</EnablePackageValidation>
     <PackageValidationBaselineVersion>8.0.0</PackageValidationBaselineVersion>
     <Authors>Michiel van Oudheusden;Christian Kadluba;Serilog Contributors</Authors>
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlColumn.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlColumn.cs
@@ -13,6 +13,7 @@ public class SqlColumn
         private SqlDbType _dataType = SqlDbType.VarChar; // backwards-compatibility default
         private string _columnName = string.Empty;
         private string _propertyName;
+        private bool _resolveHierarchicalPropertyName = true;
         private readonly List<string> _propertyNameHierarchy = new List<string>();
 
         /// <summary>
@@ -109,7 +110,20 @@ public string PropertyName
             set
             {
                 _propertyName = value;
-                ParseHierarchicalPropertyName(value);
+                ParseHierarchicalPropertyName();
+            }
+        }
+
+        /// <summary>
+        /// Controls whether hierarchical expressions in `PropertyName` are evaluated. The default is `true`.
+        /// </summary>
+        public bool ResolveHierarchicalPropertyName
+        {
+            get => _resolveHierarchicalPropertyName;
+            set
+            {
+                _resolveHierarchicalPropertyName = value;
+                ParseHierarchicalPropertyName();
             }
         }
 
@@ -130,8 +144,6 @@ internal bool HasHierarchicalPropertyName
         // and allows casting back to the Standard Column without a lot of switch gymnastics.
         internal StandardColumn? StandardColumnIdentifier { get; set; }
 
-        internal Type StandardColumnType { get; set; }
-
         /// <summary>
         /// Converts a SQL sink SqlColumn object to a System.Data.DataColumn object. The original
         /// SqlColumn object is stored in the DataColumn's ExtendedProperties collection.
@@ -170,9 +182,17 @@ internal void SetDataTypeFromConfigString(string requestedSqlType)
             DataType = sqlType;
         }
 
-        private void ParseHierarchicalPropertyName(string propertyName)
+        private void ParseHierarchicalPropertyName()
         {
-            _propertyNameHierarchy.AddRange(propertyName.Split('.'));
+            _propertyNameHierarchy.Clear();
+            if (ResolveHierarchicalPropertyName)
+            {
+                _propertyNameHierarchy.AddRange(PropertyName.Split('.'));
+            }
+            else
+            {
+                _propertyNameHierarchy.Add(PropertyName);
+            }
         }
     }
 }
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Implementations/System.Configuration/SystemConfigurationColumnOptionsProviderTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Implementations/System.Configuration/SystemConfigurationColumnOptionsProviderTests.cs
@@ -1,5 +1,6 @@
 ﻿using System.Data;
-using Moq;
+using System.Linq;
+using FluentAssertions;
 using Serilog.Configuration;
 using Serilog.Sinks.MSSqlServer.Configuration;
 using Serilog.Sinks.MSSqlServer.Tests.TestUtils;
@@ -28,7 +29,7 @@ public void ConfigureColumnOptionsReadsTraceIdColumnOptions()
             _configurationSection.TraceId.AllowNull = "false";
             _configurationSection.TraceId.DataType = "22"; // VarChar
             var columnOptions = new MSSqlServer.ColumnOptions();
-            
+
             // Act
             _sut.ConfigureColumnOptions(_configurationSection, columnOptions);
 
@@ -56,5 +57,48 @@ public void ConfigureColumnOptionsReadsSpanIdColumnOptions()
             Assert.False(columnOptions.SpanId.AllowNull);
             Assert.Equal(SqlDbType.VarChar, columnOptions.SpanId.DataType);
         }
+
+        [Fact]
+        public void ConfigureColumnOptionsReadsAdditionalColumnsResolveHierarchicalPropertyName()
+        {
+            // Arrange
+            const string columnName = "AdditionalColumn1";
+            var columnConfig = new ColumnConfig
+            {
+                ColumnName = columnName,
+                ResolveHierarchicalPropertyName = "false"
+            };
+            _configurationSection.Columns.Add(columnConfig);
+            var columnOptions = new MSSqlServer.ColumnOptions();
+
+            // Act
+            _sut.ConfigureColumnOptions(_configurationSection, columnOptions);
+
+            // Assert
+            var additionalColumn1 = columnOptions.AdditionalColumns.SingleOrDefault(c => c.ColumnName == columnName);
+            additionalColumn1.Should().NotBeNull();
+            additionalColumn1.ResolveHierarchicalPropertyName.Should().Be(false);
+        }
+
+        [Fact]
+        public void ConfigureColumnOptionsDefaultsAdditionalColumnsResolveHierarchicalPropertyName()
+        {
+            // Arrange
+            const string columnName = "AdditionalColumn1";
+            var columnConfig = new ColumnConfig
+            {
+                ColumnName = columnName
+            };
+            _configurationSection.Columns.Add(columnConfig);
+            var columnOptions = new MSSqlServer.ColumnOptions();
+
+            // Act
+            _sut.ConfigureColumnOptions(_configurationSection, columnOptions);
+
+            // Assert
+            var additionalColumn1 = columnOptions.AdditionalColumns.SingleOrDefault(c => c.ColumnName == columnName);
+            additionalColumn1.Should().NotBeNull();
+            additionalColumn1.ResolveHierarchicalPropertyName.Should().Be(true);
+        }
     }
 }
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Misc/AdditionalPropertiesTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Misc/AdditionalPropertiesTests.cs
@@ -239,5 +239,63 @@ public void WritesLogEventWithColumnsFromHierarchicalNamedProperties()
             VerifyStringColumnWritten(additionalColumn1Name, property1Value);
             VerifyIntegerColumnWritten(additionalColumn2Name, property2Value);
         }
+
+        [Fact]
+        public void WritesLogEventIgnoringHierarchicalNamedPropertiesWhenResolveHierarchicalPropertyNameIsFalse()
+        {
+            // Arrange
+            const string additionalColumn1Name = "AdditionalColumn1";
+            const string additionalProperty1Name = "AdditionalProperty1.SubProperty1";
+            const string additionalColumn2Name = "AdditionalColumn2";
+            const string additionalProperty2Name = "AdditionalProperty2.SubProperty2.SubSubProperty1";
+            var columnOptions = new MSSqlServer.ColumnOptions
+            {
+                AdditionalColumns = new List<SqlColumn>
+                {
+                    new SqlColumn
+                    {
+                        ColumnName = additionalColumn1Name,
+                        PropertyName = additionalProperty1Name,
+                        ResolveHierarchicalPropertyName = false,
+                        DataType = SqlDbType.NVarChar,
+                        AllowNull = true,
+                        DataLength = 100
+                    },
+                    new SqlColumn
+                    {
+                        ColumnName = additionalColumn2Name,
+                        PropertyName = additionalProperty2Name,
+                        ResolveHierarchicalPropertyName = false,
+                        DataType = SqlDbType.Int,
+                        AllowNull = true
+                    }
+                }
+            };
+            var property1Value = "PropertyValue1";
+            var property2Value = 2;
+
+            // Act
+            Log.Logger = new LoggerConfiguration()
+                .WriteTo.MSSqlServer(
+                    DatabaseFixture.LogEventsConnectionString,
+                    sinkOptions: new MSSqlServerSinkOptions
+                    {
+                        TableName = DatabaseFixture.LogTableName,
+                        AutoCreateSqlTable = true
+                    },
+                    columnOptions: columnOptions,
+                    formatProvider: CultureInfo.InvariantCulture)
+                .CreateLogger();
+            // Log event properties with names containing dots can be created using ForContext()
+            Log.ForContext(additionalProperty1Name, property1Value)
+                .ForContext(additionalProperty2Name, property2Value)
+                .Information("Hello {@AdditionalProperty1.SubProperty1} from thread {@AdditionalProperty2.SubProperty2.SubSubProperty1}");
+            Log.CloseAndFlush();
+
+            // Assert
+            VerifyDatabaseColumnsWereCreated(columnOptions.AdditionalColumns);
+            VerifyStringColumnWritten(additionalColumn1Name, property1Value);
+            VerifyIntegerColumnWritten(additionalColumn2Name, property2Value);
+        }
     }
 }
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/SqlColumnTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/SqlColumnTests.cs

Original file line number	Diff line number	Diff line change
`@@ -71,13 +71,17 @@ try`
`71`	`71`	`{`
`72`	`72`	`Push-Location "$testProjectPath"`
`73`	`73`
`74`		`- echo "build: Testing project in $testProjectPath"`
`75`		`- & dotnet test -c Release --collect "XPlat Code Coverage"`
`76`		`- if ($LASTEXITCODE -ne 0)`
	`74`	`+ # Run tests for different targets in sequence to avoid database tests`
	`75`	`+ # to fail because of concurrency problems`
	`76`	`+ foreach ($tfm in @( "net462", "net472", "net8.0" ))`
`77`	`77`	`{`
`78`		`- exit 2`
	`78`	`+ echo "build: Testing project in $testProjectPath for target $tfm"`
	`79`	`+ & dotnet test -c Release --collect "XPlat Code Coverage" --framework "$tfm" --results-directory "./TestResults/$tfm"`
	`80`	`+ if ($LASTEXITCODE -ne 0)`
	`81`	`+ {`
	`82`	`+ exit 2`
	`83`	`+ }`
`79`	`84`	`}`
`80`		`-`
`81`	`85`	`}`
`82`	`86`	`finally`
`83`	`87`	`{`