Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Refactoring to improve null value handling #5812

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.

Sign up for GitHub

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

Merged
snakefoot merged 3 commits into from
May 28, 2025
Merged

Refactoring to improve null value handling #5812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7cb3f8
Config classes with nullable references
snakefoot May 10, 2025
a33b532
WrapperTargetBase - No wrapped Target configured
snakefoot May 10, 2025
56853b1
More fixes
snakefoot May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions 6 src/NLog/Config/DynamicLogLevelFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public LoggingRuleLevelFilter GetSimpleFilterForUpdate()

private bool[] GenerateLogLevels()
{
var levelFilter = _levelFilter.Render(LogEventInfo.CreateNullEvent());
var levelFilter = _levelFilter?.Render(LogEventInfo.CreateNullEvent())?.Trim() ?? string.Empty;
if (string.IsNullOrEmpty(levelFilter))
return LoggingRuleLevelFilter.Off.LogLevels;

Expand Down Expand Up @@ -96,7 +96,7 @@ private bool[] GenerateLogLevels()

private LogLevel GenerateFinalMinLevel()
{
var levelFilter = _finalMinLevelFilter?.Render(LogEventInfo.CreateNullEvent());
var levelFilter = _finalMinLevelFilter?.Render(LogEventInfo.CreateNullEvent())?.Trim() ?? string.Empty;
return ParseLogLevel(levelFilter, null);
}

Expand All @@ -118,7 +118,7 @@ private LogLevel ParseLogLevel(string logLevel, LogLevel levelIfEmpty)
if (string.IsNullOrEmpty(logLevel))
return levelIfEmpty;

return LogLevel.FromString(logLevel.Trim());
return LogLevel.FromString(logLevel);
}
catch (ArgumentException ex)
{
Expand Down
13 changes: 7 additions & 6 deletions 13 src/NLog/Config/DynamicRangeLevelFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,8 @@ public LoggingRuleLevelFilter GetSimpleFilterForUpdate()

private bool[] GenerateLogLevels()
{
var minLevelFilter = _minLevel?.Render(LogEventInfo.CreateNullEvent()) ?? string.Empty;
var maxLevelFilter = _maxLevel?.Render(LogEventInfo.CreateNullEvent()) ?? string.Empty;
if (string.IsNullOrEmpty(minLevelFilter) && string.IsNullOrEmpty(maxLevelFilter))
return LoggingRuleLevelFilter.Off.LogLevels;
var minLevelFilter = _minLevel?.Render(LogEventInfo.CreateNullEvent())?.Trim() ?? string.Empty;
var maxLevelFilter = _maxLevel?.Render(LogEventInfo.CreateNullEvent())?.Trim() ?? string.Empty;

var activeFilter = _activeFilter;
if (!activeFilter.Key.Equals(new MinMaxLevels(minLevelFilter, maxLevelFilter)))
Expand All @@ -85,12 +83,15 @@ private bool[] GenerateLogLevels()

private LogLevel GenerateFinalMinLevel()
{
var levelFilter = _finalMinLevelFilter?.Render(LogEventInfo.CreateNullEvent());
var levelFilter = _finalMinLevelFilter?.Render(LogEventInfo.CreateNullEvent())?.Trim() ?? string.Empty;
return ParseLogLevel(levelFilter, null);
}

private bool[] ParseLevelRange(string minLevelFilter, string maxLevelFilter)
{
if (string.IsNullOrEmpty(minLevelFilter) && string.IsNullOrEmpty(maxLevelFilter))
return LoggingRuleLevelFilter.Off.LogLevels;

LogLevel minLevel = ParseLogLevel(minLevelFilter, LogLevel.MinLevel);
LogLevel maxLevel = ParseLogLevel(maxLevelFilter, LogLevel.MaxLevel);

Expand All @@ -112,7 +113,7 @@ private LogLevel ParseLogLevel(string logLevel, LogLevel levelIfEmpty)
if (string.IsNullOrEmpty(logLevel))
return levelIfEmpty;

return LogLevel.FromString(logLevel.Trim());
return LogLevel.FromString(logLevel);
}
catch (ArgumentException ex)
{
Expand Down
15 changes: 8 additions & 7 deletions 15 src/NLog/Config/LoggingConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,10 +845,9 @@ private static bool IsMissingServiceType(NLogDependencyResolveException resolveE
if (resolveException.ServiceType.IsAssignableFrom(serviceType))
return true;

resolveException = resolveException.InnerException as NLogDependencyResolveException;
if (resolveException != null)
if (resolveException.InnerException is NLogDependencyResolveException dependencyResolveException)
{
return IsMissingServiceType(resolveException, serviceType);
return IsMissingServiceType(dependencyResolveException, serviceType);
}

return false;
Expand Down Expand Up @@ -883,12 +882,14 @@ internal string ExpandSimpleVariables(string input)
[NotNull]
internal string ExpandSimpleVariables(string input, out string matchingVariableName)
{
string output = input;
var culture = StringComparison.OrdinalIgnoreCase;
matchingVariableName = null;

if (Variables.Count > 0 && output?.IndexOf('$') >= 0)
string output = input;

if (!string.IsNullOrEmpty(output) && Variables.Count > 0 && output.IndexOf('$') >= 0)
{
var culture = StringComparison.OrdinalIgnoreCase;

foreach (var kvp in _variables)
{
var layout = kvp.Value;
Expand All @@ -912,7 +913,7 @@ internal string ExpandSimpleVariables(string input, out string matchingVariableN
}
else
{
if (string.Equals(layoutText, input.Trim(), culture))
if (string.Equals(layoutText, input?.Trim() ?? string.Empty, culture))
{
matchingVariableName = kvp.Key;
}
Expand Down
10 changes: 3 additions & 7 deletions 10 src/NLog/Config/LoggingConfigurationElementExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static string GetOptionalValue(this ILoggingConfigurationElement element,
public static bool GetOptionalBooleanValue(this ILoggingConfigurationElement element, string attributeName,
bool defaultValue)
{
string value = element.GetOptionalValue(attributeName, null);
string value = element.GetOptionalValue(attributeName, null)?.Trim() ?? string.Empty;
if (string.IsNullOrEmpty(value))
{
return defaultValue;
Expand All @@ -117,7 +117,7 @@ public static bool GetOptionalBooleanValue(this ILoggingConfigurationElement ele
public static string GetConfigItemTypeAttribute(this ILoggingConfigurationElement element, string sectionNameForRequiredValue = null)
{
var typeAttributeValue = sectionNameForRequiredValue != null ? element.GetRequiredValue("type", sectionNameForRequiredValue) : element.GetOptionalValue("type", null);
return StripOptionalNamespacePrefix(typeAttributeValue)?.Trim();
return StripOptionalNamespacePrefix(typeAttributeValue)?.Trim() ?? string.Empty;
}

/// <summary>
Expand All @@ -131,15 +131,11 @@ public static string GetConfigItemTypeAttribute(this ILoggingConfigurationElemen
private static string StripOptionalNamespacePrefix(string attributeValue)
{
if (attributeValue is null)
{
return null;
}
return string.Empty;

int p = attributeValue.IndexOf(':');
if (p < 0)
{
return attributeValue;
}

return attributeValue.Substring(p + 1);
}
Expand Down
71 changes: 34 additions & 37 deletions 71 src/NLog/Config/LoggingConfigurationParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ private LogLevel ParseLogLevelSafe(string propertyName, string propertyValue, Lo
{
try
{
var internalLogLevel = LogLevel.FromString(propertyValue?.Trim());
return internalLogLevel;
return LogLevel.FromString(propertyValue?.Trim() ?? string.Empty);
}
catch (Exception exception)
{
Expand Down Expand Up @@ -543,11 +542,6 @@ private void ParseRulesElement(ValidatedConfigurationElement rulesElement, IList
}
}

private LogLevel LogLevelFromString(string text)
{
return LogLevel.FromString(ExpandSimpleVariables(text).Trim());
}

/// <summary>
/// Parse {Logger} xml element
/// </summary>
Expand Down Expand Up @@ -660,41 +654,43 @@ private void EnableLevelsForRule(
{
if (enableLevels != null)
{
enableLevels = ExpandSimpleVariables(enableLevels);
finalMinLevel = finalMinLevel != null ? ExpandSimpleVariables(finalMinLevel) : finalMinLevel;
enableLevels = ExpandSimpleVariables(enableLevels).Trim();
finalMinLevel = ExpandSimpleVariables(finalMinLevel).Trim();

if (IsLevelLayout(enableLevels) || IsLevelLayout(finalMinLevel))
{
SimpleLayout simpleLayout = ParseLevelLayout(enableLevels);
SimpleLayout finalMinLevelLayout = ParseLevelLayout(finalMinLevel);
var simpleLayout = ParseLevelLayout(enableLevels);
var finalMinLevelLayout = ParseLevelLayout(finalMinLevel);
rule.EnableLoggingForLevelLayout(simpleLayout, finalMinLevelLayout);
}
else
{
foreach (var logLevel in enableLevels.SplitAndTrimTokens(','))
{
rule.EnableLoggingForLevel(LogLevelFromString(logLevel));
rule.EnableLoggingForLevel(LogLevel.FromString(logLevel));
}
if (finalMinLevel != null)
rule.FinalMinLevel = LogLevelFromString(finalMinLevel);
if (!string.IsNullOrEmpty(finalMinLevel))
rule.FinalMinLevel = LogLevel.FromString(finalMinLevel);
}
}
else
{
minLevel = minLevel != null ? ExpandSimpleVariables(minLevel) : minLevel;
maxLevel = maxLevel != null ? ExpandSimpleVariables(maxLevel) : maxLevel;
finalMinLevel = finalMinLevel != null ? ExpandSimpleVariables(finalMinLevel) : finalMinLevel;
minLevel = ExpandSimpleVariables(minLevel).Trim();
maxLevel = ExpandSimpleVariables(maxLevel).Trim();
finalMinLevel = ExpandSimpleVariables(finalMinLevel).Trim();

if (IsLevelLayout(minLevel) || IsLevelLayout(maxLevel) || IsLevelLayout(finalMinLevel))
{
SimpleLayout finalMinLevelLayout = ParseLevelLayout(finalMinLevel);
SimpleLayout minLevelLayout = ParseLevelLayout(minLevel) ?? finalMinLevelLayout;
SimpleLayout maxLevelLayout = ParseLevelLayout(maxLevel);
var finalMinLevelLayout = ParseLevelLayout(finalMinLevel);
var minLevelLayout = ParseLevelLayout(minLevel) ?? finalMinLevelLayout;
var maxLevelLayout = ParseLevelLayout(maxLevel);
rule.EnableLoggingForLevelsLayout(minLevelLayout, maxLevelLayout, finalMinLevelLayout);
}
else
{
LogLevel finalMinLogLevel = finalMinLevel != null ? LogLevelFromString(finalMinLevel) : null;
LogLevel minLogLevel = minLevel != null ? LogLevelFromString(minLevel) : (finalMinLogLevel ?? LogLevel.MinLevel);
LogLevel maxLogLevel = maxLevel != null ? LogLevelFromString(maxLevel) : LogLevel.MaxLevel;
var finalMinLogLevel = string.IsNullOrEmpty(finalMinLevel) ? null : LogLevel.FromString(finalMinLevel);
var minLogLevel = string.IsNullOrEmpty(minLevel) ? (finalMinLogLevel ?? LogLevel.MinLevel) : LogLevel.FromString(minLevel);
var maxLogLevel = string.IsNullOrEmpty(maxLevel) ? LogLevel.MaxLevel : LogLevel.FromString(maxLevel);
rule.SetLoggingLevels(minLogLevel, maxLogLevel);
if (finalMinLogLevel != null)
rule.FinalMinLevel = finalMinLogLevel;
Expand Down Expand Up @@ -797,8 +793,11 @@ private void ParseLoggingRuleFilters(LoggingRule rule, ValidatedConfigurationEle
{
var filterType = filterElement.GetOptionalValue("type", null) ?? filterElement.Name;
Filter filter = FactoryCreateInstance(filterType, ConfigurationItemFactory.Default.FilterFactory);
ConfigureFromAttributesAndElements(filter, filterElement);
rule.Filters.Add(filter);
if (filter != null)
{
ConfigureFromAttributesAndElements(filter, filterElement);
rule.Filters.Add(filter);
}
}
}

Expand Down Expand Up @@ -1271,7 +1270,7 @@ private Layout TryCreateLayoutInstance(ValidatedConfigurationElement element, Ty

// Check if the 'type' attribute has been specified
string classType = element.GetConfigItemTypeAttribute();
if (classType is null)
if (string.IsNullOrEmpty(classType))
return null;

var expandedClassType = ExpandSimpleVariables(classType, out var matchingVariableName);
Expand Down Expand Up @@ -1314,7 +1313,7 @@ private T TryCreateInstance<T>(ValidatedConfigurationElement element, Type type,

// Check if the 'type' attribute has been specified
string classType = element.GetConfigItemTypeAttribute();
if (classType is null)
if (string.IsNullOrEmpty(classType))
return null;

return FactoryCreateInstance(classType, factory);
Expand Down Expand Up @@ -1408,22 +1407,20 @@ private static Target WrapWithAsyncTargetWrapper(Target target)
private Target WrapWithDefaultWrapper(Target target, ValidatedConfigurationElement defaultWrapperElement)
{
string wrapperTypeName = defaultWrapperElement.GetConfigItemTypeAttribute("targets");
Target wrapperTargetInstance = CreateTargetType(wrapperTypeName);
WrapperTargetBase wtb = wrapperTargetInstance as WrapperTargetBase;
if (wtb is null)
var wrapperTargetInstance = CreateTargetType(wrapperTypeName) as WrapperTargetBase;
if (wrapperTargetInstance is null)
{
throw new NLogConfigurationException("Target type specified on <default-wrapper /> is not a wrapper.");
throw new NLogConfigurationException($"Target type '{wrapperTypeName}' cannot be used as default target wrapper.");
}

var wtb = wrapperTargetInstance;
ParseTargetElement(wrapperTargetInstance, defaultWrapperElement);
while (wtb.WrappedTarget != null)
{
wtb = wtb.WrappedTarget as WrapperTargetBase;
if (wtb is null)
{
throw new NLogConfigurationException(
"Child target type specified on <default-wrapper /> is not a wrapper.");
}
if (wtb.WrappedTarget is WrapperTargetBase nestedWrapper)
wtb = nestedWrapper;
else
throw new NLogConfigurationException($"Target type '{wrapperTypeName}' with nested {wtb.WrappedTarget.GetType()} cannot be used as default target wrapper.");
}

#if !NET35
Expand Down
29 changes: 16 additions & 13 deletions 29 src/NLog/Config/XmlLoggingConfigurationElement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,16 @@ public XmlLoggingConfigurationElement(XmlReader reader)

public XmlLoggingConfigurationElement(XmlReader reader, bool nestedElement)
{
Parse(reader, nestedElement, out var attributes, out var children);
AttributeValues = attributes ?? ArrayHelper.Empty<KeyValuePair<string, string>>();
Children = children ?? ArrayHelper.Empty<XmlLoggingConfigurationElement>();
AttributeValues = ParseAttributes(reader, nestedElement);
LocalName = reader.LocalName;
Children = ParseChildren(reader, nestedElement, out var innerText);
Value = innerText;
}

/// <summary>
/// Gets the element name.
/// </summary>
public string LocalName { get; private set; }
public string LocalName { get; }

/// <summary>
/// Gets the dictionary of attribute values.
Expand All @@ -78,7 +79,7 @@ public XmlLoggingConfigurationElement(XmlReader reader, bool nestedElement)
/// <summary>
/// Gets the value of the element.
/// </summary>
public string Value { get; private set; }
public string Value { get; }

public string Name => LocalName;

Expand Down Expand Up @@ -153,13 +154,11 @@ public void AssertName(params string[] allowedNames)
throw new InvalidOperationException($"Assertion failed. Expected element name '{string.Join("|", allowedNames)}', actual: '{LocalName}'.");
}

private void Parse(XmlReader reader, bool nestedElement, out IList<KeyValuePair<string, string>> attributes, out IList<XmlLoggingConfigurationElement> children)
private static IList<XmlLoggingConfigurationElement> ParseChildren(XmlReader reader, bool nestedElement, out string innerText)
{
ParseAttributes(reader, nestedElement, out attributes);

LocalName = reader.LocalName;
IList<XmlLoggingConfigurationElement> children = null;

children = null;
innerText = null;

if (!reader.IsEmptyElement)
{
Expand All @@ -172,7 +171,7 @@ private void Parse(XmlReader reader, bool nestedElement, out IList<KeyValuePair<

if (reader.NodeType == XmlNodeType.CDATA || reader.NodeType == XmlNodeType.Text)
{
Value += reader.Value;
innerText += reader.Value;
continue;
}

Expand All @@ -184,11 +183,13 @@ private void Parse(XmlReader reader, bool nestedElement, out IList<KeyValuePair<
}
}
}

return children ?? ArrayHelper.Empty<XmlLoggingConfigurationElement>();
}

private static void ParseAttributes(XmlReader reader, bool nestedElement, out IList<KeyValuePair<string, string>> attributes)
private static IList<KeyValuePair<string, string>> ParseAttributes(XmlReader reader, bool nestedElement)
{
attributes = null;
IList<KeyValuePair<string, string>> attributes = null;
if (reader.MoveToFirstAttribute())
{
do
Expand All @@ -204,6 +205,8 @@ private static void ParseAttributes(XmlReader reader, bool nestedElement, out IL
while (reader.MoveToNextAttribute());
reader.MoveToElement();
}

return attributes ?? ArrayHelper.Empty<KeyValuePair<string, string>>();
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions 1 src/NLog/LayoutRenderers/Wrappers/ObjectPathRendererWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ private bool TryGetRawPropertyValue(LogEventInfo logEvent, out object propertyVa
{
if (Inner != null &&
Inner.TryGetRawValue(logEvent, out var rawValue) &&
rawValue != null &&
TryGetPropertyValue(rawValue, out propertyValue))
{
return true;
Expand Down
2 changes: 1 addition & 1 deletion 2 src/NLog/SetupExtensionsBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private static string ResolveTypeAlias<T>(params string[] trimEndings)
int endingPosition = typeAlias.IndexOf(ending, StringComparison.OrdinalIgnoreCase);
if (endingPosition > 0)
{
return typeAlias.Substring(endingPosition);
return typeAlias.Substring(0, endingPosition);
}
}

Expand Down
3 changes: 2 additions & 1 deletion 3 src/NLog/SetupLoadConfigurationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,8 @@ public static T FirstTarget<T>(this ISetupConfigurationTargetBuilder configBuild

internal static IEnumerable<Target> YieldAllTargets(Target target)
{
yield return target;
if (target != null)
yield return target;

if (target is WrapperTargetBase wrapperTarget)
{
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.