PR comments

\n\nCommit migrated from 791c96b143

src/JSInterop/Microsoft.JSInterop/src/DotNetObjectRefJsonConverterFactory.cs

@@ -2,7 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Diagnostics;
+using System.Text.Json;
 using System.Text.Json.Serialization;
 
 namespace Microsoft.JSInterop
@@ -11,13 +11,10 @@ namespace Microsoft.JSInterop
     {
         public override bool CanConvert(Type typeToConvert)
         {
-            Debug.Assert(
-                typeToConvert.IsGenericType && typeToConvert.GetGenericTypeDefinition() == typeof(DotNetObjectRef<>),
-                "We expect this to only be called for DotNetObjectRef<T> instances.");
-            return true;
+            return typeToConvert.IsGenericType && typeToConvert.GetGenericTypeDefinition() == typeof(DotNetObjectRef<>);
         }
 
-        protected override JsonConverter CreateConverter(Type typeToConvert)
+        public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions jsonSerializerOptions)
         {
             // System.Text.Json handles caching the converters per type on our behalf. No caching is required here.
             var instanceType = typeToConvert.GetGenericArguments()[0];

src/JSInterop/Microsoft.JSInterop/src/DotNetObjectReferenceJsonConverter.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.IO;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 
@@ -14,27 +13,31 @@ namespace Microsoft.JSInterop
 
         public override DotNetObjectRef<TValue> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
-            if (!reader.Read())
+            long dotNetObjectId = 0;
+
+            while (reader.Read() && reader.TokenType != JsonTokenType.EndObject)
             {
-                throw new InvalidDataException("Invalid DotNetObjectRef JSON.");
+                if (reader.TokenType == JsonTokenType.PropertyName)
+                {
+                    if (reader.ValueTextEquals(DotNetObjectRefKey.EncodedUtf8Bytes))
+                    {
+                        reader.Read();
+                        dotNetObjectId = reader.GetInt64();
+                    }
+                    else
+                    {
+                        throw new JsonException($"Unexcepted JSON property {reader.GetString()}.");
+                    }
+                }
+                else
+                {
+                    throw new JsonException($"Unexcepted JSON Token {reader.TokenType}.");
+                }
             }
 
-            if (reader.TokenType != JsonTokenType.PropertyName || !reader.ValueTextEquals(DotNetObjectRefKey.EncodedUtf8Bytes))
+            if (dotNetObjectId is 0)
             {
-                throw new InvalidDataException("Invalid DotNetObjectRef JSON.");
-            }
-
-            if (!reader.Read())
-            {
-                throw new InvalidDataException("Invalid DotNetObjectRef JSON.");
-            }
-
-            var dotNetObjectId = reader.GetInt64();
-
-            if (!reader.Read())
-            {
-                // We need to read all the data that was given to us.
-                throw new InvalidDataException("Invalid DotNetObjectRef JSON.");
+                throw new JsonException($"Required property {DotNetObjectRefKey} not found.");
             }
 
             var value = (TValue)DotNetObjectRefManager.Current.FindDotNetObject(dotNetObjectId);

src/JSInterop/Microsoft.JSInterop/test/DotNetObjectRefTest.cs

@@ -30,23 +30,5 @@ namespace Microsoft.JSInterop
             var ex = Assert.Throws<ArgumentException>(() => jsRuntime.ObjectRefManager.FindDotNetObject(objRef.ObjectId));
             Assert.StartsWith("There is no tracked object with id '1'.", ex.Message);
         });
-            }
-
-            protected internal override void EndInvokeDotNet(string callId, bool success, object resultOrError, string assemblyName, string methodIdentifier, long dotNetObjectId)
-            {
-                throw new NotImplementedException();
-            }
-        }
-
-        async Task WithJSRuntime(Action<JSRuntimeBase> testCode)
-        {
-            // Since the tests rely on the asynclocal JSRuntime.Current, ensure we
-            // are on a distinct async context with a non-null JSRuntime.Current
-            await Task.Yield();
-
-            var runtime = new TestJSRuntime();
-            JSRuntime.SetCurrentJSRuntime(runtime);
-            testCode(runtime);
-        }
     }
 }

