Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link that can reference multiple content types is not being mapped #357

Open
georgezubov opened this issue Nov 7, 2024 · 28 comments
Open

Comments

@georgezubov
Copy link

georgezubov commented Nov 7, 2024

Affected version: 8.3.0 and 8.2.1

Issue:

Hello, we have following content type structure:

public class Type1
{
    public IContentType[] Items { get; set; }
}

public class Type2: IContentType
{
    public Type3[] Items { get; set; }
}

public class Type3
{
    public IContentType Field { get; set; }
}

public interface IContentType {}

When we put object of Type2 in the items of Type1, Field of Type3 (which can be the reference to a multiple content types) is always deserialized as null.

Workaround for us is rolling back to version 7.6.1, there everything works as expected.

Here is a redacted version of the API response - test.json

Could you please take a look and help us to resolve this issue? Let me know if any additional information is required. Thanks in advance!

@Roblinde
Copy link
Collaborator

Roblinde commented Nov 9, 2024

@georgezubov this seems to work fine for me. In the class you deserialize into, are you using dynamic of JObject as any of the property types?

@georgezubov
Copy link
Author

@Roblinde no, we don't use dynamic of JObject in our models. Maybe it could also be important - there is actually one more layer above the Type1:

public class Type0
{
    public IContentType[] Items { get; set; }
}

So the object of Type1 is added to the Items of Type0.

@Roblinde
Copy link
Collaborator

@georgezubov interesting, could you share all your classes and I'll try to figure out what's going on. (Just need the relevant properties)

@georgezubov
Copy link
Author

@Roblinde , here are redacted relevant classes and interfaces (without non-related properties) -
Test1.zip. Thanks a lot for looking into it!

@Roblinde
Copy link
Collaborator

@georgezubov do you have a contenttype resolver configured as well? To map your IContentModel interface?

@Roblinde
Copy link
Collaborator

When I set the classes up like this (some minor changes from your example) I get everything resolved just fine:

public class Level1ContentModel : ContentModelWithId
{
    // an entry of Level2ContentModel is added here
    public IList<Level2ContentModel> Items { get; set; } = new List<Level2ContentModel>();
}

public class Level2ContentModel : ContentModelWithId
{
    //[JsonProperty("DifferentFieldNameInCF")]
    public IList<Level3ContentModel> Items { get; } = new List<Level3ContentModel>();
}

public sealed class Level3ContentModel : ContentModelWithId, IContent
{
    // Here we're always getting null after deserialization. We expect an instanse of Level4ContentModel
    public Level4ContentModel TargetField { get; set; }
}

public class Level4ContentModel : BaseContentModel, IContent
{
    public string Field1 { get; set; }
}

@georgezubov
Copy link
Author

@Roblinde yes, we have an implementation of IContentTypeResolver which maps specific content types using custom attribute with the content type ID.

@Roblinde
Copy link
Collaborator

@georgezubov ok, I'll set up a simple content type resolver as well and see if I can reproduce the problem.

@Roblinde
Copy link
Collaborator

@georgezubov it works fine with a contenttype resolver as well... hmm, is there anything else that could be pertinent?

My simple resolver just looked like this:

public class LevelsContentTypeResolver : IContentTypeResolver
{
    public Type Resolve(string contentTypeId)
    {
        if(contentTypeId == "<TYPE1>")
        {
            return typeof(Level1ContentModel);
        }
        if (contentTypeId == "<TYPE2>")
        {
            return typeof(Level2ContentModel);
        }
        if (contentTypeId == "TYPE3")
        {
            return typeof(Level3ContentModel);
        }
        if (contentTypeId == "TYPE4")
        {
            return typeof(Level4ContentModel);
        }

        return null;
    }
}

@georgezubov
Copy link
Author

@Roblinde actual serialization error looks like following:

JsonSerializationException: Could not create an instance of type IContentModel. Type is an interface or abstract class and cannot be instantiated. Path 'items[0].items[0].listOfItems[0].targetField.sys', line , position .

If I change a type of target field to specific type (Level4ContentModel), the instance of the class will be created, but all properties inside of it are null (except SystemProperties, which has Id, LinkType and Type fields populated).

@Roblinde
Copy link
Collaborator

Aaah, so it's a serialization issue. Is your Level4ContentModel missing from your contenttype resolver by any chance?

@georgezubov
Copy link
Author

@Roblinde nope, it is being deserialized correctly when it is referenced anywhere else (e.g. in Level1ContentModel). And also everything works as expected in version 7.7.0.

@Roblinde
Copy link
Collaborator

hmm, that is certainly weird. Version 7.7.0 introduced a faulty way of doing serialization that broke when an item was referenced multiple times so we can't go back to that anyway.

It's weird that it's pointing to targetField.sys

In your actual live example, is your Level4 entry present anywhere else in the structure or is it exactly as the test.json you provided me?

