diff --git a/src/RestSharp/Request/PropertyCache.Populator.cs b/src/RestSharp/Request/PropertyCache.Populator.cs index 4057ac999..a51fd6dbe 100644 --- a/src/RestSharp/Request/PropertyCache.Populator.cs +++ b/src/RestSharp/Request/PropertyCache.Populator.cs @@ -72,73 +72,82 @@ internal static Populator From(PropertyInfo property) { var getObject = Expression.Lambda>(convertGetterReturnToObject, entity).Compile(); - var populate = GetPopulate(getObject, property); - - return new(property.Name, populate); + var populate = GetPopulate(property); + + // Skip null property values so a DTO with unset optional properties doesn't throw. + // This matches the reflection-based AddObject. + return new( + property.Name, + (model, parameters) => { + var value = getObject(model); + if (value is null) return; + populate(value, parameters); + } + ); } - static Action> GetPopulate(Func getFormattable, RequestProperty requestProperty) - => (model, parameters) => Populate(getFormattable(model), requestProperty, parameters); + static Action> GetPopulate(Func getFormattable, RequestProperty requestProperty) + => (value, parameters) => Populate(getFormattable(value), requestProperty, parameters); - static Action> GetPopulate(Func getConvertible, RequestProperty requestProperty) - => (model, parameters) => Populate(getConvertible(model), requestProperty, parameters); + static Action> GetPopulate(Func getConvertible, RequestProperty requestProperty) + => (value, parameters) => Populate(getConvertible(value), requestProperty, parameters); - static Action> GetPopulate(Func> getFormattables, RequestProperty requestProperty) + static Action> GetPopulate(Func> getFormattables, RequestProperty requestProperty) => requestProperty.ArrayQueryType switch { - RequestArrayQueryType.CommaSeparated => (model, parameters) => PopulateCsv(getFormattables(model), requestProperty, parameters), + RequestArrayQueryType.CommaSeparated => (value, parameters) => PopulateCsv(getFormattables(value), requestProperty, parameters), RequestArrayQueryType.ArrayParameters => GetPopulateArray(getFormattables, requestProperty), _ => (_, _) => { } }; // Here we avoid the cost of checking if the format is CSV or Array every time by caching the result of this evaluation. - static Action> GetPopulate(Func> getConvertibles, RequestProperty requestProperty) + static Action> GetPopulate(Func> getConvertibles, RequestProperty requestProperty) => requestProperty.ArrayQueryType switch { - RequestArrayQueryType.CommaSeparated => (entity, parameters) => PopulateCsv(getConvertibles(entity), requestProperty, parameters), + RequestArrayQueryType.CommaSeparated => (value, parameters) => PopulateCsv(getConvertibles(value), requestProperty, parameters), RequestArrayQueryType.ArrayParameters => GetPopulateArray(getConvertibles, requestProperty), _ => (_, _) => { } }; // Here we avoid the cost of checking if the format is CSV or Array every time by caching the result of this evaluation. - static Action> GetPopulate(Func getEnumerable, RequestProperty requestProperty) + static Action> GetPopulate(Func getEnumerable, RequestProperty requestProperty) => requestProperty.ArrayQueryType switch { - RequestArrayQueryType.CommaSeparated => (entity, parameters) => PopulateCsv(getEnumerable(entity), requestProperty, parameters), + RequestArrayQueryType.CommaSeparated => (value, parameters) => PopulateCsv(getEnumerable(value), requestProperty, parameters), RequestArrayQueryType.ArrayParameters => GetPopulateArray(getEnumerable, requestProperty), _ => (_, _) => { } }; // Here we avoid the cost of checking if the format is CSV or Array every time by caching the result of this evaluation. - static Action> GetPopulate(Func getObject, RequestProperty requestProperty) + static Action> GetPopulate(Func getObject, RequestProperty requestProperty) => requestProperty.ArrayQueryType switch { - RequestArrayQueryType.CommaSeparated => (entity, parameters) => PopulateCsv(getObject(entity), requestProperty, parameters), - RequestArrayQueryType.ArrayParameters => (entity, parameters) => PopulateArray(getObject(entity), requestProperty, parameters), + RequestArrayQueryType.CommaSeparated => (value, parameters) => PopulateCsv(getObject(value), requestProperty, parameters), + RequestArrayQueryType.ArrayParameters => (value, parameters) => PopulateArray(getObject(value), requestProperty, parameters), _ => (_, _) => { } }; // Here we avoid the cost of checking if the format is CSV or Array every time by caching the result of this evaluation. - static Action> GetPopulate(Func getObject, PropertyInfo property) { + static Action> GetPopulate(PropertyInfo property) { var requestProperty = RequestProperty.From(property); // We need to use different conversion mechanisms for each return type. Simply calling `.ToString()` // on every returned object would not take into account special cases like custom formatting, enumeration etc. - // Unchecked casts here are safe because we know the return type of `getObject` is boxed if needed. + // Unchecked casts here are safe because the property value is boxed if needed. return property.PropertyType switch { var formattableType when typeof(IFormattable).IsAssignableFrom(formattableType) => GetPopulate( - entity => Unsafe.As(getObject(entity)), + value => Unsafe.As(value), requestProperty ), var convertibleType when typeof(IConvertible).IsAssignableFrom(convertibleType) => GetPopulate( - entity => Unsafe.As(getObject(entity)), + value => Unsafe.As(value), requestProperty ), var enumerableType when typeof(IEnumerable).IsAssignableFrom(enumerableType) => GetPopulateUnknown( - entity => Unsafe.As(getObject(entity)), + value => Unsafe.As(value), requestProperty ), // At this point we're not necessarily sure we can just treat this as a bare object // and use its type converter. Even though the property itself returns an object, // the object returned itself may need to be treated in a special way, so we check // it as we go. - _ => GetPopulate(getObject, requestProperty) + _ => GetPopulate(static value => value, requestProperty) }; } - static Action> GetPopulateUnknown(Func getEnumerable, RequestProperty requestProperty) { + static Action> GetPopulateUnknown(Func getEnumerable, RequestProperty requestProperty) { if (GetSingleEnumeratedTypeOrNull(requestProperty.Type) is not { } enumeratedType) { // Means we're dealing with a legacy, untyped enumerable instance. // We can just convert it into an enumerable of objects and delegate @@ -162,10 +171,10 @@ _ when typeof(IConvertible).IsAssignableFrom(enumeratedType) => GetPopulate( }; } - static Action> GetPopulateKnown(Func getEnumerable, RequestProperty requestProperty) + static Action> GetPopulateKnown(Func getEnumerable, RequestProperty requestProperty) => requestProperty.ArrayQueryType switch { - RequestArrayQueryType.CommaSeparated => (entity, parameters) => PopulateCsvUnknown( - getEnumerable(entity), + RequestArrayQueryType.CommaSeparated => (value, parameters) => PopulateCsvUnknown( + getEnumerable(value), requestProperty, parameters ), @@ -173,26 +182,32 @@ static Action> GetPopulateKnown(Func g _ => (_, _) => { } }; // Here we avoid the cost of checking if the format is CSV or Array every time by caching the result of this evaluation. - static Action> GetPopulateArray(Func> getFormattables, RequestProperty requestProperty) + static Action> GetPopulateArray( + Func> getFormattables, + RequestProperty requestProperty + ) => GetPopulateArray(getFormattables, formattable => GetStringValue(formattable, requestProperty), requestProperty); - static Action> GetPopulateArray(Func> getConvertibles, RequestProperty requestProperty) + static Action> GetPopulateArray( + Func> getConvertibles, + RequestProperty requestProperty + ) => GetPopulateArray(getConvertibles, GetStringValue, requestProperty); - static Action> GetPopulateArray( - Func> getEnumerable, + static Action> GetPopulateArray( + Func> getEnumerable, Func toString, RequestProperty requestProperty ) where V : class { // We do this to avoid recreating request property on each iteration. var newRequestProperty = requestProperty with { Name = $"{requestProperty.Name}[]" }; - return (entity, parameters) => PopulateArray(getEnumerable(entity), toString, newRequestProperty, parameters); + return (value, parameters) => PopulateArray(getEnumerable(value), toString, newRequestProperty, parameters); } - static Action> GetPopulateArray(Func getEnumerable, RequestProperty requestProperty) { + static Action> GetPopulateArray(Func getEnumerable, RequestProperty requestProperty) { // We do this to avoid recreating request property on each iteration. var newRequestProperty = requestProperty with { Name = $"{requestProperty.Name}[]" }; - return (entity, parameters) => PopulateArray(getEnumerable(entity), newRequestProperty, parameters); + return (value, parameters) => PopulateArray(getEnumerable(value), newRequestProperty, parameters); } static void Populate(IFormattable formattable, RequestProperty requestProperty, ICollection parameters) @@ -393,8 +408,8 @@ static void PopulateArrayKnown(IEnumerable enumerable, RequestProperty requestPr _ => GetStringValueKnown(@object) }; - static Func> GetEnumerableOf(Func getEnumerable, Type enumeratedType) where V : class - => enumeratedType.IsValueType ? entity => getEnumerable(entity).Cast() : entity => Unsafe.As>(getEnumerable(entity)); + static Func> GetEnumerableOf(Func getEnumerable, Type enumeratedType) where V : class + => enumeratedType.IsValueType ? value => getEnumerable(value).Cast() : value => Unsafe.As>(getEnumerable(value)); static Type? GetSingleEnumeratedTypeOrNull(Type enumerableType) { // Get all IEnumerable<> interfaces this type implements. @@ -411,4 +426,4 @@ static Func> GetEnumerableOf(Func getEnumer return enumerableInterfaces.Length == 1 ? enumerableInterfaces[0].GetGenericArguments()[0] : null; } } -} \ No newline at end of file +} diff --git a/test/RestSharp.Tests/Parameters/ObjectParameterTests.NullData.cs b/test/RestSharp.Tests/Parameters/ObjectParameterTests.NullData.cs new file mode 100644 index 000000000..7078d31b6 --- /dev/null +++ b/test/RestSharp.Tests/Parameters/ObjectParameterTests.NullData.cs @@ -0,0 +1,84 @@ +namespace RestSharp.Tests.Parameters; + +public partial class ObjectParameterTests { + [Fact] + public void AddObjectStatic_skips_null_properties() { + var data = new NullableData { Kind = "set" }; + + var request = new RestRequest().AddObjectStatic(data); + + request + .Parameters + .Should() + .ContainSingle() + .Which + .Should() + .BeEquivalentTo(new GetOrPostParameter(nameof(NullableData.Kind), "set")); + } + + [Fact] + public void AddObjectStatic_keeps_non_null_properties_and_skips_null_ones() { + var data = new NullableData { Name = "Bob", Age = 30, Link = null, Values = null, Kind = "set" }; + + var request = new RestRequest().AddObjectStatic(data); + + request + .Parameters + .Should() + .BeEquivalentTo(new[] { + new GetOrPostParameter(nameof(NullableData.Name), "Bob"), + new GetOrPostParameter(nameof(NullableData.Age), "30"), + new GetOrPostParameter(nameof(NullableData.Kind), "set") + }); + } + + [Fact] + public void AddObjectStatic_with_all_null_properties_yields_no_parameters() { + var data = new NullableData { Kind = null }; + + var request = new RestRequest().AddObjectStatic(data); + + request.Parameters.Should().BeEmpty(); + } + + [Fact] + public void AddObjectStatic_null_property_handling_matches_AddObject() { + var data = new NullableData { Name = null, Age = null, Link = null, Values = null, Kind = "set" }; + + var objStatic = new RestRequest().AddObjectStatic(data); + var reflection = new RestRequest().AddObject(data); + + objStatic.Parameters.Should().BeEquivalentTo(reflection.Parameters); + } + + [Fact] + public void AddObjectStatic_reads_each_property_once() { + var data = new ChangingData(); + + var request = new RestRequest().AddObjectStatic(data); + + request + .Parameters + .Should() + .ContainSingle() + .Which + .Should() + .BeEquivalentTo(new GetOrPostParameter(nameof(ChangingData.Value), "set")); + data.ReadCount.Should().Be(1); + } + + class NullableData { + public string Name { get; set; } + public int? Age { get; set; } + public Uri Link { get; set; } + public List Values { get; set; } + public string Kind { get; set; } = "set"; + } + + class ChangingData { + int _readCount; + + public string Value => ++_readCount == 1 ? "set" : null; + internal int ReadCount => _readCount; + } +} diff --git a/test/RestSharp.Tests/RestSharp.Tests.csproj b/test/RestSharp.Tests/RestSharp.Tests.csproj index af5fb6fa9..4778b41a4 100644 --- a/test/RestSharp.Tests/RestSharp.Tests.csproj +++ b/test/RestSharp.Tests/RestSharp.Tests.csproj @@ -29,6 +29,9 @@ + + ObjectParameterTests.cs + UrlBuilderTests.cs @@ -36,4 +39,4 @@ UrlBuilderTests.cs - \ No newline at end of file +