Fix the issue when read nested resource untyped in the request #2179
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -425,15 +425,12 @@ protected ODataJsonLightReaderNestedResourceInfo InnerReadUndeclaredProperty(IOD | |
| ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName, payloadTypeReference); | ||
| if (isCollection) | ||
| { | ||
| readerNestedResourceInfo = | ||
| ReadExpandedResourceSetNestedResourceInfo(resourceState, null, payloadTypeReference.ToStructuredType(), propertyName, /*isDeltaResourceSet*/ false); | ||
|
There was a problem hiding this comment. For my own understanding, how does removing the alternative expression fix the issue? #Pending There was a problem hiding this comment. @xuzhg, please correct my understanding if wrong, but just to make sure that I understand the change -- The old code assumed that only links would ever be sent in a request payload, so it didn't support reading nested content. In fact, it should be valid to have nested content in a request payload as well as a response payload, so removing the special case code for request payload handles nested content (whether links or actual content) for both cases. Right? There was a problem hiding this comment. Yes. For "undeclared" property, since it's undeclared, we don't know the type. Since, we don't know the type, why shall we treat it as entity/entity reference if it's a request payload. So, I think in the "undeclared" property reading, we should read it as nested resource info no matter it's request or response. |
||
| } | ||
| else | ||
| { | ||
|
There was a problem hiding this comment. Are "ReadExpandedResourceNestedResourceInfo" There was a problem hiding this comment. @mikepizzo I think There was a problem hiding this comment. The change is only in 'InnerReadUndeclaredProperty', We are in the method because It's "Undeclared" property. |
||
| readerNestedResourceInfo = ReadExpandedResourceNestedResourceInfo(resourceState, null, propertyName, payloadTypeReference.ToStructuredType(), this.MessageReaderSettings); | ||
| } | ||
|
|
||
| resourceState.PropertyAndAnnotationCollector.ValidatePropertyUniquenessOnNestedResourceInfoStart(readerNestedResourceInfo.NestedResourceInfo); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,12 +80,15 @@ public ODataJsonLightEntryAndFeedDeserializerUndeclaredTests() | |
| } | ||
| // ----------- end of edm for entry reader ----------- | ||
|
|
||
| private void ReadEntryPayload(string payload, EdmEntitySet entitySet, EdmEntityType entityType, Action<ODataReader> action, bool readUntypedAsValue = false, bool readRequest = false) | ||
| { | ||
| ODataMessageReaderSettings readerSettings = readUntypedAsValue ? UntypedAsValueReaderSettings : UntypedAsStringReaderSettings; | ||
| var message = new InMemoryMessage() { Stream = new MemoryStream(Encoding.UTF8.GetBytes(payload)) }; | ||
| message.SetHeader("Content-Type", "application/json"); | ||
|
|
||
| using (var msgReader = readRequest ? | ||
| new ODataMessageReader((IODataResponseMessage)message, readerSettings, this.serverModel) : | ||
| new ODataMessageReader((IODataResponseMessage)message, readerSettings, this.serverModel)) | ||
|
There was a problem hiding this comment. What's the difference between the consequent and the alternative expressions here? #Resolved There was a problem hiding this comment. +1 -- should line 90 cast message to IODataRequestMessage? |
||
| { | ||
| var reader = msgReader.CreateODataResourceReader(entitySet, entityType); | ||
| while (reader.Read()) | ||
|
|
@@ -624,6 +627,37 @@ public void ReadNonOpenUnknownTypeInvalidComplexAsValueTest() | |
| Assert.Equal("No.10000000999,Zixing Rd Minhang", undeclaredAddress1.Properties.First(s => string.Equals("UndeclaredStreet", s.Name, StringComparison.Ordinal)).Value); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ReadNonOpenWithoutTypeComplexAsValueTest_InRequest() | ||
| { | ||
| // non-open entity's unknown property type including string & numeric values | ||
| const string payload = @"{""@odata.context"":""http://www.sampletest.com/$metadata#serverEntitySet/$entity"",""Id"":61880128,""UndeclaredAddress1"":" | ||
| + @"{'Street':""No.999,Zixing Rd Minhang"",""UndeclaredStreet"":'No.10000000999,Zixing Rd Minhang'}}"; | ||
| ODataResource entry = null; | ||
| ODataResource undeclaredAddress1 = null; | ||
| this.ReadEntryPayload(payload, this.serverEntitySet, this.serverEntityType, reader => | ||
| { | ||
| if (reader.State == ODataReaderState.ResourceStart) | ||
| { | ||
| if (entry == null) | ||
| { | ||
| entry = (reader.Item as ODataResource); | ||
| } | ||
| else if (undeclaredAddress1 == null) | ||
| { | ||
| undeclaredAddress1 = (reader.Item as ODataResource); | ||
| } | ||
| } | ||
| }, | ||
| /*readUntypedAsValue*/ true, | ||
| /*readRequest*/ true); | ||
|
|
||
| Assert.Single(entry.Properties); | ||
| Assert.Equal("Edm.Untyped", undeclaredAddress1.TypeName); | ||
| Assert.Equal(2, undeclaredAddress1.Properties.Count()); | ||
| Assert.Equal("No.10000000999,Zixing Rd Minhang", undeclaredAddress1.Properties.First(s => string.Equals("UndeclaredStreet", s.Name, StringComparison.Ordinal)).Value); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ReadNonOpenUnknownTypeInvalidComplexNestedAsValueTest() | ||
| { | ||
|
|
@@ -768,6 +802,63 @@ public void ReadNonOpenUnknownComplexTypeCollectionWithInvalidTypeAsValueTest() | |
| Assert.Equal("No.999,Zixing Rd Minhang", address.Properties.First(s => string.Equals("Street", s.Name, StringComparison.Ordinal)).Value); | ||
| Assert.Equal("No.10000000999,Zixing Rd Minhang", address.Properties.First(s => string.Equals("UndeclaredStreet", s.Name, StringComparison.Ordinal)).Value); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ReadNonOpenUnknownComplexTypeCollectionWithInvalidTypeAsValueTest_InRequest() | ||
| { | ||
| const string payload = @"{""@odata.context"":""http://www.sampletest.com/$metadata#serverEntitySet/$entity"",""Id"":61880128,""UndeclaredFloatId"":12.3, | ||
| ""[email protected]"":""Collection(Server.NS.UnknownCollectionType)"",""UndeclaredCollection1"":[{""email"":""[email protected]""},{""email"":""[email protected]""},{""email"":""[email protected]""}],""Address"":{""Street"":""No.999,Zixing Rd Minhang"",""UndeclaredStreet"":""No.10000000999,Zixing Rd Minhang""}}"; | ||
| ODataResource entry = null; | ||
| ODataResource address = null; | ||
| ODataNestedResourceInfo undeclaredCollection1 = null; | ||
| List<ODataResource> untypedCollection = new List<ODataResource>(); | ||
| bool insideCollection = false; | ||
| this.ReadEntryPayload(payload, this.serverEntitySet, this.serverEntityType, reader => | ||
| { | ||
| if (reader.State == ODataReaderState.ResourceStart) | ||
| { | ||
| if (insideCollection) | ||
| { | ||
| untypedCollection.Add(reader.Item as ODataResource); | ||
| } | ||
| else if (entry == null) | ||
| { | ||
| entry = (reader.Item as ODataResource); | ||
| } | ||
| else if (address == null) | ||
| { | ||
| address = (reader.Item as ODataResource); | ||
| } | ||
| } | ||
| else if (reader.State == ODataReaderState.NestedResourceInfoStart) | ||
| { | ||
| ODataNestedResourceInfo nestedInfo = (reader.Item as ODataNestedResourceInfo); | ||
| if (insideCollection = nestedInfo.IsCollection == true) | ||
| { | ||
| undeclaredCollection1 = nestedInfo; | ||
| } | ||
| } | ||
| else if (reader.State == ODataReaderState.NestedResourceInfoEnd) | ||
| { | ||
| ODataNestedResourceInfo nestedInfo = (reader.Item as ODataNestedResourceInfo); | ||
| insideCollection = !(nestedInfo.IsCollection == true); | ||
| } | ||
| else if (reader.State == ODataReaderState.ResourceSetStart) | ||
| { | ||
| undeclaredCollection1.TypeAnnotation = new ODataTypeAnnotation((reader.Item as ODataResourceSet).TypeName); | ||
| } | ||
| }, | ||
| /*readUntypedAsValue*/ true, | ||
| /*readRequest*/ true); | ||
|
|
||
| Assert.Equal(2, entry.Properties.Count()); | ||
| Assert.Equal("Collection(Server.NS.UnknownCollectionType)", undeclaredCollection1.TypeAnnotation.TypeName); | ||
| Assert.Equal(3, untypedCollection.Count()); | ||
| Assert.Equal("[email protected]@[email protected]", String.Concat(untypedCollection.Select(c => ((ODataResource)c).Properties.Single(p => string.Equals(p.Name, "email", StringComparison.Ordinal)).Value))); | ||
| Assert.Equal(2, address.Properties.Count()); | ||
| Assert.Equal("No.999,Zixing Rd Minhang", address.Properties.First(s => string.Equals("Street", s.Name, StringComparison.Ordinal)).Value); | ||
| Assert.Equal("No.10000000999,Zixing Rd Minhang", address.Properties.First(s => string.Equals("UndeclaredStreet", s.Name, StringComparison.Ordinal)).Value); | ||
| } | ||
| #endregion | ||
|
|
||
| #region open entity's property unknown name + known value type | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that ReadExpandedResourceSetNestedResourceInfo (and ReadExpandedResourceNestedResourceInfo) handle the case of the payload just containing links, and that the developer experience behaves the same for this case?