IEnumerable<object> Generated for known types

[xantari]

Motivation

When generating a class for a known content type that has embedded content types they get generated with incorrect types of "object" instead of the strongly typed class. Additionally, when a constraint is "Exactly 1", the object still gets generated as an IEnumerable (implying more than 1).

Here is an example:

This got generated as follows:

public IEnumerable<object> NavigationTemplate { get; set; }

It should be expected that this get generated as this:

public NavigationTemplate NavigationTemplate { get; set; }

Proposed solution

Ideally the .NET code generator could differentiate between Exactly 1 and other values, and more importantly be bound to the correct type.

At a minimum it should have been able to figure out that it should have been an IEnumerable<NavigationType> instead of IEnumerable<object>

[petrsvihlik]

Hi @xantari , thank you for reporting the issue! We actually know about it. It has already been reported in #17

The best practice, for now, is: https://github.com/Kentico/kontent-generators-net/blob/master/README.md#handling-content-element-constraints

I'll close the other issue and leave this one open as it's better described.

Blocked: We need the content type API endpoint to return the information about content element constraints

[petrsvihlik]

Temporarily, we could allow specifying the Management API key (optionally) and retrieving the item_count_limit and allowed_content_types from the /{project_id}/types/ endpoint as described at https://docs.kontent.ai/reference/management-api-v2#section/Linked-items-element-in-type.

Logic to apply:

• if item_count_limit == 1
  • and sum(allowed_content_types) > 1 or undefined => object
  • and sum(allowed_content_types) == 1 (e.g. Article) => Article
• if item_count_limit > 1 and sum(allowed_content_types) == 1 (e.g. Article) => IEnumerable<Article>
• all other cases => IEnumerable<object>

Similarly, we could generate more specific types for collections of Assets using the asset_count_limit.

This approach, however, has some drawbacks too - one would need to store the API key securely somewhere. Which means that sharing generation scripts (such as this) with a team becomes more difficult.

[Simply007]

One approach would be #148

My approach might lead to clearer separation of the generated output: I have a slightly different idea to be able to serve sam models for Production and preview API as well.

Still generate the base models as this (or currently object instead of ContentItem):

Article.Generated.cs

TODO:

• verify if IContentItem is an interface/base calss

// multiple items of same type (scenario 1)
public IEnumerable<ContentItem> Topics { get; set; }
// single item on one type (scenario 2)
public IEnumerable<ContentItem> Hero { get; set; }
// single item of multiple types (i.e. Dragon, Zombie) (scenario 3)
public IEnumerable<ContentItem> Monster { get; set; }
// multiple items of multiple types (i.e. BannerSection, HeroSection) (scenario 4)
public IEnumerable<ContentItem> Sections { get; set; }

An of some parameter (i.e. -e) is applied you get

Article.Typed.Generated.cs

public IEnumerable<Topic> TopicsTyped => Topic.OfType<Topic>(); // (scenario 1)
public Hero HeroSingle => Hero.OfType<Hero>().FirstOrDefault();  // (scenario 2)

// not sure whether we want to have this  (scenario 3)
public Dragon MonsterSingleDragon => Monster.OfType<Dragon>().FirstOrDefault();
public Zobie MonsterSingleZombie => Monster.OfType<Dragon>().FirstOrDefault();

// not sure whether this but it is possible  (scenario 4)
// because of you have 20 properties you you'd end up with 20 properties
// And for these you might want to use 
public IEnumerable<Banner>  Sections.OfType<Banner>();
public IEnumerable<Hero>  Sections.OfType<Hero>();

---

TODO for this approach

