Skip to content

Commit

Permalink
Fix bug in GetAs<T> related to class/struct
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewlock committed Jun 24, 2024
1 parent 0f11754 commit 7b76743
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -245,26 +245,32 @@ public string AsString(string defaultValue, Func<string, bool>? validator)
return defaultValue;
}

// We have to use different methods for class/struct when we _don't_ have a null value, because NRTs don't work properly otherwise
[return: NotNullIfNotNull(nameof(getDefaultValue))]
public T? GetAs<T>(Func<DefaultResult<T>>? getDefaultValue, Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
public T GetAs<T>(Func<DefaultResult<T>> getDefaultValue, Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
{
var result = GetAs(validator, converter);
return TryHandleResult(Telemetry, Key, result, recordValue: true, getDefaultValue, out var value)
? value
: default!; // TryHandleResult always returns true as getDefaultValue != null
}

// We have a valid value
if (result is { Result: { } value, IsValid: true })
{
return value;
}

// don't have a valid value
if (getDefaultValue is null)
{
return default;
}
public T? GetAsClass<T>(Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
where T : class
{
var result = GetAs(validator, converter);
return TryHandleResult(Telemetry, Key, result, recordValue: true, getDefaultValue: null, out var value)
? value
: null;
}

var defaultValue = getDefaultValue();
Telemetry.Record(Key, defaultValue.TelemetryValue, recordValue: true, ConfigurationOrigins.Default);
return defaultValue.Result!;
public T? GetAsStruct<T>(Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
where T : struct
{
var result = GetAs(validator, converter);
return TryHandleResult(Telemetry, Key, result, recordValue: true, getDefaultValue: null, out var value)
? value
: null;
}

[return: NotNullIfNotNull(nameof(getDefaultValue))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ private static void OnConfigurationChanged(ConfigurationBuilder settings)
TraceEnabled = settings.WithKeys(ConfigurationKeys.TraceEnabled).AsBool(),
// RuntimeMetricsEnabled = settings.WithKeys(ConfigurationKeys.RuntimeMetricsEnabled).AsBool(),
// DataStreamsMonitoringEnabled = settings.WithKeys(ConfigurationKeys.DataStreamsMonitoring.Enabled).AsBool(),
SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAs<string>(null, null, s => s),
// Note: Calling GetAsClass<string>() here instead of GetAsString() as we need to get the
// "serialized JToken", which in JsonConfigurationSource is different, as it allows for non-string tokens
SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAsClass<string>(validator: null, converter: s => s),
GlobalSamplingRate = settings.WithKeys(ConfigurationKeys.GlobalSamplingRate).AsDouble(),
// SpanSamplingRules = settings.WithKeys(ConfigurationKeys.SpanSamplingRules).AsString(),
LogsInjectionEnabled = settings.WithKeys(ConfigurationKeys.LogsInjectionEnabled).AsBool(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -627,10 +627,9 @@ public void GetAs_ReturnsNullWhenCantParseAndNoDefault(string value)

var actual = new ConfigurationBuilder(source, _telemetry)
.WithKeys("key")
.GetAs(
getDefaultValue: null,
.GetAsStruct(
validator: null,
converter: _nullableConverter);
converter: _converter);

actual.Should().BeNull();
}
Expand Down

0 comments on commit 7b76743

Please sign in to comment.