kontent-ai / model-generator-net

Kontent.ai .NET model generator.
https://www.nuget.org/packages/Kontent.Ai.ModelGenerator
MIT License
17 stars 18 forks source link

Linked item fields dont get created with depth=0 even though their codenames are returned in JSON #149

Closed jrkd closed 2 years ago

jrkd commented 2 years ago

Brief bug description

What went wrong?

Linked item fields on the generated model are empty lists even when their codenames are returned in the JSON.

Repro steps

  1. Create a content model that looks like;

Example Type (codename=example_type)

  1. Create content items like so:

    • Name: Example 1 (related_pages is empty)
    • Name: Example 2 (related_pages is linked to Example 1)
  2. Make an api query with the following params /items?depth=0&system.type=example_type

The api returns the following json;

image

  1. Run the kontent-generators-net tool to generate the ExampleType class
  2. Make the same api query in C# using the DeliveryClient with GetItemsAsync<ExampleType> method
  3. ExampleType.RelatedPages property will be empty.

Expected behavior

Id expect linked content item codenames to be populated, with all other fields being empty/default/null. This is similar to what happens when you use the elements query param to only select some items.

Or you at the very least should be able to access the linked item field codenames in some form without having to access the ApiResponse JSON object directly.

This is important on our project as we are needing to make filters based on what related children are connected, but cannot do so without retrieving every field of the child object. We're currently experiencing performance issues due to this.

We can use the elements= to narrow this somewhat, but we share a lot of naming conventions between content types, so when i say i only want the image url eg depth=1&elements=related_pages,image_url, that will get the child object's image_url as well as the main object, when all we care about is the main object & child id/codenames to do further queries on.

Test environment

jrkd commented 2 years ago

Perhaps this could be solved via another option in the command line --included-linked-codename-fields which as well as the normal

IEnumerable<object> RelatedPages property, would also create a second IEnumerable<string> RelatedPagesCodenames

Simply007 commented 2 years ago

Helo @jrkd,

thanks for the suggestion. Currently, this feature is not a part of the generator.

We had a similar issue being reported in the Delivery SDK for .NET and we decided to implement a PropertyName attribute that allows you to map the linked item element to string: https://github.com/Kentico/kontent-delivery-sdk-net/issues/279

You might be able to combine this attribute with : https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Working-with-strongly-typed-models#naming-the-properties in separate Partial class and then use https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Support-for-custom-types-in-models-via-Value-Converters for mapping the data.

Is that suitable for you?

Simply007 commented 2 years ago

Simple pesudocode:


[AttributeUsage(AttributeTargets.Property)]
public class ZeroDepthLinkedItemsValueConverter :Attribute, IPropertyValueConverter<IEnumerable<string>>
{
    public Task<object> GetPropertyValueAsync<TElement>(PropertyInfo property, TElement element, ResolvingContext context) where TElement : IContentElementValue<IEnumerable<object>>
    {
        // implementation
    }
}

public partial class ExampleType
{

    [PropertyName('related_pages')]
    [ZeroDepthLinkedItemsValueConverter]
    public IEnumerable<string> RelatedPagesCodename {get; set;}
}

I guess we might want to add this sample to docs/tests and link it from both docs sections: https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Working-with-strongly-typed-models#customizing-the-property-matching and https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Working-with-strongly-typed-models#customizing-the-property-matching

Ping: @MiroKentico @winklertomas @Sevitas

winklertomas commented 2 years ago

That is correct, see LinkedItemCodenamesValueConverter unit test and Article model in the deliver NET SDK.

https://github.com/Kentico/kontent-delivery-sdk-net/blob/8f43f7f8a764058acbce14ae829a9a9424775592/Kentico.Kontent.Delivery.Tests/ValueConverterTests.cs#L61

https://github.com/Kentico/kontent-delivery-sdk-net/blob/master/Kentico.Kontent.Delivery.Tests/Models/ContentTypes/Article.cs

The PropertyName attribute was introduced to support scenarios where you would like to map the same JSON property to multiple properties from your model, then you can use the custom value converter and convert it to a codename list.

As for the delivery SDK wiki changes, there already is a mention of the PropertyName attribute, see https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Working-with-strongly-typed-models#naming-the-properties. However, I think that we could add a link directly to this unit test and mention that this is the main use-case why we implemented that. @MiroKentico could you please add it somewhere to the wiki?

Simply007 commented 2 years ago

@jrkd Did you have a chance to try the approach?

There is actual implementation for List<string> of codenames: https://github.com/Kentico/kontent-delivery-sdk-net/blob/master/Kentico.Kontent.Delivery.Tests/ValueConverterTests.cs#L30-L37

Sevitas commented 2 years ago

Hi!

This issue has gone quiet. 👻 It’s been a while since the last update here.

If we missed anything on this issue or if you want to keep it open, please reply here.

Thanks for being a part of the Kontent.ai community!