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

Nested support for Custom Models #128

Open
D-Fox opened this issue Jan 22, 2021 · 3 comments
Open

Nested support for Custom Models #128

D-Fox opened this issue Jan 22, 2021 · 3 comments

Comments

@D-Fox
Copy link

D-Fox commented Jan 22, 2021

Motivation

I want to fetch my model A class using client.getItems(A.class, params). The problem is that conversion for custom types is not working deeper in hierarchy.

@ContentItemMapping("a")
class A {
@ContentItemMapping("b") List<B> bList;
}

@ContentItemMapping("b")
class B {
  @ElementMapping("name") String name;
  @ContentItemMapping("b") List<C> cList;
}
     
@ContentItemMapping("c")
class C {
  @ElementMapping("name") String name;
}

Class A will contain list of B classes, but inside this class List<C> cList will be mapped to empty list (String name inside B class is ok).

If i fetch class B direcly using client.getItems(B.class, params), List<C> cList will be mapped correctly. So it seems it's not working only if im deeper in hierarchy.

Proposed solution

Add support to nested custom models mapping.

Additional context

There is also parameter linkedItemsDepth(int), but it seems that this is only valid for ContentItem object model, not custom models.

@Simply007
Copy link
Contributor

Simply007 commented Jan 28, 2021

Hello @D-Fox.

Thanks for the issue. I will take a look at it and create a test for that to verify if I get the problem right.

There might be a problem when a circular reference is modeled in Kontent (maybe the reason why it is not implemented).

Would it be OK for you to just extend the possibility to specify how many levels should the mapping works? It should prevent an infinite loop for circular reference (I might find another solution without the necessity of the depth parameter using some memorization - we have it in other SDKs - but I haven't seen this codebase for a while).

@D-Fox
Copy link
Author

D-Fox commented Jan 28, 2021

Thanks for the reply. Parameter similar to the linkedItemsDepth will be sufficient.

@Simply007 Simply007 mentioned this issue Feb 2, 2021
7 tasks
@Simply007
Copy link
Contributor

Hello @D-Fox, I have created a test for your situation in #129. Does the test fit the situation you are describing?

I have also drafted an enhancement in the linked pull request (hence the test is passing). But it is a change in the SDK's core and it would require a longer time to put to production to ensure we don't make any breaking changes for current users of the SDKs.

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

No branches or pull requests

2 participants