JamesNK / Newtonsoft.Json

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

MetadataTypeAttribute ignored in latest version #405

Closed georgemak closed 9 years ago

georgemak commented 10 years ago

I am not really sure if this is a bug or the functionality has changed in the latest versions but after updating a web api project from Newtonsoft.Json version 4.5.11 to version 6.0.6 I discovered that the annotations were actually ignored and the serialization occured even for the properties that were annotated as JsonIgnore.

In order to annotate the objects I use buddy classes:

[MetadataType(typeof(CustomerValidation))]
public partial class Customer
{

public class CustomerValidation
{
    [JsonIgnore]
    public System.Guid UpdatedBy_Id { get; set; }
}
}

If this is not a bug, how can the same functionality be accomplished in the latest version?

JamesNK commented 10 years ago

It should still work. This test passes for me:

    [MetadataType(typeof(CustomerValidation))]
    public partial class CustomerWithMetadataType
    {
        public System.Guid UpdatedBy_Id { get; set; }

        public class CustomerValidation
        {
            [JsonIgnore]
            public System.Guid UpdatedBy_Id { get; set; }
        }
    }

    [Test]
    public void SerializeMetadataType()
    {
        CustomerWithMetadataType c = new CustomerWithMetadataType();
        c.UpdatedBy_Id = Guid.NewGuid();

        string json = JsonConvert.SerializeObject(c);

        Assert.AreEqual("{}", json);

        CustomerWithMetadataType c2 = JsonConvert.DeserializeObject<CustomerWithMetadataType>("{'UpdatedBy_Id':'F6E0666D-13C7-4745-B486-800812C8F6DE'}");

        Assert.AreEqual(Guid.Empty, c2.UpdatedBy_Id);
    }

What version of .NET are you running on? What version of Json.NET are you using? (you can tell by looking at the assembly reference path)

georgemak commented 10 years ago

I have a .NET 4.0 website with a reference to a .NET 4.0 class library project where the entities are defined and I was using Json.NET version 6.0.6 (latest stable from Nuget) when I saw the issue. It's very strange, as soon as I updated via npm console to version 4.5.11 the issue went away.

I will try to retest by repeating the procedure tomorrow and I will let you know.

JamesNK commented 10 years ago

Works using the .NET 4.0 assembly with that test. Is the MetadataType attribute on a base class and you're serializing a class that inherits from it?

georgemak commented 10 years ago

No I am serializing the same class, I will try to reproduce the error in a simple project and upload it.

julianbueno commented 10 years ago

I have the same issue Web site running on .Net 4.5.1, I have to downgrade to 6.0.5

JamesNK commented 10 years ago

I can't reproduce this. Could someone add a complete example?

BrianMinister commented 10 years ago

The problem seems selective. I have one assembly where JsonIgnore is ignored, and another where it isn't.

What is funny, is that the assembly where Ignore fails, my collections also fail to accept their new property name.

This is very difficult to duplicate into a new project. So I am going to drop all of the libraries, clear out my bin directory (Added precaution), then re-add.

I have already seen where Nuget messed up my EF update, so maybe it did the same with Json.net.

BrianMinister commented 10 years ago

I have only been able to replicate this issue when running with the latest ASP.NET from nuget. That is, I built the original code from the release at the beginning of April, updated it to the latest ASP.NET in Sept, and Updated Everything in Oct. By rolling back to 6.0.4 everything is working.

I will try to make a sanitized version of this code, so I can post it without breaking company rules.

JamesNK commented 10 years ago

I have an idea of what might be causing this and have a potential fix (I can't reproduce it so can't test it).

In a few days I'll upload a link to a new 6.0.6 signed DLL for someone who is getting the error to try out.

istvanpuskas commented 10 years ago

Is there any update on this? I'm using the 6.0.6 (10/24/2014) version from nuget, and experiencing the same problem like JsonIgnore attribute seems ignored (how ironic :) If this issue is not fixed, I think I could provide a test scenario.

JamesNK commented 10 years ago

