Fix subtle bug when deserializing property without value

[gathogojr]

Fixes #1307 by handling a property without value gracefully when deserializing OData payloads

[xuzhg]

why don't use 'OfType', so we don't need line 494 to 499.

[gathogojr]

It was deliberate. I believe OfType would be more expensive

[gathogojr]

@xuzhg Here's the code for a simple benchmark to compare the two:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Diagnosers;
using System.Collections.Generic;

[MemoryDiagnoser]
public class Benchmarks
{
    private IEnumerable<ODataPropertyInfo> properties;

    public Benchmarks()
    {
        List<ODataPropertyInfo> properties = new()
        {
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataPropertyInfo { Name = "A" },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataPropertyInfo { Name = "A" },
            new ODataProperty { Name = "A", Value = 1 },
            new ODataProperty { Name = "A", Value = 1 },
        };

        this.properties = properties;
    }

    [Benchmark]
    public int CountPropertyValues_WithNestedCondition()
    {
        int count = 0;
        foreach (ODataPropertyInfo propertyInfo in properties)
        {
            if (propertyInfo is ODataProperty property) {
                count += property.Value;
            }
        }

        return count;
    }

    [Benchmark]
    public int CountPropertyValues_WithOfType()
    {
        int count = 0;
        foreach (ODataProperty property in properties.OfType<ODataProperty>())
        {
            count += property.Value;
        }

        return count;
    }
}

public class ODataPropertyInfo
{
    public string Name { get; set; }
}

public class ODataProperty : ODataPropertyInfo
{
    public int Value { get; set; }
}

And here are the results:

I think that LINQ overheads are responsible for the difference

[xuzhg]

Why do nothing? need more details

[gathogojr]

@xuzhg What do you suggest we should do here?

[xuzhg]

I think we 'd add at least:

1. Why do we need to catch this reader state in this reading loop?
2. Why do we do nothing in this reader state?

[habbes]

Why didn't we need a case for this state before?

[gathogojr]

@xuzhg @habbes The block for the ODataReaderState.NestedProperty should have been added when we upgraded the ODL dependency to the version that contained this change OData/odata.net#2786.

In OData/odata.net#2786, we introduced support for reading property without value. Before that, we would throw an exception if we came across a property without value. When we changed that, the ODataReaderState.NestedProperty state gets toggled when we read such a property. It's just that after upgrading the ODL dependency, we didn't add any test to verify behaviour when a payload containing a property without value is deserialized. It's easy to verify my theory here. If you add the test in this PR to ASP.NET Core OData release-8.x branch, the control flow will reach the default block and the Debug.Assert statement will fail. That's the reason we need the block that I introduced.

[xuzhg]

How about the other type of properties with value? (Complex, enum)