[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: |
|
||||||||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||||||||
| Sub-Tasks: |
|
||||||||||||||||||||||||||||||
| 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: | |||||||||||||||||||||||||||||||
| 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 |