• define naming (file name, property names)
• there might be still check of not null for multiple types (second two showcases

---

It might be possible to cut out he multi-type strongly typed definition to separate feature and combine it with https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Partial-class-customization-techniques#via-interfaces

So for Moster generate (if we suppose that Zombie and Dragon contain common snippet MosterMetadata) IMonterMetadata

public interface IMetadataMonster // not sure whether use this (1) : IContentItem
{
    // or this  (2)  IContentItemSystemAttributes System {get; set;}
    string MonsterName { get; set; }
}

public partial class Zombie: IMetadataMonster {}

public partial class Dragon: IMetadataMonster {}

and then into Article.Typed.Generated.cs

public IMetadataMonster Monster => Moster.OfType<IMetadataMonster>().FirstOrDefault();

TODO: how about multiple common snippets

[Simply007]

We should mind https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Strong-Types-Explained-%E2%80%93-DataAnnotations-attributes

We should mind these practices: https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Partial-class-customization-techniques

We should mind multiple mapping logic: https://github.com/Kentico/kontent-delivery-sdk-net/wiki/Working-with-strongly-typed-models#naming-the-properties

[Sevitas]

After further discussions we have come up with the following RFP:

• every model will implement IContentItem interface containg system property public IContentItemSystemAttributes System { get; set; }
  • Linked items will be as well of type IEnumerable<IContentItem>
  • Naming? is IContnentItem safe to use? Is there a better name?
  • ⚠ Introducing IContentItem to DAPI .NET SDK is a breaking change, is this OK?

    • There is an alternative - keep IEnumerable<object> in Article.Generated.cs and add IEnumerable<IContentItem> in Article.Typed.Generated.cs

• typed model properties will be separated into their own files
  • eg. type Article will generate files Article.cs, Article.Generated.cs, Article.Typed.Generated.cs
  • You can choose if you want to generate these typed models (*.Typed.Generated.cs) with another flag. For now, it is -e true
  • Naming? Can we use typed in the name of the file?
• typed models will cover up 4 scenarios (for production models)

Scenario 1

Linked items elements are limited to a single type with at least 1 item. Article.Generated.cs will contain

public IEnumerable<IContentItem> Topics { get; set; }

Article.Typed.Generated.cs will contain

public IEnumerable<Topic> TopicsTyped => Topic.OfType<Topic>();

Scenario 2

Linked items element limited to a single type with at most or exactly 1 item. Article.Generated.cs will contain

public IEnumerable<IContentItem> Header { get; set; }

Article.Typed.Generated.cs will contain

public Header HeaderSingle => Header.OfType<Header>().FirstOrDefault();

Scenario 3

Linked items element limited to multiple types with at least 1 at most or exactly 1 item. Article.Generated.cs will contain

public IEnumerable<IContentItem> Attachement { get; set; }

Article.Typed.Generated.cs will contain

public Image AttachementImageSingle => Attachement.OfType<Image>().FirstOrDefault();
public Other AttachementOtherSingle => Attachement.OfType<Other>().FirstOrDefault();

As a developer, you still need to check which property contains a value. Other properties will contain a null.

Scenario 4

Linked items element limited to multiple types with at least 1 item. Article.Generated.cs will contain

public IEnumerable<IContentItem> Sections { get; set; }

Article.Typed.Generated.cs will contain

public IEnumerable<Banner> SectionsBannerTyped => Sections.OfType<Banner>();
public IEnumerable<CompanyNews> SectionsCompanyNewsTyped => Sections.OfType<CompanyNews>();

As a developer, you still need to check which properties contain any items. Some properties might be empty.

Notes

1. DataAnnotations a. annotations should work as expected
2. Customizations a. customization should work as expected b. If we have decided to use class instead of interface IContentItem, customization with a class would not be possible
3. Naming a. we should make sure that name of the interface IContentItem is a reserved keyword
4. You can try to generate models for your own project using the branch using the flag '-e true'.
5. Generating typed models will require a management API key!
6. arguments to genenrate production models: -p <project_id> -e true -m false -t true -k <management_api_key>
7. Generating preview models with arg '-r true' will not generate properly typed models. All linked item elements will end up like properties with the type IEnumerable<IContentItem>.
8. Be aware that it is still a work in progress.

Questions

1. Does the naming of the typed properties make sense?
2. Does these scenarios make sense?

[mcbeev]

I think I understand it pretty well.

1. Yes, the naming to me makes sense.
2. I can't say that I run into that many issues with the current model (of "object"), but overall I think this would be helpful and eliminate extra (somewhat redundant calls) to learn more about linked items. I like the ability to still generate the old way if needed. I would be for this approach.

[Simply007]

I think I understand it pretty well.

1. Yes, the naming to me makes sense.
2. I can't say that I run into that many issues with the current model (of "object"), but overall I think this would be helpful and eliminate extra (somewhat redundant calls) to learn more about linked items. I like the ability to still generate the old way if needed. I would be for this approach.

Hey, @mcbeev thanks! I just have 2 followup questions

1. What do you think about breaking change (IEnumerable<object> -> IEnumerable<IContentItem>) vs. include IEnumerable<IContentItem> only in *.TypedGenerated.cs file?
2. Do you see any of the scenarios redundant/not worthy to implement?

[mattnield]

I'd agree, makes sense. I've not run into this too much as an issue myself, but I can see how it's going to add clarity. I think for IEnumerable<object> vs. IEnumerable<IContentItem> that I'd rather choose between the two that have them both.

[mcbeev]

@Simply007

1. As long as it it is optional or configurable (so the old way still works) I'm ok with it.
2. I could potentially see scenario 3 making scenario 2 not needed.

[frederikstonge]

Scenario 4 won't work for us because we want to keep the order of each items.

I think it is a nice idea to change "object" to "IContentItem" that includes IContentItemSystemAttributes. I had to use the base class options and change the class to an interface.

I don't know if generating the typed items is a good Idea, I prefer to do it manually for my needs.

[Enngage]

Naming conventions are ok, as well as IContentItem interface :)

