JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.8k stars 3.26k forks source link

System.Reflection.RuntimeEventInfo member type unsupported by TypeExtensions.TestAccessibility() #771

Closed tmm360 closed 8 years ago

tmm360 commented 8 years ago

I'm deserializing with JsonConvert a ViewModelBase object, from GalaSoft.MvvmLight. It has an event PropertyChangedEventHandler PropertyChanged that throws an Exception("Unexpected member type.") into method TypeExtensions.TestAccessibility()

tlmii commented 8 years ago

I am having this same problem. For me it is an object derived from ObservableObject, which has that same event on it. I get the exception in the same place, though I can't tell for sure that it is the cause (the exception doesn't state which member it is looking at) but that seems like a safe assumption.

This is a change after upgrading from the 7.0.1 nuget package to the 8.0.1 nuget package. The TestAccessibility function didn't change in that release, as far as I can tell, so it must be something somewhere up the chain.

tmm360 commented 8 years ago

You are right, ViewModelBase inherits from ObservableObject. I've used sources for get the exception position. 7.0.1 works.

tlmii commented 8 years ago

Are you using the PCL version of JSON.NET, @tmm360 ? I just tried creating a small sample to reproduce the problem and noticed that referencing the .NET 4.5 DLL of JSON.NET works fine. But the PCL one does not.

tlmii commented 8 years ago

Alright, I've tracked down the culprit, but since I'm not sure why the change was made, I'm not sure how to fix it.

In commit https://github.com/JamesNK/Newtonsoft.Json/commit/fe7b7a6c7b0edfd071e5a1aa6b5f91a0b656d1ba a particular overload of Utilities\TypeExtensions.GetMember was changed.

In 7.0.1, there was an overload that was ONLY for PORTABLE40:

#if PORTABLE40
    ...

    public static IEnumerable<MemberInfo> GetMember(this Type type, string name, MemberTypes memberType, BindingFlags bindingFlags)
    {
        return type.GetMember(name, bindingFlags).Where(m =>
        {
            if (name != null && name != m.Name)
                return false;
            if (m.MemberType() != memberType)
                return false;

            return true;
        });
    }
#endif

and then there was another one for everything else:

    public static IEnumerable<MemberInfo> GetMember(this Type type, string name, MemberTypes memberType, BindingFlags bindingFlags)
    {
        return type.GetTypeInfo().GetMembersRecursive().Where(m =>
                                                                  {
                                                                      if (name != null && name != m.Name)
                                                                          return false;
                                                                      if (m.MemberType() != memberType)
                                                                          return false;
                                                                      if (!TestAccessibility(m, bindingFlags))
                                                                          return false;

                                                                      return true;
                                                                  });
    }

that one was used for PORTABLE.

But in commit https://github.com/JamesNK/Newtonsoft.Json/commit/fe7b7a6c7b0edfd071e5a1aa6b5f91a0b656d1ba, the second overload was removed and the preprocessor directive was changed to be:

#if (PORTABLE40 || DOTNET || PORTABLE) 

instead.

Those two overloads are NOT equivalent, apparently.

In fact, I think that anyone using the PORTABLE version, with an event (or possibly just with an event on a base class) will run into this issue as a breaking change between 7.0.1 and 8.0.1.

tlmii commented 8 years ago

OK, I've figured out why they're not equivalent.

In 7.0.1, we had this:

public static IEnumerable<MemberInfo> GetMember(this Type type, string name, MemberTypes memberType, BindingFlags bindingFlags)
{
    return type.GetTypeInfo().GetMembersRecursive().Where(m =>
                                {
                                    if (name != null && name != m.Name)
                                        return false;
                                    if (m.MemberType() != memberType)
                                        return false;
                                    if (!TestAccessibility(m, bindingFlags))
                                        return false;

                                    return true;
                                });
}

so it was kind of all in one place. But the gist of it is that for each member found recursively, it checks the name, then the type, then the accessibility.

But in 8.0.1 we have this:

    public static IEnumerable<MemberInfo> GetMember(this Type type, string name, MemberTypes memberType, BindingFlags bindingFlags)
    {
        return type.GetMember(name, bindingFlags).Where(m =>
        {
            if (m.MemberType() != memberType)
                return false;

            return true;
        });
    }

which uses this overload of GetMember:

