Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix delta deleted entry deserialization failure observed when reason appears before id in payload #2787

Merged
merged 1 commit into from
Nov 8, 2023
Merged

Fix delta deleted entry deserialization failure observed when reason appears before id in payload #2787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 64 additions & 51 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,41 +225,46 @@ internal ODataDeletedResource ReadDeletedEntry()
Uri id = null;
DeltaDeletedEntryReason reason = DeltaDeletedEntryReason.Changed;

// If the current node is the id property - read it.
if (this.JsonReader.NodeType == JsonNodeType.Property &&
string.Equals(JsonLightConstants.ODataIdPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal))
while (this.JsonReader.NodeType != JsonNodeType.EndObject && this.JsonReader.NodeType != JsonNodeType.EndOfInput)
{
// Read over the property to move to its value.
this.JsonReader.Read();
if (this.JsonReader.NodeType == JsonNodeType.Property)
{
// If the current node is the id property - read it.
if (string.Equals(JsonLightConstants.ODataIdPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal))
{
// Read over the property to move to its value.
this.JsonReader.Read();

// Read the Id value.
id = this.JsonReader.ReadUriValue();
Debug.Assert(id != null, "value for Id must be provided");
}
// Read the Id value.
id = this.JsonReader.ReadUriValue();
Debug.Assert(id != null, "value for Id must be provided");

this.AssertJsonCondition(JsonNodeType.Property, JsonNodeType.EndObject);
continue;
}
// If the current node is the reason property - read it.
else if (string.Equals(JsonLightConstants.ODataReasonPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal))
{
// Read over the property to move to its value.
this.JsonReader.Read();

// If the current node is the reason property - read it.
if (this.JsonReader.NodeType == JsonNodeType.Property &&
string.Equals(JsonLightConstants.ODataReasonPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal))
{
// Read over the property to move to its value.
this.JsonReader.Read();
// Read the reason value.
if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, this.JsonReader.ReadStringValue(), StringComparison.Ordinal))
{
reason = DeltaDeletedEntryReason.Deleted;
}

// Read the reason value.
if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, this.JsonReader.ReadStringValue(), StringComparison.Ordinal))
{
reason = DeltaDeletedEntryReason.Deleted;
continue;
}
}
}

// Ignore unknown primitive properties in a 4.0 deleted entry
while (this.JsonReader.NodeType != JsonNodeType.EndObject && this.JsonReader.Read())
{
this.JsonReader.Read();

if (this.JsonReader.NodeType == JsonNodeType.StartObject || this.JsonReader.NodeType == JsonNodeType.StartArray)
{
throw new ODataException(Strings.ODataWriterCore_NestedContentNotAllowedIn40DeletedEntry);
}

// Ignore unknown primitive properties in a 4.0 deleted entry
}

return ReaderUtils.CreateDeletedResource(id, reason);
Expand Down Expand Up @@ -2485,44 +2490,52 @@ internal async Task<ODataDeletedResource> ReadDeletedEntryAsync()
Uri id = null;
DeltaDeletedEntryReason reason = DeltaDeletedEntryReason.Changed;