@georgezubov
Copy link
Author

georgezubov commented Nov 19, 2024

@Roblinde You're right, I'm sorry for the misleading, we have rolled back to the 7.6.1, not to the 7.7.0

In your actual live example, is your Level4 entry present anywhere else in the structure or is it exactly as the test.json you provided me?

In the very first response where we spotted the problem, this Level4 was referenced by 2 different entries in the structure, but after that I created separate set of entries and Level4 entry was referenced only once, the result is the same. For both cases, the entry itself is present only in includes. I can try to provide the whole redacted response if that would help.

@Roblinde
Copy link
Collaborator

If you could that would be great, I'm running out of ideas, but somehow I must be able to reproduce this and find the bug.

@georgezubov
Copy link
Author

@Roblinde Here is redacted version of the complete response - complete-response.json

@Roblinde
Copy link
Collaborator

Perfect, thank you... I'll try to reproduce!

@Roblinde
Copy link
Collaborator

Even with this response I'm not getting any errors. Models look like this:

 public class Level1ContentModel : ContentModelWithId
 {
     // an entry of Level2ContentModel is added here
     public IList<IContentModel> Items { get; set; } = new List<IContentModel>();
 }

 public class Level2ContentModel : ContentModelWithId
 {
     //[JsonProperty("DifferentFieldNameInCF")]
     public IList<Level3ContentModel> ListOfItems { get; } = new List<Level3ContentModel>();
 }

 public sealed class Level3ContentModel : ContentModelWithId, IContent
 {
     // Here we're always getting null after deserialization. We expect an instanse of Level4ContentModel
     public IContentModel TargetField { get; set; }
 }

 public class Level4ContentModel : BaseContentModel, IContent
 {
     public string Field1 { get; set; }
 }

Any obvious difference from yours?

@georgezubov
Copy link
Author

Models look correct to me. Also, we don't have any specific serialization settings configured for ContentfulClient, except the TypeNameHandling, not sure if that matters:

SerializerSettings = { TypeNameHandling = TypeNameHandling.All }

@Roblinde
Copy link
Collaborator

Hmm if you're overriding the SerializerSettings are you adding these lines as well?

SerializerSettings.Converters.Add(new AssetJsonConverter());
SerializerSettings.Converters.Add(new ContentJsonConverter());

TypeNameHandling.All is also the default for the ContentfulClient out of the box.

@georgezubov
Copy link
Author

@Roblinde no, we don't have these converters in our configuration. I've just added them to the settings but it doesn't affect the issue, removing override of the settings doesn't seem to change anything either.

@Roblinde
Copy link
Collaborator

Hmm, this is really strange... If you have the time could you try creating a new console app, adding your classes to that app and try using the SDK from there. Just a clean slate and perhaps we can pinpoint what exactly is causing the issue.

@Roblinde
Copy link
Collaborator

Hi @georgezubov I've just pushed a beta release to nuget. It includes a complete rework of the reference resolving. I would love for you to test it out and see if it solves your problem.

@georgezubov
Copy link
Author

Hi @Roblinde I've tried new beta release and we're getting NullReferenceException when trying to load our models:
Object reference not set to an instance of an object.
at Contentful.Core.Configuration.ContentfulReferenceResolver.BuildObjectGraph()
at Contentful.Core.ContentfulClient.d__22`1.MoveNext()
at redacted ...

Regrading a new console app to help with reproducing the issue - sure, I will try prepare something soon, just had to switch to the other topics.

@Roblinde
Copy link
Collaborator

@georgezubov thanks for testing. I will try to find out where this null reference can occur and see what causes it. I'll try to get a new version out shortly.

@Roblinde
Copy link
Collaborator

@georgezubov a new beta version is now available for you to try out!

@georgezubov
Copy link
Author

@Roblinde the null reference at BuildObjectGraph is gone, but I'm getting a lot of null reference exceptions here and there after deserialization - some of the fields of the referenced types are null (see example structure), in some cases where we have many references there are some null entries added in the list even though we have only one entry in the list referenced in Contentful

public class Level1ContentModel : BaseContentModel
{
        public Level2ContentModel  ReferencedEntry { get; set; }
}

public class Level2ContentModel : BaseContentModel
{
        public string Field1 { get; set; }
}

public abstract class BaseContentModel : IContentModel
{
        public string InternalId { get; set; } // this field is null in the Level2ContentModel instance

        [JsonProperty("sys")]
        protected internal SystemProperties SystemProperties { get; set; }

        [JsonProperty("$metadata")]
        protected internal ContentfulMetadata Metadata { get; set; }

        [JsonExtensionData]
        protected internal IDictionary<string, object> ExtraProperties { get; set; }
}

@Roblinde
Copy link
Collaborator

Roblinde commented Dec 3, 2024

Great, thanks for testing. I'll keep working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants