Skip to content

Commit 5d29e7d

Browse files
layomialayomia
committed
Honor propery naming policy when (de)serializing KeyValuePair instances
1 parent 76b53db commit 5d29e7d

File tree

6 files changed

+182
-23
lines changed

6 files changed

+182
-23
lines changed

‎src/libraries/System.Text.Json/src/Resources/Strings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,9 @@
345345
<data name="SerializerPropertyNameNull" xml:space="preserve">
346346
<value>The JSON property name for '{0}.{1}' cannot be null.</value>
347347
</data>
348+
<data name="SerializerPropertyNamingPolicyReturnNull" xml:space="preserve">
349+
<value>The property naming policy '{0}' cannot return a null property name.</value>
350+
</data>
348351
<data name="DeserializeDuplicateKey" xml:space="preserve">
349352
<value>An item with the same property name '{0}' has already been added.</value>
350353
</data>

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonKeyValuePairConverter.cs‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,43 @@ namespace System.Text.Json.Serialization.Converters
1010
{
1111
internal sealed class JsonKeyValuePairConverter : JsonConverterFactory
1212
{
13+
private readonly byte[] _keyName;
14+
private readonly byte[] _valueName;
15+
16+
private readonly JsonEncodedText _encodedKeyName;
17+
private readonly JsonEncodedText _encodedValueName;
18+
19+
public JsonKeyValuePairConverter(JsonSerializerOptions options)
20+
{
21+
JsonNamingPolicy namingPolicy = options.PropertyNamingPolicy;
22+
if (namingPolicy == null)
23+
{
24+
_keyName = new byte[] { (byte)'K', (byte)'e', (byte)'y' };
25+
_valueName = new byte[] { (byte)'V', (byte)'a', (byte)'l', (byte)'u', (byte)'e' };
26+
}
27+
else
28+
{
29+
string propertyName = namingPolicy.ConvertName("Key");
30+
if (propertyName == null)
31+
{
32+
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNamingPolicyReturnNull(namingPolicy.GetType());
33+
}
34+
_keyName = JsonReaderHelper.s_utf8Encoding.GetBytes(propertyName);
35+
36+
propertyName = options.PropertyNamingPolicy.ConvertName("Value");
37+
if (propertyName == null)
38+
{
39+
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNamingPolicyReturnNull(namingPolicy.GetType());
40+
}
41+
_valueName = JsonReaderHelper.s_utf8Encoding.GetBytes(propertyName);
42+
}
43+
44+
// "encoder: null" is used since the literal values of "Key" and "Value" should not normally be escaped
45+
// unless a custom encoder is used that escapes these ASCII characters (rare).
46+
_encodedKeyName = JsonEncodedText.Encode(_keyName, encoder: null);
47+
_encodedValueName = JsonEncodedText.Encode(_valueName, encoder: null);
48+
}
49+
1350
public override bool CanConvert(Type typeToConvert)
1451
{
1552
if (!typeToConvert.IsGenericType)
@@ -19,7 +56,9 @@ public override bool CanConvert(Type typeToConvert)
1956
return (generic == typeof(KeyValuePair<,>));
2057
}
2158

22-
[PreserveDependency(".ctor()", "System.Text.Json.Serialization.Converters.JsonKeyValuePairConverter`2")]
59+
[PreserveDependency(
60+
".ctor(Byte[], Byte[], System.Text.Json.JsonEncodedText, System.Text.Json.JsonEncodedText)",
61+
"System.Text.Json.Serialization.Converters.JsonKeyValuePairConverter`2")]
2362
public override JsonConverter CreateConverter(Type type, JsonSerializerOptions options)
2463
{
2564
Type keyType = type.GetGenericArguments()[0];
@@ -29,7 +68,7 @@ public override JsonConverter CreateConverter(Type type, JsonSerializerOptions o
2968
typeof(JsonKeyValuePairConverter<,>).MakeGenericType(new Type[] { keyType, valueType }),
3069
BindingFlags.Instance | BindingFlags.Public,
3170
binder: null,
32-
args: null,
71+
args: new object[] { _keyName, _valueName, _encodedKeyName, _encodedValueName },
3372
culture: null);
3473

3574
return converter;

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonValueConverterKeyValuePair.cs‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,19 @@ namespace System.Text.Json.Serialization.Converters
88
{
99
internal sealed class JsonKeyValuePairConverter<TKey, TValue> : JsonConverter<KeyValuePair<TKey, TValue>>
1010
{
11-
private const string KeyName = "Key";
12-
private const string ValueName = "Value";
11+
private readonly byte[] _keyName;
12+
private readonly byte[] _valueName;
1313

14-
// "encoder: null" is used since the literal values of "Key" and "Value" should not normally be escaped
15-
// unless a custom encoder is used that escapes these ASCII characters (rare).
16-
// Also by not specifying an encoder allows the values to be cached statically here.
17-
private static readonly JsonEncodedText _keyName = JsonEncodedText.Encode(KeyName, encoder: null);
18-
private static readonly JsonEncodedText _valueName = JsonEncodedText.Encode(ValueName, encoder: null);
14+
private readonly JsonEncodedText _encodedKeyName;
15+
private readonly JsonEncodedText _encodedValueName;
16+
17+
public JsonKeyValuePairConverter(byte[] keyName, byte[] valueName, JsonEncodedText encodedKeyName, JsonEncodedText encodedValueName)
18+
{
19+
_keyName = keyName;
20+
_valueName = valueName;
21+
_encodedKeyName = encodedKeyName;
22+
_encodedValueName = encodedValueName;
23+
}
1924

2025
public override KeyValuePair<TKey, TValue> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
2126
{
@@ -37,13 +42,12 @@ public override KeyValuePair<TKey, TValue> Read(ref Utf8JsonReader reader, Type
3742
ThrowHelper.ThrowJsonException();
3843
}
3944

40-
string propertyName = reader.GetString();
41-
if (propertyName == KeyName)
45+
if (reader.ValueTextEquals(_valueName))
4246
{
4347
k = ReadProperty<TKey>(ref reader, typeToConvert, options);
4448
keySet = true;
4549
}
46-
else if (propertyName == ValueName)
50+
else if (reader.ValueTextEquals(_keyName))
4751
{
4852
v = ReadProperty<TValue>(ref reader, typeToConvert, options);
4953
valueSet = true;
@@ -60,13 +64,12 @@ public override KeyValuePair<TKey, TValue> Read(ref Utf8JsonReader reader, Type
6064
ThrowHelper.ThrowJsonException();
6165
}
6266

63-
propertyName = reader.GetString();
64-
if (propertyName == ValueName)
67+
if (reader.ValueTextEquals(_valueName))
6568
{
6669
v = ReadProperty<TValue>(ref reader, typeToConvert, options);
6770
valueSet = true;
6871
}
69-
else if (propertyName == KeyName)
72+
else if (reader.ValueTextEquals(_keyName))
7073
{
7174
k = ReadProperty<TKey>(ref reader, typeToConvert, options);
7275
keySet = true;
@@ -131,8 +134,8 @@ private void WriteProperty<T>(Utf8JsonWriter writer, T value, JsonEncodedText na
131134
public override void Write(Utf8JsonWriter writer, KeyValuePair<TKey, TValue> value, JsonSerializerOptions options)
132135
{
133136
writer.WriteStartObject();
134-
WriteProperty(writer, value.Key, _keyName, options);
135-
WriteProperty(writer, value.Value, _valueName, options);
137+
WriteProperty(writer, value.Key, _encodedKeyName, options);
138+
WriteProperty(writer, value.Value, _encodedValueName, options);
136139
writer.WriteEndObject();
137140
}
138141
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ namespace System.Text.Json
1616
/// </summary>
1717
public sealed partial class JsonSerializerOptions
1818
{
19+
// The global list of built-in converters that override CanConvert().
20+
private readonly List<JsonConverter> _defaultFactoryConverters = GetDefaultConverters();
21+
1922
// The global list of built-in simple converters.
2023
private static readonly Dictionary<Type, JsonConverter> s_defaultSimpleConverters = GetDefaultSimpleConverters();
2124

22-
// The global list of built-in converters that override CanConvert().
23-
private static readonly List<JsonConverter> s_defaultFactoryConverters = GetDefaultConverters();
24-
2525
// The cached converters (custom or built-in).
2626
private readonly ConcurrentDictionary<Type, JsonConverter> _converters = new ConcurrentDictionary<Type, JsonConverter>();
2727

@@ -40,15 +40,15 @@ private static Dictionary<Type, JsonConverter> GetDefaultSimpleConverters()
4040
return converters;
4141
}
4242

43-
private static List<JsonConverter> GetDefaultConverters()
43+
private List<JsonConverter> GetDefaultConverters()
4444
{
4545
const int NumberOfConverters = 2;
4646

4747
var converters = new List<JsonConverter>(NumberOfConverters);
4848

4949
// Use a list for converters that implement CanConvert().
5050
converters.Add(new JsonConverterEnum());
51-
converters.Add(new JsonKeyValuePairConverter());
51+
converters.Add(new JsonKeyValuePairConverter(this));
5252

5353
// We will likely add collection converters here in the future.
5454

@@ -140,7 +140,7 @@ public JsonConverter GetConverter(Type typeToConvert)
140140
}
141141
else
142142
{
143-
foreach (JsonConverter item in s_defaultFactoryConverters)
143+
foreach (JsonConverter item in _defaultFactoryConverters)
144144
{
145145
if (item.CanConvert(typeToConvert))
146146
{

‎src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ public static void ThrowInvalidOperationException_SerializerPropertyNameNull(Typ
125125
throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNameNull, parentType, jsonPropertyInfo.PropertyInfo.Name));
126126
}
127127

128+
[MethodImpl(MethodImplOptions.NoInlining)]
129+
public static void ThrowInvalidOperationException_SerializerPropertyNamingPolicyReturnNull(Type policyType)
130+
{
131+
throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNamingPolicyReturnNull, policyType));
132+
}
133+
128134
[MethodImpl(MethodImplOptions.NoInlining)]
129135
public static void ThrowInvalidOperationException_SerializerDictionaryKeyNull(Type policyType)
130136
{

‎src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,109 @@ public static void LongPropertyNames(int propertyLength, char ch)
430430
string jsonRoundTripped = JsonSerializer.Serialize(obj, options);
431431
Assert.Equal(json, jsonRoundTripped);
432432
}
433+
434+
[Fact]
435+
public static void RootKeyValuePairSerialize()
436+
{
437+
JsonSerializerOptions options = new JsonSerializerOptions
438+
{
439+
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
440+
};
441+
442+
const string jsonWithoutPolicy = @"{""Key"":,""Value"":""MyValue""}";
443+
const string jsonWithPolicy = @"{""key"":,""value"":""MyValue""}";
444+
445+
// Baseline: Without policy.
446+
string serialized = JsonSerializer.Serialize(new KeyValuePair<string, string>("MyKey", "MyValue"));
447+
Assert.Equal(jsonWithoutPolicy, serialized);
448+
449+
KeyValuePair<string, string> kvp = JsonSerializer.Deserialize<KeyValuePair<string, string>>(serialized);
450+
Assert.Equal("MyKey", kvp.Key);
451+
Assert.Equal("MyValue", kvp.Value);
452+
453+
// No property name match.
454+
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<KeyValuePair<string, string>>(jsonWithPolicy));
455+
456+
// With policy.
457+
serialized = JsonSerializer.Serialize(new KeyValuePair<string, string>("MyKey", "MyValue"), options);
458+
Assert.Equal(jsonWithPolicy, serialized);
459+
460+
kvp = JsonSerializer.Deserialize<KeyValuePair<string, string>>(serialized, options);
461+
Assert.Equal("MyKey", kvp.Key);
462+
Assert.Equal("MyValue", kvp.Value);
463+
464+
// No property name match.
465+
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<KeyValuePair<string, string>>(jsonWithoutPolicy, options));
466+
}
467+
468+
[Fact]
469+
public static void KeyValuePairAsCollectionElementSerialize()
470+
{
471+
JsonSerializerOptions options = new JsonSerializerOptions
472+
{
473+
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
474+
};
475+
476+
const string jsonWithoutPolicy = @"[{""Key"":,""Value"":""MyValue""}]";
477+
const string jsonWithPolicy = @"[{""key"":,""value"":""MyValue""}]";
478+
479+
// Baseline: Without policy.
480+
string serialized = JsonSerializer.Serialize(new KeyValuePair[] { new KeyValuePair<string, string>("MyKey", "MyValue") });
481+
Assert.Equal(jsonWithoutPolicy, serialized);
482+
483+
KeyValuePair<string, string>[] arr = JsonSerializer.Deserialize<KeyValuePair<string, string>[]>(serialized);
484+
Assert.Equal("MyKey", arr[0].Key);
485+
Assert.Equal("MyValue", arr[0].Value);
486+
487+
// No property name match.
488+
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<KeyValuePair<string, string>[]>(jsonWithPolicy));
489+
490+
// With policy.
491+
serialized = JsonSerializer.Serialize(new KeyValuePair[] { new KeyValuePair<string, string>("MyKey", "MyValue") }, options);
492+
Assert.Equal(jsonWithPolicy, serialized);
493+
494+
arr = JsonSerializer.Deserialize<KeyValuePair<string, string>>(serialized, options);
495+
Assert.Equal("MyKey", arr[0].Key);
496+
Assert.Equal("MyValue", arr[0].Value);
497+
498+
// No property name match.
499+
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<KeyValuePair<string, string>[]>(jsonWithoutPolicy, options));
500+
}
501+
502+
[Fact]
503+
public static void KeyValuePairAsClassPropertyElementSerialize()
504+
{
505+
JsonSerializerOptions options = new JsonSerializerOptions
506+
{
507+
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
508+
};
509+
510+
const string jsonWithoutPolicy = @"{""Kvp"":{""Key"":,""Value"":""MyValue""}}";
511+
const string jsonWithPolicy = @"{""kvp"":{""key"":,""value"":""MyValue""}}";
512+
513+
// Baseline: Without policy.
514+
string serialized = JsonSerializer.Serialize(new ClassWithKVP { Kvp = new KeyValuePair<string, string>("MyKey", "MyValue") });
515+
Assert.Equal(jsonWithoutPolicy, serialized);
516+
517+
ClassWithKVP obj = JsonSerializer.Deserialize<KeyValuePair<string, string>[]>(serialized);
518+
Assert.Equal("MyKey", obj.Kvp.Key);
519+
Assert.Equal("MyValue", obj.Kvp.Value);
520+
521+
// No property name match.
522+
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<ClassWithKVP>(jsonWithPolicy));
523+
524+
// With policy.
525+
serialized = JsonSerializer.Serialize(new ClassWithKVP { Kvp = new KeyValuePair<string, string>("MyKey", "MyValue") }, options);
526+
Assert.Equal(jsonWithPolicy, serialized);
527+
528+
obj = JsonSerializer.Deserialize<ClassWithKVP>(serialized, options);
529+
Assert.Equal("MyKey", obj.Kvp.Key);
530+
Assert.Equal("MyValue", obj.Kvp.Value);
531+
532+
// No property name match.
533+
obj = JsonSerializer.Deserialize<ClassWithKVP>(jsonWithoutPolicy, options);
534+
Assert.Null(obj.Kvp);
535+
}
433536
}
434537

435538
public class OverridePropertyNameDesignTime_TestClass
@@ -495,4 +598,9 @@ public class EmptyClassWithExtensionProperty
495598
[JsonExtensionData]
496599
public IDictionary<string, JsonElement> MyOverflow { get; set; }
497600
}
601+
602+
public class ClassWithKVP
603+
{
604+
public KeyValuePair<string, string> Kvp { get; set; }
605+
}
498606
}

0 commit comments

Comments
 (0)