[Azure.Core] GeoJsonConverter serialization fix (#44835)

* GeoJsonConverter serialization fix * formatting * formatting * Update GeoJsonConverter.cs * Update CHANGELOG.md * add comment
Azure · Jul 9, 2024 · 4a0ec10 · 4a0ec10
1 parent 5050c6a
commit 4a0ec10
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 11 deletions.
diff --git a/sdk/core/Azure.Core/CHANGELOG.md b/sdk/core/Azure.Core/CHANGELOG.md
@@ -9,6 +9,8 @@
 ### Bugs Fixed
 - Fixed an issue that could result in `BearerTokenAuthenticationPolicy` fails to refresh a token, resulting in a `OperationCanceledException`.
 
+- 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)

diff --git a/sdk/core/Azure.Core/src/GeoJson/GeoJsonConverter.cs b/sdk/core/Azure.Core/src/GeoJson/GeoJsonConverter.cs
@@ -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)
+                {
+                    return null;
+                }
+
                 var arrayLength = bboxElement.GetArrayLength();
 
                 switch (arrayLength)
@@ -458,4 +473,4 @@ private static JsonElement GetRequiredProperty(JsonElement element, string name)
             return property;
         }
     }
-}
+}
diff --git a/sdk/core/Azure.Core/tests/GeoJsonSerializationTests.cs b/sdk/core/Azure.Core/tests/GeoJsonSerializationTests.cs
@@ -21,7 +21,7 @@ public GeoJsonSerializationTests(int points)
         }
 
         [Test]
-        public void CanRoundripPoint()
+        public void CanRoundTripPoint()
         {
             var input = $"{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}] }}";
 
@@ -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)} ] }}";
@@ -67,7 +77,7 @@ public void CanRoundripBBox()
         }
 
         [Test]
-        public void CanRoundripAdditionalProperties()
+        public void CanRoundTripAdditionalProperties()
         {
             var input = $"{{ \"type\": \"Point\", \"coordinates\": [{PS(0)}]," +
                         $" \"additionalNumber\": 1," +
@@ -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)}] ] ] }}";
 
@@ -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)}] ]," +
@@ -152,7 +162,7 @@ public void CanRoundripPolygonHoles()
         }
 
         [Test]
-        public void CanRoundripMultiPoint()
+        public void CanRoundTripMultiPoint()
         {
             var input = $"{{ \"type\": \"MultiPoint\", \"coordinates\": [ [{PS(0)}], [{PS(1)}] ] }}";
 
@@ -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)}] ] ] }}";
 
@@ -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)}] ] ]," +
@@ -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)}] ] }}] }}";
 
@@ -295,4 +305,4 @@ private T AssertRoundtrip<T>(string json) where T: GeoObject
             return geometry4;
         }
     }
-}
+}