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

Fix reflection based destructuring for redefined property #344

Merged
merged 11 commits into from
May 26, 2021
37 changes: 36 additions & 1 deletion Source/Serilog.Exceptions/Core/ExceptionPropertiesBag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,45 @@ public void AddProperty(string key, object? value)
}
}

this.properties.Add(key, value);
this.AddPairToProperties(key, value);
}

/// <inheritdoc />
public bool ContainsProperty(string key) => this.properties.ContainsKey(key);

private void AddPairToProperties(string key, object? value)
{
#if NET5_0
var i = 0;
while (!this.properties.TryAdd(key, value) && i < 5)
{
key += "$";
i++;
}
#else
key = this.MakeSureKeyIsUnique(key);

this.properties.Add(key, value);
#endif
}

#if !NET5_0
/// <summary>
/// We want to be as robust as possible
/// so even in case of multiple properties with the same name
/// we want to at least try carrying on and keep working.
/// </summary>
private string MakeSureKeyIsUnique(string key)
{
var i = 0;
while (this.properties.ContainsKey(key) && i < 5)
krajek marked this conversation as resolved.
Show resolved Hide resolved
{
key += "$";
krajek marked this conversation as resolved.
Show resolved Hide resolved
i++;
}

return key;
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace Serilog.Exceptions.Test.Destructurers
using Serilog.Exceptions.Core;
using Serilog.Exceptions.Destructurers;
using Xunit;
using Xunit.Abstractions;
using static LogJsonOutputUtils;

public class ReflectionBasedDestructurerTest
Expand Down Expand Up @@ -384,6 +385,18 @@ public void CanDestructureObjectWithHiddenProperty()
Assert.Equal(baseClass.HiddenProperty, info?[$"{typeof(BaseClass).FullName}.{nameof(BaseClass.HiddenProperty)}"]);
}

[Fact]
public void CanDestructureObjectWithRedefinedProperty()
{
var exception = new TestExceptionClassWithNewDefinition() { PublicProperty = 20 };

var propertiesBag = new ExceptionPropertiesBag(exception);
CreateReflectionBasedDestructurer().Destructure(exception, propertiesBag, EmptyDestructurer());

var properties = propertiesBag.GetResultDictionary();
var info = properties[nameof(TestExceptionClassWithNewDefinition.PublicProperty)];
}

private static void Test_ResultOfReflectionDestructurerShouldBeEquivalentToCustomOne(
Exception exception,
IExceptionDestructurer customDestructurer)
Expand Down Expand Up @@ -578,6 +591,11 @@ internal class TestClass
public string? ReferenceType { get; set; }
}

internal class TestExceptionClassWithNewDefinition : TestException
{
public new int PublicProperty { get; set; }
}

internal class BaseClass
{
public virtual int HiddenProperty { get; set; }
Expand Down