Skip to content

Commit

Permalink
Fix delta deleted entry deserialization failure where reason appears …
Browse files Browse the repository at this point in the history 
…before id in payload (#2787)
  • Loading branch information
@gathogojr
gathogojr authored Nov 8, 2023
1 parent bf628f7 commit a0447ab
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 55 deletions.
115 changes: 64 additions & 51 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs
View file
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
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

0 comments on commit a0447ab

Please sign in to comment.