src/JSInterop/Microsoft.JSInterop/test/DotNetObjectReferenceJsonConverterTest.cs

@@ -10,12 +10,58 @@ namespace Microsoft.JSInterop.Tests
 {
     public class DotNetObjectReferenceJsonConverterTest
     {
+        [Fact]
+        public Task Read_Throws_IfJsonIsMissingDotNetObjectProperty() => WithJSRuntime(_ =>
+        {
+            // Arrange
+            var dotNetObjectRef = DotNetObjectRef.Create(new TestModel());
+
+            var json = "{}";
+
+            // Act & Assert
+            var ex = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DotNetObjectRef<TestModel>>(json));
+            Assert.Equal("Required property __dotNetObject not found.", ex.Message);
+        });
+
+        [Fact]
+        public Task Read_Throws_IfJsonContainsUnknownContent() => WithJSRuntime(_ =>
+        {
+            // Arrange
+            var dotNetObjectRef = DotNetObjectRef.Create(new TestModel());
+
+            var json = "{\"foo\":2}";
+
+            // Act & Assert
+            var ex = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DotNetObjectRef<TestModel>>(json));
+            Assert.Equal("Unexcepted JSON property foo.", ex.Message);
+        });
+
         [Fact]
         public Task Read_ReadsJson() => WithJSRuntime(_ =>
         {
             // Arrange
-            // Throw-away value
+            var input = new TestModel();
+            var dotNetObjectRef = DotNetObjectRef.Create(input);
+            var objectId = dotNetObjectRef.ObjectId;
+
+            var json = $"{{\"__dotNetObject\":{objectId}}}";
+
+            // Act
+            var deserialized = JsonSerializer.Deserialize<DotNetObjectRef<TestModel>>(json);
+
+            // Assert
+            Assert.Same(input, deserialized.Value);
+            Assert.Equal(objectId, deserialized.ObjectId);
+        });
+
+        [Fact]
+        public Task Read_ReturnsTheCorrectInstance() => WithJSRuntime(_ =>
+        {
+            // Arrange
+            // Track a few instances and verify that the deserialized value returns the corect value.
             DotNetObjectRef.Create(new TestModel());
+            DotNetObjectRef.Create(new TestModel());
+
             var input = new TestModel();
             var dotNetObjectRef = DotNetObjectRef.Create(input);
             var objectId = dotNetObjectRef.ObjectId;
@@ -34,8 +80,6 @@ namespace Microsoft.JSInterop.Tests
         public Task Read_ReadsJson_WithFormatting() => WithJSRuntime(_ =>
         {
             // Arrange
-            // Throw-away value
-            DotNetObjectRef.Create(new TestModel());
             var input = new TestModel();
             var dotNetObjectRef = DotNetObjectRef.Create(input);
             var objectId = dotNetObjectRef.ObjectId;
@@ -57,8 +101,6 @@ namespace Microsoft.JSInterop.Tests
         public Task WriteJsonTwice_KeepsObjectId() => WithJSRuntime(_ =>
         {
             // Arrange
-            // Throw-away value
-            DotNetObjectRef.Create(new TestModel());
             var dotNetObjectRef = DotNetObjectRef.Create(new TestModel());
 
             // Act

src/JSInterop/Microsoft.JSInterop/test/TestJSRuntime.cs

@@ -13,6 +13,11 @@ namespace Microsoft.JSInterop
             throw new NotImplementedException();
         }
 
+        protected internal override void EndInvokeDotNet(string callId, bool success, object resultOrError, string assemblyName, string methodIdentifier, long dotNetObjectId)
+        {
+            throw new NotImplementedException();
+        }
+
         public static async Task WithJSRuntime(Action<JSRuntimeBase> testCode)
         {
             // Since the tests rely on the asynclocal JSRuntime.Current, ensure we