Extend KeyConverter support to DictionaryOfString

src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs

@@ -583,21 +583,12 @@ public bool TryGetInt32(out int value)
                 throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
             }
 
-            ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            if (Utf8Parser.TryParse(span, out int tmp, out int bytesConsumed)
-                && span.Length == bytesConsumed)
-            {
-                value = tmp;
-                return true;
-            }
-
-            value = 0;
-            return false;
+            return TryGetInt32AfterValidation(out value);
         }
 
         // Gets value without validation.
         // TODO: call this on KeyConverter.
-        internal bool TryGetInt32Unsafe(out int value)
+        internal bool TryGetInt32AfterValidation(out int value)
         {
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
             if (Utf8Parser.TryParse(span, out int tmp, out int bytesConsumed)
@@ -961,6 +952,11 @@ public bool TryGetGuid(out Guid value)
                 throw ThrowHelper.GetInvalidOperationException_ExpectedString(TokenType);
             }
 
+            return TryGetGuidAfterValidation(out value);
+        }
+
+        internal bool TryGetGuidAfterValidation(out Guid value)
+        {
             ReadOnlySpan<byte> span = stackalloc byte[0];
 
             if (HasValueSequence)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs

@@ -89,6 +89,12 @@ internal sealed override bool OnTryRead(
 
                 JsonConverter<TValue> elementConverter = GetElementConverter(ref state);
                 KeyConverter<TKey> keyConverter = (KeyConverter<TKey>)state.Current.JsonClassInfo.KeyConverter;
+
+                if (keyConverter == null)
+                {
+                    throw new JsonException("Dictionary key not supported");
+                }
+
                 if (elementConverter.CanUseDirectReadOrWrite)
                 {
                     // Process all elements.
@@ -107,8 +113,9 @@ internal sealed override bool OnTryRead(
                             ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypeToConvert);
                         }
 
+                        // TODO: maybe we can avoid this call when TKey is string since we can re-use the KeyConverter key.
                         state.Current.JsonPropertyNameAsString = reader.GetString()!;
-                        TKey key = GetKeyAsTKey(keyConverter, reader.GetSpan());
+                        keyConverter.ReadKey(ref reader, out TKey key);
 
                         // Read the value and add.
                         reader.ReadWithVerify();
@@ -135,7 +142,7 @@ internal sealed override bool OnTryRead(
                         }
 
                         state.Current.JsonPropertyNameAsString = reader.GetString()!;
-                        TKey key = GetKeyAsTKey(keyConverter, reader.GetSpan());
+                        keyConverter.ReadKey(ref reader, out TKey key);
 
                         reader.ReadWithVerify();
 
@@ -313,23 +320,5 @@ internal sealed override bool OnTryWrite(
 
             return success;
         }
-
-        private TKey GetKeyAsTKey(KeyConverter<TKey> keyConverter, ReadOnlySpan<byte> keyNameSpan)
-        {
-            TKey key;
-            // For DictionaryOfString*Converter, we don't use a keyCovnerter.
-            if (keyConverter == null)
-            {
-                Debug.Assert(typeof(TKey) == typeof(string));
-                // We return default since we actually use state.Current.JsonPropertyNameAsString on the Add method.
-                key = default!;
-            }
-            else
-            {
-                key = keyConverter.ReadKey(keyNameSpan);
-            }
-
-            return key;
-        }
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryOfStringTValueConverter.cs

@@ -55,13 +55,16 @@ protected internal override bool OnWriteResume(
             }
 
             JsonConverter<TValue> converter = GetValueConverter(ref state);
+            KeyConverter<string> keyConverter = (KeyConverter<string>)state.Current.JsonClassInfo.KeyConverter;
+
             if (!state.SupportContinuation && converter.CanUseDirectReadOrWrite)
             {
                 // Fast path that avoids validation and extra indirection.
                 do
                 {
-                    string key = GetKeyName(enumerator.Current.Key, ref state, options);
-                    writer.WritePropertyName(key);
+                    //string key = GetKeyName(enumerator.Current.Key, ref state, options);
+                    //writer.WritePropertyName(key);
+                    keyConverter.WriteKey(writer, enumerator.Current.Key, options, state.Current.IgnoreDictionaryKeyPolicy);
                     converter.Write(writer, enumerator.Current.Value, options);
                 } while (enumerator.MoveNext());
             }
@@ -79,8 +82,9 @@ protected internal override bool OnWriteResume(
                     if (state.Current.PropertyState < StackFramePropertyState.Name)
                     {
                         state.Current.PropertyState = StackFramePropertyState.Name;
-                        string key = GetKeyName(enumerator.Current.Key, ref state, options);
-                        writer.WritePropertyName(key);
+                        //string key = GetKeyName(enumerator.Current.Key, ref state, options);
+                        //writer.WritePropertyName(key);
+                        keyConverter.WriteKey(writer, enumerator.Current.Key, options, state.Current.IgnoreDictionaryKeyPolicy);
                     }
 
                     if (!converter.TryWrite(writer, element, options, ref state))

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryOfTKeyTValue.cs

@@ -57,7 +57,7 @@ protected internal override bool OnWriteResume(
                 // Fast path that avoids validation and extra indirection.
                 do
                 {
-                    WriteKeyName(keyConverter, enumerator.Current.Key, ref state, writer, options);
+                    keyConverter.WriteKey(writer, enumerator.Current.Key, options, state.Current.IgnoreDictionaryKeyPolicy);
                     valueConverter.Write(writer, enumerator.Current.Value, options);
                 } while (enumerator.MoveNext());
             }
@@ -76,7 +76,7 @@ protected internal override bool OnWriteResume(
                     if (state.Current.PropertyState < StackFramePropertyState.Name)
                     {
                         state.Current.PropertyState = StackFramePropertyState.Name;
-                        WriteKeyName(keyConverter, enumerator.Current.Key, ref state, writer, options);
+                        keyConverter.WriteKey(writer, enumerator.Current.Key, options, state.Current.IgnoreDictionaryKeyPolicy);
                     }
 
                     if (!valueConverter.TryWrite(writer, element, options, ref state))
@@ -91,28 +91,5 @@ protected internal override bool OnWriteResume(
 
             return true;
         }
-
-        private void WriteKeyName(KeyConverter<TKey> keyConverter, TKey key, ref WriteStack state, Utf8JsonWriter writer, JsonSerializerOptions options)
-        {
-            // DictionaryKeyPolicy.ConverterName can only take a string key name,
-            // So we avoid allocating the string when there is no DictionaryKeyPolicy.
-            if (options.DictionaryKeyPolicy == null)
-            {
-
-                int length = keyConverter.DetermineKeyLength(key);
-                Span<byte> keyNameSpan = stackalloc byte[length];
-
-                keyConverter.WriteKeySpan(keyNameSpan, key);
-                writer.WritePropertyName(keyNameSpan);
-            }
-            else
-            {
-                string keyNameString = keyConverter.WriteKey(key);
-                // Apply KeyPolicy.
-                // TODO: DictionaryKeyPolicy != null check is repeated on GetKeyName.
-                keyNameString = GetKeyName(keyNameString, ref state, options);
-                writer.WritePropertyName(keyNameString);
-            }
-        }
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableConverterFactory.cs

@@ -78,15 +78,16 @@ public override bool CanConvert(Type typeToConvert)
                 if (genericArgs[0] == typeof(string))
                 {
                     converterType = typeof(DictionaryOfStringTValueConverter<,>);
+                    dictionaryKeyType = genericArgs[0];
                     elementType = genericArgs[1];
                 }
                 else
                 {
                     //return null;
                     // Check converter for TKey instead.
                     converterType = typeof(DictionaryOfTKeyTValueConverter<,,>);
-                    dictionaryKeyType = actualTypeToConvert.GetGenericArguments()[0];//options.GetConverter(actualTypeToConvert.GetGenericArguments()[0]);
-                    elementType = actualTypeToConvert.GetGenericArguments()[1];
+                    dictionaryKeyType = genericArgs[0];//options.GetConverter(actualTypeToConvert.GetGenericArguments()[0]);
+                    elementType = genericArgs[1];
                 }
             }
             // Immutable dictionaries from System.Collections.Immutable, e.g. ImmutableDictionary<string, TValue>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/KeyConverter.cs

@@ -2,6 +2,7 @@
 using System.Buffers.Text;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Text;
 using System.Text.Unicode;
 
@@ -56,66 +57,65 @@ private static IEnumerable<KeyConverter> KeyConverters
 
     internal abstract class KeyConverter<T> : KeyConverter
     {
-        public abstract T ReadKey(ReadOnlySpan<byte> key);
-        public abstract string WriteKey(T key);
-        public abstract void WriteKeySpan(Span<byte> buffer, T Key);
-        public abstract int DetermineKeyLength(T key);
-
-        public override Type Type => typeof(T);
-    }
-
-    internal sealed class Int32KeyConverter : KeyConverter<int>
-    {
-        public override int ReadKey(ReadOnlySpan<byte> key)
+        public abstract bool ReadKey(ref Utf8JsonReader reader, out T value);
+        public void WriteKey(Utf8JsonWriter writer, [DisallowNull] T value, JsonSerializerOptions options, bool ignoreKeyPolicy)
         {
-            Utf8Parser.TryParse(key, out int parsedKey, out int _);
-            return parsedKey;
+            WriteKeyAsTOrAsString(writer, value, options, ignoreKeyPolicy);
         }
+        protected abstract void WriteKeyAsT(Utf8JsonWriter writer, T value);
 
-        public override string WriteKey(int key)
-        {
-            return key.ToString();
-        }
+        public override Type Type => typeof(T);
 
-        public override void WriteKeySpan(Span<byte> buffer, int key)
+        protected void WriteKeyAsTOrAsString(Utf8JsonWriter writer, [DisallowNull] T value, JsonSerializerOptions options, bool ignoreKeyPolicy)
         {
-            Utf8Formatter.TryFormat(key, buffer, out int _);
-        }
+            if (options.DictionaryKeyPolicy != null && !ignoreKeyPolicy)
+            {
+                // TODO: Why is value(!) neccessary?
+                string keyNameAsString = options.DictionaryKeyPolicy.ConvertName(value!.ToString()!);
 
-        public override int DetermineKeyLength(int key)
-        {
-            int length = (int)Math.Log10(Math.Abs(key)) + 1;
+                if (keyNameAsString == null)
+                {
+                    ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(options.DictionaryKeyPolicy.GetType());
+                }
 
-            if (key < 0)
+                writer.WritePropertyName(keyNameAsString);
+            }
+            else
             {
-                length++;
+                WriteKeyAsT(writer, value);
             }
-
-            return length;
         }
     }
 
-    internal sealed class GuidKeyConverter : KeyConverter<Guid>
+    internal sealed class Int32KeyConverter : KeyConverter<int>
     {
-        public override Guid ReadKey(ReadOnlySpan<byte> key)
+        public override bool ReadKey(ref Utf8JsonReader reader, out int value)
         {
-            Utf8Parser.TryParse(key, out Guid parsedKey, out int _);
-            return parsedKey;
+            return reader.TryGetInt32AfterValidation(out value);
         }
 
-        public override string WriteKey(Guid key)
-        {
-            return key.ToString();
-        }
+        protected override void WriteKeyAsT(Utf8JsonWriter writer, int key) => writer.WritePropertyName(key);
+    }
 
-        public override void WriteKeySpan(Span<byte> buffer, Guid key)
+    internal sealed class GuidKeyConverter : KeyConverter<Guid>
+    {
+        public override bool ReadKey(ref Utf8JsonReader reader, out Guid value)
         {
-            Utf8Formatter.TryFormat(key, buffer, out int _);
+            return reader.TryGetGuidAfterValidation(out value);
         }
 
-        public override int DetermineKeyLength(Guid _)
+        protected override void WriteKeyAsT(Utf8JsonWriter writer, Guid key) => writer.WritePropertyName(key);
+    }
+
+    internal sealed class StringKeyConverter : KeyConverter<string>
+    {
+        public override bool ReadKey(ref Utf8JsonReader reader, out string value)
         {
-            return JsonConstants.MaximumFormatGuidLength;
+            value = reader.GetString()!;
+
+            return true;
         }
+
+        protected override void WriteKeyAsT(Utf8JsonWriter writer, string key) => writer.WritePropertyName(key);
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

@@ -288,9 +288,12 @@ internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, Json
             bool success;
             JsonDictionaryConverter<T> dictionaryConverter = (JsonDictionaryConverter<T>)this;
 
-            if (ClassType == ClassType.Value)
+            if (ClassType == ClassType.Value) // This is always dictionary. Maybe is hit when the Dictionary<stirng, object have a custom converter?
             {
                 Debug.Assert(!state.IsContinuation);
+                Debug.Assert(false);
+
+                Debug.WriteLine("Im hit!");
 
                 int originalPropertyDepth = writer.CurrentDepth;
 
@@ -317,6 +320,7 @@ internal bool TryWriteDataExtensionProperty(Utf8JsonWriter writer, T value, Json
 
                 // Ignore the naming policy for extension data.
                 state.Current.IgnoreDictionaryKeyPolicy = true;
+                state.Current.DeclaredJsonPropertyInfo = state.Current.JsonClassInfo.ElementClassInfo!.PolicyProperty!;
 
                 success = dictionaryConverter.OnWriteResume(writer, value, options, ref state);
                 if (success)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfTTypeToConvert.cs

@@ -124,7 +124,7 @@ public override bool GetMemberAndWriteJsonExtensionData(object obj, ref WriteSta
             }
             else
             {
-                state.Current.PolymorphicJsonPropertyInfo = state.Current.DeclaredJsonPropertyInfo!.RuntimeClassInfo.ElementClassInfo!.PolicyProperty;
+                //state.Current.PolymorphicJsonPropertyInfo = state.Current.DeclaredJsonPropertyInfo!.RuntimeClassInfo.ElementClassInfo!.PolicyProperty;
                 success = Converter.TryWriteDataExtensionProperty(writer, value, Options, ref state);
             }
 

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

@@ -299,7 +299,7 @@ private static IEnumerable<JsonConverter> DefaultSimpleConverters
         // The global list of built-in key converters.
         private static readonly Dictionary<Type, KeyConverter> s_keyConverters = GetSupportedKeyConverters();
 
-        private const int NumberOfKeyConverters = 2;
+        private const int NumberOfKeyConverters = 3;
 
         private static Dictionary<Type, KeyConverter> GetSupportedKeyConverters()
         {
@@ -321,6 +321,7 @@ private static IEnumerable<KeyConverter> KeyConverters
             get
             {
                 // When adding to this, update NumberOfKeyConverters above.
+                yield return new StringKeyConverter();
                 yield return new Int32KeyConverter();
                 yield return new GuidKeyConverter();
             }

src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.String.cs

@@ -3,6 +3,8 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Buffers;
+using System.Buffers.Text;
+using System.Collections.Generic;
 using System.Diagnostics;
 
 namespace System.Text.Json
@@ -234,6 +236,28 @@ public void WritePropertyName(ReadOnlySpan<byte> utf8PropertyName)
             _tokenType = JsonTokenType.PropertyName;
         }
 
+        // TODO: move to WriteProperties.SignedNumber.cs?
+        internal void WritePropertyName(int value)
+        {
+            Span<byte> utf8PropertyName = stackalloc byte[JsonConstants.MaximumFormatInt64Length];
+
+            bool result = Utf8Formatter.TryFormat(value, utf8PropertyName, out int bytesWritten);
+            Debug.Assert(result);
+
+            WritePropertyName(utf8PropertyName.Slice(0, bytesWritten));
+        }
+
+        internal void WritePropertyName(Guid value)
+        {
+            Span<byte> utf8PropertyName = stackalloc byte[JsonConstants.MaximumFormatGuidLength];
+
+            bool result = Utf8Formatter.TryFormat(value, utf8PropertyName, out int bytesWritten);
+            Debug.Assert(result);
+
+            // Is this will always be 36?
+            WritePropertyName(utf8PropertyName);
+        }
+
         private void WriteStringEscapeProperty(ReadOnlySpan<byte> utf8PropertyName, int firstEscapeIndexProp)
         {
             Debug.Assert(int.MaxValue / JsonConstants.MaxExpansionFactorWhileEscaping >= utf8PropertyName.Length);