From 271380faba495549a3aa6a08cca28e716780a87e Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Tue, 2 May 2023 13:26:10 -0700 Subject: [PATCH 1/4] Part of Issue #885: Enable read the untyped collection --- .../Wrapper/ODataPrimitiveWrapper.cs | 31 ++ .../Wrapper/ODataReaderExtensions.cs | 28 +- .../Wrapper/ODataResourceSetWrapper.cs | 9 + .../Microsoft.AspNetCore.OData.xml | 24 + .../PublicAPI.Unshipped.txt | 4 + .../Wrapper/ODataPrimitiveWrapperTests.cs | 37 ++ .../ODataReaderExtensionsTests.Untyped.cs | 450 ++++++++++++++++++ .../Wrapper/ODataReaderExtensionsTests.cs | 6 +- ...rosoft.AspNetCore.OData.PublicApi.Net6.bsl | 7 + ...t.AspNetCore.OData.PublicApi.NetCore31.bsl | 7 + 10 files changed, 595 insertions(+), 8 deletions(-) create mode 100644 src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataPrimitiveWrapper.cs create mode 100644 test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataPrimitiveWrapperTests.cs create mode 100644 test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataPrimitiveWrapper.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataPrimitiveWrapper.cs new file mode 100644 index 000000000..fbd371d6d --- /dev/null +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataPrimitiveWrapper.cs @@ -0,0 +1,31 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +using Microsoft.OData; + +namespace Microsoft.AspNetCore.OData.Formatter.Wrapper +{ + /// + /// Encapsulates in an Untyped (declared or undeclared) collection. + /// + public class ODataPrimitiveWrapper : ODataItemWrapper + { + /// + /// Initializes a new instance of . + /// + /// The wrapped primitive value. + public ODataPrimitiveWrapper(ODataPrimitiveValue value) + { + Value = value ?? throw Error.ArgumentNull(nameof(value)); + } + + /// + /// Gets the wrapped . + /// + public ODataPrimitiveValue Value { get; } + } +} diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs index 1e1d74629..a091bdb9f 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs @@ -5,7 +5,6 @@ // //------------------------------------------------------------------------------ -using System; using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Threading.Tasks; @@ -191,6 +190,16 @@ private static void ReadODataItem(ODataReader reader, Stack it linkResourceSetWrapper.DeltaItems.Add(linkBaseWrapper); break; + case ODataReaderState.Primitive: + Contract.Assert(itemsStack.Count > 0, "The primitive should be a non-null primitive value within an untyped collection."); + // Be noted: + // 1) if a 'null' value or a resource/object in the untyped collection goes to ODataResource flow + // 2) if a collection value in the untyped collection goes to ODataResourceSet flow + // 3) Since it's untyped, there's no logic for 'Enum' value, it means it's treated as primitive value. + ODataResourceSetWrapper resourceSetParentWrapper = (ODataResourceSetWrapper)itemsStack.Peek(); + resourceSetParentWrapper.Items.Add(new ODataPrimitiveWrapper((ODataPrimitiveValue)reader.Item)); + break; + default: Contract.Assert(false, "We should never get here, it means the ODataReader reported a wrong state."); break; @@ -301,11 +310,18 @@ private static void ReadResourceSet(ODataReader reader, Stack ODataResourceSetWrapper resourceSetWrapper = new ODataResourceSetWrapper(resourceSet); if (itemsStack.Count > 0) { - ODataNestedResourceInfoWrapper parentNestedResourceInfo = (ODataNestedResourceInfoWrapper)itemsStack.Peek(); - Contract.Assert(parentNestedResourceInfo != null, "this has to be an inner resource set. inner resource sets always have a nested resource info."); - Contract.Assert(parentNestedResourceInfo.NestedResourceInfo.IsCollection == true, "Only collection nested properties can contain resource set as their child."); - Contract.Assert(parentNestedResourceInfo.NestedItems.Count == 0, "Each nested property can contain only one resource set as its direct child."); - parentNestedResourceInfo.NestedItems.Add(resourceSetWrapper); + ODataItemWrapper peekWrapper = itemsStack.Peek(); + if (peekWrapper is ODataNestedResourceInfoWrapper parentNestedResourceInfo) + { + Contract.Assert(parentNestedResourceInfo.NestedResourceInfo.IsCollection == true, "Only collection nested properties can contain resource set as their child."); + Contract.Assert(parentNestedResourceInfo.NestedItems.Count == 0, "Each nested property can contain only one resource set as its direct child."); + parentNestedResourceInfo.NestedItems.Add(resourceSetWrapper); + } + else + { + ODataResourceSetWrapper parentResourceSet = (ODataResourceSetWrapper)peekWrapper; + parentResourceSet.Items.Add(resourceSetWrapper); + } } else { diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs index 8314f4988..45744d778 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs @@ -22,6 +22,7 @@ public sealed class ODataResourceSetWrapper : ODataResourceSetBaseWrapper public ODataResourceSetWrapper(ODataResourceSet resourceSet) { Resources = new List(); + Items = new List(); ResourceSet = resourceSet; } @@ -35,5 +36,13 @@ public ODataResourceSetWrapper(ODataResourceSet resourceSet) /// Resource set only contains resources. /// public IList Resources { get; } + + /// + /// Gets the nested items of this ResourceSet. + /// Since we have 'Resources' to contain ODataResource items in the collection (we have to keep it avoid breaking changes). + /// This list is used to hold other items, for example a primitive or a collection, etc. + /// In the next major release, we should combine 'Resources' and 'Items' together. + /// + public IList Items { get; } } } diff --git a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml index 3bca9b823..63484db7c 100644 --- a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml +++ b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml @@ -5884,6 +5884,22 @@ Gets the nested items that are part of this nested resource info. + + + Encapsulates in an Untyped (declared or undeclared) collection. + + + + + Initializes a new instance of . + + The wrapped primitive value. + + + + Gets the wrapped . + + Extension methods for . @@ -5969,6 +5985,14 @@ Resource set only contains resources. + + + Gets the nested items of this ResourceSet. + Since we have 'Resources' to contain ODataResource items in the collection (we have to keep it avoid breaking changes). + This list is used to hold other items, for example a primitive or a collection, etc. + In the next major release, we should combine 'Resources' and 'Items' together. + + Encapsulates an . diff --git a/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt b/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt index 50095b519..bdddf1f50 100644 --- a/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt +++ b/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt @@ -626,10 +626,14 @@ Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataNestedResourceInfoWrapper Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataNestedResourceInfoWrapper.NestedItems.get -> System.Collections.Generic.IList Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataNestedResourceInfoWrapper.NestedResourceInfo.get -> Microsoft.OData.ODataNestedResourceInfo Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataNestedResourceInfoWrapper.ODataNestedResourceInfoWrapper(Microsoft.OData.ODataNestedResourceInfo nestedInfo) -> void +Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataPrimitiveWrapper +Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataPrimitiveWrapper.ODataPrimitiveWrapper(Microsoft.OData.ODataPrimitiveValue value) -> void +Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataPrimitiveWrapper.Value.get -> Microsoft.OData.ODataPrimitiveValue Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataReaderExtensions Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetBaseWrapper Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetBaseWrapper.ODataResourceSetBaseWrapper() -> void Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper +Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper.Items.get -> System.Collections.Generic.IList Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper.ODataResourceSetWrapper(Microsoft.OData.ODataResourceSet resourceSet) -> void Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper.Resources.get -> System.Collections.Generic.IList Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper.ResourceSet.get -> Microsoft.OData.ODataResourceSet diff --git a/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataPrimitiveWrapperTests.cs b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataPrimitiveWrapperTests.cs new file mode 100644 index 000000000..0a9283d9c --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataPrimitiveWrapperTests.cs @@ -0,0 +1,37 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +using Microsoft.AspNetCore.OData.Formatter.Wrapper; +using Microsoft.AspNetCore.OData.Tests.Commons; +using Microsoft.OData; +using Xunit; + +namespace Microsoft.AspNetCore.OData.Tests.Formatter.Wrapper +{ + public class ODataPrimitiveWrapperTests + { + [Fact] + public void Ctor_ThrowsArgumentNull_PrimitiveValue() + { + // Arrange & Act & Assert + ExceptionAssert.ThrowsArgumentNull(() => new ODataPrimitiveWrapper(null), "value"); + } + + [Fact] + public void Ctor_Sets_CorrectProperties() + { + // Arrange & Act & Assert + ODataPrimitiveValue oDataPrimitiveValue = new ODataPrimitiveValue(42); + + // Act + ODataPrimitiveWrapper wrapper = new ODataPrimitiveWrapper(oDataPrimitiveValue); + + // Assert + Assert.Same(oDataPrimitiveValue, wrapper.Value); + } + } +} diff --git a/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs new file mode 100644 index 000000000..6bf0cea7f --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs @@ -0,0 +1,450 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.OData.Formatter.Wrapper; +using Microsoft.AspNetCore.OData.TestCommon; +using Microsoft.OData; +using Microsoft.OData.Edm; +using Xunit; + +namespace Microsoft.AspNetCore.OData.Tests.Formatter.Wrapper +{ + /// + /// Here contains reading test cases for 'Edm.Untyped' (Declared or undeclared) + /// + public partial class ODataReaderExtensionsTests + { + private static readonly EdmModel UntypedModel = BuildUntypedModel(); + + private static EdmModel BuildUntypedModel() + { + EdmModel model = new EdmModel(); + + // Open Complex Type (Info) + EdmComplexType info = new EdmComplexType("NS", "Info", null, false, isOpen: true); + model.AddElement(info); + + // Open Entity Type (Person) + /* + + + + + + + */ + EdmEntityType person = new EdmEntityType("NS", "Person", null, false, isOpen: true); + model.AddElement(person); + person.AddKeys(person.AddStructuralProperty("ID", EdmCoreModel.Instance.GetInt32(false))); + person.AddStructuralProperty("Name", EdmCoreModel.Instance.GetString(true)); + person.AddStructuralProperty("Data", EdmCoreModel.Instance.GetUntyped(true)); + person.AddStructuralProperty("Infos", new EdmCollectionTypeReference + (new EdmCollectionType(new EdmComplexTypeReference(info, false)))); + + EdmEntityContainer defaultContainer = new EdmEntityContainer("NS", "Container"); + model.AddElement(defaultContainer); + EdmEntitySet people = defaultContainer.AddEntitySet("People", person); + return model; + } + + public static TheoryDataSet PrimitiveValueCases + { + get + { + var data = new TheoryDataSet + { + { "42", (decimal)42 }, // why it's decimal? see https://github.com/OData/odata.net/issues/2657 + { "9.19", (decimal)9.19 }, + { "3.1415926535897931", 3.1415926535897931m }, + { "true", true }, + { "false", false }, + { "\"false\"", "false" }, + { "\"a simple string\"", "a simple string" }, + + // looks like a Guid string, but it's string + { "\"969D9BB3-1A1D-40C7-8716-4E29CFE02E81\"", "969D9BB3-1A1D-40C7-8716-4E29CFE02E81" }, + { "\"Red\"", "Red" }, // looks like a Enum string, but it's string + { "null", null } + }; + + return data; + } + } + + [Theory] + [MemberData(nameof(PrimitiveValueCases))] + public async Task ReadOpenResource_WithUntypedPrimitiveValue_WorksAsExpected(string value, object expect) + { + // Arrange + string payload = + "{" + + "\"ID\": 17," + + $"\"Data\": {value}," + + $"\"Dynamic\": {value}" + + "}"; + + IEdmEntitySet people = UntypedModel.EntityContainer.FindEntitySet("People"); + Assert.NotNull(people); // Guard + + // Act + Func> func = mr => mr.CreateODataResourceReaderAsync(people, people.EntityType()); + ODataItemWrapper item = await ReadUntypedPayloadAsync(payload, UntypedModel, func); + + // Assert + Assert.NotNull(item); + ODataResourceWrapper resourceWrapper = Assert.IsType(item); + Assert.Empty(resourceWrapper.NestedResourceInfos); + + Assert.Collection(resourceWrapper.Resource.Properties, + p => + { + // Declared property with 'Edm.Int32' + Assert.Equal("ID", p.Name); + Assert.Equal(17, p.Value); + }, + p => + { + // Declared property with 'Edm.Untyped' + Assert.Equal("Data", p.Name); + Assert.Equal(expect, p.Value); + }, + p => + { + // Un-declared (dynamic) property + Assert.Equal("Dynamic", p.Name); + Assert.Equal(expect, p.Value); + }); + } + + [Fact] + public async Task ReadOpenResource_WithUntypedResourceValue_WorksAsExpected() + { + // Arrange + string resourceValue = "{\"Key1\": \"Value1\", \"Key2\": true}"; + string payload = + "{" + + "\"ID\": 17," + + $"\"Data\": {resourceValue}," + + $"\"Dynamic\": {resourceValue}" + + "}"; + + IEdmEntitySet people = UntypedModel.EntityContainer.FindEntitySet("People"); + Assert.NotNull(people); // Guard + + // Act + Func> func = mr => mr.CreateODataResourceReaderAsync(people, people.EntityType()); + ODataItemWrapper item = await ReadUntypedPayloadAsync(payload, UntypedModel, func); + + // Assert + Assert.NotNull(item); + ODataResourceWrapper resourceWrapper = Assert.IsType(item); + + ODataProperty property = Assert.Single(resourceWrapper.Resource.Properties); + // Declared property with 'Edm.Int32' + Assert.Equal("ID", property.Name); + Assert.Equal(17, property.Value); + + Assert.Equal(2, resourceWrapper.NestedResourceInfos.Count); + + Action verifyAction = (p, n) => + { + Assert.Equal(n, p.NestedResourceInfo.Name); + ODataResourceWrapper nestedResourceWrapper = Assert.IsType(Assert.Single(p.NestedItems)); + Assert.Empty(nestedResourceWrapper.NestedResourceInfos); + Assert.Collection(nestedResourceWrapper.Resource.Properties, + p => + { + Assert.Equal("Key1", p.Name); + Assert.Equal("Value1", p.Value); + }, + p => + { + Assert.Equal("Key2", p.Name); + Assert.True((bool)p.Value); + }); + }; + + Assert.Collection(resourceWrapper.NestedResourceInfos, + p => + { + // Declared property with 'Edm.Untyped' + verifyAction(p, "Data"); + }, + p => + { + // Un-declared (dynamic) property + verifyAction(p, "Dynamic"); + }); + } + + [Fact] + public async Task ReadOpenResource_WithUntypedCollectionValue_WorksAsExpected() + { + // Arrange + string collectionValue = "[" + + "null," + + "{\"Key1\": \"Value1\", \"Key2\": true}," + + "\"A string item\"," + + "42" + + "]"; + + string payload = + "{" + + "\"ID\": 17," + + $"\"Data\": {collectionValue}," + + $"\"Dynamic\": {collectionValue}" + + "}"; + + IEdmEntitySet people = UntypedModel.EntityContainer.FindEntitySet("People"); + Assert.NotNull(people); // Guard + + // Act + Func> func = mr => mr.CreateODataResourceReaderAsync(people, people.EntityType()); + ODataItemWrapper item = await ReadUntypedPayloadAsync(payload, UntypedModel, func); + + // Assert + Assert.NotNull(item); + ODataResourceWrapper resourceWrapper = Assert.IsType(item); + + ODataProperty property = Assert.Single(resourceWrapper.Resource.Properties); + // Declared property with 'Edm.Int32' + Assert.Equal("ID", property.Name); + Assert.Equal(17, property.Value); + + Assert.Equal(2, resourceWrapper.NestedResourceInfos.Count); + + Action verifyAction = (p, n) => + { + Assert.Equal(n, p.NestedResourceInfo.Name); + ODataResourceSetWrapper nestedResourceSetWrapper = Assert.IsType(Assert.Single(p.NestedItems)); + Assert.Equal(2, nestedResourceSetWrapper.Resources.Count); + + Assert.Collection(nestedResourceSetWrapper.Resources, + p => + { + Assert.Null(p); // since it's a 'null' value in collection, + // ODL reads it as ResourceStart, ResourceEnd with null item. + }, + p => + { + Assert.Empty(p.NestedResourceInfos); + Assert.NotNull(p.Resource); + Assert.Collection(p.Resource.Properties, + p => + { + Assert.Equal("Key1", p.Name); + Assert.Equal("Value1", p.Value); + }, + p => + { + Assert.Equal("Key2", p.Name); + Assert.True((bool)p.Value); + }); + }); + + Assert.Equal(2, nestedResourceSetWrapper.Items.Count); + Assert.Collection(nestedResourceSetWrapper.Items, + p => + { + ODataPrimitiveWrapper primitiveWrapper = Assert.IsType(p); + Assert.NotNull(primitiveWrapper.Value); + Assert.Equal("A string item", primitiveWrapper.Value.Value); + }, + p => + { + ODataPrimitiveWrapper primitiveWrapper = Assert.IsType(p); + Assert.NotNull(primitiveWrapper.Value); + Assert.Equal(42, primitiveWrapper.Value.Value); + }); + }; + + Assert.Collection(resourceWrapper.NestedResourceInfos, + p => + { + // Declared property with 'Edm.Untyped' + verifyAction(p, "Data"); + }, + p => + { + // Un-declared (dynamic) property + verifyAction(p, "Dynamic"); + }); + } + + [Fact] + public async Task ReadOpenResource_WithUntypedCollectionValue_OnNestedResourceInfo_WorksAsExpected() + { + // Arrange + string collectionValue = "[" + + "{}," + + "{\"Key1\": \"Value1\"}," + + "{\"Key2\": {}}," + + "{\"Key3\": {\"@odata.type\":\"NS.Info\"}}," + + "{\"Key4\": []}" + + "]"; + + string payload = + "{" + + $"\"Infos\": {collectionValue}" + + "}"; + + IEdmEntitySet people = UntypedModel.EntityContainer.FindEntitySet("People"); + Assert.NotNull(people); // Guard + + // Act + Func> func = mr => mr.CreateODataResourceReaderAsync(people, people.EntityType()); + ODataItemWrapper item = await ReadUntypedPayloadAsync(payload, UntypedModel, func); + + // Assert + Assert.NotNull(item); + ODataResourceWrapper resourceWrapper = Assert.IsType(item); + + Assert.Empty(resourceWrapper.Resource.Properties); + + ODataNestedResourceInfoWrapper nestedResourceInfoWrapper = Assert.Single(resourceWrapper.NestedResourceInfos); + Assert.Equal("Infos", nestedResourceInfoWrapper.NestedResourceInfo.Name); + + ODataResourceSetWrapper nestedResourceSetWrapper = Assert.IsType + (Assert.Single(nestedResourceInfoWrapper.NestedItems)); + + Assert.Collection(nestedResourceSetWrapper.Resources, + p => + { + Assert.Equal("NS.Info", p.Resource.TypeName); + + Assert.Empty(p.Resource.Properties); + Assert.Empty(p.NestedResourceInfos); + }, + p => + { + Assert.Equal("NS.Info", p.Resource.TypeName); + + Assert.Empty(p.NestedResourceInfos); + ODataProperty property = Assert.Single(p.Resource.Properties); + Assert.Equal("Key1", property.Name); + Assert.Equal("Value1", property.Value); + }, + p => + { + Assert.Equal("NS.Info", p.Resource.TypeName); + Assert.Empty(p.Resource.Properties); + + ODataNestedResourceInfoWrapper nestedInfoWrapper = Assert.Single(p.NestedResourceInfos); + Assert.Equal("Key2", nestedInfoWrapper.NestedResourceInfo.Name); + + ODataResourceWrapper nestedResourceWrapper = Assert.IsType(Assert.Single(nestedInfoWrapper.NestedItems)); + Assert.Equal("Edm.Untyped", nestedResourceWrapper.Resource.TypeName); + Assert.Empty(nestedResourceWrapper.Resource.Properties); + Assert.Empty(nestedResourceWrapper.NestedResourceInfos); + }, + p => + { + Assert.Equal("NS.Info", p.Resource.TypeName); + Assert.Empty(p.Resource.Properties); + + ODataNestedResourceInfoWrapper nestedInfoWrapper = Assert.Single(p.NestedResourceInfos); + Assert.Equal("Key3", nestedInfoWrapper.NestedResourceInfo.Name); + + ODataResourceWrapper nestedResourceWrapper = Assert.IsType(Assert.Single(nestedInfoWrapper.NestedItems)); + Assert.Equal("NS.Info", nestedResourceWrapper.Resource.TypeName); + Assert.Empty(nestedResourceWrapper.Resource.Properties); + Assert.Empty(nestedResourceWrapper.NestedResourceInfos); + }, + p => + { + Assert.Equal("NS.Info", p.Resource.TypeName); + Assert.Empty(p.Resource.Properties); + + ODataNestedResourceInfoWrapper nestedInfoWrapper = Assert.Single(p.NestedResourceInfos); + Assert.Equal("Key4", nestedInfoWrapper.NestedResourceInfo.Name); + + ODataResourceSetWrapper nestedResourceSetWrapper = Assert.IsType(Assert.Single(nestedInfoWrapper.NestedItems)); + Assert.Empty(nestedResourceSetWrapper.Resources); + }); + } + + [Fact] + public async Task ReadOpenResource_WithDeepUntypedCollectionValue_WorksAsExpected() + { + // Arrange + const int Depth = 10; + string collectionValue = "[42]"; + for (int i = 0; i < Depth; i++) + { + collectionValue = $"[42, {collectionValue}]"; + } + // [42, [42, [42, [42, [42, [42, [42, [42, [42, [42, [42]]]]]]]]]]] + + string payload = + "{" + + $"\"Data\": {collectionValue}," + + $"\"Dynamic\": {collectionValue}" + + "}"; + + IEdmEntitySet people = UntypedModel.EntityContainer.FindEntitySet("People"); + Assert.NotNull(people); // Guard + + // Act + Func> func = mr => mr.CreateODataResourceReaderAsync(people, people.EntityType()); + ODataItemWrapper item = await ReadUntypedPayloadAsync(payload, UntypedModel, func); + + // Assert + Assert.NotNull(item); + ODataResourceWrapper resourceWrapper = Assert.IsType(item); + + Assert.Empty(resourceWrapper.Resource.Properties); + Assert.Equal(2, resourceWrapper.NestedResourceInfos.Count); + + Action verifyAction = (p, n) => + { + Assert.Equal(n, p.NestedResourceInfo.Name); + ODataResourceSetWrapper nestedResourceSetWrapper = Assert.IsType(Assert.Single(p.NestedItems)); + + for (int i = 0; i <= Depth; ++i) // Why '<= Depth'? Because we have extra single primitive (42) in the deepest level. + { + Assert.Empty(nestedResourceSetWrapper.Resources); + ODataPrimitiveWrapper primitiveWrapper; + if (i == Depth) + { + primitiveWrapper = Assert.IsType(Assert.Single(nestedResourceSetWrapper.Items)); + } + else + { + Assert.Equal(2, nestedResourceSetWrapper.Items.Count); + primitiveWrapper = Assert.IsType(nestedResourceSetWrapper.Items.First()); + nestedResourceSetWrapper = Assert.IsType(nestedResourceSetWrapper.Items.Last()); + } + + Assert.NotNull(primitiveWrapper.Value); + Assert.Equal(42, primitiveWrapper.Value.Value); // in the collection, ODL reads 42 as integer. It's different with 42 in untyped single value property + } + }; + + Assert.Collection(resourceWrapper.NestedResourceInfos, + p => + { + // Declared property with 'Edm.Untyped' + verifyAction(p, "Data"); + }, + p => + { + // Un-declared (dynamic) property + verifyAction(p, "Dynamic"); + }); + } + + private async Task ReadUntypedPayloadAsync(string payload, + IEdmModel edmModel, Func> createReader, + bool readUntypedAsString = false) + { + return await ReadPayloadAsync(payload, edmModel, createReader, ODataVersion.V4, readUntypedAsString); + } + } +} diff --git a/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.cs b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.cs index 3e65bdbdc..92255f24a 100644 --- a/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.cs +++ b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.OData.Tests.Formatter.Wrapper { - public class ODataReaderExtensionsTests + public partial class ODataReaderExtensionsTests { private static readonly EdmModel Model; @@ -574,7 +574,8 @@ public async Task ReadEntityReferenceLinksSetWorksAsExpected_V401() } private async Task ReadPayloadAsync(string payload, - IEdmModel edmModel, Func> createReader, ODataVersion version = ODataVersion.V4) + IEdmModel edmModel, Func> createReader, ODataVersion version = ODataVersion.V4, + bool readUntypedAsString = false) { var message = new InMemoryMessage() { @@ -586,6 +587,7 @@ public async Task ReadEntityReferenceLinksSetWorksAsExpected_V401() { BaseUri = new Uri("http://localhost/$metadata"), EnableMessageStreamDisposal = true, + ReadUntypedAsString = readUntypedAsString, Version = version, }; diff --git a/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.Net6.bsl b/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.Net6.bsl index 4ce2d44b6..97b8eec37 100644 --- a/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.Net6.bsl +++ b/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.Net6.bsl @@ -2594,6 +2594,12 @@ public class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataEntityReferenceLi Microsoft.OData.ODataEntityReferenceLink EntityReferenceLink { public get; } } +public class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataPrimitiveWrapper : Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataItemWrapper { + public ODataPrimitiveWrapper (Microsoft.OData.ODataPrimitiveValue value) + + Microsoft.OData.ODataPrimitiveValue Value { public get; } +} + public sealed class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataDeltaDeletedLinkWrapper : Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataDeltaLinkBaseWrapper { public ODataDeltaDeletedLinkWrapper (Microsoft.OData.ODataDeltaDeletedLink deltaDeletedLink) @@ -2623,6 +2629,7 @@ public sealed class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataNestedReso public sealed class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper : Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetBaseWrapper { public ODataResourceSetWrapper (Microsoft.OData.ODataResourceSet resourceSet) + System.Collections.Generic.IList`1[[Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataItemWrapper]] Items { public get; } System.Collections.Generic.IList`1[[Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceWrapper]] Resources { public get; } Microsoft.OData.ODataResourceSet ResourceSet { public get; } } diff --git a/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.NetCore31.bsl b/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.NetCore31.bsl index 4ce2d44b6..97b8eec37 100644 --- a/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.NetCore31.bsl +++ b/test/Microsoft.AspNetCore.OData.Tests/PublicApi/Microsoft.AspNetCore.OData.PublicApi.NetCore31.bsl @@ -2594,6 +2594,12 @@ public class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataEntityReferenceLi Microsoft.OData.ODataEntityReferenceLink EntityReferenceLink { public get; } } +public class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataPrimitiveWrapper : Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataItemWrapper { + public ODataPrimitiveWrapper (Microsoft.OData.ODataPrimitiveValue value) + + Microsoft.OData.ODataPrimitiveValue Value { public get; } +} + public sealed class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataDeltaDeletedLinkWrapper : Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataDeltaLinkBaseWrapper { public ODataDeltaDeletedLinkWrapper (Microsoft.OData.ODataDeltaDeletedLink deltaDeletedLink) @@ -2623,6 +2629,7 @@ public sealed class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataNestedReso public sealed class Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetWrapper : Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceSetBaseWrapper { public ODataResourceSetWrapper (Microsoft.OData.ODataResourceSet resourceSet) + System.Collections.Generic.IList`1[[Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataItemWrapper]] Items { public get; } System.Collections.Generic.IList`1[[Microsoft.AspNetCore.OData.Formatter.Wrapper.ODataResourceWrapper]] Resources { public get; } Microsoft.OData.ODataResourceSet ResourceSet { public get; } } From 22a72c439e3d87c3277fdb351a28856d8b2b1dd9 Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Wed, 3 May 2023 10:57:20 -0700 Subject: [PATCH 2/4] Update src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clément Habinshuti --- .../Formatter/Wrapper/ODataReaderExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs index a091bdb9f..d4f01c75a 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs @@ -310,7 +310,7 @@ private static void ReadResourceSet(ODataReader reader, Stack ODataResourceSetWrapper resourceSetWrapper = new ODataResourceSetWrapper(resourceSet); if (itemsStack.Count > 0) { - ODataItemWrapper peekWrapper = itemsStack.Peek(); + ODataItemWrapper peekedWrapper = itemsStack.Peek(); if (peekWrapper is ODataNestedResourceInfoWrapper parentNestedResourceInfo) { Contract.Assert(parentNestedResourceInfo.NestedResourceInfo.IsCollection == true, "Only collection nested properties can contain resource set as their child."); From 9c91d7734977847600050b0a8e23162a22130bfc Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Wed, 3 May 2023 11:11:56 -0700 Subject: [PATCH 3/4] rename the variable name --- .../Formatter/Wrapper/ODataReaderExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs index d4f01c75a..f45ca1d4b 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs @@ -311,7 +311,7 @@ private static void ReadResourceSet(ODataReader reader, Stack if (itemsStack.Count > 0) { ODataItemWrapper peekedWrapper = itemsStack.Peek(); - if (peekWrapper is ODataNestedResourceInfoWrapper parentNestedResourceInfo) + if (peekedWrapper is ODataNestedResourceInfoWrapper parentNestedResourceInfo) { Contract.Assert(parentNestedResourceInfo.NestedResourceInfo.IsCollection == true, "Only collection nested properties can contain resource set as their child."); Contract.Assert(parentNestedResourceInfo.NestedItems.Count == 0, "Each nested property can contain only one resource set as its direct child."); @@ -319,7 +319,7 @@ private static void ReadResourceSet(ODataReader reader, Stack } else { - ODataResourceSetWrapper parentResourceSet = (ODataResourceSetWrapper)peekWrapper; + ODataResourceSetWrapper parentResourceSet = (ODataResourceSetWrapper)peekedWrapper; parentResourceSet.Items.Add(resourceSetWrapper); } } From c07a2459fd31e82fac822d4c2db2fb9087110a7a Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Wed, 3 May 2023 12:04:29 -0700 Subject: [PATCH 4/4] We consider the order does matter, let's save items in 'Resources' to 'Items' also to keep it same order --- .../ODataResourceDeserializer.cs | 1 + .../Wrapper/ODataReaderExtensions.cs | 1 + .../Wrapper/ODataResourceSetWrapper.cs | 3 ++- .../Microsoft.AspNetCore.OData.xml | 3 ++- .../ODataReaderExtensionsTests.Untyped.cs | 24 ++++++++++++++++++- 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceDeserializer.cs b/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceDeserializer.cs index fe7e96471..f6a6b09da 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceDeserializer.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceDeserializer.cs @@ -775,6 +775,7 @@ private object ReadNestedResourceInline(ODataResourceWrapper resourceWrapper, IE { ODataResourceWrapper resourceWrapper = CreateResourceWrapper(elementType, refLinkWrapper, readContext); resourceSetWrapper.Resources.Add(resourceWrapper); + resourceSetWrapper.Items.Add(resourceWrapper); // in the next major release, we should only use 'Items'. } return resourceSetWrapper; diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs index f45ca1d4b..c0fc09238 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataReaderExtensions.cs @@ -238,6 +238,7 @@ private static void ReadResource(ODataReader reader, Stack ite if (parentResourceSet != null) { parentResourceSet.Resources.Add(resourceWrapper); + parentResourceSet.Items.Add(resourceWrapper);// in the next major release, we should only use 'Items'. } else if (parentDeleteResourceSet != null) { diff --git a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs index 45744d778..2fc4fd957 100644 --- a/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs +++ b/src/Microsoft.AspNetCore.OData/Formatter/Wrapper/ODataResourceSetWrapper.cs @@ -40,7 +40,8 @@ public ODataResourceSetWrapper(ODataResourceSet resourceSet) /// /// Gets the nested items of this ResourceSet. /// Since we have 'Resources' to contain ODataResource items in the collection (we have to keep it avoid breaking changes). - /// This list is used to hold other items, for example a primitive or a collection, etc. + /// This list is used to hold other items also, for example a primitive or a collection, etc. + /// I assume the order of items does matter, so 'Items' contains all 'Resources' item to keep the same order. /// In the next major release, we should combine 'Resources' and 'Items' together. /// public IList Items { get; } diff --git a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml index 63484db7c..1d61924dd 100644 --- a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml +++ b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml @@ -5989,7 +5989,8 @@ Gets the nested items of this ResourceSet. Since we have 'Resources' to contain ODataResource items in the collection (we have to keep it avoid breaking changes). - This list is used to hold other items, for example a primitive or a collection, etc. + This list is used to hold other items also, for example a primitive or a collection, etc. + I assume the order of items does matter, so 'Items' contains all 'Resources' item to keep the same order. In the next major release, we should combine 'Resources' and 'Items' together. diff --git a/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs index 6bf0cea7f..0617a7d8b 100644 --- a/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs +++ b/test/Microsoft.AspNetCore.OData.Tests/Formatter/Wrapper/ODataReaderExtensionsTests.Untyped.cs @@ -249,8 +249,30 @@ public async Task ReadOpenResource_WithUntypedCollectionValue_WorksAsExpected() }); }); - Assert.Equal(2, nestedResourceSetWrapper.Items.Count); + Assert.Equal(4, nestedResourceSetWrapper.Items.Count); Assert.Collection(nestedResourceSetWrapper.Items, + p => + { + Assert.Null(p); // since it's a 'null' value in collection, + // ODL reads it as ResourceStart, ResourceEnd with null item. + }, + p => + { + ODataResourceWrapper resourceWrapper = Assert.IsType(p); + Assert.Empty(resourceWrapper.NestedResourceInfos); + Assert.NotNull(resourceWrapper.Resource); + Assert.Collection(resourceWrapper.Resource.Properties, + p => + { + Assert.Equal("Key1", p.Name); + Assert.Equal("Value1", p.Value); + }, + p => + { + Assert.Equal("Key2", p.Name); + Assert.True((bool)p.Value); + }); + }, p => { ODataPrimitiveWrapper primitiveWrapper = Assert.IsType(p);