// If the current node is the id property - read it.
if (this.JsonReader.NodeType == JsonNodeType.Property &&
string.Equals(JsonLightConstants.ODataIdPropertyName, await this.JsonReader.GetPropertyNameAsync().ConfigureAwait(false), StringComparison.Ordinal))
while (this.JsonReader.NodeType != JsonNodeType.EndObject && this.JsonReader.NodeType != JsonNodeType.EndOfInput)
{
// Read over the property to move to its value.
await this.JsonReader.ReadAsync()
.ConfigureAwait(false);
if (this.JsonReader.NodeType == JsonNodeType.Property)
{
string propertyName = await this.JsonReader.GetPropertyNameAsync()
.ConfigureAwait(false);

// Read the Id value.
id = await this.JsonReader.ReadUriValueAsync()
.ConfigureAwait(false);
Debug.Assert(id != null, "value for Id must be provided");
}
// If the current node is the id property - read it.
if (string.Equals(JsonLightConstants.ODataIdPropertyName, propertyName, StringComparison.Ordinal))
{
// Read over the property to move to its value.
await this.JsonReader.ReadAsync()
.ConfigureAwait(false);

this.AssertJsonCondition(JsonNodeType.Property, JsonNodeType.EndObject);
// Read the Id value.
id = await this.JsonReader.ReadUriValueAsync()
.ConfigureAwait(false);
Debug.Assert(id != null, "value for Id must be provided");

// If the current node is the reason property - read it.
if (this.JsonReader.NodeType == JsonNodeType.Property &&
string.Equals(JsonLightConstants.ODataReasonPropertyName, await this.JsonReader.GetPropertyNameAsync().ConfigureAwait(false), StringComparison.Ordinal))
{
// Read over the property to move to its value.
await this.JsonReader.ReadAsync()
.ConfigureAwait(false);
continue;
}
// If the current node is the reason property - read it.
else if (string.Equals(JsonLightConstants.ODataReasonPropertyName, propertyName, StringComparison.Ordinal))
{
// Read over the property to move to its value.
await this.JsonReader.ReadAsync()
.ConfigureAwait(false);

// Read the reason value.
if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, await this.JsonReader.ReadStringValueAsync().ConfigureAwait(false), StringComparison.Ordinal))
{
reason = DeltaDeletedEntryReason.Deleted;
// Read the reason value.
if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, await this.JsonReader.ReadStringValueAsync().ConfigureAwait(false), StringComparison.Ordinal))
{
reason = DeltaDeletedEntryReason.Deleted;
}

continue;
}
}
}

// Ignore unknown primitive properties in a 4.0 deleted entry
while (this.JsonReader.NodeType != JsonNodeType.EndObject && await this.JsonReader.ReadAsync().ConfigureAwait(false))
{
await this.JsonReader.ReadAsync().ConfigureAwait(false);

if (this.JsonReader.NodeType == JsonNodeType.StartObject || this.JsonReader.NodeType == JsonNodeType.StartArray)
{
throw new ODataException(ODataErrorStrings.ODataWriterCore_NestedContentNotAllowedIn40DeletedEntry);
}

// Ignore unknown primitive properties in a 4.0 deleted entry
}

return ReaderUtils.CreateDeletedResource(id, reason);
Expand Down
85 changes: 81 additions & 4 deletions ...onalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightResourceDeserializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ public async Task ReadDeletedResourceAsync(string payload, DeltaDeletedEntryReas

[Theory]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}", DeltaDeletedEntryReason.Deleted)]
[InlineData("{\"reason\":\"deleted\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Deleted)]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"changed\"}", DeltaDeletedEntryReason.Changed)]
[InlineData("{\"reason\":\"changed\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Changed)]
public async Task ReadDeletedEntryAsync(string payload, DeltaDeletedEntryReason deletedEntryReason)
{
using (var jsonInputContext = CreateJsonLightInputContext(payload, model))
Expand All @@ -116,11 +118,12 @@ public async Task ReadDeletedEntryAsync(string payload, DeltaDeletedEntryReason
}
}

[Fact]
public async Task ReadDeletedEntryAsync_IgnoresUnexpectedPrimitiveProperties()
[Theory]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"unexpected\":true}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"unexpected\":true,\"reason\":\"deleted\"}")]
[InlineData("{\"unexpected\":true,\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")]
public async Task ReadDeletedEntryAsync_IgnoresUnexpectedPrimitiveProperties(string payload)
{
var payload = "{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"unexpected\":true}";

using (var jsonInputContext = CreateJsonLightInputContext(payload, model))
{
var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext);
Expand All @@ -134,6 +137,45 @@ public async Task ReadDeletedEntryAsync_IgnoresUnexpectedPrimitiveProperties()
}
}

