Merge pull request #305 from microsoft/users/shlomiyosef/converter-use-porp-name-and-ignore

Update ConnectorConverter to honer JsonIgnore and JsonPropertyName

Author: msftshlomiyosef

src/libraries/Core/Microsoft.Agents.Core/Models/AIEntity.cs

@@ -16,19 +16,19 @@ public AIEntity() : base("https://schema.org/Message") {}
         /// <summary>
         /// Required. Must be "Message".
         /// </summary>
-        //[JsonPropertyName("@type")]
+        [JsonPropertyName("@type")]
         public string AtType { get; set; } = "Message";
 
         /// <summary>
         /// Required. Must be "https://schema.org"
         /// </summary>
-        //[JsonPropertyName("@context")]
+        [JsonPropertyName("@context")]
         public string AtContext { get; set; } = "https://schema.org";
 
         /// <summary>
         /// Must be left blank.
         /// </summary>
-        //[JsonPropertyName("@id")]
+        [JsonPropertyName("@id")]
         public string AtId { get; set; } = "";
 
         /// <summary>

src/libraries/Core/Microsoft.Agents.Core/Serialization/Converters/AIEntityConverter.cs

@@ -10,22 +10,7 @@ internal class AIEntityConverter : ConnectorConverter<AIEntity>
     {
         protected override void ReadExtensionData(ref Utf8JsonReader reader, AIEntity value, string propertyName, JsonSerializerOptions options)
         {
-            if (propertyName.Equals("@type"))
-            {
-                value.AtType = JsonSerializer.Deserialize<string>(ref reader, options);
-            }
-            else if (propertyName.Equals("@context"))
-            {
-                value.AtContext = JsonSerializer.Deserialize<string>(ref reader, options);
-            }
-            else if (propertyName.Equals("@id"))
-            {
-                value.AtId = JsonSerializer.Deserialize<string>(ref reader, options);
-            }
-            else
-            {
-                value.Properties.Add(propertyName, JsonSerializer.Deserialize<JsonElement>(ref reader, options));
-            }
+            value.Properties.Add(propertyName, JsonSerializer.Deserialize<JsonElement>(ref reader, options));
         }
 
         protected override bool TryReadExtensionData(ref Utf8JsonReader reader, AIEntity value, string propertyName, JsonSerializerOptions options)
@@ -57,24 +42,6 @@ protected override bool TryWriteExtensionData(Utf8JsonWriter writer, AIEntity va
 
                 return true;
             }
-            else if (propertyName.Equals(nameof(value.AtType)))
-            {
-                writer.WritePropertyName("@type");
-                writer.WriteStringValue(value.AtType);
-                return true;
-            }
-            else if (propertyName.Equals(nameof(value.AtContext)))
-            {
-                writer.WritePropertyName("@context");
-                writer.WriteStringValue(value.AtContext);
-                return true;
-            }
-            else if (propertyName.Equals(nameof(value.AtId)))
-            {
-                writer.WritePropertyName("@id");
-                writer.WriteStringValue(value.AtId);
-                return true;
-            }
 
             return false;
         }

src/libraries/Core/Microsoft.Agents.Core/Serialization/Converters/ConnectorConverter.cs

@@ -7,11 +7,16 @@
 using System;
 using System.Reflection;
 using System.Collections;
