Merge branch 'develop'

NetTopologySuite · Jun 17, 2020 · 23afedb · 23afedb
2 parents c338800 + abf26ad
commit 23afedb
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 14 deletions.
diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureConverter.cs b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureConverter.cs
@@ -41,7 +41,8 @@ public override void Write(Utf8JsonWriter writer, IFeature value, JsonSerializer
             // Add the id here if present.
             if (value.GetOptionalId(_idPropertyName) is object id)
             {
-                writer.WriteString(_idPropertyName, JsonSerializer.Serialize(id, id.GetType(), options));
+                writer.WritePropertyName("id");
+                JsonSerializer.Serialize(writer, id, id.GetType(), options);
             }
 
             // bbox (optional)
@@ -106,24 +107,19 @@ public override IFeature Read(ref Utf8JsonReader reader, Type objectType, JsonSe
                     case "id":
                         switch (reader.TokenType)
                         {
-                            case JsonTokenType.Number when reader.TryGetInt32(out int i4):
-                                feature.Id = i4;
+                            case JsonTokenType.Number when reader.TryGetDecimal(out decimal decimalValue):
+                                feature.Id = decimalValue;
                                 break;
 
-                            case JsonTokenType.Number when reader.TryGetInt64(out long i8):
-                                feature.Id = i8;
-                                break;
-
-                            case JsonTokenType.String when reader.TryGetGuid(out var guid):
-                                feature.Id = guid;
-                                break;
+                            case JsonTokenType.Number:
+                                throw new NotSupportedException("Number value cannot be boxed as a decimal: " + reader.GetString());
 
                             case JsonTokenType.String:
                                 feature.Id = reader.GetString();
                                 break;
 
                             default:
-                                throw new JsonException();
+                                throw new JsonException("A GeoJSON Feature's \"id\", if specified, must be either a JSON string or number, per RFC7946 section 3.2");
                         }
 
                         reader.Read();

diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/NetTopologySuite.IO.GeoJSON4STJ.csproj b/src/NetTopologySuite.IO.GeoJSON4STJ/NetTopologySuite.IO.GeoJSON4STJ.csproj
@@ -6,7 +6,7 @@
     <!-- MAJOR, MINOR, and PATCH are defined according to SemVer 2.0.0. -->
     <NtsMajorVersion Condition=" '$(NtsMajorVersion)' == '' ">2</NtsMajorVersion>
     <NtsMinorVersion Condition=" '$(NtsMinorVersion)' == '' ">0</NtsMinorVersion>
-    <NtsPatchVersion Condition=" '$(NtsPatchVersion)' == '' ">2</NtsPatchVersion>
+    <NtsPatchVersion Condition=" '$(NtsPatchVersion)' == '' ">3</NtsPatchVersion>
   </PropertyGroup>
 
   <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />

diff --git a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/AttributesTableConverterTest.cs b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/AttributesTableConverterTest.cs
@@ -224,8 +224,17 @@ public static void TestEquality(IAttributesTable s, IAttributesTable d, string i
                     Assert.That(dIt.MoveNext(), Is.False);
 
                 }
+                else if (sitem is Guid sGuidItem)
+                {
+                    // we box ALL string values as strings, even those that can
+                    // be converted to Guid, to simplify the number of cases
+                    // callers need to be able to deal with.
+                    Assert.That(ditem, Is.EqualTo(sGuidItem.ToString()));
+                }
                 else
+                {
                     Assert.That(ditem, Is.EqualTo(sitem));
+                }
             }
         }
 

diff --git a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureConverterTest.cs b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureConverterTest.cs
@@ -1,7 +1,12 @@
-using System.Buffers.Text;
+using System;
+using System.Collections.Generic;
+using System.Text.Json;
 using System.Text.Json.Serialization;
+
 using NetTopologySuite.Features;
 using NetTopologySuite.Geometries;