[Theory]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}", DeltaDeletedEntryReason.Deleted)]
[InlineData("{\"reason\":\"deleted\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Deleted)]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"changed\"}", DeltaDeletedEntryReason.Changed)]
[InlineData("{\"reason\":\"changed\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Changed)]
public void ReadDeletedEntry(string payload, DeltaDeletedEntryReason deletedEntryReason)
{
using (var jsonInputContext = CreateJsonLightInputContext(payload: payload, model: model, isAsync: false))
{
var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext);

AdvanceReaderToFirstProperty(jsonLightResourceDeserializer.JsonReader);

var deletedResource = jsonLightResourceDeserializer.ReadDeletedEntry();

Assert.Equal(deletedEntryReason, deletedResource.Reason);
Assert.Equal("http://tempuri.org/Customers(1)", deletedResource.Id.AbsoluteUri);
}
}

[Theory]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"unexpected\":true}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"unexpected\":true,\"reason\":\"deleted\"}")]
[InlineData("{\"unexpected\":true,\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")]
public void ReadDeletedEntry_IgnoresUnexpectedPrimitiveProperties(string payload)
{
using (var jsonInputContext = CreateJsonLightInputContext(payload: payload, model: model, isAsync: false))
{
var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext);

AdvanceReaderToFirstProperty(jsonLightResourceDeserializer.JsonReader);

var deletedResource = jsonLightResourceDeserializer.ReadDeletedEntry();

Assert.Equal(DeltaDeletedEntryReason.Deleted, deletedResource.Reason);
Assert.Equal("http://tempuri.org/Customers(1)", deletedResource.Id.AbsoluteUri);
}
}

[Fact]
public async Task ReadDeltaLinkAsync()
{
Expand Down Expand Up @@ -1732,7 +1774,11 @@ public async Task ReadDeletedResourceAsync_ThrowsExceptionForODataRemovedAnnotat

[Theory]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":{\"foo\":\"bar\"}}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":{\"foo\":\"bar\"},\"reason\":\"deleted\"}")]
[InlineData("{\"nested\":{\"foo\":\"bar\"},\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":[{\"foo\":\"bar\"}]}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":[{\"foo\":\"bar\"}],\"reason\":\"deleted\"}")]
[InlineData("{\"nested\":[{\"foo\":\"bar\"}],\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")]
public async Task ReadDeletedEntryAsync_ThrowsExceptionForNestedContent(string payload)
{
using (var jsonInputContext = CreateJsonLightInputContext(payload, model))
Expand All @@ -1750,6 +1796,30 @@ public async Task ReadDeletedEntryAsync_ThrowsExceptionForNestedContent(string p
}
}

[Theory]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":{\"foo\":\"bar\"}}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":{\"foo\":\"bar\"},\"reason\":\"deleted\"}")]
[InlineData("{\"nested\":{\"foo\":\"bar\"},\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":[{\"foo\":\"bar\"}]}")]
[InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":[{\"foo\":\"bar\"}],\"reason\":\"deleted\"}")]
[InlineData("{\"nested\":[{\"foo\":\"bar\"}],\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")]
public void ReadDeletedEntry_ThrowsExceptionForNestedContent(string payload)
{
using (var jsonInputContext = CreateJsonLightInputContext(payload: payload, model: model, isAsync: false))
{
var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext);

AdvanceReaderToFirstProperty(jsonLightResourceDeserializer.JsonReader);

var exception = Assert.Throws<ODataException>(
() => jsonLightResourceDeserializer.ReadDeletedEntry());

Assert.Equal(
ErrorStrings.ODataWriterCore_NestedContentNotAllowedIn40DeletedEntry,
exception.Message);
}
}

[Fact]
public async Task ReadNextLinkAnnotationAtResourceSetEndAsync_ThrowsExceptionForDuplicateODataNextLinkAnnotation()
{
Expand Down Expand Up @@ -1947,6 +2017,13 @@ private static async Task AdvanceReaderToFirstPropertyAsync(BufferingJsonReader
Assert.Equal(JsonNodeType.Property, bufferingJsonReader.NodeType);
}

private static void AdvanceReaderToFirstProperty(BufferingJsonReader bufferingJsonReader)
{
bufferingJsonReader.Read(); // Position the reader on the first node
bufferingJsonReader.Read(); // Read StartObject
Assert.Equal(JsonNodeType.Property, bufferingJsonReader.NodeType);
}

private void InitializeModel()
{
this.model = new EdmModel();
Expand Down