From 13ff02dc31eede8085adbd1af0eaca49dcf1dbb0 Mon Sep 17 00:00:00 2001 From: Sebastien Pouliot Date: Mon, 26 May 2014 17:27:26 -0400 Subject: [PATCH] [System.ServiceModel.Web] Fix issue with types explicitly implementing IDictionary or IDictionary<,> Issue: The previous code, with new tests [1], failed when the linker was enabled. That was caused because the code looks for the Keys and Item properties getter using reflection. The linker handled that imperfectly (it left some holes), in part because: * System.Collections.Generic.IDictionary`2 does not implement System.Collections.IDictionary; and * It preserved the explicit methods - not the implicit one that were used the the reflection-based code; That later fact is also a bug (without the linker involvment) and solved by this commit. A unit test was added to show the issue (NRE). The patch also avoid the use of reflection for IDictionary since we can simply typecast to get the same result (without the additional cost). --- .../JsonSerializationWriter.cs | 60 +++++++--- .../DataContractJsonSerializerTest.cs | 104 +++++++++++++++++- 2 files changed, 148 insertions(+), 16 deletions(-) diff --git a/mcs/class/System.ServiceModel.Web/System.Runtime.Serialization.Json/JsonSerializationWriter.cs b/mcs/class/System.ServiceModel.Web/System.Runtime.Serialization.Json/JsonSerializationWriter.cs index 9ccf447e5bf..9ea5bf907d6 100644 --- a/mcs/class/System.ServiceModel.Web/System.Runtime.Serialization.Json/JsonSerializationWriter.cs +++ b/mcs/class/System.ServiceModel.Web/System.Runtime.Serialization.Json/JsonSerializationWriter.cs @@ -121,21 +121,21 @@ namespace System.Runtime.Serialization.Json writer.WriteString (qn.Namespace); } else if (TypeMap.IsDictionary (type)) { writer.WriteAttributeString ("type", "array"); - var itemGetter = type.GetProperty ("Item"); - var keysGetter = type.GetProperty ("Keys"); - var argarr = new object [1]; - foreach (object o in (IEnumerable) keysGetter.GetValue (graph, null)) { - writer.WriteStartElement ("item"); - writer.WriteAttributeString ("type", "object"); - // outputting a KeyValuePair as - writer.WriteStartElement ("Key"); - WriteObjectContent (o, false, !(graph is Array && type.GetElementType () != typeof (object))); - writer.WriteEndElement (); - writer.WriteStartElement ("Value"); - argarr [0] = o; - WriteObjectContent (itemGetter.GetValue (graph, argarr), false, !(graph is Array && type.GetElementType () != typeof (object))); - writer.WriteEndElement (); - writer.WriteEndElement (); + bool otn = !(graph is Array && type.GetElementType () != typeof (object)); + var d = graph as IDictionary; + if (d != null) { + // Optimize the IDictionary case to avoid reflection + foreach (object k in d.Keys) + WriteItem (k, d [k], otn); + } else { + // we can't typecast to IDictionary<,> and can't use dynamic for iOS support + var itemGetter = GetDictionaryProperty (type, "Item"); + var keysGetter = GetDictionaryProperty (type, "Keys"); + var argarr = new object [1]; + foreach (object o in (IEnumerable) keysGetter.GetValue (graph, null)) { + argarr [0] = o; + WriteItem (o, itemGetter.GetValue (graph, argarr), otn); + } } } else if (graph is Array || TypeMap.IsEnumerable (type)) { writer.WriteAttributeString ("type", "array"); @@ -162,6 +162,36 @@ throw new InvalidDataContractException (String.Format ("Type {0} cannot be seria } } + void WriteItem (object key, object value, bool outputTypeName) + { + writer.WriteStartElement ("item"); + writer.WriteAttributeString ("type", "object"); + // outputting a KeyValuePair as + writer.WriteStartElement ("Key"); + WriteObjectContent (key, false, outputTypeName); + writer.WriteEndElement (); + writer.WriteStartElement ("Value"); + WriteObjectContent (value, false, outputTypeName); + writer.WriteEndElement (); + writer.WriteEndElement (); + } + + PropertyInfo GetDictionaryProperty (Type type, string propertyName) + { + var p = type.GetProperty (propertyName); + if (p != null) + return p; + // check explicit - but the generic names might differ, e.g. TKey,TValue vs T,V + var ap = type.GetProperties (BindingFlags.Instance | BindingFlags.NonPublic); + foreach (var cp in ap) { + if (!cp.Name.EndsWith (propertyName, StringComparison.Ordinal)) + continue; + if (cp.Name.StartsWith ("System.Collections.Generic.IDictionary<", StringComparison.Ordinal)) + return cp; + } + return null; + } + string FormatTypeName (Type type) { return String.Format ("{0}:#{1}", type.Name, type.Namespace); diff --git a/mcs/class/System.ServiceModel.Web/Test/System.Runtime.Serialization.Json/DataContractJsonSerializerTest.cs b/mcs/class/System.ServiceModel.Web/Test/System.Runtime.Serialization.Json/DataContractJsonSerializerTest.cs index 7bcb40ccf8f..68d9fb6a083 100644 --- a/mcs/class/System.ServiceModel.Web/Test/System.Runtime.Serialization.Json/DataContractJsonSerializerTest.cs +++ b/mcs/class/System.ServiceModel.Web/Test/System.Runtime.Serialization.Json/DataContractJsonSerializerTest.cs @@ -1453,7 +1453,24 @@ namespace MonoTests.System.Runtime.Serialization.Json Assert.AreEqual (1, dict.Count, "#2"); Assert.AreEqual ("value", dict ["key"], "#3"); } - + + [Test] + public void ExplicitCustomDictionarySerialization () + { + var dict = new MyExplicitDictionary (); + dict.Add ("key", "value"); + var serializer = new DataContractJsonSerializer (dict.GetType ()); + var stream = new MemoryStream (); + serializer.WriteObject (stream, dict); + stream.Position = 0; + + Assert.AreEqual ("[{\"Key\":\"key\",\"Value\":\"value\"}]", new StreamReader (stream).ReadToEnd (), "#1"); + stream.Position = 0; + dict = (MyExplicitDictionary) serializer.ReadObject (stream); + Assert.AreEqual (1, dict.Count, "#2"); + Assert.AreEqual ("value", dict ["key"], "#3"); + } + [Test] public void Bug13485 () { @@ -2113,6 +2130,91 @@ public class MyDictionary : System.Collections.Generic.IDictionary } } +public class MyExplicitDictionary : IDictionary { + + Dictionary dic = new Dictionary (); + + public void Add (K key, V value) + { + dic.Add (key, value); + } + + public bool ContainsKey (K key) + { + return dic.ContainsKey (key); + } + + ICollection IDictionary.Keys { + get { return dic.Keys; } + } + + public bool Remove (K key) + { + return dic.Remove (key); + } + + public bool TryGetValue (K key, out V value) + { + return dic.TryGetValue (key, out value); + } + + ICollection IDictionary.Values { + get { return dic.Values; } + } + + public V this [K key] { + get { return dic [key]; } + set { dic [key] = value; } + } + + IEnumerator IEnumerable.GetEnumerator () + { + return dic.GetEnumerator (); + } + + ICollection> Coll { + get { return (ICollection>) dic; } + } + + public void Add (KeyValuePair item) + { + Coll.Add (item); + } + + public void Clear () + { + dic.Clear (); + } + + public bool Contains (KeyValuePair item) + { + return Coll.Contains (item); + } + + public void CopyTo (KeyValuePair [] array, int arrayIndex) + { + Coll.CopyTo (array, arrayIndex); + } + + public int Count { + get { return dic.Count; } + } + + public bool IsReadOnly { + get { return Coll.IsReadOnly; } + } + + public bool Remove (KeyValuePair item) + { + return Coll.Remove (item); + } + + public IEnumerator> GetEnumerator () + { + return Coll.GetEnumerator (); + } +} + [DataContract] public class Bug13485Type { -- 2.25.1