+using System.Collections.Concurrent;
+using System.Linq;
 
 namespace Microsoft.Agents.Core.Serialization.Converters
 {
     public abstract class ConnectorConverter<T> : JsonConverter<T> where T : new()
     {
+        private static readonly ConcurrentDictionary<Type, PropertyInfo[]> PropertyCache = new();
+        private static readonly ConcurrentDictionary<(Type, bool, Type), Dictionary<string, (PropertyInfo, bool)>> JsonPropertyMetadataCache = new();
+        
         public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             if (reader.TokenType != JsonTokenType.StartObject)
@@ -21,14 +26,7 @@ public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerial
 
             var value = new T();
 
-            var properties = options.PropertyNameCaseInsensitive
-                ? new Dictionary<string, PropertyInfo>(StringComparer.OrdinalIgnoreCase)
-                : new Dictionary<string, PropertyInfo>();
-
-            foreach (var property in typeof(T).GetProperties())
-            {
-                properties.Add(property.Name, property);
-            }
+            var propertyMetadataMap = GetJsonPropertyMetadata(typeof(T), options.PropertyNameCaseInsensitive, options.PropertyNamingPolicy);
 
             while (reader.Read())
             {
@@ -41,9 +39,9 @@ public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerial
                 {
                     var propertyName = reader.GetString();
 
-                    if (properties.ContainsKey(propertyName))
+                    if (propertyMetadataMap.TryGetValue(propertyName, out var entry))
                     {
-                        ReadProperty(ref reader, value, propertyName, options, properties);
+                        ReadProperty(ref reader, value, propertyName, options, entry.Property);
                     }
                     else
                     {
@@ -59,8 +57,18 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions
         {
             writer.WriteStartObject();
 
-            foreach (var property in value.GetType().GetProperties())
+            var type = value.GetType();
+            var properties = GetCachedProperties(type);
+            var propertyMetadataMap = GetJsonPropertyMetadata(type, false, options.PropertyNamingPolicy); // case-insensitivity doesn’t matter here
+            var reverseMap = propertyMetadataMap.ToDictionary(kv => kv.Value.Property, kv => (kv.Key, kv.Value.IsIgnored));
+
+            foreach (var property in properties)
             {
+                if (!reverseMap.TryGetValue(property, out var propertyMetadata) || propertyMetadata.IsIgnored)
+                {
+                    continue;
+                }
+
                 if (!TryWriteExtensionData(writer, value, property.Name))
                 {
                     var propertyValue = property.GetValue(value);
@@ -77,9 +85,7 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions
                     if (propertyValue != null || !(options.DefaultIgnoreCondition == JsonIgnoreCondition.WhenWritingNull))
 
                     {
-                        var propertyName = options.PropertyNamingPolicy == JsonNamingPolicy.CamelCase
-                            ? JsonNamingPolicy.CamelCase.ConvertName(property.Name)
-                            : property.Name;
+                        var propertyName = propertyMetadata.Key ?? property.Name;
 
                         writer.WritePropertyName(propertyName);
 
@@ -254,10 +260,8 @@ protected void SetGenericProperty(ref Utf8JsonReader reader, Action<object> sett
             setter(deserialized);
         }
 
-        private void ReadProperty(ref Utf8JsonReader reader, T value, string propertyName, JsonSerializerOptions options, Dictionary<string, PropertyInfo> properties)
+        private void ReadProperty(ref Utf8JsonReader reader, T value, string propertyName, JsonSerializerOptions options, PropertyInfo property)
         {
-            var property = properties[propertyName];
-
             if (TryReadExtensionData(ref reader, value, property.Name, options))
             {
                 return;
@@ -292,5 +296,39 @@ private void ReadProperty(ref Utf8JsonReader reader, T value, string propertyNam
             var propertyValue = System.Text.Json.JsonSerializer.Deserialize(ref reader, property.PropertyType, options);
             property.SetValue(value, propertyValue);
         }
+
+        private static PropertyInfo[] GetCachedProperties(Type type)
+        {
+            return PropertyCache.GetOrAdd(type, static t => t.GetProperties());
+        }
+
+        private static Dictionary<string, (PropertyInfo Property, bool IsIgnored)> GetJsonPropertyMetadata(Type type, bool caseInsensitive, JsonNamingPolicy? namingPolicy)
+        {
+            var cacheKey = (type, caseInsensitive, namingPolicy?.GetType());
+            return JsonPropertyMetadataCache.GetOrAdd(cacheKey, key =>
+            {
+                var (t, insensitive, _) = key;
+                var comparer = insensitive ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal;
+                var metadata  = new Dictionary<string, (PropertyInfo, bool)>(comparer);
+
+                foreach (var prop in GetCachedProperties(t))
+                {
+                    var resolvedName = prop.GetCustomAttribute<JsonPropertyNameAttribute>()?.Name
+                        ?? namingPolicy?.ConvertName(prop.Name)
+                        ?? prop.Name;
+
+                    if (metadata.ContainsKey(resolvedName))
+                    {
+                        throw new InvalidOperationException(
+                            $"Duplicate JSON property name detected: '{resolvedName}' maps to multiple properties in type '{t.FullName}'."
+                        );
+                    }
+
+                    metadata [resolvedName] = (prop, prop.GetCustomAttribute<JsonIgnoreAttribute>()?.Condition == JsonIgnoreCondition.Always);
+                }
+
+                return metadata;
+            });
+        }
     }
 }

src/tests/Microsoft.Agents.Model.Tests/ObjectSerializationTests.cs

@@ -1,13 +1,14 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-using Microsoft.Agents.Core;
 using Microsoft.Agents.Core.Models;
 using Microsoft.Agents.Core.Serialization;
 using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Text.Json;
+using System.Text.Json.Serialization;
+using System.Threading.Tasks;
 using Xunit;
 
 namespace Microsoft.Agents.Model.Tests
@@ -362,6 +363,73 @@ public void ValidateActivitySerializer(string baseFileName)
             Assert.Equal(resultingText, outboundJson);
         }
 
+        [Fact]
+        public void ActivityWithDerivedEntitySerializationTest()
+        {
+            var jsonIn = "{\"membersAdded\":[],\"membersRemoved\":[],\"reactionsAdded\":[],\"reactionsRemoved\":[],\"attachments\":[],\"entities\":[{\"@type\":\"Message\",\"@context\":\"https://schema.org\",\"@id\":\"\",\"additionalType\":[\"AIGeneratedContent\"],\"citation\":[],\"type\":\"https://schema.org/Message\"}],\"listenFor\":[],\"textHighlights\":[]}";
+
+            var activity = ProtocolJsonSerializer.ToObject<Activity>(jsonIn);
+            var jsonOut = ProtocolJsonSerializer.ToJson(activity);
+
+            Assert.Equal(jsonIn, jsonOut);
+        }
+
+
+
+        [Fact]
+        public void WithDerivedActivitySerializationTest()
+        {
+            List<Activity> activities = [new DerivedActivity
+            {
+                Secret = "secret",
+                Public = "public"
+            }];
+            var jsonOut = ProtocolJsonSerializer.ToJson(activities);
+            var expected = "[{\"@public\":\"public\",\"membersAdded\":[],\"membersRemoved\":[],\"reactionsAdded\":[],\"reactionsRemoved\":[],\"attachments\":[],\"entities\":[],\"listenFor\":[],\"textHighlights\":[]}]";
+            
+            Assert.Equal(expected, jsonOut);
+        }
+
+        [Fact]
+        public void SerializeDeserializeIsThreadSafeUnderConcurrency()
+        {
+            var text = File.ReadAllText(Path.Combine(Directory.GetCurrentDirectory(), "Resources", "ComplexActivityPayload.json"));
+
+            var activity = ProtocolJsonSerializer.ToObject<Activity>(text);
+
+            var json = ProtocolJsonSerializer.ToJson(activity);
+
+            const int threadCount = 50;
+            var results = new Activity[threadCount];
+
+            Parallel.For(0, threadCount, i =>
+            {
+                results[i] = RoundTrip(activity);
+            });
+
+            foreach (var result in results)
+            {
+                Assert.NotNull(result);
+                AssertPropertyValues(result);
+            }
+        }
+
+        [Fact]
+        public void DuplicateNameActivitySerializationShouldThrow()
+        {
+            // This should throw an exception because the property name 'id' is duplicated.
+            var activityJson = new DuplicateNameActivity
+            {
+                Id = "12345",
+                MyId = "67890"
+            };
+            // Expect an InvalidOperationException from System.Text.Json (Note: The ConnectorConverter is not used here because the compile-time type is 'object').
+            Assert.Throws<InvalidOperationException>(() => ProtocolJsonSerializer.ToJson(activityJson));
+            // Expect an InvalidOperationException from ConnectorConverter.
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize<Activity>(activityJson, ProtocolJsonSerializer.SerializationOptions));
+        }
+
+
 #if SKIP_EMPTY_LISTS
         [Fact]
         public void EmptyListDoesntSerialzie()
@@ -450,5 +518,21 @@ private class TestObjectClass
 
             public TestObjectClass TestObject { get; set; }
         }
+
+        private class DerivedActivity : Activity
+        {
+            [JsonIgnore]
+            public string Secret { get; set; }
+
+            [JsonPropertyName("@public")]
+            public string Public { get; set; }
+        }
+
+        private class DuplicateNameActivity : Activity
+        {
+
+            [JsonPropertyName("id")]
+            public string MyId { get; set; }
+        }
     }
-}
+}