I have committed a change that I think will fix this. You could build from the latest source code and see whether the bug is fixed for you (if you have assemblies that depend on json.net you won't be able to do this because of the missing sn key)

istvanpuskas commented 10 years ago

Thanks for the answer. That happened exactly, what you mentioned. Do you plan to publish the latest version to nuget some time?

istvanpuskas commented 9 years ago

Actually I forgot something. The properties on what I'd like to use the JsonIgnore attribute is a shadowed property in a derived class ('Shadows' in vb.net, 'new' keyword in c#). It works fine on "standard" properties. Is there a way (or workaround) to make JsonIgnore work on shadowed properties?

paulduran commented 9 years ago

I have had the same issue as @istvanpuskas relating to shadowed properties.

Lets say I have two classes:

    public class Animal
    {
        public string Name { get;set;}
        public string Gender {get;set; }
    }

    public class ApiAnimal : Animal
    {
        // i dont want to serialise gender via the API
        [JsonIgnore]
        public new string Gender { get { return base.Gender; } }
    }

If I was to serialise an instance of ApiAnimal, the Gender property will still be serialised. This was not the behaviour pre Json.Net v6

My interim fix was to put the JsonIgnore properties on the base class (which is not ideal but I couldn't find another way at this stage)

Paulskit commented 9 years ago

Unfortunately not solved in newest 6.0.7 version.

BrianMinister commented 9 years ago

@Paulskit, can you post sample code somewhere that we can trace the failure with?

After I downgraded, I haven't had time to create a test project.

Paulskit commented 9 years ago

In my case, problem is 100% reproducible when using EntityFramework 6 with ProxyCreationEnabled set to true. For example, we have class

[Serializable]
public partial class FaqItem
{
    public FaqItem()
    {
        this.Sections = new HashSet<FaqSection>();
    }

    public int FaqId { get; set; }
    public string Name { get; set; }
    public bool IsDeleted { get; set; }

    public virtual ICollection<FaqSection> Sections { get; set; }
}

It's autogenerated by EntityFramework. Then we add some metadata attributes like this

[MetadataType(typeof(FaqItemMetadata))]
partial class FaqItem {
    [JsonProperty("sections")]
    public IQueryable<FaqSection> FullSections {
        get { return Sections.AsQueryable().Where(o => !o.IsDeleted).Include(o => o.Questions); }
    }
}

class FaqItemMetadata {
    [JsonIgnore]
    public virtual ICollection<FaqSection> Sections { get; set; }
}

In the latest version, Sections property is not ignored.

JamesNK commented 9 years ago

I think I might have fixed it for realz this time. At the very least the scenario @Paulskit has has been fixed.

JamesNK commented 9 years ago

I have released a new version: 6.0.8

@Paulskit your issue should be fixed.

I don't know about anyone else's issues but if you could check then that would be super.

istvanpuskas commented 9 years ago

Unfortunately not. The simplest scenario is the code that @paulduran commented. The 'Gender' property is still seems serialized. I wrote a really simple console application to test it (with 6.0.8 from nuget). Actually the value of the field is null, which suggests reflection finds the property defined in the base class.

using Newtonsoft.Json;

namespace jsonnet_cs_test { class Base { public string prop1 { get; set; } }

class Derived: Base
{
    [JsonIgnore]
    public new string prop1 { get; set; }
}

class Program
{
    static void Main(string[] args)
    {
        System.Console.WriteLine(JsonConvert.SerializeObject(new Derived { prop1 = "x" }));
        //output: {"prop1":null}

        System.Console.ReadKey();
    }
}

}

JamesNK commented 9 years ago

You and @paulduran are talking about a change that happened a year and a half ago and is not a bug. This issue is related to a recent MetadataTypeAttribute regression.

JamesNK commented 9 years ago

Is anyone still broken with MetadataTypeAttribute problems that have popped up between releases? Speak now or I'll close this issue.

philbern commented 9 years ago

I do still have this exact problem with version 6.0.8 on .net 4.5.1 and mvc web api 5.2.2

Also, setting ProxyCreationEnabled to false doesnt fix it

JamesNK commented 9 years ago

You'll need to upload an example project somewhere. Something that I can just F5 an run to see the issue. Without being able to see it I can't fix it.

philbern commented 9 years ago

Here's a simplified project reproducing the problem: https://dl.dropboxusercontent.com/u/31808436/TestJsonNet.zip

You just need to restore the nuget packages and press F5

As you will see, the problem seems related to the use of OData Expand instruction

MaxAnikin commented 9 years ago

Hello, I see the same issue with the version 6.0.8. JsonIgnore attribute is not working at all. Simple class with a few properties and some of them marked with JsonIgnore. Result json string contains all the properties. .Net version 4.5.2.

JamesNK commented 9 years ago

Is it related to metadatatypeattribute? If not then create a new issue and include your class + json.

SpiegelSoft commented 9 years ago

I am seeing the same issue when using Json.NET and OData. JsonIgnore is being ignored.

JamesNK commented 9 years ago

@philbern I tested your code with an old version of Json.NET and the result was the same. This isn't a regression. OData is turning the object that gets passed to Json.NET into a dictionary which means it has no knowledge of the attributes. If you change the method to return a list then the secret key is correctly removed.

JamesNK commented 9 years ago

I'm closing this. I fixed the original regression when I fixed @paulskit's scenario.

Paulskit commented 9 years ago

Sorry for delay. Yes, issue is fixed for my scenario. Thanks.