2

Closed

Port Fix: Enable JSON serialization of dynamic proxies

description

"INITIAL BUG:
It seems that when Entity Framework creates proxy classes for entity objects, it does not honour property attributes set on virtual navagation properties. It is a serious problem when serializing the object, as there is no easy way to skip navigation properties with JsonIgnore (JSON.Net) or ScriptIgnore (Microsoft). I believe it's also incorrect in case we wanted to do other custom processing that might require property attributes.

Repro Steps:
I created my POCO class TodoItem with the fields int Id, string Name, int AppUserId, virtual AppUser AppUser. I then went on to decorate the AppUser property with a custom [JsonIgnore] attribute like so: ""[JsonIgnore] public virtual AppUser AppUser { get; set; }"".

Actual Results:
JsonIgnore attribute is not set on the AppUser property.

Expected Results:
JsonIgnore attribute on the AppUser property should be carried over from the POCO class over to DbContext proxy class.

FURTHER FINDINGS:

3/14/2012 12:52:09 PM Edited by Diego Vega

We should also add ScriptIgnore to the _entityWrapper field:

Even after Pradeep coded the fix for the JavaScriptSerializer issue, he is getting errors due to graph cycles. I confirmed the culprit is the _entityWrapper field all proxies have, which the JavaScriptSerializer is picking up. The field references an object that contains data structures that are not serializable and also a reference back to the entity itself, which causes the cycle.

We tested several other types of serialization with EF proxies, but JSON serialization seemed to be too broken, so we decided to add IgnoreDataMemberAttribute, XmlIgnoreAttribute and NonSerializedAttribute but we missed ScriptIgnoreAttribute in _entityWrapper. In retrospective we should have added ScriptIgnoreAttribute as well.


3/5/2012 10:19:49 AM Edited by Arthur Vickers

I agree that this is a bug in the serializer, not in our code. That being said, we do currently copy custom attributes on the type that are needed for data contract and binary serialization to work correctly and I suspect if we had tested with JSON back when we wrote this code, then we might have done something similar to make this work. I don’t think we should do it now, but it is something we could consider doing in the future if this becomes a big problem and the serializer can’t be fixed.

I think the workaround of not creating proxies, while annoying, is not unreasonable for now.


3/5/2012 2:46:20 AM Edited by Diego Vega

I don't believe we have any code in proxy generation to clone custom attributes from the underlying POCO class. Instead we rely on inheritance to do its job. I.e. we would be good if the serializer was looking for ScriptIgnoreAttribute in properties and fields with inheritance, and in fact this seems to have been the intention... JavaScriptSerializer.SerializeCustomObject contains code like this for fields and properties:
        PropertyInfo[] props = type.GetProperties(BindingFlags.Public | BindingFlags.Instance | BindingFlags.GetProperty);
        foreach (PropertyInfo propInfo in props) { 

            // Ignore all properties marked as [ScriptIgnore]
            if (propInfo.IsDefined(typeof(ScriptIgnoreAttribute), true /*inherits*/)) 
                continue;
        ...
Unfortunately this code isn't correct. Per documentation at http://msdn.microsoft.com/en-us/library/system.reflection.memberinfo.isdefined.aspx, there is a known limitation in IsDefined (and in GetAttribute) on MemberInfo:

This method ignores the inherit parameter for properties and events. To search the inheritance chain for attributes on properties and events, use the appropriate overloads of the Attribute.IsDefined method.

I tried the methods in Attribute in the debugger and they are able to find ScriptIgnore without a problem:



?System.Attribute.IsDefined(posts[0].GetType().GetMember(""Blog"")[0], typeof(System.Web.Script.Serialization.ScriptIgnoreAttribute),true)

-->

true



The IsDefined method in MemberInfo can't find it:



?posts[0].GetType().GetMember(""Blog"")[0].IsDefined(typeof(System.Web.Script.Serialization.ScriptIgnoreAttribute),true)

-->

false

"

This item was migrated from the DevDiv work item tracking system [ID=358363].

This work item originated from connect.microsoft.com. A member of the EF team at Microsoft should close the related Connect issue when closing this work item.
Closed Jan 21 at 6:05 PM by BriceLambson

comments

divega wrote May 12, 2012 at 1:28 AM

I fixed this issue in in .NET 4.5 by adding ScriptIgnore to _entityWrapper. WebStack fixed the ScriptIgnoreAttribute to be inheritable. I have already responded to the customer and resolved the connect issue.

** Closed by divega 05/11/2012 6:28PM

divega wrote May 12, 2012 at 6:51 AM

Actually, we'll use this item to track porting the fix from .NET 4.5 to EF6.

vraikov wrote Jan 15, 2013 at 3:34 PM

What happened with the original issue that an inherited attribute is not visible on the proxy, which 'says' it's base class is the POCO object? I have simillar problem with XmlSerializer and XmlIgnoreAttribute

ajcvickers wrote Apr 30, 2013 at 11:02 PM

Fixed in 91138244af9e by David Obando committed by ajcvickers

TheOriginalIsUnfaithfulToTheTranslation - porting the fix to workitem 184 from the original changes made to Dev11.
Basically, this change attempts to load the assembly that contains the [ScriptIgnore] attribute and if it succeeds, it instantiates the parameterless constructor info. It is then used to decorate the field when it is marked as not serializable, along with [IgnoreDataMember] and [XmlIgnore].
See http://entityframework.codeplex.com/workitem/184 for more details.

maumar wrote May 17, 2013 at 10:32 PM

Additional fix checked in e1ec520
Change tracking proxied were not working because reference to RelationshipManager which then contains field _owner which loops back to the original entity, causing a cycle. Fix is to make proxies implemenet IEntityWithRelationships explicitly

BriceLambson wrote Jan 20 at 7:18 PM

Fixed in changeset 91138244af9eb8d80556cae7370d8ab89730bc93