Scenario 1 is nice to have.

Scenario 2 could be tricky to implement as Delivery SDK doesn't have the prior knowledge of how many items there can be and for the mapping we might need to use reflection on the model to see if there is a property with that given name and if it's enumerable or not.

Scenario 3 & 4 are quite dangerous as if types are omitted or many types are available it could easily generate hundreds of properties in a single content type model. Not sure if we want to potentially create such large type models.

Additionally, maybe we could consider separating all elements in Elements property which would be on the same level as System ? This would be more aligned with what the API actually returns.

[Sevitas]

After further discussions we have decided to implement scenarios 1, 2. The other scenarios will be solved in scenario 3 and scenario 4.

[KevinByrd-28]

Hello everyone. I would just like to push this issue to the forefront again as I am facing this problem as well. It's becoming an increasingly heavy nuisance when I run the model generator and I then have to go back through models to change the "Object" to an explicit content model for intellisense. Seems like the MG should either avoid the use of object tags all together or not overwrite edits made to the content model.

After skimming through the discussion above, Scenario 1 still wont resolve intellisence issues. Is there any plan to allow edited lines to not be overwritten by the MG?

[frederikstonge]

Hello everyone. I would just like to push this issue to the forefront again as I am facing this problem as well. It's becoming an increasingly heavy nuisance when I run the model generator and I then have to go back through models to change the "Object" to an explicit content model for intellisense. Seems like the MG should either avoid the use of object tags all together or not overwrite edits made to the content model.

Actually, you shouldn't modify the .Generated file. You should create new Property in the normal file (which is not overriden) and add properties that access the Generated one with proper "cast".

Exemple :

Item.Generated.cs

public partial class Item
{
    public const string Codename = "item";
    public const string ComponentsCodename = "components";

    public IEnumerable<object> Components { get; set; }
    public IContentItemSystemAttributes System { get; set; }
}

Item.cs

public partial class Item
{
    public IEnumerable<Type> ComponentsValue => Components?.OfType<Type>();
}

[KevinByrd-28]

Hello everyone. I would just like to push this issue to the forefront again as I am facing this problem as well. It's becoming an increasingly heavy nuisance when I run the model generator and I then have to go back through models to change the "Object" to an explicit content model for intellisense. Seems like the MG should either avoid the use of object tags all together or not overwrite edits made to the content model.

Actually, you shouldn't modify the .Generated file. You should create new Property in the normal file (which is not overriden) and add properties that access the Generated one with proper "cast". ....

Thanks for that information, that should be helpful

[Sevitas]

Hi @KevinByrd-28, I can confirm @frederikstonge's answer is correct. The generated files contain documentation comments containing information about the modification of the files.

[Simply007]

Waiting for https://github.com/kontent-ai/management-sdk-net/issues/197.