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.

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

[Azure.Core] GeoJsonConverter serialization fix #44835

Merged
merged 7 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions sdk/core/Azure.Core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

- Fixed case where a GeoJSON string could not be deserialized when the BoundingBox JSON value ("bbox") was set explicitly to null ([#44835](https://github.com/Azure/azure-sdk-for-net/pull/44835))

### Other Changes

## 1.40.0 (2024-06-06)
Expand Down
17 changes: 16 additions & 1 deletion sdk/core/Azure.Core/src/GeoJson/GeoJsonConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@ internal static GeoObject Read(JsonElement element)

if (element.TryGetProperty(BBoxProperty, out JsonElement bboxElement))
{
// According to RFC 7946, the bbox member is optional. If one is provided, it MUST
// be an array and cannot be null.
// The code below is intentionally lenient and allows a null value to be treated
// as if the bbox member was omitted. The GeoObject.BoundingBox property is already
// set to null when there is no bbox, so setting it to null when the GeoJSON data has
// a null bbox does not impact the behavior of the GeoObject class.
// This was done to be compatible with third-party GeoJSON serializers. There are some
// GeoJSON serializer packages in the broader community that either don't follow this
// part of the spec, or interpret optional as being equal to nullable.
// Note: The Azure.Core serializer follows the spec and never writes "bbox": null
if (bboxElement.ValueKind == JsonValueKind.Null)
m-redding marked this conversation as resolved.
Show resolved Hide resolved
{
return null;
}

var arrayLength = bboxElement.GetArrayLength();

switch (arrayLength)
Expand Down Expand Up @@ -458,4 +473,4 @@ private static JsonElement GetRequiredProperty(JsonElement element, string name)
return property;
}
}
}
}
30 changes: 20 additions & 10 deletions sdk/core/Azure.Core/tests/GeoJsonSerializationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public GeoJsonSerializationTests(int points)
}

[Test]
public void CanRoundripPoint()
public void CanRoundTripPoint()
{
var input = $"{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}] }}";

Expand All @@ -30,7 +30,17 @@ public void CanRoundripPoint()
}

[Test]
public void CanRoundripBBox()
public void CanRoundTripNullBBox()
{
// cspell:ignore bbox
var input = $"{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}], \"bbox\": null }}";

var point = AssertRoundtrip<GeoPoint>(input);
Assert.AreEqual(P(0), point.Coordinates);
}

[Test]
public void CanRoundTripBBox()
{
// cspell:ignore bbox
var input = $"{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}], \"bbox\": [ {PS(1)}, {PS(2)} ] }}";
Expand Down Expand Up @@ -67,7 +77,7 @@ public void CanRoundripBBox()
}

[Test]
public void CanRoundripAdditionalProperties()
public void CanRoundTripAdditionalProperties()
{
var input = $"{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}]," +
$" \"additionalNumber\": 1," +
Expand Down Expand Up @@ -101,7 +111,7 @@ public void CanRoundripAdditionalProperties()
}

[Test]
public void CanRoundripPolygon()
public void CanRoundTripPolygon()
{
var input = $" {{ \"type\": \"Polygon\", \"coordinates\": [ [ [{PS(0)}], [{PS(1)}], [{PS(2)}], [{PS(3)}], [{PS(4)}], [{PS(0)}] ] ] }}";

Expand All @@ -120,7 +130,7 @@ public void CanRoundripPolygon()
}

[Test]
public void CanRoundripPolygonHoles()
public void CanRoundTripPolygonHoles()
{
var input = $"{{ \"type\": \"Polygon\", \"coordinates\": [" +
$" [ [{PS(0)}], [{PS(1)}], [{PS(2)}], [{PS(3)}], [{PS(4)}], [{PS(0)}] ]," +
Expand Down Expand Up @@ -152,7 +162,7 @@ public void CanRoundripPolygonHoles()
}

[Test]
public void CanRoundripMultiPoint()
public void CanRoundTripMultiPoint()
{
var input = $"{{ \"type\": \"MultiPoint\", \"coordinates\": [ [{PS(0)}], [{PS(1)}] ] }}";

Expand All @@ -164,7 +174,7 @@ public void CanRoundripMultiPoint()
}

[Test]
public void CanRoundripMultiLineString()
public void CanRoundTripMultiLineString()
{
var input = $"{{ \"type\": \"MultiLineString\", \"coordinates\": [ [ [{PS(0)}], [{PS(1)}] ], [ [{PS(2)}], [{PS(3)}] ] ] }}";

Expand All @@ -185,7 +195,7 @@ public void CanRoundripMultiLineString()
}

[Test]
public void CanRoundripMultiPolygon()
public void CanRoundTripMultiPolygon()
{
var input = $" {{ \"type\": \"MultiPolygon\", \"coordinates\": [" +
$" [ [ [{PS(0)}], [{PS(1)}], [{PS(2)}], [{PS(3)}], [{PS(4)}], [{PS(0)}] ] ]," +
Expand Down Expand Up @@ -235,7 +245,7 @@ public void CanRoundripMultiPolygon()
}

[Test]
public void CanRoundripGeometryCollection()
public void CanRoundTripGeometryCollection()
{
var input = $"{{ \"type\": \"GeometryCollection\", \"geometries\": [{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}] }}, {{ \"type\": \"LineString\", \"coordinates\": [ [{PS(1)}], [{PS(2)}] ] }}] }}";

Expand Down Expand Up @@ -295,4 +305,4 @@ private T AssertRoundtrip<T>(string json) where T: GeoObject
return geometry4;
}
}
}
}