public static MemberInfo[] GetMember(this Type type, string member, BindingFlags bindingFlags)
{
   return type.GetTypeInfo().GetMembersRecursive().Where(m => m.Name == member && TestAccessibility(m, bindingFlags)).ToArray();
}

which means that in this case, in the GetMember call, it checks the name, then the accessibility... then if those pass, it calls the anonymous method and checks the type.

The accessibility check is failing because the type (in this case an event) hasn't been verified yet.

JamesNK commented 8 years ago

Could someone give an example of a class that fails?

tlmii commented 8 years ago

The class I used to test it was this:

public class MyClass : ObservableObject
{
    public string TestString { get; set; }
}

where ObservableObject is from MVVMLight. Then just instantiate an object of MyClass and try to serialize it.

It fails as long as I'm using the PCL version of JSON.NET, but works fine if I'm using .NET 4.5 (which tripped me up initially because my repro was in a console app and nuget auto-added the 4.5 version of JSON.NET)

JamesNK commented 8 years ago

I can't reproduce this. Could someone either post a complete repo or an exception with the stacktrace?

[Test]
public void SerializeObjectWithEvent()
{
    MyObservableObject o = new MyObservableObject
    {
        TestString = "Test string"
    };
    o.PropertyChanged += (sender, args) => { };

    string json = JsonConvert.SerializeObject(o, Formatting.Indented);
    Console.WriteLine(json);

    MyObservableObject o2 = JsonConvert.DeserializeObject<MyObservableObject>(json);
    Assert.AreEqual("Test string", o2.TestString);
}

public class MyObservableObject : ObservableObject
{
    public string TestString { get; set; }
}

public class ObservableObject : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected PropertyChangedEventHandler PropertyChangedHandler
    {
        get { return PropertyChanged; }
    }
}
tlmii commented 8 years ago

I've attached a repro JsonNetExceptionExample.zip.

And here is a stack trace:

at Newtonsoft.Json.Utilities.TypeExtensions.TestAccessibility(MemberInfo member, BindingFlags bindingFlags)
at Newtonsoft.Json.Utilities.TypeExtensions.<>c__DisplayClass34_0.<GetMember>b__0(MemberInfo m)
at System.Linq.Enumerable.WhereListIterator`1.MoveNext()
at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
at Newtonsoft.Json.Utilities.TypeExtensions.GetMember(Type type, String member, BindingFlags bindingFlags)
at Newtonsoft.Json.Utilities.TypeExtensions.GetMember(Type type, String name, MemberTypes memberType, BindingFlags bindingFlags)
at Newtonsoft.Json.Utilities.ReflectionUtils.GetMemberInfoFromType(Type targetType, MemberInfo memberInfo)
at Newtonsoft.Json.Serialization.JsonTypeReflector.GetAttribute[T](MemberInfo memberInfo)
at Newtonsoft.Json.Serialization.JsonTypeReflector.GetAttribute[T](Object provider)
at Newtonsoft.Json.Serialization.DefaultContractResolver.GetSerializableMembers(Type objectType)
at Newtonsoft.Json.Serialization.DefaultContractResolver.CreateProperties(Type type, MemberSerialization memberSerialization)
at Newtonsoft.Json.Serialization.DefaultContractResolver.CreateObjectContract(Type objectType)
at Newtonsoft.Json.Serialization.DefaultContractResolver.CreateContract(Type objectType)
at Newtonsoft.Json.Serialization.DefaultContractResolver.ResolveContract(Type type)
at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType)
at Newtonsoft.Json.JsonConvert.SerializeObjectInternal(Object value, Type type, JsonSerializer jsonSerializer)
at Newtonsoft.Json.JsonConvert.SerializeObject(Object value)
at JsonNetExceptionExample.Program.Main(String[] args) in c:\users\tim\documents\visual studio 2015\Projects\JsonNetExceptionExample\JsonNetExceptionExample\Program.cs:line 16
at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()

Interestingly, this stack trace seems different than the one I was getting earlier in the day, but that was on a different project. Seems similar though.

JamesNK commented 8 years ago

Thanks. I believe I have fixed this - https://github.com/JamesNK/Newtonsoft.Json/commit/1ada613903977578364d68f580e787b638a391c9

JamesNK commented 8 years ago

https://www.nuget.org/packages/Newtonsoft.Json/8.0.2