+using NetTopologySuite.IO.Converters;
+
 using NUnit.Framework;
 
 namespace NetTopologySuite.IO.GeoJSON4STJ.Test.Converters
@@ -67,17 +72,104 @@ public void WriteJsonWithArrayTest()
             //Assert.AreEqual("{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[23.1,56.2]},\"properties\":{\"test1\":[\"value1\",\"value2\"]}}", ToJson(value));
         }
 
-        public static void CheckEquality(IFeature s, IFeature d, string idPropertyName = "id")
+        [TestCaseSource(nameof(FeatureIdTestCases))]
+        public void WriteJsonWithIdTest(string idPropertyName, object id)
+        {
+            var value = new Feature
+            {
+                Geometry = GeometryFactory.Default.CreatePoint(new Coordinate(23.1, 56.2)),
+                Attributes = new AttributesTable
+                {
+                    { idPropertyName, id },
+                    { TestContext.CurrentContext.Random.GetString(), TestContext.CurrentContext.Random.NextGuid() },
+                },
+            };
+
+            var options = new JsonSerializerOptions
+            {
+                Converters =
+                {
+                    new GeoJsonConverterFactory(GeometryFactory.Default, false, idPropertyName),
+                },
+            };
+
+            string json = ToJsonString(value, options);
+            var deserialized = Deserialize(json, options);
+            CheckEquality(value, deserialized, idPropertyName);
+        }
+
+        public static IEnumerable<object[]> FeatureIdTestCases
+        {
+            get
+            {
+                var ctx = TestContext.CurrentContext;
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextByte() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextSByte() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextShort() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextUShort() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.Next() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextUInt() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextLong() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextULong() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextFloat() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextDouble() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextDecimal() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.GetString() };
+                yield return new object[] { ctx.Random.GetString(), ctx.Random.NextGuid() };
+                yield return new object[] { ctx.Random.GetString(), new DateTime(ctx.Random.NextLong(DateTime.MinValue.Ticks, DateTime.MaxValue.Ticks + 1)) };
+                yield return new object[] { ctx.Random.GetString(), new DateTimeOffset(ctx.Random.NextLong(DateTime.MinValue.Ticks, DateTime.MaxValue.Ticks + 1), TimeSpan.FromHours(ctx.Random.Next(-14, 15))) };
+            }
+        }
+
+        public static void CheckEquality(IFeature s, IFeature d, string idPropertyName = null)
         {
+            idPropertyName ??= GeoJsonConverterFactory.DefaultIdPropertyName;
+
             Assert.That(d, Is.Not.Null);
 
             Assert.That(s.Geometry.EqualsExact(d.Geometry));
 
             AttributesTableConverterTest.TestEquality(s.Attributes, d.Attributes, idPropertyName);
 
             if (s.BoundingBox != null)
+            {
                 Assert.That(d.BoundingBox, Is.EqualTo(s.BoundingBox));
+            }
+
+            if (s.GetOptionalId(idPropertyName) is object sId)
+            {
+                if (d.GetOptionalId(idPropertyName) is object dId)
+                {
+                    switch (dId)
+                    {
+                        // ALL number values get boxed as decimals.
+                        case decimal _:
+                            sId = JsonSerializer.Deserialize<decimal>(JsonSerializer.Serialize(sId));
+                            break;
+
+                        // ALL string values get boxed as strings.
+                        case string _:
+                            sId = JsonSerializer.Deserialize<string>(JsonSerializer.Serialize(sId));
+                            break;
+
+                        // RFC7946, 3.2 says "the value of this member is either
+                        // a JSON string or number.
+                        default:
+                            Assert.Fail("Feature IDs must be either a string or number.");
+                            break;
+                    }
 
+                    Assert.That(dId, Is.EqualTo(sId));
+                }
+                else
+                {
+                    Assert.Fail("s had ID, but d did not.");
+                }
+            }
+            else
+            {
+                Assert.That(d.GetOptionalId(idPropertyName) is null);
+            }
         }
 
     }