[MGNLREST-361] Configuration property to control the depth of referenced nodes Created: 21/Apr/22  Updated: 25/Jan/23  Resolved: 11/Jan/23

Status: Closed
Project: Magnolia REST Framework
Component/s: None
Affects Version/s: 2.2.12
Fix Version/s: 3.0.0, 2.2.16

Type: Bug Priority: Neutral
Reporter: Jonathan Ayala Assignee: Oanh Thai Hoang
Resolution: Fixed Votes: 1
Labels: None
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: 2d 1h Time Spent: 2d 1h
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Attachments: PNG File page-link-2-depthinreference-level1-has-child-level1.png     PNG File page-link-depthinreference-level0-no-child.png    
Issue Links:
Relates
causality
dependency
is depended upon by MGNLREST-612 Configuration property to control the... Closed
documentation
to be documented by MGNLREST-604 DOC: New depthInReferencedNode property Closed
relation
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MGNLREST-605 Implement Sub-task Closed Oanh Thai Hoang  
MGNLREST-606 Review Sub-task Completed Dai Ha  
MGNLREST-607 piQA Sub-task Completed Dai Ha  
MGNLREST-608 QA Sub-task Completed Javier Benito  
MGNLREST-610 Pr for master Sub-task Closed Oanh Thai Hoang  
Template:
Acceptance criteria:
Empty
Task DoD:
[X]* Doc/release notes changes? Comment present?
[X]* Downstream builds green?
[X]* Solution information and context easily available?
[X]* Tests
[X]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Release notes required:
Yes
Documentation update required:
Yes
Date of First Response:
Epic Link: Support
Sprint: DevX 28
Story Points: 3
Team: DeveloperX
Work Started:

 Description   

Currently it is possible to configure the referenceDepth in rest endpoints so we can limit the number of references (in depth) that are resolved. However, it is not possible to configure the depth of the referenced nodes as the property depth does in the rest endpoint configuration. The existing implementation returns the referenced nodes up to the configured referenceDepth but it is also returning all their children. This is happening because resolve method is always passing a value of -1: https://git.magnolia-cms.com/projects/MODULES/repos/rest/browse/magnolia-rest-content-delivery/src/main/java/info/magnolia/rest/delivery/jcr/v2/JcrDeliveryEndpoint.java#354

In certain cases, users wants a specific depth for the returned "root" nodes but a different depth for the referenced nodes. Therefore, this is to ask for a configuration property that controls the depth of the referenced nodes instead of using the hardcoded value of -1

In some cases when referenced nodes having big tree behind, the reference resolver resolved a very big tree in response which may cause both performance issue and un-responding to front-end side when multiple users calling the same request.



 Comments   
Comment by Jaroslav Simak [ 21/Dec/22 ]

Explore how we could configure depth for referenced content. It needs to be different from the `depth` parameter which is used for the "root" item.

Comment by Christopher Zimmermann [ 21/Dec/22 ]

I was thinking about a new property on the delivery endpoint, like 'depthInResolvedItems" which developer could use to specify the "depth" to use for a resolved reference. But this could still have the problem that I have multiple reference resolvers, and I would actually really like a different depth in each of them. For example I could have referenceResolver for a tour - which should have depth:2, and I have referenceResolver for a page which should have depth:1.

Could we rather add a new property to the config of the referenceResolver? So each one could define its own depth? I guess then we could use the same name for the property... "depth".

Comment by Viet Nguyen [ 28/Dec/22 ]

I think we can limit the depth in the reference resolver to 1 or maximum 2 levels only. Let's think about the content structure and the actual use case when customer wants to resolve the "referenced node" more than 2 level of depth.

This revealed the fact that in case customers (users) want more than 1 or 2 depth, they should redesign the content structure or use a different approach to get his desired content instead of pull down all the referenced content at that depth.

It would be much better if they (the delivery endpoint users) need the supported depth level of referenced nodes is more than 2. Then he should traverse the target tree (which should be in different workspace), get the desired nodes and match it with the root tree level (in current workspace) using the UUID if that's necessary to him.

Then the configuration point from czimmermann could be rather simple - a boolean to indicate that we resolve the "deep-reference" or not. And that's all. In case he configured a "referenceResolver" then obviously he wants to resolve the referencing nodes, the depth level = 1, in case he set deepReferenceResolver to TRUE, then depth level =2.

WDYT?

Comment by Oanh Thai Hoang [ 29/Dec/22 ]

Hi czimmermann jsimak . Here is example of another config for each resolver after discovery. Look at 

depthInReferencedNode. Default is 0

 

class: info.magnolia.rest.delivery.jcr.v2.JcrDeliveryEndpointDefinition
workspace: website
limit: 100
depth: 2
nodeTypes:
  - mgnl:page
  - mgnl:contentNode
childNodeTypes:
  - mgnl:area
  - mgnl:component
  - mgnl:contentNode
  - mgnl:page
referenceDepth: 2
referenceRepeat: true
references:
  - name: pageLink
    propertyName: pageLink
    referenceResolver:
      $type: jcrReferenceResolver
      class: info.magnolia.rest.reference.jcr.JcrReferenceResolverDefinition
      targetWorkspace: website
      expand: true
      generateLink: true
      depthInReferencedNode: 0
  - name: pageLink2
    propertyName: pageLink2
    referenceResolver:
      $type: jcrReferenceResolver
      class: info.magnolia.rest.reference.jcr.JcrReferenceResolverDefinition
      targetWorkspace: website
      expand: true
      generateLink: true
      depthInReferencedNode: 1 

My example is about page is linking to travel from travel demo

See depthInReferencedNode:0 mean no child for pageLink(travel).

See depthInReferencedNode:1 mean showing child level 1 of pageLink2

 

I don't have much use-case for resolving reference node for depth has 1 or 2 level in reference node. But I agree with viet.nguyen  rest single response should be simple, shouldn't return a huge json, limit the depthInReferencedNode is ** necessary.

Comment by Christopher Zimmermann [ 03/Jan/23 ]

oanh.thai can you see in git when we introduced that -1? Was it always there or did we add it recently?

viet.nguyen  I prefer the property "depthInReferencedNode" to your "deep-reference" concept, it is more flexible, and I think it is clearer for a developer what it does. For "deep-reference" a developer would need to review the documentation to know exactly how far it went. But thank you for your suggestion!

I agree with you viet.nguyen that the usecases for deeper depth in referenced nodes are slim. I guess most people would want a default depth in referenced nodes as 0 or 1. I am tempted to change that current default of -1, to 2. But that would be a breaking change so I am hesitant. On the other hand, we could treat it as a bug. I would consider it a bug because it makes such terrible responses now if you are linking to the website workspace. I have a hard time imagining the use case where a referenced page should return all of its subcontent.

 

Comment by Oanh Thai Hoang [ 04/Jan/23 ]

Hi czimmermann , -1 is introduced since 2018 from https://jira.magnolia-cms.com/browse/MGNLREST-184 related to localized reference resolver node. See commit

Comment by Oanh Thai Hoang [ 10/Jan/23 ]

This ticket is provided the fix for JCR only, regarding norsu endpoint, it should be another ticket: https://jira.magnolia-cms.com/browse/MGNLREST-612

Generated at Mon Feb 12 06:59:10 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.